| index da55125c4..fecb41179 100644 |
| --- a/docs/superpowers/plans/2026-10-13-zero-downtime-flag-readiness-3.md |
| +++ b/docs/superpowers/plans/2026-10-13-zero-downtime-flag-readiness-3.md |
| @@ -24,9 +24,10 @@ |
| - No new member-facing strings unless unavoidable; any that are added carry draft text and a `TODO-COPY` marker and go into `copy-review/strings.json`, and the controller stops and reports that path. |
| - Commits use the identity `claude <claude~paul@keymail.dev>`: `git -c user.name=claude -c user.email=claude~paul@keymail.dev commit ...`. Never push; the controller pushes. |
| - Sandboxed Go: `GOCACHE=$TMPDIR/gocache` on every go command. Kernel and socket tests run unsandboxed with `TMPDIR=/tmp/k`. Never set GOCACHE/GOMODCACHE/GOTMPDIR under `/tmp`. |
| +- Scratch for kernel and socket runs lives in `/tmp/k/zddr3/`, which this plan owns; remove it (`rm -rf /tmp/k/zddr3`) at the end of each run, and never touch anything else under `/tmp/k`. The GOCACHE and `/tmp/k` rules are the user's instructions for this run (Ruling 15). |
| - Kernel tests on DOCS-T1 run in a transient delegated user unit, for example: |
| - `systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k -E CARLOS_SHIM_KERNEL=1 <test binary> -test.run '<regex>' -test.v -test.count=1` |
| - (build the binary first with `go test -c -o $TMPDIR/cgroup.test ./internal/cgroup`, and copy it to `/tmp/k` if a unit's sandbox hides `$TMPDIR`). User delegation on DOCS-T1 offers `cpu memory pids`, not `io`. |
| + `systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k/zddr3 -E CARLOS_SHIM_KERNEL=1 <test binary> -test.run '<regex>' -test.v -test.count=1` |
| + (build the binary first with `go test -c -o /tmp/k/zddr3/cgroup.test ./internal/cgroup`). Package test runs that use sockets keep `TMPDIR=/tmp/k` itself, since the activator's socket paths only fit under a root that short, and `t.TempDir` cleans up after itself there. User delegation on DOCS-T1 offers `cpu memory pids`, not `io`. |
| - Do not run anything on real boxes. Never run `git worktree prune`. Never push to main or run `amadan branch merge`. |
| - Before committing: `gofmt -l` clean on touched files, `go vet` on touched packages, and `GOOS=darwin go build ./... && GOOS=windows go build ./...` when a touched file is not linux-only (linux-only code needs `//go:build linux` and a `!linux` stub). |
| - Heavy test loops run under `nice -n 19`. |
| @@ -37,22 +38,26 @@ |
| Each is ledgered by the controller as `Ruling: <what> — <why> — <cost if wrong>`. |
| |
| 1. **(Paul, 2026-10-13)** The exec-shim replaces clone3 placement; no hardening on carlos-edge is relaxed. — RestrictNamespaces makes systemd's seccomp filter answer clone3 with ENOSYS, so CLONE_INTO_CGROUP is impossible in the agent. — Cost if wrong: one extra exec of the carlos binary per spawn. |
| -2. Flat starts (the non-required path, flag off included) use the shim too. On a shim *placement* failure, a flat start falls back to today's start-then-move by PID; a generation start never does. — The flat path's contract is that accounting never breaks a spawn, and every flat start under the shipped unit already uses start-then-move today. — Cost if wrong: a flag-off tenant can still run for a moment outside its cgroup when the shim fails, as every flag-off start does today. |
| +2. Flat starts (the non-required path, flag off included) use the shim too. On a shim *placement* failure, which by the protocol's classification is reported only when non-execution is proven, a flat start falls back to today's start-then-move by PID; a generation start never does. An uncertain start is reported started, never retried, so no tenant runs twice. — The flat path's contract is that accounting never breaks a spawn, and every flat start under the shipped unit already uses start-then-move today. — Cost if wrong: a flag-off tenant can still run for a moment outside its cgroup when the shim fails, as every flag-off start does today. |
| 3. The clone3 attempt and its latch (`Manager.fallback`) leave the Manager start paths. The strict claims path keeps clone3. — Paul: keep the strict-claims path's existing placement. — Cost if wrong: with host claims on under the shipped unit, strict launches answer `ErrStrictUnsupported` (refused, never uncapped) until that path gets the shim. Claims are off everywhere and claims-on boxes decline arming; the roll notes record it as a claims-on-swaps precondition. |
| 4. The shim is `/proc/self/exe __cgexec`, used only when `cgroup.RunShimIfRequested()` has run in this process. — `/proc/self/exe` is the running build even after a roll replaces the file on disk; the hook guard stops a test binary without the hook from re-running its own tests as a "shim". — Cost if wrong: a binary that forgets the hook declines arming and its flat starts use start-then-move: loud, never unsafe. |
| 5. The tenant's path, argv and env travel over a pipe in a NUL-delimited encoding, never in the shim's argv or environment, and the shim runs with an empty environment. — Secrets stay out of `/proc/<pid>/cmdline`; no tenant variable (`GODEBUG`, `LD_*`, `GOMAXPROCS`) shapes the shim before placement; JSON would rewrite invalid UTF-8. — Cost if wrong: one pipe per spawn. |
| 6. A placement in flight counts as a member of its target cgroup and every ancestor up to the host directory: `Cleanup.Populated` answers true, `Cleanup.Members` answers `ErrPlacementInFlight`, `Cleanup.KillAndEmpty` waits for it to resolve (bounded by its ctx), and the legacy-parent guard counts one aimed at the host directory itself. — With clone3 the child appeared in its cgroup atomically; the shim spends a few milliseconds in the agent's cgroup first, and an emptiness proof must not miss it. — Cost if wrong: a stop that lands during a spawn's few milliseconds waits for it or answers unproven (the lease is kept), never a false empty. |
| -7. A new decline code `cgroup-placement-failed` reuses the approved declined phrase (`unavailableDeclined`), so it adds no copy. — Same meaning as the other declined codes for a member. — Cost if wrong: a console older than the box shows the generic unknown-code phrase for it until consoles roll (they roll after boxes). |
| +7. A new decline code `cgroup-placement-failed` reuses the approved declined phrase (`unavailableDeclined`), so it adds no copy. — Same meaning as the other declined codes for a member. — Cost if wrong: a console older than the box shows the generic unknown-code phrase for it and, because it treats an unknown code as not declined (`internal/rolloutwords/words.go:109`, `internal/console/instancepage.go:43`), cannot offer or sign a host retry for that box until it is updated (consoles roll after boxes). |
| 8. The arm-time placement check is box-wide: a failure declines the whole box, nested rows included. — One flag, one decision, as for `host-claims-on`. — Cost if wrong: a box whose nested rows could swap declines until flat placement works. |
| -9. The shim's status deadline is 10 s (`shimDeadline`). On expiry the shim is SIGKILLed and reaped, and the start is a placement failure. — A wake's budget is 30 s; a Go binary that cannot start in 10 s is a box condition. — Cost if wrong: a box that loaded refuses generation starts. |
| +9. The shim's deadline is 10 s (`shimDeadline`), once up to placement and once from the release to exec. Before `R` is seen, expiry SIGKILLs the shim, drains the status pipe, and is a placement failure only if no `R` was read (non-execution proven); with `R` read the start is reported started and nothing is killed. Failed starts are reaped with `os.Process.Wait`, which is bounded once the shim is dead, and exec.Cmd's I/O goroutines are released in the background. — A wake's budget is 30 s; a Go binary that cannot start in 10 s is a box condition. — Cost if wrong: a box that loaded refuses generation starts. |
| 10. `reset-failed` after a generation stop is best-effort: a failure is logged and the stop still answers 204. — The stop itself succeeded; the failed-list entry is cosmetic. — Cost if wrong: a failed generation unit can linger in `systemctl --failed`. |
| 11. The roll notes' probe app moves into the module as `cmd/zddprobe`, with a test that launches it with the platform's argv. — A heredoc cannot be tested. — Cost if wrong: one more `cmd/` directory. |
| 12. The 3000-seed simulation sweep runs only if the branch diff touches `internal/rollout` or the simulation's files; the shim, the arming check and the stop path are outside the modelled scheduler. — Cost if wrong: an unswept scheduler change; the final review checks the diff's paths. |
| +13. `before` (the caller's last ownership check) moves from before the fork to after placement, immediately before exec, behind a placed/release handshake (Astra plan 2). — `internal/activator/genbackend.go:69` requires the check after preparation and immediately before execution; with the shim, placement is preparation. — Cost if wrong: a refused start now costs a fork and a placement that are killed and reaped. |
| +14. The shim's exec thread gets an empty signal mask; dispositions are left to `execve`. — systemd starts every service with an empty mask, and Go's own forked child restores the mask the agent started with; the kernel test compares a shim start with a direct start under an inherited ignored SIGHUP, `Pdeathsig`, `Setpgid`, capabilities and `RLIMIT_NOFILE`. — Cost if wrong: a tenant started by an agent that inherited blocked signals would see them unblocked. |
| +15. `GOCACHE=$TMPDIR/gocache` in the sandbox and `/tmp/k` for socket and kernel runs stand (Astra plan 11). — They are the user's instructions for this run; scratch is confined to `/tmp/k/zddr3`, removed after each run. — Cost if wrong: none. |
| +16. The tenant's `cmd.Dir` is applied by the shim after placement, not by exec.Cmd at the shim's fork (Astra plan 5). — A bad `Dir` is then the command's own error (`*os.PathError{Op: "chdir"}`), never misread as a placement failure. — Cost if wrong: one `chdir` per spawn moves from Go's fork path to the shim. |
| |
| ## Review Focus |
| |
| -1. **Exact pass-through.** A tenant env with invalid UTF-8, an empty string, `=` inside a value, a 1 MiB value, and an argv with an empty element, a newline and spaces reach the tenant byte-for-byte, the same as a direct start (Task 1, `TestShimKernelPassesArgvEnvExactly`). |
| -2. **Inherited descriptors.** A `build()` that sets `ExtraFiles` (two pipes) gets them at fds 3 and 4 in the tenant, and the tenant has no other fd above 2: none of the shim's spec, directory or status fds, and none the agent opened without close-on-exec (Task 1, `TestShimKernelFDsExact`). |
| +1. **Exact pass-through.** A tenant env with invalid UTF-8, an empty value, `=` inside a value and a 100 KiB value, and an argv with an empty element, a newline and spaces reach the tenant byte-for-byte, the same as a direct start; a value over Linux's per-string limit fails with the same `E2BIG` a direct start gets (Task 1, `TestShimKernelPassesArgvEnvExactly`). |
| +2. **Inherited state.** A `build()` that sets `ExtraFiles` (two pipes) gets them at fds 3 and 4 in the tenant, and the tenant has no other fd above 2: none of the shim's spec, directory or status fds, and none the agent opened without close-on-exec. Signal mask and ignored set, capabilities, `Pdeathsig`, process group and `RLIMIT_NOFILE` equal a direct start's (Task 1, `TestShimKernelInheritedStateMatchesADirectStart`). |
| 3. **The agent's binary replaced on disk.** After a roll replaces `/opt/carlos/carlos`, the running agent's shim still execs (Task 1, `TestShimKernelSurvivesDeletedExecutable`). |
| 4. **A stop during a spawn.** `KillAndEmpty` or `Populated` on a host whose generation is being placed never reports empty while the shim is in flight (Task 2, `TestCleanupCountsPlacementInFlight`). |
| 5. **A missing tenant binary.** The start returns the command's own `fork/exec ... no such file or directory` error, not `ErrLimitNotApplied`, nothing ran, the shim is reaped, and an empty generation directory is removed (Task 1 and Task 2, `TestShimExecFailureIsTheCommands`, `TestGenerationMissingBinaryIsNotABoxCondition`). |
| @@ -102,61 +107,81 @@ Each is ledgered by the controller as `Ruling: <what> — <why> — <cost if wro |
| - `var shimExe = "/proc/self/exe"` |
| - Consumes: `ParseSelfCgroup` (`internal/cgroup/cgroup.go:155`). |
| |
| -**The protocol.** |
| +**The protocol** (revised after Astra's plan review, `CODEX-FINDINGS-ZDD-READINESS3-PLAN.md` findings 1, 2, 3 and 5). |
| + |
| +The rule the classification rests on: the shim writes `R` immediately before `execve`, and never earlier. So a status stream with no `R` proves no tenant code ran, and any stream with `R` and no `E` after it means the tenant may have run and the start is reported as started. A start is only ever called a failure when non-execution is proven; an uncertain one is a started process the caller owns (its `Wait` will show how it ended), so the activator's descendant proofs (`internal/activator/backend_exec.go:1037-1106`) still run for it, and the flat fallback never runs a tenant twice. |
| |
| Parent, in `startViaShim`: |
| 1. If `!shimAvailable()`, return `&placementError{errors.New("the placement shim is not available in this process")}`. If `cmd.Err != nil` (a failed `LookPath` in `build()`), return `cmd.Err`. |
| 2. Open `dir` with `unix.Open(dir, O_RDONLY|O_DIRECTORY|O_NOFOLLOW|O_CLOEXEC, 0)`. `unix.Fstatfs` it; anything but `unix.CGROUP2_SUPER_MAGIC` is a placement error. Compute `expected`: `dir` must start with `/sys/fs/cgroup/`; `expected` is `dir` with the `/sys/fs/cgroup` prefix cut (so it starts with `/`). Anything else is a placement error. |
| -3. Capture the tenant before rewriting anything: `path := cmd.Path`; `argv := cmd.Args`, or `[]string{cmd.Path}` when empty (exec.Cmd's own rule); `env := cmd.Environ()` (exactly what `Start` would pass, `PWD` included); `keep := 3 + len(cmd.ExtraFiles)`. |
| -4. `os.Pipe()` twice: spec (shim reads) and status (shim writes). Rewrite `cmd`: `Path = shimExe`, `Args = []string{"carlos-cgexec", ShimVerb, itoa(keep), itoa(keep+1), itoa(keep+2)}`, `Env = []string{}`, `ExtraFiles = append(original ExtraFiles, specR, dirFile, statusW)`. Leave `Dir`, `Stdin`, `Stdout`, `Stderr`, `SysProcAttr`, `Cancel`, `WaitDelay` untouched. |
| -5. Ask `before` (when set); a refusal returns `true, before's error` with nothing started. Only then `done := m.beginPlacement(dir)` (the in-flight registry, below), and `startCmd(cmd)` (`internal/cgroup/cgroup.go:332`). The registry is taken after `before` and never across it: a stop that waits on the registry may hold a lock `before` needs. Close the parent's `specR`, `dirFile` and `statusW` whatever happened. A failed `startCmd` (the carlos binary itself did not start) is a placement error; `done()` and return it. |
| -6. Write the encoded spec to `specW` with a write deadline at `shimDeadline`, then close it. Read `statusR` to EOF with a read deadline at `shimDeadline` (pipes from `os.Pipe` support deadlines). |
| -7. Decide from what was read: a line `E <errno>` is the command's own failure: `&os.PathError{Op: "fork/exec", Path: path, Err: syscall.Errno(n)}`. A line `P <reason>` is a placement error with that reason. `R` with nothing after it is success. Nothing at all (EOF without `R`) is a placement error ("the shim exited before exec"). A deadline is a placement error ("no answer from the shim within 10s"). |
| -8. On any failure: `cmd.Process.Kill()` then `cmd.Wait()` (ignore its error), so no shim outlives the call, then `done()`. On success `done()` and return `false, nil`: the process is in `dir` by then, so a proof that reads `dir` sees it. |
| - |
| -**The in-flight registry** (`inflight.go`, used by Task 2's proofs): a mutex-guarded `map[string]int` of target directories (cleaned paths), plus a `chan struct{}` that is closed and replaced whenever a count drops, so waiters wake. |
| - |
| -```go |
| -func (m *Manager) beginPlacement(dir string) (done func()) // done is idempotent |
| -func (m *Manager) placementsUnder(dir string) int // targets equal to dir or below dir+"/" |
| -func (m *Manager) placementsAt(dir string) int // targets equal to dir |
| -func (m *Manager) waitPlacementsUnder(ctx context.Context, dir string) error // nil at zero, ctx.Err() if ctx ends first |
| -``` |
| - |
| -A nil `*Manager` answers zero and nil, and its `beginPlacement` returns a no-op. |
| +3. Capture the tenant before rewriting anything: `path := cmd.Path`; `argv := cmd.Args`, or `[]string{cmd.Path}` when empty (exec.Cmd's own rule); `env := cmd.Environ()` (exactly what `Start` would pass, `PWD` included); `wd := cmd.Dir`; `keep := 3 + len(cmd.ExtraFiles)`. |
| +4. `os.Pipe()` twice: spec (parent writes the spec and later the release byte; shim reads) and status (shim writes; parent reads). Rewrite `cmd`: `Path = shimExe`, `Args = []string{"carlos-cgexec", ShimVerb, itoa(keep), itoa(keep+1), itoa(keep+2)}`, `Env = []string{}`, `Dir = ""` (the shim changes directory itself, after placement, so a bad `Dir` is reported as the command's error and never as a placement error), `ExtraFiles = append(original ExtraFiles, specR, dirFile, statusW)`. Leave `Stdin`, `Stdout`, `Stderr`, `SysProcAttr`, `Cancel`, `WaitDelay` untouched. |
| +5. One absolute deadline `D := now + shimDeadline` covers steps 5 to 7. `done := m.beginPlacement(dir)` (the in-flight registry, below), then `startCmd(cmd)` (`internal/cgroup/cgroup.go:332`). Close the parent's `specR`, `dirFile` and `statusW` whatever happened. A failed `startCmd` means the carlos binary itself did not start (the tenant's `Dir` is not part of this start): `done()`, close `specW` and `statusR`, return a placement error. |
| +6. Write the spec to `specW` (an 8-byte big-endian length, then the encoding below) with write deadline `D`. Do not close `specW`: the release byte goes on it later. |
| +7. Read `statusR` line by line with read deadline `D` until `A`, `P`, or EOF: |
| + - `P <reason>`, or EOF: the shim failed or died before placement was confirmed. No tenant code ran. |
| + - deadline: SIGKILL the shim. No release byte was sent, so it cannot have exec'd. No tenant code ran. |
| + - In both cases: `reap(cmd)` (below), `done()`, close `specW` and `statusR`, return a placement error naming the reason, "the shim exited before placement" or "no answer from the shim within 10s". |
| + - `A`: the process is in `dir` now, visible to every reader of the cgroup, so `done()` here. The registry covers only fork-to-placement, and no caller code runs inside it. |
| +8. Ask `before` (when set), now, after placement and immediately before exec, as `internal/activator/genbackend.go:69` requires. A refusal: close `specW` without the release byte (the shim reads EOF and exits 125 without exec), `reap(cmd)`, close `statusR`, return `true, before's error`. |
| +9. Write the release byte `X` to `specW` and close it. A second deadline `D2 := now + shimDeadline`. Read `statusR` until EOF with deadline `D2`: |
| + - `E chdir <errno>`: `reap(cmd)`, return `&os.PathError{Op: "chdir", Path: wd, Err: syscall.Errno(n)}`. No tenant code ran. |
| + - `E exec <errno>` (after `R`): `reap(cmd)`, return `&os.PathError{Op: "fork/exec", Path: path, Err: syscall.Errno(n)}`. No tenant code ran: `execve` returned. |
| + - EOF with no `R`: the shim died after placement and before `R`. No tenant code ran. `reap(cmd)`, return a placement error, "the shim exited before exec". |
| + - `R` then EOF: started. Close `statusR`, return `false, nil`. |
| + - deadline with `R` already read: started (an `execve` still loading). Close `statusR`, return `false, nil`, and do not kill. |
| + - deadline with no `R`: SIGKILL the shim, then read `statusR` to EOF (the shim is dead, so the write end is closed). An `R` in what was read: started, return `false, nil`; the caller's `Wait` will show the SIGKILL. No `R`: `reap(cmd)`, placement error. No tenant code ran. |
| +10. `reap(cmd)`: `cmd.Process.Kill()`, then `cmd.Process.Wait()`, which reaps the PID and is bounded (no tenant ran, so nothing else holds the child's pipes open once it is dead), then `go cmd.Wait()` so exec.Cmd's copying goroutines and pipes are released in the background without the caller's I/O holding this call (`os/exec/exec.go:979`). A failed start's `cmd` is never handed back. |
| +11. Every return closes `specW` and `statusR` exactly once (a single deferred close of whichever are still open). |
| |
| Shim, in `runShim(args []string) int` (args are the three fd numbers): |
| 1. Parse the three fds; they must be `keep`, `keep+1`, `keep+2` for some `keep >= 3`. Set `FD_CLOEXEC` on the status fd first, so every later path closes it at exec. |
| -2. Read the spec fd to EOF, at most 16 MiB, and close it. Decode it. |
| +2. Read the 8-byte length (at most 16 MiB) and then exactly that many bytes from the spec fd. Decode them. Keep the spec fd open: the release byte comes on it. |
| 3. Mark every fd `>= keep` close-on-exec: `unix.CloseRange(uint(keep), math.MaxUint32, unix.CLOSE_RANGE_CLOEXEC)`; if that answers `ENOSYS`, `EPERM` or `EINVAL`, walk `/proc/self/fd` and set `FD_CLOEXEC` on each fd `>= keep`. This covers the spec, directory and status fds and anything the agent leaked without close-on-exec. |
| 4. `unix.Fstatfs(dirFD)` must be `CGROUP2_SUPER_MAGIC`. |
| 5. `unix.Openat(dirFD, "cgroup.procs", O_WRONLY|O_NOFOLLOW|O_CLOEXEC, 0)`; write `strconv.Itoa(os.Getpid())`; close. This moves the whole process (every Go runtime thread) while it is still carlos code. |
| 6. Read `/proc/self/cgroup`; `ParseSelfCgroup`; `"/"+rel` must equal the spec's `Expected`. |
| 7. `unix.Fstat(dirFD)` and `unix.Lstat("/sys/fs/cgroup"+Expected)` must agree on `Dev` and `Ino`, so the cgroup the kernel reports is the directory the parent validated. |
| 8. Any failure in 1 to 7: write `P <step>: <error>\n` to the status fd and return 125. Tenant code has not run. |
| -9. Write `R\n`, then `syscall.Exec(spec.Path, spec.Argv, spec.Env)`. `syscall.Exec` restores the original `RLIMIT_NOFILE` (Go source `syscall/exec_unix.go:286`). If it returns, write `E <errno>\n` and return 126. |
| +9. Close the directory fd. Write `A\n`. |
| +10. Read one byte from the spec fd. Anything but `X` (EOF, an error, another byte): return 125 without writing more. The parent refused or is gone. |
| +11. If the spec's `Dir` is not empty, `os.Chdir(Dir)`; a failure writes `E chdir <errno>\n` and returns 126. |
| +12. Restore the exec state a direct start would have: `runtime.LockOSThread()`, then set this thread's signal mask to empty (`unix.PthreadSigmask(unix.SIG_SETMASK, &unix.Sigset_t{}, nil)`). Go's own forked child restores the mask the agent started with (`m.sigmask`), and systemd starts every service with an empty mask, so empty matches a direct start under systemd. Dispositions need no work: `execve` resets every signal the shim's runtime handles to default, and a signal the agent inherited as ignored (`SIGHUP`, `SIGINT`) is inherited ignored by both a direct child and the shim, whose runtime keeps it ignored. `syscall.Exec` restores the original `RLIMIT_NOFILE` (Go source `syscall/exec_unix.go:286`). `Pdeathsig`, the process group, and ambient capabilities are set at the shim's fork and survive both `execve`s exactly as they survive one. |
| +13. Write `R\n`, then `syscall.Exec(spec.Path, spec.Argv, spec.Env)`. If it returns, write `E exec <errno>\n` and return 126. |
| |
| The shim writes nothing to fds 0 to 2. |
| |
| `runProbe() int` (for Task 3): read `/proc/self/cgroup`, print `"/"+rel` and a newline to stdout, return 0; return 1 when there is no v2 line. |
| |
| -**Spec encoding** (`shim.go`): fields each terminated by `\x00`, in order: the magic `carlos-cgexec-1`, `Path`, `Expected`, `KeepFDs` (decimal), `TestHold` (`0` or `1`), `len(Argv)` (decimal), each argv element, `len(Env)` (decimal), each env entry. Encoding refuses any string that contains `\x00` (execve could not carry it anyway). Decoding refuses a wrong magic, a short or trailing buffer, or a count above 1<<20. |
| +**The in-flight registry** (`inflight.go`, used by Task 2's proofs): a mutex-guarded `map[string]int` of target directories (cleaned paths), plus a `chan struct{}` that is closed and replaced whenever a count drops, so waiters wake. |
| + |
| +```go |
| +func (m *Manager) beginPlacement(dir string) (done func()) // done is idempotent |
| +func (m *Manager) placementsUnder(dir string) int // targets equal to dir or below dir+"/" |
| +func (m *Manager) placementsAt(dir string) int // targets equal to dir |
| +func (m *Manager) waitPlacementsUnder(ctx context.Context, dir string) error // nil at zero, ctx.Err() if ctx ends first |
| +``` |
| + |
| +A nil `*Manager` answers zero and nil, and its `beginPlacement` returns a no-op. |
| + |
| +**Spec encoding** (`shim.go`): fields each terminated by `\x00`, in order: the magic `carlos-cgexec-1`, `Path`, `Dir`, `Expected`, `KeepFDs` (decimal), `TestHold` (decimal, `0` for none), `len(Argv)` (decimal), each argv element, `len(Env)` (decimal), each env entry. Encoding refuses any string that contains `\x00` (execve could not carry it anyway). Decoding refuses a wrong magic, a short or trailing buffer, or a count above 1<<20. |
| |
| ```go |
| type shimSpec struct { |
| Path string |
| + Dir string // the tenant's cmd.Dir; the shim changes to it after placement |
| Argv []string |
| Env []string |
| Expected string // "/..." as /proc/self/cgroup shows it after "0::" |
| KeepFDs int // fds 0..KeepFDs-1 are the tenant's |
| - TestHold bool // test binaries only: the shim waits for SIGUSR1 before step 4 |
| + TestHold int // test binaries only: 0 none, 1 hold before step 4, 2 hold after step 10 (before R) |
| } |
| |
| func encodeShimSpec(s shimSpec) ([]byte, error) |
| func decodeShimSpec(b []byte) (shimSpec, error) |
| ``` |
| |
| -`TestHold` is honoured only when `testing.Testing()` is true in the shim process (the package already imports `testing`, `cgroup.go:30`), so in the carlos binary it is inert. When honoured, the shim sleeps before step 4 until it is signalled, with default signal dispositions (no `signal.Notify`): a test's SIGTERM or SIGKILL then kills it before placement, which is the case under test. The parent sets the field from `var shimTestHold bool`, which only test files flip. |
| +`TestHold` is honoured only when `testing.Testing()` is true in the shim process (the package already imports `testing`, `cgroup.go:30`), so in the carlos binary it is inert. When honoured, the shim sleeps at that point until it is signalled, with default signal dispositions (no `signal.Notify`): a test's SIGTERM or SIGKILL kills it there. The parent sets the field from `var shimTestHold int`, and calls `var shimAfterRelease func()` (nil in production) between writing the release byte and reading the status, which only test files set. |
| |
| - [ ] **Step 1: Write the failing tests** |
| |
| @@ -260,7 +285,7 @@ func TestShimPlacementFailureIsReported(t *testing.T) { |
| if !isPlacementError(err) { |
| t.Fatalf("err=%v, want a placement error", err) |
| } |
| - if cmd.ProcessState == nil { |
| + if _, err := os.Stat(fmt.Sprintf("/proc/%d", cmd.Process.Pid)); err == nil { |
| t.Fatal("the shim was not reaped") |
| } |
| if _, err := os.Stat(marker); err == nil { |
| @@ -296,42 +321,69 @@ func TestPlacementsUnderCountsTheSubtree(t *testing.T) { |
| func TestWaitPlacementsUnderWakes(t *testing.T) { ... one in flight; a waiter with a 50 ms ctx returns context.DeadlineExceeded; a second waiter returns nil within 1 s of done() ... } |
| ``` |
| |
| -The exec-failure case needs a placement that succeeds, so it lives in the kernel file below. This file also covers the status parser: |
| +The exec-failure case needs a placement that succeeds, so it lives in the kernel file below. This file also covers the two status classifiers, which `shim_linux.go` factors out of `startViaShim`: |
| |
| ```go |
| -func TestShimStatusParsing(t *testing.T) { |
| +// classifyPlacement reads the stream up to placement (parent step 7). |
| +// placed is true only for "A". timedOut is the read deadline having passed. |
| +func classifyPlacement(b []byte, timedOut bool) (placed bool, err error) |
| + |
| +// classifyExec reads the stream after the release byte (parent step 9). |
| +// killed means the deadline passed with no R, the shim was SIGKILLed, and |
| +// b is everything read until EOF after that. |
| +func classifyExec(path, wd string, b []byte, killed bool) (started bool, err error) |
| + |
| +func TestShimStatusClassification(t *testing.T) { |
| for _, c := range []struct { |
| - in string |
| - placement bool |
| - execErrno syscall.Errno |
| - ok bool |
| + in string |
| + timedOut bool |
| + placed bool |
| }{ |
| - {"R\n", false, 0, true}, |
| - {"", true, 0, false}, |
| - {"P procs: permission denied\n", true, 0, false}, |
| - {"R\nE 2\n", false, syscall.ENOENT, false}, |
| - {"R\nE 13\n", false, syscall.EACCES, false}, |
| - {"garbage", true, 0, false}, |
| - } { |
| - err := parseShimStatus("/bin/app", []byte(c.in)) |
| - ... assert ok => err == nil; placement => isPlacementError(err); execErrno => errors.As *os.PathError with Op "fork/exec", Path "/bin/app", Err == execErrno ... |
| - } |
| + {"A\n", false, true}, |
| + {"P procs: permission denied\n", false, false}, |
| + {"", false, false}, // died before placement |
| + {"", true, false}, // deadline before placement |
| + {"garbage\n", false, false}, |
| + } { ... placed must match; !placed must give isPlacementError(err) ... } |
| + |
| + for _, c := range []struct { |
| + in string |
| + killed bool |
| + started bool |
| + op string // "" for a placement error, else the *os.PathError's Op |
| + errno syscall.Errno |
| + }{ |
| + {"R\n", false, true, "", 0}, |
| + {"R\n", true, true, "", 0}, // R then killed: may have exec'd, so started |
| + {"", false, false, "", 0}, // died after release, before R: placement error |
| + {"", true, false, "", 0}, // killed before R: placement error |
| + {"E chdir 2\n", false, false, "chdir", syscall.ENOENT}, |
| + {"R\nE exec 2\n", false, false, "fork/exec", syscall.ENOENT}, |
| + {"R\nE exec 7\n", false, false, "fork/exec", syscall.E2BIG}, |
| + {"garbage\n", false, false, "", 0}, |
| + } { ... started must match; when !started, op "" means isPlacementError(err), else errors.As *os.PathError with that Op, Path "/bin/app" for fork/exec and "/wd" for chdir, and Err == errno ... } |
| } |
| ``` |
| |
| -`internal/cgroup/shim_kernel_linux_test.go` (gated: `CARLOS_SHIM_KERNEL=1` inside a delegated unit; reuse `genKernelManager`'s setup at `gen_kernel_linux_test.go:27`, and skip without the variable). Each test places into `m.root/<name>` made with `os.Mkdir` and a `TierDefault` limit written through `m.applyLimit(..., true)`: |
| +`internal/cgroup/shim_kernel_linux_test.go` (gated: `CARLOS_SHIM_KERNEL=1` inside a delegated unit, with `genKernelManager`'s setup at `gen_kernel_linux_test.go:27`; without the variable every test skips, but WITH it any delegation failure is `t.Fatal`, never a skip, so a run that was asked for cannot pass by skipping). Each test places into `m.root/<name>` made with `os.Mkdir` and a `TierDefault` limit written through `m.applyLimit(..., true)`. Tenants that must not run touch a marker file first thing, and each such test asserts the marker is absent: |
| |
| - `TestShimKernelPlacesBeforeExec`: tenant `/bin/sh -c 'cat /proc/self/cgroup; echo $$'`. After `startViaShim` returns: `/proc/<pid>/cgroup` names the target; the tenant's printed cgroup is the target; the tenant's printed `$$` equals `cmd.Process.Pid` (the leader PID is the shim's, now the tenant's); `cmd.Wait()` returns nil. |
| - `TestShimKernelExitCodePassesThrough`: tenant `/bin/sh -c 'exit 7'`; `cmd.Wait()` gives an `*exec.ExitError` with `ExitCode() == 7`. |
| -- `TestShimKernelPassesArgvEnvExactly`: the test binary as tenant in a child mode (`CARLOS_SHIM_CHILD=dump`, env carrying Review Focus 1's values) writes `os.Args` and `os.Environ()` as NUL-joined bytes to stdout; compare against the same command started directly with `cmd.Start()`; also compare its `os.Getwd()` with `cmd.Dir`. |
| -- `TestShimKernelFDsExact`: `ExtraFiles` of two pipes; the agent side also opens one file with `syscall.Open` without `O_CLOEXEC` before the start (closed in Cleanup). The child mode lists `/proc/self/fd` (excluding the fd its own listing opens): exactly `0 1 2 3 4`. Also `SigBlk`, `SigIgn` from `/proc/self/status` equal a direct start's. |
| -- `TestShimExecFailureIsTheCommands`: tenant path `/nonexistent/app` (set `cmd.Path` directly so `LookPath` does not fail first): error is `*os.PathError` with `Err == syscall.ENOENT`, `!isPlacementError(err)`, `cmd.ProcessState != nil`. |
| -- `TestShimKernelPlacementFailureNeverRunsTenant`: target a cgroup the shim cannot write (`chmod 0444` its `cgroup.procs`, restore in Cleanup): placement error, marker file absent, reaped. |
| -- `TestShimKernelSignalBeforeExec`: `shimTestHold = true`; a `startCmd` seam wrapper (see `cgroup.go:332`) starts the shim and then sends SIGTERM to it; the result is a placement error ("exited before exec"), the marker is absent, `cmd.ProcessState` is set. Then the same with SIGKILL. |
| -- `TestShimKernelDeadline`: `shimTestHold = true`, `shimDeadline = 300ms` (restored in Cleanup), no signal: a placement error naming the deadline, the shim reaped, marker absent. |
| -- `TestShimKernelSurvivesDeletedExecutable`: copy `os.Args[0]` to `t.TempDir()/copy.test`, run the copy as a child test (`-test.run '^TestShimKernelDeletedChild$'`, env `CARLOS_SHIM_KERNEL=1 CARLOS_SHIM_DELETED_CHILD=1`); the child deletes its own executable, then places `/bin/true` through `startViaShim` and must succeed. |
| +- `TestShimKernelPassesArgvEnvExactly`: the test binary as tenant in a child mode (`CARLOS_SHIM_CHILD=dump`) writes `os.Args` and `os.Environ()` as NUL-joined bytes to stdout. The env carries `EMPTY=`, `EQ=a=b`, `BAD=\xff\xfe` and a 100 KiB value; argv carries `""`, `"two words"` and `"line\nbreak"`. Compare byte-for-byte against the same command started directly with `cmd.Start()`, and compare its `os.Getwd()` with `cmd.Dir`. Then a 200 KiB single value (above Linux's 32-page per-string limit): both the shim start and the direct start fail with `E2BIG`, the shim's as `*os.PathError{Op: "fork/exec"}`, not a placement error. |
| +- `TestShimKernelInheritedStateMatchesADirectStart`: before starting, the test process runs `signal.Ignore(syscall.SIGHUP)` (restored with `signal.Reset` in Cleanup), and the command sets `SysProcAttr{Setpgid: true, Pdeathsig: syscall.SIGKILL}` plus two `ExtraFiles` pipes. The agent side also opens one file with `syscall.Open` without `O_CLOEXEC` (closed in Cleanup). The child mode (`CARLOS_SHIM_CHILD=state`) prints: its `/proc/self/fd` listing (minus the listing's own fd), `SigBlk`, `SigIgn`, `CapInh`, `CapPrm`, `CapEff`, `CapBnd`, `CapAmb` and `NoNewPrivs` from `/proc/self/status`, `PR_GET_PDEATHSIG`, `getpgid(0) == getpid()`, and `RLIMIT_NOFILE` soft and hard. Every line equals a direct start's, and the fd listing is exactly `0 1 2 3 4`. |
| +- `TestShimKernelChdirFailureIsTheCommands`: `cmd.Dir` names a missing directory: the error is `*os.PathError{Op: "chdir"}` with `ENOENT`, not a placement error; reaped; marker absent. |
| +- `TestShimExecFailureIsTheCommands`: tenant path `/nonexistent/app` (set `cmd.Path` directly so `LookPath` does not fail first): `*os.PathError{Op: "fork/exec"}` with `ENOENT`, `!isPlacementError(err)`, the PID is gone from `/proc`. |
| +- `TestShimKernelPlacementFailureNeverRunsTenant`: target a cgroup the shim cannot write (`chmod 0444` its `cgroup.procs`, restore in Cleanup): placement error, marker absent, reaped. |
| +- `TestShimKernelBeforeRunsAfterPlacement`: `before` reads the target's `cgroup.events` and records `populated 1` (the shim is in place when it is asked); then a second run whose `before` refuses: `refused == true`, before's error returned, marker absent, reaped, nothing registered. |
| +- `TestShimKernelSignalBeforePlacement`: `shimTestHold = 1`; a `startCmd` seam wrapper (`cgroup.go:332`) starts the shim and sends it SIGTERM; a placement error ("exited before placement"), marker absent, reaped. Then the same with SIGKILL. |
| +- `TestShimKernelSignalBeforeExec`: `shimTestHold = 2` (after the release byte, before `R`); SIGKILL it from `shimAfterRelease`: a placement error ("exited before exec"), marker absent, reaped. |
| +- `TestShimKernelExecBeforeAcknowledgement`: `shimDeadline = 200ms` (restored in Cleanup) and a `shimAfterRelease` hook that waits until the tenant's marker exists and then sleeps 400 ms, past the deadline. The tenant ran, so the start must be reported started: nil error, `cmd.Wait()` succeeds. This pins the rule that an exec that happened is never reported as a pre-exec failure. |
| +- `TestShimKernelDeadlineBeforePlacement`: `shimTestHold = 1`, `shimDeadline = 300ms`, no signal: a placement error naming the deadline, reaped, marker absent. |
| +- `TestShimKernelSurvivesDeletedExecutable`: copy `os.Args[0]` to `t.TempDir()/copy.test`, run the copy as a child test (`-test.run '^TestShimKernelDeletedChild$'`, env `CARLOS_SHIM_KERNEL=1 CARLOS_SHIM_DELETED_CHILD=1 CARLOS_GEN_KERNEL=1`); the child deletes its own executable, then places `/bin/true` through `startViaShim` and must succeed. |
| - `TestShimKernelOverhead` (informational, never fails on time): 20 shim starts of `/bin/true` against 20 direct starts; `t.Logf` the medians. |
| |
| +A reported kernel run must show `--- PASS` for every test named here and no `--- SKIP` lines; paste the `-test.v` output. |
| + |
| - [ ] **Step 2: Run the tests to see them fail** |
| |
| Run: `cd <worktree> && GOCACHE=$TMPDIR/gocache go test ./internal/cgroup -run 'Shim' -count=1` |
| @@ -401,7 +453,7 @@ var ErrPlacementInFlight = errors.New("cgroup: a process is being placed in this |
| |
| `shimAvailable()` is `shimHooked.Load()` in `shim_linux.go` and `false` in `shim_other.go`. `shim_other.go`'s `runShim`/`runProbe` return 127, and its `startViaShim` returns `false, &placementError{errors.New("placement before exec needs linux")}`. |
| |
| -`shim_linux.go` implements the parent and shim protocol above. Factor the status decision into `parseShimStatus(path string, b []byte) error` so `TestShimStatusParsing` covers it. Keep every comment to what the code does and why. |
| +`shim_linux.go` implements the parent and shim protocol above. Factor the status decisions into `classifyPlacement` and `classifyExec` so `TestShimStatusClassification` covers them. Keep every comment to what the code does and why. |
| |
| `main.go`, first statement of `main()`: |
| |
| @@ -423,18 +475,18 @@ Expected: PASS; the kernel tests SKIP. |
| |
| Run (unsandboxed, kernel): |
| ```bash |
| -GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/cgroup.test ./internal/cgroup && \ |
| -systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k -E CARLOS_SHIM_KERNEL=1 -E CARLOS_GEN_KERNEL=1 \ |
| - /tmp/k/cgroup.test -test.run 'ShimKernel|ShimExecFailure' -test.v -test.count=1 |
| +mkdir -p /tmp/k/zddr3 && GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/zddr3/cgroup.test ./internal/cgroup && \ |
| +systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k/zddr3 -E CARLOS_SHIM_KERNEL=1 -E CARLOS_GEN_KERNEL=1 \ |
| + /tmp/k/zddr3/cgroup.test -test.run 'ShimKernel|ShimExecFailure' -test.v -test.count=1 |
| ``` |
| -Expected: every `ShimKernel` test PASS, with the overhead medians in the log. Paste the real output into the report. Then `rm /tmp/k/cgroup.test`. |
| +Expected: every `ShimKernel` test PASS, with the overhead medians in the log. Paste the real output into the report. Then `rm -rf /tmp/k/zddr3`. |
| |
| Also: `GOOS=darwin GOCACHE=$TMPDIR/gocache go build ./... && GOOS=windows GOCACHE=$TMPDIR/gocache go build ./...` and `go vet ./internal/cgroup .` |
| |
| - [ ] **Step 5: Commit** |
| |
| ```bash |
| -git add internal/cgroup/shim*.go internal/cgroup/main_test.go main.go |
| +git add internal/cgroup/shim*.go internal/cgroup/inflight*.go internal/cgroup/main_test.go internal/cgroup/cgroup.go main.go |
| git -c user.name=claude -c user.email=claude~paul@keymail.dev commit -m "cgroup: the placement exec-shim (#272)" |
| ``` |
| |
| @@ -458,13 +510,13 @@ git -c user.name=claude -c user.email=claude~paul@keymail.dev commit -m "cgroup: |
| - Flat: make the directory (failure: start plain, as today `cgroup.go:465-466`) and apply the limit best-effort as today. If `!shimAvailable()`, go straight to `startAndMoveIn` (today's fallback). Otherwise place through the shim. Success: `biasOOM`, return. Refused: as today. A placement error: log `cgroup: <host>: placement before exec failed (<err>) — using the PID-move fallback for this spawn` and `startAndMoveIn` with a fresh `build()` behind `before` (Ruling 2). Any other error is the command's: remove the directory if empty and return it, with no retry. |
| - `Manager.fallback`, the read-once comment and the unreachable required-path guard at `cgroup.go:538-546` go; keep a single explicit statement that the required path never reaches `startAndMoveIn`. |
| |
| -`FakePlacementForTest`: keep its name, signature, panic outside tests and no-`t.Parallel` note; it now swaps `placeStart` for one that runs `startGated(cmd, before)` with no shim (as if the kernel had placed it) and restores it in Cleanup. Update its doc. |
| +`FakePlacementForTest`: keep its name, signature, panic outside tests and no-`t.Parallel` note; it now swaps `placeStart` for one that runs `startGated(cmd, before)` with no shim (as if the kernel had placed it), AND sets a test-scope override that makes `shimAvailable()` answer true, restoring both in Cleanup. The override is safe because the fake never runs a shim, so it cannot make a test binary without the hook re-run itself (Astra plan 7; the activator's `TestMain` at `internal/activator/activator_test.go:5894` has no hook, and `TestSpawnInCgroupAtPutsAGenerationUnderItsParent`, `internal/activator/containment_test.go:113`, must keep passing through the fake). Update its doc. |
| |
| `startAndMoveIn` (the flat fallback, `cgroup.go:625`) registers its target too, from after `before` agrees until the PID write has returned: its child runs in the agent's cgroup for that window, and a proof must not miss it either. Give `startGated` an optional register step so both paths take the registry at the same point. |
| |
| -Cleanup (`cleanup_linux.go`): `openCleanupAt` records the manager and the directory path it pinned (`filepath.Join(m.root, ExecName(host))`, plus `/child` for a child cleanup) in `cleanupState`. `Populated` answers `true, nil` while `m.placementsUnder(path) > 0`, before reading `cgroup.events`. `Members` answers `nil, ErrPlacementInFlight` while one is in flight. `KillAndEmpty` first calls `m.waitPlacementsUnder(ctx, path)`, returning its error (unproven) if ctx ends first, then does what it does today. |
| +Cleanup (`cleanup_linux.go`): `openCleanupAt` records the manager and the directory path it pinned (`filepath.Join(m.root, ExecName(host))`, plus `/child` for a child cleanup) in `cleanupState`. The registry goes into the shared primitives, not only the public methods, because `KillAndEmpty` calls them directly (`cleanup_linux.go:334`, `:338`): `populationLocked` answers `true, nil` while `m.placementsUnder(path) > 0`, before reading `cgroup.events`, and `membersLocked` answers `nil, ErrPlacementInFlight` while one is in flight. `KillAndEmpty` first calls `m.waitPlacementsUnder(ctx, path)` (an error there is unproven), and because every emptiness check inside its loop now goes through the registry-aware `populationLocked`, a placement that registers after the wait keeps the proof from finishing until the placed process has been killed with the rest or the ctx ends (unproven). Write the doc comment on `KillAndEmpty` saying so, and that callers still exclude new starts during a proof the way they do today (the lease and the `before` hook); the registry only closes the window a shim spends outside its cgroup. |
| |
| -`ensureParent` (`gen.go:198`): after `parentProcs`, add `m.placementsAt(dir)` to `n`, so a flat start being placed into the host directory itself counts as a direct process and the generation start is refused with `ErrLegacyParentPopulated`. |
| +`ensureParent` (`gen.go:198`): after `parentProcs`, add `m.placementsAt(dir)` to `n`, so a flat start being placed into the host directory itself counts as a direct process and the generation start is refused with `ErrLegacyParentPopulated`. The `parentPopulated(dir)` branch at `gen.go:213` also counts `m.placementsUnder(dir) > 0` as populated, so a sibling generation being placed is never taken for a cold parent whose limits may be rewritten (`gen.go:219-229`). |
| |
| Check every caller of `Cleanup.Members` (`grep -rn "\.Members()" internal`) treats an error as unproven (keeps the lease, records nothing as empty). If one does not, fix that caller in this task and name it in the report. |
| |
| @@ -472,6 +524,10 @@ Check every caller of `Cleanup.Members` (`grep -rn "\.Members()" internal`) trea |
| |
| `TestCleanupCountsPlacementInFlight` (in `cleanup_linux_test.go`, over the existing fake-root helper at `cleanup_linux_test.go:28`, which builds `&Manager{root: root}` — give it `logf: t.Logf`): with a placement begun on `<host dir>/g1`, a host Cleanup's `Populated()` is `true` even though the fake `cgroup.events` says `populated 0`; `Members()` is `ErrPlacementInFlight`; `KillAndEmpty` with a 50 ms ctx returns an error and writes nothing to `cgroup.kill`; after `done()`, `Populated()` reads the file again. |
| |
| +`TestKillAndEmptySeesAPlacementRegisteredMidProof`: no placement when `KillAndEmpty` starts; through the kill-write seam the cleanup tests already use (or one added for this test), register a placement on `<host dir>/g1` right after the kill is written. With a 100 ms ctx, `KillAndEmpty` must return an error (unproven), never nil, although the fake `cgroup.events` says `populated 0`. |
| + |
| +`TestEnsureParentCountsAPlacementBelowTheParent` (in `gen_test.go`): a placement in flight on `<host dir>/g1` while `StartChildGated` starts `g2` on a parent whose fake `cgroup.events` says `populated 0`: the parent's limit files are not rewritten (the cold-parent branch is not taken). |
| + |
| `TestEnsureParentCountsAPlacementAtTheParent` (in `gen_test.go`): a placement begun on the host directory itself makes `StartChildGated` answer `ErrLegacyParentPopulated` and build nothing. |
| |
| `TestGenerationMissingBinaryIsNotABoxCondition` (in `gen_test.go`, with a `placeStart` stub returning `&os.PathError{Op: "fork/exec", Path: "/x", Err: syscall.ENOENT}`): `StartChildGated` returns that error, `errors.Is(err, ErrLimitNotApplied)` is false, and the child directory is gone. |
| @@ -506,9 +562,9 @@ mkdir -p /tmp/k && TMPDIR=/tmp/k GOCACHE=$HOME/.cache/go-build nice -n 19 go tes |
| ``` |
| Then the generation kernel tests in a delegated user unit, which now place through the shim: |
| ```bash |
| -GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/cgroup.test ./internal/cgroup && \ |
| -systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k -E CARLOS_GEN_KERNEL=1 -E CARLOS_SHIM_KERNEL=1 \ |
| - /tmp/k/cgroup.test -test.run 'GenKernel|GenOOMKernel|ShimKernel|GenerationStartRefusedBesideADirectParentMember|ClaimContainment' -test.v -test.count=1; rm -f /tmp/k/cgroup.test |
| +mkdir -p /tmp/k/zddr3 && GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/zddr3/cgroup.test ./internal/cgroup && \ |
| +systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k/zddr3 -E CARLOS_GEN_KERNEL=1 -E CARLOS_SHIM_KERNEL=1 \ |
| + /tmp/k/zddr3/cgroup.test -test.run 'GenKernel|GenOOMKernel|ShimKernel|GenerationStartRefusedBesideADirectParentMember|ClaimContainment' -test.v -test.count=1; rm -rf /tmp/k/zddr3 |
| ``` |
| Expected: PASS (paste the real output). A test that asserted clone3-specific kernel behaviour (the `cgroup.kill` respawn kill was clone3's) is updated to assert the shim's behaviour on the same scenario, and the report says what changed and why. |
| |
| @@ -539,7 +595,7 @@ git -c user.name=claude -c user.email=claude~paul@keymail.dev commit -am "cgroup |
| 2. `dir := filepath.Join(m.root, placementCheckName)`. `os.Remove(dir)` (a leftover from an earlier run is empty: the unit's restart killed its members); then `os.Mkdir(dir, 0o755)`; `defer` a removal that waits for the probe to be reaped first. |
| 3. `m.applyLimit("placement check", dir, TierDefault, true)`; an error is returned. |
| 4. `cmd := exec.Command(shimExe, ProbeVerb)` with `Env = []string{}` and `Stdout` a buffer. `m.startViaShim(dir, cmd, nil)`. Any error is returned (a placement error or not: either means the check failed). |
| -5. Wait for the probe with `ctx` (kill and reap on `ctx.Done()`). It must exit 0 and print exactly `expected` (the same string `startViaShim` computed: `dir` without `/sys/fs/cgroup`). |
| +5. Wait for the probe with `ctx` (kill and reap on `ctx.Done()`). It must exit 0 and its stdout must be exactly `expected + "\n"` (`expected` is the string `startViaShim` computed: `dir` without `/sys/fs/cgroup`). |
| 6. `memory.max` in `dir` must read back `TierDefault.MemoryMax`. |
| 7. Remove `dir`; a removal failure is logged, not returned. |
| |
| @@ -596,10 +652,12 @@ and make the existing armed-case tests (`rollout_test.go:526`, `:568` and any ot |
| 2. Sort each key into one of two explicit tables in the test: **applied** (passed as `-p Key=value` to `systemd-run --user`) and **excluded** (with a one-line reason each). A `[Service]` key in neither table fails the test, so a new hardening line forces a decision here. |
| 3. The applied table must contain at least: `RestrictNamespaces`, `SystemCallFilter`, `SystemCallErrorNumber`, `SystemCallArchitectures`, `NoNewPrivileges`, `MemoryDenyWriteExecute`, `LockPersonality`, `RestrictRealtime`, `RestrictAddressFamilies`, `ProtectProc`, `ProtectClock`, `ProtectHostname`, `ProtectKernelTunables`, `ProtectKernelModules`, `ProtectKernelLogs`, `PrivateDevices`, `ProtectSystem`, `TasksMax`. The test asserts each is present in the unit file and applied. If one of these cannot be applied in a user unit on DOCS-T1, move it to excluded with the real `systemd-run` error as its reason, and say so in the report: Task 7 carries it into the dev re-check. |
| 4. Expected exclusions, each with its reason: `ExecStart`, `ExecReload`, `Restart`, `RestartSec`, `EnvironmentFile` (process control, not sandbox); `User`, `Group`, `AmbientCapabilities`, `CapabilityBoundingSet` (a user manager cannot change identity or capabilities); `ReadWritePaths` (names box paths absent here); `Slice`, `OOMPolicy`, `Delegate` (the test passes its own `Delegate=yes`); `ProtectHome`, `PrivateTmp` only if the test binary cannot be made visible inside them (prefer keeping them and making the binary visible with `-p BindReadOnlyPaths=<binary>`; say which in the report). |
| -5. Run `systemd-run --user --wait --pipe --collect -p Delegate=yes <applied...> -E CARLOS_EDGE_HARDENING_INNER=1 -E CARLOS_SHIM_KERNEL=1 -E CARLOS_GEN_KERNEL=1 <binary> -test.run '^TestEdgeHardeningInner$' -test.v -test.count=1` and fail unless the inner run passes; stream its output to the test log. |
| +5. **Unrestricted control first** (Astra plan 6): run the inner test in a unit with only `-p Delegate=yes` and `-E CARLOS_EDGE_HARDENING_INNER=control`. There clone3 must WORK: the raw `clone3` probe answers `EINVAL` (not `ENOSYS`), and a plain `exec.Cmd` with `SysProcAttr{UseCgroupFD: true, CgroupFD: <a scratch cgroup fd>}` starts and lands in that cgroup. If the control fails, the hardened run below proves nothing, so the driver fails. |
| +6. **Hardened run:** `systemd-run --user --wait --pipe --collect -p Delegate=yes <applied...> -E CARLOS_EDGE_HARDENING_INNER=hardened -E CARLOS_SHIM_KERNEL=1 -E CARLOS_GEN_KERNEL=1 <binary> -test.run '^TestEdgeHardeningInner$' -test.v -test.count=1`; fail unless it passes. Stream both runs' output to the test log. |
| |
| -The inner test, gated by `CARLOS_EDGE_HARDENING_INNER=1`: |
| -1. **Positive control, the filter is on:** `unix.Syscall(unix.SYS_CLONE3, 0, 0, 0)` answers `ENOSYS`. And a plain `exec.Cmd` with `SysProcAttr{UseCgroupFD: true, CgroupFD: <a scratch cgroup fd>}` fails to start with `ENOSYS`. If either succeeds the test fails: the unit is not applying `RestrictNamespaces`, and nothing else here would prove anything. |
| +The inner test, gated by `CARLOS_EDGE_HARDENING_INNER` (`control` or `hardened`). Once the variable is set, nothing in it skips: a delegation failure, or any precondition it cannot establish, is `t.Fatal`. In `hardened` mode: |
| +0. **What the unit enforces, observed from inside:** `/proc/self/status` shows `Seccomp: 2` and `NoNewPrivs: 1`; a write under `/usr` fails with `EROFS` (`ProtectSystem=strict`); `/proc/1/cmdline` is unreadable or absent (`ProtectProc=invisible`); `socket(AF_NETLINK, ...)` fails (`RestrictAddressFamilies`). Print a table of every applied property marked `enforced` (observed here) or `untested` (accepted by systemd but not observed). A property in `untested` is listed in the report so Task 7 carries it to the dev re-check on the real unit. |
| +1. **The filter is on:** the raw `clone3` probe answers `ENOSYS`, and the plain `UseCgroupFD` start fails with `ENOSYS`, where the control run showed both working. |
| 2. **Placement succeeds:** `StartChildGated("hardening.example", GenChild(1), lim, lim, build, nil)` with `lim := Limit{MemoryMax: 64 << 20}` and a tenant that prints its cgroup then waits on stdin: the cgroup is `.../carlos-exec@hardening.example/g1`, and the PID the tenant prints is `cmd.Process.Pid`. |
| 3. **The limit applies:** the existing `hog` child mode (`gen_kernel_linux_test.go:65`) started the same way into `g2` is OOM-killed: `memory.events` in `g2` shows `oom_kill` of at least 1, the process dies by SIGKILL, and `g1`'s tenant is still alive. |
| 4. **Arming would arm:** `m.VerifyPlacement(ctx)` returns nil. |
| @@ -608,8 +666,8 @@ The inner test, gated by `CARLOS_EDGE_HARDENING_INNER=1`: |
| - [ ] **Step 2: Run it against a branch where it must fail**, to prove it can: temporarily make `placeStart` use the old clone3 start (a local, uncommitted edit, `setInto` + `cmd.Start()`); the inner step 2 must fail with `ENOSYS`/`ErrLimitNotApplied`. Paste that output, then revert the edit (`git diff` must show only the new test file). |
| - [ ] **Step 3: Run it for real**: |
| ```bash |
| -GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/cgroup.test ./internal/cgroup && \ |
| -cd internal/cgroup && CARLOS_EDGE_HARDENING_KERNEL=1 TMPDIR=/tmp/k /tmp/k/cgroup.test -test.run '^TestEdgeHardeningPlacement$' -test.v -test.count=1; rm -f /tmp/k/cgroup.test |
| +mkdir -p /tmp/k/zddr3 && GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/zddr3/cgroup.test ./internal/cgroup && \ |
| +cd internal/cgroup && CARLOS_EDGE_HARDENING_KERNEL=1 TMPDIR=/tmp/k/zddr3 /tmp/k/zddr3/cgroup.test -test.run '^TestEdgeHardeningPlacement$' -test.v -test.count=1; rm -rf /tmp/k/zddr3 |
| ``` |
| (unsandboxed). Expected: PASS, with the applied and excluded tables printed. Paste the real output. |
| - [ ] **Step 4: Commit** — `"cgroup: kernel test, shim placement under carlos-edge's sandbox lines (#272)"`. |
| @@ -668,7 +726,7 @@ func TestProbeServesWithThePlatformsRunArgv(t *testing.T) { |
| func TestProbeServesWithVetsArgv(t *testing.T) { ... launch(t, "-socket", sock) (vet.go:135) answers ok ... } |
| ``` |
| |
| -`getOverUnix` is a small helper with an `http.Client` whose transport dials the unix socket. The drain check: start a request, send SIGTERM 10 ms later, and assert the request answers 200 and `cmd.Wait()` returns nil. |
| +`getOverUnix` is a small helper with an `http.Client` whose transport dials the unix socket. The drain check must know the request is inside the handler before it signals (Astra plan 8): the probe writes one line `serving` to stdout at handler entry when `ZDDPROBE_TRACE=1` is set (nothing otherwise). The test sets it, starts a request, reads `serving` from the probe's stdout pipe, then sends SIGTERM, and asserts the request answers 200 and `cmd.Wait()` returns nil. |
| |
| - [ ] **Step 2: Run to see it fail:** `TMPDIR=/tmp/k GOCACHE=$TMPDIR/gocache go test ./cmd/zddprobe -count=1` (socket paths: if the sandbox refuses the socket, rerun unsandboxed with `TMPDIR=/tmp/k`). Expected: build failure. |
| - [ ] **Step 3: Implement `main.go`.** Then replace the roll notes' heredoc with: build from a platform checkout at the release commit, `for v in zdd-1 zdd-2 zdd-3 zdd-4; do CGO_ENABLED=0 GOOS=linux GOARCH=arm64 go build -ldflags "-X main.version=$v" -o "$TMPDIR/zdd-probe/probe-$v" ./cmd/zddprobe; done`, and a sentence that the probe takes the platform's `--socket` and `--db` (the round-2 dev check found the old one exited at once on `--db`). Keep the rest of "The probe" as it is, and fix its `cd`/path lines to match. |
| @@ -681,22 +739,22 @@ func TestProbeServesWithVetsArgv(t *testing.T) { ... launch(t, "-socket", sock) |
| |
| **Files:** |
| - Modify: `internal/cgroup/strict_kernel_linux_test.go:160`, `:190` |
| -- Modify: `internal/instancectl/gen.go:224-238` (`stopGen`) and its test file (find it with `grep -rln "stopGen\|StopGen" internal/instancectl/*_test.go`) |
| +- Modify: `internal/instancectl/gen.go:185-238` (`abandonGen`, `stopGen`) and its test file (find it with `grep -rln "stopGen\|StopGen" internal/instancectl/*_test.go`) |
| |
| **Behaviour.** |
| |
| (a) `TestStrictKernel` full mode builds second managers with no `logf` (`&Manager{root: m.root}` at `:160` and `:190`); production always sets one, and `freshenOne` (`recreate_linux.go:156`) logs through it, so the test panics. Give both `logf: t.Logf`. No production change. |
| |
| -(b) After `stopGen`'s stop or kill succeeds, and before `s.removeGenFiles(unit)`, run `s.Systemctl(ctx, "reset-failed", unit)`. An answer containing `not loaded` is ignored; any other error is logged with `s.logf("instance-control: reset-failed %s: %v", unit, err)` and the stop still answers 204 (Ruling 10). `reset-failed` is already one of the control service's verbs (`server.go:641`). |
| +(b) After a generation unit is successfully stopped or killed, and before its files go, reset its failed state. There are two such paths: `stopGen` (`gen.go:224`) and `abandonGen`'s successful `killGen` followed by `removeGenFiles` (`gen.go:189-197`, Astra plan 9). Share one helper, `s.resetGenFailed(ctx, unit)`, which runs `s.Systemctl(ctx, "reset-failed", unit)`. An unproven stop or kill is unchanged and does not reset. An answer containing `not loaded` is ignored; any other error is logged with `s.logf("instance-control: reset-failed %s: %v", unit, err)` and the stop still answers 204 (Ruling 10). `reset-failed` is already one of the control service's verbs (`server.go:641`). |
| |
| -- [ ] **Step 1: Write the failing test** for (b): with the recording fake `Systemctl` the instancectl tests already use, a `/stop?gen=N` (and `&kill=1`) records `reset-failed <unit>` after the stop or kill; with `reset-failed` answering an error, the response is still 204 and a log line was written; a stop that fails records no `reset-failed`. |
| +- [ ] **Step 1: Write the failing test** for (b): with the recording fake `Systemctl` the instancectl tests already use, a `/stop?gen=N` (and `&kill=1`) records `reset-failed <unit>` after the stop or kill; with `reset-failed` answering an error, the response is still 204 and a log line was written; a stop that fails records no `reset-failed`. An abandoned generation start whose kill succeeds records `reset-failed <unit>` too; one whose kill fails does not. |
| - [ ] **Step 2: Run to see it fail:** `GOCACHE=$TMPDIR/gocache go test ./internal/instancectl -run StopGen -count=1` (unsandboxed with `TMPDIR=/tmp/k` if sockets are refused). |
| - [ ] **Step 3: Implement (a) and (b).** |
| - [ ] **Step 4: Run** the instancectl package, then TestStrictKernel's full mode for real, in a delegated user unit with no RestrictNamespaces (the strict path keeps clone3, Ruling 3): |
| ```bash |
| -GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/cgroup.test ./internal/cgroup && mkdir -p /tmp/k/strictproof && \ |
| -systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k -E CARLOS_STRICT_KERNEL=full -E CARLOS_STRICT_PROOF_DIR=/tmp/k/strictproof \ |
| - /tmp/k/cgroup.test -test.run '^TestStrictKernel$' -test.v -test.count=1; rm -rf /tmp/k/cgroup.test /tmp/k/strictproof |
| +mkdir -p /tmp/k/zddr3 && GOCACHE=$TMPDIR/gocache go test -c -o /tmp/k/zddr3/cgroup.test ./internal/cgroup && mkdir -p /tmp/k/zddr3/strictproof && \ |
| +systemd-run --user --wait --pipe --collect -p Delegate=yes -E TMPDIR=/tmp/k/zddr3 -E CARLOS_STRICT_KERNEL=full -E CARLOS_STRICT_PROOF_DIR=/tmp/k/zddr3/strictproof \ |
| + /tmp/k/zddr3/cgroup.test -test.run '^TestStrictKernel$' -test.v -test.count=1; rm -rf /tmp/k/zddr3 |
| ``` |
| Expected: all five subtests PASS (round 2 saw this pass with the same fix in scratch). Paste real output. If the mode needs other variables, read `strict_kernel_linux_test.go:30-60` and set them; say which in the report. |
| - [ ] **Step 5: Commit** — `"instancectl: reset-failed a stopped generation unit; strict kernel test managers log (#272)"`. |
| @@ -715,7 +773,7 @@ Expected: all five subtests PASS (round 2 saw this pass with the same fix in scr |
| 3. **Behaviour change on roll, flag off:** every exec child now starts through the shim. What the agent log shows on a healthy box (no `placement before exec failed` lines, no PID-move lines), and what a failing shim looks like (`placement before exec failed (...) — using the PID-move fallback for this spawn`): the box still serves, as today. |
| 4. The new decline code `cgroup-placement-failed`, its log line, and that a console older than the box shows the unknown-code phrase until consoles roll. |
| 5. Host claims: the strict path keeps clone3, so under the shipped unit a claims-on box's strict launches answer `ErrStrictUnsupported`; that is now a precondition of claims-on swaps. |
| -6. **Dev re-check for Paul** (replacing round 2's blocked steps): roll; flag-off wakes answer 200 with no fallback lines; set `CARLOS_SWAP=overlap` and restart; the log shows `overlap swaps armed` and no `REFUSING`; the probe (built from `cmd/zddprobe`) under load through zdd-1 → zdd-3 with 0 failed; the hold control; a generation unit stopped by the control service is not in `systemctl --failed`; and any edge-unit line Task 4 had to exclude (the controller passes the list), checked on the real unit. |
| +6. **Dev re-check for Paul** (replacing round 2's blocked steps): roll; flag-off wakes answer 200 with no fallback lines; set `CARLOS_SWAP=overlap` and restart; the log shows `overlap swaps armed` and no `REFUSING`; the probe (built from `cmd/zddprobe`) under load through zdd-1 → zdd-3 with 0 failed; the hold control; a generation unit stopped by the control service is not in `systemctl --failed`; and any edge-unit line Task 4 had to exclude or could only mark `untested` (the controller passes the list), checked on the real unit. |
| 7. The DOCS-T1 kernel-test recipe (`systemd-run --user ... -p Delegate=yes`) and the Task 4 command. |
| |
| Update the "Still blocks flag-on" lists that name the four round-2 blockers so they point to this section. |
amadan