diff --git a/packages/core/src/agent/define.ts b/packages/core/src/agent/define.ts index 05077db..03f7d64 100644 --- a/packages/core/src/agent/define.ts +++ b/packages/core/src/agent/define.ts @@ -16,7 +16,7 @@ import type { HarnessAgentPermissionMode, } from '@ai-sdk/harness/agent' import type { SandboxDefinition } from '../sandbox/define' -import type { WorkspaceFiles, WorkspaceSource } from './workspace' +import type { Workspace, WorkspaceSource } from './workspace' import { HarnessAgent } from '@ai-sdk/harness/agent' import { resolveSandbox } from '../sandbox/define' import { createHarnessSandboxProvider } from '../sandbox/harness' @@ -93,7 +93,7 @@ export function defineAgent(definition: AgentDefinition): Agent { // Read once, not per session: the directory is the same for every session, and a local run // that creates several would otherwise walk the host filesystem again for each. - let workspace: Promise | undefined + let workspace: Promise | undefined /** * A `HarnessAgent` per session, deliberately. @@ -121,7 +121,7 @@ export function defineAgent(definition: AgentDefinition): Agent { workspace = undefined throw cause }) - await seedWorkspace(context.session, context.sessionWorkDir, await workspace) + await seedWorkspace(context.session, context.sessionWorkDir, (await workspace).files) } // The definition's own hook runs last, so it can overwrite anything the workspace // seeded rather than being overwritten by it. diff --git a/packages/core/src/agent/workspace.ts b/packages/core/src/agent/workspace.ts index 7ab69d9..191a418 100644 --- a/packages/core/src/agent/workspace.ts +++ b/packages/core/src/agent/workspace.ts @@ -13,6 +13,9 @@ * run does. A `Record` of path to content is what a Worker gets, where there is no filesystem * to read and the directory has to be inlined into the bundle ahead of time. */ +// Type-only, so it is erased at build and pulls no `node:fs` into a Worker bundle — the same +// reason every value import in this file is dynamic. +import type { Stats } from 'node:fs' /** Session-relative POSIX paths to their contents. The shape a Worker can carry. */ export type WorkspaceFiles = Readonly> @@ -30,58 +33,211 @@ function isFiles(source: WorkspaceSource): source is WorkspaceFiles { return typeof source !== 'string' && !(source instanceof URL) } +/** + * Directories skipped when the source is not a git repository. + * + * The primary ignore mechanism is the caller's own git rules, which are already maintained and + * already correct for their project. This list only covers a source that git cannot describe — + * a plain directory, or a host with no git — so it is deliberately short: the entries every + * ecosystem regenerates from source and nobody means to carry into a sandbox. + */ +export const WORKSPACE_IGNORED_DIRECTORIES: readonly string[] = [ + '.git', + 'node_modules', + 'dist', + 'build', + 'coverage', + '.next', + '.turbo', + '.cache', + 'out', +] + +/** Per-file ceiling. Above it the file is reported rather than read. */ +export const MAX_WORKSPACE_FILE_BYTES = 1024 * 1024 + +/** Ceiling across the whole workspace. Above it the seed is pathological and reading stops. */ +export const MAX_WORKSPACE_TOTAL_BYTES = 64 * 1024 * 1024 + +/** Why a file that survived the ignore rules still did not make it into the workspace. */ +export interface SkippedWorkspaceFile { + /** The path relative to the source root, in the same shape {@link WorkspaceFiles} keys use. */ + readonly path: string + readonly reason: 'binary' | 'too-large' +} + +/** + * What a source read produced. + * + * `skipped` is returned rather than logged because this package has no logger, and rather than + * handed to a callback because "reported" has to survive into something the caller can assert + * on — a workspace that quietly lost a file is the failure mode the report exists to prevent. + */ +export interface Workspace { + readonly files: WorkspaceFiles + readonly skipped: readonly SkippedWorkspaceFile[] +} + /** * Bytes to text, refusing rather than mangling. * * A workspace carries text by construction — {@link WorkspaceFiles} is a `Record` of strings and * the seed writes every entry with `writeTextFile`. A non-fatal UTF-8 decode turns a PNG, a * `.git` pack or a prebuilt binary into a string of U+FFFD and reports success, so the sandbox - * ends up holding a corrupt file under the original's name with nothing to notice it by. Naming - * the file is the only useful answer. + * ends up holding a corrupt file under the original's name with nothing to notice it by. So the + * detection stays; `undefined` is what a caller that has other files to read does with it. */ -function decodeText(bytes: Uint8Array, path: string): string { +function decodeText(bytes: Uint8Array): string | undefined { try { return new TextDecoder('utf-8', { fatal: true }).decode(bytes) } catch { - throw new TypeError(`workspace file '${path}' is not valid UTF-8; a workspace carries text only`) + return undefined } } /** - * Read a workspace source into files. + * The files git would show for the source, or `undefined` when git cannot describe it. * - * `node:fs` is reached through a dynamic import so that a bundle which never passes a path — - * the Worker case, where the directory is inlined at build time — does not pull the host - * filesystem in behind it. + * `ls-files -co --exclude-standard` is exactly the tracked plus untracked-not-ignored set, which + * means the project's own `.gitignore` — the rules its author already maintains — decides what a + * workspace carries, with no gitignore parser of ours to disagree with git about. `-z` because a + * newline is a legal character in a filename and the line-oriented form would split one in two. + * + * `node:child_process` is reached through a dynamic import for the same reason `node:fs` is: a + * bundle that only ever passes an inlined record must not pull the host process surface in. */ -export async function readWorkspace(source: WorkspaceSource): Promise { - if (isFiles(source)) { - return source +async function gitCandidates(root: string): Promise { + const [{ execFile }, { promisify }] = await Promise.all([ + import('node:child_process'), + import('node:util'), + ]) + + try { + const { stdout } = await promisify(execFile)( + 'git', + ['ls-files', '-co', '--exclude-standard', '-z'], + // Paths come out relative to the cwd, which is the shape `WorkspaceFiles` keys already use. + { cwd: root, maxBuffer: MAX_WORKSPACE_TOTAL_BYTES }, + ) + return stdout.split('\0').filter(entry => entry !== '') + } + catch (cause) { + // An output limit is the one failure that must not fall back. `walkCandidates` honours no + // `.gitignore`, so a repository whose file list is merely too long would silently start + // carrying the very `node_modules` this function exists to leave behind — the failure this + // whole path was built to prevent, reached through its own guard. + if ((cause as { code?: string } | undefined)?.code === 'ERR_CHILD_PROCESS_STDIO_MAXBUFFER') { + throw new RangeError( + `'${root}' lists more than ${MAX_WORKSPACE_TOTAL_BYTES} bytes of file names; ` + + 'narrow the workspace rather than letting it fall back to an unfiltered walk', + ) + } + // Everything else — not a repository, git not installed, git refusing the directory as + // unsafe — means the same thing here: there are no user rules to honour, so fall back. + return undefined } +} - const [{ readdir, readFile, stat }, { fileURLToPath }, { join }] = await Promise.all([ +/** + * Stat a candidate, treating only a vanished entry as nothing to read. + * + * A tracked file git lists but the working tree no longer holds, or a directory entry such as a + * submodule gitlink, is not a file and not worth reporting. Any other failure — a permission + * denial above all — is a file the caller meant to carry and cannot, so it is raised rather than + * dropped into the silence this module exists to remove. + */ +async function statCandidate(absolute: string): Promise { + const { stat } = await import('node:fs/promises') + try { + return await stat(absolute) + } + catch (cause) { + const code = (cause as { code?: string } | undefined)?.code + if (code === 'ENOENT' || code === 'ENOTDIR') { + return undefined + } + throw cause + } +} + +/** The files a plain directory walk shows, minus {@link WORKSPACE_IGNORED_DIRECTORIES}. */ +async function walkCandidates(root: string): Promise { + const { readdir } = await import('node:fs/promises') + + // `recursive` returns paths relative to the root, dotfiles included — which is the point, + // since `.claude/` is the most interesting thing a workspace carries. + const entries = await readdir(root, { recursive: true }) + return entries.filter(entry => !entry + .split(/[/\\]/) + .some(segment => WORKSPACE_IGNORED_DIRECTORIES.includes(segment))) +} + +/** + * Read the listed candidates, reporting the ones that cannot travel as text. + * + * Once the ignore rules have narrowed the set, a remaining binary is real project content — an + * icon, a font — rather than something a broad walk swept up, so refusing the whole seed over it + * would fail the case this exists to serve. The total cap is the exception: a workspace that far + * over the line is a mistake about which directory was handed over, and stopping says so. + */ +async function readCandidates(root: string, candidates: readonly string[]): Promise { + const [{ readFile }, { join }] = await Promise.all([ import('node:fs/promises'), - import('node:url'), import('node:path'), ]) - // `join` rather than a template, so a source that already ends in a separator does not - // produce a doubled one — and a filesystem root, which has nothing to strip, still reads. - const root = source instanceof URL ? fileURLToPath(source) : source const files: Record = {} - // `recursive` returns paths relative to the root, dotfiles included — which is the point, - // since `.claude/` is the most interesting thing a workspace carries. - for (const entry of await readdir(root, { recursive: true })) { + const skipped: SkippedWorkspaceFile[] = [] + let total = 0 + + for (const entry of candidates) { + // `join` rather than a template, so a root that already ends in a separator does not + // produce a doubled one — and a filesystem root, which has nothing to strip, still reads. const absolute = join(root, entry) - if (!(await stat(absolute)).isFile()) { + const stats = await statCandidate(absolute) + if (stats === undefined || !stats.isFile()) { continue } // The sandbox is Linux whatever the host is, so a Windows separator is rewritten rather // than carried into a container path. - files[entry.split('\\').join('/')] = decodeText(await readFile(absolute), absolute) + const path = entry.split('\\').join('/') + if (stats.size > MAX_WORKSPACE_FILE_BYTES) { + skipped.push({ path, reason: 'too-large' }) + continue + } + total += stats.size + if (total > MAX_WORKSPACE_TOTAL_BYTES) { + throw new RangeError( + `workspace '${root}' exceeds the ${MAX_WORKSPACE_TOTAL_BYTES} byte total limit`, + ) + } + const content = decodeText(await readFile(absolute)) + if (content === undefined) { + skipped.push({ path, reason: 'binary' }) + continue + } + files[path] = content + } + + return { files, skipped } +} + +/** + * Read a workspace source into files. + * + * `node:fs` is reached through a dynamic import so that a bundle which never passes a path — + * the Worker case, where the directory is inlined at build time — does not pull the host + * filesystem in behind it. + */ +export async function readWorkspace(source: WorkspaceSource): Promise { + if (isFiles(source)) { + return { files: source, skipped: [] } } - return files + + const { fileURLToPath } = await import('node:url') + const root = source instanceof URL ? fileURLToPath(source) : source + return readCandidates(root, await gitCandidates(root) ?? await walkCandidates(root)) } /** diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts index 6d2c52a..b767eca 100644 --- a/packages/core/src/index.ts +++ b/packages/core/src/index.ts @@ -19,4 +19,10 @@ export type { } from './agent/define' export { readWorkspace, seedWorkspace } from './agent/workspace' -export type { WorkspaceFiles, WorkspaceSource, WorkspaceWriter } from './agent/workspace' +export type { + SkippedWorkspaceFile, + Workspace, + WorkspaceFiles, + WorkspaceSource, + WorkspaceWriter, +} from './agent/workspace' diff --git a/packages/core/test/agent/workspace.test.ts b/packages/core/test/agent/workspace.test.ts index d03a5e6..6f824ac 100644 --- a/packages/core/test/agent/workspace.test.ts +++ b/packages/core/test/agent/workspace.test.ts @@ -1,10 +1,18 @@ import type { WorkspaceWriter } from '../../src/agent/workspace' import { Buffer } from 'node:buffer' -import { mkdir, mkdtemp, writeFile } from 'node:fs/promises' +import { execFile } from 'node:child_process' +import { chmod, mkdir, mkdtemp, rm, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' import { join } from 'node:path' +import { promisify } from 'node:util' import { describe, expect, it } from 'bun:test' -import { readWorkspace, seedWorkspace } from '../../src/agent/workspace' +import { + MAX_WORKSPACE_FILE_BYTES, + MAX_WORKSPACE_TOTAL_BYTES, + readWorkspace, + seedWorkspace, + WORKSPACE_IGNORED_DIRECTORIES, +} from '../../src/agent/workspace' async function fixture(): Promise { const root = await mkdtemp(join(tmpdir(), 'please-workspace-')) @@ -14,6 +22,17 @@ async function fixture(): Promise { return root } +async function gitFixture(): Promise { + const root = await fixture() + await promisify(execFile)('git', ['init', '-q'], { cwd: root }) + await writeFile(join(root, '.gitignore'), 'ignored/\n') + await mkdir(join(root, 'ignored'), { recursive: true }) + await writeFile(join(root, 'ignored', 'artifact.txt'), 'build output\n') + await writeFile(join(root, 'untracked.md'), '# untracked\n') + await promisify(execFile)('git', ['add', 'CLAUDE.md'], { cwd: root }) + return root +} + function recorder(): { writes: { path: string, content: string }[] } & WorkspaceWriter { const writes: { path: string, content: string }[] = [] return { @@ -26,7 +45,7 @@ function recorder(): { writes: { path: string, content: string }[] } & Workspace describe('readWorkspace', () => { it('reads a directory recursively, dotfiles included', async () => { - const files = await readWorkspace(await fixture()) + const { files } = await readWorkspace(await fixture()) // `.claude/` is the reason this walks hidden entries at all: it is what carries an // existing Claude Code project's agents, skills and settings into a sandbox. @@ -45,7 +64,7 @@ describe('readWorkspace', () => { }) it('accepts a file URL', async () => { - const files = await readWorkspace(new URL(`file://${await fixture()}/`)) + const { files } = await readWorkspace(new URL(`file://${await fixture()}/`)) expect(Object.keys(files)).toContain('CLAUDE.md') }) @@ -54,17 +73,124 @@ describe('readWorkspace', () => { const inlined = { 'CLAUDE.md': '# inlined\n' } // The Worker case: no filesystem to read, the directory having been inlined at build time. - expect(await readWorkspace(inlined)).toBe(inlined) + expect((await readWorkspace(inlined)).files).toBe(inlined) }) - it('refuses a file that is not UTF-8 rather than seeding a mangled copy of it', async () => { + it('skips a file that is not UTF-8 and reports it rather than failing the read', async () => { const root = await fixture() // A PNG header: valid bytes, not valid UTF-8. Decoded leniently it becomes a string of // U+FFFD, which would be written into the sandbox under the original's name and reported - // as a success — a corrupt file with nothing to notice it by. + // as a success — a corrupt file with nothing to notice it by. Once the ignore rules have + // narrowed the set it is real project content, so the read continues without it. await writeFile(join(root, 'logo.png'), Buffer.from([0x89, 0x50, 0x4E, 0x47, 0x0D, 0x0A])) - await expect(readWorkspace(root)).rejects.toThrow(/logo\.png/) + const { files, skipped } = await readWorkspace(root) + + expect(skipped).toEqual([{ path: 'logo.png', reason: 'binary' }]) + expect(Object.keys(files)).toContain('CLAUDE.md') + expect(Object.keys(files)).not.toContain('logo.png') + }) + + it('honours the source repository\'s own git ignore rules', async () => { + const { files } = await readWorkspace(await gitFixture()) + + // `git ls-files -co --exclude-standard` is the tracked plus untracked-not-ignored set, so + // the rules the project already maintains decide what travels — no parser of ours to + // disagree with git about, and an untracked file is still carried unless git ignores it. + expect(Object.keys(files).sort()).toEqual([ + '.claude/agents/verifier.md', + '.gitignore', + 'CLAUDE.md', + 'untracked.md', + ]) + }) + + it('falls back to the built-in deny list when the source is not a repository', async () => { + const root = await fixture() + await mkdir(join(root, 'node_modules'), { recursive: true }) + await writeFile(join(root, 'node_modules', 'x.js'), 'module.exports = {}\n') + + const { files } = await readWorkspace(root) + + expect(WORKSPACE_IGNORED_DIRECTORIES).toContain('node_modules') + expect(Object.keys(files)).not.toContain('node_modules/x.js') + expect(Object.keys(files)).toContain('CLAUDE.md') + }) + + it('reports a file over the per-file cap instead of reading it', async () => { + const root = await fixture() + await writeFile(join(root, 'huge.txt'), 'x'.repeat(MAX_WORKSPACE_FILE_BYTES + 1)) + + const { files, skipped } = await readWorkspace(root) + + expect(skipped).toEqual([{ path: 'huge.txt', reason: 'too-large' }]) + expect(Object.keys(files)).toContain('CLAUDE.md') + }) + + it('throws when the whole workspace exceeds the total cap', async () => { + const root = await fixture() + // Under the per-file cap each, over the total together: the per-file rule cannot catch a + // workspace that is simply the wrong directory, which is what the total cap is for. + const chunk = 'x'.repeat(MAX_WORKSPACE_FILE_BYTES) + for (let index = 0; index <= MAX_WORKSPACE_TOTAL_BYTES / MAX_WORKSPACE_FILE_BYTES; index += 1) { + await writeFile(join(root, `chunk-${index}.txt`), chunk) + } + + // Silence would be the worse answer here: a seed this size is a mistake about which + // directory was handed over, so the error says what it exceeded. + await expect(readWorkspace(root)).rejects.toThrow(String(MAX_WORKSPACE_TOTAL_BYTES)) + }) + + it('refuses a repository whose file list overruns the buffer rather than walking it', async () => { + const root = await gitFixture() + // A `git` that outpaces `maxBuffer`, which is what a repository with an enormous file list + // does. Falling back on that error would drop the ignore rules and start carrying the very + // trees this path exists to leave behind — the failure reached through its own guard. + const shim = await mkdtemp(join(tmpdir(), 'please-git-shim-')) + await writeFile(join(shim, 'git'), '#!/bin/sh\nhead -c 70000000 /dev/zero\n', { mode: 0o755 }) + const path = process.env.PATH + + process.env.PATH = `${shim}:${path ?? ''}` + try { + await expect(readWorkspace(root)).rejects.toThrow(String(MAX_WORKSPACE_TOTAL_BYTES)) + } + finally { + process.env.PATH = path + await rm(shim, { recursive: true, force: true }) + } + }) + + it('skips a tracked file the working tree no longer holds', async () => { + const root = await gitFixture() + // `git ls-files` reads the index, so a file deleted after `git add` is still listed. It is + // not there to read and it is not a file the caller lost — nothing to report. + await rm(join(root, 'CLAUDE.md')) + + const { files, skipped } = await readWorkspace(root) + + expect(Object.keys(files)).not.toContain('CLAUDE.md') + expect(skipped).toEqual([]) + }) + + it('raises when a listed file cannot be stat-ed for a reason other than absence', async () => { + // Root ignores the directory mode, so there is nothing to deny and nothing to observe. + if (process.getuid?.() === 0) { + return + } + const root = await gitFixture() + await mkdir(join(root, 'locked'), { recursive: true }) + await writeFile(join(root, 'locked', 'secret.md'), '# secret\n') + await promisify(execFile)('git', ['add', 'locked/secret.md'], { cwd: root }) + await chmod(join(root, 'locked'), 0o000) + + try { + // The file exists and was asked for; the seed simply cannot read it. Dropping it would + // hand over a workspace missing a file with nothing to say so. + await expect(readWorkspace(root)).rejects.toThrow(/EACCES|permission denied/i) + } + finally { + await chmod(join(root, 'locked'), 0o700) + } }) })