Repository navigation
A symlinked journal folder lets the host chmod and write into a folder outside the home - #2065
Merged
Merged
Conversation
The prologue of openJournal() created and chmod-ed the folder above <journal>/state.jsonl before looking at anything. When that folder was a symlink, all of it followed the link: the chmod changed a foreign directory's mode to 0700 and every later append, temp file and rename landed inside it. The leaf rule at the journal file was the only check. Ask ThinkFlowLab#2055's shared real-dir helper about the folder first and refuse anything but a plain directory, so recording stops with one warning exactly like every other journal problem, before the mkdir, the chmod or any write. A folder that does not exist yet is still created, as before, and a real folder records unchanged; saving the team never depends on the journal. Fixes ThinkFlowLab#2056
Collaborator
|
Verdict: PASS at b88a23c Ran at this tip: (the second run is the tip tarball with only Review points
|
This was referenced Nov 1, 2026
Closed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2056
Written-by: Dev-2
Owner: Dev-2
Reviewer: Birfy
Claim-Id: 3715e54f5468494999c0e69737d278cc
Branch: dev/2-journal-dir-symlink
Base: c2de64d
Tip SHA: b88a23c
What changed
openJournal()'sopen()opened withmkdir(dirname(path), { recursive: true, mode: 0o700 })andchmod(dirname(path), 0o700), then checked only the journal file (lstat+isFile()). The folder above it had no rule at all, so when<home>/journalwas a symlink to a folder outside the home themkdirwas a no-op, thechmodchanged that folder's mode to0o700, and every laterappendFile, temp file andrenamelanded inside it.The prologue now asks
#2055's shared helper —dirKind()fromcore/src/host/real-dir.js, imported rather than copied — aboutdirname(path)before anything is created or chmod-ed. Only'other'refuses: it throws the sameJournalErrorthe file-shaped rule throws, so the existingopen().catch(stop)path runs — one warning naming the path,stopped = true, and nomkdir, nochmod, no write through the link. A share of the rule is not carried a second time: no lstat, noisSymbolicLink()call outside the helper.'real'records exactly as before.'absent'still creates the folder (mkdir0700 +chmod0700): that is the fresh-home case, and refusing it would stop every first-run journal — the mutation table above shows it reds two existing#511cases. The leaf rule at the journal file (lstat+isFile()) is untouched, andstore.js's call site andconfig.stateJournalare unchanged, so saving the team never depends on the journal.Four cases in
core/test/state-journal.test.js: a link at a real folder outside the home (one warning,stoppedtrue, the target's0o755stays0o755, nothing written into it), a link at a missing path (nothing created), a plain file (no throw escapes, the file is left as it was), and the control — a real folder still records, replay still works, folder0o700, file0o600. One sentence ofdocs/subsystems/core-host.mdnames the new rule.Red first
On the base
c2de64d6f97d7b1d218118b2b81a2190ff19ad82source (core/src/host/journal.jsreverted to base, the four new tests kept): 12 tests / 11 pass / 1 fail, three runs with an identical result.a journal folder that is a symlink to a real folder outside the home records nothing and stays silent on disk (#2056)— the observable harm: the target folder's mode became0o700and the journal file was written through the link.The other three new cases answer the same way on the base and are guards, not reds: a link at a missing path already failed its
mkdir(ENOENT), a plain file already failed it (EEXIST), and a real folder always recorded — the new check must leave all three exactly as they are, which is what M2 below is about.At this tip the same file is 12 tests / 12 pass / 0 fail.
Mutations: each arm reds its own cases
Each applied to
core/src/host/journal.jsalone at this tip, the file run, then restored from copy (tree clean afterwards):mkdir/chmodprologue returnsdirKind() === 'real'(i.e.'absent'stops too)#511: a recorded 3-Bot interaction replays to a byte-identical state.json at every stepand#511: the writer prunes on disk and keeps the file privateM2 is why
'absent'stays a create: a fresh home has nojournalfolder yet, and the two#511cases that build one depend on this writer making it. The refusal arm is'other'alone.Verified on the tree this tip carries
TZ=UTC ARCH_DOCS_BASE=c2de64d6f97d7b1d218118b2b81a2190ff19ad82 npm run verifyin the clean tree at7959103bbbde9c71590e2c866e77d2e126bb5ec7(treecef8d3d7c87e98cd77ff0f73abd3cb632885e2a4), exit 0. The pushed tipb88a23c2carries that same tree; the push helper re-creates the commit on the live base, so its sha differs and its content does not.2521/2521— live basec2de64d6plus this pull request's four new cases. social541(540pass +1skip), tree127(126+1), timeline135(134+1), workflow336/336,.github/scripts563/563, interop37/37, live-e2e24/24. Skip counts are host-dependent and are reported as this run produced them.arch-docs: OK (6 changed file(s))against basec2de64d6; core lint73 warnings and 0 errors, none in a line this diff adds; plugin lints36 / 9 / 2 / 16warnings,0errors;build-client --checkexit 0.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)—node scripts/ds-bot-patches.mjs generatere-emitted the twopatches/ds-bot/files/mirrors and the tree was clean afterwards.core/dev/bench.sh smoke). The diff is one refusal in the journal writer's open path plus tests — no endpoint, dispatch, turn or delivery path is touched.Why
'absent'is allowed on purposeA folder that does not exist yet is not refused; the writer creates it, which is the meaning
real-dir.jsgives the middle state ("nothing to read" for a reader, "create it" for a writer) and what a fresh home depends on — in the mutation table above, refusing'absent'reds two existing#511cases that build the folder by saving. Only'other'stops: a link at a folder, a link at a path that does not exist, a plain file, or a check that could not run at all. That check happens at the top ofopen(), before themkdir, before thechmodand before any write, so when it refuses, the folder a link names is never chmod-ed and noappendFile, temp file orrenamereaches it: the target keeps its own mode (0o755in the case below) and stays empty. Recording stops through the existingstop()path — one warning naming the path,stopped = true— and saving the team keeps working.Boundaries this change accepts
This change judges one entry: the journal folder itself (
dirname(path)). It does not walk the ancestors. When the home itself is reached through a link, the journal folder inside that target is a real directory and is treated as this installation's own — deliberately: the journal code names that folder, and a home reached by a configured path is how the install is set up. The leak this issue closes is a link placed inside the home; whether the home's own path must also be link-free is a separate question and not part of this change.Accepted boundary: someone who can already write the home can replace the folder name with a link between the check and the
chmod.mkdirdoes not follow a link (EEXIST), butchmoddoes, so the exposure shrinks from a persistent link to a race. A local attacker able to write the home is outside this issue's threat model; closing that window would mean opening the folder withO_NOFOLLOWandchmod-ing that descriptor, which is a separate change.Scope
core/src/host/journal.js(open()'s prologue only, one import),core/test/state-journal.test.js(four appended cases), one sentence ofdocs/subsystems/core-host.md, one changelog fragment, and the twopatches/ds-bot/files/mirrorsgeneratewrites.store.js, the leaf rule, replay, retention and the client are untouched.Checkpoint
{ "pr": 2065, "claim": "Dev-2", "issue": 2056, "summary": "openJournal() created and chmod-ed the folder above the journal before checking anything, so a symlink there let the host chmod a folder outside the home to 0700 and write the journal into it. The prologue now asks the shared real-dir helper about that folder and refuses anything but a plain directory, stopping with one warning before any mkdir, chmod or write. A real folder records as before and a folder that does not exist yet is still created, which two existing #511 cases depend on. Four tests pin the three refused shapes and the control.", "files": [ "core/src/host/journal.js", "core/test/state-journal.test.js", "docs/subsystems/core-host.md", "changelog.d/2056-journal-folder-symlink.fixed.md", "patches/ds-bot/files/src/host/journal.js", "patches/ds-bot/files/test/state-journal.test.js" ], "checks": { "ran": "TZ=UTC ARCH_DOCS_BASE=c2de64d6f97d7b1d218118b2b81a2190ff19ad82 npm run verify", "result": "green", "at": "7959103bbbde9c71590e2c866e77d2e126bb5ec7" }, "next": "none — ready for review" }