figmar/BrowserSkill
1
0
Fork 0
mirror of https://github.com/Tencent/BrowserSkill.git synced 2026-09-10 13:01:23 +08:00
Code Issues Packages Projects Releases Wiki Activity

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

Browse Source 
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>
This commit is contained in:
Test User
2026-07-02 21:02:54 +08:00
co-authored by Claude
parent 9c229b2489
commit ad2a12e57e
2 changed files with 55 additions and 5 deletions
Download Patch File Download Diff File Expand all files Collapse all files
apps/extension/src/tools/__tests__/borrow-confirmation.test.ts
+42
View File
@@ -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", () => {