Skip to content

Commit a1c1edd

Browse files
committed
fix: normalize path separators for cross-platform security
- Normalize backslashes to forward slashes before path resolution - Use path.resolve() for proper absolute path handling on all OS - Add additional path traversal test cases (multi-level, absolute paths) - Fixes CI test failure on Linux where backslash wasn't recognized as separator
1 parent 03a6931 commit a1c1edd

2 files changed

Lines changed: 20 additions & 3 deletions

File tree

src/tools/read.ts

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,10 +5,15 @@ import { DocFile } from '../types.js';
55

66
export function readDoc(relativePath: string): string | null {
77
const docsDir = getDocsDir();
8-
const fullPath = path.join(docsDir, relativePath);
8+
9+
// Normalize path separators to forward slashes for cross-platform consistency
10+
const normalizedPath = relativePath.replace(/\\/g, '/');
11+
const fullPath = path.resolve(docsDir, normalizedPath);
12+
const normalizedDocsDir = path.resolve(docsDir);
913

1014
// Security check: ensure the resolved path is inside docsDir
11-
if (!fullPath.startsWith(docsDir)) {
15+
// Use path.resolve to get absolute paths and handle .. correctly
16+
if (!fullPath.startsWith(normalizedDocsDir + path.sep) && fullPath !== normalizedDocsDir) {
1217
throw new Error("Access denied: Path is outside of docs directory.");
1318
}
1419

tests/integration.test.ts

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -52,12 +52,24 @@ describe('Baritone Docs MCP Integration', () => {
5252
});
5353

5454
it('should protect against path traversal in readDoc', async () => {
55+
// Test Unix-style path traversal
5556
expect(() => {
5657
readDoc('../package.json');
5758
}).toThrow(/Access denied/);
5859

60+
// Test Windows-style path traversal (now normalized on all platforms)
5961
expect(() => {
60-
readDoc('..\\package.json'); // Windows style
62+
readDoc('..\\package.json');
63+
}).toThrow(/Access denied/);
64+
65+
// Test multiple levels of traversal
66+
expect(() => {
67+
readDoc('../../package.json');
68+
}).toThrow(/Access denied/);
69+
70+
// Test absolute paths
71+
expect(() => {
72+
readDoc('/etc/passwd');
6173
}).toThrow(/Access denied/);
6274
});
6375

0 commit comments

Comments
 (0)