Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
47 changes: 47 additions & 0 deletions src/filesystem/__tests__/path-validation.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -564,6 +564,53 @@ describe('Path Validation', () => {
}
});

// Test for macOS /tmp -> /private/tmp symlink issue (GitHub issue #3253)
// When allowed directories include BOTH original and resolved paths,
// paths through either form should be accepted
it('allows paths through both original and resolved symlink directories', async () => {
try {
// Setup: Create the actual target directory with content
const actualTargetDir = path.join(testDir, 'actual-target');
await fs.mkdir(actualTargetDir, { recursive: true });
const targetFile = path.join(actualTargetDir, 'file.txt');
await fs.writeFile(targetFile, 'FILE_CONTENT');

// Setup: Create symlink directory that points to target (simulates /tmp -> /private/tmp)
const symlinkDir = path.join(testDir, 'symlink-dir');
await fs.symlink(actualTargetDir, symlinkDir);

// Get the resolved path
const resolvedDir = await fs.realpath(symlinkDir);

// THE FIX: Store BOTH original symlink path AND resolved path in allowed directories
// This is what the server should do during startup to fix issue #3253
const allowedDirsWithBoth = [symlinkDir, resolvedDir];

// Test 1: Path through original symlink should pass validation
// (e.g., user requests /tmp/file.txt when /tmp is in allowed dirs)
const fileViaSymlink = path.join(symlinkDir, 'file.txt');
expect(isPathWithinAllowedDirectories(fileViaSymlink, allowedDirsWithBoth)).toBe(true);

// Test 2: Path through resolved directory should also pass validation
// (e.g., user requests /private/tmp/file.txt)
const fileViaResolved = path.join(resolvedDir, 'file.txt');
expect(isPathWithinAllowedDirectories(fileViaResolved, allowedDirsWithBoth)).toBe(true);

// Test 3: The resolved path of the symlink file should also pass
const resolvedFile = await fs.realpath(fileViaSymlink);
expect(isPathWithinAllowedDirectories(resolvedFile, allowedDirsWithBoth)).toBe(true);

// Verify both paths point to the same actual file
expect(resolvedFile).toBe(await fs.realpath(fileViaResolved));

} catch (error) {
// Skip if no symlink permissions on the system
if ((error as NodeJS.ErrnoException).code !== 'EPERM') {
throw error;
}
}
});

it('resolves nested symlink chains completely', async () => {
try {
// Setup: Create target file in forbidden area
Expand Down
18 changes: 14 additions & 4 deletions src/filesystem/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -39,22 +39,32 @@ if (args.length === 0) {
}

// Store allowed directories in normalized and resolved form
let allowedDirectories = await Promise.all(
// We store BOTH the original path AND the resolved path to handle symlinks correctly
// This fixes the macOS /tmp -> /private/tmp symlink issue where users specify /tmp
// but the resolved path is /private/tmp
let allowedDirectories = (await Promise.all(
args.map(async (dir) => {
const expanded = expandHome(dir);
const absolute = path.resolve(expanded);
const normalizedOriginal = normalizePath(absolute);
try {
// Security: Resolve symlinks in allowed directories during startup
// This ensures we know the real paths and can validate against them later
const resolved = await fs.realpath(absolute);
return normalizePath(resolved);
const normalizedResolved = normalizePath(resolved);
// Return both original and resolved paths if they differ
// This allows matching against either /tmp or /private/tmp on macOS
if (normalizedOriginal !== normalizedResolved) {
return [normalizedOriginal, normalizedResolved];
}
return [normalizedResolved];
} catch (error) {
// If we can't resolve (doesn't exist), use the normalized absolute path
// This allows configuring allowed dirs that will be created later
return normalizePath(absolute);
return [normalizedOriginal];
}
})
);
)).flat();

// Validate that all directories exist and are accessible
await Promise.all(allowedDirectories.map(async (dir) => {
Expand Down