From ef882f185a33827d25bad97bd4301ea0d516d7bb Mon Sep 17 00:00:00 2001 From: Taylor Mullen Date: Wed, 11 Mar 2026 15:51:42 -0700 Subject: [PATCH] fix(core): correct tool-specific approval narrowing and mode transitions - Fix list_directory to use dir_path instead of file_path in rule matching. - Fix read_many_files to use include property and fix double-stringification. - Fix web_fetch to use url/prompt properties for narrowing. - Add web_fetch to centralized AUTO_EDIT transition logic. - Add generic buildParamArgsPattern and buildDirPathArgsPattern helpers. --- packages/core/src/policy/utils.ts | 39 +++++++++++++++++----- packages/core/src/scheduler/policy.ts | 5 +-- packages/core/src/tools/ls.ts | 4 +-- packages/core/src/tools/read-many-files.ts | 6 ++-- packages/core/src/tools/web-fetch.ts | 9 +++-- 5 files changed, 42 insertions(+), 21 deletions(-) diff --git a/packages/core/src/policy/utils.ts b/packages/core/src/policy/utils.ts index f16baa6c0f..a0cb1b7451 100644 --- a/packages/core/src/policy/utils.ts +++ b/packages/core/src/policy/utils.ts @@ -89,6 +89,24 @@ export function buildArgsPatterns( return [argsPattern]; } +/** + * Builds a regex pattern to match a specific parameter and value in tool arguments. + * This is used to narrow tool approvals to specific parameters. + * + * @param paramName The name of the parameter. + * @param value The value to match. + * @returns A regex string that matches "": in a JSON string. + */ +export function buildParamArgsPattern( + paramName: string, + value: unknown, +): string { + const encodedValue = JSON.stringify(value); + // We must wrap the JSON string in escapeRegex to ensure regex control characters + // are treated as literals, preventing overly broad matches. + return escapeRegex(`"${paramName}":${encodedValue}`); +} + /** * Builds a regex pattern to match a specific file path in tool arguments. * This is used to narrow tool approvals for edit tools to specific files. @@ -97,11 +115,18 @@ export function buildArgsPatterns( * @returns A regex string that matches "file_path":"" in a JSON string. */ export function buildFilePathArgsPattern(filePath: string): string { - const encodedPath = JSON.stringify(filePath); - // We must wrap the JSON string in escapeRegex to ensure regex control characters - // (like '.' in file extensions) are treated as literals, preventing overly broad - // matches (e.g. 'foo.ts' matching 'fooXts'). - return escapeRegex(`"file_path":${encodedPath}`); + return buildParamArgsPattern('file_path', filePath); +} + +/** + * Builds a regex pattern to match a specific directory path in tool arguments. + * This is used to narrow tool approvals for list_directory tool. + * + * @param dirPath The path to the directory. + * @returns A regex string that matches "dir_path":"" in a JSON string. + */ +export function buildDirPathArgsPattern(dirPath: string): string { + return buildParamArgsPattern('dir_path', dirPath); } /** @@ -112,7 +137,5 @@ export function buildFilePathArgsPattern(filePath: string): string { * @returns A regex string that matches "pattern":"" in a JSON string. */ export function buildPatternArgsPattern(pattern: string): string { - const encodedPattern = JSON.stringify(pattern); - // We use escapeRegex to ensure regex control characters are treated as literals. - return escapeRegex(`"pattern":${encodedPattern}`); + return buildParamArgsPattern('pattern', pattern); } diff --git a/packages/core/src/scheduler/policy.ts b/packages/core/src/scheduler/policy.ts index 9a5a43735d..47329790b4 100644 --- a/packages/core/src/scheduler/policy.ts +++ b/packages/core/src/scheduler/policy.ts @@ -26,7 +26,7 @@ import { import { buildFilePathArgsPattern } from '../policy/utils.js'; import { makeRelative } from '../utils/paths.js'; import { DiscoveredMCPTool } from '../tools/mcp-tool.js'; -import { EDIT_TOOL_NAMES } from '../tools/tool-names.js'; +import { EDIT_TOOL_NAMES, WEB_FETCH_TOOL_NAME } from '../tools/tool-names.js'; import type { ValidatingToolCall } from './types.js'; import type { AgentLoopContext } from '../config/agent-loop-context.js'; @@ -115,6 +115,7 @@ export async function updatePolicy( toolInvocation?: AnyToolInvocation, ): Promise { const deps = { ...context, toolInvocation }; + // Mode Transitions (AUTO_EDIT) if (isAutoEditTransition(tool, outcome)) { deps.config.setApprovalMode(ApprovalMode.AUTO_EDIT); @@ -172,7 +173,7 @@ function isAutoEditTransition( // tools. return ( outcome === ToolConfirmationOutcome.ProceedAlways && - EDIT_TOOL_NAMES.has(tool.name) + (EDIT_TOOL_NAMES.has(tool.name) || tool.name === WEB_FETCH_TOOL_NAME) ); } diff --git a/packages/core/src/tools/ls.ts b/packages/core/src/tools/ls.ts index 1e2d1cccf8..a6850ed825 100644 --- a/packages/core/src/tools/ls.ts +++ b/packages/core/src/tools/ls.ts @@ -21,7 +21,7 @@ import type { Config } from '../config/config.js'; import { DEFAULT_FILE_FILTERING_OPTIONS } from '../config/constants.js'; import { ToolErrorType } from './tool-error.js'; import { LS_TOOL_NAME } from './tool-names.js'; -import { buildFilePathArgsPattern } from '../policy/utils.js'; +import { buildDirPathArgsPattern } from '../policy/utils.js'; import { debugLogger } from '../utils/debugLogger.js'; import { LS_DEFINITION } from './definitions/coreTools.js'; import { resolveToolDeclaration } from './definitions/resolver.js'; @@ -130,7 +130,7 @@ class LSToolInvocation extends BaseToolInvocation { _outcome: ToolConfirmationOutcome, ): PolicyUpdateOptions | undefined { return { - argsPattern: buildFilePathArgsPattern(this.params.dir_path), + argsPattern: buildDirPathArgsPattern(this.params.dir_path), }; } diff --git a/packages/core/src/tools/read-many-files.ts b/packages/core/src/tools/read-many-files.ts index 4a2ae9a4c0..c297f95ae8 100644 --- a/packages/core/src/tools/read-many-files.ts +++ b/packages/core/src/tools/read-many-files.ts @@ -18,7 +18,7 @@ import { getErrorMessage } from '../utils/errors.js'; import * as fsPromises from 'node:fs/promises'; import * as path from 'node:path'; import { glob, escape } from 'glob'; -import { buildPatternArgsPattern } from '../policy/utils.js'; +import { buildParamArgsPattern } from '../policy/utils.js'; import { detectFileType, processSingleFileContent, @@ -161,10 +161,8 @@ ${finalExclusionPatternsForDescription override getPolicyUpdateOptions( _outcome: ToolConfirmationOutcome, ): PolicyUpdateOptions | undefined { - // We join the include patterns to match the JSON stringified arguments. - // buildPatternArgsPattern handles JSON stringification. return { - argsPattern: buildPatternArgsPattern(JSON.stringify(this.params.include)), + argsPattern: buildParamArgsPattern('include', this.params.include), }; } diff --git a/packages/core/src/tools/web-fetch.ts b/packages/core/src/tools/web-fetch.ts index e4d9ebc36f..7d16fb1d76 100644 --- a/packages/core/src/tools/web-fetch.ts +++ b/packages/core/src/tools/web-fetch.ts @@ -14,7 +14,7 @@ import { type ToolConfirmationOutcome, type PolicyUpdateOptions, } from './tools.js'; -import { buildPatternArgsPattern } from '../policy/utils.js'; +import { buildParamArgsPattern } from '../policy/utils.js'; import type { MessageBus } from '../confirmation-bus/message-bus.js'; import { ToolErrorType } from './tool-error.js'; import { getErrorMessage } from '../utils/errors.js'; @@ -328,12 +328,11 @@ ${textContent} ): PolicyUpdateOptions | undefined { if (this.params.url) { return { - argsPattern: buildPatternArgsPattern(this.params.url), + argsPattern: buildParamArgsPattern('url', this.params.url), }; - } - if (this.params.prompt) { + } else if (this.params.prompt) { return { - argsPattern: buildPatternArgsPattern(this.params.prompt), + argsPattern: buildParamArgsPattern('prompt', this.params.prompt), }; } return undefined;