Merge pull request #6 from ConsultingFuture4200/fix/no-auto-approve-network-tools

fix(agent): stop network tools being granted a standing approval

Author: voidstackloop

frontend/src/lib/tool-approval.test.ts

@@ -0,0 +1,37 @@
+import { describe, it, expect } from "vitest";
+import { canAlwaysAllow } from "./tool-approval";
+
+describe("canAlwaysAllow", () => {
+    it("allows a standing grant for workspace-local reads", () => {
+        expect(canAlwaysAllow("read_file")).toBe(true);
+        expect(canAlwaysAllow("list_dir")).toBe(true);
+        expect(canAlwaysAllow("git_diff")).toBe(true);
+        expect(canAlwaysAllow("find_symbol_references")).toBe(true);
+    });
+
+    // A standing grant on these would let content the model reads steer an
+    // outbound request on every later turn with no prompt shown.
+    it("never allows a standing grant for tools that reach the network", () => {
+        expect(canAlwaysAllow("fetch_url")).toBe(false);
+        expect(canAlwaysAllow("web_search")).toBe(false);
+        expect(canAlwaysAllow("github_read_file")).toBe(false);
+        expect(canAlwaysAllow("github_list_repositories")).toBe(false);
+        expect(canAlwaysAllow("github_repository_tree")).toBe(false);
+    });
+
+    // Not read-only in either sense: it fetches a model-chosen URL and writes
+    // a PNG into the workspace.
+    it("never allows a standing grant for capture_page_screenshot", () => {
+        expect(canAlwaysAllow("capture_page_screenshot")).toBe(false);
+    });
+
+    it("never allows a standing grant for tools with side effects", () => {
+        for (const tool of ["write_file", "run_command", "run_code", "apply_patch", "delete_path", "git_commit", "http_request", "write_to_terminal"]) {
+            expect(canAlwaysAllow(tool)).toBe(false);
+        }
+    });
+
+    it("defaults to denying an unrecognised tool", () => {
+        expect(canAlwaysAllow("some_future_tool")).toBe(false);
+    });
+});

frontend/src/lib/tool-approval.ts

@@ -0,0 +1,40 @@
+// Which agent tools may be granted "always allow for this session".
+//
+// The bar for that grant is not "doesn't write to the workspace" — it's "the
+// user has nothing to lose by never being asked again". Those differ: a tool
+// can leave the workspace untouched and still reach the network or the disk.
+//
+// This matters because the model's inputs are not trusted. Agent mode reads
+// file contents and web pages, and instructions embedded in that content can
+// steer subsequent tool calls. The per-call approval prompt is what stops that
+// from becoming unattended action, so anything with a side effect the user
+// would want to see keeps its prompt. Notably that excludes:
+//
+//  - web_search, fetch_url and the github_* tools, which each send a request
+//    to a destination the model chooses — a standing grant on those is an
+//    unattended outbound channel for whatever the model has already read.
+//  - capture_page_screenshot, which fetches a model-chosen URL *and* writes a
+//    PNG into the workspace, so it is not read-only in either sense.
+//
+// Listing what qualifies, rather than what doesn't, keeps this default-deny: a
+// tool added to AGENT_TOOLS without being classified here needs per-call
+// approval until someone decides otherwise.
+export const AUTO_APPROVABLE_TOOLS = new Set([
+    "read_file",
+    "find_files",
+    "file_info",
+    "list_dir",
+    "search_files",
+    "git_status",
+    "git_diff",
+    "git_log",
+    "read_notes",
+    "get_background_output",
+    "list_background_commands",
+    "find_symbol_references",
+    "read_terminal_output",
+]);
+
+export function canAlwaysAllow(toolName: string): boolean {
+    return AUTO_APPROVABLE_TOOLS.has(toolName);
+}

frontend/src/pages/Chat.tsx

@@ -72,6 +72,7 @@ import { speakText, stopSpeaking } from "@/lib/tts";
 import { computeLineDiff } from "@/lib/diff";
 import { useToast } from "@/components/toast";
 import { isTransientError } from "@/lib/transient-errors";
+import { canAlwaysAllow } from "@/lib/tool-approval";
 import {
     COMPACTION_BUDGET_TOKENS,
     COMPACTION_KEEP_RECENT,
@@ -116,36 +117,12 @@ const RAG_THRESHOLD_CHARS = 20_000;
 // the most recent window renders by default, with older ones revealed a
 // window at a time on request rather than all at once.
 const RENDER_WINDOW_SIZE = 60;
-// Read-only tools are safe to let the model call repeatedly without a fresh
-// click each time — write_file and run_command always require explicit
-// per-call approval since they have real, potentially irreversible effects.
+
 interface PlanStep {
     text: string;
     done: boolean;
 }
 
-const READ_ONLY_TOOLS = new Set([
-    "read_file",
-    "find_files",
-    "file_info",
-    "list_dir",
-    "search_files",
-    "git_status",
-    "git_diff",
-    "git_log",
-    "web_search",
-    "fetch_url",
-    "read_notes",
-    "github_list_repositories",
-    "github_repository_tree",
-    "github_read_file",
-    "get_background_output",
-    "list_background_commands",
-    "capture_page_screenshot",
-    "find_symbol_references",
-    "read_terminal_output",
-]);
-
 // Vision models can already reason over any attached image — these just save
 // re-typing a good prompt for the common "I attached a diagram/wireframe"
 // case. Selecting one fills the composer; the user can still edit before sending.
@@ -2233,7 +2210,7 @@ export default function Chat() {
                                     >
                                         <X className="size-3.5" /> {t.deny}
                                     </Button>
-                                    {READ_ONLY_TOOLS.has(call.name) && (
+                                    {canAlwaysAllow(call.name) && (
                                         <Button
                                             size="sm"
                                             variant="ghost"