From ed6d14eab3892e9b20c1a1e4775d50c235b41a7e Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 22:01:46 +0800 Subject: [PATCH 1/6] fix(extension): fail closed on borrow confirmation --- .../src/content/BorrowConfirmationOverlay.tsx | 30 ++++----- .../BorrowConfirmationOverlay.test.tsx | 8 +-- .../__tests__/borrow-confirmation.test.ts | 38 ++++++++---- .../src/tools/borrow-confirmation.ts | 62 ++++++++----------- .../i18n/src/locales/en-US/extension.json | 2 +- .../i18n/src/locales/zh-CN/extension.json | 2 +- 6 files changed, 74 insertions(+), 68 deletions(-) diff --git a/apps/extension/src/content/BorrowConfirmationOverlay.tsx b/apps/extension/src/content/BorrowConfirmationOverlay.tsx index 09318341..9258505b 100644 --- a/apps/extension/src/content/BorrowConfirmationOverlay.tsx +++ b/apps/extension/src/content/BorrowConfirmationOverlay.tsx @@ -67,8 +67,8 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i const totalSeconds = Math.ceil(request.timeoutMs / 1000); const [secondsLeft, setSecondsLeft] = useState(totalSeconds); const [exiting, setExiting] = useState(false); - const [awaitingAutoAllow, setAwaitingAutoAllow] = useState(false); - const allowedRef = useRef(false); + const [awaitingAutoDeny, setAwaitingAutoDeny] = useState(false); + const settledRef = useRef(false); const onAllowRef = useRef(request.onAllow); const onDenyRef = useRef(request.onDeny); onAllowRef.current = request.onAllow; @@ -76,8 +76,8 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i useEffect(() => { if (secondsLeft <= 0) { - if (!allowedRef.current) { - setAwaitingAutoAllow(true); + if (!settledRef.current) { + setAwaitingAutoDeny(true); } return; } @@ -86,9 +86,9 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i }, [secondsLeft]); function triggerAllow() { - if (allowedRef.current) return; - allowedRef.current = true; - setAwaitingAutoAllow(false); + if (settledRef.current) return; + settledRef.current = true; + setAwaitingAutoDeny(false); setExiting(true); setTimeout(() => onAllowRef.current(), EXIT_ANIMATION_MS); } @@ -98,9 +98,9 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i } function handleDeny() { - if (allowedRef.current) return; - allowedRef.current = true; - setAwaitingAutoAllow(false); + if (settledRef.current) return; + settledRef.current = true; + setAwaitingAutoDeny(false); setExiting(true); setTimeout(() => onDenyRef.current(), EXIT_ANIMATION_MS); } @@ -128,13 +128,13 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i progress={progress} seconds={secondsLeft} onProgressTransitionEnd={(propertyName) => { - if (!awaitingAutoAllow || secondsLeft !== 0) return; + if (!awaitingAutoDeny || secondsLeft !== 0) return; if (propertyName !== "stroke-dashoffset") return; - triggerAllow(); + handleDeny(); }} /> - {t("borrowConfirmation.autoAllow", { count: secondsLeft })} + {t("borrowConfirmation.autoDeny", { count: secondsLeft })} @@ -156,9 +156,9 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i { - if (!awaitingAutoAllow || secondsLeft !== 0) return; + if (!awaitingAutoDeny || secondsLeft !== 0) return; if (event.propertyName !== "width") return; - triggerAllow(); + handleDeny(); }} /> diff --git a/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx b/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx index 5ac6c2e7..04be736d 100644 --- a/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx +++ b/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx @@ -12,7 +12,7 @@ describe("BorrowConfirmationOverlay", () => { vi.useRealTimers(); }); - it("auto-allows after countdown reaches zero and progress transition ends", async () => { + it("auto-denies after countdown reaches zero and progress transition ends", async () => { const onAllow = vi.fn(); const onDeny = vi.fn(); @@ -37,7 +37,7 @@ describe("BorrowConfirmationOverlay", () => { await vi.advanceTimersByTimeAsync(1000); }); } - expect(screen.getByText("0 秒后自动允许")).toBeTruthy(); + expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy(); await act(async () => { await vi.advanceTimersByTimeAsync(1000); }); @@ -54,8 +54,8 @@ describe("BorrowConfirmationOverlay", () => { await act(async () => { await vi.advanceTimersByTimeAsync(150); }); - expect(onAllow).toHaveBeenCalledTimes(1); - expect(onDeny).not.toHaveBeenCalled(); + expect(onDeny).toHaveBeenCalledTimes(1); + expect(onAllow).not.toHaveBeenCalled(); }); it("only invokes onDeny once when deny is clicked repeatedly", async () => { diff --git a/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts b/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts index 3025f409..5c4dbe6c 100644 --- a/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts +++ b/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts @@ -162,6 +162,22 @@ describe("requestBorrowConfirmation", () => { expect(await pending).toBe(false); }); + it("denies malformed or missing confirmation responses", async () => { + tabs.get.mockResolvedValue({ id: 42, title: "Tab" }); + windows.getLastFocused.mockResolvedValue( + userWindowWithActiveTab({ windowId: 11, tabId: 42, url: "https://app.example/" }), + ); + + for (const response of [undefined, {}, { allowed: true }, { type: "borrow-response" }]) { + tabs.sendMessage.mockResolvedValueOnce(response); + const pending = requestBorrowConfirmation(42, { + deps: { tabs, windows, notifications }, + }); + await vi.runAllTimersAsync(); + expect(await pending).toBe(false); + } + }); + it("skips the Agent Window when it is lastFocusedWindow and falls back to a real user window", async () => { tabs.get.mockResolvedValueOnce({ id: 42, @@ -250,7 +266,7 @@ describe("requestBorrowConfirmation", () => { expect(tabs.sendMessage.mock.calls[1]?.[0]).toBe(301); }); - it("does NOT fail-open immediately when every candidate's sendMessage fails — waits for notification button or timeout", async () => { + it("waits for a notification decision and denies when every UI path times out", async () => { const warnSpy = vi.spyOn(console, "warn").mockImplementation(() => undefined); tabs.get.mockResolvedValueOnce({ id: 42, title: "Tab" }); windows.getLastFocused.mockResolvedValueOnce( @@ -282,13 +298,13 @@ describe("requestBorrowConfirmation", () => { expect(tabs.sendMessage).toHaveBeenCalledTimes(2); // CRITICAL: previously this would already be settled(true). Now we - // require an explicit user choice (notification button) or timeout. + // require an explicit user choice (notification button) or safe timeout. expect(resolved).toBe(false); - // Allow the BACKGROUND_TIMEOUT_MS fail-open to fire so the test resolves. + // The background timeout must resolve the request without authorizing it. await vi.advanceTimersByTimeAsync(BACKGROUND_TIMEOUT_MS); - expect(await pending).toBe(true); - expect(resolvedValue).toBe(true); + expect(await pending).toBe(false); + expect(resolvedValue).toBe(false); const exhaustedWarn = warnSpy.mock.calls.find( (call) => @@ -375,7 +391,7 @@ describe("requestBorrowConfirmation", () => { expect(await pending).toBe(false); }); - it("fail-opens when no injectable user window exists at all", async () => { + it("denies when no injectable user window exists at all", async () => { tabs.get.mockResolvedValueOnce({ id: 42, title: "Stranded Tab" }); windows.getLastFocused.mockResolvedValueOnce( userWindowWithActiveTab({ windowId: 500, tabId: 5001, url: "about:blank" }), @@ -394,7 +410,7 @@ describe("requestBorrowConfirmation", () => { }); await vi.runAllTimersAsync(); - expect(await pending).toBe(true); + expect(await pending).toBe(false); expect(tabs.sendMessage).not.toHaveBeenCalled(); // No notification either — there's no actionable target window. expect(notifications.create).not.toHaveBeenCalled(); @@ -469,7 +485,7 @@ describe("requestBorrowConfirmation", () => { await pending; }); - it("fail-opens after background timeout when content script never responds", async () => { + it("denies after background timeout when content script never responds", async () => { tabs.get.mockResolvedValueOnce({ id: 42, title: "Tab" }); windows.getLastFocused.mockResolvedValueOnce( userWindowWithActiveTab({ windowId: 11, tabId: 42, url: "https://app.example/" }), @@ -480,10 +496,10 @@ describe("requestBorrowConfirmation", () => { deps: { tabs, windows, notifications }, }); await vi.advanceTimersByTimeAsync(BACKGROUND_TIMEOUT_MS); - expect(await pending).toBe(true); + expect(await pending).toBe(false); }); - it("dismisses pending overlay and fail-opens when aborted", async () => { + it("dismisses the pending overlay and denies when aborted", async () => { tabs.get.mockResolvedValueOnce({ id: 42, title: "Borrow Target" }); windows.getLastFocused.mockResolvedValueOnce( userWindowWithActiveTab({ windowId: 11, tabId: 7, url: "https://app.example/" }), @@ -500,7 +516,7 @@ describe("requestBorrowConfirmation", () => { controller.abort(); await vi.runAllTimersAsync(); - expect(await pending).toBe(true); + expect(await pending).toBe(false); expect(tabs.sendMessage).toHaveBeenCalledTimes(2); const requestMessage = tabs.sendMessage.mock.calls[0][1] as { requestId: string }; const cancelMessage = tabs.sendMessage.mock.calls[1][1] as { diff --git a/apps/extension/src/tools/borrow-confirmation.ts b/apps/extension/src/tools/borrow-confirmation.ts index 3270a203..bd72f747 100644 --- a/apps/extension/src/tools/borrow-confirmation.ts +++ b/apps/extension/src/tools/borrow-confirmation.ts @@ -114,9 +114,8 @@ const pendingBorrowNotifications = new Map(); * * This is the *fallback* authorization path that fires when no candidate * user window could host the in-page overlay (every `chrome.tabs.sendMessage` - * was rejected because the content script was not present). Without it the - * background-task fail-open at the end of `tryCandidate` would silently - * allow the borrow without any user-visible authorization step. + * was rejected because the content script was not present). It preserves an + * explicit authorization path without weakening the fail-closed timeout. */ const pendingBorrowDecisions = new Map void>(); @@ -170,7 +169,7 @@ export function attachBorrowNotificationClickHandler(deps: { * Allow / Deny buttons on the OS notification can resolve the matching * pending `requestBorrowConfirmation`. This is the explicit-authorization * fallback path used when every candidate user window's content script was - * missing — without it we'd be back to silently fail-opening the borrow. + * missing. * * Button index convention (matches `BorrowNotificationCopy`): * - 0 → Allow → `resolve(true)` @@ -326,8 +325,7 @@ async function listConfirmationCandidates( if (typeof w.id !== "number") continue; // Never bounce the overlay back into a session's Agent Window — those // pages can't host a content script when they're on the about:blank - // bootstrap URL, so sendMessage would fail-open and silently allow the - // borrow with no UI shown. + // bootstrap URL, so they cannot surface an authorization decision. if (isAgentWindowId(w.id)) continue; const activeTab = w.tabs?.find((t) => t.active === true) ?? null; if (!activeTab || typeof activeTab.id !== "number") continue; @@ -360,16 +358,14 @@ async function listConfirmationCandidates( * *explicit-authorization fallback* when every candidate fails sendMessage: * we no longer silently allow the borrow in that case, the promise stays * pending until the user clicks a notification button (resolves true/false) - * or `BACKGROUND_TIMEOUT_MS` fires (last-resort fail-open so the agent - * doesn't hang forever when even the notification path is broken). + * or `BACKGROUND_TIMEOUT_MS` fires and safely denies the request. * * Clicking the notification body (vs. its buttons) focuses the chosen user * window so the overlay becomes visible — the previous behaviour. * - * Returns `true` on user-allow (overlay or notification) OR last-resort - * fail-open (no candidate window at all; abort signal; timeout). Returns - * `false` only on explicit user-deny (overlay button or notification Deny - * button). + * Returns `true` only on an explicit user allow from the overlay or + * notification. Missing UI, aborts, malformed responses, and timeouts all + * fail closed and return `false`. */ export async function requestBorrowConfirmation( tabId: number, @@ -385,12 +381,9 @@ export async function requestBorrowConfirmation( // 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. + // SW/page hiccup, not a missing tab. 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 { const tab = await tabsApi.get(tabId); @@ -405,10 +398,10 @@ export async function requestBorrowConfirmation( const candidates = await listConfirmationCandidates(windowsApi, isAgentWindowId); if (candidates.length === 0) { - console.warn("[bsk borrow] no injectable user window available — proceeding without overlay", { + console.warn("[bsk borrow] no injectable user window available — denying borrow", { tabId, }); - return true; + return false; } const requestId = createBorrowRequestId(tabId); @@ -419,7 +412,7 @@ export async function requestBorrowConfirmation( // overlay immediately. const notificationAnchor = candidates[0]; if (!notificationAnchor) { - return true; + return false; } return new Promise((resolve) => { @@ -463,7 +456,7 @@ export async function requestBorrowConfirmation( }); }); } - settle(true); + settle(false); }; if (signal?.aborted) { @@ -473,19 +466,17 @@ export async function requestBorrowConfirmation( signal?.addEventListener("abort", onAbort, { once: true }); timeout = setTimeout(() => { - console.info("[bsk borrow] confirmation timed out — proceeding", { + console.info("[bsk borrow] confirmation timed out — denying borrow", { tabId, messageTabId: activeMessageTabId, }); - settle(true); + settle(false); }, BACKGROUND_TIMEOUT_MS); // Surface the OS notification *before* messaging any candidate so the // user has a parallel signal even if every content script is missing. // The Allow / Deny buttons let the user authorize explicitly when the - // in-page overlay never reached them — without those buttons we'd have - // to fail-open after `tryCandidate` exhausts its candidates, which is - // exactly the silent-allow bug we're fixing here. + // in-page overlay never reached them. if (notificationsApi) { pendingBorrowNotifications.set(notificationId, { windowId: notificationAnchor.windowId, @@ -499,7 +490,7 @@ export async function requestBorrowConfirmation( title: copy.title, message: copy.body(tabTitle), priority: 2, - requireInteraction: false, + requireInteraction: true, silent: false, buttons: [{ title: copy.allowButton }, { title: copy.denyButton }], }) @@ -523,15 +514,12 @@ export async function requestBorrowConfirmation( ): void => { if (settled) return; if (index >= candidates.length) { - // Every candidate rejected sendMessage. We deliberately do NOT - // fail-open here anymore: silently allowing a borrow with zero - // user-visible authorization UI is the bug. Instead we keep the - // promise pending and rely on: + // Every candidate rejected sendMessage. Keep the promise pending and + // rely on: // • the OS notification's Allow / Deny buttons for an explicit // user choice (preferred), or - // • the BACKGROUND_TIMEOUT_MS fail-open as the last-resort - // soft-fail so the agent doesn't hang forever when even the - // notification API is unavailable. + // • the BACKGROUND_TIMEOUT_MS fail-closed result so the agent does + // not hang forever when even the notification API is unavailable. // Keep the diagnostics so real-world frequency is still observable. console.warn( "[bsk borrow] every candidate user window failed sendMessage — awaiting notification button or timeout", @@ -560,7 +548,9 @@ export async function requestBorrowConfirmation( .sendMessage(candidate.tabId, message) .then((response) => { if (settled) return; - const allowed = (response as BorrowResponseMessage | undefined)?.allowed !== false; + const candidateResponse = response as BorrowResponseMessage | undefined; + const allowed = + candidateResponse?.type === "borrow-response" && candidateResponse.allowed === true; console.info("[bsk borrow] confirmation response", { tabId, allowed }); settle(allowed); }) diff --git a/packages/i18n/src/locales/en-US/extension.json b/packages/i18n/src/locales/en-US/extension.json index afe3810e..aa275ae7 100644 --- a/packages/i18n/src/locales/en-US/extension.json +++ b/packages/i18n/src/locales/en-US/extension.json @@ -52,7 +52,7 @@ "titleInactive": "Agent wants to borrow a tab", "body": "An AI agent wants to move this tab into its workspace. You can deny or allow the borrow.", "targetTab": "Target tab:", - "autoAllow": "Auto-allowing in {{count}}s", + "autoDeny": "Request expires in {{count}}s", "deny": "Deny", "allow": "Allow", "notificationTitle": "BrowserSkill: Agent wants to borrow a tab", diff --git a/packages/i18n/src/locales/zh-CN/extension.json b/packages/i18n/src/locales/zh-CN/extension.json index a4cefad5..b9e6415d 100644 --- a/packages/i18n/src/locales/zh-CN/extension.json +++ b/packages/i18n/src/locales/zh-CN/extension.json @@ -52,7 +52,7 @@ "titleInactive": "Agent 请求借用标签页", "body": "AI Agent 希望将此标签页移入工作区。您可以拒绝或允许借用。", "targetTab": "目标标签页:", - "autoAllow": "{{count}} 秒后自动允许", + "autoDeny": "{{count}} 秒后自动拒绝", "deny": "拒绝", "allow": "允许", "notificationTitle": "BrowserSkill:Agent 请求借用标签页", From 3c7a22e7c7d81476d1af1901781950768da37af8 Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 22:40:16 +0800 Subject: [PATCH 2/6] chore(ci): match Biome 2.4 formatting --- apps/extension/vitest.config.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/apps/extension/vitest.config.ts b/apps/extension/vitest.config.ts index 2e1c157e..00060bf3 100644 --- a/apps/extension/vitest.config.ts +++ b/apps/extension/vitest.config.ts @@ -21,9 +21,7 @@ export default defineConfig({ }, define: { __BSK_EXT_VERSION__: JSON.stringify(pkg.version), - __BSK_DAEMON_WS_URL__: JSON.stringify( - process.env.BSK_DAEMON_WS_URL ?? "ws://127.0.0.1:52800", - ), + __BSK_DAEMON_WS_URL__: JSON.stringify(process.env.BSK_DAEMON_WS_URL ?? "ws://127.0.0.1:52800"), }, resolve: { alias: { From d4d4c3beeb85403b8010acea13929b43363f5a26 Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 22:50:01 +0800 Subject: [PATCH 3/6] ci: retry stalled Rust job From f8907e5f5ab27bedd4cda31993f987447fb5a46b Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 22:58:57 +0800 Subject: [PATCH 4/6] ci: retry stalled Rust job From 9f82d897885d5ebe83328a8fb5cfc822a633ad38 Mon Sep 17 00:00:00 2001 From: NianJiuZst <3235467914@qq.com> Date: Fri, 10 Jul 2026 23:06:02 +0800 Subject: [PATCH 5/6] test: bind daemon ports without probe races --- crates/bsk-cli/tests/browser_list_ordering.rs | 5 +---- crates/bsk-cli/tests/browser_wait.rs | 5 +---- crates/bsk-cli/tests/cancel_forwarding.rs | 5 +---- crates/bsk-cli/tests/handshake_compat.rs | 5 +---- crates/bsk-cli/tests/per_session_queue.rs | 5 +---- crates/bsk-cli/tests/session_user_interrupt.rs | 5 +---- crates/bsk-cli/tests/sessions_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_m7_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_m8_ipc.rs | 5 +---- crates/bsk-cli/tests/tools_m9_ipc.rs | 5 +---- crates/bsk-cli/tests/ws_handshake.rs | 5 +---- 12 files changed, 12 insertions(+), 48 deletions(-) diff --git a/crates/bsk-cli/tests/browser_list_ordering.rs b/crates/bsk-cli/tests/browser_list_ordering.rs index 1c5199b9..f19f320f 100644 --- a/crates/bsk-cli/tests/browser_list_ordering.rs +++ b/crates/bsk-cli/tests/browser_list_ordering.rs @@ -16,7 +16,6 @@ use bsk_protocol::system::BrowserStatusEntry; use bsk_protocol::{ErrorCode, Method}; use rand::Rng; use serde::Deserialize; -use tokio::net::TcpListener; use tokio::sync::mpsc; fn tempfile_path(prefix: &str) -> PathBuf { @@ -30,9 +29,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-browser-list"); diff --git a/crates/bsk-cli/tests/browser_wait.rs b/crates/bsk-cli/tests/browser_wait.rs index 632a7e8c..16513e0f 100644 --- a/crates/bsk-cli/tests/browser_wait.rs +++ b/crates/bsk-cli/tests/browser_wait.rs @@ -15,7 +15,6 @@ use bsk_protocol::system::{BrowserListParams, HandshakeParams, HandshakeResult, use bsk_protocol::{BrowserPeerInfo, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -77,9 +76,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-browser-wait"); diff --git a/crates/bsk-cli/tests/cancel_forwarding.rs b/crates/bsk-cli/tests/cancel_forwarding.rs index b9913caa..e8a0aafb 100644 --- a/crates/bsk-cli/tests/cancel_forwarding.rs +++ b/crates/bsk-cli/tests/cancel_forwarding.rs @@ -20,7 +20,6 @@ use bsk_protocol::{ use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::json; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -40,9 +39,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-cancel"); diff --git a/crates/bsk-cli/tests/handshake_compat.rs b/crates/bsk-cli/tests/handshake_compat.rs index 07e14e4d..596d35e7 100644 --- a/crates/bsk-cli/tests/handshake_compat.rs +++ b/crates/bsk-cli/tests/handshake_compat.rs @@ -10,7 +10,6 @@ use bsk_protocol::system::{HandshakeParams, HandshakeResult, StatusResult}; use bsk_protocol::{BrowserPeerInfo, ErrorCode, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -28,9 +27,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-handshake"); diff --git a/crates/bsk-cli/tests/per_session_queue.rs b/crates/bsk-cli/tests/per_session_queue.rs index f548ee62..79b53559 100644 --- a/crates/bsk-cli/tests/per_session_queue.rs +++ b/crates/bsk-cli/tests/per_session_queue.rs @@ -27,7 +27,6 @@ use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde::Deserialize; use serde_json::json; -use tokio::net::TcpListener; use tokio::sync::{Mutex, mpsc}; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -46,9 +45,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-queue"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/session_user_interrupt.rs b/crates/bsk-cli/tests/session_user_interrupt.rs index 2960814f..b7d6e61f 100644 --- a/crates/bsk-cli/tests/session_user_interrupt.rs +++ b/crates/bsk-cli/tests/session_user_interrupt.rs @@ -26,7 +26,6 @@ use bsk_protocol::{ use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::json; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -46,9 +45,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); let sock = tempfile_path("bsk-test-user-interrupt"); diff --git a/crates/bsk-cli/tests/sessions_ipc.rs b/crates/bsk-cli/tests/sessions_ipc.rs index a6865203..99b15352 100644 --- a/crates/bsk-cli/tests/sessions_ipc.rs +++ b/crates/bsk-cli/tests/sessions_ipc.rs @@ -15,7 +15,6 @@ use bsk_protocol::tools::{SessionStartParams, SessionStartResult, SessionStopPar use bsk_protocol::{BrowserPeerInfo, Frame, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -39,9 +38,7 @@ async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { } async fn spawn_daemon_with_connect_wait(connect_wait: Duration) -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port).with_extension_connect_wait(connect_wait); let sock = tempfile_path("bsk-test-ipc"); diff --git a/crates/bsk-cli/tests/tools_ipc.rs b/crates/bsk-cli/tests/tools_ipc.rs index 32b77326..2857c62f 100644 --- a/crates/bsk-cli/tests/tools_ipc.rs +++ b/crates/bsk-cli/tests/tools_ipc.rs @@ -21,7 +21,6 @@ use futures_util::stream::{SplitSink, SplitStream}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::{Value, json}; -use tokio::net::TcpListener; use tokio::sync::Mutex; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -40,9 +39,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/tools_m7_ipc.rs b/crates/bsk-cli/tests/tools_m7_ipc.rs index acb94818..ee74b5e5 100644 --- a/crates/bsk-cli/tests/tools_m7_ipc.rs +++ b/crates/bsk-cli/tests/tools_m7_ipc.rs @@ -25,7 +25,6 @@ use futures_util::stream::{SplitSink, SplitStream}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::{Value, json}; -use tokio::net::TcpListener; use tokio::sync::Mutex; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -44,9 +43,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools-m7"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/tools_m8_ipc.rs b/crates/bsk-cli/tests/tools_m8_ipc.rs index 5308f7bc..14e7c72a 100644 --- a/crates/bsk-cli/tests/tools_m8_ipc.rs +++ b/crates/bsk-cli/tests/tools_m8_ipc.rs @@ -24,7 +24,6 @@ use futures_util::stream::{SplitSink, SplitStream}; use futures_util::{SinkExt, StreamExt}; use rand::Rng; use serde_json::Value; -use tokio::net::TcpListener; use tokio::sync::Mutex; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; @@ -43,9 +42,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools-m8"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/tools_m9_ipc.rs b/crates/bsk-cli/tests/tools_m9_ipc.rs index 6dd5d33d..2f8b46d9 100644 --- a/crates/bsk-cli/tests/tools_m9_ipc.rs +++ b/crates/bsk-cli/tests/tools_m9_ipc.rs @@ -30,7 +30,6 @@ use rand::Rng; use serde_json::{Value, json}; #[cfg(unix)] use tokio::io::{AsyncBufReadExt, AsyncWriteExt, BufReader}; -use tokio::net::TcpListener; #[cfg(unix)] use tokio::net::UnixStream; use tokio::sync::Mutex; @@ -53,9 +52,7 @@ fn tempfile_path(prefix: &str) -> PathBuf { } async fn spawn_daemon() -> (daemon::DaemonHandle, PathBuf) { - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let sock = tempfile_path("bsk-test-tools-m9"); let handle = daemon::run(DaemonConfig::new(port), Some(sock.clone())) .await diff --git a/crates/bsk-cli/tests/ws_handshake.rs b/crates/bsk-cli/tests/ws_handshake.rs index 146ef339..c6e2da14 100644 --- a/crates/bsk-cli/tests/ws_handshake.rs +++ b/crates/bsk-cli/tests/ws_handshake.rs @@ -8,7 +8,6 @@ use bsk::daemon::{self, DaemonConfig}; use bsk_protocol::system::{HandshakeParams, HandshakeResult}; use bsk_protocol::{BrowserPeerInfo, Method, RequestFrame, ResponseBody, ResponseFrame}; use futures_util::{SinkExt, StreamExt}; -use tokio::net::TcpListener; use tokio_tungstenite::tungstenite::handshake::client::generate_key; use tokio_tungstenite::tungstenite::http::Request; use tokio_tungstenite::tungstenite::protocol::Message; @@ -17,9 +16,7 @@ pub const TEST_EXT_ID: &str = "abcdefghijklmnopabcdefghijklmnop"; // 32 chars in pub async fn spawn_daemon() -> daemon::DaemonHandle { // Bind to any free TCP port. - let probe = TcpListener::bind("127.0.0.1:0").await.unwrap(); - let port = probe.local_addr().unwrap().port(); - drop(probe); + let port = 0; let config = DaemonConfig::new(port); daemon::run(config, None).await.unwrap() From 11ec51dfb853e4bc16da7c1c1f9a677962fe0956 Mon Sep 17 00:00:00 2001 From: BB-fat <1056871944@qq.com> Date: Sat, 18 Jul 2026 22:31:27 +0800 Subject: [PATCH 6/6] fix(extension): dismiss overlay on borrow timeout Send borrow-cancel when the confirmation timeout denies, and add a timer fallback so auto-deny does not depend only on CSS transitionend. Co-authored-by: Cursor --- .../src/content/BorrowConfirmationOverlay.tsx | 35 +++++++++++------ .../BorrowConfirmationOverlay.test.tsx | 39 +++++++++++++++++-- apps/extension/src/entrypoints/background.ts | 7 ++-- .../__tests__/borrow-confirmation.test.ts | 13 ++++++- .../src/tools/borrow-confirmation.ts | 35 ++++++++++------- 5 files changed, 95 insertions(+), 34 deletions(-) diff --git a/apps/extension/src/content/BorrowConfirmationOverlay.tsx b/apps/extension/src/content/BorrowConfirmationOverlay.tsx index 9258505b..6bb915fc 100644 --- a/apps/extension/src/content/BorrowConfirmationOverlay.tsx +++ b/apps/extension/src/content/BorrowConfirmationOverlay.tsx @@ -62,6 +62,9 @@ function BorrowToastStack({ children }: { children: React.ReactNode }) { ); } +/** Matches BorrowProgressBar / CountdownRing transition duration (duration-1000). */ +const PROGRESS_TRANSITION_MS = 1000; + function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; isModal: boolean }) { const { t } = useTranslation("extension"); const totalSeconds = Math.ceil(request.timeoutMs / 1000); @@ -74,17 +77,6 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i onAllowRef.current = request.onAllow; onDenyRef.current = request.onDeny; - useEffect(() => { - if (secondsLeft <= 0) { - if (!settledRef.current) { - setAwaitingAutoDeny(true); - } - return; - } - const id = setTimeout(() => setSecondsLeft((s) => s - 1), 1000); - return () => clearTimeout(id); - }, [secondsLeft]); - function triggerAllow() { if (settledRef.current) return; settledRef.current = true; @@ -105,6 +97,27 @@ function BorrowRequestItem({ request, isModal }: { request: BorrowRequestData; i setTimeout(() => onDenyRef.current(), EXIT_ANIMATION_MS); } + useEffect(() => { + if (secondsLeft <= 0) { + if (!settledRef.current) { + setAwaitingAutoDeny(true); + } + return; + } + const id = setTimeout(() => setSecondsLeft((s) => s - 1), 1000); + return () => clearTimeout(id); + }, [secondsLeft]); + + // Prefer transitionend for visual sync, but fall back to a timer so + // background tabs / suppressed CSS transitions still auto-deny. + useEffect(() => { + if (!awaitingAutoDeny || settledRef.current) return; + const id = setTimeout(() => { + handleDeny(); + }, PROGRESS_TRANSITION_MS); + return () => clearTimeout(id); + }, [awaitingAutoDeny]); + const progress = secondsLeft / totalSeconds; const truncatedTitle = request.tabTitle.length > 30 ? `${request.tabTitle.slice(0, 30)}…` : request.tabTitle; diff --git a/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx b/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx index 04be736d..d273ef89 100644 --- a/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx +++ b/apps/extension/src/content/__tests__/BorrowConfirmationOverlay.test.tsx @@ -38,9 +38,6 @@ describe("BorrowConfirmationOverlay", () => { }); } expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy(); - await act(async () => { - await vi.advanceTimersByTimeAsync(1000); - }); const progressCircle = container.querySelectorAll("svg circle")[1]; expect(progressCircle).toBeTruthy(); await act(() => { @@ -58,6 +55,42 @@ describe("BorrowConfirmationOverlay", () => { expect(onAllow).not.toHaveBeenCalled(); }); + it("auto-denies via timer fallback when progress transitionend never fires", async () => { + const onAllow = vi.fn(); + const onDeny = vi.fn(); + + render( + , + ); + + for (let i = 0; i < 5; i++) { + await act(async () => { + await vi.advanceTimersByTimeAsync(1000); + }); + } + expect(screen.getByText("0 秒后自动拒绝")).toBeTruthy(); + // No transitionend — the PROGRESS_TRANSITION_MS fallback must still deny. + await act(async () => { + await vi.advanceTimersByTimeAsync(1000); + }); + await act(async () => { + await vi.advanceTimersByTimeAsync(150); + }); + expect(onDeny).toHaveBeenCalledTimes(1); + expect(onAllow).not.toHaveBeenCalled(); + }); + it("only invokes onDeny once when deny is clicked repeatedly", async () => { const onAllow = vi.fn(); const onDeny = vi.fn(); diff --git a/apps/extension/src/entrypoints/background.ts b/apps/extension/src/entrypoints/background.ts index 91c73e37..c1bafb56 100644 --- a/apps/extension/src/entrypoints/background.ts +++ b/apps/extension/src/entrypoints/background.ts @@ -54,8 +54,7 @@ export default defineBackground(() => { deps: { // Skip every Agent Window when choosing where to render the // overlay — Agent Windows boot on about:blank, which has no - // content script, so sendMessage would fail-open and silently - // allow the borrow without any UI shown. + // content script, so they cannot surface an authorization decision. isAgentWindowId: (windowId) => sessions.findByWindowId(windowId) !== null, // Resolve i18n strings per-borrow so language switches take effect // without re-creating the dispatcher. @@ -82,8 +81,8 @@ export default defineBackground(() => { // The Allow / Deny buttons on the OS notification are the *explicit* // authorization fallback when every candidate user window's content // script was missing (extension just reloaded, page in BFCache, etc.). - // Without this listener those button clicks would land nowhere and we'd - // be back to fail-open-after-sendMessage-failure. + // Without this listener those button clicks would land nowhere and the + // request would only resolve via the fail-closed background timeout. if (typeof chrome.notifications?.onButtonClicked?.addListener === "function") { attachBorrowNotificationButtonHandler({ onButtonClicked: chrome.notifications.onButtonClicked, diff --git a/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts b/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts index 5c4dbe6c..6ecfd239 100644 --- a/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts +++ b/apps/extension/src/tools/__tests__/borrow-confirmation.test.ts @@ -437,6 +437,7 @@ describe("requestBorrowConfirmation", () => { expect(notificationId.startsWith(BORROW_NOTIFICATION_PREFIX)).toBe(true); expect(options.type).toBe("basic"); expect(options.title).toMatch(/borrow/i); + expect(options.requireInteraction).toBe(true); expect(notifications.clear).toHaveBeenCalledWith(notificationId); }); @@ -463,10 +464,12 @@ describe("requestBorrowConfirmation", () => { userWindowWithActiveTab({ windowId: 77, tabId: 4242, url: "https://docs.example/" }), ); // Keep sendMessage pending so the request stays live while we click. + // A second call may arrive if the background timeout dismisses the overlay. let resolveSend: (v: { type: string; allowed: boolean }) => void = () => undefined; tabs.sendMessage.mockImplementationOnce( () => new Promise((res) => (resolveSend = res as never)), ); + tabs.sendMessage.mockResolvedValue(undefined); const pending = requestBorrowConfirmation(42, { deps: { tabs, windows, notifications }, @@ -476,7 +479,7 @@ describe("requestBorrowConfirmation", () => { // Simulate user clicking the OS notification. for (const l of listeners) l(notificationId); - await vi.runAllTimersAsync(); + await Promise.resolve(); expect(windows.update).toHaveBeenCalledWith(77, { focused: true }); // Finish the borrow so the test resolves cleanly. @@ -491,12 +494,20 @@ describe("requestBorrowConfirmation", () => { userWindowWithActiveTab({ windowId: 11, tabId: 42, url: "https://app.example/" }), ); tabs.sendMessage.mockImplementationOnce(() => new Promise(() => undefined)); + tabs.sendMessage.mockResolvedValueOnce(undefined); const pending = requestBorrowConfirmation(42, { deps: { tabs, windows, notifications }, }); await vi.advanceTimersByTimeAsync(BACKGROUND_TIMEOUT_MS); expect(await pending).toBe(false); + // Timeout must dismiss any in-flight overlay so Allow cannot linger. + expect(tabs.sendMessage).toHaveBeenCalledTimes(2); + const cancelMessage = tabs.sendMessage.mock.calls[1][1] as { + type: string; + requestId: string; + }; + expect(cancelMessage.type).toBe("borrow-cancel"); }); it("dismisses the pending overlay and denies when aborted", async () => { diff --git a/apps/extension/src/tools/borrow-confirmation.ts b/apps/extension/src/tools/borrow-confirmation.ts index bd72f747..060f9354 100644 --- a/apps/extension/src/tools/borrow-confirmation.ts +++ b/apps/extension/src/tools/borrow-confirmation.ts @@ -28,8 +28,9 @@ export const CONFIRMATION_TIMEOUT_MS = 5000; const EXIT_ANIMATION_MS = 150; /** Matches BorrowConfirmationOverlay progress ring/bar transition (duration-1000). */ export const PROGRESS_TRANSITION_MS = 1000; -// UI auto-allows after countdown + progress transition + exit fade. Background -// timeout must not fire before that chain completes or deny is ignored. +// UI auto-denies after countdown + progress transition + exit fade. Background +// timeout must not fire before that chain completes or the UI decision races +// with an already-settled deny. export const BACKGROUND_TIMEOUT_MS = CONFIRMATION_TIMEOUT_MS + PROGRESS_TRANSITION_MS + EXIT_ANIMATION_MS + 500; @@ -441,21 +442,24 @@ export async function requestBorrowConfirmation( resolve(allowed); }; - const onAbort = () => { - if (activeMessageTabId !== null) { - const cancelMessage: BorrowCancelMessage = { - type: "borrow-cancel", + const dismissPendingOverlay = () => { + if (activeMessageTabId === null) return; + const cancelMessage: BorrowCancelMessage = { + type: "borrow-cancel", + requestId, + }; + void Promise.resolve(tabsApi.sendMessage(activeMessageTabId, cancelMessage)).catch((err) => { + console.debug("[bsk borrow] cancel message failed", { + tabId, + messageTabId: activeMessageTabId, requestId, - }; - void tabsApi.sendMessage(activeMessageTabId, cancelMessage).catch((err) => { - console.debug("[bsk borrow] cancel message failed", { - tabId, - messageTabId: activeMessageTabId, - requestId, - error: err instanceof Error ? err.message : String(err), - }); + error: err instanceof Error ? err.message : String(err), }); - } + }); + }; + + const onAbort = () => { + dismissPendingOverlay(); settle(false); }; @@ -470,6 +474,7 @@ export async function requestBorrowConfirmation( tabId, messageTabId: activeMessageTabId, }); + dismissPendingOverlay(); settle(false); }, BACKGROUND_TIMEOUT_MS);