From b1592457fa1e197afaac8358b834e8e6d861b6ae Mon Sep 17 00:00:00 2001 From: Peter Steinberger Date: Mon, 2 Mar 2026 16:37:23 +0000 Subject: [PATCH] perf(security): bound regex input in filters and redaction --- src/discord/monitor/exec-approvals.test.ts | 11 +++++++++ src/discord/monitor/exec-approvals.ts | 19 ++++++++++++++- src/infra/exec-approval-forwarder.test.ts | 28 ++++++++++++++++++++++ src/infra/exec-approval-forwarder.ts | 19 ++++++++++++++- src/logging/redact.test.ts | 9 +++++++ src/logging/redact.ts | 22 ++++++++++++++--- 6 files changed, 103 insertions(+), 5 deletions(-) diff --git a/src/discord/monitor/exec-approvals.test.ts b/src/discord/monitor/exec-approvals.test.ts index f3adf7089c3..1addb7ada31 100644 --- a/src/discord/monitor/exec-approvals.test.ts +++ b/src/discord/monitor/exec-approvals.test.ts @@ -318,6 +318,17 @@ describe("DiscordExecApprovalHandler.shouldHandle", () => { expect(handler.shouldHandle(createRequest({ sessionKey: `${"a".repeat(28)}!` }))).toBe(false); }); + it("matches long session keys with tail-bounded regex checks", () => { + const handler = createHandler({ + enabled: true, + approvers: ["123"], + sessionFilter: ["discord:tail$"], + }); + expect( + handler.shouldHandle(createRequest({ sessionKey: `${"x".repeat(5000)}discord:tail` })), + ).toBe(true); + }); + it("filters by discord account when session store includes account", () => { writeStore({ "agent:test-agent:discord:channel:999888777": { diff --git a/src/discord/monitor/exec-approvals.ts b/src/discord/monitor/exec-approvals.ts index 3dfcc9c2ffa..09ea174790e 100644 --- a/src/discord/monitor/exec-approvals.ts +++ b/src/discord/monitor/exec-approvals.ts @@ -34,6 +34,7 @@ import { createDiscordClient, stripUndefinedFields } from "../send.shared.js"; import { DiscordUiContainer } from "../ui.js"; const EXEC_APPROVAL_KEY = "execapproval"; +const SESSION_FILTER_REGEX_MAX_INPUT = 2048; export type { ExecApprovalRequest, ExecApprovalResolved }; @@ -367,12 +368,28 @@ export class DiscordExecApprovalHandler { if (!session) { return false; } + const head = session.slice(0, SESSION_FILTER_REGEX_MAX_INPUT); + const tail = + session.length > SESSION_FILTER_REGEX_MAX_INPUT + ? session.slice(-SESSION_FILTER_REGEX_MAX_INPUT) + : ""; const matches = config.sessionFilter.some((p) => { if (session.includes(p)) { return true; } const regex = compileSafeRegex(p); - return regex ? regex.test(session) : false; + if (!regex) { + return false; + } + regex.lastIndex = 0; + if (regex.test(head)) { + return true; + } + if (tail) { + regex.lastIndex = 0; + return regex.test(tail); + } + return false; }); if (!matches) { return false; diff --git a/src/infra/exec-approval-forwarder.test.ts b/src/infra/exec-approval-forwarder.test.ts index 8d81cc69661..4deaa6705d0 100644 --- a/src/infra/exec-approval-forwarder.test.ts +++ b/src/infra/exec-approval-forwarder.test.ts @@ -194,6 +194,34 @@ describe("exec approval forwarder", () => { expect(deliver).not.toHaveBeenCalled(); }); + it("matches long session keys with tail-bounded regex checks", async () => { + const cfg = { + approvals: { + exec: { + enabled: true, + mode: "session", + sessionFilter: ["discord:tail$"], + }, + }, + } as OpenClawConfig; + + const { deliver, forwarder } = createForwarder({ + cfg, + resolveSessionTarget: () => ({ channel: "slack", to: "U1" }), + }); + + const request = { + ...baseRequest, + request: { + ...baseRequest.request, + sessionKey: `${"x".repeat(5000)}discord:tail`, + }, + }; + + await expect(forwarder.handleRequested(request)).resolves.toBe(true); + expect(deliver).toHaveBeenCalledTimes(1); + }); + it("returns false when all targets are skipped", async () => { await expectDiscordSessionTargetRequest({ cfg: makeSessionCfg({ discordExecApprovalsEnabled: true }), diff --git a/src/infra/exec-approval-forwarder.ts b/src/infra/exec-approval-forwarder.ts index d024f91bc3a..72017a36982 100644 --- a/src/infra/exec-approval-forwarder.ts +++ b/src/infra/exec-approval-forwarder.ts @@ -22,6 +22,7 @@ import { deliverOutboundPayloads } from "./outbound/deliver.js"; import { resolveSessionDeliveryTarget } from "./outbound/targets.js"; const log = createSubsystemLogger("gateway/exec-approvals"); +const SESSION_FILTER_REGEX_MAX_INPUT = 2048; export type { ExecApprovalRequest, ExecApprovalResolved }; @@ -56,12 +57,28 @@ function normalizeMode(mode?: ExecApprovalForwardingConfig["mode"]) { } function matchSessionFilter(sessionKey: string, patterns: string[]): boolean { + const head = sessionKey.slice(0, SESSION_FILTER_REGEX_MAX_INPUT); + const tail = + sessionKey.length > SESSION_FILTER_REGEX_MAX_INPUT + ? sessionKey.slice(-SESSION_FILTER_REGEX_MAX_INPUT) + : ""; return patterns.some((pattern) => { if (sessionKey.includes(pattern)) { return true; } const regex = compileSafeRegex(pattern); - return regex ? regex.test(sessionKey) : false; + if (!regex) { + return false; + } + regex.lastIndex = 0; + if (regex.test(head)) { + return true; + } + if (tail) { + regex.lastIndex = 0; + return regex.test(tail); + } + return false; }); } diff --git a/src/logging/redact.test.ts b/src/logging/redact.test.ts index 91180619a17..96635d7f7ec 100644 --- a/src/logging/redact.test.ts +++ b/src/logging/redact.test.ts @@ -102,6 +102,15 @@ describe("redactSensitiveText", () => { expect(output).toBe(input); }); + it("redacts large payloads with bounded regex passes", () => { + const input = `${"x".repeat(40_000)} OPENAI_API_KEY=sk-1234567890abcdef ${"y".repeat(40_000)}`; + const output = redactSensitiveText(input, { + mode: "tools", + patterns: defaults, + }); + expect(output).toContain("OPENAI_API_KEY=sk-123…cdef"); + }); + it("skips redaction when mode is off", () => { const input = "OPENAI_API_KEY=sk-1234567890abcdef"; const output = redactSensitiveText(input, { diff --git a/src/logging/redact.ts b/src/logging/redact.ts index 836e9f68405..e0c38ccba5d 100644 --- a/src/logging/redact.ts +++ b/src/logging/redact.ts @@ -10,6 +10,8 @@ const DEFAULT_REDACT_MODE: RedactSensitiveMode = "tools"; const DEFAULT_REDACT_MIN_LENGTH = 18; const DEFAULT_REDACT_KEEP_START = 6; const DEFAULT_REDACT_KEEP_END = 4; +const REDACT_REGEX_CHUNK_THRESHOLD = 32_768; +const REDACT_REGEX_CHUNK_SIZE = 16_384; const DEFAULT_REDACT_PATTERNS: string[] = [ // ENV-style assignments. @@ -94,12 +96,26 @@ function redactMatch(match: string, groups: string[]): string { return match.replace(token, masked); } +function replacePatternWithBounds(text: string, pattern: RegExp): string { + const apply = (value: string) => + value.replace(pattern, (...args: string[]) => + redactMatch(args[0], args.slice(1, args.length - 2)), + ); + if (text.length <= REDACT_REGEX_CHUNK_THRESHOLD) { + return apply(text); + } + + let output = ""; + for (let index = 0; index < text.length; index += REDACT_REGEX_CHUNK_SIZE) { + output += apply(text.slice(index, index + REDACT_REGEX_CHUNK_SIZE)); + } + return output; +} + function redactText(text: string, patterns: RegExp[]): string { let next = text; for (const pattern of patterns) { - next = next.replace(pattern, (...args: string[]) => - redactMatch(args[0], args.slice(1, args.length - 2)), - ); + next = replacePatternWithBounds(next, pattern); } return next; }