Fix canvas navigation URL handling (#711)

liorb-mountapps · shanselman · Copilot · web-flow · commit c338d4111dc5 · 2026-06-25T17:31:57.000-07:00
* Fix canvas navigation URL handling

* Fix canvas navigate dispatcher timeout

Route canvas.navigate through the same dispatcher timeout guard used by canvas.eval and canvas.snapshot so stalled UI dispatch cannot hang MCP tool calls indefinitely.

Co-authored-by: Copilot &lt;223556219+Copilot@users.noreply.github.com&gt;

---------

Co-authored-by: Scott Hanselman &lt;scott@hanselman.com&gt;
Co-authored-by: Copilot &lt;223556219+Copilot@users.noreply.github.com&gt;
diff --git a/src/OpenClaw.Shared/Capabilities/CanvasCapability.cs b/src/OpenClaw.Shared/Capabilities/CanvasCapability.cs
@@ -212,6 +212,10 @@ private async Task<NodeInvokeResponse> HandleNavigateAsync(NodeInvokeRequest req
             // opener is the subscriber's word for how it serviced the request:
             // "canvas" (existing WebView2 frame), "browser" (default browser),
             // or anything else the subscriber wants to surface back to the agent.
+            if (string.Equals(opener, "denied", StringComparison.OrdinalIgnoreCase) ||
+                string.Equals(opener, "unsupported_in_canvas", StringComparison.OrdinalIgnoreCase))
+                return Success(new { navigated = false, opener, url = canonical });
+
             return Success(new { navigated = true, opener, url = canonical });
         }
         catch (Exception ex)
diff --git a/src/OpenClaw.Tray.WinUI/Helpers/CanvasGatewayUrlRewriter.cs b/src/OpenClaw.Tray.WinUI/Helpers/CanvasGatewayUrlRewriter.cs
@@ -0,0 +1,43 @@
+using OpenClaw.Shared;
+
+namespace OpenClawTray.Helpers;
+
+internal static class CanvasGatewayUrlRewriter
+{
+    public static string? ToHttpOrigin(string? gatewayUrl)
+    {
+        if (string.IsNullOrWhiteSpace(gatewayUrl))
+            return null;
+
+        var uri = new Uri(GatewayUrlHelper.NormalizeForWebSocket(gatewayUrl));
+        var httpScheme = uri.Scheme == "wss" ? "https" : "http";
+        return $"{httpScheme}://{uri.Host}:{uri.Port}";
+    }
+
+    public static string Rewrite(string url, string? effectiveGatewayOrigin, string? configuredGatewayOrigin)
+    {
+        if (string.IsNullOrEmpty(effectiveGatewayOrigin))
+            return url;
+
+        if (url.StartsWith("/", StringComparison.Ordinal))
+            return effectiveGatewayOrigin + url;
+
+        var uri = new Uri(url);
+        var urlOrigin = $"{uri.Scheme}://{uri.Host}:{uri.Port}";
+
+        if (IsGatewayOrigin(urlOrigin, effectiveGatewayOrigin, configuredGatewayOrigin) &&
+            !urlOrigin.Equals(effectiveGatewayOrigin, StringComparison.OrdinalIgnoreCase))
+        {
+            return effectiveGatewayOrigin + uri.PathAndQuery;
+        }
+
+        return url;
+    }
+
+    private static bool IsGatewayOrigin(string urlOrigin, string effectiveGatewayOrigin, string? configuredGatewayOrigin)
+    {
+        return urlOrigin.Equals(effectiveGatewayOrigin, StringComparison.OrdinalIgnoreCase) ||
+               (!string.IsNullOrEmpty(configuredGatewayOrigin) &&
+                urlOrigin.Equals(configuredGatewayOrigin, StringComparison.OrdinalIgnoreCase));
+    }
+}
diff --git a/src/OpenClaw.Tray.WinUI/Services/NodeService.cs b/src/OpenClaw.Tray.WinUI/Services/NodeService.cs
@@ -1061,7 +1061,7 @@ private void OnCanvasPresent(object? sender, CanvasPresentArgs args)
                 if (_canvasWindow == null || _canvasWindow.IsClosed)
                 {
                     _canvasWindow = new CanvasWindow();
-                    _canvasWindow.SetTrustedGatewayOrigin(GatewayUrl, _token);
+                    _canvasWindow.SetTrustedGatewayOrigin(GatewayUrl, _token, GetConfiguredGatewayUrl());
                 }
 
                 // Configure window
@@ -1134,62 +1134,52 @@ private void OnCanvasHide(object? sender, EventArgs args)
     }
     
     /// <summary>
-    /// Service a <c>canvas.navigate</c> request by launching the URL in the
-    /// OS default browser. Always — even if a WebView2 canvas window is open.
-    /// Rationale: "open this link" on Windows means the default browser, and
-    /// the embedded WebView2 canvas runs URL-rewriting (gateway-origin pinning,
-    /// CSP, etc.) that mangles arbitrary external URLs. Agents that want to
-    /// load a page inside an embedded surface should use <c>canvas.present</c>.
-    ///
-    /// Open canvas windows are NOT closed after navigate. A2UI surfaces are
-    /// control panels / dashboards / launchers, not browser frames; clicking a
-    /// link inside one shouldn't dismiss it any more than clicking a link in
-    /// the Start Menu would. Agents that want explicit teardown should call
-    /// <c>canvas.hide</c> or emit <c>deleteSurface</c>.
-    ///
-    /// CanvasCapability has already validated the URL with HttpUrlValidator;
-    /// we re-validate here as defense-in-depth so the OS-level shell-execute
-    /// can never see an unvetted string.
+    /// Service a <c>canvas.navigate</c> request inside the WebView canvas.
+    /// CanvasCapability has already validated the URL; re-validate here before
+    /// handing it to WebView2.
     /// </summary>
-    private Task<string> OnCanvasNavigate(string url)
+    private async Task<string> OnCanvasNavigate(string url)
     {
         if (!HttpUrlValidator.TryParse(url, out var canonical, out var validationError))
         {
             _logger.Warn($"OnCanvasNavigate rejected (validator): {validationError}");
             throw new InvalidOperationException($"Invalid url: {validationError}");
         }
 
-        var initialRisk = HttpUrlRiskEvaluator.Evaluate(canonical!);
+        var risk = await EnrichWithDnsRiskAsync(HttpUrlRiskEvaluator.Evaluate(canonical!)).ConfigureAwait(false);
+        if (risk.RequiresConfirmation)
+        {
+            _logger.Warn($"Canvas navigate unsupported in canvas: {OpenClaw.Shared.UrlLogSanitizer.Sanitize(risk.CanonicalOrigin)}");
+            return "unsupported_in_canvas";
+        }
 
-        // Move the entire decision off the request thread so the agent's
-        // response latency carries no signal about the user's decision (see
-        // long comment retained below). DNS resolution + prompt + launch all
-        // run from the worker.
-        _ = Task.Run(async () =>
+        var tcs = new TaskCompletionSource<string>(TaskCreationOptions.RunContinuationsAsynchronously);
+        var cts = new CancellationTokenSource();
+        if (!_dispatcherQueue.TryEnqueue(() =>
         {
+            if (cts.IsCancellationRequested) return;
             try
             {
-                // Best-effort triage: resolve DNS now so a hostname pointing at
-                // an internal IP raises the prompt. This is NOT a pin on the
-                // launched request — the OS browser performs its own DNS
-                // resolution when handed the URL, so the actual trust boundary
-                // is the user's browser zone/proxy config. A second resolve
-                // immediately before ShellExecute would not change that.
-                var pinnedRisk = await EnrichWithDnsRiskAsync(initialRisk).ConfigureAwait(false);
-                if (await ShouldLaunchAfterPromptAsync(pinnedRisk).ConfigureAwait(false))
-                    LaunchInDefaultBrowser(canonical!);
+                CloseA2UICanvasWindow();
+                EnsureCanvasWindow();
+                if (_canvasWindow == null)
+                    throw new InvalidOperationException("Canvas window unavailable");
+
+                _canvasWindow.Navigate(canonical!);
+                _canvasWindow.BringToFront(false);
+                _logger.Info($"Canvas navigate -> canvas: {OpenClaw.Shared.UrlLogSanitizer.Sanitize(canonical)}");
+                tcs.TrySetResult("canvas");
             }
             catch (Exception ex)
             {
-                _logger.Error("Canvas navigate (deferred) failed", ex);
+                tcs.TrySetException(ex);
             }
-        });
+        }))
+        {
+            tcs.TrySetException(new InvalidOperationException("Failed to dispatch canvas.navigate to UI thread"));
+        }
 
-        // The agent gets the same response shape and the same response time
-        // whether or not a confirmation prompt is needed. If we awaited the
-        // prompt here, response latency would leak the user's decision time
-        // (or even the existence of a prompt).
-        return Task.FromResult("browser");
+        return await WaitWithTimeout(tcs.Task, cts, "canvas.navigate");
     }
 
     /// <summary>
@@ -1263,7 +1253,7 @@ private async Task<bool> ShouldLaunchAfterPromptAsync(HttpUrlRiskProfile pinnedR
                 if (decision.Kind == UrlNavigationApprovalDecisionKind.Deny)
                 {
                     _navigationDenyCooldown[pinnedRisk.HostKey] = DateTimeOffset.UtcNow + NavigationDenyCooldownDuration;
-                    _logger.Warn($"Canvas navigate denied: {OpenClaw.Shared.UrlLogSanitizer.Sanitize(pinnedRisk.CanonicalOrigin)} ({decision.Reason ?? "user denied"}); already reported success to agent");
+                    _logger.Warn($"Canvas navigate denied before WebView navigation: {OpenClaw.Shared.UrlLogSanitizer.Sanitize(pinnedRisk.CanonicalOrigin)} ({decision.Reason ?? "user denied"})");
                     return false;
                 }
                 // AllowHost (session-allowlist) is currently unreachable from
@@ -1567,11 +1557,13 @@ private void EnsureCanvasWindow()
         if (_canvasWindow == null || _canvasWindow.IsClosed)
         {
             _canvasWindow = new CanvasWindow();
-            _canvasWindow.SetTrustedGatewayOrigin(GatewayUrl, _token);
+            _canvasWindow.SetTrustedGatewayOrigin(GatewayUrl, _token, GetConfiguredGatewayUrl());
         }
         _canvasWindow?.Activate();
     }
 
+    private string? GetConfiguredGatewayUrl() => _activeGatewayUrlResolver?.Invoke();
+
     // Mutable context shared with GatewayActionTransport. SessionKey is updated
     // from push props (when the agent supplies one); host/instance stay tied to
     // the node client identity. Default sessionKey is "main", matching Android's
diff --git a/src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs b/src/OpenClaw.Tray.WinUI/Windows/CanvasWindow.xaml.cs
@@ -133,6 +133,7 @@ private static bool IsSafeDataUrl(string url)
     
     public bool IsClosed { get; private set; }
     private string? _trustedGatewayOrigin;
+    private string? _configuredGatewayOrigin;
     private string? _gatewayOriginForRewrite;
     private string? _gatewayToken;
 
@@ -142,17 +143,18 @@ private static bool IsSafeDataUrl(string url)
     /// Also rewrites gateway URLs to use the node's effective connection
     /// (e.g., localhost when connected via SSH tunnel).
     /// </summary>
-    public void SetTrustedGatewayOrigin(string? gatewayUrl, string? token = null)
+    public void SetTrustedGatewayOrigin(string? gatewayUrl, string? token = null, string? configuredGatewayUrl = null)
     {
         if (string.IsNullOrEmpty(gatewayUrl)) return;
         _gatewayToken = token;
         try
         {
-            var uri = new Uri(GatewayUrlHelper.NormalizeForWebSocket(gatewayUrl));
-            var httpScheme = uri.Scheme == "wss" ? "https" : "http";
-            _trustedGatewayOrigin = $"{httpScheme}://{uri.Host}:{uri.Port}";
+            _trustedGatewayOrigin = CanvasGatewayUrlRewriter.ToHttpOrigin(gatewayUrl);
+            _configuredGatewayOrigin = string.IsNullOrWhiteSpace(configuredGatewayUrl)
+                ? _trustedGatewayOrigin
+                : CanvasGatewayUrlRewriter.ToHttpOrigin(configuredGatewayUrl);
             _gatewayOriginForRewrite = _trustedGatewayOrigin;
-            Logger.Info($"[Canvas] Trusted gateway origin: {_trustedGatewayOrigin}");
+            Logger.Info($"[Canvas] Trusted gateway origin: {_trustedGatewayOrigin}; configured gateway origin: {_configuredGatewayOrigin}");
             ConfigureGatewayAuthHeaderInjection();
         }
         catch (Exception ex)
@@ -172,24 +174,13 @@ private string RewriteGatewayUrl(string url)
         try
         {
             // Handle relative paths — prepend the gateway origin
-            if (url.StartsWith("/"))
+            var rewritten = CanvasGatewayUrlRewriter.Rewrite(url, _gatewayOriginForRewrite, _configuredGatewayOrigin);
+            if (!string.Equals(url, rewritten, StringComparison.Ordinal))
             {
-                var rewritten = _gatewayOriginForRewrite + url;
                 rewritten = AppendGatewayToken(rewritten);
-                Logger.Info($"[Canvas] Resolved relative URL to gateway origin");
-                return rewritten;
-            }
-
-            var uri = new Uri(url);
-            var httpScheme = uri.Scheme;
-            var urlOrigin = $"{httpScheme}://{uri.Host}:{uri.Port}";
-
-            // If the URL's origin differs from our effective gateway origin, rewrite it
-            if (!urlOrigin.Equals(_gatewayOriginForRewrite, StringComparison.OrdinalIgnoreCase))
-            {
-                var rewritten = _gatewayOriginForRewrite + uri.PathAndQuery;
-                rewritten = AppendGatewayToken(rewritten);
-                Logger.Info($"[Canvas] Rewrote URL to effective gateway origin");
+                Logger.Info(url.StartsWith("/", StringComparison.Ordinal)
+                    ? "[Canvas] Resolved relative URL to gateway origin"
+                    : "[Canvas] Rewrote URL to effective gateway origin");
                 return rewritten;
             }
 
@@ -548,6 +539,7 @@ private void OnWindowClosed(object sender, WindowEventArgs args)
         _canvasWatcher?.Dispose();
         _canvasWatcher = null;
         _trustedGatewayOrigin = null;
+        _configuredGatewayOrigin = null;
         _gatewayOriginForRewrite = null;
     }
     
diff --git a/tests/OpenClaw.Shared.Tests/CapabilityTests.cs b/tests/OpenClaw.Shared.Tests/CapabilityTests.cs
@@ -1688,6 +1688,28 @@ public async Task Navigate_ResponseIncludesOpenerAndCanonicalUrl()
         Assert.Contains("\"url\":\"https://example.com/Path\"", json);
     }
 
+    [Theory]
+    [InlineData("denied")]
+    [InlineData("unsupported_in_canvas")]
+    public async Task Navigate_NotOpenedByHandler_ReturnsNotNavigated(string opener)
+    {
+        var cap = new CanvasCapability(NullLogger.Instance);
+        cap.NavigateRequested += _ => Task.FromResult(opener);
+
+        var req = new NodeInvokeRequest
+        {
+            Id = "c12b-denied",
+            Command = "canvas.navigate",
+            Args = Parse("""{"url":"http://127.0.0.1:9/"}""")
+        };
+        var res = await cap.ExecuteAsync(req);
+        Assert.True(res.Ok);
+
+        var json = System.Text.Json.JsonSerializer.Serialize(res.Payload);
+        Assert.Contains($"\"opener\":\"{opener}\"", json);
+        Assert.Contains("\"navigated\":false", json);
+    }
+
     [Theory]
     [InlineData("javascript:alert(1)")]
     [InlineData("file:///C:/Windows/System32/calc.exe")]
diff --git a/tests/OpenClaw.Tray.Tests/CanvasGatewayUrlRewriterTests.cs b/tests/OpenClaw.Tray.Tests/CanvasGatewayUrlRewriterTests.cs
@@ -0,0 +1,46 @@
+using OpenClawTray.Helpers;
+
+namespace OpenClaw.Tray.Tests;
+
+public class CanvasGatewayUrlRewriterTests
+{
+    [Fact]
+    public void Rewrite_LeavesExternalUrlUntouched()
+    {
+        var rewritten = CanvasGatewayUrlRewriter.Rewrite(
+            "https://example.com/path?q=1",
+            "http://localhost:18789",
+            "https://gateway.example");
+
+        Assert.Equal("https://example.com/path?q=1", rewritten);
+    }
+
+    [Fact]
+    public void Rewrite_MapsConfiguredGatewayOriginToEffectiveTunnelOrigin()
+    {
+        var rewritten = CanvasGatewayUrlRewriter.Rewrite(
+            "https://gateway.example/__openclaw__/a2ui/?session=main",
+            CanvasGatewayUrlRewriter.ToHttpOrigin("ws://localhost:18789"),
+            CanvasGatewayUrlRewriter.ToHttpOrigin("wss://gateway.example"));
+
+        Assert.Equal("http://localhost:18789/__openclaw__/a2ui/?session=main", rewritten);
+    }
+
+    [Fact]
+    public void Rewrite_MapsRelativePathToEffectiveGatewayOrigin()
+    {
+        var rewritten = CanvasGatewayUrlRewriter.Rewrite(
+            "/__openclaw__/a2ui/",
+            "http://localhost:18789",
+            "https://gateway.example");
+
+        Assert.Equal("http://localhost:18789/__openclaw__/a2ui/", rewritten);
+    }
+
+    [Fact]
+    public void ToHttpOrigin_NormalizesWebSocketUrls()
+    {
+        Assert.Equal("https://gateway.example:443", CanvasGatewayUrlRewriter.ToHttpOrigin("wss://gateway.example"));
+        Assert.Equal("http://localhost:18789", CanvasGatewayUrlRewriter.ToHttpOrigin("ws://localhost:18789"));
+    }
+}
diff --git a/tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj b/tests/OpenClaw.Tray.Tests/OpenClaw.Tray.Tests.csproj
@@ -65,6 +65,7 @@
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Services\ToastActivationRouter.cs" Link="Services\ToastActivationRouter.cs" />
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Services\AppNotificationService.cs" Link="Services\AppNotificationService.cs" />
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Services\UsageCostApplicationPolicy.cs" Link="Services\UsageCostApplicationPolicy.cs" />
+    <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Helpers\CanvasGatewayUrlRewriter.cs" Link="Helpers\CanvasGatewayUrlRewriter.cs" />
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Helpers\TrayTooltipFormatter.cs" Link="Helpers\TrayTooltipFormatter.cs" />
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Helpers\GatewayDashboardUrlBuilder.cs" Link="Helpers\GatewayDashboardUrlBuilder.cs" />
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Services\TrayStateSnapshot.cs" Link="Services\TrayStateSnapshot.cs" />
diff --git a/tests/OpenClaw.Tray.Tests/TrayMenuWindowMarkupTests.cs b/tests/OpenClaw.Tray.Tests/TrayMenuWindowMarkupTests.cs
@@ -34,6 +34,58 @@ public void CanvasWindow_CleansUpGatewayAuthWebResourceHandler()
         Assert.DoesNotContain("WebResourceRequested += (", source);
     }
 
+    [Fact]
+    public void CanvasWindow_RewritesOnlyGatewayUrls()
+    {
+        var source = Read(
+            "src",
+            "OpenClaw.Tray.WinUI",
+            "Windows",
+            "CanvasWindow.xaml.cs");
+
+        Assert.Contains("_configuredGatewayOrigin", source);
+        Assert.Contains("CanvasGatewayUrlRewriter.Rewrite(url", source);
+        Assert.DoesNotContain("if (!urlOrigin.Equals(_gatewayOriginForRewrite", source);
+    }
+
+    [Fact]
+    public void CanvasNavigate_UsesCanvasWindow()
+    {
+        var source = Read(
+            "src",
+            "OpenClaw.Tray.WinUI",
+            "Services",
+            "NodeService.cs");
+
+        Assert.Contains("request inside the WebView canvas", source);
+        Assert.Contains("HttpUrlRiskEvaluator.Evaluate(canonical!)", source);
+        Assert.Contains("EnrichWithDnsRiskAsync", source);
+        Assert.Contains("risk.RequiresConfirmation", source);
+        Assert.Contains("return \"unsupported_in_canvas\"", source);
+        Assert.DoesNotContain("ShouldLaunchAfterPromptAsync(risk)", source);
+        Assert.Contains("_canvasWindow.Navigate(canonical!)", source);
+        Assert.Contains("tcs.TrySetResult(\"canvas\")", source);
+        Assert.Contains("Canvas navigate -> canvas", source);
+    }
+
+    [Fact]
+    public void CanvasGatewayOrigin_ComesFromActiveGatewayRecord()
+    {
+        var appSource = Read(
+            "src",
+            "OpenClaw.Tray.WinUI",
+            "App.xaml.cs");
+        var nodeServiceSource = Read(
+            "src",
+            "OpenClaw.Tray.WinUI",
+            "Services",
+            "NodeService.cs");
+
+        Assert.Contains("activeGatewayUrlResolver: () => _gatewayRegistry?.GetActive()?.Url", appSource);
+        Assert.Contains("private string? GetConfiguredGatewayUrl() => _activeGatewayUrlResolver?.Invoke();", nodeServiceSource);
+        Assert.DoesNotContain("private string? GetConfiguredGatewayUrl() => _settings?.UseSshTunnel", nodeServiceSource);
+    }
+
     [Fact]
     public void Source_DoesNotDeclareAsyncVoidHandlers()
     {
