fix(core-tools): resolve defensive path resolution for at-reference files (#27943)

This commit is contained in:
luisfelipe-alt
2026-06-16 22:05:03 +00:00
committed by GitHub
parent 926f3d9b95
commit f741d03282
6 changed files with 882 additions and 29 deletions
@@ -0,0 +1,591 @@
/**
* @license
* Copyright 2026 Google LLC
* SPDX-License-Identifier: Apache-2.0
*/
import { describe, it, expect, beforeEach, afterEach, vi } from 'vitest';
import { ReadFileTool } from './read-file.js';
import { WriteFileTool, getCorrectedFileContent } from './write-file.js';
import { EditTool } from './edit.js';
import { correctPath } from '../utils/pathCorrector.js';
import path from 'node:path';
import os from 'node:os';
import fs from 'node:fs';
import fsp from 'node:fs/promises';
import type { Config } from '../config/config.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { StandardFileSystemService } from '../services/fileSystemService.js';
import { createMockWorkspaceContext } from '../test-utils/mockWorkspaceContext.js';
import { createMockMessageBus } from '../test-utils/mock-message-bus.js';
import { isSubpath } from '../utils/paths.js';
vi.mock('../telemetry/loggers.js', () => ({
logFileOperation: vi.fn(),
logEditStrategy: vi.fn(),
logEditCorrectionEvent: vi.fn(),
}));
vi.mock('./jit-context.js', () => ({
discoverJitContext: vi.fn().mockResolvedValue(''),
appendJitContext: vi.fn().mockImplementation((content) => content),
appendJitContextToParts: vi.fn().mockImplementation((content) => content),
}));
describe('Consolidated At-Reference Path Resolution Tests (b-495551283)', () => {
let tempRootDir: string;
let mockConfigInstance: Config;
const abortSignal = new AbortController().signal;
beforeEach(async () => {
// Create a unique temporary root directory for each test run
const realTmp = await fsp.realpath(os.tmpdir());
tempRootDir = await fsp.mkdtemp(
path.join(realTmp, 'at-ref-resolution-root-'),
);
mockConfigInstance = {
getFileService: () => new FileDiscoveryService(tempRootDir),
getFileSystemService: () => new StandardFileSystemService(),
getTargetDir: () => tempRootDir,
getWorkspaceContext: () => createMockWorkspaceContext(tempRootDir),
getFileFilteringOptions: () => ({
respectGitIgnore: true,
respectGeminiIgnore: true,
}),
storage: {
getProjectTempDir: () => path.join(tempRootDir, '.temp'),
},
isInteractive: () => false,
isPlanMode: () => false,
getActiveModel: () => undefined,
getBaseLlmClient: () => undefined,
getDisableLLMCorrection: () => true,
isPathAllowed(this: Config, absolutePath: string): boolean {
const workspaceContext = this.getWorkspaceContext();
if (workspaceContext.isPathWithinWorkspace(absolutePath)) {
return true;
}
const projectTempDir = this.storage.getProjectTempDir();
return isSubpath(path.resolve(projectTempDir), absolutePath);
},
validatePathAccess(this: Config, absolutePath: string): string | null {
if (this.isPathAllowed(absolutePath)) {
return null;
}
const workspaceDirs = this.getWorkspaceContext().getDirectories();
const projectTempDir = this.storage.getProjectTempDir();
return `Path not in workspace: Attempted path "${absolutePath}" resolves outside the allowed workspace directories: ${workspaceDirs.join(', ')} or the project temp directory: ${projectTempDir}`;
},
} as unknown as Config;
// Create the policies directory and new-policies.txt file
await fsp.mkdir(path.join(tempRootDir, 'policies'), { recursive: true });
await fsp.writeFile(
path.join(tempRootDir, 'policies', 'new-policies.txt'),
'[[rule]]\ntoolName = "run_shell_command"\ndecision = "allow"\n',
'utf8',
);
});
afterEach(async () => {
// Clean up the temporary root directory
if (fs.existsSync(tempRootDir)) {
await fsp.rm(tempRootDir, { recursive: true, force: true });
}
});
it('ReadFileTool successfully reads a file when the path is prefixed with @', async () => {
const readFileTool = new ReadFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = readFileTool.build({
file_path: '@policies/new-policies.txt',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed because it defensively strips the leading '@'
expect(result.error).toBeUndefined();
expect(result.llmContent).toContain('toolName = "run_shell_command"');
});
it('ReadFileTool successfully reads a file when the path is prefixed with @/', async () => {
const readFileTool = new ReadFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = readFileTool.build({
file_path: '@/policies/new-policies.txt',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed because it defensively strips the leading '@/'
expect(result.error).toBeUndefined();
expect(result.llmContent).toContain('toolName = "run_shell_command"');
});
it('WriteFileTool successfully writes to/updates a file when the path is prefixed with @', async () => {
const writeFileTool = new WriteFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = writeFileTool.build({
file_path: '@policies/new-policies.txt',
content: '[[rule]]\nupdated_content = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and update the correct file
expect(result.error).toBeUndefined();
const incorrectFilePath = path.join(
tempRootDir,
'@policies',
'new-policies.txt',
);
const correctFilePath = path.join(
tempRootDir,
'policies',
'new-policies.txt',
);
// It should NOT have created a literal "@policies" directory
expect(fs.existsSync(incorrectFilePath)).toBe(false);
// It should have updated the correct file under "policies"
const updatedContent = await fsp.readFile(correctFilePath, 'utf8');
expect(updatedContent).toContain('updated_content = true');
});
it('WriteFileTool successfully creates a new file when the path is prefixed with @ and the parent directory exists', async () => {
const writeFileTool = new WriteFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = writeFileTool.build({
file_path: '@policies/brand-new-file.txt',
content: '[[rule]]\nbrand_new_file = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and create the correct file
expect(result.error).toBeUndefined();
const incorrectFilePath = path.join(
tempRootDir,
'@policies',
'brand-new-file.txt',
);
const correctFilePath = path.join(
tempRootDir,
'policies',
'brand-new-file.txt',
);
// It should NOT have created a literal "@policies" directory
expect(fs.existsSync(incorrectFilePath)).toBe(false);
// It should have created the correct file under "policies"
const createdContent = await fsp.readFile(correctFilePath, 'utf8');
expect(createdContent).toContain('brand_new_file = true');
});
it('WriteFileTool successfully creates a new file in a nested subdirectory when the path is prefixed with @ and the first segment exists', async () => {
const writeFileTool = new WriteFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = writeFileTool.build({
file_path: '@policies/sub/brand-new-file.txt',
content: '[[rule]]\nnested_brand_new_file = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and create the correct file
expect(result.error).toBeUndefined();
const incorrectFilePath = path.join(
tempRootDir,
'@policies',
'sub',
'brand-new-file.txt',
);
const correctFilePath = path.join(
tempRootDir,
'policies',
'sub',
'brand-new-file.txt',
);
// It should NOT have created a literal "@policies" directory
expect(fs.existsSync(incorrectFilePath)).toBe(false);
// It should have created the correct file under "policies/sub"
const createdContent = await fsp.readFile(correctFilePath, 'utf8');
expect(createdContent).toContain('nested_brand_new_file = true');
});
it('WriteFileTool successfully creates a new file in a nested subdirectory when the path is prefixed with @ and the first segment does NOT exist', async () => {
const writeFileTool = new WriteFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = writeFileTool.build({
file_path: '@new-policies/sub/brand-new-file.txt',
content: '[[rule]]\nnested_brand_new_file = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and create the correct file
expect(result.error).toBeUndefined();
const incorrectFilePath = path.join(
tempRootDir,
'@new-policies',
'sub',
'brand-new-file.txt',
);
const correctFilePath = path.join(
tempRootDir,
'new-policies',
'sub',
'brand-new-file.txt',
);
// It SHOULD have created a literal "@new-policies" directory because neither exists
expect(fs.existsSync(incorrectFilePath)).toBe(true);
// It should NOT have created the file under "new-policies/sub"
expect(fs.existsSync(correctFilePath)).toBe(false);
// Verify the content of the created file
const createdContent = await fsp.readFile(incorrectFilePath, 'utf8');
expect(createdContent).toContain('nested_brand_new_file = true');
});
it('WriteFileTool successfully creates a new file in a nested subdirectory when the path is prefixed with @/ and the first segment does NOT exist', async () => {
const writeFileTool = new WriteFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = writeFileTool.build({
file_path: '@/new-policies-alias/sub/brand-new-file.txt',
content: '[[rule]]\nnested_brand_new_file_alias = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and create the correct file
expect(result.error).toBeUndefined();
const literalAtFilePath = path.join(
tempRootDir,
'@',
'new-policies-alias',
'sub',
'brand-new-file.txt',
);
const correctFilePath = path.join(
tempRootDir,
'new-policies-alias',
'sub',
'brand-new-file.txt',
);
// It should NOT have created a literal "@" directory
expect(fs.existsSync(literalAtFilePath)).toBe(false);
expect(fs.existsSync(path.join(tempRootDir, '@'))).toBe(false);
// It should have created the file under "new-policies-alias/sub"
expect(fs.existsSync(correctFilePath)).toBe(true);
// Verify the content of the created file
const createdContent = await fsp.readFile(correctFilePath, 'utf8');
expect(createdContent).toContain('nested_brand_new_file_alias = true');
});
it('WriteFileTool successfully creates a new file in a nested subdirectory when the path is prefixed with @\\ and the first segment does NOT exist', async () => {
const writeFileTool = new WriteFileTool(
mockConfigInstance,
createMockMessageBus(),
);
const invocation = writeFileTool.build({
file_path: '@\\new-policies-alias-win\\sub\\brand-new-file.txt',
content: '[[rule]]\nnested_brand_new_file_alias_win = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and create the correct file
expect(result.error).toBeUndefined();
const isWindows = process.platform === 'win32';
const literalAtFilePath = isWindows
? path.join(
tempRootDir,
'@',
'new-policies-alias-win',
'sub',
'brand-new-file.txt',
)
: path.join(
tempRootDir,
'@\\new-policies-alias-win\\sub\\brand-new-file.txt',
);
const correctFilePath = isWindows
? path.join(
tempRootDir,
'new-policies-alias-win',
'sub',
'brand-new-file.txt',
)
: path.join(
tempRootDir,
'new-policies-alias-win\\sub\\brand-new-file.txt',
);
// It should NOT have created a literal "@" directory
expect(fs.existsSync(literalAtFilePath)).toBe(false);
expect(fs.existsSync(path.join(tempRootDir, '@'))).toBe(false);
// It should have created the file under "new-policies-alias-win/sub"
expect(fs.existsSync(correctFilePath)).toBe(true);
// Verify the content of the created file
const createdContent = await fsp.readFile(correctFilePath, 'utf8');
expect(createdContent).toContain('nested_brand_new_file_alias_win = true');
});
it('getCorrectedFileContent blocks path traversal outside the workspace', async () => {
const result = await getCorrectedFileContent(
mockConfigInstance,
'../../etc/passwd',
'malicious content',
abortSignal,
);
// The utility should fail with a path validation error
expect(result.error).toBeDefined();
expect(result.error?.message).toContain('Path not in workspace');
});
it('EditTool.getModifyContext blocks path traversal outside the workspace', async () => {
const editTool = new EditTool(mockConfigInstance, createMockMessageBus());
const modifyContext = editTool.getModifyContext(abortSignal);
// The getCurrentContent method should throw a path validation error
await expect(
modifyContext.getCurrentContent({
file_path: '../../etc/passwd',
instruction: 'read file',
old_string: '',
new_string: '',
}),
).rejects.toThrow('Path not in workspace');
// The getProposedContent method should throw a path validation error
await expect(
modifyContext.getProposedContent({
file_path: '../../etc/passwd',
instruction: 'read file',
old_string: '',
new_string: '',
}),
).rejects.toThrow('Path not in workspace');
});
it('getCorrectedFileContent handles symlink loops gracefully', async () => {
const symlinkPath1 = path.join(tempRootDir, 'symlink1');
const symlinkPath2 = path.join(tempRootDir, 'symlink2');
await fsp.symlink(symlinkPath2, symlinkPath1);
await fsp.symlink(symlinkPath1, symlinkPath2);
const result = await getCorrectedFileContent(
mockConfigInstance,
'symlink1',
'content',
abortSignal,
);
// The utility should fail gracefully with a resolution error
expect(result.error).toBeDefined();
expect(result.error?.message).toContain('Failed to resolve path');
});
it('EditTool.getModifyContext handles symlink loops gracefully by throwing a descriptive error', async () => {
const symlinkPath1 = path.join(tempRootDir, 'symlink1');
const symlinkPath2 = path.join(tempRootDir, 'symlink2');
await fsp.symlink(symlinkPath2, symlinkPath1);
await fsp.symlink(symlinkPath1, symlinkPath2);
const editTool = new EditTool(mockConfigInstance, createMockMessageBus());
const modifyContext = editTool.getModifyContext(abortSignal);
// The getCurrentContent method should throw a path resolution error
await expect(
modifyContext.getCurrentContent({
file_path: 'symlink1',
instruction: 'read file',
old_string: '',
new_string: '',
}),
).rejects.toThrow('Failed to resolve path');
// The getProposedContent method should throw a path resolution error
await expect(
modifyContext.getProposedContent({
file_path: 'symlink1',
instruction: 'read file',
old_string: '',
new_string: '',
}),
).rejects.toThrow('Failed to resolve path');
});
it('getCorrectedFileContent successfully resolves paths in Plan Mode', async () => {
const plansDir = path.join(tempRootDir, '.plans');
await fsp.mkdir(plansDir, { recursive: true });
await fsp.writeFile(
path.join(plansDir, 'plan-file.txt'),
'plan content',
'utf8',
);
const planConfigInstance = Object.assign({}, mockConfigInstance, {
isPlanMode: () => true,
getProjectRoot: () => tempRootDir,
storage: {
getProjectTempDir: () => path.join(tempRootDir, '.temp'),
getPlansDir: () => plansDir,
},
}) as unknown as Config;
const result = await getCorrectedFileContent(
planConfigInstance,
'plan-file.txt',
'new plan content',
abortSignal,
);
expect(result.error).toBeUndefined();
expect(result.originalContent).toBe('plan content');
});
it('EditTool successfully edits an existing file when the path is prefixed with @', async () => {
const editTool = new EditTool(mockConfigInstance, createMockMessageBus());
const invocation = editTool.build({
file_path: '@policies/new-policies.txt',
instruction: 'update decision rule',
old_string: 'decision = "allow"',
new_string: 'decision = "deny"',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and update the correct file
expect(result.error).toBeUndefined();
const correctFilePath = path.join(
tempRootDir,
'policies',
'new-policies.txt',
);
const updatedContent = await fsp.readFile(correctFilePath, 'utf8');
expect(updatedContent).toContain('decision = "deny"');
});
it('EditTool successfully creates a new file when the path is prefixed with @ and the parent directory exists', async () => {
const editTool = new EditTool(mockConfigInstance, createMockMessageBus());
const invocation = editTool.build({
file_path: '@policies/brand-new-edit-file.txt',
instruction: 'create new file',
old_string: '',
new_string: '[[rule]]\nbrand_new_edit_file = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and create the correct file
expect(result.error).toBeUndefined();
const incorrectFilePath = path.join(
tempRootDir,
'@policies',
'brand-new-edit-file.txt',
);
const correctFilePath = path.join(
tempRootDir,
'policies',
'brand-new-edit-file.txt',
);
// It should NOT have created a literal "@policies" directory
expect(fs.existsSync(incorrectFilePath)).toBe(false);
// It should have created the correct file under "policies"
const createdContent = await fsp.readFile(correctFilePath, 'utf8');
expect(createdContent).toContain('brand_new_edit_file = true');
});
it('EditTool successfully creates a new file in a nested subdirectory when the path is prefixed with @ and the first segment does NOT exist', async () => {
const editTool = new EditTool(mockConfigInstance, createMockMessageBus());
const invocation = editTool.build({
file_path: '@new-policies-edit/sub/brand-new-file.txt',
instruction: 'create new file in nested subdirectory',
old_string: '',
new_string: '[[rule]]\nnested_brand_new_edit_file = true\n',
});
const result = await invocation.execute({ abortSignal });
// The tool should succeed and create the correct file
expect(result.error).toBeUndefined();
const incorrectFilePath = path.join(
tempRootDir,
'@new-policies-edit',
'sub',
'brand-new-file.txt',
);
const correctFilePath = path.join(
tempRootDir,
'new-policies-edit',
'sub',
'brand-new-file.txt',
);
// It SHOULD have created a literal "@new-policies-edit" directory because neither exists
expect(fs.existsSync(incorrectFilePath)).toBe(true);
// It should NOT have created the file under "new-policies-edit/sub"
expect(fs.existsSync(correctFilePath)).toBe(false);
// Verify the content of the created file
const createdContent = await fsp.readFile(incorrectFilePath, 'utf8');
expect(createdContent).toContain('nested_brand_new_edit_file = true');
});
it('correctPath successfully resolves a path prefixed with @ to its clean counterpart', () => {
const result = correctPath(
'@policies/new-policies.txt',
mockConfigInstance,
);
expect(result.success).toBe(true);
if (result.success) {
const expectedPath = path.join(
tempRootDir,
'policies',
'new-policies.txt',
);
expect(result.correctedPath).toBe(expectedPath);
}
});
});
+106 -14
View File
@@ -27,7 +27,12 @@ import {
import { buildFilePathArgsPattern } from '../policy/utils.js';
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import { ToolErrorType } from './tool-error.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import {
makeRelative,
shortenPath,
resolveDefensiveToolPath,
resolveToRealPath,
} from '../utils/paths.js';
import { isNodeError } from '../utils/errors.js';
import { correctPath } from '../utils/pathCorrector.js';
import type { Config } from '../config/config.js';
@@ -478,11 +483,12 @@ class EditToolInvocation
);
if (this.config.isPlanMode()) {
try {
this.resolvedPath = resolveAndValidatePlanPath(
const planPath = resolveAndValidatePlanPath(
this.params.file_path,
this.config.storage.getPlansDir(),
this.config.getProjectRoot(),
);
this.resolvedPath = resolveToRealPath(planPath);
} catch (e) {
debugLogger.error(
'Failed to resolve plan path during EditTool invocation setup',
@@ -495,15 +501,33 @@ class EditToolInvocation
} else if (!path.isAbsolute(this.params.file_path)) {
const result = correctPath(this.params.file_path, this.config);
if (result.success) {
this.resolvedPath = result.correctedPath;
try {
this.resolvedPath = resolveToRealPath(result.correctedPath);
} catch {
this.resolvedPath = result.correctedPath;
}
} else {
this.resolvedPath = path.resolve(
this.config.getTargetDir(),
const sanitizedPath = resolveDefensiveToolPath(
this.params.file_path,
this.config.getTargetDir(),
);
try {
this.resolvedPath = resolveToRealPath(
path.resolve(this.config.getTargetDir(), sanitizedPath),
);
} catch {
this.resolvedPath = path.resolve(
this.config.getTargetDir(),
sanitizedPath,
);
}
}
} else {
this.resolvedPath = this.params.file_path;
try {
this.resolvedPath = resolveToRealPath(this.params.file_path);
} catch {
this.resolvedPath = this.params.file_path;
}
}
}
@@ -1094,28 +1118,43 @@ export class EditTool
let resolvedPath: string;
if (this.config.isPlanMode()) {
try {
resolvedPath = resolveAndValidatePlanPath(
const planPath = resolveAndValidatePlanPath(
params.file_path,
this.config.storage.getPlansDir(),
this.config.getProjectRoot(),
);
resolvedPath = resolveToRealPath(planPath);
} catch (err) {
return err instanceof Error ? err.message : String(err);
}
} else if (!path.isAbsolute(params.file_path)) {
const result = correctPath(params.file_path, this.config);
if (result.success) {
resolvedPath = result.correctedPath;
try {
resolvedPath = resolveToRealPath(result.correctedPath);
} catch (err) {
return err instanceof Error ? err.message : String(err);
}
} else {
resolvedPath = path.resolve(
this.config.getTargetDir(),
const sanitizedPath = resolveDefensiveToolPath(
params.file_path,
this.config.getTargetDir(),
);
try {
resolvedPath = resolveToRealPath(
path.resolve(this.config.getTargetDir(), sanitizedPath),
);
} catch (err) {
return err instanceof Error ? err.message : String(err);
}
}
} else {
resolvedPath = params.file_path;
try {
resolvedPath = resolveToRealPath(params.file_path);
} catch (err) {
return err instanceof Error ? err.message : String(err);
}
}
const newPlaceholders = detectOmissionPlaceholders(params.new_string);
if (newPlaceholders.length > 0) {
const oldPlaceholders = new Set(
@@ -1150,13 +1189,65 @@ export class EditTool
}
getModifyContext(_: AbortSignal): ModifyContext<EditToolParams> {
const resolvePath = (params: EditToolParams): string => {
let pathBeforeRealResolve: string;
try {
if (this.config.isPlanMode()) {
pathBeforeRealResolve = resolveAndValidatePlanPath(
params.file_path,
this.config.storage.getPlansDir(),
this.config.getProjectRoot(),
);
} else if (!path.isAbsolute(params.file_path)) {
const result = correctPath(params.file_path, this.config);
if (result.success) {
pathBeforeRealResolve = result.correctedPath;
} else {
const sanitizedPath = resolveDefensiveToolPath(
params.file_path,
this.config.getTargetDir(),
);
pathBeforeRealResolve = path.resolve(
this.config.getTargetDir(),
sanitizedPath,
);
}
} else {
pathBeforeRealResolve = params.file_path;
}
} catch (err) {
throw new Error(
'Failed to resolve path: ' +
(err instanceof Error ? err.message : String(err)),
);
}
let resolved: string;
try {
resolved = resolveToRealPath(pathBeforeRealResolve);
} catch (err) {
throw new Error(
'Failed to resolve path: ' +
(err instanceof Error ? err.message : String(err)),
);
}
const validationError = this.config.validatePathAccess(resolved);
if (validationError) {
throw new Error(validationError);
}
return resolved;
};
return {
getFilePath: (params: EditToolParams) => params.file_path,
getCurrentContent: async (params: EditToolParams): Promise<string> => {
try {
const resolvedPath = resolvePath(params);
return await this.config
.getFileSystemService()
.readTextFile(params.file_path);
.readTextFile(resolvedPath);
} catch (err) {
if (!isNodeError(err) || err.code !== 'ENOENT') throw err;
return '';
@@ -1164,9 +1255,10 @@ export class EditTool
},
getProposedContent: async (params: EditToolParams): Promise<string> => {
try {
const resolvedPath = resolvePath(params);
const currentContent = await this.config
.getFileSystemService()
.readTextFile(params.file_path);
.readTextFile(resolvedPath);
return applyReplacement(
currentContent,
params.old_string,
+29 -5
View File
@@ -6,7 +6,12 @@
import type { MessageBus } from '../confirmation-bus/message-bus.js';
import path from 'node:path';
import { makeRelative, shortenPath } from '../utils/paths.js';
import {
makeRelative,
shortenPath,
resolveDefensiveToolPath,
resolveToRealPath,
} from '../utils/paths.js';
import {
BaseDeclarativeTool,
BaseToolInvocation,
@@ -74,10 +79,20 @@ class ReadFileToolInvocation extends BaseToolInvocation<
_toolDisplayName?: string,
) {
super(params, messageBus, _toolName, _toolDisplayName);
this.resolvedPath = path.resolve(
this.config.getTargetDir(),
const sanitizedPath = resolveDefensiveToolPath(
this.params.file_path,
this.config.getTargetDir(),
);
try {
this.resolvedPath = resolveToRealPath(
path.resolve(this.config.getTargetDir(), sanitizedPath),
);
} catch {
this.resolvedPath = path.resolve(
this.config.getTargetDir(),
sanitizedPath,
);
}
}
getDescription(): string {
@@ -242,11 +257,20 @@ export class ReadFileTool extends BaseDeclarativeTool<
return "The 'file_path' parameter must be non-empty.";
}
const resolvedPath = path.resolve(
this.config.getTargetDir(),
const sanitizedPath = resolveDefensiveToolPath(
params.file_path,
this.config.getTargetDir(),
);
let resolvedPath: string;
try {
resolvedPath = resolveToRealPath(
path.resolve(this.config.getTargetDir(), sanitizedPath),
);
} catch (err) {
return `Failed to resolve path: ${err instanceof Error ? err.message : String(err)}`;
}
const validationError = this.config.validatePathAccess(
resolvedPath,
'read',
+91 -7
View File
@@ -28,7 +28,12 @@ import {
} from './tools.js';
import { buildFilePathArgsPattern } from '../policy/utils.js';
import { ToolErrorType } from './tool-error.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import {
makeRelative,
shortenPath,
resolveDefensiveToolPath,
resolveToRealPath,
} from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
import { ensureCorrectFileContent } from '../utils/editCorrector.js';
import { detectLineEnding } from '../utils/textUtils.js';
@@ -109,10 +114,67 @@ export async function getCorrectedFileContent(
let fileExists = false;
let correctedContent = proposedContent;
let resolvedPath: string;
if (config.isPlanMode()) {
try {
const planPath = path.isAbsolute(filePath)
? filePath
: resolveAndValidatePlanPath(
filePath,
config.storage.getPlansDir(),
config.getProjectRoot(),
);
resolvedPath = resolveToRealPath(planPath);
} catch (err) {
return {
originalContent: '',
correctedContent: proposedContent,
fileExists: false,
error: {
message:
'Failed to resolve plan path: ' +
(err instanceof Error ? err.message : String(err)),
code: 'EINVAL',
},
};
}
} else {
const sanitizedPath = path.isAbsolute(filePath)
? filePath
: resolveDefensiveToolPath(filePath, config.getTargetDir());
try {
resolvedPath = resolveToRealPath(
path.resolve(config.getTargetDir(), sanitizedPath),
);
} catch (err) {
return {
originalContent: '',
correctedContent: proposedContent,
fileExists: false,
error: {
message:
'Failed to resolve path: ' +
(err instanceof Error ? err.message : String(err)),
code: 'EINVAL',
},
};
}
}
const validationError = config.validatePathAccess(resolvedPath);
if (validationError) {
return {
originalContent: '',
correctedContent: proposedContent,
fileExists: false,
error: { message: validationError, code: 'EACCES' },
};
}
try {
originalContent = await config
.getFileSystemService()
.readTextFile(filePath);
.readTextFile(resolvedPath);
fileExists = true; // File exists and was read
} catch (err) {
if (isNodeError(err) && err.code === 'ENOENT') {
@@ -170,11 +232,12 @@ class WriteFileToolInvocation extends BaseToolInvocation<
if (this.config.isPlanMode()) {
try {
this.resolvedPath = resolveAndValidatePlanPath(
const planPath = resolveAndValidatePlanPath(
this.params.file_path,
this.config.storage.getPlansDir(),
this.config.getProjectRoot(),
);
this.resolvedPath = resolveToRealPath(planPath);
} catch (e) {
debugLogger.error(
'Failed to resolve plan path during WriteFileTool invocation setup',
@@ -184,10 +247,20 @@ class WriteFileToolInvocation extends BaseToolInvocation<
this.resolvedPath = this.params.file_path;
}
} else {
this.resolvedPath = path.resolve(
this.config.getTargetDir(),
const sanitizedPath = resolveDefensiveToolPath(
this.params.file_path,
this.config.getTargetDir(),
);
try {
this.resolvedPath = resolveToRealPath(
path.resolve(this.config.getTargetDir(), sanitizedPath),
);
} catch {
this.resolvedPath = path.resolve(
this.config.getTargetDir(),
sanitizedPath,
);
}
}
}
@@ -525,16 +598,27 @@ export class WriteFileTool
let resolvedPath: string;
if (this.config.isPlanMode()) {
try {
resolvedPath = resolveAndValidatePlanPath(
const planPath = resolveAndValidatePlanPath(
filePath,
this.config.storage.getPlansDir(),
this.config.getProjectRoot(),
);
resolvedPath = resolveToRealPath(planPath);
} catch (err) {
return err instanceof Error ? err.message : String(err);
}
} else {
resolvedPath = path.resolve(this.config.getTargetDir(), filePath);
const sanitizedPath = resolveDefensiveToolPath(
filePath,
this.config.getTargetDir(),
);
try {
resolvedPath = resolveToRealPath(
path.resolve(this.config.getTargetDir(), sanitizedPath),
);
} catch (err) {
return err instanceof Error ? err.message : String(err);
}
}
const validationError = this.config.validatePathAccess(resolvedPath);
+9 -3
View File
@@ -8,6 +8,7 @@ import * as fs from 'node:fs';
import * as path from 'node:path';
import type { Config } from '../config/config.js';
import { bfsFileSearchSync } from './bfsFileSearch.js';
import { resolveDefensiveToolPath } from './paths.js';
type SuccessfulPathCorrection = {
success: true;
@@ -34,8 +35,13 @@ export function correctPath(
filePath: string,
config: Config,
): PathCorrectionResult {
const sanitizedPath = resolveDefensiveToolPath(
filePath,
config.getTargetDir(),
);
// Check for direct path relative to the primary target directory.
const directPath = path.join(config.getTargetDir(), filePath);
const directPath = path.join(config.getTargetDir(), sanitizedPath);
if (fs.existsSync(directPath)) {
return { success: true, correctedPath: directPath };
}
@@ -43,8 +49,8 @@ export function correctPath(
// If not found directly, search across all workspace directories for ambiguous matches.
const workspaceContext = config.getWorkspaceContext();
const searchPaths = workspaceContext.getDirectories();
const basename = path.basename(filePath);
const normalizedTarget = filePath.replace(/\\/g, '/');
const basename = path.basename(sanitizedPath);
const normalizedTarget = sanitizedPath.replace(/\\/g, '/');
// Normalize path for matching and check if it ends with the provided relative path
const foundFiles = searchPaths
+56
View File
@@ -572,3 +572,59 @@ export function isTrustedSystemPath(filePath: string): boolean {
);
}
}
/**
* Defensively resolves and sanitizes a file path generated by the LLM,
* stripping user-facing reference prefixes if necessary.
*/
export function resolveDefensiveToolPath(
filePath: string,
targetDir: string,
): string {
if (filePath.includes('\0')) {
return filePath;
}
try {
const literalPath = path.resolve(targetDir, filePath);
// If the file literally exists on disk as-is, return the resolved literal path immediately
if (fs.existsSync(literalPath)) {
return filePath;
}
// If the model supplied a leading @ prefix and the literal path doesn't exist:
if (filePath.startsWith('@') && filePath.length > 1) {
if (filePath.startsWith('@/') || filePath.startsWith('@\\')) {
return filePath.substring(1).replace(/^[\\/]+/, '');
}
const strippedPath = filePath.substring(1).replace(/^[\\/]+/, '');
// Check if a literal directory/file starting with '@' exists for the first segment.
// If it does, we should preserve the '@' prefix.
const parts = strippedPath.split(/[\\/]/);
const firstSegment = parts[0];
if (firstSegment) {
const literalFirstSegment = path.resolve(targetDir, '@' + firstSegment);
if (fs.existsSync(literalFirstSegment)) {
return filePath;
}
// Only strip the '@' prefix if the stripped path's first segment actually exists
const strippedFirstSegment = path.resolve(targetDir, firstSegment);
if (fs.existsSync(strippedFirstSegment)) {
return strippedPath;
}
}
// Otherwise, preserve the '@' prefix to allow creating new files/directories starting with '@'
return filePath;
}
} catch {
// Fallback to original path if any filesystem or resolution error occurs
}
// Fallback: return the original path
return filePath;
}