From 0d4565e1294272673a7b084d5dc7b8cb53071541 Mon Sep 17 00:00:00 2001 From: Sebastion Date: Mon, 30 Mar 2026 07:40:17 +0100 Subject: [PATCH] fix: add path containment check in getFileSkeleton to prevent directory traversal (CWE-22) Validate that resolved file paths stay within rootDir before any file read. Also resolves symlinks to prevent symlink-based escapes. Without this check, an agent (or prompt-injected agent) could call get_file_skeleton with paths like "../../../../etc/passwd" to read arbitrary files outside the project root. --- src/tools/file-skeleton.ts | 27 +++++++++++- test/main/cwe22-file-skeleton.test.mjs | 60 ++++++++++++++++++++++++++ 2 files changed, 86 insertions(+), 1 deletion(-) create mode 100644 test/main/cwe22-file-skeleton.test.mjs diff --git a/src/tools/file-skeleton.ts b/src/tools/file-skeleton.ts index a7d21b9..1663277 100644 --- a/src/tools/file-skeleton.ts +++ b/src/tools/file-skeleton.ts @@ -2,7 +2,7 @@ // Returns structural skeleton: signatures, params, return types only import { analyzeFile, isSupportedFile, type FileAnalysis } from "../core/parser.js"; -import { readFile } from "fs/promises"; +import { readFile, realpath } from "fs/promises"; import { resolve } from "path"; export interface SkeletonOptions { @@ -33,9 +33,34 @@ function formatSignatureBlock(analysis: FileAnalysis): string { return lines.join("\n"); } +async function assertInsideRoot(fullPath: string, rootDir: string): Promise { + const resolvedRoot = resolve(rootDir); + const resolvedPath = resolve(fullPath); + + // Check the literal resolved path first (before symlink resolution) + if (!resolvedPath.startsWith(resolvedRoot + "/") && resolvedPath !== resolvedRoot) { + throw new Error(`Path escapes project root: ${resolvedPath}`); + } + + // Also check after resolving symlinks to prevent symlink-based escapes + try { + const realRoot = await realpath(resolvedRoot); + const realFile = await realpath(resolvedPath); + if (!realFile.startsWith(realRoot + "/") && realFile !== realRoot) { + throw new Error(`Path escapes project root after symlink resolution: ${realFile}`); + } + } catch (err) { + // If realpath fails because path doesn't exist, the initial check is sufficient + if ((err as NodeJS.ErrnoException).code === "ENOENT") return; + throw err; + } +} + export async function getFileSkeleton(options: SkeletonOptions): Promise { const fullPath = resolve(options.rootDir, options.filePath); + await assertInsideRoot(fullPath, options.rootDir); + if (!isSupportedFile(fullPath)) { const content = await readFile(fullPath, "utf-8"); const preview = content.split("\n").slice(0, 20).join("\n"); diff --git a/test/main/cwe22-file-skeleton.test.mjs b/test/main/cwe22-file-skeleton.test.mjs new file mode 100644 index 0000000..76d7212 --- /dev/null +++ b/test/main/cwe22-file-skeleton.test.mjs @@ -0,0 +1,60 @@ +// PoC: CWE-22 path traversal in getFileSkeleton +// Demonstrates that file_path with ../ can escape rootDir + +import { describe, it, after } from "node:test"; +import assert from "node:assert/strict"; +import { getFileSkeleton } from "../../build/tools/file-skeleton.js"; +import { writeFile, mkdir, rm } from "fs/promises"; +import { join, resolve } from "path"; +import { tmpdir } from "os"; + +const FIXTURE_DIR = join(tmpdir(), "cwe22-skel-test-" + process.pid); +const OUTSIDE_DIR = join(tmpdir(), "cwe22-skel-outside-" + process.pid); + +describe("CWE-22: path traversal in getFileSkeleton", async () => { + // Setup: create a confined rootDir and a file outside it + await rm(FIXTURE_DIR, { recursive: true, force: true }); + await rm(OUTSIDE_DIR, { recursive: true, force: true }); + await mkdir(FIXTURE_DIR, { recursive: true }); + await mkdir(OUTSIDE_DIR, { recursive: true }); + await writeFile(join(OUTSIDE_DIR, "secret.txt"), "SENSITIVE_DATA_LEAKED\n"); + await writeFile(join(FIXTURE_DIR, "safe.txt"), "safe content\n"); + + it("should reject path traversal with ../ that escapes rootDir", async () => { + // Compute a relative path from FIXTURE_DIR to OUTSIDE_DIR/secret.txt + const relativePath = "../cwe22-skel-outside-" + process.pid + "/secret.txt"; + + // Verify the traversal actually resolves outside rootDir + const resolved = resolve(FIXTURE_DIR, relativePath); + assert.ok(!resolved.startsWith(FIXTURE_DIR + "/"), "Precondition: path resolves outside rootDir"); + + // This should throw or reject, NOT return the file contents + await assert.rejects( + () => getFileSkeleton({ rootDir: FIXTURE_DIR, filePath: relativePath }), + (err) => { + return err instanceof Error; + }, + "getFileSkeleton should reject paths that escape rootDir" + ); + }); + + it("should reject absolute paths outside rootDir", async () => { + const absolutePath = join(OUTSIDE_DIR, "secret.txt"); + + await assert.rejects( + () => getFileSkeleton({ rootDir: FIXTURE_DIR, filePath: absolutePath }), + (err) => err instanceof Error, + "getFileSkeleton should reject absolute paths outside rootDir" + ); + }); + + it("should still allow valid relative paths within rootDir", async () => { + const result = await getFileSkeleton({ rootDir: FIXTURE_DIR, filePath: "safe.txt" }); + assert.ok(result.includes("safe content") || result.includes("Unsupported"), "Should return content for valid paths"); + }); + + after(async () => { + await rm(FIXTURE_DIR, { recursive: true, force: true }).catch(() => {}); + await rm(OUTSIDE_DIR, { recursive: true, force: true }).catch(() => {}); + }); +});