mirror of
https://github.com/google-gemini/gemini-cli.git
synced 2026-05-29 23:50:09 +00:00
fix(core): centralize path validation to prevent crashes from malformed prompts
This change consolidates path validation into the central Config.validatePathAccess method. It introduces a PathValidator utility that performs pre-flight checks for length, invalid characters, and log markers. This automatically protects all tools using workspace boundary checks. Additionally, CLI-level at-command resolution is consolidated into a shared utility. Fixes #25972
This commit is contained in:
@@ -49,6 +49,7 @@ import {
|
||||
buildAvailableModes,
|
||||
RequestPermissionResponseSchema,
|
||||
} from './acpUtils.js';
|
||||
import { resolveAtCommandPath } from '../utils/atCommandUtils.js';
|
||||
import { z } from 'zod';
|
||||
import { getAcpErrorMessage } from './acpErrors.js';
|
||||
|
||||
@@ -928,13 +929,24 @@ export class Session {
|
||||
let currentPathSpec = pathName;
|
||||
let resolvedSuccessfully = false;
|
||||
let readDirectly = false;
|
||||
try {
|
||||
const absolutePath = path.resolve(
|
||||
|
||||
const resolved = await resolveAtCommandPath(
|
||||
pathName,
|
||||
this.context.config,
|
||||
(msg) => this.debug(msg),
|
||||
);
|
||||
|
||||
let validationError: string | null = null;
|
||||
let absolutePath: string;
|
||||
|
||||
if (!resolved) {
|
||||
// If not resolved, we still check if it's an unauthorized absolute path that we can ask permission for.
|
||||
absolutePath = path.resolve(
|
||||
this.context.config.getTargetDir(),
|
||||
pathName,
|
||||
);
|
||||
|
||||
let validationError = this.context.config.validatePathAccess(
|
||||
validationError = this.context.config.validatePathAccess(
|
||||
absolutePath,
|
||||
'read',
|
||||
);
|
||||
@@ -1020,7 +1032,11 @@ export class Session {
|
||||
});
|
||||
}
|
||||
}
|
||||
} else {
|
||||
absolutePath = resolved.absolutePath;
|
||||
}
|
||||
|
||||
try {
|
||||
if (!validationError) {
|
||||
// If it's an absolute path that is authorized (e.g. added via readOnlyPaths),
|
||||
// read it directly to avoid ReadManyFilesTool absolute path resolution issues.
|
||||
@@ -1033,7 +1049,9 @@ export class Session {
|
||||
!readDirectly
|
||||
) {
|
||||
try {
|
||||
const stats = await fs.stat(absolutePath);
|
||||
const stats = resolved
|
||||
? resolved.stats
|
||||
: await fs.stat(absolutePath);
|
||||
if (stats.isFile()) {
|
||||
const fileReadResult = await processSingleFileContent(
|
||||
absolutePath,
|
||||
@@ -1092,7 +1110,9 @@ export class Session {
|
||||
}
|
||||
|
||||
if (!readDirectly) {
|
||||
const stats = await fs.stat(absolutePath);
|
||||
const stats = resolved
|
||||
? resolved.stats
|
||||
: await fs.stat(absolutePath);
|
||||
if (stats.isDirectory()) {
|
||||
currentPathSpec = pathName.endsWith('/')
|
||||
? `${pathName}**`
|
||||
|
||||
@@ -25,6 +25,7 @@ import type {
|
||||
HistoryItemToolGroup,
|
||||
IndividualToolCallDisplay,
|
||||
} from '../types.js';
|
||||
import { resolveAtCommandPath } from '../../utils/atCommandUtils.js';
|
||||
import type { UseHistoryManagerReturn } from './useHistoryManager.js';
|
||||
|
||||
const REF_CONTENT_HEADER = `\n${REFERENCE_CONTENT_START}`;
|
||||
@@ -271,103 +272,109 @@ async function resolveFilePaths(
|
||||
continue;
|
||||
}
|
||||
|
||||
for (const dir of config.getWorkspaceContext().getDirectories()) {
|
||||
try {
|
||||
const absolutePath = path.resolve(dir, pathName);
|
||||
const stats = await fs.stat(absolutePath);
|
||||
|
||||
const relativePath = path.isAbsolute(pathName)
|
||||
? path.relative(dir, absolutePath)
|
||||
: pathName;
|
||||
|
||||
if (stats.isDirectory()) {
|
||||
const pathSpec = path.join(relativePath, '**');
|
||||
resolvedFiles.push({
|
||||
part,
|
||||
pathSpec,
|
||||
displayLabel: path.isAbsolute(pathName) ? relativePath : pathName,
|
||||
absolutePath,
|
||||
});
|
||||
onDebugMessage(
|
||||
`Path ${pathName} resolved to directory, using glob: ${pathSpec}`,
|
||||
);
|
||||
} else {
|
||||
resolvedFiles.push({
|
||||
part,
|
||||
pathSpec: relativePath,
|
||||
displayLabel: path.isAbsolute(pathName) ? relativePath : pathName,
|
||||
absolutePath,
|
||||
});
|
||||
onDebugMessage(
|
||||
`Path ${pathName} resolved to file: ${absolutePath}, using relative path: ${relativePath}`,
|
||||
);
|
||||
}
|
||||
break;
|
||||
} catch (error) {
|
||||
if (isNodeError(error) && error.code === 'ENOENT') {
|
||||
if (config.getEnableRecursiveFileSearch() && globTool) {
|
||||
onDebugMessage(
|
||||
`Path ${pathName} not found directly, attempting glob search.`,
|
||||
);
|
||||
try {
|
||||
const globResult = await globTool.buildAndExecute(
|
||||
{
|
||||
pattern: `**/*${pathName}*`,
|
||||
path: dir,
|
||||
},
|
||||
signal,
|
||||
const resolved = await resolveAtCommandPath(
|
||||
pathName,
|
||||
config,
|
||||
onDebugMessage,
|
||||
);
|
||||
if (resolved) {
|
||||
const absolutePath = resolved.absolutePath;
|
||||
const relativePath = resolved.relativePath;
|
||||
const stats = resolved.stats;
|
||||
if (stats.isDirectory()) {
|
||||
const pathSpec = path.join(relativePath, '**');
|
||||
resolvedFiles.push({
|
||||
part,
|
||||
pathSpec,
|
||||
displayLabel: path.isAbsolute(pathName) ? relativePath : pathName,
|
||||
absolutePath,
|
||||
});
|
||||
onDebugMessage(
|
||||
`Path ${pathName} resolved to directory, using glob: ${pathSpec}`,
|
||||
);
|
||||
} else {
|
||||
resolvedFiles.push({
|
||||
part,
|
||||
pathSpec: relativePath,
|
||||
displayLabel: path.isAbsolute(pathName) ? relativePath : pathName,
|
||||
absolutePath,
|
||||
});
|
||||
onDebugMessage(
|
||||
`Path ${pathName} resolved to file: ${absolutePath}, using relative path: ${relativePath}`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
// If direct resolution fails, we keep the original loop for glob fallback if enabled
|
||||
for (const dir of config.getWorkspaceContext().getDirectories()) {
|
||||
try {
|
||||
const absolutePath = path.resolve(dir, pathName);
|
||||
await fs.stat(absolutePath);
|
||||
} catch (error) {
|
||||
if (isNodeError(error) && error.code === 'ENOENT') {
|
||||
if (config.getEnableRecursiveFileSearch() && globTool) {
|
||||
onDebugMessage(
|
||||
`Path ${pathName} not found directly, attempting glob search.`,
|
||||
);
|
||||
if (
|
||||
globResult.llmContent &&
|
||||
typeof globResult.llmContent === 'string' &&
|
||||
!globResult.llmContent.startsWith('No files found') &&
|
||||
!globResult.llmContent.startsWith('Error:')
|
||||
) {
|
||||
const lines = globResult.llmContent.split('\n');
|
||||
if (lines.length > 1 && lines[1]) {
|
||||
const firstMatchAbsolute = lines[1].trim();
|
||||
const pathSpec = path.relative(dir, firstMatchAbsolute);
|
||||
resolvedFiles.push({
|
||||
part,
|
||||
pathSpec,
|
||||
displayLabel: path.isAbsolute(pathName)
|
||||
? pathSpec
|
||||
: pathName,
|
||||
});
|
||||
onDebugMessage(
|
||||
`Glob search for ${pathName} found ${firstMatchAbsolute}, using relative path: ${pathSpec}`,
|
||||
);
|
||||
break;
|
||||
try {
|
||||
const globResult = await globTool.buildAndExecute(
|
||||
{
|
||||
pattern: `**/*${pathName}*`,
|
||||
path: dir,
|
||||
},
|
||||
signal,
|
||||
);
|
||||
if (
|
||||
globResult.llmContent &&
|
||||
typeof globResult.llmContent === 'string' &&
|
||||
!globResult.llmContent.startsWith('No files found') &&
|
||||
!globResult.llmContent.startsWith('Error:')
|
||||
) {
|
||||
const lines = globResult.llmContent.split('\n');
|
||||
if (lines.length > 1 && lines[1]) {
|
||||
const firstMatchAbsolute = lines[1].trim();
|
||||
const pathSpec = path.relative(dir, firstMatchAbsolute);
|
||||
resolvedFiles.push({
|
||||
part,
|
||||
pathSpec,
|
||||
displayLabel: path.isAbsolute(pathName)
|
||||
? pathSpec
|
||||
: pathName,
|
||||
});
|
||||
onDebugMessage(
|
||||
`Glob search for ${pathName} found ${firstMatchAbsolute}, using relative path: ${pathSpec}`,
|
||||
);
|
||||
break;
|
||||
} else {
|
||||
onDebugMessage(
|
||||
`Glob search for '**/*${pathName}*' did not return a usable path. Path ${pathName} will be skipped.`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
onDebugMessage(
|
||||
`Glob search for '**/*${pathName}*' did not return a usable path. Path ${pathName} will be skipped.`,
|
||||
`Glob search for '**/*${pathName}*' found no files or an error. Path ${pathName} will be skipped.`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
} catch (globError) {
|
||||
debugLogger.warn(
|
||||
`Error during glob search for ${pathName}: ${getErrorMessage(globError)}`,
|
||||
);
|
||||
onDebugMessage(
|
||||
`Glob search for '**/*${pathName}*' found no files or an error. Path ${pathName} will be skipped.`,
|
||||
`Error during glob search for ${pathName}. Path ${pathName} will be skipped.`,
|
||||
);
|
||||
}
|
||||
} catch (globError) {
|
||||
debugLogger.warn(
|
||||
`Error during glob search for ${pathName}: ${getErrorMessage(globError)}`,
|
||||
);
|
||||
} else {
|
||||
onDebugMessage(
|
||||
`Error during glob search for ${pathName}. Path ${pathName} will be skipped.`,
|
||||
`Glob tool not found. Path ${pathName} will be skipped.`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
debugLogger.warn(
|
||||
`Error stating path ${pathName}: ${getErrorMessage(error)}`,
|
||||
);
|
||||
onDebugMessage(
|
||||
`Glob tool not found. Path ${pathName} will be skipped.`,
|
||||
`Error stating path ${pathName}. Path ${pathName} will be skipped.`,
|
||||
);
|
||||
}
|
||||
} else {
|
||||
debugLogger.warn(
|
||||
`Error stating path ${pathName}: ${getErrorMessage(error)}`,
|
||||
);
|
||||
onDebugMessage(
|
||||
`Error stating path ${pathName}. Path ${pathName} will be skipped.`,
|
||||
);
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
83
packages/cli/src/utils/atCommandUtils.ts
Normal file
83
packages/cli/src/utils/atCommandUtils.ts
Normal file
@@ -0,0 +1,83 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import * as path from 'node:path';
|
||||
import * as fs from 'node:fs/promises';
|
||||
import {
|
||||
validatePath,
|
||||
isWithinRoot,
|
||||
type Config,
|
||||
} from '@google/gemini-cli-core';
|
||||
|
||||
export interface ResolvedAtCommandPath {
|
||||
absolutePath: string;
|
||||
relativePath: string;
|
||||
stats: {
|
||||
isDirectory(): boolean;
|
||||
isFile(): boolean;
|
||||
};
|
||||
}
|
||||
|
||||
/**
|
||||
* Resolves a path from an @-command, ensuring it is valid and within workspace boundaries.
|
||||
*/
|
||||
export async function resolveAtCommandPath(
|
||||
pathName: string,
|
||||
config: Config,
|
||||
onDebugMessage: (msg: string) => void = () => {},
|
||||
): Promise<ResolvedAtCommandPath | null> {
|
||||
const pathValidation = validatePath(pathName);
|
||||
if (!pathValidation.isValid) {
|
||||
onDebugMessage(
|
||||
`Skipping invalid path in @-command: ${pathName}. Reason: ${pathValidation.error}`,
|
||||
);
|
||||
return null;
|
||||
}
|
||||
|
||||
for (const dir of config.getWorkspaceContext().getDirectories()) {
|
||||
try {
|
||||
const absolutePath = path.resolve(dir, pathName);
|
||||
|
||||
const resolvedValidation = validatePath(absolutePath);
|
||||
if (!resolvedValidation.isValid) {
|
||||
onDebugMessage(
|
||||
`Skipping invalid resolved path in @-command: ${absolutePath}. Reason: ${resolvedValidation.error}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
|
||||
// Final workspace boundary check using centralized logic
|
||||
const validationError = config.validatePathAccess(absolutePath, 'read');
|
||||
if (validationError) {
|
||||
// If it's outside root, we might still allow it with explicit user permission in acpSession,
|
||||
// but for now, we follow the general rule.
|
||||
if (!isWithinRoot(absolutePath, config.getTargetDir())) {
|
||||
// Proceed to stat check, calling sites will handle permission dialogs if needed
|
||||
} else {
|
||||
onDebugMessage(
|
||||
`Skipping unauthorized path: ${absolutePath}. Reason: ${validationError}`,
|
||||
);
|
||||
continue;
|
||||
}
|
||||
}
|
||||
|
||||
const stats = await fs.stat(absolutePath);
|
||||
const relativePath = path.isAbsolute(pathName)
|
||||
? path.relative(dir, absolutePath)
|
||||
: pathName;
|
||||
|
||||
return {
|
||||
absolutePath,
|
||||
relativePath,
|
||||
stats,
|
||||
};
|
||||
} catch {
|
||||
// Ignore errors for this specific directory, try next
|
||||
}
|
||||
}
|
||||
|
||||
return null;
|
||||
}
|
||||
@@ -177,6 +177,7 @@ import { startupProfiler } from '../telemetry/startupProfiler.js';
|
||||
import type { AgentDefinition } from '../agents/types.js';
|
||||
import { fetchAdminControls } from '../code_assist/admin/admin_controls.js';
|
||||
import { isSubpath, resolveToRealPath } from '../utils/paths.js';
|
||||
import { validatePath } from '../utils/path-validator.js';
|
||||
import { InjectionService } from './injectionService.js';
|
||||
import { ExecutionLifecycleService } from '../services/executionLifecycleService.js';
|
||||
import { WORKSPACE_POLICY_TIER } from '../policy/config.js';
|
||||
@@ -3289,6 +3290,11 @@ export class Config implements McpContext, AgentLoopContext {
|
||||
absolutePath: string,
|
||||
checkType: 'read' | 'write' = 'write',
|
||||
): string | null {
|
||||
const pathValidation = validatePath(absolutePath);
|
||||
if (!pathValidation.isValid) {
|
||||
return `Invalid path: ${pathValidation.error}`;
|
||||
}
|
||||
|
||||
if (checkType === 'write' && hasScopedAutoMemoryExtractionWriteAccess()) {
|
||||
const resolvedPath = resolveToRealPath(absolutePath);
|
||||
if (
|
||||
|
||||
@@ -92,6 +92,7 @@ export * from './utils/sessionOperations.js';
|
||||
export * from './utils/planUtils.js';
|
||||
export * from './utils/approvalModeUtils.js';
|
||||
export * from './utils/fileDiffUtils.js';
|
||||
export * from './utils/path-validator.js';
|
||||
export * from './utils/retry.js';
|
||||
export * from './utils/shell-utils.js';
|
||||
export {
|
||||
|
||||
77
packages/core/src/utils/path-validator.test.ts
Normal file
77
packages/core/src/utils/path-validator.test.ts
Normal file
@@ -0,0 +1,77 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
import { describe, it, expect } from 'vitest';
|
||||
import { validatePath } from './path-validator.js';
|
||||
|
||||
describe('PathValidator', () => {
|
||||
it('should validate normal paths', () => {
|
||||
expect(validatePath('src/index.ts').isValid).toBe(true);
|
||||
expect(validatePath('/usr/local/bin').isValid).toBe(true);
|
||||
expect(validatePath('C:\\Users\\name\\Documents').isValid).toBe(true);
|
||||
expect(validatePath('relative/path/to/file.js').isValid).toBe(true);
|
||||
});
|
||||
|
||||
it('should reject empty or non-string paths', () => {
|
||||
expect(validatePath('').isValid).toBe(false);
|
||||
expect(validatePath(null as unknown as string).isValid).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject paths with newlines or control characters', () => {
|
||||
expect(validatePath('path/with\nnewline').isValid).toBe(false);
|
||||
expect(validatePath('path/with\rreturn').isValid).toBe(false);
|
||||
expect(validatePath('path/with\0null').isValid).toBe(false);
|
||||
expect(validatePath('path/with\ttab').isValid).toBe(false);
|
||||
});
|
||||
|
||||
it('should reject excessively long paths', () => {
|
||||
const longPath = 'a'.repeat(4097);
|
||||
const result = validatePath(longPath);
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('Path is too long');
|
||||
});
|
||||
|
||||
it('should reject paths with excessively long components', () => {
|
||||
const longComponent = 'a'.repeat(256);
|
||||
const result = validatePath(`path/to/${longComponent}/file`);
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain(
|
||||
'component "aaaaaaaaaaaaaaaaaaaa..." is too long',
|
||||
);
|
||||
});
|
||||
|
||||
it('should reject misinterpreted log fragments with quotes or ellipses', () => {
|
||||
const logFragment =
|
||||
'Error: No "formatTimeRange" export is defined on the lib/formatTimeRange mock.';
|
||||
const result = validatePath(logFragment);
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain('suspicious characters');
|
||||
});
|
||||
|
||||
it('should reject code snippets with braces/parens', () => {
|
||||
const codeSnippet =
|
||||
'vi.mock(import("@/lib/formatTimeRange"), async (importOriginal) => { return { ...actual }; })';
|
||||
const result = validatePath(codeSnippet);
|
||||
expect(result.isValid).toBe(false);
|
||||
expect(result.error).toContain(
|
||||
'misinterpreted log fragment or code snippet',
|
||||
);
|
||||
});
|
||||
|
||||
it('should reject misinterpreted log fragments with log markers', () => {
|
||||
expect(validatePath('FAIL tests/int/my.test.ts').isValid).toBe(false);
|
||||
expect(
|
||||
validatePath('AssertionError: expected true to be false').isValid,
|
||||
).toBe(false);
|
||||
expect(validatePath('✓ test passed').isValid).toBe(false);
|
||||
expect(validatePath('× test failed').isValid).toBe(false);
|
||||
});
|
||||
|
||||
it('should allow short paths with quotes (even if unusual)', () => {
|
||||
// Some systems might technically allow this, and we only want to block long/obvious log fragments
|
||||
expect(validatePath('file"with"quote.txt').isValid).toBe(true);
|
||||
});
|
||||
});
|
||||
103
packages/core/src/utils/path-validator.ts
Normal file
103
packages/core/src/utils/path-validator.ts
Normal file
@@ -0,0 +1,103 @@
|
||||
/**
|
||||
* @license
|
||||
* Copyright 2026 Google LLC
|
||||
* SPDX-License-Identifier: Apache-2.0
|
||||
*/
|
||||
|
||||
/**
|
||||
* Result of a path validation check.
|
||||
*/
|
||||
export interface PathValidationResult {
|
||||
isValid: boolean;
|
||||
error?: string;
|
||||
}
|
||||
|
||||
/**
|
||||
* Common path limits.
|
||||
* While some OSs support longer, 4096 is a safe cross-platform limit for absolute paths.
|
||||
* Individual components are usually limited to 255.
|
||||
*/
|
||||
const MAX_PATH_LENGTH = 4096;
|
||||
const MAX_COMPONENT_LENGTH = 255;
|
||||
|
||||
/**
|
||||
* Validates a path string for common issues that lead to system-level crashes (like ENAMETOOLONG).
|
||||
* This is intended as a "pre-flight" check for paths derived from untrusted model output.
|
||||
*/
|
||||
export function validatePath(pathStr: string): PathValidationResult {
|
||||
if (!pathStr || typeof pathStr !== 'string') {
|
||||
return { isValid: false, error: 'Path must be a non-empty string.' };
|
||||
}
|
||||
|
||||
// Check for obviously invalid characters (newlines, control characters, null bytes)
|
||||
// These often appear when the model misinterprets logs as paths.
|
||||
if (/[\n\r\0\t]/.test(pathStr)) {
|
||||
return {
|
||||
isValid: false,
|
||||
error:
|
||||
'Path contains invalid characters (newlines or control characters).',
|
||||
};
|
||||
}
|
||||
|
||||
// Check for common log/error patterns that are definitely not paths
|
||||
const logMarkers = [
|
||||
'AssertionError',
|
||||
'FAIL',
|
||||
'✓',
|
||||
'×',
|
||||
'TestingLibraryElementError',
|
||||
];
|
||||
for (const marker of logMarkers) {
|
||||
if (pathStr.includes(marker)) {
|
||||
return {
|
||||
isValid: false,
|
||||
error: 'Path appears to be a misinterpreted log fragment.',
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// If it contains characters like '{', '}', '(', ')', '[', ']' and it's long, it's likely a log
|
||||
if (pathStr.length > 50 && /[{}[\]()]/.test(pathStr)) {
|
||||
return {
|
||||
isValid: false,
|
||||
error:
|
||||
'Path appears to be a misinterpreted log fragment or code snippet.',
|
||||
};
|
||||
}
|
||||
|
||||
// Check for quotes or ellipses in "paths" - almost always a misinterpretation if not a very short name
|
||||
if (
|
||||
pathStr.includes('"') ||
|
||||
pathStr.includes("'") ||
|
||||
pathStr.includes('...')
|
||||
) {
|
||||
if (pathStr.length > 20) {
|
||||
return {
|
||||
isValid: false,
|
||||
error:
|
||||
'Path contains suspicious characters (quotes or ellipses) and is too long to be a simple filename.',
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
// Check total length
|
||||
if (pathStr.length > MAX_PATH_LENGTH) {
|
||||
return {
|
||||
isValid: false,
|
||||
error: `Path is too long (maximum ${MAX_PATH_LENGTH} characters).`,
|
||||
};
|
||||
}
|
||||
|
||||
// Check individual component lengths
|
||||
const components = pathStr.split(/[/\\]/);
|
||||
for (const component of components) {
|
||||
if (component.length > MAX_COMPONENT_LENGTH) {
|
||||
return {
|
||||
isValid: false,
|
||||
error: `Path component "${component.substring(0, 20)}..." is too long (maximum ${MAX_COMPONENT_LENGTH} characters).`,
|
||||
};
|
||||
}
|
||||
}
|
||||
|
||||
return { isValid: true };
|
||||
}
|
||||
Reference in New Issue
Block a user