From a30575126580e71713b0a8ea448d7c16a890fb13 Mon Sep 17 00:00:00 2001 From: Andrea Arturo Venti Fuentes <117413846+av1155@users.noreply.github.com> Date: Fri, 2 Oct 2026 00:57:54 -0400 Subject: [PATCH 01/16] fix(installer): accept a renumbered volume device Stored directory identities are st_dev:st_ino. macOS assigns a volume's device number at mount time, so after a reboot the same native generation directory can read 16777233:400099505 where the ledger recorded 16777234:400099505. Every Commit, snapshot read, upgrade, purge and rollback then refused with "native directory inode changed" and the update could never complete. Compare recorded identities through PersistedIdentityMatches: an exact match as before, or the same inode on the same device as its freshly opened parent, which still refuses a mount point substituted at the path. The native directory check, the rename source check and the setup state check use it. Commit records the current identities in the next ledger, so the ledger converges and older writers sharing the control root accept it again. Windows identities use the volume serial and stay exact. Refs #301 --- .../agentnotify/setup/state_identity_test.go | 48 ++++ internal/agentnotify/setup/state_unix.go | 9 +- internal/installruntime/native_path_unix.go | 6 +- internal/installruntime/native_tree.go | 62 ++++- internal/installruntime/path_other.go | 4 + internal/installruntime/path_unix.go | 26 ++ internal/installruntime/path_windows.go | 5 + .../persisted_identity_unix_test.go | 64 +++++ .../installruntime/renumbered_volume_test.go | 238 ++++++++++++++++++ internal/installruntime/transaction.go | 9 +- 10 files changed, 460 insertions(+), 11 deletions(-) create mode 100644 internal/agentnotify/setup/state_identity_test.go create mode 100644 internal/installruntime/persisted_identity_unix_test.go create mode 100644 internal/installruntime/renumbered_volume_test.go diff --git a/internal/agentnotify/setup/state_identity_test.go b/internal/agentnotify/setup/state_identity_test.go new file mode 100644 index 000000000..3604f0a59 --- /dev/null +++ b/internal/agentnotify/setup/state_identity_test.go @@ -0,0 +1,48 @@ +//go:build linux || darwin + +package setup + +import ( + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "testing" +) + +// A reboot can give the volume a new device number while every inode stays, +// so the durable setupState identity must still match its own directory. +func TestOwnedDirectoryIdentitySurvivesRenumberedVolume(t *testing.T) { + dir := filepath.Join(t.TempDir(), "state") + if err := os.Mkdir(dir, 0700); err != nil { + t.Fatal(err) + } + f, err := os.Open(dir) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + id, err := directoryID(f) + if err != nil { + t.Fatal(err) + } + dev, ino, _ := strings.Cut(id, ":") + devNumber, err := strconv.ParseUint(dev, 10, 64) + if err != nil { + t.Fatal(err) + } + inode, err := strconv.ParseUint(ino, 10, 64) + if err != nil { + t.Fatal(err) + } + if err := matches(f, id); err != nil { + t.Fatalf("exact identity refused: %v", err) + } + if err := matches(f, fmt.Sprintf("%d:%d", devNumber+1, inode)); err != nil { + t.Fatalf("identity recorded before the volume was renumbered refused: %v", err) + } + if err := matches(f, fmt.Sprintf("%d:%d", devNumber, inode+1)); err == nil { + t.Fatal("different directory accepted") + } +} diff --git a/internal/agentnotify/setup/state_unix.go b/internal/agentnotify/setup/state_unix.go index f89fb1d0b..5f96ab1df 100644 --- a/internal/agentnotify/setup/state_unix.go +++ b/internal/agentnotify/setup/state_unix.go @@ -295,7 +295,14 @@ func matches(f *os.File, want string) error { if e != nil { return e } - if id != want { + if id == want { + return nil + } + var parent unix.Stat_t + if e = unix.Fstatat(int(f.Fd()), "..", &parent, unix.AT_SYMLINK_NOFOLLOW); e != nil { + return e + } + if !installruntime.PersistedIdentityMatches(want, id, fmt.Sprintf("%d:%d", parent.Dev, parent.Ino)) { return fmt.Errorf("owned directory identity changed") } return nil diff --git a/internal/installruntime/native_path_unix.go b/internal/installruntime/native_path_unix.go index a50f09086..9d3d44e19 100644 --- a/internal/installruntime/native_path_unix.go +++ b/internal/installruntime/native_path_unix.go @@ -26,7 +26,11 @@ func renameNative(from, to string, expected []PathAnchor, sourceID ...string) er if err := unix.Fstatat(int(parent.Fd()), filepath.Base(from), &st, unix.AT_SYMLINK_NOFOLLOW); err != nil { return err } - if st.Mode&unix.S_IFMT != unix.S_IFDIR || fmt.Sprintf("%d:%d", st.Dev, st.Ino) != sourceID[0] { + parentID := "" + if len(anchors) != 0 { + parentID = anchors[len(anchors)-1].Identity + } + if st.Mode&unix.S_IFMT != unix.S_IFDIR || !PersistedIdentityMatches(sourceID[0], fmt.Sprintf("%d:%d", st.Dev, st.Ino), parentID) { return fmt.Errorf("native rename source inode changed") } } diff --git a/internal/installruntime/native_tree.go b/internal/installruntime/native_tree.go index a5003170d..852837c54 100644 --- a/internal/installruntime/native_tree.go +++ b/internal/installruntime/native_tree.go @@ -204,38 +204,84 @@ func removeFailedNativeStage(path string, expected []PathAnchor, id string) erro } func nativeDirectoryID(path string) (string, error) { - var err error + id, _, err := nativeDirectoryIdentity(path) + return id, err +} + +// nativeDirectoryIdentity also returns the parent's identity from the same +// anchored walk, which PersistedIdentityMatches needs to rule out a mount point. +func nativeDirectoryIdentity(path string) (id, parent string, err error) { path, err = platformAnchorPath(path) if err != nil { - return "", err + return "", "", err } if path == "" { - return "", nil + return "", "", nil } anchors, err := pathAnchors(filepath.Join(path, ".identity"), false) if err != nil { - return "", err + return "", "", err } if len(anchors) == 0 || anchors[len(anchors)-1].Path != path { - return "", nil + return "", "", nil + } + if len(anchors) > 1 { + parent = anchors[len(anchors)-2].Identity } - return anchors[len(anchors)-1].Identity, nil + return anchors[len(anchors)-1].Identity, parent, nil } func checkNativeDirectoryID(path, expected string) error { // Legacy records have byte identities only; new promotions always bind inodes. if expected == "" { return nil } - got, err := nativeDirectoryID(path) + got, parent, err := nativeDirectoryIdentity(path) if err != nil { return err } - if got != expected { + if !PersistedIdentityMatches(expected, got, parent) { return fmt.Errorf("native directory inode changed: %s", path) } return nil } +// refreshNativeIdentities records the current identity of every generation that +// still matches its stored one, so a ledger written before the volume was +// renumbered converges and older writers sharing it keep accepting it. +func refreshNativeIdentities(record *NativeRecord) error { + if record == nil { + return nil + } + refresh := func(path string, id *string) error { + if path == "" || *id == "" { + return nil + } + fresh, parent, err := nativeDirectoryIdentity(path) + if err != nil { + return err + } + if fresh != "" && fresh != *id && PersistedIdentityMatches(*id, fresh, parent) { + *id = fresh + } + return nil + } + if err := refresh(record.Path, &record.DirectoryID); err != nil { + return err + } + if err := refresh(record.PreviousPath, &record.PreviousDirectoryID); err != nil { + return err + } + // Retirement copies the live record, so the slice may still back the + // pre-commit ledger that crash recovery compares against. + record.Published = append([]NativeGeneration(nil), record.Published...) + for i := range record.Published { + if err := refresh(record.Published[i].Path, &record.Published[i].DirectoryID); err != nil { + return err + } + } + return nil +} + func checkOpenedNativeRoot(root *os.Root, expected []PathAnchor) error { if len(expected) == 0 { return fmt.Errorf("missing native directory anchor") diff --git a/internal/installruntime/path_other.go b/internal/installruntime/path_other.go index 01ed1e6fa..7138769aa 100644 --- a/internal/installruntime/path_other.go +++ b/internal/installruntime/path_other.go @@ -20,6 +20,10 @@ func safeRemoveDirectory(path, want string, anchors []PathAnchor) error { return fmt.Errorf("confined purge unsupported on this platform") } +func PersistedIdentityMatches(stored, fresh, freshParent string) bool { + return stored == fresh +} + func openedDirectoryIdentity(f *os.File) (string, error) { return "", fmt.Errorf("directory identity unsupported on this platform") } diff --git a/internal/installruntime/path_unix.go b/internal/installruntime/path_unix.go index 14959a718..61b7574c9 100644 --- a/internal/installruntime/path_unix.go +++ b/internal/installruntime/path_unix.go @@ -215,6 +215,32 @@ func safeRemoveDirectory(path, want string, anchors []PathAnchor) error { return parent.Sync() } +// PersistedIdentityMatches compares a device:inode identity recorded by an +// earlier process with a fresh one. A volume's device number is assigned at +// mount time and can change across reboots, so a different device is accepted +// only for the same inode on the same device as the freshly opened parent, +// which still refuses a mount point substituted at that path. +func PersistedIdentityMatches(stored, fresh, freshParent string) bool { + if stored == fresh { + return true + } + storedDev, storedIno, ok := splitDeviceIdentity(stored) + if !ok { + return false + } + freshDev, freshIno, ok := splitDeviceIdentity(fresh) + if !ok { + return false + } + parentDev, _, ok := splitDeviceIdentity(freshParent) + return ok && storedIno == freshIno && freshDev == parentDev && storedDev != freshDev +} + +func splitDeviceIdentity(id string) (dev, ino string, ok bool) { + dev, ino, ok = strings.Cut(id, ":") + return dev, ino, ok && dev != "" && ino != "" && !strings.Contains(ino, ":") +} + func openedDirectoryIdentity(f *os.File) (string, error) { var st unix.Stat_t if err := unix.Fstat(int(f.Fd()), &st); err != nil { diff --git a/internal/installruntime/path_windows.go b/internal/installruntime/path_windows.go index c5745e207..a65dee767 100644 --- a/internal/installruntime/path_windows.go +++ b/internal/installruntime/path_windows.go @@ -281,6 +281,11 @@ func safeRemoveDirectory(path, want string, anchors []PathAnchor) error { return windowsDeleteHandle(child) } +// Volume serial numbers persist across reboots, so recorded identities stay exact. +func PersistedIdentityMatches(stored, fresh, freshParent string) bool { + return stored == fresh +} + func openedDirectoryIdentity(f *os.File) (string, error) { var info windows.ByHandleFileInformation if err := windows.GetFileInformationByHandle(windows.Handle(f.Fd()), &info); err != nil { diff --git a/internal/installruntime/persisted_identity_unix_test.go b/internal/installruntime/persisted_identity_unix_test.go new file mode 100644 index 000000000..e4db4af97 --- /dev/null +++ b/internal/installruntime/persisted_identity_unix_test.go @@ -0,0 +1,64 @@ +//go:build linux || darwin + +package installruntime + +import ( + "os" + "path/filepath" + "testing" +) + +func TestPersistedIdentityMatches(t *testing.T) { + for _, tc := range []struct { + name string + stored, fresh, parent string + want bool + }{ + {"exact", "5:10", "5:10", "5:2", true}, + {"exact at a mount point", "5:10", "5:10", "7:2", true}, + {"renumbered volume", "6:10", "5:10", "5:2", true}, + {"different inode", "6:11", "5:10", "5:2", false}, + {"same device, different inode", "5:11", "5:10", "5:2", false}, + {"mount point substituted", "6:10", "5:10", "7:2", false}, + {"missing directory", "6:10", "", "5:2", false}, + {"no parent", "6:10", "5:10", "", false}, + {"empty stored", "", "5:10", "5:2", false}, + {"windows-shaped stored", "6:0:10", "5:10", "5:2", false}, + {"malformed stored", "10", "5:10", "5:2", false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := PersistedIdentityMatches(tc.stored, tc.fresh, tc.parent); got != tc.want { + t.Fatalf("PersistedIdentityMatches(%q, %q, %q) = %v", tc.stored, tc.fresh, tc.parent, got) + } + }) + } +} + +func TestRefreshNativeIdentitiesLeavesSharedRecordIntact(t *testing.T) { + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + var gens []NativeGeneration + for _, name := range []string{"generation-a.app", "generation-b.app"} { + path := filepath.Join(root, name) + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + id, err := nativeDirectoryID(path) + if err != nil { + t.Fatal(err) + } + gens = append(gens, NativeGeneration{Path: path, DirectoryID: shiftDevice(t, id)}) + } + live := NativeRecord{Path: gens[1].Path, DirectoryID: gens[1].DirectoryID, PreviousPath: gens[0].Path, PreviousDirectoryID: gens[0].DirectoryID, Published: gens} + stale := gens[0].DirectoryID + retired := live + if err := refreshNativeIdentities(&retired); err != nil { + t.Fatal(err) + } + if live.Published[0].DirectoryID != stale { + t.Fatal("refresh rewrote the record it was copied from") + } + requireFreshNativeIdentities(t, &retired) +} diff --git a/internal/installruntime/renumbered_volume_test.go b/internal/installruntime/renumbered_volume_test.go new file mode 100644 index 000000000..9a7dec9a4 --- /dev/null +++ b/internal/installruntime/renumbered_volume_test.go @@ -0,0 +1,238 @@ +//go:build linux || darwin + +package installruntime + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "testing" +) + +// shiftDevice keeps the inode and changes only the device number, which is +// what a reboot that renumbers the volume does to a recorded identity. +func shiftDevice(t *testing.T, id string) string { + t.Helper() + dev, ino, ok := strings.Cut(id, ":") + n, err := strconv.ParseUint(dev, 10, 64) + if !ok || err != nil { + t.Fatalf("unexpected identity %q", id) + } + return fmt.Sprintf("%d:%s", n+1, ino) +} + +// renumberNativeDevices rewrites the ledger as if it had been written before +// a reboot assigned the volume a different device number. +func renumberNativeDevices(t *testing.T, control string) Ledger { + t.Helper() + l, err := readLedger(control) + if err != nil || l.Native == nil { + t.Fatalf("ledger without native record: %v", err) + } + l.Native.DirectoryID = shiftDevice(t, l.Native.DirectoryID) + if l.Native.PreviousDirectoryID != "" { + l.Native.PreviousDirectoryID = shiftDevice(t, l.Native.PreviousDirectoryID) + } + for i := range l.Native.Published { + l.Native.Published[i].DirectoryID = shiftDevice(t, l.Native.Published[i].DirectoryID) + } + if err := writeJSON(filepath.Join(control, "ownership.json"), l); err != nil { + t.Fatal(err) + } + return l +} + +func requireFreshNativeIdentities(t *testing.T, record *NativeRecord) { + t.Helper() + check := func(path, id string) { + if path == "" { + return + } + fresh, err := nativeDirectoryID(path) + if err != nil { + t.Fatal(err) + } + if fresh != "" && fresh != id { + t.Fatalf("stale identity kept for %s: %s, now %s", filepath.Base(path), id, fresh) + } + } + check(record.Path, record.DirectoryID) + check(record.PreviousPath, record.PreviousDirectoryID) + for _, gen := range record.Published { + check(gen.Path, gen.DirectoryID) + } +} + +func installTwoNativeGenerations(t *testing.T) (context.Context, Request, *NativeChange, *NativeChange) { + t.Helper() + ctx, r := request(t) + var changes []*NativeChange + for i := 0; i < 2; i++ { + change, err := StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + r.Native = change + if _, err := Commit(ctx, r); err != nil { + t.Fatal(err) + } + changes = append(changes, change) + } + r.Native = nil + return ctx, r, changes[0], changes[1] +} + +func TestRenumberedVolumeKeepsNativeManaged(t *testing.T) { + t.Run("snapshot", func(t *testing.T) { + _, r, _, _ := installTwoNativeGenerations(t) + renumberNativeDevices(t, r.ControlRoot) + if _, err := ReadInstalledSnapshot(r.ControlRoot); err != nil { + t.Fatalf("renumbered volume broke the snapshot reader: %v", err) + } + }) + t.Run("commit rebinds", func(t *testing.T) { + ctx, r, _, _ := installTwoNativeGenerations(t) + renumberNativeDevices(t, r.ControlRoot) + next, err := Commit(ctx, r) + if err != nil { + t.Fatalf("renumbered volume blocked a commit: %v", err) + } + requireFreshNativeIdentities(t, next.Native) + stored, err := readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } + requireFreshNativeIdentities(t, stored.Native) + }) + t.Run("upgrade to new bytes", func(t *testing.T) { + ctx, r, _, second := installTwoNativeGenerations(t) + renumberNativeDevices(t, r.ControlRoot) + change, err := StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + r.Native = change + next, err := Commit(ctx, r) + if err != nil { + t.Fatalf("renumbered volume blocked an upgrade: %v", err) + } + if next.Native.Path != change.After.Path || next.Native.PreviousPath != second.After.Path || len(next.Native.Published) != 3 { + t.Fatalf("unexpected upgrade record: %+v", next.Native) + } + requireFreshNativeIdentities(t, next.Native) + }) + t.Run("reinstall of an older generation", func(t *testing.T) { + ctx, r, first, _ := installTwoNativeGenerations(t) + renumberNativeDevices(t, r.ControlRoot) + change, err := StageNative(ctx, r.ControlRoot, first.After.Path) + if err != nil { + t.Fatal(err) + } + r.Native = change + next, err := Commit(ctx, r) + if err != nil { + t.Fatalf("renumbered volume blocked reselecting a generation: %v", err) + } + if next.Native.Path != first.After.Path { + t.Fatalf("older bytes did not reselect their generation: %+v", next.Native) + } + requireFreshNativeIdentities(t, next.Native) + }) + t.Run("final uninstall purge", func(t *testing.T) { + ctx, r, _, second := installTwoNativeGenerations(t) + renumberNativeDevices(t, r.ControlRoot) + r.RemoveConsumer, r.PurgeNative = true, true + purged, err := Commit(ctx, r) + if err != nil { + t.Fatalf("renumbered volume blocked the purge: %v", err) + } + if purged.Native != nil { + t.Fatal("purge kept the native record") + } + if _, err := os.Stat(second.After.Path); !os.IsNotExist(err) { + t.Fatal("purge kept the active generation") + } + }) + t.Run("recovery after interruption", func(t *testing.T) { + ctx, r, _, _ := installTwoNativeGenerations(t) + before := renumberNativeDevices(t, r.ControlRoot) + r.Fault = func(phase string) error { + if phase == "transaction" { + return fmt.Errorf("simulated interruption") + } + return nil + } + if _, err := Commit(ctx, r); err == nil { + t.Fatal("interruption did not stop the commit") + } + if _, err := os.Lstat(filepath.Join(r.ControlRoot, "transaction.json")); err != nil { + t.Fatalf("interrupted commit left no journal: %v", err) + } + if stored, err := readLedger(r.ControlRoot); err != nil || stored.Native.DirectoryID != before.Native.DirectoryID { + t.Fatalf("ledger changed before recovery: %v", err) + } + recovered, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) + if err != nil { + t.Fatalf("renumbered volume blocked recovery: %v", err) + } + requireFreshNativeIdentities(t, recovered.Native) + }) +} + +func TestReplacedNativeDirectoryStillRefused(t *testing.T) { + for _, renumbered := range []bool{false, true} { + t.Run(fmt.Sprint("renumbered=", renumbered), func(t *testing.T) { + ctx, r, _, second := installTwoNativeGenerations(t) + if renumbered { + renumberNativeDevices(t, r.ControlRoot) + } + live := second.After.Path + if err := os.Rename(live, live+".moved"); err != nil { + t.Fatal(err) + } + if err := os.CopyFS(live, os.DirFS(live+".moved")); err != nil { + t.Fatal(err) + } + if _, err := Commit(ctx, r); err == nil || !strings.Contains(err.Error(), "native directory inode changed") { + t.Fatalf("replaced native directory accepted: %v", err) + } + if _, err := ReadInstalledSnapshot(r.ControlRoot); err == nil { + t.Fatal("snapshot reader accepted a replaced native directory") + } + }) + } +} + +func TestRenameNativeAcceptsRenumberedSource(t *testing.T) { + skipUnsupportedNative(t) + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + from, to := filepath.Join(root, "from.app"), filepath.Join(root, "to.app") + if err := os.Mkdir(from, 0700); err != nil { + t.Fatal(err) + } + id, err := nativeDirectoryID(from) + if err != nil || id == "" { + t.Fatalf("source identity: %q %v", id, err) + } + anchors, err := pathAnchors(to, false) + if err != nil { + t.Fatal(err) + } + dev, ino, _ := strings.Cut(id, ":") + otherInode, _ := strconv.ParseUint(ino, 10, 64) + if err := renameNative(from, to, anchors, fmt.Sprintf("%s:%d", dev, otherInode+1)); err == nil { + t.Fatal("rename accepted a source with a different inode") + } + if err := renameNative(from, to, anchors, shiftDevice(t, id)); err != nil { + t.Fatalf("rename refused a source recorded before the volume was renumbered: %v", err) + } + if _, err := os.Stat(to); err != nil { + t.Fatal(err) + } +} diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index 00d146915..2ece3a242 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -415,7 +415,8 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { return l, err } - if l.Native != nil && !policyDisableOnly(r) && !r.RevokeOpenCode && !r.RevokeGemini { + nativeChecked := l.Native != nil && !policyDisableOnly(r) && !r.RevokeOpenCode && !r.RevokeGemini + if nativeChecked { if err := validateNativeRecord(l.Native); err != nil { return l, err } @@ -688,6 +689,12 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { if err := validateNative(native); err != nil { return l, err } + // Only next changes: tx.Before must stay equal to the ledger on disk. + if nativeChecked { + if err := refreshNativeIdentities(next.Native); err != nil { + return l, err + } + } files := append([]File(nil), r.Files...) if r.PolicyEnabled != nil || len(r.PolicyFields) != 0 || (r.RemoveConsumer && len(next.Consumers) == 0) { f, e := policyFile(policyRoot, next.Enabled, policyFields, policyBefore) From 5a020bf027c7df097c3e852a4e24762fafa6f2ba Mon Sep 17 00:00:00 2001 From: Andrea Arturo Venti Fuentes <117413846+av1155@users.noreply.github.com> Date: Fri, 2 Oct 2026 01:03:21 -0400 Subject: [PATCH 02/16] fix(installer): recover journals across renumbering A transaction interrupted before a reboot keeps the parent anchors and purge entries it recorded, each with the old device number. Recovery compares them exactly, so after the volume is renumbered it refused with "managed destination parent substituted" and the pending journal blocked every later install. When a pending journal is loaded, derive a device mapping from its recorded parent chains. Each chain must name the same paths and inodes as now, and every recorded device must map to exactly one current device and back. Only then are the journal's anchors and purge entries re-keyed in memory. A chain that now crosses a different mount, or any changed inode, leaves the journal as recorded for the existing checks to refuse. Refs #301 --- .../installruntime/device_renumbering_test.go | 67 +++++++ internal/installruntime/path.go | 109 ++++++++++- internal/installruntime/path_unix.go | 5 - .../installruntime/renumbered_journal_test.go | 183 ++++++++++++++++++ internal/installruntime/transaction.go | 3 + 5 files changed, 361 insertions(+), 6 deletions(-) create mode 100644 internal/installruntime/device_renumbering_test.go create mode 100644 internal/installruntime/renumbered_journal_test.go diff --git a/internal/installruntime/device_renumbering_test.go b/internal/installruntime/device_renumbering_test.go new file mode 100644 index 000000000..22ca68511 --- /dev/null +++ b/internal/installruntime/device_renumbering_test.go @@ -0,0 +1,67 @@ +package installruntime + +import "testing" + +func chain(ids ...string) []PathAnchor { + paths := []string{"/a", "/a/b", "/a/b/c"} + out := make([]PathAnchor, len(ids)) + for i, id := range ids { + out[i] = PathAnchor{Path: paths[i], Identity: id} + } + return out +} + +func TestDeviceRenumbering(t *testing.T) { + for _, tc := range []struct { + name string + stored, fresh []PathAnchor + ok, renumber bool + }{ + {"unchanged", chain("5:2", "5:3"), chain("5:2", "5:3"), true, false}, + {"one volume renumbered", chain("6:2", "6:3"), chain("5:2", "5:3"), true, true}, + {"two volumes renumbered", chain("6:2", "9:3"), chain("5:2", "8:3"), true, true}, + {"recorded chain is a prefix", chain("6:2"), chain("5:2", "5:3"), true, true}, + {"one old volume now on two devices", chain("6:2", "6:3"), chain("5:2", "7:3"), false, false}, + {"two old volumes now on one device", chain("6:2", "9:3"), chain("5:2", "5:3"), false, false}, + {"different inode", chain("6:2", "6:4"), chain("5:2", "5:3"), false, false}, + {"recorded chain longer", chain("6:2", "6:3", "6:4"), chain("5:2", "5:3"), false, false}, + {"windows identities", chain("1:0:2"), chain("1:0:2"), false, false}, + } { + t.Run(tc.name, func(t *testing.T) { + m := newDeviceRenumbering() + if got := m.add(tc.stored, tc.fresh); got != tc.ok { + t.Fatalf("add = %v, want %v", got, tc.ok) + } + if tc.ok && m.renumbered() != tc.renumber { + t.Fatalf("renumbered = %v, want %v", m.renumbered(), tc.renumber) + } + }) + } + t.Run("different path", func(t *testing.T) { + stored := []PathAnchor{{Path: "/a", Identity: "6:2"}} + fresh := []PathAnchor{{Path: "/z", Identity: "5:2"}} + if newDeviceRenumbering().add(stored, fresh) { + t.Fatal("chain with a different path accepted") + } + }) + t.Run("one mapping across chains", func(t *testing.T) { + m := newDeviceRenumbering() + if !m.add(chain("6:2"), chain("5:2")) { + t.Fatal("first chain refused") + } + if m.add(chain("6:2", "6:7"), chain("5:2", "7:7")) { + t.Fatal("second chain remapped a known device") + } + }) + t.Run("rekey", func(t *testing.T) { + m := newDeviceRenumbering() + if !m.add(chain("6:2"), chain("5:2")) { + t.Fatal("chain refused") + } + for stored, want := range map[string]string{"6:9": "5:9", "8:9": "8:9", "": "", "1:0:2": "1:0:2"} { + if got := m.rekey(stored); got != want { + t.Fatalf("rekey(%q) = %q, want %q", stored, got, want) + } + } + }) +} diff --git a/internal/installruntime/path.go b/internal/installruntime/path.go index b98a55d9c..c37edac7d 100644 --- a/internal/installruntime/path.go +++ b/internal/installruntime/path.go @@ -1,6 +1,9 @@ package installruntime -import "fmt" +import ( + "fmt" + "strings" +) const ( // Operator-editable control JSON shares the 64 KiB budget used by @@ -30,3 +33,107 @@ func checkAnchors(want, got []PathAnchor) error { } return nil } + +// splitDeviceIdentity parses a Unix device:inode identity. Windows identities +// carry three fields and never parse, so device renumbering stays Unix-only. +func splitDeviceIdentity(id string) (dev, ino string, ok bool) { + dev, ino, ok = strings.Cut(id, ":") + return dev, ino, ok && dev != "" && ino != "" && !strings.Contains(ino, ":") +} + +// deviceRenumbering maps device numbers recorded before a reboot onto the +// current ones. Each recorded device must map to exactly one current device and +// back, so a chain that now crosses a different mount never matches. +type deviceRenumbering struct{ devices, taken map[string]string } + +func newDeviceRenumbering() deviceRenumbering { + return deviceRenumbering{devices: map[string]string{}, taken: map[string]string{}} +} + +// add accepts a recorded parent chain only when the fresh one names the same +// paths and inodes under a consistent device mapping. +func (m deviceRenumbering) add(stored, fresh []PathAnchor) bool { + if len(fresh) < len(stored) { + return false + } + for i, a := range stored { + oldDev, oldIno, ok := splitDeviceIdentity(a.Identity) + if !ok { + return false + } + newDev, newIno, ok := splitDeviceIdentity(fresh[i].Identity) + if !ok || a.Path != fresh[i].Path || oldIno != newIno { + return false + } + if d, seen := m.devices[oldDev]; seen && d != newDev { + return false + } + if d, seen := m.taken[newDev]; seen && d != oldDev { + return false + } + m.devices[oldDev], m.taken[newDev] = newDev, oldDev + } + return true +} + +func (m deviceRenumbering) renumbered() bool { + for oldDev, newDev := range m.devices { + if oldDev != newDev { + return true + } + } + return false +} + +func (m deviceRenumbering) rekey(id string) string { + dev, ino, ok := splitDeviceIdentity(id) + if newDev, known := m.devices[dev]; ok && known { + return newDev + ":" + ino + } + return id +} + +// rebindRenumberedJournal re-keys a journal interrupted before a reboot that +// renumbered the volumes, so recovery can finish. A journal whose parent chains +// do not all match under one renumbering is left for the strict checks. +func rebindRenumberedJournal(tx *transaction) error { + m := newDeviceRenumbering() + for _, f := range tx.Files { + fresh, err := pathAnchors(f.Path, false) + if err != nil { + return err + } + if !m.add(f.Parents, fresh) { + return nil + } + } + if tx.Native != nil && len(tx.Native.Parents) != 0 { + fresh, err := pathAnchors(tx.Native.After.Path, false) + if err != nil { + return err + } + if !m.add(tx.Native.Parents, fresh) { + return nil + } + } + if !m.renumbered() { + return nil + } + for i := range tx.Files { + for j := range tx.Files[i].Parents { + tx.Files[i].Parents[j].Identity = m.rekey(tx.Files[i].Parents[j].Identity) + } + } + if tx.Native != nil { + for j := range tx.Native.Parents { + tx.Native.Parents[j].Identity = m.rekey(tx.Native.Parents[j].Identity) + } + for k := range tx.Native.PurgeTrees { + for rel, entry := range tx.Native.PurgeTrees[k].Entries { + entry.ObjectID, entry.Directory = m.rekey(entry.ObjectID), m.rekey(entry.Directory) + tx.Native.PurgeTrees[k].Entries[rel] = entry + } + } + } + return nil +} diff --git a/internal/installruntime/path_unix.go b/internal/installruntime/path_unix.go index 61b7574c9..37d6cf3fe 100644 --- a/internal/installruntime/path_unix.go +++ b/internal/installruntime/path_unix.go @@ -236,11 +236,6 @@ func PersistedIdentityMatches(stored, fresh, freshParent string) bool { return ok && storedIno == freshIno && freshDev == parentDev && storedDev != freshDev } -func splitDeviceIdentity(id string) (dev, ino string, ok bool) { - dev, ino, ok = strings.Cut(id, ":") - return dev, ino, ok && dev != "" && ino != "" && !strings.Contains(ino, ":") -} - func openedDirectoryIdentity(f *os.File) (string, error) { var st unix.Stat_t if err := unix.Fstat(int(f.Fd()), &st); err != nil { diff --git a/internal/installruntime/renumbered_journal_test.go b/internal/installruntime/renumbered_journal_test.go new file mode 100644 index 000000000..0e95dd696 --- /dev/null +++ b/internal/installruntime/renumbered_journal_test.go @@ -0,0 +1,183 @@ +//go:build linux || darwin + +package installruntime + +import ( + "context" + "fmt" + "os" + "path/filepath" + "strconv" + "strings" + "testing" +) + +// renumberJournal rewrites every device number recorded in the pending journal +// through shift, as if the volumes were renumbered by a reboot after the +// interruption. The ledgers inside the journal are shifted with the same rule. +func renumberJournal(t *testing.T, control string, shift func(dev uint64, at int) uint64) { + t.Helper() + marker := filepath.Join(control, "transaction.json") + tx, err := readTransactionFile(marker) + if err != nil { + t.Fatal(err) + } + at := 0 + move := func(id string) string { + if id == "" { + return id + } + dev, ino, ok := strings.Cut(id, ":") + n, err := strconv.ParseUint(dev, 10, 64) + if !ok || err != nil { + t.Fatalf("unexpected identity %q", id) + } + at++ + return fmt.Sprintf("%d:%s", shift(n, at), ino) + } + moveRecord := func(record *NativeRecord) { + if record == nil { + return + } + record.DirectoryID = move(record.DirectoryID) + record.PreviousDirectoryID = move(record.PreviousDirectoryID) + for i := range record.Published { + record.Published[i].DirectoryID = move(record.Published[i].DirectoryID) + } + } + for i := range tx.Files { + for j := range tx.Files[i].Parents { + tx.Files[i].Parents[j].Identity = move(tx.Files[i].Parents[j].Identity) + } + } + moveRecord(tx.Before.Native) + moveRecord(tx.After.Native) + if tx.Native != nil { + for j := range tx.Native.Parents { + tx.Native.Parents[j].Identity = move(tx.Native.Parents[j].Identity) + } + tx.Native.StagedID = move(tx.Native.StagedID) + moveRecord(&tx.Native.Before) + moveRecord(&tx.Native.After) + for k := range tx.Native.PurgeTrees { + for rel, entry := range tx.Native.PurgeTrees[k].Entries { + entry.ObjectID = move(entry.ObjectID) + entry.Directory = move(entry.Directory) + tx.Native.PurgeTrees[k].Entries[rel] = entry + } + } + } + if err := writeTransaction(marker, tx); err != nil { + t.Fatal(err) + } +} + +func shiftAll(dev uint64, _ int) uint64 { return dev + 1 } + +func interrupt(boundary string) func(string) error { + return func(phase string) error { + if phase == boundary { + return fmt.Errorf("simulated interruption") + } + return nil + } +} + +func interruptedFileCommit(t *testing.T) (context.Context, Request, string) { + t.Helper() + ctx, r := request(t) + target := filepath.Join(r.RuntimeRoot, "hook") + r.Files = []File{{Path: target, Data: []byte("new"), Mode: 0755}} + r.Fault = interrupt("transaction") + if _, err := Commit(ctx, r); err == nil { + t.Fatal("interruption did not stop the commit") + } + return ctx, r, target +} + +func TestRenumberedVolumeRecoversInterruptedJournal(t *testing.T) { + t.Run("file publication", func(t *testing.T) { + ctx, r, target := interruptedFileCommit(t) + renumberJournal(t, r.ControlRoot, shiftAll) + if _, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}); err != nil { + t.Fatalf("renumbered volume blocked recovery: %v", err) + } + if data, err := os.ReadFile(target); err != nil || string(data) != "new" { + t.Fatalf("recovery did not publish: %q %v", data, err) + } + }) + t.Run("native publication", func(t *testing.T) { + ctx, r, _, _ := installTwoNativeGenerations(t) + change, err := StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + r.Native = change + r.Fault = interrupt("transaction") + if _, err := Commit(ctx, r); err == nil { + t.Fatal("interruption did not stop the commit") + } + renumberNativeDevices(t, r.ControlRoot) + renumberJournal(t, r.ControlRoot, shiftAll) + recovered, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) + if err != nil { + t.Fatalf("renumbered volume blocked native recovery: %v", err) + } + if recovered.Native.Path != change.After.Path { + t.Fatalf("recovery did not select the new generation: %+v", recovered.Native) + } + if _, err := os.Stat(change.After.Path); err != nil { + t.Fatal("recovery did not publish the new generation") + } + }) + t.Run("purge cleanup", func(t *testing.T) { + ctx, r, _, second := installTwoNativeGenerations(t) + r.RemoveConsumer, r.PurgeNative = true, true + r.Fault = interrupt("ledger") + if _, err := Commit(ctx, r); err == nil { + t.Fatal("interruption did not stop the purge") + } + renumberJournal(t, r.ControlRoot, shiftAll) + if _, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}); err != nil { + t.Fatalf("renumbered volume blocked purge recovery: %v", err) + } + entries, err := os.ReadDir(filepath.Dir(second.After.Path)) + if err != nil { + t.Fatal(err) + } + for _, entry := range entries { + if strings.HasPrefix(entry.Name(), "generation-") || strings.Contains(entry.Name(), ".purged-") { + t.Fatalf("purge recovery left %s", entry.Name()) + } + } + }) +} + +func TestRenumberedJournalStillRefusesChangedObjects(t *testing.T) { + t.Run("substituted parent", func(t *testing.T) { + ctx, r, _ := interruptedFileCommit(t) + marker := filepath.Join(r.ControlRoot, "transaction.json") + renumberJournal(t, r.ControlRoot, shiftAll) + tx, err := readTransactionFile(marker) + if err != nil { + t.Fatal(err) + } + last := &tx.Files[0].Parents[len(tx.Files[0].Parents)-1] + dev, ino, _ := strings.Cut(last.Identity, ":") + n, _ := strconv.ParseUint(ino, 10, 64) + last.Identity = fmt.Sprintf("%s:%d", dev, n+1) + if err := writeTransaction(marker, tx); err != nil { + t.Fatal(err) + } + if _, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}); err == nil { + t.Fatal("recovery accepted a parent with a different inode") + } + }) + t.Run("inconsistent renumbering", func(t *testing.T) { + ctx, r, _ := interruptedFileCommit(t) + renumberJournal(t, r.ControlRoot, func(dev uint64, at int) uint64 { return dev + uint64(at) }) + if _, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}); err == nil { + t.Fatal("recovery accepted one volume renumbered to several devices") + } + }) +} diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index 2ece3a242..fd753e949 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -373,6 +373,9 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { return l, err } if readErr == nil { + if err := rebindRenumberedJournal(&pending); err != nil { + return l, err + } if r.RollbackPending { if !reflect.DeepEqual(l, pending.Before) && !reflect.DeepEqual(l, pending.After) { return l, fmt.Errorf("rollback ledger mismatch") From 8e523729ae242f816a90292c4cd31abd4f710466 Mon Sep 17 00:00:00 2001 From: Andrea Arturo Venti Fuentes <117413846+av1155@users.noreply.github.com> Date: Fri, 2 Oct 2026 01:48:37 -0400 Subject: [PATCH 03/16] fix(installer): keep identity refresh off retirement and read errors Refreshing the retired record made qualifyRetirement see a changed live generation, so retiring a legacy candidate on a renumbered volume left a journal that neither recovery nor rollback could clear. Retirement now keeps its record unchanged and the next commit converges it. Refresh also returned any error from opening an older generation, so a damaged generation that is neither live nor previous failed every commit. Such a generation now keeps its recorded identity. Adds tests for both, for a replaced older generation, and for the mount point rule against a real nested mount. Refs #301 --- .../agentnotify/setup/state_identity_test.go | 38 +++++++ internal/installruntime/native_tree.go | 36 +++--- .../persisted_identity_unix_test.go | 44 +++++++- .../installruntime/renumbered_volume_test.go | 106 ++++++++++++++++++ internal/installruntime/transaction.go | 9 +- 5 files changed, 200 insertions(+), 33 deletions(-) diff --git a/internal/agentnotify/setup/state_identity_test.go b/internal/agentnotify/setup/state_identity_test.go index 3604f0a59..65421a6ad 100644 --- a/internal/agentnotify/setup/state_identity_test.go +++ b/internal/agentnotify/setup/state_identity_test.go @@ -9,6 +9,8 @@ import ( "strconv" "strings" "testing" + + "golang.org/x/sys/unix" ) // A reboot can give the volume a new device number while every inode stays, @@ -46,3 +48,39 @@ func TestOwnedDirectoryIdentitySurvivesRenumberedVolume(t *testing.T) { t.Fatal("different directory accepted") } } + +// At a mount point the parent sits on another device, so an identity recorded +// with a different device must still be refused there. +func TestOwnedDirectoryIdentityRefusedAtMountPoint(t *testing.T) { + mount := "" + for _, path := range []string{"/System/Volumes/VM", "/System/Volumes/Preboot", "/dev/shm", "/dev/pts", "/run/lock"} { + var st, parent unix.Stat_t + if unix.Lstat(path, &st) == nil && unix.Lstat(filepath.Dir(path), &parent) == nil && st.Mode&unix.S_IFMT == unix.S_IFDIR && st.Dev != parent.Dev { + mount = path + break + } + } + if mount == "" { + t.Skip("no mount point below a top-level directory") + } + f, err := os.Open(mount) + if err != nil { + t.Fatal(err) + } + defer func() { _ = f.Close() }() + id, err := directoryID(f) + if err != nil { + t.Fatal(err) + } + if err := matches(f, id); err != nil { + t.Fatalf("exact identity refused at a mount point: %v", err) + } + dev, ino, _ := strings.Cut(id, ":") + devNumber, err := strconv.ParseUint(dev, 10, 64) + if err != nil { + t.Fatal(err) + } + if err := matches(f, fmt.Sprintf("%d:%s", devNumber+1, ino)); err == nil { + t.Fatal("identity with another device accepted at a mount point") + } +} diff --git a/internal/installruntime/native_tree.go b/internal/installruntime/native_tree.go index 852837c54..407caffe8 100644 --- a/internal/installruntime/native_tree.go +++ b/internal/installruntime/native_tree.go @@ -7,6 +7,7 @@ import ( "io/fs" "os" "path/filepath" + "slices" "strings" "time" ) @@ -247,39 +248,28 @@ func checkNativeDirectoryID(path, expected string) error { // refreshNativeIdentities records the current identity of every generation that // still matches its stored one, so a ledger written before the volume was -// renumbered converges and older writers sharing it keep accepting it. -func refreshNativeIdentities(record *NativeRecord) error { +// renumbered converges and older writers sharing it keep accepting it. A +// generation that cannot be opened keeps its stored identity. +func refreshNativeIdentities(record *NativeRecord) { if record == nil { - return nil + return } - refresh := func(path string, id *string) error { + refresh := func(path string, id *string) { if path == "" || *id == "" { - return nil + return } fresh, parent, err := nativeDirectoryIdentity(path) - if err != nil { - return err - } - if fresh != "" && fresh != *id && PersistedIdentityMatches(*id, fresh, parent) { + if err == nil && PersistedIdentityMatches(*id, fresh, parent) { *id = fresh } - return nil - } - if err := refresh(record.Path, &record.DirectoryID); err != nil { - return err - } - if err := refresh(record.PreviousPath, &record.PreviousDirectoryID); err != nil { - return err } - // Retirement copies the live record, so the slice may still back the - // pre-commit ledger that crash recovery compares against. - record.Published = append([]NativeGeneration(nil), record.Published...) + refresh(record.Path, &record.DirectoryID) + refresh(record.PreviousPath, &record.PreviousDirectoryID) + // Never write through to a record this one was shallow-copied from. + record.Published = slices.Clone(record.Published) for i := range record.Published { - if err := refresh(record.Published[i].Path, &record.Published[i].DirectoryID); err != nil { - return err - } + refresh(record.Published[i].Path, &record.Published[i].DirectoryID) } - return nil } func checkOpenedNativeRoot(root *os.Root, expected []PathAnchor) error { diff --git a/internal/installruntime/persisted_identity_unix_test.go b/internal/installruntime/persisted_identity_unix_test.go index e4db4af97..b6cca54d8 100644 --- a/internal/installruntime/persisted_identity_unix_test.go +++ b/internal/installruntime/persisted_identity_unix_test.go @@ -6,6 +6,8 @@ import ( "os" "path/filepath" "testing" + + "golang.org/x/sys/unix" ) func TestPersistedIdentityMatches(t *testing.T) { @@ -53,12 +55,44 @@ func TestRefreshNativeIdentitiesLeavesSharedRecordIntact(t *testing.T) { } live := NativeRecord{Path: gens[1].Path, DirectoryID: gens[1].DirectoryID, PreviousPath: gens[0].Path, PreviousDirectoryID: gens[0].DirectoryID, Published: gens} stale := gens[0].DirectoryID - retired := live - if err := refreshNativeIdentities(&retired); err != nil { - t.Fatal(err) - } + copied := live + refreshNativeIdentities(&copied) if live.Published[0].DirectoryID != stale { t.Fatal("refresh rewrote the record it was copied from") } - requireFreshNativeIdentities(t, &retired) + requireFreshNativeIdentities(t, &copied) +} + +// A real mount point exercises the parent lookup: a directory on a different +// device from its parent must keep refusing an identity with another device. +func TestRenumberedIdentityRefusedAtMountPoint(t *testing.T) { + mount := nestedMountPoint(t) + id, err := nativeDirectoryID(mount) + if err != nil || id == "" { + t.Fatalf("mount point identity: %q %v", id, err) + } + if err := checkNativeDirectoryID(mount, id); err != nil { + t.Fatalf("exact identity refused at a mount point: %v", err) + } + shifted := shiftDevice(t, id) + if err := checkNativeDirectoryID(mount, shifted); err == nil { + t.Fatal("identity with another device accepted at a mount point") + } + record := NativeRecord{Path: mount, DirectoryID: shifted} + refreshNativeIdentities(&record) + if record.DirectoryID != shifted { + t.Fatal("refresh rebound a mount point") + } +} + +func nestedMountPoint(t *testing.T) string { + t.Helper() + for _, path := range []string{"/System/Volumes/VM", "/System/Volumes/Preboot", "/dev/shm", "/dev/pts", "/run/lock"} { + var st, parent unix.Stat_t + if unix.Lstat(path, &st) == nil && unix.Lstat(filepath.Dir(path), &parent) == nil && st.Mode&unix.S_IFMT == unix.S_IFDIR && st.Dev != parent.Dev { + return path + } + } + t.Skip("no mount point below a top-level directory") + return "" } diff --git a/internal/installruntime/renumbered_volume_test.go b/internal/installruntime/renumbered_volume_test.go index 9a7dec9a4..42af96617 100644 --- a/internal/installruntime/renumbered_volume_test.go +++ b/internal/installruntime/renumbered_volume_test.go @@ -180,6 +180,112 @@ func TestRenumberedVolumeKeepsNativeManaged(t *testing.T) { } requireFreshNativeIdentities(t, recovered.Native) }) + t.Run("retire a legacy candidate", func(t *testing.T) { + nativeDrainCheck = func(context.Context, string) error { return nil } + t.Cleanup(func() { nativeDrainCheck = verifyNativeDrain }) + ctx, r, _, _ := installTwoNativeGenerations(t) + l, err := readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } + candidate := filepath.Join(filepath.Dir(l.Native.Path), ".candidate-legacy.app") + if err := os.MkdirAll(filepath.Join(candidate, "Contents"), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(candidate, "Contents", "legacy"), []byte("legacy"), 0600); err != nil { + t.Fatal(err) + } + sum, err := treeFingerprint(candidate) + if err != nil { + t.Fatal(err) + } + id, err := nativeDirectoryID(candidate) + if err != nil { + t.Fatal(err) + } + l.Native.PreviousPath, l.Native.PreviousSHA256, l.Native.PreviousDirectoryID = candidate, sum, id + l.Native.DecoderFloor = 1 + if err := writeJSON(filepath.Join(r.ControlRoot, "ownership.json"), l); err != nil { + t.Fatal(err) + } + renumberNativeDevices(t, r.ControlRoot) + r.RetireNative, r.ExpectedGeneration = true, &l.Generation + retired, err := Commit(ctx, r) + if err != nil { + t.Fatalf("renumbered volume blocked retiring a candidate: %v", err) + } + if _, err := os.Lstat(candidate); retired.Native.PreviousPath != "" || !os.IsNotExist(err) { + t.Fatalf("retirement kept the candidate predecessor: %v", err) + } + r.RetireNative, r.ExpectedGeneration = false, nil + next, err := Commit(ctx, r) + if err != nil { + t.Fatalf("retirement left the install blocked: %v", err) + } + requireFreshNativeIdentities(t, next.Native) + }) +} + +// Refresh only rebinds what still matches: an older generation that was +// replaced or cannot be opened keeps its recorded identity and never blocks. +func TestRefreshKeepsUnmatchedGenerations(t *testing.T) { + for _, damage := range []string{"replaced", "symlink", "unreadable"} { + t.Run(damage, func(t *testing.T) { + if damage == "unreadable" && os.Geteuid() == 0 { + t.Skip("root opens unreadable directories") + } + ctx, r, first, _ := installTwoNativeGenerations(t) + change, err := StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + r.Native = change + if _, err := Commit(ctx, r); err != nil { + t.Fatal(err) + } + r.Native = nil + before := renumberNativeDevices(t, r.ControlRoot) + oldest := first.After.Path + switch damage { + case "replaced": + if err := os.Rename(oldest, oldest+".moved"); err != nil { + t.Fatal(err) + } + if err := os.CopyFS(oldest, os.DirFS(oldest+".moved")); err != nil { + t.Fatal(err) + } + case "symlink": + if err := os.Rename(oldest, oldest+".moved"); err != nil { + t.Fatal(err) + } + if err := os.Symlink(oldest+".moved", oldest); err != nil { + t.Fatal(err) + } + case "unreadable": + if err := os.Chmod(oldest, 0); err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = os.Chmod(oldest, 0700) }) + } + next, err := Commit(ctx, r) + if err != nil { + t.Fatalf("%s older generation blocked a commit: %v", damage, err) + } + recorded := func(record *NativeRecord) string { + for _, gen := range record.Published { + if gen.Path == oldest { + return gen.DirectoryID + } + } + t.Fatal("older generation missing from the record") + return "" + } + if got, want := recorded(next.Native), recorded(before.Native); got != want { + t.Fatalf("refresh rebound a %s generation: %s, recorded %s", damage, got, want) + } + requireFreshNativeIdentities(t, &NativeRecord{Path: next.Native.Path, DirectoryID: next.Native.DirectoryID, PreviousPath: next.Native.PreviousPath, PreviousDirectoryID: next.Native.PreviousDirectoryID}) + }) + } } func TestReplacedNativeDirectoryStillRefused(t *testing.T) { diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index fd753e949..be67ffe0c 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -692,11 +692,10 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { if err := validateNative(native); err != nil { return l, err } - // Only next changes: tx.Before must stay equal to the ledger on disk. - if nativeChecked { - if err := refreshNativeIdentities(next.Native); err != nil { - return l, err - } + // Refresh next only: tx.Before must match the ledger on disk, and + // qualifyRetirement requires the live record unchanged. + if nativeChecked && (native == nil || !native.Retire) { + refreshNativeIdentities(next.Native) } files := append([]File(nil), r.Files...) if r.PolicyEnabled != nil || len(r.PolicyFields) != 0 || (r.RemoveConsumer && len(next.Consumers) == 0) { From 50bf7b8cdf4ae981fc3a796314659daa21b4b4e1 Mon Sep 17 00:00:00 2001 From: Andrea Arturo Venti Fuentes <117413846+av1155@users.noreply.github.com> Date: Fri, 2 Oct 2026 02:24:22 -0400 Subject: [PATCH 04/16] docs(installer): state what the device mapping refuses Refs #301 --- internal/installruntime/path.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/installruntime/path.go b/internal/installruntime/path.go index c37edac7d..905b6e21d 100644 --- a/internal/installruntime/path.go +++ b/internal/installruntime/path.go @@ -43,7 +43,7 @@ func splitDeviceIdentity(id string) (dev, ino string, ok bool) { // deviceRenumbering maps device numbers recorded before a reboot onto the // current ones. Each recorded device must map to exactly one current device and -// back, so a chain that now crosses a different mount never matches. +// back, so a chain whose mount structure changed never matches. type deviceRenumbering struct{ devices, taken map[string]string } func newDeviceRenumbering() deviceRenumbering { From 857f834b97d72c1bede461c3a00175e1158c57d8 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 07:10:34 +0000 Subject: [PATCH 05/16] fix(install): tolerate macOS device renumbering safely Refresh durable identities without renaming native generations and preserve recovery journal before-images. Expose the committed native path for post-publication registration. Refs #301 Refs #302 --- cmd/claude-notifications/install_runtime.go | 9 + .../install_runtime_path_test.go | 88 ++++ .../agentnotify/setup/state_identity_test.go | 91 ++++ internal/agentnotify/setup/state_unix.go | 16 +- internal/installruntime/identity.go | 85 ++++ internal/installruntime/identity_darwin.go | 3 + .../installruntime/identity_darwin_test.go | 62 +++ internal/installruntime/identity_other.go | 5 + .../installruntime/identity_other_test.go | 9 + internal/installruntime/identity_refresh.go | 120 +++++ internal/installruntime/identity_test.go | 91 ++++ internal/installruntime/native_path_unix.go | 9 +- internal/installruntime/native_tree.go | 28 +- internal/installruntime/path_unix.go | 6 +- .../installruntime/persisted_anchor_test.go | 126 +++++ internal/installruntime/purge.go | 45 +- internal/installruntime/recovery.go | 18 + .../installruntime/renumber_commit_test.go | 462 ++++++++++++++++++ internal/installruntime/transaction.go | 32 +- 19 files changed, 1271 insertions(+), 34 deletions(-) create mode 100644 cmd/claude-notifications/install_runtime_path_test.go create mode 100644 internal/agentnotify/setup/state_identity_test.go create mode 100644 internal/installruntime/identity.go create mode 100644 internal/installruntime/identity_darwin.go create mode 100644 internal/installruntime/identity_darwin_test.go create mode 100644 internal/installruntime/identity_other.go create mode 100644 internal/installruntime/identity_other_test.go create mode 100644 internal/installruntime/identity_refresh.go create mode 100644 internal/installruntime/identity_test.go create mode 100644 internal/installruntime/persisted_anchor_test.go create mode 100644 internal/installruntime/renumber_commit_test.go diff --git a/cmd/claude-notifications/install_runtime.go b/cmd/claude-notifications/install_runtime.go index b34c053a3..c4a0e1287 100644 --- a/cmd/claude-notifications/install_runtime.go +++ b/cmd/claude-notifications/install_runtime.go @@ -30,6 +30,7 @@ func installRuntime(args []string, output io.Writer) error { refresh := flags.Bool("refresh", false, "refresh files for existing consumers without adding a registration") relocateCache := flags.Bool("relocate-versioned-cache", false, "move Claude hooks between versioned plugin caches") purge := flags.Bool("purge-native", false, "explicitly remove retained callback on final uninstall") + printNativePath := flags.Bool("print-native-path", false, "print only the committed durable native generation path") consumer := flags.String("consumer", "claude-hooks", "managed consumer identity") if err := flags.Parse(args); err != nil { return err @@ -294,6 +295,14 @@ func installRuntime(args []string, output io.Writer) error { } ledger, err := installruntime.Commit(ctx, req) if err == nil { + if *printNativePath { + path := "" + if ledger.Native != nil { + path = ledger.Native.Path + } + _, err = fmt.Fprintln(output, path) + return err + } _, _ = fmt.Fprintf(output, "managed-runtime committed generation=%d\n", ledger.Generation) if *purge { _, _ = fmt.Fprintln(output, "Callback entrypoint purge completed; pending notifications may no longer open targets. Running callbacks are not stopped.") diff --git a/cmd/claude-notifications/install_runtime_path_test.go b/cmd/claude-notifications/install_runtime_path_test.go new file mode 100644 index 000000000..e5745e843 --- /dev/null +++ b/cmd/claude-notifications/install_runtime_path_test.go @@ -0,0 +1,88 @@ +package main + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/777genius/agent-notifications/internal/installruntime" +) + +// Removing the flag, printing the default status, or returning an alias/stage +// instead of the committed Native.Path makes this contract test fail. +func TestInstallRuntimePrintNativePath(t *testing.T) { + for _, native := range []bool{false, true} { + t.Run(map[bool]string{false: "no-native", true: "durable-native"}[native], func(t *testing.T) { + if native && runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.Skip("native staging is Unix-only") + } + root := t.TempDir() + stage := filepath.Join(root, "stage") + target := filepath.Join(root, "runtime", "bin") + control := filepath.Join(root, "control") + if err := os.MkdirAll(stage, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(stage, "claude-notifications-linux-amd64"), []byte("inert "+installruntime.WriterProtocolMarker), 0700); err != nil { + t.Fatal(err) + } + if native { + app := filepath.Join(stage, "ClaudeNotifier.app", "Contents", "MacOS") + if err := os.MkdirAll(app, 0700); err != nil { + t.Fatal(err) + } + // Unknown native packages are copied and hashed, never executed. + if err := os.WriteFile(filepath.Join(app, "terminal-notifier-modern"), []byte("inert native"), 0700); err != nil { + t.Fatal(err) + } + } + var output bytes.Buffer + args := []string{"--stage", stage, "--target", target, "--control-root", control, "--print-native-path"} + if err := installRuntime(args, &output); err != nil { + t.Fatal(err) + } + raw, err := os.ReadFile(filepath.Join(control, "ownership.json")) + if err != nil { + t.Fatal(err) + } + var ledger installruntime.Ledger + if err = json.Unmarshal(raw, &ledger); err != nil { + t.Fatal(err) + } + want := "" + if ledger.Native != nil { + want = ledger.Native.Path + } + if output.String() != want+"\n" { + t.Fatalf("stdout=%q want %q", output.String(), want+"\n") + } + if native { + if want == "" || strings.Contains(want, ".candidate-") || strings.HasPrefix(want, stage) { + t.Fatalf("not published: %s", want) + } + if _, err := os.Stat(want); err != nil { + t.Fatal(err) + } + } + output.Reset() + if err := installRuntime(args[:len(args)-1], &output); err != nil { + t.Fatal(err) + } + if !strings.HasPrefix(output.String(), "managed-runtime committed generation=") { + t.Fatalf("default output changed: %q", output.String()) + } + output.Reset() + if err := installRuntime([]string{"--remove", "--purge-native", "--target", target, "--control-root", control, "--print-native-path"}, &output); err != nil { + t.Fatal(err) + } + if output.String() != "\n" { + t.Fatalf("purge stdout=%q", output.String()) + } + + }) + } +} diff --git a/internal/agentnotify/setup/state_identity_test.go b/internal/agentnotify/setup/state_identity_test.go new file mode 100644 index 000000000..6d6543f17 --- /dev/null +++ b/internal/agentnotify/setup/state_identity_test.go @@ -0,0 +1,91 @@ +//go:build linux || darwin + +package setup + +import ( + "encoding/json" + "os" + "path/filepath" + "runtime" + "strings" + "testing" + + "github.com/777genius/agent-notifications/internal/installruntime" +) + +func TestPersistedIdentitySetupOwnership(t *testing.T) { + path, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + rootPath := filepath.Join(path, "control") + if err = os.Mkdir(rootPath, 0700); err != nil { + t.Fatal(err) + } + root, err := openRoot(rootPath) + if err != nil { + t.Fatal(err) + } + defer root.Close() + state, err := mkdir(root, "state") + if err != nil { + t.Fatal(err) + } + id, err := directoryID(state) + if err != nil { + t.Fatal(err) + } + _, ino, _ := strings.Cut(id, ":") + stored := "999999:" + ino + if strings.HasPrefix(id, "999999:") { + stored = "888888:" + ino + } + err = matches(state, root, stored) + if (err == nil) != (runtime.GOOS == "darwin") { + t.Fatalf("%s persisted setup identity: %v", runtime.GOOS, err) + } + if err = matches(state, root, id); err != nil { + t.Fatal(err) + } + // Build only the structural evidence consumed by checkProvisioned. No + // notification delivery or user namespace is involved. + for _, name := range []string{"journal", "native-spool"} { + child, err := mkdir(state, name) + if err != nil { + t.Fatal(err) + } + if name == "native-spool" { + if err = createLock(child); err != nil { + t.Fatal(err) + } + } + child.Close() + } + for _, name := range []string{"namespace", "journal.json", "lock"} { + data := []byte("fixture") + if name == "namespace" { + data = []byte("namespace\n") + } + if err = os.WriteFile(filepath.Join(rootPath, "state", "journal", name), data, 0600); err != nil { + t.Fatal(err) + } + } + owner, _ := json.Marshal(ownership{Phase: "ready", DirectoryID: stored, Namespace: "namespace"}) + snapshot := installruntime.PolicySnapshot{Fields: map[string]json.RawMessage{"setupState": owner}} + err = checkProvisioned(Options{ControlRoot: rootPath}, snapshot) + if (err == nil) != (runtime.GOOS == "darwin") { + t.Fatalf("%s ready marker: %v", runtime.GOOS, err) + } + state.Close() + if err = os.Rename(filepath.Join(rootPath, "state"), filepath.Join(rootPath, "saved")); err != nil { + t.Fatal(err) + } + replacement, err := mkdir(root, "state") + if err != nil { + t.Fatal(err) + } + defer replacement.Close() + if matches(replacement, root, stored) == nil { + t.Fatal("replacement setup state accepted") + } +} diff --git a/internal/agentnotify/setup/state_unix.go b/internal/agentnotify/setup/state_unix.go index f89fb1d0b..7ed955686 100644 --- a/internal/agentnotify/setup/state_unix.go +++ b/internal/agentnotify/setup/state_unix.go @@ -89,7 +89,7 @@ func provision(ctx context.Context, o Options, s installruntime.PolicySnapshot, if e != nil { return s, pre, "", fail("unsafe_state", e) } - if e = matches(stage, owner.DirectoryID); e != nil { + if e = matches(stage, root, owner.DirectoryID); e != nil { _ = stage.Close() return s, pre, "", fail("unsafe_state", e) } @@ -132,7 +132,7 @@ func provision(ctx context.Context, o Options, s installruntime.PolicySnapshot, return s, pre, "", fail("initialization_recovery_required", fmt.Errorf("expected owned state is missing or unsafe: %w", e)) } defer func() { _ = state.Close() }() - if e = matches(state, owner.DirectoryID); e != nil { + if e = matches(state, root, owner.DirectoryID); e != nil { return s, pre, "", fail("initialization_recovery_required", e) } if location == "state" { @@ -176,7 +176,7 @@ func provision(ctx context.Context, o Options, s installruntime.PolicySnapshot, if e != nil { return s, pre, "", fail("unsafe_state", e) } - e = matches(named, owner.DirectoryID) + e = matches(named, root, owner.DirectoryID) _ = named.Close() if e != nil { return s, pre, "", fail("unsafe_state", e) @@ -290,12 +290,16 @@ func directoryID(f *os.File) (string, error) { e := unix.Fstat(int(f.Fd()), &st) return fmt.Sprintf("%d:%d", st.Dev, st.Ino), e } -func matches(f *os.File, want string) error { +func matches(f, parent *os.File, want string) error { id, e := directoryID(f) if e != nil { return e } - if id != want { + var parentStat unix.Stat_t + if e = unix.Fstat(int(parent.Fd()), &parentStat); e != nil { + return e + } + if !installruntime.MatchPersistedDirectory(want, id, fmt.Sprintf("%d", parentStat.Dev)) { return fmt.Errorf("owned directory identity changed") } return nil @@ -347,7 +351,7 @@ func checkProvisioned(o Options, s installruntime.PolicySnapshot) error { return fail("initialization_recovery_required", e) } defer func() { _ = state.Close() }() - if e = matches(state, owner.DirectoryID); e != nil { + if e = matches(state, root, owner.DirectoryID); e != nil { return fail("initialization_recovery_required", e) } j, e := openChild(state, "journal") diff --git a/internal/installruntime/identity.go b/internal/installruntime/identity.go new file mode 100644 index 000000000..8b07dad2b --- /dev/null +++ b/internal/installruntime/identity.go @@ -0,0 +1,85 @@ +package installruntime + +import ( + "fmt" + "strconv" + "strings" +) + +// Unix identities are decimal dev:ino; Darwin's dev_t can be signed. +func splitObjectID(id string) (dev, ino string, ok bool) { + dev, ino, ok = strings.Cut(id, ":") + if !ok { + return "", "", false + } + n, err := strconv.ParseUint(ino, 10, 64) + if err != nil || strconv.FormatUint(n, 10) != ino { + return "", "", false + } + magnitude := strings.TrimPrefix(dev, "-") + d, err := strconv.ParseUint(magnitude, 10, 64) + if err != nil || strconv.FormatUint(d, 10) != magnitude || dev == "-0" { + return "", "", false + } + return dev, ino, true +} + +func objectDevice(id string) string { + dev, _, ok := splitObjectID(id) + if !ok { + return "" + } + return dev +} + +// MatchPersistedDirectory compares a durable identity with a freshly observed +// physical directory. Callers must reject symlinks/non-directories first and +// supply the device of its held or no-follow observed parent. Only Darwin may +// renumber devices, and only away from a current volume boundary. +func MatchPersistedDirectory(stored, current, parentDev string) bool { + if !persistedDevRelaxed { + return stored == current + } + return matchPersistedObject(stored, current, parentDev) +} + +func matchPersistedObject(stored, current, parentDev string) bool { + sd, si, sok := splitObjectID(stored) + cd, ci, cok := splitObjectID(current) + return sok && cok && si == ci && (sd == cd || cd == parentDev) +} + +// Preserve both the inode chain and its volume partition. A bijection prevents +// a new mount boundary or merged volumes from masquerading as device renumbering. +func matchPersistedAnchorChain(want, got []PathAnchor) bool { + if len(got) < len(want) { + return false + } + forward, reverse := map[string]string{}, map[string]string{} + for i, a := range want { + b := got[i] + ad, ai, aok := splitObjectID(a.Identity) + bd, bi, bok := splitObjectID(b.Identity) + if a.Path != b.Path || !aok || !bok || ai != bi { + return false + } + if d, ok := forward[ad]; ok && d != bd { + return false + } + if d, ok := reverse[bd]; ok && d != ad { + return false + } + forward[ad], reverse[bd] = bd, ad + } + return true +} + +func checkPersistedAnchors(want, got []PathAnchor) error { + if !persistedDevRelaxed { + return checkAnchors(want, got) + } + if !matchPersistedAnchorChain(want, got) { + return fmt.Errorf("managed destination persisted parent substituted") + } + return nil +} diff --git a/internal/installruntime/identity_darwin.go b/internal/installruntime/identity_darwin.go new file mode 100644 index 000000000..1873d7da4 --- /dev/null +++ b/internal/installruntime/identity_darwin.go @@ -0,0 +1,3 @@ +package installruntime + +const persistedDevRelaxed = true diff --git a/internal/installruntime/identity_darwin_test.go b/internal/installruntime/identity_darwin_test.go new file mode 100644 index 000000000..76faabf6b --- /dev/null +++ b/internal/installruntime/identity_darwin_test.go @@ -0,0 +1,62 @@ +package installruntime + +import ( + "os" + "path/filepath" + "testing" +) + +func TestPersistedIdentityDarwinPurgeObjects(t *testing.T) { + for _, mode := range []string{"renumber", "inode", "fingerprint", "symlink"} { + t.Run(mode, func(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "asset") + if err := os.WriteFile(path, []byte("original"), 0600); err != nil { + t.Fatal(err) + } + entries, err := capturePurgeTree(root) + if err != nil { + t.Fatal(err) + } + for rel, e := range entries { + e.ObjectID = renumberID(e.ObjectID) + e.Directory = renumberID(e.Directory) + entries[rel] = e + } + if mode == "inode" || mode == "symlink" { + saved := filepath.Join(t.TempDir(), "original") + if err = os.Rename(path, saved); err != nil { + t.Fatal(err) + } + if mode == "inode" { + err = os.WriteFile(path, []byte("original"), 0600) + } else { + err = os.Symlink(saved, path) + } + if err != nil { + t.Fatal(err) + } + } else if mode == "fingerprint" { + if err = os.WriteFile(path, []byte("foreign"), 0600); err != nil { + t.Fatal(err) + } + } + err = cleanupPurgeTree(PurgeTree{Path: root, Entries: entries}, nil) + if mode == "renumber" { + if err != nil { + t.Fatal(err) + } + if _, err = os.Stat(root); !os.IsNotExist(err) { + t.Fatal("purge not completed", err) + } + } else { + if err == nil { + t.Fatal("foreign purge entry accepted") + } + if _, err = os.Lstat(path); err != nil { + t.Fatal("foreign entry removed", err) + } + } + }) + } +} diff --git a/internal/installruntime/identity_other.go b/internal/installruntime/identity_other.go new file mode 100644 index 000000000..6af6e3638 --- /dev/null +++ b/internal/installruntime/identity_other.go @@ -0,0 +1,5 @@ +//go:build !darwin + +package installruntime + +const persistedDevRelaxed = false diff --git a/internal/installruntime/identity_other_test.go b/internal/installruntime/identity_other_test.go new file mode 100644 index 000000000..c9bac1901 --- /dev/null +++ b/internal/installruntime/identity_other_test.go @@ -0,0 +1,9 @@ +//go:build !darwin + +package installruntime + +import "testing" + +func TestRenumberDarwinFilesystemDeferredToCI(t *testing.T) { + t.Skip("Darwin renumber, refresh, rollback, purge and mount behavior requires ephemeral macOS CI; Linux comparator tests are logic evidence only") +} diff --git a/internal/installruntime/identity_refresh.go b/internal/installruntime/identity_refresh.go new file mode 100644 index 000000000..cf7abbd29 --- /dev/null +++ b/internal/installruntime/identity_refresh.go @@ -0,0 +1,120 @@ +package installruntime + +import ( + "fmt" + "os" + "path/filepath" + "reflect" + "strings" +) + +// Refresh a detached ledger, never journal records or their before-images. +// Missing generations keep their durable identity (not an invitation to adopt +// a replacement). Paths and all non-identity fields remain immutable. +func refreshLedgerIdentities(l Ledger) (Ledger, error) { + if !persistedDevRelaxed || l.Native == nil { + return l, nil + } + record := *l.Native + record.Published = append([]NativeGeneration(nil), record.Published...) + // Keep nil vs empty slices significant for journal equality. + if l.Native.Published != nil && record.Published == nil { + record.Published = []NativeGeneration{} + } + l.Native = &record + refresh := func(path string, id *string) error { + if path == "" || *id == "" { + return nil + } + current, parentDev, err := nativeDirectoryIdentity(path) + if os.IsNotExist(err) { + return nil + } + if err != nil { + return err + } + if current == "" { + return nil + } + if !MatchPersistedDirectory(*id, current, parentDev) { + return fmt.Errorf("native directory inode changed: %s", path) + } + *id = current + return nil + } + if err := refresh(record.Path, &record.DirectoryID); err != nil { + return l, err + } + if err := refresh(record.PreviousPath, &record.PreviousDirectoryID); err != nil { + return l, err + } + for i := range record.Published { + if err := refresh(record.Published[i].Path, &record.Published[i].DirectoryID); err != nil { + return l, err + } + } + return l, nil +} + +// Recovery can crash after writing a refreshed ledger but before retiring its +// unchanged journal. Compare detached, disk-validated normal forms so that the +// transaction accepts its own durable after-image on every retry. All other +// ledger fields still require exact equality. +func ledgerMatchesJournal(current, snapshot Ledger, cleanup *NativeChange) bool { + if reflect.DeepEqual(current, snapshot) { + return true + } + if !persistedDevRelaxed { + return false + } + current, err := refreshLedgerIdentities(current) + if err != nil { + return false + } + snapshot, err = refreshLedgerIdentities(snapshot) + if err != nil { + return false + } + if cleanup != nil && cleanup.Retire { + alignRetiredIdentities(current.Native, snapshot.Native, cleanup.PurgeTrees) + } + return reflect.DeepEqual(current, snapshot) +} + +// A legacy private candidate can also appear in the retained inventory. Recovery +// may refresh its ID, delete it, then crash before retiring the journal. There is +// no live inode left to normalize on retry. Reconcile only this historical field, +// using the durable deletion manifest, an absent leaf, the same inode and the +// current physical parent's device. An unrecorded disappearance cannot qualify. +func alignRetiredIdentities(current, snapshot *NativeRecord, trees []PurgeTree) { + if current == nil || snapshot == nil || len(current.Published) != len(snapshot.Published) { + return + } + for i, want := range snapshot.Published { + got := current.Published[i] + if want.Path != got.Path || want.DirectoryID == got.DirectoryID || !strings.HasPrefix(filepath.Base(want.Path), ".candidate-") { + continue + } + for _, tree := range trees { + entry, ok := tree.Entries["."] + if !ok || tree.Path != want.Path || entry.Directory == "" { + continue + } + if _, err := os.Lstat(want.Path); !os.IsNotExist(err) { + continue + } + path, err := platformAnchorPath(want.Path) + if err != nil { + continue + } + anchors, err := pathAnchors(path, false) + if err != nil || len(anchors) == 0 || anchors[len(anchors)-1].Path != filepath.Dir(path) { + continue + } + parentDev := objectDevice(anchors[len(anchors)-1].Identity) + if matchPersistedObject(want.DirectoryID, got.DirectoryID, parentDev) && matchPersistedObject(entry.Directory, got.DirectoryID, parentDev) { + snapshot.Published[i].DirectoryID = got.DirectoryID + } + } + } +} diff --git a/internal/installruntime/identity_test.go b/internal/installruntime/identity_test.go new file mode 100644 index 000000000..c3b28fce4 --- /dev/null +++ b/internal/installruntime/identity_test.go @@ -0,0 +1,91 @@ +package installruntime + +import ( + "runtime" + "testing" +) + +// Pure logic evidence; actual Darwin filesystem/recovery tests live in the +// Darwin-tagged suite and must execute on ephemeral macOS CI. +func TestPersistedIdentityComparatorLogic(t *testing.T) { + for _, tc := range []struct { + name, stored, current, parent string + want bool + }{ + {"equal", "1:10", "1:10", "2", true}, + {"renumber", "1:10", "2:10", "2", true}, + {"boundary", "1:10", "2:10", "3", false}, + {"replacement", "1:10", "2:11", "2", false}, + {"malformed", "bad", "bad", "2", false}, + {"bad inode", "1:x", "2:x", "2", false}, + {"extra colon", "1:10:2", "2:10:2", "2", false}, + {"signed dev", "-1:10", "2:10", "2", true}, + {"empty", "", "", "", false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := matchPersistedObject(tc.stored, tc.current, tc.parent); got != tc.want { + t.Fatalf("match=%v want %v", got, tc.want) + } + }) + } +} + +func TestPersistedAnchorChainLogic(t *testing.T) { + chain := func(ids ...string) []PathAnchor { + a := make([]PathAnchor, len(ids)) + for i, id := range ids { + a[i] = PathAnchor{Path: string(rune('a' + i)), Identity: id} + } + return a + } + for _, tc := range []struct { + name string + before, after []PathAnchor + want bool + }{ + {"same", chain("1:1", "1:2", "2:3"), chain("1:1", "1:2", "2:3"), true}, + {"renumber", chain("1:1", "1:2", "2:3"), chain("7:1", "7:2", "8:3"), true}, + {"new mount", chain("1:1", "1:2"), chain("7:1", "8:2"), false}, + {"removed mount", chain("1:1", "2:2"), chain("7:1", "7:2"), false}, + {"inconsistent mapping", chain("1:1", "2:2", "1:3"), chain("7:1", "8:2", "9:3"), false}, + {"inode", chain("1:1"), chain("7:2"), false}, + {"shorter", chain("1:1", "1:2"), chain("7:1"), false}, + {"malformed", chain("bad"), chain("bad"), false}, + {"path", []PathAnchor{{Path: "a", Identity: "1:1"}}, []PathAnchor{{Path: "b", Identity: "7:1"}}, false}, + } { + t.Run(tc.name, func(t *testing.T) { + if got := matchPersistedAnchorChain(tc.before, tc.after); got != tc.want { + t.Fatalf("match=%v want %v", got, tc.want) + } + }) + } +} + +func TestPersistedIdentityPlatformPolicy(t *testing.T) { + relaxed := runtime.GOOS == "darwin" + if got := MatchPersistedDirectory("1:10", "2:10", "2"); got != relaxed { + t.Fatalf("%s policy=%v", runtime.GOOS, got) + } + if err := checkPersistedAnchors([]PathAnchor{{Path: "a", Identity: "1:10"}}, []PathAnchor{{Path: "a", Identity: "2:10"}}); (err == nil) != relaxed { + t.Fatalf("%s anchor policy: %v", runtime.GOOS, err) + } + // Held descriptors always use strict equality, even on Darwin. + if checkAnchors([]PathAnchor{{Path: "a", Identity: "1:10"}}, []PathAnchor{{Path: "a", Identity: "2:10"}}) == nil { + t.Fatal("held-fd anchors relaxed") + } + file := Identity{Exists: true, SHA256: "same", Mode: 0600} + if got := matchPurgeEntry(PurgeEntry{ObjectID: "1:10", File: file}, PurgeEntry{ObjectID: "2:10", File: file}, "2"); got != relaxed { + t.Fatalf("%s regular object policy=%v", runtime.GOOS, got) + } + changed := file + changed.SHA256 = "foreign" + if matchPurgeEntry(PurgeEntry{ObjectID: "1:10", File: file}, PurgeEntry{ObjectID: "2:10", File: changed}, "2") { + t.Fatal("changed content accepted") + } + if matchPurgeEntry(PurgeEntry{ObjectID: "1:10", File: file}, PurgeEntry{ObjectID: "2:11", File: file}, "2") { + t.Fatal("replaced object accepted") + } + if matchPurgeEntry(PurgeEntry{ObjectID: "1:10", File: file}, PurgeEntry{ObjectID: "2:10", File: file}, "3") { + t.Fatal("cross-volume object accepted") + } +} diff --git a/internal/installruntime/native_path_unix.go b/internal/installruntime/native_path_unix.go index a50f09086..8bb1721e4 100644 --- a/internal/installruntime/native_path_unix.go +++ b/internal/installruntime/native_path_unix.go @@ -18,15 +18,18 @@ func renameNative(from, to string, expected []PathAnchor, sourceID ...string) er return err } defer func() { _ = parent.Close() }() - if err := checkAnchors(expected, anchors); err != nil { + if err := checkPersistedAnchors(expected, anchors); err != nil { return err } if len(sourceID) != 0 && sourceID[0] != "" { - var st unix.Stat_t + var st, parentStat unix.Stat_t if err := unix.Fstatat(int(parent.Fd()), filepath.Base(from), &st, unix.AT_SYMLINK_NOFOLLOW); err != nil { return err } - if st.Mode&unix.S_IFMT != unix.S_IFDIR || fmt.Sprintf("%d:%d", st.Dev, st.Ino) != sourceID[0] { + if err := unix.Fstat(int(parent.Fd()), &parentStat); err != nil { + return err + } + if st.Mode&unix.S_IFMT != unix.S_IFDIR || !MatchPersistedDirectory(sourceID[0], fmt.Sprintf("%d:%d", st.Dev, st.Ino), fmt.Sprintf("%d", parentStat.Dev)) { return fmt.Errorf("native rename source inode changed") } } diff --git a/internal/installruntime/native_tree.go b/internal/installruntime/native_tree.go index a5003170d..393943805 100644 --- a/internal/installruntime/native_tree.go +++ b/internal/installruntime/native_tree.go @@ -95,7 +95,7 @@ func checkNativeParents(change *NativeChange) error { if err != nil { return err } - return checkAnchors(change.Parents, got) + return checkPersistedAnchors(change.Parents, got) } // Unused candidates still require ownership proof. Identical live bytes do not @@ -204,33 +204,41 @@ func removeFailedNativeStage(path string, expected []PathAnchor, id string) erro } func nativeDirectoryID(path string) (string, error) { - var err error - path, err = platformAnchorPath(path) + id, _, err := nativeDirectoryIdentity(path) + return id, err +} + +func nativeDirectoryIdentity(path string) (string, string, error) { + path, err := platformAnchorPath(path) if err != nil { - return "", err + return "", "", err } if path == "" { - return "", nil + return "", "", nil } anchors, err := pathAnchors(filepath.Join(path, ".identity"), false) if err != nil { - return "", err + return "", "", err } if len(anchors) == 0 || anchors[len(anchors)-1].Path != path { - return "", nil + return "", "", nil + } + parentDev := "" + if len(anchors) > 1 { + parentDev = objectDevice(anchors[len(anchors)-2].Identity) } - return anchors[len(anchors)-1].Identity, nil + return anchors[len(anchors)-1].Identity, parentDev, nil } func checkNativeDirectoryID(path, expected string) error { // Legacy records have byte identities only; new promotions always bind inodes. if expected == "" { return nil } - got, err := nativeDirectoryID(path) + got, parentDev, err := nativeDirectoryIdentity(path) if err != nil { return err } - if got != expected { + if !MatchPersistedDirectory(expected, got, parentDev) { return fmt.Errorf("native directory inode changed: %s", path) } return nil diff --git a/internal/installruntime/path_unix.go b/internal/installruntime/path_unix.go index 14959a718..8ec024436 100644 --- a/internal/installruntime/path_unix.go +++ b/internal/installruntime/path_unix.go @@ -206,7 +206,11 @@ func safeRemoveDirectory(path, want string, anchors []PathAnchor) error { if err != nil { return err } - if st.Mode&unix.S_IFMT != unix.S_IFDIR || fmt.Sprintf("%d:%d", st.Dev, st.Ino) != want { + var parentStat unix.Stat_t + if err := unix.Fstat(int(parent.Fd()), &parentStat); err != nil { + return err + } + if st.Mode&unix.S_IFMT != unix.S_IFDIR || !MatchPersistedDirectory(want, fmt.Sprintf("%d:%d", st.Dev, st.Ino), fmt.Sprintf("%d", parentStat.Dev)) { return fmt.Errorf("purge directory replaced: %s", path) } if err = unix.Unlinkat(int(parent.Fd()), filepath.Base(path), unix.AT_REMOVEDIR); err != nil { diff --git a/internal/installruntime/persisted_anchor_test.go b/internal/installruntime/persisted_anchor_test.go new file mode 100644 index 000000000..8a02e5450 --- /dev/null +++ b/internal/installruntime/persisted_anchor_test.go @@ -0,0 +1,126 @@ +//go:build linux || darwin + +package installruntime + +import ( + "bytes" + "fmt" + "os" + "path/filepath" + "runtime" + "testing" +) + +func TestPersistedIdentityFileRecoveryAnchors(t *testing.T) { + ctx, r := request(t) + target := filepath.Join(r.RuntimeRoot, "nested", "config") + r.Files = []File{{Path: target, Data: []byte("fixture publication"), Mode: 0600}} + r.Fault = func(phase string) error { + if phase == "transaction" { + return fmt.Errorf("fixture crash") + } + return nil + } + if _, err := Commit(ctx, r); err == nil { + t.Fatal("journal fault missing") + } + marker := filepath.Join(r.ControlRoot, "transaction.json") + tx, err := readTransactionFile(marker) + if err != nil { + t.Fatal(err) + } + mapping := map[string]string{} + for i := range tx.Files { + for j := range tx.Files[i].Parents { + a := &tx.Files[i].Parents[j] + dev, ino, ok := splitObjectID(a.Identity) + if !ok { + t.Fatal("not a Unix identity", a.Identity) + } + if mapping[dev] == "" { + mapping[dev] = fmt.Sprint(90000000 + len(mapping)) + } + a.Identity = mapping[dev] + ":" + ino + } + } + if err = writeTransaction(marker, tx); err != nil { + t.Fatal(err) + } + journal, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + r.Fault = nil + r.Files = nil + r.RecoverOnly = true + _, err = Commit(ctx, r) + if runtime.GOOS == "darwin" { + if err != nil { + t.Fatal("persisted device renumber recovery rejected", err) + } + got, err := os.ReadFile(target) + if err != nil || string(got) != "fixture publication" { + t.Fatal("publication not recovered", err) + } + if _, err = os.Stat(marker); !os.IsNotExist(err) { + t.Fatal("journal remains", err) + } + } else { + if err == nil { + t.Fatal("Linux relaxed persisted identity") + } + if _, err = os.Stat(target); !os.IsNotExist(err) { + t.Fatal("rejected recovery published a file", err) + } + got, err := os.ReadFile(marker) + if err != nil || !bytes.Equal(got, journal) { + t.Fatal("refusal rewrote journal", err) + } + } +} + +func TestPersistedIdentityObservedDirectory(t *testing.T) { + path := filepath.Join(t.TempDir(), "generation.app") + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + id, err := nativeDirectoryID(path) + if err != nil { + t.Fatal(err) + } + if err = checkNativeDirectoryID(path, id); err != nil { + t.Fatal(err) + } + dev, ino, ok := splitObjectID(id) + if !ok { + t.Fatal(id) + } + replacementDev := "999999" + if dev == replacementDev { + replacementDev = "888888" + } + err = checkNativeDirectoryID(path, replacementDev+":"+ino) + if (err == nil) != (runtime.GOOS == "darwin") { + t.Fatalf("%s observed identity policy: %v", runtime.GOOS, err) + } + // Same bytes cannot rescue an inode replacement on either platform. + saved := filepath.Join(filepath.Dir(path), "saved.app") + if err = os.Rename(path, saved); err != nil { + t.Fatal(err) + } + if err = os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + if checkNativeDirectoryID(path, replacementDev+":"+ino) == nil { + t.Fatal("replacement directory accepted") + } + if err = os.Remove(path); err != nil { + t.Fatal(err) + } + if err = os.Symlink(saved, path); err != nil { + t.Fatal(err) + } + if checkNativeDirectoryID(path, replacementDev+":"+ino) == nil { + t.Fatal("symlinked generation accepted") + } +} diff --git a/internal/installruntime/purge.go b/internal/installruntime/purge.go index 967cb48aa..fc25250e6 100644 --- a/internal/installruntime/purge.go +++ b/internal/installruntime/purge.go @@ -4,7 +4,6 @@ import ( "fmt" "os" "path/filepath" - "reflect" "sort" ) @@ -130,7 +129,11 @@ func cleanupPurgeTree(tree PurgeTree, fault func(string) error) error { return err } for rel, entry := range current { - if want, ok := tree.Entries[rel]; !ok || !reflect.DeepEqual(want, entry) { + parentDev, err := purgeParentDevice(filepath.Join(tree.Path, rel), entry.Directory != "") + if err != nil { + return err + } + if want, ok := tree.Entries[rel]; !ok || !matchPurgeEntry(want, entry, parentDev) { return fmt.Errorf("purge entry changed; preserving for inspection: %s", filepath.Join(tree.Path, rel)) } } @@ -146,10 +149,14 @@ func cleanupPurgeTree(tree PurgeTree, fault func(string) error) error { if err != nil { return err } - for _, anchor := range anchors { + for i, anchor := range anchors { sub, e := filepath.Rel(tree.Path, anchor.Path) if e == nil { - if want, ok := tree.Entries[sub]; ok && want.Directory != anchor.Identity { + parentDev := "" + if i > 0 { + parentDev = objectDevice(anchors[i-1].Identity) + } + if want, ok := tree.Entries[sub]; ok && !MatchPersistedDirectory(want.Directory, anchor.Identity, parentDev) { return fmt.Errorf("purge parent replaced: %s", anchor.Path) } } @@ -171,3 +178,33 @@ func cleanupPurgeTree(tree PurgeTree, fault func(string) error) error { } return nil } + +// Directories and regular objects share dev:ino on Unix, but file ownership +// additionally requires its exact content/mode fingerprint. Windows stays exact. +func matchPurgeEntry(want, got PurgeEntry, parentDev string) bool { + if want.File != got.File { + return false + } + if want.Directory != "" || got.Directory != "" { + return want.ObjectID == got.ObjectID && MatchPersistedDirectory(want.Directory, got.Directory, parentDev) + } + if !persistedDevRelaxed { + return want.ObjectID == got.ObjectID + } + return matchPersistedObject(want.ObjectID, got.ObjectID, parentDev) +} + +func purgeParentDevice(path string, directory bool) (string, error) { + if directory { + _, dev, err := nativeDirectoryIdentity(path) + return dev, err + } + anchors, err := pathAnchors(path, false) + if err != nil { + return "", err + } + if len(anchors) == 0 { + return "", fmt.Errorf("missing purge parent") + } + return objectDevice(anchors[len(anchors)-1].Identity), nil +} diff --git a/internal/installruntime/recovery.go b/internal/installruntime/recovery.go index 9c3989005..132ccf875 100644 --- a/internal/installruntime/recovery.go +++ b/internal/installruntime/recovery.go @@ -217,6 +217,24 @@ func checkPolicyGeneration(root string, l Ledger) error { // snapshots. It never restores an entire directory and never downgrades an // already promoted callback reader. A foreign edit makes rollback refuse. func reverseTransaction(current Ledger, tx transaction) (transaction, error) { + if tx.Native != nil && len(tx.Native.Parents) != 0 { + anchors, err := pathAnchors(tx.Native.After.Path, false) + if err != nil { + return transaction{}, err + } + if err := checkPersistedAnchors(tx.Native.Parents, anchors); err != nil { + return transaction{}, err + } + } + for _, f := range tx.Files { + anchors, err := pathAnchors(f.Path, false) + if err != nil { + return transaction{}, err + } + if err := checkPersistedAnchors(f.Parents, anchors); err != nil { + return transaction{}, err + } + } if tx.Native != nil && tx.Native.Retire { return transaction{}, fmt.Errorf("retirement deletion must finish forward recovery before rollback") } diff --git a/internal/installruntime/renumber_commit_test.go b/internal/installruntime/renumber_commit_test.go new file mode 100644 index 000000000..6b689ec2c --- /dev/null +++ b/internal/installruntime/renumber_commit_test.go @@ -0,0 +1,462 @@ +//go:build darwin + +package installruntime + +import ( + "encoding/json" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// Synthetic persisted dev numbers exercise real no-follow Darwin syscalls. No +// devices are mounted here; real volume boundaries are covered by macOS E2E. +func renumberID(id string) string { + dev, ino, ok := splitObjectID(id) + if !ok { + return id + } + // Deterministic injective mapping keeps the original volume grouping. + if strings.HasPrefix(dev, "-") { + return "8" + strings.TrimPrefix(dev, "-") + ":" + ino + } + return "9" + dev + ":" + ino +} +func renumberRecord(n *NativeRecord) { + if n == nil { + return + } + n.DirectoryID = renumberID(n.DirectoryID) + n.PreviousDirectoryID = renumberID(n.PreviousDirectoryID) + for i := range n.Published { + n.Published[i].DirectoryID = renumberID(n.Published[i].DirectoryID) + } +} +func rewriteRenumberLedger(t *testing.T, root string) Ledger { + t.Helper() + l, err := readLedger(root) + if err != nil { + t.Fatal(err) + } + renumberRecord(l.Native) + if err = writeJSON(filepath.Join(root, "ownership.json"), l); err != nil { + t.Fatal(err) + } + return l +} +func assertFreshNativeIDs(t *testing.T, l Ledger) { + t.Helper() + if l.Native == nil { + return + } + check := func(path, id string) { + if path == "" { + return + } + current, err := nativeDirectoryID(path) + if err != nil { + t.Fatal(err) + } + if current != id { + t.Fatalf("stale identity %s: %s want %s", path, id, current) + } + } + check(l.Native.Path, l.Native.DirectoryID) + check(l.Native.PreviousPath, l.Native.PreviousDirectoryID) + for _, g := range l.Native.Published { + check(g.Path, g.DirectoryID) + } +} +func TestRenumberCommitRefreshAndReselect(t *testing.T) { + ctx, r := request(t) + original := nativeFixture(t) + var err error + r.Native, err = StageNative(ctx, r.ControlRoot, original) + if err != nil { + t.Fatal(err) + } + r.Files, err = NativeAlias(r.Native, r.RuntimeRoot) + if err != nil { + t.Fatal(err) + } + first, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + firstPath := first.Native.Path + newer := nativeFixture(t) + asset := filepath.Join(newer, "Contents", "MacOS", "terminal-notifier-modern") + if err = os.WriteFile(asset, []byte("new inert callback"), 0755); err != nil { + t.Fatal(err) + } + rewriteRenumberLedger(t, r.ControlRoot) + r.Native, err = StageNative(ctx, r.ControlRoot, newer) + if err != nil { + t.Fatal(err) + } + r.Files, err = NativeAlias(r.Native, r.RuntimeRoot) + if err != nil { + t.Fatal(err) + } + second, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + assertFreshNativeIDs(t, second) + secondPath := second.Native.Path + if second.Native.PreviousPath != firstPath { + t.Fatal("previous callback path changed") + } + rewriteRenumberLedger(t, r.ControlRoot) + if _, err = ReadInstalledSnapshot(r.ControlRoot); err != nil { + t.Fatal(err) + } + aliasPath := filepath.Join(r.RuntimeRoot, "ClaudeNotifier.app") + aliasBefore, err := os.Readlink(aliasPath) + if err != nil { + t.Fatal(err) + } + r.Native = nil + r.Files = nil + unchanged, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + assertFreshNativeIDs(t, unchanged) + aliasAfter, err := os.Readlink(aliasPath) + if err != nil || aliasAfter != aliasBefore { + t.Fatal("identity refresh changed alias", err) + } + if unchanged.Native.Path != secondPath || unchanged.Native.PreviousPath != firstPath { + t.Fatal("refresh changed generation paths") + } + rewriteRenumberLedger(t, r.ControlRoot) + r.Native, err = StageNative(ctx, r.ControlRoot, original) + if err != nil { + t.Fatal(err) + } + r.Files, err = NativeAlias(r.Native, r.RuntimeRoot) + if err != nil { + t.Fatal(err) + } + selected, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + assertFreshNativeIDs(t, selected) + if selected.Native.Path != firstPath { + t.Fatalf("reselect changed published path: %s want %s", selected.Native.Path, firstPath) + } + rewriteRenumberLedger(t, r.ControlRoot) + r.Native = nil + r.Files = nil + r.RemoveConsumer = true + r.PurgeNative = true + purged, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + if purged.Native != nil { + t.Fatal("native not purged") + } + for _, path := range []string{firstPath, secondPath} { + if _, err = os.Stat(path); !os.IsNotExist(err) { + t.Fatalf("published callback remains: %s %v", path, err) + } + } +} + +func rewriteRenumberTransaction(t *testing.T, root string) transaction { + t.Helper() + marker := filepath.Join(root, "transaction.json") + tx, err := readTransactionFile(marker) + if err != nil { + t.Fatal(err) + } + renumberRecord(tx.Before.Native) + renumberRecord(tx.After.Native) + if n := tx.Native; n != nil { + renumberRecord(&n.Before) + renumberRecord(&n.After) + n.StagedID = renumberID(n.StagedID) + for i := range n.Parents { + n.Parents[i].Identity = renumberID(n.Parents[i].Identity) + } + for i := range n.PurgeTrees { + for rel, e := range n.PurgeTrees[i].Entries { + e.Directory = renumberID(e.Directory) + e.ObjectID = renumberID(e.ObjectID) + n.PurgeTrees[i].Entries[rel] = e + } + } + } + for i := range tx.Files { + for j := range tx.Files[i].Parents { + tx.Files[i].Parents[j].Identity = renumberID(tx.Files[i].Parents[j].Identity) + } + } + if err = writeTransaction(marker, tx); err != nil { + t.Fatal(err) + } + return tx +} + +// These faults prove retry idempotence after a refreshed ledger was durably +// written while transaction.json still contains the old device numbers. +func TestRenumberRecoveryLedgerRetry(t *testing.T) { + for _, mode := range []string{"forward", "forward-ledger", "rollback", "rollback-published", "purge", "purge-partial"} { + t.Run(mode, func(t *testing.T) { + ctx, r := request(t) + var err error + r.Native, err = StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + if _, err = Commit(ctx, r); err != nil { + t.Fatal(err) + } + if strings.HasPrefix(mode, "purge") { + r.Native = nil + r.RemoveConsumer = true + r.PurgeNative = true + } else { + source := nativeFixture(t) + if err = os.WriteFile(filepath.Join(source, "new"), []byte("new inert data"), 0600); err != nil { + t.Fatal(err) + } + r.Native, err = StageNative(ctx, r.ControlRoot, source) + if err != nil { + t.Fatal(err) + } + } + target := filepath.Join(r.RuntimeRoot, "config") + if !strings.HasPrefix(mode, "purge") { + r.Files = []File{{Path: target, Data: []byte("new"), Mode: 0600}} + } + r.Fault = func(phase string) error { + boundary := "transaction" + if mode == "forward-ledger" { + boundary = "ledger" + } + if mode == "rollback-published" { + boundary = "native" + } + if mode == "purge-partial" && strings.HasPrefix(phase, "purge-entry:") { + return fmt.Errorf("crash") + } + if mode != "purge-partial" && phase == boundary { + return fmt.Errorf("crash") + } + return nil + } + if _, err = Commit(ctx, r); err == nil { + t.Fatal("transaction fault missing") + } + tx := rewriteRenumberTransaction(t, r.ControlRoot) + rewriteRenumberLedger(t, r.ControlRoot) + marker := filepath.Join(r.ControlRoot, "transaction.json") + ledgerFault := func(phase string) error { + if phase == "ledger" { + return fmt.Errorf("ledger crash") + } + return nil + } + if strings.HasPrefix(mode, "rollback") { + r.RollbackPending = true + r.Fault = ledgerFault + if _, err = Commit(ctx, r); err == nil { + t.Fatal("rollback ledger fault missing") + } + tx, err = readTransactionFile(marker) + if err != nil { + t.Fatal(err) + } + } + journal, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + for attempt := 0; attempt < 2; attempt++ { + current, err := readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } + if err = recoverTransaction(ctx, r.ControlRoot, current, tx, ledgerFault); err == nil || err.Error() != "ledger crash" { + t.Fatalf("retry %d: %v", attempt, err) + } + after, err := readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } + assertFreshNativeIDs(t, after) + again, err := os.ReadFile(marker) + if err != nil || !reflect.DeepEqual(journal, again) { + t.Fatal("recovery rewrote journal before-images", err) + } + } + r.Fault = nil + r.Native = nil + r.Files = nil + r.RecoverOnly = true + result, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + assertFreshNativeIDs(t, result) + disk, err := readLedger(r.ControlRoot) + if err != nil || !reflect.DeepEqual(result, disk) { + t.Fatal("returned ledger differs from durable ledger", err) + } + if _, err = os.Stat(marker); !os.IsNotExist(err) { + t.Fatal("journal not retired", err) + } + if strings.HasPrefix(mode, "purge") && result.Native != nil { + t.Fatal("purge recovery retained native") + } + if strings.HasPrefix(mode, "forward") { + b, err := os.ReadFile(target) + if err != nil || string(b) != "new" { + t.Fatal("file promotion not recovered", err) + } + } + }) + } +} + +func TestRenumberJournalEqualityRejectsOtherChanges(t *testing.T) { + ctx, r := request(t) + var err error + r.Native, err = StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + l, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + data, err := json.Marshal(l) + if err != nil { + t.Fatal(err) + } + var snapshot Ledger + if err = json.Unmarshal(data, &snapshot); err != nil { + t.Fatal(err) + } + renumberRecord(snapshot.Native) + if !ledgerMatchesJournal(l, snapshot, nil) { + t.Fatal("refreshed after-image rejected") + } + snapshot.Enabled = !snapshot.Enabled + if ledgerMatchesJournal(l, snapshot, nil) { + t.Fatal("unrelated ledger edit accepted") + } +} + +func TestRenumberLedgerRefreshKeepsJournalAndMissingPaths(t *testing.T) { + root := t.TempDir() + path := filepath.Join(root, "published.app") + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + current, err := nativeDirectoryID(path) + if err != nil { + t.Fatal(err) + } + stored := renumberID(current) + missing := filepath.Join(root, "missing.app") + journal := Ledger{Native: &NativeRecord{Path: path, DirectoryID: stored, PreviousPath: path, PreviousDirectoryID: stored, Published: []NativeGeneration{{Path: path, DirectoryID: stored}, {Path: missing, DirectoryID: "123:456"}}}} + before, err := json.Marshal(journal) + if err != nil { + t.Fatal(err) + } + refreshed, err := refreshLedgerIdentities(journal) + if err != nil { + t.Fatal(err) + } + if refreshed.Native.DirectoryID != current || refreshed.Native.PreviousDirectoryID != current || refreshed.Native.Published[0].DirectoryID != current { + t.Fatal("not all present identities refreshed") + } + if refreshed.Native.Published[1].Path != missing || refreshed.Native.Published[1].DirectoryID != "123:456" { + t.Fatal("missing generation changed") + } + after, err := json.Marshal(journal) + if err != nil || !reflect.DeepEqual(before, after) { + t.Fatal("journal mutated during refresh", err) + } + if !ledgerMatchesJournal(refreshed, journal, nil) { + t.Fatal("refresh rejected its own after-image") + } + for _, published := range [][]NativeGeneration{nil, {}} { + l := Ledger{Native: &NativeRecord{Published: published}} + refreshed, err := refreshLedgerIdentities(l) + if err != nil || !reflect.DeepEqual(l, refreshed) { + t.Fatal("nil/empty publication changed", err) + } + } +} + +func TestRenumberRetiredInventoryRetry(t *testing.T) { + root := t.TempDir() + active := filepath.Join(root, "active.app") + retired := filepath.Join(root, ".candidate-retired") + for _, path := range []string{active, retired} { + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + } + aid, err := nativeDirectoryID(active) + if err != nil { + t.Fatal(err) + } + rid, err := nativeDirectoryID(retired) + if err != nil { + t.Fatal(err) + } + entries, err := capturePurgeTree(retired) + if err != nil { + t.Fatal(err) + } + journal := Ledger{Native: &NativeRecord{Path: active, DirectoryID: renumberID(aid), Published: []NativeGeneration{{Path: retired, DirectoryID: renumberID(rid)}}}} + current, err := refreshLedgerIdentities(journal) + if err != nil { + t.Fatal(err) + } + if err = os.Rename(retired, retired+"-saved"); err != nil { + t.Fatal(err) + } + if ledgerMatchesJournal(current, journal, nil) { + t.Fatal("unrecorded disappearance normalized") + } + cleanup := &NativeChange{Retire: true, PurgeTrees: []PurgeTree{{Path: retired, Entries: entries}}} + before, _ := json.Marshal(journal) + if !ledgerMatchesJournal(current, journal, cleanup) { + t.Fatal("refreshed/deleted private inventory rejected its after-image") + } + after, _ := json.Marshal(journal) + if !reflect.DeepEqual(before, after) { + t.Fatal("historical identity alignment changed journal") + } + _, ino, _ := splitObjectID(rid) + foreignDev := "777777" + if foreignDev == objectDevice(rid) { + foreignDev = "888888" + } + current.Native.Published[0].DirectoryID = foreignDev + ":" + ino + if ledgerMatchesJournal(current, journal, cleanup) { + t.Fatal("historical cross-volume identity accepted") + } + current.Native.Published[0].DirectoryID = "1:0" + if ledgerMatchesJournal(current, journal, cleanup) { + t.Fatal("historical inode substitution accepted") + } + current.Native.Published[0].DirectoryID = rid + if err = os.Mkdir(retired, 0700); err != nil { + t.Fatal(err) + } + if ledgerMatchesJournal(current, journal, cleanup) { + t.Fatal("replaced retired directory accepted") + } +} diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index 00d146915..4b1ccec13 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -10,7 +10,6 @@ import ( "fmt" "os" "path/filepath" - "reflect" "sort" "strings" ) @@ -374,14 +373,14 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { } if readErr == nil { if r.RollbackPending { - if !reflect.DeepEqual(l, pending.Before) && !reflect.DeepEqual(l, pending.After) { + if !ledgerMatchesJournal(l, pending.Before, pending.Native) && !ledgerMatchesJournal(l, pending.After, pending.Native) { return l, fmt.Errorf("rollback ledger mismatch") } if pending.Rollback { if e := recoverTransaction(ctx, root, l, pending, r.Fault); e != nil { return l, e } - return pending.After, nil + return readLedger(root) } reverse, e := reverseTransaction(l, pending) if e != nil { @@ -393,12 +392,15 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { if e = recoverTransaction(ctx, root, l, reverse, r.Fault); e != nil { return l, e } - return reverse.After, nil + return readLedger(root) } if err := recoverTransaction(ctx, root, l, pending, nil); err != nil { return l, err } - l = pending.After + l, err = readLedger(root) + if err != nil { + return l, err + } if r.RecoverOnly { return l, nil } @@ -778,6 +780,10 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { } } } + next, err = refreshLedgerIdentities(next) + if err != nil { + return l, err + } tx := transaction{Schema: transactionSchemaFor(next, r), Before: l, After: next, Files: files, Native: native, ConfigPaths: r.ConfigPaths} if err := writeTransaction(marker, tx); err != nil { return l, err @@ -790,10 +796,10 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { if err := recoverTransaction(ctx, root, l, tx, r.Fault); err != nil { return l, err } - return next, nil + return readLedger(root) } func recoverTransaction(ctx context.Context, root string, current Ledger, tx transaction, fault func(string) error) error { - if !reflect.DeepEqual(current, tx.Before) && !reflect.DeepEqual(current, tx.After) { + if !ledgerMatchesJournal(current, tx.Before, tx.Native) && !ledgerMatchesJournal(current, tx.After, tx.Native) { return fmt.Errorf("transaction ledger snapshot mismatch") } if current.Generation != tx.Before.Generation && current.Generation != tx.After.Generation { @@ -802,14 +808,16 @@ func recoverTransaction(ctx context.Context, root string, current Ledger, tx tra if current.ID != "" && current.ID != tx.After.ID { return fmt.Errorf("transaction owner mismatch") } - for _, f := range tx.Files { + tx.Files = append([]File(nil), tx.Files...) + for i, f := range tx.Files { anchors, e := pathAnchors(f.Path, false) if e != nil { return e } - if e = checkAnchors(f.Parents, anchors); e != nil { + if e = checkPersistedAnchors(f.Parents, anchors); e != nil { return e } + tx.Files[i].Parents = anchors got, err := replacementFingerprint(f) if err != nil { return err @@ -863,7 +871,11 @@ func recoverTransaction(ctx context.Context, root string, current Ledger, tx tra } } } - if err := writeJSON(filepath.Join(root, "ownership.json"), tx.After); err != nil { + after, err := refreshLedgerIdentities(tx.After) + if err != nil { + return err + } + if err := writeJSON(filepath.Join(root, "ownership.json"), after); err != nil { return err } if fault != nil { From bc0884d4c3bffd1c902c07d636f808abf2fb68b7 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 08:05:38 +0000 Subject: [PATCH 06/16] fix(install): preserve consent revocation and repeated reboot recovery Keep unverified native ownership unchanged for bounded policy revocation. Validate device correspondence for deleted retirement inventory across multiple boots and cover interrupted recovery. Refs #301 --- internal/installruntime/identity_refresh.go | 155 ++++- .../recovery_review_darwin_test.go | 594 ++++++++++++++++++ .../installruntime/renumber_commit_test.go | 6 +- internal/installruntime/transaction.go | 123 +++- 4 files changed, 846 insertions(+), 32 deletions(-) create mode 100644 internal/installruntime/recovery_review_darwin_test.go diff --git a/internal/installruntime/identity_refresh.go b/internal/installruntime/identity_refresh.go index cf7abbd29..6001b5a77 100644 --- a/internal/installruntime/identity_refresh.go +++ b/internal/installruntime/identity_refresh.go @@ -67,6 +67,20 @@ func ledgerMatchesJournal(current, snapshot Ledger, cleanup *NativeChange) bool if !persistedDevRelaxed { return false } + // Capture correspondence from ORIGINAL persisted IDs while live inodes can + // still prove it. Normalization would erase the intermediate boot's device. + var currentDevices, snapshotDevices deviceCorrespondence + if cleanup != nil && cleanup.Retire { + var ok bool + currentDevices, ok = nativeDeviceCorrespondence(current.Native) + if !ok { + return false + } + snapshotDevices, ok = nativeDeviceCorrespondence(snapshot.Native) + if !ok || !snapshotDevices.addAnchors(cleanup.Parents, cleanup.After.Path) { + return false + } + } current, err := refreshLedgerIdentities(current) if err != nil { return false @@ -76,17 +90,80 @@ func ledgerMatchesJournal(current, snapshot Ledger, cleanup *NativeChange) bool return false } if cleanup != nil && cleanup.Retire { - alignRetiredIdentities(current.Native, snapshot.Native, cleanup.PurgeTrees) + alignRetiredIdentities(current.Native, snapshot.Native, cleanup.PurgeTrees, currentDevices, snapshotDevices) } return reflect.DeepEqual(current, snapshot) } -// A legacy private candidate can also appear in the retained inventory. Recovery -// may refresh its ID, delete it, then crash before retiring the journal. There is -// no live inode left to normalize on retry. Reconcile only this historical field, -// using the durable deletion manifest, an absent leaf, the same inode and the -// current physical parent's device. An unrecorded disappearance cannot qualify. -func alignRetiredIdentities(current, snapshot *NativeRecord, trees []PurgeTree) { +// A correspondence is a bijection: both splits and merges of volume groups +// are refused. Its keys must come from surviving no-follow native observations +// or persisted parent anchors, never from the deleted inventory being reconciled. +type deviceCorrespondence map[string]string + +func (m deviceCorrespondence) add(stored, observed string) bool { + sd, si, sok := splitObjectID(stored) + od, oi, ook := splitObjectID(observed) + if !sok || !ook || si != oi { + return false + } + if mapped, ok := m[sd]; ok && mapped != od { + return false + } + for historical, mapped := range m { + if mapped == od && historical != sd { + return false + } + } + m[sd] = od + return true +} + +func (m deviceCorrespondence) addAnchors(stored []PathAnchor, path string) bool { + if len(stored) == 0 { + return true + } + observed, err := pathAnchors(path, false) + if err != nil || !matchPersistedAnchorChain(stored, observed) { + return false + } + for i, anchor := range stored { + if !m.add(anchor.Identity, observed[i].Identity) { + return false + } + } + return true +} + +func nativeDeviceCorrespondence(record *NativeRecord) (deviceCorrespondence, bool) { + m := deviceCorrespondence{} + if record == nil { + return m, true + } + observe := func(path, id string) bool { + if path == "" || id == "" { + return true + } + current, parentDev, err := nativeDirectoryIdentity(path) + if os.IsNotExist(err) || err == nil && current == "" { + return true + } + return err == nil && MatchPersistedDirectory(id, current, parentDev) && m.add(id, current) + } + if !observe(record.Path, record.DirectoryID) || !observe(record.PreviousPath, record.PreviousDirectoryID) { + return nil, false + } + for _, gen := range record.Published { + if !observe(gen.Path, gen.DirectoryID) { + return nil, false + } + } + return m, true +} + +// A legacy candidate can remain in Published after retirement. If deletion +// completed, there is no inode left to refresh. Only manifest-proven absence +// and device groups independently proven by survivors may align its IDs. +func alignRetiredIdentities(current, snapshot *NativeRecord, trees []PurgeTree, currentDevices, snapshotDevices deviceCorrespondence) { if current == nil || snapshot == nil || len(current.Published) != len(snapshot.Published) { return } @@ -95,24 +172,62 @@ func alignRetiredIdentities(current, snapshot *NativeRecord, trees []PurgeTree) if want.Path != got.Path || want.DirectoryID == got.DirectoryID || !strings.HasPrefix(filepath.Base(want.Path), ".candidate-") { continue } + wd, wi, wok := splitObjectID(want.DirectoryID) + gd, gi, gok := splitObjectID(got.DirectoryID) + if !wok || !gok || wi != gi { + continue + } + path, err := platformAnchorPath(want.Path) + if err != nil { + continue + } + anchors, err := pathAnchors(path, false) + if err != nil || len(anchors) == 0 || anchors[len(anchors)-1].Path != filepath.Dir(path) { + continue + } + parentDev := objectDevice(anchors[len(anchors)-1].Identity) + if parentDev == "" || currentDevices[gd] != parentDev || snapshotDevices[wd] != parentDev { + continue + } + if _, err := os.Lstat(path); !os.IsNotExist(err) { + continue + } for _, tree := range trees { entry, ok := tree.Entries["."] - if !ok || tree.Path != want.Path || entry.Directory == "" { + _, ino, valid := splitObjectID(entry.Directory) + if !ok || !valid || ino != wi || tree.Path != want.Path { continue } - if _, err := os.Lstat(want.Path); !os.IsNotExist(err) { - continue - } - path, err := platformAnchorPath(want.Path) - if err != nil { - continue - } - anchors, err := pathAnchors(path, false) - if err != nil || len(anchors) == 0 || anchors[len(anchors)-1].Path != filepath.Dir(path) { - continue + // The manifest may belong to either persisted boot. Every entry's + // device must have independent correspondence to this parent volume. + valid = true + for rel, entry := range tree.Entries { + if filepath.IsAbs(rel) || filepath.Clean(rel) != rel || rel == ".." || strings.HasPrefix(rel, ".."+string(filepath.Separator)) { + valid = false + break + } + id := entry.Directory + if id != "" { + if entry.ObjectID != "" || entry.File != (Identity{}) { + valid = false + break + } + } else { + id = entry.ObjectID + if rel == "." || !entry.File.Exists || entry.File.Link != "" { + valid = false + break + } + } + dev, _, ok := splitObjectID(id) + cd, cok := currentDevices[dev] + sd, sok := snapshotDevices[dev] + if !ok || (!cok && !sok) || (cok && cd != parentDev) || (sok && sd != parentDev) { + valid = false + break + } } - parentDev := objectDevice(anchors[len(anchors)-1].Identity) - if matchPersistedObject(want.DirectoryID, got.DirectoryID, parentDev) && matchPersistedObject(entry.Directory, got.DirectoryID, parentDev) { + if valid { snapshot.Published[i].DirectoryID = got.DirectoryID } } diff --git a/internal/installruntime/recovery_review_darwin_test.go b/internal/installruntime/recovery_review_darwin_test.go new file mode 100644 index 000000000..7facbbc16 --- /dev/null +++ b/internal/installruntime/recovery_review_darwin_test.go @@ -0,0 +1,594 @@ +//go:build darwin + +package installruntime + +import ( + "context" + "encoding/json" + "errors" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +// These fixtures never execute native code, mount volumes, or scan resources. +func reviewManagedNative(t *testing.T, channel string) (context.Context, Request, Ledger) { + t.Helper() + ctx, r := request(t) + if channel != "global" { + r.ConsumerID = channel + "-notifications" + } + r.Consumer.Registration = filepath.Join(r.ControlRoot, "receipt.json") + var err error + r.Native, err = StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + r.Files = []File{{Path: r.Consumer.Registration, Data: []byte("inert receipt"), Mode: 0600}} + l, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + enabled := true + r.Native, r.Files = nil, nil + r.RefreshOnly, r.PolicyOnly = true, true + r.ExpectedGeneration, r.PolicyEnabled = &l.Generation, &enabled + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"geminiNotifications":{"desktop":true,"webhook":true},"openCodeNotifications":{"desktop":true,"webhook":true},"foreign":{"keep":true}}`)} + l, err = Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.PolicyEnabled, r.PolicyFields = nil, nil + return ctx, r, l +} + +func reviewRevoke(t *testing.T, ctx context.Context, r Request, channel string) Request { + t.Helper() + s, err := ReadRevocationSnapshot(ctx, r.ControlRoot) + if err != nil { + t.Fatal(err) + } + r.ExpectedGeneration, r.ExpectedPolicy = &s.Generation, &s.Preimage + switch channel { + case "global": + off := false + r.PolicyEnabled = &off + case "gemini": + r.RevokeGemini = true + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"geminiNotifications":{"desktop":false,"webhook":false}}`)} + case "opencode": + r.RevokeOpenCode = true + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"openCodeNotifications":{"desktop":false,"webhook":false}}`)} + } + return r +} + +func reviewDamageNative(t *testing.T, path, damage string) func() { + t.Helper() + saved := path + "-saved" + if err := os.Rename(path, saved); err != nil { + t.Fatal(err) + } + if damage == "symlink" { + if err := os.Symlink(saved, path); err != nil { + t.Fatal(err) + } + } else { + // Identical contents, different inode: consent must not depend on a digest. + tree, err := openNativeRoot(saved) + if err != nil { + t.Fatal(err) + } + defer tree.Close() + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + id, err := nativeDirectoryID(path) + if err != nil { + t.Fatal(err) + } + if err := copyOpenedNativeTree(tree, path, id); err != nil { + t.Fatal(err) + } + } + return func() { + if err := os.RemoveAll(path); err != nil { + t.Fatal(err) + } + if err := os.Rename(saved, path); err != nil { + t.Fatal(err) + } + } +} + +func TestRecoveryReviewDamagedNativeRevocation(t *testing.T) { + for _, channel := range []string{"global", "gemini", "opencode"} { + for _, damage := range []string{"inode", "symlink"} { + for _, boundary := range []string{"none", "transaction", "policy", "ledger"} { + t.Run(channel+"/"+damage+"/"+boundary, func(t *testing.T) { + ctx, r, before := reviewManagedNative(t, channel) + restore := reviewDamageNative(t, before.Native.Path, damage) + r = reviewRevoke(t, ctx, r, channel) + crash := errors.New("review crash") + r.Fault = func(phase string) error { + if phase == boundary || boundary == "policy" && strings.HasPrefix(phase, "promotion:") { + return crash + } + return nil + } + result, err := Commit(ctx, r) + marker := filepath.Join(r.ControlRoot, "transaction.json") + if boundary != "none" { + if !errors.Is(err, crash) { + t.Fatalf("revocation failed before requested crash: %v", err) + } + journal, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + tx, err := readTransactionFile(marker) + if err != nil { + t.Fatal(err) + } + current, err := readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } + if err := recoverTransaction(ctx, r.ControlRoot, current, tx, func(phase string) error { + if phase == "ledger" { + return crash + } + return nil + }); !errors.Is(err, crash) { + t.Fatalf("policy replay depends on native availability: %v", err) + } + unchanged, err := os.ReadFile(marker) + if err != nil || !reflect.DeepEqual(journal, unchanged) { + t.Fatal("replay rewrote journal", err) + } + result, err = Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) + if err != nil { + t.Fatal(err) + } + } else if err != nil { + t.Fatalf("bounded revocation depends on native availability: %v", err) + } + expected := before + expected.Generation++ + expected.PolicyGeneration++ + if channel == "global" { + expected.Enabled = false + } + if !reflect.DeepEqual(result, expected) { + t.Fatal("revocation changed unverified native ownership or other ledger fields") + } + if _, err := os.Lstat(marker); !os.IsNotExist(err) { + t.Fatal("replay marker remains", err) + } + restore() + s, err := ReadPolicySnapshot(ctx, r.ControlRoot) + if err != nil { + t.Fatal(err) + } + var route map[string]map[string]bool + if err := json.Unmarshal(s.Fields["route"], &route); err != nil { + t.Fatal(err) + } + if channel == "global" { + if s.Policy.Enabled { + t.Fatal("restoration restored consent") + } + } else { + key := "geminiNotifications" + if channel == "opencode" { + key = "openCodeNotifications" + } + if route[key]["desktop"] || route[key]["webhook"] || !route["foreign"]["keep"] { + t.Fatal("revocation lost or foreign policy changed") + } + } + }) + } + } + } +} + +// Independently exercise replay's exception: the journal is recorded while +// native is healthy, then the inode/symlink substitution happens during downtime. +func TestRecoveryReviewPolicyReplayAfterNativeDamage(t *testing.T) { + for _, channel := range []string{"global", "gemini", "opencode"} { + for _, damage := range []string{"inode", "symlink"} { + t.Run(channel+"/"+damage, func(t *testing.T) { + ctx, r, before := reviewManagedNative(t, channel) + r = reviewRevoke(t, ctx, r, channel) + crash := errors.New("journal crash") + r.Fault = func(phase string) error { + if phase == "transaction" { + return crash + } + return nil + } + if _, err := Commit(ctx, r); !errors.Is(err, crash) { + t.Fatal("journal fixture", err) + } + reviewDamageNative(t, before.Native.Path, damage) + result, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) + if err != nil { + t.Fatalf("bounded policy replay requires native availability: %v", err) + } + expected := before + expected.Generation++ + expected.PolicyGeneration++ + if channel == "global" { + expected.Enabled = false + } + if !reflect.DeepEqual(result, expected) { + t.Fatal("policy replay adopted unverified native ownership") + } + }) + } + } +} + +// Native damage must not block the same bounded disable when the normal writer +// protocol upgrades a legacy ledger, or retains an existing pending reservation. +func TestRecoveryReviewDisableProtocolFloors(t *testing.T) { + for _, mode := range []string{"legacy", "reservation", "policy-drift"} { + t.Run(mode, func(t *testing.T) { + ctx, r, before := reviewManagedNative(t, "global") + if mode == "legacy" { + before.Schema = ledgerSchemaV1 + before.WriterFloor = 0 + } else if mode == "policy-drift" { + // A manual policy edit can disagree with the runtime fence. + // An explicit CAS-bound global disable still only revokes. + before.Enabled = false + if err := writeJSON(filepath.Join(r.ControlRoot, "policy-generation.json"), runtimePolicy{before.PolicyGeneration, false}); err != nil { + t.Fatal(err) + } + } else { + before.Schema = ledgerSchemaV3 + before.WriterFloor = ReservationWriterFloor + before.PendingMutation = &PendingMutation{ID: "inert reservation", Owner: r.Owner, IntentRef: filepath.Join(r.ControlRoot, "inert-intent")} + } + if err := writeJSON(filepath.Join(r.ControlRoot, "ownership.json"), before); err != nil { + t.Fatal(err) + } + reviewDamageNative(t, before.Native.Path, "inode") + r = reviewRevoke(t, ctx, r, "global") + result, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + expected := before + applyReservationProtocol(&expected, Request{}, before) + expected.Generation++ + expected.PolicyGeneration++ + expected.Enabled = false + if !reflect.DeepEqual(result, expected) { + t.Fatal("disable changed native/reservation or downgraded floors") + } + }) + } +} + +func TestRecoveryReviewRevocationFences(t *testing.T) { + for _, channel := range []string{"global", "gemini", "opencode"} { + t.Run(channel, func(t *testing.T) { + ctx, r, before := reviewManagedNative(t, channel) + reviewDamageNative(t, before.Native.Path, "inode") + r = reviewRevoke(t, ctx, r, channel) + for name, change := range map[string]func(*Request){ + "owner": func(r *Request) { r.Owner = "foreign" }, + "registration": func(r *Request) { r.RuntimeRoot = filepath.Join(r.RuntimeRoot, "foreign") }, + "generation": func(r *Request) { g := *r.ExpectedGeneration - 1; r.ExpectedGeneration = &g }, + "policy-cas": func(r *Request) { r.ExpectedPolicy = &Identity{Exists: true, SHA256: "foreign"} }, + "new-admission": func(r *Request) { + r.PolicyEnabled = nil + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"geminiNotifications":{"desktop":true,"webhook":true}}`)} + }, + "asset-write": func(r *Request) { + r.PolicyOnly = false + r.Files = []File{{Path: filepath.Join(r.RuntimeRoot, "foreign"), Data: []byte("foreign"), Mode: 0600}} + }, + } { + t.Run(name, func(t *testing.T) { + bad := r + change(&bad) + if _, err := Commit(ctx, bad); err == nil { + t.Fatal("broader mutation admitted") + } + l, err := readLedger(r.ControlRoot) + if err != nil || !reflect.DeepEqual(l, before) { + t.Fatal("refusal changed ledger", err) + } + got, err := Fingerprint(filepath.Join(r.ControlRoot, "agent-notifications.json")) + if err != nil || got != *r.ExpectedPolicy { + t.Fatal("refusal changed policy", err) + } + }) + } + before.WriterFloor = ReservationWriterFloor + 1 + if err := writeJSON(filepath.Join(r.ControlRoot, "ownership.json"), before); err != nil { + t.Fatal(err) + } + if _, err := Commit(ctx, r); err == nil { + t.Fatal("writer floor bypassed") + } + }) + } +} + +// Write an old A journal, recover on B until the final purge-entry deletion, +// then model a subsequent C boot by changing all surviving ledger IDs together. +// Disk inodes never change. The journal and its before-images remain untouched. +func TestRecoveryReviewRetirementSecondRenumber(t *testing.T) { + for _, mode := range []string{"same-boot", "second-renumber", "foreign-device", "inconsistent-map", "merged-group", "inode", "missing-manifest", "manifest-device", "manifest-entry-device", "replacement", "survivor", "no-survivor", "alias", "manifest-path", "manifest-escape", "inventory-field", "policy-field"} { + t.Run(mode, func(t *testing.T) { + ctx, r, l := reviewManagedNative(t, "global") + // The legacy candidate is inert and unpublished as a callback. Use the + // existing drain-check fixture seam; no OS process/resource scan is run. + oldDrain := nativeDrainCheck + nativeDrainCheck = func(context.Context, string) error { return nil } + t.Cleanup(func() { nativeDrainCheck = oldDrain }) + candidate := filepath.Join(filepath.Dir(l.Native.Path), ".candidate-retired.app") + if mode == "replacement" { + if err := os.Mkdir(candidate+"-replacement", 0700); err != nil { + t.Fatal(err) + } + } + if err := os.Mkdir(candidate, 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(candidate, "inert"), []byte("retired data"), 0600); err != nil { + t.Fatal(err) + } + rid, err := nativeDirectoryID(candidate) + if err != nil { + t.Fatal(err) + } + digest, err := treeFingerprint(candidate) + if err != nil { + t.Fatal(err) + } + l.Native.DecoderFloor = 1 + l.DecoderFloor = 1 + l.Native.PreviousPath, l.Native.PreviousDirectoryID, l.Native.PreviousSHA256 = candidate, rid, digest + l.Native.Published = append(l.Native.Published, NativeGeneration{Path: candidate, DirectoryID: rid, SHA256: digest, DecoderFloor: 1, InstalledTreeSHA256: installedNativeHash(digest, nil)}) + if err := writeJSON(filepath.Join(r.ControlRoot, "ownership.json"), l); err != nil { + t.Fatal(err) + } + r.Native = nil + r.Files = nil + r.PolicyOnly = false + r.RefreshOnly = false + r.RetireNative = true + r.ExpectedGeneration = &l.Generation + crash := errors.New("retirement crash") + r.Fault = func(phase string) error { + if phase == "transaction" { + return crash + } + return nil + } + if _, err := Commit(ctx, r); !errors.Is(err, crash) { + t.Fatal("retirement fixture", err) + } + tx := rewriteRenumberTransaction(t, r.ControlRoot) // A + rewriteRenumberLedger(t, r.ControlRoot) + marker := filepath.Join(r.ControlRoot, "transaction.json") + journal, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + current, err := readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } + err = recoverTransaction(ctx, r.ControlRoot, current, tx, func(phase string) error { + if phase == "purge-entry:"+candidate { + return crash + } + return nil + }) + if !errors.Is(err, crash) { + t.Fatal("did not crash after candidate deletion", err) + } + if _, err := os.Lstat(candidate); !os.IsNotExist(err) { + t.Fatal("candidate not deleted", err) + } + current, err = readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } // B + if mode != "same-boot" { + renumberRecord(current.Native) + } // B historical, disk is C + if mode != "same-boot" { // Distinct from A; both historical mappings need live evidence. + renumberRecord(current.Native) + } + index := len(current.Native.Published) - 1 + switch mode { + case "foreign-device": + _, ino, _ := splitObjectID(current.Native.Published[index].DirectoryID) + current.Native.Published[index].DirectoryID = "777777:" + ino + case "inconsistent-map": + current.Native.Published[0].DirectoryID = renumberID(current.Native.Published[0].DirectoryID) + case "merged-group": + i := len(tx.Native.Parents) - 1 + tx.Native.Parents[i].Identity = renumberID(tx.Native.Parents[i].Identity) + case "inode": + current.Native.Published[index].DirectoryID = objectDevice(current.Native.Published[index].DirectoryID) + ":0" + case "missing-manifest": + tx.Native.PurgeTrees = nil + case "manifest-device": + e := tx.Native.PurgeTrees[0].Entries["."] + _, ino, _ := splitObjectID(e.Directory) + e.Directory = "777777:" + ino + tx.Native.PurgeTrees[0].Entries["."] = e + case "manifest-entry-device": + e := tx.Native.PurgeTrees[0].Entries["inert"] + _, ino, _ := splitObjectID(e.ObjectID) + e.ObjectID = "777777:" + ino + tx.Native.PurgeTrees[0].Entries["inert"] = e + case "replacement": + if err := os.Rename(candidate+"-replacement", candidate); err != nil { + t.Fatal(err) + } + case "survivor": + reviewDamageNative(t, l.Native.Path, "inode") + case "no-survivor": + if err := os.Rename(l.Native.Path, l.Native.Path+"-saved"); err != nil { + t.Fatal(err) + } + case "manifest-path": + tx.Native.PurgeTrees[0].Path += "-foreign" + case "manifest-escape": + tx.Native.PurgeTrees[0].Entries["../escaped"] = tx.Native.PurgeTrees[0].Entries["inert"] + case "inventory-field": + current.Native.Published[index].SHA256 = "foreign" + case "alias": + parent := filepath.Dir(candidate) + if err := os.Rename(parent, parent+"-saved"); err != nil { + t.Fatal(err) + } + if err := os.Symlink(parent+"-saved", parent); err != nil { + t.Fatal(err) + } + case "policy-field": + current.Enabled = !current.Enabled + } + if err := writeJSON(filepath.Join(r.ControlRoot, "ownership.json"), current); err != nil { + t.Fatal(err) + } + currentBefore, _ := json.Marshal(current) + snapshotBefore, _ := json.Marshal(tx) + err = recoverTransaction(ctx, r.ControlRoot, current, tx, nil) + positive := mode == "same-boot" || mode == "second-renumber" + if positive && err != nil { + t.Fatalf("valid retirement replay refused: %v", err) + } + if !positive && err == nil { + t.Fatal("unsafe retirement replay accepted") + } + currentAfter, _ := json.Marshal(current) + snapshotAfter, _ := json.Marshal(tx) + if !reflect.DeepEqual(currentBefore, currentAfter) || !reflect.DeepEqual(snapshotBefore, snapshotAfter) { + t.Fatal("replay mutated supplied snapshots") + } + if positive { + result, err := readLedger(r.ControlRoot) + if err != nil { + t.Fatal(err) + } + if result.Generation != tx.After.Generation || result.Native.PreviousPath != "" { + t.Fatal("retirement not completed") + } + if _, err := os.Lstat(marker); !os.IsNotExist(err) { + t.Fatal("marker not retired", err) + } + } else { + unchanged, err := os.ReadFile(marker) + if err != nil || !reflect.DeepEqual(journal, unchanged) { + t.Fatal("refused recovery changed journal", err) + } + disk, err := readLedger(r.ControlRoot) + if err != nil || !reflect.DeepEqual(current, disk) { + t.Fatal("refused recovery wrote ledger", err) + } + } + }) + } +} + +func TestRecoveryReviewUnboundedPolicyReplay(t *testing.T) { + for _, mutation := range []string{"enable", "sibling", "rates", "asset", "consumer", "native", "floor", "generation", "rollback", "foreign-policy", "precision"} { + t.Run(mutation, func(t *testing.T) { + ctx, r, l := reviewManagedNative(t, "gemini") + if mutation == "precision" { + path := filepath.Join(r.ControlRoot, "agent-notifications.json") + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + t.Fatal(err) + } + fields["foreignCounter"] = json.RawMessage("9007199254740992") + if err := writeJSON(path, fields); err != nil { + t.Fatal(err) + } + } + r = reviewRevoke(t, ctx, r, "gemini") + r.Fault = func(phase string) error { + if phase == "transaction" { + return fmt.Errorf("crash") + } + return nil + } + if _, err := Commit(ctx, r); err == nil { + t.Fatal("crash missing") + } + tx, err := readTransactionFile(filepath.Join(r.ControlRoot, "transaction.json")) + if err != nil { + t.Fatal(err) + } + reviewDamageNative(t, l.Native.Path, "inode") + switch mutation { + case "enable", "sibling", "rates", "precision": + var fields map[string]json.RawMessage + if err := json.Unmarshal(tx.Files[0].Data, &fields); err != nil { + t.Fatal(err) + } + switch mutation { + case "rates": + fields["rates"] = json.RawMessage(`{"burst":1}`) + case "precision": + fields["foreignCounter"] = json.RawMessage("9007199254740993") + case "enable": + fields["route"] = json.RawMessage(`{"geminiNotifications":{"desktop":true,"webhook":true},"openCodeNotifications":{"desktop":true,"webhook":true},"foreign":{"keep":true}}`) + case "sibling": + fields["route"] = json.RawMessage(`{"geminiNotifications":{"desktop":false,"webhook":false},"openCodeNotifications":{"desktop":false,"webhook":false},"foreign":{"keep":true}}`) + } + tx.Files[0].Data, _ = json.Marshal(fields) + case "asset": + tx.Files = append(tx.Files, File{Path: filepath.Join(r.RuntimeRoot, "foreign"), Data: []byte("foreign"), Mode: 0600}) + case "consumer": + tx.After.Consumers["foreign"] = Consumer{RuntimeRoot: r.RuntimeRoot} + case "native": + n := *tx.After.Native + n.DirectoryID = "1:0" + tx.After.Native = &n + case "floor": + tx.After.WriterFloor = ReservationWriterFloor + 1 + case "generation": + tx.After.Generation++ + case "rollback": + tx.Rollback = true + case "foreign-policy": + if err := os.WriteFile(tx.Files[0].Path, []byte(`{"schemaVersion":1,"enabled":true,"foreign":true}`), 0600); err != nil { + t.Fatal(err) + } + } + policyBefore, err := Fingerprint(tx.Files[0].Path) + if err != nil { + t.Fatal(err) + } + if err := recoverTransaction(ctx, r.ControlRoot, l, tx, nil); err == nil { + t.Fatal("unbounded replay accepted damaged native") + } + got, err := Fingerprint(tx.Files[0].Path) + if err != nil || got != policyBefore { + t.Fatal("refused replay changed policy", err) + } + disk, err := readLedger(r.ControlRoot) + if err != nil || !reflect.DeepEqual(disk, l) { + t.Fatal("refused replay changed ledger", err) + } + }) + } +} diff --git a/internal/installruntime/renumber_commit_test.go b/internal/installruntime/renumber_commit_test.go index 6b689ec2c..5cf8a18bb 100644 --- a/internal/installruntime/renumber_commit_test.go +++ b/internal/installruntime/renumber_commit_test.go @@ -297,11 +297,7 @@ func TestRenumberRecoveryLedgerRetry(t *testing.T) { t.Fatal("recovery rewrote journal before-images", err) } } - r.Fault = nil - r.Native = nil - r.Files = nil - r.RecoverOnly = true - result, err := Commit(ctx, r) + result, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) if err != nil { t.Fatal(err) } diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index 4b1ccec13..8781819a0 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -1,6 +1,7 @@ package installruntime import ( + "bytes" "context" "crypto/rand" "crypto/sha256" @@ -10,6 +11,7 @@ import ( "fmt" "os" "path/filepath" + "reflect" "sort" "strings" ) @@ -780,11 +782,13 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { } } } - next, err = refreshLedgerIdentities(next) - if err != nil { - return l, err - } tx := transaction{Schema: transactionSchemaFor(next, r), Before: l, After: next, Files: files, Native: native, ConfigPaths: r.ConfigPaths} + if !boundedPolicyRevocation(root, tx) { + tx.After, err = refreshLedgerIdentities(next) + if err != nil { + return l, err + } + } if err := writeTransaction(marker, tx); err != nil { return l, err } @@ -799,6 +803,9 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { return readLedger(root) } func recoverTransaction(ctx context.Context, root string, current Ledger, tx transaction, fault func(string) error) error { + if current.WriterFloor > ReservationWriterFloor || tx.Before.WriterFloor > ReservationWriterFloor || tx.After.WriterFloor > ReservationWriterFloor { + return fmt.Errorf("installed writer floor requires a newer compatible kernel") + } if !ledgerMatchesJournal(current, tx.Before, tx.Native) && !ledgerMatchesJournal(current, tx.After, tx.Native) { return fmt.Errorf("transaction ledger snapshot mismatch") } @@ -826,6 +833,15 @@ func recoverTransaction(ctx context.Context, root string, current Ledger, tx tra return fmt.Errorf("recovery conflict; preserving foreign edit: %s", f.Path) } } + // A policy-only replay has no native promotion to validate. Check its + // detached after-image before publishing policy, unless the persisted + // mutation proves the same bounded revocation accepted by Commit. + preserveNative := boundedPolicyRevocation(root, tx) + if tx.Native == nil && !preserveNative { + if _, err := refreshLedgerIdentities(tx.After); err != nil { + return err + } + } if err := validateNative(tx.Native); err != nil { return err } @@ -871,9 +887,13 @@ func recoverTransaction(ctx context.Context, root string, current Ledger, tx tra } } } - after, err := refreshLedgerIdentities(tx.After) - if err != nil { - return err + after := tx.After + if !preserveNative { + var err error + after, err = refreshLedgerIdentities(after) + if err != nil { + return err + } } if err := writeJSON(filepath.Join(root, "ownership.json"), after); err != nil { return err @@ -898,3 +918,92 @@ func recoverTransaction(ctx context.Context, root string, current Ledger, tx tra } return syncDir(root) } + +// No request flags survive in the journal. Prove revocation from the complete +// ledger delta and the single policy file's CAS-bound before/after bytes instead. +// Unverified native ownership is retained verbatim; this grants no asset use. +func boundedPolicyRevocation(root string, tx transaction) bool { + before, after := tx.Before, tx.After + if tx.Rollback || tx.Native != nil || len(tx.Files) != 1 || before.ID == "" || before.Owner == "" || + before.Generation == 0 || after.Generation <= before.Generation || after.PolicyGeneration <= before.PolicyGeneration || + after.Generation != before.Generation+1 || after.PolicyGeneration != before.PolicyGeneration+1 || + before.WriterFloor > ReservationWriterFloor || len(before.Consumers) == 0 { + return false + } + physicalRoot, err := filepath.EvalSymlinks(root) + if err != nil { + return false + } + file := tx.Files[0] + if file.Path != filepath.Join(physicalRoot, "agent-notifications.json") || file.Remove || file.Link != "" || file.Mode != 0600 || + file.Before.Link != "" || (file.Before.Exists && identity(file.BeforeData, file.Before.Mode) != file.Before) { + return false + } + oldPolicy := UserPolicy{SchemaVersion: 1} + oldFields := map[string]json.RawMessage{} + if file.Before.Exists { + oldPolicy, oldFields, err = decodeUserPolicy(file.BeforeData) + if err != nil { + return false + } + } else if file.Before != (Identity{}) || len(file.BeforeData) != 0 { + return false + } + newPolicy, _, err := decodeUserPolicy(file.Data) + if err != nil || after.Enabled != newPolicy.Enabled { + return false + } + expected := before + // Retain the normal, monotonic protocol migration, including legacy + // ledgers and pending reservations. No other schema/floor delta qualifies. + applyReservationProtocol(&expected, Request{}, before) + expected.Generation, expected.PolicyGeneration, expected.Enabled = after.Generation, after.PolicyGeneration, after.Enabled + if !reflect.DeepEqual(expected, after) { + return false + } + // Global disable keeps every other policy member unchanged. Channel + // revocations keep global intent and require that consumer's registration. + patches := []string{"", "geminiNotifications", "openCodeNotifications"} + for _, channel := range patches { + if channel == "" && newPolicy.Enabled || channel != "" && (before.Enabled != oldPolicy.Enabled || newPolicy.Enabled != oldPolicy.Enabled || before.PendingMutation != nil) { + continue + } + registered := false + for id, consumer := range before.Consumers { + if !filepath.IsAbs(consumer.RuntimeRoot) || filepath.Clean(consumer.RuntimeRoot) != consumer.RuntimeRoot { + continue + } + if channel == "" || before.Owner == "existing-installer" && consumer.Registration != "" && + (channel == "geminiNotifications" && id == "gemini-notifications" || channel == "openCodeNotifications" && id == "opencode-notifications") { + registered = true + } + } + if !registered { + continue + } + fields := make(map[string]json.RawMessage, len(oldFields)) + for key, value := range oldFields { + fields[key] = value + } + if channel != "" { + patch := json.RawMessage(fmt.Sprintf(`{"%s":{"desktop":false,"webhook":false}}`, channel)) + if mergePolicyFields(fields, map[string]json.RawMessage{"route": patch}) != nil { + continue + } + } + want, err := policyFile(physicalRoot, newPolicy.Enabled, fields, file.Before) + if err != nil { + continue + } + // Object order and indentation are not policy changes. Strict bounded + // decoding above has already rejected duplicate/invalid JSON members. + var wanted, actual any + wantDecoder, actualDecoder := json.NewDecoder(bytes.NewReader(want.Data)), json.NewDecoder(bytes.NewReader(file.Data)) + wantDecoder.UseNumber() + actualDecoder.UseNumber() + if wantDecoder.Decode(&wanted) == nil && actualDecoder.Decode(&actual) == nil && reflect.DeepEqual(wanted, actual) { + return true + } + } + return false +} From a54060c21bfb6f6cf2855a8fba40390404f7d9fc Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 08:10:54 +0000 Subject: [PATCH 07/16] test(setup): handle ownership fixture close errors Refs #301 --- internal/agentnotify/setup/state_identity_test.go | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/internal/agentnotify/setup/state_identity_test.go b/internal/agentnotify/setup/state_identity_test.go index 6d6543f17..b6037b669 100644 --- a/internal/agentnotify/setup/state_identity_test.go +++ b/internal/agentnotify/setup/state_identity_test.go @@ -26,7 +26,7 @@ func TestPersistedIdentitySetupOwnership(t *testing.T) { if err != nil { t.Fatal(err) } - defer root.Close() + defer func() { _ = root.Close() }() state, err := mkdir(root, "state") if err != nil { t.Fatal(err) @@ -59,7 +59,9 @@ func TestPersistedIdentitySetupOwnership(t *testing.T) { t.Fatal(err) } } - child.Close() + if err = child.Close(); err != nil { + t.Fatal(err) + } } for _, name := range []string{"namespace", "journal.json", "lock"} { data := []byte("fixture") @@ -76,7 +78,9 @@ func TestPersistedIdentitySetupOwnership(t *testing.T) { if (err == nil) != (runtime.GOOS == "darwin") { t.Fatalf("%s ready marker: %v", runtime.GOOS, err) } - state.Close() + if err = state.Close(); err != nil { + t.Fatal(err) + } if err = os.Rename(filepath.Join(rootPath, "state"), filepath.Join(rootPath, "saved")); err != nil { t.Fatal(err) } @@ -84,7 +88,7 @@ func TestPersistedIdentitySetupOwnership(t *testing.T) { if err != nil { t.Fatal(err) } - defer replacement.Close() + defer func() { _ = replacement.Close() }() if matches(replacement, root, stored) == nil { t.Fatal("replacement setup state accepted") } From 7f88ef4fc2e64151a3839f2c1526635e0b7ec334 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 08:36:02 +0000 Subject: [PATCH 08/16] test(installruntime): use canonical registered runtime in revocation fixtures Refs #301 --- internal/installruntime/recovery_review_darwin_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/installruntime/recovery_review_darwin_test.go b/internal/installruntime/recovery_review_darwin_test.go index 7facbbc16..97a5fece0 100644 --- a/internal/installruntime/recovery_review_darwin_test.go +++ b/internal/installruntime/recovery_review_darwin_test.go @@ -32,6 +32,8 @@ func reviewManagedNative(t *testing.T, channel string) (context.Context, Request if err != nil { t.Fatal(err) } + // Commit canonicalizes runtime roots (for example /var -> /private/var on macOS). + r.RuntimeRoot = l.Consumers[r.ConsumerID].RuntimeRoot enabled := true r.Native, r.Files = nil, nil r.RefreshOnly, r.PolicyOnly = true, true From cfdc8b087ced6139cd151832e5f1148df34cf9c6 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 10:20:09 +0000 Subject: [PATCH 09/16] fix(installruntime): validate active native bytes before policy replay Refuse changed or missing active native content before journal publication, retaining bounded damaged-native revocation. Cover actual interrupted Commit and recovery. Refs #301 Refs #302 --- .../replay_native_content_test.go | 316 ++++++++++++++++++ internal/installruntime/transaction.go | 21 +- 2 files changed, 334 insertions(+), 3 deletions(-) create mode 100644 internal/installruntime/replay_native_content_test.go diff --git a/internal/installruntime/replay_native_content_test.go b/internal/installruntime/replay_native_content_test.go new file mode 100644 index 000000000..67fce7b7f --- /dev/null +++ b/internal/installruntime/replay_native_content_test.go @@ -0,0 +1,316 @@ +package installruntime + +import ( + "bytes" + "context" + "encoding/json" + "errors" + "os" + "path/filepath" + "reflect" + "testing" +) + +// Real durable journals must not authorize native bytes changed during downtime. +// The untouched-source repro enabled policy and removed the marker for both an +// in-place edit and a missing active directory (see task evidence). +func replayContentFixture(t *testing.T, native, predecessor bool) (context.Context, Request, Ledger) { + t.Helper() + skipUnsupportedNative(t) + sandbox, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + for key, dir := range map[string]string{"HOME": "home", "XDG_CONFIG_HOME": "config", "XDG_CACHE_HOME": "cache", "XDG_DATA_HOME": "data", "XDG_STATE_HOME": "state", "XDG_RUNTIME_DIR": "runtime", "TMPDIR": "tmp", "CLAUDE_CONFIG_DIR": "claude"} { + path := filepath.Join(sandbox, dir) + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + t.Setenv(key, path) + } + ctx, r := request(t) + r.ConsumerID = "gemini-notifications" + r.Consumer.Registration = filepath.Join(r.ControlRoot, "receipt.json") + config := filepath.Join(r.RuntimeRoot, "config.json") + r.ConfigPaths = []string{config} + r.Files = []File{{Path: r.Consumer.Registration, Data: []byte("inert receipt"), Mode: 0600}, {Path: config, Data: []byte(`{"foreign":"keep"}`), Mode: 0600}} + if native { + r.Native, err = StageNative(ctx, r.ControlRoot, nativeFixture(t)) + if err != nil { + t.Fatal(err) + } + } + l, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.RuntimeRoot = l.Consumers[r.ConsumerID].RuntimeRoot + r.Native, r.Files, r.ConfigPaths = nil, nil, nil + if predecessor { + source := nativeFixture(t) + if err := os.WriteFile(filepath.Join(source, "Contents", "MacOS", "terminal-notifier-modern"), []byte("inert upgraded callback"), 0755); err != nil { + t.Fatal(err) + } + r.Native, err = StageNative(ctx, r.ControlRoot, source) + if err != nil { + t.Fatal(err) + } + l, err = Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.Native = nil + } + // Register OpenCode too, so each bounded channel control has real ownership. + r.ConsumerID = "opencode-notifications" + l, err = Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.RefreshOnly, r.PolicyOnly = true, true + r.ExpectedGeneration = &l.Generation + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"geminiNotifications":{"desktop":true,"webhook":true},"openCodeNotifications":{"desktop":true,"webhook":true},"foreign":{"keep":true}}`)} + l, err = Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.PolicyFields = nil + return ctx, r, l +} + +func replayContentJournal(t *testing.T, ctx context.Context, r Request) transaction { + t.Helper() + crash := errors.New("interrupted after durable transaction") + r.Fault = func(phase string) error { + if phase == "transaction" { + return crash + } + return nil + } + if _, err := Commit(ctx, r); !errors.Is(err, crash) { + t.Fatalf("did not reach durable transaction fault: %v", err) + } + tx, err := readTransactionFile(filepath.Join(r.ControlRoot, "transaction.json")) + if err != nil { + t.Fatal(err) + } + if tx.Native != nil { + t.Fatal("fixture unexpectedly promotes native") + } + return tx +} + +func replayContentPreimages(t *testing.T, root, runtimeRoot string) map[string][]byte { + t.Helper() + images := map[string][]byte{} + for _, path := range []string{filepath.Join(root, "transaction.json"), filepath.Join(root, "ownership.json"), filepath.Join(root, "policy-generation.json"), filepath.Join(root, "agent-notifications.json"), filepath.Join(root, "receipt.json"), filepath.Join(runtimeRoot, "config.json")} { + data, err := os.ReadFile(path) + if err != nil { + t.Fatal(err) + } + images[path] = data + } + return images +} + +func replayContentAssertRetained(t *testing.T, images map[string][]byte) { + t.Helper() + for path, before := range images { + after, err := os.ReadFile(path) + if err != nil || !bytes.Equal(before, after) { + t.Errorf("refusal changed or removed %s: %v", path, err) + } + } +} + +func replayContentDamage(t *testing.T, record *NativeRecord, damage string) { + t.Helper() + switch damage { + case "bytes": + asset := filepath.Join(record.Path, "Contents", "MacOS", "terminal-notifier-modern") + before, err := os.Stat(asset) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(asset, []byte("foreign same-inode native bytes"), before.Mode().Perm()); err != nil { + t.Fatal(err) + } + after, err := os.Stat(asset) + if err != nil || !os.SameFile(before, after) { + t.Fatal("fixture replaced the file inode", err) + } + id, err := nativeDirectoryID(record.Path) + if err != nil || id != record.DirectoryID { + t.Fatal("fixture replaced the native directory inode", err) + } + digest, err := treeFingerprint(record.Path) + if err != nil || digest == record.SHA256 { + t.Fatal("fixture did not change native bytes", err) + } + case "missing", "inode", "symlink": + saved := record.Path + "-saved" + if err := os.Rename(record.Path, saved); err != nil { + t.Fatal(err) + } + if damage == "inode" { + if err := os.Mkdir(record.Path, 0700); err != nil { + t.Fatal(err) + } + if err := copyNativeTree(saved, record.Path); err != nil { + t.Fatal(err) + } + digest, err := treeFingerprint(record.Path) + if err != nil || digest != record.SHA256 { + t.Fatal("replacement does not have identical bytes", err) + } + id, err := nativeDirectoryID(record.Path) + if err != nil || id == record.DirectoryID { + t.Fatal("replacement has original inode", err) + } + } else if damage == "symlink" { + if err := os.Symlink(saved, record.Path); err != nil { + t.Fatal(err) + } + } else if _, err := os.Lstat(record.Path); !os.IsNotExist(err) { + t.Fatal("active path still exists", err) + } + default: + t.Fatal("unknown fixture damage") + } +} + +func TestReplayNativeContentRefusesChangedActive(t *testing.T) { + for _, mutation := range []string{"enable", "admission"} { + for _, damage := range []string{"bytes", "missing", "inode", "symlink"} { + t.Run(mutation+"/"+damage, func(t *testing.T) { + ctx, r, before := replayContentFixture(t, true, false) + r.ExpectedGeneration = &before.Generation + if mutation == "enable" { + on := true + r.PolicyEnabled = &on + } else { + on := true + r.PolicyEnabled = &on + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"geminiNotifications":{"desktop":false,"webhook":false}}`)} + var err error + before, err = Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.ExpectedGeneration, r.PolicyEnabled = &before.Generation, nil + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"geminiNotifications":{"desktop":true,"webhook":false}}`)} + } + tx := replayContentJournal(t, ctx, r) + if reflect.DeepEqual(tx.Before, tx.After) { + t.Fatal("journal has no mutation") + } + preimages := replayContentPreimages(t, r.ControlRoot, r.RuntimeRoot) + replayContentDamage(t, before.Native, damage) + // Retry must continue to refuse, retaining the original decision and bytes. + for attempt := 0; attempt < 2; attempt++ { + _, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) + if err == nil { + t.Errorf("RecoverOnly accepted %s active native during %s replay", damage, mutation) + } + replayContentAssertRetained(t, preimages) + } + }) + } + } +} + +func TestReplayNativeContentValidControls(t *testing.T) { + for _, mode := range []string{"active", "no-native", "missing-retired"} { + t.Run(mode, func(t *testing.T) { + ctx, r, before := replayContentFixture(t, mode != "no-native", mode == "missing-retired") + on := true + r.PolicyEnabled, r.ExpectedGeneration = &on, &before.Generation + tx := replayContentJournal(t, ctx, r) + if mode == "missing-retired" { + if before.Native.PreviousPath == "" { + t.Fatal("fixture lacks predecessor") + } + if err := os.Rename(before.Native.PreviousPath, before.Native.PreviousPath+"-saved"); err != nil { + t.Fatal(err) + } + } + after, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) + if err != nil { + t.Fatal("valid policy replay refused", err) + } + if !reflect.DeepEqual(after, tx.After) { + t.Fatal("replay changed the journal's ledger intent") + } + policy, err := ReadUserPolicy(r.ControlRoot) + if err != nil || !policy.Enabled { + t.Fatal("enable not published", err) + } + if _, err := os.Lstat(filepath.Join(r.ControlRoot, "transaction.json")); !os.IsNotExist(err) { + t.Fatal("completed marker retained", err) + } + snapshot, err := ReadInstalledSnapshot(r.ControlRoot) + if err != nil || !snapshot.Enabled || snapshot.Recovery { + t.Fatal("valid replay did not restore admission", err) + } + }) + } +} + +func TestReplayNativeContentBoundedRevocation(t *testing.T) { + for _, channel := range []string{"global", "gemini", "opencode"} { + for _, damage := range []string{"bytes", "missing", "inode", "symlink"} { + t.Run(channel+"/"+damage, func(t *testing.T) { + ctx, r, before := replayContentFixture(t, true, false) + on := true + r.PolicyEnabled, r.ExpectedGeneration = &on, &before.Generation + before, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.PolicyEnabled = nil + s, err := ReadRevocationSnapshot(ctx, r.ControlRoot) + if err != nil { + t.Fatal(err) + } + r.ExpectedGeneration, r.ExpectedPolicy = &s.Generation, &s.Preimage + switch channel { + case "global": + off := false + r.PolicyEnabled = &off + case "gemini": + r.ConsumerID = "gemini-notifications" + r.RevokeGemini = true + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"geminiNotifications":{"desktop":false,"webhook":false}}`)} + case "opencode": + r.RevokeOpenCode = true + r.PolicyFields = map[string]json.RawMessage{"route": json.RawMessage(`{"openCodeNotifications":{"desktop":false,"webhook":false}}`)} + } + tx := replayContentJournal(t, ctx, r) + preimages := replayContentPreimages(t, r.ControlRoot, r.RuntimeRoot) + replayContentDamage(t, before.Native, damage) + after, err := Commit(ctx, Request{ControlRoot: r.ControlRoot, RecoverOnly: true}) + if err != nil { + t.Fatal("damaged native blocked bounded revocation", err) + } + if !reflect.DeepEqual(after, tx.After) || !reflect.DeepEqual(after.Native, before.Native) { + t.Fatal("revocation changed unverified native ownership or ledger intent") + } + for _, file := range tx.Files { + data, err := os.ReadFile(file.Path) + if err != nil || !bytes.Equal(data, file.Data) { + t.Fatal("bounded policy patch not published exactly", err) + } + } + for _, path := range []string{filepath.Join(r.ControlRoot, "receipt.json"), filepath.Join(r.RuntimeRoot, "config.json")} { + data, err := os.ReadFile(path) + if err != nil || !bytes.Equal(data, preimages[path]) { + t.Fatal("revocation changed unrelated assets/config", err) + } + } + if _, err := os.Lstat(filepath.Join(r.ControlRoot, "transaction.json")); !os.IsNotExist(err) { + t.Fatal("completed revoke marker retained", err) + } + }) + } + } +} diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index 8781819a0..6eae5b782 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -833,11 +833,26 @@ func recoverTransaction(ctx context.Context, root string, current Ledger, tx tra return fmt.Errorf("recovery conflict; preserving foreign edit: %s", f.Path) } } - // A policy-only replay has no native promotion to validate. Check its - // detached after-image before publishing policy, unless the persisted - // mutation proves the same bounded revocation accepted by Commit. + // Without a native promotion, replay must still prove the active callback + // before publishing policy. Identity refresh permits missing paths and does + // not check bytes. Only a proven bounded revocation may bypass this. preserveNative := boundedPolicyRevocation(root, tx) if tx.Native == nil && !preserveNative { + if native := tx.After.Native; native != nil { + if err := validateNativeRecord(native); err != nil { + return err + } + if err := checkNativeDirectoryID(native.Path, native.DirectoryID); err != nil { + return err + } + digest, err := treeFingerprint(native.Path) + if err != nil { + return err + } + if digest == "" || digest != native.SHA256 { + return fmt.Errorf("native live fingerprint conflict") + } + } if _, err := refreshLedgerIdentities(tx.After); err != nil { return err } From 7fbdebd9b7a29c9a50aa21a44c04f7258bc8b142 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 10:22:01 +0000 Subject: [PATCH 10/16] fix(installer): print durable native path for unchanged refresh Resolve and validate the retained control-root snapshot without adoption or mutation, preserving silent default refresh and output errors. Refs #301 Refs #302 --- cmd/claude-notifications/install_runtime.go | 47 +++- .../install_runtime_path_test.go | 265 +++++++++++++++++- 2 files changed, 305 insertions(+), 7 deletions(-) diff --git a/cmd/claude-notifications/install_runtime.go b/cmd/claude-notifications/install_runtime.go index c4a0e1287..1f9098fa4 100644 --- a/cmd/claude-notifications/install_runtime.go +++ b/cmd/claude-notifications/install_runtime.go @@ -117,6 +117,37 @@ func installRuntime(args []string, output io.Writer) error { } files = changed if len(files) == 0 { + if *printNativePath { + root := *control + if root == "" { + root, err = installruntime.ControlRoot() + if err != nil { + return err + } + } + // Observe retained ownership without committing or adopting utilities. + snapshot, err := installruntime.ReadInstalledSnapshot(root) + if err != nil { + return err + } + if snapshot.Recovery { + return installruntime.ErrPolicyRecovery + } + ledger := snapshot.Ledger + if ledger.ID != "" && ledger.Owner != "existing-installer" { + return fmt.Errorf("component owned by %s at %s; explicit takeover required", ledger.Owner, ledger.RuntimeRoot) + } + if ledger.Native != nil { + root, err = installruntime.CanonicalPath(root) + if err != nil { + return err + } + if ledger.Native.Path != filepath.Clean(ledger.Native.Path) || filepath.Dir(ledger.Native.Path) != filepath.Join(root, "native") { + return fmt.Errorf("native owner outside persistent directory") + } + } + return printRuntimeNativePath(output, ledger) + } return nil } } @@ -296,12 +327,7 @@ func installRuntime(args []string, output io.Writer) error { ledger, err := installruntime.Commit(ctx, req) if err == nil { if *printNativePath { - path := "" - if ledger.Native != nil { - path = ledger.Native.Path - } - _, err = fmt.Fprintln(output, path) - return err + return printRuntimeNativePath(output, ledger) } _, _ = fmt.Fprintf(output, "managed-runtime committed generation=%d\n", ledger.Generation) if *purge { @@ -310,3 +336,12 @@ func installRuntime(args []string, output io.Writer) error { } return err } + +func printRuntimeNativePath(output io.Writer, ledger installruntime.Ledger) error { + path := "" + if ledger.Native != nil { + path = ledger.Native.Path + } + _, err := fmt.Fprintln(output, path) + return err +} diff --git a/cmd/claude-notifications/install_runtime_path_test.go b/cmd/claude-notifications/install_runtime_path_test.go index e5745e843..71800eda1 100644 --- a/cmd/claude-notifications/install_runtime_path_test.go +++ b/cmd/claude-notifications/install_runtime_path_test.go @@ -3,6 +3,8 @@ package main import ( "bytes" "encoding/json" + "errors" + "io" "os" "path/filepath" "runtime" @@ -20,7 +22,7 @@ func TestInstallRuntimePrintNativePath(t *testing.T) { if native && runtime.GOOS != "linux" && runtime.GOOS != "darwin" { t.Skip("native staging is Unix-only") } - root := t.TempDir() + root := isolatedRuntimePathRoot(t) stage := filepath.Join(root, "stage") target := filepath.Join(root, "runtime", "bin") control := filepath.Join(root, "control") @@ -86,3 +88,264 @@ func TestInstallRuntimePrintNativePath(t *testing.T) { }) } } + +// Keep every environment-derived path physical and private, including the +// default control root: it is shared state, independent of --target. +func isolatedRuntimePathRoot(t *testing.T) string { + t.Helper() + root, err := filepath.EvalSymlinks(t.TempDir()) + if err != nil { + t.Fatal(err) + } + for _, name := range []string{"HOME", "USERPROFILE", "APPDATA", "LOCALAPPDATA", "XDG_CONFIG_HOME", "XDG_CACHE_HOME", "XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_RUNTIME_DIR", "TMPDIR", "TMP", "TEMP", "CLAUDE_CONFIG_DIR"} { + path := filepath.Join(root, name) + if err := os.Mkdir(path, 0700); err != nil { + t.Fatal(err) + } + t.Setenv(name, path) + } + return root +} + +type runtimePathFixture struct { + root, stage, target, control string + ledger installruntime.Ledger + args []string +} + +func newRuntimePathFixture(t *testing.T, state string, explicitControl bool) runtimePathFixture { + t.Helper() + if state == "native" && runtime.GOOS != "linux" && runtime.GOOS != "darwin" { + t.Skip("native staging is Unix-only") + } + f := runtimePathFixture{root: isolatedRuntimePathRoot(t)} + f.stage = filepath.Join(f.root, "stage") + f.target = filepath.Join(f.root, "runtime", "bin") + var err error + f.control, err = installruntime.ControlRoot() + if err != nil { + t.Fatal(err) + } + f.control, err = installruntime.CanonicalPath(f.control) + if err != nil { + t.Fatal(err) + } + if explicitControl { + f.control = filepath.Join(f.root, "separate-control") + } + for _, path := range []string{f.stage, f.target} { + if err := os.MkdirAll(path, 0700); err != nil { + t.Fatal(err) + } + } + writeRuntimePathFile(t, filepath.Join(f.stage, "sound-preview"), []byte("inert utility"), 0700) + f.args = []string{"--stage", f.stage, "--target", f.target} + if explicitControl { + f.args = append(f.args, "--control-root", f.control) + } + if state == "native" { + writeRuntimePathFile(t, filepath.Join(f.stage, "claude-notifications-linux-amd64"), []byte("inert "+installruntime.WriterProtocolMarker), 0700) + // Unknown retained helpers remain compatible; nothing executes this fixture. + writeRuntimePathFile(t, filepath.Join(f.stage, "ClaudeNotifier.app", "Contents", "MacOS", "terminal-notifier-modern"), []byte("inert native"), 0700) + } + if state == "absent" { + writeRuntimePathFile(t, filepath.Join(f.target, "sound-preview"), []byte("inert utility"), 0700) + } else { + if err := installRuntime(f.args, io.Discard); err != nil { + t.Fatal(err) + } + // Preserve explicit user policy and foreign fields byte for byte. + writeRuntimePathFile(t, filepath.Join(f.control, "agent-notifications.json"), []byte(`{"schemaVersion":1,"enabled":false,"userChoice":"keep"}`+"\n"), 0600) + snapshot, err := installruntime.ReadInstalledSnapshot(f.control) + if err != nil || snapshot.Recovery || snapshot.Ledger.ID == "" { + t.Fatalf("invalid fixture snapshot: %+v, %v", snapshot, err) + } + f.ledger = snapshot.Ledger + if state == "native" && (f.ledger.Native == nil || f.ledger.Native.Path == "") { + t.Fatal("fixture has no durable native generation") + } + } + if state == "native" { + if err := os.Remove(filepath.Join(f.stage, "claude-notifications-linux-amd64")); err != nil { + t.Fatal(err) + } + } + // An unchanged utility absent from ownership must not be adopted by refresh. + for _, path := range []string{f.stage, f.target} { + writeRuntimePathFile(t, filepath.Join(path, "list-sounds"), []byte("unmanaged utility"), 0700) + } + f.args = append(f.args, "--refresh") // intentionally no --entry + return f +} + +func writeRuntimePathFile(t *testing.T, path string, data []byte, mode os.FileMode) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(path, data, mode); err != nil { + t.Fatal(err) + } +} + +type runtimePathState struct { + info os.FileInfo + identity installruntime.Identity +} + +func runtimePathTree(t *testing.T, root string) map[string]runtimePathState { + t.Helper() + state := map[string]runtimePathState{} + if err := filepath.WalkDir(root, func(path string, entry os.DirEntry, err error) error { + if err != nil { + return err + } + info, err := entry.Info() + if err != nil { + return err + } + var id installruntime.Identity + if !entry.IsDir() { + id, err = installruntime.Fingerprint(path) + if err != nil { + return err + } + } + state[path] = runtimePathState{info, id} + return nil + }); err != nil { + t.Fatal(err) + } + return state +} + +func assertRuntimePathTree(t *testing.T, root string, before map[string]runtimePathState) { + t.Helper() + after := runtimePathTree(t, root) + if len(before) != len(after) { + t.Errorf("refresh changed tree entries: before=%d after=%d", len(before), len(after)) + } + for path, want := range before { + got, ok := after[path] + if !ok || !os.SameFile(want.info, got.info) || want.info.Mode() != got.info.Mode() || + !want.info.ModTime().Equal(got.info.ModTime()) || want.identity != got.identity { + t.Errorf("refresh mutated or replaced %s", path) + } + } +} + +func TestInstallRuntimeRefreshPrintNativePath(t *testing.T) { + for _, state := range []string{"native", "no-native", "absent"} { + for _, explicit := range []bool{false, true} { + name := state + map[bool]string{false: "/default-control", true: "/explicit-control"}[explicit] + t.Run(name, func(t *testing.T) { + f := newRuntimePathFixture(t, state, explicit) + before := runtimePathTree(t, f.root) + var output bytes.Buffer + if err := installRuntime(f.args, &output); err != nil { + t.Fatal(err) + } + if output.Len() != 0 { + t.Errorf("default no-op refresh stdout=%q", output.String()) + } + assertRuntimePathTree(t, f.root, before) + if err := installRuntime(append(f.args, "--print-native-path"), &output); err != nil { + t.Fatal(err) + } + want := "\n" + if f.ledger.Native != nil { + want = f.ledger.Native.Path + "\n" + } + if output.String() != want { + t.Errorf("no-op refresh stdout=%q want %q", output.String(), want) + } + assertRuntimePathTree(t, f.root, before) + }) + } + } +} + +func TestInstallRuntimeRefreshPrintNativePathRefusesInvalidState(t *testing.T) { + for _, damage := range []string{"malformed-ledger", "ownership-read-error", "foreign-owner", "foreign-native-root", "pending-transaction", "policy-generation", "native-content", "native-inode", "native-missing", "native-symlink", "native-binding"} { + t.Run(damage, func(t *testing.T) { + f := newRuntimePathFixture(t, "native", true) + ownership := filepath.Join(f.control, "ownership.json") + writeLedger := func() { + data, err := json.Marshal(f.ledger) + if err != nil { + t.Fatal(err) + } + writeRuntimePathFile(t, ownership, data, 0600) + } + switch damage { + case "malformed-ledger": + writeRuntimePathFile(t, ownership, []byte("{"), 0600) + case "ownership-read-error": + if err := os.Remove(ownership); err != nil { + t.Fatal(err) + } + if err := os.Mkdir(ownership, 0700); err != nil { + t.Fatal(err) + } + case "foreign-owner": + f.ledger.Owner = "foreign-installer" + writeLedger() + case "foreign-native-root": + // A structurally valid snapshot from another control root is not + // this component's durable generation, even when hashes match. + other := newRuntimePathFixture(t, "native", true) + f.ledger.Native = other.ledger.Native + writeLedger() + case "pending-transaction": + writeRuntimePathFile(t, filepath.Join(f.control, "transaction.json"), []byte("pending, do not recover"), 0600) + case "policy-generation": + f.ledger.PolicyGeneration++ + writeLedger() + case "native-content": + writeRuntimePathFile(t, filepath.Join(f.ledger.Native.Path, "Contents", "MacOS", "terminal-notifier-modern"), []byte("foreign native"), 0700) + case "native-inode", "native-missing", "native-symlink": + old := f.ledger.Native.Path + ".retained" + if err := os.Rename(f.ledger.Native.Path, old); err != nil { + t.Fatal(err) + } + if damage == "native-inode" { + writeRuntimePathFile(t, filepath.Join(f.ledger.Native.Path, "Contents", "MacOS", "terminal-notifier-modern"), []byte("inert native"), 0700) + } else if damage == "native-symlink" { + if err := os.Symlink(old, f.ledger.Native.Path); err != nil { + t.Fatal(err) + } + } + case "native-binding": + f.ledger.Native.InstalledTreeSHA256 = "invalid" + writeLedger() + } + before := runtimePathTree(t, f.root) + var output bytes.Buffer + // Default no-op refresh stays silent and does not inspect/adopt state. + if err := installRuntime(f.args, &output); err != nil || output.Len() != 0 { + t.Fatalf("default no-op refresh changed: %q, %v", output.String(), err) + } + if err := installRuntime(append(f.args, "--print-native-path"), &output); err == nil { + t.Error("invalid installed state reported success") + } + if output.Len() != 0 { + t.Errorf("invalid native path printed: %q", output.String()) + } + assertRuntimePathTree(t, f.root, before) + }) + } +} + +type runtimePathErrorWriter struct{ err error } + +func (w runtimePathErrorWriter) Write([]byte) (int, error) { return 0, w.err } + +func TestInstallRuntimeRefreshPrintNativePathOutputError(t *testing.T) { + f := newRuntimePathFixture(t, "no-native", true) + before := runtimePathTree(t, f.root) + want := errors.New("output failed") + if err := installRuntime(append(f.args, "--print-native-path"), runtimePathErrorWriter{want}); !errors.Is(err, want) { + t.Errorf("output error=%v want %v", err, want) + } + assertRuntimePathTree(t, f.root, before) +} From 68ffd4bbe9731fdd1f5bbb3a2b0aeee978e9dbd6 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 10:58:09 +0000 Subject: [PATCH 11/16] test(install): capture fresh path identities across platforms Refs #301 Refs #302 --- cmd/claude-notifications/install_runtime_path_test.go | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/cmd/claude-notifications/install_runtime_path_test.go b/cmd/claude-notifications/install_runtime_path_test.go index 71800eda1..15b6b505f 100644 --- a/cmd/claude-notifications/install_runtime_path_test.go +++ b/cmd/claude-notifications/install_runtime_path_test.go @@ -4,6 +4,7 @@ import ( "bytes" "encoding/json" "errors" + "fmt" "io" "os" "path/filepath" @@ -200,10 +201,15 @@ func runtimePathTree(t *testing.T, root string) map[string]runtimePathState { if err != nil { return err } - info, err := entry.Info() + // Read current metadata by path, not cached Windows enumeration data. + info, err := os.Lstat(path) if err != nil { return err } + // Freeze Windows lazy file IDs before the operation under test. + if !os.SameFile(info, info) { + return fmt.Errorf("cannot capture file identity: %s", path) + } var id installruntime.Identity if !entry.IsDir() { id, err = installruntime.Fingerprint(path) @@ -229,7 +235,7 @@ func assertRuntimePathTree(t *testing.T, root string, before map[string]runtimeP got, ok := after[path] if !ok || !os.SameFile(want.info, got.info) || want.info.Mode() != got.info.Mode() || !want.info.ModTime().Equal(got.info.ModTime()) || want.identity != got.identity { - t.Errorf("refresh mutated or replaced %s", path) + t.Errorf("refresh mutated or replaced %s: sameFile=%v mode=%v/%v mtime=%v/%v identity=%+v/%+v", path, os.SameFile(want.info, got.info), want.info.Mode(), got.info.Mode(), want.info.ModTime(), got.info.ModTime(), want.identity, got.identity) } } } From cdb60e5a386425b8f7f0bae4c7b1a27528d27df2 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 10:59:33 +0000 Subject: [PATCH 12/16] test(install): report missing entries without dereferencing metadata Refs #301 Refs #302 --- cmd/claude-notifications/install_runtime_path_test.go | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cmd/claude-notifications/install_runtime_path_test.go b/cmd/claude-notifications/install_runtime_path_test.go index 15b6b505f..dfc3b8518 100644 --- a/cmd/claude-notifications/install_runtime_path_test.go +++ b/cmd/claude-notifications/install_runtime_path_test.go @@ -233,7 +233,11 @@ func assertRuntimePathTree(t *testing.T, root string, before map[string]runtimeP } for path, want := range before { got, ok := after[path] - if !ok || !os.SameFile(want.info, got.info) || want.info.Mode() != got.info.Mode() || + if !ok { + t.Errorf("refresh removed %s", path) + continue + } + if !os.SameFile(want.info, got.info) || want.info.Mode() != got.info.Mode() || !want.info.ModTime().Equal(got.info.ModTime()) || want.identity != got.identity { t.Errorf("refresh mutated or replaced %s: sameFile=%v mode=%v/%v mtime=%v/%v identity=%+v/%+v", path, os.SameFile(want.info, got.info), want.info.Mode(), got.info.Mode(), want.info.ModTime(), got.info.ModTime(), want.identity, got.identity) } From 1f40809199f0ee46bf5f849381995acbc5c88779 Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 11:30:52 +0000 Subject: [PATCH 13/16] test(install): satisfy pinned lint for native damage cases Refs #301 Refs #302 --- cmd/claude-notifications/install_runtime_path_test.go | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/cmd/claude-notifications/install_runtime_path_test.go b/cmd/claude-notifications/install_runtime_path_test.go index dfc3b8518..fb3be30b5 100644 --- a/cmd/claude-notifications/install_runtime_path_test.go +++ b/cmd/claude-notifications/install_runtime_path_test.go @@ -318,9 +318,10 @@ func TestInstallRuntimeRefreshPrintNativePathRefusesInvalidState(t *testing.T) { if err := os.Rename(f.ledger.Native.Path, old); err != nil { t.Fatal(err) } - if damage == "native-inode" { + switch damage { + case "native-inode": writeRuntimePathFile(t, filepath.Join(f.ledger.Native.Path, "Contents", "MacOS", "terminal-notifier-modern"), []byte("inert native"), 0700) - } else if damage == "native-symlink" { + case "native-symlink": if err := os.Symlink(old, f.ledger.Native.Path); err != nil { t.Fatal(err) } From e1ac6f2ff0b037c72d27c82d2bb0bf8434e9ff0e Mon Sep 17 00:00:00 2001 From: iliya Date: Fri, 2 Oct 2026 14:37:34 +0000 Subject: [PATCH 14/16] fix(install): verify retained native before journal publication Refs #301 Refs #302 --- .../policy_revocation_preflight_test.go | 160 ++++++++++++++++++ internal/installruntime/transaction.go | 44 +++-- 2 files changed, 190 insertions(+), 14 deletions(-) create mode 100644 internal/installruntime/policy_revocation_preflight_test.go diff --git a/internal/installruntime/policy_revocation_preflight_test.go b/internal/installruntime/policy_revocation_preflight_test.go new file mode 100644 index 000000000..238d0eef9 --- /dev/null +++ b/internal/installruntime/policy_revocation_preflight_test.go @@ -0,0 +1,160 @@ +package installruntime + +import ( + "bytes" + "encoding/json" + "os" + "path/filepath" + "reflect" + "testing" +) + +// Channel revocation must not strand a journal when policy drift requires +// native validation. A refused request must leave bounded global disable usable. +func TestPolicyRevocationPreflight(t *testing.T) { + for _, channel := range []string{"gemini", "opencode"} { + for _, drift := range []string{"manual-enable", "deleted-policy", "none"} { + for _, damage := range []string{"bytes", "missing", "healthy"} { + t.Run(channel+"/"+drift+"/"+damage, func(t *testing.T) { + ctx, r, before := replayContentFixture(t, true, false) + // A missing policy means disabled: start enabled to create drift. + enabled := drift != "manual-enable" + r.PolicyEnabled, r.ExpectedGeneration = &enabled, &before.Generation + before, err := Commit(ctx, r) + if err != nil { + t.Fatal(err) + } + r.PolicyEnabled = nil + policyPath := filepath.Join(r.ControlRoot, "agent-notifications.json") + switch drift { + case "manual-enable": + data, err := os.ReadFile(policyPath) + if err != nil { + t.Fatal(err) + } + var fields map[string]json.RawMessage + if err := json.Unmarshal(data, &fields); err != nil { + t.Fatal(err) + } + fields["enabled"] = json.RawMessage("true") + data, err = json.Marshal(fields) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(policyPath, data, 0600); err != nil { + t.Fatal(err) + } + case "deleted-policy": + if err := os.Remove(policyPath); err != nil { + t.Fatal(err) + } + } + policy, err := ReadUserPolicy(r.ControlRoot) + if err != nil || (policy.Enabled != before.Enabled) != (drift != "none") { + t.Fatal("fixture did not establish the intended policy drift", err) + } + if damage != "healthy" { + replayContentDamage(t, before.Native, damage) + } + snapshot, err := ReadRevocationSnapshot(ctx, r.ControlRoot) + if err != nil { + t.Fatal(err) + } + r.ExpectedGeneration, r.ExpectedPolicy = &snapshot.Generation, &snapshot.Preimage + r.ConsumerID = channel + "-notifications" + r.RevokeGemini, r.RevokeOpenCode = channel == "gemini", channel == "opencode" + policyChannel := "geminiNotifications" + if channel == "opencode" { + policyChannel = "openCodeNotifications" + } + patch, err := json.Marshal(map[string]any{policyChannel: map[string]bool{"desktop": false, "webhook": false}}) + if err != nil { + t.Fatal(err) + } + r.PolicyFields = map[string]json.RawMessage{"route": patch} + images := map[string][]byte{} + for _, name := range []string{filepath.Join(r.ControlRoot, "ownership.json"), filepath.Join(r.ControlRoot, "policy-generation.json"), filepath.Join(r.ControlRoot, "receipt.json"), filepath.Join(r.RuntimeRoot, "config.json")} { + data, err := os.ReadFile(name) + if err != nil { + t.Fatal(err) + } + images[name] = data + } + if snapshot.Preimage.Exists { + data, err := os.ReadFile(policyPath) + if err != nil { + t.Fatal(err) + } + images[policyPath] = data + } + after, revokeErr := Commit(ctx, r) + refuse := drift != "none" && damage != "healthy" + if refuse { + if revokeErr == nil { + t.Error("accepted channel revocation with policy drift and damaged native") + } + for path, data := range images { + got, err := os.ReadFile(path) + if err != nil || !bytes.Equal(got, data) { + t.Errorf("refusal mutated %s: %v", path, err) + } + } + gotPolicy, err := Fingerprint(policyPath) + if err != nil || gotPolicy != snapshot.Preimage { + t.Error("refusal changed policy bytes or existence", err) + } + } else { + if revokeErr != nil { + t.Fatal("healthy drift or bounded damaged-native control refused", revokeErr) + } + if after.Enabled != policy.Enabled || after.Generation != before.Generation+1 || !reflect.DeepEqual(after.Native, before.Native) { + t.Fatal("channel revocation changed intent, generation or native ownership") + } + data, err := os.ReadFile(policyPath) + if err != nil { + t.Fatal(err) + } + var published struct { + Route map[string]map[string]json.RawMessage `json:"route"` + } + if err := json.Unmarshal(data, &published); err != nil || string(published.Route[policyChannel]["desktop"]) != "false" || string(published.Route[policyChannel]["webhook"]) != "false" { + t.Fatal("channel revocation did not publish both false fields", err) + } + } + marker := filepath.Join(r.ControlRoot, "transaction.json") + if _, err := os.Lstat(marker); !os.IsNotExist(err) { + t.Errorf("channel revocation stranded a journal: %v (commit: %v)", err, revokeErr) + } + if !refuse { + return + } + // Retry the real policy-only global disable without repairing native + // or manually deleting a leaked marker. Old Commit returns ErrPolicyRecovery. + off := false + r.RevokeGemini, r.RevokeOpenCode = false, false + r.PolicyFields, r.PolicyEnabled = nil, &off + after, err = Commit(ctx, r) + if err != nil { + t.Fatalf("refused channel request blocked bounded global disable: %v", err) + } + if after.Enabled || after.Generation != before.Generation+1 || after.PolicyGeneration != before.PolicyGeneration+1 || !reflect.DeepEqual(after.Native, before.Native) { + t.Fatal("global disable changed native ownership or did not publish one disabled generation") + } + policy, err = ReadUserPolicy(r.ControlRoot) + if err != nil || policy.Enabled { + t.Fatal("global disable did not publish disabled policy", err) + } + for _, path := range []string{filepath.Join(r.ControlRoot, "receipt.json"), filepath.Join(r.RuntimeRoot, "config.json")} { + got, err := os.ReadFile(path) + if err != nil || !bytes.Equal(got, images[path]) { + t.Fatal("global disable changed unrelated assets", err) + } + } + if _, err := os.Lstat(marker); !os.IsNotExist(err) { + t.Fatal("global disable retained journal", err) + } + }) + } + } + } +} diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index 618a98d28..4e0b37741 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -835,6 +835,13 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { return l, err } } + // Refuse a retained-native conflict before persisting a decision that its + // own replay cannot complete. Request flags alone do not prove revocation. + if tx.Native == nil && tx.After.Native != nil && !boundedPolicyRevocation(root, tx) { + if err := validateRetainedNative(tx.After.Native); err != nil { + return l, err + } + } if err := writeTransaction(marker, tx); err != nil { return l, err } @@ -848,6 +855,27 @@ func Commit(ctx context.Context, r Request) (Ledger, error) { } return readLedger(root) } + +func validateRetainedNative(native *NativeRecord) error { + if native == nil { + return nil + } + if err := validateNativeRecord(native); err != nil { + return err + } + if err := checkNativeDirectoryID(native.Path, native.DirectoryID); err != nil { + return err + } + digest, err := treeFingerprint(native.Path) + if err != nil { + return err + } + if digest == "" || digest != native.SHA256 { + return fmt.Errorf("native live fingerprint conflict") + } + return nil +} + func recoverTransaction(ctx context.Context, root string, current Ledger, tx transaction, fault func(string) error) error { if current.WriterFloor > ReservationWriterFloor || tx.Before.WriterFloor > ReservationWriterFloor || tx.After.WriterFloor > ReservationWriterFloor { return fmt.Errorf("installed writer floor requires a newer compatible kernel") @@ -884,20 +912,8 @@ func recoverTransaction(ctx context.Context, root string, current Ledger, tx tra // not check bytes. Only a proven bounded revocation may bypass this. preserveNative := boundedPolicyRevocation(root, tx) if tx.Native == nil && !preserveNative { - if native := tx.After.Native; native != nil { - if err := validateNativeRecord(native); err != nil { - return err - } - if err := checkNativeDirectoryID(native.Path, native.DirectoryID); err != nil { - return err - } - digest, err := treeFingerprint(native.Path) - if err != nil { - return err - } - if digest == "" || digest != native.SHA256 { - return fmt.Errorf("native live fingerprint conflict") - } + if err := validateRetainedNative(tx.After.Native); err != nil { + return err } if _, err := refreshLedgerIdentities(tx.After); err != nil { return err From 989cf93bac151873b517f9bcd1500299695d1ff9 Mon Sep 17 00:00:00 2001 From: iliya Date: Sat, 3 Oct 2026 15:35:55 +0300 Subject: [PATCH 15/16] fix(install): capture coherent reverse journal parent identities Validate the original transaction-wide volume bijection, then capture all current parent chains for the new reverse decision. Preserve immutable ledger snapshots and purge inventory. Test future writer refusal beyond the current supported floor. Refs #301 Refs #302 --- internal/installruntime/copilot_policy_test.go | 5 +++-- internal/installruntime/recovery.go | 18 +++++++++++++++--- .../recovery_review_darwin_test.go | 4 ++-- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/internal/installruntime/copilot_policy_test.go b/internal/installruntime/copilot_policy_test.go index 169b0cc22..806037a81 100644 --- a/internal/installruntime/copilot_policy_test.go +++ b/internal/installruntime/copilot_policy_test.go @@ -227,9 +227,10 @@ func TestCopilotRevokeSurvivesSharedDamage(t *testing.T) { } } selection := copilotvscodeinstall.RevokeAll - if damage == "missing-native-native-only" { + switch damage { + case "missing-native-native-only": selection = copilotvscodeinstall.RevokeNative - } else if damage == "missing-native-manual-only" { + case "missing-native-manual-only": selection = copilotvscodeinstall.RevokeManual } after, err := copilotvscodeinstall.RevokeChannels(localTestContext(t), b, selection) diff --git a/internal/installruntime/recovery.go b/internal/installruntime/recovery.go index 112c6a24a..c3f9878cb 100644 --- a/internal/installruntime/recovery.go +++ b/internal/installruntime/recovery.go @@ -232,6 +232,11 @@ func checkPolicyGeneration(root string, l Ledger) error { // snapshots. It never restores an entire directory and never downgrades an // already promoted callback reader. A foreign edit makes rollback refuse. func reverseTransaction(current Ledger, tx transaction) (transaction, error) { + // Prove the original journal's common volume mapping before capturing a + // new reverse decision. Never normalize a contradictory source journal. + if err := preflightTransactionAnchors(tx); err != nil { + return transaction{}, err + } if tx.Native != nil && len(tx.Native.Parents) != 0 { anchors, err := pathAnchors(tx.Native.After.Path, false) if err != nil { @@ -241,7 +246,8 @@ func reverseTransaction(current Ledger, tx transaction) (transaction, error) { return transaction{}, err } } - for _, f := range tx.Files { + fileParents := make([][]PathAnchor, len(tx.Files)) + for i, f := range tx.Files { anchors, err := pathAnchors(f.Path, false) if err != nil { return transaction{}, err @@ -249,6 +255,9 @@ func reverseTransaction(current Ledger, tx transaction) (transaction, error) { if err := checkPersistedAnchors(f.Parents, anchors); err != nil { return transaction{}, err } + // Native.Parents is freshly captured below. File chains in the new + // decision must use the same observation epoch, while tx stays intact. + fileParents[i] = anchors } if tx.Native != nil && tx.Native.Retire { return transaction{}, fmt.Errorf("retirement deletion must finish forward recovery before rollback") @@ -324,7 +333,7 @@ func reverseTransaction(current Ledger, tx transaction) (transaction, error) { After: *after.Native, } } - for _, f := range tx.Files { + for i, f := range tx.Files { if err := checkReplacementRollback(f); err != nil { return transaction{}, err } @@ -335,7 +344,7 @@ func reverseTransaction(current Ledger, tx transaction) (transaction, error) { if actual != f.Before && actual != desired(f) { return transaction{}, fmt.Errorf("rollback conflict; preserving foreign edit: %s", f.Path) } - undo := File{Parents: f.Parents, Path: f.Path, Before: actual, Data: f.BeforeData, Mode: f.Before.Mode, Link: f.Before.Link, Remove: !f.Before.Exists} + undo := File{Parents: fileParents[i], Path: f.Path, Before: actual, Data: f.BeforeData, Mode: f.Before.Mode, Link: f.Before.Link, Remove: !f.Before.Exists} if actual.Exists && actual.Link == "" { undo.WindowsReplacementID, err = windowsReplacementIdentity(f.Path) if err != nil { @@ -357,6 +366,9 @@ func reverseTransaction(current Ledger, tx transaction) (transaction, error) { if err := validateWriterFilesAtFloor(reverse.Files, after.WriterFloor); err != nil { return transaction{}, err } + if err := preflightTransactionAnchors(reverse); err != nil { + return transaction{}, err + } return reverse, nil } diff --git a/internal/installruntime/recovery_review_darwin_test.go b/internal/installruntime/recovery_review_darwin_test.go index 97a5fece0..777a01464 100644 --- a/internal/installruntime/recovery_review_darwin_test.go +++ b/internal/installruntime/recovery_review_darwin_test.go @@ -313,7 +313,7 @@ func TestRecoveryReviewRevocationFences(t *testing.T) { } }) } - before.WriterFloor = ReservationWriterFloor + 1 + before.WriterFloor = SupportedWriterFloor + 1 if err := writeJSON(filepath.Join(r.ControlRoot, "ownership.json"), before); err != nil { t.Fatal(err) } @@ -566,7 +566,7 @@ func TestRecoveryReviewUnboundedPolicyReplay(t *testing.T) { n.DirectoryID = "1:0" tx.After.Native = &n case "floor": - tx.After.WriterFloor = ReservationWriterFloor + 1 + tx.After.WriterFloor = SupportedWriterFloor + 1 case "generation": tx.After.Generation++ case "rollback": From 50ce4768e4562af4f4330a1f24537d5fc197b9c9 Mon Sep 17 00:00:00 2001 From: iliya Date: Sat, 3 Oct 2026 16:26:57 +0300 Subject: [PATCH 16/16] fix(install): recover revoked policy through fixed root aliases Refs #301, #302 --- .../installruntime/copilot_policy_test.go | 165 ++++++++++++++++++ internal/installruntime/transaction.go | 11 +- 2 files changed, 174 insertions(+), 2 deletions(-) diff --git a/internal/installruntime/copilot_policy_test.go b/internal/installruntime/copilot_policy_test.go index 806037a81..e26243818 100644 --- a/internal/installruntime/copilot_policy_test.go +++ b/internal/installruntime/copilot_policy_test.go @@ -3,6 +3,8 @@ package installruntime_test import ( "bytes" "context" + "crypto/sha256" + "encoding/hex" "encoding/json" "fmt" "github.com/777genius/agent-notifications/internal/agentnotify/portable" @@ -503,3 +505,166 @@ func TestCopilotRecordedConsentCannotMintPhysicalProof(t *testing.T) { t.Fatal("zero physical proof never reached the authority boundary") } } + +// RED: an admitted false-only journal must survive equivalent control-root +// spellings despite damaged native assets, while a foreign binding still refuses. +func TestCopilotRevocationRecoveryControlRoot(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("native retained staging requires a supported native platform") + } + for _, name := range []string{"trailing-slash", "darwin-alias", "wrong-control"} { + t.Run(name, func(t *testing.T) { + root, runtimeRoot, b, before := registeredLocalFixture(t) + recoveryRoot := root + string(filepath.Separator) + if name == "darwin-alias" { + if runtime.GOOS != "darwin" || !strings.HasPrefix(root, "/private/var/") { + t.Skip("fixture has no supported Darwin /var alias") + } + recoveryRoot = strings.TrimPrefix(root, "/private") + } + source := filepath.Join(filepath.Dir(root), "TEST-retained.app") + if err := os.MkdirAll(filepath.Join(source, "Contents", "MacOS"), 0700); err != nil { + t.Fatal(err) + } + if err := os.WriteFile(filepath.Join(source, "Contents", "MacOS", "inert"), []byte("never executed"), 0600); err != nil { + t.Fatal(err) + } + candidate, err := installruntime.StageRetainedNative(localTestContext(t), root, source) + if err != nil { + t.Fatal(err) + } + key, consumer, _, err := b.Registration() + if err != nil { + t.Fatal(err) + } + before, err = installruntime.Commit(localTestContext(t), installruntime.Request{ControlRoot: root, RuntimeRoot: runtimeRoot, Owner: b.Owner, ConsumerID: key, RefreshOnly: true, ExpectedGeneration: &before.Generation, Native: candidate}) + if err != nil { + t.Fatal(err) + } + if err := os.Rename(before.Native.Path, before.Native.Path+"-retained"); err != nil { + t.Fatal(err) + } + snapshot, err := installruntime.ReadRevocationSnapshot(localTestContext(t), root) + if err != nil { + t.Fatal(err) + } + crash := fmt.Errorf("transaction crash") + revoke := installruntime.Request{ControlRoot: root, RuntimeRoot: runtimeRoot, Owner: b.Owner, ConsumerID: key, Consumer: consumer, PolicyOnly: true, RefreshOnly: true, RevokeCopilotVSCode: true, ExpectedGeneration: &snapshot.Generation, ExpectedPolicy: &snapshot.Preimage, + PolicyFields: map[string]json.RawMessage{"route": json.RawMessage(`{"copilotVSCodeNotifications":{"desktop":false,"webhook":false}}`)}, + Fault: func(phase string) error { + if phase == "transaction" { + return crash + } + return nil + }} + if name != "wrong-control" { + alternate := revoke + alternate.ControlRoot, alternate.Fault = recoveryRoot, nil + untouched := treeBytes(t, filepath.Dir(root)) + if _, err := installruntime.Commit(localTestContext(t), alternate); err == nil { + t.Fatal("initial revoke admitted a different binding spelling") + } + if !reflect.DeepEqual(untouched, treeBytes(t, filepath.Dir(root))) { + t.Fatal("refused initial revoke changed control state") + } + } + _, err = installruntime.Commit(localTestContext(t), revoke) + if err != crash { + t.Fatalf("expected durable transaction interruption, got %v", err) + } + marker := filepath.Join(root, "transaction.json") + if name == "wrong-control" { + // Keep registration, journal and live ledger mutually consistent: + // only the binding's control identity is foreign. + b.ControlRoot = filepath.Join(filepath.Dir(root), "TEST-other-control") + b.GlobalConfig = filepath.Join(b.ControlRoot, "agent-notifications.json") + wrongKey, wrongConsumer, _, err := b.Registration() + if err != nil { + t.Fatal(err) + } + var envelope struct { + SHA256 string + Transaction json.RawMessage + } + data, err := os.ReadFile(marker) + if err != nil { + t.Fatal(err) + } + if err := json.Unmarshal(data, &envelope); err != nil { + t.Fatal(err) + } + var journal map[string]json.RawMessage + if err := json.Unmarshal(envelope.Transaction, &journal); err != nil { + t.Fatal(err) + } + for _, state := range []string{"Before", "After"} { + var ledger installruntime.Ledger + if err := json.Unmarshal(journal[state], &ledger); err != nil { + t.Fatal(err) + } + delete(ledger.Consumers, key) + ledger.Consumers[wrongKey] = wrongConsumer + journal[state], err = json.Marshal(ledger) + if err != nil { + t.Fatal(err) + } + if state == "Before" { + if err := os.WriteFile(filepath.Join(root, "ownership.json"), journal[state], 0600); err != nil { + t.Fatal(err) + } + } + } + envelope.Transaction, err = json.Marshal(journal) + if err != nil { + t.Fatal(err) + } + sum := sha256.Sum256(envelope.Transaction) + envelope.SHA256 = hex.EncodeToString(sum[:]) + data, err = json.Marshal(envelope) + if err != nil { + t.Fatal(err) + } + if err := os.WriteFile(marker, data, 0600); err != nil { + t.Fatal(err) + } + untouched := treeBytes(t, filepath.Dir(root)) + if _, err := installruntime.Commit(localTestContext(t), installruntime.Request{ControlRoot: recoveryRoot, RecoverOnly: true}); err == nil { + t.Fatal("recovery accepted a foreign control binding") + } + if !reflect.DeepEqual(untouched, treeBytes(t, filepath.Dir(root))) { + t.Fatal("refused recovery changed control state") + } + return + } + after, err := installruntime.Commit(localTestContext(t), installruntime.Request{ControlRoot: recoveryRoot, RecoverOnly: true}) + if err != nil { + t.Fatal(err) + } + if !reflect.DeepEqual(before.Native, after.Native) { + t.Fatal("recovery changed retained native record") + } + if _, err := os.Lstat(marker); !os.IsNotExist(err) { + t.Fatalf("recovery retained journal: %v", err) + } + data, err := os.ReadFile(filepath.Join(root, "agent-notifications.json")) + if err != nil { + t.Fatal(err) + } + var policy struct { + Enabled bool + Route struct { + Local struct { + Desktop, Webhook bool + Manual struct{ Enabled bool } + } `json:"copilotVSCodeNotifications"` + } + } + if err := json.Unmarshal(data, &policy); err != nil { + t.Fatal(err) + } + if !policy.Enabled || policy.Route.Local.Desktop || policy.Route.Local.Webhook || !policy.Route.Local.Manual.Enabled { + t.Fatal("recovery failed to persist only native false policy") + } + }) + } +} diff --git a/internal/installruntime/transaction.go b/internal/installruntime/transaction.go index ef72e7fb6..4143bda36 100644 --- a/internal/installruntime/transaction.go +++ b/internal/installruntime/transaction.go @@ -1058,8 +1058,15 @@ func boundedPolicyRevocation(root string, tx transaction) bool { } if local && before.Owner == "existing-installer" && localPortableConsumer(id, consumer) { var binding struct{ ComponentID, ControlRoot string } - if json.Unmarshal([]byte(consumer.Registration), &binding) == nil && binding.ComponentID == before.ID && binding.ControlRoot == root { - registered = true + if json.Unmarshal([]byte(consumer.Registration), &binding) == nil && binding.ComponentID == before.ID { + // Admission requires the exact recorded spelling. A durable + // journal may be recovered through a fixed OS alias or a clean + // spelling, without accepting arbitrary directory symlinks. + bindingRoot, bindingErr := PhysicalPath(binding.ControlRoot) + recoveryRoot, recoveryErr := PhysicalPath(filepath.Clean(root)) + if bindingErr == nil && recoveryErr == nil && bindingRoot == recoveryRoot { + registered = true + } } } if channel == "" || before.Owner == "existing-installer" && consumer.Registration != "" &&