From 3c18473d11549af230b8b130d2e5bd7db05231b7 Mon Sep 17 00:00:00 2001 From: Jason Woltje Date: Thu, 8 Oct 2026 20:01:06 -0500 Subject: [PATCH] docs(slice1): darkwing row 39 S4 round 2 review record (approve) Co-Authored-By: Claude Opus 5.5 --- .../work/slice1-s4-review/r2-f1-probe.txt | 9 ++ .../work/slice1-s4-review/r2-f1probe.mjs | 16 +++ .../work/slice1-s4-review/r2-mutants.txt | 9 ++ .../work/slice1-s4-review/review-r2.md | 100 ++++++++++++++++++ 4 files changed, 134 insertions(+) create mode 100644 agents/darkwing/work/slice1-s4-review/r2-f1-probe.txt create mode 100644 agents/darkwing/work/slice1-s4-review/r2-f1probe.mjs create mode 100644 agents/darkwing/work/slice1-s4-review/r2-mutants.txt create mode 100644 agents/darkwing/work/slice1-s4-review/review-r2.md diff --git a/agents/darkwing/work/slice1-s4-review/r2-f1-probe.txt b/agents/darkwing/work/slice1-s4-review/r2-f1-probe.txt new file mode 100644 index 00000000..d15adfc8 --- /dev/null +++ b/agents/darkwing/work/slice1-s4-review/r2-f1-probe.txt @@ -0,0 +1,9 @@ +fresh -> opened file created +mode-700 -> opened file created +mode-750 -> refused exit 3 file created false +mode-755 -> refused exit 3 file created false +mode-711 -> refused exit 3 file created false +mode-701 -> refused exit 3 file created false +mode-770 -> refused exit 3 file created false +mode-500 -> refused exit undefined file created false +symlink-to-0700 -> refused exit 3 file created false diff --git a/agents/darkwing/work/slice1-s4-review/r2-f1probe.mjs b/agents/darkwing/work/slice1-s4-review/r2-f1probe.mjs new file mode 100644 index 00000000..f6871d04 --- /dev/null +++ b/agents/darkwing/work/slice1-s4-review/r2-f1probe.mjs @@ -0,0 +1,16 @@ +import { mkdirSync, chmodSync, symlinkSync, mkdtempSync, existsSync, rmSync } from "node:fs"; +import { join } from "node:path"; +const { openJournal } = await import(process.argv[2] + "/packages/cli/src/notifier.mjs"); +const base = mkdtempSync(join(process.env.TMPDIR, "f1-")); +const tryMode = (label, setup) => { + const dir = join(base, label); + setup(dir); + const file = join(dir, "sent.jsonl"); + try { openJournal(file); console.log(label, "-> opened", existsSync(file) ? "file created" : ""); } + catch (e) { console.log(label, "-> refused exit", e.exitCode, "file created", existsSync(file)); } +}; +tryMode("fresh", () => {}); +for (const m of [0o700, 0o750, 0o755, 0o711, 0o701, 0o770, 0o500]) tryMode("mode-" + m.toString(8), (d) => { mkdirSync(d); chmodSync(d, m); }); +tryMode("symlink-to-0700", (d) => { const real = d + "-real"; mkdirSync(real, { mode: 0o700 }); chmodSync(real, 0o700); symlinkSync(real, d); }); +for (const m of [0o500]) chmodSync(join(base, "mode-" + m.toString(8)), 0o700); +rmSync(base, { recursive: true, force: true }); diff --git a/agents/darkwing/work/slice1-s4-review/r2-mutants.txt b/agents/darkwing/work/slice1-s4-review/r2-mutants.txt new file mode 100644 index 00000000..66cab367 --- /dev/null +++ b/agents/darkwing/work/slice1-s4-review/r2-mutants.txt @@ -0,0 +1,9 @@ +Row 39 round 2, Darkwing. Tree: worktree at b9b6cf00, build.patch applied, candidate-manifest 29/29 OK. +Each mutant edits one file, runs one test file, then restores it. The manifest checked 29/29 OK afterwards. + +MF1 notifier.mjs openJournal: drop "|| (ds.mode & 0o077) !== 0" from the directory check + node --test packages/cli/tests/notifier.test.mjs: 15 pass, 1 fail (killed) + fails: "the journal: a loose file mode, a loose directory or a symlinked journal refuses" +MF3 mosaic-bus.service.in: add After=network-online.target and Wants=network-online.target under Description= + node --test packages/cli/tests/host.test.mjs: 8 pass, 1 fail (killed) + fails: "bus-service.sh renders the unit and installs it into a given directory" diff --git a/agents/darkwing/work/slice1-s4-review/review-r2.md b/agents/darkwing/work/slice1-s4-review/review-r2.md new file mode 100644 index 00000000..4a5628e9 --- /dev/null +++ b/agents/darkwing/work/slice1-s4-review/review-r2.md @@ -0,0 +1,100 @@ +# Slice 1 S4, row 39, round 2 review (Darkwing) + +Issue #1521, request comment 26854, queue rev 191, pushed at `26bd829c`. +Packet: `agents/rocko/work/slice1-s4/`, base `b9b6cf00`, 29 files. +Candidate manifest sha256 +`e858504e54d5b06581ec8b7168b82cd4895491c980aae1c62bd8a8be5e4f2f17`, +`build.patch` sha256 +`b52f7d6800538cdaf9df29300ad37672d00552e7086f496606722df3967e88f8`. +My round 1 verdict was approve, comment 26843, rev 178. + +Verdict: **approve**, comment 26855. F1 and F3 are closed the way I meant them. F2 stays a +follow-up under decision 71, as Sage said. + +## Method + +- A detached worktree at `b9b6cf00`. I ran `git apply build.patch`, then + `sha256sum -c candidate-manifest.sha256`: 29 OK. +- The round 1 patch isn't in git (Rocko's packet is untracked), so I + couldn't make an interdiff. I read the round 2 code for F1 and F3 as it + stands, plus the parts of decision 71 that touch them. +- `r2-f1probe.mjs` calls `openJournal` on directories with different modes + and on a symlinked directory. Output: `r2-f1-probe.txt`. +- I broke each fix and ran its test file. See `r2-mutants.txt`. +- `systemd-analyze --user verify` on the rendered `mosaic-bus@.service` + and `mosaic-bus@acme.service`. + +## Suites + +| Suite | Result | +|---|---| +| `node --test 'packages/cli/tests/*.test.mjs'` | 49/49 | +| `node --test 'packages/discord/tests/*.test.mjs'` | 178/178 | +| `systemd-analyze --user verify` (template and instance) | rc 0, no output | + +Node is v26.8.1. Rocko's gate has the rest. I didn't rerun test-task or +test-release here: Docker can't create new compose networks on this host +right now, and the zai account is over its 5-hour limit. + +## F1: journal directory mode (closed) + +`openJournal` now calls `lstatSync` on the directory after `mkdirSync`. It +refuses with exit 3 unless the directory is a real directory, owned by +this uid, with no group or other bits. That check comes before the journal +file is opened, so a refusal leaves no `sent.jsonl` behind. + +| Directory | Result | +|---|---| +| created by `openJournal` | opens | +| 0700 | opens | +| 0750, 0755, 0711, 0701, 0770 | exit 3, no file created | +| symlink to a 0700 directory | exit 3, no file created | +| 0500 | refused, raw `EACCES` (note 2) | + +With the `(ds.mode & 0o077) !== 0` clause removed, "the journal: a loose +file mode, a loose directory or a symlinked journal refuses" fails. The +test holds the fix. + +The cli README, `docs/TOOLS.md` and `BUILD.md` all tell the operator to +create the directory 0700 first. + +## F3: network-online.target (closed) + +`mosaic-bus.service.in` has no `After=` or `Wants=` lines. The README +explains that a user unit can't order on the system's +`network-online.target` and that the notifier retries Discord itself. The +host test asserts that the rendered unit doesn't match `/network-online/`. +Putting the two lines back fails "bus-service.sh renders the unit and +installs it into a given directory". `systemd-analyze --user verify` is +clean. + +## Notes (not blocking) + +1. **The install message doesn't mention the directory.** + `scripts/bus-service.sh` ends with "write + `/notify//notify.json`, mode 0600" and says nothing + about creating the directory 0700. An operator who follows only that + message runs `mkdir -p`, gets 0755, and the host refuses with exit 3. + That is fail-closed, and the error names the problem, but this is the + path that produced F1 in round 1. One more line in the heredoc would do + it: `mkdir -m 0700 -p /notify/`. +2. **A 0500 directory gives a raw `EACCES`.** It has no group or other + bits, so it passes the directory check. Then `openSync` with `O_CREAT` + fails, and `openJournal` rethrows anything other than `ELOOP` as it + came. The error isn't a `CliError` and carries no exit code. The host + still exits 3: + `notifier-process.mjs` catches it and sends `{ok:false}`, and the host + turns that into "notifier refused to start" with exit 3. Exit 3 is in + `RestartPreventExitStatus`, so systemd doesn't loop. +3. **The check and the open use the path separately.** `lstatSync(dir)` + runs, then `openSync(file, ... O_NOFOLLOW)`. Someone who can write to + `/notify` could swap the directory between the two calls. + `O_NOFOLLOW` covers only the last component. That needs write access + to a directory the operator owns, so I'd leave it. Opening the directory + once and checking with `fstat` would close the gap if anyone cares later. + +## Files + +- `review-r2.md`, this file. +- `r2-f1probe.mjs`, `r2-f1-probe.txt`: the F1 probe and its output. +- `r2-mutants.txt`: MF1 and MF3.