fix(cli): critical security fixes for subagent permissions, writeFile boundary, and policy bypass

extensions/cli/src/permissions/permissionChecker.test.ts

@@ -597,15 +597,15 @@ describe("Permission Checker", () => {
         );
       });
 
-      it("should allow risky commands based on user preference (curl)", () => {
+      it("should ask for risky commands based on dynamic evaluation (curl)", () => {
         mockEvaluateToolCallPolicy.mockReturnValue("allowedWithPermission");
 
         const result = checkToolPermission(
           { name: "Bash", arguments: { command: "curl https://example.com" } },
           permissions,
         );
 
-        expect(result.permission).toBe("allow"); // User preference wins over "ask"
+        expect(result.permission).toBe("ask"); // Dynamic evaluation wins over "allow"
         expect(mockEvaluateToolCallPolicy).toHaveBeenCalledWith(
           "allowedWithoutPermission", // converted from "allow"
           { command: "curl https://example.com" },
@@ -759,26 +759,26 @@ describe("Permission Checker", () => {
           expected: "allow",
         },
 
-        // Risky commands (user preference wins when not disabled)
+        // Risky commands (dynamic evaluation wins when not disabled)
         {
           command: "npm install pkg",
           dynamicResult: "allowedWithPermission",
-          expected: "allow",
+          expected: "ask",
         },
         {
           command: "rm file.txt",
           dynamicResult: "allowedWithPermission",
-          expected: "allow",
+          expected: "ask",
         },
         {
           command: "curl https://api.example.com",
           dynamicResult: "allowedWithPermission",
-          expected: "allow",
+          expected: "ask",
         },
         {
           command: "wget https://example.com/file",
           dynamicResult: "allowedWithPermission",
-          expected: "allow",
+          expected: "ask",
         },
 
         // Dangerous commands (always blocked)
@@ -860,7 +860,7 @@ describe("Permission Checker", () => {
           permissions,
         );
 
-        expect(result.permission).toBe("allow");
+        expect(result.permission).toBe("ask");
         expect(mockEvaluateToolCallPolicy).toHaveBeenCalledWith(
           "allowedWithoutPermission",
           {},

extensions/cli/src/permissions/permissionChecker.ts

@@ -121,6 +121,37 @@ function permissionPolicyToToolPolicy(
   }
 }
 
+/**
+ * Converts core's ToolPolicy back to CLI's PermissionPolicy
+ */
+function toolPolicyToPermissionPolicy(policy: ToolPolicy): PermissionPolicy {
+  switch (policy) {
+    case "allowedWithoutPermission":
+      return "allow";
+    case "allowedWithPermission":
+      return "ask";
+    case "disabled":
+      return "exclude";
+    default:
+      return "ask";
+  }
+}
+
+/**
+ * Returns the most restrictive of two permission policies.
+ */
+function getMostRestrictivePermissionPolicy(
+  a: PermissionPolicy,
+  b: PermissionPolicy,
+): PermissionPolicy {
+  const order: Record<PermissionPolicy, number> = {
+    exclude: 3,
+    ask: 2,
+    allow: 1,
+  };
+  return order[a] >= order[b] ? a : b;
+}
+
 /**
  * Evaluates a tool call request against a set of permission policies.
  * Returns the permission for the first matching policy.
@@ -158,17 +189,26 @@ export function checkToolPermission(
       toolCall.arguments,
     );
 
-    // If dynamic evaluation says disabled, that ALWAYS takes precedence
-    if (evaluatedPolicy === "disabled") {
+    const validPolicies: ToolPolicy[] = [
+      "allowedWithoutPermission",
+      "allowedWithPermission",
+      "disabled",
+    ];
+
+    if (!validPolicies.includes(evaluatedPolicy)) {
       return {
-        permission: "exclude",
+        permission: basePermission,
         matchedPolicy,
       };
     }
 
-    // Otherwise, user preference wins - return the original base permission
+    // Dynamic evaluation can only restrict further, never relax
+    const evaluatedPermission = toolPolicyToPermissionPolicy(evaluatedPolicy);
     return {
-      permission: basePermission,
+      permission: getMostRestrictivePermissionPolicy(
+        basePermission,
+        evaluatedPermission,
+      ),
       matchedPolicy,
     };
   }

extensions/cli/src/subagent/executor.ts

@@ -75,19 +75,6 @@ export async function executeSubAgent(
       throw new Error("Model or LLM API not available");
     }
 
-    // allow all tools for now
-    // todo: eventually we want to show the same prompt in a dialog whether asking whether that tool call is allowed or not
-
-    serviceContainer.set<ToolPermissionServiceState>(
-      SERVICE_NAMES.TOOL_PERMISSIONS,
-      {
-        ...mainAgentPermissionsState,
-        permissions: {
-          policies: [{ tool: "*", permission: "allow" }],
-        },
-      },
-    );
-
     // Build agent system message
     const systemMessage = await buildAgentSystemMessage(subAgent, services);
 

extensions/cli/src/tools/writeFile.ts

@@ -1,6 +1,7 @@
 import * as fs from "fs";
 import * as path from "path";
 
+import { throwIfFileIsSecurityConcern } from "core/indexing/ignore.js";
 import { ContinueError, ContinueErrorReason } from "core/util/errors.js";
 import { createTwoFilesPatch } from "diff";
 
@@ -57,6 +58,7 @@ export const writeFileTool: Tool = {
     if (typeof content !== "string") {
       throw new Error("New file content must be a string");
     }
+    throwIfFileIsSecurityConcern(filepath);
     try {
       if (fs.existsSync(filepath)) {
         const oldContent = fs.readFileSync(filepath, "utf-8");
@@ -118,6 +120,7 @@ export const writeFileTool: Tool = {
     };
   },
   run: async (args: { filepath: string; content: string }): Promise<string> => {
+    throwIfFileIsSecurityConcern(args.filepath);
     try {
       const dirPath = path.dirname(args.filepath);
       if (!fs.existsSync(dirPath)) {