From 7980af8c4ed0255b21b9dfea154b34d684348a15 Mon Sep 17 00:00:00 2001 From: Day Dreamer <87640074+HugoXK@users.noreply.github.com> Date: Thu, 5 Nov 2026 00:30:03 +0800 Subject: [PATCH] host: a team file linked to a folder is not described as read through the link MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `stateNotPlain` answered every symbolic link with the sentence #2404 wrote for a link to a plain file: read through the link, name replaced by the next save. A link to a folder is read by `readFile(statePath)` as a folder (EISDIR), so the store lands in `stuck`, `writable()` stays false and no save ever replaces the name — the line above the `Read-only` row denied the read that row reports and promised a save that cannot happen. `stat` resolves the link for its metadata only (the bytes behind it are never opened); only a plain file target earns the link sentence, while a folder, a link to nothing or a check that cannot run keeps the journal leaf's sentence, which is true of them. --- .../2416-state-file-link-target.fixed.md | 1 + core/src/host/diagnostics.js | 13 +++++-- core/test/diagnostics.test.js | 34 ++++++++++++++++++ docs/subsystems/core-host.md | 2 +- patches/ds-bot/0001-core-fixes.patch | 15 +++++--- patches/ds-bot/0007-version-and-tooling.patch | 36 ++++++++++++++++++- 6 files changed, 92 insertions(+), 9 deletions(-) create mode 100644 changelog.d/2416-state-file-link-target.fixed.md diff --git a/changelog.d/2416-state-file-link-target.fixed.md b/changelog.d/2416-state-file-link-target.fixed.md new file mode 100644 index 000000000..9a57e1a5a --- /dev/null +++ b/changelog.d/2416-state-file-link-target.fixed.md @@ -0,0 +1 @@ +The diagnostics report no longer says a linked team file is read through the link when the link points at a folder: `readFile` fails on a folder (`EISDIR`), so the store lands in `stuck` and no save ever replaces the name. `state file:` now keeps `not a plain file (nothing is read or written)` for a link to a folder or to nothing, and keeps the read-through-the-link sentence only when the link resolves to a plain file. The target is asked with `stat` for its metadata only — the bytes behind the link are never opened. diff --git a/core/src/host/diagnostics.js b/core/src/host/diagnostics.js index cc62c74cc..970cd6c88 100644 --- a/core/src/host/diagnostics.js +++ b/core/src/host/diagnostics.js @@ -248,13 +248,20 @@ export function install(rt) { const NOT_PLAIN = 'not a plain file (nothing is read or written)' // The team file is not a journal leaf (#2404): the store reads it through the link // (`readFile(statePath)` follows the last name) and its next save renames a temp name over this - // one (`rename(temp, statePath)`), so a link is read and replaced, not ignored. A directory or a - // check that cannot run keeps the leaf sentence above: no read of those lands either. + // one (`rename(temp, statePath)`), so a link to a plain file is read and replaced, not ignored. A + // directory or a check that cannot run keeps the leaf sentence above: no read of those lands + // either. const LINKED_STATE = 'not a plain file (described only; the store reads the team file through the link and the next save replaces the name)' + // A link names a target, and the store's answer depends on it: a link to a plain file is the + // shape above, while a link to a folder makes `readFile(statePath)` fail with EISDIR — the store + // lands in `stuck`, `writable()` stays false and no save ever renames over the name, so the leaf + // sentence is the true one. `stat` follows the link for its metadata only; the bytes behind it + // stay unread, which is all this report may do with them. const stateNotPlain = async path => { try { const info = await lstat(path) - return info.isSymbolicLink() || (info.isFile() && info.nlink > 1) ? LINKED_STATE : NOT_PLAIN + if (!info.isSymbolicLink()) return info.isFile() && info.nlink > 1 ? LINKED_STATE : NOT_PLAIN + return (await stat(path)).isFile() ? LINKED_STATE : NOT_PLAIN } catch { return NOT_PLAIN } diff --git a/core/test/diagnostics.test.js b/core/test/diagnostics.test.js index 2d3509000..eda770117 100644 --- a/core/test/diagnostics.test.js +++ b/core/test/diagnostics.test.js @@ -757,6 +757,40 @@ test('a linked team file is described as read through the link, not as nothing b assert.equal(readFileSync(plain, 'utf8'), newer, 'the other name keeps its bytes') }) +// A link does not say what it points at, and the store's answer depends on the target (#2404 wrote +// the link sentence for one shape of it). A linked plain file is read through the link and replaced +// by the next save; a link to a folder is read by `readFile(statePath)` as a folder — EISDIR, the +// store lands in `stuck`, `writable()` stays false and no save ever replaces the name. So the line +// above a `Read-only` row that says the read failed denied that read and promised a save that cannot +// happen. The target is asked with `stat` for its metadata only — the bytes behind the link stay +// unread, which is all this report may do with them — and only a plain file earns the link sentence; +// a folder or a link to nothing keeps the leaf sentence, which is true of both. +test('a team file linking to a folder keeps the leaf sentence, not the read-through-link one', async (t) => { + const elsewhere = mkdtempSync(join(tmpdir(), 'ds-bot-diag-target-')) + t.after(() => rmSync(elsewhere, { recursive: true, force: true, maxRetries: 5 })) + const folder = join(elsewhere, 'a-folder') + mkdirSync(folder) + writeFileSync(join(folder, 'kept.txt'), 'a file that stays\n') + + const linked = await realHost(t, { prepare: statePath => symlinkSync(folder, statePath) }) + const line = linked.report.markdown.match(/^state file: (.*)$/m)?.[1] + assert.ok(line !== undefined, 'the report still names the team file') + assert.match(line, /^not a plain file \(nothing is read or written\)$/, + 'a link the store cannot read through is not described as read through') + assert.match(linked.report.markdown, /^\| Read-only \| The Bot team file .* could not be read \(EISDIR\)/m, + 'the row below it says that read happened and failed') + assert.deepEqual(readdirSync(folder), ['kept.txt'], 'the folder behind the link keeps exactly what it had') + + const linkHome = join(elsewhere, 'link-home') + mkdirSync(linkHome) + const dangling = host(linkHome, { config: { diagnosticsFile: false }, statePath: join(linkHome, 'state.json') }) + symlinkSync(join(elsewhere, 'not-there.json'), join(linkHome, 'state.json')) + const danglingReport = await dangling.diagnostics.report({}) + assert.match(danglingReport.markdown, /^state file: not a plain file \(nothing is read or written\)$/m, + 'a link to no file is not described as read through either') + assert.equal(existsSync(join(elsewhere, 'not-there.json')), false, 'asking the link creates nothing') +}) + // The line is read information, not a write: a window another dsh keeps read-only still names the // team file, the way it still counts the team it shows. test('a read-only window names the team file too (#2394)', async (t) => { diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index 853a77305..8fb056a1f 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -407,7 +407,7 @@ Work a member hands a Bot with `message_bot` from its group Session runs in a hi - Host diagnostics never depend on `ctx.logger`: DSH may export no logger, and a test or plugin-local host may carry one without `warn`. `rt.warn` still writes stderr and the diagnostics log in that case (`src/host/diagnostics.js`). Plugins reach the same log through `flybot.warn` (#68); see [plugin-runtime](plugin-runtime.md). - The `Read-only` row of a report repeats this host's own answer rather than one constant: `rt.readOnly()` is `undefined` while the window writes, a sentence about a live `state.lock` when another dsh holds the team, and a sentence about the team file when it is `stuck` (a newer version, a file that will not parse, an unreadable path). The report shows that sentence — `clean(cut(answer, 200))`, so saved keys, the home folder and `|` are handled like any other report text — the same one the sidebar shows, so a newer team file reads `update FlyBot` and an unreadable one reads `could not be read`, never `another dsh holds the team`; a writable window still adds no row ([#2167](https://github.com/ThinkFlowLab/FlyBot/issues/2167)). - The log itself is one leaf file under the home, and a write asks it before it appends ([#2089](https://github.com/ThinkFlowLab/FlyBot/issues/2089)): when `logs/diagnostics.jsonl` is a link, a hard link or anything other than a plain file of its own — the line is dropped with one line on stderr that carries no home folder, the entries stay readable in memory, and `clear` leaves a file behind such a link untouched rather than truncating it. Past 1 MB the leaf moves aside to `diagnostics.jsonl.1`, and `clear` removes that name as well — in the same write chain, behind the same leaf guard — so a cleared log leaves no rotated lines behind. The truncate runs first and the removal only after it: a leaf that cannot be emptied keeps the rotated copy where it is, because those lines are then the only copy left of them. The removal keeps a `try` of its own: a copy the host cannot remove (a directory under that name, a folder without its write bit) keeps its bytes, is named on one stderr line that carries no folder, and never turns a Clear that emptied the leaf into one that leaves every line on disk. The truncate keeps a `try` of its own for the same reason ([#2364](https://github.com/ThinkFlowLab/FlyBot/issues/2364)): the leaf guard asks the file's kind, not its mode, so a plain `0444` leaf passes it and the write fails with `EACCES` — that failure is named on one stderr line, `[ds-bot] diagnostics log not truncated: `, which carries no folder, the same way `append` names its own; the answer it leaves behind is `leaf: 'kept'`. `clear` answers with what it did — `{ leaf: 'truncated' | 'kept' | 'absent', rotated: 'removed' | 'kept' | 'none' }` — and `diagnostics-clear` passes that result through instead of a constant, so the Feedback page's sentence matches it: the unchanged one, one that names the leftover `logs/diagnostics.jsonl.1`, or one that does not claim a cleared log ([#2360](https://github.com/ThinkFlowLab/FlyBot/issues/2360)). `rotated` says what this call did to that name: `removed` when it deleted one, `kept` when one was there and still is — the removal failed, or the truncate failed first and nothing was removed — and `none` when there was none; a leaf the guard refuses never reaches the probe, so it answers `none`. What the window reads moves only with an emptied leaf: `entries()` answers the old lines through `cleared` plus the live ones in `fresh`, and `logText()` behind the Download reads the same, so a Clear that leaves the leaf full changes neither of them — the page can no longer show "not cleared" next to "0 recent errors" while every byte is still on disk. Only a leaf that really was truncated clears both — plus a host that keeps no log file at all (`diagnosticsFile: false`), which has no file and no list to keep; an `ENOENT` write leaves both alone like every other refusal ([#2364](https://github.com/ThinkFlowLab/FlyBot/issues/2364)). That is the answer, one stderr line and nothing else that moves: the truncate, the `fileKind` guard, the removal rule and the `try` keep the behaviour they had, and a leaf that is not there yet still gets the empty file the write always landed there. `absent` therefore means nothing was on disk to clear — this host keeps no log file (`diagnosticsFile: false`), or the folder is not there so the write found nothing to truncate — not a leaf the call refused. The report says whether that same file is being written, in the shape the journal line already has: `diagnostics log: recording` while the appends land, `not written ()` after an append the host could not make, and `not written (not a plain file of its own)` when the leaf guard refused the write, so a bundle that lists recent errors also says when the file holding them is missing or short and why. The value follows the last append attempt, not the first refusal: a host that puts a plain file of its own back writes the next entry into it and says `recording` again, and a leaf that has become a plain `0444` file says `not written (EACCES)` on the next attempt, while the guard's own stderr line stays one per host ([#2089](https://github.com/ThinkFlowLab/FlyBot/issues/2089)); a host with the log off (`diagnosticsFile: false`) adds no line at all, the rule the journal line keeps ([#2383](https://github.com/ThinkFlowLab/FlyBot/issues/2383)). -- The report also names the team file itself, right after the table that counts the team ([#2394](https://github.com/ThinkFlowLab/FlyBot/issues/2394)): `state file: ( bytes, last written )` for a plain file with this one name, `state file: none yet (this computer has not saved the team)` before this computer has one, and `state file: not a plain file (described only; the store reads the team file through the link and the next save replaces the name)` when the path is a link or a file with a second name (`nlink > 1`) — the store's own shape, which reads the team file through the name and renames a temp name over it on the next save, so the line must not claim nothing is read or written ([#2404](https://github.com/ThinkFlowLab/FlyBot/issues/2404)). A directory or a check that cannot run keeps the journal leaf's `state file: not a plain file (nothing is read or written)`, which is true of it: no read of those lands. It is built from `lstat` alone like the journal's line, and that is the point: `state.json` holds the members, the messages and the saved keys, so the report may describe the file and must never open it — a report can say how large the team file is and when it was last written without reading a byte of it. The path goes through the same home redaction, the line is read information rather than a write action so a read-only window draws it too, and the wording of `state journal:`, `journal file:` and the `Read-only` row is unchanged. +- The report also names the team file itself, right after the table that counts the team ([#2394](https://github.com/ThinkFlowLab/FlyBot/issues/2394)): `state file: ( bytes, last written )` for a plain file with this one name, `state file: none yet (this computer has not saved the team)` before this computer has one, and `state file: not a plain file (described only; the store reads the team file through the link and the next save replaces the name)` when the path is a link to a plain file or a file with a second name (`nlink > 1`) — the store's own shape, which reads the team file through the name and renames a temp name over it on the next save, so the line must not claim nothing is read or written ([#2404](https://github.com/ThinkFlowLab/FlyBot/issues/2404)). A directory, a link to a directory, a link to nothing or a check that cannot run keeps the journal leaf's `state file: not a plain file (nothing is read or written)`, which is true of them: no read of those lands, and for a folder behind the link the store's read fails as one (`EISDIR`), so it lands in `stuck`, `writable()` stays false and no save ever renames over the name. Both sentences are metadata only — `lstat` for the name, plus `stat` on a link to ask what it points at, never open, read or stream the target; that is the point: `state.json` holds the members, the messages and the saved keys, so the report may describe the file and must never open it — a report can say how large the team file is and when it was last written without reading a byte of it. The path goes through the same home redaction, the line is read information rather than a write action so a read-only window draws it too, and the wording of `state journal:`, `journal file:` and the `Read-only` row is unchanged. - The report names the file the journal records into, in the journal's own shape and without opening it ([#2388](https://github.com/ThinkFlowLab/FlyBot/issues/2388)): `journal file: ( bytes, last written )` for a plain file with this one name, `journal file: none yet` before the first save creates it, and `journal file: not a plain file (nothing is read or written)` when the path is a link, a file with a second name (`nlink > 1`) or the check cannot run. `lstat` is the only call — never `open`, `readFile` or `createReadStream` — so the line needs no read permission on the file and touches nothing another name holds; the path goes through the same home redaction the rest of the report uses, and a host without a journal adds no line, like the `state journal:` sentence it follows. The line says what the file is, not that a write happened: `state journal:` keeps answering whether the host is recording. ### Untrusted fences in every shared room diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index a7f0ef5b6..518080991 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -5187,7 +5187,7 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js console.error(`[ds-bot] ${line}${frames.length > 0 ? `\n${clean(frames.join('\n'))}` : ''}`) note({ level: 'warn', -@@ -187,24 +187,91 @@ export function install(rt) { +@@ -187,24 +187,98 @@ export function install(rt) { })) } @@ -5200,13 +5200,20 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js + const NOT_PLAIN = 'not a plain file (nothing is read or written)' + // The team file is not a journal leaf (#2404): the store reads it through the link + // (`readFile(statePath)` follows the last name) and its next save renames a temp name over this -+ // one (`rename(temp, statePath)`), so a link is read and replaced, not ignored. A directory or a -+ // check that cannot run keeps the leaf sentence above: no read of those lands either. ++ // one (`rename(temp, statePath)`), so a link to a plain file is read and replaced, not ignored. A ++ // directory or a check that cannot run keeps the leaf sentence above: no read of those lands ++ // either. + const LINKED_STATE = 'not a plain file (described only; the store reads the team file through the link and the next save replaces the name)' ++ // A link names a target, and the store's answer depends on it: a link to a plain file is the ++ // shape above, while a link to a folder makes `readFile(statePath)` fail with EISDIR — the store ++ // lands in `stuck`, `writable()` stays false and no save ever renames over the name, so the leaf ++ // sentence is the true one. `stat` follows the link for its metadata only; the bytes behind it ++ // stay unread, which is all this report may do with them. + const stateNotPlain = async path => { + try { + const info = await lstat(path) -+ return info.isSymbolicLink() || (info.isFile() && info.nlink > 1) ? LINKED_STATE : NOT_PLAIN ++ if (!info.isSymbolicLink()) return info.isFile() && info.nlink > 1 ? LINKED_STATE : NOT_PLAIN ++ return (await stat(path)).isFile() ? LINKED_STATE : NOT_PLAIN + } catch { + return NOT_PLAIN + } diff --git a/patches/ds-bot/0007-version-and-tooling.patch b/patches/ds-bot/0007-version-and-tooling.patch index 633119983..31e8edf3d 100644 --- a/patches/ds-bot/0007-version-and-tooling.patch +++ b/patches/ds-bot/0007-version-and-tooling.patch @@ -1005,7 +1005,7 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js test('diagnostics append fallback redacts an error message', async (t) => { const home = mkdtempSync(join(tmpdir(), 'ds-bot-diag-')) t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) -@@ -133,6 +133,533 @@ test('diagnostics append fallback redacts an error message', async (t) => { +@@ -133,6 +133,567 @@ test('diagnostics append fallback redacts an error message', async (t) => { assert.match(stderr.join('\n'), /api_key=\[hidden\]/) }) @@ -1491,6 +1491,40 @@ diff --git a/test/diagnostics.test.js b/test/diagnostics.test.js + assert.equal(readFileSync(plain, 'utf8'), newer, 'the other name keeps its bytes') +}) + ++// A link does not say what it points at, and the store's answer depends on the target (#2404 wrote ++// the link sentence for one shape of it). A linked plain file is read through the link and replaced ++// by the next save; a link to a folder is read by `readFile(statePath)` as a folder — EISDIR, the ++// store lands in `stuck`, `writable()` stays false and no save ever replaces the name. So the line ++// above a `Read-only` row that says the read failed denied that read and promised a save that cannot ++// happen. The target is asked with `stat` for its metadata only — the bytes behind the link stay ++// unread, which is all this report may do with them — and only a plain file earns the link sentence; ++// a folder or a link to nothing keeps the leaf sentence, which is true of both. ++test('a team file linking to a folder keeps the leaf sentence, not the read-through-link one', async (t) => { ++ const elsewhere = mkdtempSync(join(tmpdir(), 'ds-bot-diag-target-')) ++ t.after(() => rmSync(elsewhere, { recursive: true, force: true, maxRetries: 5 })) ++ const folder = join(elsewhere, 'a-folder') ++ mkdirSync(folder) ++ writeFileSync(join(folder, 'kept.txt'), 'a file that stays\n') ++ ++ const linked = await realHost(t, { prepare: statePath => symlinkSync(folder, statePath) }) ++ const line = linked.report.markdown.match(/^state file: (.*)$/m)?.[1] ++ assert.ok(line !== undefined, 'the report still names the team file') ++ assert.match(line, /^not a plain file \(nothing is read or written\)$/, ++ 'a link the store cannot read through is not described as read through') ++ assert.match(linked.report.markdown, /^\| Read-only \| The Bot team file .* could not be read \(EISDIR\)/m, ++ 'the row below it says that read happened and failed') ++ assert.deepEqual(readdirSync(folder), ['kept.txt'], 'the folder behind the link keeps exactly what it had') ++ ++ const linkHome = join(elsewhere, 'link-home') ++ mkdirSync(linkHome) ++ const dangling = host(linkHome, { config: { diagnosticsFile: false }, statePath: join(linkHome, 'state.json') }) ++ symlinkSync(join(elsewhere, 'not-there.json'), join(linkHome, 'state.json')) ++ const danglingReport = await dangling.diagnostics.report({}) ++ assert.match(danglingReport.markdown, /^state file: not a plain file \(nothing is read or written\)$/m, ++ 'a link to no file is not described as read through either') ++ assert.equal(existsSync(join(elsewhere, 'not-there.json')), false, 'asking the link creates nothing') ++}) ++ +// The line is read information, not a write: a window another dsh keeps read-only still names the +// team file, the way it still counts the team it shows. +test('a read-only window names the team file too (#2394)', async (t) => {