Skip to content

fix(extension): require borrowing before tab access#15

Open
NianJiuZst wants to merge 4 commits into
Tencent:mainfrom
NianJiuZst:codex/require-borrow-for-tab-reads
Open

fix(extension): require borrowing before tab access#15
NianJiuZst wants to merge 4 commits into
Tencent:mainfrom
NianJiuZst:codex/require-borrow-for-tab-reads

Conversation

@NianJiuZst

@NianJiuZst NianJiuZst commented Jul 10, 2026

Copy link
Copy Markdown
Contributor

What problem this solves

BrowserSkill promises that an existing user tab must be explicitly borrowed before the agent touches it. The write-oriented handlers already enforce that boundary, but the shared explicit tab_id resolver treated every normal user window as visible to the active session.

That meant an agent could list user tabs, take a tab id, and call observation tools such as snapshot, get-html, screenshot, or console without showing the borrow confirmation or moving the tab into the Agent Window. The same resolver is also used by evaluate, wait, navigation, human-loop, and interaction handlers, so the inconsistency existed at the common authorization boundary rather than in one isolated command.

This was especially risky for already-authenticated pages because a read-only command could expose page content before the user had granted per-tab access. Existing tests encoded the old behavior by expecting screenshot and console access to succeed against a tab in a regular user window.

How this fixes it

The shared target resolver now requires every explicit tab to belong to the current session's Agent Window. A pre-existing user tab becomes eligible only after tab_borrow moves it into that window. Tabs owned by another session continue to return not_found, so this change does not weaken cross-session isolation or reveal another session's window membership.

The resolver returns the existing structured permission_denied / agent_window_scope error for an ordinary user tab. This gives callers a consistent, actionable explanation: borrow the tab before reading or interacting with it.

Tests now cover both sides of the boundary:

  • an explicit user tab is rejected before CDP or screenshot capture is invoked;
  • an explicit tab in the current Agent Window remains accessible;
  • screenshot and console regressions assert that no browser-side read occurs on denial.

User impact

Agents can still discover user tabs through tab list so they can identify a borrow target, but they cannot inspect page content until the user-approved borrow flow completes. Agent-created tabs and already-borrowed tabs continue to work because both live in the session's Agent Window.

This aligns runtime behavior with the README and privacy-policy promise that pre-existing tabs stay outside agent control until explicitly borrowed.

Validation

  • corepack pnpm --filter @browser-skill/extension test -- src/tools/__tests__/shared.test.ts src/tools/__tests__/observation.test.ts src/tools/__tests__/console.test.ts
    • 35 test files passed
    • 375 tests passed
  • 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:01
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