diff --git a/plugin/appstore/supervisor.go b/plugin/appstore/supervisor.go index dedd160..a71885d 100644 --- a/plugin/appstore/supervisor.go +++ b/plugin/appstore/supervisor.go @@ -13,6 +13,7 @@ import ( "os" "os/exec" "path/filepath" + "reflect" "runtime" "strconv" "strings" @@ -628,7 +629,34 @@ func (s *supervisor) rescanForNew() []*installedApp { for _, a := range apps { if existing, ok := s.installed[a.Manifest.ID]; ok { if existing.Manifest.AppVersion == a.Manifest.AppVersion { - continue // same version, nothing to do + if existing.Manifest.Binary.SHA256 == a.Manifest.Binary.SHA256 { + continue // same version, same binary: nothing to do + } + // Same version, different binary: a rebuilt bundle was + // reinstalled over the running app (`pilotctl appstore upgrade` + // reports "rebuilt (same version, new bundle)"). Keeping the old + // in-memory manifest would verify the new binary against the old + // pin until the app is suspended, so swap it in like an upgrade. + if cancel, cancelOk := s.appCancel[a.Manifest.ID]; cancelOk { + cancel() + delete(s.appCancel, a.Manifest.ID) + } + delete(s.ready, a.Manifest.ID) + delete(s.crashes, a.Manifest.ID) + if err := os.Remove(filepath.Join(a.Dir, suspendedMarkerName)); err != nil && !errors.Is(err, os.ErrNotExist) { + s.logger.Printf("rescan: app id=%s: remove suspended marker: %v", a.Manifest.ID, err) + } + s.logger.Printf("rescan: rebuilt bundle detected: app=%s %s binary %s → %s — restarting", + a.Manifest.ID, a.Manifest.AppVersion, shortSHA(existing.Manifest.Binary.SHA256), shortSHA(a.Manifest.Binary.SHA256)) + s.writeAuditLine(a, auditEvent{ + Event: "rebuild-applied", + Reason: fmt.Sprintf("rescan: %s rebuilt, binary %s → %s", a.Manifest.AppVersion, shortSHA(existing.Manifest.Binary.SHA256), shortSHA(a.Manifest.Binary.SHA256)), + SHA256: a.Manifest.Binary.SHA256, + BinaryAt: a.BinaryPath, + }) + s.installed[a.Manifest.ID] = a + fresh = append(fresh, a) + continue } if compareVersions(a.Manifest.AppVersion, existing.Manifest.AppVersion) < 0 { s.logger.Printf("rescan: downgrade refused: app=%s new=%s old=%s — keeping existing version", @@ -1342,22 +1370,28 @@ func (s *supervisor) awaitReady(ctx context.Context, appID string, timeout time. // ── identity hookup ──────────────────────────────────────────────────── // daemonAddrFromDeps reads the daemon's pilot address out of Deps. -// Uses Go's structural typing so the supervisor doesn't import the real -// coreapi package — any Identity-like value with an Address() string -// method works (which is exactly the coreapi.Identity contract). +// The supervisor doesn't import the real coreapi package, so it looks the +// Address method up by name: coreapi.Identity.Address() returns a +// protocol.Addr struct (a fmt.Stringer), not a string. Asserting +// `Address() string` never matched the real daemon, so every supervised app +// was handed the sentinel below. A plain `Address() string` still works. // // Falls back to a sentinel when no Identity is wired (tests that pass // an empty Deps); the sentinel is intentionally non-routable so a // production misconfiguration fails fast rather than silently using // the wrong address. -type identityAddresser interface { - Address() string -} - func daemonAddrFromDeps(deps Deps) string { if deps.Identity != nil { - if id, ok := deps.Identity.(identityAddresser); ok { - if addr := id.Address(); addr != "" { + m := reflect.ValueOf(deps.Identity).MethodByName("Address") + if m.IsValid() && m.Type().NumIn() == 0 && m.Type().NumOut() == 1 { + var addr string + switch v := m.Call(nil)[0].Interface().(type) { + case string: + addr = v + case fmt.Stringer: + addr = v.String() + } + if addr != "" { return addr } } @@ -1391,3 +1425,11 @@ func resolveUnder(base, rel string) (string, error) { } return joined, nil } + +// shortSHA abbreviates a hex digest for log lines. +func shortSHA(h string) string { + if len(h) > 12 { + return h[:12] + } + return h +} diff --git a/plugin/appstore/zz4_downgrade_test.go b/plugin/appstore/zz4_downgrade_test.go index 20f3155..f37dd84 100644 --- a/plugin/appstore/zz4_downgrade_test.go +++ b/plugin/appstore/zz4_downgrade_test.go @@ -145,6 +145,13 @@ func TestRegisterSameVersionIsIdempotent(t *testing.T) { // signed with a fresh ed25519 keypair so scanInstalled's signature // verification (PILOT-98) accepts it. func writeAppDirWithVersion(t *testing.T, root, id, version string) string { + t.Helper() + return writeAppDirWithVersionSHA(t, root, id, version, "0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef") +} + +// writeAppDirWithVersionSHA is writeAppDirWithVersion with an explicit +// binary.sha256 pin, to model a rebuilt bundle at the same version. +func writeAppDirWithVersionSHA(t *testing.T, root, id, version, binSHA string) string { t.Helper() dir := filepath.Join(root, id) if err := os.MkdirAll(dir, 0o755); err != nil { @@ -162,8 +169,8 @@ func writeAppDirWithVersion(t *testing.T, root, id, version string) string { pubB64 := base64.StdEncoding.EncodeToString(pub) template := fmt.Sprintf( - `{"id":%q,"app_version":%q,"manifest_version":1,"binary":{"runtime":"go","path":"bin/x","sha256":"0123456789abcdef0123456789abcdef0123456789abcdef0123456789abcdef"},"grants":[{"cap":"net.dial","target":"*"}],"store":{"publisher":"ed25519:%s","signature":""}}`, - id, version, pubB64, + `{"id":%q,"app_version":%q,"manifest_version":1,"binary":{"runtime":"go","path":"bin/x","sha256":%q},"grants":[{"cap":"net.dial","target":"*"}],"store":{"publisher":"ed25519:%s","signature":""}}`, + id, version, binSHA, pubB64, ) m, err := manifest.Parse([]byte(template)) if err != nil { @@ -341,3 +348,61 @@ func TestRescanAuditLogsDowngradeRefusal(t *testing.T) { t.Errorf("audit log missing downgrade-refused event:\n%s", string(data)) } } + +// TestRescanAppliesSameVersionRebuild: reinstalling a rebuilt bundle at the +// same version (new binary, new pin) while the daemon runs must swap the new +// manifest in. Before, rescan skipped any same-version manifest, so the +// supervisor verified the new binary against the old pin until it suspended +// the app ("sha256 mismatch ... >=10 consecutive verify failures"). +func TestRescanAppliesSameVersionRebuild(t *testing.T) { + root := t.TempDir() + oldSHA := strings.Repeat("a", 64) + newSHA := strings.Repeat("b", 64) + appDir := writeAppDirWithVersionSHA(t, root, "io.test.app", "1.0.0", oldSHA) + + sup := newSupervisor(Config{ + CataloguePublisher: testCatPub, + InstallRoot: root, + RescanInterval: 20 * 1e6, + }, Deps{}, newQuietLogger(t)) + if fresh := sup.rescanForNew(); len(fresh) != 1 { + t.Fatalf("initial discovery: fresh=%d, want 1", len(fresh)) + } + // Unchanged on disk: nothing to do. + if fresh := sup.rescanForNew(); len(fresh) != 0 { + t.Fatalf("unchanged manifest re-registered: fresh=%d", len(fresh)) + } + canceled := false + sup.mu.Lock() + sup.appCancel["io.test.app"] = func() { canceled = true } + sup.crashes["io.test.app"] = &crashRecord{} + sup.mu.Unlock() + if err := os.WriteFile(filepath.Join(appDir, suspendedMarkerName), nil, 0o600); err != nil { + t.Fatal(err) + } + + writeAppDirWithVersionSHA(t, root, "io.test.app", "1.0.0", newSHA) + if fresh := sup.rescanForNew(); len(fresh) != 1 { + t.Fatalf("rebuild not applied: fresh=%d, want 1", len(fresh)) + } + sup.mu.RLock() + got := sup.installed["io.test.app"].Manifest.Binary.SHA256 + _, crashKept := sup.crashes["io.test.app"] + sup.mu.RUnlock() + if got != newSHA { + t.Errorf("in-memory pin = %s, want the rebuilt binary's %s", got, newSHA) + } + if !canceled { + t.Error("the old supervise goroutine was not canceled") + } + if crashKept { + t.Error("crash record not cleared for the rebuilt app") + } + if _, err := os.Stat(filepath.Join(appDir, suspendedMarkerName)); !os.IsNotExist(err) { + t.Errorf(".suspended marker should be cleared, stat err=%v", err) + } + data, _ := os.ReadFile(filepath.Join(appDir, supervisorLogName)) + if !strings.Contains(string(data), "rebuild-applied") { + t.Errorf("audit log missing rebuild-applied:\n%s", data) + } +} diff --git a/plugin/appstore/zz_service_call_test.go b/plugin/appstore/zz_service_call_test.go index 91355e7..c4291d2 100644 --- a/plugin/appstore/zz_service_call_test.go +++ b/plugin/appstore/zz_service_call_test.go @@ -3,6 +3,7 @@ package appstore import ( "context" "errors" + "fmt" "testing" ) @@ -15,7 +16,7 @@ func TestService_Call_NotStarted(t *testing.T) { } } -// fakeIdentityAddr satisfies the identityAddresser interface in supervisor.go. +// fakeIdentityAddr has an Address() string method. type fakeIdentityAddr struct{ addr string } func (f *fakeIdentityAddr) Address() string { return f.addr } @@ -37,8 +38,7 @@ func TestDaemonAddrFromDeps_EmptyAddressFallsBackToSentinel(t *testing.T) { } } -// fakeIdentityNoAddr satisfies coreapi.Identity but NOT the identityAddresser -// interface (no Address method) — exercises the type-assertion miss branch. +// fakeIdentityNoAddr has no Address method at all. type fakeIdentityNoAddr struct{} func (fakeIdentityNoAddr) NodeID() uint32 { return 1 } @@ -51,6 +51,47 @@ func TestDaemonAddrFromDeps_IdentityWithoutAddressFallsBack(t *testing.T) { } } +// fakeProtoAddr mirrors common/protocol.Addr: a struct whose String() renders +// the address. coreapi.Identity.Address() returns this, not a string. +type fakeProtoAddr struct { + Network uint16 + Node uint32 +} + +func (a fakeProtoAddr) String() string { + return fmt.Sprintf("%d:%04X.%04X.%04X", a.Network, a.Network, (a.Node>>16)&0xFFFF, a.Node&0xFFFF) +} + +// fakeCoreIdentity has coreapi.Identity's real Address signature. +type fakeCoreIdentity struct{ addr fakeProtoAddr } + +func (f fakeCoreIdentity) Address() fakeProtoAddr { return f.addr } +func (f fakeCoreIdentity) NodeID() uint32 { return f.addr.Node } + +// The real daemon hands the supervisor a coreapi.Identity; its address must +// reach the app, not the sentinel. Before the fix every wallet on every node +// was started with --addr 0:0001.0000.0000. +func TestDaemonAddrFromDeps_CoreapiIdentityAddress(t *testing.T) { + t.Parallel() + got := daemonAddrFromDeps(Deps{Identity: fakeCoreIdentity{addr: fakeProtoAddr{Node: 0x3D971}}}) + if got != "0:0000.0003.D971" { + t.Errorf("got %q, want 0:0000.0003.D971", got) + } +} + +// Address methods that take arguments or return something unprintable are +// not addresses; they fall back to the sentinel rather than panicking. +type fakeIdentityOddAddr struct{} + +func (fakeIdentityOddAddr) Address(int) string { return "x" } + +func TestDaemonAddrFromDeps_UnusableAddressMethodFallsBack(t *testing.T) { + t.Parallel() + if got := daemonAddrFromDeps(Deps{Identity: fakeIdentityOddAddr{}}); got != "0:0001.0000.0000" { + t.Errorf("got %q, want sentinel", got) + } +} + // TestSupervisor_Call_NilArgsAndOut covers the args/out nil-passthrough path. func TestSupervisor_Call_NilArgsAndOut(t *testing.T) { t.Parallel()