Skip to content

Commit f79af80

Browse files
betegonclaude
andcommitted
fix(init): use opendir for listDir and validate symlinks during traversal
Replace the recursive readdirSync walk with fs.promises.opendir({ recursive, bufferSize: 1024 }) for cleaner iteration. Add symlink validation via safePath on each entry so symlinks pointing outside the project directory are excluded from listings. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
1 parent 828ebfc commit f79af80

File tree

4 files changed

+72
-38
lines changed

4 files changed

+72
-38
lines changed

src/lib/init/local-ops.ts

Lines changed: 41 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -284,8 +284,10 @@ function safePath(cwd: string, relative: string): string {
284284
* Pre-compute directory listing before the first API call.
285285
* Uses the same parameters the server's discover-context step would request.
286286
*/
287-
export function precomputeDirListing(directory: string): DirEntry[] {
288-
const result = listDir({
287+
export async function precomputeDirListing(
288+
directory: string
289+
): Promise<DirEntry[]> {
290+
const result = await listDir({
289291
type: "local-op",
290292
operation: "list-dir",
291293
cwd: directory,
@@ -344,7 +346,10 @@ export async function handleLocalOp(
344346
}
345347
}
346348

347-
function listDir(payload: ListDirPayload): LocalOpResult {
349+
/** Directory names that are listed at their level but never recursed into. */
350+
const SKIP_DIRS = new Set(["node_modules"]);
351+
352+
async function listDir(payload: ListDirPayload): Promise<LocalOpResult> {
348353
const { cwd, params } = payload;
349354
const targetPath = safePath(cwd, params.path);
350355
const maxDepth = params.maxDepth ?? 3;
@@ -353,40 +358,48 @@ function listDir(payload: ListDirPayload): LocalOpResult {
353358

354359
const entries: DirEntry[] = [];
355360

356-
// biome-ignore lint/complexity/noExcessiveCognitiveComplexity: walking the directory tree is a complex operation
357-
function walk(dir: string, depth: number): void {
358-
if (entries.length >= maxEntries || depth > maxDepth) {
359-
return;
360-
}
361+
try {
362+
const dir = await fs.promises.opendir(targetPath, {
363+
recursive,
364+
bufferSize: 1024,
365+
});
361366

362-
let dirEntries: fs.Dirent[];
363-
try {
364-
dirEntries = fs.readdirSync(dir, { withFileTypes: true });
365-
} catch {
366-
return;
367-
}
367+
for await (const entry of dir) {
368+
if (entries.length >= maxEntries) break;
369+
370+
const relFromTarget = path.relative(targetPath, entry.parentPath);
371+
const depth =
372+
relFromTarget === "" ? 0 : relFromTarget.split(path.sep).length;
368373

369-
for (const entry of dirEntries) {
370-
if (entries.length >= maxEntries) {
371-
return;
374+
if (depth > maxDepth) continue;
375+
376+
// Skip entries nested inside hidden dirs or node_modules,
377+
// but still list the skip-dirs themselves at their parent level.
378+
if (relFromTarget !== "") {
379+
const segments = relFromTarget.split(path.sep);
380+
if (segments.some((s) => s.startsWith(".") || SKIP_DIRS.has(s))) {
381+
continue;
382+
}
372383
}
373384

374-
const relPath = path.relative(cwd, path.join(dir, entry.name));
375-
const type = entry.isDirectory() ? "directory" : "file";
376-
entries.push({ name: entry.name, path: relPath, type });
385+
const relPath = path.relative(cwd, path.join(entry.parentPath, entry.name));
377386

378-
if (
379-
recursive &&
380-
entry.isDirectory() &&
381-
!entry.name.startsWith(".") &&
382-
entry.name !== "node_modules"
383-
) {
384-
walk(path.join(dir, entry.name), depth + 1);
387+
// If this entry is a symlink, verify it doesn't escape the project directory.
388+
if (entry.isSymbolicLink()) {
389+
try {
390+
safePath(cwd, relPath);
391+
} catch {
392+
continue;
393+
}
385394
}
395+
396+
const type = entry.isDirectory() ? "directory" : "file";
397+
entries.push({ name: entry.name, path: relPath, type });
386398
}
399+
} catch {
400+
// Directory doesn't exist or can't be read
387401
}
388402

389-
walk(targetPath, 0);
390403
return { ok: true, data: { entries } };
391404
}
392405

src/lib/init/wizard-runner.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -623,7 +623,7 @@ export async function runWizard(initialOptions: WizardOptions): Promise<void> {
623623
let run: Awaited<ReturnType<typeof workflow.createRun>>;
624624
let result: WorkflowRunResult;
625625
try {
626-
const dirListing = precomputeDirListing(directory);
626+
const dirListing = await precomputeDirListing(directory);
627627
spin.message("Connecting to wizard...");
628628
run = await workflow.createRun();
629629
result = assertWorkflowResult(

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

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -462,6 +462,26 @@ describe("handleLocalOp", () => {
462462
// Depth 2+ should not be reached
463463
expect(paths).not.toContain(join("a", "b", "c"));
464464
});
465+
466+
test("excludes symlinks that point outside project directory", async () => {
467+
writeFileSync(join(testDir, "legit.ts"), "x");
468+
symlinkSync("/tmp", join(testDir, "escape-link"));
469+
470+
const payload: ListDirPayload = {
471+
type: "local-op",
472+
operation: "list-dir",
473+
cwd: testDir,
474+
params: { path: "." },
475+
};
476+
477+
const result = await handleLocalOp(payload, options);
478+
const entries = (result.data as { entries: Array<{ name: string }> })
479+
.entries;
480+
const names = entries.map((e) => e.name);
481+
482+
expect(names).toContain("legit.ts");
483+
expect(names).not.toContain("escape-link");
484+
});
465485
});
466486

467487
describe("read-files", () => {
@@ -906,11 +926,11 @@ describe("precomputeDirListing", () => {
906926
rmSync(testDir, { recursive: true, force: true });
907927
});
908928

909-
test("returns DirEntry[] directly", () => {
929+
test("returns DirEntry[] directly", async () => {
910930
writeFileSync(join(testDir, "app.ts"), "x");
911931
mkdirSync(join(testDir, "src"));
912932

913-
const entries = precomputeDirListing(testDir);
933+
const entries = await precomputeDirListing(testDir);
914934

915935
expect(Array.isArray(entries)).toBe(true);
916936
expect(entries.length).toBeGreaterThanOrEqual(2);
@@ -926,16 +946,16 @@ describe("precomputeDirListing", () => {
926946
expect(dir?.type).toBe("directory");
927947
});
928948

929-
test("returns empty array for non-existent directory", () => {
930-
const entries = precomputeDirListing(join(testDir, "nope"));
949+
test("returns empty array for non-existent directory", async () => {
950+
const entries = await precomputeDirListing(join(testDir, "nope"));
931951
expect(entries).toEqual([]);
932952
});
933953

934-
test("recursively lists nested entries", () => {
954+
test("recursively lists nested entries", async () => {
935955
mkdirSync(join(testDir, "a"));
936956
writeFileSync(join(testDir, "a", "nested.ts"), "x");
937957

938-
const entries = precomputeDirListing(testDir);
958+
const entries = await precomputeDirListing(testDir);
939959
const paths = entries.map((e) => e.path);
940960
expect(paths).toContain(join("a", "nested.ts"));
941961
});

test/lib/init/wizard-runner.test.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -193,9 +193,10 @@ beforeEach(() => {
193193
ok: true,
194194
data: { results: [] },
195195
});
196-
precomputeDirListingSpy = spyOn(ops, "precomputeDirListing").mockReturnValue(
197-
[]
198-
);
196+
precomputeDirListingSpy = spyOn(
197+
ops,
198+
"precomputeDirListing"
199+
).mockResolvedValue([]);
199200
handleInteractiveSpy = spyOn(inter, "handleInteractive").mockResolvedValue({
200201
action: "continue",
201202
});

0 commit comments

Comments
 (0)