From ca0f0a85f543fc23021854d2aff0def2c5a2b882 Mon Sep 17 00:00:00 2001 From: "A.K.M. Adib" Date: Fri, 6 Mar 2026 18:14:18 -0500 Subject: [PATCH] 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. --- packages/core/src/config/config.ts | 17 +++-------------- packages/core/src/config/storage.test.ts | 8 +++----- packages/core/src/utils/paths.test.ts | 19 +++++++++++++++++++ packages/core/src/utils/paths.ts | 21 ++++++++++++++------- 4 files changed, 39 insertions(+), 26 deletions(-) diff --git a/packages/core/src/config/config.ts b/packages/core/src/config/config.ts index e4c0fef6eb..453631710c 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'; @@ -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); } 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..2e13fcd3f8 100644 --- a/packages/core/src/utils/paths.test.ts +++ b/packages/core/src/utils/paths.test.ts @@ -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', () => { diff --git a/packages/core/src/utils/paths.ts b/packages/core/src/utils/paths.ts index f446f31d90..b74e538c50 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,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; } }