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
28 changes: 24 additions & 4 deletions src/scoring/preview.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type { ContributorEvidenceRecord, JsonValue, RepositoryRecord, RepoTimeDecayOverrides, ScoringModelSnapshotRecord, ScorePreviewRecord } from "../types";
import { DEFAULT_SCORING_CONSTANTS } from "./model";
import { nowIso } from "../utils/json";
import { hasUnsafeWildcardCount } from "../signals/change-guardrail";

export type ScorePreviewInput = {
repoFullName: string;
Expand Down Expand Up @@ -927,12 +928,21 @@ export function labelMatchesPattern(label: string, pattern: string): boolean {
// Compiled fnmatch→RegExp matchers are memoized by pattern. The same small,
// config-derived set of label keys is matched on every scored PR/issue, so the
// per-call recompile inside the nested label loops in engine.ts is pure waste.
// Keys are configured label patterns (bounded, not attacker-supplied), so the
// cache needs no eviction bound. The compiled RegExp carries only the "i" flag
// (no global/sticky `lastIndex` state), so sharing one instance across calls is
// safe and byte-identical to recompiling on every call.
// Keys come from a repo's registryConfig.labelMultipliers, sourced from the externally-fetched gittensor
// registry (registry/sync.ts + registry/normalize.ts, not a value this repo's own maintainer directly controls
// via .gittensory.yml) — so the pattern SET is small per repo, but individual pattern CONTENT is untrusted, not
// literally attacker-supplied-per-request the way GitHub PR content is. The cache still needs no eviction bound
// (the key set is bounded by real registry entries, not attacker-loop-controlled at request time), but the
// wildcard-count cap below (#2456) matters regardless of that distinction. The compiled RegExp carries only the
// "i" flag (no global/sticky `lastIndex` state), so sharing one instance across calls is safe and byte-identical
// to recompiling on every call.
const labelPatternRegExpCache = new Map<string, RegExp>();

// A RegExp that never matches any input — mirrors change-guardrail.ts's identical NEVER_MATCHES fallback for an
// over-complex pattern, so a pathological registry entry degrades to "this label multiplier never applies"
// instead of hanging the scoring path that evaluates it.
const LABEL_PATTERN_NEVER_MATCHES = /^(?!)$/;

// Upstream resolves label multipliers by matching each configured key as a Python `fnmatch` GLOB, not a
// literal string: `fnmatch(label.lower(), pattern.lower())` in
// gittensor/validator/oss_contributions/label_resolution.py, so a repo can configure `type:*`, `kind/*`, or
Expand All @@ -946,6 +956,16 @@ const labelPatternRegExpCache = new Map<string, RegExp>();
function labelPatternToRegExp(pattern: string): RegExp {
const cached = labelPatternRegExpCache.get(pattern);
if (cached !== undefined) 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)) {
labelPatternRegExpCache.set(pattern, LABEL_PATTERN_NEVER_MATCHES);
return LABEL_PATTERN_NEVER_MATCHES;
}
let regex = "";
let i = 0;
while (i < pattern.length) {
Expand Down
23 changes: 23 additions & 0 deletions test/unit/scoring.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1913,4 +1913,27 @@ describe("label pattern matcher memoization (#2106)", () => {
expect(labelMatchesPattern("bug", "bug")).toBe(true);
expect(labelMatchesPattern("bugfix", "bug")).toBe(false);
});

it("SECURITY (ReDoS, #2456): a label pattern with too many chained wildcards no longer risks catastrophic backtracking — it fails SAFE TOWARD NO MULTIPLIER (never matches) instead of ever compiling the pathological pattern", () => {
// 3 chained wildcards is already empirically dangerous for the identical `.*`-chaining shape this reuses
// from change-guardrail.ts's globToRegExp (see MAX_GLOB_WILDCARD_GROUPS's rationale: over 2 seconds at a
// ~4,000-char adversarial input) — 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. Unlike the
// guardrail glob (which fails toward MATCHING, the safe direction for a security hold), a label multiplier
// pattern fails toward NEVER matching — the safe direction here is "no multiplier applies", not "every label
// gets a multiplier".
const pathological = "*-*-*-final";
const adversarialLabel = "a-".repeat(2000) + "X"; // ~4,000 chars — the empirically dangerous length for 3 wildcards
const start = Date.now();
expect(labelMatchesPattern(adversarialLabel, pathological)).toBe(false);
expect(labelMatchesPattern("completely-unrelated-label", pathological)).toBe(false);
expect(labelMatchesPattern("", pathological)).toBe(false);
expect(labelMatchesPattern("a-b-c-final", pathological)).toBe(false); // even a "near miss" that would otherwise match
expect(Date.now() - start).toBeLessThan(1000);
});

it("a label pattern AT the safe cap (2 wildcards) still compiles and matches NORMALLY, not the fail-safe path — proves the cap is inclusive, not exclusive", () => {
expect(labelMatchesPattern("type-bug-fix", "type-*-*")).toBe(true);
expect(labelMatchesPattern("type-bug", "type-*-*")).toBe(false);
});
});
Loading