Skip to content

Commit d3aa20b

Browse files
authored
fix(core): resolve symlinks for non-existent paths during validation (google-gemini#21487)
1 parent 17c7322 commit d3aa20b

5 files changed

Lines changed: 55 additions & 56 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';
@@ -2389,25 +2388,15 @@ export class Config implements McpContext {
23892388
* @returns true if the path is allowed, false otherwise.
23902389
*/
23912390
isPathAllowed(absolutePath: string): boolean {
2392-
const realpath = (p: string) => {
2393-
let resolved: string;
2394-
try {
2395-
resolved = fs.realpathSync(p);
2396-
} catch {
2397-
resolved = path.resolve(p);
2398-
}
2399-
return os.platform() === 'win32' ? resolved.toLowerCase() : resolved;
2400-
};
2401-
2402-
const resolvedPath = realpath(absolutePath);
2391+
const resolvedPath = resolveToRealPath(absolutePath);
24032392

24042393
const workspaceContext = this.getWorkspaceContext();
24052394
if (workspaceContext.isPathWithinWorkspace(resolvedPath)) {
24062395
return true;
24072396
}
24082397

24092398
const projectTempDir = this.storage.getProjectTempDir();
2410-
const resolvedTempDir = realpath(projectTempDir);
2399+
const resolvedTempDir = resolveToRealPath(projectTempDir);
24112400

24122401
return isSubpath(resolvedTempDir, resolvedPath);
24132402
}

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 & 28 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
/**
@@ -262,15 +247,4 @@ export class WorkspaceContext {
262247
!path.isAbsolute(relative)
263248
);
264249
}
265-
266-
/**
267-
* Checks if a file path is a symbolic link that points to a file.
268-
*/
269-
private isFileSymlink(filePath: string): boolean {
270-
try {
271-
return !fs.readlinkSync(filePath).endsWith('/');
272-
} catch (_error) {
273-
return false;
274-
}
275-
}
276250
}

0 commit comments

Comments
 (0)