From ad2a12e57e9ea54ae467c941546100b751776786 Mon Sep 17 00:00:00 2001
From: Test User <test@example.com>
Date: Thu, 2 Jul 2026 21:02:54 +0800
Subject: [PATCH] fix(extension): don't fail-open the borrow gate on a
 transient tabs.get error
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

requestBorrowConfirmation returned `true` (approve the borrow) when
`chrome.tabs.get(tabId)` threw. That fail-open was not one of the
documented fail-open cases in the function's own doc comment (no
candidate window; abort signal; timeout), and the tab was already
fetched successfully moments earlier by the borrow pipeline
(validateBorrowTarget), so a failure here is a transient SW/page hiccup
— not a missing tab. Silently approving a borrow with no confirmation
UI shown defeats the entire human-in-the-loop gate for seizing a user's
logged-in tab.

On a tabs.get failure, fall back to a generic title (the numeric tab id)
and still run the overlay + OS-notification confirmation flow so the
user gets a real chance to approve or deny.

Adds two regression tests (both fail on the unpatched code). Full
extension suite green (361 tests).

Co-Authored-By: Claude <noreply@anthropic.com>
---
 .../__tests__/borrow-confirmation.test.ts     | 42 +++++++++++++++++++
 .../src/tools/borrow-confirmation.ts          | 18 +++++---
 2 files changed, 55 insertions(+), 5 deletions(-)

diff --git a/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts b/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts
index b168983..3025f40 100644
--- a/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts
+++ b/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts
@@ -510,6 +510,48 @@ describe("requestBorrowConfirmation", () => {
     expect(cancelMessage.type).toBe("borrow-cancel");
     expect(cancelMessage.requestId).toBe(requestMessage.requestId);
   });
+
+  it("does not fail-open when tabs.get rejects — still surfaces confirmation to a candidate", async () => {
+    // Regression: a transient tabs.get failure (SW teardown / page hiccup)
+    // used to return true immediately, approving the borrow with no
+    // confirmation UI. The tab was already fetched by the borrow pipeline
+    // moments earlier, so the failure is transient — we must fall back to a
+    // generic title and still ask the user.
+    tabs.get.mockRejectedValueOnce(new Error("tab info unavailable"));
+    windows.getLastFocused.mockResolvedValueOnce(
+      userWindowWithActiveTab({ windowId: 50, tabId: 7, url: "https://app.example/" }),
+    );
+    tabs.sendMessage.mockResolvedValueOnce({ type: "borrow-response", allowed: false });
+
+    const pending = requestBorrowConfirmation(42, {
+      deps: { tabs, windows, notifications },
+    });
+    await vi.runAllTimersAsync();
+    const allowed = await pending;
+
+    // The user's explicit deny must be honoured — NOT auto-allowed by the
+    // earlier tabs.get rejection.
+    expect(allowed).toBe(false);
+    expect(tabs.sendMessage).toHaveBeenCalledTimes(1);
+    expect(notifications.create).toHaveBeenCalledTimes(1);
+  });
+
+  it("still allows an explicit overlay allow after a tabs.get rejection", async () => {
+    tabs.get.mockRejectedValueOnce(new Error("tab info unavailable"));
+    windows.getLastFocused.mockResolvedValueOnce(
+      userWindowWithActiveTab({ windowId: 50, tabId: 7, url: "https://app.example/" }),
+    );
+    tabs.sendMessage.mockResolvedValueOnce({ type: "borrow-response", allowed: true });
+
+    const pending = requestBorrowConfirmation(42, {
+      deps: { tabs, windows, notifications },
+    });
+    await vi.runAllTimersAsync();
+    const allowed = await pending;
+
+    expect(allowed).toBe(true);
+    expect(tabs.sendMessage).toHaveBeenCalledTimes(1);
+  });
 });
 
 describe("isInjectableContentScriptUrl", () => {
diff --git a/apps/extension/src/tools/borrow-confirmation.ts b/apps/extension/src/tools/borrow-confirmation.ts
index 1e357a4..3270a20 100644
--- a/apps/extension/src/tools/borrow-confirmation.ts
+++ b/apps/extension/src/tools/borrow-confirmation.ts
@@ -383,17 +383,25 @@ export async function requestBorrowConfirmation(
   const isAgentWindowId = deps.isAgentWindowId ?? (() => false);
   const copy = deps.notificationCopy ?? DEFAULT_NOTIFICATION_COPY;
 
-  let tab: chrome.tabs.Tab;
+  // The borrow pipeline already fetched this tab moments earlier
+  // (validateBorrowTarget in tabs.ts), so a failure here is a transient
+  // SW/page hiccup, not a missing tab. Do NOT fail-open: silently allowing
+  // a borrow with no confirmation UI contradicts the gate's purpose and is
+  // not one of the documented fail-open cases (no candidate window; abort;
+  // timeout — see this function's doc comment). Fall back to a generic
+  // title and still surface the overlay + notification so the user gets a
+  // real chance to approve or deny.
+  let tabTitle: string;
   try {
-    tab = await tabsApi.get(tabId);
+    const tab = await tabsApi.get(tabId);
+    tabTitle = tab.title ?? tab.url ?? String(tabId);
   } catch (err) {
-    console.warn("[bsk borrow] could not get tab info — proceeding", {
+    console.warn("[bsk borrow] could not get tab info — using fallback title", {
       tabId,
       error: err instanceof Error ? err.message : String(err),
     });
-    return true;
+    tabTitle = String(tabId);
   }
-  const tabTitle = tab.title ?? tab.url ?? String(tabId);
 
   const candidates = await listConfirmationCandidates(windowsApi, isAgentWindowId);
   if (candidates.length === 0) {