mirror of
https://github.com/openclaw/openclaw.git
synced 2026-07-25 15:11:14 +00:00
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
This commit is contained in:
committed by
GitHub
parent
9ce8ac94b9
commit
d54da31541
@@ -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",
|
||||
|
||||
@@ -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
|
||||
|
||||
@@ -611,7 +611,8 @@ subsystem references.
|
||||
|
||||
<Note>
|
||||
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.
|
||||
|
||||
92
docs/gateway/security/rate-limiting.md
Normal file
92
docs/gateway/security/rate-limiting.md
Normal file
@@ -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 <method>` 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.
|
||||
@@ -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,
|
||||
|
||||
@@ -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<string, unknown> = {}) => {
|
||||
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<string, unknown> }
|
||||
| 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<string, unknown> }
|
||||
| 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 () => {
|
||||
|
||||
Reference in New Issue
Block a user