From 28431b84ccc418108838fb00b9d109c55a239386 Mon Sep 17 00:00:00 2001 From: AI-Reviewer-QS Date: Sat, 14 Feb 2026 08:46:12 +0800 Subject: [PATCH] fix(gateway): prune expired entries instead of clearing all hook auth failure state (#15848) Merged via /review-pr -> /prepare-pr -> /merge-pr. Prepared head SHA: 188a40e8a35112b9ea2df23dd0a940b9be1eac1d Co-authored-by: AI-Reviewer-QS <255312808+AI-Reviewer-QS@users.noreply.github.com> Co-authored-by: steipete <58493+steipete@users.noreply.github.com> Reviewed-by: @steipete --- CHANGELOG.md | 1 + .../list.list-command.forward-compat.test.ts | 20 ++-- .../server-http.hooks-request-timeout.test.ts | 99 +++++++++++++++++++ src/gateway/server-http.ts | 23 ++++- 4 files changed, 132 insertions(+), 11 deletions(-) create mode 100644 src/gateway/server-http.hooks-request-timeout.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index 9a695f7619e..eb75439c69b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -36,6 +36,7 @@ Docs: https://docs.openclaw.ai - MS Teams: preserve parsed mention entities/text when appending OneDrive fallback file links, and accept broader real-world Teams mention ID formats (`29:...`, `8:orgid:...`) while still rejecting placeholder patterns. (#15436) Thanks @hyojin. - Security/Audit: distinguish external webhooks (`hooks.enabled`) from internal hooks (`hooks.internal.enabled`) in attack-surface summaries to avoid false exposure signals when only internal hooks are enabled. (#13474) Thanks @mcaxtr. - Security/Onboarding: clarify multi-user DM isolation remediation with explicit `openclaw config set session.dmScope ...` commands in security audit, doctor security, and channel onboarding guidance. (#13129) Thanks @VintLin. +- Gateway/Hooks: preserve `408` for hook request-body timeout responses while keeping bounded auth-failure cache eviction behavior, with timeout-status regression coverage. (#15848) Thanks @AI-Reviewer-QS. - Security/Audit: add misconfiguration checks for sandbox Docker config with sandbox mode off, ineffective `gateway.nodes.denyCommands` entries, global minimal tool-profile overrides by agent profiles, and permissive extension-plugin tool reachability. - Security/Link understanding: block loopback/internal host patterns and private/mapped IPv6 addresses in extracted URL handling to close SSRF bypasses in link CLI flows. (#15604) Thanks @AI-Reviewer-QS. - Android/Nodes: harden `app.update` by requiring HTTPS and gateway-host URL matching plus SHA-256 verification, stream URL camera downloads to disk with size guards to avoid memory spikes, and stop signing release builds with debug keys. (#13541) Thanks @smartprogrammer93. diff --git a/src/commands/models/list.list-command.forward-compat.test.ts b/src/commands/models/list.list-command.forward-compat.test.ts index 811797eb5e7..396509f8a31 100644 --- a/src/commands/models/list.list-command.forward-compat.test.ts +++ b/src/commands/models/list.list-command.forward-compat.test.ts @@ -4,7 +4,7 @@ const mocks = vi.hoisted(() => { const printModelTable = vi.fn(); return { loadConfig: vi.fn().mockReturnValue({ - agents: { defaults: { model: { primary: "openai-codex/gpt-5.3-codex-spark" } } }, + agents: { defaults: { model: { primary: "openai-codex/gpt-5.3-codex" } } }, models: { providers: {} }, }), ensureAuthProfileStore: vi.fn().mockReturnValue({ version: 1, profiles: {}, order: {} }), @@ -14,8 +14,8 @@ const mocks = vi.hoisted(() => { resolveConfiguredEntries: vi.fn().mockReturnValue({ entries: [ { - key: "openai-codex/gpt-5.3-codex-spark", - ref: { provider: "openai-codex", model: "gpt-5.3-codex-spark" }, + key: "openai-codex/gpt-5.3-codex", + ref: { provider: "openai-codex", model: "gpt-5.3-codex" }, tags: new Set(["configured"]), aliases: [], }, @@ -24,8 +24,8 @@ const mocks = vi.hoisted(() => { printModelTable, resolveForwardCompatModel: vi.fn().mockReturnValue({ provider: "openai-codex", - id: "gpt-5.3-codex-spark", - name: "GPT-5.3 Codex Spark", + id: "gpt-5.3-codex", + name: "GPT-5.3 Codex", api: "openai-codex-responses", baseUrl: "https://chatgpt.com/backend-api", input: ["text"], @@ -76,7 +76,7 @@ vi.mock("../../agents/model-forward-compat.js", async (importOriginal) => { import { modelsListCommand } from "./list.list-command.js"; describe("modelsListCommand forward-compat", () => { - it("does not mark configured codex spark as missing when forward-compat can build a fallback", async () => { + it("does not mark configured codex model as missing when forward-compat can build a fallback", async () => { const runtime = { log: vi.fn(), error: vi.fn() }; await modelsListCommand({ json: true }, runtime as never); @@ -88,9 +88,9 @@ describe("modelsListCommand forward-compat", () => { missing: boolean; }>; - const spark = rows.find((r) => r.key === "openai-codex/gpt-5.3-codex-spark"); - expect(spark).toBeTruthy(); - expect(spark?.missing).toBe(false); - expect(spark?.tags).not.toContain("missing"); + const codex = rows.find((r) => r.key === "openai-codex/gpt-5.3-codex"); + expect(codex).toBeTruthy(); + expect(codex?.missing).toBe(false); + expect(codex?.tags).not.toContain("missing"); }); }); diff --git a/src/gateway/server-http.hooks-request-timeout.test.ts b/src/gateway/server-http.hooks-request-timeout.test.ts new file mode 100644 index 00000000000..e76c243d5c1 --- /dev/null +++ b/src/gateway/server-http.hooks-request-timeout.test.ts @@ -0,0 +1,99 @@ +import type { IncomingMessage, ServerResponse } from "node:http"; +import { beforeEach, describe, expect, test, vi } from "vitest"; +import type { createSubsystemLogger } from "../logging/subsystem.js"; +import type { HooksConfigResolved } from "./hooks.js"; + +const { readJsonBodyMock } = vi.hoisted(() => ({ + readJsonBodyMock: vi.fn(), +})); + +vi.mock("./hooks.js", async (importOriginal) => { + const actual = await importOriginal(); + return { + ...actual, + readJsonBody: readJsonBodyMock, + }; +}); + +import { createHooksRequestHandler } from "./server-http.js"; + +function createHooksConfig(): HooksConfigResolved { + return { + basePath: "/hooks", + token: "hook-secret", + maxBodyBytes: 1024, + mappings: [], + agentPolicy: { + defaultAgentId: "main", + knownAgentIds: new Set(["main"]), + allowedAgentIds: undefined, + }, + sessionPolicy: { + allowRequestSessionKey: false, + defaultSessionKey: undefined, + allowedSessionKeyPrefixes: undefined, + }, + }; +} + +function createRequest(): IncomingMessage { + return { + method: "POST", + url: "/hooks/wake", + headers: { + host: "127.0.0.1:18789", + authorization: "Bearer hook-secret", + }, + socket: { remoteAddress: "127.0.0.1" }, + } as IncomingMessage; +} + +function createResponse(): { + res: ServerResponse; + end: ReturnType; + setHeader: ReturnType; +} { + const setHeader = vi.fn(); + const end = vi.fn(); + const res = { + statusCode: 200, + setHeader, + end, + } as unknown as ServerResponse; + return { res, end, setHeader }; +} + +describe("createHooksRequestHandler timeout status mapping", () => { + beforeEach(() => { + readJsonBodyMock.mockReset(); + }); + + test("returns 408 for request body timeout", async () => { + readJsonBodyMock.mockResolvedValue({ ok: false, error: "request body timeout" }); + const dispatchWakeHook = vi.fn(); + const dispatchAgentHook = vi.fn(() => "run-1"); + const handler = createHooksRequestHandler({ + getHooksConfig: () => createHooksConfig(), + bindHost: "127.0.0.1", + port: 18789, + logHooks: { + warn: vi.fn(), + debug: vi.fn(), + info: vi.fn(), + error: vi.fn(), + } as unknown as ReturnType, + dispatchWakeHook, + dispatchAgentHook, + }); + const req = createRequest(); + const { res, end } = createResponse(); + + const handled = await handler(req, res); + + expect(handled).toBe(true); + expect(res.statusCode).toBe(408); + expect(end).toHaveBeenCalledWith(JSON.stringify({ ok: false, error: "request body timeout" })); + expect(dispatchWakeHook).not.toHaveBeenCalled(); + expect(dispatchAgentHook).not.toHaveBeenCalled(); + }); +}); diff --git a/src/gateway/server-http.ts b/src/gateway/server-http.ts index 7b5630d1a11..1bdd5acf240 100644 --- a/src/gateway/server-http.ts +++ b/src/gateway/server-http.ts @@ -207,13 +207,34 @@ export function createHooksRequestHandler( nowMs: number, ): { throttled: boolean; retryAfterSeconds?: number } => { if (!hookAuthFailures.has(clientKey) && hookAuthFailures.size >= HOOK_AUTH_FAILURE_TRACK_MAX) { - hookAuthFailures.clear(); + // Prune expired entries instead of clearing all state. + for (const [key, entry] of hookAuthFailures) { + if (nowMs - entry.windowStartedAtMs >= HOOK_AUTH_FAILURE_WINDOW_MS) { + hookAuthFailures.delete(key); + } + } + // If still at capacity after pruning, drop the oldest half. + if (hookAuthFailures.size >= HOOK_AUTH_FAILURE_TRACK_MAX) { + let toRemove = Math.floor(hookAuthFailures.size / 2); + for (const key of hookAuthFailures.keys()) { + if (toRemove <= 0) { + break; + } + hookAuthFailures.delete(key); + toRemove--; + } + } } const current = hookAuthFailures.get(clientKey); const expired = !current || nowMs - current.windowStartedAtMs >= HOOK_AUTH_FAILURE_WINDOW_MS; const next: HookAuthFailure = expired ? { count: 1, windowStartedAtMs: nowMs } : { count: current.count + 1, windowStartedAtMs: current.windowStartedAtMs }; + // Delete-before-set refreshes Map insertion order so recently-active + // clients are not evicted before dormant ones during oldest-half eviction. + if (hookAuthFailures.has(clientKey)) { + hookAuthFailures.delete(clientKey); + } hookAuthFailures.set(clientKey, next); if (next.count <= HOOK_AUTH_FAILURE_LIMIT) { return { throttled: false };