fix(compression): harden RTK raw-output redaction + ReDoS guard for custom filters (F5.3) (#4203)

* fix(compression): broaden RTK raw-output secret redaction (Basic/Proxy auth + keys)

F5.3 follow-up. SECRET_PATTERNS missed Authorization / Proxy-Authorization with
Basic (curl -v emits "Basic <base64>") and credential field names that don't
contain token/secret/password as a substring (private_key, access_key, credential).
Add a (Proxy-)Authorization Bearer|Basic pattern and broaden the key-name
alternation. All patterns are flat (no nested quantifiers → no ReDoS).

* fix(compression): drop ReDoS-prone patterns from custom RTK filters

F5.3 follow-up. Custom filters (DATA_DIR/rtk/filters.json) carry user-supplied
regex strings compiled and run against untrusted tool output; a nested unbounded
quantifier ((a+)+, (a*)*, ([a-z]+)+ …) can cause catastrophic backtracking.
validateRtkFilter now drops such patterns via a conservative, dependency-free
heuristic (isReDoSProne) applied to every regex-bearing array in the canonical and
legacy filter shapes. safe-regex would be ideal but is not installable in this
symlinked worktree; the heuristic catches the common single-group nested-quantifier
shapes and is itself linear.

Surfaced (not fixed here): a canonical pack filter omitting `preserve` crashes
validateRtkFilter because rtkFilterPreserveSchema.default({}) leaves the inner arrays
undefined — tracked as a pre-existing fragility.

open-sse/services/compression/engines/rtk/filterSchema.ts

@@ -142,14 +142,37 @@ function isCanonicalFilter(value: z.infer<typeof rtkFilterSchema>): 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,

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 <base64>).
+  [/((?:Proxy-)?Authorization:\s*(?:Bearer|Basic)\s+)[A-Za-z0-9._~+/-]+=*/gi, "$1[REDACTED]"],
 ];
 
 function dataDir(): string {

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");
+  });
+});

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");
+  });
 });