diff --git a/.changeset/deep-experts-thank.md b/.changeset/deep-experts-thank.md new file mode 100644 index 00000000..d4dadb84 --- /dev/null +++ b/.changeset/deep-experts-thank.md @@ -0,0 +1,5 @@ +--- +"deepagents": patch +--- + +fix(deepagents): fast-glob follows directory symlink cycles leading to ELOOP crashes diff --git a/libs/deepagents/src/backends/filesystem.test.ts b/libs/deepagents/src/backends/filesystem.test.ts index cdbc7e4b..18cf14df 100644 --- a/libs/deepagents/src/backends/filesystem.test.ts +++ b/libs/deepagents/src/backends/filesystem.test.ts @@ -1,4 +1,4 @@ -import { describe, it, expect, beforeEach, afterEach } from "vitest"; +import { describe, it, expect, beforeEach, afterEach, vi } from "vitest"; import * as fs from "fs/promises"; import * as fsSync from "fs"; import * as path from "path"; @@ -532,3 +532,124 @@ describe("FilesystemBackend", () => { }); }); }); + +/** + * Returns true when the platform lets an unprivileged process create symlinks + */ +function symlinksSupported(): boolean { + const probe = fsSync.mkdtempSync( + path.join(os.tmpdir(), "deepagents-symlink-probe-"), + ); + try { + fsSync.symlinkSync("target", path.join(probe, "link")); + return true; + } catch { + return false; + } finally { + fsSync.rmSync(probe, { recursive: true, force: true }); + } +} +const CAN_SYMLINK = symlinksSupported(); + +describe("FilesystemBackend symlink cycle handling", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = createTempDir(); + }); + + afterEach(async () => { + await removeDir(tmpDir); + }); + + /** + * Build a tree with real files, an `ln -s . sub/sub` cycle (must not be + * descended into), and a symlink-to-file `alias.txt -> real.txt` (must still + * be reported). + */ + async function buildCycle(root: string) { + await writeFile(path.join(root, "real.txt"), "needle-content"); + await writeFile(path.join(root, "sub", "inner.txt"), "needle-content"); + await fs.symlink(".", path.join(root, "sub", "sub")); + await fs.symlink(path.join(root, "real.txt"), path.join(root, "alias.txt")); + } + + // A file reached by following the cycle looks like `.../sub/sub/inner.txt`; + // the real file is only ever `.../sub/inner.txt`. + const followedCycle = (p: string) => /sub[\\/]sub/.test(p); + + it.skipIf(!CAN_SYMLINK)( + "glob does not descend into a directory symlink cycle", + async () => { + await buildCycle(tmpDir); + const backend = new FilesystemBackend({ + rootDir: tmpDir, + virtualMode: false, + }); + + const result = await backend.glob("**/*", tmpDir); + + expect(result.error).toBeUndefined(); + const paths = result.files!.map((f) => f.path); + expect(paths.some((p) => p.endsWith("inner.txt"))).toBe(true); + expect(paths.some((p) => p.endsWith("real.txt"))).toBe(true); + // A symlink pointing at a regular file must still be reported (not + // silently dropped by `onlyFiles` + `followSymbolicLinks: false`). + expect(paths.some((p) => p.endsWith("alias.txt"))).toBe(true); + expect(paths.some(followedCycle)).toBe(false); + }, + ); + + it.skipIf(!CAN_SYMLINK)( + "grep does not descend into a directory symlink cycle", + async () => { + await buildCycle(tmpDir); + const backend = new FilesystemBackend({ + rootDir: tmpDir, + virtualMode: false, + }); + + const result = await backend.grep("needle-content", tmpDir); + + expect(result.error).toBeUndefined(); + const paths = (result.matches ?? []).map((m) => m.path); + expect(paths.some((p) => p.endsWith("inner.txt"))).toBe(true); + expect(paths.some(followedCycle)).toBe(false); + }, + ); + + it.skipIf(!CAN_SYMLINK)( + "grep fallback does not follow a symlink out of the search root", + async () => { + // A secret file OUTSIDE the workspace, reachable only via an in-tree + // symlink. Reading through the link would leak it past the root. + const outside = createTempDir(); + try { + await writeFile(path.join(outside, "secret.txt"), "EXFIL_MARKER"); + await writeFile(path.join(tmpDir, "normal.txt"), "nothing here"); + await fs.symlink( + path.join(outside, "secret.txt"), + path.join(tmpDir, "alias.txt"), + ); + + const backend = new FilesystemBackend({ + rootDir: tmpDir, + virtualMode: true, + }); + // Force the non-ripgrep path; that fallback is what this guards. + vi.spyOn( + backend as unknown as { ripgrepSearch: () => Promise }, + "ripgrepSearch", + ).mockResolvedValue(null); + + const result = await backend.grep("EXFIL_MARKER", "/"); + + // The marker exists only in the out-of-tree file; the fallback must not + // read it through the symlink. + expect(result.matches ?? []).toHaveLength(0); + } finally { + await removeDir(outside); + } + }, + ); +}); diff --git a/libs/deepagents/src/backends/filesystem.ts b/libs/deepagents/src/backends/filesystem.ts index 9286a98b..0be87ac3 100644 --- a/libs/deepagents/src/backends/filesystem.ts +++ b/libs/deepagents/src/backends/filesystem.ts @@ -619,12 +619,17 @@ export class FilesystemBackend implements BackendProtocolV2 { const stat = await fs.stat(baseFull); const root = stat.isDirectory() ? baseFull : path.dirname(baseFull); - // Use fast-glob to recursively find all files + // `onlyFiles: true` with `followSymbolicLinks: false` drops symlink entries + // at enumeration, so we never `readFile` a symlink target (which would + // bypass the O_NOFOLLOW protection used by read()/write()/edit() and escape + // the search root). This matches ripgrep's default no-follow behavior, so + // the fallback and primary grep paths return the same files. const files = await fg("**/*", { cwd: root, absolute: true, onlyFiles: true, dot: true, + followSymbolicLinks: false, }); for (const fp of files) { @@ -709,12 +714,20 @@ export class FilesystemBackend implements BackendProtocolV2 { const results: FileInfo[] = []; try { - // Use fast-glob for pattern matching + // `followSymbolicLinks: false` stops fast-glob from descending into + // symlinked directories, which otherwise loop forever on a self- + // referential symlink (e.g. `sub/sub -> .`) until the OS throws ELOOP. + // `onlyFiles: false` (rather than `true`) is deliberate: fast-glob's + // `onlyFiles` filter uses lstat and would drop symlinks-to-files entirely, + // regressing results like `alias.ts -> real.ts`. The `stat().isFile()` + // check below follows each link to re-include those files while excluding + // directories. const matches = await fg(pattern, { cwd: resolvedSearchPath, absolute: true, - onlyFiles: true, + onlyFiles: false, dot: true, + followSymbolicLinks: false, }); for (const matchedPath of matches) { diff --git a/libs/deepagents/src/backends/local-shell.test.ts b/libs/deepagents/src/backends/local-shell.test.ts index cc54d29a..64e8ff22 100644 --- a/libs/deepagents/src/backends/local-shell.test.ts +++ b/libs/deepagents/src/backends/local-shell.test.ts @@ -400,3 +400,69 @@ describe("LocalShellBackend", () => { }); }); }); + +/** + * Returns true when the platform lets an unprivileged process create symlinks + */ +function symlinksSupported(): boolean { + const probe = fsSync.mkdtempSync( + path.join(os.tmpdir(), "deepagents-symlink-probe-"), + ); + try { + fsSync.symlinkSync("target", path.join(probe, "link")); + return true; + } catch { + return false; + } finally { + fsSync.rmSync(probe, { recursive: true, force: true }); + } +} +const CAN_SYMLINK = symlinksSupported(); + +describe("LocalShellBackend symlink cycle handling", () => { + let tmpDir: string; + + beforeEach(() => { + tmpDir = createTempDir(); + }); + + afterEach(async () => { + await removeDir(tmpDir); + }); + + it.skipIf(!CAN_SYMLINK)( + "glob does not descend into a directory symlink cycle", + async () => { + await fs.mkdir(path.join(tmpDir, "sub"), { recursive: true }); + await fs.writeFile(path.join(tmpDir, "real.txt"), "x", "utf-8"); + await fs.writeFile(path.join(tmpDir, "sub", "inner.txt"), "x", "utf-8"); + // symlink -> file (must be reported as a file) + await fs.symlink( + path.join(tmpDir, "real.txt"), + path.join(tmpDir, "alias.txt"), + ); + // symlink -> dir (must be listed as a directory, but not descended into) + await fs.symlink(path.join(tmpDir, "sub"), path.join(tmpDir, "linkdir")); + // self-referential cycle (must not be descended into) + await fs.symlink(".", path.join(tmpDir, "sub", "sub")); + + const backend = new LocalShellBackend({ rootDir: tmpDir }); + const result = await backend.glob("**/*", tmpDir); + + expect(result.error).toBeUndefined(); + const files = result.files!; + const filePaths = files.filter((f) => !f.is_dir).map((f) => f.path); + const dirPaths = files.filter((f) => f.is_dir).map((f) => f.path); + + // Real files and the symlink-to-file are all reported as files. + expect(filePaths.some((p) => p.endsWith("real.txt"))).toBe(true); + expect(filePaths.some((p) => p.endsWith("inner.txt"))).toBe(true); + expect(filePaths.some((p) => p.endsWith("alias.txt"))).toBe(true); + // The symlinked directory is listed... + expect(dirPaths.some((p) => p.endsWith("linkdir"))).toBe(true); + // ...but nothing is reached *through* it or through the cycle. + expect(filePaths.some((p) => /linkdir[\\/]/.test(p))).toBe(false); + expect(filePaths.some((p) => /sub[\\/]sub[\\/]/.test(p))).toBe(false); + }, + ); +}); diff --git a/libs/deepagents/src/backends/local-shell.ts b/libs/deepagents/src/backends/local-shell.ts index 712dfef5..0bdd0335 100644 --- a/libs/deepagents/src/backends/local-shell.ts +++ b/libs/deepagents/src/backends/local-shell.ts @@ -302,13 +302,15 @@ export class LocalShellBackend const formatPath = (rel: string) => (this.virtualMode ? `/${rel}` : rel); - const globOpts = { cwd: resolvedSearchPath, absolute: false, dot: true }; - const [fileMatches, dirMatches] = await Promise.all([ - fg(pattern, { ...globOpts, onlyFiles: true }), - fg(pattern, { ...globOpts, onlyDirectories: true }), - ]); + const matches = await fg(pattern, { + cwd: resolvedSearchPath, + absolute: false, + dot: true, + onlyFiles: false, + followSymbolicLinks: false, + }); - const statFile = async (match: string): Promise => { + const classify = async (match: string): Promise => { try { const entryStat = await fs.stat(path.join(resolvedSearchPath, match)); if (entryStat.isFile()) { @@ -319,15 +321,6 @@ export class LocalShellBackend modified_at: entryStat.mtime.toISOString(), }; } - } catch { - /* skip unstatable entries */ - } - return null; - }; - - const statDir = async (match: string): Promise => { - try { - const entryStat = await fs.stat(path.join(resolvedSearchPath, match)); if (entryStat.isDirectory()) { return { path: formatPath(match), @@ -337,19 +330,13 @@ export class LocalShellBackend }; } } catch { - /* skip unstatable entries */ + /* skip unstatable entries (e.g. broken symlinks) */ } return null; }; - const [fileInfos, dirInfos] = await Promise.all([ - Promise.all(fileMatches.map(statFile)), - Promise.all(dirMatches.map(statDir)), - ]); - - const results = [...fileInfos, ...dirInfos].filter( - (info): info is FileInfo => info !== null, - ); + const infos = await Promise.all(matches.map(classify)); + const results = infos.filter((info): info is FileInfo => info !== null); results.sort((a, b) => a.path.localeCompare(b.path)); return { files: results }; }