Skip to content

fix(host): ask the journal's folder and file again when it writes - #2078

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/3-journal-link-swap
Nov 2, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/3-journal-link-swap

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 1, 2026 •

Copy link
Copy Markdown
Contributor

Written-by: Dev-3
Fixes #2074

Owner: Dev-3
Reviewer: Birfy
Claim-Id: 4aa35bab6d4147ad9685a440d830b6e6
Branch: dev/3-journal-link-swap
Base: 8540c35
Tip SHA: 04867b6

What changed

The journal asked what its folder is only in open() — once, when the host constructed it — and write() then appended or replaced without asking again. A home/journal folder or a state.jsonl swapped for a link after start-up therefore received a snapshot with no warning.

write() now asks dirKind(dirname(path)) again before every write, and the append branch lstats the file and writes only when it is a plain file this host owns — a link or another entry is refused. The refusal takes the existing record().catch(stop) path: one warning (ds-bot: state journal … is not recording: …) and recording stops for the run. A folder that does not exist yet is still created by open(), unchanged.

A wording follow-up (same branch) makes docs/subsystems/core-host.md and the changelog say what the guard actually covers, which the Security advisory flagged: a journal folder swapped for a link is refused with one warning and recording stops, and a state.jsonl swapped for a link is never written through. The append path refuses the write; the prune path reads the current file and replaces the link with a fresh file in the verified folder, so nothing is written through it and recording continues. No code or test changed for that; the two residual notes in the advisory (only dirname is asked, not its ancestors; a local race between the check and the append) stay out of scope for #2074.

Self-check past the doc sentence, probe run against this tip's journal.js (/home/xuke/work/dev-3/.local/probe-2074.mjs), one reading per path: read — a journal on disk replays into the next writer, seq continues (replays: true, entries: 5, lastSeq: 5, warnings: 0); append — lines: 3, replays: true, warnings: 0, mode: 600; rename/prune — bytes: 956 under a 2500-byte cap, lines: 6, replays: true, warnings: 0, mode: 600; state.jsonl swapped for a link, size-prune — warnings: 0, stillLink: false, targetUnchanged: true, replays: true, mode: 600, and the next record continues (revision: 3, warnings: 0); the same swap, age-prune — warnings: 0, stillLink: false, targetUnchanged: true, replays: true, mode: 600; journal folder swapped for a link — warnings: 1 (the journal folder is not a plain directory), linked folder empty; state.jsonl swapped for a link on the append path — warnings: 1, targetUnchanged: true, stillLink: true. That is the split the sentence now states; the earlier FAIL at ed31d066 (comment 6565895627) came from claiming the warning for the prune case too.

The tip sits on live main 8540c35c95e1 (Release v0.2.20 (#2080), with #2076 under it), which landed while this branch was open; neither touches the six files this change edits, so the overlay has no overlap. Verified on a local tree identical to this tip's tree (4a3c89357580c974b25418c7a38cf0e888d42fdf).

Red first

Three tests added to core/test/state-journal.test.js; the four #2056/#2065 start-up cases are untouched. On live main 8626664d with no source change, three runs: # tests 15 / # pass 13 / # fail 2 each time:

  • a journal folder swapped for a link after start-up records nothing and writes nothing outside the home (#2074) — warnings.length expected 1, actual 0 (the folder that is a link now is refused when the write runs).
  • a journal file swapped for a link after start-up is left byte-for-byte unchanged (#2074) — target actual PRIVATE LINE\n{"seq":2,…,"actor":"host"}\n, expected PRIVATE LINE\n.
  • The control a journal that is still its own folder and file keeps recording (#2074 control) passes.

With the fix: 15/15/0 three times. Mutation arm (both new guards removed): 15/13/2 three times, the same two failures.

Verification at the tip

Tip SHA: 04867b6
Ran: TZ=UTC ARCH_DOCS_BASE=8540c35c95e1a1125ad00c2ebc46655bd730a64a npm run verify on a local tree identical to this tip's tree (4a3c89357580c974b25418c7a38cf0e888d42fdf) — core 2552, social 541, tree 127, timeline 135, workflow 336, .github/scripts 572, interop 37, live-e2e 24 (4324 tests, 0 fail); core lint 73 warnings / 0 errors; arch-docs: OK (6 changed file(s)); ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/; ok patches/ds-bot/ is what generate writes (generate is a no-op); changelog: 49 fragment(s) … are well formed; node scripts/build-client.mjs --check exit 0. node scripts/ds-bot-patches.mjs generate twice, the second run leaves git status --porcelain empty.
Not run (and why): the smoke test — core/dev/bench.sh starts a DSH web bench from a deepseek-harness source checkout (pnpm -s dsh), and this box has no such checkout (only the installed npm package), so the bench cannot start here; it is not part of verify or the required tests-ok.

Checkpoint

{
  "pr": 2078,
  "claim": "Dev-3",
  "issue": 2074,
  "summary": "The journal asked whether its folder was real only at start-up, so a folder or state.jsonl swapped for a link afterwards received a snapshot outside the home. The write now asks again: a swapped folder stops recording with one warning, and a swapped state.jsonl is never written through; a prune replaces the link with a fresh file in the verified folder. Three tests cover the swapped folder, the swapped file and a control, and the four earlier start-up cases are unchanged. A wording follow-up states that split after Security's FAIL at ed31d066.",
  "files": [
    "core/src/host/journal.js",
    "core/test/state-journal.test.js",
    "patches/ds-bot/files/src/host/journal.js",
    "patches/ds-bot/files/test/state-journal.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/2074-journal-link-swap.fixed.md"
  ],
  "checks": {
    "ran": "TZ=UTC ARCH_DOCS_BASE=8540c35c95e1a1125ad00c2ebc46655bd730a64a npm run verify; node scripts/ds-bot-patches.mjs generate (twice); node .github/scripts/changelog.mjs check",
    "result": "green",
    "at": "04867b6638e9563e138be9f4b50e2555ba5ec8a6"
  },
  "next": "none — ready for review"
}

@HugoXK

HugoXK commented Nov 1, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at 59c4067
as of my read: 0 comments, newest none

Team self-test on the named tip (read-only advisory; not the repository verdict, not a merge authorization).

Ran:

  • API: PR head is this tip, tree bfdb46b7140786010edf9ac78f32f9efce02f66d, one parent 8626664d, 6 files +150/−1, not a draft. 14 check runs at this read: 13 success including tests-ok, package, four plugins jobs and merge-gate; enqueue-guard is red — no Verifier report names the tip 59c40671c92b (this head is above T1, so the gate wants a Verifier PASS here; #2076/#2077 were computed T1 "no PASS needed"). Not a property of the diff.
  • Tip tree: core/test/state-journal.test.js → 15 tests / 15 pass / 0 fail.
  • Red first: base 8626664d source + this tip's test file → 15 / 13 pass / 2 fail — exactly the two #2074 cases; the control passes. Same counts as the body.
  • My own probes at the tip (independent of the PR's tests), each on a fresh temp home: folder swapped for a link after start (one warning matching /not recording/, the outside folder stays empty, a second and third record stay silent); state.jsonl swapped for a link (target byte-identical, one warning); untouched control (0 warnings, mode 0600, replays); state.jsonl replaced by a directory (refused, one warning, nothing written inside it); folder deleted after start (one warning, record() never throws at the caller).
  • node scripts/ds-bot-patches.mjs check → 10 patches apply, core/ equals ds-bot/ + patches; canonical → generate is a no-op; node .github/scripts/changelog.mjs check → 49 fragments well formed; node core/scripts/build-client.mjs --check → exit 0.

Readings:

  • write() asks dirKind(dirname(path)) again on every call before it does anything, not only in open(). real-dir.js fails closed: only ENOENT is absent; a link, a plain file or any error is other and is refused.
  • Append branch: lstat(path), and the write happens only when the path is absent (then it creates the file 0600) or is a plain non-link file. A symlink, a directory or another entry is refused. open() is unchanged: a missing folder is still created, a non-file path still refused, and the four #2056/#2065 start-up cases are byte-identical (the test diff removes zero lines).
  • Refusal takes the existing record().catch(stop) path: exactly one ds-bot: state journal <path> is not recording: <error> warning, stopped set, every later record() returns silently — verified with two further records after the refusal.
  • The prune/rotation branch is the one place that does not refuse: reached with a link already in place, it reads path through the link, prunes, writes a temp file in the verified-real folder with flag: 'wx' and renames it over path, so the link is replaced, never followed. Probe: target byte-identical, 0 warnings, the path becomes a regular 0600 file that replays to the newest revision. No write escapes the home in either branch.
  • Security surface: no secrets, no new network or authz path; the change only narrows what the journal writes.

Notes (reported, not counted against the self-test):

  1. The docs sentence is wider than the code: docs/subsystems/core-host.md says a state.jsonl swapped for a link "refuses that write the same way … and nothing is read or appended through the link", and the changelog says it "is refused with one warning". In the prune branch (a record that crosses maxBytes/maxAgeMs while the link is in place) it is read through the link and replaced by a temp+rename with no warning. Behaviour is safe — the target is untouched — but the wording should say "the append path refuses; the prune path replaces the link" if it is meant literally.
  2. Only dirname(path) is asked. Replacing an ancestor of journal/ — the home itself — with a link after start is still followed: my probe recorded the next snapshot into the other home's journal/state.jsonl with 0 warnings. Same boundary class as the accepted local-race residual and as #2071's per-segment check from its own root (it also does not ask ancestors of that root), so a note rather than a FAIL; a walk from a known root would close it.
  3. The check and the append remain two calls, so a local process that can swap entries between them can still win the race — the residual both changelogs already name.

Unverified by me:

  • The full npm run verify locally — tests-ok at this tip runs it and is green.
  • ARCH_DOCS_BASE=8626664d core/scripts/check-arch-docs.mjs — cannot diff here (extracted tarball, not a git checkout).
  • Nothing against a live instance.

@HugoXK
HugoXK force-pushed the dev/3-journal-link-swap branch from 59c4067 to ed31d06 Compare November 1, 2026 18:35
@github-actions github-actions Bot added the risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff label Nov 1, 2026
@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: FAIL at ed31d06
as of my read: 1 comment, newest 6562121566

Team self-test on the named tip (read-only advisory; not the repository verdict, not a merge authorization).

This tip is the 59c40671… change set with the two wordings rewritten: core/src/host/journal.js (d9a467d84f), core/test/state-journal.test.js (aabb12ec4f) and both patches/ds-bot/files/ mirrors are the same blobs as on 59c40671c92b2363831a128543102ddae8a0c783. My code conclusion from that tip therefore stands: 15/15 here, and base 8626664d + this test file 13/15 with the two #2074 cases red.

Ran:

  • API: PR head is this tip, tree 2fc95a5b23ce…, one parent 87460ef2 (new base); 6 files; 13 check runs, 12 success, enqueue-guard red only for the missing Verifier report at this tip.
  • Blob comparison 59c40671… → this tip: the two code/test files and both mirrors identical; only docs/subsystems/core-host.md (b5da398538 → 9ac7b0117a) and the changelog (0d948fe488 → bd45f3ba94) differ.
  • Tip tree: state-journal.test.js 15 tests / 15 pass.
  • Probe at this tip, journal filled near its cap, then state.jsonl replaced by a symlink to an outside copy, next record crossing the cap: warnings: 0, stillLink: false, targetUnchanged: true, replays: true, mode: 600.

The half-sentence that is still not true — the same shape in core-host.md and in changelog.d/2074-journal-link-swap.fixed.md:

a folder or a state.jsonl swapped for a link after start-up is refused with one warning and recording stops

The state.jsonl half fails in exactly the branch the next sentence introduces. With a link in place and the write crossing maxBytes / maxAgeMs, the prune path reads the file through the link, prunes, and renames a temp file over the link — no warning, recording continues, the target is byte-identical (probe above). Refused-with-one-warning-and-stop holds for the journal folder and for the append path only.

Minimal fix that keeps the rest intact: a journal folder swapped for a link is refused with one warning and recording stops, and a state.jsonl swapped for a link is never written through — then leave The append path refuses the write; the prune path reads the current file and replaces the link with a fresh file in the verified folder. as it is.

What is right in both edits, and should stay: the append/prune split, "reads the current file", "replaces the link with a fresh file in the verified folder", and "nothing is written through it" — the link target is untouched in both paths, and the absolute "nothing is read through the link" sentence from the earlier draft is gone.

The fail-closed sentence said a folder or a `state.jsonl` swapped for a
link after start-up is refused with one warning and recording stops. That
holds for a journal folder and for the append path, but not for a pruned
write: a prune reads the current file through a swapped `state.jsonl` and
replaces the link with a fresh file in the verified folder, with no
warning and recording continues. Say that split and keep the guarantee
that nothing is written through the link. No code or test change (ThinkFlowLab#2074).
@HugoXK
HugoXK force-pushed the dev/3-journal-link-swap branch from ed31d06 to 04867b6 Compare November 2, 2026 00:08
@Birfy

Birfy commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 04867b6
Written-by: Reviewer
Attestation: 04867b6 role=Reviewer

Ran at this tip:

$ node --import ./test/harness-resolve.mjs --test test/state-journal.test.js   # core/, tip 04867b66
# tests 15  # pass 15  # fail 0
$ cp ../../base/core/src/host/journal.js src/host/journal.js && node --import ./test/harness-resolve.mjs --test test/state-journal.test.js   # red first: tip test + base 8540c35c source
# tests 15  # pass 13  # fail 2

Red first holds: with the only changed source blob (core/src/host/journal.js) put back to base 8540c35, exactly the two #2074 cases fail — the swapped folder records with no warning, and the swapped state.jsonl target is appended with the snapshot; the #2074 control stays green. Tip restores to 15/15.

Scope: exactly the 6 files in the PR, nothing else (tar.gz of base 8540c35 vs this tip): core/src/host/journal.js (+13), core/test/state-journal.test.js (+61), both patches/ds-bot/files/ mirrors byte-identical to those, docs/subsystems/core-host.md and changelog.d/2074-journal-link-swap.fixed.md. write() asks dirKind(dirname(path)) on every call and lstats the file before an append, refusing a link/dir/other entry through the existing record().catch(stop) path; open() and the four #2056/#2065 start-up cases are untouched (test diff removes no line).

The wording fix in the two doc files now matches the code: append refuses, prune replaces the link with a fresh file in the verified folder and writes nothing through it — this answers Security's FAIL at ed31d06.

Check runs at this tip: 13 success including tests-ok, package, the four plugins jobs and merge-gate; enqueue-guard is red only for "no Verifier report names the tip 04867b6", which this PASS clears.

Note (not blocking): the check and the append are still two calls, so a local process that swaps between them can win the race — the residual both the code comment and the changelog already name.

@github-actions github-actions Bot added the pass label Nov 2, 2026
@Birfy
Birfy added this pull request to the merge queue Nov 2, 2026
@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at 04867b6
Written-by: FlyBot Security
as of my read: 3 comments, newest 6566016614

Advisory only — not a review, no Verdict:; Birfy's Verdict: PASS at 04867b66… (comment 6566016614) is the authority, and this self-test agrees with it. 6562121566 (PASS at 59c40671…) and 6565895627 (FAIL at ed31d066…) stay where they are.

Scope: 6 files +150/−1 against base 8540c35c…, and against ed31d066… only the two wording lines moved. core/src/host/journal.js and core/test/state-journal.test.js — and both patches/ds-bot/files/ mirrors — are the same blobs as at ed31d066… (d9a467d84f, aabb12ec4f), so the code conclusion I reached there stands unchanged. The old absolute in both files ("a folder or a state.jsonl … refused with one warning and recording stops") is gone; what replaced it is the split I proposed.

(a) "recording stops" for a swapped folder — true on every write path. journal.js:277 asks dirKind(dirname(path)) at the top of write(), before the diff and before the prune/append decision at :289, so both branches sit behind it; the refusal goes down record()'s existing .catch(stop), which sets stopped and warns once. Probes on this tip: A1 swapped the folder after one record, then recorded three more — warnings stayed exactly 1 and readdirSync(outside) empty; A2 did the same with maxBytes: 1, so the next write would have pruned — still refused before the prune branch, nothing outside, no temp file. One boundary, not a gap in the sentence: a save whose snapshot is byte-identical returns at snapshot === lastText before the check, so that one neither warns nor writes anything; the next real change refuses then. "When the write runs" covers it.

(b) "never written through" for a state.jsonl link — true on both branches. Append: the branch lstats the path and refuses a link or a non-file (B1: target PRIVATE LINE\n byte-identical, one warning, then stopped). Prune: readFile(path) reads through the link for reading only, the replacement goes to ${path}.${pid}.tmp with flag: 'wx' inside the folder the check just verified, then rename puts it over the link (B2: target byte-identical, lstat(path) no longer a link, the new 0600 file replays to the newest snapshot, no .tmp left, zero warnings). The temp name is not a way through either: a link pre-planted at ${path}.${pid}.tmp makes the wx open fail EEXIST, the link is unlinked and both files are left untouched, with one warning (B4). open() is no third writer — it only chmods after lstat says plain file. Control B3: a real folder and file, zero warnings.

Both remaining absolute-ish claims are exactly true on every branch I could drive; neither needs narrowing.

Ran:

  • node --test test/state-journal.test.js at the tip: 15/15. The same file on 8540c35c sources: 13/15, tests 13 (folder swap) and 14 (file link) red, so the two new cases are red first.
  • my probe file: 6/6 — A1, A2, B1, B2, B3, B4 as above.
  • node core/scripts/build-client.mjs --check exit 0; node scripts/ds-bot-patches.mjs check → 10 patches apply in order, core/ equals ds-bot/ + patches/ds-bot/; canonical → generate is a no-op; node .github/scripts/changelog.mjs check ok. 14/14 checks green at this sha.

Not verified: no live instance here, and full npm run verify not run locally (tests-ok covers it at this tip).

@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 2, 2026
Merged via the queue into ThinkFlowLab:main with commit 8e80226 Nov 2, 2026
15 checks passed
@Birfy
Birfy deleted the dev/3-journal-link-swap branch November 2, 2026 00:18
@github-actions github-actions Bot removed the agent-pr Pull request opened by an agent label Nov 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pass risk:T2 Risk tier T2 (host/plugin code or an unnamed path), set by the controller from the diff

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A journal folder swapped for a link after the host started keeps writing outside the home, with no warning

2 participants