diff --git a/docs/CONNECTION_ARCHITECTURE.md b/docs/CONNECTION_ARCHITECTURE.md index dee65b1cb..dee1d5a3b 100644 --- a/docs/CONNECTION_ARCHITECTURE.md +++ b/docs/CONNECTION_ARCHITECTURE.md @@ -266,6 +266,8 @@ The bootstrap token is cleared only after operator and node role tokens are both **Approval boundaries**: `GatewayConnectionManager` leaves node-pair command-trust requests and reapproval pending for explicit operator approval. It may automatically approve and reconnect only an explicitly typed device-pair request used for a device role upgrade. +In-chat exec approval cards sanitize the gateway's command and message before rendering. If either cannot be reviewed in full because it is truncated, suppressed, or conceals command syntax, the card offers only Deny; the chat provider also rejects Allow RPCs that are not permitted by the matching pending card. + ## Inbound pairing approval (operator) When **another** device or node requests pairing, the gateway broadcasts `device.pair.requested` / `node.pair.requested` to operators with pairing scope. `OpenClawGatewayClient` refreshes the pending lists and raises `DevicePairListUpdated` / `NodePairListUpdated`, which `GatewayService` forwards via its `PairListsChanged` event. diff --git a/docs/GATEWAY_FIXTURE_TESTING.md b/docs/GATEWAY_FIXTURE_TESTING.md index db74610b8..91144bfe7 100644 --- a/docs/GATEWAY_FIXTURE_TESTING.md +++ b/docs/GATEWAY_FIXTURE_TESTING.md @@ -34,6 +34,9 @@ writes receive protocol errors, not simulated success. Local preferences such as chat tool visibility can be saved inside the disposable profile. This is not a fixture for the legacy WebView Gateway dashboard, installation, uninstallation, host management, or node command execution. +The one write-shaped exception is `exec.approval.resolve`: the fixture validates +the fixture-owned approval ID and decision, records only that safe correlation, +and returns a synthetic acknowledgement without executing a command. The host is `tests\OpenClaw.GatewayFixtureHost`, a plain .NET console app, not a test container. The request-driven server and scenario live in diff --git a/src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs b/src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs index b88285bf6..494d8c86c 100644 --- a/src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs +++ b/src/OpenClaw.Tray.WinUI/Chat/ChatConversationState.cs @@ -481,6 +481,18 @@ internal bool TryRaiseKeylessDiagnostic() return GetOrCreateTimelineLocked(threadId).PendingPermission?.RequestId; } + internal bool CanRespondToPermission(string threadId, string requestId, string action) + { + lock (_gate) + { + var pending = GetOrCreateTimelineLocked(threadId).PendingPermission; + return pending is not null + && string.Equals(pending.RequestId, requestId, StringComparison.Ordinal) + && ChatPermissionActionKeys.NormalizeActions(pending.Actions) + .Contains(action, StringComparer.OrdinalIgnoreCase); + } + } + internal void ActivateHistoryGeneration(long generation) { lock (_gate) diff --git a/src/OpenClaw.Tray.WinUI/Chat/ChatEventMapper.cs b/src/OpenClaw.Tray.WinUI/Chat/ChatEventMapper.cs index 9cd58266d..7f816fc9d 100644 --- a/src/OpenClaw.Tray.WinUI/Chat/ChatEventMapper.cs +++ b/src/OpenClaw.Tray.WinUI/Chat/ChatEventMapper.cs @@ -1,6 +1,7 @@ using System.Text.Json; using OpenClaw.Chat; using OpenClaw.Shared; +using OpenClaw.Shared.ExecApprovals; namespace OpenClawTray.Chat; @@ -462,18 +463,30 @@ private static ChatEventMapping MapApproval(AgentEventInfo evt) return new(null); var host = StringProperty(evt.Data, "host"); - var command = StringProperty(evt.Data, "command"); + var commandStatus = ExecApprovalCommandDisplaySanitizer.SanitizeWithStatus(StringProperty(evt.Data, "command")); var title = StringProperty(evt.Data, "title"); - var message = StringProperty(evt.Data, "message"); - var detail = string.IsNullOrEmpty(message) - ? command - : string.IsNullOrEmpty(command) ? message : message + "\n\n" + command; + var messageStatus = ExecApprovalCommandDisplaySanitizer.SanitizeWithStatus(StringProperty(evt.Data, "message")); + var detail = string.IsNullOrEmpty(messageStatus.Text) + ? commandStatus.Text + : string.IsNullOrEmpty(commandStatus.Text) + ? messageStatus.Text + : messageStatus.Text + "\n\n" + commandStatus.Text; + if (string.IsNullOrWhiteSpace(commandStatus.Text)) + { + const string missingCommand = + "No command was included with this approval, so only Deny is available."; + detail = string.IsNullOrEmpty(detail) ? missingCommand : detail + "\n\n" + missingCommand; + } + + var canReviewInFull = !string.IsNullOrWhiteSpace(commandStatus.Text) + && !commandStatus.Truncated && !commandStatus.Oversized && !commandStatus.UnsafeConcealment + && !messageStatus.Truncated && !messageStatus.Oversized && !messageStatus.UnsafeConcealment; var mapped = new ChatPermissionRequestEvent( requestId, !string.IsNullOrEmpty(title) ? title : "Exec approval", !string.IsNullOrEmpty(host) ? host : "node", detail, - ChatPermissionActionKeys.ExecApprovalDefaults); + canReviewInFull ? ChatPermissionActionKeys.ExecApprovalDefaults : [ChatPermissionActionKeys.Deny]); var alternateId = !string.IsNullOrEmpty(slug) ? approvalId : slug; return new(mapped, new ChatApprovalIdentity(requestId, alternateId)); } diff --git a/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs b/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs index aff1e0b63..9f5485868 100644 --- a/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs +++ b/src/OpenClaw.Tray.WinUI/Chat/OpenClawChatDataProvider.cs @@ -938,6 +938,11 @@ public async Task RespondToPermissionAsync(string threadId, string requestId, st return; var decision = NormalizeApprovalAction(action); + if (!_state.CanRespondToPermission(threadId, requestId, decision)) + { + Logger.Warn("[Approval] response blocked: no matching pending approval permits this decision"); + return; + } // Use the operator-approvals gateway RPC (``exec.approval.resolve``) // rather than the ``/approve `` chat slash command. // diff --git a/tests/OpenClaw.Shared.Tests/GatewayFixtureProtocolTests.cs b/tests/OpenClaw.Shared.Tests/GatewayFixtureProtocolTests.cs index 2121b743c..2658ec1ff 100644 --- a/tests/OpenClaw.Shared.Tests/GatewayFixtureProtocolTests.cs +++ b/tests/OpenClaw.Shared.Tests/GatewayFixtureProtocolTests.cs @@ -241,7 +241,6 @@ public async Task RealClient_InvalidOrUnknownParametersFailExplicitly(string met [InlineData("config.patch")] [InlineData("config.apply")] [InlineData("node.invoke")] - [InlineData("exec.approval.resolve")] public async Task RealClient_WritesAreRejectedWithoutChangingScenario(string method) { var token = CreateToken(); @@ -259,6 +258,122 @@ public async Task RealClient_WritesAreRejectedWithoutChangingScenario(string met Assert.Empty(server.UnexpectedRequests); } + [Fact] + public async Task RealClient_ApprovalResolveGetsSyntheticAckAndRecordsSafeCorrelation() + { + var token = CreateToken(); + await using var server = await FixtureGatewayServer.StartAsync(GatewayScenario.CreateBrowse(), token); + await using var connected = await ConnectedClient.OpenAsync(server, token); + await server.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-approval-1", + command = "echo fixture", + }); + + await connected.Client.ResolveExecApprovalAsync("fixture-approval-1", "allow-once"); + + var request = await server.WaitForCompletedRequestAsync("exec.approval.resolve"); + Assert.Equal("fixture-approval-1", request.ApprovalId); + Assert.Equal("allow-once", request.Decision); + Assert.Equal("ok", request.Outcome); + Assert.Empty(server.UnexpectedRequests); + } + + [Fact] + public async Task RealClient_ApprovalResolveInvalidDecisionDoesNotConsumeIssuedId() + { + var token = CreateToken(); + await using var server = await FixtureGatewayServer.StartAsync(GatewayScenario.CreateBrowse(), token); + await using var connected = await ConnectedClient.OpenAsync(server, token); + await server.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-approval-invalid", + command = "echo fixture", + }); + + var error = await Assert.ThrowsAsync(() => + connected.Client.SendWizardRequestAsync( + "exec.approval.resolve", + new { id = "fixture-approval-invalid", decision = "approve" })); + await connected.Client.ResolveExecApprovalAsync("fixture-approval-invalid", "allow-once"); + + Assert.Contains("Unsupported approval decision", error.Message); + var requests = server.Requests + .Where(request => request.Method == "exec.approval.resolve") + .ToArray(); + Assert.Collection( + requests, + request => + { + Assert.Equal("fixture-approval-invalid", request.ApprovalId); + Assert.Equal("", request.Decision); + Assert.Equal("error:INVALID_PARAMS", request.Outcome); + }, + request => + { + Assert.Equal("fixture-approval-invalid", request.ApprovalId); + Assert.Equal("allow-once", request.Decision); + Assert.Equal("ok", request.Outcome); + }); + Assert.Empty(server.UnexpectedRequests); + } + + [Fact] + public async Task RealClient_ApprovalResolveRejectsUnissuedFixtureId() + { + var token = CreateToken(); + await using var server = await FixtureGatewayServer.StartAsync(GatewayScenario.CreateBrowse(), token); + await using var connected = await ConnectedClient.OpenAsync(server, token); + + var error = await Assert.ThrowsAsync(() => + connected.Client.ResolveExecApprovalAsync("fixture-never-issued", "deny")); + + Assert.Contains("not issued by the fixture", error.Message); + var request = Assert.Single(server.Requests, request => request.Method == "exec.approval.resolve"); + Assert.Equal("fixture-never-issued", request.ApprovalId); + Assert.Equal("deny", request.Decision); + Assert.Equal("error:INVALID_PARAMS", request.Outcome); + Assert.Empty(server.UnexpectedRequests); + } + + [Fact] + public async Task RealClient_ApprovalResolveAcknowledgesIssuedIdOnlyOnce() + { + var token = CreateToken(); + await using var server = await FixtureGatewayServer.StartAsync(GatewayScenario.CreateBrowse(), token); + await using var connected = await ConnectedClient.OpenAsync(server, token); + await server.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-one-shot", + command = "echo fixture", + }); + + var attempts = await Task.WhenAll(Enumerable.Range(0, 2).Select(async _ => + { + try + { + await connected.Client.ResolveExecApprovalAsync("fixture-one-shot", "allow-once"); + return "ok"; + } + catch (InvalidOperationException ex) + { + Assert.Contains("not issued by the fixture", ex.Message); + return "rejected"; + } + })); + + Assert.Equal(1, attempts.Count(result => result == "ok")); + Assert.Equal(1, attempts.Count(result => result == "rejected")); + Assert.Contains(server.Requests, request => + request.Method == "exec.approval.resolve" && request.Outcome == "ok"); + Assert.Contains(server.Requests, request => + request.Method == "exec.approval.resolve" && request.Outcome == "error:INVALID_PARAMS"); + Assert.Empty(server.UnexpectedRequests); + } + [Fact] public async Task RealClient_UnknownMethodIsRecordedAndNeverReturnsFallbackSuccessOrCredentials() { diff --git a/tests/OpenClaw.TestSupport/Gateway/FixtureGatewayServer.cs b/tests/OpenClaw.TestSupport/Gateway/FixtureGatewayServer.cs index f166e67ba..fdda0c96f 100644 --- a/tests/OpenClaw.TestSupport/Gateway/FixtureGatewayServer.cs +++ b/tests/OpenClaw.TestSupport/Gateway/FixtureGatewayServer.cs @@ -8,7 +8,12 @@ namespace OpenClaw.TestSupport.Gateway; /// Safe diagnostic metadata only. Request IDs, credentials and payloads are never retained. -public sealed record GatewayFixtureRequest(string Method, string? SessionKey, string Outcome); +public sealed record GatewayFixtureRequest( + string Method, + string? SessionKey, + string Outcome, + string? Decision = null, + string? ApprovalId = null); /// /// An independently owned, operator-only loopback Gateway. Responses are selected by @@ -25,6 +30,8 @@ public sealed class FixtureGatewayServer : IAsyncDisposable private readonly List _requests = []; private readonly List _unexpectedIndices = []; private readonly HashSet _subscriptions = []; + private readonly HashSet _issuedApprovalIds = new(StringComparer.Ordinal); + private readonly Dictionary _authenticatedConnections = []; private readonly Dictionary _historyGates = new(StringComparer.Ordinal); private readonly TaskCompletionSource _handshake = NewSignal(); private readonly TaskCompletionSource _accepted = NewSignal(); @@ -34,6 +41,7 @@ public sealed class FixtureGatewayServer : IAsyncDisposable private int _connectionCount; private int _activeConnectionCount; private static readonly JsonElement EmptyParameters = JsonSerializer.SerializeToElement(new { }); + private sealed record ActiveConnection(WebSocket Socket, SemaphoreSlim SendLock, CancellationToken CancellationToken); public Uri Endpoint { get; } public Task HandshakeCompleted => _handshake.Task; @@ -127,6 +135,92 @@ public async Task WaitForRequestAsync( } } + public async Task WaitForCompletedRequestAsync( + string method, + string? sessionKey = null, + int occurrence = 1, + CancellationToken cancellationToken = default) + { + ArgumentOutOfRangeException.ThrowIfNegativeOrZero(occurrence); + using var wait = CancellationTokenSource.CreateLinkedTokenSource(cancellationToken, _lifetime.Token); + while (true) + { + Task changed; + lock (_sync) + { + var request = _requests + .Where(request => request.Method == method + && (sessionKey is null || request.SessionKey == sessionKey) + && request.Outcome != "pending") + .Skip(occurrence - 1) + .FirstOrDefault(); + if (request is not null) + return request; + changed = _requestChanged.Task; + } + await changed.WaitAsync(wait.Token); + } + } + + public async Task PublishAgentEventAsync( + string sessionKey, + object data, + CancellationToken cancellationToken = default) + { + ArgumentException.ThrowIfNullOrWhiteSpace(sessionKey); + ArgumentNullException.ThrowIfNull(data); + ActiveConnection[] targets; + lock (_sync) + targets = _authenticatedConnections + .OrderByDescending(pair => pair.Key) + .Select(pair => pair.Value) + .Where(connection => connection.Socket.State == WebSocketState.Open) + .ToArray(); + if (targets.Length == 0) + throw new InvalidOperationException("No authenticated fixture connection is available."); + + var eventData = JsonSerializer.SerializeToElement(data); + var approvalId = ReadString(eventData, "approvalId"); + if (ReadString(eventData, "phase") == "requested" + && approvalId is { Length: <= 64 } + && approvalId.StartsWith("fixture-", StringComparison.Ordinal)) + { + lock (_sync) + _issuedApprovalIds.Add(approvalId); + } + + var message = new + { + type = "event", + @event = "agent", + payload = new + { + runId = "", + seq = 0, + stream = "approval", + ts = DateTimeOffset.UtcNow.ToUnixTimeMilliseconds(), + sessionKey, + data = eventData, + }, + }; + foreach (var target in targets) + { + try + { + using var send = CancellationTokenSource.CreateLinkedTokenSource( + cancellationToken, + target.CancellationToken); + await SendAsync(target.Socket, target.SendLock, message, send.Token); + return; + } + catch (OperationCanceledException) when (target.CancellationToken.IsCancellationRequested) { } + catch (ObjectDisposedException) { } + catch (WebSocketException) { } + } + + throw new InvalidOperationException("Authenticated fixture connections closed before the event was sent."); + } + private async Task AcceptLoopAsync() { try @@ -209,7 +303,11 @@ private async Task ServeConnectionAsync(TcpClient tcp, int connectionId) var parameters = root.TryGetProperty("params", out var p) ? p : EmptyParameters; var key = ReadString(parameters, "sessionKey") ?? ReadString(parameters, "key"); - var requestIndex = Record(method!, _scenario.ContainsSession(key ?? "") ? key : key is null ? null : ""); + var requestIndex = Record( + method!, + _scenario.ContainsSession(key ?? "") ? key : key is null ? null : "", + SafeApprovalDecision(method!, parameters), + SafeApprovalId(method!, parameters)); if (!authenticated || method == "connect") { try @@ -224,6 +322,9 @@ private async Task ServeConnectionAsync(TcpClient tcp, int connectionId) type = "res", id, ok = true, payload = _scenario.CreateHello($"fixture-connection-{connectionId}") }, connection.Token); authenticated = true; + lock (_sync) + _authenticatedConnections[connectionId] = + new ActiveConnection(socket, sendLock, connection.Token); Complete(requestIndex, "ok"); _handshake.TrySetResult(); } @@ -262,7 +363,11 @@ private async Task ServeConnectionAsync(TcpClient tcp, int connectionId) finally { socket?.Dispose(); - lock (_sync) _subscriptions.Remove(connectionId); + lock (_sync) + { + _subscriptions.Remove(connectionId); + _authenticatedConnections.Remove(connectionId); + } Interlocked.Decrement(ref _activeConnectionCount); } } @@ -280,7 +385,29 @@ private async Task RespondAsync( { if (parameters.ValueKind != JsonValueKind.Object) throw new FixtureRequestException("INVALID_PARAMS", "params must be an object."); + if (method == "exec.approval.resolve") + { + var approvalId = ReadString(parameters, "id"); + lock (_sync) + { + if (approvalId is null || !_issuedApprovalIds.Contains(approvalId)) + throw new FixtureRequestException( + "INVALID_PARAMS", + "Approval ID was not issued by the fixture."); + } + } payload = _scenario.Respond(method, parameters); + if (method == "exec.approval.resolve") + { + var approvalId = ReadString(parameters, "id"); + lock (_sync) + { + if (approvalId is null || !_issuedApprovalIds.Remove(approvalId)) + throw new FixtureRequestException( + "INVALID_PARAMS", + "Approval ID was already consumed by the fixture."); + } + } } catch (FixtureRequestException ex) { @@ -326,12 +453,16 @@ private void Authenticate(JsonElement p, string nonce) throw new FixtureRequestException("INVALID_PARAMS", "Expected a signed operator envelope for this challenge."); } - private int Record(string method, string? key) + private int Record( + string method, + string? key, + string? decision = null, + string? approvalId = null) { lock (_sync) { var index = _requests.Count; - _requests.Add(new GatewayFixtureRequest(method, key, "pending")); + _requests.Add(new GatewayFixtureRequest(method, key, "pending", decision, approvalId)); SignalRequestChanged(); return index; } @@ -356,6 +487,26 @@ private void SignalRequestChanged() } private static TaskCompletionSource NewSignal() => new(TaskCreationOptions.RunContinuationsAsynchronously); + private static string? SafeApprovalDecision(string method, JsonElement parameters) + { + if (method != "exec.approval.resolve") + return null; + var decision = ReadString(parameters, "decision"); + return decision is "allow-once" or "allow-always" or "deny" + ? decision + : ""; + } + + private static string? SafeApprovalId(string method, JsonElement parameters) + { + if (method != "exec.approval.resolve") + return null; + var approvalId = ReadString(parameters, "id"); + return approvalId is { Length: <= 64 } && approvalId.StartsWith("fixture-", StringComparison.Ordinal) + ? approvalId + : ""; + } + private static string? ReadString(JsonElement p, string name) => p.ValueKind == JsonValueKind.Object && p.TryGetProperty(name, out var value) && value.ValueKind == JsonValueKind.String ? value.GetString() : null; diff --git a/tests/OpenClaw.TestSupport/Gateway/GatewayScenario.cs b/tests/OpenClaw.TestSupport/Gateway/GatewayScenario.cs index 436a6a3dc..065802c20 100644 --- a/tests/OpenClaw.TestSupport/Gateway/GatewayScenario.cs +++ b/tests/OpenClaw.TestSupport/Gateway/GatewayScenario.cs @@ -206,7 +206,7 @@ public static GatewayScenario CreateBrowse() type = GatewayProtocolContract.HelloOkType, protocol = ProtocolVersion, server = new { version = "fixture-1", connId = connectionId }, - features = new { methods = ReadMethods, events = new[] { "connect.challenge", "sessions.changed" } }, + features = new { methods = ReadMethods, events = new[] { "connect.challenge", "sessions.changed", "agent" } }, snapshot = new { presence = Array.Empty(), health = _reads["health"], @@ -220,6 +220,8 @@ public static GatewayScenario CreateBrowse() internal object Respond(string method, JsonElement parameters) { + if (method == "exec.approval.resolve") + return ResolveApproval(parameters); if (IsWrite(method)) throw new FixtureRequestException("FIXTURE_READ_ONLY", "Fixture Gateway is read-only. This operation is not executed."); return method switch @@ -235,6 +237,16 @@ _ when _reads.ContainsKey(method) => Read(method, parameters), }; } + private static object ResolveApproval(JsonElement p) + { + ValidateProperties(p, "id", "decision"); + _ = RequiredString(p, "id"); + var decision = RequiredString(p, "decision"); + if (decision is not ("allow-once" or "allow-always" or "deny")) + throw new FixtureRequestException("INVALID_PARAMS", "Unsupported approval decision."); + return new { ok = true, decision }; + } + private object ListSessions(JsonElement p) { ValidateProperties(p, "agentId", "limit", "activeMinutes", "includeGlobal", "includeUnknown", "includeDerivedTitles", "includeLastMessage"); diff --git a/tests/OpenClaw.Tray.Tests/ChatRuntimeOwnersTests.cs b/tests/OpenClaw.Tray.Tests/ChatRuntimeOwnersTests.cs index c850cbc47..4fc67a8c0 100644 --- a/tests/OpenClaw.Tray.Tests/ChatRuntimeOwnersTests.cs +++ b/tests/OpenClaw.Tray.Tests/ChatRuntimeOwnersTests.cs @@ -1,6 +1,7 @@ using System.Text.Json; using OpenClaw.Chat; using OpenClaw.Shared; +using OpenClaw.Shared.ExecApprovals; using OpenClawTray.Chat; namespace OpenClaw.Tray.Tests; @@ -1050,6 +1051,90 @@ public void Map_ApprovalRequestPreservesIdentityAndActions() Assert.Equal("approve-1", request.RequestId); Assert.Equal("approval-uuid", mapping.Approval?.AlternateId); Assert.Equal(ChatPermissionActionKeys.ExecApprovalDefaults, request.Actions); + Assert.Equal("echo ok", request.Detail); + } + + [Fact] + public void Map_ApprovalRequestSanitizesCommandAndMessage() + { + var command = "echo " + char.ConvertFromUtf32(0x202E) + "ok" + char.ConvertFromUtf32(0x200B); + var message = "review" + char.ConvertFromUtf32(0x2028) + "this"; + const string requestId = "approval-req-bidi"; + using var document = JsonDocument.Parse( + JsonSerializer.Serialize(new Dictionary + { + ["phase"] = "requested", + ["approvalId"] = requestId, + ["command"] = command, + ["message"] = message, + })); + + var mapping = ChatEventMapper.Map(new AgentEventInfo + { + Stream = "approval", + SessionKey = "main", + Data = document.RootElement.Clone(), + }); + + var request = Assert.IsType(mapping.Event); + var expected = ExecApprovalCommandDisplaySanitizer.Sanitize(message) + + "\n\n" + + ExecApprovalCommandDisplaySanitizer.Sanitize(command); + Assert.Equal(requestId, request.RequestId); + Assert.Equal(expected, request.Detail); + Assert.Equal(ChatPermissionActionKeys.ExecApprovalDefaults, request.Actions); + } + + public static TheoryData UnreviewableApprovalText => new() + { + { "", "Please approve" }, + { new string('x', 300 * 1024), "" }, + { new string('x', 20 * 1024), "" }, + { "echo API_SECRET=sk-abc123456789012345678", "" }, + { "echo client_id=visible&app_se" + char.ConvertFromUtf32(0x200B) + "cret=opaque-app-secret", "" }, + { "echo ok", new string('x', 300 * 1024) }, + { "echo ok", new string('x', 20 * 1024) }, + { "echo ok", "echo API_SECRET=sk-abc123456789012345678" }, + }; + + [Theory] + [MemberData(nameof(UnreviewableApprovalText))] + public void Map_ApprovalRequestOffersOnlyDenyWhenTextCannotBeReviewed(string command, string message) + { + using var document = JsonDocument.Parse( + JsonSerializer.Serialize(new { phase = "requested", approvalId = "unsafe-1", command, message })); + + var mapping = ChatEventMapper.Map(new AgentEventInfo + { + Stream = "approval", + SessionKey = "main", + Data = document.RootElement.Clone(), + }); + + var request = Assert.IsType(mapping.Event); + Assert.Equal([ChatPermissionActionKeys.Deny], request.Actions); + Assert.Equal("unsafe-1", request.RequestId); + if (string.IsNullOrEmpty(command)) + Assert.Contains("only Deny is available", request.Detail); + } + + [Fact] + public void Map_ApprovalRequestKeepsAllowForReviewSafeRedaction() + { + const string command = "curl https://example.test/search?key=$GOOGLE_KEY"; + using var document = JsonDocument.Parse( + JsonSerializer.Serialize(new { phase = "requested", approvalId = "safe-1", command })); + + var mapping = ChatEventMapper.Map(new AgentEventInfo + { + Stream = "approval", + SessionKey = "main", + Data = document.RootElement.Clone(), + }); + + var request = Assert.IsType(mapping.Event); + Assert.Equal(ChatPermissionActionKeys.ExecApprovalDefaults, request.Actions); + Assert.Equal(ExecApprovalCommandDisplaySanitizer.Sanitize(command), request.Detail); } [Fact] diff --git a/tests/OpenClaw.Tray.Tests/OpenClawChatDataProviderTests.cs b/tests/OpenClaw.Tray.Tests/OpenClawChatDataProviderTests.cs index 4d62ccfe2..0671ce460 100644 --- a/tests/OpenClaw.Tray.Tests/OpenClawChatDataProviderTests.cs +++ b/tests/OpenClaw.Tray.Tests/OpenClawChatDataProviderTests.cs @@ -1,10 +1,12 @@ using OpenClaw.Chat; using OpenClaw.Shared; using OpenClaw.Shared.Telemetry; +using OpenClaw.TestSupport.Gateway; using OpenClawTray.Chat; using System.Collections.Concurrent; using System.Diagnostics; using System.Diagnostics.Metrics; +using System.Security.Cryptography; using System.Text.Json; namespace OpenClaw.Tray.Tests; @@ -14115,6 +14117,35 @@ public async Task RespondToPermissionAsync_AllowAlwaysRoutesAllowAlwaysThroughRp Assert.Null(snapshots[^1].Timelines["main"].PendingPermission); } + [Fact] + public async Task RespondToPermissionAsync_UnreviewableCommandBlocksDirectAllowButPermitsDeny() + { + var (bridge, provider, snapshots, _) = CreateProvider(new[] { MainSession() }); + await provider.LoadAsync(); + bridge.RaiseAgent(MakeAgentEvent("approval", JsonSerializer.Serialize(new + { + phase = "requested", + approvalId = "appr-unsafe-1", + command = new string('x', 20 * 1024), + }))); + + var pending = snapshots[^1].Timelines["main"].PendingPermission; + Assert.NotNull(pending); + Assert.Equal([ChatPermissionActionKeys.Deny], pending.Actions); + + await provider.RespondToPermissionAsync("main", "appr-unsafe-1", allow: true); + await provider.RespondToPermissionAsync("main", "appr-unsafe-1", ChatPermissionActionKeys.AllowAlways); + + Assert.Empty(bridge.ResolvedApprovals); + Assert.Equal("appr-unsafe-1", snapshots[^1].Timelines["main"].PendingPermission?.RequestId); + + await provider.RespondToPermissionAsync("main", "appr-unsafe-1", allow: false); + + Assert.Single(bridge.ResolvedApprovals); + Assert.Equal(ChatPermissionActionKeys.Deny, bridge.ResolvedApprovals[0].Decision); + Assert.Null(snapshots[^1].Timelines["main"].PendingPermission); + } + [Fact] public async Task RespondToPermissionAsync_RpcThrows_BannerPreservedForRetry() { @@ -14396,6 +14427,97 @@ public async Task RuntimeGolden_PublicSnapshotPreservesCrossDomainState() } } + [Fact] + public async Task RespondToPermissionAsync_SupersededRequestBlocksStaleAllow() + { + var (bridge, provider, snapshots, _) = CreateProvider(new[] { MainSession() }); + await provider.LoadAsync(); + bridge.RaiseAgent(MakeApprovalRequestedEvent("appr-stale-1")); + bridge.RaiseAgent(MakeApprovalRequestedEvent("appr-current-2")); + + await provider.RespondToPermissionAsync("main", "appr-stale-1", allow: true); + + Assert.Empty(bridge.ResolvedApprovals); + Assert.Equal("appr-current-2", snapshots[^1].Timelines["main"].PendingPermission?.RequestId); + + await provider.RespondToPermissionAsync("main", "appr-current-2", allow: true); + + var resolved = Assert.Single(bridge.ResolvedApprovals); + Assert.Equal("appr-current-2", resolved.Id); + Assert.Equal(ChatPermissionActionKeys.AllowOnce, resolved.Decision); + Assert.Null(snapshots[^1].Timelines["main"].PendingPermission); + } + + [Fact] + public async Task RespondToPermissionAsync_StaleAllowStopsBeforeProductionWebSocketIo() + { + var token = Convert.ToHexString(RandomNumberGenerator.GetBytes(32)); + await using var server = await FixtureGatewayServer.StartAsync(GatewayScenario.CreateBrowse(), token); + using var identity = new OpenClaw.TestSupport.TempDirectory(); + using var client = new OpenClawGatewayClient( + server.Endpoint.AbsoluteUri, + token, + NullLogger.Instance, + identityPath: identity.Path, + ignoreStoredDeviceToken: true, + persistHandshakeDeviceTokens: false); + var handshake = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + client.HandshakeSucceeded += (_, _) => handshake.TrySetResult(); + await client.ConnectAsync(); + await handshake.Task.WaitAsync(TimeSpan.FromSeconds(10)); + + using var bridge = new GatewayClientChatBridge(client); + var snapshots = new List(); + await using var provider = new OpenClawChatDataProvider(bridge); + provider.Changed += (_, args) => snapshots.Add(args.Snapshot); + await provider.LoadAsync(); + string? PendingPermissionId() => + snapshots.LastOrDefault() is { } snapshot + && snapshot.Timelines.TryGetValue(GatewayScenario.MainSessionKey, out var timeline) + ? timeline.PendingPermission?.RequestId + : null; + + await server.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-stale-provider", + command = "echo stale", + }); + await WaitForConditionAsync(() => PendingPermissionId() == "fixture-stale-provider"); + await server.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-current-provider", + command = "echo current", + }); + await WaitForConditionAsync(() => PendingPermissionId() == "fixture-current-provider"); + + await provider.RespondToPermissionAsync( + GatewayScenario.MainSessionKey, + "fixture-stale-provider", + allow: true); + await Task.Delay(100); + + Assert.DoesNotContain( + server.Requests, + request => request.Method == "exec.approval.resolve" + && request.ApprovalId == "fixture-stale-provider"); + Assert.Equal( + "fixture-current-provider", + PendingPermissionId()); + + await provider.RespondToPermissionAsync( + GatewayScenario.MainSessionKey, + "fixture-current-provider", + allow: true); + var resolved = await server.WaitForCompletedRequestAsync("exec.approval.resolve"); + + Assert.Equal("fixture-current-provider", resolved.ApprovalId); + Assert.Equal("allow-once", resolved.Decision); + Assert.Equal("ok", resolved.Outcome); + await client.DisconnectAsync(); + } + [Fact] public async Task RuntimeGolden_ResetAtomicallyClearsQueueAndAdvancesGeneration() { diff --git a/tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.cs b/tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.cs index bc2979b45..05a77149b 100644 --- a/tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.cs +++ b/tests/OpenClaw.Tray.UITests/GatewayFixtureUiTests.cs @@ -162,6 +162,82 @@ await WaitUiAsync(run, () => RenderConsumedHistory(run, GatewayScenario.LongSess }); } + [GatewayFixtureUiFact] + [Trait("Category", "GatewayFixture")] + public async Task ApprovalCardsGateAllowAndResolveThroughGateway() + { + await WithAppAsync(async run => + { + await run.InvokeAsync("app.navigate", new { page = "chat" }); + await WaitUiAsync(run, () => FindById(run, "ChatComposerInput") is not null, "native chat composer"); + await WaitHistoryAsync(run, GatewayScenario.MainSessionKey); + + await run.Gateway.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-safe-approval", + command = "echo fixture-safe", + title = "Command approval requested", + }); + await WaitUiAsync(run, () => FindButton(run, "Allow once") is not null, "reviewable Allow button"); + Assert.NotNull(FindButton(run, "Always allow")); + Assert.NotNull(FindButton(run, "Deny once")); + Invoke(FindButton(run, "Allow once")!); + await run.WaitForAsync(() => Task.FromResult(run.Gateway.Requests.Any(request => + request.Method == "exec.approval.resolve" + && request.Decision == "allow-once" + && request.Outcome == "ok")), "reviewable Allow RPC"); + var allowed = run.Gateway.Requests.Single(request => + request.Method == "exec.approval.resolve" && request.Decision == "allow-once"); + Assert.Equal("fixture-safe-approval", allowed.ApprovalId); + Assert.Equal("allow-once", allowed.Decision); + Assert.Equal("ok", allowed.Outcome); + + await run.Gateway.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-unreviewable-approval", + command = "", + message = "Please approve this request.", + title = "Command approval requested", + }); + await WaitUiAsync(run, () => FindButton(run, "Deny once") is not null + && FindText(run, "only Deny is available") is not null, "Deny-only approval"); + Assert.Null(FindButton(run, "Allow once")); + Assert.Null(FindButton(run, "Always allow")); + await CaptureIfRequestedAsync(run, "approval-deny-only.png"); + Invoke(FindButton(run, "Deny once")!); + await run.WaitForAsync(() => Task.FromResult(run.Gateway.Requests.Any(request => + request.Method == "exec.approval.resolve" + && request.Decision == "deny" + && request.Outcome == "ok")), "Deny-only RPC"); + var denied = run.Gateway.Requests.Single(request => + request.Method == "exec.approval.resolve" && request.Decision == "deny"); + Assert.Equal("fixture-unreviewable-approval", denied.ApprovalId); + Assert.Equal("deny", denied.Decision); + Assert.Equal("ok", denied.Outcome); + + await run.Gateway.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "requested", + approvalId = "fixture-superseded-approval", + command = "echo superseded", + title = "Command approval requested", + }); + await WaitUiAsync(run, () => FindButton(run, "Allow once") is not null, "superseded Allow button"); + await run.Gateway.PublishAgentEventAsync(GatewayScenario.MainSessionKey, new + { + phase = "resolved", + approvalId = "fixture-superseded-approval", + decision = "deny", + }); + await WaitUiAsync(run, () => FindButton(run, "Allow once") is null + && FindButton(run, "Always allow") is null, "superseded actions removed"); + await Task.Delay(200); + Assert.Equal(2, run.Gateway.Requests.Count(request => request.Method == "exec.approval.resolve")); + }); + } + private async Task WithAppAsync(Func test) { var appPath = Environment.GetEnvironmentVariable("OPENCLAW_GATEWAY_FIXTURE_APP") @@ -249,6 +325,11 @@ private static void Invoke(AutomationElement element) private static AutomationElement? FindById(GatewayFixtureRun run, string id) => FindInApp(run, new PropertyCondition(AutomationElement.AutomationIdProperty, id)); + private static AutomationElement? FindButton(GatewayFixtureRun run, string name) => + FindInApp(run, new AndCondition( + new PropertyCondition(AutomationElement.ControlTypeProperty, ControlType.Button), + new PropertyCondition(AutomationElement.NameProperty, name))); + private static AutomationElement? FindText(GatewayFixtureRun run, string text) { foreach (var window in AppWindows(run))