docs(slice1): darkwing row 39 S4 round 2 review record (approve)

Co-Authored-By: Claude Opus 5.5 <[email protected]>
2026-10-08 20:01:06 -05:00
parent 26bd829c1d
commit 3c18473d11
4 changed files with 134 additions and 0 deletions
@@ -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
@@ -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 });
@@ -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"
@@ -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 `[email protected]`
+  and `[email protected]`.
+
+## 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
+   `<dataRoot>/notify/<business>/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 <dataRoot>/notify/<business>`.
+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
+   `<dataRoot>/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.