fix: resolve symlinks for non-existent paths during validation

The path validation logic in Config.isPathAllowed failed when attempting
to write a new file to a directory that is a symbolic link. This happened
because fs.realpathSync fails on non-existent paths, falling back to an
unresolved path which then mismatches with the resolved project temporary
directory during the isSubpath check.

This commit updates resolveToRealPath to robustly resolve parent
directories even if the leaf file does not exist, and updates
isPathAllowed to use this improved helper.
This commit is contained in:
A.K.M. Adib
2026-03-06 18:14:18 -05:00
parent 53a3fc9776
commit ca0f0a85f5
4 changed files with 39 additions and 26 deletions

View File

@@ -6,7 +6,6 @@
import * as fs from 'node:fs';
import * as path from 'node:path';
import * as os from 'node:os';
import { inspect } from 'node:util';
import process from 'node:process';
import {
@@ -146,7 +145,7 @@ import { SkillManager, type SkillDefinition } from '../skills/skillManager.js';
import { startupProfiler } from '../telemetry/startupProfiler.js';
import type { AgentDefinition } from '../agents/types.js';
import { fetchAdminControls } from '../code_assist/admin/admin_controls.js';
import { isSubpath } from '../utils/paths.js';
import { isSubpath, resolveToRealPath } from '../utils/paths.js';
import { UserHintService } from './userHintService.js';
import { WORKSPACE_POLICY_TIER } from '../policy/config.js';
import { loadPoliciesFromToml } from '../policy/toml-loader.js';
@@ -2374,17 +2373,7 @@ export class Config implements McpContext {
* @returns true if the path is allowed, false otherwise.
*/
isPathAllowed(absolutePath: string): boolean {
const realpath = (p: string) => {
let resolved: string;
try {
resolved = fs.realpathSync(p);
} catch {
resolved = path.resolve(p);
}
return os.platform() === 'win32' ? resolved.toLowerCase() : resolved;
};
const resolvedPath = realpath(absolutePath);
const resolvedPath = resolveToRealPath(absolutePath);
const workspaceContext = this.getWorkspaceContext();
if (workspaceContext.isPathWithinWorkspace(resolvedPath)) {
@@ -2392,7 +2381,7 @@ export class Config implements McpContext {
}
const projectTempDir = this.storage.getProjectTempDir();
const resolvedTempDir = realpath(projectTempDir);
const resolvedTempDir = resolveToRealPath(projectTempDir);
return isSubpath(resolvedTempDir, resolvedPath);
}

View File

@@ -24,7 +24,7 @@ vi.mock('fs', async (importOriginal) => {
});
import { Storage } from './storage.js';
import { GEMINI_DIR, homedir } from '../utils/paths.js';
import { GEMINI_DIR, homedir, resolveToRealPath } from '../utils/paths.js';
import { ProjectRegistry } from './projectRegistry.js';
import { StorageMigration } from './storageMigration.js';
@@ -279,8 +279,7 @@ describe('Storage additional helpers', () => {
name: 'custom absolute path outside throws',
customDir: '/absolute/path/to/plans',
expected: '',
expectedError:
"Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '/tmp/project'.",
expectedError: `Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
},
{
name: 'absolute path that happens to be inside project root',
@@ -306,8 +305,7 @@ describe('Storage additional helpers', () => {
name: 'escaping relative path throws',
customDir: '../escaped-plans',
expected: '',
expectedError:
"Custom plans directory '../escaped-plans' resolves to '/tmp/escaped-plans', which is outside the project root '/tmp/project'.",
expectedError: `Custom plans directory '../escaped-plans' resolves to '${resolveToRealPath(path.resolve(projectRoot, '../escaped-plans'))}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
},
{
name: 'hidden directory starting with ..',

View File

@@ -521,6 +521,25 @@ describe('resolveToRealPath', () => {
expect(resolveToRealPath(input)).toBe(expected);
});
it('should recursively resolve symlinks for non-existent child paths', () => {
const parentPath = path.resolve('/some/parent/path');
const resolvedParentPath = path.resolve('/resolved/parent/path');
const childPath = path.resolve(parentPath, 'child', 'file.txt');
const expectedPath = path.resolve(resolvedParentPath, 'child', 'file.txt');
vi.spyOn(fs, 'realpathSync').mockImplementation((p) => {
const pStr = p.toString();
if (pStr === parentPath) {
return resolvedParentPath;
}
const err = new Error('ENOENT') as NodeJS.ErrnoException;
err.code = 'ENOENT';
throw err;
});
expect(resolveToRealPath(childPath)).toBe(expectedPath);
});
});
describe('normalizePath', () => {

View File

@@ -359,8 +359,8 @@ export function isSubpath(parentPath: string, childPath: string): boolean {
* @param pathStr The path string to resolve.
* @returns The resolved real path.
*/
export function resolveToRealPath(path: string): string {
let resolvedPath = path;
export function resolveToRealPath(pathStr: string): string {
let resolvedPath = pathStr;
try {
if (resolvedPath.startsWith('file://')) {
@@ -372,11 +372,18 @@ export function resolveToRealPath(path: string): string {
// Ignore error (e.g. malformed URI), keep path from previous step
}
return robustRealpath(path.resolve(resolvedPath));
}
function robustRealpath(p: string): string {
try {
return fs.realpathSync(resolvedPath);
} catch (_e) {
// If realpathSync fails, it might be because the path doesn't exist.
// In that case, we can fall back to the path processed.
return resolvedPath;
return fs.realpathSync(p);
} catch (e: unknown) {
if (e && typeof e === 'object' && 'code' in e && e.code === 'ENOENT') {
const parent = path.dirname(p);
if (parent === p) return p;
return path.join(robustRealpath(parent), path.basename(p));
}
return p;
}
}