Skip to content

Commit 487c0a5

Browse files
authored
fix: prevent path traversal in file APIs (#16)
fix: prevent path traversal in file tree and preview APIs
2 parents 8dcc19d + 0f05e4f commit 487c0a5

3 files changed

Lines changed: 221 additions & 15 deletions

File tree

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,157 @@
1+
/**
2+
* Unit tests for file API path traversal security fixes.
3+
*
4+
* Run with: npx tsx src/__tests__/unit/files-security.test.ts
5+
*
6+
* Tests verify that:
7+
* 1. isPathSafe correctly prevents path traversal attacks
8+
* 2. Paths outside the base directory are rejected
9+
* 3. Symlink-based escapes are caught
10+
* 4. Edge cases (root, same path, trailing separators) are handled
11+
*/
12+
13+
import { describe, it, after } from 'node:test';
14+
import assert from 'node:assert/strict';
15+
import path from 'path';
16+
import os from 'os';
17+
import fs from 'fs';
18+
19+
// Import the function under test
20+
import { isPathSafe } from '../../lib/files';
21+
22+
describe('isPathSafe', () => {
23+
it('should allow paths within the base directory', () => {
24+
assert.equal(isPathSafe('/home/user/project', '/home/user/project/src/index.ts'), true);
25+
assert.equal(isPathSafe('/home/user/project', '/home/user/project/package.json'), true);
26+
assert.equal(isPathSafe('/home/user/project', '/home/user/project/src/lib/utils.ts'), true);
27+
});
28+
29+
it('should allow the base directory itself', () => {
30+
assert.equal(isPathSafe('/home/user/project', '/home/user/project'), true);
31+
});
32+
33+
it('should reject paths outside the base directory', () => {
34+
assert.equal(isPathSafe('/home/user/project', '/home/user/other'), false);
35+
assert.equal(isPathSafe('/home/user/project', '/home/user'), false);
36+
assert.equal(isPathSafe('/home/user/project', '/etc/passwd'), false);
37+
assert.equal(isPathSafe('/home/user/project', '/tmp/malicious'), false);
38+
});
39+
40+
it('should reject path traversal via ../', () => {
41+
// path.resolve will normalize these, but the resolved path should be outside base
42+
const base = '/home/user/project';
43+
const traversal = path.resolve(base, '../../etc/passwd');
44+
assert.equal(isPathSafe(base, traversal), false);
45+
});
46+
47+
it('should reject directory names that are prefixes but not parents', () => {
48+
// /home/user/project-evil should NOT be allowed under /home/user/project
49+
assert.equal(isPathSafe('/home/user/project', '/home/user/project-evil/file.txt'), false);
50+
assert.equal(isPathSafe('/home/user/project', '/home/user/projectx'), false);
51+
});
52+
53+
it('should handle Windows-style paths if on Windows', () => {
54+
if (process.platform === 'win32') {
55+
assert.equal(isPathSafe('C:\\Users\\user\\project', 'C:\\Users\\user\\project\\src\\index.ts'), true);
56+
assert.equal(isPathSafe('C:\\Users\\user\\project', 'D:\\other\\file.txt'), false);
57+
}
58+
});
59+
});
60+
61+
describe('File API path traversal scenarios', () => {
62+
const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'codepilot-test-'));
63+
const projectDir = path.join(tmpDir, 'myproject');
64+
const secretFile = path.join(tmpDir, 'secret.txt');
65+
66+
// Setup test fixtures
67+
fs.mkdirSync(projectDir, { recursive: true });
68+
fs.mkdirSync(path.join(projectDir, 'src'), { recursive: true });
69+
fs.writeFileSync(path.join(projectDir, 'index.ts'), 'console.log("hello");\n');
70+
fs.writeFileSync(path.join(projectDir, 'src', 'app.ts'), 'export default {};\n');
71+
fs.writeFileSync(secretFile, 'TOP SECRET DATA\n');
72+
73+
it('should allow reading files inside the project', () => {
74+
const filePath = path.join(projectDir, 'index.ts');
75+
assert.equal(isPathSafe(projectDir, filePath), true);
76+
});
77+
78+
it('should allow reading files in subdirectories', () => {
79+
const filePath = path.join(projectDir, 'src', 'app.ts');
80+
assert.equal(isPathSafe(projectDir, filePath), true);
81+
});
82+
83+
it('should block reading files outside the project via relative path', () => {
84+
const maliciousPath = path.resolve(projectDir, '..', 'secret.txt');
85+
assert.equal(isPathSafe(projectDir, maliciousPath), false);
86+
// Verify the secret file actually exists (test is meaningful)
87+
assert.equal(fs.existsSync(maliciousPath), true);
88+
});
89+
90+
it('should block reading system files', () => {
91+
assert.equal(isPathSafe(projectDir, '/etc/passwd'), false);
92+
assert.equal(isPathSafe(projectDir, '/etc/shadow'), false);
93+
});
94+
95+
it('should block reading via encoded traversal after resolution', () => {
96+
// Even if someone tries URL-encoded ../, path.resolve normalizes it
97+
const resolved = path.resolve(projectDir, '..', '..', 'etc', 'passwd');
98+
assert.equal(isPathSafe(projectDir, resolved), false);
99+
});
100+
101+
// Symlink test (only on Unix-like systems)
102+
if (process.platform !== 'win32') {
103+
it('should block symlink escape from project directory', () => {
104+
const symlinkPath = path.join(projectDir, 'escape-link');
105+
try {
106+
fs.symlinkSync('/etc', symlinkPath);
107+
const resolvedSymlink = fs.realpathSync(path.join(symlinkPath, 'passwd'));
108+
assert.equal(isPathSafe(projectDir, resolvedSymlink), false);
109+
} finally {
110+
try { fs.unlinkSync(symlinkPath); } catch { /* cleanup */ }
111+
}
112+
});
113+
}
114+
115+
// Cleanup test fixtures
116+
after(() => {
117+
fs.rmSync(tmpDir, { recursive: true, force: true });
118+
});
119+
});
120+
121+
describe('baseDir validation', () => {
122+
it('should reject baseDir set to root (bypass attempt)', () => {
123+
// If baseDir=/, every path would pass isPathSafe — must be blocked
124+
const homeDir = os.homedir();
125+
assert.equal(isPathSafe(homeDir, '/'), false);
126+
});
127+
128+
it('should reject baseDir outside home directory', () => {
129+
const homeDir = os.homedir();
130+
assert.equal(isPathSafe(homeDir, '/tmp'), false);
131+
assert.equal(isPathSafe(homeDir, '/etc'), false);
132+
assert.equal(isPathSafe(homeDir, '/var/log'), false);
133+
});
134+
135+
it('should allow baseDir inside home directory', () => {
136+
const homeDir = os.homedir();
137+
const projectDir = path.join(homeDir, 'projects', 'myapp');
138+
assert.equal(isPathSafe(homeDir, projectDir), true);
139+
});
140+
141+
it('should allow baseDir equal to home directory', () => {
142+
const homeDir = os.homedir();
143+
assert.equal(isPathSafe(homeDir, homeDir), true);
144+
});
145+
146+
it('should block files outside home when no baseDir provided (fallback)', () => {
147+
const homeDir = os.homedir();
148+
assert.equal(isPathSafe(homeDir, '/etc/passwd'), false);
149+
assert.equal(isPathSafe(homeDir, '/tmp/malicious'), false);
150+
});
151+
152+
it('should allow files inside home when no baseDir provided (fallback)', () => {
153+
const homeDir = os.homedir();
154+
const filePath = path.join(homeDir, 'documents', 'file.txt');
155+
assert.equal(isPathSafe(homeDir, filePath), true);
156+
});
157+
});

src/app/api/files/preview/route.ts

Lines changed: 32 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -15,16 +15,40 @@ export async function GET(request: NextRequest) {
1515
}
1616

1717
const path = require('path');
18+
const os = require('os');
1819
const resolvedPath = path.resolve(filePath);
20+
const homeDir = os.homedir();
1921

20-
// Basic safety: the file must exist under some reasonable base
21-
// We check it resolves to an absolute path and is not a traversal attempt
22-
const dir = path.dirname(resolvedPath);
23-
if (!isPathSafe(dir, resolvedPath)) {
24-
return NextResponse.json<ErrorResponse>(
25-
{ error: 'Invalid file path' },
26-
{ status: 403 }
27-
);
22+
// Validate that the file is within the session's working directory.
23+
// The baseDir parameter should be the session's working directory,
24+
// which acts as the trust boundary for file access.
25+
// The baseDir itself must be under the user's home directory to prevent
26+
// attackers from setting baseDir=/ to bypass all restrictions.
27+
const baseDir = searchParams.get('baseDir');
28+
if (baseDir) {
29+
const resolvedBase = path.resolve(baseDir);
30+
// Ensure baseDir is within the home directory (prevent baseDir=/ bypass)
31+
if (!isPathSafe(homeDir, resolvedBase)) {
32+
return NextResponse.json<ErrorResponse>(
33+
{ error: 'Base directory is outside the allowed scope' },
34+
{ status: 403 }
35+
);
36+
}
37+
if (!isPathSafe(resolvedBase, resolvedPath)) {
38+
return NextResponse.json<ErrorResponse>(
39+
{ error: 'File is outside the project scope' },
40+
{ status: 403 }
41+
);
42+
}
43+
} else {
44+
// Fallback: without a baseDir, restrict to the user's home directory
45+
// to prevent reading arbitrary system files like /etc/passwd
46+
if (!isPathSafe(homeDir, resolvedPath)) {
47+
return NextResponse.json<ErrorResponse>(
48+
{ error: 'File is outside the allowed scope' },
49+
{ status: 403 }
50+
);
51+
}
2852
}
2953

3054
try {

src/app/api/files/route.ts

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,16 +14,41 @@ export async function GET(request: NextRequest) {
1414
);
1515
}
1616

17-
// Safety: resolve absolute path and validate
1817
const path = require('path');
18+
const os = require('os');
1919
const resolvedDir = path.resolve(dir);
20+
const homeDir = os.homedir();
2021

21-
// Only allow scanning within the provided directory
22-
if (!isPathSafe(resolvedDir, resolvedDir)) {
23-
return NextResponse.json<ErrorResponse>(
24-
{ error: 'Invalid directory path' },
25-
{ status: 403 }
26-
);
22+
// Use baseDir (the session's working directory) as the trust boundary.
23+
// The baseDir itself must be under the user's home directory to prevent
24+
// attackers from setting baseDir=/ to bypass all restrictions.
25+
// If no baseDir is provided, fall back to the user's home directory
26+
// to prevent scanning arbitrary system directories.
27+
const baseDir = searchParams.get('baseDir');
28+
if (baseDir) {
29+
const resolvedBase = path.resolve(baseDir);
30+
// Ensure baseDir is within the home directory (prevent baseDir=/ bypass)
31+
if (!isPathSafe(homeDir, resolvedBase)) {
32+
return NextResponse.json<ErrorResponse>(
33+
{ error: 'Base directory is outside the allowed scope' },
34+
{ status: 403 }
35+
);
36+
}
37+
if (!isPathSafe(resolvedBase, resolvedDir)) {
38+
return NextResponse.json<ErrorResponse>(
39+
{ error: 'Directory is outside the project scope' },
40+
{ status: 403 }
41+
);
42+
}
43+
} else {
44+
// Fallback: without a baseDir, restrict to the user's home directory
45+
// to prevent scanning arbitrary system directories like /etc
46+
if (!isPathSafe(homeDir, resolvedDir)) {
47+
return NextResponse.json<ErrorResponse>(
48+
{ error: 'Directory is outside the allowed scope' },
49+
{ status: 403 }
50+
);
51+
}
2752
}
2853

2954
try {

0 commit comments

Comments
 (0)