diff --git a/changelog.d/2037-decision-name-guard.fixed.md b/changelog.d/2037-decision-name-guard.fixed.md new file mode 100644 index 000000000..e2e66f0f5 --- /dev/null +++ b/changelog.d/2037-decision-name-guard.fixed.md @@ -0,0 +1 @@ +A decision file name can no longer reach outside the project, and a symlinked Project State file is no longer read ([#2037](https://github.com/ThinkFlowLab/FlyBot/issues/2037)): the board accepted any name that started with `decisions/` and ended in `.md`, so a hand-edited `decisionFile` or a crafted folder name such as `decisions/zz/../../../outside.md` put a file outside the folder in a row — with its `path` and its first line as `title` — and the same name could come back through the `known` fallback past the 200-row cut. One rule now decides a decision name at the point the listing is produced: exactly one segment under `decisions/`, ending in `.md`, used by the listing, the reader and the fallback alike; a symlink is refused too, and `project_state` answers `absent` for a symlinked file instead of handing over the text it points at. diff --git a/core/src/host/project-board.js b/core/src/host/project-board.js index c6cd8e424..d031b58b4 100644 --- a/core/src/host/project-board.js +++ b/core/src/host/project-board.js @@ -16,10 +16,10 @@ // Read-only by construction: it reads `state.tasks`, `state.meetings` and the Project bindings, // asks each registered provider for the workflow refs of tasks that still exist, and returns the // sections. It writes nothing, starts nothing and is not called unless someone asks for the board. -import { open, readFile, stat } from 'node:fs/promises' +import { open } from 'node:fs/promises' import { join } from 'node:path' import { columnOf } from '../shared/task-board.js' -import { DECISIONS_DIR, listProjectState, sha256 } from './project-state.js' +import { DECISIONS_DIR, describeFile, isDecisionPath, listProjectState } from './project-state.js' import { cut } from './text.js' import { advanceRefusal } from './meetings.js' import { readMetrics } from './project-metrics.js' @@ -247,11 +247,12 @@ export function install(rt) { const archivedOf = async (meeting) => { const dir = text(meeting?.decisionDir) const path = text(meeting?.decisionFile) - if (dir === '' || !path.startsWith(`${DECISIONS_DIR}/`) || !path.endsWith('.md')) return undefined + // The listing accepts one shape only, and this read is part of it: a name that is not + // `decisions/.md`, or a symlink standing in for that file, is refused here too. Reading + // it would put a file outside the folder in the rows, and the past-cut fallback reads this set. + if (dir === '' || !isDecisionPath(path)) return undefined try { - const bytes = await readFile(join(dir, path)) - const info = await stat(join(dir, path)) - return { dir, path, bytes: bytes.length, sha256: sha256(bytes), mtime: new Date(info.mtimeMs).toISOString() } + return { dir, ...await describeFile(dir, path) } } catch { // Absent or unreadable is not a row: the meeting keeps its own record, with no path. return undefined @@ -274,7 +275,7 @@ export function install(rt) { } if (state.ok !== true) return { dir, files: [], total: 0, reason: state.reason } found.push(...state.files - .filter(file => file.path.startsWith(`${DECISIONS_DIR}/`) && file.path.endsWith('.md')) + .filter(file => isDecisionPath(file.path)) .map(file => ({ ...file, dir }))) } // The other source: the folder each decided meeting of this Project recorded, one file each. diff --git a/core/src/host/project-state.js b/core/src/host/project-state.js index 8039004fd..30bf9ab58 100644 --- a/core/src/host/project-state.js +++ b/core/src/host/project-state.js @@ -9,7 +9,7 @@ // file and a file too large to hand over each answer with the reason, never with an empty // string that would read as an empty document. import { createHash } from 'node:crypto' -import { readdir, readFile, stat } from 'node:fs/promises' +import { lstat, readdir, readFile, stat } from 'node:fs/promises' import { join } from 'node:path' import { defineTool } from './tool-schema.js' import { output } from './tools.js' @@ -22,7 +22,9 @@ export const DECISIONS_DIR = 'decisions' // the context it was meant to save. Over the bound the answer is the size, not the text. export const MAX_BYTES = 200_000 -const isDecisionPath = path => /^decisions\/[^/]+\.md$/.test(path) +// The one name shape a decision has: exactly one segment under `decisions/`, ending in `.md`. +// The lister, the reader and the board all take a name only when it matches this. +export const isDecisionPath = path => /^decisions\/[^/]+\.md$/.test(path) const allowed = path => PROJECT_STATE_PATHS.includes(path) || isDecisionPath(path) export const sha256 = bytes => createHash('sha256').update(bytes).digest('hex') @@ -48,9 +50,22 @@ async function decisionFiles(dir) { .sort() } -async function describe(dir, path) { +// One file sitting directly in the folder, never through a link. `readdir` already reports a +// symlink as "not a plain file", and reading a name — here, in the reader, and on the board — +// answers the same way: a symlink is an absent file, not its target. The `lstat` is what makes +// that one rule hold for a name that was never listed, a name from a record, or `PROJECT.md`. +async function fileAt(dir, path) { + const info = await lstat(join(dir, path)) + if (!info.isFile()) throw new Error(`not a plain file: ${path}`) + return info +} + +// One Project State file with its byte size, content hash and last-modified time. Exported +// because the board reads the file a decided meeting archived, and it must refuse exactly what +// this reader refuses — one rule, one place. +export async function describeFile(dir, path) { + const info = await fileAt(dir, path) const bytes = await readFile(join(dir, path)) - const info = await stat(join(dir, path)) return { path, bytes: bytes.length, sha256: sha256(bytes), mtime: new Date(info.mtimeMs).toISOString() } } @@ -66,12 +81,12 @@ export async function listProjectState(dir, { repository = true } = {}) { const found = [] for (const path of PROJECT_STATE_PATHS) { try { - found.push(await describe(dir, path)) + found.push(await describeFile(dir, path)) } catch { /* an absent Project State file is simply not listed */ } } for (const path of await decisionFiles(dir)) { try { - found.push(await describe(dir, path)) + found.push(await describeFile(dir, path)) } catch { /* a file that vanished between readdir and read is not listed */ } } return { ok: true, dir, files: found } @@ -88,11 +103,11 @@ export async function readProjectState(dir, file) { if (!await repositoryAt(dir)) return { ok: false, reason: `not a repository: ${dir}` } let info try { - info = await stat(join(dir, path)) + // A symlink answers like a file that is not there — never with the text it points at. + info = await fileAt(dir, path) } catch { return { ok: false, reason: `absent: ${path}` } } - if (!info.isFile()) return { ok: false, reason: `absent: ${path}` } if (info.size > MAX_BYTES) return { ok: false, reason: `too large: ${path} is ${info.size} bytes (bound ${MAX_BYTES})` } const bytes = await readFile(join(dir, path)) return { ok: true, path, bytes: bytes.length, sha256: sha256(bytes), mtime: new Date(info.mtimeMs).toISOString(), text: bytes.toString('utf8') } diff --git a/core/test/project-board.test.js b/core/test/project-board.test.js index b1ee23194..c64348ed1 100644 --- a/core/test/project-board.test.js +++ b/core/test/project-board.test.js @@ -2,7 +2,7 @@ // that same id through `run.task.id`. The aggregate reads both views as one row, never twice, // and writes nothing back. import assert from 'node:assert/strict' -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { test } from 'node:test' @@ -557,6 +557,127 @@ test('#2033: a decision name that points outside the folder never becomes a path } }) +// A hand-edited record, or a name crafted into a decisions folder, can point `decisionFile` +// anywhere. The board lists one shape only — a regular `decisions/.md` — and that same rule +// has to refuse every other name where the listing is produced, so neither a row nor the past-cut +// fallback reads outside the folder. Each class below is a refusal, never a silent skip. +const traversalFolder = () => { + const base = mkdtempSync(join(tmpdir(), 'ds-bot-decisions-escape-')) + const dir = join(base, 'project') + mkdirSync(join(dir, '.git'), { recursive: true }) + mkdirSync(join(dir, 'decisions')) + mkdirSync(join(dir, 'decisions', 'zz')) + mkdirSync(join(dir, 'decisions', 'sub')) + writeFileSync(join(dir, 'decisions', '2026-01-02-legit.md'), '# The real decision\n') + writeFileSync(join(dir, 'decisions', 'sub', '2026-01-02-deep.md'), '# Left out of the listing\n') + writeFileSync(join(dir, 'decisions', 'handwritten-notes.md'), '# Written by hand\n') + writeFileSync(join(base, 'outside.md'), '# Outside the folder\n') + symlinkSync(join(base, 'outside.md'), join(dir, 'decisions', '2026-01-02-link.md')) + return { base, dir } +} + +const traversalSeed = (dir, base, ids) => { + const team = seed() + team.projects['project-p1'].workDir = dir + const at = (id, decisionFile) => ({ + id, status: 'decided', question: `Q ${id}`, owner: 'Chief', project: 'project-p1', + decisionDir: dir, decisionFile, createdAt: 1, updatedAt: 5, decidedAt: 6, + }) + const all = { + 'm-dotdot': 'decisions/zz/../../../outside.md', + 'm-absolute-out': join(base, 'outside.md'), + 'm-absolute-root': '/etc/passwd', + 'm-symlink': 'decisions/2026-01-02-link.md', + 'm-deep': 'decisions/sub/2026-01-02-deep.md', + 'm-legit': 'decisions/2026-01-02-legit.md', + } + team.meetings = Object.fromEntries(ids.map(id => [id, at(id, all[id])])) + return team +} + +// Each case gets only its own meetings, so a leak in one class cannot make another class red. +async function startTraversal(ids) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-board-')) + const { base, dir } = traversalFolder() + const { rt, stop } = await start(home, traversalSeed(dir, base, ids)) + return { rt, base, dir, stop } +} + +const decidedRow = (board, id) => board.meetings.decided.find(meeting => meeting.id === id) + +test('a decision name with a .. segment never becomes a row or a past-cut path', async () => { + const { rt, base, stop } = await startTraversal(['m-dotdot']) + try { + const board = await rt.projectBoard('project-p1') + assert.equal(decidedRow(board, 'm-dotdot').path, '', 'a name with a .. segment is refused, not projected') + assert.equal(board.decisions.files.some(file => file.path.includes('..')), false, 'no row names a .. path') + assert.equal(board.decisions.files.some(file => file.title === 'Outside the folder'), false, 'no row carries the outside title') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('an absolute decision name is refused, never read', async () => { + const { rt, base, stop } = await startTraversal(['m-absolute-out', 'm-absolute-root']) + try { + const board = await rt.projectBoard('project-p1') + assert.equal(decidedRow(board, 'm-absolute-out').path, '', 'an absolute path is refused') + assert.equal(decidedRow(board, 'm-absolute-root').path, '', 'and so is a root absolute path') + assert.equal('sha256' in decidedRow(board, 'm-absolute-out'), false) + assert.equal(board.decisions.files.some(file => file.path.startsWith('/')), false, 'no row names an absolute path') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('a symlinked decision name is refused, never followed', async () => { + const { rt, base, stop } = await startTraversal(['m-symlink']) + try { + const board = await rt.projectBoard('project-p1') + const row = decidedRow(board, 'm-symlink') + assert.equal(row.path, '', 'a symlink is not a decision file this listing found') + assert.equal('sha256' in row, false, 'and it carries no hash of the file it points at') + assert.equal(board.decisions.files.some(file => file.path === 'decisions/2026-01-02-link.md'), false, 'the link is not a row') + assert.equal(board.decisions.files.some(file => file.title === 'Outside the folder'), false, 'and its target is never titled') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('a decision name this listing did not read never becomes a past-cut path', async () => { + const { rt, base, stop } = await startTraversal(['m-deep']) + try { + const board = await rt.projectBoard('project-p1') + const row = decidedRow(board, 'm-deep') + assert.equal(row.path, '', 'the fallback names only what this read listed, so a name it never read is not a path') + assert.equal('sha256' in row, false) + assert.equal(board.decisions.files.some(file => file.path === 'decisions/sub/2026-01-02-deep.md'), false, 'the listing reads one level only') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('a regular decision name is still listed, titled and hashed', async () => { + const { rt, base, stop } = await startTraversal(['m-legit']) + try { + const board = await rt.projectBoard('project-p1') + const file = board.decisions.files.find(row => row.path === 'decisions/2026-01-02-legit.md') + assert.equal(file.title, 'The real decision', 'a legal name is still listed with its own title') + const handwritten = board.decisions.files.find(row => row.path === 'decisions/handwritten-notes.md') + assert.equal(handwritten.title, 'Written by hand', 'a legal name with no date is listed too: the rule is the shape, not the date') + const row = decidedRow(board, 'm-legit') + assert.equal(row.path, 'decisions/2026-01-02-legit.md', 'and the meeting still finds it') + assert.match(row.sha256, /^[0-9a-f]{64}$/) + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + // #2005: a decision file's title is its first heading, bounded at 200 units. A heading whose 200th // unit is the first half of an emoji used to reach the board as an unpaired surrogate, so the // Decisions section carried half a character. diff --git a/core/test/project-state.test.js b/core/test/project-state.test.js index 4f2d080fc..05a354e09 100644 --- a/core/test/project-state.test.js +++ b/core/test/project-state.test.js @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { test } from 'node:test' @@ -93,6 +93,32 @@ test('#1702: read one returns its bytes, bounded, and reports a missing file', a assert.match(bigRow, /\t200001 bytes\t/, 'a file over the read bound still lists with its size') }) +// A symlink is a name for a file somewhere else, and this reader answers for one folder. A name +// that stands for a link is therefore absent — the same answer as a file that is not there — and +// never the target's text. `readdir` already leaves a link out of the listing, and the read has to +// agree with it; a name from a record can reach the same file without ever being listed. +test('#2037: a symlinked Project State file is not followed, it answers as absent', async (t) => { + const dir = projectStateFolder() + const outside = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-outside-')) + writeFileSync(join(outside, 'secret.md'), '# Outside secret\n') + const { tool, stop } = await boot({ dir }) + t.after(async () => { + await stop() + rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) + rmSync(outside, { recursive: true, force: true, maxRetries: 5 }) + }) + + symlinkSync(join(outside, 'secret.md'), join(dir, 'decisions/0002-link.md')) + assert.equal(await tool({ file: 'decisions/0002-link.md' }), 'absent: decisions/0002-link.md', 'a link answers like a missing file, never with the text it points at') + + rmSync(join(dir, 'PROJECT.md')) + symlinkSync(join(outside, 'secret.md'), join(dir, 'PROJECT.md')) + assert.equal(await tool({ file: 'PROJECT.md' }), 'absent: PROJECT.md', 'the same answer for a named Project State file') + const lines = (await tool({})).split('\n') + assert.equal(lines.some(line => line.startsWith('PROJECT.md\t')), false, 'a link is not listed either') + assert.equal(lines.some(line => line.startsWith('decisions/0002-link.md\t')), false, 'and neither is a linked decision') +}) + test('#1702: a folder with no repository answers not a repository', async (t) => { const dir = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-nogit-')) writeFileSync(join(dir, 'PROJECT.md'), '# Project\n') diff --git a/docs/subsystems/core-host.md b/docs/subsystems/core-host.md index e03ee0740..778e2a61b 100644 --- a/docs/subsystems/core-host.md +++ b/docs/subsystems/core-host.md @@ -50,7 +50,7 @@ The server side of the patched `ds-bot` core (`core/`): the part of [ds-bot-core | `core/src/host/experience.js` | Experience library read API (#874 Step 2). `lookupExperience({ repo, techStack, query, root })` reads `core/dev/experience/`. Data and `check.mjs` stay under `core/dev/` (see [ds-bot-core](ds-bot-core.md)). Since #1899 the 600-character bound on `trigger`, `wrong` and `right` counts code points through `codePoints` (`text.js`), so an emoji costs one character, not two, and an entry written before this change whose text is over 600 UTF-16 units but within 600 characters still loads through the read path. | | `core/src/host/usage.js` | Usage ledger at `$DSH_HOME/bot/usage.sqlite` (`calls`). Ledger v3 (#903): every row carries `harness`, `issue_pr`, `tool_calls`, `wall_ms`, plus existing token columns (`withUsageFields` / `missingUsageFields`). Session turns are metered via `foldCalls` over turn events (`tool/call` count, wall time from attempt start, `issuePr` from turn text or `rt.noteUsageIssuePr(sessionId, ref)`), not from llm/stream alone. A triage tier is stamped with `rt.noteUsageHarness(sessionId, idOrLabels)` and lands on the row's `harness` field; the resume cursor (`marks.fold`) stays provider / model / retry. Sample row fixture: `core/test/fixtures/usage-sample.sqlite`. Tests: `core/test/usage-fields.test.js`, `core/test/usage.test.js`. | | `core/src/host/tool-schema.js` | The tool definitions the model sees: its `defineTool` wraps DSH's and completes the compiled schema with `explicitObjectShape`, which fills `properties: {}` and `required: []` on every object node (recursing through `properties`, `items`, `oneOf`/`anyOf`/`allOf`, `$defs`, …). A strict OpenAI-compatible gateway reads an absent member as null and rejects the whole request (`null is not of type "array"`), so one argument-less tool would break every turn; argument validation keeps using DSH's schema unchanged. Every host module registers tools through this wrapper. Tests: `core/test/tool-schema.test.js`. | -| `core/src/host/project-state.js` | The `project_state` tool (#1702, #1550 Stage 2.4): the Project State files of the Session working folder (`PROJECT.md`, `ROADMAP.md`, `docs/governance.md`, `decisions/*.md`, `LESSONS.md`), listed with byte size, sha256 content hash and last-modified time, or one file handed over whole up to 200,000 bytes. A caller compares the hash with the copy it already had, so a stale copy is visible instead of assumed. Answers `absent`, `too large` (with the size) or `not a repository` rather than an empty document, refuses any path outside that list, and reads nothing at install time: no prompt, system message or Session context is written by a call. Tests: `core/test/project-state.test.js`. | +| `core/src/host/project-state.js` | The `project_state` tool (#1702, #1550 Stage 2.4): the Project State files of the Session working folder (`PROJECT.md`, `ROADMAP.md`, `docs/governance.md`, `decisions/*.md`, `LESSONS.md`), listed with byte size, sha256 content hash and last-modified time, or one file handed over whole up to 200,000 bytes. A caller compares the hash with the copy it already had, so a stale copy is visible instead of assumed. Answers `absent`, `too large` (with the size) or `not a repository` rather than an empty document, refuses any path outside that list, and reads nothing at install time, no prompt, system message or Session context is written by a call. Since #2037 a decision name has exactly one shape — one path segment under `decisions/`, ending in `.md` — and `isDecisionPath` is the one rule for it: the listing, this reader and the board take a name only when it matches, and both this reader and the board open a file through `describeFile`, which `lstat`s the name and refuses anything that is not a plain file, so a symlink answers `absent` — the shape a missing file already has — instead of the text it points at ([#2037](https://github.com/ThinkFlowLab/FlyBot/issues/2037)). Tests: `core/test/project-state.test.js`. | | `core/src/host/project-assistant.js` | The Project Assistant (#1712, #1550 Stage 2.5): one hidden Bot and Session per Project, created by `project-create` through the roster and refusing an unknown or archived id, with exactly one 09:00 Asia/Shanghai task on that Session through the host's own `schedule-create` path and removed with the Project by `project-delete` — never a cron inside the plugin, and no backfill for a Project made before this slice. Memory lives in `state.projectMemory[id]` (`{decisions, questions, digest}`, behind `rt.projectMemoryOf` / `appendProjectMemory` / `setProjectDigest` and the `project-memory` / `project-memory-write` endpoints) keyed by Project id, so one Project cannot read another's entries. Since #1847 that bound is enforced twice, from one value: `MAX_ENTRY` (600, in `constants.js`) is checked by the assistant writer and again by `appendProjectMemory`, the one write door, and since #1886 both count code points (`codePoints` in `src/host/text.js`, one helper since #1895), not UTF-16 units, so a 600-character emoji note is stored rather than refused as 1,200 units; the endpoint refuses an empty entry (`A Project memory entry needs text`) and one over the cap (`A Project memory entry is at most 600 characters`) instead of storing a blank row or cutting the text, and a refusal leaves the table and `project.updatedAt` untouched; the panel quotes either sentence, in Chinese under a zh locale (`core/src/client/i18n.js`). `buildDigest` answers, in order, what merged since the previous digest, what closed, what is still in progress and on whom, and the next slice, using only board rows (`source`/`column`/`assignee`/`updatedAt`) and the Project State file list; a day with no change is one line, one day has one note and one `deliver()`, and the second build answers `skipped`. Identity is the install's GitHub connector account (the owner's own on a single-person install): with no connector signed in, or on a multi-member host whose `projects.ownerUserId` is not this host's own user, `digest` refuses and escalates instead of posting as the shared app, and every write carries `Written-by: Project Assistant ()` and `Posted-for: `. Since #1733 the memory and the digest are per (Project, owner) — `state.projectMemory[projectId].owners[ownerId]` with the primary owner mirrored on the row, behind `rt.projectMemoryFor(id, ownerId)`, `appendProjectMemory(id, kind, entry)`, `setProjectDigest(id, note, ownerId)` — and one `projectBoard` read plus one `listProjectState` read serve one render per owner, so the facts are shared and the personalisation is not; a read outside `ownersOf(project)` is refused naming that boundary. Since #1846 the digest reads the Project's own board, not one member's page: `rt.projectBoard(id, null, { internal: true })` is the one opt-in that skips the per-reader `taskVisible` rule, and the channel builds its options from the zone alone, so no request body can ask for it. Tests: `core/test/project-assistant.test.js`, `core/test/project-digest-authz.test.js`. | | `core/src/host/project-chief.js` | The Project Chief (#1859): every Project gets its own Chief, created with the Project by `project-create` — never the user's main Chief acting part-time. `rt.projectChief` has the same shape as `rt.projectAssistant` (`create(projectId)`, `chiefOf(projectId)`, `remove(projectId)`), the record carries `chief: { botId, sessionId, createdAt }` beside `assistant`, and the spawned Bot is named `Project Chief ()`, bound with `bindProject` so it carries `projectId`, and given instructions that name that one Project and forbid answering for any other (another Project's name is never quoted). A host without the seam logs once and answers `undefined`, so `project-create` still creates the Project with `chief: { error }`; `project-delete` retires the Chief through `deleteBot` and clears its binding. Tests: `core/test/projects.test.js`. | | `core/src/host/project-team.js` | The Project's team dossier (#1864), read-only. `rt.projectTeam(projectId)` answers `{ rows }` in the board's order — the Project's Chief, its assistant, then `roles.maintainers` / `roles.members` — with each row `{ botId, name, role, persona, model, color, createdAt, memory: { head, topics }, skills, rooms }`. `persona` is the Bot's `instructions`; `memory.head` is its own `MEMORY.md` (read through `rt.memoryDigest` in `memory.js`, which also supplies the topic names and never a topic body); `skills` is the attached capabilities the library records as `skill` (a pack contributes its skill children); `rooms` is only the rooms this Project owns. Every string and list is capped (`DOSSIER_LIMITS`: persona and memory head 4000, 64 topics, 64 skills, 32 rooms), a deleted Bot is a `null` row instead of an error for the whole answer, and an unknown Project answers `{ rows: [] }`. The endpoint sits beside `project-read` in `authz.js`'s `TASK_ENDPOINTS`, per the owner decision that persona and memory are shared by the Project's members. Tests: `core/test/project-team.test.js`. | @@ -211,7 +211,7 @@ An admin's `@` used to be the room's control plane: every `@Dev #1234 rebase and ### Team state and access -- The saved team (`state.json`) is versioned. Version 2 (#47) carries the identity schema — `members` records, a per-Bot `ownerId`, the owner's `designatedChiefId`. A legacy version-1 file is migrated on load with a one-time `state.json.v1` backup; an existing backup counts only when it is a readable file holding a parseable version-1 state, and anything else (or any malformed ownership) fails closed (read-only). An older binary refuses a newer version read-only. This is schema foundation only — team mode stays disabled, see [team](team.md). On top of it, a durable **Project** record (#1691): `state.projects` holds `{id, name, repository, defaultBranch, ownerUserId, createdAt, updatedAt, status}`, `bot.projectId` / `room.projectId` and `state.projectRuns[runId]` bind a member (a group, a Bot or a workflow run) to at most one Project, and the `api/bot` endpoints `projects`, `project-create`, `project-read`, `project-bind`, `project-of` and `project-board` sit in the task allow list. An unbound member resolves `null`, so an install with zero Projects behaves exactly as before. The read-only board over those bindings is `project-board` (#1696): Project id in, `{ tasks: [...] }` out, one row per core task id with `source` (`bot`/`group`/`workflow`), `column`, `assignee`, `updatedAt` and `runId`/`nodeId` when a run holds it. `core/src/host/project-board.js` reads `state.tasks` and the bindings and asks the registered providers through `rt.projectRunRefs` (`flybot.tasks.onProjectProvider`); it never reads `workflows.json` and writes nothing. Since #2021 the board's record halves say what their caps left out and cut the right end of the list: the decision read answers `total` — the files left after the one-row-per-path dedup, before the cut — and takes `.slice(-DECISIONS_MAX)` over the still-ascending sort, so a Project past 200 decisions keeps the newest instead of dropping the ones just written and the order on screen does not change, while the meetings read answers `decidedTotal`/`pendingTotal` per list beside the 100 rows it already kept newest; a list at or under its cap answers the same rows as before, plus the count ([#2021](https://github.com/ThinkFlowLab/FlyBot/issues/2021)). Since #2033 the lookup a decided meeting uses for its own file falls back to the un-truncated dedupe set of that same read when the name is not among the kept rows, so a file that fell off the oldest end of the cut is still named by the meeting (`path` set, and no `sha256`, because only the kept rows carry one) while a name the folder no longer holds still answers `path: ''`; the fallback reads names from that dedupe set and never asks the filesystem, so a crafted `../outside.md` yields no path either ([#2033](https://github.com/ThinkFlowLab/FlyBot/issues/2033)). Since [#2029](https://github.com/ThinkFlowLab/FlyBot/issues/2029) a blocked card also carries the age of its own question: `blockedOf(task, now)` answers `since` (the `inputRequest.requestedAt` stamp `tasks.js` wrote, preserved across a re-ask) and `waitedMs` (`now - since`, the same shape `waitingOf` answers), both `null` when the ask wrote no stamp — never derived from `createdAt`/`updatedAt` — and `buildQueues` sorts `blocked` oldest-first with the comparator `waiting.sort` already uses, so an undated card sorts last; the Waiting group's own output is unchanged. Tests: `core/test/project-board.test.js`. On top of the record and the board sits the **Project Assistant** (#1712, Stage 2.5): `project-delete` removes the Project, its bindings and its memory; the assistant's digest is one stored note plus one `deliver()` to the owner, idempotent per Asia/Shanghai day, and its identity is the connector account and never the shared app — no connector signed in, or a multi-member host whose Project owner is not this host's own user, refuses and escalates with nothing written. Per-owner memory (#1733) keys those three fields by `(Project, owner)` under `projectMemory[id].owners`, mirrors the primary owner onto the row itself so a single-owner Project reads as before, and migrates a pre-#1733 record onto its primary owner on the first write instead of replacing it; the boundary a memory read is checked against is the Project's own owner list, so an owner asking for another owner's entries is refused naming that list, while the assistant's own render walks every owner. A card needs an owner's decision when the Bot that owns the card belongs to that owner, never from the board's `assignee` (a Bot name or null); the ask names the card's own title, at most `MAX_ACTIONABLE` (5) per owner. A quiet window on the assistant record holds a due digest and releases it once, labelled, when the window ends; a window covering 09:00 delivers nothing that day; an owner's opt-out stops generation and pauses the shared 09:00 task once every owner is off, and a re-enable resumes it; delivery goes to each owner's own Session and falls back to the main Bot only for the primary owner. A held digest is released by the next `digest()` call, not by the 09:00 task, which does not make one after a window ends. On top of it, member auth (#48) adds host module `src/host/member-auth.js`, installed after `store` in `MODULES` (`core/index.js`): `invitations` and `authSessions` maps live on state (no `STATE_VERSION` bump), the channel resolves the actor from the `x-flybot-member-session` header, and endpoints are `auth-invite` / `auth-redeem` / `auth-revoke` / `auth-whoami` / `auth-remove-member` / `auth-revoke-invite` with `body.actorId` ignored; `auth-invite` alone also opens for an anonymous caller on a host with no sessions yet (bootstrap first invite), `auth-revoke-invite` kills an unredeemed invitation (redeeming it afterwards fails with `revoked`), `removeMember` refuses a member that still owns a Bot (`has_bots`), and the auth mutations are serialized behind an in-process lock so concurrent `removeMember` calls cannot both pass the last-owner check. Sign-in surface only — still not team mode; details in [team](team.md). On top of it, owner-scoped access (#49) adds host module `src/host/authz.js`, installed after member-auth: when any `authSessions` exist, the channel `authorize()` default-denies cross-owner Bot/room/session access, blocks shared non-isolatable resources (secrets, connectors, folder picks, browser bridge, update-install, diagnostics, set-prefs, `add-agent` — the DSH Agent is one per deployment and carries no owner) with `bot/blocked-shared` when more than one member exists, stamps `create-bot` with `actor.memberId`, and filters `view()`/`live()` to the actor; with no sessions the single-human behavior is unchanged. Session targets resolve through the store's owner maps — a Bot id via `botOf`, current chats and group Sessions and earlier parts via `selfOf`/`groupOwners`/`pastOwners` (a shared room Session also resolves for its current participants, #50) — and unresolvable ids deny. `approval-answer` gates on the approval's own Bot or room (resolved by approval id), never on caller-supplied session fields. `flybot/*` plugin endpoints (src/host/hooks.js) reach any signed-in actor for reads — except a Bot Social read naming a `body.botId` the actor does not own, which `bot/forbidden` refuses before the plugin handler registers that Bot's identity or syncs its grants (#1788, an owner-decided tightening); plugin writes follow the shared rule. `view()`/`live()` narrowing (`rt.ownedView`/`rt.ownedLive` in authz.js) keeps per-session entries (`questions`, `exchangeTurns`) the actor owns, and deployment-wide surfaces (agents, secrets, connectors, memory reviews) only while a single member exists — the same rule as the shared writes; rooms the actor belongs to and the Bots in them (#50) stay visible. `create-room` refuses a request with neither a member Bot nor any participant. On top of it, room membership (#50) adds host module `src/host/room-membership.js`, installed after groups: a room may carry typed `participants` (`human` / local `bot` / granted remote `bot` with `handle` + owner fields, roles `admin` / `member`, `joinedSeq` / `leftSeq`); log lines carry `authorId` + `authorKind`; history is the membership window (current members and the owner of an invited Bot see `seq >=` their interval; leave withdraws access; rejoin starts a new interval); local cross-owner rooms accept each side's *granted* Bots (#90: default deny, Chief included); a friend's Bot from `social/remote-bots` (`cleanRemote`) may be added and ungranted Bots stay private, and a room with a friend's Bot holds only granted local Bots (a Bot that joined by room code, `viaCode`, only for that room); `view()` labels remotes with an owner badge and shows an invited owner **Shared room from \**. Face-to-face rooms (#83) reuse the same `participants` / membership-window / owner-badge presentation with `origin: 'face-to-face'`: room-only remotes may sit in the room without a person-grant or a friendship, `presentRoom` treats the viewer as a member (not Shared room from), leave skips the two-Bot floor, and `fanoutRoomLine` goes to `rt.faceRoomFanout` instead of friend `flybotSend`. Still not team mode — see [team](team.md). Per-Bot grants (#90) add host module `src/host/reach.js`, installed after room-membership: `state.grants[botId]` is the list of independent grants (one Bot → one grantee, optionally room-scoped); default deny, Chief included; a Bot whose owner is not a member has none. Grants are set with `rt.setGrants`, exposed as `BotView.granted`, `flybot.bot.hasGrant` / `hasAnyGrant`, `flybot.grants` and the `bot.grants` event. `reach()` is the owner policy: a friend's Bot may reach only local Bots granted to that sender, only a granted Bot (or one code-joined in that room) may message a friend's Bot, another owner's Bot is reached only if both sides have matching grants or both are members of the room the message belongs to. It is not a middleware: hooks.js runs it before the `bot.send` chain, again on approval release, and on every deliver with a sender or remote origin (resumed, retried, deferred, re-sent), so no verdict or `bypass` can skip it and a queued message is judged by what is true when it is delivered. Revoking a grant, or replacing its Chief, removes it from rooms that have friends' Bots or several owners at once (`rt.evictUnshared`). Approval binding (#52) completes the held-send half of this: `state.flybot.approvals` records carry `requestId`, a sha256 `version` over the request content, server-derived `ownerId` and `origin` (`via`/`hop`/audience) — claims in the message (`meta`, text) never confer authority. `flybot/decide` is allowed through authz for any signed-in actor and the per-record check happens in hooks (`decide` requires the sender Bot's owner's session, `bot/forbidden` otherwise, `bot/unauthorized` without a session); `release()` revalidates before dispatch and marks the record `stale` when the request changed, the sender or target Bot is gone (also invalidated eagerly on `deleteBot`), or the Bot changed owners; a demoted Chief's sender-side holds are invalidated eagerly on `setMain` (not inside `staleReason`), and `release()` re-checks immediately before dispatch so an in-flight approve cannot go out after a concurrent invalidation. Unknown `invalidateFor` field values throw. `flybot/approvals` lists only the actor's own holds while enforcement is on; decided stale records keep their attribution in state but appear in no member's list. Which statuses still wait for that decision is one rule in `src/shared/approval-waiting.js` (#754): `decide()`, `heldOf()` in the chat view, the in-flight filter that invalidates holds, `PENDING_MAX` and the `APPROVALS_KEPT` cleanup all ask `isApprovalWaiting()`, and `channel.js` counts a row's waiting replies with it, so a `pending_approval` hold (Bot Social's own spelling) is decidable, drawn as waiting and never dropped mid-wait. +- The saved team (`state.json`) is versioned. Version 2 (#47) carries the identity schema — `members` records, a per-Bot `ownerId`, the owner's `designatedChiefId`. A legacy version-1 file is migrated on load with a one-time `state.json.v1` backup; an existing backup counts only when it is a readable file holding a parseable version-1 state, and anything else (or any malformed ownership) fails closed (read-only). An older binary refuses a newer version read-only. This is schema foundation only — team mode stays disabled, see [team](team.md). On top of it, a durable **Project** record (#1691): `state.projects` holds `{id, name, repository, defaultBranch, ownerUserId, createdAt, updatedAt, status}`, `bot.projectId` / `room.projectId` and `state.projectRuns[runId]` bind a member (a group, a Bot or a workflow run) to at most one Project, and the `api/bot` endpoints `projects`, `project-create`, `project-read`, `project-bind`, `project-of` and `project-board` sit in the task allow list. An unbound member resolves `null`, so an install with zero Projects behaves exactly as before. The read-only board over those bindings is `project-board` (#1696): Project id in, `{ tasks: [...] }` out, one row per core task id with `source` (`bot`/`group`/`workflow`), `column`, `assignee`, `updatedAt` and `runId`/`nodeId` when a run holds it. `core/src/host/project-board.js` reads `state.tasks` and the bindings and asks the registered providers through `rt.projectRunRefs` (`flybot.tasks.onProjectProvider`); it never reads `workflows.json` and writes nothing. Since #2021 the board's record halves say what their caps left out and cut the right end of the list: the decision read answers `total` — the files left after the one-row-per-path dedup, before the cut — and takes `.slice(-DECISIONS_MAX)` over the still-ascending sort, so a Project past 200 decisions keeps the newest instead of dropping the ones just written and the order on screen does not change, while the meetings read answers `decidedTotal`/`pendingTotal` per list beside the 100 rows it already kept newest; a list at or under its cap answers the same rows as before, plus the count ([#2021](https://github.com/ThinkFlowLab/FlyBot/issues/2021)). Since #2033 the lookup a decided meeting uses for its own file falls back to the un-truncated dedupe set of that same read when the name is not among the kept rows, so a file that fell off the oldest end of the cut is still named by the meeting (`path` set, and no `sha256`, because only the kept rows carry one) while a name the folder no longer holds still answers `path: ''`; the fallback reads names from that dedupe set and never asks the filesystem, so a crafted `../outside.md` yields no path either ([#2033](https://github.com/ThinkFlowLab/FlyBot/issues/2033)). Since #2037 the name is also checked where the listing is produced, by the same `isDecisionPath` the reader uses: a name with a `..` segment, an absolute path, a symlink standing in for the file, or a name this read never listed yields no row and no meeting `path` (and no `sha256`), so the fallback past the cut cannot bring one back — `archivedOf` reads through `describeFile`, the reader's own `lstat`, never `stat` ([#2037](https://github.com/ThinkFlowLab/FlyBot/issues/2037)). Since [#2029](https://github.com/ThinkFlowLab/FlyBot/issues/2029) a blocked card also carries the age of its own question: `blockedOf(task, now)` answers `since` (the `inputRequest.requestedAt` stamp `tasks.js` wrote, preserved across a re-ask) and `waitedMs` (`now - since`, the same shape `waitingOf` answers), both `null` when the ask wrote no stamp — never derived from `createdAt`/`updatedAt` — and `buildQueues` sorts `blocked` oldest-first with the comparator `waiting.sort` already uses, so an undated card sorts last; the Waiting group's own output is unchanged. Tests: `core/test/project-board.test.js`. On top of the record and the board sits the **Project Assistant** (#1712, Stage 2.5): `project-delete` removes the Project, its bindings and its memory; the assistant's digest is one stored note plus one `deliver()` to the owner, idempotent per Asia/Shanghai day, and its identity is the connector account and never the shared app — no connector signed in, or a multi-member host whose Project owner is not this host's own user, refuses and escalates with nothing written. Per-owner memory (#1733) keys those three fields by `(Project, owner)` under `projectMemory[id].owners`, mirrors the primary owner onto the row itself so a single-owner Project reads as before, and migrates a pre-#1733 record onto its primary owner on the first write instead of replacing it; the boundary a memory read is checked against is the Project's own owner list, so an owner asking for another owner's entries is refused naming that list, while the assistant's own render walks every owner. A card needs an owner's decision when the Bot that owns the card belongs to that owner, never from the board's `assignee` (a Bot name or null); the ask names the card's own title, at most `MAX_ACTIONABLE` (5) per owner. A quiet window on the assistant record holds a due digest and releases it once, labelled, when the window ends; a window covering 09:00 delivers nothing that day; an owner's opt-out stops generation and pauses the shared 09:00 task once every owner is off, and a re-enable resumes it; delivery goes to each owner's own Session and falls back to the main Bot only for the primary owner. A held digest is released by the next `digest()` call, not by the 09:00 task, which does not make one after a window ends. On top of it, member auth (#48) adds host module `src/host/member-auth.js`, installed after `store` in `MODULES` (`core/index.js`): `invitations` and `authSessions` maps live on state (no `STATE_VERSION` bump), the channel resolves the actor from the `x-flybot-member-session` header, and endpoints are `auth-invite` / `auth-redeem` / `auth-revoke` / `auth-whoami` / `auth-remove-member` / `auth-revoke-invite` with `body.actorId` ignored; `auth-invite` alone also opens for an anonymous caller on a host with no sessions yet (bootstrap first invite), `auth-revoke-invite` kills an unredeemed invitation (redeeming it afterwards fails with `revoked`), `removeMember` refuses a member that still owns a Bot (`has_bots`), and the auth mutations are serialized behind an in-process lock so concurrent `removeMember` calls cannot both pass the last-owner check. Sign-in surface only — still not team mode; details in [team](team.md). On top of it, owner-scoped access (#49) adds host module `src/host/authz.js`, installed after member-auth: when any `authSessions` exist, the channel `authorize()` default-denies cross-owner Bot/room/session access, blocks shared non-isolatable resources (secrets, connectors, folder picks, browser bridge, update-install, diagnostics, set-prefs, `add-agent` — the DSH Agent is one per deployment and carries no owner) with `bot/blocked-shared` when more than one member exists, stamps `create-bot` with `actor.memberId`, and filters `view()`/`live()` to the actor; with no sessions the single-human behavior is unchanged. Session targets resolve through the store's owner maps — a Bot id via `botOf`, current chats and group Sessions and earlier parts via `selfOf`/`groupOwners`/`pastOwners` (a shared room Session also resolves for its current participants, #50) — and unresolvable ids deny. `approval-answer` gates on the approval's own Bot or room (resolved by approval id), never on caller-supplied session fields. `flybot/*` plugin endpoints (src/host/hooks.js) reach any signed-in actor for reads — except a Bot Social read naming a `body.botId` the actor does not own, which `bot/forbidden` refuses before the plugin handler registers that Bot's identity or syncs its grants (#1788, an owner-decided tightening); plugin writes follow the shared rule. `view()`/`live()` narrowing (`rt.ownedView`/`rt.ownedLive` in authz.js) keeps per-session entries (`questions`, `exchangeTurns`) the actor owns, and deployment-wide surfaces (agents, secrets, connectors, memory reviews) only while a single member exists — the same rule as the shared writes; rooms the actor belongs to and the Bots in them (#50) stay visible. `create-room` refuses a request with neither a member Bot nor any participant. On top of it, room membership (#50) adds host module `src/host/room-membership.js`, installed after groups: a room may carry typed `participants` (`human` / local `bot` / granted remote `bot` with `handle` + owner fields, roles `admin` / `member`, `joinedSeq` / `leftSeq`); log lines carry `authorId` + `authorKind`; history is the membership window (current members and the owner of an invited Bot see `seq >=` their interval; leave withdraws access; rejoin starts a new interval); local cross-owner rooms accept each side's *granted* Bots (#90: default deny, Chief included); a friend's Bot from `social/remote-bots` (`cleanRemote`) may be added and ungranted Bots stay private, and a room with a friend's Bot holds only granted local Bots (a Bot that joined by room code, `viaCode`, only for that room); `view()` labels remotes with an owner badge and shows an invited owner **Shared room from \**. Face-to-face rooms (#83) reuse the same `participants` / membership-window / owner-badge presentation with `origin: 'face-to-face'`: room-only remotes may sit in the room without a person-grant or a friendship, `presentRoom` treats the viewer as a member (not Shared room from), leave skips the two-Bot floor, and `fanoutRoomLine` goes to `rt.faceRoomFanout` instead of friend `flybotSend`. Still not team mode — see [team](team.md). Per-Bot grants (#90) add host module `src/host/reach.js`, installed after room-membership: `state.grants[botId]` is the list of independent grants (one Bot → one grantee, optionally room-scoped); default deny, Chief included; a Bot whose owner is not a member has none. Grants are set with `rt.setGrants`, exposed as `BotView.granted`, `flybot.bot.hasGrant` / `hasAnyGrant`, `flybot.grants` and the `bot.grants` event. `reach()` is the owner policy: a friend's Bot may reach only local Bots granted to that sender, only a granted Bot (or one code-joined in that room) may message a friend's Bot, another owner's Bot is reached only if both sides have matching grants or both are members of the room the message belongs to. It is not a middleware: hooks.js runs it before the `bot.send` chain, again on approval release, and on every deliver with a sender or remote origin (resumed, retried, deferred, re-sent), so no verdict or `bypass` can skip it and a queued message is judged by what is true when it is delivered. Revoking a grant, or replacing its Chief, removes it from rooms that have friends' Bots or several owners at once (`rt.evictUnshared`). Approval binding (#52) completes the held-send half of this: `state.flybot.approvals` records carry `requestId`, a sha256 `version` over the request content, server-derived `ownerId` and `origin` (`via`/`hop`/audience) — claims in the message (`meta`, text) never confer authority. `flybot/decide` is allowed through authz for any signed-in actor and the per-record check happens in hooks (`decide` requires the sender Bot's owner's session, `bot/forbidden` otherwise, `bot/unauthorized` without a session); `release()` revalidates before dispatch and marks the record `stale` when the request changed, the sender or target Bot is gone (also invalidated eagerly on `deleteBot`), or the Bot changed owners; a demoted Chief's sender-side holds are invalidated eagerly on `setMain` (not inside `staleReason`), and `release()` re-checks immediately before dispatch so an in-flight approve cannot go out after a concurrent invalidation. Unknown `invalidateFor` field values throw. `flybot/approvals` lists only the actor's own holds while enforcement is on; decided stale records keep their attribution in state but appear in no member's list. Which statuses still wait for that decision is one rule in `src/shared/approval-waiting.js` (#754): `decide()`, `heldOf()` in the chat view, the in-flight filter that invalidates holds, `PENDING_MAX` and the `APPROVALS_KEPT` cleanup all ask `isApprovalWaiting()`, and `channel.js` counts a row's waiting replies with it, so a `pending_approval` hold (Bot Social's own spelling) is decidable, drawn as waiting and never dropped mid-wait. - One writer per home (`src/host/home-lock.js`): the first dsh to load `~/.dsh` takes `state.lock` (pid, host, a random token in the file, its mtime bumped on a timer) and a second window on the same data only reads, answering `bot/read-only` and naming the other host. Takeover is **on the next write**, not on a poll (#1027): a window that was read-only claims the free lock inside `save()` too, and a change made outside a request (a plugin, a scheduled task, a delivery) belongs in `rt.mutate(fn)`, which takes over and adopts what the other host left *before* the change is made and written. The writers that changed `rt.state` inline and then called `save()` are gone (#1048): `delivery.js` (a friend's first line and its unread mark, a team exchange) and `agents.js` (add, adopt, archive and remove of the DSH Agent) take the free lock and adopt first, re-resolving their records inside the change, so a write from a window that was read-only keeps what the other host left instead of replacing it with its older copy. `save()` itself never adopts: `adopt()` replaces `rt.state` whole, so adopting there would discard a change the caller has already made (`#1032`). Either takeover prints one line — `the other dsh has closed; this one now saves the Bot team (was pid on )` — from `refresh()` and from `save()` alike, so a window that writes over the file another host left leaves a trace (#1041). A lock file still naming a live holder on this host (its pid answers) keeps the window read-only, which is the #14 rule. A write that fails does not decide the delivery (#1096): the friend-line mark and the exchange record both land before the agent has the line, so a rejected `mutate()` (the lock was taken away, or the disk refused) prints one warning naming what was not recorded and the line is still handed over — a throw there would drop a message that exists nowhere else. The write chain is also readable: `rt.savesQueued()` is a monotonic watermark — every save this host has accepted since it started, never decremented — and `rt.savesSettled()` resolves once the chain is empty, so a caller that must read `state.json` after a save it did not await — a rollback's own write, in the tests — waits on the store instead of on a delay, where a wait long enough on an idle machine reads a half-written team on a busy one (#1368). diff --git a/patches/ds-bot/files/src/host/project-board.js b/patches/ds-bot/files/src/host/project-board.js index c6cd8e424..d031b58b4 100644 --- a/patches/ds-bot/files/src/host/project-board.js +++ b/patches/ds-bot/files/src/host/project-board.js @@ -16,10 +16,10 @@ // Read-only by construction: it reads `state.tasks`, `state.meetings` and the Project bindings, // asks each registered provider for the workflow refs of tasks that still exist, and returns the // sections. It writes nothing, starts nothing and is not called unless someone asks for the board. -import { open, readFile, stat } from 'node:fs/promises' +import { open } from 'node:fs/promises' import { join } from 'node:path' import { columnOf } from '../shared/task-board.js' -import { DECISIONS_DIR, listProjectState, sha256 } from './project-state.js' +import { DECISIONS_DIR, describeFile, isDecisionPath, listProjectState } from './project-state.js' import { cut } from './text.js' import { advanceRefusal } from './meetings.js' import { readMetrics } from './project-metrics.js' @@ -247,11 +247,12 @@ export function install(rt) { const archivedOf = async (meeting) => { const dir = text(meeting?.decisionDir) const path = text(meeting?.decisionFile) - if (dir === '' || !path.startsWith(`${DECISIONS_DIR}/`) || !path.endsWith('.md')) return undefined + // The listing accepts one shape only, and this read is part of it: a name that is not + // `decisions/.md`, or a symlink standing in for that file, is refused here too. Reading + // it would put a file outside the folder in the rows, and the past-cut fallback reads this set. + if (dir === '' || !isDecisionPath(path)) return undefined try { - const bytes = await readFile(join(dir, path)) - const info = await stat(join(dir, path)) - return { dir, path, bytes: bytes.length, sha256: sha256(bytes), mtime: new Date(info.mtimeMs).toISOString() } + return { dir, ...await describeFile(dir, path) } } catch { // Absent or unreadable is not a row: the meeting keeps its own record, with no path. return undefined @@ -274,7 +275,7 @@ export function install(rt) { } if (state.ok !== true) return { dir, files: [], total: 0, reason: state.reason } found.push(...state.files - .filter(file => file.path.startsWith(`${DECISIONS_DIR}/`) && file.path.endsWith('.md')) + .filter(file => isDecisionPath(file.path)) .map(file => ({ ...file, dir }))) } // The other source: the folder each decided meeting of this Project recorded, one file each. diff --git a/patches/ds-bot/files/src/host/project-state.js b/patches/ds-bot/files/src/host/project-state.js index 8039004fd..30bf9ab58 100644 --- a/patches/ds-bot/files/src/host/project-state.js +++ b/patches/ds-bot/files/src/host/project-state.js @@ -9,7 +9,7 @@ // file and a file too large to hand over each answer with the reason, never with an empty // string that would read as an empty document. import { createHash } from 'node:crypto' -import { readdir, readFile, stat } from 'node:fs/promises' +import { lstat, readdir, readFile, stat } from 'node:fs/promises' import { join } from 'node:path' import { defineTool } from './tool-schema.js' import { output } from './tools.js' @@ -22,7 +22,9 @@ export const DECISIONS_DIR = 'decisions' // the context it was meant to save. Over the bound the answer is the size, not the text. export const MAX_BYTES = 200_000 -const isDecisionPath = path => /^decisions\/[^/]+\.md$/.test(path) +// The one name shape a decision has: exactly one segment under `decisions/`, ending in `.md`. +// The lister, the reader and the board all take a name only when it matches this. +export const isDecisionPath = path => /^decisions\/[^/]+\.md$/.test(path) const allowed = path => PROJECT_STATE_PATHS.includes(path) || isDecisionPath(path) export const sha256 = bytes => createHash('sha256').update(bytes).digest('hex') @@ -48,9 +50,22 @@ async function decisionFiles(dir) { .sort() } -async function describe(dir, path) { +// One file sitting directly in the folder, never through a link. `readdir` already reports a +// symlink as "not a plain file", and reading a name — here, in the reader, and on the board — +// answers the same way: a symlink is an absent file, not its target. The `lstat` is what makes +// that one rule hold for a name that was never listed, a name from a record, or `PROJECT.md`. +async function fileAt(dir, path) { + const info = await lstat(join(dir, path)) + if (!info.isFile()) throw new Error(`not a plain file: ${path}`) + return info +} + +// One Project State file with its byte size, content hash and last-modified time. Exported +// because the board reads the file a decided meeting archived, and it must refuse exactly what +// this reader refuses — one rule, one place. +export async function describeFile(dir, path) { + const info = await fileAt(dir, path) const bytes = await readFile(join(dir, path)) - const info = await stat(join(dir, path)) return { path, bytes: bytes.length, sha256: sha256(bytes), mtime: new Date(info.mtimeMs).toISOString() } } @@ -66,12 +81,12 @@ export async function listProjectState(dir, { repository = true } = {}) { const found = [] for (const path of PROJECT_STATE_PATHS) { try { - found.push(await describe(dir, path)) + found.push(await describeFile(dir, path)) } catch { /* an absent Project State file is simply not listed */ } } for (const path of await decisionFiles(dir)) { try { - found.push(await describe(dir, path)) + found.push(await describeFile(dir, path)) } catch { /* a file that vanished between readdir and read is not listed */ } } return { ok: true, dir, files: found } @@ -88,11 +103,11 @@ export async function readProjectState(dir, file) { if (!await repositoryAt(dir)) return { ok: false, reason: `not a repository: ${dir}` } let info try { - info = await stat(join(dir, path)) + // A symlink answers like a file that is not there — never with the text it points at. + info = await fileAt(dir, path) } catch { return { ok: false, reason: `absent: ${path}` } } - if (!info.isFile()) return { ok: false, reason: `absent: ${path}` } if (info.size > MAX_BYTES) return { ok: false, reason: `too large: ${path} is ${info.size} bytes (bound ${MAX_BYTES})` } const bytes = await readFile(join(dir, path)) return { ok: true, path, bytes: bytes.length, sha256: sha256(bytes), mtime: new Date(info.mtimeMs).toISOString(), text: bytes.toString('utf8') } diff --git a/patches/ds-bot/files/test/project-board.test.js b/patches/ds-bot/files/test/project-board.test.js index b1ee23194..c64348ed1 100644 --- a/patches/ds-bot/files/test/project-board.test.js +++ b/patches/ds-bot/files/test/project-board.test.js @@ -2,7 +2,7 @@ // that same id through `run.task.id`. The aggregate reads both views as one row, never twice, // and writes nothing back. import assert from 'node:assert/strict' -import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, writeFileSync } from 'node:fs' +import { existsSync, mkdirSync, mkdtempSync, readFileSync, rmSync, statSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join, resolve } from 'node:path' import { test } from 'node:test' @@ -557,6 +557,127 @@ test('#2033: a decision name that points outside the folder never becomes a path } }) +// A hand-edited record, or a name crafted into a decisions folder, can point `decisionFile` +// anywhere. The board lists one shape only — a regular `decisions/.md` — and that same rule +// has to refuse every other name where the listing is produced, so neither a row nor the past-cut +// fallback reads outside the folder. Each class below is a refusal, never a silent skip. +const traversalFolder = () => { + const base = mkdtempSync(join(tmpdir(), 'ds-bot-decisions-escape-')) + const dir = join(base, 'project') + mkdirSync(join(dir, '.git'), { recursive: true }) + mkdirSync(join(dir, 'decisions')) + mkdirSync(join(dir, 'decisions', 'zz')) + mkdirSync(join(dir, 'decisions', 'sub')) + writeFileSync(join(dir, 'decisions', '2026-01-02-legit.md'), '# The real decision\n') + writeFileSync(join(dir, 'decisions', 'sub', '2026-01-02-deep.md'), '# Left out of the listing\n') + writeFileSync(join(dir, 'decisions', 'handwritten-notes.md'), '# Written by hand\n') + writeFileSync(join(base, 'outside.md'), '# Outside the folder\n') + symlinkSync(join(base, 'outside.md'), join(dir, 'decisions', '2026-01-02-link.md')) + return { base, dir } +} + +const traversalSeed = (dir, base, ids) => { + const team = seed() + team.projects['project-p1'].workDir = dir + const at = (id, decisionFile) => ({ + id, status: 'decided', question: `Q ${id}`, owner: 'Chief', project: 'project-p1', + decisionDir: dir, decisionFile, createdAt: 1, updatedAt: 5, decidedAt: 6, + }) + const all = { + 'm-dotdot': 'decisions/zz/../../../outside.md', + 'm-absolute-out': join(base, 'outside.md'), + 'm-absolute-root': '/etc/passwd', + 'm-symlink': 'decisions/2026-01-02-link.md', + 'm-deep': 'decisions/sub/2026-01-02-deep.md', + 'm-legit': 'decisions/2026-01-02-legit.md', + } + team.meetings = Object.fromEntries(ids.map(id => [id, at(id, all[id])])) + return team +} + +// Each case gets only its own meetings, so a leak in one class cannot make another class red. +async function startTraversal(ids) { + const home = mkdtempSync(join(tmpdir(), 'ds-bot-board-')) + const { base, dir } = traversalFolder() + const { rt, stop } = await start(home, traversalSeed(dir, base, ids)) + return { rt, base, dir, stop } +} + +const decidedRow = (board, id) => board.meetings.decided.find(meeting => meeting.id === id) + +test('a decision name with a .. segment never becomes a row or a past-cut path', async () => { + const { rt, base, stop } = await startTraversal(['m-dotdot']) + try { + const board = await rt.projectBoard('project-p1') + assert.equal(decidedRow(board, 'm-dotdot').path, '', 'a name with a .. segment is refused, not projected') + assert.equal(board.decisions.files.some(file => file.path.includes('..')), false, 'no row names a .. path') + assert.equal(board.decisions.files.some(file => file.title === 'Outside the folder'), false, 'no row carries the outside title') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('an absolute decision name is refused, never read', async () => { + const { rt, base, stop } = await startTraversal(['m-absolute-out', 'm-absolute-root']) + try { + const board = await rt.projectBoard('project-p1') + assert.equal(decidedRow(board, 'm-absolute-out').path, '', 'an absolute path is refused') + assert.equal(decidedRow(board, 'm-absolute-root').path, '', 'and so is a root absolute path') + assert.equal('sha256' in decidedRow(board, 'm-absolute-out'), false) + assert.equal(board.decisions.files.some(file => file.path.startsWith('/')), false, 'no row names an absolute path') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('a symlinked decision name is refused, never followed', async () => { + const { rt, base, stop } = await startTraversal(['m-symlink']) + try { + const board = await rt.projectBoard('project-p1') + const row = decidedRow(board, 'm-symlink') + assert.equal(row.path, '', 'a symlink is not a decision file this listing found') + assert.equal('sha256' in row, false, 'and it carries no hash of the file it points at') + assert.equal(board.decisions.files.some(file => file.path === 'decisions/2026-01-02-link.md'), false, 'the link is not a row') + assert.equal(board.decisions.files.some(file => file.title === 'Outside the folder'), false, 'and its target is never titled') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('a decision name this listing did not read never becomes a past-cut path', async () => { + const { rt, base, stop } = await startTraversal(['m-deep']) + try { + const board = await rt.projectBoard('project-p1') + const row = decidedRow(board, 'm-deep') + assert.equal(row.path, '', 'the fallback names only what this read listed, so a name it never read is not a path') + assert.equal('sha256' in row, false) + assert.equal(board.decisions.files.some(file => file.path === 'decisions/sub/2026-01-02-deep.md'), false, 'the listing reads one level only') + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + +test('a regular decision name is still listed, titled and hashed', async () => { + const { rt, base, stop } = await startTraversal(['m-legit']) + try { + const board = await rt.projectBoard('project-p1') + const file = board.decisions.files.find(row => row.path === 'decisions/2026-01-02-legit.md') + assert.equal(file.title, 'The real decision', 'a legal name is still listed with its own title') + const handwritten = board.decisions.files.find(row => row.path === 'decisions/handwritten-notes.md') + assert.equal(handwritten.title, 'Written by hand', 'a legal name with no date is listed too: the rule is the shape, not the date') + const row = decidedRow(board, 'm-legit') + assert.equal(row.path, 'decisions/2026-01-02-legit.md', 'and the meeting still finds it') + assert.match(row.sha256, /^[0-9a-f]{64}$/) + } finally { + await stop() + rmSync(base, { recursive: true, force: true }) + } +}) + // #2005: a decision file's title is its first heading, bounded at 200 units. A heading whose 200th // unit is the first half of an emoji used to reach the board as an unpaired surrogate, so the // Decisions section carried half a character. diff --git a/patches/ds-bot/files/test/project-state.test.js b/patches/ds-bot/files/test/project-state.test.js index 4f2d080fc..05a354e09 100644 --- a/patches/ds-bot/files/test/project-state.test.js +++ b/patches/ds-bot/files/test/project-state.test.js @@ -1,5 +1,5 @@ import assert from 'node:assert/strict' -import { mkdirSync, mkdtempSync, readFileSync, rmSync, writeFileSync } from 'node:fs' +import { mkdirSync, mkdtempSync, readFileSync, rmSync, symlinkSync, writeFileSync } from 'node:fs' import { tmpdir } from 'node:os' import { join } from 'node:path' import { test } from 'node:test' @@ -93,6 +93,32 @@ test('#1702: read one returns its bytes, bounded, and reports a missing file', a assert.match(bigRow, /\t200001 bytes\t/, 'a file over the read bound still lists with its size') }) +// A symlink is a name for a file somewhere else, and this reader answers for one folder. A name +// that stands for a link is therefore absent — the same answer as a file that is not there — and +// never the target's text. `readdir` already leaves a link out of the listing, and the read has to +// agree with it; a name from a record can reach the same file without ever being listed. +test('#2037: a symlinked Project State file is not followed, it answers as absent', async (t) => { + const dir = projectStateFolder() + const outside = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-outside-')) + writeFileSync(join(outside, 'secret.md'), '# Outside secret\n') + const { tool, stop } = await boot({ dir }) + t.after(async () => { + await stop() + rmSync(dir, { recursive: true, force: true, maxRetries: 5 }) + rmSync(outside, { recursive: true, force: true, maxRetries: 5 }) + }) + + symlinkSync(join(outside, 'secret.md'), join(dir, 'decisions/0002-link.md')) + assert.equal(await tool({ file: 'decisions/0002-link.md' }), 'absent: decisions/0002-link.md', 'a link answers like a missing file, never with the text it points at') + + rmSync(join(dir, 'PROJECT.md')) + symlinkSync(join(outside, 'secret.md'), join(dir, 'PROJECT.md')) + assert.equal(await tool({ file: 'PROJECT.md' }), 'absent: PROJECT.md', 'the same answer for a named Project State file') + const lines = (await tool({})).split('\n') + assert.equal(lines.some(line => line.startsWith('PROJECT.md\t')), false, 'a link is not listed either') + assert.equal(lines.some(line => line.startsWith('decisions/0002-link.md\t')), false, 'and neither is a linked decision') +}) + test('#1702: a folder with no repository answers not a repository', async (t) => { const dir = mkdtempSync(join(tmpdir(), 'ds-bot-project-state-nogit-')) writeFileSync(join(dir, 'PROJECT.md'), '# Project\n')