diff --git a/CHANGELOG.md b/CHANGELOG.md index 65c6d2990..64e8ad6bb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ Docs: https://docs.openclaw.ai ### Fixes +- Gateway/WS: close repeated post-handshake `unauthorized role:*` request floods per connection and sample duplicate rejection logs, preventing a single misbehaving client from degrading gateway responsiveness. (#20168) Thanks @acy103, @vibecodooor, and @vincentkoc. - Agents/Reasoning: when model-default thinking is active (for example `thinking=low`), keep auto-reasoning disabled unless explicitly enabled, preventing `Reasoning:` thinking-block leakage in channel replies. (#24335, #24290) thanks @Kay-051. - Auto-reply/Inbound metadata: hide direct-chat `message_id`/`message_id_full` and sender metadata only from normalized chat type (not sender-id sentinels), preserving group metadata visibility and preventing sender-id spoofed direct-mode classification. (#24373) thanks @jd316. - Security/Exec: detect obfuscated commands before exec allowlist decisions and require explicit approval for obfuscation patterns. (#8592) Thanks @CornBrother0x and @vincentkoc. diff --git a/src/gateway/server/ws-connection/message-handler.ts b/src/gateway/server/ws-connection/message-handler.ts index e8f8659a9..eb62269c8 100644 --- a/src/gateway/server/ws-connection/message-handler.ts +++ b/src/gateway/server/ws-connection/message-handler.ts @@ -78,6 +78,7 @@ import { resolveControlUiAuthPolicy, shouldSkipControlUiPairing, } from "./connect-policy.js"; +import { isUnauthorizedRoleError, UnauthorizedFloodGuard } from "./unauthorized-flood-guard.js"; type SubsystemLogger = ReturnType; @@ -190,6 +191,7 @@ export function attachGatewayWsMessageHandler(params: { } const isWebchatConnect = (p: ConnectParams | null | undefined) => isWebchatClient(p?.client); + const unauthorizedFloodGuard = new UnauthorizedFloodGuard(); socket.on("message", async (data) => { if (isClosed()) { @@ -908,6 +910,33 @@ export function attachGatewayWsMessageHandler(params: { meta?: Record, ) => { send({ type: "res", id: req.id, ok, payload, error }); + const unauthorizedRoleError = isUnauthorizedRoleError(error); + let logMeta = meta; + if (unauthorizedRoleError) { + const unauthorizedDecision = unauthorizedFloodGuard.registerUnauthorized(); + if (unauthorizedDecision.suppressedSinceLastLog > 0) { + logMeta = { + ...logMeta, + suppressedUnauthorizedResponses: unauthorizedDecision.suppressedSinceLastLog, + }; + } + if (!unauthorizedDecision.shouldLog) { + return; + } + if (unauthorizedDecision.shouldClose) { + setCloseCause("repeated-unauthorized-requests", { + unauthorizedCount: unauthorizedDecision.count, + method: req.method, + }); + queueMicrotask(() => close(1008, "repeated unauthorized calls")); + } + logMeta = { + ...logMeta, + unauthorizedCount: unauthorizedDecision.count, + }; + } else { + unauthorizedFloodGuard.reset(); + } logWs("out", "res", { connId, id: req.id, @@ -915,7 +944,7 @@ export function attachGatewayWsMessageHandler(params: { method: req.method, errorCode: error?.code, errorMessage: error?.message, - ...meta, + ...logMeta, }); }; diff --git a/src/gateway/server/ws-connection/unauthorized-flood-guard.test.ts b/src/gateway/server/ws-connection/unauthorized-flood-guard.test.ts new file mode 100644 index 000000000..8c750570d --- /dev/null +++ b/src/gateway/server/ws-connection/unauthorized-flood-guard.test.ts @@ -0,0 +1,67 @@ +import { describe, expect, it } from "vitest"; +import { ErrorCodes, errorShape } from "../../protocol/index.js"; +import { isUnauthorizedRoleError, UnauthorizedFloodGuard } from "./unauthorized-flood-guard.js"; + +describe("UnauthorizedFloodGuard", () => { + it("suppresses repeated unauthorized responses and closes after threshold", () => { + const guard = new UnauthorizedFloodGuard({ closeAfter: 2, logEvery: 3 }); + + const first = guard.registerUnauthorized(); + expect(first).toEqual({ + shouldClose: false, + shouldLog: true, + count: 1, + suppressedSinceLastLog: 0, + }); + + const second = guard.registerUnauthorized(); + expect(second).toEqual({ + shouldClose: false, + shouldLog: false, + count: 2, + suppressedSinceLastLog: 0, + }); + + const third = guard.registerUnauthorized(); + expect(third).toEqual({ + shouldClose: true, + shouldLog: true, + count: 3, + suppressedSinceLastLog: 1, + }); + }); + + it("resets counters", () => { + const guard = new UnauthorizedFloodGuard({ closeAfter: 10, logEvery: 50 }); + guard.registerUnauthorized(); + guard.registerUnauthorized(); + guard.reset(); + + const next = guard.registerUnauthorized(); + expect(next).toEqual({ + shouldClose: false, + shouldLog: true, + count: 1, + suppressedSinceLastLog: 0, + }); + }); +}); + +describe("isUnauthorizedRoleError", () => { + it("detects unauthorized role responses", () => { + expect( + isUnauthorizedRoleError(errorShape(ErrorCodes.INVALID_REQUEST, "unauthorized role: node")), + ).toBe(true); + }); + + it("ignores non-role authorization errors", () => { + expect( + isUnauthorizedRoleError( + errorShape(ErrorCodes.INVALID_REQUEST, "missing scope: operator.admin"), + ), + ).toBe(false); + expect(isUnauthorizedRoleError(errorShape(ErrorCodes.UNAVAILABLE, "service unavailable"))).toBe( + false, + ); + }); +}); diff --git a/src/gateway/server/ws-connection/unauthorized-flood-guard.ts b/src/gateway/server/ws-connection/unauthorized-flood-guard.ts new file mode 100644 index 000000000..f7a7636b5 --- /dev/null +++ b/src/gateway/server/ws-connection/unauthorized-flood-guard.ts @@ -0,0 +1,69 @@ +import { ErrorCodes, type ErrorShape } from "../../protocol/index.js"; + +export type UnauthorizedFloodGuardOptions = { + closeAfter?: number; + logEvery?: number; +}; + +export type UnauthorizedFloodDecision = { + shouldClose: boolean; + shouldLog: boolean; + count: number; + suppressedSinceLastLog: number; +}; + +const DEFAULT_CLOSE_AFTER = 10; +const DEFAULT_LOG_EVERY = 100; + +export class UnauthorizedFloodGuard { + private readonly closeAfter: number; + private readonly logEvery: number; + private count = 0; + private suppressedSinceLastLog = 0; + + constructor(options?: UnauthorizedFloodGuardOptions) { + this.closeAfter = Math.max(1, Math.floor(options?.closeAfter ?? DEFAULT_CLOSE_AFTER)); + this.logEvery = Math.max(1, Math.floor(options?.logEvery ?? DEFAULT_LOG_EVERY)); + } + + registerUnauthorized(): UnauthorizedFloodDecision { + this.count += 1; + const shouldClose = this.count > this.closeAfter; + const shouldLog = this.count === 1 || this.count % this.logEvery === 0 || shouldClose; + + if (!shouldLog) { + this.suppressedSinceLastLog += 1; + return { + shouldClose, + shouldLog: false, + count: this.count, + suppressedSinceLastLog: 0, + }; + } + + const suppressedSinceLastLog = this.suppressedSinceLastLog; + this.suppressedSinceLastLog = 0; + return { + shouldClose, + shouldLog: true, + count: this.count, + suppressedSinceLastLog, + }; + } + + reset(): void { + this.count = 0; + this.suppressedSinceLastLog = 0; + } +} + +export function isUnauthorizedRoleError(error?: ErrorShape): boolean { + if (!error) { + return false; + } + return ( + error.code === ErrorCodes.INVALID_REQUEST && + typeof error.message === "string" && + error.message.startsWith("unauthorized role:") + ); +}