Skip to content

fix(scoring): cap labelPatternToRegExp wildcard groups to prevent ReDoS#2482

Merged
JSONbored merged 1 commit into
mainfrom
fix/scoring-redos-cap-2456
Jul 2, 2026
Merged

fix(scoring): cap labelPatternToRegExp wildcard groups to prevent ReDoS#2482
JSONbored merged 1 commit into
mainfrom
fix/scoring-redos-cap-2456

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Summary

  • labelPatternToRegExp (src/scoring/preview.ts) compiled a registry-supplied label_multipliers key into a RegExp via unbounded chained-wildcard translation with no length/complexity cap — the exact class of bug fix(signals): cap globToRegExp wildcard count to prevent ReDoS #2445 fixed in src/signals/change-guardrail.ts's globToRegExp (which caps chained wildcard groups at 2, after benchmarking confirmed catastrophic backtracking: over 2 seconds at ~4,000 chars for 3 chained groups). This sibling function was never patched.
  • The pattern originates from a repo's registryConfig.labelMultipliers, sourced from the externally-fetched gittensor registry (registry/sync.ts + registry/normalize.ts) with no validation — not a value a repo's own maintainer directly controls via .gittensory.yml, despite an inaccurate in-code comment claiming the key set was "bounded, not attacker-supplied" (updated as part of this fix). Reachable via the public score-preview API, the MCP tool, and the per-PR label-audit signal (labelMatchesPattern), so one bad registry entry could hang scoring for every PR on that repo.
  • Reuses change-guardrail.ts's already-exported hasUnsafeWildcardCount directly (same *-group counting, same empirically-safe threshold) rather than reimplementing it — a * in this fnmatch-style pattern language has the identical "any run of chars" semantics as that glob compiler's *, so the same catastrophic-backtracking risk and safe boundary apply. An over-complex pattern now compiles to a safe never-match RegExp instead of a pathological one, mirroring globToRegExp's own NEVER_MATCHES fallback design.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run actionlint
  • npm run typecheck
  • npm run test:coverage — new code is 100% line+branch covered in the edited range (verified against coverage/lcov.info). Global 96.56%/95.53%.
  • npm run test:workers
  • npm run build:mcp
  • npm run test:mcp-pack
  • npm run ui:openapi:check
  • npm run ui:lint
  • npm run ui:typecheck
  • npm run ui:build
  • npm audit --audit-level=moderate — 0 vulnerabilities
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries

Full suite: 6064/6064 passing. Also ran npm run db:migrations:check (unaffected, green).

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. N/A (not an auth path), but added a timing-bound ReDoS regression test mirroring change-guardrail.test.ts's exact pattern: a 3-wildcard pathological pattern resolves instantly (<1s) against a ~4,000-char adversarial label, plus a not-over-the-cap test proving normal 2-wildcard patterns still compile and match correctly.
  • API/OpenAPI/MCP behavior is updated and tested where needed. No OpenAPI/MCP surface changed (internal scoring logic).
  • UI changes — N/A, no UI changed.
  • Visible UI changes — N/A, no UI changed.
  • Public docs/changelogs are updated where needed; changelogs are only edited for release-prep PRs. No changelog edit.

Notes

labelPatternToRegExp compiled a registry-supplied label_multipliers
key into a RegExp with unbounded chained-wildcard translation and no
length/complexity cap -- the exact class of bug #2445 fixed in
change-guardrail.ts's globToRegExp, which caps chained wildcard groups
at 2 after benchmarking confirmed catastrophic backtracking (over 2s
at ~4,000 chars for 3 chained groups). This sibling was never patched.

The pattern originates from a repo's registryConfig.labelMultipliers,
sourced from the externally-fetched gittensor registry (registry/
sync.ts + registry/normalize.ts) with no validation -- not a value
a repo's own maintainer directly controls via .gittensory.yml, despite
an inaccurate in-code comment claiming the key set was "bounded, not
attacker-supplied". Reachable via the public score-preview API, the
MCP tool, and the per-PR label-audit signal, so one bad registry entry
could hang scoring for every PR on that repo.

Reuses change-guardrail.ts's exported hasUnsafeWildcardCount directly
(same *-group counting, same empirically-safe threshold) rather than
reimplementing it -- a `*` in this fnmatch-style pattern language has
the identical "any run of chars" semantics as that glob compiler's
`*`, so the same catastrophic-backtracking risk and safe boundary
apply. An over-complex pattern now compiles to a safe never-match
instead of a pathological RegExp.

Fixes #2456
@dosubot dosubot Bot added the size:S label Jul 2, 2026
@loopover-orb

loopover-orb Bot commented Jul 2, 2026

Copy link
Copy Markdown

Warning

🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨🟨

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-02 05:44:00 UTC

2 files · 1 AI reviewer · no blockers · readiness 68/100 · CI pending · blocked

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change adds a shared wildcard-group cap before compiling registry-derived label patterns, caching an always-failing RegExp for over-complex patterns so label matching degrades to no match instead of compiling a pathological expression. The test covers both the fail-closed path for three wildcard groups and the inclusive boundary for two groups, which matches the intended behavior described in the diff. I do not see a reachable correctness break in the provided change.

Nits — 7 non-blocking
  • nit: test/unit/scoring.test.ts:1927 uses a wall-clock `Date.now() - start < 1000` assertion, which can be noisy on slow or overloaded runners even though the functional behavior is the important guarantee.
  • nit: src/scoring/preview.ts:931 adds a very long explanatory comment block with issue/history details that may be better shortened to the invariant and source of trust boundary.
  • test/unit/scoring.test.ts:1927: keep the ReDoS regression test focused on functional fail-closed behavior, or use a smaller deterministic timing budget pattern only if the suite already has stable performance-test conventions.
  • src/scoring/preview.ts:931: trim the cache comment to the durable facts: registry-derived patterns are bounded in count, untrusted in content, and compiled regexes are safe to share because they only use the `i` flag.
  • Pull request duplicates other open work — Check for an existing pull request or issue covering this change and coordinate or consolidate before continuing.
  • Readiness score is below the configured threshold — Use the readiness panel as advisory maintainer context; the score does not block this PR.
  • Touches a guarded path — held for manual review — A maintainer must review and merge this change.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ⚠️ Missing No linked issue or no-issue rationale found.
Related work ⚠️ 3 scoped overlaps Top overlaps are listed below; lower-confidence bulk is hidden.
Change scope ❌ 8/20 High review scope from cached public metadata (size label size:S; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 65 registered-repo PR(s), 55 merged, 554 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 554 issue(s).
Gate result ⚠️ Not blocking Advisory; not blocking this PR.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 65 PR(s), 554 issue(s).
  • Related work: Titles/paths share 7 meaningful terms. (issue #2456)
  • Related work: Titles/paths share 8 meaningful terms. (PR #2481)
  • Related work: Items reference the same linked issue fix(scoring): cap labelPatternToRegExp wildcard groups to prevent ReDoS #2456. (issue #2456, PR #2481)
  • Additional title-only matches omitted; title-only overlap does not block.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Review top overlaps.
  • Add a concise scope and risk note.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
  • Check active issues and PRs before submitting.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed


💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →.

Checked by Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@JSONbored JSONbored self-assigned this Jul 2, 2026
@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 2, 2026
@codecov

codecov Bot commented Jul 2, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.95%. Comparing base (118d537) to head (6549d63).
⚠️ Report is 26 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2482      +/-   ##
==========================================
+ Coverage   95.93%   95.95%   +0.01%     
==========================================
  Files         225      226       +1     
  Lines       25338    25391      +53     
  Branches     9218     9235      +17     
==========================================
+ Hits        24308    24363      +55     
  Misses        417      417              
+ Partials      613      611       -2     
Files with missing lines Coverage Δ
src/scoring/preview.ts 99.11% <100.00%> (+0.01%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored
JSONbored merged commit 0288f5f into main Jul 2, 2026
12 checks passed
@JSONbored
JSONbored deleted the fix/scoring-redos-cap-2456 branch July 2, 2026 06:19
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 2, 2026
@github-actions github-actions Bot mentioned this pull request Jul 2, 2026
12 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier.

Projects

No open projects
Status: Done

Development

Successfully merging this pull request may close these issues.

fix(scoring): cap labelPatternToRegExp wildcard groups to prevent ReDoS

1 participant