diff --git a/packages/runtime/src/__tests__/builtin-tools.test.ts b/packages/runtime/src/__tests__/builtin-tools.test.ts index 361c070fefa..b8eb4cec5b3 100644 --- a/packages/runtime/src/__tests__/builtin-tools.test.ts +++ b/packages/runtime/src/__tests__/builtin-tools.test.ts @@ -2181,6 +2181,69 @@ describe('builtin read tools path containment', () => { ); assert.strictEqual(JSON.stringify(absoluteGrepResult).includes('main.ts'), true); }); + + test('Glob brace expansion cannot list entries outside the session cwd', async () => { + const root = await mkdtemp(join(tmpdir(), 'maka-glob-brace-')); + const outside = await mkdtemp(join(tmpdir(), 'maka-glob-outside-')); + try { + await mkdir(join(root, 'a'), { recursive: true }); + await mkdir(join(root, 'b'), { recursive: true }); + await writeFile(join(root, 'a', 'inner.txt'), 'in', 'utf8'); + await writeFile(join(root, 'b', 'other.txt'), 'other', 'utf8'); + await writeFile(join(outside, 'secret.txt'), 'out', 'utf8'); + const glob = tool('Glob'); + + // `{a,../*}` carries no literal `..` segment, so the lexical pattern guard + // admits it, yet the `../*` brace branch walks above the session cwd. + const escaped = await runTool(glob, { pattern: '{a,../*}' }, root); + const escapedFiles = (escaped as { files: string[] }).files; + assert.strictEqual( + escapedFiles.some((file) => file.startsWith('../')), + false, + `glob listed entries outside the session cwd: ${JSON.stringify(escapedFiles)}`, + ); + // The `../*` branch also matches the session cwd itself under its own + // name; glob reports that one as `.`, which is inside and stays. + assert.deepStrictEqual(escapedFiles.sort(), ['.', 'a']); + + // Brace patterns that stay inside keep their full results. + const contained = await runTool(glob, { pattern: '{a/*.txt,b/*.txt}' }, root); + assert.deepStrictEqual((contained as { files: string[] }).files.sort(), [ + 'a/inner.txt', + 'b/other.txt', + ]); + } finally { + await rm(root, { recursive: true, force: true }); + await rm(outside, { recursive: true, force: true }); + } + }); + + test('Glob drops matches reached through a directory symlink that leaves the session cwd', async () => { + const root = await mkdtemp(join(tmpdir(), 'maka-glob-link-')); + const outside = await mkdtemp(join(tmpdir(), 'maka-glob-linkout-')); + try { + await mkdir(join(root, 'src'), { recursive: true }); + await writeFile(join(root, 'src', 'inner.txt'), 'in', 'utf8'); + await writeFile(join(outside, 'secret.txt'), 'out', 'utf8'); + await symlink(outside, join(root, 'link-out')); + const glob = tool('Glob'); + + // `resolve()` normalizes the spelling but does not follow the directory + // symlink, so lexical containment admits `link-out/secret.txt` even + // though its real location is outside; containment must be decided in + // real-path space, matching the skills-discovery idiom. + const linked = await runTool(glob, { pattern: 'link-out/**/*.txt' }, root); + assert.deepStrictEqual((linked as { files: string[] }).files, []); + + // A symlinked directory that stays inside the root keeps its matches. + await symlink(join(root, 'src'), join(root, 'alias')); + const contained = await runTool(glob, { pattern: 'alias/*.txt' }, root); + assert.deepStrictEqual((contained as { files: string[] }).files, ['alias/inner.txt']); + } finally { + await rm(root, { recursive: true, force: true }); + await rm(outside, { recursive: true, force: true }); + } + }); }); describe('builtin write tools path containment', () => { diff --git a/packages/runtime/src/filesystem-executor.ts b/packages/runtime/src/filesystem-executor.ts index 99921db8dc2..02dc70a2dd9 100644 --- a/packages/runtime/src/filesystem-executor.ts +++ b/packages/runtime/src/filesystem-executor.ts @@ -666,6 +666,15 @@ function createWorkspaceFilesystemExecutor( const { files, truncated } = await workspace.globFiles({ cwd: base, pattern: operation.pattern, + // Brace expansion can walk above the session cwd without the + // pattern text ever containing a `..` segment, so the lexical + // guard alone cannot contain workspace globs; results are + // filtered against the session cwd instead. The root shares the + // canonical spelling of the search base (the walker's cwd), or a + // symlinked workspace root (/var → /private/var) would reject + // every legitimate entry. Host scope has no boundary to escape + // and keeps raw results. + ...(scope === 'workspace' ? { containmentRoot: await canonicalExistingPath(cwd) } : {}), ...(operation.limit !== undefined ? { limit: operation.limit } : {}), ...(abortSignal ? { abortSignal } : {}), }); diff --git a/packages/runtime/src/filesystem-worker/operations.ts b/packages/runtime/src/filesystem-worker/operations.ts index 24deca9ae0c..22428f6684c 100644 --- a/packages/runtime/src/filesystem-worker/operations.ts +++ b/packages/runtime/src/filesystem-worker/operations.ts @@ -392,7 +392,13 @@ export async function executeFilesystemOperation( )), }; } - return { kind: 'glob', ...(await globFiles({ ...operation, cwd: path })) }; + // Brace expansion can walk above the search root without the pattern + // text ever containing a `..` segment, so results are filtered back to + // the search root the lexical guard promises. + return { + kind: 'glob', + ...(await globFiles({ ...operation, cwd: path, containmentRoot: path })), + }; } case 'grep': { const path = await resolveExistingAllowed( diff --git a/packages/runtime/src/glob-search.ts b/packages/runtime/src/glob-search.ts index a514ab163fd..329efefa572 100644 --- a/packages/runtime/src/glob-search.ts +++ b/packages/runtime/src/glob-search.ts @@ -20,12 +20,23 @@ import { promises as fs, readdir } from 'node:fs'; import { resolve } from 'node:path'; import { globIterate } from 'glob'; +import { isPathInside } from './path-containment.js'; export async function globFiles(input: { cwd: string; pattern: string; limit?: number; abortSignal?: AbortSignal; + /** + * When set, matched paths that resolve outside this root are dropped. Glob + * expands braces before walking, so a lexically clean pattern like + * `{a,../*}` can still walk above its search root; the walk cannot be + * contained, but its results can. Matches are compared in real-path space: + * `resolve()` normalizes spellings but does not follow directory symlinks, + * so a pattern naming a `link -> ../outside` directory stays lexically + * inside while its real location escapes. + */ + containmentRoot?: string; }): Promise<{ files: string[]; truncated: boolean }> { let failure: NodeJS.ErrnoException | undefined; const directories = new Set([resolve(input.cwd)]); @@ -33,6 +44,13 @@ export async function globFiles(input: { const limit = input.limit ?? 200; let probingOverflow = false; let probeIncomplete = false; + // Containment is decided on real paths (skills-discovery idiom: realpath + // both sides, then the pure predicate). Cost: one realpath for the root plus + // at most `limit` for the kept-or-dropped matches, on an interactive tool + // call. A root that cannot be realpath'd is an infrastructure failure and + // propagates; the walk cannot claim containment it cannot place. + const containmentReal = + input.containmentRoot !== undefined ? await fs.realpath(input.containmentRoot) : undefined; function record(error: NodeJS.ErrnoException, path: string): void { // Speculative literal components may miss. A directory already admitted by // cwd, stat, or enumeration disappearing instead makes this walk incomplete. @@ -71,6 +89,21 @@ export async function globFiles(input: { }, }, })) { + // Dropped escapes do not count toward the limit: the cap bounds returned + // files, not walked ones. + if (containmentReal !== undefined) { + let matchReal: string; + try { + matchReal = await fs.realpath(resolve(input.cwd, file)); + } catch { + // The match cannot be placed in the root's path space. Dropping it + // silently would read the walk as complete, so flag incompleteness + // the way vanished directories already do. + probeIncomplete = true; + continue; + } + if (!isPathInside(containmentReal, matchReal)) continue; + } if (files.length >= limit) { // One match past the cap is the only proof the pattern had more to give. truncated = true; diff --git a/packages/runtime/src/workspace-executor.ts b/packages/runtime/src/workspace-executor.ts index fc09a8507bc..1980abe5779 100644 --- a/packages/runtime/src/workspace-executor.ts +++ b/packages/runtime/src/workspace-executor.ts @@ -202,6 +202,8 @@ export interface WorkspaceGlobInput { pattern: string; limit?: number; abortSignal?: AbortSignal; + /** When set, matched paths that resolve outside this root are dropped. */ + containmentRoot?: string; } export interface WorkspaceGlobResult {