From b62fcb3297255fe19e0e145a9bbc83c744c73050 Mon Sep 17 00:00:00 2001 From: Dev-3 Date: Mon, 2 Nov 2026 04:03:28 +0000 Subject: [PATCH] fix(host): ask the temp name before five writes land through it The team state, OCR cache, Automation-tasks defaults, Bot Social server choice and workflow store wrote through ..tmp without asking that name. A link pre-set there took the write outside the folder and renamed the link into place. The writes now refuse a temp name that is not absent or a plain file; the destination needs no check because rename replaces it, link included. --- changelog.d/2109-temp-name-sweep.fixed.md | 1 + core/src/host/automation-default.js | 11 ++- core/src/host/images.js | 4 + core/src/host/real-dir.js | 8 ++ core/src/host/store.js | 4 + core/test/automation-default.test.js | 21 ++++- core/test/images.test.js | 74 ++++++++++++++- core/test/roster.test.js | 5 +- core/test/store.test.js | 37 +++++++- docs/subsystems/core-host.md | 2 +- docs/subsystems/social.md | 2 +- docs/subsystems/workflow.md | 2 +- patches/ds-bot/0001-core-fixes.patch | 94 ++++++++++++++++++- patches/ds-bot/0002-flybot-api-host.patch | 13 ++- patches/ds-bot/0009-identity-schema.patch | 13 ++- patches/ds-bot/0014-flybot-branding.patch | 46 ++++++++- .../files/src/host/automation-default.js | 11 ++- patches/ds-bot/files/src/host/real-dir.js | 8 ++ .../files/test/automation-default.test.js | 21 ++++- plugins/social/src/server-config.js | 11 ++- .../social/test/server-config-temp.test.js | 40 ++++++++ plugins/workflow/src/service.js | 11 ++- plugins/workflow/test/temp-name.test.js | 28 ++++++ 23 files changed, 441 insertions(+), 26 deletions(-) create mode 100644 changelog.d/2109-temp-name-sweep.fixed.md create mode 100644 plugins/social/test/server-config-temp.test.js create mode 100644 plugins/workflow/test/temp-name.test.js diff --git a/changelog.d/2109-temp-name-sweep.fixed.md b/changelog.d/2109-temp-name-sweep.fixed.md new file mode 100644 index 000000000..41f569370 --- /dev/null +++ b/changelog.d/2109-temp-name-sweep.fixed.md @@ -0,0 +1 @@ +A link left at the temporary name a write lands through no longer takes the write: the team state, the OCR cache, the Automation-tasks defaults, the Bot Social server choice and the workflow store ask `..tmp` as a file before writing, so a link (or a folder) there refuses the save, with the temp name in the message, instead of putting the bytes outside the folder and renaming the link into place. A window between the check and the write remains, as with every `lstat`-then-write guard. ([#2109](https://github.com/ThinkFlowLab/FlyBot/issues/2109)). diff --git a/core/src/host/automation-default.js b/core/src/host/automation-default.js index d9e7c1a31..1d441947c 100644 --- a/core/src/host/automation-default.js +++ b/core/src/host/automation-default.js @@ -8,6 +8,7 @@ // A DSH that does not offer the bundle is left alone, and so is a second dsh on the same data. import { mkdir, readFile, rename, unlink, writeFile } from 'node:fs/promises' import { join } from 'node:path' +import { tempWritable } from './real-dir.js' export const SCHEDULE_BUNDLE = '@deepseek-ai/dsh-experimental-schedule-bundle' const MARKER = 'dsh-defaults.json' @@ -34,6 +35,9 @@ export async function readOffers(file) { async function writeOffers(file, offers) { await mkdir(join(file, '..'), { recursive: true }) 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.`) try { await writeFile(temp, `${JSON.stringify({ version: 1, offers }, null, 2)}\n`, 'utf8') await rename(temp, file) @@ -50,7 +54,7 @@ async function writeOffers(file, offers) { // 'unrecorded' the offer could not be written down, so nothing was changed // 'enabled' switched on // 'failed' the manager refused; the note is withdrawn so the next start tries again -export async function offerBundle({ manager, bundle = SCHEDULE_BUNDLE, profile, file, now = () => new Date() }) { +export async function offerBundle({ manager, bundle = SCHEDULE_BUNDLE, profile, file, now = () => new Date(), warn = () => {} }) { let offers try { offers = await readOffers(file) @@ -70,7 +74,8 @@ export async function offerBundle({ manager, bundle = SCHEDULE_BUNDLE, profile, } try { await note(now().toISOString()) - } catch { + } catch (error) { + warn('ds-bot: the Automation tasks offer was not written down: %s', error?.message ?? error) return 'unrecorded' } if (entry.enabled === true) return 'already-on' @@ -91,7 +96,7 @@ export function install(rt) { // The window that shows the team without owning it leaves the profile to the other one. if (rt.readOnly() !== undefined) return const profile = ctx.get?.('profileContext')?.name ?? 'default' - const outcome = await offerBundle({ manager: ctx.get?.('pluginManager'), profile, file: join(home, MARKER) }) + const outcome = await offerBundle({ manager: ctx.get?.('pluginManager'), profile, file: join(home, MARKER), warn: rt.warn }) if (outcome === 'failed') rt.warn('ds-bot: could not switch on Automation tasks; switch them on from the Plugins page') } diff --git a/core/src/host/images.js b/core/src/host/images.js index bf80c633d..0639f927b 100644 --- a/core/src/host/images.js +++ b/core/src/host/images.js @@ -6,6 +6,7 @@ import { mkdir, readFile, rename, stat, writeFile } from 'node:fs/promises' import { join } from 'node:path' import { BlockAssembler } from '@deepseek-ai/dsh-llm' import { OCR_CACHE_MAX, OCR_PROMPT, OCR_RETRY_MS, OCR_TIMEOUT_MS } from './constants.js' +import { tempWritable } from './real-dir.js' import { splitRef } from './text.js' export function offloadTargets(session, beforeSeq) { @@ -131,6 +132,9 @@ export function install(rt) { ocrWriting = ocrWriting.then(async () => { await mkdir(home, { recursive: true }) 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.`) await writeFile(temp, snapshot, 'utf8') await rename(temp, ocrPath) }).catch(error => rt.warn('ds-bot: OCR cache write failed: %s', error)) diff --git a/core/src/host/real-dir.js b/core/src/host/real-dir.js index 68b9309f1..de82dad09 100644 --- a/core/src/host/real-dir.js +++ b/core/src/host/real-dir.js @@ -47,6 +47,14 @@ export async function fileKind(path) { } } +// A temp name a write lands through is a leaf like the file it becomes (temp-name sweep): the +// `writeFile` follows a link at the last component, so a link left at `..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' + export function fileKindSync(path) { try { return fileKindOf(lstatSync(path)) diff --git a/core/src/host/store.js b/core/src/host/store.js index bd49b36e1..6cf111a4a 100644 --- a/core/src/host/store.js +++ b/core/src/host/store.js @@ -8,6 +8,7 @@ import { join } from 'node:path' import { LEGACY_THEMES, MAX_ENTRY, MAX_EXCHANGES, ROOM_MODES } from './constants.js' import { homeLock } from './home-lock.js' import { openJournal } from './journal.js' +import { tempWritable } from './real-dir.js' import { codePoints, currentColor, normalize, unquote } from './text.js' // The saved team's format version. Version 2 (#47) adds the identity schema: member @@ -925,6 +926,9 @@ export function install(rt) { const snapshot = JSON.stringify(state, null, 2) const run = writing.then(async () => { 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.`) try { await mkdir(home, { recursive: true }) const ensureLock = () => { diff --git a/core/test/automation-default.test.js b/core/test/automation-default.test.js index e227d95da..12c4cdeb4 100644 --- a/core/test/automation-default.test.js +++ b/core/test/automation-default.test.js @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, 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' @@ -142,3 +142,22 @@ test('config.automationTasks = false never offers it', (t) => { assert.equal(ctx.calls.includes('effect ds-bot: automation tasks default'), false) assert.equal(rt.offerAutomationTasks, undefined) }) + +// The defaults file lands through `..tmp` and is renamed into place. The temp name is a +// leaf like the file (temp-name sweep): a link left there would take the offer outside the team +// folder and the rename would put the link where `dsh-defaults.json` belongs. A refused write is +// reported once, and the offer is not recorded. +test('a linked defaults temp name does not take the offer; the file is not created', async (t) => { + const file = scratch(t) + const outside = join(dirname(file), 'outside-defaults.json') + writeFileSync(outside, 'OUTSIDE\n') + symlinkSync(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 when the offer cannot be written down') + assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') + 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')) +}) diff --git a/core/test/images.test.js b/core/test/images.test.js index b7b5c63a9..cae6e0924 100644 --- a/core/test/images.test.js +++ b/core/test/images.test.js @@ -1,9 +1,10 @@ import assert from 'node:assert/strict' -import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, 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' -import { attachmentFilePath, attachmentObjectPath, imagePlaceholder, offloadTargets } from '../src/host/images.js' +import { attachmentFilePath, attachmentObjectPath, imagePlaceholder, install, offloadTargets } from '../src/host/images.js' +import { fakeContext } from './fake-ctx.js' test('offloadTargets selects live images from prior user and tool messages', () => { const events = [ @@ -115,3 +116,72 @@ test('#779: the placeholder names the file, its size and its type; without a pat // An unnamed image keeps the sha prefix the host has always shown. assert.match(imagePlaceholder({ attachmentId: id }), /\[Image sha256:bdc77c70:/) }) + +// The OCR cache lands through `/ocr.json..tmp` and is renamed into place. That temp name +// is a leaf like the cache (temp-name sweep): `writeFile` follows a link there, so a pre-set link +// would take the cache outside the home and the rename would put the link where `ocr.json` belongs. +const OCR_ATTACHMENT = { attachmentId: `sha256:${'b'.repeat(64)}`, name: 'triangle.png' } + +function ocrRuntime(t) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-images-ocr-')) + const warnings = [] + const ctx = fakeContext() + Object.assign(ctx, { + llm: { + listProviders: () => [{ id: 'deepseek', name: 'DeepSeek' }], + listModels: async () => [{ id: 'vision', input: ['text', 'image'] }], + resolveModelInfo: async () => ({ inputModalities: ['text', 'image'] }), + stream: async function* () { + yield { type: 'text-delta', index: 0, text: 'A red triangle.' } + yield { type: 'finish', reason: { kind: 'stop' } } + }, + }, + }) + const rt = { + ctx, + home, + config: {}, + warn: (...args) => warnings.push(args.join(' ')), + load: async () => {}, + readOnly: () => undefined, + roomOf: () => 'r1', + isBotSession: () => true, + ownRequest: () => false, + catalog: async () => [{ ref: 'deepseek/vision', name: 'Vision', input: ['image'] }], + defaultRef: () => 'deepseek/vision', + tagUsage: messages => messages, + } + install(rt) + t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) + return { rt, home, warnings } +} + +// The cache write is queued after the reading, so wait for the effect instead of guessing a delay. +async function waitFor(check) { + for (let i = 0; i < 300; i += 1) { + if (await check()) return + await new Promise(resolve => setTimeout(resolve, 10)) + } +} + +test('a linked OCR cache temp name does not take the cache; ocr.json stays a plain file', async (t) => { + const { rt, home, warnings } = ocrRuntime(t) + const outside = join(home, 'outside-ocr.txt') + writeFileSync(outside, 'OUTSIDE\n') + symlinkSync(outside, join(home, `ocr.json.${process.pid}.tmp`)) + await rt.imagesForGroup([OCR_ATTACHMENT], 'r1') + await waitFor(() => readFileSync(outside, 'utf8') !== 'OUTSIDE\n' || warnings.length > 0) + assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') + const cache = join(home, 'ocr.json') + assert.equal(existsSync(cache) ? lstatSync(cache).isSymbolicLink() : false, false, 'the cache is not a link') + assert.ok(warnings.some(line => /not a plain file/.test(line)), warnings.join('\n')) +}) + +test('an ordinary OCR cache temp name is written and renamed as before', async (t) => { + const { rt, home, warnings } = ocrRuntime(t) + await rt.imagesForGroup([OCR_ATTACHMENT], 'r1') + const cache = join(home, 'ocr.json') + await waitFor(() => existsSync(cache)) + assert.equal(warnings.length, 0) + assert.equal(JSON.parse(readFileSync(cache, 'utf8'))[OCR_ATTACHMENT.attachmentId].text, 'A red triangle.') +}) diff --git a/core/test/roster.test.js b/core/test/roster.test.js index 8eaf39509..dcb3dbb42 100644 --- a/core/test/roster.test.js +++ b/core/test/roster.test.js @@ -541,7 +541,10 @@ test('a write that fails at save time archives the Session it made', async (t) = ctx.workspaceRegistry.archiveSession = async (id) => { archived.push(id); return archive(id) } const answer = await call('create-bot', { name: 'Phantom' }) assert.equal(answer.ok, false) - assert.equal(answer.error.code, 'bot/internal') + // The temp-name guard refuses before `writeFile` reaches EISDIR, so the failure is a refusal + // that names the temp path; the rollback this test is about is unchanged. + assert.equal(answer.error.code, 'bot/refused') + assert.match(answer.error.message, /not a plain file/) assert.deepEqual(archived, created) const team = (await call('state', {})).value assert.equal(team.bots.some(bot => bot.name === 'Phantom'), false) diff --git a/core/test/store.test.js b/core/test/store.test.js index 45c6841df..6877027c1 100644 --- a/core/test/store.test.js +++ b/core/test/store.test.js @@ -68,7 +68,7 @@ test('a failed state write rejects and leaves the saved file unchanged', async ( const original = fs.readFileSync(statePath) fs.mkdirSync(`${statePath}.${process.pid}.tmp`) rt.state.prefs.marker = 'not saved' - await assert.rejects(rt.save(), { code: 'EISDIR' }) + await assert.rejects(rt.save()) assert.deepEqual(fs.readFileSync(statePath), original) }) @@ -320,3 +320,38 @@ test('a roster id that is not an own key finds no Bot and no group (#1704)', asy assert.equal(rt.findBot('a').name, 'Chief') assert.equal(rt.findRooms('Ops')[0].id, 'r1') }) + +// The state write lands through `.json..tmp` and then renames it into place. The temp +// name is a leaf like the state file (temp-name sweep): `writeFile` would follow a link there, put +// the team outside the home and then rename the link over the state file. Only the target is +// replaced by `rename`, link included, never written through. +test('a linked state temp name does not take the save; the state file stays a plain file', async (t) => { + const { rt, home, statePath, warnings } = runtime(t) + await rt.load() + const outside = join(home, 'outside-state.json') + fs.writeFileSync(outside, 'OUTSIDE\n') + fs.symlinkSync(outside, `${statePath}.${process.pid}.tmp`) + rt.state.prefs.sweep = 'not saved' + await assert.rejects(rt.save()) + assert.equal(fs.readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') + assert.equal(fs.lstatSync(statePath).isSymbolicLink(), false, 'the state file is still a plain file') + assert.equal(JSON.parse(fs.readFileSync(statePath, 'utf8')).prefs.sweep, undefined, 'the refused save did not land') + assert.ok(warnings.some(line => /not a plain file/.test(line.join(' '))), 'the refusal is named') +}) + +// The other half of the same shape: a link at the destination itself is replaced by the rename, so +// it never needs the temp check. This is why the sweep only adds the temp name. +test('a link at the state file itself is replaced, not written through', async (t) => { + const { rt, home, statePath } = runtime(t) + await rt.load() + await rt.save() + const outside = join(home, 'outside-state.json') + fs.writeFileSync(outside, 'OUTSIDE\n') + fs.rmSync(statePath) + fs.symlinkSync(outside, statePath) + rt.state.prefs.sweep = 'kept' + await rt.save() + assert.equal(fs.readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the rename never follows the target link') + assert.equal(fs.lstatSync(statePath).isSymbolicLink(), false, 'the link is replaced by the saved file') + assert.equal(JSON.parse(fs.readFileSync(statePath, 'utf8')).prefs.sweep, 'kept') +}) diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index d340b0fed..1117e0142 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -229,7 +229,7 @@ An admin's `@` used to be the room's control plane: every `@Dev #1234 rebase and - `src/host/memory-search.js` derives two recall aids from the memory files that `src/host/memory.js` already reads; both are pure and rebuildable, and the Markdown files stay the only source of truth. - Hybrid ranking: `termsOf`/`buildIndex`/`searchIndex` build a sparse TF-IDF vector per entry over words, word character trigrams (4+ letters, so a shared stem matches across endings) and CJK uni/bi-grams, and rank by cosine. `recall` uses it only when called with `semantic: "true"`; the default exact-word path is unchanged, so an existing call scores and prints exactly as before. - Graph: `buildGraph`/`relatedOf` turn `[[topic]]` references into topic→topic edges and a `by:` metadata value into file→Bot edges; `recall` with `related` walks them (undirected, up to two hops) and lists what is near a topic or Bot. Tests: `core/test/memory-search.test.js` (pure) and the two `recall` cases in `core/test/memory.test.js`. -- The memory folders are entries of their own ([#2057](https://github.com/ThinkFlowLab/FlyBot/issues/2057), [#2068](https://github.com/ThinkFlowLab/FlyBot/issues/2068)): one resolver, `folderKind()` in `src/host/memory.js`, walks every segment from `/memory` down — the root, `memory/bots` for a Bot scope, the scope folder — and answers `real` / `absent` / `other`; `readFolder()` and `writeFolder()` are its two doors. A reader (`scopeDir`, the snapshot read, `savedSummary`, `reviewed.json`) takes only `real`, so a link on any of those segments cannot feed another folder's `MEMORY.md`, topics, frozen prompt snapshot or summary into a prompt, a `recall`, the memory panel or the Project dossier; `absent` and `other` answer what a missing folder answers (the one empty `MEMORY.md` entry and no topics). A host write or unlink (the snapshot, the summary, `reviewed.json`, the archive, the journal — and since #2068 the entry writers behind `remember`/`edit`/`forget`) still creates a folder that is `absent`, as it always did, and refuses `other` instead of writing or deleting through the link; `workflow-store.js` does the same for a workspace's `workflows/` on the read and write side. `other` is named once per folder and process in one warning (`ds-bot: memory folder is not a plain directory …; keeping it empty`, or the `workflow folder` form), because a folder that reads as empty and a folder that cannot be read look alike. The journal's own file is asked as a file too ([#2079](https://github.com/ThinkFlowLab/FlyBot/issues/2079)): `journal.jsonl` must be `absent` (created) or a plain non-link file before a record is appended, and a link, a directory or a check that cannot run drops that one record with a warning once per file and process, while the memory write itself still lands — the journal is a side channel. The temporary name a write lands through is asked the same way ([#2097](https://github.com/ThinkFlowLab/FlyBot/issues/2097)): `..tmp` for an entry write and `.tmp` for a frozen snapshot must be `absent` or a plain non-link file, because `writeFile` follows a link there and the `rename` after it would put the link where the memory file belongs — a link refuses the write with one warning, nothing is renamed into place and the file keeps what it had, while a check that cannot run refuses the same way. The panel can say which of the two it is looking at ([#2101](https://github.com/ThinkFlowLab/FlyBot/issues/2101)): `memory-view` carries `blocked`, one `{ scope, path }` per scope the host refused (`other` only — `absent` is an ordinary empty home), and the memory panel prints one line naming that folder above its empty title, while an empty or not-yet-created folder renders exactly the copy it rendered before. Out of scope, deliberately: a single `*.md`/`*.json` leaf that is a link is still read, and a local process that can write inside the home can still win a redirect race. +- The memory folders are entries of their own ([#2057](https://github.com/ThinkFlowLab/FlyBot/issues/2057), [#2068](https://github.com/ThinkFlowLab/FlyBot/issues/2068)): one resolver, `folderKind()` in `src/host/memory.js`, walks every segment from `/memory` down — the root, `memory/bots` for a Bot scope, the scope folder — and answers `real` / `absent` / `other`; `readFolder()` and `writeFolder()` are its two doors. A reader (`scopeDir`, the snapshot read, `savedSummary`, `reviewed.json`) takes only `real`, so a link on any of those segments cannot feed another folder's `MEMORY.md`, topics, frozen prompt snapshot or summary into a prompt, a `recall`, the memory panel or the Project dossier; `absent` and `other` answer what a missing folder answers (the one empty `MEMORY.md` entry and no topics). A host write or unlink (the snapshot, the summary, `reviewed.json`, the archive, the journal — and since #2068 the entry writers behind `remember`/`edit`/`forget`) still creates a folder that is `absent`, as it always did, and refuses `other` instead of writing or deleting through the link; `workflow-store.js` does the same for a workspace's `workflows/` on the read and write side. `other` is named once per folder and process in one warning (`ds-bot: memory folder is not a plain directory …; keeping it empty`, or the `workflow folder` form), because a folder that reads as empty and a folder that cannot be read look alike. The journal's own file is asked as a file too ([#2079](https://github.com/ThinkFlowLab/FlyBot/issues/2079)): `journal.jsonl` must be `absent` (created) or a plain non-link file before a record is appended, and a link, a directory or a check that cannot run drops that one record with a warning once per file and process, while the memory write itself still lands — the journal is a side channel. The temporary name a write lands through is asked the same way ([#2097](https://github.com/ThinkFlowLab/FlyBot/issues/2097)): `..tmp` for an entry write and `.tmp` for a frozen snapshot must be `absent` or a plain non-link file, because `writeFile` follows a link there and the `rename` after it would put the link where the memory file belongs — a link refuses the write with one warning, nothing is renamed into place and the file keeps what it had, while a check that cannot run refuses the same way. The same temp-name guard covers the team state (`state.json..tmp`), the OCR cache (`ocr.json..tmp`) and the Automation-tasks defaults (`dsh-defaults.json..tmp`, refused as `unrecorded` with one warning): a link, a folder or a check that cannot run refuses the write before its first byte, while the destination itself needs no check because `rename` replaces the name, link included. The panel can say which of the two it is looking at ([#2101](https://github.com/ThinkFlowLab/FlyBot/issues/2101)): `memory-view` carries `blocked`, one `{ scope, path }` per scope the host refused (`other` only — `absent` is an ordinary empty home), and the memory panel prints one line naming that folder above its empty title, while an empty or not-yet-created folder renders exactly the copy it rendered before. Out of scope, deliberately: a single `*.md`/`*.json` leaf that is a link is still read, and a local process that can write inside the home can still win a redirect race. - `recall` shows at most 30 entries (`RECALL_MAX` in `src/host/memory.js`) and now says when it cut the list ([#2001](https://github.com/ThinkFlowLab/FlyBot/issues/2001)): the exact-word path prints `N more entries match; narrow the query or read a topic` with the real count it already holds, and `semantic: "true"` asks `searchIndex` for one past the cap and prints `more entries are close in meaning than are shown; narrow the query or read a topic` because that index returns the top k only. The cap stays 30; a reply that shows every match adds neither line. ### Plugin memory seam (#450 item 1) diff --git a/docs/subsystems/social.md b/docs/subsystems/social.md index 26fcaca31..50271fd0d 100644 --- a/docs/subsystems/social.md +++ b/docs/subsystems/social.md @@ -151,7 +151,7 @@ Creating a group opens its Session with zero turns, and the host draws no sessio ### The server in Settings (#337) -- **Where it comes from.** `config.server` in the profile patch is the default. A server picked in Settings → **Bot Social** is kept in `flybot-social-server.json` (mode 600, next to the credentials file) and wins while it exists; choosing the default again removes the file. Nothing configured anywhere behaves as before (the start fails with `server URL is required`, `Social is not running: …`; the page says **No server set**). The page reads `social/server` (a read; `{ server, source: settings|config|none, defaultServer, state: connected|connecting|not-configured|error, handle, error, canChange, known[], platform? }`: never a token) and calls `social/server-check` (it makes the host fetch an address the caller names, so it follows the write rules like `social/server-set`). +- **Where it comes from.** `config.server` in the profile patch is the default. A server picked in Settings → **Bot Social** is kept in `flybot-social-server.json` (mode 600, next to the credentials file) and wins while it exists; choosing the default again removes the file. It is written through `flybot-social-server.json..tmp`, and that temp name is asked as a file first (temp-name sweep): a link left there refuses the save instead of putting the choice outside the folder and renaming the link into place. Nothing configured anywhere behaves as before (the start fails with `server URL is required`, `Social is not running: …`; the page says **No server set**). The page reads `social/server` (a read; `{ server, source: settings|config|none, defaultServer, state: connected|connecting|not-configured|error, handle, error, canChange, known[], platform? }`: never a token) and calls `social/server-check` (it makes the host fetch an address the caller names, so it follows the write rules like `social/server-set`). - **Validation.** `parseServerUrl`: `https://` only, `http://` just for `localhost`, `127.0.0.1` and `[::1]`; no user name or password, query or fragment; at most 200 characters; an address without a scheme gets `https://` (`http://` for those local hosts). `probeServer` GETs `/v1/platform` without credentials, 8 s deadline, `redirect: 'manual'` (a redirect is refused, so the saved address is the checked one); the answer must look like Bot Social (a `name`, a `platform_key` or a `features` list) and its `features` are shown (cleaned, at most 40) in words (`featureLine` in `client.js`: `typed_by_owner` → typing as you, `mentions` → @mentions, `presence` → online status; an unknown id is only counted, and every raw id is in the line's tooltip, #421). `social/server-set` repeats both before it touches anything, so a UI that skipped **Check** gains nothing. - **A server is an identity.** The default server keeps its files. Any other server gets `flybot-social.json.` (credentials; the same suffix `startSocial` already used for backups, `tag` = first 8 hex of the sha-256 of the normalized address) and `flybot-social-settings..json`; a different server never reads another's file, and no credentials file is ever deleted or rewritten by a switch. The per-Bot parts that live in host state (the `member` identity, the `autoApprove` marks and the person/room grants, which are Bot Social ids and handles) are parked in `flybot-social-bots..json` (no token) and cleared before the new server starts, so a stale grant to `@bob.home` can never apply to a different Bob on the new server; switching back restores them and the saved credentials, without a second onboarding. The old server's friends' Bots leave the list when the new server's first roster fetch replaces it (`remote.publish`); face-to-face rooms already hosted stay as local transcripts, but nothing polls them. - **Applied live, with a way back.** `social/server-set` (needs `confirm: true`; a write, so a read-only host and a multi-member team refuse it) stops the social layer, parks the old server's Bot identities, saves the choice, restores what the new server had, and starts again (first start onboards). If that start fails or takes over 45 s, the previous server's files, identities and choice are put back, it is started again, and the error says so; a start that finishes late is disposed. A disposed instance never re-arms its poll: a face poll already in flight when `dispose()` runs cannot install a new interval on the discarded layer, so it never keeps polling (or delivering from) the server it was built for. When the switch succeeds and the address really changed, the previous server's hosted face-to-face rooms — open or still forming (candidate) — are marked `left` here — a read-only past room whose composer is gone, since the new server cannot reach them. Renaming this home on the same server is not a switch: that server still reaches those rooms, so only a new address ends them (the room and its composer stay as they were). Switching back does not reopen them: a room this home left stays a past room (`faceSnapshotPast`, #182), and a line to it is refused (`bot/room-ended`). The Settings page says plainly that this is a new identity, that friends and sharing stay on the old server, and that the old sign-in is kept as a backup and never deleted. Tests: `plugins/social/test/social.test.js` (`Settings server`, `server controller`, `parseServerUrl`, `probeServer`). diff --git a/docs/subsystems/workflow.md b/docs/subsystems/workflow.md index f7f75fe32..1b6221ad1 100644 --- a/docs/subsystems/workflow.md +++ b/docs/subsystems/workflow.md @@ -40,7 +40,7 @@ Everything in `plugins/workflow/`: the graph rules, the run store, the matcher, ## Entry points - Endpoint `workflow/state` and `workflow/templates` are reads. -- `workflow/create`, `workflow/rename`, `workflow/delete`, `workflow/assign`, `workflow/concurrency`, `workflow/graph`, `workflow/advance`, `workflow/reset`, `workflow/ingest` write. `workflow/concurrency` sets that workflow's `maxConcurrent` (an integer from 1 to 8); the same control is in Settings → Workflow graphs. `workflow/step-timeout` sets that workflow's `stepTimeoutMinutes` (an integer from 1 to 120, or `null` to go back to the default), and `workflow/create` and `define_workflow` accept the same field, refused outside 1..120 rather than dropped (#1868). `workflow/rename` updates a workflow's name and optional description; read-only hosts are refused. `workflow/delete` removes a workflow from state, releases its ephemeral Bots if a run exists, and cleans logs/runs/routes and every task claim that named it, so the next `ingest` of that task matches again instead of asking `assign` for a workflow that no longer exists; a load still drops a workflow claim whose workflow is gone, so a store written before this fix heals (#1632). `workflow/ingest` also takes `rematch: true`: that one task's claim and route are dropped before matching, the panel's per-task action (#1647). `workflow/assign` reads `taskId` (or `id`), `title`, `brief`, `callerBotId`, `projectId` and `botId`; any other key is named in the response's `ignored` array instead of being dropped in silence (#1869). +- `workflow/create`, `workflow/rename`, `workflow/delete`, `workflow/assign`, `workflow/concurrency`, `workflow/graph`, `workflow/advance`, `workflow/reset`, `workflow/ingest` write. `workflow/concurrency` sets that workflow's `maxConcurrent` (an integer from 1 to 8); the same control is in Settings → Workflow graphs. `workflow/step-timeout` sets that workflow's `stepTimeoutMinutes` (an integer from 1 to 120, or `null` to go back to the default), and `workflow/create` and `define_workflow` accept the same field, refused outside 1..120 rather than dropped (#1868). Every write lands through `workflows.json..tmp`, and that temp name is asked as a file first (temp-name sweep): a link left there refuses the write, with the temp name in the answer, instead of putting the workflows outside their folder and renaming the link into place. `workflow/rename` updates a workflow's name and optional description; read-only hosts are refused. `workflow/delete` removes a workflow from state, releases its ephemeral Bots if a run exists, and cleans logs/runs/routes and every task claim that named it, so the next `ingest` of that task matches again instead of asking `assign` for a workflow that no longer exists; a load still drops a workflow claim whose workflow is gone, so a store written before this fix heals (#1632). `workflow/ingest` also takes `rematch: true`: that one task's claim and route are dropped before matching, the panel's per-task action (#1647). `workflow/assign` reads `taskId` (or `id`), `title`, `brief`, `callerBotId`, `projectId` and `botId`; any other key is named in the response's `ignored` array instead of being dropped in silence (#1869). - Tool `run_workflow` starts a named run with a brief. Tool `define_workflow` writes the same graph the canvas edits, and takes the same `stepTimeoutMinutes` (1..120) so a lane created that way does not silently inherit the 45-minute default (#1868). - `flybot.tasks.onWrite` calls `ingest`. A new Bot match returns `{ ownerBotId }`. A Workflow match starts the run inside the plugin and leaves the card's owner to the host. A host that offers no `tasks.onWrite` warns once and reports `tasksWired: false` in `workflow/state`, and the Workflows panel says that a board card cannot start a run there (#1462). diff --git a/patches/ds-bot/0001-core-fixes.patch b/patches/ds-bot/0001-core-fixes.patch index a3ccdd8fe..dc23d2c3e 100644 --- a/patches/ds-bot/0001-core-fixes.patch +++ b/patches/ds-bot/0001-core-fixes.patch @@ -4854,7 +4854,7 @@ diff --git a/src/host/diagnostics.js b/src/host/diagnostics.js diff --git a/src/host/images.js b/src/host/images.js --- a/src/host/images.js +++ b/src/host/images.js -@@ -2,7 +2,7 @@ +@@ -2,10 +2,11 @@ // reaches a model that cannot read them, each image becomes text from a vision model. // Texts are kept per attachment, so later requests repeat the same words and the // prompt cache still hits. @@ -4863,6 +4863,10 @@ diff --git a/src/host/images.js b/src/host/images.js import { join } from 'node:path' import { BlockAssembler } from '@deepseek-ai/dsh-llm' import { OCR_CACHE_MAX, OCR_PROMPT, OCR_RETRY_MS, OCR_TIMEOUT_MS } from './constants.js' ++import { tempWritable } from './real-dir.js' + import { splitRef } from './text.js' + + export function offloadTargets(session, beforeSeq) { @@ -27,6 +27,56 @@ export function offloadTargets(session, beforeSeq) { return targets } @@ -4920,6 +4924,16 @@ diff --git a/src/host/images.js b/src/host/images.js export function install(rt) { const { ctx, config, home } = rt +@@ -81,6 +81,9 @@ export function install(rt) { + ocrWriting = ocrWriting.then(async () => { + await mkdir(home, { recursive: true }) + 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.`) + await writeFile(temp, snapshot, 'utf8') + await rename(temp, ocrPath) + }).catch(error => rt.warn('ds-bot: OCR cache write failed: %s', error)) @@ -128,16 +128,18 @@ export function install(rt) { return ocrPending.get(id) } @@ -10773,18 +10787,19 @@ diff --git a/test/i18n.test.js b/test/i18n.test.js diff --git a/test/images.test.js b/test/images.test.js --- a/test/images.test.js +++ b/test/images.test.js -@@ -1,6 +1,9 @@ +@@ -1,6 +1,10 @@ import assert from 'node:assert/strict' -+import { mkdirSync, mkdtempSync, rmSync, writeFileSync } from 'node:fs' ++import { existsSync, 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' -import { offloadTargets } from '../src/host/images.js' -+import { attachmentFilePath, attachmentObjectPath, imagePlaceholder, offloadTargets } from '../src/host/images.js' ++import { attachmentFilePath, attachmentObjectPath, imagePlaceholder, install, offloadTargets } from '../src/host/images.js' ++import { fakeContext } from './fake-ctx.js' test('offloadTargets selects live images from prior user and tool messages', () => { const events = [ -@@ -32,3 +32,83 @@ test('offloadTargets selects live images from prior user and tool messages', () +@@ -32,3 +32,152 @@ test('offloadTargets selects live images from prior user and tool messages', () { seq: 2, imageIndexes: [0] }, ]) }) @@ -10868,6 +10883,75 @@ diff --git a/test/images.test.js b/test/images.test.js + // An unnamed image keeps the sha prefix the host has always shown. + assert.match(imagePlaceholder({ attachmentId: id }), /\[Image sha256:bdc77c70:/) +}) ++ ++// The OCR cache lands through `/ocr.json..tmp` and is renamed into place. That temp name ++// is a leaf like the cache (temp-name sweep): `writeFile` follows a link there, so a pre-set link ++// would take the cache outside the home and the rename would put the link where `ocr.json` belongs. ++const OCR_ATTACHMENT = { attachmentId: `sha256:${'b'.repeat(64)}`, name: 'triangle.png' } ++ ++function ocrRuntime(t) { ++ const home = mkdtempSync(join(tmpdir(), 'ds-bot-images-ocr-')) ++ const warnings = [] ++ const ctx = fakeContext() ++ Object.assign(ctx, { ++ llm: { ++ listProviders: () => [{ id: 'deepseek', name: 'DeepSeek' }], ++ listModels: async () => [{ id: 'vision', input: ['text', 'image'] }], ++ resolveModelInfo: async () => ({ inputModalities: ['text', 'image'] }), ++ stream: async function* () { ++ yield { type: 'text-delta', index: 0, text: 'A red triangle.' } ++ yield { type: 'finish', reason: { kind: 'stop' } } ++ }, ++ }, ++ }) ++ const rt = { ++ ctx, ++ home, ++ config: {}, ++ warn: (...args) => warnings.push(args.join(' ')), ++ load: async () => {}, ++ readOnly: () => undefined, ++ roomOf: () => 'r1', ++ isBotSession: () => true, ++ ownRequest: () => false, ++ catalog: async () => [{ ref: 'deepseek/vision', name: 'Vision', input: ['image'] }], ++ defaultRef: () => 'deepseek/vision', ++ tagUsage: messages => messages, ++ } ++ install(rt) ++ t.after(() => rmSync(home, { recursive: true, force: true, maxRetries: 5 })) ++ return { rt, home, warnings } ++} ++ ++// The cache write is queued after the reading, so wait for the effect instead of guessing a delay. ++async function waitFor(check) { ++ for (let i = 0; i < 300; i += 1) { ++ if (await check()) return ++ await new Promise(resolve => setTimeout(resolve, 10)) ++ } ++} ++ ++test('a linked OCR cache temp name does not take the cache; ocr.json stays a plain file', async (t) => { ++ const { rt, home, warnings } = ocrRuntime(t) ++ const outside = join(home, 'outside-ocr.txt') ++ writeFileSync(outside, 'OUTSIDE\n') ++ symlinkSync(outside, join(home, `ocr.json.${process.pid}.tmp`)) ++ await rt.imagesForGroup([OCR_ATTACHMENT], 'r1') ++ await waitFor(() => readFileSync(outside, 'utf8') !== 'OUTSIDE\n' || warnings.length > 0) ++ assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') ++ const cache = join(home, 'ocr.json') ++ assert.equal(existsSync(cache) ? lstatSync(cache).isSymbolicLink() : false, false, 'the cache is not a link') ++ assert.ok(warnings.some(line => /not a plain file/.test(line)), warnings.join('\n')) ++}) ++ ++test('an ordinary OCR cache temp name is written and renamed as before', async (t) => { ++ const { rt, home, warnings } = ocrRuntime(t) ++ await rt.imagesForGroup([OCR_ATTACHMENT], 'r1') ++ const cache = join(home, 'ocr.json') ++ await waitFor(() => existsSync(cache)) ++ assert.equal(warnings.length, 0) ++ assert.equal(JSON.parse(readFileSync(cache, 'utf8'))[OCR_ATTACHMENT.attachmentId].text, 'A red triangle.') ++}) diff --git a/test/install.test.js b/test/install.test.js --- a/test/install.test.js +++ b/test/install.test.js diff --git a/patches/ds-bot/0002-flybot-api-host.patch b/patches/ds-bot/0002-flybot-api-host.patch index a9b49e4f0..e3f1ae88d 100644 --- a/patches/ds-bot/0002-flybot-api-host.patch +++ b/patches/ds-bot/0002-flybot-api-host.patch @@ -784,7 +784,7 @@ diff --git a/src/host/roster.js b/src/host/roster.js diff --git a/src/host/store.js b/src/host/store.js --- a/src/host/store.js +++ b/src/host/store.js -@@ -1,21 +1,50 @@ +@@ -1,21 +1,51 @@ // Store: the saved team (state.json), its migrations, the lock that keeps a second dsh // host on the same data from writing it, and the indexes that map a Session to the Bot // or group chat it belongs to. @@ -798,6 +798,7 @@ diff --git a/src/host/store.js b/src/host/store.js import { homeLock } from './home-lock.js' -import { currentColor, normalize, unquote } from './text.js' +import { openJournal } from './journal.js' ++import { tempWritable } from './real-dir.js' +import { codePoints, currentColor, normalize, unquote } from './text.js' + +// The saved team's format version. Version 2 (#47) adds the identity schema: member @@ -1548,7 +1549,7 @@ diff --git a/src/host/store.js b/src/host/store.js warnedReadOnly = false adopt(await readFileState()) return -@@ -235,11 +235,37 @@ export function install(rt) { +@@ -235,16 +235,45 @@ export function install(rt) { return warnReadOnly() } @@ -1586,6 +1587,14 @@ diff --git a/src/host/store.js b/src/host/store.js state.revision += 1 state.exchanges = state.exchanges.slice(-MAX_EXCHANGES) const snapshot = JSON.stringify(state, null, 2) + const run = writing.then(async () => { + 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.`) + try { + await mkdir(home, { recursive: true }) + const ensureLock = () => { @@ -260,9 +260,10 @@ export function install(rt) { ensureLock() await rename(temp, statePath) diff --git a/patches/ds-bot/0009-identity-schema.patch b/patches/ds-bot/0009-identity-schema.patch index 9610adecc..983481b99 100644 --- a/patches/ds-bot/0009-identity-schema.patch +++ b/patches/ds-bot/0009-identity-schema.patch @@ -355,7 +355,18 @@ diff --git a/test/roster.test.js b/test/roster.test.js const names = Object.values(JSON.parse(readFileSync(statePath, 'utf8')).bots).map(bot => bot.name) assert.deepEqual(names.sort(), ['Chief', 'Kept', 'Warm']) }) -@@ -526,3 +526,65 @@ test('a write that fails at save time archives the Session it made', async (t) = +@@ -519,10 +519,75 @@ test('a write that fails at save time archives the Session it made', async (t) = + ctx.workspaceRegistry.archiveSession = async (id) => { archived.push(id); return archive(id) } + const answer = await call('create-bot', { name: 'Phantom' }) + assert.equal(answer.ok, false) +- assert.equal(answer.error.code, 'bot/internal') ++ // The temp-name guard refuses before `writeFile` reaches EISDIR, so the failure is a refusal ++ // that names the temp path; the rollback this test is about is unchanged. ++ assert.equal(answer.error.code, 'bot/refused') ++ assert.match(answer.error.message, /not a plain file/) + assert.deepEqual(archived, created) + const team = (await call('state', {})).value + assert.equal(team.bots.some(bot => bot.name === 'Phantom'), false) const saved = JSON.parse(readFileSync(join(home, 'state.json'), 'utf8')) assert.equal(Object.values(saved.bots).some(bot => bot.name === 'Phantom'), false) }) diff --git a/patches/ds-bot/0014-flybot-branding.patch b/patches/ds-bot/0014-flybot-branding.patch index dbe92db85..d49ce1227 100644 --- a/patches/ds-bot/0014-flybot-branding.patch +++ b/patches/ds-bot/0014-flybot-branding.patch @@ -2028,6 +2028,15 @@ diff --git a/test/secrets.test.js b/test/secrets.test.js diff --git a/test/store.test.js b/test/store.test.js --- a/test/store.test.js +++ b/test/store.test.js +@@ -68,7 +68,7 @@ test('a failed state write rejects and leaves the saved file unchanged', async ( + const original = fs.readFileSync(statePath) + fs.mkdirSync(`${statePath}.${process.pid}.tmp`) + rt.state.prefs.marker = 'not saved' +- await assert.rejects(rt.save(), { code: 'EISDIR' }) ++ await assert.rejects(rt.save()) + assert.deepEqual(fs.readFileSync(statePath), original) + }) + @@ -139,7 +139,7 @@ test('a state from a newer version remains untouched', async (t) => { const original = JSON.stringify({ version: 999, revision: 77, recoveryMarker: 'keep' }) const { rt, statePath } = runtime(t, original) @@ -2037,7 +2046,7 @@ diff --git a/test/store.test.js b/test/store.test.js rt.state.prefs.marker = 'not saved' await rt.save() assert.equal(fs.readFileSync(statePath, 'utf8'), original) -@@ -285,3 +285,38 @@ test('dispose waits for a pending save and prevents later writes', async (t) => +@@ -285,3 +285,73 @@ test('dispose waits for a pending save and prevents later writes', async (t) => restore() } }) @@ -2076,6 +2085,41 @@ diff --git a/test/store.test.js b/test/store.test.js + assert.equal(rt.findBot('a').name, 'Chief') + assert.equal(rt.findRooms('Ops')[0].id, 'r1') +}) ++ ++// The state write lands through `.json..tmp` and then renames it into place. The temp ++// name is a leaf like the state file (temp-name sweep): `writeFile` would follow a link there, put ++// the team outside the home and then rename the link over the state file. Only the target is ++// replaced by `rename`, link included, never written through. ++test('a linked state temp name does not take the save; the state file stays a plain file', async (t) => { ++ const { rt, home, statePath, warnings } = runtime(t) ++ await rt.load() ++ const outside = join(home, 'outside-state.json') ++ fs.writeFileSync(outside, 'OUTSIDE\n') ++ fs.symlinkSync(outside, `${statePath}.${process.pid}.tmp`) ++ rt.state.prefs.sweep = 'not saved' ++ await assert.rejects(rt.save()) ++ assert.equal(fs.readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') ++ assert.equal(fs.lstatSync(statePath).isSymbolicLink(), false, 'the state file is still a plain file') ++ assert.equal(JSON.parse(fs.readFileSync(statePath, 'utf8')).prefs.sweep, undefined, 'the refused save did not land') ++ assert.ok(warnings.some(line => /not a plain file/.test(line.join(' '))), 'the refusal is named') ++}) ++ ++// The other half of the same shape: a link at the destination itself is replaced by the rename, so ++// it never needs the temp check. This is why the sweep only adds the temp name. ++test('a link at the state file itself is replaced, not written through', async (t) => { ++ const { rt, home, statePath } = runtime(t) ++ await rt.load() ++ await rt.save() ++ const outside = join(home, 'outside-state.json') ++ fs.writeFileSync(outside, 'OUTSIDE\n') ++ fs.rmSync(statePath) ++ fs.symlinkSync(outside, statePath) ++ rt.state.prefs.sweep = 'kept' ++ await rt.save() ++ assert.equal(fs.readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the rename never follows the target link') ++ assert.equal(fs.lstatSync(statePath).isSymbolicLink(), false, 'the link is replaced by the saved file') ++ assert.equal(JSON.parse(fs.readFileSync(statePath, 'utf8')).prefs.sweep, 'kept') ++}) diff --git a/test/update.test.js b/test/update.test.js --- a/test/update.test.js +++ b/test/update.test.js diff --git a/patches/ds-bot/files/src/host/automation-default.js b/patches/ds-bot/files/src/host/automation-default.js index d9e7c1a31..1d441947c 100644 --- a/patches/ds-bot/files/src/host/automation-default.js +++ b/patches/ds-bot/files/src/host/automation-default.js @@ -8,6 +8,7 @@ // A DSH that does not offer the bundle is left alone, and so is a second dsh on the same data. import { mkdir, readFile, rename, unlink, writeFile } from 'node:fs/promises' import { join } from 'node:path' +import { tempWritable } from './real-dir.js' export const SCHEDULE_BUNDLE = '@deepseek-ai/dsh-experimental-schedule-bundle' const MARKER = 'dsh-defaults.json' @@ -34,6 +35,9 @@ export async function readOffers(file) { async function writeOffers(file, offers) { await mkdir(join(file, '..'), { recursive: true }) 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.`) try { await writeFile(temp, `${JSON.stringify({ version: 1, offers }, null, 2)}\n`, 'utf8') await rename(temp, file) @@ -50,7 +54,7 @@ async function writeOffers(file, offers) { // 'unrecorded' the offer could not be written down, so nothing was changed // 'enabled' switched on // 'failed' the manager refused; the note is withdrawn so the next start tries again -export async function offerBundle({ manager, bundle = SCHEDULE_BUNDLE, profile, file, now = () => new Date() }) { +export async function offerBundle({ manager, bundle = SCHEDULE_BUNDLE, profile, file, now = () => new Date(), warn = () => {} }) { let offers try { offers = await readOffers(file) @@ -70,7 +74,8 @@ export async function offerBundle({ manager, bundle = SCHEDULE_BUNDLE, profile, } try { await note(now().toISOString()) - } catch { + } catch (error) { + warn('ds-bot: the Automation tasks offer was not written down: %s', error?.message ?? error) return 'unrecorded' } if (entry.enabled === true) return 'already-on' @@ -91,7 +96,7 @@ export function install(rt) { // The window that shows the team without owning it leaves the profile to the other one. if (rt.readOnly() !== undefined) return const profile = ctx.get?.('profileContext')?.name ?? 'default' - const outcome = await offerBundle({ manager: ctx.get?.('pluginManager'), profile, file: join(home, MARKER) }) + const outcome = await offerBundle({ manager: ctx.get?.('pluginManager'), profile, file: join(home, MARKER), warn: rt.warn }) if (outcome === 'failed') rt.warn('ds-bot: could not switch on Automation tasks; switch them on from the Plugins page') } diff --git a/patches/ds-bot/files/src/host/real-dir.js b/patches/ds-bot/files/src/host/real-dir.js index 68b9309f1..de82dad09 100644 --- a/patches/ds-bot/files/src/host/real-dir.js +++ b/patches/ds-bot/files/src/host/real-dir.js @@ -47,6 +47,14 @@ export async function fileKind(path) { } } +// A temp name a write lands through is a leaf like the file it becomes (temp-name sweep): the +// `writeFile` follows a link at the last component, so a link left at `..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' + export function fileKindSync(path) { try { return fileKindOf(lstatSync(path)) diff --git a/patches/ds-bot/files/test/automation-default.test.js b/patches/ds-bot/files/test/automation-default.test.js index e227d95da..12c4cdeb4 100644 --- a/patches/ds-bot/files/test/automation-default.test.js +++ b/patches/ds-bot/files/test/automation-default.test.js @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { existsSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { existsSync, 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' @@ -142,3 +142,22 @@ test('config.automationTasks = false never offers it', (t) => { assert.equal(ctx.calls.includes('effect ds-bot: automation tasks default'), false) assert.equal(rt.offerAutomationTasks, undefined) }) + +// The defaults file lands through `..tmp` and is renamed into place. The temp name is a +// leaf like the file (temp-name sweep): a link left there would take the offer outside the team +// folder and the rename would put the link where `dsh-defaults.json` belongs. A refused write is +// reported once, and the offer is not recorded. +test('a linked defaults temp name does not take the offer; the file is not created', async (t) => { + const file = scratch(t) + const outside = join(dirname(file), 'outside-defaults.json') + writeFileSync(outside, 'OUTSIDE\n') + symlinkSync(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 when the offer cannot be written down') + assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') + 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')) +}) diff --git a/plugins/social/src/server-config.js b/plugins/social/src/server-config.js index 23efd4e17..da1312ae5 100644 --- a/plugins/social/src/server-config.js +++ b/plugins/social/src/server-config.js @@ -8,7 +8,7 @@ // on the local Bots (parked in `flybot-social-bots..json` while another server is active). // Nothing here deletes a credentials file, and nothing here reads, returns or logs a token. import { createHash } from 'node:crypto' -import { chmodSync, mkdirSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs' +import { chmodSync, lstatSync, mkdirSync, readdirSync, readFileSync, renameSync, rmSync, writeFileSync } from 'node:fs' import { basename, dirname, join } from 'node:path' import { socialFetch } from './mcp-client.js' @@ -67,9 +67,18 @@ export async function probeServer(server, { fetch: fetchImpl = socialFetch, time return { name: cleanLine(info.name, 60) || 'Bot Social', version: cleanLine(info.version, 30) || undefined, features } } +// The temp name a private file lands through is a leaf like the file itself (temp-name sweep): a +// link left at `..tmp` would take the contents outside the team folder and the rename +// would put the link where the file belongs. Absent is ours to create, a plain leftover is +// replaceable, and anything else refuses the write before the first byte. +const tempWritable = (path) => { + try { return lstatSync(path).isFile() } catch (error) { return error?.code === 'ENOENT' } +} + const writePrivate = (path, value) => { mkdirSync(dirname(path), { recursive: true }) const temp = `${path}.${process.pid}.tmp` + if (!tempWritable(temp)) throw new Error(`Not saved: ${temp} is not a plain file, so nothing was written there.`) writeFileSync(temp, JSON.stringify(value, null, 2), { mode: 0o600 }) renameSync(temp, path) try { chmodSync(path, 0o600) } catch { /* file systems that ignore the write mode */ } diff --git a/plugins/social/test/server-config-temp.test.js b/plugins/social/test/server-config-temp.test.js new file mode 100644 index 000000000..e0d16d864 --- /dev/null +++ b/plugins/social/test/server-config-temp.test.js @@ -0,0 +1,40 @@ +// The chosen server is kept in `flybot-social-server.json`, written through +// `..tmp` and renamed into place. That temp name is a leaf like the file +// (temp-name sweep): a link left there would take the choice outside the team folder and the +// rename would put the link where the override belongs. +import assert from 'node:assert/strict' +import { existsSync, lstatSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createServerController } from '../src/server-config.js' + +const fly = () => ({ + bot: { list: () => [], get: () => undefined }, + fields: { get: () => undefined, set() {} }, + grants: { of: () => [], set: async () => {} }, + readOnly: () => false, +}) + +test('a linked server-override temp name does not take the choice; the file stays absent', async () => { + const home = mkdtempSync(join(tmpdir(), 'flybot-social-temp-')) + const controller = createServerController({ + base: { server: 'https://a.test', credentialsPath: join(home, 'flybot-social.json'), displayName: 'Me' }, + flybot: fly(), + probe: async server => ({ name: 'Bot Social', features: [], server }), + start: async () => ({ home: { handle: '@me.home' }, dispose: () => {} }), + startTimeoutMs: 20, + }) + try { + const outside = join(home, 'outside-server.json') + writeFileSync(outside, 'OUTSIDE\n') + symlinkSync(outside, join(home, `flybot-social-server.json.${process.pid}.tmp`)) + await assert.rejects(controller.set({ server: 'https://b.test', confirm: true }), /not a plain file/) + assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') + const override = join(home, 'flybot-social-server.json') + assert.equal(existsSync(override) ? lstatSync(override).isSymbolicLink() : false, false, 'the override is not a link') + } finally { + controller.dispose() + rmSync(home, { recursive: true, force: true }) + } +}) diff --git a/plugins/workflow/src/service.js b/plugins/workflow/src/service.js index b96825987..913f5cc0b 100644 --- a/plugins/workflow/src/service.js +++ b/plugins/workflow/src/service.js @@ -3,7 +3,7 @@ // (default 2). A live node stays running until its Bot replies. import { randomBytes } from 'node:crypto' -import { mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' +import { lstatSync, mkdirSync, readFileSync, renameSync, writeFileSync } from 'node:fs' import { join } from 'node:path' import { releaseAll, spawnAll } from './ephemeral.js' import { @@ -187,6 +187,14 @@ function blankRun(workflow, task, now, history = []) { } } +// The temp name the store lands through is a leaf like the store itself (temp-name sweep): a link +// left at `..tmp` would take the workflows outside their folder and the rename would put +// the link where the store belongs. Absent is ours to create, a plain leftover is replaceable, and +// anything else refuses the write before the first byte. +const tempWritable = (path) => { + try { return lstatSync(path).isFile() } catch (error) { return error?.code === 'ENOENT' } +} + export function createWorkflowService({ dir, persist = true, now = () => Date.now(), bots = () => [], readOnly = () => false, warn = () => {}, tasksWired = false, spawn, release, dispatch, // #1856: the host's Project seam. `readProject(id)` answers the record or null; `bindRun(runId, @@ -311,6 +319,7 @@ export function createWorkflowService({ state.seq = seq mkdirSync(dir, { recursive: true }) const tmp = `${file}.${process.pid}.tmp` + if (!tempWritable(tmp)) throw new Error(`Not saved: ${tmp} is not a plain file, so the workflow store was not written there.`) writeFileSync(tmp, JSON.stringify(state)) renameSync(tmp, file) } diff --git a/plugins/workflow/test/temp-name.test.js b/plugins/workflow/test/temp-name.test.js new file mode 100644 index 000000000..713a0a1c4 --- /dev/null +++ b/plugins/workflow/test/temp-name.test.js @@ -0,0 +1,28 @@ +// The workflow store lands through `/workflows.json..tmp` and is renamed into place. +// That temp name is a leaf like the store (temp-name sweep): a link left there would take the +// workflows outside their folder and the rename would put the link where the store belongs. +import assert from 'node:assert/strict' +import { existsSync, lstatSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { test } from 'node:test' +import { createWorkflowService } from '../src/service.js' +import { DELIVERY_ID } from '../src/template.js' + +test('a linked workflow-store temp name does not take the store; workflows.json stays absent', () => { + const dir = mkdtempSync(join(tmpdir(), 'flybot-wf-temp-')) + try { + const svc = createWorkflowService({ dir, persist: true, now: () => 5, bots: () => [] }) + const outside = join(dir, 'outside-workflows.json') + writeFileSync(outside, 'OUTSIDE\n') + symlinkSync(outside, join(dir, `workflows.json.${process.pid}.tmp`)) + const saved = svc.setConcurrency({ workflowId: DELIVERY_ID, maxConcurrent: 3 }) + assert.equal(saved.ok, false, 'the write is refused') + assert.match(saved.error, /not a plain file/) + assert.equal(readFileSync(outside, 'utf8'), 'OUTSIDE\n', 'the file behind the temp link is not written through') + const file = join(dir, 'workflows.json') + assert.equal(existsSync(file) ? lstatSync(file).isSymbolicLink() : false, false, 'the store is not a link') + } finally { + rmSync(dir, { recursive: true, force: true }) + } +})