Skip to content

Commit a83b145

Browse files
authored
Merge pull request #3254 from wingding12/fix/filesystem-macos-symlink-path-resolution
fix(filesystem): resolve symlinked allowed directories to both forms
2 parents 1b96551 + 8f2e9cc commit a83b145

2 files changed

Lines changed: 61 additions & 4 deletions

File tree

src/filesystem/__tests__/path-validation.test.ts

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,53 @@ describe('Path Validation', () => {
564564
}
565565
});
566566

567+
// Test for macOS /tmp -> /private/tmp symlink issue (GitHub issue #3253)
568+
// When allowed directories include BOTH original and resolved paths,
569+
// paths through either form should be accepted
570+
it('allows paths through both original and resolved symlink directories', async () => {
571+
try {
572+
// Setup: Create the actual target directory with content
573+
const actualTargetDir = path.join(testDir, 'actual-target');
574+
await fs.mkdir(actualTargetDir, { recursive: true });
575+
const targetFile = path.join(actualTargetDir, 'file.txt');
576+
await fs.writeFile(targetFile, 'FILE_CONTENT');
577+
578+
// Setup: Create symlink directory that points to target (simulates /tmp -> /private/tmp)
579+
const symlinkDir = path.join(testDir, 'symlink-dir');
580+
await fs.symlink(actualTargetDir, symlinkDir);
581+
582+
// Get the resolved path
583+
const resolvedDir = await fs.realpath(symlinkDir);
584+
585+
// THE FIX: Store BOTH original symlink path AND resolved path in allowed directories
586+
// This is what the server should do during startup to fix issue #3253
587+
const allowedDirsWithBoth = [symlinkDir, resolvedDir];
588+
589+
// Test 1: Path through original symlink should pass validation
590+
// (e.g., user requests /tmp/file.txt when /tmp is in allowed dirs)
591+
const fileViaSymlink = path.join(symlinkDir, 'file.txt');
592+
expect(isPathWithinAllowedDirectories(fileViaSymlink, allowedDirsWithBoth)).toBe(true);
593+
594+
// Test 2: Path through resolved directory should also pass validation
595+
// (e.g., user requests /private/tmp/file.txt)
596+
const fileViaResolved = path.join(resolvedDir, 'file.txt');
597+
expect(isPathWithinAllowedDirectories(fileViaResolved, allowedDirsWithBoth)).toBe(true);
598+
599+
// Test 3: The resolved path of the symlink file should also pass
600+
const resolvedFile = await fs.realpath(fileViaSymlink);
601+
expect(isPathWithinAllowedDirectories(resolvedFile, allowedDirsWithBoth)).toBe(true);
602+
603+
// Verify both paths point to the same actual file
604+
expect(resolvedFile).toBe(await fs.realpath(fileViaResolved));
605+
606+
} catch (error) {
607+
// Skip if no symlink permissions on the system
608+
if ((error as NodeJS.ErrnoException).code !== 'EPERM') {
609+
throw error;
610+
}
611+
}
612+
});
613+
567614
it('resolves nested symlink chains completely', async () => {
568615
try {
569616
// Setup: Create target file in forbidden area

src/filesystem/index.ts

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -39,22 +39,32 @@ if (args.length === 0) {
3939
}
4040

4141
// Store allowed directories in normalized and resolved form
42-
let allowedDirectories = await Promise.all(
42+
// We store BOTH the original path AND the resolved path to handle symlinks correctly
43+
// This fixes the macOS /tmp -> /private/tmp symlink issue where users specify /tmp
44+
// but the resolved path is /private/tmp
45+
let allowedDirectories = (await Promise.all(
4346
args.map(async (dir) => {
4447
const expanded = expandHome(dir);
4548
const absolute = path.resolve(expanded);
49+
const normalizedOriginal = normalizePath(absolute);
4650
try {
4751
// Security: Resolve symlinks in allowed directories during startup
4852
// This ensures we know the real paths and can validate against them later
4953
const resolved = await fs.realpath(absolute);
50-
return normalizePath(resolved);
54+
const normalizedResolved = normalizePath(resolved);
55+
// Return both original and resolved paths if they differ
56+
// This allows matching against either /tmp or /private/tmp on macOS
57+
if (normalizedOriginal !== normalizedResolved) {
58+
return [normalizedOriginal, normalizedResolved];
59+
}
60+
return [normalizedResolved];
5161
} catch (error) {
5262
// If we can't resolve (doesn't exist), use the normalized absolute path
5363
// This allows configuring allowed dirs that will be created later
54-
return normalizePath(absolute);
64+
return [normalizedOriginal];
5565
}
5666
})
57-
);
67+
)).flat();
5868

5969
// Filter to only accessible directories, warn about inaccessible ones
6070
const accessibleDirectories: string[] = [];

0 commit comments

Comments
 (0)