From b402197ff3a0d1a1c479f91632819cdf9a58f114 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Tue, 20 Oct 2026 13:51:09 -0500 Subject: [PATCH] duckworth: federation design r1 review (duckworth-0001), changes B1-B5 on Gizmoduck 6925f78f0 --- agents/duckworth/work/federation-r1/REVIEW.md | 91 +++++++++++++++++++ docs/SESSIONS.md | 1 + 2 files changed, 92 insertions(+) create mode 100644 agents/duckworth/work/federation-r1/REVIEW.md diff --git a/agents/duckworth/work/federation-r1/REVIEW.md b/agents/duckworth/work/federation-r1/REVIEW.md new file mode 100644 index 000000000..08db37ab3 --- /dev/null +++ b/agents/duckworth/work/federation-r1/REVIEW.md @@ -0,0 +1,91 @@ +# Review: Gizmoduck federation design r1 (duckworth-0001) + +Reviewer Duckworth (Opus 5.5), 2026-10-20. Static review, no code run. +Subject: agents/gizmoduck/work/federation/DESIGN.md at 6925f78f0 (blob 0f4570d478f5), branch +gizmoduck/federation-design. Code at origin/refactor 9ae8c73ac. Sage's Q175 design read at d55d86bb0. +Binding inputs: CONSTITUTION, SYSTEM, TARGET 2026-10-20 addendum, DECISIONS Q175-Q178, GAP-MAP, PLAN. + +## Verdict: changes, B1-B5 + +The direction is right. The receiving Console verifies the person (Q175), the broker enforces the grant, +and the access file sits outside the agent-writable business file. Five gaps stop it from meeting criteria +1, 2 and 5 as written. + +## Blockers + +B1. The bus takes the user's identity from the caller. `user-cli.mjs` carries `{ business, user }` and is +"proven the way tester-cli.mjs is" (DESIGN.md:171-172). The tester proof shows only that some process runs +the tester CLI. It allows agent binaries and agent markers by design (packages/bus/src/tester.mjs:5-9, :57). +The `user` field is whatever that process says. Any T3 seat or agent shell on the hub's uid can bind as any +granted user. That is a forwarded identity, which Q175 rules out. Fix: use the human proof's walk, which +refuses agent binaries (packages/bus/src/human.mjs:83-88). Bind the user CLI to the Console's tree, the +way #225 binds human-cli. State the residual risk in the design, as Sage's design section 8 does. Add an R3 +mutant where the user CLI is run from an agent-marked parent and accepted. + +B2. "No new ownership code" (DESIGN.md:180-181) is false. `#owner` returns `'tester'` or `s.human` +(packages/bus/src/broker.mjs:364-366), which is undefined for a user. The reply-chain path checks ownership +for the tester only (`#testerOwns`, broker.mjs:352-355, called from `#threadFor` at :337 and :342), and +`#threadFor` special-cases `s.human` only (:326). A user principal needs explicit handling in all three. +Without it, a `message.send` with `in_reply_to` can land in another user's thread. Fix: name the changes +in 3.6. Add an R3 test and mutant for a reply-chain send into another user's thread, or keep +`message.send` off every level's list and state that. + +B3. The level verb lists are never enumerated, and the Console reads are business-wide. "Read the board" +for a viewer (DESIGN.md:162) means `usage`, `limits`, `inbox`, `tasks`, `agents`, `trail`, `decisions`, +`credentials` and `messages` (packages/bus/src/views.mjs:3-13). The broker serves these per business, not +per project or instance (broker.mjs:2144, :2174, :2195, :2217). A project-scoped grant would therefore show +Melanie's own messages, decisions, trail and usage to a viewer. Fix: list the exact verbs for each level, and +for each read verb state the filter: granted projects, granted instances, own threads. Otherwise refuse the +verb. The R3 gate needs a leak test per allowed read verb, plus a mutant that drops the filter. + +B4. Revoking a grant doesn't reliably end runs (criterion 2). +- (a) No trigger. The broker re-reads access.json only when a request arrives (DESIGN.md:174-176). Runs end + only "on a revision where a user loses a grant" (:193-194). A hand edit, which :167 allows, fires nothing + until the next request. Expiry (:156) never makes a revision at all, so an expired user's runs never end. +- (b) Ending a claim stops the run only "at its next bus call" (broker.mjs:2273-2275), so a turn already in + flight keeps working. +- (c) The launcher starts a thread again when it has pending messages and no holder (packages/cli/src/launcher.mjs:533), + launched as the business human (:542). Its `startedFor` guard lives in memory and clears on restart. A + revoked user's queued message can therefore start a fresh run. + +Fix: the launch check refuses a thread whose `user:` owner has no live grant. Define the trigger: +R5 calls a bus verb, plus a check at each launch and on a timer for expiry and hand edits. Say whether the +runner gets SIGTERM. The R4 gate should cover a hand edit, an expiry, and a pending message followed by a +launcher restart, each with a mutant. R4 depends on whatever row provides the trigger, so fix the order. + +B5. On one uid, Melanie's hub isn't isolated from Jason's factory (criterion 5). The design says so +(DESIGN.md:223-227, :274-275), but it leaves the fix as a question (Q3, :284) and R8 doesn't depend on it +(:264). With one uid, any factory agent can read the hub's data root and write its access.json. It can also +run the hub's human-cli and act as the hub owner. Sage's design section 8 states the same limit for the bus +human proof. Grants don't matter while that holds. Fix: make a separate uid per instance (config dir and data +root 0700) a precondition of R8, with a check. A process on the factory's uid must be refused by the hub's +proof (`p.uid !== process.getuid()`, human.mjs:67) and must be unable to read the hub data root. If Jason +accepts running on one uid for 10-27, record that as his decision through Mos, since it changes a security +boundary. + +## Notes (do not block) + +N1. The v1 claims hold. The OIDs are at ca.service.ts:22-24, the guard reads the TLS peer cert at +federation-auth.guard.ts:93-106, and main.ts:50-53 builds the adapter with no TLS. `requestCert` appears +only in a guard comment (:36). The line count is about 4,470 non-spec lines (9,198 with tests), not +"about 4,100". The verdict stands: none of the code is reusable. +N2. The other cited lines match: config.mjs:19, runtime.mjs:140 and :169, requirements.json:61, +business.mjs:160, broker.mjs:298-306, 356-362, 367-378, 526-538, 596-609, 763-799, webui bus.mjs:1-9 and +:27-31, mosaic-config.mjs:112-116, organization.mjs:83, bus-service.sh:200. One error: business.mjs:24 +(DESIGN.md:220) is a comment. `configDir` is at :25-28. +N3. R1 is marked "depends on none", yet it validates against `console.sso` providers and `federation.peers`. +Pass both in as arguments so R1 stays pure. +N4. "Human-only" in R5 is cosmetic while one uid holds (B5). A direct file write bypasses both the CLI and +the receipt. Say so. +N5. Run the same file checks on access.json that Sage's loader runs on the secret slot: refuse a symlink, +a non-regular file, another owner, and group or other write. +N6. A granted instance runs with its full role authority, launched as the business human (launcher.mjs:542). +`instances` is the real capability limit of a grant, so the grant docs should say that. +N7. `console.sso` sits in config.json, so removing a provider takes effect only after a Console restart. +:195-196 implies it takes effect on the next request. +N8. Scope: the slice amounts to T1.3's multi-user access, plus `home` and peer labels. It meets "shared +access between disparate instances" in the sense of people signing in to the other instance. +"Federation is configurable" is met only through grants and labels. Moving agent-to-agent traffic past +10-27 (Q1) narrows a requirement Jason gave verbatim, so Mos should put it to Jason, not settle it alone. +N9. Audience binding (3.3) relies on the factory and the hub having distinct client ids at the provider. +Add a line to R8's checklist that confirms the two client ids differ. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index 9f20adc24..b142717ea 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1361,3 +1361,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T18:34:53Z | Filbert (Opus 5.5, 048f3c63) | Q175 SSO design review r2 (DESIGN.md blob 3eb718b5e131 at 1a268249e), static | changes: C1 config.json console key fails old loaders (mosaic-config.mjs:113 rejects unknown keys; key add inside phase 2 window, rollback removes it, loader test), C2 app.js signs out only on opaqueredirect so 401 never shows sign-in; notes on tests 15/7/9a, Q-B 200-HTML classification, scratch Console flags; Q-E keep iss/sub escaped and capped; verdict sent to Sage, no Gitea/queue record 2026-10-20T18:40:20Z | Filbert (Opus 5.5, 048f3c63) | Q175 SSO design review r3 (DESIGN.md blob a152fd58e18e at 817e8f51b), static | approve on condition D1 (slot file checks stay out of shared loader, shape-only there) as a line edit; D2 notes: slot root from configDir, scratch dataRoot, name discovery GETs for Mos; C1/C2 and notes verified; Mos C1 sequence confirm still open; no Gitea/queue record 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