From 0e702f106313c1c63a32a6e7b3dbb5e96e620656 Mon Sep 17 00:00:00 2001 From: Pavan Kumar Gondhi Date: Mon, 4 May 2026 23:16:07 +0530 Subject: [PATCH] fix(gateway): clamp unbound websocket auth scopes [AI] (#77413) * fix: clamp unapproved trusted proxy websocket scopes * addressing claude review * addressing claude review * addressing ci * addressing ci * docs: add changelog entry for PR merge --- CHANGELOG.md | 1 + .../server-methods/config-write-flow.ts | 46 ++++++- .../server-methods/config.shared-auth.test.ts | 117 ++++++++++++++++++ .../server.auth.compat-baseline.test.ts | 88 ++++++++++++- src/gateway/server.auth.control-ui.suite.ts | 62 ++++++++++ src/gateway/server.impl.ts | 7 +- src/gateway/server/ws-connection.ts | 6 +- .../server/ws-connection/message-handler.ts | 30 +++-- .../server/ws-shared-generation.test.ts | 57 +++++++++ src/gateway/server/ws-shared-generation.ts | 38 +++++- 10 files changed, 435 insertions(+), 17 deletions(-) create mode 100644 src/gateway/server/ws-shared-generation.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 3b517c21ba4..3eaf4e39e7c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- fix(gateway): clamp unbound websocket auth scopes [AI]. (#77413) Thanks @pgondhi987. - Gate zalouser startup name matching [AI]. (#77411) Thanks @pgondhi987. - fix(device-pair): require pairing scope for pair command [AI]. (#76377) Thanks @pgondhi987. - fix(qqbot): keep private commands off framework surface [AI]. (#77212) Thanks @pgondhi987. diff --git a/src/gateway/server-methods/config-write-flow.ts b/src/gateway/server-methods/config-write-flow.ts index 522a8988335..deffd26af02 100644 --- a/src/gateway/server-methods/config-write-flow.ts +++ b/src/gateway/server-methods/config-write-flow.ts @@ -13,7 +13,7 @@ import { } from "../../infra/restart-sentinel.js"; import { scheduleGatewaySigusr1Restart } from "../../infra/restart.js"; import { getActiveSecretsRuntimeSnapshot } from "../../secrets/runtime.js"; -import { resolveEffectiveSharedGatewayAuth } from "../auth.js"; +import { resolveEffectiveSharedGatewayAuth, resolveGatewayAuth } from "../auth.js"; import { buildGatewayReloadPlan } from "../config-reload-plan.js"; import { resolveGatewayReloadSettings } from "../config-reload-settings.js"; import { formatControlPlaneActor, type ControlPlaneActor } from "../control-plane-audit.js"; @@ -31,7 +31,51 @@ export function resolveGatewayConfigPath(snapshot?: Pick): { + userHeader: string | undefined; + requiredHeaders: string[]; + allowUsers: string[]; + allowLoopback: boolean | undefined; +} { + return { + userHeader: auth.trustedProxy?.userHeader, + requiredHeaders: normalizeStringListForAuthCompare(auth.trustedProxy?.requiredHeaders), + allowUsers: normalizeStringListForAuthCompare(auth.trustedProxy?.allowUsers), + allowLoopback: auth.trustedProxy?.allowLoopback, + }; +} + export function didSharedGatewayAuthChange(prev: OpenClawConfig, next: OpenClawConfig): boolean { + const prevResolvedAuth = resolveGatewayAuth({ + authConfig: prev.gateway?.auth, + env: process.env, + tailscaleMode: prev.gateway?.tailscale?.mode, + }); + const nextResolvedAuth = resolveGatewayAuth({ + authConfig: next.gateway?.auth, + env: process.env, + tailscaleMode: next.gateway?.tailscale?.mode, + }); + if (prevResolvedAuth.mode === "trusted-proxy" || nextResolvedAuth.mode === "trusted-proxy") { + if (prevResolvedAuth.mode !== nextResolvedAuth.mode) { + return true; + } + return ( + !isDeepStrictEqual( + normalizeTrustedProxyAuthForCompare(prevResolvedAuth), + normalizeTrustedProxyAuthForCompare(nextResolvedAuth), + ) || + !isDeepStrictEqual( + normalizeStringListForAuthCompare(prev.gateway?.trustedProxies), + normalizeStringListForAuthCompare(next.gateway?.trustedProxies), + ) + ); + } + const prevAuth = resolveEffectiveSharedGatewayAuth({ authConfig: prev.gateway?.auth, env: process.env, diff --git a/src/gateway/server-methods/config.shared-auth.test.ts b/src/gateway/server-methods/config.shared-auth.test.ts index cffa90c73ce..78f5e6437a5 100644 --- a/src/gateway/server-methods/config.shared-auth.test.ts +++ b/src/gateway/server-methods/config.shared-auth.test.ts @@ -168,6 +168,123 @@ describe("config shared auth disconnects", () => { expect(disconnectClientsUsingSharedGatewayAuth).not.toHaveBeenCalled(); }); + it("disconnects gateway-auth clients when active trusted-proxy policy changes", async () => { + const prevConfig: OpenClawConfig = { + gateway: { + auth: { + mode: "trusted-proxy", + trustedProxy: { + userHeader: "x-forwarded-user", + allowUsers: ["alice@example.com"], + }, + }, + trustedProxies: ["127.0.0.1"], + }, + }; + readConfigFileSnapshotForWriteMock.mockResolvedValue(createConfigWriteSnapshot(prevConfig)); + + const { options, disconnectClientsUsingSharedGatewayAuth } = createConfigHandlerHarness({ + method: "config.patch", + params: { + baseHash: "base-hash", + raw: JSON.stringify({ + gateway: { + auth: { + trustedProxy: { + userHeader: "x-forwarded-user", + allowUsers: ["bob@example.com"], + }, + }, + }, + }), + restartDelayMs: 1_000, + }, + }); + + await configHandlers["config.patch"](options); + await flushConfigHandlerMicrotasks(); + + expect(scheduleGatewaySigusr1RestartMock).not.toHaveBeenCalled(); + expect(disconnectClientsUsingSharedGatewayAuth).toHaveBeenCalledTimes(1); + }); + + it("disconnects gateway-auth clients when trusted-proxy source list changes", async () => { + const prevConfig: OpenClawConfig = { + gateway: { + auth: { + mode: "trusted-proxy", + trustedProxy: { + userHeader: "x-forwarded-user", + }, + }, + trustedProxies: ["127.0.0.1"], + }, + }; + readConfigFileSnapshotForWriteMock.mockResolvedValue(createConfigWriteSnapshot(prevConfig)); + + const { options, disconnectClientsUsingSharedGatewayAuth } = createConfigHandlerHarness({ + method: "config.patch", + params: { + baseHash: "base-hash", + raw: JSON.stringify({ + gateway: { + trustedProxies: ["10.0.0.10"], + }, + }), + restartDelayMs: 1_000, + }, + }); + + await configHandlers["config.patch"](options); + await flushConfigHandlerMicrotasks(); + + expect(scheduleGatewaySigusr1RestartMock).not.toHaveBeenCalled(); + expect(disconnectClientsUsingSharedGatewayAuth).toHaveBeenCalledTimes(1); + }); + + it("does not disconnect gateway-auth clients when trusted-proxy lists are reordered", async () => { + const prevConfig: OpenClawConfig = { + gateway: { + auth: { + mode: "trusted-proxy", + trustedProxy: { + userHeader: "x-forwarded-user", + requiredHeaders: ["x-forwarded-proto", "x-forwarded-host"], + allowUsers: ["alice@example.com", "bob@example.com"], + }, + }, + trustedProxies: ["127.0.0.1", "10.0.0.10"], + }, + }; + readConfigFileSnapshotForWriteMock.mockResolvedValue(createConfigWriteSnapshot(prevConfig)); + + const { options, disconnectClientsUsingSharedGatewayAuth } = createConfigHandlerHarness({ + method: "config.patch", + params: { + baseHash: "base-hash", + raw: JSON.stringify({ + gateway: { + auth: { + trustedProxy: { + userHeader: "x-forwarded-user", + requiredHeaders: ["x-forwarded-host", "x-forwarded-proto"], + allowUsers: ["bob@example.com", "alice@example.com"], + }, + }, + trustedProxies: ["10.0.0.10", "127.0.0.1"], + }, + }), + restartDelayMs: 1_000, + }, + }); + + await configHandlers["config.patch"](options); + await flushConfigHandlerMicrotasks(); + + expect(scheduleGatewaySigusr1RestartMock).not.toHaveBeenCalled(); + expect(disconnectClientsUsingSharedGatewayAuth).not.toHaveBeenCalled(); + }); + it("still schedules a direct restart for hot mode when the reloader cannot apply the change", async () => { const prevConfig: OpenClawConfig = { gateway: { diff --git a/src/gateway/server.auth.compat-baseline.test.ts b/src/gateway/server.auth.compat-baseline.test.ts index 7542b03c2f2..285f021a855 100644 --- a/src/gateway/server.auth.compat-baseline.test.ts +++ b/src/gateway/server.auth.compat-baseline.test.ts @@ -6,7 +6,9 @@ import { connectReq, CONTROL_UI_CLIENT, ConnectErrorDetailCodes, + createSignedDevice, getFreePort, + readConnectChallengeNonce, openWs, originForPort, rpcReq, @@ -312,7 +314,7 @@ describe("gateway auth compatibility baseline", () => { testState.gatewayAuth = { mode: "none" }; delete process.env.OPENCLAW_GATEWAY_TOKEN; port = await getFreePort(); - server = await startGatewayServer(port); + server = await startGatewayServer(port, { controlUiEnabled: true }); }); afterAll(async () => { @@ -329,5 +331,89 @@ describe("gateway auth compatibility baseline", () => { ws.close(); } }); + + test("keeps auth-none control ui first-connect token absence unchanged", async () => { + const ws = await openWs(port, { origin: originForPort(port) }); + try { + const deviceIdentityPath = path.join( + os.tmpdir(), + `openclaw-auth-none-control-ui-first-${process.pid}-${port}.json`, + ); + const res = await connectReq(ws, { + skipDefaultAuth: true, + client: { ...CONTROL_UI_CLIENT }, + scopes: ["operator.read"], + deviceIdentityPath, + }); + expect(res.ok).toBe(true); + const helloOk = res.payload as + | { + auth?: { + deviceToken?: unknown; + }; + } + | undefined; + expect(helloOk?.auth?.deviceToken).toBeUndefined(); + } finally { + ws.close(); + } + }); + + test("keeps auth-none control ui stale-key token handoff unchanged", async () => { + const ws = await openWs(port, { origin: originForPort(port) }); + try { + const { loadOrCreateDeviceIdentity, publicKeyRawBase64UrlFromPem } = + await import("../infra/device-identity.js"); + const { approveDevicePairing, requestDevicePairing } = + await import("../infra/device-pairing.js"); + const nonce = await readConnectChallengeNonce(ws); + const identityPath = path.join( + os.tmpdir(), + `openclaw-auth-none-control-ui-${process.pid}-${port}.json`, + ); + const staleIdentityPath = path.join( + os.tmpdir(), + `openclaw-auth-none-control-ui-stale-${process.pid}-${port}.json`, + ); + const { identity, device } = await createSignedDevice({ + token: null, + scopes: ["operator.read"], + clientId: CONTROL_UI_CLIENT.id, + clientMode: CONTROL_UI_CLIENT.mode, + identityPath, + nonce, + }); + const staleIdentity = loadOrCreateDeviceIdentity(staleIdentityPath); + const pending = await requestDevicePairing({ + deviceId: identity.deviceId, + publicKey: publicKeyRawBase64UrlFromPem(staleIdentity.publicKeyPem), + clientId: CONTROL_UI_CLIENT.id, + clientMode: CONTROL_UI_CLIENT.mode, + role: "operator", + scopes: ["operator.read"], + }); + await approveDevicePairing(pending.request.requestId, { + callerScopes: ["operator.admin"], + }); + + const res = await connectReq(ws, { + skipDefaultAuth: true, + client: { ...CONTROL_UI_CLIENT }, + scopes: ["operator.read"], + device, + }); + expect(res.ok).toBe(true); + const helloOk = res.payload as + | { + auth?: { + deviceToken?: unknown; + }; + } + | undefined; + expect(typeof helloOk?.auth?.deviceToken).toBe("string"); + } finally { + ws.close(); + } + }); }); }); diff --git a/src/gateway/server.auth.control-ui.suite.ts b/src/gateway/server.auth.control-ui.suite.ts index bc2048bb856..a8589113cd5 100644 --- a/src/gateway/server.auth.control-ui.suite.ts +++ b/src/gateway/server.auth.control-ui.suite.ts @@ -314,6 +314,68 @@ export function registerControlUiAndPairingSuite(): void { }); }); + test("clamps trusted-proxy control ui scopes for unpaired device identity", async () => { + const { replaceConfigFile } = await import("../config/config.js"); + testState.gatewayAuth = undefined; + testState.gatewayControlUi = { + ...testState.gatewayControlUi, + allowedOrigins: ["https://localhost"], + }; + await replaceConfigFile({ + nextConfig: { + gateway: { + auth: { + mode: "trusted-proxy", + trustedProxy: { + userHeader: "x-forwarded-user", + requiredHeaders: ["x-forwarded-proto"], + allowLoopback: true, + }, + }, + trustedProxies: ["127.0.0.1"], + controlUi: { + allowedOrigins: ["https://localhost"], + }, + }, + }, + afterWrite: { mode: "auto" }, + }); + await withControlUiGatewayServer(async ({ port }) => { + const ws = await openWs(port, TRUSTED_PROXY_CONTROL_UI_HEADERS); + try { + const challengeNonce = await readConnectChallengeNonce(ws); + const { device } = await createSignedDevice({ + token: null, + role: "operator", + scopes: ["operator.admin"], + clientId: CONTROL_UI_CLIENT.id, + clientMode: CONTROL_UI_CLIENT.mode, + nonce: challengeNonce, + }); + const res = await connectReq(ws, { + skipDefaultAuth: true, + scopes: ["operator.admin"], + device, + client: { ...CONTROL_UI_CLIENT }, + }); + expect(res.ok).toBe(true); + const payload = res.payload as + | { + auth?: { scopes?: string[]; deviceToken?: string }; + } + | undefined; + expect(payload?.auth?.scopes).toEqual([]); + expect(payload?.auth?.deviceToken).toBeUndefined(); + + const admin = await rpcReq(ws, "set-heartbeats", { enabled: false }); + expect(admin.ok).toBe(false); + expect(admin.error?.message ?? "").toContain("missing scope"); + } finally { + ws.close(); + } + }); + }); + test("allows localhost ui clients without device identity when insecure auth is enabled", async () => { testState.gatewayControlUi = { allowInsecureAuth: true }; const { server, ws, port, prevToken } = await startControlUiServerWithClient("secret", { diff --git a/src/gateway/server.impl.ts b/src/gateway/server.impl.ts index 8c92fa07008..ce3ba130d75 100644 --- a/src/gateway/server.impl.ts +++ b/src/gateway/server.impl.ts @@ -735,9 +735,13 @@ export async function startGatewayServer( env: process.env, tailscaleMode, }), + config.gateway?.trustedProxies, ); const resolveCurrentSharedGatewaySessionGeneration = () => - resolveSharedGatewaySessionGeneration(getResolvedAuth()); + resolveSharedGatewaySessionGeneration( + getResolvedAuth(), + getRuntimeConfig().gateway?.trustedProxies, + ); const resolveSharedGatewaySessionGenerationForRuntimeSnapshot = () => resolveSharedGatewaySessionGeneration( resolveGatewayAuth({ @@ -746,6 +750,7 @@ export async function startGatewayServer( env: process.env, tailscaleMode, }), + getRuntimeConfig().gateway?.trustedProxies, ); const sharedGatewaySessionGenerationState: SharedGatewaySessionGenerationState = { current: resolveCurrentSharedGatewaySessionGeneration(), diff --git a/src/gateway/server/ws-connection.ts b/src/gateway/server/ws-connection.ts index c36aa488906..7ea32a99e42 100644 --- a/src/gateway/server/ws-connection.ts +++ b/src/gateway/server/ws-connection.ts @@ -1,6 +1,7 @@ import { randomUUID } from "node:crypto"; import type { Socket } from "node:net"; import type { RawData, WebSocket, WebSocketServer } from "ws"; +import { getRuntimeConfig } from "../../config/io.js"; import { resolveCanvasHostUrl } from "../../infra/canvas-host-url.js"; import { removeRemoteNodeInfo } from "../../infra/skills-remote.js"; import { upsertPresence } from "../../infra/system-presence.js"; @@ -205,7 +206,10 @@ export function attachGatewayWsConnectionHandler(params: AttachGatewayWsConnecti resolvedAuth, getResolvedAuth = () => resolvedAuth, getRequiredSharedGatewaySessionGeneration = () => - resolveSharedGatewaySessionGeneration(getResolvedAuth()), + resolveSharedGatewaySessionGeneration( + getResolvedAuth(), + getRuntimeConfig().gateway?.trustedProxies, + ), rateLimiter, browserRateLimiter, isStartupPending, diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index 9cd048aa56d..84b35be9118 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -837,9 +837,11 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar rejectUnauthorized(authResult); return; } - if (authMethod === "token" || authMethod === "password") { - const sharedGatewaySessionGeneration = - resolveSharedGatewaySessionGeneration(resolvedAuth); + if (authMethod === "token" || authMethod === "password" || authMethod === "trusted-proxy") { + const sharedGatewaySessionGeneration = resolveSharedGatewaySessionGeneration( + resolvedAuth, + trustedProxies, + ); const requiredSharedGatewaySessionGeneration = getRequiredSharedGatewaySessionGeneration?.(); if ( @@ -874,6 +876,7 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar resolvedAuth.mode, authMethod, ); + let hasServerApprovedDeviceTokenBaseline = false; if (device && devicePublicKey) { const formatAuditList = (items: string[] | undefined): string => { if (!items || items.length === 0) { @@ -1133,8 +1136,17 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar if (!ok) { return; } + hasServerApprovedDeviceTokenBaseline = true; + } else if (trustedProxyAuthOk) { + clearUnboundScopes(); + } else if ( + skipControlUiPairingForDevice || + (skipLocalBackendSelfPairing && authMethod !== "device-token") + ) { + hasServerApprovedDeviceTokenBaseline = true; } } else { + hasServerApprovedDeviceTokenBaseline = true; const claimedPlatform = connectParams.client.platform; const pairedPlatform = paired.platform; const claimedDeviceFamily = connectParams.client.deviceFamily; @@ -1222,9 +1234,10 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar } } - const deviceToken = device - ? await ensureDeviceToken({ deviceId: device.id, role, scopes }) - : null; + const deviceToken = + device && hasServerApprovedDeviceTokenBaseline + ? await ensureDeviceToken({ deviceId: device.id, role, scopes }) + : null; const bootstrapDeviceTokens: Array<{ deviceToken: string; role: string; @@ -1303,9 +1316,10 @@ export function attachGatewayWsMessageHandler(params: GatewayWsMessageHandlerPar const canvasCapabilityExpiresAtMs = canvasCapability ? Date.now() + CANVAS_CAPABILITY_TTL_MS : undefined; - const usesSharedGatewayAuth = authMethod === "token" || authMethod === "password"; + const usesSharedGatewayAuth = + authMethod === "token" || authMethod === "password" || authMethod === "trusted-proxy"; const sharedGatewaySessionGeneration = usesSharedGatewayAuth - ? resolveSharedGatewaySessionGeneration(resolvedAuth) + ? resolveSharedGatewaySessionGeneration(resolvedAuth, trustedProxies) : undefined; const scopedCanvasHostUrl = canvasHostUrl && canvasCapability diff --git a/src/gateway/server/ws-shared-generation.test.ts b/src/gateway/server/ws-shared-generation.test.ts new file mode 100644 index 00000000000..9bc2f32b4f2 --- /dev/null +++ b/src/gateway/server/ws-shared-generation.test.ts @@ -0,0 +1,57 @@ +import { describe, expect, it } from "vitest"; +import { resolveSharedGatewaySessionGeneration } from "./ws-shared-generation.js"; + +describe("resolveSharedGatewaySessionGeneration", () => { + it("tracks trusted-proxy policy inputs", () => { + const baseAuth = { + mode: "trusted-proxy" as const, + allowTailscale: false, + trustedProxy: { + userHeader: "x-forwarded-user", + requiredHeaders: ["x-forwarded-proto", "x-forwarded-host"], + allowUsers: ["alice@example.com", "bob@example.com"], + }, + }; + + const base = resolveSharedGatewaySessionGeneration(baseAuth, ["127.0.0.1", "10.0.0.10"]); + expect(base).toBeDefined(); + expect( + resolveSharedGatewaySessionGeneration( + { + ...baseAuth, + trustedProxy: { + ...baseAuth.trustedProxy, + requiredHeaders: ["x-forwarded-host", "x-forwarded-proto"], + allowUsers: ["bob@example.com", "alice@example.com"], + }, + }, + ["10.0.0.10", "127.0.0.1"], + ), + ).toBe(base); + expect( + resolveSharedGatewaySessionGeneration( + { + ...baseAuth, + trustedProxy: { + ...baseAuth.trustedProxy, + allowUsers: ["carol@example.com"], + }, + }, + ["127.0.0.1", "10.0.0.10"], + ), + ).not.toBe(base); + expect(resolveSharedGatewaySessionGeneration(baseAuth, ["10.0.0.11"])).not.toBe(base); + }); + + it("keeps shared-secret generations independent from proxy allowlists", () => { + const auth = { + mode: "token" as const, + allowTailscale: false, + token: "shared-token", + }; + + expect(resolveSharedGatewaySessionGeneration(auth, ["127.0.0.1"])).toBe( + resolveSharedGatewaySessionGeneration(auth, ["10.0.0.10"]), + ); + }); +}); diff --git a/src/gateway/server/ws-shared-generation.ts b/src/gateway/server/ws-shared-generation.ts index 27890458594..1003a25f612 100644 --- a/src/gateway/server/ws-shared-generation.ts +++ b/src/gateway/server/ws-shared-generation.ts @@ -1,4 +1,5 @@ import { createHash } from "node:crypto"; +import type { GatewayTrustedProxyConfig } from "../../config/types.gateway.js"; import type { ResolvedGatewayAuth } from "../auth.js"; function resolveSharedSecret( @@ -18,14 +19,41 @@ function resolveSharedSecret( return null; } +function normalizeTrustedProxyConfig(trustedProxy: GatewayTrustedProxyConfig | undefined): { + userHeader: string | undefined; + requiredHeaders: string[]; + allowUsers: string[]; + allowLoopback: boolean | undefined; +} { + return { + userHeader: trustedProxy?.userHeader, + requiredHeaders: [...(trustedProxy?.requiredHeaders ?? [])].toSorted(), + allowUsers: [...(trustedProxy?.allowUsers ?? [])].toSorted(), + allowLoopback: trustedProxy?.allowLoopback, + }; +} + export function resolveSharedGatewaySessionGeneration( auth: ResolvedGatewayAuth, + trustedProxies?: readonly string[], ): string | undefined { const shared = resolveSharedSecret(auth); - if (!shared) { - return undefined; + if (shared) { + return createHash("sha256") + .update(`${shared.mode}\u0000${shared.secret}`, "utf8") + .digest("base64url"); } - return createHash("sha256") - .update(`${shared.mode}\u0000${shared.secret}`, "utf8") - .digest("base64url"); + if (auth.mode === "trusted-proxy") { + return createHash("sha256") + .update( + JSON.stringify({ + mode: auth.mode, + trustedProxy: normalizeTrustedProxyConfig(auth.trustedProxy), + trustedProxies: [...(trustedProxies ?? [])].toSorted(), + }), + "utf8", + ) + .digest("base64url"); + } + return undefined; }