diff --git a/agents/duckworth/work/row195/REVIEW-r3.md b/agents/duckworth/work/row195/REVIEW-r3.md new file mode 100644 index 000000000..c8a5625b0 --- /dev/null +++ b/agents/duckworth/work/row195/REVIEW-r3.md @@ -0,0 +1,127 @@ +# Review: Darkwing row 195 DESIGN r4, the Case 1 install on 10.1.1.53 (duckworth-0051) + +Reviewer Duckworth (Opus 5.5), 2026-10-20. Subject, per Darkwing's request and mos-1098: +- darkwing/row195 at 0075be358, a fast-forward of r3 9a16d7795. The diff touches DESIGN.md only + (+299 -138). +- `agents/darkwing/work/queue-195/DESIGN.md`, blob 3f8590016b40, sha256 66c64625…e57b (checked), 519 lines. +- Sections 0, 1, 4, 5, 6 and 7 changed, so I read the whole file. + +Cited code I checked at 0075be358: +- `scripts/stack-release.sh`: + - `cd "$(dirname "$0")/.."` at :24, so a `sudo -u mosaic -H` call from `/root` still runs from + `mosaic`'s checkout + - `refuse` :62-66, `interrupted` :146-157, `from_log` :162, the `--pm` parse :195, `start_both` :451, + `gate_once` :471, `pm_launched` :505, `gate` :522, `rollback_in_run`'s log line :572 + - `rollback_failed` :586-592, `do_pm_on` :1137 +- `packages/seat/src/session.mjs:83-93`: `ENV_ALLOW` has no `DISABLE_AUTOUPDATER`, and nothing comes from + `~/.claude` +- `packages/seat/src/sandbox.mjs`: :1-20, `BWRAP` at :43, the read-only `/etc` at :85 +- `packages/cli/src/launcher.mjs:675`: `launchPm` starts `b.launch.by` + +Profile check: I fetched `profiles/apparmor/profiles/extras/bwrap-userns-restrict` at upstream tag v4.0.1 +from gitlab.com/apparmor/apparmor. Its sha256 is `11d39094…64f9`, the hash §0 and §1 pin. I read all of +it. I ran nothing on .53 and nothing on dragon-lin's live state. + +## Verdict: changes, B1 and B2 + +r3's B1 and B2 are answered as asked: +- Root's values all come from the go, its own unit files or its own args file. No argv carries an + activation id. `--to` is checked against `from_log previous` in A1's plan. A2f has its own row, and + root has its own step log. +- `confirm` runs `gate`, which polls until the timeout. The external PM check reads `launches list` + only. It is bound to `launch.by` and to the bus unit's `ActiveEnterTimestamp`, with `stale-start` on + top. `pm-on` and `--pm-after-harness` refuse `external-pm`. +- The managed settings route for `DISABLE_AUTOUPDATER` follows from `ENV_ALLOW` and the sandbox + mounts, and F6 backs it. +- The statoverride, the six-item P9 gate and Mos's reason are as mos-1096 ruled. + +## Blockers + +B1. The rollback can't find the activation that the failure just logged. +- `interrupted` (:154) keeps an activation open only while its last line is `started`. Any later line + closes it (`else open.delete`). +- A5's failure logs `result gate-failed code rollback pending` (:258), and A3's failure logs + `result failed code rollback pending` (:256). Either line takes the activation out of the + `interrupted` set. +- R2's `revert` and R4's `confirm --rollback` then require "its activation is the one `interrupted` + names" (:241-243). So they refuse `open-activation`, and root exits 2 with UNITS stopped. + +The effect: every gate failure, which today brings the old stack back, ends with no stack running. + +A3 has a second gap. It writes `AWAITING-UNITS` last (:256). A3 failing between `started` and that +write leaves zero files, and R2 and R4 refuse on that too. + +Fix, either way: +- No interim result line. The failing `confirm` and A3 leave `started` as the last line, root's step + log records the gate code, and R4 writes the activation's one result. +- Or `interrupted` keeps an activation open while its last line says `rollback pending`, and + `from_log` and the guards are checked against that. + +Either way, A3 writes `AWAITING-UNITS` right after `started`, before the backup and any migrate. + +Add a case to `stack-release-external.test.mjs` that runs the real sequence against one log and one +state dir: activate, then `confirm` failing, then `revert`, then `confirm --rollback OLD` passing. Add +the same sequence with A3 failing after `started`. The `case1-repin.test.mjs` fakes don't cover this, +because they don't run `stack-release.sh`. Add a mutant that restores today's `interrupted` rule. + +B2, from mos-1098 item 1. Nobody owns the hand-placed profile after an upgrade. +- §1 installs `/etc/apparmor.d/bwrap-userns-restrict` as a loose file that no package owns. +- Ubuntu 24.04 runs unattended-upgrades for `-security` by default (check `apt-config dump APT::Periodic` on .53, read-only), and that's the path an apparmor or + bubblewrap fix takes. So an upgrade can arrive with no person in the loop. +- The profile declares `abi `, which a newer parser is meant to honour. That's the + expectation, not something the design tests. + +If the profile fails to load, the host fails closed on reach. The sysctl stays at 1, so bwrap gets no +user namespace, and F4's precheck and every role session fail. That widens nothing, but nothing +reports it either. + +Name in §1, §4 and the run sheet: +- (a) Who checks the profile and when. For example, root-owned `case1-repin.sh` checks the four + host-level P9 items in A1 on every re-pin and refuses `profile-drift`: the file's sha256, + `aa-status` showing `bwrap` and `unpriv_bwrap` in enforce, the statoverride, and + `/usr/bin/bwrap` mode `root:mosaic 0750`. Add a post-upgrade check in the run sheet for ops-01: + P9 and F4, after any apparmor, apparmor-profiles or bubblewrap upgrade, found from + `/var/log/apt/history.log` at each window. +- (b) What a failed load looks like, and that it's a stop, not a fix-forward. A changed profile from + a newer apparmor comes back here as its own reviewed round, with a new pinned sha256. + +Give A1's refusal a `case1-repin.test.mjs` case. + +## Notes (do not block) + +N1, from mos-1098 item 2. Only one profile governs `/usr/bin/bwrap` in r4. r3's `mosaic-bwrap` (the +4b form, `flags=(unconfined)`) isn't installed any more. §1 step 2 replaces it, and §6 says so. Ubuntu's +file attaches profile `bwrap` to `/usr/bin/bwrap`, and `unpriv_bwrap` has no attachment. + +Two profiles with the same attachment would be a conflict. So: +- P9 adds a seventh item: `/etc/apparmor.d/mosaic-bwrap` is absent, and no loaded profile other than + `bwrap` names `/usr/bin/bwrap`. +- The run sheet says r3's profile is never installed. + +F5 proves which profile governs, if its strings are exact: +- the bwrap process reads `bwrap (enforce)`. Under r3's profile it would read + `mosaic-bwrap (unconfined)`. +- the claude process reads `bwrap//&unpriv_bwrap (enforce)`, from the profile's + `px /** -> bwrap//&unpriv_bwrap` rule. + +§5 writes the second as "`unpriv_bwrap` (stacked)". Give the exact string in §5 and the test fixture. + +N2. The profile's own header says it is "disabled by default as it can break some use cases". It also +gives `bwrap` itself `allow capability`, `mount` and `pivot_root` over `/{**,}`. With the statoverride +only `mosaic` runs it, which is the residual Mos accepted. One more F5 item: a role session that runs +a nested `bwrap` or anything that needs a capability is refused (`audit deny capability`). If the +harness or a tool the chief uses needs one, F3 fails. That's the check working, not a reason to widen +the profile. + +N3. "before it logs or migrates anything" (:256, :327) isn't quite right. `refuse` (:65) writes a +`result refused` line with no activation outside `--plan`. That's harmless, and it closes nothing. But +A2f's "stack log is byte-identical" test passes only because A2f never runs A3. Say "before it logs +`started`". + +N4. `ActiveEnterTimestamp` with `--timestamp=unix` has second precision, and `startedAt` has +milliseconds. The `>=` bound holds, because the bus starts the PM after the unit is active. Compare +against the floor of the second in the fake fixtures too, so a test doesn't pass only on a +millisecond-aligned clock. + +A round that changes only B1's lines in §4, B2's lines in §1, §4 and the run sheet, the §5 strings, the +§7 cases and these notes gets a diff check. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 3139045a6..e18b15187 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1404,3 +1404,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T20:49Z | Duckworth (Opus 5.5) | #423 step 4 revised scripts (step4.sh 5aea3856, check4.sh 8f49a409, containers.sh 837bb7a4, postboot.sh, boot-session f0645604) (duckworth-0048) | C1-C4 met, C5 stale shas in boot-session block, N1-N3 2026-10-20T20:51Z | Duckworth (Opus 5.5) | #423 step 4 N1/N2 diff and C5 check, step4.sh cc16d384, boot-session ed44ec81 (duckworth-0049) | holds, no new condition 2026-10-20T20:59Z | Duckworth (Opus 5.5) | row 195 DESIGN r3 review, Darkwing 9a16d7795 blob 0fea86616b36 (duckworth-0050) | changes, B1 ACT/TO hand-off, B2 gate poll and PM journal read +2026-10-20T21:11Z | Duckworth (Opus 5.5) | row 195 DESIGN r4 review, Darkwing 0075be358 blob 3f8590016b40, with mos-1098 items (duckworth-0051) | changes, B1 rollback open-activation, B2 profile upgrade owner