diff --git a/open-sse/services/compression/engines/rtk/filterSchema.ts b/open-sse/services/compression/engines/rtk/filterSchema.ts index b3d2842b45..b4bfa70774 100644 --- a/open-sse/services/compression/engines/rtk/filterSchema.ts +++ b/open-sse/services/compression/engines/rtk/filterSchema.ts @@ -142,14 +142,37 @@ function isCanonicalFilter(value: z.infer): value is Rtk return "label" in value && "match" in value && "rules" in value; } +/** + * Conservative, dependency-free ReDoS guard. Custom RTK filters (DATA_DIR/rtk/filters.json) + * carry user-supplied regex strings that are compiled and run against untrusted tool output; + * a nested unbounded quantifier ((a+)+, (a*)*, ([a-z]+)+, (a+|b)+ …) causes catastrophic + * backtracking. This flags the common single-group nested-quantifier shapes so the loader + * never compiles them. Heuristic by design — a full analysis would use `safe-regex` (not + * installable in this symlinked worktree); it is itself linear (no nested quantifier). + */ +export function isReDoSProne(pattern: string): boolean { + return /\([^()]*(?:[+*]|\{\d+,\})[^()]*\)\s*(?:[+*]|\{\d+,\})/.test(pattern); +} + +function dropReDoSProne(patterns: string[]): string[] { + return patterns.filter((p) => !isReDoSProne(p)); +} + export function validateRtkFilter(value: unknown): RtkFilterDefinition { const parsed = rtkFilterSchema.parse(value); if (!isCanonicalFilter(parsed)) { + const collapse = dropReDoSProne(parsed.collapsePatterns); return { ...parsed, + stripPatterns: dropReDoSProne(parsed.stripPatterns), + keepPatterns: dropReDoSProne(parsed.keepPatterns), + priorityPatterns: dropReDoSProne(parsed.priorityPatterns), + collapsePatterns: collapse, + replace: parsed.replace.filter((r) => !isReDoSProne(r.pattern)), + matchOutput: parsed.matchOutput.filter((r) => !isReDoSProne(r.pattern)), commandPatterns: [], matchPatterns: [], - deduplicate: parsed.collapsePatterns.length > 0, + deduplicate: collapse.length > 0, }; } @@ -159,17 +182,17 @@ export function validateRtkFilter(value: unknown): RtkFilterDefinition { name: parsed.label, description: parsed.description, commandTypes: parsed.match.outputTypes, - commandPatterns: parsed.match.commands, - matchPatterns: parsed.match.patterns, + commandPatterns: dropReDoSProne(parsed.match.commands), + matchPatterns: dropReDoSProne(parsed.match.patterns), category: parsed.category, priority: parsed.priority, - stripPatterns: parsed.rules.dropPatterns, - keepPatterns: parsed.rules.includePatterns, - priorityPatterns: preservePatterns, - collapsePatterns: parsed.rules.collapsePatterns, + stripPatterns: dropReDoSProne(parsed.rules.dropPatterns), + keepPatterns: dropReDoSProne(parsed.rules.includePatterns), + priorityPatterns: dropReDoSProne(preservePatterns), + collapsePatterns: dropReDoSProne(parsed.rules.collapsePatterns), stripAnsi: parsed.rules.stripAnsi, - replace: parsed.rules.replace, - matchOutput: parsed.rules.matchOutput, + replace: parsed.rules.replace.filter((r) => !isReDoSProne(r.pattern)), + matchOutput: parsed.rules.matchOutput.filter((r) => !isReDoSProne(r.pattern)), truncateLineAt: parsed.rules.truncateLineAt, onEmpty: parsed.rules.onEmpty, filterStderr: parsed.rules.filterStderr, diff --git a/open-sse/services/compression/engines/rtk/rawOutput.ts b/open-sse/services/compression/engines/rtk/rawOutput.ts index b536e08713..57c2e4da34 100644 --- a/open-sse/services/compression/engines/rtk/rawOutput.ts +++ b/open-sse/services/compression/engines/rtk/rawOutput.ts @@ -19,8 +19,14 @@ const SECRET_PATTERNS: Array<[RegExp, string]> = [ [/\b(sk-[A-Za-z0-9_-]{16,})\b/g, "[REDACTED_OPENAI_KEY]"], [/\b(xox[baprs]-[A-Za-z0-9-]{16,})\b/g, "[REDACTED_SLACK_TOKEN]"], [/\b(AKIA[0-9A-Z]{16})\b/g, "[REDACTED_AWS_KEY]"], - [/((?:api[_-]?key|token|secret|password)\s*[:=]\s*)("[^"]+"|'[^']+'|[^\s]+)/gi, "$1[REDACTED]"], - [/(Authorization:\s*Bearer\s+)[A-Za-z0-9._~+/-]+=*/gi, "$1[REDACTED]"], + // key=value / key: value for common credential field names (flat alternation — no nesting, + // so no ReDoS). Covers names the bare token/secret/password set misses (private_key, etc). + [ + /((?:api[_-]?key|api[_-]?token|access[_-]?key|access[_-]?token|client[_-]?secret|auth[_-]?token|private[_-]?key|secret[_-]?key|credentials?|token|secret|password)\s*[:=]\s*)("[^"]+"|'[^']+'|[^\s]+)/gi, + "$1[REDACTED]", + ], + // Authorization / Proxy-Authorization with Bearer OR Basic (curl -v emits Basic ). + [/((?:Proxy-)?Authorization:\s*(?:Bearer|Basic)\s+)[A-Za-z0-9._~+/-]+=*/gi, "$1[REDACTED]"], ]; function dataDir(): string { diff --git a/tests/unit/compression/rtk-filter-redos-guard.test.ts b/tests/unit/compression/rtk-filter-redos-guard.test.ts new file mode 100644 index 0000000000..fa5463aedb --- /dev/null +++ b/tests/unit/compression/rtk-filter-redos-guard.test.ts @@ -0,0 +1,54 @@ +import { describe, it } from "node:test"; +import assert from "node:assert/strict"; + +import { + validateRtkFilter, + isReDoSProne, +} from "../../../open-sse/services/compression/engines/rtk/filterSchema.ts"; + +// Custom RTK filters (DATA_DIR/rtk/filters.json) carry user-supplied regex strings that are +// compiled and run against untrusted tool output. A nested unbounded quantifier ((a+)+, (a*)*) +// can trigger catastrophic backtracking (ReDoS). validateRtkFilter must drop such patterns so +// they are never compiled. Dependency-free heuristic (safe-regex not installable here). +describe("RTK filter ReDoS guard", () => { + it("flags nested unbounded quantifiers and accepts safe patterns", () => { + assert.equal(isReDoSProne("(a+)+"), true); + assert.equal(isReDoSProne("(a*)*"), true); + assert.equal(isReDoSProne("([a-z]+)+"), true); + assert.equal(isReDoSProne("(a+|b)+"), true); + assert.equal(isReDoSProne("(?:\\d+)+"), true); + + assert.equal(isReDoSProne("(ab)+"), false); + assert.equal(isReDoSProne("\\d{1,8}"), false); + assert.equal(isReDoSProne("error|fail|FAIL"), false); + assert.equal(isReDoSProne("^\\s*$"), false); + }); + + it("strips ReDoS-prone patterns from a canonical filter at validation", () => { + const def = validateRtkFilter({ + id: "x", + label: "X", + category: "generic", + match: { commands: [], patterns: ["(a+)+", "ERROR\\b"], outputTypes: [] }, + rules: { dropPatterns: ["(.*)*", "^\\s*$"] }, + // preserve provided explicitly: a pack filter omitting it crashes validateRtkFilter + // (pre-existing: rtkFilterPreserveSchema.default({}) leaves errorPatterns undefined). + preserve: { errorPatterns: [], summaryPatterns: [] }, + }); + + assert.deepEqual(def.matchPatterns, ["ERROR\\b"], "catastrophic matchPattern dropped"); + assert.deepEqual(def.stripPatterns, ["^\\s*$"], "catastrophic dropPattern removed"); + }); + + it("strips ReDoS-prone patterns from a legacy filter at validation", () => { + const def = validateRtkFilter({ + id: "y", + name: "Y", + category: "generic", + commandTypes: ["shell"], + stripPatterns: ["([a-z]+)*", "keepme"], + }); + + assert.deepEqual(def.stripPatterns, ["keepme"], "catastrophic legacy stripPattern removed"); + }); +}); diff --git a/tests/unit/compression/rtk-raw-output.test.ts b/tests/unit/compression/rtk-raw-output.test.ts index d4e739d224..7344eb6b30 100644 --- a/tests/unit/compression/rtk-raw-output.test.ts +++ b/tests/unit/compression/rtk-raw-output.test.ts @@ -146,4 +146,22 @@ describe("RTK raw output retention", () => { assert.equal(pointer, null); fs.rmSync(blocker, { force: true }); }); + + it("redacts Basic/Proxy auth headers and key fields the base patterns miss", () => { + const basic = redactRtkRawOutput("> Authorization: Basic dXNlcjpwYXNzd29yZA=="); + assert.equal(basic.redacted, true); + assert.ok(!basic.text.includes("dXNlcjpwYXNzd29yZA=="), "Authorization: Basic base64 redacted"); + + const proxy = redactRtkRawOutput("Proxy-Authorization: Basic c2VjcmV0OnBhc3M="); + assert.equal(proxy.redacted, true); + assert.ok(!proxy.text.includes("c2VjcmV0OnBhc3M="), "Proxy-Authorization redacted"); + + const pkey = redactRtkRawOutput("private_key=abc123def456ghi"); + assert.equal(pkey.redacted, true); + assert.ok(!pkey.text.includes("abc123def456ghi"), "private_key value redacted"); + + const cred = redactRtkRawOutput("credential: my-credential-value-x"); + assert.equal(cred.redacted, true); + assert.ok(!cred.text.includes("my-credential-value-x"), "credential value redacted"); + }); });