openclaw/openclaw-windows-node
openclaw/openclaw-windows-node
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 4 Wiki Activity

fix(security): require MCP auth before method dispatch

Browse Source
This commit is contained in:
Vincent Koc 2026-04-30 02:36:14 -07:00
parent 5ef3707509
commit 424f69083b
No known key found for this signature in database
3 changed files with 51 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

10
docs/MCP_MODE.md
View File

@ -73,11 +73,11 @@ The bridge takes a `Func<IReadOnlyList<INodeCapability>>` rather than a snapshot
`OpenClaw.Shared/Mcp/McpHttpServer.cs` is `System.Net.HttpListener` bound to `http://127.0.0.1:8765/`. Loopback-only by construction; not reachable from any other machine even with firewall holes. A defensive `IPAddress.IsLoopback` check on each request acts as belt-and-suspenders.
`GET /` returns a friendly text probe. `POST /` is JSON-RPC. Anything else → `405`.
`GET /` returns a friendly text probe. `POST /` is JSON-RPC. Anything else → `405`. When a bearer token is configured, every verb must pass the token gate before method dispatch.
## Authentication
The HTTP transport requires a bearer token on every `POST`. Defense-in-depth on top of loopback bind + Origin/Host checks: if an attacker can run code in *any* local user context they can reach `127.0.0.1:8765`, so we don't want the listener to be open-by-construction.
The HTTP transport requires a bearer token on every request. Defense-in-depth on top of loopback bind + Origin/Host checks: if an attacker can run code in *any* local user context they can reach `127.0.0.1:8765`, so we don't want the listener to be open-by-construction.
**Where the token lives.** `%APPDATA%\OpenClawTray\mcp-token.txt`. The exact path is composed by `NodeService.McpTokenPath` from `SettingsManager.SettingsDirectoryPath`, so the test-suite override `OPENCLAW_TRAY_DATA_DIR` isolates the token file too. The file inherits the parent directory's ACL — by default only the current user (and SYSTEM/Administrators) can read it.
@ -87,7 +87,7 @@ The HTTP transport requires a bearer token on every `POST`. Defense-in-depth on
**Lifetime.** The token is **persistent across tray restarts**. It's only regenerated if the file is deleted or its contents are emptied. There is no automatic rotation.
**On the wire.** Every `POST /` must carry `Authorization: Bearer <token>`. Missing or wrong token → `401 Unauthorized` with no body. `GET /` is unauthenticated (it's just a "yes I'm here" probe).
**On the wire.** Every request must carry `Authorization: Bearer <token>` when the server has a configured token. Missing or wrong token → `401 Unauthorized` with no body. `GET /` remains a "yes I'm here" probe after auth passes.
**How users find it.** Settings → Developer Mode → MCP section shows the live token (masked, with Reveal/Copy buttons) and the storage path. For agents that read from disk (Claude Code, custom scripts), pointing them at `McpTokenPath` is preferable to embedding the token in their prompt or config — the path is stable, the token is a secret. For agents that only accept literal bearer values in config (Claude Desktop, Cursor), use Copy.
@ -218,8 +218,8 @@ curl -X POST http://127.0.0.1:8765/ -H "Content-Type: text/plain" --data '{"json
These should **succeed**:
```powershell
curl http://127.0.0.1:8765/ # GET probe
curl -X POST http://127.0.0.1:8765/ -H "Content-Type: application/json" -d '{"jsonrpc":"2.0","id":1,"method":"ping"}'
curl http://127.0.0.1:8765/ -H "Authorization: Bearer <token>" # GET probe
curl -X POST http://127.0.0.1:8765/ -H "Authorization: Bearer <token>" -H "Content-Type: application/json" -d '{"jsonrpc":"2.0","id":1,"method":"ping"}'
```
## What's deliberately deferred

31
src/OpenClaw.Shared/Mcp/McpHttpServer.cs
View File

@ -26,8 +26,8 @@ namespace OpenClaw.Shared.Mcp;
/// satisfy (no Access-Control-Allow-Origin), so the cross-origin call
/// fails before reaching capability code.
///
/// Bearer-token auth in front of the JSON-RPC body. Required on every POST when
/// constructed with a non-null token (the tray always passes one — see
/// Bearer-token auth in front of request dispatch. Required on every request
/// when constructed with a non-null token (the tray always passes one — see
/// <c>NodeService.McpTokenPath</c> / <c>McpAuthToken.LoadOrCreate</c>; legacy
/// callers that pass null disable the check, kept for in-process tests). The
/// token defends against untrusted local processes that could otherwise reach
@ -66,9 +66,9 @@ public sealed class McpHttpServer : IDisposable
private readonly IOpenClawLogger _logger;
private readonly HttpListener _listener;
/// <summary>
/// Required bearer token for POST requests. Empty/null disables auth (the
/// Required bearer token for HTTP requests. Empty/null disables auth (the
/// pre-auth contract — kept so existing dev configs keep working). When set,
/// every POST must carry <c>Authorization: Bearer &lt;token&gt;</c>.
/// every request must carry <c>Authorization: Bearer &lt;token&gt;</c>.
/// </summary>
private string? _authToken;
private readonly CancellationTokenSource _cts = new();
@ -227,6 +227,18 @@ public sealed class McpHttpServer : IDisposable
return;
}
// Bearer-token check. Defends against untrusted local processes
// (browser helpers, editor extensions) that share the loopback
// surface with the legitimate MCP client. Token lives in a
// user-only-readable file under %LOCALAPPDATA%; CLI/agent
// registration reads from there. Keep this before method dispatch
// so alternate verbs cannot bypass the configured token gate.
if (authToken != null && !IsAuthorized(authToken, ctx.Request.Headers["Authorization"]))
{
Reject(ctx, HttpStatusCode.Unauthorized, "missing or invalid bearer token");
return;
}
if (ctx.Request.HttpMethod == "GET")
{
// Friendly probe response — useful for confirming the server is up
@ -242,17 +254,6 @@ public sealed class McpHttpServer : IDisposable
return;
}
// Bearer-token check. Defends against untrusted local processes
// (browser helpers, editor extensions) that share the loopback
// surface with the legitimate MCP client. Token lives in a
// user-only-readable file under %LOCALAPPDATA%; CLI/agent
// registration reads from there.
if (authToken != null && !IsAuthorized(authToken, ctx.Request.Headers["Authorization"]))
{
Reject(ctx, HttpStatusCode.Unauthorized, "missing or invalid bearer token");
return;
}
// Force application/json on POST. Combined with the Origin check,
// this means a browser cross-origin fetch must use a non-simple
// Content-Type and trigger a CORS preflight, which we don't honor.

30
tests/OpenClaw.Shared.Tests/McpHttpServerTests.cs
View File

@ -415,6 +415,36 @@ public class McpHttpServerTests
Assert.Equal(System.Net.HttpStatusCode.OK, resp3.StatusCode);
}
[Fact]
public async Task Auth_RequiresBearerToken_BeforeMethodDispatch_When_TokenConfigured()
{
const string token = "supersecret";
var port = FreePort();
var bridge = new McpToolBridge(() => new INodeCapability[] { new FakeCapability() });
using var server = new McpHttpServer(bridge, port, NullLogger.Instance, token);
server.Start();
using var http = new HttpClient { BaseAddress = new Uri($"http://127.0.0.1:{port}/") };
var unauthenticatedGet = await http.GetAsync("");
Assert.Equal(System.Net.HttpStatusCode.Unauthorized, unauthenticatedGet.StatusCode);
var unauthenticatedPut = await http.PutAsync(
"",
new StringContent("{}", System.Text.Encoding.UTF8, "application/json"));
Assert.Equal(System.Net.HttpStatusCode.Unauthorized, unauthenticatedPut.StatusCode);
http.DefaultRequestHeaders.Authorization = new System.Net.Http.Headers.AuthenticationHeaderValue("Bearer", token);
var authorizedGet = await http.GetAsync("");
Assert.Equal(System.Net.HttpStatusCode.OK, authorizedGet.StatusCode);
var authorizedPut = await http.PutAsync(
"",
new StringContent("{}", System.Text.Encoding.UTF8, "application/json"));
Assert.Equal(System.Net.HttpStatusCode.MethodNotAllowed, authorizedPut.StatusCode);
}
[Fact]
public async Task Auth_NoToken_AllowsAllRequests_LegacyDevMode()
{