Skip to content

engine(scoring): count fnmatch wildcard groups the way labelPatternToRegExp compiles them, not the way path globs do #9994

Description

@JSONbored

⚠️ Definition of Done: this issue must be completed in full, in a single PR. Do not split this
work across multiple PRs, and do not defer any Deliverable below to a follow-up issue. A PR that
satisfies only some of the Deliverables, stubs a required test, or leaves a checkbox
partially-done does NOT resolve this issue and will be closed.

Context

labelPatternToRegExp guards against catastrophic backtracking by reusing the path-glob compiler's
wildcard-group predicate. packages/loopover-engine/src/scoring/label-match.ts:54:

  // 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) ...
  if (hasUnsafeWildcardCount(pattern)) {
    setLabelPatternRegExpCacheEntry(pattern, LABEL_PATTERN_NEVER_MATCHES);
    return LABEL_PATTERN_NEVER_MATCHES;
  }

The stated equivalence does not hold for **. countWildcardGroups
(packages/loopover-engine/src/signals/change-guardrail.ts:44) deliberately treats a ** PAIR as ONE
group, because the PATH compiler collapses it into a single .*
(packages/loopover-engine/src/signals/change-guardrail.ts:89-92):

    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

The FNMATCH compiler in label-match.ts has no ** concept. It emits one .* per *
(packages/loopover-engine/src/scoring/label-match.ts:69):

    if (char === "*") {
      regex += ".*";

So the count and the compiled regex disagree whenever the pattern contains **:

pattern countWildcardGroups actual .* groups compiled hasUnsafeWildcardCount
*a**b 2 3 false (accepted)
**/** 2 4 false (accepted)
a**b**c 2 4 false (accepted)

The cap's own benchmark, recorded at packages/loopover-engine/src/signals/change-guardrail.ts:24, puts
3 groups at "OVER 2 SECONDS at just ~4,000 chars" and 4 groups at "35 SECONDS at just 1,614 chars". The
comment at packages/loopover-engine/src/signals/change-guardrail.ts:30 states the cap "lives INSIDE
globToRegExp itself ... every caller is protected automatically" — label-match.ts is a caller that calls
the predicate but compiles its own regex, so it is exactly the case that is NOT automatically protected.

labelMatchesPattern is reachable with attacker-influenced left-hand input: ScorePreviewInput.labels
(packages/loopover-engine/src/scoring/preview.ts, consumed at
packages/loopover-engine/src/scoring/preview.ts:951) is caller-supplied, and the same matcher runs on
every scored PR through packages/loopover-engine/src/advisory/gate-advisory.ts:389 and
packages/loopover-engine/src/signals/engine.ts:1084. The right-hand patterns are
registryConfig.labelMultipliers keys, which label-match.ts:19 itself describes as "untrusted".

Requirements

  • labelPatternToRegExp must reject a pattern whose count of COMPILED wildcard groups exceeds
    MAX_GLOB_WILDCARD_GROUPS, where for fnmatch each individual * is one group (there is no ** pairing
    rule) and each ? is not a group (it compiles to a single ., which cannot backtrack ambiguously).
  • The threshold value itself must remain MAX_GLOB_WILDCARD_GROUPS from
    packages/loopover-engine/src/signals/change-guardrail.ts:37, imported, not redeclared — a caller with
    its own independently-chosen threshold is the exact drift
    packages/loopover-engine/src/signals/change-guardrail.ts:57-61 warns about. Export the constant from
    change-guardrail.ts if it is not already exported; do not copy its literal value into label-match.ts.
  • A rejected pattern must degrade to the existing LABEL_PATTERN_NEVER_MATCHES and must still be cached,
    exactly as today — the failure mode ("this label multiplier never applies") is correct and must not change.
  • What must NOT change: hasUnsafeWildcardCount, countWildcardGroups, globToRegExp, matchesAny and
    every path-glob consumer keep their current **-is-one-group semantics byte-identically — that rule is
    correct for the path compiler and rejecting public/**/*.json would break
    src/review/content-lane/spec-resolver.ts:40. Patterns with 2 or fewer compiled groups (type:*,
    kind/*, priority:?, a*b*c) must keep matching exactly as they do today, including the
    [seq] / [!seq] / invalid-range handling at packages/loopover-engine/src/scoring/label-match.ts:73-94
    and the LRU cache behaviour at packages/loopover-engine/src/scoring/label-match.ts:109.

⚠️ Required pattern: keep the shape of the existing guard at
packages/loopover-engine/src/scoring/label-match.ts:60 — a single predicate call before compilation,
falling back to LABEL_PATTERN_NEVER_MATCHES — and add a small fnmatch-specific counter next to it in
label-match.ts that counts raw * characters. What does NOT satisfy this issue: (a) changing
countWildcardGroups in change-guardrail.ts to count each * separately, which would start rejecting
legitimate 2-group path globs like public/**/*.json repo-wide and break the content lane; (b) collapsing
runs of * into one before counting, which makes **/** count as 2 while it still compiles to 4 .*;
(c) replacing the fnmatch compiler with globToRegExp, which has different (segment-bounded) semantics and
would silently change every configured label multiplier's matching; (d) a test-only PR.

Deliverables

  • packages/loopover-engine/src/scoring/label-match.tslabelMatchesPattern("anything", "*a**b")
    returns false (the pattern is rejected as over-complex), where today it compiles ^.*a.*.*b$.
  • packages/loopover-engine/src/scoring/label-match.tslabelMatchesPattern("x/y", "**/**") returns
    false.
  • packages/loopover-engine/src/scoring/label-match.ts — the preserved cases still pass:
    labelMatchesPattern("type:bug-fix", "type:*") is true,
    labelMatchesPattern("priority:1", "priority:?") is true,
    labelMatchesPattern("a-b-c", "a*b*c") is true, and
    labelMatchesPattern("kind:bug", "kind:[bc]ug") is true.
  • MAX_GLOB_WILDCARD_GROUPS exported from
    packages/loopover-engine/src/signals/change-guardrail.ts and imported by label-match.ts; the
    literal 2 must not appear as a new threshold constant in label-match.ts.
  • A regression test at packages/loopover-engine/test/label-match.test.ts (this file does not exist
    yet; create it, importing from ../dist/scoring/label-match.js per the convention in
    packages/loopover-engine/test/content-lane-flag.test.ts) named for this bug, asserting the
    three-.*-from-two-counted-groups case ("*a**b") is rejected.
  • Root-suite coverage in test/unit/scoring.test.ts (which already imports labelMatchesPattern, see
    test/unit/scoring.test.ts:4) asserting the same rejection plus the preserved 2-group cases.

All Deliverables above are required in a single PR. A PR that satisfies only some of them — for example one
that adds the fnmatch counter but hardcodes 2 in label-match.ts instead of importing the shared
constant, or one that adds only the root test/unit/scoring.test.ts assertions — does not resolve this issue.

Test Coverage Requirements

This repo enforces 99%+ Codecov patch coverage, branch-counted. vitest.config.ts's
coverage.include covers src/**/*.ts and packages/loopover-engine/src/**/*.ts — both touched paths
(packages/loopover-engine/src/scoring/label-match.ts and
packages/loopover-engine/src/signals/change-guardrail.ts) ARE measured.

Branches introduced or touched, each needing BOTH arms tested: the new fnmatch group-count check (over cap →
LABEL_PATTERN_NEVER_MATCHES returned and cached; at or under cap → normal compilation); and the existing
cache-hit / cache-miss arms around it at packages/loopover-engine/src/scoring/label-match.ts:46-53, which
must still be exercised for a rejected pattern (call labelMatchesPattern with the same over-complex
pattern twice and assert the second call is served from the cache via
labelPatternRegExpCacheKeysForTest).

Engine lines are credited by two uploads whose hits are unioned — add the test to
packages/loopover-engine/test/** as well as any root test/** coverage, or the patch gate can still fail.

Expected Outcome

The ReDoS cap that label-match.ts claims to inherit is actually enforced against the regex it really
compiles: an fnmatch label pattern can no longer smuggle 3 or 4 chained .* groups past a predicate that
was counting them as 2, so a hostile registry labelMultipliers key can no longer hang scoring on an
adversarial label supplied through the score-preview API, the MCP tool, or the per-PR label audit.

Links & Resources

  • packages/loopover-engine/src/scoring/label-match.ts:54 — the guard and the equivalence claim that fails for **
  • packages/loopover-engine/src/scoring/label-match.ts:69 — one .* emitted per *
  • packages/loopover-engine/src/signals/change-guardrail.ts:24 — the backtracking benchmark the cap is set from
  • packages/loopover-engine/src/signals/change-guardrail.ts:44countWildcardGroups' **-is-one-group rule
  • packages/loopover-engine/src/scoring/preview.ts:951 — the score-preview call site
  • packages/loopover-engine/src/advisory/gate-advisory.ts:389, packages/loopover-engine/src/signals/engine.ts:1084 — the per-PR call sites

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions