From a20c41200341c8cb06e624b6728963cb7ac535d7 Mon Sep 17 00:00:00 2001 From: Alexgodoroja Date: Fri, 25 Sep 2026 15:30:03 +0200 Subject: [PATCH 1/2] supervisor: pass apps the daemon's real pilot address daemonAddrFromDeps asserted Identity to an Address() string method, but coreapi.Identity.Address() returns protocol.Addr. The assertion never matched the real daemon, so every supervised app, including every wallet on the network, started with the sentinel --addr 0:0001.0000.0000. The method is now looked up by name and its result used when it is a string or a fmt.Stringer; anything else still falls back to the sentinel. Co-Authored-By: Claude Opus 5.5 (1M context) --- plugin/appstore/supervisor.go | 25 ++++++++----- plugin/appstore/zz_service_call_test.go | 47 +++++++++++++++++++++++-- 2 files changed, 60 insertions(+), 12 deletions(-) diff --git a/plugin/appstore/supervisor.go b/plugin/appstore/supervisor.go index e5c6733..b8b829a 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" @@ -1328,22 +1329,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 } } 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() From 1f453d2bae2594922a8382a0d6e9250dfbbb750d Mon Sep 17 00:00:00 2001 From: Alexgodoroja Date: Fri, 25 Sep 2026 18:46:59 +0200 Subject: [PATCH 2/2] supervisor: pick up a rebuilt bundle reinstalled at the same version rescanForNew skipped any on-disk manifest whose app_version matched the running one. When a rebuilt bundle is reinstalled at the same version (pilotctl appstore upgrade: "rebuilt (same version, new bundle)"), the new binary and manifest land on disk but the supervisor keeps verifying against the old binary pin, fails ten times and suspends the app; appstore restart reuses the same stale manifest, so only a daemon restart recovered it. A same-version manifest with a different binary.sha256 is now swapped in like an upgrade: the old supervise goroutine is canceled, the crash record and .suspended marker are cleared, and a rebuild-applied audit event is written. Co-Authored-By: Claude Opus 5.5 (1M context) --- plugin/appstore/supervisor.go | 37 +++++++++++++- plugin/appstore/zz4_downgrade_test.go | 69 ++++++++++++++++++++++++++- 2 files changed, 103 insertions(+), 3 deletions(-) diff --git a/plugin/appstore/supervisor.go b/plugin/appstore/supervisor.go index b8b829a..2a64b20 100644 --- a/plugin/appstore/supervisor.go +++ b/plugin/appstore/supervisor.go @@ -629,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", @@ -1384,3 +1411,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) + } +}