fix(core): secure argsPattern and revert WEB_FETCH_TOOL_NAME escalation (#22104)

Co-authored-by: Taylor Mullen <ntaylormullen@google.com>

packages/core/src/core/contentGenerator.test.ts

@@ -505,6 +505,8 @@ describe('createContentGenerator', () => {
   });
 
   it('should not include baseUrl in httpOptions when GOOGLE_GEMINI_BASE_URL is not set', async () => {
+    vi.stubEnv('GOOGLE_GEMINI_BASE_URL', '');
+
     const mockConfig = {
       getModel: vi.fn().mockReturnValue('gemini-pro'),
       getProxy: vi.fn().mockReturnValue(undefined),

packages/core/src/policy/policies/plan.toml

@@ -98,7 +98,7 @@ toolName = ["write_file", "replace"]
 decision = "allow"
 priority = 70
 modes = ["plan"]
-argsPattern = "\"file_path\":\"[^\"]+[\\\\/]+\\.gemini[\\\\/]+tmp[\\\\/]+[\\w-]+[\\\\/]+[\\w-]+[\\\\/]+plans[\\\\/]+[\\w-]+\\.md\""
+argsPattern = "\\x00\"file_path\":\"[^\"]+[\\\\/]+\\.gemini[\\\\/]+tmp[\\\\/]+[\\w-]+[\\\\/]+[\\w-]+[\\\\/]+plans[\\\\/]+[\\w-]+\\.md\"\\x00"
 
 # Explicitly Deny other write operations in Plan mode with a clear message.
 [[rule]]

packages/core/src/policy/stable-stringify.ts

@@ -57,7 +57,11 @@
  * // Returns: '{"safe":"data"}'
  */
 export function stableStringify(obj: unknown): string {
-  const stringify = (currentObj: unknown, ancestors: Set<unknown>): string => {
+  const stringify = (
+    currentObj: unknown,
+    ancestors: Set<unknown>,
+    isTopLevel = false,
+  ): string => {
     // Handle primitives and null
     if (currentObj === undefined) {
       return 'null'; // undefined in arrays becomes null in JSON
@@ -89,7 +93,10 @@ export function stableStringify(obj: unknown): string {
           if (jsonValue === null) {
             return 'null';
           }
-          return stringify(jsonValue, ancestors);
+          // The result of toJSON is effectively a new object graph, but it
+          // takes the place of the current node, so we preserve the top-level
+          // status of the current node.
+          return stringify(jsonValue, ancestors, isTopLevel);
         } catch {
           // If toJSON throws, treat as a regular object
         }
@@ -101,7 +108,7 @@ export function stableStringify(obj: unknown): string {
           if (item === undefined || typeof item === 'function') {
             return 'null';
           }
-          return stringify(item, ancestors);
+          return stringify(item, ancestors, false);
         });
         return '[' + items.join(',') + ']';
       }
@@ -115,7 +122,17 @@ export function stableStringify(obj: unknown): string {
         const value = (currentObj as Record<string, unknown>)[key];
         // Skip undefined and function values in objects (per JSON spec)
         if (value !== undefined && typeof value !== 'function') {
-          pairs.push(JSON.stringify(key) + ':' + stringify(value, ancestors));
+          let pairStr =
+            JSON.stringify(key) + ':' + stringify(value, ancestors, false);
+
+          if (isTopLevel) {
+            // We use a null byte (\0) to denote structural boundaries.
+            // This is safe because any literal \0 in the user's data will
+            // be escaped by JSON.stringify into "\u0000" before reaching here.
+            pairStr = '\0' + pairStr + '\0';
+          }
+
+          pairs.push(pairStr);
         }
       }
 
@@ -125,5 +142,5 @@ export function stableStringify(obj: unknown): string {
     }
   };
 
-  return stringify(obj, new Set());
+  return stringify(obj, new Set(), true);
 }

packages/core/src/policy/utils.ts

@@ -89,6 +89,25 @@ 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 "<paramName>":<value> in a JSON string.
+ */
+export function buildParamArgsPattern(
+  paramName: string,
+  value: unknown,
+): string {
+  const encodedValue = JSON.stringify(value);
+  // We wrap the JSON string in escapeRegex and prepend/append \\0 to explicitly
+  // match top-level JSON properties generated by stableStringify, preventing
+  // argument injection bypass attacks.
+  return `\\\\0${escapeRegex(`"${paramName}":${encodedValue}`)}\\\\0`;
+}
+
 /**
  * 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 +116,18 @@ export function buildArgsPatterns(
  * @returns A regex string that matches "file_path":"<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":"<path>" in a JSON string.
+ */
+export function buildDirPathArgsPattern(dirPath: string): string {
+  return buildParamArgsPattern('dir_path', dirPath);
 }
 
 /**
@@ -112,7 +138,5 @@ export function buildFilePathArgsPattern(filePath: string): string {
  * @returns A regex string that matches "pattern":"<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);
 }

packages/core/src/scheduler/policy.test.ts

@@ -660,7 +660,8 @@ describe('policy.ts', () => {
       expect(mockMessageBus.publish).toHaveBeenCalledWith(
         expect.objectContaining({
           toolName: 'write_file',
-          argsPattern: escapeRegex('"file_path":"src/foo.ts"'),
+          argsPattern:
+            '\\\\0' + escapeRegex('"file_path":"src/foo.ts"') + '\\\\0',
         }),
       );
     });

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<LSToolParams, ToolResult> {
     _outcome: ToolConfirmationOutcome,
   ): PolicyUpdateOptions | undefined {
     return {
-      argsPattern: buildFilePathArgsPattern(this.params.dir_path),
+      argsPattern: buildDirPathArgsPattern(this.params.dir_path),
     };
   }
 

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),
     };
   }
 

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;