diff --git a/changelog.d/disk-read-root-and-fifos.fixed.md b/changelog.d/disk-read-root-and-fifos.fixed.md new file mode 100644 index 000000000..8c629c36a --- /dev/null +++ b/changelog.d/disk-read-root-and-fifos.fixed.md @@ -0,0 +1,20 @@ +- **The workspace's disk reads refuse a mount root that isn't the mount's + directory.** `readFileFromDisk`, `readBinaryFromDisk` and `read_image` read + whatever was at a mount's root path, so a root renamed away and replaced by + a link to another directory, or by another directory, was read as the + mount. Once the workspace has recorded the root's identity, with the first + evidence it gathers there, such a root is refused as `mount_unavailable`, + in the words a sync uses for it, until a sync with `acceptRoot` takes it. + A root that is gone, or is a file, is `mount_unavailable` too, with errno + ENOENT or ENOTDIR, where it was "File not found". A root that changes + during a read, until the file has been opened and checked, gets the same + code and errno as one changed before the read. +- **`readBinaryFromDisk`'s `maxBytes` bounds the read itself.** A file that + grows past it after the read's size check is refused as "File too large", + having loaded at most `maxBytes` bytes and one more, where it was read + whole. +- **Two opens no longer wait on a FIFO.** A directory's sync after a push + opened the directory blocking, so a FIFO put at its path held the sync + until something opened it for writing. The tool presentation file's read + did the same, synchronously, holding the whole process. Each now opens + without waiting, and refuses what isn't a directory or a regular file. diff --git a/src/modules/workspace/disk-agreement.ts b/src/modules/workspace/disk-agreement.ts index cd63fa899..c509d0c5c 100644 --- a/src/modules/workspace/disk-agreement.ts +++ b/src/modules/workspace/disk-agreement.ts @@ -181,6 +181,12 @@ export class DiskAgreement { * place owes none (recordWalk). */ private unsynced = false; + /** + * Whether memory holds a whole state: false until a rebuild first runs to + * its end. A rebuild fills a new map and swaps it in only at its end, so + * one that throws partway leaves whatever whole state came before it. + */ + private whole = false; /** Whether this host observes file modes (not on Windows): where it doesn't, no record it writes claims one. */ private readonly modesObserved: () => boolean; @@ -206,8 +212,10 @@ export class DiskAgreement { * recorded (the store may hold it where this process can't read it back), * and a decision from stale P can record what reopening contradicts, such * as a conflict that stays after the right evidence is back. So until the - * store is reopened, no evidence is read, written or checkpointed, and - * every pass and tool call that needs it fails. + * store is reopened, no evidence is read (but for rootIdentityForRead's + * one exception), written or checkpointed, and every pass and tool call + * that needs it fails. The same holds, the exception included, when the + * open's rebuild threw: there is no whole evidence to read. */ ensureReconciled(): void { if (this.journal.needsReopen) { @@ -216,6 +224,12 @@ export class DiskAgreement { 'an append to its journal failed inside the store, and the evidence may lack what it recorded', ); } + if (!this.whole) { + throw new Error( + 'Disk-agreement evidence is unavailable until the store is reopened, as at the next start: ' + + 'reading it back from the store failed', + ); + } if (this.journal.needsReconcile) this.rebuild(); } @@ -253,16 +267,24 @@ export class DiskAgreement { const mount = replayed.get(entry.mount); if (mount) apply(mount, entry); } - this.mounts.clear(); + // Filled apart and swapped in at the end, so an append that throws here + // leaves memory as it was, whole, rather than with the mounts after it + // missing. Nothing reaches that today: only an open with a moved mount + // appends here, the workspace opens each agreement once, and after the + // open a rebuild finds every mount's record and appends none. + const rebuilt = new Map(); for (const { name, root } of this.configured) { const found = replayed.get(name); if (found && sameRootPath(found.root, root)) { - this.mounts.set(name, found); + rebuilt.set(name, found); } else { - this.mounts.set(name, { root, paths: new Map(), walks: new Map() }); + rebuilt.set(name, { root, paths: new Map(), walks: new Map() }); this.journal.append({ t: 'mount', mount: name, root }); } } + this.mounts.clear(); + for (const [name, mount] of rebuilt) this.mounts.set(name, mount); + this.whole = true; } /** The identity of the root this mount's evidence was gathered under, once any was. */ @@ -271,6 +293,25 @@ export class DiskAgreement { return this.mounts.get(mount)?.identity; } + /** + * The identity a disk read checks the mount's root against: the one in + * memory once memory is whole, in every state of the journal, and an error + * until then. It is the last identity whose append succeeded, since write() + * appends before it applies, and no reconcile can change it, since root + * entries are written without a sync; were one made durable, its failed + * sync would leave the identity from before it: an old one, which refuses + * the new root, or none, as for a mount with no recorded identity. This + * is the one exception to ensureReconciled's "no evidence is read": the + * layer stops reading so that a decision from stale evidence can't record + * what reopening contradicts, and a read records nothing. Refusing reads + * instead would stop every host read of the workspace's disk until a + * restart. + */ + rootIdentityForRead(mount: string): RootIdentity | undefined { + if (!this.whole) this.ensureReconciled(); // throws: there's no whole state to read + return this.mounts.get(mount)?.identity; + } + /** * Record the root's identity just before the first evidence a mount gains: * the caller saw it as the root while gathering that evidence. A no-op once diff --git a/src/modules/workspace/index.ts b/src/modules/workspace/index.ts index fdc6f62bc..825d67f59 100644 --- a/src/modules/workspace/index.ts +++ b/src/modules/workspace/index.ts @@ -40,7 +40,7 @@ import { MountWatcher, type FsChange } from './watcher.js'; import { hashContent, DEFAULT_MAX_FILE_SIZE } from './sync.js'; import { DiskAgreement, sameRootIdentity } from './disk-agreement.js'; import { BranchIntents } from './branch-intent.js'; -import { IGNORED, NONBLOCK, OTHER_MOUNT, checkDir, diskSpelling, filesystemTrustsCtime, inOtherMount, leadsInto, linkLeads, linkOnTheWay, linkRefusal, linkWords, locateRoot, nestedAtReal, nestedHolder, passedOverAt, readPrefix, type LinkFound, type NestedMount } from './observe.js'; +import { IGNORED, NONBLOCK, OTHER_MOUNT, ROOT_REPLACED, ROOT_UNAVAILABLE, checkDir, checkRoot, diskSpelling, filesystemTrustsCtime, inOtherMount, leadsInto, linkLeads, linkOnTheWay, linkRefusal, linkWords, locateRoot, nestedAtReal, nestedHolder, passedOverAt, readPrefix, type CheckedDir, type LinkFound, type MountView, type NestedMount } from './observe.js'; import { TOOL_BATCH_HOOK_TIMEOUT_MS, ensureStateRegistered } from '../../module-registry.js'; import { type MountRuntime, @@ -121,8 +121,11 @@ class WorkspaceImageReadError extends Error { /** * Why a workspace-owned filesystem read was refused. Distinguishes the cases a * peer needs to react to differently: a path that never resolved inside the - * mount (`unknown_mount`, `traversal`), a mount whose root is gone - * (`mount_unavailable`), an ordinary missing file (`not_found`), a directory + * mount (`unknown_mount`, `traversal`), a mount whose root is gone, isn't a + * directory, or isn't the directory the workspace was using + * (`mount_unavailable`: with errno ENOENT or ENOTDIR when no directory is + * there at all, with none for another directory), an ordinary missing file + * or one that isn't a regular file, such as a FIFO (`not_found`), a directory * (`directory`), a file that is itself a symlink, refused by the mount's * `followSymlinks` policy (`symlink`), a symlink among the path's directories * on a mount that doesn't follow symlinks, leading to a directory or a file @@ -133,7 +136,8 @@ class WorkspaceImageReadError extends Error { * (`changed`), and a path in another mount's directory nested in the one * named, or reached through a symlink among the path's directories leading * there (`other_mount`), whose message names the mount that holds it and the - * path to use there. + * path to use there. A `mount_unavailable` for a root whose realpath or stat + * fails otherwise carries that errno, such as EACCES. */ export type WorkspaceReadErrorCode = | 'unknown_mount' @@ -150,6 +154,8 @@ export type WorkspaceReadErrorCode = /** Where in the open sequence a `WorkspaceReadError` originated (diagnostics). */ export type WorkspaceReadStage = | 'parse' + /** The mount root's check: a directory, the one the workspace was using. */ + | 'root' | 'readlink' | 'lstat' | 'realpath_root' @@ -231,6 +237,20 @@ function errnoOf(err: unknown): string | undefined { : undefined; } +/** + * What is at a mount root's path now, followed as the root is: for a + * directory, undefined; for anything else, ENOTDIR; and where the path + * can't be followed to anything, the errno its stat failed with, ENOENT + * when nothing is there. + */ +async function rootAbsence(root: string): Promise { + try { + return (await stat(root)).isDirectory() ? undefined : 'ENOTDIR'; + } catch (err) { + return errnoOf(err); + } +} + /** read_image's refusal of a file over the mount's size limit, wherever it's found to be. */ function imageTooLarge(maxSize: number, mountPrefixedPath: string): WorkspaceImageReadError { return new WorkspaceImageReadError('too_large', `Image file exceeds max size (${maxSize} bytes): ${mountPrefixedPath}`); @@ -934,6 +954,25 @@ export class WorkspaceModule implements Module { return this.agreement; } + /** + * A mount as observation reads it, with the root identity the workspace + * recorded for it, if any: what a pass reads beneath (runtime), with + * rootIdentity's, and what a disk read checks its root against (readRoot), + * with rootIdentityForRead's. + */ + private viewOf(mount: MountState, rootIdentity: MountView['rootIdentity']): MountView { + return { + root: resolve(mount.config.path), + name: mount.config.name, + rootIdentity, + followSymlinks: mount.config.followSymlinks === true, + maxFileSize: mount.config.maxFileSize ?? DEFAULT_MAX_FILE_SIZE, + ignore: mount.config.ignore ?? [], + others: this.othersOf(mount.config.name), + nested: this.nestedOf(mount.config.name), + }; + } + /** The view reconciliation needs of a mount, refreshed per pass. */ private async runtime(mount: MountState): Promise { const root = resolve(mount.config.path); @@ -946,16 +985,7 @@ export class WorkspaceModule implements Module { } return { name: mount.config.name, - view: { - root, - name: mount.config.name, - rootIdentity: this.agreementOrThrow().rootIdentity(mount.config.name), - followSymlinks: mount.config.followSymlinks === true, - maxFileSize: mount.config.maxFileSize ?? DEFAULT_MAX_FILE_SIZE, - ignore: mount.config.ignore ?? [], - others: this.othersOf(mount.config.name), - nested: this.nestedOf(mount.config.name), - }, + view: this.viewOf(mount, this.agreementOrThrow().rootIdentity(mount.config.name)), rootReal, ctimeTrustedDev: trustedDev, treeStateId: mount.treeStateId, @@ -1873,6 +1903,11 @@ export class WorkspaceModule implements Module { * `followSymlinks` holds for the file and the directories on its way, and * what it reaches stays canonically inside the mount. Deliberately does NOT * sync, register, or hash anything — it reads and returns. + * + * `maxBytes` refuses a larger file as "File too large", having loaded at + * most `maxBytes` bytes and one more: the reads decide where the file ends, + * not the size fstat gave at the open, so the bound holds for a file that + * grows while it's read. */ async readBinaryFromDisk( mountPrefixedPath: string, @@ -1886,16 +1921,24 @@ export class WorkspaceModule implements Module { return { error: error instanceof Error ? error.message : String(error) }; } const absolutePath = resolve(mount.config.path, relativePath); + const maxBytes = opts.maxBytes; let tooLarge: string | undefined; try { - const { handle } = await this.openContainedFile(mount, relativePath, mountPrefixedPath, (fileStat) => { - if (opts.maxBytes !== undefined && fileStat.size > opts.maxBytes) { - tooLarge = `File too large: ${fileStat.size} > ${opts.maxBytes} bytes`; + const { handle, fileStat } = await this.openContainedFile(mount, relativePath, mountPrefixedPath, (fileStat) => { + if (maxBytes !== undefined && fileStat.size > maxBytes) { + tooLarge = `File too large: ${fileStat.size} > ${maxBytes} bytes`; throw new Error(tooLarge); } }); try { - return { data: await handle.readFile(), absolutePath }; + if (maxBytes === undefined) return { data: await handle.readFile(), absolutePath }; + // No more than maxBytes is loaded: a file grown past it since the + // fstat above is refused here, as one already over it was there. Its + // size is an fstat's after the read, and at least one more than the + // bytes kept, so the refusal's size is always past maxBytes. + const { bytes, more } = await readPrefix(handle, maxBytes, fileStat.size); + if (more) return { error: `File too large: ${Math.max((await handle.stat()).size, bytes.length + 1)} > ${maxBytes} bytes` }; + return { data: bytes, absolutePath }; } finally { await handle.close(); } @@ -2174,29 +2217,95 @@ export class WorkspaceModule implements Module { return this.validateImageBytes(blob, mountPrefixedPath, maxSize); } + /** + * The mount root a disk read reads beneath: resolved, then checked as + * observation checks it (checkRoot), so the read and the evidence agree on + * which directory the mount is. Once the workspace has recorded the root's + * identity, a root that resolves to any other directory is refused, in + * ROOT_REPLACED's words; with none recorded yet, it need only be a + * directory there. A refusal where no directory is at the root's path + * carries the errno that gives (rootAbsence: ENOENT when nothing is there + * and ENOTDIR when something else is, which a caller can read as no file, + * or the errno the root's stat failed with otherwise, such as EACCES). A + * root whose realpath fails carries realpath's errno, and one that is + * another directory carries none. The identity is rootIdentityForRead's, + * which answers while the evidence awaits reopening; where even it can't + * be read, as when the evidence was never read back whole, the read is + * refused too, with an errno only where no directory is at the root's path. + */ + private async readRoot(mount: MountState): Promise<{ real: string; dir: CheckedDir }> { + const name = mount.config.name; + const root = resolve(mount.config.path); + let real: string; + try { + real = await realpath(root); + } catch (err) { + throw new WorkspaceReadError('mount_unavailable', `Mount unavailable: ${name}`, 'realpath_root', errnoOf(err)); + } + let rootIdentity: MountView['rootIdentity']; + try { + rootIdentity = this.agreementOrThrow().rootIdentityForRead(name); + } catch (err) { + // The evidence can't be read back, so neither can the check be made. + // Where no directory is at the root's path, its errno still says so. + throw new WorkspaceReadError('mount_unavailable', + `Mount unavailable: ${name}: ${err instanceof Error ? err.message : String(err)}`, 'root', await rootAbsence(root)); + } + const checked = await checkRoot(this.viewOf(mount, rootIdentity), real); + if ('dir' in checked) return { real, dir: checked.dir }; + // Another directory is no absence. For any other refusal, what is at the + // root's path now decides: nothing, something that isn't a directory, or + // a directory the path has come to name since it was resolved (no errno). + const errno = checked.reason === ROOT_REPLACED ? undefined : await rootAbsence(root); + throw new WorkspaceReadError('mount_unavailable', `Mount unavailable: ${name}: ${checked.reason}`, 'root', errno); + } + + /** + * The refusal for a root that stopped being the directory a read checked, + * with the code and errno readRoot gives what is at its path now (its stage + * and words are this check's): gone, or not a directory, with ENOENT or + * ENOTDIR, so a caller reads a root gone mid-read as one gone before it; or + * another directory, with none. + */ + private async rootMoved(mount: MountState): Promise { + const errno = await rootAbsence(resolve(mount.config.path)); + const reason = errno === undefined ? ROOT_REPLACED : ROOT_UNAVAILABLE; + return new WorkspaceReadError('mount_unavailable', `Mount unavailable: ${mount.config.name}: ${reason}`, 'root', errno); + } + /** * Open a mount-relative file for reading with the mount boundary enforced on * the *filesystem*, not just the path string: * - * 0. On a mount that does not `followSymlinks`, refuse a path with a symlink + * 0. Check the root (readRoot) before anything beneath it: under a root + * that isn't the mount's directory, no path says anything about the + * mount's files, an absence included. + * 1. On a mount that does not `followSymlinks`, refuse a path with a symlink * among the directories on its way beneath the mount's root, wherever it * leads (linkOnTheWay), before anything resolves it: as `escape` for a * link leading outside the mount, as `other_mount` for one into a nested * mount's directory, and as `parent_symlink` for any other. - * 1. `lstat` the lexical path — refuse directories; refuse a final-component + * 2. `lstat` the lexical path — refuse directories; refuse a final-component * symlink when the mount does not `followSymlinks`; refuse anything else * that isn't a regular file, such as a FIFO, without opening it. - * 2. Open with `O_NOFOLLOW` where the platform has it (POSIX) so the refusal + * 3. Open with `O_NOFOLLOW` where the platform has it (POSIX) so the refusal * holds against a symlink swapped in between lstat and open; where it * doesn't (Windows), re-`lstat` after opening instead. `O_NONBLOCK`, so a * FIFO swapped in meanwhile doesn't wait for a writer. - * 3. `realpath` both the mount root and the opened path. On a mount that - * does not `followSymlinks`, refuse it again for a symlinked directory on - * its way, which catches one swapped in since step 0 (linkRefusal). Then - * require canonical containment — this is what catches an *intermediate* - * symlinked directory escaping the mount, and a followed symlink whose - * target lands outside, on every platform (junctions included). - * 4. Confirm the descriptor and the canonical path are the same inode. + * 4. `realpath` the opened path. On a mount that does not `followSymlinks`, + * refuse it again for a symlinked directory on its way, which catches one + * swapped in since step 1 (linkRefusal). Then require canonical + * containment in the root step 0 resolved — this is what catches an + * *intermediate* symlinked directory escaping the mount, and a followed + * symlink whose target lands outside, on every platform (junctions + * included). + * 5. Confirm the descriptor and the canonical path are the same inode, and + * the root still the directory step 0 checked. + * + * Whatever steps 1 to 5 refuse, a missing file included, is said of the + * mount's files only while the root is still that directory. If it isn't, + * the read is refused with the code and errno step 0 gives what is at the + * root's path then (rootMoved), as step 5's own check of the root does. * * `inspect` runs last, after the boundary's checks, so a caller's own check * of the file, such as a size limit, never answers for a file the boundary @@ -2207,6 +2316,25 @@ export class WorkspaceModule implements Module { relativePath: string, mountPrefixedPath: string, inspect?: (fileStat: Stats) => void, + ): Promise<{ handle: Awaited>; fileStat: Stats; realFilePath: string }> { + const root = await this.readRoot(mount); + try { + return await this.openBeneathRoot(mount, root, relativePath, mountPrefixedPath, inspect); + } catch (err) { + if (err instanceof WorkspaceReadError && err.code !== 'mount_unavailable' && !(await root.dir.stillHere())) { + throw await this.rootMoved(mount); + } + throw err; + } + } + + /** Steps 1 to 5 of openContainedFile, beneath the root that step 0 checked. */ + private async openBeneathRoot( + mount: MountState, + { real: realMountRoot, dir: rootDir }: { real: string; dir: CheckedDir }, + relativePath: string, + mountPrefixedPath: string, + inspect?: (fileStat: Stats) => void, ): Promise<{ handle: Awaited>; fileStat: Stats; realFilePath: string }> { const lexicalPath = resolve(mount.config.path, relativePath); const follow = mount.config.followSymlinks === true; @@ -2241,13 +2369,6 @@ export class WorkspaceModule implements Module { throw new WorkspaceReadError('not_found', `Not a regular file: ${mountPrefixedPath}`, 'lstat'); } - let realMountRoot: string; - try { - realMountRoot = await realpath(mount.config.path); - } catch (err) { - throw new WorkspaceReadError('mount_unavailable', `Mount unavailable: ${mount.config.name}`, 'realpath_root', errnoOf(err)); - } - let handle: Awaited>; try { // O_NONBLOCK: a FIFO swapped in since the lstat opens at once instead of @@ -2335,6 +2456,12 @@ export class WorkspaceModule implements Module { if (pathStat.dev !== fileStat.dev || pathStat.ino !== fileStat.ino) { throw new WorkspaceReadError('changed', `File changed during read: ${mountPrefixedPath}`, 'stat'); } + // Containment compares paths, so it can't see another directory put at + // the root's own path since step 0 (a drive unmounted mid-read): the + // root is asked again, as an effect asks it (effects.ts moved()). This + // sees the root as it is now, so a root replaced and put back before it + // isn't seen (observe.ts's header says why no check can be sure). + if (!(await rootDir.stillHere())) throw await this.rootMoved(mount); // The caller's checks of the file, such as a size limit, come after the // boundary's, so they never answer for a file the boundary refuses. @@ -2362,11 +2489,20 @@ export class WorkspaceModule implements Module { * (`truncated`). The reads decide where the file ends, not the size fstat * gave at the open, so the bound holds for a file that grows while it's * read. + * - Once the workspace has recorded the mount root's identity, with the + * first evidence it gathers there, a root that is another directory is + * an unavailable mount: a replaced directory, a link swapped in for it, + * or an unmounted drive's mountpoint. In any mount, recorded or not, a + * root that stops being the directory the read checked until the file + * has been opened and checked is unavailable too: with ENOENT or + * ENOTDIR where no directory is there, as before the read, and with no + * errno for another directory. The file's bytes are then read through + * the descriptor that was checked. * * Throws {@link WorkspaceReadError} with a `code` distinguishing unknown * mount, lexical traversal, a path in another mount's directory nested in - * the one named (`other_mount`), unavailable mount, missing file, - * directory, policy-denied symlink, outside-mount target, and + * the one named (`other_mount`), unavailable mount, missing or non-regular + * file, directory, policy-denied symlink, outside-mount target, and * file-changed-during-read. */ async readFileFromDisk( @@ -2419,7 +2555,9 @@ export class WorkspaceModule implements Module { const p = mountPrefixedPath; switch (err.code) { case 'mount_unavailable': - return new WorkspaceImageReadError('mount_unavailable', `Mount unavailable: ${mount.config.name}`); + // With why, where the root's check says, such as a directory that + // isn't the mount's. + return new WorkspaceImageReadError('mount_unavailable', err.message); case 'directory': return new WorkspaceImageReadError('directory', `Path is a directory: ${p}`); case 'symlink': diff --git a/src/modules/workspace/observe.ts b/src/modules/workspace/observe.ts index e0e8fc841..6986d4cf1 100644 --- a/src/modules/workspace/observe.ts +++ b/src/modules/workspace/observe.ts @@ -89,10 +89,12 @@ export type NestedMount = { dir: string; name: string; real?: string }; export type StatFingerprint = Omit; /** - * Opened with every read or write of a workspace path, where the platform has - * it: for a regular file it changes nothing, and a FIFO or other special file - * swapped in at the path neither blocks the open nor passes the regular-file - * check that follows. + * Opened with every open of a workspace path that may already be there, a + * read, a write in place or a directory's sync, where the platform has it: + * for a regular file or a directory it changes nothing, and a FIFO or other + * special file swapped in at the path neither blocks the open nor passes the + * check of its kind that follows. A create needs none: O_CREAT | O_EXCL fails + * at once on anything already at the path. */ export const NONBLOCK = typeof fsConstants.O_NONBLOCK === 'number' ? fsConstants.O_NONBLOCK : 0; @@ -502,7 +504,11 @@ export class CheckedDir { */ async sync(): Promise { if (process.platform === 'win32') return; - const handle = await open(this.real, 'r'); + // NONBLOCK: a FIFO put at its path since it was checked opens at once, + // and is refused below as not this directory, instead of holding the + // open, and with it the mount's turn, until something opens it for + // writing. + const handle = await open(this.real, fsConstants.O_RDONLY | NONBLOCK); try { const info = await handle.stat({ bigint: true }); if (!info.isDirectory() || info.dev !== this.dev || info.ino !== this.ino) throw new Error(CHANGED); diff --git a/src/tool-presentation.ts b/src/tool-presentation.ts index 9e45111c8..9168b0803 100644 --- a/src/tool-presentation.ts +++ b/src/tool-presentation.ts @@ -1,5 +1,5 @@ import { createHash, randomUUID } from 'node:crypto'; -import { fchmodSync, closeSync, fstatSync, lstatSync, openSync, readSync, renameSync, unlinkSync, writeFileSync } from 'node:fs'; +import { constants, fchmodSync, closeSync, fstatSync, lstatSync, openSync, readSync, renameSync, unlinkSync, writeFileSync } from 'node:fs'; import { closestNames } from './closest-names.js'; import { boundedErrorMessage, errorMessage } from './error-message.js'; import type { ToolDefinition, ToolResult } from './types/events.js'; @@ -24,6 +24,13 @@ export interface PresentationSnapshot { entries: { name: string; source: string; visible: boolean; originalDescription: string; description: string; descriptionSource?: string; inputSchema: ToolDefinition['inputSchema'] }[]; } const MAX_BYTES = 256 * 1024; +/** + * Opened with each read of a presentation file, as observe.ts opens workspace + * files: a FIFO at the operator's path opens at once and fails the regular-file + * check, where a blocking open would hold the whole process (it's synchronous) + * until something opened the FIFO for writing. + */ +const NONBLOCK = typeof constants.O_NONBLOCK === 'number' ? constants.O_NONBLOCK : 0; const MAX_DESCRIPTION_CHARS = 32768; const protectedNames = new Set(['set_tool_visibility', 'set_tool_description', 'workspace--read']); export const isPresentationTool = (name: string): boolean => name === 'set_tool_visibility' || name === 'set_tool_description'; @@ -173,7 +180,7 @@ export class ToolPresentation { constructor(readonly config: ToolPresentationConfig) {} private read(path = this.config.path, allowMissing = true): string { let fd: number; - try { fd = openSync(path, 'r'); } + try { fd = openSync(path, constants.O_RDONLY | NONBLOCK); } catch (error) { const code = (error as NodeJS.ErrnoException).code; if (code === 'ENOENT') { diff --git a/test/tool-presentation.test.ts b/test/tool-presentation.test.ts index 092ec448e..a60e8b4e8 100644 --- a/test/tool-presentation.test.ts +++ b/test/tool-presentation.test.ts @@ -2,6 +2,7 @@ import fs from 'node:fs'; import {syncBuiltinESMExports} from 'node:module'; import {test} from 'node:test'; import assert from 'node:assert/strict'; +import {spawnSync} from 'node:child_process'; import {mkdtempSync,rmSync,writeFileSync,readFileSync,chmodSync,statSync,existsSync,mkdirSync} from 'node:fs'; import {tmpdir} from 'node:os'; import {join} from 'node:path'; @@ -499,3 +500,23 @@ test('a problem lists at most ten names, then how many more',()=>{ assert.ok(defaultsFile.includes(`(it sets "visible" for ${listed(twelve('big--t'))}, and a defaults file may set only descriptions`),defaultsFile); } finally {rmSync(dir,{recursive:true,force:true});} }); + +test('a FIFO at the presentation file is refused at once, not waited on with the process held',{skip:process.platform==='win32'?'no FIFOs':false},()=>{ + const dir=mkdtempSync(join(tmpdir(),'presentation-fifo-')); + try { + const path=join(dir,'tools.json'); + assert.equal(spawnSync('mkfifo',[path]).status,0,'mkfifo'); + // Read in a child process, bounded: the read is synchronous, so an open that + // waits for a writer holds the whole process it runs in. + const module=new URL('../src/tool-presentation.js',import.meta.url).href; + const script=`const {ToolPresentation}=await import(${JSON.stringify(module)});` + +`const s=new ToolPresentation({path:${JSON.stringify(path)},cataloguePath:'board/tools.md'}).resolve([{name:'example',description:'original',inputSchema:{type:'object'}}]);` + +'console.log(JSON.stringify({diagnostics:s.diagnostics,description:s.advertised[0].description}));'; + const child=spawnSync(process.execPath,['--input-type=module','-e',script],{encoding:'utf8',timeout:10_000}); + assert.equal(child.signal,null,`the read waited on the FIFO until the child was stopped (${child.signal})`); + assert.equal(child.status,0,child.stderr); + const out=JSON.parse(child.stdout); + assert.match(out.diagnostics[0],/^Your tool presentation file can't be used \(it isn't a regular file\)/); + assert.equal(out.description,'original','the default description stays'); + } finally {rmSync(dir,{recursive:true,force:true});} +}); diff --git a/test/workspace-read-file-from-disk.test.ts b/test/workspace-read-file-from-disk.test.ts index c3f57826e..2cf164597 100644 --- a/test/workspace-read-file-from-disk.test.ts +++ b/test/workspace-read-file-from-disk.test.ts @@ -12,19 +12,28 @@ * Symlink cases create real symlinks; on Windows that needs a privilege the * test runner may not have (EPERM), in which case those cases are skipped — * the non-symlink cases still run everywhere. + * + * Once the workspace has recorded the mount root's identity, the same read + * refuses a root that is another directory: a link swapped in for it passed + * containment, which compares canonical paths. It asks the root again while + * it reads, and answers what it finds as before the read. */ import test, { type TestContext } from 'node:test'; import assert from 'node:assert/strict'; import { execFileSync } from 'node:child_process'; -import { appendFileSync, chmodSync, closeSync, constants as fsConstants, mkdirSync, mkdtempSync, openSync, rmSync, statSync, symlinkSync, truncateSync, writeFileSync } from 'node:fs'; +import { appendFileSync, chmodSync, closeSync, constants as fsConstants, mkdirSync, mkdtempSync, openSync, realpathSync, renameSync, rmSync, statSync, symlinkSync, truncateSync, writeFileSync } from 'node:fs'; import { createRequire, syncBuiltinESMExports } from 'node:module'; import { tmpdir } from 'node:os'; import { join } from 'node:path'; import { JsStore } from '@animalabs/chronicle'; import { WorkspaceModule, WorkspaceReadError } from '../src/modules/workspace/index.js'; +import { ROOT_REPLACED, ROOT_UNAVAILABLE } from '../src/modules/workspace/observe.js'; import { changedAfterFstat, countingLoads, withFileHandle } from './helpers/file-handle.js'; +/** A valid 1x1 PNG, for read_image. */ +const ONE_PIXEL_PNG = Buffer.from('iVBORw0KGgoAAAANSUhEUgAAAAEAAAABCAYAAAAfFcSJAAAADUlEQVR4nGP4////fwAJ+wP9KobjigAAAABJRU5ErkJggg==', 'base64'); + function setup(t: TestContext, options?: { followSymlinks?: boolean }) { const root = mkdtempSync(join(tmpdir(), 'af-read-from-disk-')); const mountDir = join(root, 'mount'); @@ -38,7 +47,36 @@ function setup(t: TestContext, options?: { followSymlinks?: boolean }) { store.close(); rmSync(root, { recursive: true, force: true }); }); - return { root, mountDir, workspace }; + return { root, mountDir, store, workspace }; +} + +/** A workspace whose mount has a synced file, so the root's identity is recorded. */ +async function recorded(t: TestContext) { + const env = setup(t); + writeFileSync(join(env.mountDir, 'a.txt'), 'inside'); + const synced = await env.workspace.handleToolCall({ id: 'seed', name: 'sync', input: { mount: 'work' } }); + assert.equal(synced.success, true, synced.error); + const agreement = (env.workspace as unknown as { agreement: { rootIdentity(mount: string): unknown } }).agreement; + assert.ok(agreement.rootIdentity('work'), 'the sync recorded the root'); + return env; +} + +/** Run `fn` with node:fs/promises' `name` replaced by `make(original)`, as the module under test sees it. */ +async function withFsPromises( + name: 'open' | 'lstat' | 'stat' | 'realpath', + make: (original: (...args: any[]) => Promise) => (...args: any[]) => Promise, + fn: () => Promise, +): Promise { + const fsp = createRequire(import.meta.url)('node:fs/promises') as Record Promise>; + const original = fsp[name]!; + fsp[name] = make(original); + syncBuiltinESMExports(); + try { + return await fn(); + } finally { + fsp[name] = original; + syncBuiltinESMExports(); + } } /** Create a symlink, or skip the test when the platform refuses (Windows without the privilege). */ @@ -96,7 +134,8 @@ test('missing files and directories are distinguished', async (t) => { const { mountDir, workspace } = setup(t); mkdirSync(join(mountDir, 'dir')); - await expectRefusal(workspace.readFileFromDisk('work/missing.txt'), 'not_found'); + const missing = await expectRefusal(workspace.readFileFromDisk('work/missing.txt'), 'not_found'); + assert.equal(missing.errno, 'ENOENT', 'the errno a host reads as no file'); await expectRefusal(workspace.readFileFromDisk('work/dir'), 'directory'); await expectRefusal(workspace.readFileFromDisk('work'), 'directory'); }); @@ -382,3 +421,375 @@ test('read_image keeps its historical error wording after sharing the containmen const missing = await workspace.handleToolCall({ id: 'c2', name: 'read_image', input: { path: 'work/nope.png' } }); assert.equal(missing.error, 'File not found: work/nope.png'); }); + +test('readBinaryFromDisk with maxBytes refuses a file that grows past it after the open, having loaded no more than maxBytes and one byte', async (t) => { + const { mountDir, workspace } = setup(t); + const path = join(mountDir, 'upload.bin'); + for (const start of [Buffer.alloc(10, 0x61), Buffer.alloc(0)]) { + writeFileSync(path, start); // a file empty at the open, too: its fstat gives no size to read up to + const { result, loaded } = await countingLoads(() => changedAfterFstat(path, () => appendFileSync(path, Buffer.alloc(5000, 0x62)), + () => workspace.readBinaryFromDisk('work/upload.bin', { maxBytes: 100 }))); + assert.deepEqual(result, { error: `File too large: ${start.length + 5000} > 100 bytes` }, 'refused, with the size the read found'); + assert.ok(loaded <= 101, `loaded ${loaded} bytes: no more than maxBytes, and the one byte that shows the file goes on`); + } +}); + +test('readBinaryFromDisk refuses a file found past maxBytes with a size past it, even when the file shrinks once read', async (t) => { + const { mountDir, workspace } = setup(t); + const path = join(mountDir, 'upload.bin'); + writeFileSync(path, Buffer.alloc(50, 0x61)); + const ino = String(statSync(path, { bigint: true }).ino); + let stats = 0; + const result = await withFileHandle({ + stat: (stat) => async function (this: unknown, ...args: unknown[]) { + const found = await stat.apply(this, args); + if (String(found.ino) !== ino) return found; + stats++; + if (stats === 1) appendFileSync(path, Buffer.alloc(5000, 0x62)); // grows after the open's fstat + if (stats === 2) truncateSync(path, 10); // and shrinks before the fstat after the read + return stats === 2 ? stat.apply(this, args) : found; + }, + }, () => workspace.readBinaryFromDisk('work/upload.bin', { maxBytes: 100 })); + assert.equal(stats, 2, 'the fstat after the read was made'); + assert.deepEqual(result, { error: 'File too large: 101 > 100 bytes' }, 'the bytes the read found, where the fstat after it says 10'); +}); + +test('readBinaryFromDisk with maxBytes reads a file that grows after the open but stays within it, whole', async (t) => { + const { mountDir, workspace } = setup(t); + const path = join(mountDir, 'upload.bin'); + writeFileSync(path, Buffer.alloc(10, 0x61)); + const result = await changedAfterFstat(path, () => appendFileSync(path, Buffer.alloc(20, 0x62)), + () => workspace.readBinaryFromDisk('work/upload.bin', { maxBytes: 100 })); + assert.ok('data' in result, JSON.stringify(result)); + assert.deepEqual(result.data, Buffer.concat([Buffer.alloc(10, 0x61), Buffer.alloc(20, 0x62)])); +}); + +for (const replacement of ['link', 'directory'] as const) { + const what = replacement === 'link' ? 'a link to a directory outside the mount' : "another directory at the root's path"; + test(`once the workspace has recorded the mount root, a root that is ${what} is refused by every disk reader, until a sync accepts it`, async (t) => { + const { root, mountDir, workspace } = await recorded(t); + const elsewhere = join(root, 'elsewhere'); + mkdirSync(elsewhere); + writeFileSync(join(elsewhere, 'secret.txt'), 'SECRET'); + writeFileSync(join(elsewhere, 'pixel.png'), ONE_PIXEL_PNG); + renameSync(mountDir, join(root, 'mount-away')); + if (replacement === 'link') { + if (!symlinkOrSkip(t, elsewhere, mountDir, 'dir')) return; + } else { + renameSync(elsewhere, mountDir); // as an unmounted drive's mountpoint would be + } + + const replaced = `Mount unavailable: work: ${ROOT_REPLACED}`; + const err = await expectRefusal(workspace.readFileFromDisk('work/secret.txt'), 'mount_unavailable'); + assert.equal(err.message, replaced); + assert.equal(err.stage, 'root'); + assert.equal(err.errno, undefined, 'no errno: another directory is not a missing one'); + // Nor is anything beneath it absent: the workspace's own file isn't "not found" there. + assert.equal((await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable')).message, replaced); + assert.deepEqual(await workspace.readBinaryFromDisk('work/secret.txt'), { error: replaced }); + const image = await workspace.handleToolCall({ id: 'image', name: 'read_image', input: { path: 'work/pixel.png' } }); + assert.equal(image.error, replaced); + + // The remedy the refusal names: a sync with acceptRoot takes the root as it is now. + const accepted = await workspace.handleToolCall({ id: 'accept', name: 'sync', input: { mount: 'work', acceptRoot: true } }); + assert.equal(accepted.success, true, accepted.error); + assert.equal((await workspace.readFileFromDisk('work/secret.txt')).bytes.toString(), 'SECRET'); + }); +} + +test('a root replaced by another directory between its check and the open is refused, not read', async (t) => { + const { root, mountDir, workspace } = await recorded(t); + const file = join(mountDir, 'a.txt'); + let replaced = false; + const err = await withFsPromises('open', (open) => async (path: unknown, ...rest: unknown[]) => { + if (!replaced && String(path) === file) { + replaced = true; // the root was checked; the file is about to be opened through its path + renameSync(mountDir, join(root, 'mount-away')); + mkdirSync(mountDir); + writeFileSync(file, 'the replacement'); + } + return open(path, ...rest); + }, () => workspace.readFileFromDisk('work/a.txt').then( + (read) => assert.fail(`read the replacement's file: ${read.bytes.toString()}`), + (e: unknown) => e, + )); + assert.equal(replaced, true, 'the swap ran'); + assert.ok(err instanceof WorkspaceReadError && err.code === 'mount_unavailable', `refused: ${String(err)}`); + assert.equal(err.message, `Mount unavailable: work: ${ROOT_REPLACED}`); + assert.equal(err.stage, 'root'); +}); + +test('a mount whose directory is gone, or is a file, is unavailable before anything beneath it is looked at, with the errno a path there gets', async (t) => { + const { root, mountDir, workspace } = setup(t); + writeFileSync(join(mountDir, 'a.txt'), 'inside'); + renameSync(mountDir, join(root, 'mount-away')); + const gone = await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable'); + assert.equal(gone.message, 'Mount unavailable: work'); + assert.equal(gone.errno, 'ENOENT'); + // readBinaryFromDisk answers with the refusal's message, where it said "File not found on disk". + assert.deepEqual(await workspace.readBinaryFromDisk('work/a.txt'), { error: 'Mount unavailable: work' }); + writeFileSync(mountDir, 'a file where the directory was'); + const file = await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable'); + assert.equal(file.message, `Mount unavailable: work: ${ROOT_UNAVAILABLE}`); + assert.equal(file.errno, 'ENOTDIR'); + assert.deepEqual(await workspace.readBinaryFromDisk('work/a.txt'), { error: `Mount unavailable: work: ${ROOT_UNAVAILABLE}` }); +}); + +// The root is checked before anything beneath it, and asked again before +// the read answers anything about a path beneath it: a missing file, a file +// that changed, or the file it goes on to read. Each time, what is at the +// root's path then decides the code and errno, as at the first check. +const MOMENTS = { + // The file's lstat, the first thing looked up beneath the root. + 'before the file is found': { call: 'lstat', nth: 1, after: false }, + // The file's realpath, its recheck once it's open. + 'between the open and the file\'s recheck': { call: 'realpath', nth: 1, after: false }, + // The stat of the file's canonical path, the last check before the root's own. + 'once the file is open': { call: 'stat', nth: 1, after: true }, +} as const; +for (const [moment, { call, nth, after }] of Object.entries(MOMENTS)) { + for (const state of ['removed', 'replaced by a file', 'replaced by another directory', 'replaced by another directory holding the file'] as const) { + const skip = process.platform === 'win32' && moment !== 'before the file is found' + ? 'it renames the directory of a file the read holds open, which Windows may refuse' + : false; + test(`a root ${state} ${moment} is refused with the code and errno the root's check gives that state`, { skip }, async (t) => { + const { root, mountDir, workspace } = await recorded(t); + const file = join(mountDir, 'a.txt'); + const at = call === 'stat' ? realpathSync(file) : file; + let calls = 0; + let moved = false; + const move = () => { + moved = true; + renameSync(mountDir, join(root, 'mount-away')); + if (state === 'replaced by a file') writeFileSync(mountDir, 'a file where the directory was'); + if (state.startsWith('replaced by another directory')) mkdirSync(mountDir); + if (state === 'replaced by another directory holding the file') writeFileSync(file, 'the replacement'); + }; + const err = await withFsPromises(call, (original) => async (path: unknown, ...rest: unknown[]) => { + if (moved || String(path) !== at || ++calls !== nth) return original(path, ...rest); + if (!after) move(); + const result = await original(path, ...rest); + if (after) move(); + return result; + }, () => workspace.readFileFromDisk('work/a.txt').then( + (read) => assert.fail(`read through the moved root: ${read.bytes.toString()}`), + (e: unknown) => e, + )); + assert.equal(moved, true, 'the root moved'); + assert.ok(err instanceof WorkspaceReadError && err.code === 'mount_unavailable', `refused as unavailable: ${String(err)}`); + assert.equal(err.stage, 'root'); + if (state.startsWith('replaced by another directory')) { + assert.equal(err.message, `Mount unavailable: work: ${ROOT_REPLACED}`); + assert.equal(err.errno, undefined, 'no errno: another directory is not a missing one'); + } else { + assert.equal(err.message, `Mount unavailable: work: ${ROOT_UNAVAILABLE}`); + assert.equal(err.errno, state === 'removed' ? 'ENOENT' : 'ENOTDIR', 'the errno a root in that state gets before the read'); + } + }); + } +} + +// A mount configured through a link, re-pointed to another directory while a +// read runs, as a blue-green deploy swaps one. The directory the read checked +// is untouched: only the configured path's realpath shows that the mount is +// now another directory. +for (const [moment, { call, nth, after }] of Object.entries(MOMENTS)) { + const skip = process.platform === 'win32' ? 'it renames a link over the mount\'s link, which Windows may refuse' : false; + test(`a mount configured through a link that is re-pointed ${moment} is refused as replaced, not read`, { skip }, async (t) => { + const root = mkdtempSync(join(tmpdir(), 'af-read-from-disk-')); + let store: JsStore | undefined; + t.after(() => { + store?.close(); + rmSync(root, { recursive: true, force: true }); + }); + const blue = join(root, 'blue'); + const green = join(root, 'green'); + mkdirSync(blue); + mkdirSync(green); + writeFileSync(join(blue, 'a.txt'), 'blue'); + writeFileSync(join(green, 'a.txt'), 'green'); + const link = join(root, 'current'); + if (!symlinkOrSkip(t, blue, link, 'dir')) return; + store = JsStore.openOrCreate({ path: join(root, 'workspace.chronicle') }); + const workspace = new WorkspaceModule({ mounts: [{ name: 'work', path: link, mode: 'read-write', watch: 'never' }] }); + workspace.initStore(store); + const synced = await workspace.handleToolCall({ id: 'seed', name: 'sync', input: { mount: 'work' } }); + assert.equal(synced.success, true, synced.error); + + const file = join(link, 'a.txt'); + const at = call === 'stat' ? realpathSync(file) : file; + let calls = 0; + let moved = false; + const repoint = () => { + moved = true; + symlinkSync(green, join(root, 'next'), 'dir'); + renameSync(join(root, 'next'), link); + }; + const err = await withFsPromises(call, (original) => async (path: unknown, ...rest: unknown[]) => { + if (moved || String(path) !== at || ++calls !== nth) return original(path, ...rest); + if (!after) repoint(); + const result = await original(path, ...rest); + if (after) repoint(); + return result; + }, () => workspace.readFileFromDisk('work/a.txt').then( + (read) => assert.fail(`read through the re-pointed link: ${read.bytes.toString()}`), + (e: unknown) => e, + )); + assert.equal(moved, true, 'the link was re-pointed'); + assert.ok(err instanceof WorkspaceReadError && err.code === 'mount_unavailable', `refused as unavailable: ${String(err)}`); + assert.equal(err.message, `Mount unavailable: work: ${ROOT_REPLACED}`); + assert.equal(err.stage, 'root'); + assert.equal(err.errno, undefined, 'no errno: another directory is not a missing one'); + }); +} + +test('a mount with no recorded identity is asked about its root mid-read too', async (t) => { + const { root, mountDir, workspace } = setup(t); + const file = join(mountDir, 'a.txt'); + writeFileSync(file, 'inside'); + let moved = false; + const err = await withFsPromises('lstat', (lstat) => async (path: unknown, ...rest: unknown[]) => { + if (!moved && String(path) === file) { + moved = true; // the root was checked, with no identity to compare; the file is looked up next + renameSync(mountDir, join(root, 'mount-away')); + mkdirSync(mountDir); + } + return lstat(path, ...rest); + }, () => workspace.readFileFromDisk('work/a.txt').then( + (read) => assert.fail(`read through the moved root: ${read.bytes.toString()}`), + (e: unknown) => e, + )); + assert.equal(moved, true, 'the root moved'); + assert.ok(err instanceof WorkspaceReadError && err.code === 'mount_unavailable', `refused as unavailable: ${String(err)}`); + assert.equal(err.stage, 'root'); + assert.equal(err.errno, undefined, 'not reported missing from the directory put in its place'); +}); + +test('while the disk evidence awaits reopening, a read goes on, checked against the root identity last recorded', async (t) => { + const { root, mountDir, store, workspace } = await recorded(t); + writeFileSync(join(mountDir, 'b.txt'), 'more'); + // An append to the evidence fails inside the store, so the evidence can't + // be read again until the store is reopened. + const patched = store as unknown as { appendJson: (type: string, payload: unknown) => unknown }; + const append = patched.appendJson; + patched.appendJson = function (this: unknown, type: string, payload: unknown) { + if (type === 'workspace/disk-agreement') throw new Error('injected: the store refused the append'); + return append.call(this, type, payload); + }; + try { + const synced = await workspace.handleToolCall({ id: 'sync', name: 'sync', input: { mount: 'work' } }); + assert.equal(synced.success, false, 'the sync that would record b.txt failed'); + } finally { + patched.appendJson = append; + } + const again = await workspace.handleToolCall({ id: 'again', name: 'sync', input: { mount: 'work' } }); + assert.match(again.error ?? '', /until the store is reopened/, 'the evidence awaits reopening'); + + assert.equal((await workspace.readFileFromDisk('work/a.txt')).bytes.toString(), 'inside', 'the mount\'s own file is read'); + renameSync(mountDir, join(root, 'mount-away')); + mkdirSync(mountDir); + writeFileSync(join(mountDir, 'a.txt'), 'the replacement'); + const err = await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable'); + assert.equal(err.message, `Mount unavailable: work: ${ROOT_REPLACED}`, 'another directory is refused, as before the failure'); +}); + +test("a sync with acceptRoot whose append fails leaves reads refusing the root it was to accept", async (t) => { + const { root, mountDir, store, workspace } = await recorded(t); + renameSync(mountDir, join(root, 'mount-away')); + mkdirSync(mountDir); + writeFileSync(join(mountDir, 'a.txt'), 'the replacement'); + const patched = store as unknown as { appendJson: (type: string, payload: unknown) => unknown }; + const append = patched.appendJson; + patched.appendJson = function (this: unknown, type: string, payload: unknown) { + if (type === 'workspace/disk-agreement') throw new Error('injected: the store refused the append'); + return append.call(this, type, payload); + }; + try { + const accepted = await workspace.handleToolCall({ id: 'accept', name: 'sync', input: { mount: 'work', acceptRoot: true } }); + assert.equal(accepted.success, false, 'the sync whose append failed fails'); + } finally { + patched.appendJson = append; + } + // The identity in memory is still the one from before: append comes before apply. + const err = await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable'); + assert.equal(err.message, `Mount unavailable: work: ${ROOT_REPLACED}`); +}); + +/** + * Mounts recorded in a store, which is then closed and opened again, as at a + * restart, by a workspace whose mounts are `again`, with `breaking` making the + * store fail as that open reads or writes the disk evidence. Its initStore + * throws; the workspace is returned for reads all the same. + */ +async function reopenedBroken( + t: TestContext, + first: Array<{ name: string; path: string }>, + again: Array<{ name: string; path: string }>, + breaking: (store: Record unknown>) => void, +) { + const root = mkdtempSync(join(tmpdir(), 'af-read-from-disk-')); + const path = join(root, 'workspace.chronicle'); + let store: JsStore | undefined; + t.after(() => { + store?.close(); + rmSync(root, { recursive: true, force: true }); + }); + const mounts = (list: typeof first) => list.map(({ name, path: dir }) => ({ name, path: join(root, dir), mode: 'read-write' as const, watch: 'never' as const })); + for (const { path: dir } of [...first, ...again]) mkdirSync(join(root, dir), { recursive: true }); + for (const { path: dir } of first) writeFileSync(join(root, dir, 'a.txt'), `inside ${dir}`); + store = JsStore.openOrCreate({ path }); + const before = new WorkspaceModule({ mounts: mounts(first) }); + before.initStore(store); + for (const { name } of first) { + const synced = await before.handleToolCall({ id: `seed-${name}`, name: 'sync', input: { mount: name } }); + assert.equal(synced.success, true, synced.error); + } + store.close(); + store = JsStore.openOrCreate({ path }); + breaking(store as unknown as Record unknown>); + const workspace = new WorkspaceModule({ mounts: mounts(again) }); + assert.throws(() => workspace.initStore(store!), /injected/, 'the open fails'); + return { root, workspace }; +} + +test("a workspace whose open couldn't read its disk evidence back refuses disk reads, with no directory's errno only where there's none", async (t) => { + const mount = [{ name: 'work', path: 'mount' }]; + const { root, workspace } = await reopenedBroken(t, mount, mount, (store) => { + const ids = store.getRecordIdsByType!; + store.getRecordIdsByType = function (this: unknown, type: string) { + if (String(type).startsWith('workspace/disk-agreement')) throw new Error('injected: the store could not be read'); + return ids.call(this, type); + }; + }); + const err = await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable'); + assert.equal(err.stage, 'root'); + assert.match(err.message, /^Mount unavailable: work: Disk-agreement evidence is unavailable until the store is reopened/); + assert.equal(err.errno, undefined, 'a directory is there'); + rmSync(join(root, 'mount'), { recursive: true }); + writeFileSync(join(root, 'mount'), 'a file where the directory was'); + const file = await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable'); + assert.equal(file.errno, 'ENOTDIR', 'no directory is there, whatever the evidence'); +}); + +test("a workspace whose open failed to record a changed mount refuses disk reads of every mount, the ones after it included", async (t) => { + // The first mount's directory changed since the evidence was recorded, so + // the open appends its record first, and that append fails in the store. + const { root, workspace } = await reopenedBroken(t, + [{ name: 'work', path: 'mount' }, { name: 'other', path: 'other' }], + [{ name: 'work', path: 'moved' }, { name: 'other', path: 'other' }], + (store) => { + const append = store.appendJson!; + store.appendJson = function (this: unknown, type: string, payload: unknown) { + if (type === 'workspace/disk-agreement') throw new Error('injected: the store refused the append'); + return append.call(this, type, payload); + }; + }); + // A directory put at the later mount's root is refused, not read unchecked. + renameSync(join(root, 'other'), join(root, 'other-away')); + mkdirSync(join(root, 'other')); + writeFileSync(join(root, 'other', 'a.txt'), 'the replacement'); + const other = await expectRefusal(workspace.readFileFromDisk('other/a.txt'), 'mount_unavailable'); + assert.match(other.message, /^Mount unavailable: other: Disk-agreement evidence is unavailable until the store is reopened/); + const work = await expectRefusal(workspace.readFileFromDisk('work/a.txt'), 'mount_unavailable'); + assert.match(work.message, /^Mount unavailable: work: Disk-agreement evidence is unavailable until the store is reopened/); +}); diff --git a/test/workspace-three-way.test.ts b/test/workspace-three-way.test.ts index f7ac690aa..77f8464f6 100644 --- a/test/workspace-three-way.test.ts +++ b/test/workspace-three-way.test.ts @@ -13,7 +13,7 @@ import { describe, test, type TestContext } from 'node:test'; import { TOOL_BATCH_HOOK_TIMEOUT_MS } from '../src/module-registry.js'; import assert from 'node:assert/strict'; import { spawnSync } from 'node:child_process'; -import { appendFileSync, chmodSync, constants as fsConstants, existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, readdirSync, readFileSync, realpathSync, renameSync, rmSync, statSync, symlinkSync, truncateSync, unlinkSync, utimesSync, writeFileSync } from 'node:fs'; +import { appendFileSync, chmodSync, closeSync, constants as fsConstants, existsSync, linkSync, lstatSync, mkdirSync, mkdtempSync, openSync, readdirSync, readFileSync, realpathSync, renameSync, rmSync, statSync, symlinkSync, truncateSync, unlinkSync, utimesSync, writeFileSync } from 'node:fs'; import { open, statfs } from 'node:fs/promises'; import { createHash } from 'node:crypto'; import { createRequire, syncBuiltinESMExports } from 'node:module'; @@ -3891,6 +3891,32 @@ describe('restarts and faults', () => { assert.equal(agreement.needsBarrier, true); }); + test('a rebuild whose append fails partway leaves memory as it was, whole, with the identity a disk read checks', (t) => { + // Nothing reaches this today: after the open, a rebuild finds every + // mount's record and appends none. A later open with a moved mount does. + const env = new Env(t); + const agreement = new DiskAgreement(env.store); + agreement.open([{ name: 'work', root: '/srv/work' }, { name: 'other', root: '/srv/other' }]); + const identity = { dev: '1', ino: '2' }; + agreement.recordRootIdentity('other', identity); + const store = env.store as unknown as { appendJson: (type: string, payload: unknown) => unknown }; + const append = store.appendJson; + store.appendJson = function (this: unknown, type: string, payload: unknown) { + if (type === DISK_AGREEMENT_RECORD_TYPE) throw new Error('injected: the store refused the append'); + return append.call(this, type, payload); + }; + try { + // 'work' has moved, so the rebuild appends its record before it reaches 'other'. + assert.throws(() => agreement.open([{ name: 'work', root: '/srv/moved' }, { name: 'other', root: '/srv/other' }]), /injected/); + } finally { + store.appendJson = append; + } + // Through a structural type, so this builds against a DiskAgreement + // without the accessor too, and fails there rather than not building. + const read = agreement as unknown as { rootIdentityForRead(mount: string): unknown }; + assert.deepEqual(read.rootIdentityForRead('other'), identity, 'the mount after the failed append keeps its identity'); + }); + test('an append that fails inside the store leaves no decision to stale evidence: passes refuse until a reopen, which finds the evidence', async (t) => { const env = new Env(t); let m = await env.open(); @@ -4745,6 +4771,40 @@ describe('trust and identity at the point of use', () => { assert.equal(read.hash, createHash('sha256').update(grown).digest('hex')); }); + test("a FIFO swapped in at a checked directory's path doesn't hold up its sync", { skip: process.platform === 'win32' ? 'no FIFOs or directory syncs' : false }, async (t) => { + const env = new Env(t); + mkdirSync(env.disk('sub'), { recursive: true }); + const rootReal = realpathSync(env.dir); + const dir = join(rootReal, 'sub'); + let swapped = false; + const synced = withFsPromises('open', (original) => async (...args: any[]) => { + if (!swapped && args[0] === dir) { + swapped = true; // checked as a directory, then a FIFO put at its path before the sync opens it + rmSync(dir, { recursive: true }); + assert.equal(spawnSync('mkfifo', [dir]).status, 0, 'mkfifo'); + } + return original(...args); + }, () => syncDirectories(viewOf(env.dir), rootReal, ['sub'])); + let timer: ReturnType | undefined; + try { + const failed = await Promise.race([synced, new Promise((_, reject) => { + // Only a sync held by the FIFO comes near this: without one, it settles at once. + timer = setTimeout(() => reject(new Error('the sync waited on the FIFO')), 10_000); + })]); + assert.equal(swapped, true, 'the swap ran'); + assert.match(failed.get('sub') ?? '', /changed while it was observed/); + } finally { + clearTimeout(timer); + // An open still waiting on the FIFO is let go before the mount is removed. + try { + closeSync(openSync(dir, fsConstants.O_WRONLY | fsConstants.O_NONBLOCK)); + } catch { + // ENXIO: nothing is waiting. + } + await synced.catch(() => {}); + } + }); + /** A root identified as it is now, and a way to replace it: renamed away, with an empty `sub` in its place. */ const replaceable = (env: Env) => { mkdirSync(env.disk('sub'), { recursive: true });