fix(browser.proxy): only advertise capability when shared gateway token is available#602
fix(browser.proxy): only advertise capability when shared gateway token is available#602QQSHI13 wants to merge 3 commits into
Conversation
|
Codex review: needs real behavior proof before merge. Reviewed June 5, 2026, 8:53 PM ET / 00:53 UTC. Summary Reproducibility: yes. for the core auth bug: current main passes the node bearer token into BrowserProxyCapability, and contributor terminal proof shows the auth rejection changing on the shared-token path. No high-confidence proof covers the no-shared-token skip/fallback path introduced by this PR. Review metrics: 3 noteworthy metrics.
Merge readiness Overall follows the weaker of proof and patch quality, so missing proof can cap an otherwise strong patch. Rank-up moves:
Proof guidance:
Risk before merge
Maintainer options:
Next step before merge
Security Review findings
Review detailsBest possible solution: Use SharedGatewayToken for browser-control auth when available, preserve the existing token-absent browser.proxy path with visible Settings guidance, and validate both shared-token and no-shared-token behavior unless maintainers explicitly approve a documented fail-closed contract. Do we have a high-confidence way to reproduce the issue? Yes for the core auth bug: current main passes the node bearer token into BrowserProxyCapability, and contributor terminal proof shows the auth rejection changing on the shared-token path. No high-confidence proof covers the no-shared-token skip/fallback path introduced by this PR. Is this the best way to solve the issue? No, not as-is. Passing SharedGatewayToken when present is the narrow fix direction, but suppressing browser.proxy when the shared token is missing is broader than the tested and documented current behavior. Full review comments:
Overall correctness: patch is incorrect AGENTS.md: found and applied where relevant. Codex review notes: model gpt-5.5, reasoning high; reviewed against 4be005707f44. Label changesLabel justifications:
Evidence reviewedWhat 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
|
77a9f61 to
1f932c5
Compare
Real behavior proof
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. |
|
@clawsweeper re-review |
|
🦞👀 Command router queued. I will update this comment with the next step. Re-review progress:
|
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
|
@clawsweeper re-review |
|
🦞🧹 I asked ClawSweeper to review this item again. Re-review progress:
|
…able The Windows Node was advertising browser.proxy with its node device token, which cannot authenticate against the browser control host. The browser control host expects the shared gateway token. Changes: - NodeService: add sharedGatewayTokenResolver callback - Only register BrowserProxyCapability when a shared gateway token is available - Pass the shared token (not the node bearer token) to BrowserProxyCapability - App.xaml.cs: wire resolver to GatewayRegistry.GetActive().SharedGatewayToken Fixes: the node no longer breaks browser commands by intercepting them with wrong credentials.
Add SessionKey property to OpenClawNotification so toast activations can route to the specific session that produced the assistant response. Update EmitChatNotification to accept and populate the session key from the gateway chat event payload.
Previously, clicking a session in the Sessions tab or a chat toast
notification always opened the default/main session instead of the
specific session.
Changes:
- Add App.PendingChatSessionKey as a fallback when HubWindow does not
exist (background notifications, closed HubWindow).
- Thread sessionKey through ToastActivationRouter (Action<string?>).
- Embed sessionKey in chat toast arguments and route it on activation.
- ChatPage (functional): consume pending key from both HubWindow and
App fallbacks; use !string.IsNullOrEmpty guard to treat empty string
as null.
- ChatPage (WebView): consume pending key and append &session={key} to
the navigation URL, or pass it through GatewayChatHelper on initial
WebView init.
- SessionsPage: write key to CurrentApp.PendingChatSessionKey in
addition to hub.PendingChatSessionKey.
- Update tests for the new OpenChat Action<string?> contract.
Fixes session routing for both native FunctionalUI and legacy WebView2
chat surfaces.
Problem
The Windows Node advertises
browser.proxycapability even when it only has a node device token (from pairing/bootstrap). TheBrowserProxyCapabilityforwards browser commands to the browser control host atgatewayPort + 2and uses this token for Bearer auth.The browser control host expects the shared gateway token, not the node device token. When the node intercepts
browser.proxycommands with the wrong token, auth fails with:This breaks browser functionality for all profiles (
user,openclaw, etc.) because the gateway routesbrowser.proxyto any node advertising the capability.Root cause
In
NodeService.RegisterCapabilities():_tokencomes fromAttachClient()(node bearer token).NodeServicehad no access to the shared gateway token stored inGatewayRegistry.Fix
Func<string?>? sharedGatewayTokenResolvertoNodeServiceconstructorRegisterCapabilities(), only registerBrowserProxyCapabilitywhen the resolver returns a non-empty shared token_token) toBrowserProxyCapabilityApp.xaml.csto read from_gatewayRegistry?.GetActive()?.SharedGatewayTokenBehavior change
browser.proxy(if settings toggle is on), auth fails silentlybrowser.proxywhen a shared gateway token is saved in the registry. Falls back to not advertising the capability, letting the gateway handle browser commands via its own local path.Validation
build.ps1✅dotnet test tests/OpenClaw.Shared.Tests/OpenClaw.Shared.Tests.csproj --no-restore✅ (2023 passed)NodeService.csis not compiled into the Tray test project (it is not in the<Compile Include>list)