fix(core): unescape shell-escaped file paths in Edit, WriteFile, and ReadFile tools (#3820)
Some checks are pending
Qwen Code CI / Post Coverage Comment (push) Blocked by required conditions
Qwen Code CI / Lint (push) Waiting to run
Qwen Code CI / Test (push) Blocked by required conditions
Qwen Code CI / Test-1 (push) Blocked by required conditions
Qwen Code CI / Test-2 (push) Blocked by required conditions
Qwen Code CI / Test-3 (push) Blocked by required conditions
Qwen Code CI / Test-4 (push) Blocked by required conditions
Qwen Code CI / Test-5 (push) Blocked by required conditions
Qwen Code CI / Test-8 (push) Blocked by required conditions
Qwen Code CI / Test-6 (push) Blocked by required conditions
Qwen Code CI / Test-7 (push) Blocked by required conditions
Qwen Code CI / CodeQL (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:docker (push) Waiting to run
E2E Tests / E2E Test (Linux) - sandbox:none (push) Waiting to run
E2E Tests / E2E Test - macOS (push) Waiting to run

* fix(core): unescape shell-escaped file_path in Edit, WriteFile, and ReadFile tools

Shell-escaped paths (e.g. my\ file.txt) from at-completion can reach
the LLM without going through atCommandProcessor unescaping.  When
the LLM passes such an escaped path to a file tool, the backslash is
treated as a literal character in the filename, causing the tool to
fail with file-not-found.

Add defensive unescapePath() normalization at the start of each
tool validateToolParamValues(), so that escaped paths are
converted back to real filesystem paths before any I/O.

Also normalize the path in coreToolScheduler conditional-rules
injection path for consistency.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): harden unescapePath — Windows safety, speculation paths, hooks, and consistency

- Make unescapePath a no-op on win32 to avoid corrupting paths like
  C:\(v2)\file.txt where backslashes are path separators.
- Apply unescapePath in speculationToolGate before overlay FS lookups
  so speculation mode finds files given escaped paths.
- Normalize file_path/path in hook toolInput so custom hooks receive
  actual filesystem paths.
- Add unescapePath to Grep, Glob, Ls, and RipGrep validateToolParamValues
  for parity with the file tools.
- Apply unescapePath in getModifyContext callbacks (Edit, WriteFile) so
  the modify-with-editor flow works when request.args still holds escaped
  paths.
- Add .trim() to Edit and WriteFile path normalization for consistency
  with ReadFile.
- Use .trim() in conditional-rules matchAndConsume for parity.
- Gate literal-backslash test to non-win32; add Windows no-op test for
  unescapePath.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): persist unescaped paths in validators, cover LSP filePath, centralize modifyWithEditor normalization

- Mutate params.path in Glob/Grep/Ls/RipGrep validators so invocations
  receive the normalized path, not the original escaped form.
- Add unescapePath to LspTool.validateToolParamValues for filePath.
- Extend scheduler path normalization to filePath + notebook_path
  keys, mirroring speculationToolGate's key set.
- Move unescapePath from getModifyContext callbacks into
  coreToolScheduler's modifyWithEditor path to avoid double-unescape
  and keep a single normalization site.
- Add .trim() to speculationToolGate paths for consistency.
- Show normalized path in ls.ts validation error message.

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): address review feedback for unescapePath PR

- Extract shared PATH_ARG_KEYS constant to eliminate duplicate key lists
  across coreToolScheduler and speculationToolGate
- Add notebook_path to modifyWithEditor unescape block (was missing)
- Hoist unescapePath regex as module-level constant (UNESCAPE_REGEX)
- Add debug log when unescapePath actually modifies a path
- Replace early-return Windows guards with vitest skipIf in tests
- Add tests for escaped paths in Glob, Grep, Ls, RipGrep, LSP validators
- Add test for conditional rules matchAndConsume with unescaped paths

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): fix import order in grep.ts, consolidate ls.ts imports, win32-guard write-file tests

- grep.ts: move debugLogger declaration after all import statements
- ls.ts: consolidate two imports from ../utils/paths.js into one
- write-file.test.ts: add skipIf(process.platform === 'win32') to
  shell-escape tests (unescapePath is a no-op on win32)

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(core): address review — double-unescape, circular import, lsp asymmetry, win32 test guards

- coreToolScheduler.ts: remove redundant unescapePath in conditional-rules
  injection (toolInput was already normalized; unescapePath is not
  idempotent for \\X sequences)
- paths.ts: remove debugLogger to break circular import chain
  (paths → debugLogger → storage → paths). The single debug log line is
  low signal — dropped entirely.
- lsp.ts: remove dead .trim() checks in FILE_REQUIRED_OPERATIONS and
  RANGE_REQUIRED_OPERATIONS (filePath is already trimmed by validator)
- Add it.skipIf(process.platform === 'win32') to 11 escaped-path tests
  across edit.test.ts, read-file.test.ts, glob.test.ts, grep.test.ts,
  ripGrep.test.ts, ls.test.ts, lsp.test.ts

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(test): add win32 skipIf guards to atCommandProcessor, rulesDiscovery, fileSearch tests

Four more escaped-path tests that fail on Windows because
unescapePath is a no-op on win32:
- atCommandProcessor.test.ts: two @-command unescape tests
- rulesDiscovery.test.ts: shell-escaped match test
- fileSearch.test.ts: special-char escaping test

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

* fix(test): consolidate duplicate imports in rulesDiscovery.test.ts

Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>

---------

Co-authored-by: cici <cici@cicideMacBook-Pro.local>
Co-authored-by: Qwen-Coder <qwen-coder@alibabacloud.com>
This commit is contained in:
2026-05-05 01:33:36 +08:00 committed by GitHub
parent 3631c01e17
commit 2e69d641d5
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
23 changed files with 568 additions and 175 deletions

View file

@ -219,36 +219,39 @@ describe('handleAtCommand', () => {
});
});
it('should correctly unescape paths with escaped spaces', async () => {
const fileContent = 'This is the file content.';
const filePath = await createTestFile(
path.join(testRootDir, 'path', 'to', 'my file.txt'),
fileContent,
);
const escapedpath = path.join(testRootDir, 'path', 'to', 'my\\ file.txt');
const query = `@${escapedpath}`;
it.skipIf(process.platform === 'win32')(
'should correctly unescape paths with escaped spaces',
async () => {
const fileContent = 'This is the file content.';
const filePath = await createTestFile(
path.join(testRootDir, 'path', 'to', 'my file.txt'),
fileContent,
);
const escapedpath = path.join(testRootDir, 'path', 'to', 'my\\ file.txt');
const query = `@${escapedpath}`;
const result = await handleAtCommand({
query,
config: mockConfig,
onDebugMessage: mockOnDebugMessage,
messageId: 125,
signal: abortController.signal,
});
const result = await handleAtCommand({
query,
config: mockConfig,
onDebugMessage: mockOnDebugMessage,
messageId: 125,
signal: abortController.signal,
});
expect(result.processedQuery).toEqual([
{ text: `@${filePath}` },
{ text: '\n--- Content from referenced files ---' },
{ text: `\nContent from ${filePath}:\n` },
{ text: fileContent },
{ text: '\n--- End of content ---' },
]);
expect(result.shouldProceed).toBe(true);
// toolDisplays should be returned for caller to add to UI history
expect(result.toolDisplays).toBeDefined();
expect(result.toolDisplays).toHaveLength(1);
expect(result.toolDisplays![0].status).toBe(ToolCallStatus.Success);
});
expect(result.processedQuery).toEqual([
{ text: `@${filePath}` },
{ text: '\n--- Content from referenced files ---' },
{ text: `\nContent from ${filePath}:\n` },
{ text: fileContent },
{ text: '\n--- End of content ---' },
]);
expect(result.shouldProceed).toBe(true);
// toolDisplays should be returned for caller to add to UI history
expect(result.toolDisplays).toBeDefined();
expect(result.toolDisplays).toHaveLength(1);
expect(result.toolDisplays![0].status).toBe(ToolCallStatus.Success);
},
);
it('should handle multiple @file references', async () => {
const content1 = 'Content file1';
@ -782,34 +785,37 @@ describe('handleAtCommand', () => {
});
});
it('should still handle escaped spaces in paths before punctuation', async () => {
const fileContent = 'Spaced file content';
const filePath = await createTestFile(
path.join(testRootDir, 'spaced file.txt'),
fileContent,
);
const escapedPath = path.join(testRootDir, 'spaced\\ file.txt');
const query = `Check @${escapedPath}, it has spaces.`;
it.skipIf(process.platform === 'win32')(
'should still handle escaped spaces in paths before punctuation',
async () => {
const fileContent = 'Spaced file content';
const filePath = await createTestFile(
path.join(testRootDir, 'spaced file.txt'),
fileContent,
);
const escapedPath = path.join(testRootDir, 'spaced\\ file.txt');
const query = `Check @${escapedPath}, it has spaces.`;
const result = await handleAtCommand({
query,
config: mockConfig,
onDebugMessage: mockOnDebugMessage,
messageId: 412,
signal: abortController.signal,
});
const result = await handleAtCommand({
query,
config: mockConfig,
onDebugMessage: mockOnDebugMessage,
messageId: 412,
signal: abortController.signal,
});
expect(result).toMatchObject({
processedQuery: [
{ text: `Check @${filePath}, it has spaces.` },
{ text: '\n--- Content from referenced files ---' },
{ text: `\nContent from ${filePath}:\n` },
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
});
expect(result).toMatchObject({
processedQuery: [
{ text: `Check @${filePath}, it has spaces.` },
{ text: '\n--- Content from referenced files ---' },
{ text: `\nContent from ${filePath}:\n` },
{ text: fileContent },
{ text: '\n--- End of content ---' },
],
shouldProceed: true,
});
},
);
it('should not break file paths with periods in extensions', async () => {
const fileContent = 'TypeScript content';

View file

@ -50,6 +50,7 @@ import type {
import { fileURLToPath } from 'node:url';
import { ToolNames, ToolNamesMigration } from '../tools/tool-names.js';
import { escapeXml } from '../utils/xml.js';
import { unescapePath, PATH_ARG_KEYS } from '../utils/paths.js';
import { CONCURRENCY_SAFE_KINDS } from '../tools/tools.js';
import { isShellCommandReadOnly } from '../utils/shellReadOnlyChecker.js';
import { stripShellWrapper } from '../utils/shell-utils.js';
@ -1532,10 +1533,23 @@ export class CoreToolScheduler {
isModifying: true,
} as ToolCallConfirmationDetails);
// Normalize shell-escaped paths so the editor receives actual
// filesystem paths (request.args may still hold escaped values
// since buildInvocation normalizes a structuredClone).
const normalizedArgs = {
...waitingToolCall.request.args,
} as typeof waitingToolCall.request.args;
for (const key of PATH_ARG_KEYS) {
if (typeof normalizedArgs[key] === 'string') {
(normalizedArgs as Record<string, unknown>)[key] = unescapePath(
String(normalizedArgs[key]).trim(),
);
}
}
const { updatedParams, updatedDiff } = await modifyWithEditor<
typeof waitingToolCall.request.args
>(
waitingToolCall.request.args,
normalizedArgs,
modifyContext as ModifyContext<typeof waitingToolCall.request.args>,
editorType,
signal,
@ -1746,6 +1760,14 @@ export class CoreToolScheduler {
const invocation = scheduledCall.invocation;
const toolInput = scheduledCall.request.args as Record<string, unknown>;
// Normalize shell-escaped path params so hooks operate on actual filesystem
// paths, matching the normalization done in tool validation.
for (const key of PATH_ARG_KEYS) {
if (typeof toolInput[key] === 'string') {
toolInput[key] = unescapePath(String(toolInput[key]).trim());
}
}
// Generate unique tool_use_id for hook tracking
const toolUseId = generateToolUseId();
@ -1908,7 +1930,9 @@ export class CoreToolScheduler {
// FS_PATH_TOOL_NAMES) so MCP / non-FS tools that reuse those
// parameter names with different semantics never enter the
// activation pipeline.
const candidatePaths = extractToolFilePaths(toolName, toolInput);
const candidatePaths = extractToolFilePaths(toolName, toolInput).map(
(p) => unescapePath(p),
);
if (candidatePaths.length > 0) {
const rulesRegistry = this.config.getConditionalRulesRegistry();

View file

@ -18,6 +18,7 @@
import { ToolNames } from '../tools/tool-names.js';
import { isShellCommandReadOnlyAST } from '../utils/shellAstParser.js';
import { ApprovalMode } from '../config/config.js';
import { unescapePath, PATH_ARG_KEYS } from '../utils/paths.js';
import type { OverlayFs } from './overlayFs.js';
export interface ToolGateResult {
@ -117,10 +118,11 @@ async function resolveReadPaths(
args: Record<string, unknown>,
overlayFs: OverlayFs,
): Promise<void> {
const pathKeys = ['file_path', 'filePath', 'path', 'notebook_path'];
for (const key of pathKeys) {
for (const key of PATH_ARG_KEYS) {
if (typeof args[key] === 'string') {
args[key] = overlayFs.resolveReadPath(args[key] as string);
args[key] = overlayFs.resolveReadPath(
unescapePath(String(args[key]).trim()),
);
return;
}
}
@ -134,11 +136,11 @@ export async function rewritePathArgs(
args: Record<string, unknown>,
overlayFs: OverlayFs,
): Promise<void> {
// Common path argument names used by Edit and WriteFile tools
const pathKeys = ['file_path', 'filePath', 'path', 'notebook_path'];
for (const key of pathKeys) {
for (const key of PATH_ARG_KEYS) {
if (typeof args[key] === 'string') {
args[key] = await overlayFs.redirectWrite(args[key] as string);
args[key] = await overlayFs.redirectWrite(
unescapePath(String(args[key]).trim()),
);
return;
}
}

View file

@ -233,6 +233,60 @@ describe('EditTool', () => {
const error = tool.validateToolParams(params);
expect(error).toBeNull();
});
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped spaces in file_path',
() => {
const escapedPath = path.join(rootDir, 'my\\ file.txt');
const params: EditToolParams = {
file_path: escapedPath,
old_string: 'old',
new_string: 'new',
};
expect(tool.validateToolParams(params)).toBeNull();
expect(params.file_path).toBe(path.join(rootDir, 'my file.txt'));
},
);
it.skipIf(process.platform === 'win32')(
'should unescape multiple shell-escaped characters in file_path',
() => {
const escapedPath = path.join(
rootDir,
'project\\ \\(v2\\)\\ \\&\\ more.txt',
);
const params: EditToolParams = {
file_path: escapedPath,
old_string: 'old',
new_string: 'new',
};
expect(tool.validateToolParams(params)).toBeNull();
expect(params.file_path).toBe(
path.join(rootDir, 'project (v2) & more.txt'),
);
},
);
it.skipIf(process.platform === 'win32')(
'should preserve literal backslashes in file_path',
() => {
// On Windows, backslashes are path separators, and unescapePath is a
// no-op. This test only validates literal-backslash preservation on
// platforms where backslashes are not path separators.
const pathWithBackslash = path.join(
rootDir,
'path\\\\with\\\\slashes.txt',
);
const params: EditToolParams = {
file_path: pathWithBackslash,
old_string: 'old',
new_string: 'new',
};
expect(tool.validateToolParams(params)).toBeNull();
// Double backslashes (literal) should be preserved
expect(params.file_path).toBe(pathWithBackslash);
},
);
});
describe('getConfirmationDetails', () => {
@ -905,6 +959,52 @@ describe('EditTool', () => {
});
});
describe.skipIf(process.platform === 'win32')(
'escaped paths with spaces (end-to-end)',
() => {
it('should read and edit a file whose name contains spaces when given an escaped path', async () => {
// Create a file with spaces in its name on disk
const realFileName = 'my spaced file.txt';
const realPath = path.join(rootDir, realFileName);
fs.writeFileSync(realPath, 'Hello old world!', 'utf8');
// Pass an ESCAPED path (as the LLM might from at-completion)
const escapedPath = path.join(rootDir, 'my\\ spaced\\ file.txt');
const params: EditToolParams = {
file_path: escapedPath,
old_string: 'old',
new_string: 'new',
};
(mockConfig.getApprovalMode as Mock).mockReturnValueOnce(
ApprovalMode.AUTO_EDIT,
);
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
// Should succeed — not fail with file-not-found
expect(result.llmContent).toMatch(/Showing lines \d+-\d+ of \d+/);
expect(fs.readFileSync(realPath, 'utf8')).toBe('Hello new world!');
});
it('should fail gracefully when escaped path points to nonexistent file', async () => {
const escapedPath = path.join(rootDir, 'nonexistent\\ file.txt');
const params: EditToolParams = {
file_path: escapedPath,
old_string: 'old',
new_string: 'new',
};
const invocation = tool.build(params);
const result = await invocation.execute(new AbortController().signal);
// Should report file-not-found (unescaped path used, file truly doesn't exist)
expect(result.error?.type).toBe(ToolErrorType.FILE_NOT_FOUND);
});
},
);
describe('workspace boundary validation', () => {
it('should validate paths are within workspace root', () => {
const validPath = {

View file

@ -17,7 +17,7 @@ import type {
import type { PermissionDecision } from '../permissions/types.js';
import { BaseDeclarativeTool, Kind, ToolConfirmationOutcome } from './tools.js';
import { ToolErrorType } from './tool-error.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { makeRelative, shortenPath, unescapePath } from '../utils/paths.js';
import { isNodeError } from '../utils/errors.js';
import type { Config } from '../config/config.js';
import { ApprovalMode } from '../config/config.js';
@ -591,6 +591,10 @@ Expectation for required parameters:
protected override validateToolParamValues(
params: EditToolParams,
): string | null {
// Normalize shell-escaped paths (e.g. "my\ file.txt" → "my file.txt")
// that may reach the LLM via at-completion or manual typing.
params.file_path = unescapePath(params.file_path.trim());
if (!params.file_path) {
return "The 'file_path' parameter must be non-empty.";
}

View file

@ -361,6 +361,22 @@ describe('GlobTool', () => {
'Path is not a directory',
);
});
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped path',
async () => {
// Create a directory with a space so the unescaped path exists
const dirWithSpace = path.join(tempRootDir, 'sub dir');
await fs.mkdir(dirWithSpace);
const params: GlobToolParams = {
pattern: '*.ts',
path: path.join(tempRootDir, 'sub\\ dir'),
};
expect(globTool.validateToolParams(params)).toBeNull();
// Path should be normalized in place
expect(params.path).toBe(dirWithSpace);
},
);
});
describe('workspace boundary validation', () => {

View file

@ -10,7 +10,11 @@ import { glob, escape } from 'glob';
import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { ToolNames, ToolDisplayNames } from './tool-names.js';
import { resolveAndValidatePath, isSubpath } from '../utils/paths.js';
import {
resolveAndValidatePath,
isSubpath,
unescapePath,
} from '../utils/paths.js';
import { getMemoryBaseDir } from '../memory/paths.js';
import { type Config } from '../config/config.js';
import type { PermissionDecision } from '../permissions/types.js';
@ -348,6 +352,7 @@ export class GlobTool extends BaseDeclarativeTool<GlobToolParams, ToolResult> {
// Only validate path if one is provided
if (params.path) {
params.path = unescapePath(params.path.trim());
try {
resolveAndValidatePath(this.config, params.path, {
allowExternalPaths: true,

View file

@ -174,6 +174,21 @@ describe('GrepTool', () => {
`Path is not a directory: ${filePath}`,
);
});
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped path',
async () => {
// Create a directory with a space so the unescaped path exists
const dirWithSpace = path.join(tempRootDir, 'sub dir');
await fs.mkdir(dirWithSpace);
const params: GrepToolParams = {
pattern: 'hello',
path: path.join(tempRootDir, 'sub\\ dir'),
};
expect(grepTool.validateToolParams(params)).toBeNull();
expect(params.path).toBe(dirWithSpace);
},
);
});
describe('execute', () => {

View file

@ -13,9 +13,12 @@ import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { ToolNames, ToolDisplayNames } from './tool-names.js';
import { createDebugLogger } from '../utils/debugLogger.js';
import {
resolveAndValidatePath,
isSubpath,
unescapePath,
} from '../utils/paths.js';
const debugLogger = createDebugLogger('GREP');
import { resolveAndValidatePath, isSubpath } from '../utils/paths.js';
import { getMemoryBaseDir } from '../memory/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
import { isGitRepository } from '../utils/gitUtils.js';
@ -25,6 +28,8 @@ import type { FileExclusions } from '../utils/ignorePatterns.js';
import { ToolErrorType } from './tool-error.js';
import { isCommandAvailable } from '../utils/shell-utils.js';
const debugLogger = createDebugLogger('GREP');
// --- Interfaces ---
/**
@ -615,6 +620,7 @@ export class GrepTool extends BaseDeclarativeTool<GrepToolParams, ToolResult> {
// Only validate path if one is provided
if (params.path) {
params.path = unescapePath(params.path.trim());
try {
resolveAndValidatePath(this.config, params.path, {
allowExternalPaths: true,

View file

@ -9,6 +9,7 @@ import fs from 'node:fs/promises';
import path from 'node:path';
import os from 'node:os';
import { LSTool } from './ls.js';
import type { LSToolParams } from './ls.js';
import type { Config } from '../config/config.js';
import { FileDiscoveryService } from '../services/fileDiscoveryService.js';
import { ToolErrorType } from './tool-error.js';
@ -393,4 +394,21 @@ describe('LSTool', () => {
expect(result.returnDisplay).toBe('Listed 1 item(s)');
});
});
describe('validateToolParams', () => {
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped path',
async () => {
// Create a directory with a space so the unescaped path exists
const dirWithSpace = path.join(tempRootDir, 'sub dir');
await fs.mkdir(dirWithSpace);
const params: LSToolParams = {
path: path.join(tempRootDir, 'sub\\ dir'),
};
const result = lsTool.validateToolParams(params);
expect(result).toBeNull();
expect(params.path).toBe(dirWithSpace);
},
);
});
});

View file

@ -8,8 +8,13 @@ import fs from 'node:fs/promises';
import path from 'node:path';
import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { isSubpaths, isSubpath } from '../utils/paths.js';
import {
makeRelative,
shortenPath,
unescapePath,
isSubpaths,
isSubpath,
} from '../utils/paths.js';
import type { Config } from '../config/config.js';
import type { PermissionDecision } from '../permissions/types.js';
import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js';
@ -355,6 +360,8 @@ export class LSTool extends BaseDeclarativeTool<LSToolParams, ToolResult> {
protected override validateToolParamValues(
params: LSToolParams,
): string | null {
params.path = unescapePath(params.path.trim());
if (!path.isAbsolute(params.path)) {
return `Path must be absolute: ${params.path}`;
}

View file

@ -277,6 +277,21 @@ describe('LspTool', () => {
} as LspToolParams);
expect(result).toBe('query is required for workspaceSymbol.');
});
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped filePath',
() => {
const params: LspToolParams = {
operation: 'goToDefinition',
filePath: 'src/app\\ file.ts',
line: 10,
character: 5,
};
const result = tool.validateToolParams(params);
expect(result).toBeNull();
expect(params.filePath).toBe('src/app file.ts');
},
);
});
});

View file

@ -9,6 +9,7 @@ import { fileURLToPath, pathToFileURL } from 'node:url';
import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { ToolDisplayNames, ToolNames } from './tool-names.js';
import { unescapePath } from '../utils/paths.js';
import type { Config } from '../config/config.js';
import type {
LspCallHierarchyIncomingCall,
@ -1155,8 +1156,13 @@ export class LspTool extends BaseDeclarativeTool<LspToolParams, ToolResult> {
): string | null {
const operation = params.operation;
// Normalize shell-escaped paths (e.g. "my\ file.txt" → "my file.txt")
if (params.filePath) {
params.filePath = unescapePath(params.filePath.trim());
}
if (LOCATION_REQUIRED_OPERATIONS.has(operation)) {
if (!params.filePath || params.filePath.trim() === '') {
if (!params.filePath) {
return `filePath is required for ${operation}.`;
}
if (typeof params.line !== 'number') {
@ -1165,7 +1171,7 @@ export class LspTool extends BaseDeclarativeTool<LspToolParams, ToolResult> {
}
if (FILE_REQUIRED_OPERATIONS.has(operation)) {
if (!params.filePath || params.filePath.trim() === '') {
if (!params.filePath) {
return `filePath is required for ${operation}.`;
}
}
@ -1183,7 +1189,7 @@ export class LspTool extends BaseDeclarativeTool<LspToolParams, ToolResult> {
}
if (RANGE_REQUIRED_OPERATIONS.has(operation)) {
if (!params.filePath || params.filePath.trim() === '') {
if (!params.filePath) {
return `filePath is required for ${operation}.`;
}
if (typeof params.line !== 'number') {

View file

@ -82,6 +82,21 @@ describe('ReadFileTool', () => {
);
});
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped spaces in file_path',
() => {
const escapedPath = path.join(tempRootDir, 'my\\ file.txt');
const params: ReadFileToolParams = {
file_path: escapedPath,
};
const invocation = tool.build(params);
expect(invocation).toBeDefined();
expect(invocation.params.file_path).toBe(
path.join(tempRootDir, 'my file.txt'),
);
},
);
it('should allow path outside root (external path support)', () => {
const params: ReadFileToolParams = {
file_path: '/outside/root.txt',
@ -268,6 +283,29 @@ describe('ReadFileTool', () => {
});
});
it.skipIf(process.platform === 'win32')(
'should read a file with spaces in its name when given an escaped path',
async () => {
const realFileName = 'my spaced read.txt';
const realPath = path.join(tempRootDir, realFileName);
const fileContent = 'Content with spaces in filename.';
await fsp.writeFile(realPath, fileContent, 'utf-8');
// Pass an ESCAPED path (as the LLM might from at-completion)
const escapedPath = path.join(tempRootDir, 'my\\ spaced\\ read.txt');
const params: ReadFileToolParams = { file_path: escapedPath };
const invocation = tool.build(params) as ToolInvocation<
ReadFileToolParams,
ToolResult
>;
expect(await invocation.execute(abortSignal)).toEqual({
llmContent: fileContent,
returnDisplay: '',
});
},
);
it('should return error if path is a directory', async () => {
const dirPath = path.join(tempRootDir, 'directory');
await fsp.mkdir(dirPath);

View file

@ -8,7 +8,7 @@ import os from 'node:os';
import path from 'node:path';
import fs from 'node:fs/promises';
import type { Stats } from 'node:fs';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { makeRelative, shortenPath, unescapePath } from '../utils/paths.js';
import type { ToolInvocation, ToolLocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { ToolNames, ToolDisplayNames } from './tool-names.js';
@ -383,8 +383,12 @@ export class ReadFileTool extends BaseDeclarativeTool<
protected override validateToolParamValues(
params: ReadFileToolParams,
): string | null {
const filePath = params.file_path;
if (params.file_path.trim() === '') {
// Normalize shell-escaped paths (e.g. "my\ file.txt" → "my file.txt")
// that may reach the LLM via at-completion or manual typing.
const filePath = unescapePath(params.file_path.trim());
params.file_path = filePath;
if (!filePath) {
return "The 'file_path' parameter must be non-empty.";
}

View file

@ -140,6 +140,21 @@ describe('RipGrepTool', () => {
const params: RipGrepToolParams = { pattern: 'hello', path: filePath };
expect(grepTool.validateToolParams(params)).toBeNull();
});
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped path',
async () => {
// Create a directory with a space so the unescaped path exists
const dirWithSpace = path.join(tempRootDir, 'sub dir');
await fs.mkdir(dirWithSpace);
const params: RipGrepToolParams = {
pattern: 'hello',
path: path.join(tempRootDir, 'sub\\ dir'),
};
expect(grepTool.validateToolParams(params)).toBeNull();
expect(params.path).toBe(dirWithSpace);
},
);
});
describe('execute', () => {

View file

@ -9,7 +9,7 @@ import path from 'node:path';
import type { ToolInvocation, ToolResult } from './tools.js';
import { BaseDeclarativeTool, BaseToolInvocation, Kind } from './tools.js';
import { ToolNames } from './tool-names.js';
import { resolveAndValidatePath } from '../utils/paths.js';
import { resolveAndValidatePath, unescapePath } from '../utils/paths.js';
import { getErrorMessage } from '../utils/errors.js';
import type { Config } from '../config/config.js';
import { runRipgrep } from '../utils/ripgrepUtils.js';
@ -424,6 +424,7 @@ export class RipGrepTool extends BaseDeclarativeTool<
// Only validate path if one is provided
if (params.path) {
params.path = unescapePath(params.path.trim());
try {
resolveAndValidatePath(this.config, params.path, {
allowFiles: true,

View file

@ -178,6 +178,24 @@ describe('WriteFileTool', () => {
};
expect(() => tool.build(params)).toThrow(`Missing or empty "file_path"`);
});
it.skipIf(process.platform === 'win32')(
'should unescape shell-escaped spaces in file_path',
() => {
// On Windows, unescapePath is a no-op and backslashes are path
// separators, so the expected unescape behavior doesn't apply.
const escapedPath = path.join(rootDir, 'my\\ file.txt');
const params = {
file_path: escapedPath,
content: 'hello',
};
const invocation = tool.build(params);
expect(invocation).toBeDefined();
expect(invocation.params.file_path).toBe(
path.join(rootDir, 'my file.txt'),
);
},
);
});
describe('shouldConfirmExecute', () => {
@ -458,6 +476,37 @@ describe('WriteFileTool', () => {
expect(result.llmContent).not.toMatch(/User modified the `content`/);
});
it.skipIf(process.platform === 'win32')(
'should write to a file with spaces in its name when given an escaped path',
async () => {
// On Windows, unescapePath is a no-op and backslashes are path
// separators, so shell-escaping behavior doesn't apply.
const realPath = path.join(rootDir, 'my spaced write.txt');
const escapedPath = path.join(rootDir, 'my\\ spaced\\ write.txt');
const content = 'Written via escaped path.';
const params = { file_path: escapedPath, content };
const invocation = tool.build(params);
const confirmDetails =
await invocation.getConfirmationDetails(abortSignal);
if (
typeof confirmDetails === 'object' &&
'onConfirm' in confirmDetails &&
confirmDetails.onConfirm
) {
await confirmDetails.onConfirm(ToolConfirmationOutcome.ProceedOnce);
}
const result = await invocation.execute(abortSignal);
// Should succeed — file created at the unescaped (real) path
expect(result.llmContent).toMatch(/Successfully created and wrote/);
expect(fs.existsSync(realPath)).toBe(true);
expect(fs.readFileSync(realPath, 'utf8')).toBe(content);
},
);
});
describe('workspace boundary validation', () => {

View file

@ -32,7 +32,7 @@ import {
detectLineEnding,
} from '../services/fileSystemService.js';
import type { LineEnding } from '../services/fileSystemService.js';
import { makeRelative, shortenPath } from '../utils/paths.js';
import { makeRelative, shortenPath, unescapePath } from '../utils/paths.js';
import { getErrorMessage, isNodeError } from '../utils/errors.js';
import { DEFAULT_DIFF_OPTIONS, getDiffStat } from './diffOptions.js';
import { ToolNames, ToolDisplayNames } from './tool-names.js';
@ -404,7 +404,10 @@ export class WriteFileTool
protected override validateToolParamValues(
params: WriteFileToolParams,
): string | null {
const filePath = params.file_path;
// Normalize shell-escaped paths (e.g. "my\ file.txt" → "my file.txt")
// that may reach the LLM via at-completion or manual typing.
const filePath = unescapePath(params.file_path.trim());
params.file_path = filePath;
if (!filePath) {
return `Missing or empty "file_path"`;

View file

@ -642,35 +642,38 @@ describe('FileSearch', () => {
expect(limitedResults).toEqual(['file1.js', 'file2.js']);
});
it('should handle file paths with special characters that need escaping', async () => {
tmpDir = await createTmpDir({
src: {
'file with (special) chars.txt': '',
'another-file.txt': '',
},
});
it.skipIf(process.platform === 'win32')(
'should handle file paths with special characters that need escaping',
async () => {
tmpDir = await createTmpDir({
src: {
'file with (special) chars.txt': '',
'another-file.txt': '',
},
});
const fileSearch = FileSearchFactory.create({
projectRoot: tmpDir,
useGitignore: false,
useQwenignore: false,
ignoreDirs: [],
cache: false,
cacheTtl: 0,
enableRecursiveFileSearch: true,
enableFuzzySearch: true,
});
const fileSearch = FileSearchFactory.create({
projectRoot: tmpDir,
useGitignore: false,
useQwenignore: false,
ignoreDirs: [],
cache: false,
cacheTtl: 0,
enableRecursiveFileSearch: true,
enableFuzzySearch: true,
});
await fileSearch.initialize();
await fileSearch.initialize();
// Search for the file using a pattern that contains special characters.
// The `unescapePath` function should handle the escaped path correctly.
const results = await fileSearch.search(
'src/file with \\(special\\) chars.txt',
);
// Search for the file using a pattern that contains special characters.
// The `unescapePath` function should handle the escaped path correctly.
const results = await fileSearch.search(
'src/file with \\(special\\) chars.txt',
);
expect(results).toEqual(['src/file with (special) chars.txt']);
});
expect(results).toEqual(['src/file with (special) chars.txt']);
},
);
describe('DirectoryFileSearch', () => {
it('should search for files in the current directory', async () => {

View file

@ -192,81 +192,97 @@ describe('escapePath', () => {
});
describe('unescapePath', () => {
it('should unescape spaces', () => {
expect(unescapePath('my\\ file.txt')).toBe('my file.txt');
});
const isWindows = process.platform === 'win32';
it('should unescape tabs', () => {
expect(unescapePath('file\\\twith\\\ttabs.txt')).toBe(
'file\twith\ttabs.txt',
// On Windows, backslashes are path separators, not shell escape chars.
// unescapePath is intentionally a no-op on win32.
it.skipIf(!isWindows)('should be a no-op on Windows', () => {
expect(unescapePath('C:\\Users\\my file.txt')).toBe(
'C:\\Users\\my file.txt',
);
expect(unescapePath('C:\\(v2)\\file.txt')).toBe('C:\\(v2)\\file.txt');
expect(unescapePath('path\\to\\file\\ name.txt')).toBe(
'path\\to\\file\\ name.txt',
);
});
it('should unescape parentheses', () => {
expect(unescapePath('file\\(1\\).txt')).toBe('file(1).txt');
});
it('should unescape square brackets', () => {
expect(unescapePath('file\\[backup\\].txt')).toBe('file[backup].txt');
});
it('should unescape curly braces', () => {
expect(unescapePath('file\\{temp\\}.txt')).toBe('file{temp}.txt');
});
it('should unescape multiple special characters', () => {
expect(unescapePath('my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt')).toBe(
'my file (backup) [v1.2].txt',
);
});
it('should handle paths without escaped characters', () => {
expect(unescapePath('normalfile.txt')).toBe('normalfile.txt');
expect(unescapePath('path/to/normalfile.txt')).toBe(
'path/to/normalfile.txt',
);
});
it('should handle all special characters', () => {
expect(
unescapePath(
'\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\~\\<\\>',
),
).toBe(' ()[]{};&|*?$`\'"#!~<>');
});
it('should be the inverse of escapePath', () => {
const testCases = [
'my file.txt',
'file(1).txt',
'file[backup].txt',
'My Documents/Project (2024)/file [backup].txt',
'file with $special &chars!.txt',
' ()[]{};&|*?$`\'"#!~<>',
'file\twith\ttabs.txt',
];
testCases.forEach((testCase) => {
expect(unescapePath(escapePath(testCase))).toBe(testCase);
describe.skipIf(isWindows)('on Unix', () => {
it('should unescape spaces', () => {
expect(unescapePath('my\\ file.txt')).toBe('my file.txt');
});
});
it('should handle empty strings', () => {
expect(unescapePath('')).toBe('');
});
it('should unescape tabs', () => {
expect(unescapePath('file\\\twith\\\ttabs.txt')).toBe(
'file\twith\ttabs.txt',
);
});
it('should not affect backslashes not followed by special characters', () => {
expect(unescapePath('file\\name.txt')).toBe('file\\name.txt');
expect(unescapePath('path\\to\\file.txt')).toBe('path\\to\\file.txt');
});
it('should unescape parentheses', () => {
expect(unescapePath('file\\(1\\).txt')).toBe('file(1).txt');
});
it('should handle escaped backslashes in unescaping', () => {
// Should correctly unescape when there are escaped backslashes
expect(unescapePath('path\\\\\\ file.txt')).toBe('path\\\\ file.txt');
expect(unescapePath('path\\\\\\\\\\ file.txt')).toBe(
'path\\\\\\\\ file.txt',
);
expect(unescapePath('file\\\\\\(test\\).txt')).toBe('file\\\\(test).txt');
it('should unescape square brackets', () => {
expect(unescapePath('file\\[backup\\].txt')).toBe('file[backup].txt');
});
it('should unescape curly braces', () => {
expect(unescapePath('file\\{temp\\}.txt')).toBe('file{temp}.txt');
});
it('should unescape multiple special characters', () => {
expect(unescapePath('my\\ file\\ \\(backup\\)\\ \\[v1.2\\].txt')).toBe(
'my file (backup) [v1.2].txt',
);
});
it('should handle paths without escaped characters', () => {
expect(unescapePath('normalfile.txt')).toBe('normalfile.txt');
expect(unescapePath('path/to/normalfile.txt')).toBe(
'path/to/normalfile.txt',
);
});
it('should handle all special characters', () => {
expect(
unescapePath(
'\\ \\(\\)\\[\\]\\{\\}\\;\\&\\|\\*\\?\\$\\`\\\'\\"\\#\\!\\~\\<\\>',
),
).toBe(' ()[]{};&|*?$`\'"#!~<>');
});
it('should be the inverse of escapePath', () => {
const testCases = [
'my file.txt',
'file(1).txt',
'file[backup].txt',
'My Documents/Project (2024)/file [backup].txt',
'file with $special &chars!.txt',
' ()[]{};&|*?$`\'"#!~<>',
'file\twith\ttabs.txt',
];
testCases.forEach((testCase) => {
expect(unescapePath(escapePath(testCase))).toBe(testCase);
});
});
it('should handle empty strings', () => {
expect(unescapePath('')).toBe('');
});
it('should not affect backslashes not followed by special characters', () => {
expect(unescapePath('file\\name.txt')).toBe('file\\name.txt');
expect(unescapePath('path\\to\\file.txt')).toBe('path\\to\\file.txt');
});
it('should handle escaped backslashes in unescaping', () => {
// Should correctly unescape when there are escaped backslashes
expect(unescapePath('path\\\\\\ file.txt')).toBe('path\\\\ file.txt');
expect(unescapePath('path\\\\\\\\\\ file.txt')).toBe(
'path\\\\\\\\ file.txt',
);
expect(unescapePath('file\\\\\\(test\\).txt')).toBe('file\\\\(test).txt');
});
});
});

View file

@ -46,6 +46,22 @@ export function _resetValidatePathCacheForTest(): void {
*/
export const SHELL_SPECIAL_CHARS = /[ \t()[\]{};|*?$`'"#&<>!~]/;
// Single shared list of path-argument keys used across file tools.
// file_path (Edit, ReadFile, WriteFile), path (Glob, Grep, Ls, RipGrep),
// filePath (Lsp), notebook_path.
export const PATH_ARG_KEYS = [
'file_path',
'path',
'filePath',
'notebook_path',
] as const;
/** Compiled regex for unescapePath — hoisted to avoid re-compilation per call. */
const UNESCAPE_REGEX = (() => {
const inner = SHELL_SPECIAL_CHARS.source.slice(1, -1);
return new RegExp(`\\\\([${inner}])`, 'g');
})();
/**
* Replaces the home directory with a tilde.
* @param path - The path to tildeify.
@ -205,12 +221,17 @@ export function escapePath(filePath: string): string {
/**
* Unescapes special characters in a file path.
* Removes backslash escaping from shell metacharacters.
*
* On Windows, backslashes are path separators, not shell escape characters
* (PowerShell uses backtick, cmd.exe uses caret). Skipping unescaping on
* win32 avoids corrupting valid absolute paths like C:\(v2)\file.txt.
*/
export function unescapePath(filePath: string): string {
return filePath.replace(
new RegExp(`\\\\([${SHELL_SPECIAL_CHARS.source.slice(1, -1)}])`, 'g'),
'$1',
);
if (os.platform() === 'win32') {
return filePath;
}
const unescaped = filePath.replace(UNESCAPE_REGEX, '$1');
return unescaped;
}
/**

View file

@ -13,7 +13,7 @@ import {
loadRules,
ConditionalRulesRegistry,
} from './rulesDiscovery.js';
import { QWEN_DIR } from './paths.js';
import { QWEN_DIR, unescapePath } from './paths.js';
vi.mock('os', async (importOriginal) => {
const actualOs = await importOriginal<typeof os>();
@ -457,5 +457,24 @@ Use hooks.`,
reg.matchAndConsume('/totally/other/place/file.ts'),
).toBeUndefined();
});
it.skipIf(process.platform === 'win32')(
'should match shell-escaped file paths after unescaping',
() => {
// On Windows, unescapePath is a no-op (backslash is a path
// separator, not a shell escape character).
const reg = new ConditionalRulesRegistry(
[rule('/r/ts.md', ['src/**/*.tsx'], 'Use hooks.')],
'/project',
);
const escapedPath = 'src/App\\ file.tsx';
const normalizedPath = unescapePath(escapedPath.trim());
expect(normalizedPath).toBe('src/App file.tsx');
const result = reg.matchAndConsume(
path.resolve('/project', normalizedPath),
);
expect(result).toContain('Use hooks.');
},
);
});
});