From 716df559ec2c5a1e803886d13b26ebd02e4d84da Mon Sep 17 00:00:00 2001 From: Bartok9 Date: Fri, 24 Jul 2026 03:33:58 -0400 Subject: [PATCH] fix(browser): gate non-http(s) URLs in browser.tabs new Mirror browser.navigate approval for tabs action=new so file://, javascript:, and other non-http(s) schemes cannot open a secondary tab without the operator gate. Closes the bypass surface verified on main. Signed-off-by: Bartok9 --- src/tools/browser/browser-tools.test.ts | 63 ++++++++++++++++++++++++- src/tools/browser/index.ts | 2 +- src/tools/browser/tabs.ts | 34 +++++++++++-- 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/src/tools/browser/browser-tools.test.ts b/src/tools/browser/browser-tools.test.ts index b042f31..b2499a6 100644 --- a/src/tools/browser/browser-tools.test.ts +++ b/src/tools/browser/browser-tools.test.ts @@ -326,12 +326,73 @@ describe("browser tools on a fake backend", () => { }); it("tabs list rendering includes the active marker", async () => { - const tool = buildBrowserTabsTool(backend); + const tool = buildBrowserTabsTool(backend, autoApprove()); const result = await tool.run({ action: "list" }, CTX); expect(result.status).toBe("ok"); expect(result.summary).toContain("*[0] Example"); }); + it("tabs new with http(s) URL does not require approval", async () => { + const gate = new ApprovalGate({ + emit: (req) => gate.reject(req.approvalId, "should not be called"), + }); + const tool = buildBrowserTabsTool(backend, { + approvals: gate, + approvalRequired: true, + }); + const result = await tool.run( + { action: "new", url: "https://example.com/docs" }, + CTX, + ); + expect(result.status).toBe("ok"); + expect(backend.lastTabs).toEqual({ + action: "new", + url: "https://example.com/docs", + }); + }); + + it("tabs new requires approval for non-http(s) schemes", async () => { + const gate = new ApprovalGate({ + emit: (req) => gate.reject(req.approvalId, "denied file scheme"), + }); + const tool = buildBrowserTabsTool(backend, { + approvals: gate, + approvalRequired: true, + }); + await expect( + tool.run({ action: "new", url: "file:///etc/passwd" }, CTX), + ).rejects.toMatchObject({ name: "ApprovalDeniedError" }); + expect(backend.lastTabs).toBeNull(); + expect(backend.readyCalls).toBe(0); + }); + + it("tabs new with javascript: URL is gated the same way", async () => { + const gate = new ApprovalGate({ + emit: (req) => gate.reject(req.approvalId, "denied js scheme"), + }); + const tool = buildBrowserTabsTool(backend, { + approvals: gate, + approvalRequired: true, + }); + await expect( + tool.run({ action: "new", url: "javascript:alert(1)" }, CTX), + ).rejects.toMatchObject({ name: "ApprovalDeniedError" }); + expect(backend.lastTabs).toBeNull(); + }); + + it("tabs list never hits the URL approval gate", async () => { + const gate = new ApprovalGate({ + emit: (req) => gate.reject(req.approvalId, "list must not ask"), + }); + const tool = buildBrowserTabsTool(backend, { + approvals: gate, + approvalRequired: true, + }); + const result = await tool.run({ action: "list" }, CTX); + expect(result.status).toBe("ok"); + expect(backend.lastTabs?.action).toBe("list"); + }); + it("buildBrowserTools registers all browser tools", () => { const registry = new ToolRegistry(); for (const tool of buildBrowserTools(backend, autoApprove())) { diff --git a/src/tools/browser/index.ts b/src/tools/browser/index.ts index c886455..03f8fcd 100644 --- a/src/tools/browser/index.ts +++ b/src/tools/browser/index.ts @@ -70,7 +70,7 @@ export function buildBrowserTools( buildBrowserTypeTool(backend), buildBrowserReadAriaTool(backend), buildBrowserSearchTool(backend), - buildBrowserTabsTool(backend), + buildBrowserTabsTool(backend, dangerous), buildBrowserScrollTool(backend), ]; } diff --git a/src/tools/browser/tabs.ts b/src/tools/browser/tabs.ts index d97807a..1713c0a 100644 --- a/src/tools/browser/tabs.ts +++ b/src/tools/browser/tabs.ts @@ -1,14 +1,22 @@ import { compressToolResult } from "../../compressor/result-compressor.js"; import type { ToolDefinition } from "../tool-registry.js"; import type { BrowserBackend, TabsInput } from "./browser-backend.js"; +import { isSafeUrl } from "./navigate.js"; +import { + requireApproval, + type DangerousToolOptions, +} from "../../approval/dangerous-tool.js"; -export function buildBrowserTabsTool(backend: BrowserBackend): ToolDefinition { +export function buildBrowserTabsTool( + backend: BrowserBackend, + options: DangerousToolOptions, +): ToolDefinition { return { name: "browser.tabs", description: - "Manage browser tabs: list, switch, close, or open a new one.", + "Manage browser tabs: list, switch, close, or open a new one. Opening a new tab with a non-http(s) URL requires approval (same gate as browser.navigate).", readonly: false, - async run(rawArgs) { + async run(rawArgs, ctx) { const action = rawArgs.action; if ( action !== "list" && @@ -24,9 +32,27 @@ export function buildBrowserTabsTool(backend: BrowserBackend): ToolDefinition { if (typeof rawArgs.index === "number" && Number.isInteger(rawArgs.index)) { input.index = rawArgs.index; } - if (typeof rawArgs.url === "string") { + if (typeof rawArgs.url === "string" && rawArgs.url.length > 0) { input.url = rawArgs.url; } + + // Mirror browser.navigate: tabs.new with a URL is the same danger surface + // (file://, javascript:, data:, chrome://). Without this gate the model + // can exfiltrate local files or execute script by opening a secondary tab. + if (action === "new" && input.url !== undefined && !isSafeUrl(input.url)) { + await requireApproval( + options, + { + sessionId: ctx.sessionId, + tool: "browser.tabs", + reason: "non-http(s) URL requested for new tab", + preview: input.url, + affectedResources: [input.url], + }, + ctx.signal, + ); + } + await backend.ensureReady(); const result = await backend.tabs(input); const rendered = result.tabs