Skip to content

Fix canvas WebView2 surface auth via plugin-node cap URL#588

Draft
christineyan4 wants to merge 1 commit into
openclaw:mainfrom
christineyan4:canvas-bug
Draft

Fix canvas WebView2 surface auth via plugin-node cap URL#588
christineyan4 wants to merge 1 commit into
openclaw:mainfrom
christineyan4:canvas-bug

Conversation

@christineyan4

Copy link
Copy Markdown
Contributor

Problem

The Windows tray's Canvas WebView2 hits 401 Unauthorized when navigating to /__openclaw__/canvas/* paths because it builds URLs against the gateway origin only, omitting the plugin-node capability token the gateway requires for cross-node plugin surface routes.

Fix

The gateway already advertises a cap-scoped surface URL in its hello-ok response under pluginSurfaceUrls.canvas (e.g. http://<gateway>/__openclaw__/cap/<token>). This PR plumbs that URL through to CanvasWindow and uses it as the base for /__openclaw__/canvas/* rewrites.

Additionally:

  • Treats the cap URL as a trusted origin prefix in IsUrlSafe so a cap URL whose host doesn't match _trustedGatewayOrigin (e.g. 127.0.0.1 cap vs localhost origin) isn't rejected by the safety check.
  • Clears the cached cap URL when a subsequent hello-ok/health update omits the canvas key, so a rotated/revoked token doesn't leave the node using a stale URL.

Files changed

File Change
src/OpenClaw.Shared/Models.cs Parse pluginSurfaceUrls.canvas from hello-ok
src/OpenClaw.Shared/WindowsNodeClient.cs Expose CanvasSurfaceUrl; clear when canvas key absent
src/OpenClaw.Tray.WinUI/Services/NodeService.cs Push URL into CanvasWindow on connect + on update
src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs Rewrite /__openclaw__/canvas/* via cap base; trust as IsUrlSafe prefix

Net: +128 / -14 across 4 files. No new files, no API changes outside the tray.

Testing

  • ✅ Existing tray test suite passes (859 tests)
  • ✅ Build clean on net10.0-windows10.0.22621.0 / win-arm64 (0 warnings, 0 errors)
  • ✅ Verified E2E: canvas.present with /__openclaw__/canvas/generative-art.html now renders the real content instead of the dashboard SPA shell / 401 page.

Known limitations (deferred)

  • TOCTOU race between canvas.present and the dispatcher-enqueued SetTrustedGatewayOrigin — theoretical, never observed.
  • Absolute-form canvas URLs (https://gw/__openclaw__/canvas/*) still go through the gateway origin path; gateway doesn't currently emit those.

The Windows tray's Canvas WebView2 was hitting 401 when navigating
to /__openclaw__/canvas/* paths because it built URLs against the
gateway origin only, omitting the plugin-node capability token the
gateway requires for cross-node plugin surface routes.

The gateway already advertises a cap-scoped surface URL in its
hello-ok response (pluginSurfaceUrls.canvas), e.g.
  http://<gateway>/__openclaw__/cap/<token>

This change plumbs that URL through to CanvasWindow and uses it as
the base for /__openclaw__/canvas/* rewrites, and as an additional
trusted origin prefix in IsUrlSafe so a cap URL whose host doesn't
match _trustedGatewayOrigin (e.g. 127.0.0.1 cap vs localhost origin)
is not rejected by the safety check.

Also clears the cached cap URL when a subsequent hello-ok/health
update omits the canvas key, so a rotated/revoked token doesn't
leave the node using a stale URL.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@clawsweeper

clawsweeper Bot commented May 29, 2026

Copy link
Copy Markdown

Codex review: needs real behavior proof before merge. Reviewed June 17, 2026, 3:38 PM ET / 19:38 UTC.

Summary
The branch parses pluginSurfaceUrls.canvas, exposes it through WindowsNodeClient, and uses it in NodeService and CanvasWindow to rewrite canvas WebView2 routes through the gateway cap-scoped surface URL.

Reproducibility: no. high-confidence live reproduction is included. Source inspection shows current main rewrites relative /__openclaw__/canvas/* paths to the bare gateway origin and does not read pluginSurfaceUrls, which matches the reported 401 failure mode when the gateway requires a cap-scoped surface URL.

Review metrics: 2 noteworthy metrics.

  • Changed surface: 4 files, +128/-14. The patch crosses shared handshake parsing, node-client cache state, tray service propagation, and WebView URL safety.
  • WebView trust path: 1 URL-safety path broadened. The diff accepts a gateway-advertised cap prefix before the dangerous URL regex, which needs maintainer-visible scrutiny.

Merge readiness
Overall: 🧂 unranked krab
Proof: 🧂 unranked krab
Patch quality: 🧂 unranked krab
Result: blocked until 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:

  • [P1] Validate pluginSurfaceUrls.canvas as an absolute http(s) cap route before trusting or rewriting through it.
  • [P1] Propagate missing or empty canvas surface maps to WindowsNodeClient and any open CanvasWindow, with focused regression coverage.
  • Attach redacted real behavior proof showing canvas.present renders through the cap URL without a 401.

Proof guidance:

  • [P1] Needs real behavior proof before merge: The PR body claims E2E verification, but no inspectable after-fix screenshot, recording, terminal output, live output, linked artifact, or redacted log is attached. 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.

Mantis proof suggestion
A real desktop proof would materially help because the changed behavior is visible WebView2 canvas routing with auth diagnostics. A maintainer can ask Mantis to capture proof by posting this exact PR comment:

@openclaw-mantis visual task: verify Windows tray canvas.present for /__openclaw__/canvas/generative-art.html renders via the cap-scoped URL and include diagnostics showing no 401.

Risk before merge

  • [P1] Merging as-is broadens WebView navigation trust to a gateway-advertised string before the existing dangerous scheme and private-network checks run.
  • [P1] Cap rotation or removal can leave an already-open canvas window using and trusting a stale cap URL because empty or missing surface maps are not propagated to the window.
  • [P1] The PR body reports E2E verification, but no inspectable after-fix proof is attached for maintainers to verify the real WebView2 canvas auth path.

Maintainer options:

  1. Validate and revoke cap URLs before merge (recommended)
    Require the branch to validate pluginSurfaceUrls.canvas, preserve explicit removal signals, refresh existing canvas windows on removal, and prove the happy-path plus revocation behavior before landing.
  2. Accept gateway-expanded trust explicitly
    Maintainers could intentionally treat hello-ok surface URLs as a WebView trust boundary, but that should be a documented choice with tests for malformed and revoked cap values.

Next step before merge

  • [P1] Contributor-visible proof is missing, and the remaining blockers include security-sensitive trust and token-revocation behavior that need review before automation takes over.

Security
Needs attention: The diff widens WebView URL trust using an unvalidated cap URL from the gateway handshake.

Review findings

  • [P1] Validate cap URLs before trusting them — src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:95-96
  • [P2] Propagate cap removal to open canvas windows — src/OpenClaw.Tray.WinUI/Services/NodeService.cs:832-834
  • [P2] Preserve empty surface maps as revocation signals — src/OpenClaw.Shared/Models.cs:706-707
Review details

Best possible solution:

Validate the advertised canvas surface URL as an absolute http(s) cap route, propagate empty or missing canvas-surface updates through the client and any open CanvasWindow, add focused regression tests, and require redacted live proof.

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

No high-confidence live reproduction is included. Source inspection shows current main rewrites relative /__openclaw__/canvas/* paths to the bare gateway origin and does not read pluginSurfaceUrls, which matches the reported 401 failure mode when the gateway requires a cap-scoped surface URL.

Is this the best way to solve the issue?

No for the current branch. The cap-scoped routing approach is likely the right narrow fix, but the implementation needs URL validation and complete stale-cap clearing before it is safe to merge.

Full review comments:

  • [P1] Validate cap URLs before trusting them — src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:95-96
    _canvasSurfaceBaseUrl is copied from hello-ok and then accepted before the dangerous URL pattern runs. A malformed or non-cap value such as a dangerous scheme would bypass the existing canvas navigation guard, so parse it and only trust absolute http(s) cap routes.
    Confidence: 0.86
  • [P2] Propagate cap removal to open canvas windows — src/OpenClaw.Tray.WinUI/Services/NodeService.cs:832-834
    This only refreshes an existing CanvasWindow when a non-empty canvas URL is present. If a later update removes the canvas surface during rotation or revocation, the already-open window keeps its old trusted cap base and can continue navigating through stale credentials.
    Confidence: 0.9
  • [P2] Preserve empty surface maps as revocation signals — src/OpenClaw.Shared/Models.cs:706-707
    Discarding an empty pluginSurfaceUrls object makes {} indistinguishable from an omitted field, so downstream cache clearing never runs for an explicit removal. Preserve the empty map or another explicit removal signal so revocation can clear cached canvas URLs.
    Confidence: 0.88

Overall correctness: patch is incorrect
Overall confidence: 0.9

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning high; reviewed against 6811d6a3e21b.

Label changes

Label justifications:

  • P2: This is a normal-priority canvas auth fix with limited blast radius, but it has concrete merge blockers before landing.
  • merge-risk: 🚨 security-boundary: The diff changes the WebView URL safety boundary by trusting a cap URL string from the gateway handshake.
  • merge-risk: 🚨 auth-provider: The diff changes token-scoped canvas routing and can leave stale cap credentials in an open window after removal or rotation.
  • rating: 🧂 unranked krab: Overall readiness is 🧂 unranked krab; proof is 🧂 unranked krab and patch quality is 🧂 unranked krab.
  • status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: The PR body claims E2E verification, but no inspectable after-fix screenshot, recording, terminal output, live output, linked artifact, or redacted log is attached. 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

Security concerns:

  • [medium] Unvalidated cap URL bypasses canvas URL guards — src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:95
    IsUrlSafe returns true for _canvasSurfaceBaseUrl before the dangerous-scheme regex, and the base URL is assigned from hello-ok without checking that it is an absolute http(s) cap route.
    Confidence: 0.86

What I checked:

  • Repository policy read: Full AGENTS.md was read; its connection, node, tray action, credential, and validation guidance is relevant to this auth-routing review, while build/tests were not run because this was a read-only review with no file changes. (AGENTS.md:1, 6811d6a3e21b)
  • Architecture guidance applied: Connection docs make GatewayConnectionManager the owner of connection lifecycle and document strict credential precedence, so changing canvas auth routing and node self-info propagation is compatibility- and auth-sensitive. (docs/CONNECTION_ARCHITECTURE.md:1, 6811d6a3e21b)
  • Current main lacks the requested cap-surface plumbing: Current main rewrites relative canvas URLs against _gatewayOriginForRewrite and does not parse or store pluginSurfaceUrls, so the central behavior is not already implemented. (src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:170, 6811d6a3e21b)
  • Shared model currently has no plugin surface field: GatewaySelfInfo.FromHelloOk on current main parses server, policy, and snapshot fields only; the cap-surface map is absent. (src/OpenClaw.Shared/Models.cs:695, 6811d6a3e21b)
  • Latest release lacks the fix too: Tag v0.6.3 contains no pluginSurfaceUrls, PluginSurfaceUrls, canvasSurface, __openclaw__/cap, or pluginSurface matches in the relevant model and canvas files. (85445c78066b)
  • PR broadens WebView trust before validation: At PR head, IsUrlSafe accepts _canvasSurfaceBaseUrl via MatchesTrustedPrefix before applying the dangerous URL pattern, while the base URL comes from the handshake string without scheme/path validation. (src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:95, 59841e1b62f8)

Likely related people:

  • Scott Hanselman: Recent hardening commits touched both CanvasWindow and NodeService, directly adjacent to this PR's WebView auth and trust boundary changes. (role: recent area contributor; confidence: high; commits: 0d4fcbd50ad5, d23f8ca50013; files: src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs, src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
  • AlexAlves87: Auth- and bridge-relevant WebView2 native-to-SPA canvas behavior appears in their merged CanvasWindow bridge commit. (role: feature introducer; confidence: medium; commits: e832229a9e4b; files: src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs)
  • Ranjesh: Recent current-main history and blame show broad changes to the same CanvasWindow, Models, WindowsNodeClient, and NodeService surfaces after the PR base. (role: recent adjacent contributor; confidence: medium; commits: 077d44cc6cd2, ea36b12f9e4c; files: src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs, src/OpenClaw.Shared/Models.cs, src/OpenClaw.Shared/WindowsNodeClient.cs)
  • christineyan4: Separate from authorship of this PR, prior merged history shows related NodeService canvas-window activation work. (role: prior canvas contributor; confidence: medium; commits: 82408b8d7fa0; files: src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
  • Mike Harsh: The node ownership unification commit touched NodeService, which is the connection lifecycle surface this PR extends. (role: connection lifecycle contributor; confidence: low; commits: 0816eb0eefc0; files: src/OpenClaw.Tray.WinUI/Services/NodeService.cs)
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.

@clawsweeper clawsweeper Bot added rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask. P2 Normal priority bug or improvement with limited blast radius. merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. labels May 29, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

merge-risk: 🚨 auth-provider 🚨 Merging this PR could break OAuth, tokens, provider routing, model choice, or credentials. merge-risk: 🚨 security-boundary 🚨 Merging this PR could weaken sandboxing, authorization, credentials, or sensitive data. P2 Normal priority bug or improvement with limited blast radius. rating: 🧂 unranked krab Not merge-ready due to missing proof or serious correctness/safety concerns. status: 📣 needs proof The PR needs real behavior proof before ClawSweeper can clear the contributor ask.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant