Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
31 changes: 23 additions & 8 deletions packages/loopover-engine/src/scoring/label-match.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { hasUnsafeWildcardCount } from "../signals/change-guardrail.js";
import { MAX_GLOB_WILDCARD_GROUPS } from "../signals/change-guardrail.js";

export function labelMatchesPattern(label: string, pattern: string): boolean {
return labelPatternToRegExp(pattern.toLowerCase()).test(label.toLowerCase());
Expand Down Expand Up @@ -42,6 +42,19 @@ const LABEL_PATTERN_NEVER_MATCHES = /^(?!)$/;
// change-guardrail.ts (there `*` stops at `/` and `?` is literal): labels are flat strings, so `*` matches any
// run, `?` any single character, and `[seq]`/`[!seq]` a character class. Literal keys are unaffected — for a
// pattern with no glob metacharacter the RegExp is an exact match, so existing configs score identically.

/** Count the backtracking-capable wildcard GROUPS this fnmatch compiler will emit: one per raw `*` (each
* compiles to a `.*` below), with NO `**`-is-one-group rule — unlike change-guardrail's path-glob counter,
* this compiler has no globstar concept, so `**` is two `.*` groups, not one (#9994). `?` is not counted (it
* compiles to a single `.`, which cannot backtrack ambiguously), and neither are `[…]` classes. */
function fnmatchWildcardGroups(pattern: string): number {
let count = 0;
for (let i = 0; i < pattern.length; i += 1) {
if (pattern.charAt(i) === "*") count += 1;
}
return count;
}

function labelPatternToRegExp(pattern: string): RegExp {
const cached = labelPatternRegExpCache.get(pattern);
if (cached !== undefined) {
Expand All @@ -51,13 +64,15 @@ function labelPatternToRegExp(pattern: string): RegExp {
labelPatternRegExpCache.set(pattern, cached);
return cached;
}
// Reuses change-guardrail.ts's wildcard-GROUP counting (a `*` here matches the same "any run of chars"
// semantics as that glob compiler's `*`, so the same catastrophic-backtracking risk and the same empirically-
// safe threshold apply) — an over-complex registry-sourced label_multipliers key degrades to a safe never-match
// instead of hanging RegExp.test() on an adversarial near-miss label (#2456). Reachable via the public
// score-preview API, the MCP tool, and the per-PR label-audit signal, so one bad registry entry could otherwise
// hang scoring for every PR on that repo.
if (hasUnsafeWildcardCount(pattern)) {
// Reject an over-complex registry-sourced label_multipliers key so it degrades to a safe never-match instead
// of hanging RegExp.test() on an adversarial near-miss label (#2456). Reachable via the public score-preview
// API, the MCP tool, and the per-PR label-audit signal, so one bad registry entry could otherwise hang
// scoring for every PR on that repo. Counting is fnmatch-specific, NOT change-guardrail's path-glob count:
// this compiler emits one `.*` per `*` with no `**` pairing (see below), so `**` is TWO backtracking groups
// here, not the single `.*` the path compiler collapses it into (#9994) — counting via that predicate would
// undercount `**` and admit a glob this compiler builds into a catastrophic-backtracking RegExp. The threshold
// itself is the shared MAX_GLOB_WILDCARD_GROUPS, so the two surfaces stay on one empirically-safe boundary.
if (fnmatchWildcardGroups(pattern) > MAX_GLOB_WILDCARD_GROUPS) {
setLabelPatternRegExpCacheEntry(pattern, LABEL_PATTERN_NEVER_MATCHES);
return LABEL_PATTERN_NEVER_MATCHES;
}
Expand Down
5 changes: 4 additions & 1 deletion packages/loopover-engine/src/signals/change-guardrail.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,10 @@ export function canonicalize(value: string): string {
// 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;
// Exported (#9994) so label-match.ts's fnmatch compiler applies the SAME empirically-safe threshold rather
// than redeclaring its own literal — a second, independently-chosen cap is exactly the drift the
// hasUnsafeWildcardCount export below warns about.
export 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
Expand Down
34 changes: 34 additions & 0 deletions packages/loopover-engine/test/label-match.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
import { test } from "node:test";
import assert from "node:assert/strict";

import {
clearLabelPatternRegExpCacheForTest,
labelMatchesPattern,
labelPatternRegExpCacheKeysForTest,
} from "../dist/scoring/label-match.js";

// #9994: the fnmatch compiler emits one `.*` per `*` and has no `**` concept, so `*a**b` compiles to THREE
// `.*` groups. The old guard reused change-guardrail's path-glob counter, which scores a `**` pair as ONE
// group, undercounting `**` and admitting a pattern this compiler builds into a catastrophic-backtracking
// RegExp. Counting is now fnmatch-specific (one per raw `*`), so `**`-containing patterns over the cap are
// rejected — they fail SAFE toward no-multiplier (never match).
test("#9994: a pattern whose COMPILED groups exceed the cap via `**` is rejected (never matches)", () => {
clearLabelPatternRegExpCacheForTest();
assert.equal(labelMatchesPattern("anything", "*a**b"), false); // 3 stars → 3 groups → rejected
assert.equal(labelMatchesPattern("x/y", "**/**"), false); // 4 stars → 4 groups → rejected
});

test("#9994: the preserved 2-group and non-`*` cases still match exactly", () => {
assert.equal(labelMatchesPattern("type:bug-fix", "type:*"), true);
assert.equal(labelMatchesPattern("priority:1", "priority:?"), true); // `?` is not a counted group
assert.equal(labelMatchesPattern("a-b-c", "a*b*c"), true); // 2 stars, at the cap
assert.equal(labelMatchesPattern("kind:bug", "kind:[bc]ug"), true); // classes are not counted groups
});

test("#9994: a rejected over-complex pattern is still cached (repeated read served from cache)", () => {
clearLabelPatternRegExpCacheForTest();
assert.equal(labelMatchesPattern("anything", "*a**b"), false);
assert.ok(labelPatternRegExpCacheKeysForTest().includes("*a**b"));
assert.equal(labelMatchesPattern("something-else", "*a**b"), false); // cache-hit arm, still false
assert.equal(labelPatternRegExpCacheKeysForTest().filter((k) => k === "*a**b").length, 1);
});
33 changes: 31 additions & 2 deletions test/unit/scoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -960,9 +960,12 @@ NOVELTY_BONUS_SCALAR = 3
// Regex metacharacters in a literal key stay literal: `.` matches only a dot, not any char.
expect(labelMultiplierFor({ "v1.0": 1.1 }, ["v1.0"])).toBe(1.1);
expect(labelMultiplierFor({ "v1.0": 1.1 }, ["v1x0"])).toBe(1);
// `**/` counts as one wildcard group and matches across path-like label segments.
// #9994: this fnmatch compiler counts one wildcard group per raw `*` (it has no `**`-is-one-group rule —
// that is the PATH compiler's semantics), so `**/bug` and `**bug` are 2 groups (at the cap → still
// compile and match), but `public/**/*.json` is THREE (`**` + `*`) and is now rejected as over-complex,
// failing SAFE toward no multiplier — where the old path-glob count wrongly scored it as 2 and matched it.
expect(labelMultiplierFor({ "**/bug": 1.45 }, ["feature/bug"])).toBe(1.45);
expect(labelMultiplierFor({ "public/**/*.json": 1.2 }, ["public/release/config.json"])).toBe(1.2);
expect(labelMultiplierFor({ "public/**/*.json": 1.2 }, ["public/release/config.json"])).toBe(1);
expect(labelMultiplierFor({ "**bug": 1.35 }, ["feature-bug"])).toBe(1.35);
// When several patterns match, the highest multiplier wins (mirrors upstream `max(...)`).
expect(labelMultiplierFor({ "kind/*": 1.1, "*/bug": 1.6 }, ["kind/bug"])).toBe(1.6);
Expand Down Expand Up @@ -2150,4 +2153,30 @@ describe("label pattern matcher memoization (#2106)", () => {
expect(labelMatchesPattern("type-bug-fix", "type-*-*")).toBe(true);
expect(labelMatchesPattern("type-bug", "type-*-*")).toBe(false);
});

it("#9994: a `**`-containing pattern is counted by its COMPILED groups (one per raw *), not the path-glob `**`-is-one rule, so it is rejected", () => {
// The fnmatch compiler emits one `.*` per `*` and has no `**` concept, so `*a**b` compiles to THREE `.*`
// groups. change-guardrail's path counter scored it as 2 (a `**` pair = one group) and wrongly ACCEPTED it,
// admitting a pattern this compiler builds into a catastrophic-backtracking RegExp. All three fail SAFE
// toward no-multiplier (never matches).
clearLabelPatternRegExpCacheForTest();
expect(labelMatchesPattern("anything", "*a**b")).toBe(false); // 3 stars → 3 compiled groups → rejected
expect(labelMatchesPattern("x/y", "**/**")).toBe(false); // 4 stars → 4 compiled groups → rejected
expect(labelMatchesPattern("abc", "a**b**c")).toBe(false); // 4 stars → rejected

// The preserved 2-group cases and non-`*` metacharacters still compile and match exactly as before.
expect(labelMatchesPattern("type:bug-fix", "type:*")).toBe(true);
expect(labelMatchesPattern("priority:1", "priority:?")).toBe(true); // `?` is not a counted group
expect(labelMatchesPattern("a-b-c", "a*b*c")).toBe(true);
expect(labelMatchesPattern("kind:bug", "kind:[bc]ug")).toBe(true); // classes are not counted groups
});

it("#9994: a rejected over-complex pattern is still cached, so a repeated read is served from the cache", () => {
clearLabelPatternRegExpCacheForTest();
expect(labelMatchesPattern("anything", "*a**b")).toBe(false);
expect(labelPatternRegExpCacheKeysForTest()).toContain("*a**b");
// Second read of the same over-complex pattern is served from the cache (cache-hit arm), still false.
expect(labelMatchesPattern("something-else", "*a**b")).toBe(false);
expect(labelPatternRegExpCacheKeysForTest().filter((k) => k === "*a**b")).toHaveLength(1);
});
});