From 3a74dc00bfb7a8fc6b520cfbc53b9c67833fd8d5 Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Sat, 7 Mar 2026 22:35:38 +0000 Subject: [PATCH] fix(gateway): land #38725 from @ademczuk Source: #38725 / 533ff3e70bdb9fd184392935e8b2f5043b176fca by @ademczuk. Thanks @ademczuk. Co-authored-by: ademczuk --- CHANGELOG.md | 1 + src/gateway/auth.test.ts | 52 +++++++++++++++++++++++++++ src/gateway/auth.ts | 6 ++-- src/gateway/reconnect-gating.test.ts | 53 ++++++++++++++++++++++++++++ ui/src/ui/gateway.ts | 32 +++++++++++++++-- 5 files changed, 140 insertions(+), 4 deletions(-) create mode 100644 src/gateway/reconnect-gating.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 54eb59255e7..58ef4307a30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -79,6 +79,7 @@ Docs: https://docs.openclaw.ai - Sessions/daily reset transcript archival: archive prior transcript files during stale-session scheduled/daily resets by capturing the previous session entry before rollover, preventing orphaned transcript files on disk. (#35493) Thanks @byungsker. - Feishu/group slash command detection: normalize group mention wrappers before command-authorization probing so mention-prefixed commands (for example `@Bot/model` and `@Bot /reset`) are recognized as gateway commands instead of being forwarded to the agent. (#35994) Thanks @liuxiaopai-ai. - Control UI/auth token separation: keep the shared gateway token in browser auth validation while reserving cached device tokens for signed device payloads, preventing false `device token mismatch` disconnects after restart/rotation. Landed from contributor PR #37382 by @FradSer. Thanks @FradSer. +- Gateway/browser auth reconnect hardening: stop counting missing token/password submissions as auth rate-limit failures, and stop auto-reconnecting Control UI clients on non-recoverable auth errors so misconfigured browser tabs no longer lock out healthy sessions. Landed from contributor PR #38725 by @ademczuk. Thanks @ademczuk. - Agents/context pruning: guard assistant thinking/text char estimation against malformed blocks (missing `thinking`/`text` strings or null entries) so pruning no longer crashes with malformed provider content. (openclaw#35146) thanks @Sid-Qin. - Agents/transcript policy: set `preserveSignatures` to Anthropic-only handling in `resolveTranscriptPolicy` so Anthropic thinking signatures are preserved while non-Anthropic providers remain unchanged. (#32813) thanks @Sid-Qin. - Agents/schema cleaning: detect Venice + Grok model IDs as xAI-proxied targets so unsupported JSON Schema keywords are stripped before requests, preventing Venice/Grok `Invalid arguments` failures. (openclaw#35355) thanks @Sid-Qin. diff --git a/src/gateway/auth.test.ts b/src/gateway/auth.test.ts index 803d22a181b..cb85fca810c 100644 --- a/src/gateway/auth.test.ts +++ b/src/gateway/auth.test.ts @@ -367,6 +367,58 @@ describe("gateway auth", () => { expect(limiter.check).toHaveBeenCalledWith(undefined, "custom-scope"); expect(limiter.recordFailure).toHaveBeenCalledWith(undefined, "custom-scope"); }); + + it("does not record rate-limit failure for missing token (misconfigured client, not brute-force)", async () => { + const limiter = createLimiterSpy(); + const res = await authorizeGatewayConnect({ + auth: { mode: "token", token: "secret", allowTailscale: false }, + connectAuth: null, + rateLimiter: limiter, + }); + + expect(res.ok).toBe(false); + expect(res.reason).toBe("token_missing"); + expect(limiter.recordFailure).not.toHaveBeenCalled(); + }); + + it("does not record rate-limit failure for missing password (misconfigured client, not brute-force)", async () => { + const limiter = createLimiterSpy(); + const res = await authorizeGatewayConnect({ + auth: { mode: "password", password: "secret", allowTailscale: false }, + connectAuth: null, + rateLimiter: limiter, + }); + + expect(res.ok).toBe(false); + expect(res.reason).toBe("password_missing"); + expect(limiter.recordFailure).not.toHaveBeenCalled(); + }); + + it("still records rate-limit failure for wrong token (brute-force attempt)", async () => { + const limiter = createLimiterSpy(); + const res = await authorizeGatewayConnect({ + auth: { mode: "token", token: "secret", allowTailscale: false }, + connectAuth: { token: "wrong" }, + rateLimiter: limiter, + }); + + expect(res.ok).toBe(false); + expect(res.reason).toBe("token_mismatch"); + expect(limiter.recordFailure).toHaveBeenCalled(); + }); + + it("still records rate-limit failure for wrong password (brute-force attempt)", async () => { + const limiter = createLimiterSpy(); + const res = await authorizeGatewayConnect({ + auth: { mode: "password", password: "secret", allowTailscale: false }, + connectAuth: { password: "wrong" }, + rateLimiter: limiter, + }); + + expect(res.ok).toBe(false); + expect(res.reason).toBe("password_mismatch"); + expect(limiter.recordFailure).toHaveBeenCalled(); + }); }); describe("trusted-proxy auth", () => { diff --git a/src/gateway/auth.ts b/src/gateway/auth.ts index 467d14d4337..cb1673778f4 100644 --- a/src/gateway/auth.ts +++ b/src/gateway/auth.ts @@ -439,7 +439,9 @@ export async function authorizeGatewayConnect( return { ok: false, reason: "token_missing_config" }; } if (!connectAuth?.token) { - limiter?.recordFailure(ip, rateLimitScope); + // Don't burn rate-limit slots for missing credentials — the client + // simply hasn't provided a token yet (e.g. bare browser open). + // Only actual *wrong* credentials should count as failures. return { ok: false, reason: "token_missing" }; } if (!safeEqualSecret(connectAuth.token, auth.token)) { @@ -456,7 +458,7 @@ export async function authorizeGatewayConnect( return { ok: false, reason: "password_missing_config" }; } if (!password) { - limiter?.recordFailure(ip, rateLimitScope); + // Same as token_missing — don't penalize absent credentials. return { ok: false, reason: "password_missing" }; } if (!safeEqualSecret(password, auth.password)) { diff --git a/src/gateway/reconnect-gating.test.ts b/src/gateway/reconnect-gating.test.ts new file mode 100644 index 00000000000..3ea02e21820 --- /dev/null +++ b/src/gateway/reconnect-gating.test.ts @@ -0,0 +1,53 @@ +import { describe, expect, it } from "vitest"; +import { type GatewayErrorInfo, isNonRecoverableAuthError } from "../../ui/src/ui/gateway.ts"; +import { ConnectErrorDetailCodes } from "./protocol/connect-error-details.js"; + +function makeError(detailCode: string): GatewayErrorInfo { + return { code: "connect_failed", message: "auth failed", details: { code: detailCode } }; +} + +describe("isNonRecoverableAuthError", () => { + it("returns false for undefined error (normal disconnect)", () => { + expect(isNonRecoverableAuthError(undefined)).toBe(false); + }); + + it("returns false for errors without detail codes (network issues)", () => { + expect(isNonRecoverableAuthError({ code: "connect_failed", message: "timeout" })).toBe(false); + }); + + it("blocks reconnect for AUTH_TOKEN_MISSING (misconfigured client)", () => { + expect(isNonRecoverableAuthError(makeError(ConnectErrorDetailCodes.AUTH_TOKEN_MISSING))).toBe( + true, + ); + }); + + it("blocks reconnect for AUTH_PASSWORD_MISSING", () => { + expect( + isNonRecoverableAuthError(makeError(ConnectErrorDetailCodes.AUTH_PASSWORD_MISSING)), + ).toBe(true); + }); + + it("blocks reconnect for AUTH_PASSWORD_MISMATCH (wrong password won't self-correct)", () => { + expect( + isNonRecoverableAuthError(makeError(ConnectErrorDetailCodes.AUTH_PASSWORD_MISMATCH)), + ).toBe(true); + }); + + it("blocks reconnect for AUTH_RATE_LIMITED (reconnecting burns more slots)", () => { + expect(isNonRecoverableAuthError(makeError(ConnectErrorDetailCodes.AUTH_RATE_LIMITED))).toBe( + true, + ); + }); + + it("allows reconnect for AUTH_TOKEN_MISMATCH (device-token fallback flow)", () => { + // Browser client fallback: stale device token → mismatch → sendConnect() clears it → + // next reconnect uses opts.token (shared gateway token). Blocking here breaks recovery. + expect(isNonRecoverableAuthError(makeError(ConnectErrorDetailCodes.AUTH_TOKEN_MISMATCH))).toBe( + false, + ); + }); + + it("allows reconnect for unrecognized detail codes (future-proof)", () => { + expect(isNonRecoverableAuthError(makeError("SOME_FUTURE_CODE"))).toBe(false); + }); +}); diff --git a/ui/src/ui/gateway.ts b/ui/src/ui/gateway.ts index 01e5ec971d2..e5b747ae523 100644 --- a/ui/src/ui/gateway.ts +++ b/ui/src/ui/gateway.ts @@ -5,7 +5,10 @@ import { type GatewayClientMode, type GatewayClientName, } from "../../../src/gateway/protocol/client-info.js"; -import { readConnectErrorDetailCode } from "../../../src/gateway/protocol/connect-error-details.js"; +import { + ConnectErrorDetailCodes, + readConnectErrorDetailCode, +} from "../../../src/gateway/protocol/connect-error-details.js"; import { clearDeviceAuthToken, loadDeviceAuthToken, storeDeviceAuthToken } from "./device-auth.ts"; import { loadOrCreateDeviceIdentity, signDevicePayload } from "./device-identity.ts"; import { generateUUID } from "./uuid.ts"; @@ -50,6 +53,29 @@ export function resolveGatewayErrorDetailCode( return readConnectErrorDetailCode(error?.details); } +/** + * Auth errors that won't resolve without user action — don't auto-reconnect. + * + * NOTE: AUTH_TOKEN_MISMATCH is intentionally NOT included here because the + * browser client has a device-token fallback flow: a stale cached device token + * triggers a mismatch, sendConnect() clears it, and the next reconnect retries + * with opts.token (the shared gateway token). Blocking reconnect on mismatch + * would break that fallback. The rate limiter still catches persistent wrong + * tokens after N failures → AUTH_RATE_LIMITED stops the loop. + */ +export function isNonRecoverableAuthError(error: GatewayErrorInfo | undefined): boolean { + if (!error) { + return false; + } + const code = resolveGatewayErrorDetailCode(error); + return ( + code === ConnectErrorDetailCodes.AUTH_TOKEN_MISSING || + code === ConnectErrorDetailCodes.AUTH_PASSWORD_MISSING || + code === ConnectErrorDetailCodes.AUTH_PASSWORD_MISMATCH || + code === ConnectErrorDetailCodes.AUTH_RATE_LIMITED + ); +} + export type GatewayHelloOk = { type: "hello-ok"; protocol: number; @@ -135,7 +161,9 @@ export class GatewayBrowserClient { this.ws = null; this.flushPending(new Error(`gateway closed (${ev.code}): ${reason}`)); this.opts.onClose?.({ code: ev.code, reason, error: connectError }); - this.scheduleReconnect(); + if (!isNonRecoverableAuthError(connectError)) { + this.scheduleReconnect(); + } }); this.ws.addEventListener("error", () => { // ignored; close handler will fire