mirror of
https://github.com/moltbot/moltbot.git
synced 2026-04-24 23:21:30 +00:00
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: 188a40e8a3
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
This commit is contained in:
@@ -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.
|
||||
|
||||
@@ -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");
|
||||
});
|
||||
});
|
||||
|
||||
99
src/gateway/server-http.hooks-request-timeout.test.ts
Normal file
99
src/gateway/server-http.hooks-request-timeout.test.ts
Normal file
@@ -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<typeof import("./hooks.js")>();
|
||||
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<typeof vi.fn>;
|
||||
setHeader: ReturnType<typeof vi.fn>;
|
||||
} {
|
||||
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<typeof createSubsystemLogger>,
|
||||
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();
|
||||
});
|
||||
});
|
||||
@@ -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 };
|
||||
|
||||
Reference in New Issue
Block a user