Skip to content

Commit 54afae1

Browse files
committed
fix(core): 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.
1 parent 6c3a906 commit 54afae1

4 files changed

Lines changed: 39 additions & 26 deletions

File tree

packages/core/src/config/config.ts

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,6 @@
66

77
import * as fs from 'node:fs';
88
import * as path from 'node:path';
9-
import * as os from 'node:os';
109
import { inspect } from 'node:util';
1110
import process from 'node:process';
1211
import {
@@ -146,7 +145,7 @@ import { SkillManager, type SkillDefinition } from '../skills/skillManager.js';
146145
import { startupProfiler } from '../telemetry/startupProfiler.js';
147146
import type { AgentDefinition } from '../agents/types.js';
148147
import { fetchAdminControls } from '../code_assist/admin/admin_controls.js';
149-
import { isSubpath } from '../utils/paths.js';
148+
import { isSubpath, resolveToRealPath } from '../utils/paths.js';
150149
import { UserHintService } from './userHintService.js';
151150
import { WORKSPACE_POLICY_TIER } from '../policy/config.js';
152151
import { loadPoliciesFromToml } from '../policy/toml-loader.js';
@@ -2374,25 +2373,15 @@ export class Config implements McpContext {
23742373
* @returns true if the path is allowed, false otherwise.
23752374
*/
23762375
isPathAllowed(absolutePath: string): boolean {
2377-
const realpath = (p: string) => {
2378-
let resolved: string;
2379-
try {
2380-
resolved = fs.realpathSync(p);
2381-
} catch {
2382-
resolved = path.resolve(p);
2383-
}
2384-
return os.platform() === 'win32' ? resolved.toLowerCase() : resolved;
2385-
};
2386-
2387-
const resolvedPath = realpath(absolutePath);
2376+
const resolvedPath = resolveToRealPath(absolutePath);
23882377

23892378
const workspaceContext = this.getWorkspaceContext();
23902379
if (workspaceContext.isPathWithinWorkspace(resolvedPath)) {
23912380
return true;
23922381
}
23932382

23942383
const projectTempDir = this.storage.getProjectTempDir();
2395-
const resolvedTempDir = realpath(projectTempDir);
2384+
const resolvedTempDir = resolveToRealPath(projectTempDir);
23962385

23972386
return isSubpath(resolvedTempDir, resolvedPath);
23982387
}

packages/core/src/config/storage.test.ts

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ vi.mock('fs', async (importOriginal) => {
2424
});
2525

2626
import { Storage } from './storage.js';
27-
import { GEMINI_DIR, homedir } from '../utils/paths.js';
27+
import { GEMINI_DIR, homedir, resolveToRealPath } from '../utils/paths.js';
2828
import { ProjectRegistry } from './projectRegistry.js';
2929
import { StorageMigration } from './storageMigration.js';
3030

@@ -279,8 +279,7 @@ describe('Storage – additional helpers', () => {
279279
name: 'custom absolute path outside throws',
280280
customDir: '/absolute/path/to/plans',
281281
expected: '',
282-
expectedError:
283-
"Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '/tmp/project'.",
282+
expectedError: `Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
284283
},
285284
{
286285
name: 'absolute path that happens to be inside project root',
@@ -306,8 +305,7 @@ describe('Storage – additional helpers', () => {
306305
name: 'escaping relative path throws',
307306
customDir: '../escaped-plans',
308307
expected: '',
309-
expectedError:
310-
"Custom plans directory '../escaped-plans' resolves to '/tmp/escaped-plans', which is outside the project root '/tmp/project'.",
308+
expectedError: `Custom plans directory '../escaped-plans' resolves to '${resolveToRealPath(path.resolve(projectRoot, '../escaped-plans'))}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
311309
},
312310
{
313311
name: 'hidden directory starting with ..',

packages/core/src/utils/paths.test.ts

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -521,6 +521,25 @@ describe('resolveToRealPath', () => {
521521

522522
expect(resolveToRealPath(input)).toBe(expected);
523523
});
524+
525+
it('should recursively resolve symlinks for non-existent child paths', () => {
526+
const parentPath = path.resolve('/some/parent/path');
527+
const resolvedParentPath = path.resolve('/resolved/parent/path');
528+
const childPath = path.resolve(parentPath, 'child', 'file.txt');
529+
const expectedPath = path.resolve(resolvedParentPath, 'child', 'file.txt');
530+
531+
vi.spyOn(fs, 'realpathSync').mockImplementation((p) => {
532+
const pStr = p.toString();
533+
if (pStr === parentPath) {
534+
return resolvedParentPath;
535+
}
536+
const err = new Error('ENOENT') as NodeJS.ErrnoException;
537+
err.code = 'ENOENT';
538+
throw err;
539+
});
540+
541+
expect(resolveToRealPath(childPath)).toBe(expectedPath);
542+
});
524543
});
525544

526545
describe('normalizePath', () => {

packages/core/src/utils/paths.ts

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -359,8 +359,8 @@ export function isSubpath(parentPath: string, childPath: string): boolean {
359359
* @param pathStr The path string to resolve.
360360
* @returns The resolved real path.
361361
*/
362-
export function resolveToRealPath(path: string): string {
363-
let resolvedPath = path;
362+
export function resolveToRealPath(pathStr: string): string {
363+
let resolvedPath = pathStr;
364364

365365
try {
366366
if (resolvedPath.startsWith('file://')) {
@@ -372,11 +372,18 @@ export function resolveToRealPath(path: string): string {
372372
// Ignore error (e.g. malformed URI), keep path from previous step
373373
}
374374

375+
return robustRealpath(path.resolve(resolvedPath));
376+
}
377+
378+
function robustRealpath(p: string): string {
375379
try {
376-
return fs.realpathSync(resolvedPath);
377-
} catch (_e) {
378-
// If realpathSync fails, it might be because the path doesn't exist.
379-
// In that case, we can fall back to the path processed.
380-
return resolvedPath;
380+
return fs.realpathSync(p);
381+
} catch (e: unknown) {
382+
if (e && typeof e === 'object' && 'code' in e && e.code === 'ENOENT') {
383+
const parent = path.dirname(p);
384+
if (parent === p) return p;
385+
return path.join(robustRealpath(parent), path.basename(p));
386+
}
387+
return p;
381388
}
382389
}

0 commit comments

Comments
 (0)