Merge pull request #5 from hobostay/fix/borrow-confirmation-tabs-get-fail-open

fix(extension): don't fail-open the borrow gate on a transient tabs.get error

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", () => {

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) {