From f4470306afa1f76a0a8902db903a5ceb5903c553 Mon Sep 17 00:00:00 2001 From: telli Date: Sun, 27 Sep 2026 20:11:04 -0700 Subject: [PATCH 1/8] fix(gateway): require operator role for /ws chat /ws authenticated callers but never checked their role, so a viewer account token, viewer browser session, or OIDC user without an operator role claim could submit agent turns and approval decisions. REST submission (POST /api/integration/messages) already requires operator. Resolve the caller through AuthorizeOperatorRequest, the chain the HTTP API uses, and close connections below operator with 1008 (PolicyViolation). Closing after accept instead of a 403 handshake keeps web chat usable: browsers cannot read handshake status codes, and web chat already treats 1008 as an authorization failure rather than reconnecting. /ws/live is unchanged: it bridges the live model without the agent pipeline or tools. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/AUTHENTICATION.md | 21 +++++- docs/zh-CN/AUTHENTICATION.md | 21 +++++- .../Endpoints/WebSocketEndpoints.cs | 17 +++++ .../GatewayAdminEndpointTests.cs | 73 +++++++++++++++++++ 5 files changed, 129 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 77b0cfdd..9cf6fbb4 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ All notable changes to this project are tracked in this file. ### Security +- Required the `operator` role for `/ws` chat, matching `POST /api/integration/messages`. Viewer identities, including OIDC users without an operator role claim, were previously admitted and could submit agent turns and approval decisions. They are now closed with code 1008, which web chat reports as an authorization failure. Grant `operator` to accounts and OIDC users who should chat. - Bound tool-approval decisions to the original requester (`channelId` + `senderId`) for non-loopback/public binds. - Kept `POST /tools/approve` as an explicit admin override path. - Added WhatsApp official webhook signature validation support (`ValidateSignature`, `WebhookAppSecret`/`WebhookAppSecretRef`). diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index 0d05562c..af9915db 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -130,7 +130,7 @@ Request enters ### 3.2 WebSocket Authentication Flow -WebSocket endpoints (`/ws`, `/ws/live`) use a two-phase authentication flow: +WebSocket endpoints (`/ws`, `/ws/live`) authenticate in Phase 1. `/ws` then applies the chat role check (Phase 2) and resolves the user ID (Phase 3): **Phase 1: `TryValidateWebSocketRequest` → `IsAuthorizedRequest`** @@ -148,7 +148,24 @@ WebSocket request (/ws) └─ Passed ──→ Accept WebSocket connection ``` -**Phase 2: `TryResolveAuthorizedUserIdForWebSocket`** +**Phase 2 (`/ws` only): `CanSubmitChat`** + +Every `/ws` frame becomes agent input, so the connection needs the same `operator` role as `POST /api/integration/messages`. The role is resolved through `AuthorizeOperatorRequest`, the same chain the HTTP API uses: + +``` +WebSocket accepted (/ws) + │ + ├─ Role below operator, or identity not allowed by organization policy? + │ ── yes ──→ close 1008 (PolicyViolation) "Chat requires the operator role." + │ + └─ Passed ──→ Phase 3 +``` + +The connection is accepted and then closed, rather than rejected with 403 during the handshake. Browsers cannot read a failed handshake's status code, but web chat treats close code 1008 as an authorization failure and stops reconnecting. + +This applies to OIDC identities too. A JWT without the configured `RoleClaim` resolves to `viewer` and cannot chat, so grant `operator` through the claim to users who should use web chat. + +**Phase 3: `TryResolveAuthorizedUserIdForWebSocket`** ``` WebSocket connected diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index d77576e8..6c4642f7 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -130,7 +130,7 @@ HTTP API 端点使用 `AuthorizeOperatorRequest` 方法（[EndpointHelpers.cs](. ### 3.2 WebSocket 认证流程 -WebSocket 端点 (`/ws`, `/ws/live`) 使用两步认证流程： +WebSocket 端点 (`/ws`, `/ws/live`) 在第一步完成认证；`/ws` 随后执行聊天角色检查（第二步）并解析用户 ID（第三步）： **第一步：`TryValidateWebSocketRequest` → `IsAuthorizedRequest`** @@ -148,7 +148,24 @@ WebSocket 请求 (/ws) └─ 通过 ──→ 接受 WebSocket 连接 ``` -**第二步：`TryResolveAuthorizedUserIdForWebSocket`** +**第二步（仅 `/ws`）：`CanSubmitChat`** + +每个 `/ws` 帧都会成为智能体输入，因此连接需要与 `POST /api/integration/messages` 相同的 `operator` 角色。角色通过 `AuthorizeOperatorRequest` 解析，与 HTTP API 使用同一认证链： + +``` +WebSocket 已接受 (/ws) + │ + ├─ 角色低于 operator，或身份不被组织策略允许？ + │ ── 是 ──→ 关闭 1008 (PolicyViolation) "Chat requires the operator role." + │ + └─ 通过 ──→ 第三步 +``` + +连接会先被接受再关闭，而不是在握手阶段返回 403。浏览器无法读取握手失败的状态码，而 Web Chat 会将关闭码 1008 视为授权失败并停止重连。 + +OIDC 身份同样适用。缺少所配置 `RoleClaim` 的 JWT 会解析为 `viewer`，无法聊天；请通过该声明为需要使用 Web Chat 的用户授予 `operator` 角色。 + +**第三步：`TryResolveAuthorizedUserIdForWebSocket`** ``` WebSocket 已连接 diff --git a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs index 8ad01a2d..5e3e49dc 100644 --- a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs @@ -22,6 +22,16 @@ public static void MapOpenClawWebSocketEndpoints( return; var ws = await ctx.WebSockets.AcceptWebSocketAsync(); + + // Every /ws frame becomes agent input, so require the same role as POST /api/integration/messages. + // Close after accepting rather than returning 403: browsers cannot read a failed handshake's status, + // but web chat treats close code 1008 as an authorization failure and stops reconnecting. + if (!CanSubmitChat(ctx, startup)) + { + await ws.CloseAsync(WebSocketCloseStatus.PolicyViolation, "Chat requires the operator role.", ctx.RequestAborted); + return; + } + var clientId = ctx.Connection.Id; TryResolveAuthorizedUserIdForWebSocket(ctx, startup, out var userId); await runtime.WebSocketChannel.HandleConnectionAsync(ws, clientId, ctx.Connection.RemoteIpAddress, ctx.RequestAborted, userId); @@ -95,6 +105,13 @@ private static bool TryValidateWebSocketRequest( return true; } + internal static bool CanSubmitChat(HttpContext ctx, GatewayStartupContext startup) + { + var browserSessions = ctx.RequestServices.GetRequiredService(); + var auth = EndpointHelpers.AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); + return auth.IsAuthorized && EndpointHelpers.IsRoleAllowed(auth.Role, "integration.mutate.chat", out _); + } + internal static bool TryResolveAuthorizedUserIdForWebSocket(HttpContext ctx, GatewayStartupContext startup, out string? authenticatedUserId) { authenticatedUserId = null; diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index 7abb8400..640f4a34 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -2,6 +2,7 @@ using System.IO.Compression; using System.Net; using System.Net.Http.Headers; +using System.Net.WebSockets; using System.Runtime.CompilerServices; using System.Threading.Channels; using System.Text.RegularExpressions; @@ -339,6 +340,78 @@ public async Task AuthSession_AccountTokenFlow_ReportsIdentity() Assert.Equal("viewer", loginPayload.RootElement.GetProperty("role").GetString()); } + [Fact] + public async Task WebSocketChat_WhenViewerAccountToken_ShouldCloseWithPolicyViolation() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "ws-viewer", OperatorRoleNames.Viewer); + + using var ws = await ConnectWebSocketAsync(harness, token); + var received = await ReceiveWithinAsync(ws, TimeSpan.FromSeconds(5)); + + Assert.NotNull(received); + Assert.Equal(WebSocketMessageType.Close, received.MessageType); + Assert.Equal(WebSocketCloseStatus.PolicyViolation, ws.CloseStatus); + Assert.Contains("operator", ws.CloseStatusDescription, StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task WebSocketChat_WhenOperatorAccountToken_ShouldStayOpen() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "ws-operator", OperatorRoleNames.Operator); + + using var ws = await ConnectWebSocketAsync(harness, token); + + Assert.Null(await ReceiveWithinAsync(ws, TimeSpan.FromMilliseconds(500))); + } + + [Fact] + public async Task WebSocketChat_WhenOpenLoopbackWithoutCredentials_ShouldStayOpen() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: false); + + using var ws = await ConnectWebSocketAsync(harness, bearerToken: null); + + Assert.Null(await ReceiveWithinAsync(ws, TimeSpan.FromMilliseconds(500))); + } + + private static string CreateAccountToken(GatewayTestHarness harness, string username, string role) + { + var operatorAccounts = harness.App.Services.GetRequiredService(); + var account = operatorAccounts.Create(new OperatorAccountCreateRequest + { + Username = username, + Password = "P@ssw0rd123!", + Role = role + }); + var token = operatorAccounts.CreateToken(account.Id, new OperatorAccountTokenCreateRequest { Label = username }); + Assert.NotNull(token); + return token!.Token; + } + + private static async Task ConnectWebSocketAsync(GatewayTestHarness harness, string? bearerToken) + { + var client = harness.App.GetTestServer().CreateWebSocketClient(); + if (bearerToken is not null) + client.ConfigureRequest = request => request.Headers.Authorization = $"Bearer {bearerToken}"; + return await client.ConnectAsync(new Uri("ws://localhost/ws"), CancellationToken.None); + } + + // Returns null when nothing arrives in time, which for these tests means the server kept the connection open. + private static async Task ReceiveWithinAsync(WebSocket ws, TimeSpan timeout) + { + using var cts = new CancellationTokenSource(timeout); + try + { + return await ws.ReceiveAsync(new byte[1024], cts.Token); + } + catch (OperationCanceledException) + { + return null; + } + } + [Fact] public async Task HarnessContracts_AdminApi_RequiresAuthAndSupportsCreateListDetailAndStatus() { From 6a28ecf8d5674578162c3043c67fd9506c4ee717 Mon Sep 17 00:00:00 2001 From: telli Date: Sun, 27 Sep 2026 20:42:34 -0700 Subject: [PATCH 2/8] fix(gateway): require operator role for all agent execution surfaces /ws was the first of several surfaces that authenticated callers but never checked their role. /v1/chat/completions, /v1/responses, A2A execution, /apps/chat, and the send_message/run_workflow/ respond_workflow MCP tools let any authenticated identity - viewer account tokens, viewer browser sessions, OIDC users without an operator role claim - run the agent with tools, while the REST routes they mirror require operator. New accounts default to viewer, so this was the common case rather than the edge case. All of them now go through EndpointHelpers.CanExecuteAgent, the role chain the HTTP API uses: - /v1/*: 403 with an OpenAI-style permission_error body. - A2A execution and /apps/chat: 403. /apps/chat also stops running the agent for an unauthenticated loopback client IP, which behind a same-host proxy without TrustForwardedHeaders was every caller. - MCP: only the three mutating tools fail, with a tool error result; read-only tools stay available to viewers. OpenClawWebSocketClient now raises OnClosed with the gateway's close status and reason, and Companion uses it to mark itself disconnected and show the reason instead of appearing connected after a 1008 close. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 8 +- docs/AUTHENTICATION.md | 18 ++- docs/MCPAPP.md | 2 +- docs/USER_GUIDE.md | 4 +- docs/a2a.md | 2 +- docs/workflow-backends.md | 4 +- docs/zh-CN/AUTHENTICATION.md | 18 ++- .../OpenClawWebSocketClient.cs | 10 ++ .../Services/GatewayWebSocketClient.cs | 10 ++ .../ViewModels/MainWindowViewModel.cs | 15 ++ .../A2A/A2AEndpointExtensions.cs | 6 + .../Endpoints/AppsEndpoints.cs | 8 + .../Endpoints/EndpointHelpers.cs | 26 +++ .../OpenAiEndpoints.ChatCompletions.cs | 3 + .../Endpoints/OpenAiEndpoints.Responses.cs | 3 + .../Endpoints/OpenAiEndpoints.cs | 15 ++ .../Endpoints/WebSocketEndpoints.cs | 11 +- .../Mcp/McpServiceExtensions.cs | 2 + src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs | 36 ++++- src/OpenClaw.Tests/AppsEndpointsTests.cs | 2 + .../CompanionConnectionTests.cs | 68 ++++++++ .../GatewayAdminEndpointTests.cs | 149 +++++++++++++++++- .../OpenClawWebSocketClientTests.cs | 32 ++++ src/OpenClaw.Tests/TestWebSocket.cs | 14 +- 24 files changed, 434 insertions(+), 32 deletions(-) create mode 100644 src/OpenClaw.Tests/CompanionConnectionTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 9cf6fbb4..312f9cf0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,7 +36,13 @@ All notable changes to this project are tracked in this file. ### Security -- Required the `operator` role for `/ws` chat, matching `POST /api/integration/messages`. Viewer identities, including OIDC users without an operator role claim, were previously admitted and could submit agent turns and approval decisions. They are now closed with code 1008, which web chat reports as an authorization failure. Grant `operator` to accounts and OIDC users who should chat. +- Required the `operator` role wherever a request runs the agent or mutates state, matching `POST /api/integration/messages`. Previously any authenticated identity, including viewer account tokens, viewer browser sessions, and OIDC users without an operator role claim, could run the agent with tools through these surfaces. New operator accounts default to `viewer`, so grant `operator` to accounts used for Companion, CLI/TUI chat, and API clients. + - `/ws`: closed with code 1008, which web chat reports as an authorization failure. + - `POST /v1/chat/completions` and `POST /v1/responses`: 403 with an OpenAI-style `permission_error` body. + - A2A execution paths: 403. Discovery stays public. + - `POST /apps/chat`: 403. It also no longer runs the agent for an unauthenticated caller whose client IP is loopback, which behind a same-host reverse proxy without `TrustForwardedHeaders` was every caller. + - MCP `openclaw.send_message`, `openclaw.run_workflow`, and `openclaw.respond_workflow`: tool error result. Read-only MCP tools stay available to viewers. +- Added `OpenClawWebSocketClient.OnClosed`, raised with the gateway's close status and reason. Companion now marks itself disconnected and shows the reason instead of appearing connected after the gateway closes the socket. - Bound tool-approval decisions to the original requester (`channelId` + `senderId`) for non-loopback/public binds. - Kept `POST /tools/approve` as an explicit admin override path. - Added WhatsApp official webhook signature validation support (`ValidateSignature`, `WebhookAppSecret`/`WebhookAppSecretRef`). diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index af9915db..136c2b3c 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -148,7 +148,7 @@ WebSocket request (/ws) └─ Passed ──→ Accept WebSocket connection ``` -**Phase 2 (`/ws` only): `CanSubmitChat`** +**Phase 2 (`/ws` only): `EndpointHelpers.CanExecuteAgent`** Every `/ws` frame becomes agent input, so the connection needs the same `operator` role as `POST /api/integration/messages`. The role is resolved through `AuthorizeOperatorRequest`, the same chain the HTTP API uses: @@ -177,7 +177,21 @@ WebSocket connected └─ Call AuthorizeOperatorRequest() ──→ extract AccountId as userId ``` -### 3.3 `IsAuthorizedRequest` — Detailed Logic +### 3.3 Role Required for Agent Execution + +`IsAuthorizedRequest` only establishes that a caller is authenticated. Surfaces that turn a request into agent input or another mutation also require the `operator` role, the same role as `POST /api/integration/messages`, through `EndpointHelpers.CanExecuteAgent`: + +| Surface | Below operator | +|---------|----------------| +| `/ws` | Accepted, then closed with 1008 (PolicyViolation) | +| `POST /v1/chat/completions`, `POST /v1/responses` | 403 with an OpenAI-style `permission_error` body | +| A2A execution paths (discovery stays public) | 403 | +| `POST /apps/chat` | 403. A loopback client IP no longer suffices on its own | +| MCP `openclaw.send_message`, `openclaw.run_workflow`, `openclaw.respond_workflow` | Tool error result; read-only MCP tools stay available to viewers | + +Bootstrap tokens and open loopback resolve to `admin` and are unaffected. New operator accounts default to `viewer`, so accounts used for Companion, CLI/TUI chat, or API clients need the `operator` role. + +### 3.4 `IsAuthorizedRequest` — Detailed Logic ```csharp // Step 1: Loopback exemption diff --git a/docs/MCPAPP.md b/docs/MCPAPP.md index b7087ba6..be19c2ed 100644 --- a/docs/MCPAPP.md +++ b/docs/MCPAPP.md @@ -37,7 +37,7 @@ OpenClaw.NET exposes a small gateway-facing host surface for browser-side MCP Ap | Route | Purpose | |-------|---------| | `/apps/health` | Returns the selected MCP App id plus the gateway MCP endpoint the browser should connect to | -| `/apps/chat` | Streams chat-host SSE events (`session`, `text`, `tool`, `result`, `done`) into the existing `GatewayAppRuntime` | +| `/apps/chat` | Streams chat-host SSE events (`session`, `text`, `tool`, `result`, `done`) into the existing `GatewayAppRuntime`. Requires the `operator` role | | `/apps/mcp/{appId}` | Proxies MCP requests to the already connected `McpClient` for that App | The important detail is that browser UIs should connect to `/apps/mcp/{appId}`, not directly to the App's raw upstream MCP URL. That keeps browser-driven MCP calls and model-driven MCP calls on the same OpenClaw-managed session. diff --git a/docs/USER_GUIDE.md b/docs/USER_GUIDE.md index 0f8030b0..9c67d161 100644 --- a/docs/USER_GUIDE.md +++ b/docs/USER_GUIDE.md @@ -102,14 +102,14 @@ For the full A2A behavior and operator notes, see [a2a.md](a2a.md). OpenClaw.NET now has three fixed operator roles: - `viewer`: read-only dashboard, audit, setup status, observability, and export access -- `operator`: viewer permissions plus approvals, memory/profile/learning changes, automation execution, session promotion, and webhook replay +- `operator`: viewer permissions plus approvals, chat and other agent execution, memory/profile/learning changes, automation execution, session promotion, and webhook replay - `admin`: operator permissions plus settings, plugins, provider policies, accounts, and organization policy Recommended auth flow: 1. Use `OPENCLAW_AUTH_TOKEN` once on a non-loopback deployment to bootstrap the first operator account. 2. Sign into `/admin` with the operator account username and password. -3. Exchange credentials for an operator account token when setting up Companion, API clients, CLI automation, or websocket integrations. +3. Exchange credentials for an operator account token when setting up Companion, API clients, CLI automation, or websocket integrations. Clients that chat or run the agent need an account with the `operator` role; new accounts default to `viewer`. Operator token exchange is available at `POST /auth/operator-token`. diff --git a/docs/a2a.md b/docs/a2a.md index d04f5e74..51d6d382 100644 --- a/docs/a2a.md +++ b/docs/a2a.md @@ -88,7 +88,7 @@ With that configuration, the Agent Card advertises endpoints such as `https://ag ## Authentication -Discovery is public by default so standard A2A card resolvers can fetch the Agent Card. Execution endpoints continue to use the gateway authentication and IP rate limiting policy. +Discovery is public by default so standard A2A card resolvers can fetch the Agent Card. Execution endpoints continue to use the gateway authentication and IP rate limiting policy, and require the `operator` role. For public deployments, configure gateway authentication before exposing the A2A execution paths. diff --git a/docs/workflow-backends.md b/docs/workflow-backends.md index 6a41f438..3d3f0209 100644 --- a/docs/workflow-backends.md +++ b/docs/workflow-backends.md @@ -43,9 +43,9 @@ MCP tools expose the same surface: | Tool | Purpose | | --- | --- | | `openclaw.list_workflows` | List configured workflow backends. | -| `openclaw.run_workflow` | Start a workflow run. | +| `openclaw.run_workflow` | Start a workflow run. Requires the `operator` role. | | `openclaw.get_workflow_run` | Read current status, events, pending inputs, and output. | -| `openclaw.respond_workflow` | Send a human or system response to a pending input port. | +| `openclaw.respond_workflow` | Send a human or system response to a pending input port. Requires the `operator` role. | ## Status Model diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index 6c4642f7..b390c909 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -148,7 +148,7 @@ WebSocket 请求 (/ws) └─ 通过 ──→ 接受 WebSocket 连接 ``` -**第二步（仅 `/ws`）：`CanSubmitChat`** +**第二步（仅 `/ws`）：`EndpointHelpers.CanExecuteAgent`** 每个 `/ws` 帧都会成为智能体输入，因此连接需要与 `POST /api/integration/messages` 相同的 `operator` 角色。角色通过 `AuthorizeOperatorRequest` 解析，与 HTTP API 使用同一认证链： @@ -177,7 +177,21 @@ WebSocket 已连接 └─ 调用 AuthorizeOperatorRequest() ──→ 提取 AccountId 作为 userId ``` -### 3.3 `IsAuthorizedRequest` 详细逻辑 +### 3.3 智能体执行所需角色 + +`IsAuthorizedRequest` 只确认调用方已通过认证。会把请求变为智能体输入或其他变更的入口，还需要 `operator` 角色（与 `POST /api/integration/messages` 相同），由 `EndpointHelpers.CanExecuteAgent` 检查： + +| 入口 | 角色低于 operator 时 | +|------|----------------------| +| `/ws` | 先接受，再以 1008 (PolicyViolation) 关闭 | +| `POST /v1/chat/completions`、`POST /v1/responses` | 403，返回 OpenAI 风格的 `permission_error` 响应体 | +| A2A 执行路径（发现端点仍然公开） | 403 | +| `POST /apps/chat` | 403。仅凭回环客户端 IP 不再足够 | +| MCP `openclaw.send_message`、`openclaw.run_workflow`、`openclaw.respond_workflow` | 返回工具错误结果；只读 MCP 工具对 viewer 仍可用 | + +引导令牌和开放回环会解析为 `admin`，不受影响。新建的操作员账户默认为 `viewer`，因此用于 Companion、CLI/TUI 聊天或 API 客户端的账户需要 `operator` 角色。 + +### 3.4 `IsAuthorizedRequest` 详细逻辑 ```csharp // 第 1 步：Loopback 豁免 diff --git a/src/OpenClaw.Client/OpenClawWebSocketClient.cs b/src/OpenClaw.Client/OpenClawWebSocketClient.cs index bb615b11..1621f13d 100644 --- a/src/OpenClaw.Client/OpenClawWebSocketClient.cs +++ b/src/OpenClaw.Client/OpenClawWebSocketClient.cs @@ -35,6 +35,13 @@ public bool IsConnected public event Action? OnEnvelopeReceived; public event Action? OnError; + /// + /// Raised when the gateway closes the connection, with its close status and reason + /// (for example 1008 PolicyViolation when the account lacks the operator role). + /// Not raised when this client disconnects. + /// + public event Action? OnClosed; + public async Task ConnectAsync(Uri wsUri, string? bearerToken, CancellationToken ct) { await DisconnectAsync(ct); @@ -171,7 +178,10 @@ private async Task ReceiveLoopAsync(WebSocket ws, CancellationToken ct) { result = await ws.ReceiveAsync(buffer, ct); if (result.MessageType == WebSocketMessageType.Close) + { + OnClosed?.Invoke(result.CloseStatus, result.CloseStatusDescription); return; + } if (writer.WrittenCount + result.Count > _maxMessageBytes) throw new InvalidOperationException("Inbound message too large."); diff --git a/src/OpenClaw.Companion/Services/GatewayWebSocketClient.cs b/src/OpenClaw.Companion/Services/GatewayWebSocketClient.cs index 879c3075..95775184 100644 --- a/src/OpenClaw.Companion/Services/GatewayWebSocketClient.cs +++ b/src/OpenClaw.Companion/Services/GatewayWebSocketClient.cs @@ -12,6 +12,7 @@ public GatewayWebSocketClient(int maxMessageBytes = 256 * 1024) _inner.OnTextMessage += text => OnTextMessage?.Invoke(text); _inner.OnEnvelopeReceived += envelope => OnEnvelopeReceived?.Invoke(envelope); _inner.OnError += error => OnError?.Invoke(error); + _inner.OnClosed += (status, reason) => OnClosed?.Invoke(status, reason); } public bool IsConnected @@ -20,6 +21,7 @@ public bool IsConnected public event Action? OnTextMessage; public event Action? OnEnvelopeReceived; public event Action? OnError; + public event Action? OnClosed; public async Task ConnectAsync(Uri wsUri, string? bearerToken, CancellationToken ct) => await _inner.ConnectAsync(wsUri, bearerToken, ct); @@ -45,4 +47,12 @@ internal void SetConnectedSocketForTest(WebSocket ws) System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); method?.Invoke(_inner, [ws]); } + + internal Task RunReceiveLoopForTest(WebSocket ws, CancellationToken ct) + { + var method = typeof(OpenClaw.Client.OpenClawWebSocketClient).GetMethod( + "RunReceiveLoopForTest", + System.Reflection.BindingFlags.Instance | System.Reflection.BindingFlags.NonPublic); + return (Task)method!.Invoke(_inner, [ws, ct])!; + } } diff --git a/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs index 05820357..42e405fe 100644 --- a/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs +++ b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs @@ -104,6 +104,7 @@ public MainWindowViewModel( _client.OnTextMessage += HandleInboundText; _client.OnEnvelopeReceived += HandleCanvasEnvelope; _client.OnError += err => AddSystemMessage($"Error: {err}"); + _client.OnClosed += HandleServerClosed; Messages.CollectionChanged += (_, _) => { OnPropertyChanged(nameof(HasMessages)); @@ -445,6 +446,20 @@ private static bool LooksLikeToolFailureText(string? text) private static string AppendNextStep(string? nextStep) => string.IsNullOrWhiteSpace(nextStep) ? string.Empty : $" {nextStep}"; + // Show the gateway's reason: 1008 covers a missing operator role as well as rate and connection limits. + private void HandleServerClosed(System.Net.WebSockets.WebSocketCloseStatus? status, string? reason) + { + var message = string.IsNullOrWhiteSpace(reason) + ? "The gateway closed the connection." + : $"The gateway closed the connection: {reason}"; + Dispatcher.UIThread.Post(() => + { + IsConnected = false; + Status = "Disconnected"; + AddSystemMessageCore(message); + }); + } + private void AddSystemMessage(string text) { Dispatcher.UIThread.Post(() => AddSystemMessageCore(text)); diff --git a/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs b/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs index c23e8097..4b501af5 100644 --- a/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs +++ b/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs @@ -74,6 +74,12 @@ public static void UseOpenClawA2AAuth( return; } + if (!EndpointHelpers.CanExecuteAgent(ctx, startup)) + { + await EndpointHelpers.WriteOperatorRoleRequiredAsync(ctx); + return; + } + if (!runtime.Operations.ActorRateLimits.TryConsume( "ip", EndpointHelpers.GetRemoteIpKey(ctx), diff --git a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs index ce3df42a..4247bf0e 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs @@ -47,6 +47,14 @@ public static void MapOpenClawAppsEndpoints( return; } + // AppsAuthorized admits any loopback client IP, which behind a same-host proxy is every caller; + // running the agent needs a resolved operator identity on top of that. + if (!EndpointHelpers.CanExecuteAgent(ctx, startup)) + { + await EndpointHelpers.WriteOperatorRoleRequiredAsync(ctx); + return; + } + JsonNode? body; try { diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 032bca72..a822b19a 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -1,5 +1,6 @@ using System.Security.Claims; using System.Text; +using System.Text.Json; using Microsoft.AspNetCore.Http.Features; using OpenClaw.Core.Models; using OpenClaw.Core.Security; @@ -326,6 +327,31 @@ public static (OperatorAuthorizationResult? Authorization, IResult? Failure) Aut return (auth, null); } + internal const string OperatorRoleRequiredMessage = "This action requires the operator role."; + + /// + /// Surfaces that turn a request into agent input or another mutation (chat, the OpenAI-compatible API, + /// A2A, MCP Apps chat, mutating MCP tools) require the same role as POST /api/integration/messages. + /// Authentication alone is not enough: viewer credentials must stay read-only. + /// + public static bool CanExecuteAgent(HttpContext ctx, GatewayStartupContext startup) + { + var browserSessions = ctx.RequestServices.GetRequiredService(); + var auth = AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); + return auth.IsAuthorized && IsRoleAllowed(auth.Role, "integration.mutate.agent", out _); + } + + public static async Task WriteOperatorRoleRequiredAsync(HttpContext ctx) + { + ctx.Response.StatusCode = StatusCodes.Status403Forbidden; + ctx.Response.ContentType = "application/json"; + await JsonSerializer.SerializeAsync( + ctx.Response.Body, + new OperationStatusResponse { Success = false, Error = OperatorRoleRequiredMessage }, + CoreJsonContext.Default.OperationStatusResponse, + ctx.RequestAborted); + } + public static bool IsRoleAllowed(string grantedRole, string endpointScope, out string requiredRole) { requiredRole = GetRequiredRole(endpointScope); diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs index 504d6115..356c3f00 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs @@ -22,6 +22,9 @@ private static void MapChatCompletionsEndpoint( return; } + if (await TryRejectBelowOperatorAsync(ctx, startup)) + return; + if (!runtime.Operations.ActorRateLimits.TryConsume("ip", EndpointHelpers.GetRemoteIpKey(ctx), "openai_http", out var blockedByPolicyId)) { ctx.Response.StatusCode = StatusCodes.Status429TooManyRequests; diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs index 9269bc0f..0adfdfe2 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs @@ -22,6 +22,9 @@ private static void MapResponsesEndpoint( return; } + if (await TryRejectBelowOperatorAsync(ctx, startup)) + return; + if (!runtime.Operations.ActorRateLimits.TryConsume("ip", EndpointHelpers.GetRemoteIpKey(ctx), "openai_http", out var blockedByPolicyId)) { ctx.Response.StatusCode = StatusCodes.Status429TooManyRequests; diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs index ac2fa773..e747aab8 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs @@ -10,6 +10,10 @@ internal static partial class OpenAiEndpoints private const string StableSessionHeader = "X-OpenClaw-Session-Id"; private const string NoImplicitToolsAllowed = "__openclaw_openai_no_implicit_tools__"; + // OpenAI-shaped so SDK clients surface the message instead of a bare status. + private const string OperatorRoleRequiredErrorJson = + $$$"""{"error":{"message":"{{{EndpointHelpers.OperatorRoleRequiredMessage}}}","type":"permission_error","code":"insufficient_role"}}"""; + public static void MapOpenClawOpenAiEndpoints( this WebApplication app, GatewayStartupContext startup, @@ -19,6 +23,17 @@ public static void MapOpenClawOpenAiEndpoints( MapResponsesEndpoint(app, startup, runtime); } + private static async Task TryRejectBelowOperatorAsync(HttpContext ctx, GatewayStartupContext startup) + { + if (EndpointHelpers.CanExecuteAgent(ctx, startup)) + return false; + + ctx.Response.StatusCode = StatusCodes.Status403Forbidden; + ctx.Response.ContentType = "application/json"; + await ctx.Response.WriteAsync(OperatorRoleRequiredErrorJson, ctx.RequestAborted); + return true; + } + private static void ApplyImplicitToolPolicy(Session session, GatewayAppRuntime runtime, string? presetId) { ClearImplicitToolSuppression(session); diff --git a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs index 5e3e49dc..af908099 100644 --- a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs @@ -26,9 +26,9 @@ public static void MapOpenClawWebSocketEndpoints( // Every /ws frame becomes agent input, so require the same role as POST /api/integration/messages. // Close after accepting rather than returning 403: browsers cannot read a failed handshake's status, // but web chat treats close code 1008 as an authorization failure and stops reconnecting. - if (!CanSubmitChat(ctx, startup)) + if (!EndpointHelpers.CanExecuteAgent(ctx, startup)) { - await ws.CloseAsync(WebSocketCloseStatus.PolicyViolation, "Chat requires the operator role.", ctx.RequestAborted); + await ws.CloseAsync(WebSocketCloseStatus.PolicyViolation, EndpointHelpers.OperatorRoleRequiredMessage, ctx.RequestAborted); return; } @@ -105,13 +105,6 @@ private static bool TryValidateWebSocketRequest( return true; } - internal static bool CanSubmitChat(HttpContext ctx, GatewayStartupContext startup) - { - var browserSessions = ctx.RequestServices.GetRequiredService(); - var auth = EndpointHelpers.AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); - return auth.IsAuthorized && EndpointHelpers.IsRoleAllowed(auth.Role, "integration.mutate.chat", out _); - } - internal static bool TryResolveAuthorizedUserIdForWebSocket(HttpContext ctx, GatewayStartupContext startup, out string? authenticatedUserId) { authenticatedUserId = null; diff --git a/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs b/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs index 5efe4df7..f8279a37 100644 --- a/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs +++ b/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs @@ -23,6 +23,8 @@ public static IServiceCollection AddOpenClawMcpServices( GatewayStartupContext startup) { services.TryAddSingleton(); + // Mutating MCP tools check the caller's role; HTTP handlers run in the request's execution context. + services.AddHttpContextAccessor(); services.AddSingleton(sp => { diff --git a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs index 2efc2d6a..196d1357 100644 --- a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs +++ b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs @@ -1,8 +1,11 @@ using System.ComponentModel; using System.Text.Json; +using ModelContextProtocol; using ModelContextProtocol.Server; using OpenClaw.Core.Models; +using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Composition; +using OpenClaw.Gateway.Endpoints; namespace OpenClaw.Gateway.Mcp; @@ -15,8 +18,15 @@ namespace OpenClaw.Gateway.Mcp; internal sealed class OpenClawMcpTools { private readonly IntegrationApiFacade _facade; + private readonly IHttpContextAccessor _httpContextAccessor; + private readonly GatewayStartupContext _startup; - public OpenClawMcpTools(IntegrationApiFacade facade) => _facade = facade; + public OpenClawMcpTools(IntegrationApiFacade facade, IHttpContextAccessor httpContextAccessor, GatewayStartupContext startup) + { + _facade = facade; + _httpContextAccessor = httpContextAccessor; + _startup = startup; + } [McpServerTool(Name = "openclaw.get_dashboard", ReadOnly = true), Description("Get the aggregated operator dashboard snapshot.")] @@ -207,7 +217,9 @@ public async Task RunWorkflow( [Description("Optional sender ID.")] string? senderId = null, [Description("Optional session ID.")] string? sessionId = null, CancellationToken ct = default) - => JsonSerializer.Serialize( + { + RequireOperator(); + return JsonSerializer.Serialize( await _facade.RunWorkflowAsync( workflowId, new AgentWorkflowRequest @@ -220,6 +232,7 @@ await _facade.RunWorkflowAsync( }, ct), CoreJsonContext.Default.AgentWorkflowRunResult); + } [McpServerTool(Name = "openclaw.get_workflow_run", ReadOnly = true), Description("Get the current status snapshot for a durable workflow run.")] @@ -242,7 +255,9 @@ public async Task RespondWorkflow( [Description("Optional actor ID for the responder.")] string? actorId = null, [Description("Optional JSON object or value passed as response payload.")] string? payloadJson = null, CancellationToken ct = default) - => JsonSerializer.Serialize( + { + RequireOperator(); + return JsonSerializer.Serialize( await _facade.RespondWorkflowRunAsync( workflowId, runId, @@ -256,6 +271,7 @@ await _facade.RespondWorkflowRunAsync( }, ct), CoreJsonContext.Default.AgentWorkflowRunSnapshot); + } [McpServerTool(Name = "openclaw.query_runtime_events", ReadOnly = true), Description("Query recent runtime events.")] @@ -288,7 +304,9 @@ public async Task SendMessage( [Description("Optional idempotency message ID.")] string? messageId = null, [Description("Optional reply-to message ID.")] string? replyToMessageId = null, CancellationToken ct = default) - => JsonSerializer.Serialize( + { + RequireOperator(); + return JsonSerializer.Serialize( await _facade.QueueMessageAsync(new IntegrationMessageRequest { Text = text, @@ -299,6 +317,16 @@ await _facade.QueueMessageAsync(new IntegrationMessageRequest ReplyToMessageId = replyToMessageId }, ct), CoreJsonContext.Default.IntegrationMessageResponse); + } + + // Read-only tools stay open to viewers; mutating tools require the role their REST equivalents require. + // Without a request context (e.g. a detached task) the caller is unknown, so deny. + private void RequireOperator() + { + var ctx = _httpContextAccessor.HttpContext; + if (ctx is null || !EndpointHelpers.CanExecuteAgent(ctx, _startup)) + throw new McpException(EndpointHelpers.OperatorRoleRequiredMessage); + } private static JsonElement? ParsePayloadJson(string? payloadJson) { diff --git a/src/OpenClaw.Tests/AppsEndpointsTests.cs b/src/OpenClaw.Tests/AppsEndpointsTests.cs index 8d593fbc..6e687d49 100644 --- a/src/OpenClaw.Tests/AppsEndpointsTests.cs +++ b/src/OpenClaw.Tests/AppsEndpointsTests.cs @@ -18,6 +18,7 @@ using OpenClaw.Core.Pipeline; using OpenClaw.Core.Plugins; using OpenClaw.Core.Security; +using OpenClaw.Gateway; using OpenClaw.Core.Sessions; using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Composition; @@ -179,6 +180,7 @@ await File.WriteAllTextAsync( var builder = WebApplication.CreateSlimBuilder(); builder.WebHost.UseTestServer(); builder.Services.AddOpenClawMcpAppServices(config.McpApps); + builder.Services.AddSingleton(new BrowserSessionAuthService(config)); var runtime = CreateRuntime(config, agentRuntime); var app = builder.Build(); diff --git a/src/OpenClaw.Tests/CompanionConnectionTests.cs b/src/OpenClaw.Tests/CompanionConnectionTests.cs new file mode 100644 index 00000000..66eed41a --- /dev/null +++ b/src/OpenClaw.Tests/CompanionConnectionTests.cs @@ -0,0 +1,68 @@ +using System.Net.WebSockets; +using Avalonia.Headless.XUnit; +using Avalonia.Threading; +using OpenClaw.Companion.Models; +using OpenClaw.Companion.Services; +using OpenClaw.Companion.ViewModels; +using Xunit; + +namespace OpenClaw.Tests; + +public sealed class CompanionConnectionTests : IDisposable +{ + private readonly List _tempDirs = []; + + public void Dispose() + { + foreach (var dir in _tempDirs) + { + try { Directory.Delete(dir, recursive: true); } + catch { } + } + } + + [AvaloniaFact] + public async Task ServerClose_WhenPolicyViolation_ShouldDisconnectAndExplainOperatorRole() + { + var (vm, client) = CreateConnectedViewModel(); + var ws = new TestWebSocket(); + ws.QueueClose(WebSocketCloseStatus.PolicyViolation, "This action requires the operator role."); + + await client.RunReceiveLoopForTest(ws, CancellationToken.None); + Dispatcher.UIThread.RunJobs(); + + Assert.False(vm.IsConnected); + Assert.Equal("Disconnected", vm.Status); + var message = Assert.Single(vm.Messages, m => m.Role == ChatRole.System); + Assert.Contains("operator role", message.Text, StringComparison.OrdinalIgnoreCase); + } + + [AvaloniaFact] + public async Task ServerClose_WhenNormalClosure_ShouldDisconnectWithoutRoleHint() + { + var (vm, client) = CreateConnectedViewModel(); + var ws = new TestWebSocket(); + ws.QueueClose(WebSocketCloseStatus.NormalClosure, "shutting down"); + + await client.RunReceiveLoopForTest(ws, CancellationToken.None); + Dispatcher.UIThread.RunJobs(); + + Assert.False(vm.IsConnected); + var message = Assert.Single(vm.Messages, m => m.Role == ChatRole.System); + Assert.DoesNotContain("operator", message.Text, StringComparison.OrdinalIgnoreCase); + } + + private (MainWindowViewModel ViewModel, GatewayWebSocketClient Client) CreateConnectedViewModel() + { + var dir = Path.Combine(Path.GetTempPath(), "openclaw-companion-connection-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + _tempDirs.Add(dir); + var client = new GatewayWebSocketClient(); + var vm = new MainWindowViewModel(new SettingsStore(dir), client) + { + IsConnected = true, + Status = "Connected" + }; + return (vm, client); + } +} diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index 640f4a34..5d37e89d 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -33,6 +33,7 @@ using OpenClaw.Core.Sessions; using OpenClaw.Core.Skills; using OpenClaw.Gateway; +using OpenClaw.Gateway.A2A; using OpenClaw.Gateway.Backends; using OpenClaw.Gateway.Bootstrap; using ModelContextProtocol.AspNetCore; @@ -42,6 +43,7 @@ using OpenClaw.Gateway.Tools; using OpenClaw.Gateway.Mcp; using OpenClaw.Gateway.Models; +using OpenClaw.MicrosoftAgentFrameworkAdapter; using OpenClaw.Payments.Core; using Xunit; @@ -376,6 +378,143 @@ public async Task WebSocketChat_WhenOpenLoopbackWithoutCredentials_ShouldStayOpe Assert.Null(await ReceiveWithinAsync(ws, TimeSpan.FromMilliseconds(500))); } + [Theory] + [InlineData("/v1/chat/completions", """{"messages":[{"role":"user","content":"hello"}]}""")] + [InlineData("/v1/responses", """{"input":"hello"}""")] + public async Task OpenAiEndpoints_WhenViewerAccountToken_ShouldReturnForbiddenWithoutRunningAgent(string path, string body) + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "openai-viewer", OperatorRoleNames.Viewer); + + using var request = new HttpRequestMessage(HttpMethod.Post, path) { Content = JsonContent(body) }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + var response = await harness.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + using var payload = await ReadJsonAsync(response); + var error = payload.RootElement.GetProperty("error"); + Assert.Equal("permission_error", error.GetProperty("type").GetString()); + Assert.Contains("operator", error.GetProperty("message").GetString(), StringComparison.OrdinalIgnoreCase); + await harness.Runtime.AgentRuntime.DidNotReceiveWithAnyArgs().RunAsync(default!, default!, default, default, default); + } + + [Fact] + public async Task ChatCompletions_WhenOperatorAccountToken_ShouldRunAgent() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "openai-operator", OperatorRoleNames.Operator); + harness.Runtime.AgentRuntime.RunAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns("operator reply"); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/v1/chat/completions") + { + Content = JsonContent("""{"messages":[{"role":"user","content":"hello"}]}""") + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + var response = await harness.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + using var payload = await ReadJsonAsync(response); + Assert.Equal("operator reply", payload.RootElement.GetProperty("choices")[0].GetProperty("message").GetProperty("content").GetString()); + } + + [Fact] + public async Task AppsChat_WhenViewerAccountToken_ShouldReturnForbiddenWithoutRunningAgent() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "apps-viewer", OperatorRoleNames.Viewer); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/apps/chat") { Content = JsonContent("""{"message":"hello"}""") }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + var response = await harness.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + harness.Runtime.AgentRuntime.DidNotReceiveWithAnyArgs().RunStreamingAsync(default!, default!, default); + } + + [Fact] + public async Task AppsChat_WhenLoopbackClientReachesNonLoopbackBindWithoutCredentials_ShouldNotRunAgent() + { + // A same-host reverse proxy without trusted forwarded headers makes every caller look like loopback. + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configureApp: app => app.Use(async (ctx, next) => + { + ctx.Connection.RemoteIpAddress = IPAddress.Loopback; + await next(ctx); + })); + + var response = await harness.Client.PostAsync("/apps/chat", JsonContent("""{"message":"hello"}""")); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + harness.Runtime.AgentRuntime.DidNotReceiveWithAnyArgs().RunStreamingAsync(default!, default!, default); + } + + [Theory] + [InlineData(OperatorRoleNames.Viewer, HttpStatusCode.Forbidden)] + [InlineData(OperatorRoleNames.Operator, HttpStatusCode.OK)] + public async Task A2AExecution_WhenAccountToken_ShouldRequireOperatorRole(string role, HttpStatusCode expected) + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configureServices: (services, _) => services.Configure(options => options.EnableA2A = true), + configureApp: app => app.MapPost("/a2a", () => Results.Ok())); + var token = CreateAccountToken(harness, "a2a-" + role, role); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/a2a") { Content = JsonContent("{}") }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + var response = await harness.Client.SendAsync(request); + + Assert.Equal(expected, response.StatusCode); + } + + [Theory] + [InlineData("openclaw.send_message", """{"text":"hello"}""")] + [InlineData("openclaw.run_workflow", """{"workflowId":"wf","input":"hello"}""")] + [InlineData("openclaw.respond_workflow", """{"workflowId":"wf","runId":"run","portId":"port"}""")] + public async Task McpMutatingTool_WhenViewerAccountToken_ShouldReturnToolError(string toolName, string arguments) + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "mcp-viewer", OperatorRoleNames.Viewer); + + using var result = await CallMcpToolAsync(harness, token, toolName, arguments); + + Assert.True(result.RootElement.GetProperty("isError").GetBoolean()); + Assert.Contains("operator", result.RootElement.GetProperty("content")[0].GetProperty("text").GetString(), StringComparison.OrdinalIgnoreCase); + } + + [Fact] + public async Task McpReadOnlyTool_WhenViewerAccountToken_ShouldSucceed() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "mcp-reader", OperatorRoleNames.Viewer); + + using var result = await CallMcpToolAsync(harness, token, "openclaw.get_status", "{}"); + + Assert.False(result.RootElement.TryGetProperty("isError", out var isError) && isError.GetBoolean()); + Assert.Contains("activeSessions", result.RootElement.GetProperty("content")[0].GetProperty("text").GetString()); + } + + private static async Task CallMcpToolAsync(GatewayTestHarness harness, string bearerToken, string toolName, string argumentsJson) + { + using var request = new HttpRequestMessage(HttpMethod.Post, "/mcp") + { + Content = JsonContent($$$"""{"jsonrpc":"2.0","id":1,"method":"tools/call","params":{"name":"{{{toolName}}}","arguments":{{{argumentsJson}}}}}""") + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", bearerToken); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/event-stream")); + var response = await harness.Client.SendAsync(request); + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + using var payload = await ReadMcpJsonAsync(response); + return JsonDocument.Parse(payload.RootElement.GetProperty("result").GetRawText()); + } + private static string CreateAccountToken(GatewayTestHarness harness, string username, string role) { var operatorAccounts = harness.App.Services.GetRequiredService(); @@ -7823,9 +7962,10 @@ private static async Task CreateHarnessAsync( Action? configure = null, Func? memoryStoreFactory = null, Action? configureServices = null, - GatewayRuntimeState? runtimeStateOverride = null) + GatewayRuntimeState? runtimeStateOverride = null, + Action? configureApp = null) { - return await CreateHarnessAsyncInternal(nonLoopbackBind, configure, memoryStoreFactory, configureServices, runtimeStateOverride); + return await CreateHarnessAsyncInternal(nonLoopbackBind, configure, memoryStoreFactory, configureServices, runtimeStateOverride, configureApp); } private static async Task CreateHarnessAsyncInternal( @@ -7833,7 +7973,8 @@ private static async Task CreateHarnessAsyncInternal( Action? configure, Func? memoryStoreFactory, Action? configureServices, - GatewayRuntimeState? runtimeStateOverride) + GatewayRuntimeState? runtimeStateOverride, + Action? configureApp) { var storagePath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "openclaw-admin-tests", Guid.NewGuid().ToString("N")); Directory.CreateDirectory(storagePath); @@ -7949,6 +8090,8 @@ private static async Task CreateHarnessAsyncInternal( var runtime = CreateRuntime(config, storagePath, memoryStore, sessionManager, heartbeatService); app.InitializeMcpRuntime(runtime); app.UseOpenClawMcpAuth(startup, runtime); + app.UseOpenClawA2AAuth(startup, runtime); + configureApp?.Invoke(app); app.MapOpenApi("/openapi/{documentName}.json"); app.MapOpenClawEndpoints(startup, runtime); app.MapMcp("/mcp"); diff --git a/src/OpenClaw.Tests/OpenClawWebSocketClientTests.cs b/src/OpenClaw.Tests/OpenClawWebSocketClientTests.cs index 49fe976d..fa5908fa 100644 --- a/src/OpenClaw.Tests/OpenClawWebSocketClientTests.cs +++ b/src/OpenClaw.Tests/OpenClawWebSocketClientTests.cs @@ -1,3 +1,4 @@ +using System.Net.WebSockets; using OpenClaw.Client; using Xunit; @@ -54,4 +55,35 @@ public async Task ReceiveLoop_CallbackException_RaisesOnErrorAndContinues() Assert.Equal(["first", "second"], received); Assert.Equal("boom", error); } + + [Fact] + public async Task ReceiveLoop_WhenServerCloses_ShouldRaiseOnClosedWithStatusAndReason() + { + var client = new OpenClawWebSocketClient(); + var ws = new TestWebSocket(); + ws.QueueClose(WebSocketCloseStatus.PolicyViolation, "This action requires the operator role."); + (WebSocketCloseStatus? Status, string? Reason)? closed = null; + client.OnClosed += (status, reason) => closed = (status, reason); + + await client.RunReceiveLoopForTest(ws, TestContext.Current.CancellationToken); + + Assert.Equal((WebSocketCloseStatus.PolicyViolation, "This action requires the operator role."), closed); + } + + [Fact] + public async Task ReceiveLoop_WhenCancelledByClient_ShouldNotRaiseOnClosed() + { + var client = new OpenClawWebSocketClient(); + var ws = new TestWebSocket(); + ws.BlockReceiveUntilCancelled(); + var raised = false; + client.OnClosed += (_, _) => raised = true; + using var cts = new CancellationTokenSource(); + + var loop = client.RunReceiveLoopForTest(ws, cts.Token); + await cts.CancelAsync(); + await loop; + + Assert.False(raised); + } } diff --git a/src/OpenClaw.Tests/TestWebSocket.cs b/src/OpenClaw.Tests/TestWebSocket.cs index 9144ab6e..2a6c8c00 100644 --- a/src/OpenClaw.Tests/TestWebSocket.cs +++ b/src/OpenClaw.Tests/TestWebSocket.cs @@ -13,6 +13,7 @@ internal sealed class TestWebSocket : WebSocket private TaskCompletionSource? _sendRelease; private WebSocketState _state = WebSocketState.Open; + private (WebSocketCloseStatus? Status, string? Description) _queuedClose; public IReadOnlyCollection Sent => _sent.ToArray(); @@ -22,8 +23,11 @@ public void QueueReceiveText(string text, bool endOfMessage = true) public void QueueReceiveBytes(byte[] bytes, bool endOfMessage) => _receive.Enqueue((bytes, WebSocketMessageType.Text, endOfMessage)); - public void QueueClose() - => _receive.Enqueue((Array.Empty(), WebSocketMessageType.Close, true)); + public void QueueClose(WebSocketCloseStatus? status = null, string? description = null) + { + _queuedClose = (status, description); + _receive.Enqueue((Array.Empty(), WebSocketMessageType.Close, true)); + } public void QueueReceiveException(Exception exception) => _receiveExceptions.Enqueue(exception); @@ -43,8 +47,8 @@ public Task WaitForSendToStartAsync() public void ReleaseBlockedSend() => _sendRelease?.TrySetResult(true); - public override WebSocketCloseStatus? CloseStatus { get; } - public override string? CloseStatusDescription { get; } + public override WebSocketCloseStatus? CloseStatus => _state == WebSocketState.CloseReceived ? _queuedClose.Status : null; + public override string? CloseStatusDescription => _state == WebSocketState.CloseReceived ? _queuedClose.Description : null; public override WebSocketState State => _state; public override string? SubProtocol { get; } @@ -79,7 +83,7 @@ public override Task ReceiveAsync(ArraySegment buf if (next.Type == WebSocketMessageType.Close) { _state = WebSocketState.CloseReceived; - return Task.FromResult(new WebSocketReceiveResult(0, WebSocketMessageType.Close, true)); + return Task.FromResult(new WebSocketReceiveResult(0, WebSocketMessageType.Close, true, _queuedClose.Status, _queuedClose.Description)); } if (buffer.Array is null) From e13fd9bb3de43e77031c40ebb61147be9d502b8b Mon Sep 17 00:00:00 2001 From: telli Date: Sun, 27 Sep 2026 21:05:56 -0700 Subject: [PATCH 3/8] fix(gateway): stop trusting loopback client IPs on /apps routes /apps/health, /apps/chat, and the /apps/mcp/{appId} proxy admitted any request whose client IP was loopback, independent of the bind address and AlwaysRequireAuth. Behind a same-host reverse proxy without TrustForwardedHeaders every caller has a loopback IP, so these routes answered unauthenticated requests: /apps/mcp served MCP App tool calls and /apps/health leaked app details, even with AlwaysRequireAuth on. Use the gateway's bind-based rule like every other endpoint: open only on a loopback-bound gateway without AlwaysRequireAuth. Local MCP App development on a loopback-bound gateway is unchanged. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 3 +- docs/AUTHENTICATION.md | 2 +- docs/MCPAPP.md | 2 + docs/zh-CN/AUTHENTICATION.md | 2 +- docs/zh-CN/MCPAPP.md | 4 +- .../Endpoints/AppsEndpoints.cs | 12 +-- .../Endpoints/AppsMcpProxyEndpoint.cs | 6 +- .../GatewayAdminEndpointTests.cs | 81 ++++++++++++++++--- 8 files changed, 86 insertions(+), 26 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 312f9cf0..db1d7fbb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -40,8 +40,9 @@ All notable changes to this project are tracked in this file. - `/ws`: closed with code 1008, which web chat reports as an authorization failure. - `POST /v1/chat/completions` and `POST /v1/responses`: 403 with an OpenAI-style `permission_error` body. - A2A execution paths: 403. Discovery stays public. - - `POST /apps/chat`: 403. It also no longer runs the agent for an unauthenticated caller whose client IP is loopback, which behind a same-host reverse proxy without `TrustForwardedHeaders` was every caller. + - `POST /apps/chat`: 403. - MCP `openclaw.send_message`, `openclaw.run_workflow`, and `openclaw.respond_workflow`: tool error result. Read-only MCP tools stay available to viewers. +- Stopped trusting a loopback client IP on `/apps/health`, `/apps/chat`, and `/apps/mcp/{appId}`. Behind a same-host reverse proxy without `TrustForwardedHeaders`, every caller has a loopback IP, so these routes answered unauthenticated requests, including agent runs and MCP App tool calls, and ignored `AlwaysRequireAuth`. They now follow the gateway's bind-based rule: open only on a loopback-bound gateway without `AlwaysRequireAuth`. - Added `OpenClawWebSocketClient.OnClosed`, raised with the gateway's close status and reason. Companion now marks itself disconnected and shows the reason instead of appearing connected after the gateway closes the socket. - Bound tool-approval decisions to the original requester (`channelId` + `senderId`) for non-loopback/public binds. - Kept `POST /tools/approve` as an explicit admin override path. diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index 136c2b3c..5e0ca954 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -186,7 +186,7 @@ WebSocket connected | `/ws` | Accepted, then closed with 1008 (PolicyViolation) | | `POST /v1/chat/completions`, `POST /v1/responses` | 403 with an OpenAI-style `permission_error` body | | A2A execution paths (discovery stays public) | 403 | -| `POST /apps/chat` | 403. A loopback client IP no longer suffices on its own | +| `POST /apps/chat` | 403 | | MCP `openclaw.send_message`, `openclaw.run_workflow`, `openclaw.respond_workflow` | Tool error result; read-only MCP tools stay available to viewers | Bootstrap tokens and open loopback resolve to `admin` and are unaffected. New operator accounts default to `viewer`, so accounts used for Companion, CLI/TUI chat, or API clients need the `operator` role. diff --git a/docs/MCPAPP.md b/docs/MCPAPP.md index be19c2ed..984183f5 100644 --- a/docs/MCPAPP.md +++ b/docs/MCPAPP.md @@ -42,6 +42,8 @@ OpenClaw.NET exposes a small gateway-facing host surface for browser-side MCP Ap The important detail is that browser UIs should connect to `/apps/mcp/{appId}`, not directly to the App's raw upstream MCP URL. That keeps browser-driven MCP calls and model-driven MCP calls on the same OpenClaw-managed session. +The `/apps/*` routes use the gateway's normal authentication. They are open without credentials only when the gateway is bound to loopback and `AlwaysRequireAuth` is off. Otherwise the browser host must send a token or browser session. A loopback client IP is not trusted on its own: behind a same-host reverse proxy every caller has one. + ### Session Reuse Behavior - `/apps/health` returns an `mcp` URL that points back to the gateway's own `/apps/mcp/{appId}` route. diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index b390c909..b67a0e37 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -186,7 +186,7 @@ WebSocket 已连接 | `/ws` | 先接受，再以 1008 (PolicyViolation) 关闭 | | `POST /v1/chat/completions`、`POST /v1/responses` | 403，返回 OpenAI 风格的 `permission_error` 响应体 | | A2A 执行路径（发现端点仍然公开） | 403 | -| `POST /apps/chat` | 403。仅凭回环客户端 IP 不再足够 | +| `POST /apps/chat` | 403 | | MCP `openclaw.send_message`、`openclaw.run_workflow`、`openclaw.respond_workflow` | 返回工具错误结果；只读 MCP 工具对 viewer 仍可用 | 引导令牌和开放回环会解析为 `admin`，不受影响。新建的操作员账户默认为 `viewer`，因此用于 Companion、CLI/TUI 聊天或 API 客户端的账户需要 `operator` 角色。 diff --git a/docs/zh-CN/MCPAPP.md b/docs/zh-CN/MCPAPP.md index 47418f90..8b95e216 100644 --- a/docs/zh-CN/MCPAPP.md +++ b/docs/zh-CN/MCPAPP.md @@ -37,11 +37,13 @@ OpenClaw.NET 为浏览器侧 MCP App UI 暴露了一组面向 gateway 的 host | 路由 | 用途 | |------|------| | `/apps/health` | 返回当前选中的 MCP App id，以及浏览器应连接的 gateway MCP 端点 | -| `/apps/chat` | 把浏览器 host 的聊天请求桥接到现有 `GatewayAppRuntime`，并输出 `session`、`text`、`tool`、`result`、`done` 形状的 SSE | +| `/apps/chat` | 把浏览器 host 的聊天请求桥接到现有 `GatewayAppRuntime`，并输出 `session`、`text`、`tool`、`result`、`done` 形状的 SSE。需要 `operator` 角色 | | `/apps/mcp/{appId}` | 把 MCP 请求代理到该 App 已经连接好的 `McpClient` | 关键点是：浏览器 UI 应连接 `/apps/mcp/{appId}`，而不是直接连接 MCP App 的原始上游 URL。这样浏览器触发的 MCP 调用与 Agent 触发的 MCP 调用才能落在同一条 OpenClaw 管理的会话上。 +`/apps/*` 路由使用 gateway 的常规认证。只有当 gateway 绑定在回环地址且 `AlwaysRequireAuth` 关闭时，才无需凭据即可访问；否则浏览器 host 必须携带令牌或浏览器会话。仅凭回环客户端 IP 不会被信任：在同机反向代理之后，每个调用方的 IP 都是回环地址。 + ### 会话复用行为 - `/apps/health` 返回的 `mcp` 字段指向 gateway 自己的 `/apps/mcp/{appId}` 路由。 diff --git a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs index 4247bf0e..f88936e8 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs @@ -47,8 +47,6 @@ public static void MapOpenClawAppsEndpoints( return; } - // AppsAuthorized admits any loopback client IP, which behind a same-host proxy is every caller; - // running the agent needs a resolved operator identity on top of that. if (!EndpointHelpers.CanExecuteAgent(ctx, startup)) { await EndpointHelpers.WriteOperatorRoleRequiredAsync(ctx); @@ -125,14 +123,10 @@ public static void MapOpenClawAppsEndpoints( }); } + // Local trust follows the bind address, as on every other endpoint. A loopback client IP is not an + // identity: behind a same-host reverse proxy every caller has one, and it would bypass AlwaysRequireAuth. private static bool AppsAuthorized(HttpContext ctx, GatewayStartupContext startup) - { - var ip = ctx.Connection.RemoteIpAddress; - if (ip is not null && System.Net.IPAddress.IsLoopback(ip)) - return true; - - return EndpointHelpers.IsAuthorizedRequest(ctx, startup.Config, startup.IsNonLoopbackBind); - } + => EndpointHelpers.IsAuthorizedRequest(ctx, startup.Config, startup.IsNonLoopbackBind); private static string? AsString(JsonNode? node) => node is JsonValue value && value.TryGetValue(out var text) ? text : null; diff --git a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs index 7a77ce16..9ac86671 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs @@ -81,11 +81,9 @@ public static void MapOpenClawAppsMcpProxy(this WebApplication app, GatewayStart { app.MapMcp("/apps/mcp/{serverId}").AddEndpointFilter(async (ctx, next) => { + // Same bind-based rule as AppsEndpoints: a loopback client IP alone is not trusted. var httpContext = ctx.HttpContext; - var ip = httpContext.Connection.RemoteIpAddress; - var authorized = (ip is not null && System.Net.IPAddress.IsLoopback(ip)) - || EndpointHelpers.IsAuthorizedRequest(httpContext, startup.Config, startup.IsNonLoopbackBind); - if (!authorized) + if (!EndpointHelpers.IsAuthorizedRequest(httpContext, startup.Config, startup.IsNonLoopbackBind)) { httpContext.Response.StatusCode = StatusCodes.Status401Unauthorized; return Results.Empty; diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index 5d37e89d..8da0f6fb 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -43,6 +43,7 @@ using OpenClaw.Gateway.Tools; using OpenClaw.Gateway.Mcp; using OpenClaw.Gateway.Models; +using OpenClaw.McpApp; using OpenClaw.MicrosoftAgentFrameworkAdapter; using OpenClaw.Payments.Core; using Xunit; @@ -440,21 +441,83 @@ public async Task AppsChat_WhenViewerAccountToken_ShouldReturnForbiddenWithoutRu [Fact] public async Task AppsChat_WhenLoopbackClientReachesNonLoopbackBindWithoutCredentials_ShouldNotRunAgent() { - // A same-host reverse proxy without trusted forwarded headers makes every caller look like loopback. - await using var harness = await CreateHarnessAsync( - nonLoopbackBind: true, - configureApp: app => app.Use(async (ctx, next) => - { - ctx.Connection.RemoteIpAddress = IPAddress.Loopback; - await next(ctx); - })); + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true, configureApp: SimulateLoopbackClient); var response = await harness.Client.PostAsync("/apps/chat", JsonContent("""{"message":"hello"}""")); - Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); harness.Runtime.AgentRuntime.DidNotReceiveWithAnyArgs().RunStreamingAsync(default!, default!, default); } + [Theory] + [InlineData("GET", "/apps/health")] + [InlineData("POST", "/apps/mcp/inventory-app")] + public async Task AppsHostRoutes_WhenLoopbackClientReachesNonLoopbackBindWithoutCredentials_ShouldReturnUnauthorized(string method, string path) + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configureServices: AddMcpAppServices, + configureApp: SimulateLoopbackClient); + + var response = await SendAppsHostRequestAsync(harness, method, path, bearerToken: null); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Theory] + [InlineData("GET", "/apps/health")] + [InlineData("POST", "/apps/mcp/inventory-app")] + public async Task AppsHostRoutes_WhenAlwaysRequireAuthOnLoopbackBindWithoutCredentials_ShouldReturnUnauthorized(string method, string path) + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: false, + configure: config => config.Security.AlwaysRequireAuth = true, + configureServices: AddMcpAppServices, + configureApp: SimulateLoopbackClient); + + var response = await SendAppsHostRequestAsync(harness, method, path, bearerToken: null); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task AppsHealth_WhenNonLoopbackBindWithBearerToken_ShouldSucceed() + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configureServices: AddMcpAppServices, + configureApp: SimulateLoopbackClient); + + var response = await SendAppsHostRequestAsync(harness, "GET", "/apps/health", harness.AuthToken); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + } + + private static void AddMcpAppServices(IServiceCollection services, GatewayConfig config) + => services.AddOpenClawMcpAppServices(config.McpApps); + + // A same-host reverse proxy without trusted forwarded headers makes every caller look like loopback. + private static void SimulateLoopbackClient(WebApplication app) + => app.Use(async (ctx, next) => + { + ctx.Connection.RemoteIpAddress = IPAddress.Loopback; + await next(ctx); + }); + + private static async Task SendAppsHostRequestAsync(GatewayTestHarness harness, string method, string path, string? bearerToken) + { + using var request = new HttpRequestMessage(new HttpMethod(method), path); + if (method == "POST") + { + request.Content = JsonContent("""{"jsonrpc":"2.0","id":1,"method":"tools/list","params":{}}"""); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("application/json")); + request.Headers.Accept.Add(new MediaTypeWithQualityHeaderValue("text/event-stream")); + } + if (bearerToken is not null) + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", bearerToken); + return await harness.Client.SendAsync(request); + } + [Theory] [InlineData(OperatorRoleNames.Viewer, HttpStatusCode.Forbidden)] [InlineData(OperatorRoleNames.Operator, HttpStatusCode.OK)] From 2ddec71e4f55d3834b2a472ec4a3b993fc79b4df Mon Sep 17 00:00:00 2001 From: telli Date: Sun, 27 Sep 2026 21:28:25 -0700 Subject: [PATCH 4/8] feat(gateway): log agent-execution denials and add temporary opt-out Operator-role enforcement on agent execution surfaces is breaking for deployments whose accounts were left on the default viewer role, and admins had no way to see who was affected. - CanExecuteAgent logs each denial under OpenClaw.Gateway.Authorization with the surface (or MCP tool), auth mode, account, and role. The credential is never logged. - Security.AllowViewerAgentExecution (default false) restores the old behavior for authenticated identities below operator for one release. Every admission it grants is logged, and admin posture reports the viewer_agent_execution_allowed risk flag. Unauthenticated callers are still rejected. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 + docs/AUTHENTICATION.md | 3 + docs/zh-CN/AUTHENTICATION.md | 3 + src/OpenClaw.Core/Models/GatewayConfig.cs | 7 ++ .../Endpoints/EndpointHelpers.cs | 31 ++++- src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs | 10 +- .../SecurityPostureBuilder.cs | 6 + .../GatewayAdminEndpointTests.cs | 107 ++++++++++++++++++ 8 files changed, 162 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index db1d7fbb..552e101a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -42,6 +42,8 @@ All notable changes to this project are tracked in this file. - A2A execution paths: 403. Discovery stays public. - `POST /apps/chat`: 403. - MCP `openclaw.send_message`, `openclaw.run_workflow`, and `openclaw.respond_workflow`: tool error result. Read-only MCP tools stay available to viewers. + - Each denial is logged under `OpenClaw.Gateway.Authorization` with the surface, account, and role, so admins can find accounts to promote. + - Migration aid: `OpenClaw:Security:AllowViewerAgentExecution=true` restores the previous behavior for authenticated identities below `operator`, logs each such request, and adds the `viewer_agent_execution_allowed` risk flag to `admin posture`. It is temporary and will be removed in the next release. - Stopped trusting a loopback client IP on `/apps/health`, `/apps/chat`, and `/apps/mcp/{appId}`. Behind a same-host reverse proxy without `TrustForwardedHeaders`, every caller has a loopback IP, so these routes answered unauthenticated requests, including agent runs and MCP App tool calls, and ignored `AlwaysRequireAuth`. They now follow the gateway's bind-based rule: open only on a loopback-bound gateway without `AlwaysRequireAuth`. - Added `OpenClawWebSocketClient.OnClosed`, raised with the gateway's close status and reason. Companion now marks itself disconnected and shows the reason instead of appearing connected after the gateway closes the socket. - Bound tool-approval decisions to the original requester (`channelId` + `senderId`) for non-loopback/public binds. diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index 5e0ca954..baee26ac 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -16,6 +16,7 @@ Authentication configuration lives under the `OpenClaw.Security` node in `appset |-------|------|---------|-------------| | `AuthToken` | `string?` | `null` | Static bootstrap token. When `null`, bootstrap auth is disabled | | `AlwaysRequireAuth` | `bool` | `false` | When `true`, even loopback-bound requests must carry valid credentials | +| `AllowViewerAgentExecution` | `bool` | `false` | **Temporary, to be removed in the next release.** When `true`, identities below `operator` can still run the agent (see [3.3](#33-role-required-for-agent-execution)). Each such request is logged and `admin posture` reports the risk | | `AuthMode` | `string` | `"token"` | Authentication mode: `"token"` or `"oidc"` | | `AllowQueryStringToken` | `bool` | `false` | Whether to accept tokens from the `?token=` query string parameter | | `BrowserSessionIdleMinutes` | `int` | `60` | Idle timeout for browser admin sessions (minutes) | @@ -191,6 +192,8 @@ WebSocket connected Bootstrap tokens and open loopback resolve to `admin` and are unaffected. New operator accounts default to `viewer`, so accounts used for Companion, CLI/TUI chat, or API clients need the `operator` role. +Each denial is logged as a warning under the `OpenClaw.Gateway.Authorization` category, naming the surface, auth mode, account, and role (never the credential), so admins can find the accounts to promote. To migrate without an outage, set `OpenClaw:Security:AllowViewerAgentExecution=true`, watch the log for the admitted accounts, grant them `operator`, then turn the setting off. The setting is temporary and will be removed in the next release. + ### 3.4 `IsAuthorizedRequest` — Detailed Logic ```csharp diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index b67a0e37..6a02c682 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -16,6 +16,7 @@ OpenClaw.NET Gateway 支持多层认证体系，涵盖静态令牌、OIDC/JWT Be |------|------|--------|------| | `AuthToken` | `string?` | `null` | 静态 Bootstrap 令牌。`null` 时禁用 Bootstrap 认证 | | `AlwaysRequireAuth` | `bool` | `false` | `true` 时，即使是 loopback 绑定也需要认证 | +| `AllowViewerAgentExecution` | `bool` | `false` | **临时设置，将在下一个版本移除。** `true` 时，低于 `operator` 的身份仍可执行智能体（见 3.3 节）。每个此类请求都会记录日志，`admin posture` 也会报告该风险 | | `AuthMode` | `string` | `"token"` | 认证模式：`"token"` 或 `"oidc"` | | `AllowQueryStringToken` | `bool` | `false` | 是否允许从查询字符串 `?token=` 读取令牌 | | `BrowserSessionIdleMinutes` | `int` | `60` | 浏览器会话空闲超时（分钟） | @@ -191,6 +192,8 @@ WebSocket 已连接 引导令牌和开放回环会解析为 `admin`，不受影响。新建的操作员账户默认为 `viewer`，因此用于 Companion、CLI/TUI 聊天或 API 客户端的账户需要 `operator` 角色。 +每次拒绝都会在 `OpenClaw.Gateway.Authorization` 类别下记录一条警告日志，包含入口、认证方式、账户和角色（绝不包含凭据），便于管理员找出需要提升角色的账户。如需无中断迁移，可设置 `OpenClaw:Security:AllowViewerAgentExecution=true`，从日志中找出被放行的账户，为其授予 `operator` 角色，然后关闭该设置。该设置是临时的，将在下一个版本移除。 + ### 3.4 `IsAuthorizedRequest` 详细逻辑 ```csharp diff --git a/src/OpenClaw.Core/Models/GatewayConfig.cs b/src/OpenClaw.Core/Models/GatewayConfig.cs index c186df3a..e892d07e 100644 --- a/src/OpenClaw.Core/Models/GatewayConfig.cs +++ b/src/OpenClaw.Core/Models/GatewayConfig.cs @@ -384,6 +384,13 @@ public sealed class SecurityConfig public string[] KnownProxies { get; set; } = []; public bool RequireRequesterMatchForHttpToolApproval { get; set; } = false; + /// + /// Temporary compatibility switch, to be removed in the next release. When true, authenticated identities + /// below the operator role (such as viewer accounts) can still run the agent through /ws, /v1/*, A2A, + /// /apps/chat, and mutating MCP tools. Each such request is logged so the accounts can be promoted. + /// + public bool AllowViewerAgentExecution { get; set; } = false; + /// /// When binding to a non-loopback address, the gateway refuses to start if the local tooling /// is configured in an unsafe way (e.g. shell enabled or wildcard roots). Set this to true diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index a822b19a..225c1684 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -333,12 +333,39 @@ public static (OperatorAuthorizationResult? Authorization, IResult? Failure) Aut /// Surfaces that turn a request into agent input or another mutation (chat, the OpenAI-compatible API, /// A2A, MCP Apps chat, mutating MCP tools) require the same role as POST /api/integration/messages. /// Authentication alone is not enough: viewer credentials must stay read-only. + /// Denials, and admissions under Security.AllowViewerAgentExecution, are logged with the account so + /// admins can find identities that need the operator role. /// - public static bool CanExecuteAgent(HttpContext ctx, GatewayStartupContext startup) + public static bool CanExecuteAgent(HttpContext ctx, GatewayStartupContext startup, string? action = null) { var browserSessions = ctx.RequestServices.GetRequiredService(); var auth = AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); - return auth.IsAuthorized && IsRoleAllowed(auth.Role, "integration.mutate.agent", out _); + if (auth.IsAuthorized && IsRoleAllowed(auth.Role, "integration.mutate.agent", out _)) + return true; + + var logger = ctx.RequestServices.GetRequiredService().CreateLogger("OpenClaw.Gateway.Authorization"); + action ??= ctx.Request.Path.Value; + + if (!auth.IsAuthorized) + { + logger.LogWarning( + "Denied {Action}: the credential is not accepted by the operator authorization chain (for example a bootstrap token disabled by organization policy).", + action); + return false; + } + + if (startup.Config.Security.AllowViewerAgentExecution) + { + logger.LogWarning( + "Allowed {Action} for {AuthMode} account {AccountId} ({Username}) with role {Role} only because Security.AllowViewerAgentExecution is on. Grant the operator role before that setting is removed.", + action, auth.AuthMode, auth.AccountId, auth.Username, auth.Role); + return true; + } + + logger.LogWarning( + "Denied {Action} for {AuthMode} account {AccountId} ({Username}) with role {Role}: running the agent requires the operator role.", + action, auth.AuthMode, auth.AccountId, auth.Username, auth.Role); + return false; } public static async Task WriteOperatorRoleRequiredAsync(HttpContext ctx) diff --git a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs index 196d1357..0603cf1f 100644 --- a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs +++ b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs @@ -218,7 +218,7 @@ public async Task RunWorkflow( [Description("Optional session ID.")] string? sessionId = null, CancellationToken ct = default) { - RequireOperator(); + RequireOperator("openclaw.run_workflow"); return JsonSerializer.Serialize( await _facade.RunWorkflowAsync( workflowId, @@ -256,7 +256,7 @@ public async Task RespondWorkflow( [Description("Optional JSON object or value passed as response payload.")] string? payloadJson = null, CancellationToken ct = default) { - RequireOperator(); + RequireOperator("openclaw.respond_workflow"); return JsonSerializer.Serialize( await _facade.RespondWorkflowRunAsync( workflowId, @@ -305,7 +305,7 @@ public async Task SendMessage( [Description("Optional reply-to message ID.")] string? replyToMessageId = null, CancellationToken ct = default) { - RequireOperator(); + RequireOperator("openclaw.send_message"); return JsonSerializer.Serialize( await _facade.QueueMessageAsync(new IntegrationMessageRequest { @@ -321,10 +321,10 @@ await _facade.QueueMessageAsync(new IntegrationMessageRequest // Read-only tools stay open to viewers; mutating tools require the role their REST equivalents require. // Without a request context (e.g. a detached task) the caller is unknown, so deny. - private void RequireOperator() + private void RequireOperator(string toolName) { var ctx = _httpContextAccessor.HttpContext; - if (ctx is null || !EndpointHelpers.CanExecuteAgent(ctx, _startup)) + if (ctx is null || !EndpointHelpers.CanExecuteAgent(ctx, _startup, $"MCP tool {toolName}")) throw new McpException(EndpointHelpers.OperatorRoleRequiredMessage); } diff --git a/src/OpenClaw.Gateway/SecurityPostureBuilder.cs b/src/OpenClaw.Gateway/SecurityPostureBuilder.cs index 7d0e0bff..3973f05e 100644 --- a/src/OpenClaw.Gateway/SecurityPostureBuilder.cs +++ b/src/OpenClaw.Gateway/SecurityPostureBuilder.cs @@ -99,6 +99,12 @@ public static SecurityPostureResponse Build(GatewayStartupContext startup, Gatew recommendations.Add("Enable Discord interaction signature validation before exposing a public bind."); } + if (config.Security.AllowViewerAgentExecution) + { + riskFlags.Add("viewer_agent_execution_allowed"); + recommendations.Add("Grant the operator role to accounts that chat or run the agent, then turn off OpenClaw:Security:AllowViewerAgentExecution. The setting is temporary and will be removed."); + } + if (browserAvailability.ConfiguredEnabled && !browserAvailability.Registered) { riskFlags.Add("browser_tool_unavailable"); diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index 8da0f6fb..fd70dcb0 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -13,6 +13,7 @@ using Microsoft.AspNetCore.Routing; using Microsoft.AspNetCore.TestHost; using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Logging.Abstractions; using NSubstitute; using OpenClaw.Client; @@ -563,6 +564,112 @@ public async Task McpReadOnlyTool_WhenViewerAccountToken_ShouldSucceed() Assert.Contains("activeSessions", result.RootElement.GetProperty("content")[0].GetProperty("text").GetString()); } + [Fact] + public async Task AgentExecution_WhenViewerDenied_ShouldLogAccountAndRoleWithoutToken() + { + var logs = new CapturingLoggerProvider(); + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configureServices: (services, _) => services.AddSingleton(logs)); + var token = CreateAccountToken(harness, "denied-viewer", OperatorRoleNames.Viewer); + + var response = await SendChatCompletionAsync(harness, token); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + var entry = Assert.Single(logs.Warnings, message => message.Contains("/v1/chat/completions", StringComparison.Ordinal)); + Assert.Contains("denied-viewer", entry, StringComparison.Ordinal); + Assert.Contains("viewer", entry, StringComparison.Ordinal); + Assert.DoesNotContain(token, entry, StringComparison.Ordinal); + } + + [Fact] + public async Task AgentExecution_WhenViewerAndAllowViewerAgentExecution_ShouldRunAgentAndLog() + { + var logs = new CapturingLoggerProvider(); + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configure: config => config.Security.AllowViewerAgentExecution = true, + configureServices: (services, _) => services.AddSingleton(logs)); + var token = CreateAccountToken(harness, "legacy-viewer", OperatorRoleNames.Viewer); + harness.Runtime.AgentRuntime.RunAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns("viewer reply"); + + var response = await SendChatCompletionAsync(harness, token); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + var entry = Assert.Single(logs.Warnings, message => message.Contains("AllowViewerAgentExecution", StringComparison.Ordinal)); + Assert.Contains("legacy-viewer", entry, StringComparison.Ordinal); + } + + [Fact] + public async Task AgentExecution_WhenAllowViewerAgentExecutionWithoutCredentials_ShouldStillReject() + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configure: config => config.Security.AllowViewerAgentExecution = true); + + var response = await SendChatCompletionAsync(harness, bearerToken: "not-a-real-token"); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + + [Fact] + public async Task AdminPosture_WhenAllowViewerAgentExecution_ShouldReportRisk() + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configure: config => config.Security.AllowViewerAgentExecution = true); + + using var request = new HttpRequestMessage(HttpMethod.Get, "/admin/posture"); + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", harness.AuthToken); + var response = await harness.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + using var payload = await ReadJsonAsync(response); + Assert.Contains( + payload.RootElement.GetProperty("riskFlags").EnumerateArray().Select(static item => item.GetString()).OfType(), + flag => flag == "viewer_agent_execution_allowed"); + } + + private static async Task SendChatCompletionAsync(GatewayTestHarness harness, string bearerToken) + { + using var request = new HttpRequestMessage(HttpMethod.Post, "/v1/chat/completions") + { + Content = JsonContent("""{"messages":[{"role":"user","content":"hello"}]}""") + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", bearerToken); + return await harness.Client.SendAsync(request); + } + + private sealed class CapturingLoggerProvider : ILoggerProvider + { + private readonly ConcurrentQueue<(LogLevel Level, string Message)> _entries = new(); + + public IEnumerable Warnings + => _entries.Where(static entry => entry.Level == LogLevel.Warning).Select(static entry => entry.Message); + + public ILogger CreateLogger(string categoryName) => new CapturingLogger(_entries); + + public void Dispose() + { + } + + private sealed class CapturingLogger(ConcurrentQueue<(LogLevel Level, string Message)> entries) : ILogger + { + public IDisposable? BeginScope(TState state) where TState : notnull => null; + + public bool IsEnabled(LogLevel logLevel) => true; + + public void Log(LogLevel logLevel, EventId eventId, TState state, Exception? exception, Func formatter) + => entries.Enqueue((logLevel, formatter(state, exception))); + } + } + private static async Task CallMcpToolAsync(GatewayTestHarness harness, string bearerToken, string toolName, string argumentsJson) { using var request = new HttpRequestMessage(HttpMethod.Post, "/mcp") From 42af38b1f5a84b285ab84e0a2480ae72ae6ab2b7 Mon Sep 17 00:00:00 2001 From: telli Date: Sun, 27 Sep 2026 22:04:33 -0700 Subject: [PATCH 5/8] feat(admin): explain role capabilities when creating operator accounts New accounts default to viewer, and viewers can no longer chat or run the agent. The account form now shows what the selected role can do, starting with the viewer hint that clients which chat need operator. Co-Authored-By: Claude Opus 5.5 (1M context) --- src/OpenClaw.Gateway/wwwroot/admin.html | 12 ++++++++++++ src/OpenClaw.Tests/GatewayAdminEndpointTests.cs | 11 +++++++++++ 2 files changed, 23 insertions(+) diff --git a/src/OpenClaw.Gateway/wwwroot/admin.html b/src/OpenClaw.Gateway/wwwroot/admin.html index 46b9456d..62c9809f 100644 --- a/src/OpenClaw.Gateway/wwwroot/admin.html +++ b/src/OpenClaw.Gateway/wwwroot/admin.html @@ -993,6 +993,7 @@

Operator Accounts

+
Read-only: can't chat or run the agent from web chat, Companion, the CLI, or API clients. Choose operator for those.
@@ -3228,6 +3229,16 @@

Notes

`; } + const operatorAccountRoleHints = { + viewer: "Read-only: can't chat or run the agent from web chat, Companion, the CLI, or API clients. Choose operator for those.", + operator: 'Can chat, run the agent, and decide approvals.', + admin: 'Operator access plus settings, plugins, accounts, and organization policy.' + }; + + function updateOperatorAccountRoleHint() { + document.getElementById('operator-account-role-hint').textContent = operatorAccountRoleHints[operatorAccountRoleInput.value] || ''; + } + async function createOperatorAccount() { if (!requireRole('admin', 'Creating operator accounts')) return; @@ -5835,6 +5846,7 @@

Persisted (${data.persisted.returnedCount})

document.getElementById('setup-wizard-run-button').addEventListener('click', runFirstOperatorWizard); document.getElementById('operator-accounts-refresh-button').addEventListener('click', loadOperatorAccounts); document.getElementById('operator-account-create-button').addEventListener('click', createOperatorAccount); + operatorAccountRoleInput.addEventListener('change', updateOperatorAccountRoleHint); document.getElementById('organization-policy-refresh-button').addEventListener('click', loadOrganizationPolicy); document.getElementById('organization-policy-save-button').addEventListener('click', saveOrganizationPolicy); document.getElementById('observability-refresh-button').addEventListener('click', loadObservability); diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index fd70dcb0..395216eb 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -7397,6 +7397,17 @@ public async Task AdminUi_ContainsDedicatedWhatsAppSetupControls() Assert.Contains("/admin/channels/whatsapp/auth/stream", html, StringComparison.Ordinal); } + [Fact] + public async Task AdminUi_OperatorAccountRole_ShouldExplainViewerCannotChat() + { + var adminHtmlPath = Path.GetFullPath(Path.Join(AppContext.BaseDirectory, "../../../../../src/OpenClaw.Gateway/wwwroot/admin.html")); + var html = await File.ReadAllTextAsync(adminHtmlPath); + + Assert.Contains("id=\"operator-account-role-hint\"", html, StringComparison.Ordinal); + Assert.Contains("Read-only: can't chat or run the agent", html, StringComparison.Ordinal); + Assert.Contains("operatorAccountRoleInput.addEventListener('change', updateOperatorAccountRoleHint)", html, StringComparison.Ordinal); + } + [Theory] [InlineData("ws://127.0.0.1:18789/ws", "http://127.0.0.1:18789")] [InlineData("wss://example.com/ws", "https://example.com")] From b8d1f5af673e3fc97943d66795b4d462d0e32c6d Mon Sep 17 00:00:00 2001 From: telli Date: Sun, 27 Sep 2026 22:11:48 -0700 Subject: [PATCH 6/8] fix(gateway): require operator role for MCP App tool calls The /apps/mcp/{appId} proxy forwarded tools/call from any authenticated role to the same upstream App session the agent uses, with a caller-chosen _meta.sessionId. App tools can change state, and the only legitimate client, an MCP App UI inside the /apps/chat host, already requires operator. tools/call now goes through CanExecuteAgent and returns a tool error below operator; tools/list and resources stay available to any authenticated role. The caller is read from the current request, so a stateful session cannot inherit the identity that opened it. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/AUTHENTICATION.md | 1 + docs/MCPAPP.md | 2 +- docs/zh-CN/AUTHENTICATION.md | 1 + docs/zh-CN/MCPAPP.md | 2 +- src/OpenClaw.Core/Models/GatewayConfig.cs | 2 +- .../Endpoints/AppsMcpProxyEndpoint.cs | 12 +++ .../Endpoints/EndpointHelpers.cs | 2 +- .../AppsMcpProxyEndpointTests.cs | 80 ++++++++++++++++++- 9 files changed, 97 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 552e101a..5411dcda 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -41,6 +41,7 @@ All notable changes to this project are tracked in this file. - `POST /v1/chat/completions` and `POST /v1/responses`: 403 with an OpenAI-style `permission_error` body. - A2A execution paths: 403. Discovery stays public. - `POST /apps/chat`: 403. + - `tools/call` through the `/apps/mcp/{appId}` MCP App proxy: tool error result. Listing and reading App tools and resources stay available to any authenticated role. - MCP `openclaw.send_message`, `openclaw.run_workflow`, and `openclaw.respond_workflow`: tool error result. Read-only MCP tools stay available to viewers. - Each denial is logged under `OpenClaw.Gateway.Authorization` with the surface, account, and role, so admins can find accounts to promote. - Migration aid: `OpenClaw:Security:AllowViewerAgentExecution=true` restores the previous behavior for authenticated identities below `operator`, logs each such request, and adds the `viewer_agent_execution_allowed` risk flag to `admin posture`. It is temporary and will be removed in the next release. diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index baee26ac..1ce6e2db 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -188,6 +188,7 @@ WebSocket connected | `POST /v1/chat/completions`, `POST /v1/responses` | 403 with an OpenAI-style `permission_error` body | | A2A execution paths (discovery stays public) | 403 | | `POST /apps/chat` | 403 | +| `/apps/mcp/{appId}` `tools/call` | Tool error result; listing and reading App tools and resources stay available | | MCP `openclaw.send_message`, `openclaw.run_workflow`, `openclaw.respond_workflow` | Tool error result; read-only MCP tools stay available to viewers | Bootstrap tokens and open loopback resolve to `admin` and are unaffected. New operator accounts default to `viewer`, so accounts used for Companion, CLI/TUI chat, or API clients need the `operator` role. diff --git a/docs/MCPAPP.md b/docs/MCPAPP.md index 984183f5..4ff6f00f 100644 --- a/docs/MCPAPP.md +++ b/docs/MCPAPP.md @@ -38,7 +38,7 @@ OpenClaw.NET exposes a small gateway-facing host surface for browser-side MCP Ap |-------|---------| | `/apps/health` | Returns the selected MCP App id plus the gateway MCP endpoint the browser should connect to | | `/apps/chat` | Streams chat-host SSE events (`session`, `text`, `tool`, `result`, `done`) into the existing `GatewayAppRuntime`. Requires the `operator` role | -| `/apps/mcp/{appId}` | Proxies MCP requests to the already connected `McpClient` for that App | +| `/apps/mcp/{appId}` | Proxies MCP requests to the already connected `McpClient` for that App. `tools/call` requires the `operator` role; listing and reading stay available to any authenticated role | The important detail is that browser UIs should connect to `/apps/mcp/{appId}`, not directly to the App's raw upstream MCP URL. That keeps browser-driven MCP calls and model-driven MCP calls on the same OpenClaw-managed session. diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index 6a02c682..13d03dde 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -188,6 +188,7 @@ WebSocket 已连接 | `POST /v1/chat/completions`、`POST /v1/responses` | 403，返回 OpenAI 风格的 `permission_error` 响应体 | | A2A 执行路径（发现端点仍然公开） | 403 | | `POST /apps/chat` | 403 | +| `/apps/mcp/{appId}` 的 `tools/call` | 返回工具错误结果；列出和读取 App 工具与资源仍可用 | | MCP `openclaw.send_message`、`openclaw.run_workflow`、`openclaw.respond_workflow` | 返回工具错误结果；只读 MCP 工具对 viewer 仍可用 | 引导令牌和开放回环会解析为 `admin`，不受影响。新建的操作员账户默认为 `viewer`，因此用于 Companion、CLI/TUI 聊天或 API 客户端的账户需要 `operator` 角色。 diff --git a/docs/zh-CN/MCPAPP.md b/docs/zh-CN/MCPAPP.md index 8b95e216..016b9dbb 100644 --- a/docs/zh-CN/MCPAPP.md +++ b/docs/zh-CN/MCPAPP.md @@ -38,7 +38,7 @@ OpenClaw.NET 为浏览器侧 MCP App UI 暴露了一组面向 gateway 的 host |------|------| | `/apps/health` | 返回当前选中的 MCP App id，以及浏览器应连接的 gateway MCP 端点 | | `/apps/chat` | 把浏览器 host 的聊天请求桥接到现有 `GatewayAppRuntime`，并输出 `session`、`text`、`tool`、`result`、`done` 形状的 SSE。需要 `operator` 角色 | -| `/apps/mcp/{appId}` | 把 MCP 请求代理到该 App 已经连接好的 `McpClient` | +| `/apps/mcp/{appId}` | 把 MCP 请求代理到该 App 已经连接好的 `McpClient`。`tools/call` 需要 `operator` 角色；列表和读取对任何已认证角色仍可用 | 关键点是：浏览器 UI 应连接 `/apps/mcp/{appId}`，而不是直接连接 MCP App 的原始上游 URL。这样浏览器触发的 MCP 调用与 Agent 触发的 MCP 调用才能落在同一条 OpenClaw 管理的会话上。 diff --git a/src/OpenClaw.Core/Models/GatewayConfig.cs b/src/OpenClaw.Core/Models/GatewayConfig.cs index e892d07e..17c5b175 100644 --- a/src/OpenClaw.Core/Models/GatewayConfig.cs +++ b/src/OpenClaw.Core/Models/GatewayConfig.cs @@ -387,7 +387,7 @@ public sealed class SecurityConfig /// /// Temporary compatibility switch, to be removed in the next release. When true, authenticated identities /// below the operator role (such as viewer accounts) can still run the agent through /ws, /v1/*, A2A, - /// /apps/chat, and mutating MCP tools. Each such request is logged so the accounts can be promoted. + /// /apps/chat, MCP App tool calls, and mutating MCP tools. Each such request is logged so the accounts can be promoted. /// public bool AllowViewerAgentExecution { get; set; } = false; diff --git a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs index 9ac86671..775c1139 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs @@ -1,4 +1,5 @@ using Microsoft.Extensions.DependencyInjection; +using ModelContextProtocol; using ModelContextProtocol.Protocol; using ModelContextProtocol.Server; using OpenClaw.Gateway.Bootstrap; @@ -53,6 +54,9 @@ public static async Task ConfigureSessionOptionsAsync( return; } + var startup = httpContext.RequestServices.GetRequiredService(); + var httpContextAccessor = httpContext.RequestServices.GetService(); + sessionOptions.Handlers.ListToolsHandler = async (ctx, ct2) => await upstream.ListToolsAsync(ctx.Params ?? new ListToolsRequestParams(), ct2); @@ -65,6 +69,14 @@ public static async Task ConfigureSessionOptionsAsync( sessionOptions.Handlers.CallToolHandler = async (ctx, ct2) => { var callParams = ctx.Params!; + + // App tools can change state and share the agent's upstream session, so calling them needs the same + // operator role as /apps/chat, the host these UIs run in. Listing and reading stay open to viewers. + // Check the current request rather than the one that opened a stateful session. + var caller = httpContextAccessor?.HttpContext ?? httpContext; + if (!EndpointHelpers.CanExecuteAgent(caller, startup, $"MCP App tool {serverId}/{callParams.Name}")) + throw new McpException(EndpointHelpers.OperatorRoleRequiredMessage); + if (!string.IsNullOrEmpty(sessionId)) { callParams.Meta ??= new JsonObject(); diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 225c1684..a80659e2 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -331,7 +331,7 @@ public static (OperatorAuthorizationResult? Authorization, IResult? Failure) Aut /// /// Surfaces that turn a request into agent input or another mutation (chat, the OpenAI-compatible API, - /// A2A, MCP Apps chat, mutating MCP tools) require the same role as POST /api/integration/messages. + /// A2A, MCP Apps chat and tool calls, mutating MCP tools) require the same role as POST /api/integration/messages. /// Authentication alone is not enough: viewer credentials must stay read-only. /// Denials, and admissions under Security.AllowViewerAgentExecution, are logged with the account so /// admins can find identities that need the operator role. diff --git a/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs b/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs index 18b78973..ceaa0d7d 100644 --- a/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs +++ b/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs @@ -9,6 +9,7 @@ using ModelContextProtocol.Server; using OpenClaw.Core.Models; using OpenClaw.Core.Plugins; +using OpenClaw.Gateway; using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Endpoints; using OpenClaw.McpApp; @@ -21,6 +22,7 @@ public sealed class AppsMcpProxyEndpointTests : IAsyncDisposable { private readonly List _apps = []; private readonly List _tempDirs = []; + private int _upstreamToolCalls; [Fact] public void GatewayConfig_McpCompatibility_Defaults_AreStrictAndDiscoveryFirst() @@ -64,6 +66,46 @@ public async Task CallTool_InjectsSessionIdFromQueryIntoMeta() Assert.Equal("abc123", doc.GetProperty("sessionId").GetString()); } + [Fact] + public async Task CallTool_WhenViewerAccountToken_ShouldReturnOperatorErrorWithoutCallingUpstream() + { + var upstreamUrl = await StartFakeUpstreamAsync(); + await using var gateway = await StartGatewayWithProxyAsync("inventory-app", upstreamUrl, nonLoopbackBind: true); + await using var mcpClient = await CreateProxyClientAsync(gateway, CreateAccountToken(gateway, OperatorRoleNames.Viewer)); + + var result = await mcpClient.CallToolAsync("echo_session", cancellationToken: CancellationToken.None); + + Assert.True(result.IsError); + var text = Assert.IsType(Assert.Single(result.Content)).Text; + Assert.Contains("operator", text, StringComparison.OrdinalIgnoreCase); + Assert.Equal(0, _upstreamToolCalls); + } + + [Fact] + public async Task ToolsList_WhenViewerAccountToken_ShouldStillPassThrough() + { + var upstreamUrl = await StartFakeUpstreamAsync(); + await using var gateway = await StartGatewayWithProxyAsync("inventory-app", upstreamUrl, nonLoopbackBind: true); + await using var mcpClient = await CreateProxyClientAsync(gateway, CreateAccountToken(gateway, OperatorRoleNames.Viewer)); + + var tools = await mcpClient.ListToolsAsync(cancellationToken: CancellationToken.None); + + Assert.Contains(tools, t => t.Name == "echo_session"); + } + + [Fact] + public async Task CallTool_WhenOperatorAccountToken_ShouldReachUpstream() + { + var upstreamUrl = await StartFakeUpstreamAsync(); + await using var gateway = await StartGatewayWithProxyAsync("inventory-app", upstreamUrl, nonLoopbackBind: true); + await using var mcpClient = await CreateProxyClientAsync(gateway, CreateAccountToken(gateway, OperatorRoleNames.Operator)); + + var result = await mcpClient.CallToolAsync("echo_session", cancellationToken: CancellationToken.None); + + Assert.NotEqual(true, result.IsError); + Assert.Equal(1, _upstreamToolCalls); + } + [Fact] public async Task CallTool_UnknownAppId_ReturnsActionableErrorPayload() { @@ -158,6 +200,7 @@ private async Task StartFakeUpstreamAsync() })) .WithCallToolHandler((ctx, _) => { + Interlocked.Increment(ref _upstreamToolCalls); var sessionId = ctx.Params?.Meta?["sessionId"]?.ToString(); return ValueTask.FromResult(new CallToolResult { @@ -173,7 +216,38 @@ private async Task StartFakeUpstreamAsync() return $"{app.Urls.Single().TrimEnd('/')}/mcp"; } - private async Task StartGatewayWithProxyAsync(string appId, string upstreamUrl) + private static string CreateAccountToken(GatewayProxyTestHarness gateway, string role) + { + var accounts = gateway.App.Services.GetRequiredService(); + var account = accounts.Create(new OperatorAccountCreateRequest + { + Username = "proxy-" + role, + Password = "P@ssw0rd123!", + Role = role + }); + return accounts.CreateToken(account.Id, new OperatorAccountTokenCreateRequest { Label = role })!.Token; + } + + private static Task CreateProxyClientAsync(GatewayProxyTestHarness gateway, string bearerToken) + => McpClient.CreateAsync( + new HttpClientTransport(new HttpClientTransportOptions + { + Endpoint = new Uri($"{gateway.BaseAddress}apps/mcp/inventory-app"), + AdditionalHeaders = new Dictionary { ["Authorization"] = $"Bearer {bearerToken}" } + }), + cancellationToken: CancellationToken.None); + + // Mirrors the production registrations the proxy's authorization depends on. + private static void AddGatewayAuthServices(IServiceCollection services, GatewayConfig config, GatewayStartupContext startup, string storagePath) + { + services.AddSingleton(startup); + services.AddHttpContextAccessor(); + services.AddSingleton(new BrowserSessionAuthService(config)); + services.AddSingleton(new OperatorAccountService(storagePath, NullLogger.Instance)); + services.AddSingleton(new OrganizationPolicyService(storagePath, NullLogger.Instance)); + } + + private async Task StartGatewayWithProxyAsync(string appId, string upstreamUrl, bool nonLoopbackBind = false) { var root = Path.Combine(Path.GetTempPath(), "openclaw-apps-proxy-tests", Guid.NewGuid().ToString("N")); Directory.CreateDirectory(root); @@ -207,13 +281,14 @@ await File.WriteAllTextAsync( { Config = config, RuntimeState = RuntimeModeResolver.Resolve(config.Runtime), - IsNonLoopbackBind = false, + IsNonLoopbackBind = nonLoopbackBind, WorkspacePath = null, }; var builder = WebApplication.CreateSlimBuilder(); builder.WebHost.UseUrls("http://127.0.0.1:0"); builder.Services.AddOpenClawMcpAppServices(config.McpApps); + AddGatewayAuthServices(builder.Services, config, startup, root); builder.Services.AddMcpServer(options => { options.ServerInfo = new Implementation { Name = "OpenClaw Gateway MCP", Version = "1.0.0" }; @@ -275,6 +350,7 @@ await File.WriteAllTextAsync( var builder = WebApplication.CreateSlimBuilder(); builder.WebHost.UseUrls("http://127.0.0.1:0"); builder.Services.AddOpenClawMcpAppServices(config.McpApps); + AddGatewayAuthServices(builder.Services, config, startup, root); builder.Services.AddMcpServer(options => { options.ServerInfo = new Implementation { Name = "OpenClaw Gateway MCP", Version = "1.0.0" }; From 63edc3bd7ad864b92cc15dce01eed34e169e0910 Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 09:13:29 -0700 Subject: [PATCH 7/8] fix(gateway): close authorization edge cases --- .../ViewModels/MainWindowViewModel.cs | 3 +++ .../Endpoints/AppsMcpProxyEndpoint.cs | 9 ++++---- .../Endpoints/EndpointHelpers.cs | 10 ++++---- .../Mcp/McpServiceExtensions.cs | 14 ++++++++++- src/OpenClaw.Gateway/wwwroot/admin.html | 4 ++-- .../AppsMcpProxyEndpointTests.cs | 20 +++++++++++++++- .../CompanionConnectionTests.cs | 23 +++++++++++++++---- .../GatewayAdminEndpointTests.cs | 23 ++++++++++++++++++- 8 files changed, 89 insertions(+), 17 deletions(-) diff --git a/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs index 42e405fe..08e9d62b 100644 --- a/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs +++ b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs @@ -454,6 +454,9 @@ private void HandleServerClosed(System.Net.WebSockets.WebSocketCloseStatus? stat : $"The gateway closed the connection: {reason}"; Dispatcher.UIThread.Post(() => { + if (_client.IsConnected) + return; + IsConnected = false; Status = "Disconnected"; AddSystemMessageCore(message); diff --git a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs index 775c1139..75f7a72a 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs @@ -72,9 +72,10 @@ public static async Task ConfigureSessionOptionsAsync( // App tools can change state and share the agent's upstream session, so calling them needs the same // operator role as /apps/chat, the host these UIs run in. Listing and reading stay open to viewers. - // Check the current request rather than the one that opened a stateful session. - var caller = httpContextAccessor?.HttpContext ?? httpContext; - if (!EndpointHelpers.CanExecuteAgent(caller, startup, $"MCP App tool {serverId}/{callParams.Name}")) + // Check the current request rather than the one that opened a stateful session. Dynamic App tools + // run synchronously so the request context remains available; if it is ever absent, fail closed. + var caller = httpContextAccessor?.HttpContext; + if (caller is null || !EndpointHelpers.CanExecuteAgent(caller, startup, $"MCP App tool {serverId}/{callParams.Name}")) throw new McpException(EndpointHelpers.OperatorRoleRequiredMessage); if (!string.IsNullOrEmpty(sessionId)) @@ -104,4 +105,4 @@ public static void MapOpenClawAppsMcpProxy(this WebApplication app, GatewayStart return await next(ctx); }); } -} \ No newline at end of file +} diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index a80659e2..fbcf3e44 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -42,13 +42,18 @@ public static bool IsAuthorizedRequest(HttpContext ctx, GatewayConfig config, bo if (!isNonLoopbackBind && !config.Security.AlwaysRequireAuth && !config.Security.IsOidcMode) return true; + var organizationPolicy = ctx.RequestServices.GetService(); + var policy = organizationPolicy?.GetSnapshot() ?? new OrganizationPolicySnapshot(); + // OIDC mode OR JWT token: UseAuthentication() middleware validated the JWT // and populated ctx.User. Accept the request if the user is authenticated. if (ctx.User.Identity?.IsAuthenticated == true) return true; // Static AuthToken check (bootstrap token). - if (!string.IsNullOrWhiteSpace(config.AuthToken)) + if (policy.BootstrapTokenEnabled && + IsAllowedAuthMode(policy, OrganizationAuthModeNames.BootstrapToken) && + !string.IsNullOrWhiteSpace(config.AuthToken)) { var token = GatewaySecurity.GetToken(ctx, config.Security.AllowQueryStringToken); if (GatewaySecurity.IsTokenValid(token, config.AuthToken)) @@ -57,9 +62,6 @@ public static bool IsAuthorizedRequest(HttpContext ctx, GatewayConfig config, bo // Fall through to operator account tokens and browser sessions // so that AlwaysRequireAuth works with non-bootstrap auth methods. - var organizationPolicy = ctx.RequestServices.GetService(); - var policy = organizationPolicy?.GetSnapshot() ?? new OrganizationPolicySnapshot(); - // Operator account token. if (IsAllowedAuthMode(policy, OrganizationAuthModeNames.AccountToken)) { diff --git a/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs b/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs index f8279a37..da08bbde 100644 --- a/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs +++ b/src/OpenClaw.Gateway/Mcp/McpServiceExtensions.cs @@ -47,7 +47,12 @@ public static IServiceCollection AddOpenClawMcpServices( && !startup.Config.McpCompatibility.ForceLegacyInitialize; options.ConfigureSessionOptions = AppsMcpProxyEndpoint.ConfigureSessionOptionsAsync; }) - .WithTasks(new InMemoryMcpTaskStore()) + .WithTasks(new InMemoryMcpTaskStore(), options => + { + options.ExecutionModeSelector = request => GetTaskExecutionMode( + request.Params?.Name, + hasMatchedPrimitive: request.MatchedPrimitive is not null); + }) .WithTools() .WithResources() .WithPrompts(); @@ -55,6 +60,13 @@ public static IServiceCollection AddOpenClawMcpServices( return services; } + internal static McpTaskExecutionMode GetTaskExecutionMode(string? toolName, bool hasMatchedPrimitive = true) + => !hasMatchedPrimitive || toolName is "openclaw.run_workflow" + or "openclaw.respond_workflow" + or "openclaw.send_message" + ? McpTaskExecutionMode.Synchronous + : McpTaskExecutionMode.Optional; + /// /// Populates after the runtime is created. /// Must be called before any MCP requests are served. diff --git a/src/OpenClaw.Gateway/wwwroot/admin.html b/src/OpenClaw.Gateway/wwwroot/admin.html index 62c9809f..76ffef74 100644 --- a/src/OpenClaw.Gateway/wwwroot/admin.html +++ b/src/OpenClaw.Gateway/wwwroot/admin.html @@ -993,7 +993,7 @@

Operator Accounts

-
Read-only: can't chat or run the agent from web chat, Companion, the CLI, or API clients. Choose operator for those.
+
Read-only by default: can't chat or run the agent from web chat, Companion, the CLI, or API clients. Choose operator for those; Security.AllowViewerAgentExecution is a temporary migration exception.
@@ -3230,7 +3230,7 @@

Notes

} const operatorAccountRoleHints = { - viewer: "Read-only: can't chat or run the agent from web chat, Companion, the CLI, or API clients. Choose operator for those.", + viewer: "Read-only by default: can't chat or run the agent from web chat, Companion, the CLI, or API clients. Choose operator for those; Security.AllowViewerAgentExecution is a temporary migration exception.", operator: 'Can chat, run the agent, and decide approvals.', admin: 'Operator access plus settings, plugins, accounts, and organization policy.' }; diff --git a/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs b/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs index ceaa0d7d..215e826a 100644 --- a/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs +++ b/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs @@ -12,6 +12,7 @@ using OpenClaw.Gateway; using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Endpoints; +using OpenClaw.Gateway.Mcp; using OpenClaw.McpApp; using OpenClaw.McpApp.Models; using Xunit; @@ -181,6 +182,23 @@ public async Task GatewayMcpServer_AdvertisesTasksExtension_WhenEnabled() Assert.Equal(JsonValueKind.Object, tasksCapability.ValueKind); } + [Theory] + [InlineData("openclaw.run_workflow")] + [InlineData("openclaw.respond_workflow")] + [InlineData("openclaw.send_message")] + public void GatewayMcpServer_MutatingTools_RunSynchronously(string toolName) + => Assert.Equal(McpTaskExecutionMode.Synchronous, McpServiceExtensions.GetTaskExecutionMode(toolName)); + + [Fact] + public void GatewayMcpServer_ReadOnlyTools_RemainTaskCapable() + => Assert.Equal(McpTaskExecutionMode.Optional, McpServiceExtensions.GetTaskExecutionMode("openclaw.get_status")); + + [Fact] + public void GatewayMcpServer_DynamicAppTools_RunSynchronously() + => Assert.Equal( + McpTaskExecutionMode.Synchronous, + McpServiceExtensions.GetTaskExecutionMode("echo_session", hasMatchedPrimitive: false)); + private async Task StartFakeUpstreamAsync() { var builder = WebApplication.CreateSlimBuilder(); @@ -407,4 +425,4 @@ public async ValueTask DisposeAsync() await App.DisposeAsync(); } } -} \ No newline at end of file +} diff --git a/src/OpenClaw.Tests/CompanionConnectionTests.cs b/src/OpenClaw.Tests/CompanionConnectionTests.cs index 66eed41a..e0c71b2a 100644 --- a/src/OpenClaw.Tests/CompanionConnectionTests.cs +++ b/src/OpenClaw.Tests/CompanionConnectionTests.cs @@ -15,10 +15,8 @@ public sealed class CompanionConnectionTests : IDisposable public void Dispose() { foreach (var dir in _tempDirs) - { - try { Directory.Delete(dir, recursive: true); } - catch { } - } + if (Directory.Exists(dir)) + Directory.Delete(dir, recursive: true); } [AvaloniaFact] @@ -52,6 +50,23 @@ public async Task ServerClose_WhenNormalClosure_ShouldDisconnectWithoutRoleHint( Assert.DoesNotContain("operator", message.Text, StringComparison.OrdinalIgnoreCase); } + [AvaloniaFact] + public async Task ServerClose_WhenClientReconnectsBeforeUiDispatch_ShouldKeepConnected() + { + var (vm, client) = CreateConnectedViewModel(); + var closedSocket = new TestWebSocket(); + closedSocket.QueueClose(WebSocketCloseStatus.PolicyViolation, "This action requires the operator role."); + + await client.RunReceiveLoopForTest(closedSocket, CancellationToken.None); + using var reconnectedSocket = new TestWebSocket(); + client.SetConnectedSocketForTest(reconnectedSocket); + Dispatcher.UIThread.RunJobs(); + + Assert.True(vm.IsConnected); + Assert.Equal("Connected", vm.Status); + Assert.DoesNotContain(vm.Messages, message => message.Role == ChatRole.System); + } + private (MainWindowViewModel ViewModel, GatewayWebSocketClient Client) CreateConnectedViewModel() { var dir = Path.Combine(Path.GetTempPath(), "openclaw-companion-connection-tests", Guid.NewGuid().ToString("N")); diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index 395216eb..777b1b2e 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -494,6 +494,26 @@ public async Task AppsHealth_WhenNonLoopbackBindWithBearerToken_ShouldSucceed() Assert.Equal(HttpStatusCode.OK, response.StatusCode); } + [Theory] + [InlineData("GET", "/apps/health")] + [InlineData("POST", "/apps/mcp/inventory-app")] + public async Task AppsHostRoutes_WhenBootstrapDisabled_ShouldRejectBootstrapToken(string method, string path) + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configureServices: AddMcpAppServices); + var policy = harness.App.Services.GetRequiredService(); + policy.Update(new OrganizationPolicySnapshot + { + BootstrapTokenEnabled = false, + AllowedAuthModes = [OrganizationAuthModeNames.AccountToken, OrganizationAuthModeNames.BrowserSession] + }); + + var response = await SendAppsHostRequestAsync(harness, method, path, harness.AuthToken); + + Assert.Equal(HttpStatusCode.Unauthorized, response.StatusCode); + } + private static void AddMcpAppServices(IServiceCollection services, GatewayConfig config) => services.AddOpenClawMcpAppServices(config.McpApps); @@ -7404,7 +7424,8 @@ public async Task AdminUi_OperatorAccountRole_ShouldExplainViewerCannotChat() var html = await File.ReadAllTextAsync(adminHtmlPath); Assert.Contains("id=\"operator-account-role-hint\"", html, StringComparison.Ordinal); - Assert.Contains("Read-only: can't chat or run the agent", html, StringComparison.Ordinal); + Assert.Contains("Read-only by default: can't chat or run the agent", html, StringComparison.Ordinal); + Assert.Contains("Security.AllowViewerAgentExecution is a temporary migration exception", html, StringComparison.Ordinal); Assert.Contains("operatorAccountRoleInput.addEventListener('change', updateOperatorAccountRoleHint)", html, StringComparison.Ordinal); } From a4a56979019c0ef1ded32db6bb9cba8ecdf23d61 Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 09:33:07 -0700 Subject: [PATCH 8/8] fix(gateway): require csrf for mcp app mutations --- .../Endpoints/AppsMcpProxyEndpoint.cs | 6 ++- .../Endpoints/EndpointHelpers.cs | 8 ++- .../AppsMcpProxyEndpointTests.cs | 53 +++++++++++++++++++ .../CompanionConnectionTests.cs | 5 +- 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs index 75f7a72a..7d37941d 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsMcpProxyEndpoint.cs @@ -75,7 +75,11 @@ public static async Task ConfigureSessionOptionsAsync( // Check the current request rather than the one that opened a stateful session. Dynamic App tools // run synchronously so the request context remains available; if it is ever absent, fail closed. var caller = httpContextAccessor?.HttpContext; - if (caller is null || !EndpointHelpers.CanExecuteAgent(caller, startup, $"MCP App tool {serverId}/{callParams.Name}")) + if (caller is null || !EndpointHelpers.CanExecuteAgent( + caller, + startup, + $"MCP App tool {serverId}/{callParams.Name}", + requireCsrf: true)) throw new McpException(EndpointHelpers.OperatorRoleRequiredMessage); if (!string.IsNullOrEmpty(sessionId)) diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index fbcf3e44..209ec51c 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -338,10 +338,14 @@ public static (OperatorAuthorizationResult? Authorization, IResult? Failure) Aut /// Denials, and admissions under Security.AllowViewerAgentExecution, are logged with the account so /// admins can find identities that need the operator role. /// - public static bool CanExecuteAgent(HttpContext ctx, GatewayStartupContext startup, string? action = null) + public static bool CanExecuteAgent( + HttpContext ctx, + GatewayStartupContext startup, + string? action = null, + bool requireCsrf = false) { var browserSessions = ctx.RequestServices.GetRequiredService(); - var auth = AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); + var auth = AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf); if (auth.IsAuthorized && IsRoleAllowed(auth.Role, "integration.mutate.agent", out _)) return true; diff --git a/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs b/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs index 215e826a..e15d40f0 100644 --- a/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs +++ b/src/OpenClaw.Tests/AppsMcpProxyEndpointTests.cs @@ -107,6 +107,38 @@ public async Task CallTool_WhenOperatorAccountToken_ShouldReachUpstream() Assert.Equal(1, _upstreamToolCalls); } + [Fact] + public async Task CallTool_WhenBrowserSessionOmitsCsrf_ShouldReturnOperatorErrorWithoutCallingUpstream() + { + var upstreamUrl = await StartFakeUpstreamAsync(); + await using var gateway = await StartGatewayWithProxyAsync("inventory-app", upstreamUrl, nonLoopbackBind: true); + var browserSessions = gateway.App.Services.GetRequiredService(); + var ticket = browserSessions.Create(remember: false); + await using var mcpClient = await CreateBrowserSessionProxyClientAsync(gateway, ticket, includeCsrf: false); + + var result = await mcpClient.CallToolAsync("echo_session", cancellationToken: CancellationToken.None); + + Assert.True(result.IsError); + var text = Assert.IsType(Assert.Single(result.Content)).Text; + Assert.Contains("operator", text, StringComparison.OrdinalIgnoreCase); + Assert.Equal(0, _upstreamToolCalls); + } + + [Fact] + public async Task CallTool_WhenBrowserSessionIncludesCsrf_ShouldReachUpstream() + { + var upstreamUrl = await StartFakeUpstreamAsync(); + await using var gateway = await StartGatewayWithProxyAsync("inventory-app", upstreamUrl, nonLoopbackBind: true); + var browserSessions = gateway.App.Services.GetRequiredService(); + var ticket = browserSessions.Create(remember: false); + await using var mcpClient = await CreateBrowserSessionProxyClientAsync(gateway, ticket, includeCsrf: true); + + var result = await mcpClient.CallToolAsync("echo_session", cancellationToken: CancellationToken.None); + + Assert.NotEqual(true, result.IsError); + Assert.Equal(1, _upstreamToolCalls); + } + [Fact] public async Task CallTool_UnknownAppId_ReturnsActionableErrorPayload() { @@ -255,6 +287,27 @@ private static Task CreateProxyClientAsync(GatewayProxyTestHarness ga }), cancellationToken: CancellationToken.None); + private static Task CreateBrowserSessionProxyClientAsync( + GatewayProxyTestHarness gateway, + BrowserSessionTicket ticket, + bool includeCsrf) + { + var headers = new Dictionary + { + ["Cookie"] = $"{BrowserSessionAuthService.CookieName}={ticket.SessionId}" + }; + if (includeCsrf) + headers[BrowserSessionAuthService.CsrfHeaderName] = ticket.CsrfToken; + + return McpClient.CreateAsync( + new HttpClientTransport(new HttpClientTransportOptions + { + Endpoint = new Uri($"{gateway.BaseAddress}apps/mcp/inventory-app"), + AdditionalHeaders = headers + }), + cancellationToken: CancellationToken.None); + } + // Mirrors the production registrations the proxy's authorization depends on. private static void AddGatewayAuthServices(IServiceCollection services, GatewayConfig config, GatewayStartupContext startup, string storagePath) { diff --git a/src/OpenClaw.Tests/CompanionConnectionTests.cs b/src/OpenClaw.Tests/CompanionConnectionTests.cs index e0c71b2a..24d3b076 100644 --- a/src/OpenClaw.Tests/CompanionConnectionTests.cs +++ b/src/OpenClaw.Tests/CompanionConnectionTests.cs @@ -14,9 +14,8 @@ public sealed class CompanionConnectionTests : IDisposable public void Dispose() { - foreach (var dir in _tempDirs) - if (Directory.Exists(dir)) - Directory.Delete(dir, recursive: true); + foreach (var dir in _tempDirs.Where(Directory.Exists)) + Directory.Delete(dir, recursive: true); } [AvaloniaFact]