Skip to content

The five remaining write-and-rename sites never ask their temp name #2109

Closed

Description

@HugoXK

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead
Claimed-by: FlyBot Lead
Claim-Id: bea1f8081add4980b6cce903fdde1bee
Branch: dev/3-temp-name-sweep
Base: 4d4b171
At: 2026-11-02T03:44:28Z

Implementation: FlyBot (ours) — PR follows within 24h. Do not pick up without a pickup-ok comment on this issue.

Written-by: FlyBot Lead

What

#2097 (PR #2100) taught the memory writers to ask the temp name a write lands through, and #2103 (PR #2108) does the same for the decision log's trim. Five writers are still on the old shape: write "${target}.${process.pid}.tmp", then rename it over the target, with no question asked about the temp name first.

  • core/src/host/store.js:927 — "${statePath}.${process.pid}.tmp", renamed over state.json at :941
  • core/src/host/images.js:133 — "${ocrPath}.${process.pid}.tmp", renamed over <home>/ocr.json at :135
  • core/src/host/automation-default.js:36 — "${file}.${process.pid}.tmp", renamed at :39 (the bundle-defaults file)
  • plugins/social/src/server-config.js:72 — "${path}.${process.pid}.tmp", renameSync at :74 (the server config)
  • plugins/workflow/src/service.js:313 — "${file}.${process.pid}.tmp", renameSync at :315

None of the five asks the temp name today. writeFile/writeFileSync follow a link at the last component, so a link planted at the temp name receives the payload — outside the home — and the rename afterwards puts that link where the target belongs, exactly as in #2097. The pid in the name is not a secret: processes are visible, so an attacker who can write in the home can plant <target>.<pid>.tmp before the write.

Reproduction (live main c0a76ede, before the fix)

Per site, in a temporary home, with the writing process's pid known: plant a link at <target>.<pid>.tmp pointing at a file outside the home, then trigger the write that uses the temp name.

  • store: state.json write → the outside file receives the state JSON (it was 'OUTSIDE\n', one probe found it replaced by the cache/state payload), and state.json becomes that link. Red-first unit case on base, three identical runs: 16 tests / 15 pass / 1 fail, Missing expected rejection.
  • images: OCR cache write → the outside file, 'OUTSIDE\n', becomes the cache JSON. 7 / 6 / 1, three runs.
  • automation-default: the bundle-defaults offer → offerBundle answered 'enabled' where it should have answered 'unrecorded'. 13 / 12 / 1, three runs.
  • social server-config: set() resolved where it should have rejected. 1 / 0 / 1, three runs.
  • workflow service: setConcurrency() returned saved.ok === true. 1 / 0 / 1, three runs.

With the fix in place the same five suites are 16/16, 7/7, 13/13, 1/1, 1/1, each three identical runs.

Expected behaviour

Each writer asks the temp name before it writes: when the name exists and is anything other than a plain file (lstat says not a regular file, or lstat fails for any reason other than ENOENT), the write does not happen, the rename does not happen, the target stays the file it was, and the refusal is reported in the words that module already uses for a failed write:

  • store: save() rejects (existing ds-bot: state write failed: … warning)
  • images: existing ds-bot: OCR cache write failed: … warning
  • automation: offerBundle answers 'unrecorded' plus one ds-bot: the Automation tasks offer was not written down: … warning
  • social: set() rejects with the temp path in the text
  • workflow: setConcurrency() returns { ok: false, error } with the temp path in the text

A healthy home is unchanged: the same temp name, the same write, the same rename.

The target leaf is deliberately not guarded here: rename(2) replaces the directory entry itself and does not follow the last component, so a link at the target is safely replaced rather than written through. core/test/store.test.js (a link at the state file itself is replaced, not written through) already holds that on base and must stay green; adding a target check would turn a safe replacement into a refusal and change behaviour the issue does not ask to change.

One existing assertion changes (decided)

core/test/roster.test.js:544 today expects bot/internal, which it got from EISDIR when the temp name was a directory. With the guard in front of the write, the same situation is a refusal and reports bot/refused with the temp path named. Keeping bot/internal would mean inventing an errno-shaped code for the guard; we do not invent errno. The assertion moves to bot/refused, and what that case is actually about — the Session is archived and the team holds no Phantom — stays green. This only changes the classification of a write that was already not landing on a plain temp name.

Why these five and not others

  • core/src/host/memory.js:549/827 — already asks the temp name (#2100).
  • core/src/host/decisions.js — the trim's temp name is #2103 / PR #2108.
  • core/src/host/workflow-store.js:67, core/src/host/diagnostics.js:153/276 — ask the target leaf, not a temp name; no temp name is written there.
  • core/src/host/journal.js:293 — writes with flag: 'wx', so a pre-existing name fails with EEXIST before any payload is written; already fail-closed.
  • core/src/host/secrets.js:165 — the temp name carries randomUUID(); it cannot be planted.
  • core/src/host/meetings.js:339 — the name carries randomBytes(4), and the payload is written through the returned file handle, not by path, so renaming cannot redirect it; not plantable and not the same class.

Duplicate check (2026-11-02)

git grep on live c0a76ede for .tmp + rename: the core/src/host modules plus the two plugin modules above. Issue search for temp in titles returns #2108 (open, decision-log trim), #2100 + #2097 (closed, memory writers), and two 2026-10-18 macOS fixture issues (#314/#315). No open or closed issue covers these five sites.

Scope

Five sites across three packages. plugins/* shipped sources may not import core (plugins/social/test/packaging.test.js: an import that leaves the plugin breaks the installed bundle), so: one shared helper in core/src/host/real-dir.js for the three core sites, and a same-shaped helper inside each plugin for its own site — the package boundary decides the file count, not a preference for copies.

ds-bot/ mirrors are generated (node scripts/ds-bot-patches.mjs generate), plus one changelog fragment named after this issue.

Activity

  1. HugoXK commented on Nov 2, 2026

    @HugoXK
    ContributorAuthor

    Release

    Written-by: FlyBot Lead
    Released: bea1f8081add4980b6cce903fdde1bee
    Reason: merged as a91701d at 2026-11-02T04:15:24Z (PR #2110)
    At: 2026-11-02T04:15:59Z

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions