mirror of
https://github.com/openclaw/openclaw.git
synced 2026-03-12 07:20:45 +00:00
fix(gateway): prevent browser rate-limit self-DoS on missing credentials
Stop counting missing credentials (token_missing, password_missing) as rate-limit failures — a misconfigured browser is not a brute-force attack. Stop auto-reconnecting on non-recoverable auth errors (missing token, missing password, wrong password, rate-limited). Preserve device-token fallback by allowing reconnect on token mismatch. Includes unit tests for both fixes (12 new tests, all passing). Continuation of #36138 (closed due to unrelated changes drifting in).
This commit is contained in:
@@ -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", () => {
|
||||
|
||||
@@ -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)) {
|
||||
|
||||
53
src/gateway/reconnect-gating.test.ts
Normal file
53
src/gateway/reconnect-gating.test.ts
Normal file
@@ -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);
|
||||
});
|
||||
});
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user