Fix canvas navigation URL handling

liorb-mountapps · liorb-mountapps · commit 764265df5e13 · 2026-06-10T14:51:19.000+03:00
diff --git a/src/OpenClaw.Shared/Capabilities/CanvasCapability.cs b/src/OpenClaw.Shared/Capabilities/CanvasCapability.cs
@@ -212,6 +212,9 @@ 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))
+                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/App.xaml.cs b/src/OpenClaw.Tray.WinUI/App.xaml.cs
@@ -1715,7 +1715,8 @@ private void OnManagerStateChanged(object? sender, GatewayConnectionSnapshot sna
                 () => _keepAliveWindow?.Content as FrameworkElement,
                 settings,
                 enableMcpServer: settings.EnableMcpServer,
-                identityDataPath: IdentityDataPath);
+                identityDataPath: IdentityDataPath,
+                configuredGatewayUrlProvider: () => _gatewayRegistry?.GetActive()?.Url);
             _nodeService.StatusChanged += OnNodeStatusChanged;
             _nodeService.NotificationRequested += OnNodeNotificationRequested;
             _nodeService.ToastRequested += OnNodeToastRequested;
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
@@ -25,6 +25,7 @@ public sealed class NodeService : IDisposable, IAsyncDisposable
     private readonly IOpenClawLogger _logger;
     private readonly DispatcherQueue _dispatcherQueue;
     private readonly Func<FrameworkElement?> _rootProvider;
+    private readonly Func<string?>? _configuredGatewayUrlProvider;
     private readonly SettingsManager? _settings;
     private readonly SemaphoreSlim _consentLock = new(1, 1);
     private readonly object _disposeLock = new();
@@ -188,13 +189,15 @@ public NodeService(
         Func<FrameworkElement?>? rootProvider = null,
         SettingsManager? settings = null,
         bool enableMcpServer = false,
-        string? identityDataPath = null)
+        string? identityDataPath = null,
+        Func<string?>? configuredGatewayUrlProvider = null)
     {
         _logger = logger;
         _dispatcherQueue = dispatcherQueue;
         _dataPath = dataPath;
         _identityDataPath = string.IsNullOrWhiteSpace(identityDataPath) ? dataPath : identityDataPath;
         _rootProvider = rootProvider ?? (() => null);
+        _configuredGatewayUrlProvider = configuredGatewayUrlProvider;
         _settings = settings;
         _enableMcpServer = enableMcpServer;
         _screenCaptureService = new ScreenCaptureService(logger);
@@ -860,7 +863,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
@@ -931,62 +934,47 @@ 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 (!await ShouldLaunchAfterPromptAsync(risk).ConfigureAwait(false))
+            return "denied";
 
-        // 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);
+        if (!_dispatcherQueue.TryEnqueue(() =>
         {
             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 tcs.Task.ConfigureAwait(false);
     }
 
     /// <summary>
@@ -1060,7 +1048,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
@@ -1342,11 +1330,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() => _configuredGatewayUrlProvider?.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
@@ -129,6 +129,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;
 
@@ -138,17 +139,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)
@@ -168,24 +170,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;
             }
 
@@ -489,6 +480,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
@@ -1431,6 +1431,26 @@ public async Task Navigate_ResponseIncludesOpenerAndCanonicalUrl()
         Assert.Contains("\"url\":\"https://example.com/Path\"", json);
     }
 
+    [Fact]
+    public async Task Navigate_DeniedByHandler_ReturnsNotNavigated()
+    {
+        var cap = new CanvasCapability(NullLogger.Instance);
+        cap.NavigateRequested += _ => Task.FromResult("denied");
+
+        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\":\"denied\"", 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
@@ -58,6 +58,7 @@
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Services\NodeInvokeActivityFormatter.cs" Link="Services\NodeInvokeActivityFormatter.cs" />
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Services\NodeCapabilityGating.cs" Link="Services\NodeCapabilityGating.cs" />
     <Compile Include="..\..\src\OpenClaw.Tray.WinUI\Services\ToastActivationRouter.cs" Link="Services\ToastActivationRouter.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
