Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions changelog.d/hardlink-temp-name.fixed.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
A write that lands through a temporary name now refuses a hard link left there as well as a symbolic one: `lstat` reports a hard link as a plain file, so the team state, the OCR cache, the Automation-tasks defaults, the memory files and snapshot, the decision log's trim and the Bot Social server choice could still be written into an inode another name held outside the Bot's home. The guard asks for a file with only its own name (`nlink === 1`), while the plain leftover a crashed host leaves behind stays replaceable.
2 changes: 1 addition & 1 deletion core/src/host/automation-default.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ async function writeOffers(file, offers) {
const temp = `${file}.${process.pid}.tmp`
// The temp name is a leaf like the defaults file: a link there would take the offer outside
// the team folder and the rename would put the link over dsh-defaults.json (temp-name sweep).
if (!tempWritable(temp)) throw new Error(`Not saved: ${temp} is not a plain file, so nothing was written there.`)
if (!tempWritable(temp)) throw new Error(`Not saved: ${temp} is not a plain file of its own, so nothing was written there.`)
try {
await writeFile(temp, `${JSON.stringify({ version: 1, offers }, null, 2)}\n`, 'utf8')
await rename(temp, file)
Expand Down
8 changes: 4 additions & 4 deletions core/src/host/decisions.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ import { dirname, join } from 'node:path'
import { cut } from './text.js'
import { defineTool } from './tool-schema.js'
import { looksSecret } from './memory.js'
import { fileKind } from './real-dir.js'
import { fileKind, tempWritable } from './real-dir.js'
import { output } from './tools.js'

const OPTION_MAX = 200
Expand Down Expand Up @@ -78,11 +78,11 @@ export function install(rt) {
const next = kept.length > 0 ? `${kept.join('\n')}\n` : ''
if (next !== text) {
const tmp = `${path}.tmp`
if (await fileKind(tmp) === 'other') {
if (!tempWritable(tmp)) {
trimRefused = true
if (!refusedTemp) {
refusedTemp = true
rt.warn('ds-bot: the decision log was not trimmed: %s is not a plain file', tmp)
rt.warn('ds-bot: the decision log was not trimmed: %s is not a plain file of its own', tmp)
}
return kept.length
}
Expand Down Expand Up @@ -162,7 +162,7 @@ export function install(rt) {
if (wrote === 'refused') return 'Not recorded: the decision log is not a plain file.'
// The feed payload drops the id to a safe shape and adds the live botId (a consumer keys by it).
rt.fireFeed?.('decision', { botId, option, choice, ...(why ? { why } : {}), time: entry.time })
if (wrote === 'untrimmed') return `Written: ${choice} "${option}"${why ? ` — ${why}` : ''} — the log was not trimmed: ${path}.tmp is not a plain file.`
if (wrote === 'untrimmed') return `Written: ${choice} "${option}"${why ? ` — ${why}` : ''} — the log was not trimmed: ${path}.tmp is not a plain file of its own.`
return `Recorded: ${choice} "${option}"${why ? ` — ${why}` : ''}.`
},
}))
Expand Down
2 changes: 1 addition & 1 deletion core/src/host/images.js
Original file line number Diff line number Diff line change
Expand Up @@ -134,7 +134,7 @@ export function install(rt) {
const temp = `${ocrPath}.${process.pid}.tmp`
// The temp name is a leaf like the cache: a link there would take the readings outside
// the home and the rename would put the link over ocr.json (temp-name sweep).
if (!tempWritable(temp)) throw new Error(`Not saved: ${temp} is not a plain file, so the OCR cache was not written there.`)
if (!tempWritable(temp)) throw new Error(`Not saved: ${temp} is not a plain file of its own, so the OCR cache was not written there.`)
await writeFile(temp, snapshot, 'utf8')
await rename(temp, ocrPath)
}).catch(error => rt.warn('ds-bot: OCR cache write failed: %s', error))
Expand Down
10 changes: 5 additions & 5 deletions core/src/host/memory.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import { codePointStart, cut } from '../shared/text-boundary.js'
import { output } from './tools.js'
import { retiredBot } from './store.js'
import { botNode, buildGraph, buildIndex, relatedOf, searchIndex, topicNode } from './memory-search.js'
import { dirKindSync, fileKindSync } from './real-dir.js'
import { dirKindSync, fileKindSync, tempWritable } from './real-dir.js'

// Fixed sizes, in characters. Both MEMORY.md files ride in every request of every
// part, so together they stay near 8,000 characters, a few thousand tokens. A topic is
Expand Down Expand Up @@ -453,7 +453,7 @@ export function install(rt) {
const warnFile = (path, what = 'memory journal', effect = 'dropping the record') => {
if (warnedFiles.has(path)) return
warnedFiles.add(path)
rt.warn(`ds-bot: ${what} is not a plain file %s; ${effect}`, path)
rt.warn(`ds-bot: ${what} is not a plain file of its own %s; ${effect}`, path)
}
// The one answer for "what is this folder": every segment from `memory/` down is asked on its
// own, because `lstat` follows a link above the last component — `memory/bots` or a
Expand Down Expand Up @@ -547,9 +547,9 @@ export function install(rt) {
async function writeText(path, text) {
await mkdir(dirname(path), { recursive: true })
const temp = `${path}.${process.pid}.tmp`
if (fileKindSync(temp) === 'other') {
if (!tempWritable(temp)) {
warnFile(temp, 'the memory write', 'nothing is written there')
throw new Error(`Not saved: ${temp} is not a plain file, so nothing is written there.`)
throw new Error(`Not saved: ${temp} is not a plain file of its own, so nothing is written there.`)
}
await writeFile(temp, text, 'utf8')
await rename(temp, path)
Expand Down Expand Up @@ -825,7 +825,7 @@ export function install(rt) {
// The snapshot's temp name is a leaf like the entry's (#2097): a link left there would
// take the frozen memory outside and then replace the snapshot with the link.
const temp = `${path}.tmp`
if (fileKindSync(temp) === 'other') warnFile(temp, 'the memory snapshot', 'nothing is written there')
if (!tempWritable(temp)) warnFile(temp, 'the memory snapshot', 'nothing is written there')
else {
writeFileSync(temp, text, 'utf8')
renameSync(temp, path)
Expand Down
16 changes: 13 additions & 3 deletions core/src/host/real-dir.js
Original file line number Diff line number Diff line change
Expand Up @@ -51,9 +51,19 @@ export async function fileKind(path) {
// `writeFile` follows a link at the last component, so a link left at `<file>.<pid>.tmp` would take
// the bytes outside the home and the `rename` after them would put the link where the file belongs.
// `absent` is a writer's to create and a plain leftover under this host's own pid is replaceable;
// anything else refuses the write before the first byte. The destination itself needs no such
// check: `rename` replaces the name, link included, instead of writing through it.
export const tempWritable = (path) => fileKindSync(path) !== 'other'
// anything else refuses the write before the first byte. A hard link is a plain file to `lstat`
// while its bytes are also a name outside this home (a write lands in the shared inode, not in a
// link), so a name with another name (`nlink > 1`) is refused the same way; a crashed host's own
// leftover is one name and stays replaceable. The destination itself needs no such check: `rename`
// replaces the name, link included, instead of writing through it.
export const tempWritable = (path) => {
try {
const info = lstatSync(path)
return info.isFile() && !info.isSymbolicLink() && info.nlink === 1
} catch (error) {
return error?.code === 'ENOENT'
}
}

export function fileKindSync(path) {
try {
Expand Down
2 changes: 1 addition & 1 deletion core/src/host/store.js
Original file line number Diff line number Diff line change
Expand Up @@ -928,7 +928,7 @@ export function install(rt) {
const temp = `${statePath}.${process.pid}.tmp`
// The temp name is a leaf like the state file itself: a link there would take the team
// outside the home and the rename would put the link over state.json (temp-name sweep).
if (!tempWritable(temp)) throw new Error(`Not saved: ${temp} is not a plain file, so the Bot team was not written there.`)
if (!tempWritable(temp)) throw new Error(`Not saved: ${temp} is not a plain file of its own, so the Bot team was not written there.`)
try {
await mkdir(home, { recursive: true })
const ensureLock = () => {
Expand Down
22 changes: 21 additions & 1 deletion core/test/automation-default.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict'
import { existsSync, lstatSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { existsSync, linkSync, lstatSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { dirname, join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -161,3 +161,23 @@ test('a linked defaults temp name does not take the offer; the file is not creat
assert.equal(existsSync(file) ? lstatSync(file).isSymbolicLink() : false, false, 'the defaults file is not a link')
assert.ok(warnings.some(line => /not a plain file/.test(line)), warnings.join('\n'))
})

// The temp name can be a hard link, which `lstat` reports as a plain file: the offer would land in
// the inode the other name holds. It is refused before the write, once, and the bundle is not
// switched on.
test('a hard link at the defaults temp name keeps the offer unwritten', async (t) => {
const file = scratch(t)
const outside = join(dirname(file), 'outside-defaults.json')
writeFileSync(outside, 'OUTSIDE\n')
linkSync(outside, `${file}.${process.pid}.tmp`)
const warnings = []
const plugins = manager()
const outcome = await offerBundle({ manager: plugins, profile: 'web', file, warn: (...args) => warnings.push(args.join(' ')) })
assert.equal(outcome, 'unrecorded')
assert.equal(plugins.on, false, 'the bundle is not switched on')
assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the shared inode keeps its bytes')
assert.equal(lstatSync(outside).nlink, 2, 'the outside file still carries the temp name')
assert.equal(existsSync(file), false, 'the defaults file is not created')
assert.ok(warnings.some(line => /not a plain file/.test(line)), warnings.join('\n'))
assert.ok(warnings.some(line => line.includes(`${file}.${process.pid}.tmp`)), 'the warning names the temp path')
})
34 changes: 33 additions & 1 deletion core/test/decisions.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict'
import { existsSync, lstatSync, mkdtempSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { existsSync, linkSync, lstatSync, mkdtempSync, readFileSync, readlinkSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -180,3 +180,35 @@ test('#2103: a linked temp name does not take the trim; the log stays a plain fi
assert.equal(existsSync(`${plain}.tmp`), false, 'the healthy trim renames its temp name away')
assert.deepEqual(healthyWarnings, [])
})

// The trim's temp name can be a hard link, which `lstat` reports as a plain file: the retained log
// would be written into the inode the other name holds. It is refused before the write, warned
// once, and the log keeps its lines.
test('#P-hardlink: a hard link at the trim temp name does not take the trim', async (t) => {
const line = (n) => `${JSON.stringify({ time: 1_000 + n, bot: 'a', botName: 'Chief', option: `kept ${n}`, choice: 'used' })}\n`
const full = (n) => Array.from({ length: n }, (_, i) => line(i)).join('')

const { ctx, home, tool, stop } = await boot({ decisions: true })
t.after(stop)
const elsewhere = mkdtempSync(join(tmpdir(), 'ds-bot-decisions-tmp-target-'))
t.after(() => rmSync(elsewhere, { recursive: true, force: true, maxRetries: 5 }))
const target = join(elsewhere, 'outside.txt')
const before = 'PRE-EXISTING BYTES\n'
writeFileSync(target, before)
const log = join(home, 'decisions.jsonl')
writeFileSync(log, full(2_000))
linkSync(target, `${log}.tmp`)

const warnings = []
ctx.logger.warn = (...args) => warnings.push(args.join(' '))
const first = await tool('record_decision', { option: 'Trigger the trim', choice: 'used' })

assert.equal(readFileSync(target, 'utf8'), before, 'the shared inode keeps its bytes')
assert.equal(lstatSync(target).nlink, 2, 'the outside file still carries the temp name')
const lines = readFileSync(log, 'utf8').split('\n').filter(Boolean)
assert.equal(lines.length, 2_001, 'the refused trim leaves the log as it was, plus the new line')
assert.doesNotMatch(first, /^Recorded:/, 'the answer does not claim a clean record')
assert.equal(warnings.length, 1, 'one warning for the temp name')
assert.match(warnings[0], /decisions\.jsonl\.tmp/)
assert.match(warnings[0], /not a plain file/)
})
20 changes: 19 additions & 1 deletion core/test/images.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict'
import { existsSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { tmpdir } from 'node:os'
import { join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -185,3 +185,21 @@ test('an ordinary OCR cache temp name is written and renamed as before', async (
assert.equal(warnings.length, 0)
assert.equal(JSON.parse(readFileSync(cache, 'utf8'))[OCR_ATTACHMENT.attachmentId].text, 'A red triangle.')
})

// The temp name can be a hard link: `lstat` reports a plain file, so the link check alone lets the
// cache land in the inode the other name holds. The refusal names the temp path and leaves both
// files as they were.
test('a hard link at the OCR cache temp name does not take the cache', async (t) => {
const { rt, home, warnings } = ocrRuntime(t)
const outside = join(home, 'outside-ocr.txt')
writeFileSync(outside, 'OUTSIDE\n')
linkSync(outside, join(home, `ocr.json.${process.pid}.tmp`))
await rt.imagesForGroup([OCR_ATTACHMENT], 'r1')
await waitFor(() => warnings.length > 0)
assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the shared inode keeps its bytes')
assert.equal(lstatSync(outside).nlink, 2, 'the outside file still carries the temp name')
const cache = join(home, 'ocr.json')
assert.equal(existsSync(cache), false, 'the cache is not created from the shared file')
assert.ok(warnings.some(line => /not a plain file/.test(line)), warnings.join('\n'))
assert.ok(warnings.some(line => line.includes(`${cache}.${process.pid}.tmp`)), 'the warning names the temp path')
})
65 changes: 62 additions & 3 deletions core/test/memory.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import assert from 'node:assert/strict'
import { existsSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs'
import { hostname, tmpdir } from 'node:os'
import { dirname, join } from 'node:path'
import { test } from 'node:test'
Expand Down Expand Up @@ -1724,8 +1724,8 @@ test('#2097: a link left at a writer\'s temp name takes no entry, and the memory
t.after(() => { console.error = original })
const { tool, stop } = start({ home })
t.after(() => stop())
assert.match(await tool('remember', { text: 'Ship on Friday', scope: 'team' }, 'g3'), /is not a plain file, so nothing is written there/)
assert.match(await tool('remember', { text: 'Ship on Monday', scope: 'team' }, 'g3'), /is not a plain file, so nothing is written there/)
assert.match(await tool('remember', { text: 'Ship on Friday', scope: 'team' }, 'g3'), /is not a plain file of its own, so nothing is written there/)
assert.match(await tool('remember', { text: 'Ship on Monday', scope: 'team' }, 'g3'), /is not a plain file of its own, so nothing is written there/)
assert.equal(readFileSync(target, 'utf8'), 'PRE-EXISTING LINE\n', 'the file the link names is byte-identical')
assert.equal(lstatSync(written).isSymbolicLink(), false, 'the memory file is still a plain file, not the link')
assert.equal(readFileSync(written, 'utf8'), '# Memory\n\n- Kept by hand [added: 2026-10-01]\n', 'the memory file kept what it had')
Expand Down Expand Up @@ -1858,3 +1858,62 @@ test('#2068: a linked summaries folder gets no summary written through it', asyn
for (let i = 0; i < 50; i++) await settle()
assert.deepEqual(readdirSync(outside), [], 'nothing was written through the link')
})

// The temp name can be a hard link, which `lstat` reports as a plain file: the entry would land in
// the inode the other name holds and the memory file would then name that shared file. A name that
// is a file but not only this one is refused like a link, and the memory file keeps what it had.
test('#P-hardlink: a hard link at a writer\'s temp name takes no entry', async (t) => {
const home = mkdtempSync(join(tmpdir(), 'ds-bot-memory-'))
const outside = mkdtempSync(join(tmpdir(), 'ds-bot-memory-outside-'))
const target = join(outside, 'taken.md')
writeFileSync(target, 'PRE-EXISTING LINE\n')
const written = join(home, 'memory', 'team', 'MEMORY.md')
mkdirSync(dirname(written), { recursive: true })
writeFileSync(written, '# Memory\n\n- Kept by hand [added: 2026-10-01]\n')
const temp = `${written}.${process.pid}.tmp`
linkSync(target, temp)
t.after(() => rmSync(outside, { recursive: true, force: true }))
const lines = []
const original = console.error
console.error = (...args) => { lines.push(args.join(' ')) }
t.after(() => { console.error = original })
const { tool, stop } = start({ home })
t.after(() => stop())
assert.match(await tool('remember', { text: 'Ship on Friday', scope: 'team' }, 'g3'), /is not a plain file of its own, so nothing is written there/)
assert.equal(readFileSync(target, 'utf8'), 'PRE-EXISTING LINE\n', 'the shared inode is byte-identical')
assert.equal(lstatSync(target).nlink, 2, 'the outside file still carries the temp name')
assert.equal(readFileSync(written, 'utf8'), '# Memory\n\n- Kept by hand [added: 2026-10-01]\n', 'the memory file kept what it had')
const warned = lines.filter(line => line.includes('not a plain file'))
assert.equal(warned.length, 1, 'one warning per temp name per process')
assert.ok(warned[0].includes(temp), `the warning names the temp name: ${warned[0]}`)
})

// The snapshot's temp name carries the same shape: a hard link there would freeze the memory into
// the shared inode and then put that file where the snapshot belongs.
test('#P-hardlink: a hard link at the snapshot temp name is not written through', async (t) => {
const home = mkdtempSync(join(tmpdir(), 'ds-bot-memory-'))
const outside = mkdtempSync(join(tmpdir(), 'ds-bot-memory-outside-'))
const target = join(outside, 'snapshot.md')
writeFileSync(target, 'PRE-EXISTING LINE\n')
const snapshots = join(home, 'memory', 'snapshots')
mkdirSync(snapshots, { recursive: true })
const snapshot = join(snapshots, 'b2.md')
const temp = `${snapshot}.tmp`
linkSync(target, temp)
t.after(() => rmSync(outside, { recursive: true, force: true }))
const lines = []
const original = console.error
console.error = (...args) => { lines.push(args.join(' ')) }
t.after(() => { console.error = original })
const { section, call, stop } = start({ home })
t.after(() => stop())
await call('state', {})
const text = section('b2')
assert.match(text, /The team memory is shared by every Bot/, 'the section is built even when it cannot be frozen')
assert.equal(readFileSync(target, 'utf8'), 'PRE-EXISTING LINE\n', 'the shared inode is byte-identical')
assert.equal(lstatSync(target).nlink, 2, 'the outside file still carries the temp name')
assert.equal(existsSync(snapshot), false, 'the shared file was not renamed into the snapshot place')
const warned = lines.filter(line => line.includes('not a plain file'))
assert.equal(warned.length, 1, 'said once for the temp name')
assert.ok(warned[0].includes(temp), `the warning names the temp name: ${warned[0]}`)
})
Loading
Loading