From 8ada0f4ae2f2113be9ea007d2bb4b9a7da9a9b2b Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Fri, 29 May 2026 00:58:16 -0400 Subject: [PATCH] fix(gateway): default non-finite auth guard limits --- .../handshake-auth-log-limiter.test.ts | 25 +++++++++++++++++++ .../handshake-auth-log-limiter.ts | 6 +++-- .../unauthorized-flood-guard.test.ts | 19 ++++++++++++++ .../ws-connection/unauthorized-flood-guard.ts | 5 ++-- 4 files changed, 51 insertions(+), 4 deletions(-) diff --git a/src/gateway/server/ws-connection/handshake-auth-log-limiter.test.ts b/src/gateway/server/ws-connection/handshake-auth-log-limiter.test.ts index 2d19781a404..b03820843a4 100644 --- a/src/gateway/server/ws-connection/handshake-auth-log-limiter.test.ts +++ b/src/gateway/server/ws-connection/handshake-auth-log-limiter.test.ts @@ -47,6 +47,31 @@ describe("HandshakeAuthLogLimiter", () => { }); }); + it("uses default limits for non-finite options", () => { + const limiter = new HandshakeAuthLogLimiter({ + intervalMs: Number.NaN, + maxEntries: Number.POSITIVE_INFINITY, + }); + + expect(limiter.register("first", 0)).toEqual({ + shouldLog: true, + suppressedSinceLastLog: 0, + }); + expect(limiter.register("first", 1_000)).toEqual({ + shouldLog: false, + suppressedSinceLastLog: 0, + }); + + for (let i = 0; i < 256; i += 1) { + limiter.register(`key-${i}`, i); + } + + expect(limiter.register("first", 2_000)).toEqual({ + shouldLog: true, + suppressedSinceLastLog: 0, + }); + }); + it("only rate-limits benign missing-credential startup retries", () => { expect( shouldLimitMissingCredentialAuthLog({ diff --git a/src/gateway/server/ws-connection/handshake-auth-log-limiter.ts b/src/gateway/server/ws-connection/handshake-auth-log-limiter.ts index 2203806a8a2..066cea76dd7 100644 --- a/src/gateway/server/ws-connection/handshake-auth-log-limiter.ts +++ b/src/gateway/server/ws-connection/handshake-auth-log-limiter.ts @@ -1,3 +1,5 @@ +import { resolveIntegerOption } from "../../../shared/number-coercion.js"; + export type HandshakeAuthLogDecision = { shouldLog: boolean; suppressedSinceLastLog: number; @@ -14,8 +16,8 @@ export class HandshakeAuthLogLimiter { private readonly entries = new Map(); constructor(options?: { intervalMs?: number; maxEntries?: number }) { - this.intervalMs = Math.max(1, Math.floor(options?.intervalMs ?? 30_000)); - this.maxEntries = Math.max(1, Math.floor(options?.maxEntries ?? 256)); + this.intervalMs = resolveIntegerOption(options?.intervalMs, 30_000, { min: 1 }); + this.maxEntries = resolveIntegerOption(options?.maxEntries, 256, { min: 1 }); } register(key: string, nowMs = Date.now()): HandshakeAuthLogDecision { diff --git a/src/gateway/server/ws-connection/unauthorized-flood-guard.test.ts b/src/gateway/server/ws-connection/unauthorized-flood-guard.test.ts index 946519d1619..dd2dc563198 100644 --- a/src/gateway/server/ws-connection/unauthorized-flood-guard.test.ts +++ b/src/gateway/server/ws-connection/unauthorized-flood-guard.test.ts @@ -31,6 +31,25 @@ describe("UnauthorizedFloodGuard", () => { }); }); + it("uses default thresholds for non-finite options", () => { + const guard = new UnauthorizedFloodGuard({ + closeAfter: Number.NaN, + logEvery: Number.POSITIVE_INFINITY, + }); + + for (let i = 0; i < 10; i += 1) { + expect(guard.registerUnauthorized().shouldClose).toBe(false); + } + + const eleventh = guard.registerUnauthorized(); + expect(eleventh).toMatchObject({ + shouldClose: true, + shouldLog: true, + count: 11, + suppressedSinceLastLog: 9, + }); + }); + it("resets counters", () => { const guard = new UnauthorizedFloodGuard({ closeAfter: 10, logEvery: 50 }); guard.registerUnauthorized(); diff --git a/src/gateway/server/ws-connection/unauthorized-flood-guard.ts b/src/gateway/server/ws-connection/unauthorized-flood-guard.ts index 2898e8d13c3..48f27593c1f 100644 --- a/src/gateway/server/ws-connection/unauthorized-flood-guard.ts +++ b/src/gateway/server/ws-connection/unauthorized-flood-guard.ts @@ -1,4 +1,5 @@ import { ErrorCodes, type ErrorShape } from "../../../../packages/gateway-protocol/src/index.js"; +import { resolveIntegerOption } from "../../../shared/number-coercion.js"; export type UnauthorizedFloodGuardOptions = { closeAfter?: number; @@ -22,8 +23,8 @@ export class UnauthorizedFloodGuard { 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)); + this.closeAfter = resolveIntegerOption(options?.closeAfter, DEFAULT_CLOSE_AFTER, { min: 1 }); + this.logEvery = resolveIntegerOption(options?.logEvery, DEFAULT_LOG_EVERY, { min: 1 }); } registerUnauthorized(): UnauthorizedFloodDecision {