Skip to content

Commit fc901d5

Browse files
betegonclaude
andcommitted
fix(init): use manual recursion in listDir to avoid symlink traversal
opendir({ recursive: true }) follows symlinks into their targets before we can filter them, causing EACCES errors that abort the entire listing. Additionally, Bun's opendir defers ENOENT to iteration rather than throwing at open time. Switch to non-recursive opendir with manual walk, and add catch around iteration for robustness. Add test for nested symlinks in recursive mode. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent ff011cd commit fc901d5

File tree

2 files changed

+87
-61
lines changed

2 files changed

+87
-61
lines changed

src/lib/init/local-ops.ts

Lines changed: 66 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -346,30 +346,11 @@ export async function handleLocalOp(
346346
}
347347
}
348348

349-
/** Directory names that are listed at their level but never recursed into. */
349+
/** Directory names that are listed but never recursed into. */
350350
const SKIP_DIRS = new Set(["node_modules"]);
351351

352-
/**
353-
* Check whether an entry is inside a hidden dir or node_modules.
354-
* Top-level skip-dirs (relFromTarget === "") are still listed.
355-
*/
356-
function isInsideSkippedDir(relFromTarget: string): boolean {
357-
if (relFromTarget === "") {
358-
return false;
359-
}
360-
const segments = relFromTarget.split(path.sep);
361-
return segments.some((s) => s.startsWith(".") || SKIP_DIRS.has(s));
362-
}
363-
364-
/** Return true when a symlink resolves to a path outside `cwd`. */
365-
function isEscapingSymlink(
366-
entry: fs.Dirent,
367-
cwd: string,
368-
relPath: string
369-
): boolean {
370-
if (!entry.isSymbolicLink()) {
371-
return false;
372-
}
352+
/** Return true if a symlink escapes the project directory. */
353+
function isEscapingSymlink(cwd: string, relPath: string): boolean {
373354
try {
374355
safePath(cwd, relPath);
375356
return false;
@@ -378,62 +359,86 @@ function isEscapingSymlink(
378359
}
379360
}
380361

381-
/** Convert a Dirent to a DirEntry, or return null if it should be skipped. */
382-
function toDirEntry(
383-
entry: fs.Dirent,
384-
cwd: string,
385-
targetPath: string,
386-
maxDepth: number
387-
): DirEntry | null {
388-
const relFromTarget = path.relative(targetPath, entry.parentPath);
389-
const depth = relFromTarget === "" ? 0 : relFromTarget.split(path.sep).length;
390-
391-
if (depth > maxDepth) {
392-
return null;
393-
}
394-
if (isInsideSkippedDir(relFromTarget)) {
395-
return null;
362+
/** Whether a directory entry should be recursed into. */
363+
function shouldRecurse(entry: fs.Dirent): boolean {
364+
if (!entry.isDirectory() || entry.isSymbolicLink()) {
365+
return false;
396366
}
367+
return !(entry.name.startsWith(".") || SKIP_DIRS.has(entry.name));
368+
}
397369

398-
const relPath = path.relative(cwd, path.join(entry.parentPath, entry.name));
370+
type WalkContext = {
371+
cwd: string;
372+
recursive: boolean;
373+
maxDepth: number;
374+
maxEntries: number;
375+
entries: DirEntry[];
376+
};
399377

400-
if (isEscapingSymlink(entry, cwd, relPath)) {
401-
return null;
378+
/** Process a single dirent during directory walking. */
379+
async function processDirEntry(
380+
ctx: WalkContext,
381+
dir: string,
382+
entry: fs.Dirent,
383+
depth: number
384+
): Promise<void> {
385+
const relPath = path.relative(ctx.cwd, path.join(dir, entry.name));
386+
if (entry.isSymbolicLink() && isEscapingSymlink(ctx.cwd, relPath)) {
387+
return;
402388
}
403389

404390
const type = entry.isDirectory() ? "directory" : "file";
405-
return { name: entry.name, path: relPath, type };
406-
}
391+
ctx.entries.push({ name: entry.name, path: relPath, type });
407392

408-
async function listDir(payload: ListDirPayload): Promise<LocalOpResult> {
409-
const { cwd, params } = payload;
410-
const targetPath = safePath(cwd, params.path);
411-
const maxDepth = params.maxDepth ?? 3;
412-
const maxEntries = params.maxEntries ?? 500;
413-
const recursive = params.recursive ?? false;
393+
if (ctx.recursive && shouldRecurse(entry)) {
394+
await walkDir(ctx, path.join(dir, entry.name), depth + 1);
395+
}
396+
}
414397

415-
const entries: DirEntry[] = [];
398+
async function walkDir(
399+
ctx: WalkContext,
400+
dir: string,
401+
depth: number
402+
): Promise<void> {
403+
if (ctx.entries.length >= ctx.maxEntries || depth > ctx.maxDepth) {
404+
return;
405+
}
416406

407+
let handle: fs.Dir;
417408
try {
418-
const dir = await fs.promises.opendir(targetPath, {
419-
recursive,
420-
bufferSize: 1024,
421-
});
409+
handle = await fs.promises.opendir(dir, { bufferSize: 1024 });
410+
} catch {
411+
return;
412+
}
422413

423-
for await (const dirent of dir) {
424-
if (entries.length >= maxEntries) {
414+
try {
415+
for await (const entry of handle) {
416+
if (ctx.entries.length >= ctx.maxEntries) {
425417
break;
426418
}
427-
const parsed = toDirEntry(dirent, cwd, targetPath, maxDepth);
428-
if (parsed) {
429-
entries.push(parsed);
430-
}
419+
await processDirEntry(ctx, dir, entry, depth);
431420
}
432421
} catch {
433-
// Directory doesn't exist or can't be read
422+
// Directory unreadable (ENOENT, EACCES, etc.) — skip gracefully
423+
} finally {
424+
await handle.close();
434425
}
426+
}
427+
428+
async function listDir(payload: ListDirPayload): Promise<LocalOpResult> {
429+
const { cwd, params } = payload;
430+
const targetPath = safePath(cwd, params.path);
431+
432+
const ctx: WalkContext = {
433+
cwd,
434+
recursive: params.recursive ?? false,
435+
maxDepth: params.maxDepth ?? 3,
436+
maxEntries: params.maxEntries ?? 500,
437+
entries: [],
438+
};
435439

436-
return { ok: true, data: { entries } };
440+
await walkDir(ctx, targetPath, 0);
441+
return { ok: true, data: { entries: ctx.entries } };
437442
}
438443

439444
async function readSingleFile(

test/lib/init/local-ops.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -482,6 +482,27 @@ describe("handleLocalOp", () => {
482482
expect(names).toContain("legit.ts");
483483
expect(names).not.toContain("escape-link");
484484
});
485+
486+
test("excludes nested symlinks that point outside project directory in recursive mode", async () => {
487+
mkdirSync(join(testDir, "sub"));
488+
writeFileSync(join(testDir, "sub", "legit.ts"), "x");
489+
symlinkSync("/tmp", join(testDir, "sub", "escape-link"));
490+
491+
const payload: ListDirPayload = {
492+
type: "local-op",
493+
operation: "list-dir",
494+
cwd: testDir,
495+
params: { path: ".", recursive: true, maxDepth: 3 },
496+
};
497+
498+
const result = await handleLocalOp(payload, options);
499+
const entries = (result.data as { entries: Array<{ path: string }> })
500+
.entries;
501+
const paths = entries.map((e) => e.path);
502+
503+
expect(paths).toContain(join("sub", "legit.ts"));
504+
expect(paths).not.toContain(join("sub", "escape-link"));
505+
});
485506
});
486507

487508
describe("read-files", () => {

0 commit comments

Comments
 (0)