From 7837194ab54fb66ee88153734c9be53b10085340 Mon Sep 17 00:00:00 2001 From: Adib234 <30782825+Adib234@users.noreply.github.com> Date: Mon, 9 Mar 2026 12:02:13 -0400 Subject: [PATCH] fix(core): resolve symlinks for non-existent paths during validation (#21487) --- packages/core/src/config/config.ts | 17 ++--------- packages/core/src/config/storage.test.ts | 8 ++---- packages/core/src/utils/paths.test.ts | 25 +++++++++++++++-- packages/core/src/utils/paths.ts | 31 ++++++++++++++++----- packages/core/src/utils/workspaceContext.ts | 30 ++------------------ 5 files changed, 55 insertions(+), 56 deletions(-) diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index 775547e3ec..adb36ca3a3 100644 --- a/packages/core/src/config/config.ts +++ b/packages/core/src/config/config.ts @@ -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'; @@ -2389,17 +2388,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)) { @@ -2407,7 +2396,7 @@ export class Config implements McpContext { } const projectTempDir = this.storage.getProjectTempDir(); - const resolvedTempDir = realpath(projectTempDir); + const resolvedTempDir = resolveToRealPath(projectTempDir); return isSubpath(resolvedTempDir, resolvedPath); } diff --git a/packages/core/src/config/storage.test.ts b/packages/core/src/config/storage.test.ts index 15b49d12f1..6b1cd39d88 100644 --- a/packages/core/src/config/storage.test.ts +++ b/packages/core/src/config/storage.test.ts @@ -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 ..', diff --git a/packages/core/src/utils/paths.test.ts b/packages/core/src/utils/paths.test.ts index a3151438bb..227afaf44a 100644 --- a/packages/core/src/utils/paths.test.ts +++ b/packages/core/src/utils/paths.test.ts @@ -510,9 +510,11 @@ describe('resolveToRealPath', () => { expect(resolveToRealPath(input)).toBe(expected); }); - it('should return decoded path even if fs.realpathSync fails', () => { + it('should return decoded path even if fs.realpathSync fails with ENOENT', () => { vi.spyOn(fs, 'realpathSync').mockImplementationOnce(() => { - throw new Error('File not found'); + const err = new Error('File not found') as NodeJS.ErrnoException; + err.code = 'ENOENT'; + throw err; }); const p = path.resolve('path', 'to', 'New Project'); @@ -521,6 +523,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', () => { diff --git a/packages/core/src/utils/paths.ts b/packages/core/src/utils/paths.ts index f446f31d90..aa167e3558 100644 --- a/packages/core/src/utils/paths.ts +++ b/packages/core/src/utils/paths.ts @@ -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,28 @@ 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') { + try { + const stat = fs.lstatSync(p); + if (stat.isSymbolicLink()) { + const target = fs.readlinkSync(p); + const resolvedTarget = path.resolve(path.dirname(p), target); + return robustRealpath(resolvedTarget); + } + } catch { + // Not a symlink, or lstat failed. Just resolve parent. + } + const parent = path.dirname(p); + if (parent === p) return p; + return path.join(robustRealpath(parent), path.basename(p)); + } + throw e; } } diff --git a/packages/core/src/utils/workspaceContext.ts b/packages/core/src/utils/workspaceContext.ts index dfb47ce3be..7ca59fb184 100755 --- a/packages/core/src/utils/workspaceContext.ts +++ b/packages/core/src/utils/workspaceContext.ts @@ -4,10 +4,10 @@ * SPDX-License-Identifier: Apache-2.0 */ -import { isNodeError } from '../utils/errors.js'; import * as fs from 'node:fs'; import * as path from 'node:path'; import { debugLogger } from './debugLogger.js'; +import { resolveToRealPath } from './paths.js'; export type Unsubscribe = () => void; @@ -227,22 +227,7 @@ export class WorkspaceContext { * if it did exist. */ private fullyResolvedPath(pathToCheck: string): string { - try { - return fs.realpathSync(path.resolve(this.targetDir, pathToCheck)); - } catch (e: unknown) { - if ( - isNodeError(e) && - e.code === 'ENOENT' && - e.path && - // realpathSync does not set e.path correctly for symlinks to - // non-existent files. - !this.isFileSymlink(e.path) - ) { - // If it doesn't exist, e.path contains the fully resolved path. - return e.path; - } - throw e; - } + return resolveToRealPath(path.resolve(this.targetDir, pathToCheck)); } /** @@ -262,15 +247,4 @@ export class WorkspaceContext { !path.isAbsolute(relative) ); } - - /** - * Checks if a file path is a symbolic link that points to a file. - */ - private isFileSymlink(filePath: string): boolean { - try { - return !fs.readlinkSync(filePath).endsWith('/'); - } catch (_error) { - return false; - } - } }