amadan
plan: claim Plan 4 rev 3, umask-independent directory modes
Co-Authored-By: Codex <noreply@openai.com> Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
1 file changed,
+173
−4
docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md+173 −4
CI passed — run details
diff --git a/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md b/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md| index 9d0fa520f..3576620c8 100644 |
| --- a/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md |
| +++ b/docs/superpowers/plans/2026-10-05-host-lifecycle-claim-4-park-engine.md |
| @@ -1,5 +1,7 @@ |
| # Host Lifecycle Claim, Plan 4: the park engine |
| +Rev 3 answers review round 2 (one P2). |
| + |
| Rev 2 answers the independent review (one P1, three P2). |
| > **For agentic workers:** REQUIRED SUB-SKILL: Use superpowers:subagent-driven-development (recommended) or superpowers:executing-plans to implement this plan task-by-task. Steps use checkbox (`- [ ]`) syntax for tracking. |
| @@ -28,7 +30,7 @@ Rev 2 answers the independent review (one P1, three P2). |
| - **Authority and leases are explicit dependencies.** `Authorize` must validate the current operation/phase, chore owner, original installation/scope and durable stop evidence or lost-box fence from Plan 3/8; for generation work it also validates moves' sealing/publication conditions. A no-op callback appears only in tests. `View` must perform complete marker/selection reads for the supplied observation and return that exact ETag. `Leases.Acquire(bucket, location)` is an **aggregate lease acquisition**: hold and renew the replica leases for every readable `instances`/`moves` location plus any moves-owned staging-work lease required for that scope. It is not permission to hold only the instance lock while mutating staging. `Check` refuses cancellation, expiry and lost ownership. `Close` stops renewal; only successful acknowledgement calls conditional `Release`, which removes only this worker's held lock version(s). Plans 5/6 supply these adapters; no production adapter is invented here. Each engine is bound to its own region's bucket; no sibling-bucket mutation (N13). |
| - **Local identity and order.** Persist the original `hostguard.Captured{Incarnation, Stamp}` when selecting retirement work and reuse it on every retry. `RetireArtifacts(..., S)` parks only inventory entries tagged S, while the caller's captured stamp is the installation active during this operation; it changes no stamp. `RetireInstallation` durably persists the inactive stamp before the first artifact move on every attempt, including an already-inactive retry, then parks all artifacts under the same flock. An inactive retry must still match the last retired stamp. Activation is still `hostguard.Activate`; never use installation retirement to clean a predecessor. |
| - **Inventory and journal.** Inventory paths are relative to DataDir and are disjoint roots; a directory entry owns its entire subtree at the recorded stamp. Flat layouts enroll the host directory, DB/WAL/SHM, `<host>.env`, `<host>.config.env`, `<host>-sidecar.env`, generation/lineage markers and every other host-bound operational path. Nested layouts enroll the resolved instance directory. Use `RecordInventory` before materialization, including on layout changes; retire old paths before changing a populated layout. Missing inventory refuses; an explicitly empty entry can acknowledge. A never-installed box uses `AcknowledgeEmpty`, which additionally proves the local record is absent under the guard. |
| -- **Local destination and R9:** `<DataDir>/.parked/<incarnation>/<host>/<retirement-id>/artifacts/<inventory-relative-path>`, with `.journal.json` beside `artifacts/`. `.parked`, DataDir and Plan 3's guard directories must be provisioned with trusted ownership; `.parked` uses setgid group `carlos`, mode **2770**, like the guard directories. Plan 6 owns that provisioning. This library uses `Mkdirat` only for descendants of the pinned `.parked` descriptor; it does not change pathname ownership/mode. Every traversal uses directory descriptors and `O_NOFOLLOW`, every artifact inspection uses `AT_SYMLINK_NOFOLLOW`, and artifact moves use **`RENAME_NOREPLACE`**. Journal temporaries use exclusive, no-follow opens and fd-based mode/fsync; journal replacement is atomic. Fsync source/destination and journal directories before completion. `EXDEV`, conflicting destinations, changed source inodes, symlinks and any other error keep the chore open. Artifacts are never deleted. |
| +- **Local destination and R9:** `<DataDir>/.parked/<incarnation>/<host>/<retirement-id>/artifacts/<inventory-relative-path>`, with `.journal.json` beside `artifacts/`. `.parked`, DataDir and Plan 3's guard directories must be provisioned with trusted ownership; `.parked` uses setgid group `carlos`, mode **2770**, like the guard directories. Plan 6 owns that provisioning. This library uses `Mkdirat` only for descendants of the pinned `.parked` descriptor. Open and validate each descendant, including on `EEXIST`; use `Fchmod(fd, 02770)` when its owner matches the effective UID, otherwise verify the required mode bits and refuse if any are missing. Fsync the descendant and its parent before proceeding. It does not change pathname ownership/mode. Every traversal uses directory descriptors and `O_NOFOLLOW`, every artifact inspection uses `AT_SYMLINK_NOFOLLOW`, and artifact moves use **`RENAME_NOREPLACE`**. Journal temporaries use exclusive, no-follow opens and fd-based mode/fsync; journal replacement is atomic. Fsync source/destination and journal directories before completion. `EXDEV`, conflicting destinations, changed source inodes, symlinks and any other error keep the chore open. Artifacts are never deleted. |
| - **Portable build, fail-closed local operation.** Linux implements `Renameat2`; Darwin and Windows compile a local-retirement refusal stub that runs before stamp mutation. Remote parking remains portable. Do not create Go caches under `/tmp`; use the default shared cache. If the sandbox refuses writes there, use the already-existing shared on-disk `$TMPDIR/gocache`, never a per-task cache. |
| ## Review Focus |
| @@ -1757,10 +1759,14 @@ import ( |
| "encoding/json" |
| "errors" |
| "os" |
| + "os/exec" |
| "path/filepath" |
| + "syscall" |
| "testing" |
| "time" |
| + "golang.org/x/sys/unix" |
| + |
| "github.com/carlosframework/platform/internal/hostclaim" |
| "github.com/carlosframework/platform/internal/hostguard" |
| "github.com/carlosframework/platform/internal/release" |
| @@ -1930,6 +1936,148 @@ func TestLocalResumeAfterEveryRenameHoldsGuard(t *testing.T) { |
| } |
| } |
| +func TestLocalRestrictiveUmask(t *testing.T) { |
| + // Umask is process-wide: this test and its fixture setup must not run in parallel. |
| + f := localFixtureFor(t, map[string]uint64{"nested/a": 5, "nested/b": 5}) |
| + f.write(t, "nested/a", "a") |
| + f.write(t, "nested/b", "b") |
| + group := os.Getegid() |
| + if os.Geteuid() == 0 { |
| + group = 65534 |
| + } |
| + for _, path := range []string{f.l.DataDir, filepath.Join(f.l.DataDir, ".parked"), filepath.Join(f.l.DataDir, "nested")} { |
| + dir, err := os.Open(path) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if os.Geteuid() == 0 { |
| + if err := dir.Chown(0, group); err != nil { |
| + dir.Close() |
| + t.Fatal(err) |
| + } |
| + } |
| + if err := dir.Chmod(os.ModeSetgid | 0770); err != nil { |
| + dir.Close() |
| + t.Fatal(err) |
| + } |
| + dir.Close() |
| + } |
| + oldUmask := syscall.Umask(0o077) |
| + defer syscall.Umask(oldUmask) |
| + crash := errors.New("crash under restrictive umask") |
| + f.l.afterRename = func(string) error { return crash } |
| + if err := f.l.RetireInstallation(t.Context(), f.host, f.captured, "local", "delete"); !errors.Is(err, crash) { |
| + t.Fatal(err) |
| + } |
| + base := filepath.Join(f.l.DataDir, ".parked", f.captured.Incarnation) |
| + event := filepath.Join(base, f.host, "retirement") |
| + for _, path := range []string{base, filepath.Join(base, f.host), event, filepath.Join(event, "artifacts"), filepath.Join(event, "artifacts", "nested")} { |
| + var st unix.Stat_t |
| + if err := unix.Lstat(path, &st); err != nil { |
| + t.Fatal(err) |
| + } |
| + if st.Mode&unix.S_IFMT != unix.S_IFDIR || st.Mode&07777 != 02770 || st.Gid != uint32(group) { |
| + t.Fatalf("archive directory %s: mode=%#o gid=%d", path, st.Mode, st.Gid) |
| + } |
| + } |
| + if os.Geteuid() == 0 { |
| + body, err := os.ReadFile(filepath.Join(event, ".journal.json")) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + dir, err := os.Open(f.l.DataDir) |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + defer dir.Close() |
| + exe, err := os.Executable() |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + run := func(refuse bool) { |
| + t.Helper() |
| + cmd := exec.Command(exe, "-test.run=^TestLocalRestrictiveUmaskWorker$") |
| + cmd.ExtraFiles = []*os.File{dir} |
| + cmd.Env = append(os.Environ(), "PARK_UMASK_WORKER=1", "PARK_UMASK_JOURNAL="+string(body)) |
| + if refuse { |
| + cmd.Env = append(cmd.Env, "PARK_UMASK_REFUSE=1") |
| + } |
| + if out, err := cmd.CombinedOutput(); err != nil { |
| + t.Fatalf("group worker: %v\n%s", err, out) |
| + } |
| + } |
| + // A worker must refuse, not chmod, directories owned by another UID. |
| + for _, mode := range []os.FileMode{os.ModeSetgid | 0750, 0770} { |
| + path := filepath.Join(event, "artifacts") |
| + if err := os.Chmod(path, mode); err != nil { |
| + t.Fatal(err) |
| + } |
| + run(true) |
| + st, err := os.Stat(path) |
| + if err != nil || st.Mode()&(os.ModeSetgid|0777) != mode { |
| + t.Fatal("foreign directory permissions changed", err) |
| + } |
| + if err := os.Chmod(path, os.ModeSetgid|0770); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + run(false) |
| + } else { |
| + t.Log("mode and group assertions passed; cross-user subprocess requires root") |
| + } |
| + f.l.afterRename = nil |
| + if err := f.l.RetireInstallation(t.Context(), f.host, f.captured, "local", "delete"); err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, name := range []string{"a", "b"} { |
| + if body, err := os.ReadFile(f.parked("nested/" + name)); err != nil || string(body) != name { |
| + t.Fatalf("resumed artifact %s: %q %v", name, body, err) |
| + } |
| + } |
| + obs, err := hostclaim.Read(t.Context(), f.l.Claims, f.host) |
| + if err != nil || len(obs.Claim.Obligations) != 0 { |
| + t.Fatal("resumed retirement not acknowledged", err) |
| + } |
| +} |
| + |
| +func TestLocalRestrictiveUmaskWorker(t *testing.T) { |
| + if os.Getenv("PARK_UMASK_WORKER") != "1" { |
| + t.Skip("subprocess helper") |
| + } |
| + var want localJournal |
| + if err := json.Unmarshal([]byte(os.Getenv("PARK_UMASK_JOURNAL")), &want); err != nil { |
| + t.Fatal(err) |
| + } |
| + // Enter the fixture through an inherited descriptor; its temp ancestors may be private. |
| + if err := unix.Fchdir(3); err != nil { |
| + t.Fatal(err) |
| + } |
| + for _, change := range []func() error{ |
| + func() error { return syscall.Setgroups([]int{65534}) }, |
| + func() error { return syscall.Setgid(65534) }, |
| + func() error { return syscall.Setuid(65534) }, |
| + } { |
| + if err := change(); err != nil { |
| + t.Fatal(err) |
| + } |
| + } |
| + oldUmask := syscall.Umask(0o077) |
| + defer syscall.Umask(oldUmask) |
| + err := (Local{DataDir: "."}).parkLocal(t.Context(), &want) |
| + if os.Getenv("PARK_UMASK_REFUSE") == "1" { |
| + if !errors.Is(err, ErrLocalInventory) { |
| + t.Fatalf("foreign directory without required mode accepted: %v", err) |
| + } |
| + return |
| + } |
| + if err != nil { |
| + t.Fatal(err) |
| + } |
| + if _, err := os.Stat("nested/b"); !errors.Is(err, os.ErrNotExist) { |
| + t.Fatal("second process did not retire remaining artifact", err) |
| + } |
| +} |
| + |
| func TestLocalRefusesSymlinksAndOverlaps(t *testing.T) { |
| for _, place := range []string{"source", "ancestor", "park-root", "destination", "overlap", "missing-inventory"} { |
| t.Run(place, func(t *testing.T) { |
| @@ -2757,9 +2905,6 @@ func localDirAt(parent int, name string, create bool) (int, error) { |
| if err := unix.Mkdirat(parent, name, 0770); err != nil && !errors.Is(err, unix.EEXIST) { |
| return -1, err |
| } |
| - if err := unix.Fsync(parent); err != nil { |
| - return -1, err |
| - } |
| } |
| fd, err := unix.Openat(parent, name, unix.O_RDONLY|unix.O_DIRECTORY|unix.O_NOFOLLOW|unix.O_CLOEXEC, 0) |
| if err != nil { |
| @@ -2773,6 +2918,26 @@ func localDirAt(parent int, name string, create bool) (int, error) { |
| } |
| return -1, ErrLocalInventory |
| } |
| + if create { |
| + // A retry can encounter another worker's directory. Root also respects its owner. |
| + if st.Uid == uint32(os.Geteuid()) { |
| + if err := unix.Fchmod(fd, 02770); err != nil { |
| + unix.Close(fd) |
| + return -1, err |
| + } |
| + } else if st.Mode&02770 != 02770 { |
| + unix.Close(fd) |
| + return -1, ErrLocalInventory |
| + } |
| + if err := unix.Fsync(fd); err != nil { |
| + unix.Close(fd) |
| + return -1, err |
| + } |
| + if err := unix.Fsync(parent); err != nil { |
| + unix.Close(fd) |
| + return -1, err |
| + } |
| + } |
| return fd, nil |
| } |
| @@ -3144,6 +3309,8 @@ Plans 5–7 remain behind `CARLOS_HOST_CLAIM` until Plan 8 publishes the shared |
| ## Self-review |
| +Rev 3 resolves review round 2's P2: `localDirAt` now establishes setgid and group write through the validated descriptor, independently of umask. It repairs an owned directory on `EEXIST`, verifies a foreign-owned directory without changing it, and fsyncs the directory before its parent. `TestLocalRestrictiveUmask` interrupts retirement under `syscall.Umask(0o077)` (restored afterward), checks mode `2770` and inherited group on every archive descendant, then resumes. When run as root, a subprocess with UID/GID 65534 resumes journal writes and the remaining rename in root-owned archive directories; it also refuses foreign directories missing group write or setgid. Non-root runs assert the mode bits and group, then complete the same-user retry. |
| + |
| Rev 2 resolves the independent review's findings: |
| | Finding | Change and regression | |
| @@ -3189,3 +3356,5 @@ A separate **gpt-6-astra** authority review, used because Opus was unavailable, |
| Rev 2 verification: extracted all **11 complete Go blocks** and applied the exact claim-edit script to a scratch checkout of cached `origin/feat/hostclaim-boxguard` (`01ed2bd9`, including the storage and claim dependencies). `go test ./internal/park ./internal/hostguard ./internal/hostclaim -count=1`, the same suite with `-race`, and `go vet` for those packages all passed. All three packages built for Darwin/amd64, Windows/amd64 and Linux/arm64. Restoring each reported defect separately made its new regression fail; the revised sources were then restored and checked against the Markdown. Verification used the existing shared on-disk cache after the default cache returned a read-only-filesystem error. The full repository gate remains an implementation task. |
| A separate **gpt-6-astra** authority review of the rev 2 changes, used because Opus was unavailable, reported no blockers. The cached Plan 3 branch had no durability repair helper, and network access prevented checking a newer remote tip; Task 3 therefore includes the local durable-rewrite implementation and the instruction to reuse a suitable Plan 3 helper when available. Only this plan was changed; nothing was staged or committed. |
| + |
| +Rev 3 verification: extracted all **11 complete Go blocks** against the same cached `01ed2bd9` dependencies and applied the exact claim-edit script. The park, hostguard and hostclaim package tests passed, including with `-race`; `go vet` passed for all three packages. All three packages built for Darwin/amd64, Windows/amd64 and Linux/arm64. The restrictive-umask regression passed as UID 1001, checking mode `2770`, inherited group and completed retry; the root-only cross-user branch was compiled and reviewed but could not run in this non-root session. Restoring the original `localDirAt` made the regression fail with archive mode `02700`; the verified helper was then restored. Verification used the existing shared on-disk cache because the default cache was read-only. A separate **gpt-6-astra** authority review, used because Opus was unavailable through the delegation tool, reported no blockers. Only this plan was changed; nothing was staged or committed. The full repository gate remains an implementation task. |