Skip to content

feat: native WinUI A2UI renderer + MCP/security hardening - #239

Merged
shanselman merged 23 commits into
openclaw:masterfrom
codemonkeychris:feat/a2ui-native-winui
Apr 30, 2026
Merged

feat: native WinUI A2UI renderer + MCP/security hardening#239
shanselman merged 23 commits into
openclaw:masterfrom
codemonkeychris:feat/a2ui-native-winui

Conversation

@codemonkeychris

Copy link
Copy Markdown
Contributor

Summary

Brings the Windows tray to feature parity with the Android client on A2UI v0.8 and works through a security review of the resulting attack surface.

  • Native WinUI A2UI renderer. Replaces the Lit/WebView2 path with a XAML-native renderer hosting all 18 v0.8 components (Row, Column, List, Card, Tabs, Modal, Divider, Text, Image, Icon, Video, AudioPlayer, Button, CheckBox, TextField, DateTimeInput, MultipleChoice, Slider). Catalog-strict registry, JSONL envelope routing, A2UIValue tagged-union resolution, surface-scoped data model with subscriptions, action.context dataBinding scoping, secret-path redaction. SVG support added on top of D2D, with DOCTYPE strip + decode budgeting.
  • Local MCP server. Bearer-token auth, loopback-only bind, Origin/Host/Content-Type CSRF gate, per-request deadline + handler concurrency cap with drain-on-Dispose. Black-box integration tests cover every advertised tool. Tray Settings surfaces the endpoint, masked token, and a reset button.
  • canvas.navigate routes to the OS default browser (no longer requires an embedded WebView2 surface). High-risk URLs (loopback / private IP / link-local / no-dot intranet / non-HTTPS / IP literal / Windows Internet Security Manager Local-Machine or Intranet zone) require user confirmation via a Win32 MessageBox prompt; the agent always receives the same success response immediately and the prompt+launch run fire-and-forget so the user's decision and decision time aren't exposed via timing.
  • Media safety. Image / Video / AudioPlayer route through MediaResolver with HTTPS-only allowlist, DNS resolution + non-public-IP rejection (defends against DNS rebinding), data: URL size cap, Content-Length and stream-side byte caps for image fetches, and per-image cancellation tokens.
  • jsonlPath TOCTOU. canvas.a2ui.push jsonlPath rejects unresolved reparse points, opens the file once with FileStream, re-validates the resolved path with GetFinalPathNameByHandle, and enforces the size cap on the open stream.
  • Token sanitization. TokenSanitizer redacts Authorization: Bearer, JSON token/secret/bearer fields, and bare 43-char base64url tokens from log + jsonl diagnostics + malformed-line logs.

All Win32/COM APIs in the URL security path (MessageBoxW, IInternetSecurityManager::MapUrlToZone via documented CLSID/IID) are public and shipped in the current Windows SDK.

Test plan

  • OpenClaw.Shared.Tests — 1039 passed
  • OpenClaw.Tray.Tests — 219 passed
  • OpenClaw.Tray.IntegrationTests (OPENCLAW_RUN_INTEGRATION=1, real tray spawned per fixture) — 18 passed
  • OpenClaw.Tray.UITests — 62 passed
  • Manual: canvas.navigate http://www.example.com from MCP client — confirmation prompt appears, "Yes" launches default browser, "No" silently blocks, agent gets identical success in both cases
  • Manual: rich A2UI surface push round-trips through native renderer with canvas.a2ui.dump returning the redacted state snapshot
  • Manual: MCP token reset from Settings — old token invalidated, new token hot-swapped without restarting the listener

🤖 Generated with Claude Code

codemonkeychris and others added 19 commits April 28, 2026 11:27
- McpToolBridge.tools/list now returns curated per-command descriptions
  (sourced from openclaw docs and capability code) for all 17 commands
  across system/canvas/screen/camera capabilities; unknown commands fall
  back to the generic stub.

- New OpenClaw.Tray.IntegrationTests project: spawns the WinUI tray exe
  in MCP-only mode against an isolated data dir + free port, polls the
  HTTP listener for readiness, then drives every tool over JSON-RPC.
  18 tests cover all four capabilities; hardware-gated commands
  (canvas eval/snapshot, camera snap, screen capture) accept either
  success or a well-formed tool error. Opt-in via OPENCLAW_RUN_INTEGRATION=1.

- Tray-app testability hooks: OPENCLAW_TRAY_DATA_DIR isolates settings,
  logs, run marker, and the single-instance mutex so a test instance
  runs alongside the user's real tray; OPENCLAW_MCP_PORT overrides the
  hard-coded default port. Update check is suppressed when an isolated
  data dir is set.

Coverage with subprocess instrumentation (dotnet-coverage collect):
McpHttpServer 56%, NodeService 59%, capture services 58%,
SettingsManager 54% (all 0% before).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a build step alongside Shared.Tests and Tray.Tests so the integration
test project is compiled in CI. We don't run it in CI — the tests spawn
the WinUI tray exe and require a desktop session, which the hosted runners
don't reliably provide. Local-only via OPENCLAW_RUN_INTEGRATION=1 for now.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The new canvas.a2ui.dump and canvas.caps commands shipped without unit
coverage in the capability layer. Adds CanHandle assertions plus
execution-path tests for both: not-open error paths, success paths
returning structured JSON payloads (not quoted strings), and the
default-when-no-handler shape that canvas.caps returns.

Closes the test gap flagged in local-mcp-a2ui-closeout.md item #1.
Adds tests/OpenClaw.Tray.UITests, an end-to-end test project that drives
the real A2UIRouter + SurfaceHost + renderer pipeline against a live
WinUI3 visual tree. 50 tests across:

  - SmokeTests: harness boots an Application + hidden Window on a
    dedicated STA dispatcher thread, the container hosts XAML, and
    Application.Resources holds the expected Fluent style keys.
  - A2UIRenderingTests: original 6 end-to-end scenarios (Text, Column,
    DataModelUpdate reactivity, Button click via IInvokeProvider,
    UnknownRenderer fallback, DeleteSurface).
  - A2UIControlMatrixTests: one test per catalog component (Row,
    List vert/horiz, Card, Tabs, Modal, Divider H/V, Text usageHint
    h1-h5/caption/body, Icon w/ glyph spot-checks + theory, CheckBox
    literal + path-bound, TextField short/long/obscured, DateTimeInput
    date+time vs date-only, MultipleChoice single + multi, Slider
    literal + path-bound).
  - A2UIThemeTests: spacing, radius, primaryColor, font reach the
    surface; absent styles don't pollute the resource scope.
  - A2UIDashboardScaleTest: ~60-component dashboard exercising every
    renderer with 4-deep nesting + theming + post-render data update.

Harness notes captured in code comments for future maintainers:
  - <UseWinUI>true</UseWinUI> breaks the test SDK's program-file gen.
    The test project intentionally omits it and pulls WinUI3 in via
    package + ProjectReference instead.
  - WindowsAppSDKSelfContained=true skips copying Microsoft.TestPlatform
    DLLs; left off here so testhost wiring works.
  - Application.Resources can't be touched in the App ctor (COMException
    in WinAppSDK 1.8) — TestApp.MergeStandardResources is called
    post-construction.
  - WinUI types are thread-affine; visual-tree walks must run inside
    the dispatcher.
  - Templated controls (TextBox, PasswordBox, MultipleChoice ListView)
    fail PRI resource lookup when mounted in unpackaged test processes.
    The matrix tests walk the renderer's logical tree (Panel.Children,
    Border.Child, ContentControl.Content, etc.) instead of the visual
    tree, which exercises every renderer's output without needing
    template apply.

Slow-mode demo: SLOW_UI_TESTS=1 keeps the window visible at 900x640
and inserts pauses between steps, so a human can watch each scenario
render. SLOW_UI_STEP_MS=N overrides the per-step pause length.

Also fills in IconRenderer.MapName, which previously mapped every
v0.8 icon-name to an empty string. Each name now resolves to the
closest-meaning Segoe Fluent Icons codepoint, with unknown/empty
names falling back to the Help glyph instead of an empty box.
A theory-based matrix test pins eight representative mappings plus
the unknown-name fallback.
Adds A2UICanvasIntegrationTests, which drives a v0.8 surface exercising
one component per catalog category through the full MCP HTTP →
A2UICanvasWindow → A2UIRouter → SurfaceHost → live XAML pipeline inside
the shipped tray exe. The existing per-renderer UITests verify each
component in isolation; this test verifies they survive the full hop
end to end.

Flow:
  1. canvas.a2ui.reset (defensive — class fixture is shared)
  2. canvas.a2ui.push with one big surfaceUpdate + beginRendering +
     seed dataModelUpdate (Card → Column wrapping Text/Icon/Divider/
     Row(Button×2)/Tabs/CheckBox bound to /agreed/Slider bound to
     /volume), plus theme styles
  3. canvas.caps → assert renderer="native", a2ui.introspect=true
  4. canvas.a2ui.dump → assert surfaceCount=1, root, every declared
     component id + componentName + properties, seeded data model
  5. dataModelUpdate → re-dump → assert bound values changed without
     a tree rebuild
  6. canvas.snapshot → accept either success (validate full 8-byte PNG
     signature) or a well-formed tool error (window activation +
     first layout pass are racy on locked desktops)
  7. canvas.a2ui.reset → dump → surfaceCount=0

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Addresses approved items from C:\temp\a2ui-unified-review.md. Critical and
most High items implemented; Medium/Low followups remaining are tracked in
the review doc's status table.

Critical
- C1 SurfaceHost cycle/depth/component caps (no StackOverflow on adversarial
  surface graphs); inline error placeholder names the offending component.
- C2 Action context now scoped to the source component's dataBinding
  (explicit array or implicit walk over its other properties); secret paths
  require an explicit dataBinding opt-in.
- C3 SecretRedactor + RenderContext.MarkSecretPath: obscured TextField paths
  registered, redacted from canvas.a2ui.dump and dropped from action context;
  password*/secret*/token* key denylist applies to all three sinks.
- C4 Renderer-throw fallback rebuilds via inline placeholder, never the
  __error__ registry indirection that previously double-faulted.
- C5 CanvasCapability caps inline + jsonlPath JSONL at 4 MiB / 4096 lines
  with symlink-resolved temp validation; A2UIMessageParser streams without
  Split() and rejects oversize lines; DataModelStore caps per-update entries,
  valueMap depth, key length, and string value length.

High
- H1 Settings.A2UIImageHosts plumbed end-to-end. Default empty (data: only);
  agents/devs add hosts via settings.json until SettingsWindow UI lands.
- H2 MediaResolver streams remote bodies into a 8 MiB cap with
  ResponseHeadersRead + Content-Length pre-check + cancellation token.
- H4 LRU/sweep on ActionDispatcher debounce dict, bounded-with-oldest-eviction
  fallback queue, surface count cap (64), per-surface component cap on
  applies (matches renderer).
- H5 McpAuthToken: 256-bit base64url bearer token at <datadir>\mcp-token.txt;
  McpHttpServer requires Authorization: Bearer <token> on POSTs (constant-time
  compare). CLI/agent registration just reads the file. Token-less ctor kept
  for legacy/dev.
- H6 LoggingActionTransport drops the action envelope from logs by default;
  full envelope behind explicit LogFullEnvelope flag.
- H7 SurfaceModel.Sync lock guards Get/SetByPointer; CloneRoot snapshots the
  tree atomically for canvas.a2ui.dump.
- H8 TabsRenderer keys subscriptions by tab index (not Guid.NewGuid()); fixes
  per-render subscription leak and null-childId collisions.
- H9 McpHttpServer.Dispose now unconditionally runs the drain path. Stop
  idempotence keyed on a separate _stopping flag rather than _disposed.
- H10 INodeCapability.ExecuteAsync(req, CancellationToken) overload added;
  McpToolBridge plumbs the request CT through. RequestTimeoutMs raised to
  360s to cover screen.record's 5-min ceiling without orphaning capture.
- H11 A2UIMessageParser logs malformed lines instead of silently dropping.
- H12 RenderTargetBitmap.RenderAsync wrapped — translates the
  COMException/ArgumentException seen on minimised windows into a structured
  CANVAS_SNAPSHOT_NOT_VISIBLE error.
- H13 ImageRenderer.Update uses a generation token so a slow earlier load
  cannot clobber a faster later one.
- H14 AutomationHelpers wires A2UI label/description onto
  AutomationProperties.Name/HelpText across Button, CheckBox, TextField,
  DateTimeInput, MultipleChoice, Slider, Image, Icon, Video, AudioPlayer.

Medium / Low
- M3 MCP tool descriptions and integration test names match the canonical
  OpenClaw protocol (screen.snapshot/record, camera.list/snap/clip). No more
  screen.list/screen.capture drift.
- M5 jsonlPath FileInfo.ResolveLinkTarget(true) before temp-root check.
- M7 NodeService TaskCompletionSources use RunContinuationsAsynchronously and
  TrySet* — no inline UI-thread continuations, no double-complete throws.
- M8 ActionDispatcher.Send serialized via SemaphoreSlim(1); preserves order.
- M9 GatewayActionTransport captures _clientProvider() once per delivery.
- M10 NodeService._a2uiCanvasWindow invariant documented.
- M11 DateTimeInputRenderer falls back to ISO-8601 when outputFormat throws.
- M13 Whole-tree replace coerces non-object values to {value: <scalar>}.
- M14 Resources.TryGetValue on Card/Divider brushes and Text/AudioPlayer
  styles (replaces silent catch{}).
- M16 Surface Title pulled from styles.title when present.
- M18 Slider step / stepSize → StepFrequency.
- M19 Image avatar usageHint wraps in CornerRadius border.
- M21 App.xaml.cs single-instance mutex name uses a SHA-256 prefix of
  DataDirOverride (no GetHashCode randomization or int.MinValue overflow).
- M22 A2UIRouter.SnapshotSurfaces() for stable enumeration.
- M23 WindowsNodeClient.SendCanvasA2UIActionAsync takes JsonObject.
- M27 List renders via ItemsRepeater + StackLayout — virtualized (was a flat
  StackPanel that instantiated all children up front).
- L1 Sanitize agent-controlled IDs in router log lines.
- L2 Subscription key delimiter swapped to U+001E so component IDs containing
  "::" can't collide with sub-keys.
- L3 ActionDispatcher debounce dict bounded.
- L7 Card padding and Divider margin honor Theme.Spacing.

Tests
- 944 OpenClaw.Shared.Tests passing (5 new: 3 A2UI capability size caps,
  2 McpHttpServer auth coverage).
- 201 OpenClaw.Tray.Tests passing.
- Integration test fixture picks up the bearer token from <datadir>\mcp-token.txt
  after the tray writes it.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
ListVertical/ListHorizontal_RendersScrollViewer_WrappingStackPanel were
asserting on the old non-virtualized shape. Renamed to
…WrappingItemsRepeater and assert on ScrollViewer.Content (ItemsRepeater)
+ Layout.Orientation + ItemsSource. Children realize on layout pass and
aren't present in the unmounted visual tree, so we check the
ItemsSource sequence instead of counting realized TextBlocks.

Also extends the integration tools/list assertion failure message to
include the actual command list — the prior failure was unhelpful when
diagnosing why screen.record was missing (still under investigation; the
test was never run in CI per M30 and was already broken pre-this-branch).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
SVG decode goes through D2D's static SVG 1.1 subset (SvgImageSource → ID2D1SvgDocument): no scripts, no SMIL, no <foreignObject>, no external fetches. Same allowlist + 2 MiB cap as PNG/JPEG/WebP. Defense-in-depth: <!DOCTYPE …> pre-strip (kills XXE/billion-laughs structurally before D2D's parser sees the bytes) and an 8 s wall-clock timeout (path complexity isn't bounded by the byte cap). 12 tests cover happy path, allowlist, script/foreignObject/external-ref inertness, XXE, billion-laughs, and pathological-geometry render-time bound. Skill doc updated to steer agents toward inline data:image/svg+xml over arbitrary HTTPS raster URLs, since A2UIImageHosts defaults to empty.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds a Bearer token row to the Settings → Developer Mode → MCP section: read-only TextBox showing the token (masked with bullets by default), Reveal/Hide toggle, Copy button, and a caption with the storage path. Pasting the literal token is what most MCP clients (Claude Desktop, Cursor) actually want; the path is a hint for filesystem-capable agents that prefer reading from disk. NodeService.McpTokenPath is now the single source of truth for the path; McpAuthToken's class summary, McpHttpServer's "no auth yet" comment, and docs/MCP_MODE.md are updated to match the implemented reality (token at %APPDATA%\OpenClawTray\mcp-token.txt, lazily created on first MCP server start, persistent across restarts, 43 base64url chars).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…CI gates

Second sweep through the unified A2UI review (M1–M31, L4–L11). Highlights:
- Modal renders as a real ContentDialog (was Expander stub).
- MediaResolver pins DNS via SocketsHttpHandler.ConnectCallback; rejects
  non-public addresses to defend against rebinding (M4) and exposes a
  test-seam HttpMessageHandler ctor (M25).
- Tab layout in A2UICanvasWindow now diffs incrementally and preserves the
  selected tab across surface add/remove (M15).
- SurfaceHost short-circuits Rebuild when ApplyComponents receives an
  identical def set; preserves focus/scroll for re-emitted surfaces (M1).
- IA2UITelemetry seam + structured Push events from A2UIRouter (M29).
- CanvasCapability Func events use single-subscriber accessors that throw
  on a second subscribe instead of silently winning (M26).
- SettingsWindow takes a Func<NodeService?> provider; App.OnSettingsSaved
  refreshes status so the open window updates after node-service rebuild (M31).
- Logger rotates on append, not just static init (M12).
- MultipleChoice single mode writes scalar string, not one-element array (M20).
- Hardcoded "Canvas" / "Waiting for content..." / "Unsupported component:"
  strings localized across all five locales (L6).
- CI now builds + runs OpenClaw.Tray.IntegrationTests (with
  OPENCLAW_RUN_INTEGRATION=1) and OpenClaw.Tray.UITests (M30).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two related fixes from the same A2UI gateway-routing session.

A2UI action routing (Button clicks → agent turn):
- WindowsNodeClient.SendNodeEventAsync emits the gateway's `node.event`
  JSON-RPC method with `payloadJSON` as a stringified blob, mirroring the
  Android `GatewaySession.sendNodeEvent` wire shape. Replaces the broken
  `canvas.a2ui.action` method that the gateway never registered.
- AgentMessageFormatter ports the Kotlin `formatAgentMessage` byte-for-byte
  so the tagged `CANVAS_A2UI action=… session=… ctx=…` user message the
  LLM sees is identical regardless of which node emitted it.
- GatewayActionTransport now formats the action and ships it via
  `agent.request` so the gateway's existing dispatcher
  (server-node-events.ts:380) appends it as a user turn and runs an agent
  step. deliver=false keeps the raw line out of the chat bubble.
- NodeService threads sessionKey/host/instanceId through a small
  IGatewayActionContext seam; sessionKey is refreshed from push props,
  defaulting to "main" (matching Android's resolveMainSessionKey fallback).
- A2UIAction.Id auto-generates per-Raise so the gateway-side `key` field
  dedups double-clicks. ActionStatus event added for renderer feedback.

canvas.navigate hardening:
- HttpUrlValidator (new) gates agent-supplied URLs: absolute, http/https
  only, no userinfo, non-empty host. Returns canonical AbsoluteUri.
- CanvasCapability.NavigateRequested becomes Func<string,Task<string>>
  returning the opener that serviced the request; response carries
  `{navigated, opener, url}` so the agent can tell which path was taken.
- NodeService.OnCanvasNavigate always launches the OS default browser via
  `Process.Start { UseShellExecute = true }` (validator-gated) and closes
  any open canvas windows. The legacy WebView2 navigate path mangled
  external URLs (gateway-origin pinning rewrites) so it's gone.
- canvas.caps now advertises navigate=true unconditionally since the
  browser fallback is always available.

Stale UI tests fixed: Modal renderer changed from Expander to a trigger
Button + ContentDialog in 5b9c468; updated assertions in
A2UIControlMatrixTests and A2UIDashboardScaleTest.

Tests: 973 Shared.Tests + 218 Tray.Tests + 62 UITests, all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Capture the v0.8 wire format, standard catalog, and a side-by-side scorecard so PR reviewers can quickly tell intentional deviations from gaps. Grades Lit B- (4 documented TODOs, JSON-string auto-parse, no SSRF defense, thin tests) and WinUI A- (action-context allowlist, DNS-rebinding defense, list virtualization, secret denylist).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
CanvasNavigate_ReturnsNavigated used about:blank, which f9ac311 (HttpUrlValidator) now rejects — switch to https://example.com/. SystemExecApprovals_GetThenSet_PersistsRule and SystemRun_Echo_ReturnsExpectedOutput now pass baseHash from execApprovals.get so the server accepts the policy update.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ath TOCTOU

Implements the five approved fixes from the recent security review.

- canvas.navigate gates high-risk URLs (loopback / private / link-local /
  no-dot intranet / non-HTTPS / IP literal / Windows zone classifier
  via IInternetSecurityManager::MapUrlToZone) behind a Win32 MessageBox
  confirmation. The agent always receives the same success response
  immediately; prompt and browser launch run fire-and-forget so the
  user's decision and decision time are not exposed via timing.
- A2UI Video / AudioPlayer route through MediaResolver.ResolveMediaUriAsync
  for DNS resolution + non-public-IP rejection before MediaSource creation.
- SVG decode adds per-image cancellation, a process-wide concurrency
  semaphore, and a wall-clock timeout via linked CTS.
- MCP bearer token gets a Settings reset button and hot-swap on the
  running server; TokenSanitizer redacts bearer values from log + jsonl
  diagnostics; malformed-line logs are sanitized.
- canvas.a2ui.push jsonlPath rejects unresolved reparse points, opens
  the file once with FileStream + GetFinalPathNameByHandle to revalidate
  the resolved path is in temp, and enforces the size cap on the stream.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Implements the approved items from the merged worklist
(security threat model + multi-agent code review + third-reviewer pass).
Sequenced: merge-blockers first, then disposal/SSRF/protocol hardening,
then localization + a11y + perf + polish.

Tier 0 (merge blockers, all approved items):
- T0-1: SecretRedactor secret paths now ALWAYS dropped from action
  context, regardless of dataBinding=["/"] root wildcard.
- T0-2: SecretRedactor denylist expanded with apiKey/bearer/authorization/
  pin/otp/mfa/credential/session/cookie/auth/refresh/private/access;
  17 new unit tests.
- T0-3: NodeService._allowedNavigationHosts switched HashSet→
  ConcurrentDictionary (concurrent navigates corrupted the bucket).
- T0-5: SettingsWindow MCP token clipboard auto-clears after 30s if
  contents still match (Edge/Chrome password-manager pattern).
- T0-6: McpAuthToken sets explicit ACLs on token file + tray data
  directory (current user + SYSTEM + Administrators only) and verifies
  ownership on read; SettingsManager applies ACL to %APPDATA%\OpenClawTray.
- T0-7 + T0-N1: MCP token Settings UI fully localized to all 5 locales;
  reset action gated behind a Fluent ContentDialog confirmation
  (with Win32 MessageBox fallback) so a single click can't invalidate
  every configured local MCP client.
- T0-8: UrlNavigationApprovalService prompt body localized to all 5
  locales (caption, body, zone/agent/host lines, reasons header,
  yes/no hints).
- T0-9: CI coverage tool moved from coverlet.collector to
  dotnet-coverage so the integration tests' spawned tray exe gets
  instrumented (matches feedback_coverage_subprocess.md).

Tier 1 (resource hygiene + SSRF + action egress):
- T1-1/T1-2: MediaResolver and ActionDispatcher are now IDisposable;
  NodeService.Dispose tears them down. Closes HttpClient +
  SocketsHttpHandler connection pool and SemaphoreSlim handle leaks.
- T1-3: DisplayRenderers ImageRenderer disposes the prior timer-bound
  CancellationTokenSource and registers a surface-rebuild cleanup
  subscription so in-flight loads can't outlive the visual tree.
- T1-4 + T2-N5: Logger and DiagnosticsJsonlService rewritten on top of
  bounded Channel<string> + dedicated background writer (drop-oldest).
  Callers no longer block on disk I/O. Write failures bump
  Logger.LastWriteError and mirror to System.Diagnostics.Trace so a
  release build is no longer diagnostics-blind on disk-full / locked
  log dirs.
- T1-5: HttpUrlRiskEvaluator surfaces an IDN/punycode mismatch as a
  Reason so visually-deceptive hosts (e.g. Cyrillic 'а' apple.com) no
  longer clear RequiresConfirmation.
- T2-N4: HttpUrlRiskEvaluator.IsPublicAddress now rejects ::,
  IPv4-mapped/-compatible, 2001:db8::/32, 2001:0000::/32, 100::/64,
  and 2002::/16 (6to4); 11 new unit tests.
- T1-6: MediaResolver SocketsHttpHandler sets AllowAutoRedirect=false
  so an allowlisted host can't 30x to a non-allowlisted hostname.
- T1-7: canvas.navigate now pins DNS at approval time AND re-resolves
  immediately before launch; non-public results raise
  RequiresConfirmation. The decision is made off the request thread
  so the agent's response latency carries no signal of user choice.
- T1-8: New OpenClaw.Shared.UrlLogSanitizer drops query+fragment from
  any URL bound for disk-backed logs; applied to CanvasCapability
  navigate logs, UrlNavigationApprovalService decision log,
  NodeService deny log, and MediaResolver Truncate(). 14 unit tests.
- T1-10: AgentMessageFormatter moves default=update_canvas BEFORE the
  agent-controlled ctx={...} JSON so a hostile component can't shadow
  the sentinel via context values.
- T1-11: GatewayActionTransport.BuildAgentRequestPayload sanitizes the
  top-level sessionKey, not just the session= tag rendering, so the
  routing field can't carry path separators / control chars.
- T1-12: McpToolBridge gets curated descriptions for canvas.a2ui.dump
  (flagged as READ-ALL of every surface), canvas.caps,
  canvas.a2ui.pushJSONL.
- T1-N1: NodeService.OnPairingStatusChanged guards args.DeviceId
  against null/short — previous Substring(0,16) would throw out of the
  event handler and suppress PairingStatusChanged.
- T1-N3: McpAuthToken.LoadOrCreate distinguishes 'missing' from
  'unreadable' (throws the read failure instead of silently rotating
  the token) and uses the same temp-file+atomic-rename pattern as
  Reset for first-time creation.

Tier 2:
- T2-1: NodeService sealed.
- T2-21/22/23: McpHttpServer ReadBodyAsync uses ArrayPool<byte>;
  MaxConcurrentHandlers raised 8→16; AcceptLoopAsync waits 50ms
  instead of 0 so transient bursts get queued briefly.
- T2-34: A2UIRouter.GetOrCreateSurface returns null at the cap (was
  returning the first existing surface, which corrupted unrelated
  surface state).
- T2-35: Removed unreachable UrlNavigationApprovalDecisionKind.AllowHost
  enum value (Win32 prompt has no third button).
- T2-36/37: A2UI control fallback strings (Modal Close, Tab, Select
  date, Select...) and A2UICanvasWindow Title resourced to all 5
  locales; XAML literal Title="Canvas" removed so non-en-US users
  don't see a flash of English.
- T2-N1: A2UIProtocol.ParseComponent rejects non-object component
  payloads instead of silently substituting an empty JsonObject.
- T2-N2: Empty A2UI envelope objects return null instead of
  UnknownEnvelopeMessage{Kind=""}.
- T2-N3: A2UIRouter.DispatchToUI checks TryEnqueue's Boolean and logs
  on dispatcher unavailability instead of silently dropping pushes.
- T2-N6: A2UICanvasWindow.Closed unsubscribes router events and calls
  ResetAll so router state can't outlive the window.
- T2-N7: A2UICanvasWindow empty-state icon marked
  AccessibilityView=Raw; WaitingForContentText gets LiveSetting.
  IconRenderer no longer announces raw enum names (moreHoriz,
  accountCircle) to Narrator — falls back to AccessibilityView=Raw.

Tier 3 polish:
- T3-9: McpHttpServer rejects requests carrying Sec-Fetch-Site /
  Sec-Fetch-Mode / Referer (browser context indicators) even when
  Origin is absent.
- T3-10: McpHttpServer.IsHostAllowed accepts bracketed IPv6 loopback
  '[::1]:port'.
- T3-12: EmptySet hand-rolled IReadOnlySet replaced with shared
  FrozenSet<string>.Empty.
- T3-16: UnknownRenderer marked internal.

Worklist locations: C:\temp\merged-worklist-2026-04-28.md.
1290 unit tests pass; WinUI Debug win-x64 build clean.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…nTests

CI runs `dotnet build tests/OpenClaw.Tray.Tests -c Debug -r win-x64
--no-restore` (and similarly for IntegrationTests), but neither csproj
declared RuntimeIdentifiers. The top-level `dotnet restore` therefore
generated only default-RID assets, and the no-restore build then failed
with NETSDK1047 ("Assets file 'project.assets.json' doesn't have a
target for 'net10.0/win-x64'"). Run #25070772564 hit this on PR openclaw#239.

Declaring `<RuntimeIdentifiers>win-x64;win-arm64</RuntimeIdentifiers>`
makes restore generate the matching RID assets so the CI no-restore
build succeeds. Matches the existing pattern in Tray.UITests.csproj.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Two A2UI lifecycle bugs surfaced once we were running fully native:

1. Surfaces don't scroll. A2UI's mental model is the HTML body — agents
   build tall surfaces (e.g. the renderer matrix has six "distribution:*"
   sections stacked vertically) and rely on the document scrolling. A
   native WinUI ContentControl / TabViewItem doesn't scroll, so any
   surface taller than the viewport just clipped at the bottom.

   Wrap each surface's RootElement in a ScrollViewer
   (VerticalScrollBarVisibility=Auto, HorizontalScrollBarVisibility=
   Disabled, ZoomMode=Disabled) at mount time in A2UICanvasWindow,
   for both single- and multi-surface modes. Cache the wrapper per
   surfaceId so reconcile passes don't churn ScrollViewer instances
   (and lose scroll position) on every UpdateLayout. Drop stale
   wrappers when their surface is gone, and clear the cache on
   window close.

2. canvas.navigate dismisses the canvas. NodeService.LaunchAndCleanup
   unconditionally closed both the WebView2 and the A2UI canvas window
   after Process.Start. That was a holdover from when canvas == WebView2
   and "navigate" semantically meant "you don't need this frame anymore,
   here's where to go." With native A2UI the canvas is a control surface
   (dashboard / launcher), not a browser frame; clicking a link in a
   dashboard shouldn't nuke the dashboard, the same way clicking a link
   in the Start Menu doesn't close the Start Menu.

   Drop the auto-close. Rename LaunchAndCleanup → LaunchInDefaultBrowser
   to match what it actually does. Agents that want explicit teardown
   after navigate can call canvas.hide or emit deleteSurface — those
   tools already exist and that's where lifecycle decisions belong.

   Behavior-change risk is small: agents that expected the canvas to
   close after navigate will see it stay open (mildly surprising, not
   breaking), whereas the previous behavior actively broke the
   launcher / control-panel pattern, which is the much wider use case.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
PR openclaw#239's UI test job failed all 62 tests with
"Package dependency criteria could not be resolved" out of
MddBootstrapInitialize2. The hosted windows-2025 runner doesn't
preinstall the Microsoft.WindowsAppRuntime.1.8 framework MSIX,
and the test fixture was calling Bootstrap.Initialize expecting
a system-registered runtime.

Mirror the main app's deployment: set
WindowsAppSDKSelfContained=true + WindowsPackageType=None on the
test project so the runtime DLLs ship next to the test host and
the bootstrapper resolves them locally. Suppress the SDK's
auto-init source injection (WindowsAppSDKBootstrapInitialize/
UndockedRegFreeWinRTInitialize=false) since the referenced
OpenClaw.Tray.WinUI assembly already carries a compiled-in copy —
re-injection caused CS0436 type conflicts.

Devs cloning the repo no longer need a separate WinAppRuntime
install either.

Verified locally: 62/62 UI tests pass.
@codemonkeychris

Copy link
Copy Markdown
Contributor Author

CI failure diagnosis + fix

The previous CI fix (e0b8d0d) cleared the build error (NETSDK1047 / RID assets) but exposed a latent runtime bug: all 62 UI tests crashed in UIThreadFixture with

System.Runtime.InteropServices.COMException : Package dependency criteria could not be resolved.
   at MddBootstrapInitialize2_Throw(...)
   at Bootstrap.Initialize(0x00010008, "")
   at OpenClaw.Tray.UITests.UIThreadFixture.UIThreadProc()  // line 138

Root cause. The fixture calls Bootstrap.Initialize(1.8, stable) expecting a system-registered Microsoft.WindowsAppRuntime.1.8 framework MSIX. The hosted windows-2025 runner doesn't preinstall it. The misleading inline comment at .github/workflows/ci.yml:108-110 had been incorrectly assuming otherwise.

Fix (this commit). Mirror the main app's deployment shape on the UI test project — WindowsAppSDKSelfContained=true + WindowsPackageType=None. Runtime DLLs ship next to the test host and Bootstrap.Initialize resolves them locally, no system MSIX required. Also suppresses the SDK's auto-init source injection (WindowsAppSDKBootstrapInitialize / UndockedRegFreeWinRTInitialize=false) because the referenced OpenClaw.Tray.WinUI assembly already compiles in the same auto-initializer — re-injection caused CS0436 type conflicts.

Verified locally: 62/62 UI tests pass in 1m02s. Net effect for devs cloning the repo: no separate WinAppRuntime install needed.

The previous attempt (6c81899) tried to put the WinAppSDK into
self-contained mode for the UI tests, but that doesn't survive the
xunit testhost: testhost.exe runs from the .NET SDK directory, not
the test bin folder, so the SDK's P/Invoke-based auto-initializer
can't probe for Microsoft.WindowsAppRuntime.dll. Bootstrap.Initialize
kept failing because the runner image doesn't have a system
WindowsAppRuntime registered.

Reverting to framework-dependent deployment for the test project and
installing the matching runtime explicitly in CI. Version pinned to
1.8.260101001 so the framework MSIX major.minor matches the
NuGet-referenced Microsoft.WindowsAppSDK.

Verified locally: 62/62 UI tests pass.
@codemonkeychris

Copy link
Copy Markdown
Contributor Author

CI green ✅ (commit b94e3cd)

Self-contained deployment (the previous attempt at 6c81899) didn't survive the xunit testhost on CI: testhost.exe runs from the .NET SDK directory, not the test bin folder, so the WinAppSDK's P/Invoke-based UndockedRegFreeWinRT auto-initializer can't probe for Microsoft.WindowsAppRuntime.dll. Reverted that and went with the simpler runner-side fix instead — added an explicit "Install WindowsAppRuntime 1.8" step in ci.yml (version pinned to 1.8.260101001 to match the NuGet reference) and restored the Bootstrap.Initialize call in UIThreadFixture.

All checks now passing — UI test job runs in ~4 min and reports 62/62 pass.

@shanselman

Copy link
Copy Markdown
Collaborator

Thanks Chris — this is a substantial piece of work, and the CI/test coverage here is impressive. I did two review passes (Opus 4.7 + GPT-5.5). Below is the short list of actionable feedback. I’m intentionally not bikeshedding the size/style because I know we want to move fast.

High-confidence items — both review passes agree these are worth fixing or explicitly scoping before merge:

  1. Video/Audio media DNS-rebinding claim does not match the implementation.

    • MediaResolver does the strong protected path for images via the safe HttpClient/connect callback, but ResolveMediaUriAsync returns the original hostname URI for video/audio and DisplayRenderers passes that to MediaSource.CreateFromUri(...).
    • That means the Windows media stack performs its own DNS lookup at playback time, outside the resolver, so validation-time DNS and playback-time DNS can differ.
    • Suggested fix: either route video/audio through a safe local/proxy fetch path, disable/limit remote video/audio for now, or adjust the PR description/docs to say DNS-rebinding protection applies to image fetches but not MediaSource-backed playback.
  2. canvas.navigate approval prompts can be spammed.

    • NodeService.OnCanvasNavigate starts a fire-and-forget task per request, and each risky URL can create a topmost foreground MessageBoxW.
    • A local MCP client/agent with a valid token could loop canvas.navigate and stack prompts, creating a UX DoS / habituation problem.
    • Suggested fix: serialize prompts globally, coalesce repeats per HostKey while one is pending, and add a short deny cooldown / rate limit so repeated denied hosts auto-deny without more prompts.
  3. The pre-launch DNS re-check is best-effort only, not a real pin.

    • The code re-resolves immediately before launch, then launches the original hostname URL in the default browser. The browser does its own DNS lookup, so the re-check does not actually constrain the final network target.
    • Suggested fix: either remove that re-check or update comments/docs to describe it as best-effort triage only, with the browser as the trust boundary.

Medium-confidence / small follow-ups — one pass found these; they look easy to address but I wouldn’t block the whole PR on them if the high-confidence items are handled:

  1. McpAuthToken.Reset can orphan the staged temp token file on failure.

    • LoadOrCreate already has the try/catch cleanup pattern; Reset should mirror it around WriteAllText / ACL / File.Move.
  2. Narrow auth token hot-swap race in McpHttpServer.

    • HandleAsync checks _authToken != null directly, while IsAuthorized uses Volatile.Read.
    • Suggested fix: snapshot the auth token once with Volatile.Read(ref _authToken) at the top of request handling and use that consistently for the null test and compare.

Overall: the direction looks strong and the security work is thoughtful. I’d be comfortable moving fast once #1 and #2 are fixed, and #3 is either corrected or documented honestly.

…ate-limit, auth-token race

Addresses Scott's two-pass review (Opus 4.7 + GPT-5.5) on PR openclaw#239.

- MediaResolver: drop the DNS-rebinding claim for Video/AudioPlayer.
  ResolveMediaUriAsync re-resolved DNS but then handed the hostname to
  MediaSource.CreateFromUri, which re-resolves at playback — the pin
  never held for media. Replace with sync TryResolveMediaUri (HTTPS +
  allowlist + IP-literal-public). Image fetches keep the
  SocketsHttpHandler.ConnectCallback pin. Docs corrected to scope the
  guarantee.
- NodeService: serialize canvas.navigate prompts behind a global
  semaphore, coalesce concurrent prompts per HostKey via a shared TCS,
  and silently auto-deny for 30s after a Deny. A token-holding agent
  looping canvas.navigate can no longer stack topmost MessageBoxes.
- NodeService: drop the misleading pre-launch DNS re-check. The OS
  browser does its own resolution when handed the URL, so a second
  resolve here was best-effort triage at best — comment now says so.
- McpAuthToken.Reset: wrap WriteAllText/ACL/Move in try/catch with
  temp-file cleanup, mirroring LoadOrCreate.
- McpHttpServer.HandleAsync: snapshot _authToken once via Volatile.Read
  and pass to a now-static IsAuthorized, so the null-test and the
  comparison cannot observe two different values across a hot-swap.

Tests: Shared 1045, Tray 245, Integration 18, UI 62 — all green.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@codemonkeychris

Copy link
Copy Markdown
Contributor Author

Review pass addressed (commit 108aba9)

Thanks @shanselman — went through both passes' findings. All five addressed:

1. Video/Audio DNS-rebinding claim. Confirmed the gap: ResolveMediaUriAsync validated DNS but then passed the hostname URI to MediaSource.CreateFromUri, which performs its own resolution at playback — the pin never held for media. Took the docs/scope path: replaced the async method with sync TryResolveMediaUri (HTTPS+allowlist + IP-literal-public check, no DNS lookup), and updated components.md, data-and-actions.md, grading.md, and README.md to scope the DNS-rebinding guarantee to image fetches only. Image path keeps the SocketsHttpHandler.ConnectCallback pin (where it actually holds). A local-proxy approach for media was scoped out — narrow threat (allowlisted host turns hostile via DNS) vs. significant code; can revisit if the threat model shifts.

2. canvas.navigate prompt spam. Added in NodeService:

  • SemaphoreSlim(1,1) to serialize prompts globally — no overlapping topmost MessageBoxes across hosts.
  • ConcurrentDictionary<HostKey, Task<Decision>> to coalesce concurrent requests for the same host onto a shared TCS — second call inherits the first prompt's decision.
  • 30-second deny cooldown per HostKey — repeated requests for a denied host auto-deny silently. Re-checked under the gate too, so a queued request can't escape a Deny that landed while it was waiting.

Net effect: a token-holding agent looping canvas.navigate can produce at most one prompt every 30s per host, regardless of call rate. Agent still gets immediate "browser" success on every call.

3. Pre-launch DNS re-check. Agree — re-resolving here doesn't constrain the browser's lookup. Removed the second EnrichWithDnsRiskAsync and its abort branch. Updated the comment to be honest: validation-time DNS is best-effort triage; the OS browser is the actual trust boundary.

4. McpAuthToken.Reset orphan temp file. Mirrored the LoadOrCreate pattern — WriteAllText/ACL/Move wrapped in try/catch that deletes the temp file on failure and rethrows.

5. McpHttpServer token snapshot race. Snapshot _authToken once via Volatile.Read at the top of HandleAsync; pass it explicitly to a now-static IsAuthorized(authToken, header). Single read, no possibility of the null-test and the compare seeing different values.

Tests (re-run locally, all green):

  • OpenClaw.Shared.Tests — 1045 passed
  • OpenClaw.Tray.Tests — 245 passed
  • OpenClaw.Tray.IntegrationTests (OPENCLAW_RUN_INTEGRATION=1) — 18 passed
  • OpenClaw.Tray.UITests — 62 passed

Integration tests exercise canvas.navigate through the real tray process, but they should not open the developer or CI machine's default browser. Start the isolated test tray with OPENCLAW_SUPPRESS_EXTERNAL_BROWSER=1 and have NodeService log/skip ShellExecute only when that explicit test flag is present.

Production canvas.navigate behavior remains unchanged because the flag is absent by default.

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

Copy link
Copy Markdown
Collaborator

Added one small test-safety commit on top of Chris's review-pass fixes: 394695c test: suppress browser launch in integration tray.

Why: the integration suite exercises canvas.navigate through a real spawned tray process, so the previous https://example.com/ test input could open the developer/CI machine's default browser. The test should verify the MCP/tray wiring without creating external desktop side effects.

What changed:

  • the integration fixture starts the isolated tray with OPENCLAW_SUPPRESS_EXTERNAL_BROWSER=1
  • NodeService logs and skips the ShellExecute browser launch only when that explicit flag is present
  • production canvas.navigate behavior is unchanged because the flag is absent by default

Local validation after the commit:

  • ./build.ps1 passed
  • OpenClaw.Shared.Tests: 1045 passed / 20 skipped
  • OpenClaw.Tray.Tests: 245 passed
  • OpenClaw.Tray.IntegrationTests -r win-arm64: 18 passed
  • OpenClaw.Tray.UITests -r win-arm64: 62 passed

@shanselman

Copy link
Copy Markdown
Collaborator

Thanks @codemonkeychris — this is a big, valuable step for Windows parity. I verified the final branch locally after the browser-launch test-safety patch:

  • ./build.ps1 passed
  • OpenClaw.Shared.Tests: 1045 passed / 20 skipped
  • OpenClaw.Tray.Tests: 245 passed
  • OpenClaw.Tray.IntegrationTests -r win-arm64: 18 passed
  • OpenClaw.Tray.UITests -r win-arm64: 62 passed
  • local smoke demo rendered a native A2UI surface through canvas.a2ui.push in a real tray process

CI is green on the final commit. Merging now with appreciation.

@shanselman
shanselman merged commit 9fa43f3 into openclaw:master Apr 30, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants