Skip to content

fix(extension): fail closed on borrow confirmation#16

Open
NianJiuZst wants to merge 5 commits into
Tencent:mainfrom
NianJiuZst:codex/fail-closed-tab-borrow
Open

fix(extension): fail closed on borrow confirmation#16
NianJiuZst wants to merge 5 commits into
Tencent:mainfrom
NianJiuZst:codex/fail-closed-tab-borrow

Conversation

@NianJiuZst

@NianJiuZst NianJiuZst commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What problem this solves

The tab-borrow flow was presented as an explicit per-tab authorization gate, but several fallback paths treated the absence of a denial as approval. The in-page overlay automatically allowed a request when its countdown completed. The background coordinator also returned true when no injectable user window existed, when the content script never replied before the timeout, and when a reply was missing or malformed.

As a result, a user who did nothing could still have a pre-existing tab moved into the Agent Window. The weakest runtime path was also the most privileged outcome: failures in the confirmation UI, messaging, or notification delivery silently authorized the borrow instead of preserving the user's current browser state.

This contradicted the extension's privacy contract and made it impossible to distinguish an intentional Allow click from a timeout or broken UI path. Existing tests explicitly asserted the old fail-open outcomes, so this needed a behavior change rather than only additional coverage.

How this fixes it

The authorization decision is now fail-closed end to end:

  • the overlay countdown expires by invoking Deny rather than Allow;
  • no eligible confirmation window returns false immediately;
  • aborting a pending request dismisses the overlay and returns false;
  • the background safety timeout returns false;
  • only a well-formed { type: "borrow-response", allowed: true } reply grants access;
  • missing, partial, or unexpected responses are denied;
  • the OS notification is marked requireInteraction so Chrome is less likely to hide the explicit fallback before the user acts.

The notification Allow button remains a valid explicit authorization path when content-script messaging fails. The timeout still guarantees that the agent call eventually completes, but completion no longer changes browser state without consent.

The English and Chinese countdown text now states that the request expires or is automatically denied, matching the actual result.

User impact

Borrowing continues to work when the user deliberately clicks Allow in either the in-page overlay or the Chrome notification. Clicking Deny, ignoring the request, losing the content-script connection, closing the confirmation surface, or hitting a restricted page all leave the tab where it was.

Automation that previously relied on passive timeout approval will now receive the existing cancelled result and must ask the user to approve the borrow. This is intentional: unattended automation can create or use Agent Window tabs without weakening the authorization boundary around pre-existing user tabs.

Validation

  • corepack pnpm --filter @browser-skill/extension test
    • 35 test files passed
    • 374 tests passed
  • Added regression coverage for:
    • overlay timeout auto-denial;
    • missing confirmation candidates;
    • background timeout;
    • abort cleanup;
    • malformed and missing responses;
    • explicit notification Allow and Deny decisions.
  • corepack pnpm --filter @browser-skill/extension compile
  • git diff --check

CI Baseline Compatibility

This branch also carries the one-line Biome 2.4 formatter output for apps/extension/vitest.config.ts. The same formatting mismatch currently fails the Frontend job on upstream/main; this minimal compatibility commit is intentionally separate from the functional fix and lets this PR run the repository CI suite to completion.

CI Reliability Follow-up

GitHub Actions exposed a pre-existing test-only port allocation race: integration helpers probed an ephemeral port, released it, and then asked the daemon to bind the same number, allowing another process to claim it in between. The resulting Address already in use failure was unrelated to the functional changes. The PR now lets each daemon bind port 0 directly and reads the assigned address from its handle, removing the TOCTOU window across all affected integration helpers.

@NianJiuZst NianJiuZst marked this pull request as ready for review July 10, 2026 14:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant