fix(host): the state journal refuses a hard link at its own path

changelog.d/2131-journal-hardlink.fixed.md

@@ -0,0 +1 @@
+The state journal now refuses a `journal/state.jsonl` that is a hard link: its bytes are also a name outside this home, so the start-up read and the append both require the file to carry only this one name (`nlink === 1`). A shared inode is refused exactly like a symbolic link — one warning, nothing read, chmod-ed or appended — while a journal with one name records as before.

core/src/host/journal.js

@@ -256,7 +256,7 @@ export function openJournal({ path, warn, now = Date.now, maxBytes = JOURNAL_MAX
       if (error?.code === 'ENOENT') return
       throw error
     }
-    if (!info.isFile()) throw new JournalError('the journal path is not a plain file')
+    if (!info.isFile() || info.nlink !== 1) throw new JournalError('the journal path is not a plain file')
     await chmod(path, 0o600)
     const text = await readFile(path, 'utf8')
     const entries = parseJournal(text)
@@ -302,14 +302,15 @@ export function openJournal({ path, warn, now = Date.now, maxBytes = JOURNAL_MAX
       oldest = kept[1]?.t
     } else {
       // #2074: an append goes to a plain file this host owns and nothing else — a link standing in
-      // for the file would have the snapshot appended to whatever file it names.
+      // for the file would have the snapshot appended to whatever file it names, and a hard link's
+      // other name holds the same bytes, so only a file with this one name is this host's own.
       let info
       try {
         info = await lstat(path)
       } catch (error) {
         if (error?.code !== 'ENOENT') throw error
       }
-      if (info !== undefined && (info.isSymbolicLink() || !info.isFile())) throw new JournalError('the journal path is not a plain file')
+      if (info !== undefined && (info.isSymbolicLink() || !info.isFile() || info.nlink !== 1)) throw new JournalError('the journal path is not a plain file')
       await appendFile(path, text, { mode: 0o600 })
       bytes += added
       oldest = firstChange

core/test/state-journal.test.js

@@ -1,7 +1,7 @@
 import assert from 'node:assert/strict'
 import { spawnSync } from 'node:child_process'
 import { randomUUID } from 'node:crypto'
-import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'
+import { chmodSync, existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
@@ -389,6 +389,74 @@ test('a journal file swapped for a link after start-up is left byte-for-byte unc
   rmSync(outside, { recursive: true, force: true })
 })
 
+// A hard link is `lstat`'s idea of a plain file and not a link, so both checks let it through
+// (#2074 asked whether the name is this host's; #2115 answered it for the other name a file can
+// have): the journal took another file's bytes as the team's history, chmod-ed that file and
+// appended to it. One name is this host's, two names are not.
+test('a journal file that is a hard link is not the team’s history at start-up (#2074 shape)', async () => {
+  const home = tempHome()
+  const outside = mkdtempSync(join(tmpdir(), 'ds-bot-journal-outside-'))
+  const target = join(outside, 'someone-else.jsonl')
+  // A legal journal file: one base line, so the start-up read would take it as this team's history.
+  const line = lines([{ seq: 1, t: 0, op: 'base', next: { version: 2, revision: 7 }, actor: 'host' }])
+  writeFileSync(target, line)
+  if (posix) chmodSync(target, 0o644)
+  mkdirSync(join(home, 'journal'))
+  const path = join(home, 'journal', 'state.jsonl')
+  linkSync(target, path)
+  assert.equal(lstatSync(path).nlink, 2, 'the journal name shares its file with another name')
+  const warnings = []
+  const journal = openJournal({ path, warn: (...args) => warnings.push(args.join(' ')) })
+  await journal.record(stateText({ version: 2, revision: 8 }))
+  assert.equal(readFileSync(target, 'utf8'), line, 'the file the other name holds is not read as history or appended to')
+  if (posix) assert.equal(statSync(target).mode & 0o777, 0o644, 'it keeps its mode')
+  assert.equal(warnings.length, 1, 'the name is refused once, not once per save')
+  assert.match(warnings[0], /not recording/)
+  assert.equal(lstatSync(path).nlink, 2, 'the refused name is left as it was')
+  rmSync(home, { recursive: true, force: true })
+  rmSync(outside, { recursive: true, force: true })
+})
+
+test('a journal file swapped for a hard link after start-up is not appended to (#2074 shape)', async () => {
+  const home = tempHome()
+  const outside = mkdtempSync(join(tmpdir(), 'ds-bot-journal-outside-'))
+  const target = join(outside, 'someone-else.txt')
+  writeFileSync(target, 'PRIVATE LINE\n')
+  if (posix) chmodSync(target, 0o644)
+  const folder = join(home, 'journal')
+  mkdirSync(folder)
+  const path = join(folder, 'state.jsonl')
+  const warnings = []
+  const journal = openJournal({ path, warn: (...args) => warnings.push(args.join(' ')) })
+  await journal.record(stateText({ version: 2, revision: 1 }))
+  assert.deepEqual(warnings, [])
+  rmSync(path)
+  linkSync(target, path)
+  assert.equal(lstatSync(path).nlink, 2, 'the journal name shares its file with another name now')
+  await journal.record(stateText({ version: 2, revision: 2 }))
+  assert.equal(readFileSync(target, 'utf8'), 'PRIVATE LINE\n', 'the file the other name holds is not appended to')
+  if (posix) assert.equal(statSync(target).mode & 0o777, 0o644, 'it keeps its mode')
+  assert.equal(warnings.length, 1, 'the file that has a second name now is refused when the write runs')
+  assert.match(warnings[0], /not recording/)
+  rmSync(home, { recursive: true, force: true })
+  rmSync(outside, { recursive: true, force: true })
+})
+
+test('a journal file that is still a plain file of its own keeps recording (#2074 control)', async () => {
+  const home = tempHome()
+  mkdirSync(join(home, 'journal'))
+  const path = join(home, 'journal', 'state.jsonl')
+  const warnings = []
+  const journal = openJournal({ path, warn: (...args) => warnings.push(args.join(' ')) })
+  await journal.record(stateText({ version: 2, revision: 1 }))
+  await journal.record(stateText({ version: 2, revision: 2 }))
+  assert.deepEqual(warnings, [])
+  assert.equal(lstatSync(path).nlink, 1, 'the journal file carries only the name this host gave it')
+  if (posix) assert.equal(statSync(path).mode & 0o777, 0o600)
+  assert.equal(stateText(replayTo(readFileSync(path, 'utf8'))), stateText({ version: 2, revision: 2 }))
+  rmSync(home, { recursive: true, force: true })
+})
+
 test('a journal that is still its own folder and file keeps recording (#2074 control)', async () => {
   const home = tempHome()
   mkdirSync(join(home, 'journal'))

docs/subsystems/core-host.md

@@ -556,7 +556,7 @@ A local, replayable record of every change to the saved team, for multi-Bot bugs
 - **On.** `stateJournal: true` in the `bot` config. Off (the default) nothing is opened or written, so every other test runs as before.
 - **What.** `src/host/journal.js`. After each successful `state.json` write, `store.js` hands the exact snapshot text to the journal, which appends the difference from the last recorded snapshot to `<home>/journal/state.jsonl` (file mode 0600, folder 0700). Lines are `{ seq, t, op, key, prev, next, botId?, actor }`; `op` is `base` (the whole team: the first line, and any change only a key reorder explains), `set`, `delete`, or `append` (`prevLength`, `drop`, the new items: a group log or other list that grew or slid). Paths go at most three keys deep (`bots.<id>.<field>`); `botId` is set on a `bots.<id>` path; `actor` is `host` for a save and `retention` for a folded base (per-call attribution is not recorded). Only the host that holds the state lock writes, so a read-only window records nothing.
 - **Retention.** When a write would pass 20 MB, or the oldest change is older than 7 days, the oldest lines are dropped down to 90% of each cap and folded into the base line (atomic rewrite), so the kept lines still replay. A team that alone exceeds the cap stops recording.
-- **Fail closed.** Replay (`replayTo(text, seq)`) refuses the whole journal on an unreadable or incomplete line, an unknown op, a gap in `seq`, a first line that is not a base, or a `prev` that does not match what the earlier lines built; a `seq` outside the file is refused, not approximated. On start the writer replays the existing file the same way; a journal that does not replay, a path that is not a plain file, or a failed write stops recording for that run with a warning and leaves the file as it is. Saving the team never depends on the journal. The folder above the journal must be a real directory too (#2056): a link, a plain file or a check that cannot run refuses before the `mkdir`/`chmod` that would follow it, so neither the permissions nor a write reach the folder a link names; a folder that does not exist yet is still this writer's to create, as before. Since [#2074](https://github.com/ThinkFlowLab/FlyBot/issues/2074) the same question is asked again when the write runs: a journal folder swapped for a link is refused with one warning and recording stops, and a `state.jsonl` swapped for a link is never written through. The append path refuses the write; the prune path reads the current file and replaces the link with a fresh file in the verified folder, so nothing is written through it.
+- **Fail closed.** Replay (`replayTo(text, seq)`) refuses the whole journal on an unreadable or incomplete line, an unknown op, a gap in `seq`, a first line that is not a base, or a `prev` that does not match what the earlier lines built; a `seq` outside the file is refused, not approximated. On start the writer replays the existing file the same way; a journal that does not replay, a path that is not a plain file, or a failed write stops recording for that run with a warning and leaves the file as it is. Saving the team never depends on the journal. The folder above the journal must be a real directory too (#2056): a link, a plain file or a check that cannot run refuses before the `mkdir`/`chmod` that would follow it, so neither the permissions nor a write reach the folder a link names; a folder that does not exist yet is still this writer's to create, as before. Since [#2074](https://github.com/ThinkFlowLab/FlyBot/issues/2074) the same question is asked again when the write runs: a journal folder swapped for a link is refused with one warning and recording stops, and a `state.jsonl` swapped for a link is never written through. A hard link reads as a plain file to `lstat` while its bytes are also a name outside this home, so both the start-up read and the append also require the file to carry only this one name (`nlink === 1`): a shared inode is refused exactly like a link — one warning, nothing read, chmod-ed or appended — and a journal with one name records as before. The append path refuses the write; the prune path reads the current file and replaces the link with a fresh file in the verified folder, so nothing is written through it.
 - **Replay.** `core/scripts/replay.mjs` (dev-only, not in the package, see [install](install.md)) prints or writes (`--out`, new file only, 0600) the team after a given `--to <seq>`; it makes no network or process calls. Test: `core/test/state-journal.test.js` (a 3-Bot round replays byte-identical at every step).
 
 ### Standing tool approvals (#540)

patches/ds-bot/files/src/host/journal.js

@@ -256,7 +256,7 @@ export function openJournal({ path, warn, now = Date.now, maxBytes = JOURNAL_MAX
       if (error?.code === 'ENOENT') return
       throw error
     }
-    if (!info.isFile()) throw new JournalError('the journal path is not a plain file')
+    if (!info.isFile() || info.nlink !== 1) throw new JournalError('the journal path is not a plain file')
     await chmod(path, 0o600)
     const text = await readFile(path, 'utf8')
     const entries = parseJournal(text)
@@ -302,14 +302,15 @@ export function openJournal({ path, warn, now = Date.now, maxBytes = JOURNAL_MAX
       oldest = kept[1]?.t
     } else {
       // #2074: an append goes to a plain file this host owns and nothing else — a link standing in
-      // for the file would have the snapshot appended to whatever file it names.
+      // for the file would have the snapshot appended to whatever file it names, and a hard link's
+      // other name holds the same bytes, so only a file with this one name is this host's own.
       let info
       try {
         info = await lstat(path)
       } catch (error) {
         if (error?.code !== 'ENOENT') throw error
       }
-      if (info !== undefined && (info.isSymbolicLink() || !info.isFile())) throw new JournalError('the journal path is not a plain file')
+      if (info !== undefined && (info.isSymbolicLink() || !info.isFile() || info.nlink !== 1)) throw new JournalError('the journal path is not a plain file')
       await appendFile(path, text, { mode: 0o600 })
       bytes += added
       oldest = firstChange

patches/ds-bot/files/test/state-journal.test.js

@@ -1,7 +1,7 @@
 import assert from 'node:assert/strict'
 import { spawnSync } from 'node:child_process'
 import { randomUUID } from 'node:crypto'
-import { chmodSync, existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'
+import { chmodSync, existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { test } from 'node:test'
@@ -389,6 +389,74 @@ test('a journal file swapped for a link after start-up is left byte-for-byte unc
   rmSync(outside, { recursive: true, force: true })
 })
 
+// A hard link is `lstat`'s idea of a plain file and not a link, so both checks let it through
+// (#2074 asked whether the name is this host's; #2115 answered it for the other name a file can
+// have): the journal took another file's bytes as the team's history, chmod-ed that file and
+// appended to it. One name is this host's, two names are not.
+test('a journal file that is a hard link is not the team’s history at start-up (#2074 shape)', async () => {
+  const home = tempHome()
+  const outside = mkdtempSync(join(tmpdir(), 'ds-bot-journal-outside-'))
+  const target = join(outside, 'someone-else.jsonl')
+  // A legal journal file: one base line, so the start-up read would take it as this team's history.
+  const line = lines([{ seq: 1, t: 0, op: 'base', next: { version: 2, revision: 7 }, actor: 'host' }])
+  writeFileSync(target, line)
+  if (posix) chmodSync(target, 0o644)
+  mkdirSync(join(home, 'journal'))
+  const path = join(home, 'journal', 'state.jsonl')
+  linkSync(target, path)
+  assert.equal(lstatSync(path).nlink, 2, 'the journal name shares its file with another name')
+  const warnings = []
+  const journal = openJournal({ path, warn: (...args) => warnings.push(args.join(' ')) })
+  await journal.record(stateText({ version: 2, revision: 8 }))
+  assert.equal(readFileSync(target, 'utf8'), line, 'the file the other name holds is not read as history or appended to')
+  if (posix) assert.equal(statSync(target).mode & 0o777, 0o644, 'it keeps its mode')
+  assert.equal(warnings.length, 1, 'the name is refused once, not once per save')
+  assert.match(warnings[0], /not recording/)
+  assert.equal(lstatSync(path).nlink, 2, 'the refused name is left as it was')
+  rmSync(home, { recursive: true, force: true })
+  rmSync(outside, { recursive: true, force: true })
+})
+
+test('a journal file swapped for a hard link after start-up is not appended to (#2074 shape)', async () => {
+  const home = tempHome()
+  const outside = mkdtempSync(join(tmpdir(), 'ds-bot-journal-outside-'))
+  const target = join(outside, 'someone-else.txt')
+  writeFileSync(target, 'PRIVATE LINE\n')
+  if (posix) chmodSync(target, 0o644)
+  const folder = join(home, 'journal')
+  mkdirSync(folder)
+  const path = join(folder, 'state.jsonl')
+  const warnings = []
+  const journal = openJournal({ path, warn: (...args) => warnings.push(args.join(' ')) })
+  await journal.record(stateText({ version: 2, revision: 1 }))
+  assert.deepEqual(warnings, [])
+  rmSync(path)
+  linkSync(target, path)
+  assert.equal(lstatSync(path).nlink, 2, 'the journal name shares its file with another name now')
+  await journal.record(stateText({ version: 2, revision: 2 }))
+  assert.equal(readFileSync(target, 'utf8'), 'PRIVATE LINE\n', 'the file the other name holds is not appended to')
+  if (posix) assert.equal(statSync(target).mode & 0o777, 0o644, 'it keeps its mode')
+  assert.equal(warnings.length, 1, 'the file that has a second name now is refused when the write runs')
+  assert.match(warnings[0], /not recording/)
+  rmSync(home, { recursive: true, force: true })
+  rmSync(outside, { recursive: true, force: true })
+})
+
+test('a journal file that is still a plain file of its own keeps recording (#2074 control)', async () => {
+  const home = tempHome()
+  mkdirSync(join(home, 'journal'))
+  const path = join(home, 'journal', 'state.jsonl')
+  const warnings = []
+  const journal = openJournal({ path, warn: (...args) => warnings.push(args.join(' ')) })
+  await journal.record(stateText({ version: 2, revision: 1 }))
+  await journal.record(stateText({ version: 2, revision: 2 }))
+  assert.deepEqual(warnings, [])
+  assert.equal(lstatSync(path).nlink, 1, 'the journal file carries only the name this host gave it')
+  if (posix) assert.equal(statSync(path).mode & 0o777, 0o600)
+  assert.equal(stateText(replayTo(readFileSync(path, 'utf8'))), stateText({ version: 2, revision: 2 }))
+  rmSync(home, { recursive: true, force: true })
+})
+
 test('a journal that is still its own folder and file keeps recording (#2074 control)', async () => {
   const home = tempHome()
   mkdirSync(join(home, 'journal'))