feat(browser): add secure per-tab copilot panel#109817
Conversation
openclaw-barnacle
Bot
added
app: web-ui
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3faba78ca2
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| (!opts?.sessionKeys?.length || | ||
| !opts.sessionKeys.some((sessionKey) => | ||
| params.sessionMessageSubscribers?.get(sessionKey).has(c.connId), | ||
| )) |
There was a problem hiding this comment.
For a browser copilot or other session-scoped-events client that also requests tool-events, chat.ts registers the initiating connection in toolEventRecipients and later sends tool lifecycle agent frames with broadcastToConnIds(..., runToolRecipients) without sessionKeys. This new gate drops those targeted frames before sending, so copilot-originated runs cannot receive live tool start/end updates even though the run-recipient registry already authorized that connection; pass the delivery session key on that path or recognize run-tool recipients as an authorized scoped source.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
rating: 🧂 unranked krab
|
Codex review: found issues before merge. Reviewed July 17, 2026, 7:40 PM ET / 23:40 UTC. Summary Reproducibility: yes. from source: a scoped client can enter the broad session subscriber path, while the new allowlist does not cover all session-bearing events; the related follow-up also reports a real-Gateway reproduction. Review metrics: 2 noteworthy metrics.
Stored data model Root-cause cluster Members:
Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything. Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Risk before merge
Maintainer options:
Next step before merge
Maintainer decision needed
Security Review findings
Review detailsBest possible solution: Deny broad Do we have a high-confidence way to reproduce the issue? Yes, from source: a scoped client can enter the broad session subscriber path, while the new allowlist does not cover all session-bearing events; the related follow-up also reports a real-Gateway reproduction. Is this the best way to solve the issue? No: the tab-bound design is plausible, but merging before broad session subscription is denied leaves the advertised per-tab isolation incomplete; the narrow root-boundary denial is safer than expanding event-by-event filtering. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: unclear because the file could not be read completely. Codex review notes: model internal, reasoning high; reviewed against 03d5d88a14e4. Label changesLabel justifications:
Evidence reviewedSecurity concerns:
What I checked:
Likely related people:
What the crustacean ranks mean
Shiny media proof means a screenshot, video, or linked artifact directly shows the changed behavior. Runtime, network, CSP, and security claims still need visible diagnostics. How this review workflow works
Review history (12 earlier review cycles; latest 8 shown)
|
|
@steipete this is awesome to see. May I assist further? Happy to take the Stage 2 you flagged — bringing the #93680 side panel onto these contracts once Stage 1 lands, as the UI client rather than a parallel path. Concretely: rebase the panel to connect as the Would you prefer that as a follow-up PR stacked on this once it merges, or folded in some other way? Happy to work to whatever shape suits you. |
3faba78 to
1c39716
Compare
Dependency GuardThis PR changes dependency-related files. Maintainers should confirm these changes are intentional. Changed files:
Maintainer follow-up:
|
github-actions
Bot
added
the
dependencies-changed
openclaw-barnacle
Bot
added
docs
Dependency graph changes notedThis PR includes dependency graph changes. The dependency guard is informational because the PR author is a repository admin or a member of
Security review is still recommended before merge when the dependency graph change is intentional. |
github-actions
Bot
added
the
dependencies-changed
steipete
changed the title
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1c397169d1
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| }, | ||
| onConnectFailure: (error, { plan }) => { | ||
| const details = error.details && typeof error.details === "object" ? error.details : {}; | ||
| if (details.code === "AUTH_DEVICE_TOKEN_MISMATCH") { |
There was a problem hiding this comment.
If a stored browser-copilot device token is rejected by the new dedicated identity/origin check (for example after the Chrome extension ID/origin changes, or when an existing pairing lacks browserOrigin), connect-device-pairing.ts returns NOT_PAIRED without a details.code or request id. This handler only clears the token for AUTH_DEVICE_TOKEN_MISMATCH, so resolveCopilotClose keeps retrying with the same rejected token and never creates a fresh pairing request, leaving the side panel stuck until extension storage is manually wiped. Treat that identity/origin NOT_PAIRED rejection as token recovery, or have the server return a pairing-required detail so the next connect can re-pair.
Useful? React with 👍 / 👎.
1c39716 to
322d163
Compare
clawsweeper
Bot
added
proof: 📸 screenshot
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 322d1631e5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| return clientId === GATEWAY_CLIENT_NAMES.CONTROL_UI; | ||
| return ( | ||
| clientId === GATEWAY_CLIENT_NAMES.CONTROL_UI || | ||
| clientId === GATEWAY_CLIENT_NAMES.BROWSER_COPILOT |
There was a problem hiding this comment.
When this classifies openclaw-browser-copilot as a browser operator UI, a normal side-panel chat.send with toolBindings.browser now satisfies the restart-safe eligibility in src/gateway/server-methods/chat-send-session.ts:132-147, but the restart-safe fingerprint in src/gateway/server-methods/chat-restart-recovery.ts:75-87 only covers message and sender identity. After a gateway crash/setup failure, retry/adoption with the same idempotency key and text can therefore keep the durable claim while changing the browser tab binding, so the recovered run may act on a different tab than the one admitted. Exclude tool-bound copilot sends from restart-safe recovery or bind the fingerprint to the tool binding.
Useful? React with 👍 / 👎.
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
Co-authored-by: Cameron Beeley <[email protected]>
d4af120 to
933db40
Compare
anagnorisis2peripeteia
left a comment
anagnorisis2peripeteia
left a comment
There was a problem hiding this comment.
One more, lower-priority than the subscribe gap — same shape on the tool side, flagging for defense-in-depth.
The per-run browser tab binding's targetId is client-supplied and not server-side ownership-validated. The gateway does admission only (src/gateway/server-methods/chat-send-request.ts:160-161: "Gateway owns only paired-client admission"), delegating binding validation to the plugin. But the plugin validates shape (extensions/browser/src/browser-tool-binding.ts parseBrowserTabToolBinding) and agent-override (applyBrowserTabToolBinding pins the agent to the run binding) — neither checks that toolBindings.browser.targetId belongs to this connection's / session's shared tab.
relay-bridge.ts tabByTargetId does constrain dispatch to currently-shared, attached tabs, so this isn't reachable by a web page or the agent (both are pinned to the binding). The residual is a compromised or malicious paired copilot connection sending chat.send with toolBindings.browser.targetId set to another shared tab's target, driving a tab outside its own binding. Low-moderate / defense-in-depth (single-operator, and it needs the other tab's targetId) — but it's the same "client-supplied identifier not ownership-checked" pattern as the subscribe gap.
Belt-and-suspenders fix: validate binding.targetId / tabId against the connection's own shared-tab set at admission (or in the plugin binding validation), so a copilot run can only bind to a tab it actually shares. Happy to fold this into #110184 if useful.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 933db40cf0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| } | ||
| case "unpair": { | ||
| await chrome.storage.local.remove(["relayUrl", "token"]); | ||
| await chrome.storage.local.remove(["relayUrl", "gatewayUrl", "token"]); |
There was a problem hiding this comment.
When a user clicks Unpair after the new side-panel flow has connected once, this only removes the relay pairing fields; the copilot identity and issued Gateway token stored under copilotDeviceIdentitiesV1/copilotDeviceTokensV1 remain in chrome.storage.local. If the same browser later pastes a pairing string for the same Gateway, CopilotGatewayClient reloads that old operator device token and reconnects without a fresh device approval, so Unpair no longer actually clears this browser's Gateway credential. Clear the copilot identity/token state for the gateway (or all copilot gateways) as part of this path.
Useful? React with 👍 / 👎.
clawsweeper
Bot
added
proof: sufficient
Co-authored-by: Cameron Beeley <[email protected]>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 78559af68a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| isBrowserCopilotClient(clientInfo) && | ||
| !stopCommand && | ||
| (!p.toolBindings || !Object.hasOwn(p.toolBindings, "browser")) | ||
| ) { | ||
| return { ok: false, error: "browser copilot runs require an explicit browser tool binding" }; |
There was a problem hiding this comment.
When the Browser plugin/tool is disabled or filtered out for the target agent (for example browser.enabled=false, the browser plugin disabled, or a restrictive tool allow/deny policy), this gate still admits openclaw-browser-copilot sends as long as a toolBindings.browser object is present. The binding is only validated/applied later when the Browser tool is actually constructed in extensions/browser/plugin-registration.ts, so with no Browser tool the panel can start a tab-bound run that cannot act on the admitted tab and may complete using unrelated tools/text. Please fail the send/preparation when the browser tool will not be available for this run, or ensure the copilot-owned tool grant is added before dispatch.
Useful? React with 👍 / 👎.
Co-authored-by: Cameron Beeley <[email protected]>
clawsweeper
Bot
added
the
proof: 📸 screenshot
|
Merged via squash.
|
|
@steipete thanks so much for this. could you change the @ reference to my gh and not my discord, @anagnorisis2peripeteia please? <3 |
* feat(browser): add copilot security contracts * fix(gateway): expose verified client identity to handlers * feat(browser): add secure per-tab copilot panel Co-authored-by: Cameron Beeley <[email protected]> * refactor(browser): separate copilot gateway hint custody * fix(browser): preserve legacy pairing parse shape * fix(browser): harden copilot lifecycle custody Co-authored-by: Cameron Beeley <[email protected]> * fix(browser): enforce copilot lifecycle boundaries Co-authored-by: Cameron Beeley <[email protected]> * style(browser): format copilot sources Co-authored-by: Cameron Beeley <[email protected]> * fix(browser): preserve copilot consent revocation Co-authored-by: Cameron Beeley <[email protected]> * refactor(browser): split copilot custody owners Co-authored-by: Cameron Beeley <[email protected]> * test(browser): normalize websocket array buffers Co-authored-by: Cameron Beeley <[email protected]> * chore(protocol): regenerate Swift gateway models Co-authored-by: Cameron Beeley <[email protected]> * refactor(browser): model copilot runtime entrypoints Co-authored-by: Cameron Beeley <[email protected]> * fix(browser): honor extension build boundaries Co-authored-by: Cameron Beeley <[email protected]> * test(gateway): assert targeted chat delivery Co-authored-by: Cameron Beeley <[email protected]> * test(gateway): cover targeted delivery calls Co-authored-by: Cameron Beeley <[email protected]> * fix(browser): declare copilot build dependencies Co-authored-by: Cameron Beeley <[email protected]> * fix(ci): clear browser copilot gate failures Co-authored-by: Cameron Beeley <[email protected]> * test(ci): cover copilot lint exclusion Co-authored-by: Cameron Beeley <[email protected]> * fix(browser): gate copilot on relay custody Co-authored-by: Cameron Beeley <[email protected]> * test(browser): bound copilot relay frames Co-authored-by: Cameron Beeley <[email protected]> --------- Co-authored-by: Cameron Beeley <[email protected]>
3 participants
Related: #93680
What Problem This Solves
The browser side-panel copilot needs one isolated agent session per tab without letting page content, another tab, or a stale panel acquire a different browser target or receive another session's events. Closing a tab must also end live delivery and run custody without deleting its transcript.
Why This Change Was Made
This rebuild gives the browser copilot a dedicated canonical device-pairing identity with issued-token reuse, Gateway-owned session subscription allowlists with pre-delivery filtering, and typed tab bindings enforced by the browser tool. Chrome receives a capability-bound panel URL through
chrome.sidePanel.setOptions({ tabId, path, enabled })for each shared tab; there is no global default panel.The extension service worker owns session creation, panel capability custody, debugger attachment, subscription, abort, and archival. Tab close first revokes delivery/debugger access and aborts any active run, then marks the Gateway session archived. The transcript remains in that archived Gateway session until normal user/admin session deletion; the extension stores no parallel transcript or silent live-session fallback. Failed abort/archive RPCs remain durable recovery work and retry without restoring access.
Based on @2CB2's side-panel implementation in #93680, which established the product direction and served as the implementation basis for this secure rebuild.
User Impact
Users can open a copilot panel scoped to the current shared tab. Two tabs receive distinct sessions and histories, unshared tabs get a clear denial/share flow, and closing a tab archives its session while preserving the transcript for later inspection.
Evidence