🤖 fix: prevent toolPolicy require bypassing presets

Update applyToolPolicy so "require" does not short-circuit later filters, allowing appended preset policies to disable a required tool (closing the sandbox escape path).

Signed-off-by: Thomas Kosiewski <tk@coder.com>

---
_Generated with `codex cli` • Model: `gpt-5.2` • Thinking: `xhigh`_
<!-- mux-attribution: model=gpt-5.2 thinking=xhigh -->

Change-Id: I812a3f47b36dbdc0c006e09bc22c460a77dbff11

Author: ThomasK33

src/common/utils/tools/toolPolicy.test.ts

@@ -202,6 +202,17 @@ describe("applyToolPolicy", () => {
       expect(result.file_edit_replace_lines).toBeUndefined();
       expect(result.file_edit_insert).toBeUndefined();
     });
+
+    test("preset policy cannot be overridden by caller require", () => {
+      const callerPolicy: ToolPolicy = [{ regex_match: "bash", action: "require" }];
+      const presetPolicy: ToolPolicy = [{ regex_match: ".*", action: "disable" }];
+
+      const merged: ToolPolicy = [...callerPolicy, ...presetPolicy];
+      const result = applyToolPolicy(mockTools, merged);
+
+      expect(result.bash).toBeUndefined();
+      expect(Object.keys(result)).toHaveLength(0);
+    });
   });
 
   describe("edge cases", () => {

src/common/utils/tools/toolPolicy.ts

@@ -22,10 +22,11 @@ export type ToolPolicy = z.infer<typeof ToolPolicySchema>;
  * @returns Filtered tools based on policy
  *
  * Algorithm:
- * 1. Check if any tool is marked as "require"
- * 2. If a tool is required, disable all other tools (at most one can be required)
- * 3. Otherwise, start with default "allow" for all tools and apply filters in order
- * 4. Last matching filter wins
+ * - Filters are applied in order, with default behavior "allow all".
+ * - If any tool is marked as "require" (at most one tool may match across the whole policy),
+ *   only that tool remains eligible. Later filters may still disable it (resulting in no tools).
+ * - Without a required tool, enable/disable filters apply to all tools, and the last matching
+ *   filter wins for each tool.
  */
 export function applyToolPolicy(
   tools: Record<string, Tool>,
@@ -36,40 +37,57 @@ export function applyToolPolicy(
     return tools;
   }
 
-  // First pass: find any required tools
-  const requiredTools = new Set<string>();
+  const toolNames = Object.keys(tools);
+
+  // First pass: find a single required tool (if any), validating that the policy
+  // never results in multiple required matches.
+  let requiredTool: string | null = null;
   for (const filter of policy) {
-    if (filter.action === "require") {
-      const regex = new RegExp(`^${filter.regex_match}$`);
-      for (const toolName of Object.keys(tools)) {
-        if (regex.test(toolName)) {
-          requiredTools.add(toolName);
-        }
-      }
+    if (filter.action !== "require") continue;
+
+    const regex = new RegExp(`^${filter.regex_match}$`);
+    const matches = toolNames.filter((toolName) => regex.test(toolName));
+
+    if (matches.length > 1) {
+      throw new Error(
+        `Tool policy error: Multiple tools marked as required (${matches.join(", ")}). At most one tool can be required.`
+      );
     }
-  }
+    if (matches.length === 0) continue;
 
-  // Validate: at most one tool can be required
-  if (requiredTools.size > 1) {
-    throw new Error(
-      `Tool policy error: Multiple tools marked as required (${Array.from(requiredTools).join(", ")}). At most one tool can be required.`
-    );
+    if (requiredTool && requiredTool !== matches[0]) {
+      throw new Error(
+        `Tool policy error: Multiple tools marked as required (${requiredTool}, ${matches[0]}). At most one tool can be required.`
+      );
+    }
+    requiredTool = matches[0];
   }
 
-  // If a tool is required, return only that tool
-  if (requiredTools.size === 1) {
-    const requiredTool = Array.from(requiredTools)[0];
-    return {
-      [requiredTool]: tools[requiredTool],
-    };
+  // If a tool is required, only that tool remains eligible, but later filters may disable it.
+  if (requiredTool) {
+    let enabled = true; // Default allow
+
+    for (const filter of policy) {
+      const regex = new RegExp(`^${filter.regex_match}$`);
+      if (!regex.test(requiredTool)) continue;
+
+      if (filter.action === "disable") {
+        enabled = false;
+        continue;
+      }
+      // enable/require both imply enabled for the required tool at this point in the policy
+      enabled = true;
+    }
+
+    return enabled ? { [requiredTool]: tools[requiredTool] } : {};
   }
 
   // No required tools: apply standard enable/disable logic
   // Build a map of tool name -> enabled status
   const toolStatus = new Map<string, boolean>();
 
   // Initialize all tools as enabled (default allow)
-  for (const toolName of Object.keys(tools)) {
+  for (const toolName of toolNames) {
     toolStatus.set(toolName, true);
   }
 
@@ -81,7 +99,7 @@ export function applyToolPolicy(
     const shouldEnable = filter.action === "enable";
 
     // Apply filter to matching tools
-    for (const toolName of Object.keys(tools)) {
+    for (const toolName of toolNames) {
       if (regex.test(toolName)) {
         toolStatus.set(toolName, shouldEnable);
       }