Skip to content

Decisions and diagnostics logs are written through when the leaf is a link #2089

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: eed1f2ac94ca44de9b5a6fff28394f7f
Branch: dev/2-decisions-diagnostics-leaf
Base: fca35af
At: 2026-11-02T00:50:47Z

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

Written-by: FlyBot Lead

What happens today

#2079 / PR #2083 fixes the same class of defect for the memory journal and the workflow save/setDefault leaves: a leaf that is a symlink is written through to whatever it points at, outside the host's directory. Two more writers of the same shape were left out of that scope and still write through:

  • core/src/host/decisions.js:70-71 — inside the writing chain, mkdir(dirname(path), { recursive: true }) then appendFile(path, line, 'utf8').
  • core/src/host/diagnostics.js:142/144/145 — mkdir(dirname(logPath), { recursive: true }), then (when the file is over LOG_MAX_BYTES) rename(logPath, logPath + '.1'), then appendFile(logPath, entry, 'utf8').
  • core/src/host/diagnostics.js clear() — truncates logPath directly.

Neither module asks what the leaf is: neither imports lstat. decisions.js's record() catches the failure into rt.warn('ds-bot: a decision was not written: %s') and still answers the tool caller Recorded: …, so a decision that never reached a plain file is reported as recorded.

Reproduction

On live main fca35af6f29a10214f239b78a2087b82845ba4ce: point the decision log (or the diagnostics log) at a symlink whose target is a file outside the host directory, then record a decision (or force a diagnostics entry). Measured with the new tests:

  • decision log: the outside target grows from PRE-EXISTING LINE\n to that line plus two JSON records ({"time":…,"bot":"a","botName":"Chief","option":"Keep the link","choice":"rejected"}, Keep it again), while the tool answers Recorded: rejected "Keep the link".
  • diagnostics log: the target gains two {"level":"warn","source":"host","text":…} lines; clear() truncates the target to an empty string.
  • untouched sources with only those cases added: 27 tests / 25 pass / 2 fail, exit 1, three identical runs; both failures are the two cases above.
  • with the guards: 27 / 27 / 0, three identical runs.

Expected behaviour

  • A leaf that is not a plain file (a symlink, a dangling symlink, a directory, anything lstat reports as not a regular file) is refused, not followed: the target is left byte-untouched and no new line lands anywhere.
  • decisions.js: the record is dropped, one warning is emitted, the caller is told it was not recorded, and no decision feed event fires (a tool that says "recorded" while the feed says "a decision was taken" and the disk has neither is worse than the drop).
  • diagnostics.js: the entry is dropped, one stderr line per host says the log is not a plain file (no home path in the message), entries() still reads what is held in memory, clear() does not truncate behind a link, and the size-based rename only runs after the guard.
  • Both guards sit inside each module's own writing chain, before the write — an API-entry check would miss already queued writes.
  • Ordinary write failures keep today's behaviour (warn, answer unchanged): this refuses only the not-a-plain-file case.

Scope

  • core/src/host/decisions.js, core/src/host/diagnostics.js, core/src/host/real-dir.js (a fileKind helper, same shape as the existing dirKind: lstat, absent only on ENOENT, anything else other, fail closed), their tests, one changelog fragment changelog.d/<this issue>-<slug>.fixed.md, and the one-line doc update in docs/subsystems/core-host.md.
  • The patches/ds-bot mirrors are regenerated, not hand-written: real-dir.js and decisions.js carry files/ mirrors, diagnostics.js lands in 0001-core-fixes.patch, test edits in 0007-version-and-tooling.patch; generate must be idempotent.
  • Not in scope, recorded here so it is not lost: home/logs itself being a symlink is a directory gap (mkdir(recursive) follows a linked directory) — a different issue, not this PR. decisions.js's trim() uses writeFile(tmp) + rename(tmp, path), which replaces the link itself rather than writing through it; it is not part of this defect.

Gates

  • Red-first on the PR base, three identical runs (both cases red), then three identical green runs after the fix.
  • Per-writer mutation arms: drop the record() guard → 1 red; drop the append() guard → 1 red; drop only the clear() guard → 1 red (clear does not truncate what is behind the link), so all three write points are guarded, not just the appends.
  • TZ=UTC ARCH_DOCS_BASE=<base> npm run verify exit 0, ds-bot-patches check (patches apply in order, core/ equals ds-bot/ + patches/), build-client --check, generate twice with no drift, changelog fragment well formed.
  • Security-surface change: a FlyBot Security advisory must land on the final tip before the single review request is sent.
  • Tier T2, effort S.

Duplicates

Searched live main and the tracker: the only open report of this class is #2079 / PR #2083 (memory journal + workflow save/setDefault), a different set of files; #2056 / #2065 (symlinked journal folder) are closed and about a directory outside the home, not a leaf. No open issue covers the decision log or the diagnostics log, and real-dir.js appears in no other open issue.

Activity

  1. HugoXK commented on Nov 2, 2026

    @HugoXK
    ContributorAuthor

    Release

    Written-by: FlyBot Lead
    Released: eed1f2ac94ca44de9b5a6fff28394f7f
    Reason: Merged as 1778add (verdict PASS at 616b36c, merge 02:10:21Z)
    At: 2026-11-02T02:11:15Z

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