From f9328a0f9fc1812cca3b28372be5dd9e4a7f2c19 Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 14:54:56 -0700 Subject: [PATCH 1/8] perf(gateway): verify each request's account token once Account-token verification runs PBKDF2 (120,000 iterations, about 17 ms of CPU) inside OperatorAccountService's global lock and then rewrites the accounts file. The role checks added in this branch meant that /v1/*, /apps/chat, A2A, /ws, /ws/live, and mutating MCP tools verified the same token twice per request, halving account-token throughput on those surfaces. Cache the verification outcome in HttpContext.Items for the rest of the request. Revocation, disabling, and role changes still apply from the next request. Co-Authored-By: Claude Opus 5.5 (1M context) --- .../Endpoints/EndpointHelpers.cs | 28 ++++++- .../EndpointHelpersAuthenticationTests.cs | 81 +++++++++++++++++++ 2 files changed, 107 insertions(+), 2 deletions(-) create mode 100644 src/OpenClaw.Tests/EndpointHelpersAuthenticationTests.cs diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 209ec51c..1c899934 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -69,7 +69,7 @@ public static bool IsAuthorizedRequest(HttpContext ctx, GatewayConfig config, bo if (operatorAccounts is not null) { var token = GatewaySecurity.GetToken(ctx, config.Security.AllowQueryStringToken); - if (!string.IsNullOrWhiteSpace(token) && operatorAccounts.TryAuthenticateToken(token, out _)) + if (!string.IsNullOrWhiteSpace(token) && TryAuthenticateAccountToken(ctx, operatorAccounts, token, out _)) return true; } } @@ -161,7 +161,7 @@ public static OperatorAuthorizationResult AuthorizeOperatorRequest( if (IsAllowedAuthMode(policy, OrganizationAuthModeNames.AccountToken) && !string.IsNullOrWhiteSpace(token) && operatorAccounts is not null && - operatorAccounts.TryAuthenticateToken(token, out var accountIdentity)) + TryAuthenticateAccountToken(ctx, operatorAccounts, token, out var accountIdentity)) { return new OperatorAuthorizationResult( true, @@ -203,6 +203,30 @@ operatorAccounts is not null && IsBootstrapAdmin: false); } + private sealed record AccountTokenVerification(string Token, bool Succeeded, OperatorIdentitySnapshot? Identity); + + // Token verification is deliberately slow (PBKDF2) and serialized inside OperatorAccountService, and one + // request can pass through several checks (authentication, role, identity). Verify each request's token + // once and reuse the outcome; revocation still applies from the next request. + private static bool TryAuthenticateAccountToken( + HttpContext ctx, + OperatorAccountService operatorAccounts, + string token, + out OperatorIdentitySnapshot? identity) + { + if (ctx.Items.TryGetValue(typeof(AccountTokenVerification), out var cached) && + cached is AccountTokenVerification previous && + string.Equals(previous.Token, token, StringComparison.Ordinal)) + { + identity = previous.Identity; + return previous.Succeeded; + } + + var succeeded = operatorAccounts.TryAuthenticateToken(token, out identity); + ctx.Items[typeof(AccountTokenVerification)] = new AccountTokenVerification(token, succeeded, identity); + return succeeded; + } + public static bool TrySetMaxRequestBodySize(HttpContext ctx, long maxBytes) { var feature = ctx.Features.Get(); diff --git a/src/OpenClaw.Tests/EndpointHelpersAuthenticationTests.cs b/src/OpenClaw.Tests/EndpointHelpersAuthenticationTests.cs new file mode 100644 index 00000000..3bf10f14 --- /dev/null +++ b/src/OpenClaw.Tests/EndpointHelpersAuthenticationTests.cs @@ -0,0 +1,81 @@ +using Microsoft.AspNetCore.Http; +using Microsoft.Extensions.DependencyInjection; +using Microsoft.Extensions.Logging.Abstractions; +using OpenClaw.Core.Models; +using OpenClaw.Gateway; +using OpenClaw.Gateway.Bootstrap; +using OpenClaw.Gateway.Endpoints; +using Xunit; + +namespace OpenClaw.Tests; + +public sealed class EndpointHelpersAuthenticationTests : IDisposable +{ + private readonly string _storagePath = Path.Combine(Path.GetTempPath(), "openclaw-endpoint-auth-tests", Guid.NewGuid().ToString("N")); + + public void Dispose() + { + try { Directory.Delete(_storagePath, recursive: true); } + catch { } + } + + [Fact] + public void AccountToken_WhenCheckedTwiceInOneRequest_ShouldBeVerifiedOnce() + { + var (startup, services, accounts, accountId, token) = CreateOperatorToken(); + var ctx = CreateRequest(services, token.Token); + + Assert.True(EndpointHelpers.IsAuthorizedRequest(ctx, startup.Config, startup.IsNonLoopbackBind)); + + // Revoking after the first check makes a second verification observable: a re-run of the slow + // token check would now fail, while a reused result for this request still succeeds. + Assert.True(accounts.RevokeToken(accountId, token.TokenInfo!.Id)); + var auth = EndpointHelpers.AuthorizeOperatorRequest(ctx, startup, services.GetRequiredService(), requireCsrf: false); + + Assert.True(auth.IsAuthorized); + Assert.Equal(accountId, auth.AccountId); + } + + [Fact] + public void AccountToken_WhenRevokedBeforeNextRequest_ShouldBeRejected() + { + var (startup, services, accounts, accountId, token) = CreateOperatorToken(); + Assert.True(EndpointHelpers.IsAuthorizedRequest(CreateRequest(services, token.Token), startup.Config, startup.IsNonLoopbackBind)); + + Assert.True(accounts.RevokeToken(accountId, token.TokenInfo!.Id)); + + Assert.False(EndpointHelpers.IsAuthorizedRequest(CreateRequest(services, token.Token), startup.Config, startup.IsNonLoopbackBind)); + } + + private (GatewayStartupContext Startup, ServiceProvider Services, OperatorAccountService Accounts, string AccountId, OperatorAccountTokenCreateResponse Token) CreateOperatorToken() + { + var config = new GatewayConfig { AuthToken = "bootstrap-token" }; + var startup = new GatewayStartupContext + { + Config = config, + RuntimeState = RuntimeModeResolver.Resolve(config.Runtime, dynamicCodeSupported: true), + IsNonLoopbackBind = true + }; + var accounts = new OperatorAccountService(_storagePath, NullLogger.Instance); + var account = accounts.Create(new OperatorAccountCreateRequest + { + Username = "memo-operator", + Password = "P@ssw0rd123!", + Role = OperatorRoleNames.Operator + }); + var token = accounts.CreateToken(account.Id, new OperatorAccountTokenCreateRequest { Label = "memo" })!; + + var services = new ServiceCollection(); + services.AddSingleton(new BrowserSessionAuthService(config)); + services.AddSingleton(accounts); + services.AddSingleton(new OrganizationPolicyService(_storagePath, NullLogger.Instance)); + return (startup, services.BuildServiceProvider(), accounts, account.Id, token); + } + + private static DefaultHttpContext CreateRequest(IServiceProvider services, string token) + { + var ctx = new DefaultHttpContext { RequestServices = services }; + ctx.Request.Headers.Authorization = $"Bearer {token}"; + return ctx; + } +} From bac8ec728cfe0f89dbcdbb481a506a72a81e0723 Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 12:08:10 -0700 Subject: [PATCH 2/8] fix(gateway): require operator role for the /ws/live model bridge /ws/live admitted any authenticated role. It runs no tools, but it opens a live session on the gateway's provider credentials, so a viewer token could spend them. It now uses CanExecuteAgent like /ws and closes below operator with 1008. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/AUTHENTICATION.md | 5 ++-- docs/zh-CN/AUTHENTICATION.md | 5 ++-- src/OpenClaw.Core/Models/GatewayConfig.cs | 3 +- .../Endpoints/EndpointHelpers.cs | 5 ++-- .../Endpoints/WebSocketEndpoints.cs | 8 +++++ .../GatewayAdminEndpointTests.cs | 30 +++++++++++++++++-- 7 files changed, 48 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5411dcda..3d5cc2ef 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -38,6 +38,7 @@ All notable changes to this project are tracked in this file. - 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. + - `/ws/live`: closed with code 1008. The live model bridge runs no tools but spends provider credentials. - `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. diff --git a/docs/AUTHENTICATION.md b/docs/AUTHENTICATION.md index 1ce6e2db..a1655f5c 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -131,7 +131,7 @@ Request enters ### 3.2 WebSocket 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): +WebSocket endpoints (`/ws`, `/ws/live`) authenticate in Phase 1 and then apply the role check (Phase 2). `/ws` also resolves the user ID (Phase 3): **Phase 1: `TryValidateWebSocketRequest` → `IsAuthorizedRequest`** @@ -149,7 +149,7 @@ WebSocket request (/ws) └─ Passed ──→ Accept WebSocket connection ``` -**Phase 2 (`/ws` only): `EndpointHelpers.CanExecuteAgent`** +**Phase 2: `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: @@ -185,6 +185,7 @@ WebSocket connected | Surface | Below operator | |---------|----------------| | `/ws` | Accepted, then closed with 1008 (PolicyViolation) | +| `/ws/live` | Accepted, then closed with 1008. The live bridge runs no tools but spends provider credentials | | `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 | diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index 13d03dde..9ef7bc48 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -131,7 +131,7 @@ HTTP API 端点使用 `AuthorizeOperatorRequest` 方法（[EndpointHelpers.cs](. ### 3.2 WebSocket 认证流程 -WebSocket 端点 (`/ws`, `/ws/live`) 在第一步完成认证；`/ws` 随后执行聊天角色检查（第二步）并解析用户 ID（第三步）： +WebSocket 端点 (`/ws`, `/ws/live`) 在第一步完成认证，随后执行角色检查（第二步）；`/ws` 还会解析用户 ID（第三步）： **第一步：`TryValidateWebSocketRequest` → `IsAuthorizedRequest`** @@ -149,7 +149,7 @@ WebSocket 请求 (/ws) └─ 通过 ──→ 接受 WebSocket 连接 ``` -**第二步（仅 `/ws`）：`EndpointHelpers.CanExecuteAgent`** +**第二步：`EndpointHelpers.CanExecuteAgent`** 每个 `/ws` 帧都会成为智能体输入，因此连接需要与 `POST /api/integration/messages` 相同的 `operator` 角色。角色通过 `AuthorizeOperatorRequest` 解析，与 HTTP API 使用同一认证链： @@ -185,6 +185,7 @@ WebSocket 已连接 | 入口 | 角色低于 operator 时 | |------|----------------------| | `/ws` | 先接受，再以 1008 (PolicyViolation) 关闭 | +| `/ws/live` | 先接受，再以 1008 关闭。实时桥接不运行工具，但会消耗提供商凭据额度 | | `POST /v1/chat/completions`、`POST /v1/responses` | 403，返回 OpenAI 风格的 `permission_error` 响应体 | | A2A 执行路径（发现端点仍然公开） | 403 | | `POST /apps/chat` | 403 | diff --git a/src/OpenClaw.Core/Models/GatewayConfig.cs b/src/OpenClaw.Core/Models/GatewayConfig.cs index 17c5b175..4de35924 100644 --- a/src/OpenClaw.Core/Models/GatewayConfig.cs +++ b/src/OpenClaw.Core/Models/GatewayConfig.cs @@ -387,7 +387,8 @@ 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, MCP App tool calls, and mutating MCP tools. Each such request is logged so the accounts can be promoted. + /// /apps/chat, MCP App tool calls, mutating MCP tools, and /ws/live. Each such request is logged so the accounts + /// can be promoted. /// public bool AllowViewerAgentExecution { get; set; } = false; diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 1c899934..25152a78 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -357,7 +357,8 @@ 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 and tool calls, mutating MCP tools) require the same role as POST /api/integration/messages. + /// A2A, MCP Apps chat and tool calls, mutating MCP tools, the live model bridge) 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. @@ -393,7 +394,7 @@ public static bool CanExecuteAgent( } logger.LogWarning( - "Denied {Action} for {AuthMode} account {AccountId} ({Username}) with role {Role}: running the agent requires the operator role.", + "Denied {Action} for {AuthMode} account {AccountId} ({Username}) with role {Role}: this action requires the operator role.", action, auth.AuthMode, auth.AccountId, auth.Username, auth.Role); return false; } diff --git a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs index af908099..7b3fbc1b 100644 --- a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs @@ -43,6 +43,14 @@ public static void MapOpenClawWebSocketEndpoints( return; var ws = await ctx.WebSockets.AcceptWebSocketAsync(); + + // The live bridge runs no tools but spends provider credentials, so it needs the same role as /ws. + if (!EndpointHelpers.CanExecuteAgent(ctx, startup)) + { + await ws.CloseAsync(WebSocketCloseStatus.PolicyViolation, EndpointHelpers.OperatorRoleRequiredMessage, ctx.RequestAborted); + return; + } + try { var openRequest = await ReceiveLiveOpenRequestAsync(ws, ctx.RequestAborted); diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index 777b1b2e..d2af936b 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -380,6 +380,32 @@ public async Task WebSocketChat_WhenOpenLoopbackWithoutCredentials_ShouldStayOpe Assert.Null(await ReceiveWithinAsync(ws, TimeSpan.FromMilliseconds(500))); } + [Fact] + public async Task WebSocketLive_WhenViewerAccountToken_ShouldCloseWithPolicyViolation() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "live-viewer", OperatorRoleNames.Viewer); + + using var ws = await ConnectWebSocketAsync(harness, token, "/ws/live"); + 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 WebSocketLive_WhenOperatorAccountToken_ShouldStayOpen() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var token = CreateAccountToken(harness, "live-operator", OperatorRoleNames.Operator); + + using var ws = await ConnectWebSocketAsync(harness, token, "/ws/live"); + + Assert.Null(await ReceiveWithinAsync(ws, TimeSpan.FromMilliseconds(500))); + } + [Theory] [InlineData("/v1/chat/completions", """{"messages":[{"role":"user","content":"hello"}]}""")] [InlineData("/v1/responses", """{"input":"hello"}""")] @@ -719,12 +745,12 @@ private static string CreateAccountToken(GatewayTestHarness harness, string user return token!.Token; } - private static async Task ConnectWebSocketAsync(GatewayTestHarness harness, string? bearerToken) + private static async Task ConnectWebSocketAsync(GatewayTestHarness harness, string? bearerToken, string path = "/ws") { 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); + return await client.ConnectAsync(new Uri("ws://localhost" + path), CancellationToken.None); } // Returns null when nothing arrives in time, which for these tests means the server kept the connection open. From 4e64d0ee21891dd37f3993d73aee73cd8032a1db Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 12:08:10 -0700 Subject: [PATCH 3/8] feat(companion): check the account role before opening chat Companion opened the chat socket before loading the account's role, so a viewer was admitted and then closed by the gateway. It now loads the role first; when the gateway reports a role below operator it explains that chat needs the operator role instead of connecting, and keeps the read-only status views. A role that is only the no-token placeholder does not block the connection, and the role is applied as soon as the auth session loads rather than after the setup status call. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 +- .../ViewModels/MainWindowViewModel.cs | 18 ++++++- .../CompanionConnectionTests.cs | 54 +++++++++++++++++++ 3 files changed, 71 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3d5cc2ef..b47d4edb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,7 @@ All notable changes to this project are tracked in this file. - 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. +- 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. It also checks the account's role before connecting: a gateway-reported role below `operator` gets an explanation instead of a chat connection, and the read-only status views stay available. - 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/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs index 08e9d62b..201e5906 100644 --- a/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs +++ b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs @@ -65,6 +65,8 @@ public sealed partial class MainWindowViewModel : ViewModelBase [ObservableProperty] private string _operatorRole = OperatorRoleNames.Viewer; + private bool _operatorRoleReportedByGateway; + [ObservableProperty] private string _operatorAuthMode = "account_token"; @@ -526,11 +528,21 @@ private async Task ConnectAsync() SaveSettings(); Status = "Connecting…"; + + // Learn the role first: a viewer would be admitted and then closed by the gateway, so say why up front + // and keep the read-only status views. Only trust a role the gateway reported, not the no-token placeholder. + await LoadAdminStatusAsyncInternal(); + if (_operatorRoleReportedByGateway && !IsBootstrapAdmin && !OperatorRoleNames.CanAccess(OperatorRole, OperatorRoleNames.Operator)) + { + Status = "Disconnected"; + AddSystemMessage($"Signed in as {OperatorIdentity} with the {OperatorRole} role. Chat needs the operator role; ask an admin to change this account's role."); + return; + } + await _client.ConnectAsync(uri, string.IsNullOrWhiteSpace(AuthToken) ? null : AuthToken, CancellationToken.None); IsConnected = true; Status = "Connected"; await SendCanvasReadyAsync(); - await LoadAdminStatusAsyncInternal(); await LoadWhatsAppSetupAsync(); } catch (Exception ex) @@ -628,6 +640,7 @@ private async Task LoadAdminStatusAsync() private async Task LoadAdminStatusAsyncInternal() { + _operatorRoleReportedByGateway = false; using var client = CreateAdminClient(out var error); if (client is null) { @@ -647,8 +660,9 @@ private async Task LoadAdminStatusAsyncInternal() try { var auth = await client.GetAuthSessionAsync(CancellationToken.None); - var setup = await client.GetSetupStatusAsync(CancellationToken.None); ApplyOperatorIdentity(auth.AuthMode, auth.Role, auth.DisplayName, auth.Username, auth.IsBootstrapAdmin); + _operatorRoleReportedByGateway = true; + var setup = await client.GetSetupStatusAsync(CancellationToken.None); AdminStatus = auth.IsBootstrapAdmin ? "Using bootstrap/breakglass admin auth." : $"Authenticated as {auth.DisplayName ?? auth.Username ?? "operator"} via {auth.AuthMode}."; diff --git a/src/OpenClaw.Tests/CompanionConnectionTests.cs b/src/OpenClaw.Tests/CompanionConnectionTests.cs index 24d3b076..fb6807b2 100644 --- a/src/OpenClaw.Tests/CompanionConnectionTests.cs +++ b/src/OpenClaw.Tests/CompanionConnectionTests.cs @@ -1,8 +1,11 @@ +using System.Net; using System.Net.WebSockets; +using System.Text; using Avalonia.Headless.XUnit; using Avalonia.Threading; using OpenClaw.Companion.Models; using OpenClaw.Companion.Services; +using OpenClaw.Client; using OpenClaw.Companion.ViewModels; using Xunit; @@ -66,6 +69,57 @@ public async Task ServerClose_WhenClientReconnectsBeforeUiDispatch_ShouldKeepCon Assert.DoesNotContain(vm.Messages, message => message.Role == ChatRole.System); } + [AvaloniaFact] + public async Task Connect_WhenGatewayReportsViewerRole_ShouldExplainWithoutOpeningChat() + { + var vm = CreateViewModelWithAuthSession("""{"authMode":"account_token","role":"viewer","username":"reader"}"""); + vm.AuthToken = "viewer-token"; + + await vm.ConnectCommand.ExecuteAsync(null); + Dispatcher.UIThread.RunJobs(); + + Assert.False(vm.IsConnected); + Assert.Equal("Disconnected", vm.Status); + Assert.Contains(vm.Messages, m => m.Text.Contains("operator role", StringComparison.OrdinalIgnoreCase)); + Assert.DoesNotContain(vm.Messages, m => m.Text.StartsWith("Connect failed", StringComparison.Ordinal)); + } + + [AvaloniaFact] + public async Task Connect_WhenNoTokenLoaded_ShouldStillAttemptChat() + { + // Without a token the viewer role is only a placeholder, not something the gateway reported. + var vm = CreateViewModelWithAuthSession("""{"authMode":"account_token","role":"viewer"}"""); + + await vm.ConnectCommand.ExecuteAsync(null); + Dispatcher.UIThread.RunJobs(); + + Assert.Contains(vm.Messages, m => m.Text.StartsWith("Connect failed", StringComparison.Ordinal)); + Assert.DoesNotContain(vm.Messages, m => m.Text.Contains("operator role", StringComparison.OrdinalIgnoreCase)); + } + + private MainWindowViewModel CreateViewModelWithAuthSession(string authSessionJson) + { + var dir = Path.Combine(Path.GetTempPath(), "openclaw-companion-connection-tests", Guid.NewGuid().ToString("N")); + Directory.CreateDirectory(dir); + _tempDirs.Add(dir); + var vm = new MainWindowViewModel( + new SettingsStore(dir), + new GatewayWebSocketClient(), + (baseUrl, authToken) => new OpenClawHttpClient(baseUrl, authToken, new HttpClient(new CallbackHandler(request => + request.RequestUri!.AbsolutePath == "/auth/session" + ? new HttpResponseMessage(HttpStatusCode.OK) { Content = new StringContent(authSessionJson, Encoding.UTF8, "application/json") } + : new HttpResponseMessage(HttpStatusCode.NotFound))))); + // Nothing listens here, so an attempted chat connection fails fast and visibly. + vm.ServerUrl = "ws://127.0.0.1:9/ws"; + return vm; + } + + private sealed class CallbackHandler(Func callback) : HttpMessageHandler + { + protected override Task SendAsync(HttpRequestMessage request, CancellationToken cancellationToken) + => Task.FromResult(callback(request)); + } + private (MainWindowViewModel ViewModel, GatewayWebSocketClient Client) CreateConnectedViewModel() { var dir = Path.Combine(Path.GetTempPath(), "openclaw-companion-connection-tests", Guid.NewGuid().ToString("N")); From 00e20e0139f60fad6f3b364301a6731832817d9e Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 15:03:57 -0700 Subject: [PATCH 4/8] fix(gateway): run turns as the signed-in account, not a claimed sender Session.AuthenticatedUserId scopes per-user capability bindings and is passed to MCP servers as _meta.userId, falling back to SenderId. Only /ws set it, so turns from REST messages, MCP send_message, A2A, /v1/*, and /apps/chat ran as whatever senderId or A2A contextId the caller supplied. - EndpointHelpers.ResolveAuthenticatedAccountId resolves the signed-in account (reusing the per-request token verification). REST and MCP stamp it on queued messages; /v1/*, /apps/chat, and the A2A bridge set it on the session. A2A captures it in middleware through an async-local, so detached SDK work still sees it. - The worker no longer lets an account-less external turn inherit the previous writer's account: after an operator posted into a Telegram session, the Telegram user's turns ran as that operator. System, scheduled, automation, and background-continuation turns still act on the session's behalf and keep its identity. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 + docs/AUTHENTICATION.md | 17 ++ docs/zh-CN/AUTHENTICATION.md | 17 ++ src/OpenClaw.Gateway/A2A/A2ACallerContext.cs | 16 ++ .../A2A/A2AEndpointExtensions.cs | 2 + .../A2A/OpenClawA2AExecutionBridge.cs | 3 + .../Composition/IntegrationApiFacade.cs | 12 +- .../Endpoints/AppsEndpoints.cs | 1 + .../Endpoints/EndpointHelpers.cs | 12 ++ .../Endpoints/IntegrationEndpoints.cs | 2 +- .../OpenAiEndpoints.ChatCompletions.cs | 4 + .../Endpoints/OpenAiEndpoints.Responses.cs | 4 + .../Extensions/GatewayInboundMessageWorker.cs | 6 +- .../Extensions/TurnIdentity.cs | 35 +++++ src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs | 7 +- .../GatewayAdminEndpointTests.cs | 148 ++++++++++++++++++ src/OpenClaw.Tests/TurnIdentityTests.cs | 68 ++++++++ 17 files changed, 345 insertions(+), 11 deletions(-) create mode 100644 src/OpenClaw.Gateway/A2A/A2ACallerContext.cs create mode 100644 src/OpenClaw.Gateway/Extensions/TurnIdentity.cs create mode 100644 src/OpenClaw.Tests/TurnIdentityTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index b47d4edb..188c4936 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,8 @@ All notable changes to this project are tracked in this file. - 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`. +- Turns now run as the signed-in account instead of a caller-supplied sender id. `Session.AuthenticatedUserId` scopes per-user capability bindings and is passed to MCP servers as `_meta.userId`. Previously only `/ws` set it, so REST messages, MCP `send_message`, A2A, `/v1/*`, and `/apps/chat` turns ran as whatever `senderId` or `contextId` the caller supplied. MCP servers that read `_meta.userId` now receive account ids for those turns. +- A turn from an external sender without an account no longer inherits the previous writer's account. Previously, after an operator posted into a Telegram session, the Telegram user's next turns ran as that operator. System, scheduled, automation, and background-continuation turns keep the session's identity. - 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. It also checks the account's role before connecting: a gateway-reported role below `operator` gets an explanation instead of a chat connection, and the read-only status views stay available. - 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 a1655f5c..10ee10da 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -236,6 +236,23 @@ return false; // 401 Unauthorized --- +### 3.5 Turn Identity + +`Session.AuthenticatedUserId` is the identity a turn runs with. It scopes per-user capability bindings and is passed to MCP servers as `_meta.userId`. When it is empty, those fall back to the session's `SenderId`. + +Surfaces that run turns set it from the signed-in account (`EndpointHelpers.ResolveAuthenticatedAccountId`), never from a caller-supplied sender id: + +| Surface | Identity source | +|---------|-----------------| +| `POST /api/integration/messages`, MCP `openclaw.send_message` | Account of the request; the body's `senderId` is only used for routing and display | +| `/ws` | Account resolved at connection | +| `POST /v1/chat/completions`, `POST /v1/responses`, `POST /apps/chat` | Account of the request | +| A2A execution | Account of the request; the A2A `contextId` is only the sender id | + +Open loopback and bootstrap callers have no account, so their turns run without one. + +A pipeline turn from an external sender without an account runs without one too. It does not inherit the account of whoever wrote to the session before. For example, a Telegram user's turn never runs as an operator who posted into that Telegram session. System, scheduled, automation, and background-continuation turns act on the session's behalf and keep its identity. + ## 4. Middleware Pipeline Authentication middleware is registered in `Program.cs` in the following order: diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index 9ef7bc48..a713e782 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -236,6 +236,23 @@ return false; // 401 Unauthorized --- +### 3.5 轮次身份 + +`Session.AuthenticatedUserId` 是一个轮次运行时所用的身份。它划分按用户的能力绑定范围，并以 `_meta.userId` 传给 MCP 服务器。为空时，二者回退到会话的 `SenderId`。 + +运行轮次的入口会根据已登录账户设置它（`EndpointHelpers.ResolveAuthenticatedAccountId`），绝不采用调用方提供的发送者 ID： + +| 入口 | 身份来源 | +|------|----------| +| `POST /api/integration/messages`、MCP `openclaw.send_message` | 请求的账户；请求体中的 `senderId` 只用于路由和显示 | +| `/ws` | 连接时解析出的账户 | +| `POST /v1/chat/completions`、`POST /v1/responses`、`POST /apps/chat` | 请求的账户 | +| A2A 执行 | 请求的账户；A2A 的 `contextId` 只作为发送者 ID | + +开放回环和引导令牌调用方没有账户，因此它们的轮次不带账户身份运行。 + +来自外部发送者且不带账户的管道轮次同样不带账户身份运行，不会继承此前写入该会话的账户。例如，Telegram 用户的轮次绝不会以曾向该 Telegram 会话发消息的操作员身份运行。系统、定时、自动化和后台续跑轮次代表会话执行，保留会话原有身份。 + ## 四、中间件管道 认证相关的中间件在 `Program.cs` 中按以下顺序注册： diff --git a/src/OpenClaw.Gateway/A2A/A2ACallerContext.cs b/src/OpenClaw.Gateway/A2A/A2ACallerContext.cs new file mode 100644 index 00000000..b300c206 --- /dev/null +++ b/src/OpenClaw.Gateway/A2A/A2ACallerContext.cs @@ -0,0 +1,16 @@ +namespace OpenClaw.Gateway.A2A; + +/// +/// The signed-in account behind the current A2A request. The A2A middleware sets it before the SDK runs the +/// handler; unlike the HTTP context, an async-local value still flows into work the SDK detaches from the request. +/// +internal static class A2ACallerContext +{ + private static readonly AsyncLocal Current = new(); + + public static string? AccountId + { + get => Current.Value; + set => Current.Value = value; + } +} diff --git a/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs b/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs index 4b501af5..e4488471 100644 --- a/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs +++ b/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs @@ -80,6 +80,8 @@ public static void UseOpenClawA2AAuth( return; } + A2ACallerContext.AccountId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + if (!runtime.Operations.ActorRateLimits.TryConsume( "ip", EndpointHelpers.GetRemoteIpKey(ctx), diff --git a/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs b/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs index a1f4a153..37315782 100644 --- a/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs +++ b/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs @@ -32,6 +32,9 @@ public async Task ExecuteStreamingAsync( await using var sessionLock = await runtime.SessionManager.AcquireSessionLockAsync(session.Id, cancellationToken); + // The A2A request's SenderId is the caller's contextId, so the turn's identity comes from the signed-in account. + session.AuthenticatedUserId = A2ACallerContext.AccountId; + var (handled, commandResponse) = await runtime.CommandProcessor.TryProcessCommandAsync( session, request.UserText, diff --git a/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs b/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs index 9372c469..a780593a 100644 --- a/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs +++ b/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs @@ -787,7 +787,14 @@ public async Task ListLearningProposalsAsync(strin return rejected; } - public async Task QueueMessageAsync(IntegrationMessageRequest request, CancellationToken cancellationToken) + /// + /// The signed-in account resolved by the calling surface. It becomes the turn's identity; the request's + /// senderId is caller-supplied and only used for routing and display. + /// + public async Task QueueMessageAsync( + IntegrationMessageRequest request, + CancellationToken cancellationToken, + string? authenticatedUserId = null) { var effectiveChannelId = string.IsNullOrWhiteSpace(request.ChannelId) ? "integration-api" : request.ChannelId.Trim(); var effectiveSenderId = string.IsNullOrWhiteSpace(request.SenderId) ? "http-client" : request.SenderId.Trim(); @@ -805,7 +812,8 @@ public async Task QueueMessageAsync(IntegrationMessa Type = "user_message", Text = request.Text, MessageId = request.MessageId, - ReplyToMessageId = request.ReplyToMessageId + ReplyToMessageId = request.ReplyToMessageId, + AuthenticatedUserId = authenticatedUserId }; if (!_runtime.Pipeline.InboundWriter.TryWrite(message)) diff --git a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs index f88936e8..d7dfbf73 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs @@ -82,6 +82,7 @@ public static void MapOpenClawAppsEndpoints( ? $"apps-{Guid.NewGuid():N}" : requestedSessionId!; var session = await runtime.SessionManager.GetOrCreateByIdAsync(sessionId, "apps", sessionId, ct); + session.AuthenticatedUserId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); await SendAsync(ctx, new JsonObject { ["type"] = "session", ["sessionId"] = sessionId }, ct); diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 25152a78..199c4341 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -399,6 +399,18 @@ public static bool CanExecuteAgent( return false; } + /// + /// The signed-in account behind a request, used as the identity of the turns it starts (per-user capability + /// scope and the userId passed to MCP servers) in place of any caller-supplied sender id. + /// Null for open loopback and bootstrap callers, which have no account and are admin-equivalent. + /// + public static string? ResolveAuthenticatedAccountId(HttpContext ctx, GatewayStartupContext startup) + { + var browserSessions = ctx.RequestServices.GetRequiredService(); + var auth = AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); + return auth.IsAuthorized && !string.IsNullOrWhiteSpace(auth.AccountId) ? auth.AccountId : null; + } + public static async Task WriteOperatorRoleRequiredAsync(HttpContext ctx) { ctx.Response.StatusCode = StatusCodes.Status403Forbidden; diff --git a/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs index b61fe05e..747a028e 100644 --- a/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs @@ -849,7 +849,7 @@ await runtime.PaymentRuntime.GetPaymentStatusAsync(id, provider, BuildPaymentCon } return Results.Json( - await facade.QueueMessageAsync(request, ctx.RequestAborted), + await facade.QueueMessageAsync(request, ctx.RequestAborted, EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup)), CoreJsonContext.Default.IntegrationMessageResponse, statusCode: StatusCodes.Status202Accepted); }); diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs index 356c3f00..2decf367 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs @@ -98,6 +98,10 @@ private static void MapChatCompletionsEndpoint( persistStableSessionOnExit = true; } + // The turn runs as the signed-in account: it scopes per-user capability bindings and is the + // userId MCP servers see. Callers without an account (open loopback, bootstrap) run without one. + session.AuthenticatedUserId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + var httpMwCtx = new MessageContext { ChannelId = "openai-http", diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs index 0adfdfe2..893500ec 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs @@ -87,6 +87,10 @@ private static void MapResponsesEndpoint( persistStableSessionOnExit = true; } + // The turn runs as the signed-in account: it scopes per-user capability bindings and is the + // userId MCP servers see. Callers without an account (open loopback, bootstrap) run without one. + session.AuthenticatedUserId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + var httpMwCtx = new MessageContext { ChannelId = "openai-responses", diff --git a/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs b/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs index 6ac0b73b..a78820fa 100644 --- a/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs +++ b/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs @@ -385,12 +385,8 @@ await pipeline.OutboundWriter.WriteAsync(new OutboundMessage sessionLock = await sessionManager.AcquireSessionLockAsync(session.Id, processingCt); - if (!string.IsNullOrWhiteSpace(msg.AuthenticatedUserId) && - !string.Equals(session.AuthenticatedUserId, msg.AuthenticatedUserId, StringComparison.Ordinal)) - { - session.AuthenticatedUserId = msg.AuthenticatedUserId; + if (TurnIdentity.Apply(session, msg)) await sessionManager.PersistAsync(session, processingCt, sessionLockHeld: true); - } if (automationService is not null && !string.IsNullOrWhiteSpace(msg.CronJobName)) { diff --git a/src/OpenClaw.Gateway/Extensions/TurnIdentity.cs b/src/OpenClaw.Gateway/Extensions/TurnIdentity.cs new file mode 100644 index 00000000..448c0fd1 --- /dev/null +++ b/src/OpenClaw.Gateway/Extensions/TurnIdentity.cs @@ -0,0 +1,35 @@ +using OpenClaw.Core.Models; + +namespace OpenClaw.Gateway.Extensions; + +/// +/// Decides whose identity a pipeline turn runs with. scopes per-user +/// capability bindings and is passed to MCP servers as the userId, so it must describe the caller of the current +/// turn rather than whoever last wrote to the session. +/// +internal static class TurnIdentity +{ + /// Returns true when the session's identity changed and should be persisted. + public static bool Apply(Session session, InboundMessage message) + { + string? identity; + if (!string.IsNullOrWhiteSpace(message.AuthenticatedUserId)) + identity = message.AuthenticatedUserId; + else if (ActsOnSessionsBehalf(message)) + return false; + else + identity = null; // an external sender without an account must not inherit a previous writer's account + + if (string.Equals(session.AuthenticatedUserId, identity, StringComparison.Ordinal)) + return false; + + session.AuthenticatedUserId = identity; + return true; + } + + private static bool ActsOnSessionsBehalf(InboundMessage message) + => message.IsSystem + || !string.IsNullOrWhiteSpace(message.CronJobName) + || !string.IsNullOrWhiteSpace(message.AutomationRunId) + || !string.IsNullOrWhiteSpace(message.BackgroundRunId); +} diff --git a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs index 0603cf1f..81efc048 100644 --- a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs +++ b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs @@ -305,7 +305,7 @@ public async Task SendMessage( [Description("Optional reply-to message ID.")] string? replyToMessageId = null, CancellationToken ct = default) { - RequireOperator("openclaw.send_message"); + var caller = RequireOperator("openclaw.send_message"); return JsonSerializer.Serialize( await _facade.QueueMessageAsync(new IntegrationMessageRequest { @@ -315,17 +315,18 @@ await _facade.QueueMessageAsync(new IntegrationMessageRequest SessionId = sessionId, MessageId = messageId, ReplyToMessageId = replyToMessageId - }, ct), + }, ct, EndpointHelpers.ResolveAuthenticatedAccountId(caller, _startup)), 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(string toolName) + private HttpContext RequireOperator(string toolName) { var ctx = _httpContextAccessor.HttpContext; if (ctx is null || !EndpointHelpers.CanExecuteAgent(ctx, _startup, $"MCP tool {toolName}")) throw new McpException(EndpointHelpers.OperatorRoleRequiredMessage); + return ctx; } private static JsonElement? ParsePayloadJson(string? payloadJson) diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index d2af936b..1f408f60 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -716,6 +716,154 @@ public void Log(LogLevel logLevel, EventId eventId, TState state, Except } } + [Fact] + public async Task IntegrationMessages_WhenAccountToken_ShouldStampAccountNotClaimedSender() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (token, accountId) = CreateAccountTokenWithId(harness, "rest-operator", OperatorRoleNames.Operator); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/api/integration/messages") + { + Content = JsonContent("""{"text":"hello","senderId":"someone-else","sessionId":"stamp-rest"}""") + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + var response = await harness.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Accepted, response.StatusCode); + Assert.True(harness.Runtime.Pipeline.InboundReader.TryRead(out var queued)); + Assert.Equal("someone-else", queued.SenderId); + Assert.Equal(accountId, queued.AuthenticatedUserId); + } + + [Fact] + public async Task McpSendMessage_WhenAccountToken_ShouldStampAccountNotClaimedSender() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (token, accountId) = CreateAccountTokenWithId(harness, "mcp-operator", OperatorRoleNames.Operator); + + using var result = await CallMcpToolAsync(harness, token, "openclaw.send_message", """{"text":"hello","senderId":"someone-else"}"""); + + Assert.False(result.RootElement.TryGetProperty("isError", out var isError) && isError.GetBoolean()); + Assert.True(harness.Runtime.Pipeline.InboundReader.TryRead(out var queued)); + Assert.Equal(accountId, queued.AuthenticatedUserId); + } + + [Fact] + public async Task ChatCompletions_WhenAccountToken_ShouldRunTurnAsAccount() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (token, accountId) = CreateAccountTokenWithId(harness, "openai-identity", OperatorRoleNames.Operator); + Session? ran = null; + harness.Runtime.AgentRuntime.RunAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(callInfo => + { + ran = callInfo.Arg(); + return Task.FromResult("ok"); + }); + + var response = await SendChatCompletionAsync(harness, token); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(accountId, ran?.AuthenticatedUserId); + } + + [Fact] + public async Task AppsChat_WhenAccountToken_ShouldRunTurnAsAccount() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (token, accountId) = CreateAccountTokenWithId(harness, "apps-identity", OperatorRoleNames.Operator); + Session? ran = null; + harness.Runtime.AgentRuntime.RunStreamingAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(callInfo => + { + ran = callInfo.Arg(); + return NoAgentEvents(); + }); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/apps/chat") { Content = JsonContent("""{"message":"hello","sessionId":"stamp-apps"}""") }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + var response = await harness.Client.SendAsync(request); + await response.Content.ReadAsStringAsync(); + + Assert.Equal(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(accountId, ran?.AuthenticatedUserId); + } + + [Fact] + public async Task A2AExecution_WhenAccountToken_ShouldExposeCallerAccountToHandlers() + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configureServices: (services, _) => services.Configure(options => options.EnableA2A = true), + configureApp: app => app.MapPost("/a2a", () => Results.Text(A2ACallerContext.AccountId ?? ""))); + var (token, accountId) = CreateAccountTokenWithId(harness, "a2a-identity", OperatorRoleNames.Operator); + + 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(HttpStatusCode.OK, response.StatusCode); + Assert.Equal(accountId, await response.Content.ReadAsStringAsync()); + } + + [Fact] + public async Task A2ABridge_WhenCallerAccountKnown_ShouldRunTurnAsAccount() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + Session? ran = null; + harness.Runtime.AgentRuntime.RunStreamingAsync(Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(callInfo => + { + ran = callInfo.Arg(); + return NoAgentEvents(); + }); + var bridge = new OpenClawA2AExecutionBridge(new GatewayRuntimeHolder { Runtime = harness.Runtime }, NullLogger.Instance); + + A2ACallerContext.AccountId = "acct-a2a"; + try + { + await bridge.ExecuteStreamingAsync( + new OpenClaw.MicrosoftAgentFrameworkAdapter.A2A.OpenClawA2AExecutionRequest + { + SessionId = "stamp-a2a", + ChannelId = "a2a", + SenderId = "claimed-context", + UserText = "hello" + }, + (_, _) => ValueTask.CompletedTask, + CancellationToken.None); + } + finally + { + A2ACallerContext.AccountId = null; + } + + Assert.Equal("acct-a2a", ran?.AuthenticatedUserId); + } + + private static async IAsyncEnumerable NoAgentEvents() + { + await Task.CompletedTask; + yield break; + } + + private static (string Token, string AccountId) CreateAccountTokenWithId(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 + }); + return (operatorAccounts.CreateToken(account.Id, new OperatorAccountTokenCreateRequest { Label = username })!.Token, account.Id); + } + private static async Task CallMcpToolAsync(GatewayTestHarness harness, string bearerToken, string toolName, string argumentsJson) { using var request = new HttpRequestMessage(HttpMethod.Post, "/mcp") diff --git a/src/OpenClaw.Tests/TurnIdentityTests.cs b/src/OpenClaw.Tests/TurnIdentityTests.cs new file mode 100644 index 00000000..0e5e6f16 --- /dev/null +++ b/src/OpenClaw.Tests/TurnIdentityTests.cs @@ -0,0 +1,68 @@ +using OpenClaw.Core.Models; +using OpenClaw.Gateway.Extensions; +using Xunit; + +namespace OpenClaw.Tests; + +public sealed class TurnIdentityTests +{ + [Fact] + public void Apply_WhenMessageCarriesAccount_ShouldRunAsThatAccount() + { + var session = NewSession(previousAccount: "acct-a"); + + TurnIdentity.Apply(session, Message(authenticatedUserId: "acct-b")); + + Assert.Equal("acct-b", session.AuthenticatedUserId); + } + + [Fact] + public void Apply_WhenExternalSenderHasNoAccount_ShouldNotInheritPreviousAccount() + { + // An operator wrote into this Telegram session earlier; the Telegram user's own turn must not run as that operator. + var session = NewSession(previousAccount: "acct-operator"); + + TurnIdentity.Apply(session, Message(authenticatedUserId: null)); + + Assert.Null(session.AuthenticatedUserId); + } + + [Theory] + [InlineData("system")] + [InlineData("cron")] + [InlineData("automation")] + [InlineData("background")] + public void Apply_WhenTurnActsOnSessionsBehalf_ShouldKeepSessionIdentity(string kind) + { + var session = NewSession(previousAccount: "acct-owner"); + var message = kind switch + { + "system" => Message(authenticatedUserId: null) with { IsSystem = true }, + "cron" => Message(authenticatedUserId: null) with { CronJobName = "nightly" }, + "automation" => Message(authenticatedUserId: null) with { AutomationRunId = "run-1" }, + _ => Message(authenticatedUserId: null) with { BackgroundRunId = "bg-1" } + }; + + TurnIdentity.Apply(session, message); + + Assert.Equal("acct-owner", session.AuthenticatedUserId); + } + + private static Session NewSession(string? previousAccount) + => new() + { + Id = "telegram:user-1", + ChannelId = "telegram", + SenderId = "user-1", + AuthenticatedUserId = previousAccount + }; + + private static InboundMessage Message(string? authenticatedUserId) + => new() + { + ChannelId = "telegram", + SenderId = "user-1", + Text = "hi", + AuthenticatedUserId = authenticatedUserId + }; +} From 59218b3e4a80eb761fe46627ba7652fd37b37c99 Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 15:19:58 -0700 Subject: [PATCH 5/8] feat(gateway): record session owners and restrict writes to owner or admin Sessions are keyed only by id, so any operator who knew a session id could post into another account's conversation. - Session.OwnerAccountId is set once, when a signed-in account creates the session (pipeline, /v1/*, /apps/chat, A2A), and never reassigned. - SessionAccess.CanWrite: owned sessions accept the owner or an admin; unowned sessions (channels, cron, older data) stay open and are never claimed by writing to them; callers without an account (bootstrap, open loopback, channel and system turns) are not restricted. - Enforced on every write surface: REST and MCP send_message refuse before queueing (403 / tool error), the worker refuses pipeline turns including /ws with a reply, /apps/chat returns 403, and the A2A bridge emits an error event. Admin status travels with the message because OIDC admins are not in the account store. - Reads are unchanged. GET /api/integration/sessions?owner=me lists the caller's own sessions, backed by an owner filter in the file and SQLite stores and in the shared summary filter. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/AUTHENTICATION.md | 17 +++ docs/zh-CN/AUTHENTICATION.md | 17 +++ src/OpenClaw.Channels/WebSocketChannel.cs | 4 +- src/OpenClaw.Core/Memory/FileMemoryStore.cs | 7 +- src/OpenClaw.Core/Memory/SqliteMemoryStore.cs | 7 +- src/OpenClaw.Core/Models/Messages.cs | 3 + src/OpenClaw.Core/Models/Session.cs | 6 + .../Models/SessionAdminModels.cs | 2 + src/OpenClaw.Core/Sessions/SessionAccess.cs | 20 +++ src/OpenClaw.Core/Sessions/SessionManager.cs | 7 +- src/OpenClaw.Gateway/A2A/A2ACallerContext.cs | 16 +- .../A2A/A2AEndpointExtensions.cs | 4 +- .../A2A/OpenClawA2AExecutionBridge.cs | 12 +- .../Composition/IntegrationApiFacade.cs | 43 ++++-- .../Composition/SessionAdminListing.cs | 8 + .../Endpoints/AdminEndpoints.Sessions.cs | 3 +- .../Endpoints/AppsEndpoints.cs | 12 +- .../Endpoints/EndpointHelpers.cs | 23 ++- .../Endpoints/IntegrationEndpoints.cs | 31 +++- .../OpenAiEndpoints.ChatCompletions.cs | 7 +- .../Endpoints/OpenAiEndpoints.Responses.cs | 7 +- .../Endpoints/WebSocketEndpoints.cs | 3 +- .../Extensions/GatewayInboundMessageWorker.cs | 17 ++- src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs | 25 ++-- .../GatewayAdminEndpointTests.cs | 141 ++++++++++++++++++ src/OpenClaw.Tests/GatewayWorkersTests.cs | 97 ++++++++++++ src/OpenClaw.Tests/SessionOwnershipTests.cs | 90 +++++++++++ src/OpenClaw.Tests/WebSocketChannelTests.cs | 22 +++ 29 files changed, 602 insertions(+), 50 deletions(-) create mode 100644 src/OpenClaw.Core/Sessions/SessionAccess.cs create mode 100644 src/OpenClaw.Tests/SessionOwnershipTests.cs diff --git a/CHANGELOG.md b/CHANGELOG.md index 188c4936..886d7c79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,6 +49,7 @@ All notable changes to this project are tracked in this file. - 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`. - Turns now run as the signed-in account instead of a caller-supplied sender id. `Session.AuthenticatedUserId` scopes per-user capability bindings and is passed to MCP servers as `_meta.userId`. Previously only `/ws` set it, so REST messages, MCP `send_message`, A2A, `/v1/*`, and `/apps/chat` turns ran as whatever `senderId` or `contextId` the caller supplied. MCP servers that read `_meta.userId` now receive account ids for those turns. - A turn from an external sender without an account no longer inherits the previous writer's account. Previously, after an operator posted into a Telegram session, the Telegram user's next turns ran as that operator. System, scheduled, automation, and background-continuation turns keep the session's identity. +- Sessions record the account that created them as `ownerAccountId`, and only the owner or an admin can post to an owned session. Every surface refuses other accounts: REST (403), MCP `send_message` (tool error), `/apps/chat` (403), A2A (error event), and pipeline turns including `/ws` (reply). Previously any operator could post into any session whose id it knew. Unowned sessions (channels, cron, sessions created before this change) stay open and are never claimed by writing to them. Reading is unchanged. `GET /api/integration/sessions?owner=me` lists the caller's own sessions. - 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. It also checks the account's role before connecting: a gateway-reported role below `operator` gets an explanation instead of a chat connection, and the read-only status views stay available. - 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 10ee10da..714d537c 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -253,6 +253,23 @@ Open loopback and bootstrap callers have no account, so their turns run without A pipeline turn from an external sender without an account runs without one too. It does not inherit the account of whoever wrote to the session before. For example, a Telegram user's turn never runs as an operator who posted into that Telegram session. System, scheduled, automation, and background-continuation turns act on the session's behalf and keep its identity. +### 3.6 Session Ownership + +A session created by a signed-in account records it as `Session.OwnerAccountId`. The owner is set once, at creation, and never reassigned. + +| Session | Who can post to it | +|---------|--------------------| +| Owned | The owner and admins. Other accounts are refused on every surface: REST returns 403, MCP returns a tool error, `/apps/chat` returns 403, A2A returns an error event, and pipeline turns (including `/ws`) get a reply saying the conversation belongs to another account | +| Unowned (created by channels, cron, bootstrap or loopback callers, or before ownership existed) | Anyone allowed to post. Writing to an unowned session never claims it | + +Callers without an account (bootstrap, open loopback, channel and system turns) are not restricted by ownership. They are admin-equivalent, or they address sessions by their own keys. + +Reading stays open to every role that can read sessions, so dashboards and audit are unaffected. + +`GET /api/integration/sessions?owner=me` lists only the caller's own sessions, active and persisted. `SessionSummary.ownerAccountId` carries the owner. Callers without an account own no sessions. + +`/v1/*` sessions are already scoped to the calling credential. They record the owner but need no extra check. + ## 4. Middleware Pipeline Authentication middleware is registered in `Program.cs` in the following order: diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index a713e782..aff1a770 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -253,6 +253,23 @@ return false; // 401 Unauthorized 来自外部发送者且不带账户的管道轮次同样不带账户身份运行，不会继承此前写入该会话的账户。例如，Telegram 用户的轮次绝不会以曾向该 Telegram 会话发消息的操作员身份运行。系统、定时、自动化和后台续跑轮次代表会话执行，保留会话原有身份。 +### 3.6 会话归属 + +由已登录账户创建的会话会把该账户记录为 `Session.OwnerAccountId`。归属只在创建时设置一次，之后不会改变。 + +| 会话 | 谁可以向其发送消息 | +|------|--------------------| +| 有归属 | 所有者和管理员。其他账户在所有入口都会被拒绝：REST 返回 403，MCP 返回工具错误，`/apps/chat` 返回 403，A2A 返回错误事件，管道轮次（包括 `/ws`）会收到“该会话属于另一个账户”的回复 | +| 无归属（由渠道、定时任务、引导令牌或回环调用方创建，或创建于引入归属之前） | 任何被允许发送消息的调用方。向无归属会话写入永远不会占有它 | + +没有账户的调用方（引导令牌、开放回环、渠道和系统轮次）不受归属限制。它们等同于管理员，或者按各自的键访问会话。 + +读取对所有能读取会话的角色保持开放，仪表盘和审计不受影响。 + +`GET /api/integration/sessions?owner=me` 只列出调用方自己的会话（活动和持久化的）。`SessionSummary.ownerAccountId` 给出所有者。没有账户的调用方不拥有任何会话。 + +`/v1/*` 会话已经按调用凭据隔离，会记录所有者，但不需要额外检查。 + ## 四、中间件管道 认证相关的中间件在 `Program.cs` 中按以下顺序注册： diff --git a/src/OpenClaw.Channels/WebSocketChannel.cs b/src/OpenClaw.Channels/WebSocketChannel.cs index 75e0d596..71b755af 100644 --- a/src/OpenClaw.Channels/WebSocketChannel.cs +++ b/src/OpenClaw.Channels/WebSocketChannel.cs @@ -78,7 +78,8 @@ public async Task HandleConnectionAsync( string clientId, IPAddress? remoteIp, CancellationToken ct, - string? authenticatedUserId = null) + string? authenticatedUserId = null, + bool authenticatedUserIsAdmin = false) { if (!TryAddConnection(clientId, ws, remoteIp, out var state)) { @@ -132,6 +133,7 @@ await SendEnvelopeToStateAsync( SenderId = clientId, RequestCancellation = ct, AuthenticatedUserId = authenticatedUserId, + AuthenticatedUserIsAdmin = authenticatedUserIsAdmin, SessionId = parsed.SessionId, Type = parsed.Type, Text = parsed.Text ?? "", diff --git a/src/OpenClaw.Core/Memory/FileMemoryStore.cs b/src/OpenClaw.Core/Memory/FileMemoryStore.cs index 1c5a1bf1..199117bc 100644 --- a/src/OpenClaw.Core/Memory/FileMemoryStore.cs +++ b/src/OpenClaw.Core/Memory/FileMemoryStore.cs @@ -1193,6 +1193,10 @@ public async ValueTask ListSessionsAsync( if (query.State is { } state && session.State != state) continue; + if (!string.IsNullOrEmpty(query.OwnerAccountId) && + !string.Equals(session.OwnerAccountId, query.OwnerAccountId, StringComparison.Ordinal)) + continue; + if (!string.IsNullOrEmpty(query.Search)) { var s = query.Search; @@ -1224,7 +1228,8 @@ public async ValueTask ListSessionsAsync( RunState = session.RunState, BackgroundRunObjective = session.BackgroundRun?.Objective, BackgroundContinuationCount = session.BackgroundRun?.ContinuationCount ?? 0, - IsActive = false + IsActive = false, + OwnerAccountId = session.OwnerAccountId }); } catch { /* skip corrupt files */ } diff --git a/src/OpenClaw.Core/Memory/SqliteMemoryStore.cs b/src/OpenClaw.Core/Memory/SqliteMemoryStore.cs index ffe9a1de..e11976cc 100644 --- a/src/OpenClaw.Core/Memory/SqliteMemoryStore.cs +++ b/src/OpenClaw.Core/Memory/SqliteMemoryStore.cs @@ -1218,6 +1218,8 @@ public async ValueTask ListSessionsAsync( where.Append(" AND (json_extract(json,'$.state') = $stateInt OR json_extract(json,'$.state') = $stateText)"); if (!string.IsNullOrEmpty(query.Search)) where.Append(" AND (id LIKE $search OR json_extract(json,'$.channelId') LIKE $search OR json_extract(json,'$.senderId') LIKE $search)"); + if (!string.IsNullOrEmpty(query.OwnerAccountId)) + where.Append(" AND json_extract(json,'$.ownerAccountId') = $ownerAccountId"); await using var countCmd = conn.CreateCommand(); countCmd.CommandText = $"SELECT COUNT(*) FROM sessions {where}"; @@ -1231,6 +1233,7 @@ public async ValueTask ListSessionsAsync( countCmd.Parameters.AddWithValue("$stateText", query.State.Value.ToString()); } if (!string.IsNullOrEmpty(query.Search)) countCmd.Parameters.AddWithValue("$search", $"%{query.Search}%"); + if (!string.IsNullOrEmpty(query.OwnerAccountId)) countCmd.Parameters.AddWithValue("$ownerAccountId", query.OwnerAccountId); var total = Convert.ToInt32(await countCmd.ExecuteScalarAsync(ct) ?? 0); var skip = (page - 1) * pageSize; @@ -1251,6 +1254,7 @@ ORDER BY json_extract(json,'$.lastActiveAt') DESC, id ASC cmd.Parameters.AddWithValue("$stateText", query.State.Value.ToString()); } if (!string.IsNullOrEmpty(query.Search)) cmd.Parameters.AddWithValue("$search", $"%{query.Search}%"); + if (!string.IsNullOrEmpty(query.OwnerAccountId)) cmd.Parameters.AddWithValue("$ownerAccountId", query.OwnerAccountId); cmd.Parameters.AddWithValue("$limit", pageSize); cmd.Parameters.AddWithValue("$offset", skip); @@ -1279,7 +1283,8 @@ ORDER BY json_extract(json,'$.lastActiveAt') DESC, id ASC RunState = session.RunState, BackgroundRunObjective = session.BackgroundRun?.Objective, BackgroundContinuationCount = session.BackgroundRun?.ContinuationCount ?? 0, - IsActive = false + IsActive = false, + OwnerAccountId = session.OwnerAccountId }); } diff --git a/src/OpenClaw.Core/Models/Messages.cs b/src/OpenClaw.Core/Models/Messages.cs index 2323c033..2169f545 100644 --- a/src/OpenClaw.Core/Models/Messages.cs +++ b/src/OpenClaw.Core/Models/Messages.cs @@ -54,6 +54,9 @@ public sealed record InboundMessage /// public string? AuthenticatedUserId { get; init; } + /// True when holds the admin role, which may write to any owned session. + public bool AuthenticatedUserIsAdmin { get; init; } + /// /// Multiple media attachments (e.g. several images in one message). /// When present, each attachment generates its own marker line in the pipeline text. diff --git a/src/OpenClaw.Core/Models/Session.cs b/src/OpenClaw.Core/Models/Session.cs index c058ef7b..bf75c3ad 100644 --- a/src/OpenClaw.Core/Models/Session.cs +++ b/src/OpenClaw.Core/Models/Session.cs @@ -35,6 +35,12 @@ public sealed class Session /// public string? AuthenticatedUserId { get; set; } + /// + /// Account that created the session, set once at creation and never reassigned. Owned sessions accept turns + /// only from the owner or an admin; sessions created without an account (channels, cron, older data) stay unowned. + /// + public string? OwnerAccountId { get; set; } + public StableSessionBindingInfo? StableSessionBinding { get; set; } public DateTimeOffset CreatedAt { get; init; } = DateTimeOffset.UtcNow; public DateTimeOffset LastActiveAt { get; set; } = DateTimeOffset.UtcNow; diff --git a/src/OpenClaw.Core/Models/SessionAdminModels.cs b/src/OpenClaw.Core/Models/SessionAdminModels.cs index e1fd8539..96d72892 100644 --- a/src/OpenClaw.Core/Models/SessionAdminModels.cs +++ b/src/OpenClaw.Core/Models/SessionAdminModels.cs @@ -20,6 +20,7 @@ public sealed class SessionSummary public string? BackgroundRunObjective { get; init; } public int BackgroundContinuationCount { get; init; } public bool IsActive { get; init; } + public string? OwnerAccountId { get; init; } } public sealed class PagedSessionList @@ -41,4 +42,5 @@ public sealed class SessionListQuery public SessionState? State { get; init; } public bool? Starred { get; init; } public string? Tag { get; init; } + public string? OwnerAccountId { get; init; } } diff --git a/src/OpenClaw.Core/Sessions/SessionAccess.cs b/src/OpenClaw.Core/Sessions/SessionAccess.cs new file mode 100644 index 00000000..ab12797a --- /dev/null +++ b/src/OpenClaw.Core/Sessions/SessionAccess.cs @@ -0,0 +1,20 @@ +using OpenClaw.Core.Models; + +namespace OpenClaw.Core.Sessions; + +/// +/// Write access to a session. Owned sessions accept turns from the owner or an admin. Unowned sessions (created +/// by channels, cron, or before ownership existed) stay open, and writing to one never claims it. Callers without +/// an account are admin-equivalent credentials (bootstrap, open loopback) or channel and system flows, which +/// address sessions by their own keys. +/// +public static class SessionAccess +{ + public const string DeniedMessage = "This conversation belongs to another account."; + + public static bool CanWrite(Session session, string? accountId, bool isAdmin) + => string.IsNullOrWhiteSpace(session.OwnerAccountId) + || string.IsNullOrWhiteSpace(accountId) + || isAdmin + || string.Equals(session.OwnerAccountId, accountId, StringComparison.Ordinal); +} diff --git a/src/OpenClaw.Core/Sessions/SessionManager.cs b/src/OpenClaw.Core/Sessions/SessionManager.cs index aa824e1d..5a5aaaf4 100644 --- a/src/OpenClaw.Core/Sessions/SessionManager.cs +++ b/src/OpenClaw.Core/Sessions/SessionManager.cs @@ -45,17 +45,17 @@ public SessionManager(IMemoryStore store, GatewayConfig config, ILogger? logger /// Get or create a session for the given channel+sender pair. /// Session key is deterministic: channelId:senderId /// - public async ValueTask GetOrCreateAsync(string channelId, string senderId, CancellationToken ct) + public async ValueTask GetOrCreateAsync(string channelId, string senderId, CancellationToken ct, string? ownerAccountId = null) { var key = string.Concat(channelId, ":", senderId); - return await GetOrCreateByIdAsync(key, channelId, senderId, ct); + return await GetOrCreateByIdAsync(key, channelId, senderId, ct, ownerAccountId); } /// /// Get or create a session for an explicit session id. Useful for cron jobs and webhooks /// that want stable, named sessions independent of channel+sender. /// - public async ValueTask GetOrCreateByIdAsync(string sessionId, string channelId, string senderId, CancellationToken ct) + public async ValueTask GetOrCreateByIdAsync(string sessionId, string channelId, string senderId, CancellationToken ct, string? ownerAccountId = null) { if (string.IsNullOrWhiteSpace(sessionId)) throw new ArgumentException("sessionId must be set.", nameof(sessionId)); @@ -109,6 +109,7 @@ public async ValueTask GetOrCreateByIdAsync(string sessionId, string ch Id = key, ChannelId = channelId, SenderId = senderId, + OwnerAccountId = string.IsNullOrWhiteSpace(ownerAccountId) ? null : ownerAccountId, LastActiveAt = now }; diff --git a/src/OpenClaw.Gateway/A2A/A2ACallerContext.cs b/src/OpenClaw.Gateway/A2A/A2ACallerContext.cs index b300c206..d0a99574 100644 --- a/src/OpenClaw.Gateway/A2A/A2ACallerContext.cs +++ b/src/OpenClaw.Gateway/A2A/A2ACallerContext.cs @@ -1,16 +1,24 @@ namespace OpenClaw.Gateway.A2A; /// -/// The signed-in account behind the current A2A request. The A2A middleware sets it before the SDK runs the +/// The signed-in caller behind the current A2A request. The A2A middleware sets it before the SDK runs the /// handler; unlike the HTTP context, an async-local value still flows into work the SDK detaches from the request. /// internal static class A2ACallerContext { - private static readonly AsyncLocal Current = new(); + private static readonly AsyncLocal CurrentAccountId = new(); + private static readonly AsyncLocal CurrentIsAdmin = new(); public static string? AccountId { - get => Current.Value; - set => Current.Value = value; + get => CurrentAccountId.Value; + set => CurrentAccountId.Value = value; + } + + /// Whether the caller holds the admin role, which may write to sessions other accounts own. + public static bool IsAdmin + { + get => CurrentIsAdmin.Value; + set => CurrentIsAdmin.Value = value; } } diff --git a/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs b/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs index e4488471..ff674dce 100644 --- a/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs +++ b/src/OpenClaw.Gateway/A2A/A2AEndpointExtensions.cs @@ -80,7 +80,9 @@ public static void UseOpenClawA2AAuth( return; } - A2ACallerContext.AccountId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + var caller = EndpointHelpers.ResolveCaller(ctx, startup); + A2ACallerContext.AccountId = caller.AccountId; + A2ACallerContext.IsAdmin = caller.IsAdmin; if (!runtime.Operations.ActorRateLimits.TryConsume( "ip", diff --git a/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs b/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs index 37315782..8692dbc3 100644 --- a/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs +++ b/src/OpenClaw.Gateway/A2A/OpenClawA2AExecutionBridge.cs @@ -1,5 +1,6 @@ using OpenClaw.Core.Middleware; using OpenClaw.Core.Models; +using OpenClaw.Core.Sessions; using OpenClaw.Gateway.Mcp; using OpenClaw.MicrosoftAgentFrameworkAdapter.A2A; @@ -28,10 +29,19 @@ public async Task ExecuteStreamingAsync( request.SessionId, request.ChannelId, request.SenderId, - cancellationToken); + cancellationToken, + A2ACallerContext.AccountId); await using var sessionLock = await runtime.SessionManager.AcquireSessionLockAsync(session.Id, cancellationToken); + // A2A task ids can name an existing session, so apply the same ownership rule as the other surfaces. + if (!SessionAccess.CanWrite(session, A2ACallerContext.AccountId, A2ACallerContext.IsAdmin)) + { + await onEvent(AgentStreamEvent.ErrorOccurred(SessionAccess.DeniedMessage), cancellationToken); + await onEvent(AgentStreamEvent.Complete(), cancellationToken); + return; + } + // The A2A request's SenderId is the caller's contextId, so the turn's identity comes from the signed-in account. session.AuthenticatedUserId = A2ACallerContext.AccountId; diff --git a/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs b/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs index a780593a..2beaa36b 100644 --- a/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs +++ b/src/OpenClaw.Gateway/Composition/IntegrationApiFacade.cs @@ -3,6 +3,7 @@ using OpenClaw.Core.Compatibility; using OpenClaw.Core.Abstractions; using OpenClaw.Core.Models; +using OpenClaw.Core.Sessions; using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Workflows; @@ -134,7 +135,8 @@ public async Task ListSessionsAsync(int page, int p TotalOutputTokens = session.TotalOutputTokens, TotalCacheReadTokens = session.TotalCacheReadTokens, TotalCacheWriteTokens = session.TotalCacheWriteTokens, - IsActive = true + IsActive = true, + OwnerAccountId = session.OwnerAccountId }) .ToArray(); @@ -291,7 +293,8 @@ public async Task GetOperatorDashboardAsync(Reliabili TotalOutputTokens = session.TotalOutputTokens, TotalCacheReadTokens = session.TotalCacheReadTokens, TotalCacheWriteTokens = session.TotalCacheWriteTokens, - IsActive = true + IsActive = true, + OwnerAccountId = session.OwnerAccountId }) .ToArray(); @@ -794,13 +797,10 @@ public async Task ListLearningProposalsAsync(strin public async Task QueueMessageAsync( IntegrationMessageRequest request, CancellationToken cancellationToken, - string? authenticatedUserId = null) + string? authenticatedUserId = null, + bool authenticatedUserIsAdmin = false) { - var effectiveChannelId = string.IsNullOrWhiteSpace(request.ChannelId) ? "integration-api" : request.ChannelId.Trim(); - var effectiveSenderId = string.IsNullOrWhiteSpace(request.SenderId) ? "http-client" : request.SenderId.Trim(); - var effectiveSessionId = string.IsNullOrWhiteSpace(request.SessionId) - ? $"{effectiveChannelId}:{effectiveSenderId}" - : request.SessionId.Trim(); + var (effectiveChannelId, effectiveSenderId, effectiveSessionId) = ResolveTarget(request); await _runtime.RecentSenders.RecordAsync(effectiveChannelId, effectiveSenderId, senderName: null, cancellationToken); @@ -813,7 +813,8 @@ public async Task QueueMessageAsync( Text = request.Text, MessageId = request.MessageId, ReplyToMessageId = request.ReplyToMessageId, - AuthenticatedUserId = authenticatedUserId + AuthenticatedUserId = authenticatedUserId, + AuthenticatedUserIsAdmin = authenticatedUserIsAdmin }; if (!_runtime.Pipeline.InboundWriter.TryWrite(message)) @@ -946,6 +947,24 @@ private static UserProfile NormalizeProfile(string actorId, UserProfile profile) }; } + /// + /// Whether the caller may post into the session a message request targets. Checked before queueing so the + /// caller gets a synchronous refusal; the worker enforces the same rule when the turn runs. + /// + public async Task CanWriteSessionAsync(IntegrationMessageRequest request, string? accountId, bool isAdmin, CancellationToken cancellationToken) + { + var session = await _runtime.SessionManager.LoadAsync(ResolveTarget(request).SessionId, cancellationToken); + return session is null || SessionAccess.CanWrite(session, accountId, isAdmin); + } + + private static (string ChannelId, string SenderId, string SessionId) ResolveTarget(IntegrationMessageRequest request) + { + var channelId = string.IsNullOrWhiteSpace(request.ChannelId) ? "integration-api" : request.ChannelId.Trim(); + var senderId = string.IsNullOrWhiteSpace(request.SenderId) ? "http-client" : request.SenderId.Trim(); + var sessionId = string.IsNullOrWhiteSpace(request.SessionId) ? $"{channelId}:{senderId}" : request.SessionId.Trim(); + return (channelId, senderId, sessionId); + } + public static SessionListQuery BuildSessionQuery( string? search, string? channelId, @@ -954,7 +973,8 @@ public static SessionListQuery BuildSessionQuery( DateTimeOffset? toUtc, string? state, bool? starred, - string? tag) + string? tag, + string? ownerAccountId = null) { return new SessionListQuery { @@ -965,7 +985,8 @@ public static SessionListQuery BuildSessionQuery( ToUtc = toUtc, State = ParseSessionState(state), Starred = starred, - Tag = string.IsNullOrWhiteSpace(tag) ? null : tag.Trim() + Tag = string.IsNullOrWhiteSpace(tag) ? null : tag.Trim(), + OwnerAccountId = string.IsNullOrWhiteSpace(ownerAccountId) ? null : ownerAccountId }; } diff --git a/src/OpenClaw.Gateway/Composition/SessionAdminListing.cs b/src/OpenClaw.Gateway/Composition/SessionAdminListing.cs index f563a313..33ec0bf2 100644 --- a/src/OpenClaw.Gateway/Composition/SessionAdminListing.cs +++ b/src/OpenClaw.Gateway/Composition/SessionAdminListing.cs @@ -27,6 +27,10 @@ public static bool MatchesSessionQuery( if (query.State is { } state && session.State != state) return false; + if (!string.IsNullOrWhiteSpace(query.OwnerAccountId) && + !string.Equals(session.OwnerAccountId, query.OwnerAccountId, StringComparison.Ordinal)) + return false; + var metadata = metadataById.TryGetValue(session.Id, out var storedMetadata) ? storedMetadata : new SessionMetadataSnapshot { SessionId = session.Id, Starred = false, Tags = [] }; @@ -77,6 +81,10 @@ public static bool MatchesSummaryQuery( if (query.State is { } state && summary.State != state) return false; + if (!string.IsNullOrWhiteSpace(query.OwnerAccountId) && + !string.Equals(summary.OwnerAccountId, query.OwnerAccountId, StringComparison.Ordinal)) + return false; + var metadata = metadataById.TryGetValue(summary.Id, out var storedMetadata) ? storedMetadata : new SessionMetadataSnapshot { SessionId = summary.Id, Starred = false, Tags = [] }; diff --git a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs index 331eb6a9..8039fb1d 100644 --- a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs +++ b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs @@ -86,7 +86,8 @@ private static void MapSessionEndpoints(WebApplication app, AdminEndpointService RunState = session.RunState, BackgroundRunObjective = session.BackgroundRun?.Objective, BackgroundContinuationCount = session.BackgroundRun?.ContinuationCount ?? 0, - IsActive = true + IsActive = true, + OwnerAccountId = session.OwnerAccountId }) .ToArray(); diff --git a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs index d7dfbf73..24615761 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs @@ -1,6 +1,7 @@ using System.Text; using System.Text.Json.Nodes; using OpenClaw.Core.Models; +using OpenClaw.Core.Sessions; using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Composition; using OpenClaw.McpApp; @@ -81,8 +82,15 @@ public static void MapOpenClawAppsEndpoints( var sessionId = string.IsNullOrWhiteSpace(requestedSessionId) ? $"apps-{Guid.NewGuid():N}" : requestedSessionId!; - var session = await runtime.SessionManager.GetOrCreateByIdAsync(sessionId, "apps", sessionId, ct); - session.AuthenticatedUserId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + var caller = EndpointHelpers.ResolveCaller(ctx, startup); + var session = await runtime.SessionManager.GetOrCreateByIdAsync(sessionId, "apps", sessionId, ct, caller.AccountId); + if (!SessionAccess.CanWrite(session, caller.AccountId, caller.IsAdmin)) + { + await EndpointHelpers.WriteForbiddenAsync(ctx, SessionAccess.DeniedMessage); + return; + } + + session.AuthenticatedUserId = caller.AccountId; await SendAsync(ctx, new JsonObject { ["type"] = "session", ["sessionId"] = sessionId }, ct); diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 199c4341..9051ab27 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -405,19 +405,36 @@ public static bool CanExecuteAgent( /// Null for open loopback and bootstrap callers, which have no account and are admin-equivalent. /// public static string? ResolveAuthenticatedAccountId(HttpContext ctx, GatewayStartupContext startup) + => ResolveCaller(ctx, startup).AccountId; + + internal readonly record struct CallerAccount(string? AccountId, bool IsAdmin); + + /// + /// The signed-in account behind a request and whether it holds the admin role, which may write to sessions + /// owned by other accounts. Open loopback and bootstrap callers have no account and count as admin. + /// + internal static CallerAccount ResolveCaller(HttpContext ctx, GatewayStartupContext startup) { var browserSessions = ctx.RequestServices.GetRequiredService(); var auth = AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); - return auth.IsAuthorized && !string.IsNullOrWhiteSpace(auth.AccountId) ? auth.AccountId : null; + if (!auth.IsAuthorized) + return default; + + return new CallerAccount( + string.IsNullOrWhiteSpace(auth.AccountId) ? null : auth.AccountId, + auth.IsBootstrapAdmin || OperatorRoleNames.CanAccess(auth.Role, OperatorRoleNames.Admin)); } - public static async Task WriteOperatorRoleRequiredAsync(HttpContext ctx) + public static Task WriteOperatorRoleRequiredAsync(HttpContext ctx) + => WriteForbiddenAsync(ctx, OperatorRoleRequiredMessage); + + public static async Task WriteForbiddenAsync(HttpContext ctx, string error) { ctx.Response.StatusCode = StatusCodes.Status403Forbidden; ctx.Response.ContentType = "application/json"; await JsonSerializer.SerializeAsync( ctx.Response.Body, - new OperationStatusResponse { Success = false, Error = OperatorRoleRequiredMessage }, + new OperationStatusResponse { Success = false, Error = error }, CoreJsonContext.Default.OperationStatusResponse, ctx.RequestAborted); } diff --git a/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs index 747a028e..813e6ebf 100644 --- a/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/IntegrationEndpoints.cs @@ -1,5 +1,6 @@ using OpenClaw.Core.Abstractions; using OpenClaw.Core.Models; +using OpenClaw.Core.Sessions; using System.Text.Json; using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Composition; @@ -185,7 +186,24 @@ await facade.GetDashboardAsync(ctx.RequestAborted), var starred = GetQueryBool(ctx, "starred"); var tag = GetOptionalQueryString(ctx, "tag"); - var query = IntegrationApiFacade.BuildSessionQuery(search, channelId, senderId, fromUtc, toUtc, state, starred, tag); + // owner=me lists the caller's own sessions; callers without an account (bootstrap, open loopback) own none. + string? ownerAccountId = null; + if (string.Equals(GetOptionalQueryString(ctx, "owner"), "me", StringComparison.OrdinalIgnoreCase)) + { + ownerAccountId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + if (ownerAccountId is null) + { + return Results.Json( + new IntegrationSessionsResponse + { + Filters = new SessionListQuery(), + Persisted = new PagedSessionList { Page = page, PageSize = pageSize, HasMore = false, Items = [] } + }, + CoreJsonContext.Default.IntegrationSessionsResponse); + } + } + + var query = IntegrationApiFacade.BuildSessionQuery(search, channelId, senderId, fromUtc, toUtc, state, starred, tag, ownerAccountId); return Results.Json( await facade.ListSessionsAsync(page, pageSize, query, ctx.RequestAborted), CoreJsonContext.Default.IntegrationSessionsResponse); @@ -848,8 +866,17 @@ await runtime.PaymentRuntime.GetPaymentStatusAsync(id, provider, BuildPaymentCon statusCode: StatusCodes.Status400BadRequest); } + var caller = EndpointHelpers.ResolveCaller(ctx, startup); + if (!await facade.CanWriteSessionAsync(request, caller.AccountId, caller.IsAdmin, ctx.RequestAborted)) + { + return Results.Json( + new OperationStatusResponse { Success = false, Error = SessionAccess.DeniedMessage }, + CoreJsonContext.Default.OperationStatusResponse, + statusCode: StatusCodes.Status403Forbidden); + } + return Results.Json( - await facade.QueueMessageAsync(request, ctx.RequestAborted, EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup)), + await facade.QueueMessageAsync(request, ctx.RequestAborted, caller.AccountId, caller.IsAdmin), CoreJsonContext.Default.IntegrationMessageResponse, statusCode: StatusCodes.Status202Accepted); }); diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs index 2decf367..574eb92d 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs @@ -76,9 +76,10 @@ private static void MapChatCompletionsEndpoint( ? CreateStableSessionBinding(stableSessionId!, requesterKey) : null; var requestId = $"oai-http:{Guid.NewGuid():N}"; + var accountId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); var session = stableBinding is not null - ? await runtime.SessionManager.GetOrCreateByIdAsync(BuildScopedStableSessionId(stableBinding), "openai-http", requesterKey, ctx.RequestAborted) - : await runtime.SessionManager.GetOrCreateAsync("openai-http", requestId, ctx.RequestAborted); + ? await runtime.SessionManager.GetOrCreateByIdAsync(BuildScopedStableSessionId(stableBinding), "openai-http", requesterKey, ctx.RequestAborted, accountId) + : await runtime.SessionManager.GetOrCreateAsync("openai-http", requestId, ctx.RequestAborted, accountId); IAsyncDisposable? stableSessionLock = null; var persistStableSessionOnExit = false; ToolApprovalCallback? approvalCallback = null; @@ -100,7 +101,7 @@ private static void MapChatCompletionsEndpoint( // The turn runs as the signed-in account: it scopes per-user capability bindings and is the // userId MCP servers see. Callers without an account (open loopback, bootstrap) run without one. - session.AuthenticatedUserId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + session.AuthenticatedUserId = accountId; var httpMwCtx = new MessageContext { diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs index 893500ec..ef51b0a5 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs @@ -65,9 +65,10 @@ private static void MapResponsesEndpoint( ? CreateStableSessionBinding(stableSessionId!, requesterKey) : null; var requestId = $"oai-resp:{Guid.NewGuid():N}"; + var accountId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); var session = stableBinding is not null - ? await runtime.SessionManager.GetOrCreateByIdAsync(BuildScopedStableSessionId(stableBinding), "openai-responses", requesterKey, ctx.RequestAborted) - : await runtime.SessionManager.GetOrCreateAsync("openai-responses", requestId, ctx.RequestAborted); + ? await runtime.SessionManager.GetOrCreateByIdAsync(BuildScopedStableSessionId(stableBinding), "openai-responses", requesterKey, ctx.RequestAborted, accountId) + : await runtime.SessionManager.GetOrCreateAsync("openai-responses", requestId, ctx.RequestAborted, accountId); IAsyncDisposable? stableSessionLock = null; var persistStableSessionOnExit = false; ToolApprovalCallback? approvalCallback = null; @@ -89,7 +90,7 @@ private static void MapResponsesEndpoint( // The turn runs as the signed-in account: it scopes per-user capability bindings and is the // userId MCP servers see. Callers without an account (open loopback, bootstrap) run without one. - session.AuthenticatedUserId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + session.AuthenticatedUserId = accountId; var httpMwCtx = new MessageContext { diff --git a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs index 7b3fbc1b..a4b99d63 100644 --- a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs @@ -34,7 +34,8 @@ public static void MapOpenClawWebSocketEndpoints( var clientId = ctx.Connection.Id; TryResolveAuthorizedUserIdForWebSocket(ctx, startup, out var userId); - await runtime.WebSocketChannel.HandleConnectionAsync(ws, clientId, ctx.Connection.RemoteIpAddress, ctx.RequestAborted, userId); + var isAdmin = EndpointHelpers.ResolveCaller(ctx, startup).IsAdmin; + await runtime.WebSocketChannel.HandleConnectionAsync(ws, clientId, ctx.Connection.RemoteIpAddress, ctx.RequestAborted, userId, isAdmin); }); app.Map("/ws/live", async (HttpContext ctx) => diff --git a/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs b/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs index a78820fa..fc41b3f3 100644 --- a/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs +++ b/src/OpenClaw.Gateway/Extensions/GatewayInboundMessageWorker.cs @@ -313,11 +313,24 @@ await pipeline.OutboundWriter.WriteAsync(new OutboundMessage var resolvedRoute = routeResolver?.Resolve(msg.ChannelId, msg.SenderId); session = msg.SessionId is not null - ? await sessionManager.GetOrCreateByIdAsync(msg.SessionId, msg.ChannelId, conversationRecipientId, lifetime.ApplicationStopping) - : await sessionManager.GetOrCreateAsync(msg.ChannelId, conversationRecipientId, lifetime.ApplicationStopping); + ? await sessionManager.GetOrCreateByIdAsync(msg.SessionId, msg.ChannelId, conversationRecipientId, lifetime.ApplicationStopping, msg.AuthenticatedUserId) + : await sessionManager.GetOrCreateAsync(msg.ChannelId, conversationRecipientId, lifetime.ApplicationStopping, msg.AuthenticatedUserId); if (session is null) throw new InvalidOperationException("Session manager returned null session."); + if (!SessionAccess.CanWrite(session, msg.AuthenticatedUserId, msg.AuthenticatedUserIsAdmin)) + { + await pipeline.OutboundWriter.WriteAsync(new OutboundMessage + { + ChannelId = msg.ChannelId, + RecipientId = conversationRecipientId, + AccountId = msg.AccountId, + Text = SessionAccess.DeniedMessage, + ReplyToMessageId = msg.MessageId + }, lifetime.ApplicationStopping); + continue; + } + // Abort intercept: handle /stop, /cancel, /abort BEFORE acquiring the session lock // so the abort does not wait for the in-flight execution to release the lock. if (!msg.IsSystem && abortRegistry is not null && IsAbortCommand(msg.Text)) diff --git a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs index 81efc048..c92208bd 100644 --- a/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs +++ b/src/OpenClaw.Gateway/Mcp/OpenClawMcpTools.cs @@ -3,6 +3,7 @@ using ModelContextProtocol; using ModelContextProtocol.Server; using OpenClaw.Core.Models; +using OpenClaw.Core.Sessions; using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Composition; using OpenClaw.Gateway.Endpoints; @@ -305,17 +306,21 @@ public async Task SendMessage( [Description("Optional reply-to message ID.")] string? replyToMessageId = null, CancellationToken ct = default) { - var caller = RequireOperator("openclaw.send_message"); + var caller = EndpointHelpers.ResolveCaller(RequireOperator("openclaw.send_message"), _startup); + var request = new IntegrationMessageRequest + { + Text = text, + ChannelId = channelId, + SenderId = senderId, + SessionId = sessionId, + MessageId = messageId, + ReplyToMessageId = replyToMessageId + }; + if (!await _facade.CanWriteSessionAsync(request, caller.AccountId, caller.IsAdmin, ct)) + throw new McpException(SessionAccess.DeniedMessage); + return JsonSerializer.Serialize( - await _facade.QueueMessageAsync(new IntegrationMessageRequest - { - Text = text, - ChannelId = channelId, - SenderId = senderId, - SessionId = sessionId, - MessageId = messageId, - ReplyToMessageId = replyToMessageId - }, ct, EndpointHelpers.ResolveAuthenticatedAccountId(caller, _startup)), + await _facade.QueueMessageAsync(request, ct, caller.AccountId, caller.IsAdmin), CoreJsonContext.Default.IntegrationMessageResponse); } diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index 1f408f60..90eb4110 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -846,6 +846,147 @@ await bridge.ExecuteStreamingAsync( Assert.Equal("acct-a2a", ran?.AuthenticatedUserId); } + [Fact] + public async Task IntegrationMessages_WhenSessionOwnedByAnotherAccount_ShouldRejectUnlessAdmin() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (_, ownerId) = CreateAccountTokenWithId(harness, "rest-owner", OperatorRoleNames.Operator); + var (otherToken, _) = CreateAccountTokenWithId(harness, "rest-other", OperatorRoleNames.Operator); + var (adminToken, _) = CreateAccountTokenWithId(harness, "rest-admin", OperatorRoleNames.Admin); + await harness.Runtime.SessionManager.GetOrCreateByIdAsync("owned-rest", "agentqi-mobile", "owner", CancellationToken.None, ownerAccountId: ownerId); + + async Task PostAsync(string token) + { + using var request = new HttpRequestMessage(HttpMethod.Post, "/api/integration/messages") + { + Content = JsonContent("""{"text":"hello","sessionId":"owned-rest"}""") + }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", token); + return await harness.Client.SendAsync(request); + } + + var denied = await PostAsync(otherToken); + Assert.Equal(HttpStatusCode.Forbidden, denied.StatusCode); + Assert.False(harness.Runtime.Pipeline.InboundReader.TryRead(out _)); + + var admitted = await PostAsync(adminToken); + Assert.Equal(HttpStatusCode.Accepted, admitted.StatusCode); + Assert.True(harness.Runtime.Pipeline.InboundReader.TryRead(out var queued)); + Assert.True(queued.AuthenticatedUserIsAdmin); + } + + [Fact] + public async Task McpSendMessage_WhenSessionOwnedByAnotherAccount_ShouldReturnToolError() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (_, ownerId) = CreateAccountTokenWithId(harness, "mcp-owner", OperatorRoleNames.Operator); + var (otherToken, _) = CreateAccountTokenWithId(harness, "mcp-other", OperatorRoleNames.Operator); + await harness.Runtime.SessionManager.GetOrCreateByIdAsync("owned-mcp", "agentqi-mobile", "owner", CancellationToken.None, ownerAccountId: ownerId); + + using var result = await CallMcpToolAsync(harness, otherToken, "openclaw.send_message", """{"text":"hello","sessionId":"owned-mcp"}"""); + + Assert.True(result.RootElement.GetProperty("isError").GetBoolean()); + Assert.Contains("another account", result.RootElement.GetProperty("content")[0].GetProperty("text").GetString(), StringComparison.Ordinal); + Assert.False(harness.Runtime.Pipeline.InboundReader.TryRead(out _)); + } + + [Fact] + public async Task AppsChat_WhenSessionOwnedByAnotherAccount_ShouldReturnForbidden() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (_, ownerId) = CreateAccountTokenWithId(harness, "apps-owner", OperatorRoleNames.Operator); + var (otherToken, _) = CreateAccountTokenWithId(harness, "apps-other", OperatorRoleNames.Operator); + await harness.Runtime.SessionManager.GetOrCreateByIdAsync("owned-apps", "apps", "owned-apps", CancellationToken.None, ownerAccountId: ownerId); + + using var request = new HttpRequestMessage(HttpMethod.Post, "/apps/chat") { Content = JsonContent("""{"message":"hello","sessionId":"owned-apps"}""") }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", otherToken); + 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 A2ABridge_WhenSessionOwnedByAnotherAccount_ShouldNotRunAgent() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + await harness.Runtime.SessionManager.GetOrCreateByIdAsync("owned-a2a", "a2a", "ctx", CancellationToken.None, ownerAccountId: "acct-owner"); + var bridge = new OpenClawA2AExecutionBridge(new GatewayRuntimeHolder { Runtime = harness.Runtime }, NullLogger.Instance); + var events = new List(); + + A2ACallerContext.AccountId = "acct-other"; + try + { + await bridge.ExecuteStreamingAsync( + new OpenClaw.MicrosoftAgentFrameworkAdapter.A2A.OpenClawA2AExecutionRequest + { + SessionId = "owned-a2a", + ChannelId = "a2a", + SenderId = "ctx", + UserText = "hello" + }, + (evt, _) => + { + events.Add(evt); + return ValueTask.CompletedTask; + }, + CancellationToken.None); + } + finally + { + A2ACallerContext.AccountId = null; + } + + harness.Runtime.AgentRuntime.DidNotReceiveWithAnyArgs().RunStreamingAsync(default!, default!, default); + Assert.Contains(events, evt => evt.Type == AgentStreamEventType.Error && evt.Content == SessionAccess.DeniedMessage); + } + + [Fact] + public async Task ChatCompletions_WhenAccountToken_ShouldRecordSessionOwner() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (token, accountId) = CreateAccountTokenWithId(harness, "openai-owner", OperatorRoleNames.Operator); + Session? ran = null; + harness.Runtime.AgentRuntime.RunAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(callInfo => + { + ran = callInfo.Arg(); + return Task.FromResult("ok"); + }); + + await SendChatCompletionAsync(harness, token); + + Assert.Equal(accountId, ran?.OwnerAccountId); + } + + [Fact] + public async Task IntegrationSessions_WhenOwnerIsMe_ShouldListOnlyCallerOwnedSessions() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (token, accountId) = CreateAccountTokenWithId(harness, "list-owner", OperatorRoleNames.Viewer); + await harness.Runtime.SessionManager.GetOrCreateByIdAsync("mine-active", "agentqi-mobile", "me", CancellationToken.None, ownerAccountId: accountId); + await harness.Runtime.SessionManager.GetOrCreateByIdAsync("theirs-active", "agentqi-mobile", "them", CancellationToken.None, ownerAccountId: "acct-someone-else"); + await harness.MemoryStore.SaveSessionAsync(new Session { Id = "mine-stored", ChannelId = "agentqi-mobile", SenderId = "me", OwnerAccountId = accountId }, CancellationToken.None); + await harness.MemoryStore.SaveSessionAsync(new Session { Id = "unowned-stored", ChannelId = "telegram", SenderId = "t" }, CancellationToken.None); + + using var request = new HttpRequestMessage(HttpMethod.Get, "/api/integration/sessions?owner=me"); + 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); + var active = payload.RootElement.GetProperty("active").EnumerateArray().Select(static item => item.GetProperty("id").GetString()!).ToArray(); + var persisted = payload.RootElement.GetProperty("persisted").GetProperty("items").EnumerateArray().Select(static item => item.GetProperty("id").GetString()!).ToArray(); + Assert.Equal(["mine-active"], active); + Assert.Equal(["mine-stored"], persisted); + Assert.Equal(accountId, payload.RootElement.GetProperty("active")[0].GetProperty("ownerAccountId").GetString()); + } + private static async IAsyncEnumerable NoAgentEvents() { await Task.CompletedTask; diff --git a/src/OpenClaw.Tests/GatewayWorkersTests.cs b/src/OpenClaw.Tests/GatewayWorkersTests.cs index 0da220f6..e70d5fb6 100644 --- a/src/OpenClaw.Tests/GatewayWorkersTests.cs +++ b/src/OpenClaw.Tests/GatewayWorkersTests.cs @@ -1586,6 +1586,103 @@ await WaitForAsync( Assert.Equal("acct-123", sessionManager.TryGetActive("telegram", "sender-1")?.AuthenticatedUserId); } + [Fact] + public async Task Start_OwnedSession_RecordsCreatorAndRejectsOtherAccountsExceptAdmins() + { + var storagePath = System.IO.Path.Combine(System.IO.Path.GetTempPath(), "openclaw-worker-tests", Guid.NewGuid().ToString("N")); + var store = new FileMemoryStore(storagePath, 4); + var config = new GatewayConfig + { + Memory = new MemoryConfig { StoragePath = storagePath }, + Tooling = new ToolingConfig { EnableBrowserTool = false }, + Channels = new ChannelsConfig { Telegram = new TelegramChannelConfig { DmPolicy = "open" } } + }; + var sessionManager = new SessionManager(store, config, NullLogger.Instance); + var heartbeatService = new HeartbeatService(config, store, sessionManager, NullLogger.Instance); + var pipeline = new MessagePipeline(); + await using var adapter = new RecordingChannelAdapter("telegram"); + var agentRuntime = Substitute.For(); + var turns = 0; + agentRuntime.RunTurnAsync(Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any(), Arg.Any()) + .Returns(_ => + { + Interlocked.Increment(ref turns); + return Task.FromResult(AgentTurnResult.Completed("ok")); + }); + var runtimeMetrics = new OpenClaw.Core.Observability.RuntimeMetrics(); + var providerRegistry = new LlmProviderRegistry(); + var providerPolicies = new ProviderPolicyService(storagePath, NullLogger.Instance); + var runtimeEvents = new RuntimeEventStore(storagePath, NullLogger.Instance); + var operations = new RuntimeOperationsState + { + ProviderPolicies = providerPolicies, + ProviderRegistry = providerRegistry, + LlmExecution = new GatewayLlmExecutionService( + config, + providerRegistry, + providerPolicies, + runtimeEvents, + runtimeMetrics, + new OpenClaw.Core.Observability.ProviderUsageTracker(), + NullLogger.Instance), + PluginHealth = new PluginHealthService(storagePath, NullLogger.Instance), + ApprovalGrants = new ToolApprovalGrantStore(storagePath, NullLogger.Instance), + RuntimeEvents = runtimeEvents, + OperatorAudit = new OperatorAuditStore(storagePath, NullLogger.Instance), + WebhookDeliveries = new WebhookDeliveryStore(storagePath, NullLogger.Instance), + ActorRateLimits = new ActorRateLimitService(storagePath, NullLogger.Instance), + SessionMetadata = new SessionMetadataStore(storagePath, NullLogger.Instance) + }; + + using var lifetime = new TestApplicationLifetime(); + GatewayWorkers.Start( + lifetime, + NullLogger.Instance, + workerCount: 1, + isNonLoopbackBind: false, + sessionManager, + new ConcurrentDictionary(), + new ConcurrentDictionary(), + pipeline, + new MiddlewarePipeline([]), + new WebSocketChannel(config.WebSocket), + agentRuntime, + new Dictionary(StringComparer.Ordinal) { ["telegram"] = adapter }, + config, + cronScheduler: null, + heartbeatService, + new ToolApprovalService(), + new ApprovalAuditStore(storagePath, NullLogger.Instance), + new OpenClaw.Core.Security.PairingManager(storagePath, NullLogger.Instance), + new ChatCommandProcessor(sessionManager), + operations); + + async Task SendAsync(string accountId, bool isAdmin, string messageId) + { + await pipeline.InboundWriter.WriteAsync(new InboundMessage + { + ChannelId = "telegram", + SenderId = "shared-sender", + SessionId = "owned-by-worker", + Text = "hello", + MessageId = messageId, + AuthenticatedUserId = accountId, + AuthenticatedUserIsAdmin = isAdmin + }); + using var timeout = new CancellationTokenSource(TimeSpan.FromSeconds(2)); + return (await adapter.ReadAsync(timeout.Token)).Text; + } + + Assert.Equal("ok", await SendAsync("acct-a", isAdmin: false, "m1")); + Assert.Equal("acct-a", (await sessionManager.LoadAsync("owned-by-worker", CancellationToken.None))!.OwnerAccountId); + + Assert.Equal(SessionAccess.DeniedMessage, await SendAsync("acct-b", isAdmin: false, "m2")); + Assert.Equal(1, Volatile.Read(ref turns)); + + Assert.Equal("ok", await SendAsync("acct-admin", isAdmin: true, "m3")); + Assert.Equal(2, Volatile.Read(ref turns)); + } + private static HeartbeatConfigDto CreateManagedHeartbeatConfig() => new() { diff --git a/src/OpenClaw.Tests/SessionOwnershipTests.cs b/src/OpenClaw.Tests/SessionOwnershipTests.cs new file mode 100644 index 00000000..2265a89a --- /dev/null +++ b/src/OpenClaw.Tests/SessionOwnershipTests.cs @@ -0,0 +1,90 @@ +using Microsoft.Extensions.Logging.Abstractions; +using OpenClaw.Core.Abstractions; +using OpenClaw.Core.Memory; +using OpenClaw.Core.Models; +using OpenClaw.Core.Sessions; +using Xunit; + +namespace OpenClaw.Tests; + +public sealed class SessionOwnershipTests : IDisposable +{ + private readonly string _storagePath = Path.Combine(Path.GetTempPath(), "openclaw-session-ownership-tests", Guid.NewGuid().ToString("N")); + + public void Dispose() + { + try { Directory.Delete(_storagePath, recursive: true); } + catch { } + } + + [Fact] + public async Task GetOrCreateById_WhenCreating_ShouldRecordOwner() + { + var manager = CreateManager(); + + var session = await manager.GetOrCreateByIdAsync("owned", "api", "sender", CancellationToken.None, ownerAccountId: "acct-a"); + + Assert.Equal("acct-a", session.OwnerAccountId); + } + + [Fact] + public async Task GetOrCreateById_WhenSessionExists_ShouldNotReassignOwner() + { + var manager = CreateManager(); + await manager.GetOrCreateByIdAsync("owned", "api", "sender", CancellationToken.None, ownerAccountId: "acct-a"); + + var again = await manager.GetOrCreateByIdAsync("owned", "api", "sender", CancellationToken.None, ownerAccountId: "acct-b"); + + Assert.Equal("acct-a", again.OwnerAccountId); + } + + [Theory] + [InlineData(null, "acct-b", false, true)] // unowned sessions stay open; writing never claims them + [InlineData("acct-a", "acct-a", false, true)] // owner + [InlineData("acct-a", "acct-b", false, false)] // another account + [InlineData("acct-a", "acct-b", true, true)] // admin + [InlineData("acct-a", null, false, true)] // no account: bootstrap, open loopback, channel and system turns + public void CanWrite_ShouldAllowOwnerOrAdminOnOwnedSessions(string? owner, string? accountId, bool isAdmin, bool expected) + { + var session = new Session { Id = "s", ChannelId = "api", SenderId = "sender", OwnerAccountId = owner }; + + Assert.Equal(expected, SessionAccess.CanWrite(session, accountId, isAdmin)); + } + + [Fact] + public async Task FileStore_ListSessions_WhenOwnerFilter_ShouldReturnOnlyOwned() + { + var store = new FileMemoryStore(_storagePath, 4); + await SeedAsync(store); + + var page = await store.ListSessionsAsync(1, 50, new SessionListQuery { OwnerAccountId = "acct-a" }, CancellationToken.None); + + var item = Assert.Single(page.Items); + Assert.Equal("mine", item.Id); + Assert.Equal("acct-a", item.OwnerAccountId); + } + + [Fact] + public async Task SqliteStore_ListSessions_WhenOwnerFilter_ShouldReturnOnlyOwned() + { + Directory.CreateDirectory(_storagePath); + using var store = new SqliteMemoryStore(Path.Combine(_storagePath, "memory.db"), enableFts: false); + await SeedAsync(store); + + var page = await store.ListSessionsAsync(1, 50, new SessionListQuery { OwnerAccountId = "acct-a" }, CancellationToken.None); + + var item = Assert.Single(page.Items); + Assert.Equal("mine", item.Id); + Assert.Equal("acct-a", item.OwnerAccountId); + } + + private SessionManager CreateManager() + => new(new FileMemoryStore(_storagePath, 4), new GatewayConfig { Memory = new MemoryConfig { StoragePath = _storagePath } }, NullLogger.Instance); + + private static async Task SeedAsync(IMemoryStore store) + { + await store.SaveSessionAsync(new Session { Id = "mine", ChannelId = "api", SenderId = "a", OwnerAccountId = "acct-a" }, CancellationToken.None); + await store.SaveSessionAsync(new Session { Id = "theirs", ChannelId = "api", SenderId = "b", OwnerAccountId = "acct-b" }, CancellationToken.None); + await store.SaveSessionAsync(new Session { Id = "unowned", ChannelId = "telegram", SenderId = "c" }, CancellationToken.None); + } +} diff --git a/src/OpenClaw.Tests/WebSocketChannelTests.cs b/src/OpenClaw.Tests/WebSocketChannelTests.cs index eeda344f..11d2bd6b 100644 --- a/src/OpenClaw.Tests/WebSocketChannelTests.cs +++ b/src/OpenClaw.Tests/WebSocketChannelTests.cs @@ -134,6 +134,28 @@ public async Task HandleConnectionAsync_ReassemblesFragmentedMessage() Assert.Equal("hello", received!.Text); } + [Fact] + public async Task HandleConnectionAsync_WhenCallerIsAdmin_ShouldMarkMessagesAsAdmin() + { + var channel = new WebSocketChannel(new WebSocketConfig { MaxMessageBytes = 1024 }); + var ws = new TestWebSocket(); + ws.QueueReceiveText("hello"); + ws.QueueClose(); + + InboundMessage? received = null; + channel.OnMessageReceived += (msg, _) => + { + received = msg; + return ValueTask.CompletedTask; + }; + + await channel.HandleConnectionAsync(ws, "client", IPAddress.Loopback, TestContext.Current.CancellationToken, authenticatedUserId: "acct-admin", authenticatedUserIsAdmin: true); + + Assert.NotNull(received); + Assert.Equal("acct-admin", received!.AuthenticatedUserId); + Assert.True(received.AuthenticatedUserIsAdmin); + } + [Fact] public async Task HandleConnectionAsync_AcceptsLegacyContentEnvelope() { From 9eab8077ee6226648bccb595574e8116d875f4df Mon Sep 17 00:00:00 2001 From: telli Date: Mon, 28 Sep 2026 16:07:11 -0700 Subject: [PATCH 6/8] fix(gateway): run /apps/chat turns one at a time and save them POST /apps/chat was the only turn surface that neither took the session lock nor saved the session afterwards. Two requests for the same sessionId ran concurrently against one history, and a turn was saved only if its session later expired or was evicted, so a restart could lose it. Hold the session lock from the ownership check through the turn, as the A2A bridge does, and save the session when the turn ends, including when the client disconnects mid-turn. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 1 + docs/MCPAPP.md | 2 +- docs/zh-CN/MCPAPP.md | 2 +- .../Endpoints/AppsEndpoints.cs | 8 ++ src/OpenClaw.Tests/AppsEndpointsTests.cs | 78 ++++++++++++++++++- 5 files changed, 85 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 886d7c79..26d4a291 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,6 +50,7 @@ All notable changes to this project are tracked in this file. - Turns now run as the signed-in account instead of a caller-supplied sender id. `Session.AuthenticatedUserId` scopes per-user capability bindings and is passed to MCP servers as `_meta.userId`. Previously only `/ws` set it, so REST messages, MCP `send_message`, A2A, `/v1/*`, and `/apps/chat` turns ran as whatever `senderId` or `contextId` the caller supplied. MCP servers that read `_meta.userId` now receive account ids for those turns. - A turn from an external sender without an account no longer inherits the previous writer's account. Previously, after an operator posted into a Telegram session, the Telegram user's next turns ran as that operator. System, scheduled, automation, and background-continuation turns keep the session's identity. - Sessions record the account that created them as `ownerAccountId`, and only the owner or an admin can post to an owned session. Every surface refuses other accounts: REST (403), MCP `send_message` (tool error), `/apps/chat` (403), A2A (error event), and pipeline turns including `/ws` (reply). Previously any operator could post into any session whose id it knew. Unowned sessions (channels, cron, sessions created before this change) stay open and are never claimed by writing to them. Reading is unchanged. `GET /api/integration/sessions?owner=me` lists the caller's own sessions. +- `POST /apps/chat` now holds the session lock for the whole turn and saves the session afterwards, like the other turn surfaces. Previously two requests for the same `sessionId` ran concurrently against one history, and a turn was saved only if its session later expired or was evicted, so a gateway restart could lose it. - 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. It also checks the account's role before connecting: a gateway-reported role below `operator` gets an explanation instead of a chat connection, and the read-only status views stay available. - 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/MCPAPP.md b/docs/MCPAPP.md index 4ff6f00f..6990db88 100644 --- a/docs/MCPAPP.md +++ b/docs/MCPAPP.md @@ -49,7 +49,7 @@ The `/apps/*` routes use the gateway's normal authentication. They are open with - `/apps/health` returns an `mcp` URL that points back to the gateway's own `/apps/mcp/{appId}` route. - `/apps/mcp/{appId}` forwards `tools/list`, `resources/list`, `resources/read`, and `tools/call` to the App already loaded in `McpAppRegistry`. - When the browser includes `?sessionId=...` on the MCP endpoint URL, OpenClaw injects that value into `tools/call` as `_meta.sessionId` before forwarding upstream. -- `/apps/chat` creates or resumes a gateway session with that same id and streams host-friendly SSE frames back to the browser. +- `/apps/chat` creates or resumes a gateway session with that same id and streams host-friendly SSE frames back to the browser. Turns on one session run one at a time: a second request for the same id waits until the first turn finishes. The session is saved after each turn. This is the bridge that lets a rich MCP App UI and the Agent collaborate against the same App session instead of creating two unrelated MCP connections. diff --git a/docs/zh-CN/MCPAPP.md b/docs/zh-CN/MCPAPP.md index 016b9dbb..024f9991 100644 --- a/docs/zh-CN/MCPAPP.md +++ b/docs/zh-CN/MCPAPP.md @@ -49,7 +49,7 @@ OpenClaw.NET 为浏览器侧 MCP App UI 暴露了一组面向 gateway 的 host - `/apps/health` 返回的 `mcp` 字段指向 gateway 自己的 `/apps/mcp/{appId}` 路由。 - `/apps/mcp/{appId}` 会把 `tools/list`、`resources/list`、`resources/read`、`tools/call` 转发给 `McpAppRegistry` 中已加载的 App。 - 如果浏览器在 MCP 端点 URL 上带了 `?sessionId=...`，OpenClaw 会在转发 `tools/call` 前把它注入到 `_meta.sessionId`。 -- `/apps/chat` 会用同一个 session id 创建或恢复 gateway 会话，并把 host 友好的 SSE 事件流回浏览器。 +- `/apps/chat` 会用同一个 session id 创建或恢复 gateway 会话，并把 host 友好的 SSE 事件流回浏览器。同一会话的轮次依次执行：对同一 id 的第二个请求会等待第一轮结束。每轮结束后会保存会话。 这就是交互式 MCP App UI 和 Agent 能够围绕同一个 App 会话协作的桥梁，而不是各自单独建一条不相关的 MCP 连接。 diff --git a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs index 24615761..987a4689 100644 --- a/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/AppsEndpoints.cs @@ -84,6 +84,9 @@ public static void MapOpenClawAppsEndpoints( : requestedSessionId!; var caller = EndpointHelpers.ResolveCaller(ctx, startup); var session = await runtime.SessionManager.GetOrCreateByIdAsync(sessionId, "apps", sessionId, ct, caller.AccountId); + + // One turn at a time per session, as on the other surfaces: a second request waits for the first to finish. + await using var sessionLock = await runtime.SessionManager.AcquireSessionLockAsync(session.Id, ct); if (!SessionAccess.CanWrite(session, caller.AccountId, caller.IsAdmin)) { await EndpointHelpers.WriteForbiddenAsync(ctx, SessionAccess.DeniedMessage); @@ -129,6 +132,11 @@ public static void MapOpenClawAppsEndpoints( { await SendAsync(ctx, new JsonObject { ["type"] = "error", ["error"] = ex.Message }, ct); } + finally + { + // Saved even when the client disconnects mid-turn, so the history the turn produced is kept. + await runtime.SessionManager.PersistAsync(session, CancellationToken.None, sessionLockHeld: true); + } }); } diff --git a/src/OpenClaw.Tests/AppsEndpointsTests.cs b/src/OpenClaw.Tests/AppsEndpointsTests.cs index 6e687d49..b5ad45d1 100644 --- a/src/OpenClaw.Tests/AppsEndpointsTests.cs +++ b/src/OpenClaw.Tests/AppsEndpointsTests.cs @@ -105,6 +105,76 @@ public async Task Chat_StreamsEvents_AndPassesUiEventsIntoPrompt() Assert.Contains("继续处理库存差异", capturedPrompt, StringComparison.Ordinal); } + [Fact] + public async Task Chat_ConcurrentTurnsOnSameSession_RunOneAtATime() + { + var firstTurnStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var secondTurnStarted = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var releaseFirstTurn = new TaskCompletionSource(TaskCreationOptions.RunContinuationsAsynchronously); + var turns = 0; + var agentRuntime = Substitute.For(); + agentRuntime.RunStreamingAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(_ => HeldTurn()); + + async IAsyncEnumerable HeldTurn() + { + if (Interlocked.Increment(ref turns) == 1) + { + firstTurnStarted.SetResult(); + await releaseFirstTurn.Task; + } + else + { + secondTurnStarted.SetResult(); + } + + yield return AgentStreamEvent.Complete(); + } + + await using var harness = await StartGatewayAsync(null, agentRuntime); + var ct = TestContext.Current.CancellationToken; + + var first = harness.Client.PostAsync("/apps/chat", JsonContent.Create(new { message = "one", sessionId = "apps-serial" }), ct); + await firstTurnStarted.Task.WaitAsync(TimeSpan.FromSeconds(10), ct); + var second = harness.Client.PostAsync("/apps/chat", JsonContent.Create(new { message = "two", sessionId = "apps-serial" }), ct); + + // While the first turn holds the session, the second must wait rather than run alongside it. + var overlapped = await Task.WhenAny(secondTurnStarted.Task, Task.Delay(TimeSpan.FromMilliseconds(500), ct)) == secondTurnStarted.Task; + releaseFirstTurn.SetResult(); + (await first).EnsureSuccessStatusCode(); + (await second).EnsureSuccessStatusCode(); + + Assert.False(overlapped); + Assert.True(secondTurnStarted.Task.IsCompletedSuccessfully); + } + + [Fact] + public async Task Chat_PersistsSessionAfterTurn() + { + var store = new TestMemoryStore(); + var agentRuntime = Substitute.For(); + agentRuntime.RunStreamingAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns(_ => StreamEvents()); + + await using var harness = await StartGatewayAsync(null, agentRuntime, store); + var ct = TestContext.Current.CancellationToken; + + var response = await harness.Client.PostAsync("/apps/chat", JsonContent.Create(new { message = "hello", sessionId = "apps-persisted" }), ct); + response.EnsureSuccessStatusCode(); + + Assert.NotNull(await store.GetSessionAsync("apps-persisted", ct)); + } + [Fact] public void CorsAllowHeaders_IncludeMcpSessionHeaders() { @@ -133,7 +203,7 @@ private async Task StartFakeUpstreamAsync() return $"{app.Urls.Single().TrimEnd('/')}/mcp"; } - private async Task StartGatewayAsync(string? upstreamUrl, IAgentRuntime agentRuntime) + private async Task StartGatewayAsync(string? upstreamUrl, IAgentRuntime agentRuntime, IMemoryStore? store = null) { var config = new GatewayConfig { @@ -182,7 +252,7 @@ await File.WriteAllTextAsync( builder.Services.AddOpenClawMcpAppServices(config.McpApps); builder.Services.AddSingleton(new BrowserSessionAuthService(config)); - var runtime = CreateRuntime(config, agentRuntime); + var runtime = CreateRuntime(config, agentRuntime, store ?? new TestMemoryStore()); var app = builder.Build(); app.MapOpenClawAppsEndpoints(startup, runtime); await app.StartAsync(); @@ -196,9 +266,9 @@ await File.WriteAllTextAsync( return new AppsGatewayTestHarness(app, client, registry); } - private static GatewayAppRuntime CreateRuntime(GatewayConfig config, IAgentRuntime agentRuntime) + private static GatewayAppRuntime CreateRuntime(GatewayConfig config, IAgentRuntime agentRuntime, IMemoryStore store) { - var sessionManager = new SessionManager(new TestMemoryStore(), config, NullLogger.Instance); + var sessionManager = new SessionManager(store, config, NullLogger.Instance); return new GatewayAppRuntime { AgentRuntime = agentRuntime, From 6abb971c9454aa6bb27916b3e26f9136885a05b9 Mon Sep 17 00:00:00 2001 From: telli Date: Tue, 29 Sep 2026 03:58:25 -0700 Subject: [PATCH 7/8] fix(companion): follow the gateway's agent-execution answer, not the role Companion's preflight refused to open chat for any gateway-reported role below operator. With Security.AllowViewerAgentExecution on, the gateway still admits those viewers, so the documented migration switch did not work for Companion users. GET /auth/session now reports canExecuteAgent, computed by the same rule CanExecuteAgent enforces, including the opt-out. Companion blocks only when the gateway says false; when the field is absent (older gateways) it connects and lets the gateway decide. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 +- docs/AUTHENTICATION.md | 2 ++ docs/zh-CN/AUTHENTICATION.md | 2 ++ .../ViewModels/MainWindowViewModel.cs | 13 ++++---- src/OpenClaw.Core/Models/AdminApiModels.cs | 6 ++++ .../Endpoints/AdminEndpoints.Support.cs | 1 + .../Endpoints/EndpointHelpers.cs | 5 +++ .../CompanionConnectionTests.cs | 32 +++++++++++++++++-- .../GatewayAdminEndpointTests.cs | 20 ++++++++++++ 9 files changed, 74 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b47d4edb..61040045 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,7 +47,7 @@ All notable changes to this project are tracked in this file. - 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. It also checks the account's role before connecting: a gateway-reported role below `operator` gets an explanation instead of a chat connection, and the read-only status views stay available. +- 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. It also asks the gateway before connecting: `GET /auth/session` now reports `canExecuteAgent`, which follows `AllowViewerAgentExecution`, and when it is `false` Companion explains the missing `operator` role instead of opening chat. The read-only status views stay available. Against a gateway that does not report the field, Companion connects and the gateway decides. - 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 a1655f5c..abbb476d 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -196,6 +196,8 @@ Bootstrap tokens and open loopback resolve to `admin` and are unaffected. New op 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. +`GET /auth/session` reports the result of this check as `canExecuteAgent`, including the effect of `AllowViewerAgentExecution`, so clients can explain a refusal before connecting. Companion uses it; a gateway that predates the field omits it. + ### 3.4 `IsAuthorizedRequest` — Detailed Logic ```csharp diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index 9ef7bc48..0eb7df21 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -196,6 +196,8 @@ WebSocket 已连接 每次拒绝都会在 `OpenClaw.Gateway.Authorization` 类别下记录一条警告日志，包含入口、认证方式、账户和角色（绝不包含凭据），便于管理员找出需要提升角色的账户。如需无中断迁移，可设置 `OpenClaw:Security:AllowViewerAgentExecution=true`，从日志中找出被放行的账户，为其授予 `operator` 角色，然后关闭该设置。该设置是临时的，将在下一个版本移除。 +`GET /auth/session` 会以 `canExecuteAgent` 字段报告这项检查的结果（包括 `AllowViewerAgentExecution` 的影响），客户端可以在连接前说明拒绝原因。Companion 会使用该字段；早于此字段的网关不会返回它。 + ### 3.4 `IsAuthorizedRequest` 详细逻辑 ```csharp diff --git a/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs index 201e5906..cc319159 100644 --- a/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs +++ b/src/OpenClaw.Companion/ViewModels/MainWindowViewModel.cs @@ -65,7 +65,7 @@ public sealed partial class MainWindowViewModel : ViewModelBase [ObservableProperty] private string _operatorRole = OperatorRoleNames.Viewer; - private bool _operatorRoleReportedByGateway; + private bool? _agentExecutionAllowedByGateway; [ObservableProperty] private string _operatorAuthMode = "account_token"; @@ -529,10 +529,11 @@ private async Task ConnectAsync() Status = "Connecting…"; - // Learn the role first: a viewer would be admitted and then closed by the gateway, so say why up front - // and keep the read-only status views. Only trust a role the gateway reported, not the no-token placeholder. + // Ask the gateway first: an account it won't let run the agent would be admitted and then closed, so say why + // up front and keep the read-only status views. Only its answer counts, not the role: a viewer can still chat + // under Security.AllowViewerAgentExecution, and a gateway that doesn't report it decides at connect time. await LoadAdminStatusAsyncInternal(); - if (_operatorRoleReportedByGateway && !IsBootstrapAdmin && !OperatorRoleNames.CanAccess(OperatorRole, OperatorRoleNames.Operator)) + if (_agentExecutionAllowedByGateway == false) { Status = "Disconnected"; AddSystemMessage($"Signed in as {OperatorIdentity} with the {OperatorRole} role. Chat needs the operator role; ask an admin to change this account's role."); @@ -640,7 +641,7 @@ private async Task LoadAdminStatusAsync() private async Task LoadAdminStatusAsyncInternal() { - _operatorRoleReportedByGateway = false; + _agentExecutionAllowedByGateway = null; using var client = CreateAdminClient(out var error); if (client is null) { @@ -661,7 +662,7 @@ private async Task LoadAdminStatusAsyncInternal() { var auth = await client.GetAuthSessionAsync(CancellationToken.None); ApplyOperatorIdentity(auth.AuthMode, auth.Role, auth.DisplayName, auth.Username, auth.IsBootstrapAdmin); - _operatorRoleReportedByGateway = true; + _agentExecutionAllowedByGateway = auth.CanExecuteAgent; var setup = await client.GetSetupStatusAsync(CancellationToken.None); AdminStatus = auth.IsBootstrapAdmin ? "Using bootstrap/breakglass admin auth." diff --git a/src/OpenClaw.Core/Models/AdminApiModels.cs b/src/OpenClaw.Core/Models/AdminApiModels.cs index 9c5f22e1..34c5dd00 100644 --- a/src/OpenClaw.Core/Models/AdminApiModels.cs +++ b/src/OpenClaw.Core/Models/AdminApiModels.cs @@ -27,6 +27,12 @@ public sealed class AuthSessionResponse public string? Username { get; init; } public string? DisplayName { get; init; } public bool IsBootstrapAdmin { get; init; } + + /// + /// Whether this caller may run the agent (chat over /ws, /v1/*, A2A, and the other agent surfaces). + /// Null from gateways that predate the field; those decide only when the client connects. + /// + public bool? CanExecuteAgent { get; init; } public bool PublicBind { get; init; } public string[] AllowedAuthModes { get; init; } = []; public string EffectiveToolSurface { get; init; } = "web"; diff --git a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs index e916e8e7..ba6c8dbc 100644 --- a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs +++ b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs @@ -234,6 +234,7 @@ private static AuthSessionResponse MapAuthSessionResponse( Username = auth.Username, DisplayName = auth.DisplayName, IsBootstrapAdmin = auth.IsBootstrapAdmin, + CanExecuteAgent = EndpointHelpers.AllowsAgentExecution(auth, startup), PublicBind = startup.IsNonLoopbackBind, AllowedAuthModes = [.. policy.AllowedAuthModes], EffectiveToolSurface = preset.Surface, diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 25152a78..f7d69bf4 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -363,6 +363,11 @@ 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. /// + // The rule CanExecuteAgent enforces, without its logging, so /auth/session can report it before a client tries. + internal static bool AllowsAgentExecution(OperatorAuthorizationResult auth, GatewayStartupContext startup) + => auth.IsAuthorized + && (IsRoleAllowed(auth.Role, "integration.mutate.agent", out _) || startup.Config.Security.AllowViewerAgentExecution); + public static bool CanExecuteAgent( HttpContext ctx, GatewayStartupContext startup, diff --git a/src/OpenClaw.Tests/CompanionConnectionTests.cs b/src/OpenClaw.Tests/CompanionConnectionTests.cs index fb6807b2..db522b02 100644 --- a/src/OpenClaw.Tests/CompanionConnectionTests.cs +++ b/src/OpenClaw.Tests/CompanionConnectionTests.cs @@ -70,9 +70,9 @@ public async Task ServerClose_WhenClientReconnectsBeforeUiDispatch_ShouldKeepCon } [AvaloniaFact] - public async Task Connect_WhenGatewayReportsViewerRole_ShouldExplainWithoutOpeningChat() + public async Task Connect_WhenGatewayReportsAgentExecutionDenied_ShouldExplainWithoutOpeningChat() { - var vm = CreateViewModelWithAuthSession("""{"authMode":"account_token","role":"viewer","username":"reader"}"""); + var vm = CreateViewModelWithAuthSession("""{"authMode":"account_token","role":"viewer","username":"reader","canExecuteAgent":false}"""); vm.AuthToken = "viewer-token"; await vm.ConnectCommand.ExecuteAsync(null); @@ -84,6 +84,34 @@ public async Task Connect_WhenGatewayReportsViewerRole_ShouldExplainWithoutOpeni Assert.DoesNotContain(vm.Messages, m => m.Text.StartsWith("Connect failed", StringComparison.Ordinal)); } + [AvaloniaFact] + public async Task Connect_WhenGatewayAllowsViewerAgentExecution_ShouldAttemptChat() + { + // Security.AllowViewerAgentExecution lets a viewer chat, so the role alone must not stop Companion. + var vm = CreateViewModelWithAuthSession("""{"authMode":"account_token","role":"viewer","username":"reader","canExecuteAgent":true}"""); + vm.AuthToken = "viewer-token"; + + await vm.ConnectCommand.ExecuteAsync(null); + Dispatcher.UIThread.RunJobs(); + + Assert.Contains(vm.Messages, m => m.Text.StartsWith("Connect failed", StringComparison.Ordinal)); + Assert.DoesNotContain(vm.Messages, m => m.Text.Contains("operator role", StringComparison.OrdinalIgnoreCase)); + } + + [AvaloniaFact] + public async Task Connect_WhenGatewayDoesNotReportAgentExecution_ShouldAttemptChat() + { + // An older gateway reports only the role; it decides at connect time, so Companion must not guess. + var vm = CreateViewModelWithAuthSession("""{"authMode":"account_token","role":"viewer","username":"reader"}"""); + vm.AuthToken = "viewer-token"; + + await vm.ConnectCommand.ExecuteAsync(null); + Dispatcher.UIThread.RunJobs(); + + Assert.Contains(vm.Messages, m => m.Text.StartsWith("Connect failed", StringComparison.Ordinal)); + Assert.DoesNotContain(vm.Messages, m => m.Text.Contains("operator role", StringComparison.OrdinalIgnoreCase)); + } + [AvaloniaFact] public async Task Connect_WhenNoTokenLoaded_ShouldStillAttemptChat() { diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index d2af936b..3a871248 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -652,6 +652,26 @@ public async Task AgentExecution_WhenViewerAndAllowViewerAgentExecution_ShouldRu Assert.Contains("legacy-viewer", entry, StringComparison.Ordinal); } + [Theory] + [InlineData(OperatorRoleNames.Viewer, false, false)] + [InlineData(OperatorRoleNames.Operator, false, true)] + [InlineData(OperatorRoleNames.Viewer, true, true)] + public async Task AuthSession_ShouldReportWhetherTheCallerCanRunTheAgent(string role, bool allowViewerAgentExecution, bool expected) + { + await using var harness = await CreateHarnessAsync( + nonLoopbackBind: true, + configure: config => config.Security.AllowViewerAgentExecution = allowViewerAgentExecution); + var token = CreateAccountToken(harness, $"session-{role}", role); + + using var request = new HttpRequestMessage(HttpMethod.Get, "/auth/session"); + 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(expected, payload.RootElement.GetProperty("canExecuteAgent").GetBoolean()); + } + [Fact] public async Task AgentExecution_WhenAllowViewerAgentExecutionWithoutCredentials_ShouldStillReject() { From 0b0e44879b0ed3081d8af43bbe4e5a2f4150aae1 Mon Sep 17 00:00:00 2001 From: telli Date: Tue, 29 Sep 2026 04:26:24 -0700 Subject: [PATCH 8/8] fix(gateway): close three session-ownership gaps found in review - /ws resolved its account with a helper that returned nothing on a loopback-bound gateway, even with AlwaysRequireAuth or OIDC. Those turns carried no account, so the owner check treated them as accountless and let them write anywhere, and new sessions had no owner. /ws now uses the same caller resolution as the other surfaces. - /v1/* stable sessions are keyed by the bearer token's hash or, for browser sessions, the client address. Two signed-in accounts behind one address derived the same session and the second ran the first's conversation. Both endpoints now apply the owner check under the session lock and return 403 session_forbidden. - Session management (delete, metadata, abort, branch restore, guided recovery) stayed operator-wide, so another operator could delete an owned session and recreate the id as its own. Those routes now follow the owner-or-admin rule. Co-Authored-By: Claude Opus 5.5 (1M context) --- CHANGELOG.md | 2 +- docs/AUTHENTICATION.md | 6 +- docs/zh-CN/AUTHENTICATION.md | 6 +- .../Endpoints/AdminEndpoints.Recovery.cs | 1 + .../Endpoints/AdminEndpoints.Sessions.cs | 11 ++- .../Endpoints/AdminEndpoints.Support.cs | 20 +++++ .../Endpoints/EndpointHelpers.cs | 6 +- .../OpenAiEndpoints.ChatCompletions.cs | 6 +- .../Endpoints/OpenAiEndpoints.Responses.cs | 6 +- .../Endpoints/OpenAiEndpoints.cs | 17 ++++ .../Endpoints/WebSocketEndpoints.cs | 31 +------- .../GatewayAdminEndpointTests.cs | 78 +++++++++++++++++++ src/OpenClaw.Tests/WebSocketEndpointsTests.cs | 53 ++++++++++--- 13 files changed, 198 insertions(+), 45 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a2f5440c..255d1b73 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -49,7 +49,7 @@ All notable changes to this project are tracked in this file. - 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`. - Turns now run as the signed-in account instead of a caller-supplied sender id. `Session.AuthenticatedUserId` scopes per-user capability bindings and is passed to MCP servers as `_meta.userId`. Previously only `/ws` set it, so REST messages, MCP `send_message`, A2A, `/v1/*`, and `/apps/chat` turns ran as whatever `senderId` or `contextId` the caller supplied. MCP servers that read `_meta.userId` now receive account ids for those turns. - A turn from an external sender without an account no longer inherits the previous writer's account. Previously, after an operator posted into a Telegram session, the Telegram user's next turns ran as that operator. System, scheduled, automation, and background-continuation turns keep the session's identity. -- Sessions record the account that created them as `ownerAccountId`, and only the owner or an admin can post to an owned session. Every surface refuses other accounts: REST (403), MCP `send_message` (tool error), `/apps/chat` (403), A2A (error event), and pipeline turns including `/ws` (reply). Previously any operator could post into any session whose id it knew. Unowned sessions (channels, cron, sessions created before this change) stay open and are never claimed by writing to them. Reading is unchanged. `GET /api/integration/sessions?owner=me` lists the caller's own sessions. +- Sessions record the account that created them as `ownerAccountId`, and only the owner or an admin can post to an owned session. Every surface refuses other accounts: REST (403), MCP `send_message` (tool error), `/apps/chat` (403), `/v1/*` stable sessions (403, `session_forbidden`), A2A (error event), and pipeline turns including `/ws` (reply). Session management follows the same rule: delete, metadata, abort, branch restore, and guided recovery return 403 for other non-admin accounts. `/ws` now resolves its caller like the other surfaces, so loopback-bound gateways with `AlwaysRequireAuth` or OIDC record and enforce owners. Previously any operator could post into any session whose id it knew. Unowned sessions (channels, cron, sessions created before this change) stay open and are never claimed by writing to them. Reading is unchanged. `GET /api/integration/sessions?owner=me` lists the caller's own sessions. - 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. It also asks the gateway before connecting: `GET /auth/session` now reports `canExecuteAgent`, which follows `AllowViewerAgentExecution`, and when it is `false` Companion explains the missing `operator` role instead of opening chat. The read-only status views stay available. Against a gateway that does not report the field, Companion connects and the gateway decides. - 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 e856312e..6a65d87b 100644 --- a/docs/AUTHENTICATION.md +++ b/docs/AUTHENTICATION.md @@ -261,16 +261,18 @@ A session created by a signed-in account records it as `Session.OwnerAccountId`. | Session | Who can post to it | |---------|--------------------| -| Owned | The owner and admins. Other accounts are refused on every surface: REST returns 403, MCP returns a tool error, `/apps/chat` returns 403, A2A returns an error event, and pipeline turns (including `/ws`) get a reply saying the conversation belongs to another account | +| Owned | The owner and admins. Other accounts are refused on every surface: REST returns 403, MCP returns a tool error, `/apps/chat` returns 403, `/v1/*` stable sessions return 403 with code `session_forbidden`, A2A returns an error event, and pipeline turns (including `/ws`) get a reply saying the conversation belongs to another account | | Unowned (created by channels, cron, bootstrap or loopback callers, or before ownership existed) | Anyone allowed to post. Writing to an unowned session never claims it | Callers without an account (bootstrap, open loopback, channel and system turns) are not restricted by ownership. They are admin-equivalent, or they address sessions by their own keys. +The same rule covers session management: deleting a session (`DELETE /admin/sessions/{id}`), changing its metadata, aborting its run, restoring one of its branches, and guided recovery return 403 for other non-admin accounts. Promoting a session to an automation only reads it and is unaffected. + Reading stays open to every role that can read sessions, so dashboards and audit are unaffected. `GET /api/integration/sessions?owner=me` lists only the caller's own sessions, active and persisted. `SessionSummary.ownerAccountId` carries the owner. Callers without an account own no sessions. -`/v1/*` sessions are already scoped to the calling credential. They record the owner but need no extra check. +`/v1/*` stable sessions (`X-OpenClaw-Session-Id`) are keyed by the bearer token's hash or, for browser sessions, the client address, so two signed-in accounts behind one address can derive the same session; the owner check keeps them apart. `/ws` resolves its caller the same way as the other surfaces, so a loopback-bound gateway that still requires auth (`AlwaysRequireAuth` or OIDC) records owners and enforces them. ## 4. Middleware Pipeline diff --git a/docs/zh-CN/AUTHENTICATION.md b/docs/zh-CN/AUTHENTICATION.md index db83340e..8aacb30f 100644 --- a/docs/zh-CN/AUTHENTICATION.md +++ b/docs/zh-CN/AUTHENTICATION.md @@ -261,16 +261,18 @@ return false; // 401 Unauthorized | 会话 | 谁可以向其发送消息 | |------|--------------------| -| 有归属 | 所有者和管理员。其他账户在所有入口都会被拒绝：REST 返回 403，MCP 返回工具错误，`/apps/chat` 返回 403，A2A 返回错误事件，管道轮次（包括 `/ws`）会收到“该会话属于另一个账户”的回复 | +| 有归属 | 所有者和管理员。其他账户在所有入口都会被拒绝：REST 返回 403，MCP 返回工具错误，`/apps/chat` 返回 403，`/v1/*` 稳定会话返回 403（错误码 `session_forbidden`），A2A 返回错误事件，管道轮次（包括 `/ws`）会收到“该会话属于另一个账户”的回复 | | 无归属（由渠道、定时任务、引导令牌或回环调用方创建，或创建于引入归属之前） | 任何被允许发送消息的调用方。向无归属会话写入永远不会占有它 | 没有账户的调用方（引导令牌、开放回环、渠道和系统轮次）不受归属限制。它们等同于管理员，或者按各自的键访问会话。 +同一规则也适用于会话管理：删除会话（`DELETE /admin/sessions/{id}`）、修改元数据、中止运行、恢复分支以及引导式恢复，对其他非管理员账户返回 403。将会话提升为自动化只读取会话，不受影响。 + 读取对所有能读取会话的角色保持开放，仪表盘和审计不受影响。 `GET /api/integration/sessions?owner=me` 只列出调用方自己的会话（活动和持久化的）。`SessionSummary.ownerAccountId` 给出所有者。没有账户的调用方不拥有任何会话。 -`/v1/*` 会话已经按调用凭据隔离，会记录所有者，但不需要额外检查。 +`/v1/*` 稳定会话（`X-OpenClaw-Session-Id`）按 Bearer 令牌的哈希或（浏览器会话时）客户端地址派生，因此同一地址后的两个已登录账户可能得到同一个会话；归属检查将它们区分开。`/ws` 与其他入口使用相同的调用方解析，因此仍要求认证的回环绑定网关（`AlwaysRequireAuth` 或 OIDC）也会记录并执行归属。 ## 四、中间件管道 diff --git a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Recovery.cs b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Recovery.cs index aac6d915..03a65dea 100644 --- a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Recovery.cs +++ b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Recovery.cs @@ -42,6 +42,7 @@ private static void MapRecoveryEndpoints(WebApplication app, AdminEndpointServic requireCsrf: true, endpointScope: "admin.sessions.recovery.mutate"); if (authorization.Failure is not null) return authorization.Failure; var auth = authorization.Authorization!; + if (await RejectUnlessSessionWriterAsync(services.Runtime.SessionManager, id, auth, ctx.RequestAborted) is { } denied) return denied; if (!EndpointHelpers.TryConsumeOperatorRateLimit(ctx, services.Operations, auth, "admin.control", out _)) return Results.StatusCode(429); var body = await EndpointHelpers.TryReadBodyTextAsync(ctx, 100_000, ctx.RequestAborted); if (!body.Success) return Results.BadRequest(); diff --git a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs index 8039fb1d..4ec0d91a 100644 --- a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs +++ b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Sessions.cs @@ -308,6 +308,9 @@ private static void MapSessionEndpoints(WebApplication app, AdminEndpointService }); } + if (await RejectUnlessSessionWriterAsync(runtime.SessionManager, sessionId, auth, ctx.RequestAborted) is { } restoreDenied) + return restoreDenied; + var session = await runtime.SessionManager.LoadAsync(sessionId, ctx.RequestAborted); if (session is null) { @@ -374,6 +377,8 @@ private static void MapSessionEndpoints(WebApplication app, AdminEndpointService if (authResult.Failure is not null) return authResult.Failure; var auth = authResult.Authorization!; + if (await RejectUnlessSessionWriterAsync(runtime.SessionManager, id, auth, ctx.RequestAborted) is { } metadataDenied) + return metadataDenied; var requestPayload = await ReadJsonBodyAsync(ctx, CoreJsonContext.Default.SessionMetadataUpdateRequest); if (requestPayload.Failure is not null) @@ -455,12 +460,14 @@ private static void MapSessionEndpoints(WebApplication app, AdminEndpointService return Results.Json(ids, CoreJsonContext.Default.ListString); }); - app.MapPost("/admin/sessions/{id}/abort", (HttpContext ctx, string id) => + app.MapPost("/admin/sessions/{id}/abort", async (HttpContext ctx, string id) => { var authResult = AuthorizeOperator(ctx, startup, browserSessions, operations, requireCsrf: true, endpointScope: "admin.sessions.abort"); if (authResult.Failure is not null) return authResult.Failure; var auth = authResult.Authorization!; + if (await RejectUnlessSessionWriterAsync(runtime.SessionManager, id, auth, ctx.RequestAborted) is { } abortDenied) + return abortDenied; if (!EndpointHelpers.TryConsumeOperatorRateLimit(ctx, operations, auth, "admin.control", out var blockedByPolicyId)) return Results.Json(new OperationStatusResponse { Success = false, Message = $"Rate limit exceeded by policy '{blockedByPolicyId}'." }, CoreJsonContext.Default.OperationStatusResponse, statusCode: StatusCodes.Status429TooManyRequests); @@ -483,6 +490,8 @@ private static void MapSessionEndpoints(WebApplication app, AdminEndpointService if (authResult.Failure is not null) return authResult.Failure; var auth = authResult.Authorization!; + if (await RejectUnlessSessionWriterAsync(runtime.SessionManager, id, auth, ctx.RequestAborted) is { } deleteDenied) + return deleteDenied; if (!EndpointHelpers.TryConsumeOperatorRateLimit(ctx, operations, auth, "admin.control", out var blockedByPolicyId)) return Results.Json(new OperationStatusResponse { Success = false, Message = $"Rate limit exceeded by policy '{blockedByPolicyId}'." }, CoreJsonContext.Default.OperationStatusResponse, statusCode: StatusCodes.Status429TooManyRequests); diff --git a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs index ba6c8dbc..bb68078d 100644 --- a/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs +++ b/src/OpenClaw.Gateway/Endpoints/AdminEndpoints.Support.cs @@ -17,6 +17,7 @@ using OpenClaw.Core.Pipeline; using OpenClaw.Core.Plugins; using OpenClaw.Core.Security; +using OpenClaw.Core.Sessions; using OpenClaw.Core.Skills; using OpenClaw.Core.Validation; using OpenClaw.Gateway; @@ -206,6 +207,25 @@ private static void RecordOperatorAudit( }); } + // Session writes follow the owner-or-admin rule the chat surfaces apply: an operator may change only sessions + // it created or that have no owner. Reads stay open to every role that can read sessions. + private static async Task RejectUnlessSessionWriterAsync( + SessionManager sessions, + string sessionId, + EndpointHelpers.OperatorAuthorizationResult auth, + CancellationToken ct) + { + var session = await sessions.LoadAsync(sessionId, ct); + var caller = EndpointHelpers.ToCaller(auth); + if (session is null || SessionAccess.CanWrite(session, caller.AccountId, caller.IsAdmin)) + return null; + + return Results.Json( + new OperationStatusResponse { Success = false, Error = SessionAccess.DeniedMessage }, + CoreJsonContext.Default.OperationStatusResponse, + statusCode: StatusCodes.Status403Forbidden); + } + private static AuthSessionResponse MapAuthSessionResponse( EndpointHelpers.OperatorAuthorizationResult auth, GatewayStartupContext startup, diff --git a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs index 6000c021..552ec660 100644 --- a/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs +++ b/src/OpenClaw.Gateway/Endpoints/EndpointHelpers.cs @@ -421,7 +421,11 @@ public static bool CanExecuteAgent( internal static CallerAccount ResolveCaller(HttpContext ctx, GatewayStartupContext startup) { var browserSessions = ctx.RequestServices.GetRequiredService(); - var auth = AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); + return ToCaller(AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false)); + } + + internal static CallerAccount ToCaller(OperatorAuthorizationResult auth) + { if (!auth.IsAuthorized) return default; diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs index 574eb92d..d98dece3 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.ChatCompletions.cs @@ -76,7 +76,8 @@ private static void MapChatCompletionsEndpoint( ? CreateStableSessionBinding(stableSessionId!, requesterKey) : null; var requestId = $"oai-http:{Guid.NewGuid():N}"; - var accountId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + var caller = EndpointHelpers.ResolveCaller(ctx, startup); + var accountId = caller.AccountId; var session = stableBinding is not null ? await runtime.SessionManager.GetOrCreateByIdAsync(BuildScopedStableSessionId(stableBinding), "openai-http", requesterKey, ctx.RequestAborted, accountId) : await runtime.SessionManager.GetOrCreateAsync("openai-http", requestId, ctx.RequestAborted, accountId); @@ -96,6 +97,9 @@ private static void MapChatCompletionsEndpoint( return; } + if (await TryRejectOtherAccountsSessionAsync(ctx, session, caller)) + return; + persistStableSessionOnExit = true; } diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs index ef51b0a5..9937242e 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.Responses.cs @@ -65,7 +65,8 @@ private static void MapResponsesEndpoint( ? CreateStableSessionBinding(stableSessionId!, requesterKey) : null; var requestId = $"oai-resp:{Guid.NewGuid():N}"; - var accountId = EndpointHelpers.ResolveAuthenticatedAccountId(ctx, startup); + var caller = EndpointHelpers.ResolveCaller(ctx, startup); + var accountId = caller.AccountId; var session = stableBinding is not null ? await runtime.SessionManager.GetOrCreateByIdAsync(BuildScopedStableSessionId(stableBinding), "openai-responses", requesterKey, ctx.RequestAborted, accountId) : await runtime.SessionManager.GetOrCreateAsync("openai-responses", requestId, ctx.RequestAborted, accountId); @@ -85,6 +86,9 @@ private static void MapResponsesEndpoint( return; } + if (await TryRejectOtherAccountsSessionAsync(ctx, session, caller)) + return; + persistStableSessionOnExit = true; } diff --git a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs index e747aab8..a2ac28e2 100644 --- a/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/OpenAiEndpoints.cs @@ -1,6 +1,7 @@ using OpenClaw.Gateway.Bootstrap; using OpenClaw.Gateway.Composition; using OpenClaw.Core.Models; +using OpenClaw.Core.Sessions; namespace OpenClaw.Gateway.Endpoints; @@ -14,6 +15,9 @@ internal static partial class OpenAiEndpoints private const string OperatorRoleRequiredErrorJson = $$$"""{"error":{"message":"{{{EndpointHelpers.OperatorRoleRequiredMessage}}}","type":"permission_error","code":"insufficient_role"}}"""; + private const string SessionForbiddenErrorJson = + $$$"""{"error":{"message":"{{{SessionAccess.DeniedMessage}}}","type":"permission_error","code":"session_forbidden"}}"""; + public static void MapOpenClawOpenAiEndpoints( this WebApplication app, GatewayStartupContext startup, @@ -34,6 +38,19 @@ private static async Task TryRejectBelowOperatorAsync(HttpContext ctx, Gat return true; } + // A stable session is keyed by the bearer token's hash or, for browser sessions, the client address, so two + // signed-in accounts behind one address can derive the same session. The owner check keeps them apart. + private static async Task TryRejectOtherAccountsSessionAsync(HttpContext ctx, Session session, EndpointHelpers.CallerAccount caller) + { + if (SessionAccess.CanWrite(session, caller.AccountId, caller.IsAdmin)) + return false; + + ctx.Response.StatusCode = StatusCodes.Status403Forbidden; + ctx.Response.ContentType = "application/json"; + await ctx.Response.WriteAsync(SessionForbiddenErrorJson, 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 a4b99d63..d71d40f8 100644 --- a/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs +++ b/src/OpenClaw.Gateway/Endpoints/WebSocketEndpoints.cs @@ -1,5 +1,4 @@ using System.Net.WebSockets; -using System.Security.Claims; using System.Text; using System.Text.Json; using OpenClaw.Core.Security; @@ -33,9 +32,10 @@ public static void MapOpenClawWebSocketEndpoints( } var clientId = ctx.Connection.Id; - TryResolveAuthorizedUserIdForWebSocket(ctx, startup, out var userId); - var isAdmin = EndpointHelpers.ResolveCaller(ctx, startup).IsAdmin; - await runtime.WebSocketChannel.HandleConnectionAsync(ws, clientId, ctx.Connection.RemoteIpAddress, ctx.RequestAborted, userId, isAdmin); + // Resolve the caller as the other surfaces do, so a loopback-bound gateway that still requires auth + // (AlwaysRequireAuth or OIDC) stamps the account and the session owner check applies to /ws turns. + var caller = EndpointHelpers.ResolveCaller(ctx, startup); + await runtime.WebSocketChannel.HandleConnectionAsync(ws, clientId, ctx.Connection.RemoteIpAddress, ctx.RequestAborted, caller.AccountId, caller.IsAdmin); }); app.Map("/ws/live", async (HttpContext ctx) => @@ -114,29 +114,6 @@ private static bool TryValidateWebSocketRequest( return true; } - internal static bool TryResolveAuthorizedUserIdForWebSocket(HttpContext ctx, GatewayStartupContext startup, out string? authenticatedUserId) - { - authenticatedUserId = null; - - if (!startup.IsNonLoopbackBind) - return true; - - if (ctx.User.Identity?.IsAuthenticated == true) - { - authenticatedUserId = (ctx.User.FindFirst(ClaimTypes.NameIdentifier) ?? ctx.User.FindFirst("sub"))?.Value; - if (!string.IsNullOrWhiteSpace(authenticatedUserId)) - return true; - } - - var browserSessions = ctx.RequestServices.GetRequiredService(); - var auth = EndpointHelpers.AuthorizeOperatorRequest(ctx, startup, browserSessions, requireCsrf: false); - if (!auth.IsAuthorized) - return false; - - authenticatedUserId = auth.AccountId; - return true; - } - private static bool IsOriginAllowed(HttpContext ctx, GatewayAppRuntime runtime) { if (!ctx.Request.Headers.TryGetValue("Origin", out var origin)) diff --git a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs index ba944344..9af0328f 100644 --- a/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs +++ b/src/OpenClaw.Tests/GatewayAdminEndpointTests.cs @@ -961,6 +961,84 @@ await bridge.ExecuteStreamingAsync( Assert.Contains(events, evt => evt.Type == AgentStreamEventType.Error && evt.Content == SessionAccess.DeniedMessage); } + [Theory] + [InlineData("/v1/chat/completions", """{"messages":[{"role":"user","content":"hello"}]}""")] + [InlineData("/v1/responses", """{"input":"hello"}""")] + public async Task OpenAiStableSession_WhenOwnedByAnotherBrowserAccount_ShouldReturnForbidden(string path, string body) + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (ownerToken, _) = CreateAccountTokenWithId(harness, "cookie-owner", OperatorRoleNames.Operator); + var (otherToken, _) = CreateAccountTokenWithId(harness, "cookie-other", OperatorRoleNames.Operator); + var (ownerCookie, _) = await LoginAsync(harness.Client, ownerToken); + var (otherCookie, _) = await LoginAsync(harness.Client, otherToken); + harness.Runtime.AgentRuntime.RunAsync( + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any(), + Arg.Any()) + .Returns("ok"); + + // Browser sessions have no bearer token, so both accounts derive the stable session from the same client address. + async Task SendAsync(string cookie) + { + using var request = new HttpRequestMessage(HttpMethod.Post, path) { Content = JsonContent(body) }; + request.Headers.Add("Cookie", cookie); + request.Headers.Add("X-OpenClaw-Session-Id", "shared-stable"); + return await harness.Client.SendAsync(request); + } + + Assert.Equal(HttpStatusCode.OK, (await SendAsync(ownerCookie)).StatusCode); + var denied = await SendAsync(otherCookie); + + Assert.Equal(HttpStatusCode.Forbidden, denied.StatusCode); + using var payload = await ReadJsonAsync(denied); + Assert.Equal("session_forbidden", payload.RootElement.GetProperty("error").GetProperty("code").GetString()); + } + + [Theory] + [InlineData("DELETE", "/admin/sessions/owned-admin")] + [InlineData("POST", "/admin/sessions/owned-admin/metadata")] + [InlineData("POST", "/admin/sessions/owned-admin/abort")] + [InlineData("POST", "/admin/branches/owned-admin:branch:b1/restore")] + [InlineData("POST", "/admin/sessions/owned-admin/recovery")] + public async Task SessionManagement_WhenSessionOwnedByAnotherAccount_ShouldReturnForbidden(string method, string path) + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (_, ownerId) = CreateAccountTokenWithId(harness, "manage-owner", OperatorRoleNames.Operator); + var (otherToken, _) = CreateAccountTokenWithId(harness, "manage-other", OperatorRoleNames.Operator); + await harness.MemoryStore.SaveSessionAsync( + new Session { Id = "owned-admin", ChannelId = "agentqi-mobile", SenderId = "owner", OwnerAccountId = ownerId }, + CancellationToken.None); + + using var request = new HttpRequestMessage(new HttpMethod(method), path) { Content = JsonContent("{}") }; + request.Headers.Authorization = new AuthenticationHeaderValue("Bearer", otherToken); + var response = await harness.Client.SendAsync(request); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + Assert.NotNull(await harness.MemoryStore.GetSessionAsync("owned-admin", CancellationToken.None)); + } + + [Fact] + public async Task SessionManagement_WhenOwnerOrAdmin_ShouldAllowWrites() + { + await using var harness = await CreateHarnessAsync(nonLoopbackBind: true); + var (ownerToken, ownerId) = CreateAccountTokenWithId(harness, "manage-self", OperatorRoleNames.Operator); + var (adminToken, _) = CreateAccountTokenWithId(harness, "manage-admin", OperatorRoleNames.Admin); + await harness.MemoryStore.SaveSessionAsync( + new Session { Id = "owned-admin", ChannelId = "agentqi-mobile", SenderId = "owner", OwnerAccountId = ownerId }, + CancellationToken.None); + + using var metadata = new HttpRequestMessage(HttpMethod.Post, "/admin/sessions/owned-admin/metadata") { Content = JsonContent("""{"starred":true}""") }; + metadata.Headers.Authorization = new AuthenticationHeaderValue("Bearer", ownerToken); + Assert.Equal(HttpStatusCode.OK, (await harness.Client.SendAsync(metadata)).StatusCode); + + using var delete = new HttpRequestMessage(HttpMethod.Delete, "/admin/sessions/owned-admin"); + delete.Headers.Authorization = new AuthenticationHeaderValue("Bearer", adminToken); + Assert.Equal(HttpStatusCode.OK, (await harness.Client.SendAsync(delete)).StatusCode); + Assert.Null(await harness.MemoryStore.GetSessionAsync("owned-admin", CancellationToken.None)); + } + [Fact] public async Task ChatCompletions_WhenAccountToken_ShouldRecordSessionOwner() { diff --git a/src/OpenClaw.Tests/WebSocketEndpointsTests.cs b/src/OpenClaw.Tests/WebSocketEndpointsTests.cs index 74910dd9..d7323739 100644 --- a/src/OpenClaw.Tests/WebSocketEndpointsTests.cs +++ b/src/OpenClaw.Tests/WebSocketEndpointsTests.cs @@ -13,12 +13,13 @@ namespace OpenClaw.Tests; public sealed class WebSocketEndpointsTests { [Fact] - public void TryResolveAuthorizedUserIdForWebSocket_PrefersAuthenticatedPrincipalClaim() + public void ResolveCaller_WhenOidcPrincipal_ReturnsSubject() { var config = new GatewayConfig { AuthToken = "bootstrap-token" }; + config.Security.Oidc.Authority = "https://issuer.example"; var startup = new GatewayStartupContext { Config = config, @@ -39,14 +40,13 @@ public void TryResolveAuthorizedUserIdForWebSocket_PrefersAuthenticatedPrincipal authenticationType: "oidc")) }; - var ok = WebSocketEndpoints.TryResolveAuthorizedUserIdForWebSocket(ctx, startup, out var authenticatedUserId); + var authenticatedUserId = EndpointHelpers.ResolveCaller(ctx, startup).AccountId; - Assert.True(ok); Assert.Equal("oidc-user-1", authenticatedUserId); } [Fact] - public void TryResolveAuthorizedUserIdForWebSocket_AcceptsBrowserSessionAccountId() + public void ResolveCaller_WhenBrowserSession_ReturnsAccountId() { var storagePath = Path.Combine(Path.GetTempPath(), "openclaw-websocket-endpoint-tests", Guid.NewGuid().ToString("N")); var config = new GatewayConfig @@ -81,14 +81,50 @@ public void TryResolveAuthorizedUserIdForWebSocket_AcceptsBrowserSessionAccountI }; ctx.Request.Headers.Cookie = $"{BrowserSessionAuthService.CookieName}={ticket.SessionId}"; - var ok = WebSocketEndpoints.TryResolveAuthorizedUserIdForWebSocket(ctx, startup, out var authenticatedUserId); + var authenticatedUserId = EndpointHelpers.ResolveCaller(ctx, startup).AccountId; - Assert.True(ok); Assert.Equal("acct-browser", authenticatedUserId); } [Fact] - public void TryResolveAuthorizedUserIdForWebSocket_AcceptsAccountTokenAccountId() + public void ResolveCaller_WhenLoopbackBindRequiresAuth_ReturnsAccountId() + { + // Loopback-bound but with AlwaysRequireAuth, so callers are authenticated accounts, not open loopback. + // Losing the account here would exempt /ws turns from the session owner check. + var storagePath = Path.Combine(Path.GetTempPath(), "openclaw-websocket-endpoint-tests", Guid.NewGuid().ToString("N")); + var config = new GatewayConfig { AuthToken = "bootstrap-token" }; + config.Security.AlwaysRequireAuth = true; + var startup = new GatewayStartupContext + { + Config = config, + RuntimeState = RuntimeModeResolver.Resolve(config.Runtime, dynamicCodeSupported: true), + IsNonLoopbackBind = false + }; + + var operatorAccounts = new OperatorAccountService(storagePath, NullLogger.Instance); + var created = operatorAccounts.Create(new OperatorAccountCreateRequest + { + Username = "loopback-user", + Password = "P@ssw0rd123!", + Role = OperatorRoleNames.Operator + }); + var token = operatorAccounts.CreateToken(created.Id, new OperatorAccountTokenCreateRequest { Label = "ws-loopback" }); + + var services = new ServiceCollection(); + services.AddSingleton(new BrowserSessionAuthService(config)); + services.AddSingleton(operatorAccounts); + services.AddSingleton(new OrganizationPolicyService(storagePath, NullLogger.Instance)); + + var ctx = new DefaultHttpContext { RequestServices = services.BuildServiceProvider() }; + ctx.Request.Headers.Authorization = $"Bearer {token!.Token}"; + + var authenticatedUserId = EndpointHelpers.ResolveCaller(ctx, startup).AccountId; + + Assert.Equal(created.Id, authenticatedUserId); + } + + [Fact] + public void ResolveCaller_WhenAccountToken_ReturnsAccountId() { var storagePath = Path.Combine(Path.GetTempPath(), "openclaw-websocket-endpoint-tests", Guid.NewGuid().ToString("N")); var config = new GatewayConfig @@ -127,9 +163,8 @@ public void TryResolveAuthorizedUserIdForWebSocket_AcceptsAccountTokenAccountId( }; ctx.Request.Headers.Authorization = $"Bearer {token!.Token}"; - var ok = WebSocketEndpoints.TryResolveAuthorizedUserIdForWebSocket(ctx, startup, out var authenticatedUserId); + var authenticatedUserId = EndpointHelpers.ResolveCaller(ctx, startup).AccountId; - Assert.True(ok); Assert.Equal(created.Id, authenticatedUserId); } } \ No newline at end of file