From d54da31541762da342d3b8e262bb53e6119016cd Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Thu, 16 Jul 2026 21:44:43 -0700 Subject: [PATCH] fix(gateway): surface retryAfterMs on rate-limited auth rejections and document rate limiting (#109630) * fix(gateway): surface retryAfterMs on rate-limited auth rejections and document rate limiting * docs: refresh generated docs map * chore: defer release-owned changelog entry --- docs/docs.json | 1 + docs/docs_map.md | 7 ++ docs/gateway/configuration.md | 3 +- docs/gateway/security/rate-limiting.md | 92 +++++++++++++++++++ .../server/ws-connection/connect-auth.ts | 8 ++ ...essage-handler.post-connect-health.test.ts | 71 +++++++++++++- 6 files changed, 180 insertions(+), 2 deletions(-) create mode 100644 docs/gateway/security/rate-limiting.md diff --git a/docs/docs.json b/docs/docs.json index 4a4cbf0cdcd9..962ba7f0864e 100644 --- a/docs/docs.json +++ b/docs/docs.json @@ -1663,6 +1663,7 @@ "gateway/security/secure-file-operations", "gateway/security/shrinkwrap", "gateway/security/audit-checks", + "gateway/security/rate-limiting", "gateway/operator-scopes", "gateway/sandboxing", "gateway/openshell", diff --git a/docs/docs_map.md b/docs/docs_map.md index 143adcb74ef6..99f393980e2f 100644 --- a/docs/docs_map.md +++ b/docs/docs_map.md @@ -3990,6 +3990,13 @@ Do not edit it by hand; run `pnpm docs:map:gen`. - H2: Secret scanning - H2: Reporting security issues +## gateway/security/rate-limiting.md + +- Route: /gateway/security/rate-limiting +- Headings: + - H2: Authentication attempts (pre-auth) + - H2: Control-plane writes (post-auth backstop) + ## gateway/security/secure-file-operations.md - Route: /gateway/security/secure-file-operations diff --git a/docs/gateway/configuration.md b/docs/gateway/configuration.md index 18be283def8e..f73e3932c906 100644 --- a/docs/gateway/configuration.md +++ b/docs/gateway/configuration.md @@ -611,7 +611,8 @@ subsystem references. Control-plane writes (`config.apply`, `config.patch`, `update.run`) are -rate-limited to 3 requests per 60 seconds per `deviceId+clientIp`. Restart +rate-limited to 30 requests per 60 seconds, per method, per +`deviceId+clientIp`; see [Rate limiting](/gateway/security/rate-limiting). Restart requests coalesce and then enforce a 30-second cooldown between restart cycles. `update.status` is read-only but admin-scoped because the restart sentinel can include update step summaries and command output tails. diff --git a/docs/gateway/security/rate-limiting.md b/docs/gateway/security/rate-limiting.md new file mode 100644 index 000000000000..f3a4f509329b --- /dev/null +++ b/docs/gateway/security/rate-limiting.md @@ -0,0 +1,92 @@ +--- +summary: "Gateway request rate limiting: pre-auth brute-force lockout and the control-plane write backstop" +read_when: + - A client sees `rate limit exceeded for ` or `AUTH_RATE_LIMITED` errors + - You want to tune or disable `gateway.auth.rateLimit` + - You are reasoning about brute-force protection on an exposed Gateway +title: "Rate limiting" +--- + +The Gateway enforces two independent rate limiters. They protect different +boundaries and fail with different error shapes. + +## Authentication attempts (pre-auth) + +Failed authentication attempts are throttled per client IP, before any +request handling. This is the brute-force guard for exposed Gateways. + +- Only _wrong_ credentials count. Missing credentials (a client that never + sent a token) and successful authentications do not consume budget; a + successful auth resets the counter for that IP. +- Defaults: 10 failures per 60 seconds, then a 5 minute lockout for that IP. +- Loopback (`127.0.0.1` / `::1`) is exempt by default so local CLI sessions + cannot be locked out. +- Counters are scoped per credential class (shared token/password, device + token, node pairing, bootstrap token, hook auth, ...), so a flood against + one surface does not displace another. + +While locked out, connection attempts fail with: + +```json +{ + "code": "INVALID_REQUEST", + "message": "unauthorized: too many failed authentication attempts (retry later)", + "retryable": true, + "retryAfterMs": 297000, + "details": { + "code": "AUTH_RATE_LIMITED", + "authReason": "rate_limited", + "recommendedNextStep": "wait_then_retry" + } +} +``` + +Attempts from other IPs (including loopback) are unaffected during a lockout. + +Tune it under `gateway.auth.rateLimit` in `openclaw.json`: + +```json +{ + "gateway": { + "auth": { + "rateLimit": { + "maxAttempts": 10, + "windowMs": 60000, + "lockoutMs": 300000, + "exemptLoopback": true + } + } + } +} +``` + +Repeated `AUTH_RATE_LIMITED` entries in the Gateway log mean someone is +guessing credentials; see the [exposure runbook](/gateway/security/exposure-runbook). + +## Control-plane writes (post-auth backstop) + +Write-side admin RPCs (`config.apply`, `config.patch`, `plugins.install`, +`plugins.setEnabled`, `plugins.uninstall`, `update.run`, `worktrees.*`, +`gateway.restart.request`, ...) are additionally rate-limited **after** +authorization: 30 requests per 60 seconds, per method, per +`deviceId+clientIp`. + +This is not a security boundary — callers already hold `operator.admin` — it +is a backstop that bounds runaway client or agent loops hammering expensive +operations. Interactive use never hits it; each method has its own bucket, so +toggling a plugin does not consume the budget of config writes. + +When exceeded, the request fails with a retryable error: + +```json +{ + "code": "UNAVAILABLE", + "message": "rate limit exceeded for config.patch; retry after 35s", + "retryable": true, + "retryAfterMs": 34539, + "details": { "method": "config.patch", "limit": "30 per 60s" } +} +``` + +Clients should honor `retryAfterMs`. The limit is fixed (not configurable); +buckets expire on their own and are pruned by Gateway maintenance. diff --git a/src/gateway/server/ws-connection/connect-auth.ts b/src/gateway/server/ws-connection/connect-auth.ts index c2992c68af5e..7cc362c7c993 100644 --- a/src/gateway/server/ws-connection/connect-auth.ts +++ b/src/gateway/server/ws-connection/connect-auth.ts @@ -195,6 +195,14 @@ export async function authenticateGatewayConnect( client: connectParams.client, }); sendHandshakeErrorResponse(ErrorCodes.INVALID_REQUEST, authMessage, { + ...(failedAuth.rateLimited === true + ? { + retryable: true, + ...(failedAuth.retryAfterMs !== undefined + ? { retryAfterMs: failedAuth.retryAfterMs } + : {}), + } + : {}), details: { code: resolveAuthConnectErrorDetailCode(failedAuth.reason), authReason: failedAuth.reason, diff --git a/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts b/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts index 8f06224b0a57..d9c2797f86f1 100644 --- a/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts +++ b/src/gateway/server/ws-connection/message-handler.post-connect-health.test.ts @@ -2,7 +2,8 @@ import type { IncomingMessage } from "node:http"; import { beforeEach, describe, expect, it, vi } from "vitest"; import type { WebSocket } from "ws"; -import { PROTOCOL_VERSION } from "../../../../packages/gateway-protocol/src/index.js"; +import { ConnectErrorDetailCodes } from "../../../../packages/gateway-protocol/src/connect-error-details.js"; +import { ErrorCodes, PROTOCOL_VERSION } from "../../../../packages/gateway-protocol/src/index.js"; import type { HealthSummary } from "../../../commands/health.types.js"; import { onInternalDiagnosticEvent, @@ -10,6 +11,7 @@ import { type DiagnosticSecurityEvent, } from "../../../infra/diagnostic-events.js"; import { mintAgentRuntimeIdentityToken } from "../../agent-runtime-identity-token.js"; +import type { AuthRateLimiter } from "../../auth-rate-limit.js"; import type { ResolvedGatewayAuth } from "../../auth.js"; import { getOperatorApprovalRuntimeToken } from "../../operator-approval-runtime-token.js"; import { handleGatewayRequest } from "../../server-methods.js"; @@ -184,6 +186,7 @@ function attachGatewayHarness(options: { remoteAddr?: string; localAddr?: string; resolvedAuth?: ResolvedGatewayAuth; + rateLimiter?: AuthRateLimiter; client?: unknown; close?: CloseGatewayConnection; isClosed?: () => boolean; @@ -229,6 +232,7 @@ function attachGatewayHarness(options: { requestOrigin: options.requestOrigin, connectNonce: options.connectNonce, getResolvedAuth: () => resolvedAuth, + rateLimiter: options.rateLimiter, gatewayMethods: [], events: [], extraHandlers: {}, @@ -259,6 +263,7 @@ function attachGatewayHarness(options: { const sendMessage = onMessage; return { advanceHandshakePhase, + send, socketSend, sendRequest: (id: string, method: string, params: Record = {}) => { sendMessage( @@ -585,6 +590,70 @@ describe("attachGatewayWsMessageHandler post-connect health refresh", () => { }); expect(JSON.stringify(captured.events)).not.toContain("wrong-token"); expect(JSON.stringify(captured.events)).not.toContain("gateway-token"); + const response = harness.send.mock.calls.at(0)?.[0] as + | { error?: Record } + | undefined; + expect(response?.error).not.toHaveProperty("retryable"); + expect(response?.error).not.toHaveProperty("retryAfterMs"); + }); + + it("returns retry timing when gateway auth is rate-limited", async () => { + const retryAfterMs = 15_000; + const rateLimiter: AuthRateLimiter = { + check: vi.fn(() => ({ allowed: false, remaining: 0, retryAfterMs })), + recordFailure: vi.fn(), + reset: vi.fn(), + size: vi.fn(() => 0), + prune: vi.fn(), + dispose: vi.fn(), + }; + const close = createCloseMock(); + const harness = attachGatewayHarness({ + connId: "conn-auth-rate-limited", + connectNonce: "nonce-auth-rate-limited", + requestHost: "gateway.example.com:18789", + remoteAddr: "203.0.113.51", + resolvedAuth: { + mode: "token", + token: "test-token", + allowTailscale: false, + }, + rateLimiter, + close, + }); + + harness.sendConnect("connect-auth-rate-limited", { + minProtocol: PROTOCOL_VERSION, + maxProtocol: PROTOCOL_VERSION, + client: { + id: "gateway-client", + version: "dev", + platform: "test", + mode: "backend", + }, + role: "operator", + scopes: [], + caps: [], + auth: { token: "test-token" }, + }); + + await vi.waitFor(() => { + expect(close).toHaveBeenCalledWith(1008, expect.stringContaining("retry later")); + }); + + const response = harness.send.mock.calls.at(0)?.[0] as + | { error?: Record } + | undefined; + expect(response?.error).toMatchObject({ + code: ErrorCodes.INVALID_REQUEST, + message: "unauthorized: too many failed authentication attempts (retry later)", + retryable: true, + details: { + code: ConnectErrorDetailCodes.AUTH_RATE_LIMITED, + authReason: "rate_limited", + }, + }); + expect(response?.error?.retryAfterMs).toBeGreaterThan(0); }); it("records credential and hello preparation phases during connect", async () => {