fix(gateway): deny session-scoped clients the broad sessions.subscribe (per-tab isolation) by anagnorisis2peripeteia · Pull Request #110184 · openclaw/openclaw

anagnorisis2peripeteia · 2026-07-17T21:28:11Z

Hardens the per-tab session isolation added in this branch. Follow-up to my review: #109817 (review)

What Problem This Solves

A session-scoped (browser-copilot / SESSION_SCOPED_EVENTS) connection can call the legacy broad sessions.subscribe and join the flat sessionEventSubscribers set. handleTranscriptUpdateBroadcast (src/gateway/server-session-events.ts:176-184) unions that set into session.message / sessions.changed recipients, and those events aren't in SESSION_SUBSCRIPTION_EVENTS (src/gateway/server-broadcast.ts:80), so they skip the per-tab opt-out filter — delivering every session's live transcript to a connection the per-tab model documents as isolated (contra the comment at server-broadcast.ts:212).

Why This Change Was Made

The per-tab boundary is enforced only on the per-session subscription path. A scoped client should never join the broad fanout; it subscribes per session via sessions.messages.subscribe (which is all the bundled extension ever calls — extensions/browser/chrome-extension/modules/copilot-background.js:374). Denying the broad path for scoped clients closes the gap at the root without touching delivery mechanics.

Adding the events to SESSION_SUBSCRIPTION_EVENTS instead over-blocks: those broadcasts don't pass opts.sessionKeys, so the per-session filter would also drop own-session messages.

User Impact

Defense-in-depth. Not drive-by-web-page reachable (the extension only uses the per-session path), but a compromised or malicious browser-copilot connection could otherwise read every session's live transcript rather than only its own tab's. No behavior change for the stock extension.

Evidence

Dynamic repro on this branch's head (real gateway harness, green on gateway-core / gateway-server / gateway-client) confirmed the cross-session leak pre-fix; post-fix the foreign transcript is no longer delivered.
Added a regression test in src/gateway/session-message-events.test.ts (browser-copilot broad sessions.subscribe rejected + foreign session.message not delivered).
Full session-message-events suite stays green (22/22).

Co-authored-by: Cameron Beeley <[email protected]>

A browser-copilot / SESSION_SCOPED_EVENTS connection could call the legacy broad sessions.subscribe and join the flat sessionEventSubscribers set, which server-session-events unions into session.message / sessions.changed recipients. Those events are not in SESSION_SUBSCRIPTION_EVENTS, so they skip the per-tab opt-out filter, delivering every session's live transcript to a connection the per-tab model documents as isolated. Deny the broad path for session-scoped clients; they subscribe per session via sessions.messages.subscribe. Adds a regression test: browser-copilot broad sessions.subscribe is rejected and a foreign session's transcript is not delivered.

clawsweeper · 2026-07-17T21:35:10Z

Codex review: needs real behavior proof before merge. Reviewed July 17, 2026, 5:31 PM ET / 21:31 UTC.

Summary
The branch rejects broad sessions.subscribe calls from browser-copilot or other session-scoped Gateway clients and adds a real-Gateway regression test for foreign-session transcript delivery.

Reproducibility: no. independently verified high-confidence live reproduction is available in this review. The branch supplies a concrete real-Gateway harness path and identifies the broad-subscriber delivery route, so the condition is source-reproducible but needs inspectable run output.

Review metrics: none identified.

Stored data model
Persistent data-model change detected: durable storage schema: extensions/browser/chrome-extension/modules/copilot-gateway-lifecycle.js, durable storage schema: extensions/browser/chrome-extension/modules/copilot-session-registry.js, serialized state: extensions/browser/chrome-extension/modules/copilot-gateway.test.ts, serialized state: src/gateway/server-methods/sessions-subscriptions.ts, serialized state: src/gateway/session-message-events.test.ts, serialized state: src/state/openclaw-state-db.generated.d.ts, and 4 more. Confirm migration or upgrade compatibility proof before merge.

Root-cause cluster
Relationship: partial_overlap
Canonical: #109817
Summary: This PR is a focused security hardening follow-up for the open per-tab browser-copilot implementation, rather than an independent product direction.

Members:

canonical: feat(browser): add secure per-tab copilot panel #109817 - The parent PR owns the browser-copilot per-tab session model and the per-session delivery contract this change hardens.

Proposal only: this assessment does not dispatch repair, suppress jobs, mutate sibling items, close, or merge anything.

Merge readiness
Overall: 🦪 silver shellfish
Proof: 🦪 silver shellfish
Patch quality: 🐚 platinum hermit
Result: blocked until stronger real behavior proof is added.

Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch.

Rank-up moves:

Rebase or cherry-pick the focused hardening commit onto the current feat/browser-copilot-contracts head and refresh mergeability.
[P1] Attach a redacted terminal transcript, log artifact, or recording from the real Gateway harness showing pre-fix foreign delivery and post-fix rejection/no foreign delivery.

Proof guidance:

[P1] Needs stronger real behavior proof before merge: The PR body asserts a successful real Gateway harness repro, but provides no inspectable redacted output or artifact showing the foreign transcript was delivered before and withheld after the fix; add that evidence before merge and redact private endpoints, tokens, and identifiers. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Risk before merge

[P1] The branch is currently reported dirty against the open parent branch; the focused fix should be rebased or cherry-picked onto the parent’s current head before merging.
[P1] The PR body describes a dynamic pre/post Gateway harness reproduction, but it does not include inspectable redacted terminal output, logs, recording, or linked artifact proving the after-fix result on this head.

Maintainer options:

Decide the mitigation before merge
Land the narrow subscription guard as part of the parent browser-copilot work after a clean rebase, with a redacted real Gateway harness transcript showing that a scoped client cannot receive a foreign session’s live transcript.
Pause or close
Do not merge this PR until maintainers decide whether the risk is worth taking.

Next step before merge

[P1] A reviewer should verify the rebased exact head and inspect contributor-supplied real Gateway proof; no narrow automated repair can provide evidence from the contributor’s setup.

Security
Cleared: The focused head commit strengthens a transcript-isolation boundary and introduces no concrete new supply-chain or security weakness beyond the already-reviewed parent feature surface.

Review details

Best possible solution:

Land the narrow subscription guard as part of the parent browser-copilot work after a clean rebase, with a redacted real Gateway harness transcript showing that a scoped client cannot receive a foreign session’s live transcript.

Do we have a high-confidence way to reproduce the issue?

No independently verified high-confidence live reproduction is available in this review. The branch supplies a concrete real-Gateway harness path and identifies the broad-subscriber delivery route, so the condition is source-reproducible but needs inspectable run output.

Is this the best way to solve the issue?

Yes, conditionally: rejecting the unsafe broad subscription at its ingress point is narrower than altering broadcast filtering and preserves the intended per-session subscription API. The change should first be rebased onto the parent branch’s current contract.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 0546e15c2e81.

Label changes

Label changes:

add P1: The patch addresses a potential cross-session live-transcript exposure through a session-scoped Gateway client, a material privacy and security-boundary concern.
add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
add status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body asserts a successful real Gateway harness repro, but provides no inspectable redacted output or artifact showing the foreign transcript was delivered before and withheld after the fix; add that evidence before merge and redact private endpoints, tokens, and identifiers. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Label justifications:

P1: The patch addresses a potential cross-session live-transcript exposure through a session-scoped Gateway client, a material privacy and security-boundary concern.
rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🐚 platinum hermit.
status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs stronger real behavior proof before merge: The PR body asserts a successful real Gateway harness repro, but provides no inspectable redacted output or artifact showing the foreign transcript was delivered before and withheld after the fix; add that evidence before merge and redact private endpoints, tokens, and identifiers. After adding proof, update the PR body; ClawSweeper should re-review automatically. If it does not, the PR author or someone with repository write access can comment @clawsweeper re-review.

Evidence reviewed

What I checked:

Focused isolation guard: The proposed handler rejects sessions.subscribe when the authenticated client is browser copilot or advertises SESSION_SCOPED_EVENTS, directing it to sessions.messages.subscribe; this closes the broad subscriber registry path described in the PR. (src/gateway/server-methods/sessions-subscriptions.ts, c2b8ccdc2fd9)
Regression coverage: The branch adds a gateway-harness scenario that pairs a browser-copilot client, rejects broad subscription, and checks that a foreign session transcript is not delivered. (src/gateway/session-message-events.test.ts, c2b8ccdc2fd9)
Parent feature relationship: The open parent implementation establishes the per-tab browser-copilot session model and documents Gateway-owned per-session delivery; this follow-up is a narrow hardening patch for that same boundary. (src/gateway/server-broadcast.ts, fdc8df41bead)
Branch state: GitHub reports this PR as dirty against its open parent branch, so the final one-commit hardening change needs a fresh merge-base/rebase review before it can land safely. (c2b8ccdc2fd9)

Likely related people:

steipete: Authored the browser-copilot security-contract commits on the parent branch and requested review after the parent base reference changed. (role: parent feature author and recent gateway/browser area contributor; confidence: high; commits: a2a6266ca11f, c213d5e7a3a8, cff2a99f4e9e; files: src/gateway/server-broadcast.ts, src/gateway/server/ws-connection/connect-admission.ts, extensions/browser/chrome-extension/modules/copilot-background.js)

What the crustacean ranks mean

🦀 challenger crab: rare, exceptional readiness with strong proof, clean implementation, and convincing validation.
🦞 diamond lobster: very strong readiness with only minor maintainer review expected.
🐚 platinum hermit: good normal PR, likely mergeable with ordinary maintainer review.
🦐 gold shrimp: useful signal, but proof or patch confidence is still limited.
🦪 silver shellfish: thin signal; proof, validation, or implementation needs work.
🧂 unranked krab: not merge-ready because proof is missing/unusable or there are serious correctness or safety concerns.
🌊 off-meta tidepool: rating does not apply to this item.

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

ClawSweeper keeps one durable marker-backed review comment per issue or PR.
Re-runs edit this comment so the latest verdict, findings, and automation markers stay together instead of adding duplicate bot comments.
A fresh review can be triggered by eligible @clawsweeper re-review comments, exact-item GitHub events, scheduled/background review runs, or manual workflow dispatch.
PR/issue authors and users with repository write access can comment @clawsweeper re-review or @clawsweeper re-run on an open PR or issue to request a fresh review only.
Maintainers can also comment @clawsweeper review to request a fresh review only.
Fresh-review commands do not start repair, autofix, rebase, CI repair, or automerge.
Maintainer-only repair and merge flows require explicit commands such as @clawsweeper autofix, @clawsweeper automerge, @clawsweeper fix ci, or @clawsweeper address review.
Maintainers can comment @clawsweeper explain to ask for more context, or @clawsweeper stop to stop active automation.

steipete and others added 17 commits July 17, 2026 20:46

feat(browser): add copilot security contracts

a2a6266

fix(gateway): expose verified client identity to handlers

c213d5e

feat(browser): add secure per-tab copilot panel

cff2a99

Co-authored-by: Cameron Beeley <[email protected]>

refactor(browser): separate copilot gateway hint custody

fa33a08

fix(browser): preserve legacy pairing parse shape

ef86d68

fix(browser): harden copilot lifecycle custody

77c37a1

Co-authored-by: Cameron Beeley <[email protected]>

fix(browser): enforce copilot lifecycle boundaries

feff49b

Co-authored-by: Cameron Beeley <[email protected]>

style(browser): format copilot sources

6278925

Co-authored-by: Cameron Beeley <[email protected]>

fix(browser): preserve copilot consent revocation

1c949e3

Co-authored-by: Cameron Beeley <[email protected]>

refactor(browser): split copilot custody owners

88a30e2

Co-authored-by: Cameron Beeley <[email protected]>

test(browser): normalize websocket array buffers

aa10d36

Co-authored-by: Cameron Beeley <[email protected]>

chore(protocol): regenerate Swift gateway models

f644970

Co-authored-by: Cameron Beeley <[email protected]>

refactor(browser): model copilot runtime entrypoints

7732547

Co-authored-by: Cameron Beeley <[email protected]>

fix(browser): honor extension build boundaries

3242d10

Co-authored-by: Cameron Beeley <[email protected]>

test(gateway): assert targeted chat delivery

c97f4a4

Co-authored-by: Cameron Beeley <[email protected]>

test(gateway): cover targeted delivery calls

f136288

Co-authored-by: Cameron Beeley <[email protected]>

openclaw-barnacle Bot added gateway Gateway runtime size: S labels Jul 17, 2026

anagnorisis2peripeteia mentioned this pull request Jul 17, 2026

feat(browser): add secure per-tab copilot panel #109817

Merged

steipete force-pushed the feat/browser-copilot-contracts branch from f136288 to fdc8df4 Compare July 17, 2026 21:30

steipete requested a review from a team as a code owner July 17, 2026 21:30

steipete force-pushed the feat/browser-copilot-contracts branch 2 times, most recently from d4af120 to 933db40 Compare July 17, 2026 22:09

steipete deleted the branch openclaw:feat/browser-copilot-contracts July 18, 2026 00:00

steipete closed this Jul 18, 2026

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

fix(gateway): deny session-scoped clients the broad sessions.subscribe (per-tab isolation)#110184

fix(gateway): deny session-scoped clients the broad sessions.subscribe (per-tab isolation)#110184
anagnorisis2peripeteia wants to merge 17 commits into
openclaw:feat/browser-copilot-contractsfrom
anagnorisis2peripeteia:fix/browser-copilot-broad-subscribe-isolation

anagnorisis2peripeteia commented Jul 17, 2026

Uh oh!

clawsweeper Bot commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Uh oh!

Conversation

anagnorisis2peripeteia commented Jul 17, 2026

What Problem This Solves

Why This Change Was Made

User Impact

Evidence

Uh oh!

clawsweeper Bot commented Jul 17, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants