diff --git a/agents/darkwing/work/queue-195/BUILD.md b/agents/darkwing/work/queue-195/BUILD.md index 317d6adef..5d1b0fde1 100644 --- a/agents/darkwing/work/queue-195/BUILD.md +++ b/agents/darkwing/work/queue-195/BUILD.md @@ -151,3 +151,11 @@ every run behind my busy check and Sage's, chained with `&&`. Summed: 818 node tests passed with 0 failed, and 429 selftests passed with 0 failed. The compose network was removed afterwards, and the sweep found no process left in the worktree, the output directory or the TMPDIR. This file's results are a doc-only commit on top of the build, so nothing that ran has changed. + +## Correction to deviation 6 (follow-up, 2026-10-21) + +Deviation 6 says the node F9 runs is root's own. At 37171f65b the script didn't check that. F9 ran +`--node` as root before F10's path check, so the header's "root itself only reads" was false +(Duckworth's N1, `agents/duckworth/work/row195/REVIEW-m1.md` at 6afcee2a5). A host installed as designed +was safe, because `case2-unit.sh` refused a non-root-owned `--node`. Row 245 carries the fix and its +tests: `agents/darkwing/work/queue-245/BUILD.md`. diff --git a/agents/darkwing/work/queue-245/BUILD.md b/agents/darkwing/work/queue-245/BUILD.md new file mode 100644 index 000000000..a6fefae4b --- /dev/null +++ b/agents/darkwing/work/queue-245/BUILD.md @@ -0,0 +1,91 @@ +# Row 245 build: Duckworth's N1-N3 on `scripts/case1-accept.sh` (row 195 follow-up) + +Duckworth approved row 195's manifest round 1 (duckworth-0088, `agents/duckworth/work/row195/REVIEW-m1.md` +at 6afcee2a5) with three notes that don't block. Row 195's code landed as b65014647, and the notes are in +`docs/plans/DEFERRED.md` (d6cff0cdd). Row 245 carries them: brief `docs/plans/2026-10-20_hub-own-uid.md` ยง +"Row 195 follow-up" at 62ddc96cc, owner Darkwing, reviewer Duckworth, gate owner Sage (sage-1923). + +The branch is `darkwing/row245`, on 62ddc96cc. Its first commit is a cherry-pick of `darkwing/row195-n1` +1e3c4767c, which was written on 37171f65b and never run. The script, test and TOOLS.md blobs are the same +as in 1e3c4767c, and none of those paths changed between 37171f65b and 62ddc96cc. + +## What changes + +N1. F9 ran `$ROOT$NODE` as root (`case1-accept.sh:298` at 37171f65b) before F10 checked that `--node` is +the pinned path. Duckworth offered two fixes: F9's parse through `AS_USER`, or F10's path check first. I +took the path check, plus one more: +- F9 now runs node only when `--node` equals `/opt/mosaic-stack/node-<--node-pin>/bin/node`. +- Then `not_root_only` walks from that file up to `/`. Each entry must be the running user's (root's on the + host) with no group or other write, the rule `case2-unit.sh` `root_only` applied when it rendered the + units. A symlink fails it, because its mode is 777. The first entry that breaks the rule goes in the + evidence. + +I didn't run the parse through `AS_USER`. That would hand a check about a root file to the stack's user, +and a process running as `mosaic` shares that node's uid. Whether it could change what node prints then +turns on the host's ptrace setting, and the root-only check doesn't depend on that. Sage agreed in +sage-1877. Rechecking ownership costs one loop and doesn't depend on +how the host was installed. + +N2. The header (:9-15) and the `docs/TOOLS.md` entry now list what root runs: `systemctl show` and +`is-enabled` (F1, P11), `curl` on the loopback healthz (F2), `sudo -l` (P1, P3), `sudo -u jarvis bwrap +--version` and `hostcheck.sh --check` (P9), and node for F9 under the rule above. + +N3. F6 matched `--claude` by prefix, so `/opt/mosaic-stack/claude-/../claude-2.0.9/...` passed the +path check. F6 now fails on any `..` segment, before it runs `--version`. F10 compares for equality, so +it needed no change. + +Row 195's BUILD.md gets a correction entry for deviation 6. Row 195's DESIGN.md is unchanged. Its "either +works" wording at `DESIGN.md:336-338` is superseded by gosalyn-0087 (root writes `dev.args` at W10), and +it stays in place: an approved design isn't edited (sage-1877). + +## Tests + +`packages/cli/tests/case1-accept.test.mjs`: +- The node stub touches `node-as-root` in the state directory when something runs it outside the fake + `sudo` (which sets HOME to the stack's home). `host()` sets the node file's directories and the scratch + root to 0755, so the umask can't fail F9. +- "every check passing" also asserts that root ran the pinned node. +- New: "F9 won't run --node as root off the pin path or with a writable directory above it". Another node + path fails F9 and F10, with no root run. A group-writable `bin/` and a symlinked node each fail F9 + alone, with the bad path in the evidence and no root run. +- The F6 test gains a `..` case: a path under `claude-/` by prefix that names 2.0.9's binary, which + prints the pin. F6 alone fails, with "has a .. segment". + +That makes 23 tests. All 23 pass on Sage's slot (results below). + +## Mutants + +The twelve from row 195 still apply, unchanged. These four are new, each one exact replacement: + +| # | Mutant | Expected killer | +|---|---|---| +| 13 | F9 runs any `--node` (the pin check becomes `true`) | F9 won't run (another node) | +| 14 | F9 skips the ownership walk (`bad=""`) | F9 won't run (writable `bin/`, symlink) | +| 15 | `not_root_only` checks only the file, not the directories above it | F9 won't run (writable `bin/`) | +| 16 | F6 accepts a `..` segment | F6 stub, path and `..` | + +The brief's gate asks for a mutant for each of N1 and N3. Mutants 13-15 are N1's, and 16 is N3's. + +## Slot plan + +Same shape as row 195's steps, on this row's tip: the single test file, then the sixteen mutants, then +the gate (the cli suite and every `scripts/test-*.sh`), each behind the busy checks and chained with +`&&`. The scripts are in `/mnt/storage/scratch/darkwing-245-out/`. + +## Slot results (grant sage-1961, 2026-10-21) + +All three steps ran on 0590a29fc with node v26.8.1, launched from `/`. Both busy checks were CLEAR +before each step. + +- Singles, 11:27:46-11:27:59Z: `case1-accept.test.mjs` passed 23, failed 0. +- Mutants, 11:28:10-11:31:20Z: all 16 killed, each with exit 1. The runner restored the script, and + its sha256 875bb5b7 matches the committed blob. Rows 1-12 died as in row 195. 13, 14 and 15 died on + "F9 won't run --node as root off the pin path or with a writable directory above it". 16 died on + "F6 fails on a stub that prints nothing, on a path outside claude- and on a .. segment". +- Gate, 11:31:20-11:42:51Z: the cli suite passed 664, failed 0, skipped 0. Every `scripts/test-*.sh` + exited 0: auth 15, conductor 17, config 31, discord 78, extension-package 18, foundation 44, + land-work 86, queue 28 (its node part 156/0), release 14, task 98. That's 429 selftests, none failed. +- Afterwards no process had its cwd in the worktree, the out dir or TMPDIR, and the + `darkwing-r245_default` network was gone. + +Logs: `/mnt/storage/scratch/darkwing-245-out/` (`step1.log`, `mut.out`, `mut/m1-16.log`, `gate.out`). diff --git a/docs/TOOLS.md b/docs/TOOLS.md index 94a1b3791..879375a87 100644 --- a/docs/TOOLS.md +++ b/docs/TOOLS.md @@ -905,9 +905,13 @@ args file (`/etc/mosaic-stack/dev.args` on .53), which must pass `stack-upgrade.sh`'s rules, say `--profile factory` and name `--business`. Every command that runs the stack's code runs as `sudo -u USER -H env PATH=:/usr/local/bin:/usr/bin:/bin` from `/`. -Root only reads systemd, `ss`, `/proc`, the account database and file -metadata. The one write is F3's message and its reply, and it never opens -`claude-session.env`. +Root itself reads `ss`, `/proc`, the account database and file metadata. It +runs `systemctl show` and `is-enabled`, `curl` on the Console's loopback +`/healthz`, `sudo -l`, `sudo -u jarvis bwrap --version`, `hostcheck.sh +--check`, and node to parse `managed-settings.json` for F9. F9 runs node only +when `--node` is the `--node-pin` path, and the file and every directory above +it are root's with no group or other write. The one write is F3's message and +its reply, and it never opens `claude-session.env`. It prints one line per check, ` `, then `count N` for each mark and a result line. The functional checks diff --git a/packages/cli/tests/case1-accept.test.mjs b/packages/cli/tests/case1-accept.test.mjs index bdbb4ad66..b9d676c52 100644 --- a/packages/cli/tests/case1-accept.test.mjs +++ b/packages/cli/tests/case1-accept.test.mjs @@ -7,7 +7,7 @@ import { test } from "node:test"; import assert from "node:assert/strict"; import { spawnSync } from "node:child_process"; -import { chmodSync, existsSync, mkdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; +import { chmodSync, existsSync, mkdirSync, readFileSync, renameSync, rmSync, symlinkSync, writeFileSync } from "node:fs"; import { dirname, join } from "node:path"; import { fileURLToPath } from "node:url"; import { tmp } from "./helpers.mjs"; @@ -77,10 +77,13 @@ args=(); for a in "$@"; do if [[ "$a" == /* ]] && [ -e "$ROOT_DIR$a" ]; then arg HOME="$ROOT_DIR/srv/mosaic" PATH="$np$PATH" exec "$ROOT_DIR$cmd" "\${args[@]}"`, }; -// Executables inside the tree, run by the fake sudo or by the script itself. +// Executables inside the tree, run by the fake sudo or by the script itself. Node marks a run outside +// the fake sudo, which sets HOME to the stack's home, so a test can tell whether root ran it. +const NODE_STUB = `[ "$HOME" = "$ROOT_DIR/srv/mosaic" ] || touch "$HC/node-as-root" +[ "\${1:-}" = --version ] && { cat "$HC/node-version"; exit 0; } +exec "$REAL_NODE" "$@"`; const TREE = { - [NODE]: `[ "\${1:-}" = --version ] && { cat "$HC/node-version"; exit 0; } -exec "$REAL_NODE" "$@"`, + [NODE]: NODE_STUB, [CLAUDE]: `[ "\${1:-}" = --version ] && cat "$HC/claude-version"; exit 0`, [CLI]: `${log("mosaic")} case "$1" in @@ -151,6 +154,9 @@ function host(t) { for (const [name, text] of Object.entries(state)) put(join(hc, name), text); for (const [name, body] of Object.entries(FAKES)) put(join(bin, name), `#!/usr/bin/env bash\n${name === "stat" ? "" : log(name)}\n${body}\n`, 0o755); for (const [path, body] of Object.entries(TREE)) put(join(root, path), `#!/usr/bin/env bash\n${body}\n`, 0o755); + // F9 runs node only when it and each directory above it have no group or other write, whatever the umask. + for (let d = dirname(NODE); d !== "/"; d = dirname(d)) chmodSync(join(root, d), 0o755); + chmodSync(root, 0o755); return { dir, root, hc, bin, args }; } @@ -204,6 +210,7 @@ test("row 195 accept: every check passing exits 0, with each P mark and counts t assert.match(r.out, /^F5 bwrap-label pass bwrap 300 reads 'bwrap \(enforce\)' and its claude descendant 302 reads/m); assert.match(r.out, /^F7 pm-live pass one live launch: pm run r-0002$/m); assert.ok(!(r.out + r.err).includes(CRED_VALUE)); + assert.ok(existsSync(join(h.hc, "node-as-root")), "F9 runs the pinned, root-only node as root"); }); test("row 195 accept: exits 1 with each F check failing alone, and its line names it", (t) => { @@ -274,7 +281,7 @@ test("row 195 accept: F5 fails with no bwrap of mosaic's, and passes on comm alo assert.deepEqual(failed(mustRun(h)), []); }); -test("row 195 accept: F6 fails on a stub that prints nothing and on a path outside claude-", (t) => { +test("row 195 accept: F6 fails on a stub that prints nothing, on a path outside claude- and on a .. segment", (t) => { let h = host(t); put(join(h.hc, "claude-version"), ""); let r = mustRun(h); @@ -286,6 +293,14 @@ test("row 195 accept: F6 fails on a stub that prints nothing and on a path outsi r = mustRun(h); assert.deepEqual(failed(r), ["F6"]); assert.match(r.out, /^F6 claude-pin FAIL --claude \S+claude-2\.0\.9\S+ is not under \/opt\/mosaic-stack\/claude-2\.1\.0\/$/m); + // Under claude-/ by prefix, but the binary it names is 2.0.9's, which answers with the pin. + h = host(t); + const dotdot = CLAUDE.replace(`claude-${CLAUDE_PIN}/`, `claude-${CLAUDE_PIN}/../claude-2.0.9/`); + put(h.args, ARGS.map((a) => (a === CLAUDE ? dotdot : a)).join("\n") + "\n", 0o644); + put(join(h.root, other), `#!/usr/bin/env bash\necho "${CLAUDE_PIN} (Claude Code)"\n`, 0o755); + r = mustRun(h); + assert.deepEqual(failed(r), ["F6"], r.out); + assert.match(r.out, /^F6 claude-pin FAIL --claude \S+\/claude-2\.1\.0\/\.\.\/claude-2\.0\.9\/\S+ has a \.\. segment$/m); }); test("row 195 accept: F7 fails on two live PM launches, one for another business, and no launch.by", (t) => { @@ -328,6 +343,31 @@ test("row 195 accept: F9 fails on another owner, a group-writable mode and a num } }); +test("row 195 accept: F9 won't run --node as root off the pin path or with a writable directory above it", (t) => { + // Another node that works for the stack's user: F9 refuses to run it, and F10 fails on the path. + let h = host(t); + const other = NODE.replace(NODE_PIN, "v24.0.9"); + put(h.args, ARGS.map((a) => (a === NODE ? other : a)).join("\n") + "\n", 0o644); + put(join(h.root, other), `#!/usr/bin/env bash\n${NODE_STUB}\n`, 0o755); + let r = mustRun(h); + assert.deepEqual(failed(r), ["F9", "F10"], r.out); + assert.match(r.out, /^F9 autoupdater-off FAIL root won't run --node \/opt\/mosaic-stack\/node-v24\.0\.9\/bin\/node: it is not the --node-pin path$/m); + assert.ok(!existsSync(join(h.hc, "node-as-root"))); + // The pinned path, under a group-writable directory and then a symlinked node: F9 alone fails, and + // root doesn't run it. + for (const [fault, bad] of [ + [(x) => chmodSync(join(x.root, dirname(NODE)), 0o775), "/opt/mosaic-stack/node-v24\\.1\\.0/bin \\(uid \\d+, mode 775\\)"], + [(x) => { const f = join(x.root, NODE); renameSync(f, `${f}.real`); symlinkSync(`${f}.real`, f); }, "/opt/mosaic-stack/node-v24\\.1\\.0/bin/node \\(uid \\d+, mode 777\\)"], + ]) { + h = host(t); + fault(h); + r = mustRun(h); + assert.deepEqual(failed(r), ["F9"], r.out); + assert.match(r.out, new RegExp(`^F9 autoupdater-off FAIL root won't run --node \\S+: ${bad} is not root's alone$`, "m")); + assert.ok(!existsSync(join(h.hc, "node-as-root"))); + } +}); + test("row 195 accept: F11 fails on one *.json under discord/ and passes on an empty directory", (t) => { let h = host(t); put(join(h.root, HOME, ".mosaic-dev", "discord", "chief.json"), "{}"); diff --git a/scripts/case1-accept.sh b/scripts/case1-accept.sh index d1c14fd45..833ef19c5 100755 --- a/scripts/case1-accept.sh +++ b/scripts/case1-accept.sh @@ -7,9 +7,13 @@ # Root runs it on the Case 1 host after the install window, from a root-owned checkout. It reads the # user, home, copy, node, claude and both ports from the args file, which must pass stack-upgrade.sh's # rules and be a factory file for --business. Every command that runs the stack's code runs as -# `sudo -u USER -H env PATH=:/usr/local/bin:/usr/bin:/bin` from /. Root itself only reads -# systemd, ss, /proc, the account database and file metadata. It changes nothing on the host but the -# message F3 sends and its reply, and it never opens claude-session.env. +# `sudo -u USER -H env PATH=:/usr/local/bin:/usr/bin:/bin` from /. Root itself reads ss, +# /proc, the account database and file metadata, and runs these: systemctl show and is-enabled (F1, +# P11), curl on the Console's loopback healthz (F2), sudo -l (P1, P3), sudo -u jarvis bwrap --version +# and hostcheck.sh --check (P9), and node to parse managed-settings.json (F9). F9 runs node only when +# --node is the pinned path and it and every directory above it are root's with no group or other +# write. The script changes nothing on the host but the message F3 sends and its reply, and it never +# opens claude-session.env. # # One line per check, ` `, then `count N` for each mark and a # result line. F checks are `pass` or `FAIL`. P checks are `closed`, `accepted ` or @@ -259,6 +263,7 @@ check_f4() { check_f6() { local want="/opt/mosaic-stack/claude-$CLAUDE_PIN/" v [[ "$CLAUDE" == "$want"* ]] || { ffail F6 claude-pin "--claude $CLAUDE is not under $want"; return; } + [[ "$CLAUDE/" != */../* ]] || { ffail F6 claude-pin "--claude $CLAUDE has a .. segment"; return; } v="$("${AS_USER[@]}" "$CLAUDE" --version 2>/dev/null | sed -n 1p)" if [ "$v" = "$CLAUDE_PIN (Claude Code)" ]; then fpass F6 claude-pin "$CLAUDE prints '$v'" else ffail F6 claude-pin "$CLAUDE --version prints '$(clean "$v")', not '$CLAUDE_PIN (Claude Code)'"; fi @@ -291,10 +296,27 @@ check_f8() { else ffail F8 credential-file "$CRED is '$s', not '$S_USER 600 regular file'"; fi } +# Empty when $1 and every directory above it, up to /, are the running user's (root's on the host) with +# no group or other write: case2-unit.sh's root_only rule for --node, checked again before root runs it. +# Otherwise the first path that breaks the rule. A symlink breaks it, because its mode is 777. +not_root_only() { + local p="$1" st + while :; do + st="$(stat -c '%u %a' -- "$ROOT$p" 2>/dev/null)" || { echo "$p (cannot stat)"; return; } + [ "${st% *}" = "$(id -u)" ] && (( (8#${st#* } & 8#022) == 0 )) || { echo "$p (uid ${st% *}, mode ${st#* })"; return; } + [ "$p" = / ] && return + p="$(dirname -- "$p")" + done +} + check_f9() { - local s v + local s v bad s="$(stat -c '%U:%G %a %F' -- "$ROOT$SETTINGS" 2>/dev/null)" || s="missing" [ ! -L "$ROOT$SETTINGS" ] && [ "$s" = "root:root 644 regular file" ] || { ffail F9 autoupdater-off "$SETTINGS is '$s', not 'root:root 644 regular file'"; return; } + # Root runs node here, so it runs only the pinned path, and only when nothing but root can replace it. + [ "$NODE" = "/opt/mosaic-stack/node-$NODE_PIN/bin/node" ] || { ffail F9 autoupdater-off "root won't run --node $NODE: it is not the --node-pin path"; return; } + bad="$(not_root_only "$NODE")" + [ -z "$bad" ] || { ffail F9 autoupdater-off "root won't run --node $NODE: $bad is not root's alone"; return; } v="$("$ROOT$NODE" -e 'const s=JSON.parse(require("node:fs").readFileSync(process.argv[1],"utf8"));process.stdout.write(JSON.stringify(s?.env?.DISABLE_AUTOUPDATER??null))' -- "$ROOT$SETTINGS" 2>/dev/null)" || v="unreadable" if [ "$v" = '"1"' ]; then fpass F9 autoupdater-off "$SETTINGS is root:root 0644 with env.DISABLE_AUTOUPDATER \"1\"" else ffail F9 autoupdater-off "$SETTINGS env.DISABLE_AUTOUPDATER is $v, not \"1\""; fi