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.
This commit is contained in:
1 parent
4ac1f8cea3
commit
84de2d1de8
3 files changed
+155
-15
No files matched your search
@@ -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 });
|
||||
});
|
||||
});
|
||||
@@ -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<ErrorResponse>(
|
||||
{ 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<ErrorResponse>(
|
||||
{ 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<ErrorResponse>(
|
||||
{ error: 'File is outside the allowed scope' },
|
||||
{ status: 403 }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
|
||||
@@ -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<ErrorResponse>(
|
||||
{ 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<ErrorResponse>(
|
||||
{ error: 'Directory is outside the project scope' },
|
||||
{ status: 403 }
|
||||
);
|
||||
}
|
||||
}
|
||||
|
||||
try {
|
||||
|
||||
Reference in new issue
Block a user