fix(canvas): a later navigation can open a private address

[SebTardif]

What Problem This Solves

Fixes: canvas content can open a private or loopback address after the first URL check, including from data HTML.

User Impact

User impact: data HTML is no longer presented, and a later navigation is cancelled unless it passes the same URL check as the first navigation.

Why This Change Was Made

The first Navigate check allowed data:text/html and did not run again when the page navigated or redirected.

Evidence

Before data HTML was rejected:

Assert.False() Failure
Expected: False
Actual:   True
url: data:text/html,<h1>x</h1>
url: data:text/html;charset=utf-8,x
Failed!  - Failed: 2, Passed: 4, Total: 6

After the check:

Passed!  - Failed: 0, Passed: 6, Skipped: 0, Total: 6, Duration: 23 ms

data:text/plain stays allowed. http://127.0.0.1/ and http://192.168.1.5/ are rejected. https://example.com/ stays allowed.

Change Type

• Bug fix
• Feature
• Refactor
• Docs or instructions
• Tests or validation
• Security hardening
• Chore or infrastructure

Scope

• Tray or WinUI UX
• Windows node capability
• Local MCP or winnode
• Gateway, connection, or pairing
• Permissions, privacy, or security
• Tests, CI, or docs

Required proof pools

• windows-winui-interactive: canvas navigation policy changed.

Validation

• ./build.ps1: succeeded
• Shared tests: 4098 passed, 32 skipped, then the worktree readme lookup passed with OPENCLAW_REPO_ROOT set
• Tray tests: 3070 passed, 5 failed on existing CRLF source-contract checks in files this change does not touch

Focused command:

dotnet test ./tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj --filter FullyQualifiedName~CanvasNavigationPolicyTests

Real Behavior Proof

• Environment tested: Windows, fix/f071-canvas-navigation-guard at 9bf410b01bd981993bef6386e43a9c6b5ee056c5
• PR head or commit tested: 9bf410b01bd981993bef6386e43a9c6b5ee056c5
• Exact steps or command run: classify data:text/html, data:text/plain, 127.0.0.1, 192.168.1.5, and https://example.com/
• Evidence after fix: HTML data URLs are not presentable. Passed, 6 of 6, 23 ms. NavigationStarting calls the same check and cancels when it fails.
• Observed result: a later navigation to a private address is cancelled
• Screenshot or artifact links verified? No
• Not verified or blocked: a live WebView redirect was not driven

Real behavior proof

• Behavior or issue addressed: canvas could navigate to a private address after the first URL check
• Real environment tested: Windows, patched tree at 9bf410b
• Exact steps or command run after this patch: classify data HTML, data plain text, loopback, LAN, and a public https URL
• Evidence after fix: terminal output changed from Actual True on data HTML to Passed, 6 of 6
• Observed result after fix: data HTML is rejected and private targets fail the navigation check
• What was not tested: a live WebView redirect inside the canvas window

Security Impact

• New permissions or capabilities? No
• Secrets or tokens handling changed? No
• New or changed network calls? No
• Command or tool execution surface changed? No
• Data access scope changed? Yes
• If any answer is Yes, explain the risk and mitigation: canvas present no longer accepts data HTML, and NavigationStarting cancels a target that fails the existing private-host check.

Compatibility and Migration

• Backward compatible? Yes
• Config or environment changes? No
• Migration needed? No

Review Conversations

• I replied to or resolved every bot review conversation addressed by this PR.
• I left unresolved only conversations that still need maintainer judgment.

[clawsweeper]

Codex review: needs real behavior proof before merge. Reviewed September 24, 2026, 3:27 PM ET / 19:27 UTC (Revision 3).

ClawSweeper review

What this changes

The branch moves canvas URL checks into a tested policy, rejects HTML data URLs, checks later WebView navigations, and cancels page-created windows.

Merge readiness

⛔ Blocked before merge - 6 items remain

This PR remains useful: current main still lacks a check for later canvas navigations, and the new head addresses the previously reported popup gap. Maintainer approval of the compatibility change and current-head WebView proof remain necessary before merge.

Priority: P1
Reviewed head: ef6be20ec32c494a31f74a33e5ac3a458404e689
Owner decision: Required. See Decision needed.

Review scores

Measure	Result	What it means
Overall readiness	🦪 silver shellfish (2/6)	The focused patch addresses the prior popup finding, but live boundary proof and an explicit compatibility decision remain outstanding.
Proof confidence	🦪 silver shellfish (2/6)	Needs real behavior proof before merge: Authority-chain proof required: The canvas window now consumes agent-supplied navigation targets in WebView2, but the supplied Windows output exercises only the extracted policy at an earlier head. An isolated current-head WebView trace must show an allowed destination and private redirect and popup rejection before network access; redact private addresses, tokens, and endpoints. No stored-data contract changes. 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.
Patch quality	🦐 gold shrimp (3/6)	Security review found an item that needs attention.

Verification

Check	Result	Evidence
Real behavior	Needs proof	Needs real behavior proof before merge: Authority-chain proof required: The canvas window now consumes agent-supplied navigation targets in WebView2, but the supplied Windows output exercises only the extracted policy at an earlier head. An isolated current-head WebView trace must show an allowed destination and private redirect and popup rejection before network access; redact private addresses, tokens, and endpoints. No stored-data contract changes. 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	7 items	Introduced navigation checks: The introduced diff registers NavigationStarting and NewWindowRequested; the handlers check later main-frame destinations and cancel popups. Compatibility change: The branch rejects data:text/html; the pinned base accepted both text/html and text/plain data URLs. Current main still needs the change: At fetched main, CanvasWindow registers NavigationCompleted but has no NavigationStarting or NewWindowRequested handler.
Findings	None	None.
Security	Needs attention	Unverified final navigation boundary: Policy tests do not show whether WebView2 cancels a private redirect or popup before a network request; the contributor and maintainer both identify the missing live capture.

How this fits together

The Windows tray canvas displays agent-supplied URLs or HTML in WebView2. Canvas navigation checks decide which destinations that content may open.

flowchart LR
A[Agent canvas request] --> B[Canvas window]
B --> C[URL policy]
C --> D[WebView navigation]
D --> E[Later navigation check]
D --> F[Popup cancellation]
E --> G[Allowed page or cancelled request]

Loading

Decision needed

Question	Recommendation
Should canvas.present continue supporting data:text/html URLs, or should this PR intentionally remove that behavior?	Approve the restriction: Document the changed canvas contract and accept that existing HTML data URL callers must use another supported path.

Why: The pinned base allowed HTML data URLs, while this security patch removes them; the maintainer discussion requests an explicit compatibility decision.

Before merge

• Add real behavior proof - Needs real behavior proof before merge: Authority-chain proof required: The canvas window now consumes agent-supplied navigation targets in WebView2, but the supplied Windows output exercises only the extracted policy at an earlier head. An isolated current-head WebView trace must show an allowed destination and private redirect and popup rejection before network access; redact private addresses, tokens, and endpoints. No stored-data contract changes. 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.
• Resolve security concern: Unverified final navigation boundary - Policy tests do not show whether WebView2 cancels a private redirect or popup before a network request; the contributor and maintainer both identify the missing live capture.
• Resolve merge risk (P1) - Existing canvas content using data:text/html will stop opening; a maintainer has requested an explicit decision on that compatibility change.
• Resolve merge risk (P1) - The changed WebView navigation boundary lacks current-head live proof that allowed navigation succeeds and private redirects and popups are stopped before network access.
• Complete next step (P2) - Obtain the maintainer's HTML data URL compatibility decision and add current-head isolated WebView proof for allowed navigation, blocked private redirect, and blocked popup before merge. Redact private endpoints and credentials; updating the PR body should trigger review, or ask a maintainer to comment @clawsweeper re-review.
• Resolve maintainer decision - Resolve the maintainer decision shown above before merge.

Findings

• [medium] Unverified final navigation boundary — src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:434

Agent review details

Security

Needs attention: The source adds destination guards, but their final WebView effect remains unverified at the current head.

Review metrics

Metric	Value	Why it matters
Code and test delta	production +95/-64 lines; tests +45 lines	The production growth creates a reusable policy and event guards, while tests cover classification rather than live WebView behavior.

Merge-risk options

Maintainer options:

1. Approve and document the restriction (recommended)
   Explicitly accept the HTML data URL compatibility change and capture current-head WebView evidence for allowed and blocked destinations.
2. Preserve supported HTML URLs
   Provide a constrained HTML path with focused compatibility coverage and the same final-navigation proof.

Technical review

Best possible solution:

Keep the later-navigation guard, settle the supported HTML data URL contract, and demonstrate allowed and blocked destinations in an isolated current-head canvas window.

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

Yes, from source: current main checks the initial canvas URL but does not register a later-navigation handler. A live redirect has not been exercised in the supplied proof.

Is this the best way to solve the issue?

Unclear until the HTML compatibility choice is made. Rechecking WebView navigation is a focused repair, and popup cancellation addresses the prior finding, but tests alone do not establish the final network boundary.

AGENTS.md: found and applied where relevant.

Codex review notes: model internal, reasoning medium; reviewed against 273b0182745a.

Labels

Label changes:

• add rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• remove rating: 🧂 unranked krab: Current PR rating is rating: 🦪 silver shellfish, so this older rating label is no longer current.

Label justifications:

• P1: The PR addresses a security-sensitive canvas navigation path that can affect real users.
• merge-risk: 🚨 compatibility: The branch rejects HTML data URLs that the pinned base allowed.
• merge-risk: 🚨 security-boundary: The new final-navigation checks have no current-head WebView proof of blocking private requests before network access.
• rating: 🦪 silver shellfish: Overall readiness is 🦪 silver shellfish; proof is 🦪 silver shellfish and patch quality is 🦐 gold shrimp.
• status: 📣 needs proof: The PR needs real behavior proof before ClawSweeper can clear the contributor ask. Needs real behavior proof before merge: Authority-chain proof required: The canvas window now consumes agent-supplied navigation targets in WebView2, but the supplied Windows output exercises only the extracted policy at an earlier head. An isolated current-head WebView trace must show an allowed destination and private redirect and popup rejection before network access; redact private addresses, tokens, and endpoints. No stored-data contract changes. 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

Security concerns:

• [medium] Unverified final navigation boundary — src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:434
  Policy tests do not show whether WebView2 cancels a private redirect or popup before a network request; the contributor and maintainer both identify the missing live capture.
  Confidence: 0.94

What I checked:

• Introduced navigation checks: The introduced diff registers NavigationStarting and NewWindowRequested; the handlers check later main-frame destinations and cancel popups. (src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:319, ef6be20ec32c)
• Compatibility change: The branch rejects data:text/html; the pinned base accepted both text/html and text/plain data URLs. (src/OpenClaw.Tray.WinUI/Windows/CanvasNavigationPolicy.cs:42, ef6be20ec32c)
• Current main still needs the change: At fetched main, CanvasWindow registers NavigationCompleted but has no NavigationStarting or NewWindowRequested handler. (src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:1, 273b0182745a)
• Prior finding addressed: The latest introduced commit adds NewWindowRequested cancellation. The previous ClawSweeper finding concerned unguarded page-created windows. (src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs:442, ef6be20ec32c)
• Maintainer disposition: A collaborator requested a decision on rejecting previously allowed HTML data URLs and current-head isolated WebView proof of an allowed navigation, blocked private redirect, and blocked popup. The later contributor comment says those live captures have not run. See fix(canvas): a later navigation can open a private address #1503 (comment) and fix(canvas): a later navigation can open a private address #1503 (comment).
• Focused tests only: The added tests exercise URL classification; the PR body reports a Windows focused test run but explicitly says no live WebView redirect was driven. (tests/OpenClaw.Tray.Tests/CanvasNavigationPolicyTests.cs:36, ef6be20ec32c)

Likely related people:

• shanselman: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)
• Lior Balmas: Suggested for follow-up; no historical authorship or introduction is verified. (role: unverified routing candidate; confidence: low)

Rank-up moves

Optional improvements that raise the rating; they are not merge blockers.

• Capture a redacted, isolated current-head WebView trace showing allowed navigation and blocked private redirect and popup before network I/O.
• Record the maintainer's decision on HTML data URL compatibility and document the resulting canvas contract.

Rating scale

Score	Internal tier	Crab rank	Meaning
6/6	S	🦀 challenger crab	Exceptional readiness
5/6	A	🦞 diamond lobster	Very strong readiness
4/6	B	🐚 platinum hermit	Good normal PR; ordinary maintainer review
3/6	C	🦐 gold shrimp	Useful, but confidence is limited
2/6	D	🦪 silver shellfish	Proof or implementation needs work
1/6	F	🧂 unranked krab	Not merge-ready
N/A	NA	🌊 off-meta tidepool	Rating does not apply

Overall follows the weaker of proof and patch quality.
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.

Workflow

• 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.

History

Review history (2 earlier review cycles)

• reviewed 2026-09-24T01:55:46.473Z sha 9bf410b :: needs real behavior proof before merge. :: [P1] Guard page-created windows before they reach private addresses
• reviewed 2026-09-24T02:11:48.139Z sha 18b4e87 :: needs real behavior proof before merge. :: [P1] Guard page-created windows before they open private addresses

[shanselman]

Maintainer disposition at head 18b4e87c8506: the main-frame NavigationStarting check is useful, but WebView2 page-created windows go through NewWindowRequested and are not covered here. Please apply the same destination policy to that path, and decide/document whether rejecting previously allowed data:text/html is an intended compatibility change. Before merge we need current-head isolated WebView proof of allowed navigation plus a blocked private redirect and popup before network access (redacted endpoints). These are two bounded gaps, not ten independent code defects. Keep this original PR; a small maintainer patch is welcome if useful.

[karkarl]

Global triage: NEEDS_HUMAN_TEST. Take confidence 60%; recommendation confidence 85%; effort small-to-medium; risk medium.

Reviewed exact head 18b4e87c8506. The main-frame NavigationStarting recheck closes a real normalization gap and about:blank remains allowed. Runtime boundaries are incomplete: there is no NewWindowRequested or frame-navigation guard, so popup and subframe paths bypass this policy. IsPresentableDataUrl is public and throws for non-data: inputs even though its only caller prechecks. The red E2E jobs match issue #1498 and are not caused by this diff.

Owner: author/maintainer. Make the helper non-throwing or private, cover or explicitly scope out popups, document the data:text/html compatibility break, and capture windows-winui-interactive proof for allowed navigation, blocked private redirect, and blocked popup before network I/O. This PR conflicts with #1504 in the Tray test project file.

[SebTardif]

ef6be20 makes IsPresentableDataUrl return false for null, empty, and short data: input instead of throwing. data:text/html stays rejected; only text/plain and the RFC 2397 default are presentable. NewWindowRequested is cancelled, so canvas popups are out of scope.

Local filter CanvasNavigationPolicy: 10 passed. Live captures of an allowed navigation, a blocked private redirect, and a blocked popup were not run.