mirror of
https://github.com/langchain-ai/deepagentsjs.git
synced 2026-07-20 00:46:22 -04:00
fix(deepagents): fast-glob follows directory symlink cycles leading to ELOOP crashes (#668)
### Summary `fast-glob` follows symbolic links by default so the `glob`/`grep` tools in `FilesystemBackend` and `LocalShellBackend` walked directory symlink cycles recursively causing `ELOOP` crashes. and aborted the run over a target repo. This PR fixes by adding`followSymbolicLinks: false` to all four `fast-glob` calls. Bonus: this keeps traversal inside the search root. ### Tests Unit tests in `filesystem.test.ts` and `local-shell.test.ts` verifying that a `sub/sub -> .` cycle now traverses cleanly without traversing symlinks. This is an upstream fix for: https://github.com/langchain-ai/openwiki/issues/72
This commit is contained in:
@@ -0,0 +1,5 @@
|
||||
---
|
||||
"deepagents": patch
|
||||
---
|
||||
|
||||
fix(deepagents): fast-glob follows directory symlink cycles leading to ELOOP crashes
|
||||
@@ -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<null> },
|
||||
"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);
|
||||
}
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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) {
|
||||
|
||||
@@ -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);
|
||||
},
|
||||
);
|
||||
});
|
||||
|
||||
@@ -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<FileInfo | null> => {
|
||||
const classify = async (match: string): Promise<FileInfo | null> => {
|
||||
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<FileInfo | null> => {
|
||||
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 };
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user