From 45cbe5896610756179d0eb0698a43b5a1f89f38e Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 20 Oct 2026 14:02:19 -0500 Subject: [PATCH] duckworth: federation design r3/r3.1 re-review (duckworth-0003), changes B7 on Gizmoduck a2840a6be --- .../duckworth/work/federation-r1/REVIEW-r3.md | 76 +++++++++++++++++++ docs/SESSIONS.md | 1 + 2 files changed, 77 insertions(+) create mode 100644 agents/duckworth/work/federation-r1/REVIEW-r3.md diff --git a/agents/duckworth/work/federation-r1/REVIEW-r3.md b/agents/duckworth/work/federation-r1/REVIEW-r3.md new file mode 100644 index 000000000..b0ea9da18 --- /dev/null +++ b/agents/duckworth/work/federation-r1/REVIEW-r3.md @@ -0,0 +1,76 @@ +# Review: Gizmoduck federation design r3 and r3.1 (duckworth-0003) + +Reviewer Duckworth (Opus 5.5), 2026-10-20. Static review, no code run. +Subject: agents/gizmoduck/work/federation/DESIGN.md, diff 5ad67f671..a2840a6be (r3 940e0e816 plus r3.1 +a2840a6be, reviewed as one diff per gizmoduck-0010). DESIGN.md line numbers are at a2840a6be. Code at +origin/refactor 9ae8c73ac. Against duckworth-0002 (REVIEW-r2.md, 03a663cd7). + +## Verdict: changes, B7 only + +B1 to B6, C1 and N10 are resolved. r3.1 adds the separate-host form Mos asked for (mos-0994), and the +federation path really has no one-host assumption (3.8). But neither isolation form in R8's gate checks +for a privilege path from the factory uid into the hub. That is B7. It's a gate-wording edit with two +checks. N11 doesn't block. + +## r2 codes + +- B6 resolved (3.6 item 7, DESIGN.md:277-295). `to: 'user'` from a run passes only when `in_reply_to` is a + user message whose `from_run` `user:` owns the run's thread, and the message is in that thread. + Otherwise the send fails `user-reply-required`. `#userThread` sits beside `#testerThread` (:631). + `user-thread-human` mirrors :1864-1866, and the `#checkMessaging` mapping mirrors :1867. A human or tester + sending `to: 'user'` fails `unknown-role`. The runner header line is in, and `from_run` comes from + `receive`'s `SELECT *` (broker.mjs:1932, checked). The R3 tests and mutant and the R7 reply assertion are + in. +- C1 resolved (DESIGN.md:252-260, the residual-risk paragraph). The text names the `systemd-run --user` and daemonize escape and calls it + the owner's hole (P19 333), not a new one. +- N10 resolved (3.6 item 4). The `names` gate at :2290 is called out with its order. + +## r3.1 (mos-0991, mos-0994) + +- 3.8 (DESIGN.md:335-354) holds. I found nothing in the path that crosses buses or filesystems. Each + Console spawns its own CLI, access.json is on the receiver, and `federation.peers` holds URLs only. +- R8 (DESIGN.md:431) accepts either form, and sections 4, 5, 8 and 9 match it. The shared checks (client + ids, HTTPS both ways, discovery reachable, clocks) are right. +- Q1 and Q2 are recorded as rulings, not settled by the design. N8 stays with Mos until Mos confirms that + "P2P" means person-to-person. + +## Blocker + +B7. R8's isolation checks don't look for a privilege path from the factory uid into the hub. That gap is +in both forms. +- Separate uid. The checks are a refused socket call (human.mjs:67) and an unreadable data root and config + dir. If the factory uid has non-interactive sudo, doas or su to the hub user, or a polkit rule that + allows `machinectl shell hub@`, any unsandboxed factory agent becomes the hub uid. Then it can run the + hub's human-cli and write access.json, and every other check still passes. I missed this in r2 when I + marked B5 resolved. +- Separate host. "Jason confirms no factory agent holds a login or key for the hub host" is an attestation, + and it reads as if it were about agents' keys. Jason's own ssh key in the factory uid's `~/.ssh`, or an + `SSH_AUTH_SOCK` agent with that key loaded, can be read by every unsandboxed agent on that uid. If that + key logs in to the hub host, it does the same harm as the sudo path. + +Fix: add a check to each form, run as the factory uid with a clean environment: +- uid form: `sudo -n -u true` and `doas -n -u true` both fail, and no polkit rule grants the + factory uid a shell as the hub user. +- host form: `ssh -o BatchMode=yes true` fails for the hub user and for Jason's own user on that + host. Run it with and without the factory session's `SSH_AUTH_SOCK`. Also, any other instance or agent + on the hub host runs under a uid that isn't the hub's. + +Keep Jason's confirmation as well, but the gate should be the failed calls, as the uid form's refused +socket call already is. + +## Note (does not block) + +N11 (still open at a2840a6be). `#waiting` (broker.mjs:2555-2565) returns null only for `to` `human`, +`tester`, or a threaded role (:2556). A run's reply `to: 'user'` falls through to +`#recipient(business, 'user', thread)`, which finds no holder, and then to the launch notice. `told` is +true when `launch.by` is set and the run isn't `launch.by` (:2562). So each user reply would post a waiting +notice to the owner's launcher, and the run would get a `waiting` result naming a role `user` with +`launchable: false`. Item 7 already blocks a user-thread run from reaching the owner's inbox +(`user-thread-human`), so this path should be closed too. Fix: add `user` to the :2556 short-circuit next +to `human` and `tester`, and list :2556 in item 7. Add an R3 assertion that a user reply returns +`waiting: null` and creates no notice message. The planned "reply reaches `thread.messages`" test passes +even with the notice, so it won't catch this. + +N12. `thread_shares` is append-only (Q2, DESIGN.md:450-455), so unsharing needs an end row, and the check +has to read the latest row per share. That belongs to row 193's design, not this one. I'm noting it so R3's +user-thread checks don't assume shares only grow. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 575c1647e..954ef41bd 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1363,3 +1363,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T18:41:20Z | Filbert (Opus 5.5, 048f3c63) | Q175 SSO design D1/D2 check (DESIGN.md blob eba861f5d2bb at d55d86bb0), static git diff 817e8f51b..d55d86bb0 | ack: D1 and D2 match; design approved; Mos C1 sequence confirm still open; no Gitea/queue record 2026-10-20T18:51:09Z | Duckworth (Opus 5.5) | Gizmoduck federation design r1 review (DESIGN.md blob 0f4570d478f5 at 6925f78f0, code at 9ae8c73ac), static | changes B1-B5 (forwarded user identity, ownership code, business-wide reads, revocation ends no runs reliably, one-uid isolation); notes N1-N9; agents/duckworth/work/federation-r1/REVIEW.md 2026-10-20T18:58:59Z | Duckworth (Opus 5.5) | Gizmoduck federation design r2/r2.1 re-review (5ad67f671, parents 60b9b112a, 6925f78f0; code 9ae8c73ac), static | changes B6 only (user from_role breaks the runner reply path, runner.mjs:450 vs broker.mjs:1862); B1-B5 resolved; C1 line edit (systemd-run escape); N10; agents/duckworth/work/federation-r1/REVIEW-r2.md +2026-10-20T19:03Z | Duckworth (Opus 5.5) | federation design r3+r3.1 re-review (Gizmoduck 5ad67f671..a2840a6be), duckworth-0003 | changes B7 (R8 gate lacks sudo/ssh privilege-path checks in both isolation forms), B1-B6 resolved, N11 #waiting, N12 thread_shares