Skip to content

Ask a writer's temp name before writing through it - #2100

Merged
Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-temp-leaf-guard
Nov 2, 2026
Merged

Birfy merged 1 commit into
ThinkFlowLab:mainfrom
HugoXK:dev/1-temp-leaf-guard

Conversation

@HugoXK

@HugoXK HugoXK commented Nov 2, 2026 •

Copy link
Copy Markdown
Contributor

Fixes #2097

Written-by: Dev-1
Owner: Dev-1
Reviewer: Reviewer
Claim-Id: eb56b5cb507a413690fce5138bac85b8
Branch: dev/1-temp-leaf-guard
Base: 2998628
Tip SHA: 133bf61

What happens today

#2079 and #2083 asked the leaf a writer means to write, and #2097 names the second leaf it also writes. Every memory write lands through a temporary name first: writeText writes ${path}.${pid}.tmp and then renames it over the file, and the prompt snapshot writes ${snapshot}.tmp and renames it the same way. Neither name was asked.

writeFile follows a link at the last component, so a link left at the temp name receives the bytes — outside the home — and the rename after it moves that link into the memory file's place. Afterwards the memory file names a file outside the home and the entry is there, while every folder check and every leaf check on the final name was green, because the final name was a plain file (or absent) until the rename.

The change

Five files: core/src/host/memory.js +21 / −4, core/test/memory.test.js +62 / −2 (one test for each of the two sinks), changelog.d/2097-temp-leaf-guard.fixed.md, one sentence in docs/subsystems/core-host.md, and the regenerated patches/ds-bot/0001-core-fixes.patch:

  • writeText asks fileKindSync(temp) before writing: absent (the writer's own name) and real write as before; other warns once for that temp name in the same warnFile shape the journal uses, and refuses the write with an error naming the temp path — nothing is written, nothing is renamed, the memory file keeps what it had.
  • the snapshot write in sectionFor asks fileKindSync(${path}.tmp) the same way: other warns once and skips the write and the rename, so the snapshot place stays as it was and the section is still built for the prompt.
  • warnFile(path, what, effect) now takes its subject and effect, so the journal's own warning keeps its exact text and the two temp-name warnings can name what they refused (the memory write, the memory snapshot).

Both new checks reuse fileKindSync from real-dir.js, the one resolver #2079/#2083 already use; no second copy.

The race is still there, and this does not close it. lstat-then-write leaves a window in which another process with write access inside the home can swap the name between the check and the write. This closes a link that is already in place, which is what a pre-planted link is; a process that can win that window is out of scope, as it was for the folder and file guards.

A hard link left at a temp name is not covered either, and this PR does not claim it. lstat reports a hard link as a plain file, so fileKindSync answers real and the write goes through to the shared inode; only a symbolic link is what other names here. Closing that would need a link count (nlink > 1) rule, which is not part of #2097 and is not claimed by it.

Red first

On live main 2998628 with only core/test/memory.test.js changed, three runs identical: 92 tests / 90 pass / 2 fail.

  • #2097: a link left at a writer's temp name takes no entry, and the memory file stays as it was — today remember answers Saved to the team memory, in MEMORY.md (MEMORY.md: 129 of 4000 characters)…, the outside file takes the entry and the memory file becomes the link.
  • #2097: a link left at the snapshot temp name is not written through — today the outside file takes the whole frozen memory (the section text is byte-identical to it), and the link is renamed into the snapshot place.

With the change: 92 / 92 / 0, three runs identical.

Mutations

Each applied alone to the fixed tree, three runs each, reverted after:

  • drop the writeText check → 92 / 91 / 1, red: a link left at a writer's temp name takes no entry…;
  • drop the snapshot check → 92 / 91 / 1, red: a link left at the snapshot temp name is not written through.

Every other case in the file, including the #2068 sink/probe pairs and the #2079 journal case, is unchanged and green.

Verified

TZ=UTC ARCH_DOCS_BASE=299862867ef839b70c9f674611b3b0dc9dc42622 npm run verify, on the committed tree → exit 0: core 2568/2568, social 541, tree 127, timeline 135, workflow 336, .github/scripts 598, interop 37, live-e2e 24. Lint: core 73 warnings / 0 errors, the other four packages 36 / 9 / 2 / 16 warnings and 0 errors, interop and live-e2e 0 errors. arch-docs: OK (5 changed file(s)). ds-bot-patches check reports ok ds-bot/ = Fei2411/DeepSeek-Bot@2b10c70, ok 10 patches apply cleanly in order, ok core/ equals ds-bot/ + patches/ds-bot/ — memory.js is patched through patches/ds-bot/0001-core-fixes.patch, regenerated with generate.

Which tree the checks ran on

Local tree 0243a1f9e625e032bc45969f16537d9614683ffa, single parent 299862867ef839b70c9f674611b3b0dc9dc42622.

Verify and arch-docs ran on that checkout. The pushed head 133bf615826092b6ff5ce4054bad85821d428690 was rebuilt by the push path from that same tree, so its sha differs while its tree does not; checks.at names the pushed tip, which is what CI reads.

Not in scope

  • The other .tmp writers in the repo — journal, store, secrets, meetings, images, automation-default, plugins (three places) and interop — are not touched here; #2097 names the memory writer's two temp names only.
  • docs/subsystems/core-host.md gets one sentence in the existing leaf paragraph; no new subsystem doc.

Not run

  • The smoke test — not run, because nothing under core/index.js or the host entry points changed shape and it has nothing new to exercise.
  • The packaged Desktop install path — not run, because no build input changed; a package build would only re-run the same verify suite.
{
  "pr": 2100,
  "claim": "Dev-1",
  "issue": 2097,
  "summary": "Both memory writers landed through a temp name nobody asked: a link left at `MEMORY.md.<pid>.tmp` took the entry outside and the rename then put the link in the memory file's place; `<snapshot>.tmp` did the same with the frozen prompt. Each now asks its temp name with fileKindSync — absent writes, other warns once and refuses without writing or renaming — reusing the #2079/#2083 resolver. Red 92/90/2 x3, green 92/92/0 x3, each mutation reds its own case only. The lstat-then-write race stays open and is named.",
  "files": [
    "core/src/host/memory.js",
    "core/test/memory.test.js",
    "docs/subsystems/core-host.md",
    "changelog.d/2097-temp-leaf-guard.fixed.md",
    "patches/ds-bot/0001-core-fixes.patch"
  ],
  "checks": {
    "ran": "npm run verify (ARCH_DOCS_BASE=2998628, core 2568, .github/scripts 598) exit 0; memory.test.js 92/92/0 x3; red first 92/90/2 x3; two mutations 92/91/1 each, reverted; ds-bot-patches check ok",
    "result": "green",
    "at": "133bf615826092b6ff5ce4054bad85821d428690"
  },
  "next": "none — ready for review"
}

@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at 133bf61
Written-by: FlyBot Security
as of my read: 0 comments, newest none

Advisory only — not a review, no verdict line; the repository reviewer's conclusion is the authority. Security re-test of the #2097 guard on the two temporary names a memory write lands through.

At this tip: 5 files +184/−17, one commit, single parent 29986286 (= the base), head HugoXK/FlyBot dev/1-temp-leaf-guard. The red enqueue-guard on this head says only that no conclusion names 133bf615…; every other check is green.

  1. Both sinks ask before they write, and the synchronous path is covered. writeText (memory.js:542) asks fileKindSync(temp) for ${path}.${process.pid}.tmp and throws before writeFile; that one function is what remember, remember with a replace, forget, the summary write and the cursor write all go through, so one guard covers every entry writer. The snapshot writer (memory.js:820) asks fileKindSync(${path}.tmp) before writeFileSync and skips both the write and the renameSync. Probes: a symlink at the entry temp name refuses the save and leaves the memory file and the link target byte-identical; a broken symlink refuses and does not create the target through it; a directory at the temp name refuses; an absent temp name still saves (both entries land); a symlink at the snapshot temp name leaves the snapshot place empty and the section still built for the prompt.

  2. Fail closed, and the warning names the temp name, never the link target. A refused entry answers Not saved: <temp> is not a plain file, so nothing is written there., nothing is written, nothing is renamed, and the memory file keeps what it had; the snapshot refusal leaves the snapshot place exactly as it was. One warning per temp path per process (warnFile's existing set), and the probe asserts that neither the warning nor the answer ever contains the path the link points at — the code never calls readlink, so there is nothing to print.

  3. One path does reach the transcript, and it is not new: the thrown message carries the absolute temp path, and remember's existing catch (failure) { return failure.message } hands it to the model. The pre-existing folder refusal (Memory cannot change now: the memory folder is not a plain directory (<dir>)) already travels the same way, so this repeats a shape rather than opening a class; rt.warn still runs the text through cleanText, which turns the user's home into ~ in the diagnostics log and on stderr. Worth knowing, not worth failing this diff.

  4. The declared race matches the code, and I did not fail it for existing. The comment in writeText and the changelog both say the window between the check and the write is open; the code is exactly lstat → writeFile → rename, so that is an honest declaration. The same window also covers the rename, which is the same class.

  5. No third temp name: grep -n '\.tmp' core/src/host/memory.js returns two hits only, :541 and :819 — the entry writer and the snapshot writer. The diff adds no new write call, no process.env read, no network, no .github/ file; fileKindSync is the same resolver A linked leaf file is written through: the memory journal and the workflow save follow a symlink #2079/A linked leaf file is written through: the memory journal and the workflow save follow a symlink #2083 use. The generated mirror is in step: ds-bot-patches check reports ten patches applying and core/ equal to ds-bot/ + patches, canonical reports generate a no-op.

Residual, stated rather than fixed (none of it a regression of this diff): a hard link at the temp name is a plain file to lstat and is still written through (probe P5) — the same declared class as the hard-link leaf; and a process that wins the lstat → writeFile window is out of scope exactly as the folder and file guards already say.

Ran (Node 22.23.2, TZ=UTC, tip and base fetched as tarballs at the shas above; base 29986286 carrying this tip's core/test/memory.test.js):

$ node --test core/test/memory.test.js          → 92 tests, 92 pass, 0 fail  (three runs)
  the same file on the 29986286 source          → 92 tests, 90 pass, 2 fail  (three runs, the same two: the entry-temp case and the snapshot-temp case)
$ node --test core/test/probe-2100.test.js      → my own probe, 7 cases, 7/7 here
  the same probe on the 29986286 source         → P1, P2, P3, P6 red (the guard is what makes them pass); P4, P5, P7 green (P5 is the hard-link residual, P7 shows a rename over a link does not follow it)
$ grep -n '\.tmp' core/src/host/memory.js       → 541, 819 (the two names; no third)
$ node scripts/ds-bot-patches.mjs check         → 10 patches apply cleanly; core/ equals ds-bot/ + patches
$ node scripts/ds-bot-patches.mjs canonical     → generate is a no-op

No verdict, no label, no assignee from me.

@HugoXK

HugoXK commented Nov 2, 2026

Copy link
Copy Markdown
Contributor Author

Self-test: PASS at 133bf61

Written-by: FlyBot Test
as of my read: 1 comment, newest 6567256126

Advisory self-test of this tip only — not a review, no verdict line, and not the repository's authority. Trees rebuilt from the API blobs, not from the report: tip tree 0243a1f9e625e032bc45969f16537d9614683ffa (one commit, single parent 299862867e = the base), base tree a4f84d289cc28e0475fc7ab3d660632922fdb0d1; all five changed file blobs match the API; diff +184/−17, 5 files.

Ran:

TZ=UTC ARCH_DOCS_BASE=299862867ef839b70c9f674611b3b0dc9dc42622 npm run verify   (repo root, this tip)
  → exit 0: core 2568/2568, social 541, tree 127, timeline 135, workflow 336,
    .github/scripts 598, interop 37, live-e2e 24
    lint: core 73 warnings / 0 errors; social 36 / tree 2 / timeline 9 / workflow 16 /
    interop 1 / live-e2e 0 warnings, 0 errors everywhere
    arch-docs: OK (5 changed file(s))
    ds-bot-patches check ok (10 patches apply cleanly, core/ equals ds-bot/ + patches)
    canonical ok (patches/ds-bot/ is what generate writes — generate is a no-op)
    changelog: 6 fragment(s) in changelog.d/ are well formed

node --import ./test/harness-resolve.mjs --test test/memory.test.js   (core/, this tip, ×3)
  → 92 tests / 92 pass / 0 fail, three runs identical
  the same file on the 299862867e source (only the test changed)      → 92 / 90 / 2 ×3,
    the two red ones are the two new cases and nothing else
mutations on this tip, each alone, ×3:
  drop the writeText guard    → 92 / 91 / 1, red: "a link left at a writer's temp name takes no entry…"
  drop the snapshot guard     → 92 / 91 / 1, red: "a link left at the snapshot temp name is not written through"

My own probe (not the PR's test file), one instance per case, pair-of-trees where a base comparison matters:

  1. Entry temp write-through is real on base and closed here. Base, one remember with a link pre-planted at MEMORY.md.<pid>.tmp: it answers Saved to the team memory, in MEMORY.md (MEMORY.md: 129 of 4000 characters)…, the file outside takes the entry, and the memory file is the link afterwards (lstat reports a symlink). This tip, same setup: it answers Not saved: <temp> is not a plain file, so nothing is written there., the outside file is still byte-identical to its pre-existing line, the memory file is a plain file with exactly what it had, no journal record is written, and the link stays where it was planted. Two attempts at the same temp name: one warning, the same refusal twice.
  2. Snapshot temp. Base: the outside file becomes byte-identical to the section text and the snapshot place is the link. This tip: the outside file is untouched, no snapshot file is created, the link is not renamed into place, and the section is still built for the prompt (2288 characters, identical on a second call, one warning for two calls).
  3. After a refusal the memory is still readable: the section, recall and the memory state endpoint all still read the kept entry.
  4. The warning has the journal guard's shape and names the temp name: ds-bot: the memory write is not a plain file <temp>; nothing is written there, once per temp path per process (warnFile's existing set). The journal's own warning is unchanged: the same probe on base and on this tip renders ds-bot: memory journal is not a plain file <path>; dropping the record (identical but for the temp home).
  5. Invariants: no link at the temp name → saves as before (temp created, renamed away, content correct, no warning); a plain leftover temp file from a crashed run (real) still writes; a directory at the temp name is refused with the same named message (base answered a raw EISDIR … here, so this is a small improvement, not a regression).
  6. The declared race matches the code — core/src/host/memory.js:542 and :820 are lstat then write with no O_NOFOLLOW and no lock, so the window the body and the changelog name is real and is not closed here; not failed for that.

Where the body and my read differ (none of it changes the reading above):

  • the body says +25 / −3 in core/src/host/memory.js; the actual numstat is +21 / −4 (25 is the --stat changed-line total: 21 additions + 4 deletions). The PR-wide +184/−17 and 5 files are correct.
  • the change paragraph opens with "Three files", but the diff has 5 (the source, the test, the docs sentence, the changelog fragment, and the regenerated patches/ds-bot/0001-core-fixes.patch).
  • the body's "Which tree the checks ran on" names local commit 921514829b; the API answers 422 for it, so only the tree is checkable — and it does match (0243a1f9…), so the claim holds in the part that matters.

Residual, stated and not failed (not a regression of this diff): a hard link at the temp name is a plain file to lstat, so it is still written through (probed: the save lands in the linked file and the memory file keeps a second name for it) — the same declared class as the hard-link leaf, and the body does not claim to close it.

CI on this head: tests-ok, package, judge, merge-gate, reviewed tip, risk tier and the four plugin jobs are green; the only non-success is enqueue-guard (do not enqueue #2100: no Verifier report names the tip 133bf6158260), which names the missing conclusion, not a test or lint failure.

Not verified here: real-browser DOM/screenshots/clipboard, the smoke test and benches, a packaged Desktop build, Windows, the CI package job beyond its green check, and any path that calls the live GitHub API.

@Birfy

Birfy commented Nov 2, 2026

Copy link
Copy Markdown
Collaborator

Verdict: PASS at 133bf61
Written-by: Reviewer
Attestation: 133bf61 role=Reviewer

Ran at this tip:

$ cd p2100tip/core && DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/memory.test.js
# tests 92  # pass 92  # fail 0
$ cp p2100base/core/src/host/memory.js p2100tip/core/src/host/memory.js
$ cd p2100tip/core && DSH_HARNESS=/opt/flybot node --import ./test/harness-resolve.mjs --test test/memory.test.js
# tests 92  # pass 90  # fail 2
$ cd p2100tip && node scripts/ds-bot-patches.mjs check
# ok 10 patches apply cleanly in order; ok core/ equals ds-bot/ + patches/ds-bot/
  • Red first holds. Tip tarball at 133bf61 is green 92/92/0. With only the changed source (core/src/host/memory.js) swapped back to base blob 2998628 (tests kept from the tip), the suite is 92/90/2 and the two failures are exactly the two new #2097 cases — "a link left at a writer's temp name takes no entry…" and "…snapshot temp name is not written through". The source file was restored afterwards (cmp identical to the tip blob).
  • The guard does what the issue asks. writeText asks fileKindSync(temp) before writeFile; other warns once (same warnFile dedupe) and throws before any write or rename, so the outside file keeps its bytes and the memory file stays a plain file (test asserts both, plus one warning per temp name per process). The snapshot path asks <snapshot>.tmp, skips write+rename on other, and still builds the section for the prompt (asserted).
  • No collateral. warnFile(path, what, effect) keeps its old text through defaults, so the #2068 folder cases and the #2079 journal case stay green; all writeText callers (add/replace/remove, summaries, cursors) sit behind the one temp check, and no other .tmp writer in this module exists (grep: only the two guards).
  • Scope. 5 files exactly as the body lists (core/src/host/memory.js, core/test/memory.test.js, docs/subsystems/core-host.md, changelog.d/2097-temp-leaf-guard.fixed.md, patches/ds-bot/0001-core-fixes.patch); base is current main 2998628; nothing unrelated. The lstat-then-write race is named in the code comment, the doc sentence and the body as still open — consistent, not overstated.
  • CI at this tip. tests-ok, Node 24 · DSH 0.2.0-rc.2, package, plugins (social/tree/timeline/workflow), judge, merge-gate and risk tier (report only, T2) all success; the one cancelled agent-audience is a duplicate run. enqueue-guard is red only for "no Verifier report names the tip 133bf61" — this comment is that report.
  • Checkpoint in the body is present and within limits (1203 chars, summary 514, 5 files).

I ran only the affected suite (core/test/memory.test.js) on the tip and on the reverted source; the full suite is covered by tests-ok on this same tip, and ds-bot-patches check was re-run here to confirm the patched core/ copy matches patches/ds-bot/.

Not run: full npm run verify locally (tip CI covers it); smoke/packaged Desktop (no entry-point or build input changed, per body).

@Birfy Birfy added the agent-pr Pull request opened by an agent label Nov 2, 2026
@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
Merged via the queue into ThinkFlowLab:main with commit c4e425f Nov 2, 2026
16 of 17 checks passed
@Birfy
Birfy deleted the dev/1-temp-leaf-guard branch November 2, 2026 02:07
@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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

A pre-existing link at a writer's temp name is followed, so the leaf guard is bypassed

2 participants