diff --git a/agents/webby/work/case2-r11/REVIEW.md b/agents/webby/work/case2-r11/REVIEW.md new file mode 100644 index 000000000..731c87f6b --- /dev/null +++ b/agents/webby/work/case2-r11/REVIEW.md @@ -0,0 +1,74 @@ +# Webby review: Case 2 design r11 (#416), Console without [Install], render rules, 9.3 + +Webby, 2026-10-20T21:14Z (from `date -u`). Author: Gosalyn (gosalyn-0048, gosalyn-0051). Two commits on +gosalyn/case2-design, reviewed together as r11: +- fb20a61b4: DESIGN blob 49eafb5a5666, rows blob edbc493fb092. +- eac92d956: rows only, rows blob 3a86d5f048dc. This is the one to re-pin. + +Diffs: `git diff 4281b0064 fb20a61b4` and `git diff fb20a61b4 eac92d956`. I read B1 against mos-1097 and +the port squat against mos-1100. I read the cited code at fb20a61b4. Its packages and scripts differ +from origin/refactor only in scripts/stack-release.sh, which none of these citations use. This is a +static review. I ran nothing on any host. + +## Verdict: APPROVE + +B1 is fixed, per mos-1097. The Console unit renders with no `[Install]`, the script enables only the bus +unit, and step 9 makes Jason the one who starts it, after each boot too. The tradeoff after an unplanned +reboot is written down. Q185 (jason-master-restart DECISIONS.md:1573) does put him in the homelab for +1012 reboots. Mos asked for two facts, and 9.3 gives both with citations: +- Who shares the Console's uid: the bus host and its children, the Console and its `human-cli.mjs` + children (webui bus.mjs:1-3), every session inside bwrap, and the admin steps. +- What a bwrap session can reach. The port, yes. sandbox.mjs:67 has no `--unshare-net`, and :29-31 says + the network isn't confined. The proof file, no. The binds at :101-116 are all under the run directory, + the workspace, the system paths or the sockets, and :21-22 says there's no `~/.config`. The one + harness mount, claude-code's `mounts()` (plugins/harness/claude-code/index.mjs:73-77), is only the + pinned binary. + +The 401/403 reading of serve.mjs holds. The proof check at :495-503 comes before any auth-header read, a +proof header with a loopback Host gets 403 at :497, and every write route needs `named` (:526, :532, +:537, :542, :546). + +B2 is fixed. I listed every token in both templates. The bus has `@REPO@`, `@PATH@`, `@CLAUDE_ENV@`, +`@PM_FLAGS@`, `%h` and `%i`. The Console has `@REPO@`, `@PATH@`, `@NODE@` and `@FLAGS@`. Every `@` token +and `%h` has a rule, and `%i` is left alone, which is right for a system unit. The leftover refusal is +`@[A-Z_]+@` or `%h`, so it doesn't catch `%i`. The `@PATH@` rule fixes the case I missed in r10. With +`--node /usr/bin/node`, webui-service.sh:246-248 would let a node in /usr/local/bin win, and there's a +test for that case. + +B3 is fixed. I checked the four lines against the sandbox: +- Every read-write bind (workspace, run directory, thread store) and every socket is under the data + root, inside ``: config.mjs:19, git-socket.mjs:1, launcher.mjs:5. So `ReadWritePaths=` + covers them. +- Nothing uses `/run/user`, so `ProtectHome=yes` takes away nothing the bus needs. +- No line restricts namespaces, and the refusal for `RestrictNamespaces=`, `PrivateUsers=` and + `SystemCallFilter=` has a test. +- The step 9 session start is the real check, and it waits on #383 as stated. + +N1-N5 from r10 are all in the C2-13a brief, with tests. + +The port squat and its fix are right. A session inside bwrap holds capabilities only in its own user +namespace, which doesn't own the host's network namespace, so it can't bind below the port floor. cli.mjs +:27 takes ports 0..65535, so serve needs no change. eac92d956 adds both mos-1100 conditions: +- The precheck refuses a floor at or below the port. +- The bind test builds a real session argv with `binds()` (sandbox.mjs:101) and `sandboxArgv()` (:162). + It expects `EACCES` below the floor, runs a control that binds successfully, and fails rather than + skips on a host with a low floor. + +## Non-blocking + +N1. A correction to my own r10 B1. I cited the dragon-lin forgery acceptance as decision 94. It is +decision 95 (lead-decisions.md:2048, the text at :2058-2062). r11 9.3 has it right. Decision 94 +(:2003) is the one about Jason's start, which r11's "Changes from r10" also has right. + +N2. The bind test's control binds "a free port of 1024 or above". On a host whose floor is above 1024, +that control fails for the wrong reason. Pick the control port at or above the floor the test just +read, and the low port below it. It's one line in the brief. + +N3. The precheck reads the Console's `--port` from the installed unit, so it can only pass after step 2c. +Say what it does when no Console unit is installed, presumably refuse with a named error, add a case for +it, and say in section 10 that the precheck runs after 2c. + +N4. `AmbientCapabilities=` carries across exec, so each `human-cli.mjs` child the Console starts also +holds `CAP_NET_BIND_SERVICE`. That's harmless today, because they run reviewed code and start no +session. Add one sentence to 9.3 so that a later Console child that starts something doesn't inherit the +capability unnoticed. diff --git a/docs/SESSIONS.md b/docs/SESSIONS.md index bb6b3947c..a2a7849da 100644 --- a/docs/SESSIONS.md +++ b/docs/SESSIONS.md @@ -1391,3 +1391,4 @@ are never rewritten or removed; corrections are new entries. 2026-10-20T20:50Z | Webby (Opus 5.5, reviewer seat) | row 197 implementation review r3 for Herb (herb-0026, N1-N3 delta, BUILD.patch 55fe390d), static | approve; notes agents/webby/work/queue-197/REVIEW-impl-r3.md 2026-10-20T20:55Z | Webby (Opus 5.5, reviewer seat) | row A brief review (ZAI_API_KEY allowlist) for Sage/Gosalyn (a2d94b2c2, rows blob 81140d017441), static | approve, N1-N2; notes agents/webby/work/case2-rowA/REVIEW-brief.md 2026-10-20T21:01Z | Webby (Opus 5.5, reviewer seat) | Case 2 design r10 review (#416) for Gosalyn (4281b0064, blob 4a1860e698e0, two system units), static | changes B1 Console boot start is Jason's call, B2 PATH/NODE/FLAGS render, B3 hardening lines unlisted, N1-N5; notes agents/webby/work/case2-r10/REVIEW.md +2026-10-20T21:15:35Z | Webby (Opus 5.5, reviewer seat) | Case 2 design r11 review (#416) for Gosalyn (fb20a61b4 + eac92d956, rows blob 3a86d5f048dc), static | APPROVE, N1-N4; notes agents/webby/work/case2-r11/REVIEW.md