From 84de2d1de8e323aef70036a4aaf10b284a9a999b Mon Sep 17 00:00:00 2001 From: liuxiaopai-ai Date: Sun, 8 Feb 2026 15:55:32 +0800 Subject: [PATCH 1/2] fix: prevent path traversal in file tree and preview APIs The file tree API (/api/files) and file preview API (/api/files/preview) had ineffective path safety checks that could allow reading arbitrary files on the host system. Issues fixed: - /api/files: isPathSafe(resolvedDir, resolvedDir) always returns true since it compares the directory against itself - /api/files/preview: isPathSafe(dirname, filePath) always returns true since a file is always inside its own parent directory Changes: - Add baseDir parameter to both APIs so the frontend can pass the session's working directory as a trust boundary - When baseDir is provided, validate that the requested path is within it - For /api/files/preview without baseDir, fall back to restricting access to the user's home directory (prevents /etc/passwd etc.) - Add comprehensive unit tests covering path traversal, prefix attacks, symlink escapes, and edge cases (13 tests, all passing) Security: This is a defense-in-depth measure. CodePilot runs locally so the attack surface is limited, but proper validation prevents accidental exposure when the dev server is bound to non-localhost interfaces. --- src/__tests__/unit/files-security.test.ts | 119 ++++++++++++++++++++++ src/app/api/files/preview/route.ts | 31 ++++-- src/app/api/files/route.ts | 20 ++-- 3 files changed, 155 insertions(+), 15 deletions(-) create mode 100644 src/__tests__/unit/files-security.test.ts diff --git a/src/__tests__/unit/files-security.test.ts b/src/__tests__/unit/files-security.test.ts new file mode 100644 index 0000000..f88e4a4 --- /dev/null +++ b/src/__tests__/unit/files-security.test.ts @@ -0,0 +1,119 @@ +/** + * Unit tests for file API path traversal security fixes. + * + * Run with: npx tsx src/__tests__/unit/files-security.test.ts + * + * Tests verify that: + * 1. isPathSafe correctly prevents path traversal attacks + * 2. Paths outside the base directory are rejected + * 3. Symlink-based escapes are caught + * 4. Edge cases (root, same path, trailing separators) are handled + */ + +import { describe, it } from 'node:test'; +import assert from 'node:assert/strict'; +import path from 'path'; +import os from 'os'; +import fs from 'fs'; + +// Import the function under test +import { isPathSafe } from '../../lib/files'; + +describe('isPathSafe', () => { + it('should allow paths within the base directory', () => { + assert.equal(isPathSafe('/home/user/project', '/home/user/project/src/index.ts'), true); + assert.equal(isPathSafe('/home/user/project', '/home/user/project/package.json'), true); + assert.equal(isPathSafe('/home/user/project', '/home/user/project/src/lib/utils.ts'), true); + }); + + it('should allow the base directory itself', () => { + assert.equal(isPathSafe('/home/user/project', '/home/user/project'), true); + }); + + it('should reject paths outside the base directory', () => { + assert.equal(isPathSafe('/home/user/project', '/home/user/other'), false); + assert.equal(isPathSafe('/home/user/project', '/home/user'), false); + assert.equal(isPathSafe('/home/user/project', '/etc/passwd'), false); + assert.equal(isPathSafe('/home/user/project', '/tmp/malicious'), false); + }); + + it('should reject path traversal via ../', () => { + // path.resolve will normalize these, but the resolved path should be outside base + const base = '/home/user/project'; + const traversal = path.resolve(base, '../../etc/passwd'); + assert.equal(isPathSafe(base, traversal), false); + }); + + it('should reject directory names that are prefixes but not parents', () => { + // /home/user/project-evil should NOT be allowed under /home/user/project + assert.equal(isPathSafe('/home/user/project', '/home/user/project-evil/file.txt'), false); + assert.equal(isPathSafe('/home/user/project', '/home/user/projectx'), false); + }); + + it('should handle Windows-style paths if on Windows', () => { + if (process.platform === 'win32') { + assert.equal(isPathSafe('C:\\Users\\user\\project', 'C:\\Users\\user\\project\\src\\index.ts'), true); + assert.equal(isPathSafe('C:\\Users\\user\\project', 'D:\\other\\file.txt'), false); + } + }); +}); + +describe('File API path traversal scenarios', () => { + const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codepilot-test-')); + const projectDir = path.join(tmpDir, 'myproject'); + const secretFile = path.join(tmpDir, 'secret.txt'); + + // Setup test fixtures + fs.mkdirSync(projectDir, { recursive: true }); + fs.mkdirSync(path.join(projectDir, 'src'), { recursive: true }); + fs.writeFileSync(path.join(projectDir, 'index.ts'), 'console.log("hello");\n'); + fs.writeFileSync(path.join(projectDir, 'src', 'app.ts'), 'export default {};\n'); + fs.writeFileSync(secretFile, 'TOP SECRET DATA\n'); + + it('should allow reading files inside the project', () => { + const filePath = path.join(projectDir, 'index.ts'); + assert.equal(isPathSafe(projectDir, filePath), true); + }); + + it('should allow reading files in subdirectories', () => { + const filePath = path.join(projectDir, 'src', 'app.ts'); + assert.equal(isPathSafe(projectDir, filePath), true); + }); + + it('should block reading files outside the project via relative path', () => { + const maliciousPath = path.resolve(projectDir, '..', 'secret.txt'); + assert.equal(isPathSafe(projectDir, maliciousPath), false); + // Verify the secret file actually exists (test is meaningful) + assert.equal(fs.existsSync(maliciousPath), true); + }); + + it('should block reading system files', () => { + assert.equal(isPathSafe(projectDir, '/etc/passwd'), false); + assert.equal(isPathSafe(projectDir, '/etc/shadow'), false); + }); + + it('should block reading via encoded traversal after resolution', () => { + // Even if someone tries URL-encoded ../, path.resolve normalizes it + const resolved = path.resolve(projectDir, '..', '..', 'etc', 'passwd'); + assert.equal(isPathSafe(projectDir, resolved), false); + }); + + // Symlink test (only on Unix-like systems) + if (process.platform !== 'win32') { + it('should block symlink escape from project directory', () => { + const symlinkPath = path.join(projectDir, 'escape-link'); + try { + fs.symlinkSync('/etc', symlinkPath); + const resolvedSymlink = fs.realpathSync(path.join(symlinkPath, 'passwd')); + assert.equal(isPathSafe(projectDir, resolvedSymlink), false); + } finally { + try { fs.unlinkSync(symlinkPath); } catch { /* cleanup */ } + } + }); + } + + // Cleanup + it('cleanup test fixtures', () => { + fs.rmSync(tmpDir, { recursive: true, force: true }); + }); +}); diff --git a/src/app/api/files/preview/route.ts b/src/app/api/files/preview/route.ts index 437614f..75945de 100644 --- a/src/app/api/files/preview/route.ts +++ b/src/app/api/files/preview/route.ts @@ -17,14 +17,29 @@ export async function GET(request: NextRequest) { const path = require('path'); const resolvedPath = path.resolve(filePath); - // Basic safety: the file must exist under some reasonable base - // We check it resolves to an absolute path and is not a traversal attempt - const dir = path.dirname(resolvedPath); - if (!isPathSafe(dir, resolvedPath)) { - return NextResponse.json( - { error: 'Invalid file path' }, - { status: 403 } - ); + // Validate that the file is within the session's working directory. + // The baseDir parameter should be the session's working directory, + // which acts as the trust boundary for file access. + const baseDir = searchParams.get('baseDir'); + if (baseDir) { + const resolvedBase = path.resolve(baseDir); + if (!isPathSafe(resolvedBase, resolvedPath)) { + return NextResponse.json( + { error: 'File is outside the project scope' }, + { status: 403 } + ); + } + } else { + // Fallback: without a baseDir, restrict to the user's home directory + // to prevent reading arbitrary system files like /etc/passwd + const os = require('os'); + const homeDir = os.homedir(); + if (!isPathSafe(homeDir, resolvedPath)) { + return NextResponse.json( + { error: 'File is outside the allowed scope' }, + { status: 403 } + ); + } } try { diff --git a/src/app/api/files/route.ts b/src/app/api/files/route.ts index 37983b4..51b6402 100644 --- a/src/app/api/files/route.ts +++ b/src/app/api/files/route.ts @@ -14,16 +14,22 @@ export async function GET(request: NextRequest) { ); } - // Safety: resolve absolute path and validate const path = require('path'); const resolvedDir = path.resolve(dir); - // Only allow scanning within the provided directory - if (!isPathSafe(resolvedDir, resolvedDir)) { - return NextResponse.json( - { error: 'Invalid directory path' }, - { status: 403 } - ); + // Use baseDir (the session's working directory) as the trust boundary. + // If no baseDir is provided, fall back to the requested directory itself + // (preserves backward compatibility while still preventing traversal + // when the frontend supplies the session working directory). + const baseDir = searchParams.get('baseDir'); + if (baseDir) { + const resolvedBase = path.resolve(baseDir); + if (!isPathSafe(resolvedBase, resolvedDir)) { + return NextResponse.json( + { error: 'Directory is outside the project scope' }, + { status: 403 } + ); + } } try { From 0f05e4fd0f976de3a7a0575d812d39bd7e58e4fe Mon Sep 17 00:00:00 2001 From: liuxiaopai-ai Date: Sun, 8 Feb 2026 21:36:56 +0800 Subject: [PATCH 2/2] fix: address code review - fallback to homedir, validate baseDir, use afterAll Address all 3 points from @op7418's review: 1. No baseDir fallback (severe): Both /api/files and /api/files/preview now fall back to os.homedir() when no baseDir is provided, preventing access to arbitrary system directories/files. 2. baseDir bypass via baseDir=/ (medium): Both APIs now validate that baseDir itself is within the user's home directory. Setting baseDir=/ will be rejected with 403. 3. Test cleanup (minor): Replaced 'it(cleanup...)' with proper after() hook. Added 6 new tests for baseDir validation (18 total, all pass). --- src/__tests__/unit/files-security.test.ts | 44 +++++++++++++++++++++-- src/app/api/files/preview/route.ts | 13 +++++-- src/app/api/files/route.ts | 25 +++++++++++-- 3 files changed, 74 insertions(+), 8 deletions(-) diff --git a/src/__tests__/unit/files-security.test.ts b/src/__tests__/unit/files-security.test.ts index f88e4a4..9d6728f 100644 --- a/src/__tests__/unit/files-security.test.ts +++ b/src/__tests__/unit/files-security.test.ts @@ -10,7 +10,7 @@ * 4. Edge cases (root, same path, trailing separators) are handled */ -import { describe, it } from 'node:test'; +import { describe, it, after } from 'node:test'; import assert from 'node:assert/strict'; import path from 'path'; import os from 'os'; @@ -112,8 +112,46 @@ describe('File API path traversal scenarios', () => { }); } - // Cleanup - it('cleanup test fixtures', () => { + // Cleanup test fixtures + after(() => { fs.rmSync(tmpDir, { recursive: true, force: true }); }); }); + +describe('baseDir validation', () => { + it('should reject baseDir set to root (bypass attempt)', () => { + // If baseDir=/, every path would pass isPathSafe — must be blocked + const homeDir = os.homedir(); + assert.equal(isPathSafe(homeDir, '/'), false); + }); + + it('should reject baseDir outside home directory', () => { + const homeDir = os.homedir(); + assert.equal(isPathSafe(homeDir, '/tmp'), false); + assert.equal(isPathSafe(homeDir, '/etc'), false); + assert.equal(isPathSafe(homeDir, '/var/log'), false); + }); + + it('should allow baseDir inside home directory', () => { + const homeDir = os.homedir(); + const projectDir = path.join(homeDir, 'projects', 'myapp'); + assert.equal(isPathSafe(homeDir, projectDir), true); + }); + + it('should allow baseDir equal to home directory', () => { + const homeDir = os.homedir(); + assert.equal(isPathSafe(homeDir, homeDir), true); + }); + + it('should block files outside home when no baseDir provided (fallback)', () => { + const homeDir = os.homedir(); + assert.equal(isPathSafe(homeDir, '/etc/passwd'), false); + assert.equal(isPathSafe(homeDir, '/tmp/malicious'), false); + }); + + it('should allow files inside home when no baseDir provided (fallback)', () => { + const homeDir = os.homedir(); + const filePath = path.join(homeDir, 'documents', 'file.txt'); + assert.equal(isPathSafe(homeDir, filePath), true); + }); +}); diff --git a/src/app/api/files/preview/route.ts b/src/app/api/files/preview/route.ts index 75945de..d764828 100644 --- a/src/app/api/files/preview/route.ts +++ b/src/app/api/files/preview/route.ts @@ -15,14 +15,25 @@ export async function GET(request: NextRequest) { } const path = require('path'); + const os = require('os'); const resolvedPath = path.resolve(filePath); + const homeDir = os.homedir(); // Validate that the file is within the session's working directory. // The baseDir parameter should be the session's working directory, // which acts as the trust boundary for file access. + // The baseDir itself must be under the user's home directory to prevent + // attackers from setting baseDir=/ to bypass all restrictions. const baseDir = searchParams.get('baseDir'); if (baseDir) { const resolvedBase = path.resolve(baseDir); + // Ensure baseDir is within the home directory (prevent baseDir=/ bypass) + if (!isPathSafe(homeDir, resolvedBase)) { + return NextResponse.json( + { error: 'Base directory is outside the allowed scope' }, + { status: 403 } + ); + } if (!isPathSafe(resolvedBase, resolvedPath)) { return NextResponse.json( { error: 'File is outside the project scope' }, @@ -32,8 +43,6 @@ export async function GET(request: NextRequest) { } else { // Fallback: without a baseDir, restrict to the user's home directory // to prevent reading arbitrary system files like /etc/passwd - const os = require('os'); - const homeDir = os.homedir(); if (!isPathSafe(homeDir, resolvedPath)) { return NextResponse.json( { error: 'File is outside the allowed scope' }, diff --git a/src/app/api/files/route.ts b/src/app/api/files/route.ts index 51b6402..be731a1 100644 --- a/src/app/api/files/route.ts +++ b/src/app/api/files/route.ts @@ -15,21 +15,40 @@ export async function GET(request: NextRequest) { } const path = require('path'); + const os = require('os'); const resolvedDir = path.resolve(dir); + const homeDir = os.homedir(); // Use baseDir (the session's working directory) as the trust boundary. - // If no baseDir is provided, fall back to the requested directory itself - // (preserves backward compatibility while still preventing traversal - // when the frontend supplies the session working directory). + // The baseDir itself must be under the user's home directory to prevent + // attackers from setting baseDir=/ to bypass all restrictions. + // If no baseDir is provided, fall back to the user's home directory + // to prevent scanning arbitrary system directories. const baseDir = searchParams.get('baseDir'); if (baseDir) { const resolvedBase = path.resolve(baseDir); + // Ensure baseDir is within the home directory (prevent baseDir=/ bypass) + if (!isPathSafe(homeDir, resolvedBase)) { + return NextResponse.json( + { error: 'Base directory is outside the allowed scope' }, + { status: 403 } + ); + } if (!isPathSafe(resolvedBase, resolvedDir)) { return NextResponse.json( { error: 'Directory is outside the project scope' }, { status: 403 } ); } + } else { + // Fallback: without a baseDir, restrict to the user's home directory + // to prevent scanning arbitrary system directories like /etc + if (!isPathSafe(homeDir, resolvedDir)) { + return NextResponse.json( + { error: 'Directory is outside the allowed scope' }, + { status: 403 } + ); + } } try {