fix(signals): cap globToRegExp wildcard count to prevent ReDoS

src/signals/change-guardrail.ts

@@ -12,11 +12,74 @@ function canonicalize(value: string): string {
   return value.replace(/\\/g, "/").replace(/^\.\//, "").replace(/^\/+/, "").toLowerCase();
 }
 
+// globToRegExp's COMPILATION is linear-time, but the COMPILED pattern's .test() can be polynomial-to-exponential
+// time on an adversarial near-miss input when MULTIPLE wildcard GROUPS chain in one glob (a "group" is one `*`
+// OR one `**` — a `**` pair compiles to a SINGLE `.*`, not two independent wildcards, so it must be counted as
+// ONE group, not two characters; see countWildcardGroups below). Both group TYPES contribute to the same danger
+// once chained — `[^/]*` groups separated by a literal that class doesn't exclude (e.g. "-", not "/") back-
+// track ambiguously, and `.*` groups back-track ambiguously EVEN when "/"-separated, since `.*` crosses `/`
+// freely. Re-benchmarked against `path` lengths GitHub can plausibly deliver via a deeply nested file path in a
+// malicious PR (both `path` and, via `.gittensory.yml`'s contentLane.*Glob fields, the glob itself can be
+// attacker-influenced in the same PR):
+//   2 wildcard groups (any mix of `*`/`**`, any arrangement): sub-second even at a wildly implausible 32,000-
+//                       char adversarial path (worst case observed: ~400ms) — quadratic, bounded, never a
+//                       realistic hang.
+//   3 wildcard groups: OVER 2 SECONDS at just ~4,000 chars for one chained-`*` shape, over 100ms at ~1,600
+//                       chars for a chained-`**` shape — already dangerous well within a plausible path length.
+//   4+ wildcard groups: confirmed catastrophic — 35 SECONDS at just 1,614 chars for 4 chained `**` groups.
+// hardGuardrailGlobs today are 100% hardcoded engine constants (see review/guardrail-config.ts) — no
+// maintainer/contributor input reaches globToRegExp via that path today, and none of those real globs exceed 1
+// wildcard group — but it is also exported for reuse by other maintainer-config-driven consumers
+// (content-lane/spec-resolver.ts, whose real globs like "public/**/*.json" are exactly 2 groups: this cap must
+// stay inclusive of that legitimate shape, not just "safer than before"), so the cap lives INSIDE globToRegExp
+// itself (not just in a wrapper like matchesAny below) — every caller, present or future, direct or indirect, is
+// protected automatically rather than needing to separately remember the risk. The boundary is set at the
+// highest GROUP count proven safe by the benchmark above (2) — a boundary that itself sits inside the
+// empirically dangerous range would defeat the point of a cap.
+const MAX_GLOB_WILDCARD_GROUPS = 2;
+
+/** Count `*` GROUPS in `glob` — a `**` pair is ONE group (it compiles to a single `.*`, see globToRegExp), not
+ *  two. Mirrors globToRegExp's own tokenization exactly (including consuming a `**`'s trailing `/`) so the count
+ *  reflects the actual number of backtracking-capable groups the compiled RegExp will contain, not raw `*`
+ *  character count (which would double-count every globstar and reject legitimate globs like
+ *  "public/**\/*.json" — 2 real groups — as if they were 3-groups-dangerous). */
+function countWildcardGroups(glob: string): number {
+  let count = 0;
+  for (let i = 0; i < glob.length; i += 1) {
+    if (glob.charAt(i) !== "*") continue;
+    count += 1;
+    if (glob.charAt(i + 1) === "*") {
+      i += 1; // consume the second star of the "**" pair — one group, not two
+      if (glob.charAt(i + 1) === "/") i += 1; // `**/` also matches zero segments, mirroring globToRegExp
+    }
+  }
+  return count;
+}
+
+/** True if `glob` has more wildcard GROUPS than can be safely compiled to a RegExp without risking catastrophic
+ *  backtracking (see the MAX_GLOB_WILDCARD_GROUPS rationale above). */
+function hasUnsafeWildcardCount(glob: string): boolean {
+  return countWildcardGroups(glob) > MAX_GLOB_WILDCARD_GROUPS;
+}
+
+// A RegExp that never matches any input, at any position — the safe, conservative compiled form of an
+// over-complex glob. "Never matches" (not "matches everything") is the correct default HERE because
+// globToRegExp has no context on caller intent, and a false "matches everything" would be actively wrong for a
+// non-guardrail caller (e.g. content-lane file-scope matching, where "matches everything" would misclassify
+// every changed file as a registry submission). A caller whose OWN semantics want the opposite fail direction
+// (a security guardrail, where under-protection is worse than an unnecessary hold) checks hasUnsafeWildcardCount
+// itself and overrides — see matchesAny below.
+const NEVER_MATCHES = /^(?!)$/;
+
 /** Convert a path glob (`*` matches within a segment, `**` matches across `/`) to an anchored RegExp. The
  *  glob is canonicalized first, so matching is case-insensitive against a canonicalized path. Exported for
  *  reuse anywhere a maintainer-supplied path pattern needs compiling — never compile a raw regex string from
- *  config (ReDoS risk); this linear-time glob compiler is the one safe path pattern this codebase uses. */
+ *  config (ReDoS risk); this linear-time glob compiler is the one safe path pattern this codebase uses.
+ *
+ *  An over-complex glob (see MAX_GLOB_WILDCARD_GROUPS) short-circuits to NEVER_MATCHES instead of being compiled —
+ *  this function never returns a RegExp that risks catastrophic backtracking on .test(), for any input. */
 export function globToRegExp(glob: string): RegExp {
+  if (hasUnsafeWildcardCount(glob)) return NEVER_MATCHES;
   const canonical = canonicalize(glob);
   let re = "";
   for (let i = 0; i < canonical.length; i += 1) {
@@ -38,10 +101,18 @@ export function globToRegExp(glob: string): RegExp {
   return new RegExp(`^${re}$`);
 }
 
-/** True if `path` matches any of the globs (`*` within a segment, `**` across `/`), case-insensitively. */
+/**
+ * True if `path` matches any of the globs (`*` within a segment, `**` across `/`), case-insensitively. A glob
+ * with more wildcards than can be safely compiled (see hasUnsafeWildcardCount) is treated as matching EVERY
+ * path — fail SAFE TOWARD GUARDING, mirroring isGuardrailHit's own "unknown ⇒ treat as a hit" philosophy (an
+ * over-complex guardrail glob still forces manual review) rather than the NEVER_MATCHES default globToRegExp
+ * itself falls back to, which would silently disable the maintainer's intended protection — the worse failure
+ * mode for a safety guardrail specifically (see globToRegExp's own docstring for why NEVER_MATCHES is still the
+ * right default for globToRegExp as a general-purpose compiler).
+ */
 export function matchesAny(path: string, globs: string[]): boolean {
   const canonicalPath = canonicalize(path);
-  return globs.some((g) => globToRegExp(g).test(canonicalPath));
+  return globs.some((g) => hasUnsafeWildcardCount(g) || globToRegExp(g).test(canonicalPath));
 }
 
 /**

test/unit/change-guardrail.test.ts

@@ -1,5 +1,45 @@
 import { describe, expect, it } from "vitest";
-import { changedPathsHittingGuardrail, isGuardrailHit, matchesAny } from "../../src/signals/change-guardrail";
+import { changedPathsHittingGuardrail, globToRegExp, isGuardrailHit, matchesAny } from "../../src/signals/change-guardrail";
+
+describe("globToRegExp (the exported compiler itself — must be safe for ANY direct caller, not just matchesAny)", () => {
+  it("compiles an ordinary glob to a working anchored RegExp", () => {
+    expect(globToRegExp("scripts/**").test("scripts/build.mjs")).toBe(true);
+    expect(globToRegExp("src/*.ts").test("src/auth.ts")).toBe(true);
+    expect(globToRegExp("src/*.ts").test("src/auth/session.ts")).toBe(false);
+  });
+
+  it("SECURITY (ReDoS): called DIRECTLY (bypassing matchesAny entirely) on a pathological glob, resolves instantly against a genuinely adversarial multi-KB path and matches nothing — the cap lives inside the compiler itself, not just in matchesAny's wrapper", () => {
+    // 3 chained single-segment wildcards is already empirically dangerous (over 2 seconds at ~4,000 chars — see
+    // MAX_GLOB_WILDCARD_GROUPS's rationale), so this glob alone proves the cap rejects the FIRST unsafe value, not
+    // just an extreme one.
+    const pathological = "src/*-*-*-final.ts";
+    const adversarialPath = "src/" + "a-".repeat(2000) + "X"; // ~4,000 chars — the empirically dangerous length for 3 wildcards
+    const start = Date.now();
+    const compiled = globToRegExp(pathological);
+    expect(compiled.test(adversarialPath)).toBe(false);
+    expect(compiled.test("completely/unrelated/path.md")).toBe(false);
+    expect(compiled.test("")).toBe(false);
+    expect(compiled.test("src/a-b-c-final.ts")).toBe(false); // even a "near miss" that would otherwise match
+    expect(Date.now() - start).toBeLessThan(1000);
+  });
+
+  it("a glob AT the safe cap (2 wildcards), called directly, still compiles and matches normally — proves the cap is inclusive, not exclusive", () => {
+    const atCap = "src/*/*.ts";
+    expect(globToRegExp(atCap).test("src/a/f.ts")).toBe(true);
+    expect(globToRegExp(atCap).test("src/a/f.js")).toBe(false);
+  });
+
+  it("SECURITY (ReDoS, correctness of the group-vs-character count): a single `**` globstar is ONE wildcard group (not two), so it never gets anywhere near the cap on its own", () => {
+    expect(globToRegExp("scripts/**").test("scripts/deep/nested/build.mjs")).toBe(true);
+  });
+
+  it("a `**` globstar PLUS a single `*` — 3 raw star CHARACTERS but only 2 wildcard GROUPS — compiles and matches normally, not the fail-safe path. This is the real content-lane/spec-resolver.ts shape (e.g. an artifactGlob like \"public/**/*.json\"); counting raw `*` characters instead of groups would wrongly reject it", () => {
+    const mixed = "public/**/*.json";
+    expect(globToRegExp(mixed).test("public/deep/nested/report.json")).toBe(true);
+    expect(globToRegExp(mixed).test("public/report.json")).toBe(true); // `**/` also matches zero segments
+    expect(globToRegExp(mixed).test("public/deep/nested/report.txt")).toBe(false); // wrong extension
+  });
+});
 
 describe("change-guardrail glob matching", () => {
   it("`**` matches across path separators (a guarded dir guards its whole subtree)", () => {
@@ -53,6 +93,44 @@ describe("change-guardrail glob matching", () => {
     // FAIL-SAFE (#1062): guardrails configured but the changed-file set is empty (unknown) ⇒ treat as a hit.
     expect(isGuardrailHit([], globs)).toBe(true);
   });
+
+  it("SECURITY (ReDoS): a glob with too many chained wildcards no longer risks catastrophic backtracking — it fails SAFE TOWARD GUARDING (matches every path) instead of ever compiling the pathological pattern", () => {
+    // 3 chained single-segment wildcards is already empirically dangerous (see MAX_GLOB_WILDCARD_GROUPS's rationale:
+    // over 2 seconds at a ~4,000-char adversarial path) — one over the cap, proving the boundary itself is safe,
+    // not just an extreme over-the-top example. Must resolve INSTANTLY even against that adversarial length.
+    const pathological = "src/*-*-*-final.ts";
+    const adversarialPath = "src/" + "a-".repeat(2000) + "X";
+    const start = Date.now();
+    // A pathological guardrail glob still HOLDS the PR for manual review (the safe direction for a guardrail —
+    // silently disabling protection would be far worse than an unnecessary hold).
+    expect(matchesAny(adversarialPath, [pathological])).toBe(true);
+    expect(matchesAny("completely/unrelated/path.md", [pathological])).toBe(true);
+    expect(matchesAny("", [pathological])).toBe(true);
+    expect(Date.now() - start).toBeLessThan(1000);
+    expect(changedPathsHittingGuardrail(["unrelated/file.ts"], [pathological])).toEqual(["unrelated/file.ts"]);
+    expect(isGuardrailHit(["unrelated/file.ts"], [pathological])).toBe(true);
+  });
+
+  it("SECURITY (ReDoS): a glob AT the safe cap (2 wildcards) still compiles and matches NORMALLY (not the fail-safe path)", () => {
+    // Exactly 2 stars: at the cap, still safely compiled/evaluated — proves the cap is inclusive, not exclusive,
+    // and that ordinary (non-pathological) multi-wildcard globs keep their real matching semantics. This is also
+    // the shape of nearly every real guardrail glob in production (a single `**` = 2 wildcard characters).
+    const atCap = "src/*/*.ts";
+    expect(matchesAny("src/a/f.ts", [atCap])).toBe(true);
+    expect(matchesAny("src/a/f.js", [atCap])).toBe(false); // wrong extension — genuinely doesn't match
+  });
+
+  it("a wildcard-free literal glob (e.g. an exact guarded file like '.gittensory.yml') is never treated as unsafe", () => {
+    expect(matchesAny(".gittensory.yml", [".gittensory.yml"])).toBe(true);
+    expect(matchesAny("other-file.yml", [".gittensory.yml"])).toBe(false);
+  });
+
+  it("a mix of one pathological glob among otherwise-fine globs still forces a hold for ANY path (fail-safe dominates)", () => {
+    const globs = ["docs/**", "src/*-*-*-final.ts"];
+    // "docs/**" alone would not match this path, but the pathological glob's fail-safe short-circuits matchesAny
+    // to true for every path once any configured glob is judged unsafe to compile.
+    expect(matchesAny("completely/unrelated.md", globs)).toBe(true);
+  });
 });
 
 // #flood-readiness: the live hard-guardrail globs must guard crucial files that live OUTSIDE the dir-prefix