Skip to content

Commit c478436

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 c478436

5 files changed

Lines changed: 55 additions & 45 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: 23 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -510,9 +510,11 @@ describe('resolveToRealPath', () => {
510510
expect(resolveToRealPath(input)).toBe(expected);
511511
});
512512

513-
it('should return decoded path even if fs.realpathSync fails', () => {
513+
it('should return decoded path even if fs.realpathSync fails with ENOENT', () => {
514514
vi.spyOn(fs, 'realpathSync').mockImplementationOnce(() => {
515-
throw new Error('File not found');
515+
const err = new Error('File not found') as NodeJS.ErrnoException;
516+
err.code = 'ENOENT';
517+
throw err;
516518
});
517519

518520
const p = path.resolve('path', 'to', 'New Project');
@@ -521,6 +523,25 @@ describe('resolveToRealPath', () => {
521523

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

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

packages/core/src/utils/paths.ts

Lines changed: 24 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,28 @@ 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+
try {
384+
const stat = fs.lstatSync(p);
385+
if (stat.isSymbolicLink()) {
386+
const target = fs.readlinkSync(p);
387+
const resolvedTarget = path.resolve(path.dirname(p), target);
388+
return robustRealpath(resolvedTarget);
389+
}
390+
} catch {
391+
// Not a symlink, or lstat failed. Just resolve parent.
392+
}
393+
const parent = path.dirname(p);
394+
if (parent === p) return p;
395+
return path.join(robustRealpath(parent), path.basename(p));
396+
}
397+
throw e;
381398
}
382399
}

packages/core/src/utils/workspaceContext.ts

Lines changed: 2 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -4,10 +4,10 @@
44
* SPDX-License-Identifier: Apache-2.0
55
*/
66

7-
import { isNodeError } from '../utils/errors.js';
87
import * as fs from 'node:fs';
98
import * as path from 'node:path';
109
import { debugLogger } from './debugLogger.js';
10+
import { resolveToRealPath } from './paths.js';
1111

1212
export type Unsubscribe = () => void;
1313

@@ -227,22 +227,7 @@ export class WorkspaceContext {
227227
* if it did exist.
228228
*/
229229
private fullyResolvedPath(pathToCheck: string): string {
230-
try {
231-
return fs.realpathSync(path.resolve(this.targetDir, pathToCheck));
232-
} catch (e: unknown) {
233-
if (
234-
isNodeError(e) &&
235-
e.code === 'ENOENT' &&
236-
e.path &&
237-
// realpathSync does not set e.path correctly for symlinks to
238-
// non-existent files.
239-
!this.isFileSymlink(e.path)
240-
) {
241-
// If it doesn't exist, e.path contains the fully resolved path.
242-
return e.path;
243-
}
244-
throw e;
245-
}
230+
return resolveToRealPath(path.resolve(this.targetDir, pathToCheck));
246231
}
247232

248233
/**

0 commit comments

Comments
 (0)