From 5906b5ab6ef15a8c3d4384244a017d524f84ac4e Mon Sep 17 00:00:00 2001
From: Jayden Liang <jaydenliang81@gmail.com>
Date: Wed, 28 Jan 2026 20:49:26 -0800
Subject: [PATCH] fix(browser-tool): disallow close without targetId to avoid
 unsafe tab ownership

The close action previously allowed falling back to the active tab when
no targetId was provided. This implicit behavior makes tab ownership
ambiguous and unsafe in the multi-tool cooperative (concurrent) scenarios.

Without an explicit targetId, a close action may unintentionally close
a tab not created or owned by the caller.

This change enforces an explicit targetId for closing tabs and fails fast
otherwise, making destructive browser actions deterministic and ownership-safe.
---
 src/agents/tools/browser-tool.test.ts | 32 +++++++++++++++++++++++++++
 src/agents/tools/browser-tool.ts      |  8 ++++++-
 2 files changed, 39 insertions(+), 1 deletion(-)

diff --git a/src/agents/tools/browser-tool.test.ts b/src/agents/tools/browser-tool.test.ts
index 7248a7a2f..f6d0f8e75 100644
--- a/src/agents/tools/browser-tool.test.ts
+++ b/src/agents/tools/browser-tool.test.ts
@@ -289,3 +289,35 @@ describe("browser tool snapshot labels", () => {
     expect(result?.content?.[1]).toMatchObject({ type: "image" });
   });
 });
+
+describe("browser tool tab open and close behaviors", () => {
+  afterEach(() => {
+    vi.clearAllMocks();
+    configMocks.loadConfig.mockReturnValue({ browser: {} });
+  });
+
+  it("close a tab by targetId should be allowed in profile = 'clawd'", async () => {
+    const tool = createBrowserTool({ sandboxBridgeUrl: "http://127.0.0.1:9999" });
+    await tool.execute?.("", { action: "close", targetId: "a real target id", profile: "clawd" });
+    expect(browserClientMocks.browserCloseTab).toHaveBeenCalledWith(
+      "http://127.0.0.1:9999",
+      "a real target id",
+      { profile: "clawd" },
+    );
+  });
+
+  it("close a tab by targetId should be allowed in profile = 'chrome'", async () => {
+    const tool = createBrowserTool({ sandboxBridgeUrl: "http://127.0.0.1:9999" });
+    await tool.execute?.("", { action: "close", targetId: "a real target id", profile: "clawd" });
+    expect(browserClientMocks.browserCloseTab).toHaveBeenCalledWith(
+      "http://127.0.0.1:9999",
+      "a real target id",
+      { profile: "clawd" },
+    );
+  });
+
+  it("close a tab without targetId should throw error", async () => {
+    const tool = createBrowserTool({ sandboxBridgeUrl: "http://127.0.0.1:9999" });
+    await expect(tool.execute?.("", { action: "close" })).rejects.toThrow();
+  });
+});
diff --git a/src/agents/tools/browser-tool.ts b/src/agents/tools/browser-tool.ts
index b28da2fc7..a127b8722 100644
--- a/src/agents/tools/browser-tool.ts
+++ b/src/agents/tools/browser-tool.ts
@@ -407,7 +407,13 @@ export function createBrowserTool(opts?: {
             return jsonResult(result);
           }
           if (targetId) await browserCloseTab(baseUrl, targetId, { profile });
-          else await browserAct(baseUrl, { kind: "close" }, { profile });
+          else {
+            // Known issue: The 'close' action does not verify the ownership of the current tab before atempting to close it, leading to potential action conflicts in concurrent scenarios. For now, enforce a strict close action with targetId for proxyRequest: path = "/act". The following line is commented out to prevent accidental usage. Instead, we throw an error to inform the user.
+            // await browserAct(baseUrl, { kind: "close" }, { profile });
+            throw new Error(
+              `Closing a tab without providing a targetId is not allowed. (But it was somehow allowed to do so in a previous version!). To avoid closing a tab not belonging to you, ensure you maintain a set of tabs by their targetId and provide a targetId when closing.`,
+            );
+          }
           return jsonResult({ ok: true });
         }
         case "snapshot": {