mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-03-12 07:01:09 -07:00
fix(core): resolve symlinks for non-existent paths during validation (#21487)
This commit is contained in:
@@ -6,7 +6,6 @@
|
|||||||
|
|
||||||
import * as fs from 'node:fs';
|
import * as fs from 'node:fs';
|
||||||
import * as path from 'node:path';
|
import * as path from 'node:path';
|
||||||
import * as os from 'node:os';
|
|
||||||
import { inspect } from 'node:util';
|
import { inspect } from 'node:util';
|
||||||
import process from 'node:process';
|
import process from 'node:process';
|
||||||
import {
|
import {
|
||||||
@@ -146,7 +145,7 @@ import { SkillManager, type SkillDefinition } from '../skills/skillManager.js';
|
|||||||
import { startupProfiler } from '../telemetry/startupProfiler.js';
|
import { startupProfiler } from '../telemetry/startupProfiler.js';
|
||||||
import type { AgentDefinition } from '../agents/types.js';
|
import type { AgentDefinition } from '../agents/types.js';
|
||||||
import { fetchAdminControls } from '../code_assist/admin/admin_controls.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 { UserHintService } from './userHintService.js';
|
||||||
import { WORKSPACE_POLICY_TIER } from '../policy/config.js';
|
import { WORKSPACE_POLICY_TIER } from '../policy/config.js';
|
||||||
import { loadPoliciesFromToml } from '../policy/toml-loader.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.
|
* @returns true if the path is allowed, false otherwise.
|
||||||
*/
|
*/
|
||||||
isPathAllowed(absolutePath: string): boolean {
|
isPathAllowed(absolutePath: string): boolean {
|
||||||
const realpath = (p: string) => {
|
const resolvedPath = resolveToRealPath(absolutePath);
|
||||||
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 workspaceContext = this.getWorkspaceContext();
|
const workspaceContext = this.getWorkspaceContext();
|
||||||
if (workspaceContext.isPathWithinWorkspace(resolvedPath)) {
|
if (workspaceContext.isPathWithinWorkspace(resolvedPath)) {
|
||||||
@@ -2407,7 +2396,7 @@ export class Config implements McpContext {
|
|||||||
}
|
}
|
||||||
|
|
||||||
const projectTempDir = this.storage.getProjectTempDir();
|
const projectTempDir = this.storage.getProjectTempDir();
|
||||||
const resolvedTempDir = realpath(projectTempDir);
|
const resolvedTempDir = resolveToRealPath(projectTempDir);
|
||||||
|
|
||||||
return isSubpath(resolvedTempDir, resolvedPath);
|
return isSubpath(resolvedTempDir, resolvedPath);
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -24,7 +24,7 @@ vi.mock('fs', async (importOriginal) => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
import { Storage } from './storage.js';
|
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 { ProjectRegistry } from './projectRegistry.js';
|
||||||
import { StorageMigration } from './storageMigration.js';
|
import { StorageMigration } from './storageMigration.js';
|
||||||
|
|
||||||
@@ -279,8 +279,7 @@ describe('Storage – additional helpers', () => {
|
|||||||
name: 'custom absolute path outside throws',
|
name: 'custom absolute path outside throws',
|
||||||
customDir: '/absolute/path/to/plans',
|
customDir: '/absolute/path/to/plans',
|
||||||
expected: '',
|
expected: '',
|
||||||
expectedError:
|
expectedError: `Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
|
||||||
"Custom plans directory '/absolute/path/to/plans' resolves to '/absolute/path/to/plans', which is outside the project root '/tmp/project'.",
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: 'absolute path that happens to be inside project root',
|
name: 'absolute path that happens to be inside project root',
|
||||||
@@ -306,8 +305,7 @@ describe('Storage – additional helpers', () => {
|
|||||||
name: 'escaping relative path throws',
|
name: 'escaping relative path throws',
|
||||||
customDir: '../escaped-plans',
|
customDir: '../escaped-plans',
|
||||||
expected: '',
|
expected: '',
|
||||||
expectedError:
|
expectedError: `Custom plans directory '../escaped-plans' resolves to '${resolveToRealPath(path.resolve(projectRoot, '../escaped-plans'))}', which is outside the project root '${resolveToRealPath(projectRoot)}'.`,
|
||||||
"Custom plans directory '../escaped-plans' resolves to '/tmp/escaped-plans', which is outside the project root '/tmp/project'.",
|
|
||||||
},
|
},
|
||||||
{
|
{
|
||||||
name: 'hidden directory starting with ..',
|
name: 'hidden directory starting with ..',
|
||||||
|
|||||||
@@ -510,9 +510,11 @@ describe('resolveToRealPath', () => {
|
|||||||
expect(resolveToRealPath(input)).toBe(expected);
|
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(() => {
|
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');
|
const p = path.resolve('path', 'to', 'New Project');
|
||||||
@@ -521,6 +523,25 @@ describe('resolveToRealPath', () => {
|
|||||||
|
|
||||||
expect(resolveToRealPath(input)).toBe(expected);
|
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', () => {
|
describe('normalizePath', () => {
|
||||||
|
|||||||
@@ -359,8 +359,8 @@ export function isSubpath(parentPath: string, childPath: string): boolean {
|
|||||||
* @param pathStr The path string to resolve.
|
* @param pathStr The path string to resolve.
|
||||||
* @returns The resolved real path.
|
* @returns The resolved real path.
|
||||||
*/
|
*/
|
||||||
export function resolveToRealPath(path: string): string {
|
export function resolveToRealPath(pathStr: string): string {
|
||||||
let resolvedPath = path;
|
let resolvedPath = pathStr;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
if (resolvedPath.startsWith('file://')) {
|
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
|
// Ignore error (e.g. malformed URI), keep path from previous step
|
||||||
}
|
}
|
||||||
|
|
||||||
|
return robustRealpath(path.resolve(resolvedPath));
|
||||||
|
}
|
||||||
|
|
||||||
|
function robustRealpath(p: string): string {
|
||||||
try {
|
try {
|
||||||
return fs.realpathSync(resolvedPath);
|
return fs.realpathSync(p);
|
||||||
} catch (_e) {
|
} catch (e: unknown) {
|
||||||
// If realpathSync fails, it might be because the path doesn't exist.
|
if (e && typeof e === 'object' && 'code' in e && e.code === 'ENOENT') {
|
||||||
// In that case, we can fall back to the path processed.
|
try {
|
||||||
return resolvedPath;
|
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;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -4,10 +4,10 @@
|
|||||||
* SPDX-License-Identifier: Apache-2.0
|
* SPDX-License-Identifier: Apache-2.0
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { isNodeError } from '../utils/errors.js';
|
|
||||||
import * as fs from 'node:fs';
|
import * as fs from 'node:fs';
|
||||||
import * as path from 'node:path';
|
import * as path from 'node:path';
|
||||||
import { debugLogger } from './debugLogger.js';
|
import { debugLogger } from './debugLogger.js';
|
||||||
|
import { resolveToRealPath } from './paths.js';
|
||||||
|
|
||||||
export type Unsubscribe = () => void;
|
export type Unsubscribe = () => void;
|
||||||
|
|
||||||
@@ -227,22 +227,7 @@ export class WorkspaceContext {
|
|||||||
* if it did exist.
|
* if it did exist.
|
||||||
*/
|
*/
|
||||||
private fullyResolvedPath(pathToCheck: string): string {
|
private fullyResolvedPath(pathToCheck: string): string {
|
||||||
try {
|
return resolveToRealPath(path.resolve(this.targetDir, pathToCheck));
|
||||||
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;
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
/**
|
||||||
@@ -262,15 +247,4 @@ export class WorkspaceContext {
|
|||||||
!path.isAbsolute(relative)
|
!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;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|||||||
Reference in New Issue
Block a user