Skip to content

fix(scoring): bound label-pattern RegExp cache with LRU eviction - #2513

Merged
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-unbounded-regex-cache-vulnerability
Jul 2, 2026
Merged

fix(scoring): bound label-pattern RegExp cache with LRU eviction#2513
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-unbounded-regex-cache-vulnerability

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • A recent change introduced an unbounded module-level cache of compiled RegExp objects for label patterns, which can grow without bound when registry/config sources supply many unique patterns and cause availability/memory exhaustion.
  • The intent is to retain the runtime benefit of memoizing compiled matchers while preventing persistent memory growth from attacker- or config-driven churn.

Description

  • Add a bounded cache limit LABEL_PATTERN_REGEXP_CACHE_MAX_ENTRIES = 256 and evict the oldest entry when the limit is reached in labelPatternToRegExp in src/scoring/preview.ts.
  • Refresh an entry's recency on cache hits by re-inserting the key so the cache behaves as a simple LRU, and preserve the existing fnmatch→RegExp translation semantics.
  • Export small test helpers clearLabelPatternRegExpCacheForTest and labelPatternRegExpCacheKeysForTest so tests can deterministically exercise caching behavior, and add a regression unit test that fills the cache, asserts the bound, refreshes an entry, and verifies LRU eviction in test/unit/scoring.test.ts.

Testing

  • Ran the targeted unit tests with npx vitest run test/unit/scoring.test.ts -t "label pattern cache|fnmatch globs" and the added tests passed.
  • Ran npm run typecheck and TypeScript typecheck succeeded with no errors.
  • Ran a scoped coverage run for the modified unit test file which executed successfully, but a full global coverage threshold run failed as expected in this environment (the repository requires the entire suite for global thresholds).
  • Attempting the full local gate npm run test:ci and npm audit --audit-level=moderate was blocked by external network/setup issues (actionlint setup / npm audit endpoint) in this environment, so those checks were not completed here.

Codex Task

@loopover-orb

loopover-orb Bot commented Jul 2, 2026

Copy link
Copy Markdown
Contributor

Warning

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

⏸️ Gittensory review result - manual review recommended

Review updated: 2026-07-02 07:24:29 UTC

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

⏸️ Suggested Action - Manual Review

  • Touches a guarded path — held for manual review

Review summary
The change replaces the unbounded label-pattern RegExp memoization with a fixed-size Map-backed LRU and updates both normal and unsafe-pattern insertions to go through the bounded path. The hit path refreshes recency correctly with delete+set, and the regression test exercises the exact cache-fill, hit-refresh, and eviction behavior introduced here. No visible hunk changes label matching semantics beyond cache retention, so the diff is safe enough to proceed.

Nits — 5 non-blocking
  • nit: test/unit/scoring.test.ts:874 clears the shared cache at the end of the test, but a failed assertion before that line would leave module state polluted for later tests; prefer an afterEach/finally cleanup around this cache-specific test.
  • nit: src/scoring/preview.ts:1024 exports production-module test helpers, which is acceptable here but should stay explicitly test-only and undocumented as public API.
  • test/unit/scoring.test.ts:874 wrap the cache mutation test in try/finally or add a local afterEach cleanup so failures do not leak cache contents into subsequent tests.
  • src/scoring/preview.ts:1024 add a short comment above the ForTest exports stating they are not part of the runtime/public scoring API.
  • 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 ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (size label size:M; 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, 548 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 65 PR(s), 548 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), 548 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • Explain no-issue PR.
  • Triage stale or unlinked PRs.
  • No action.
  • Link the issue being solved, or explicitly explain why this is a no-issue PR.
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

@loopover-orb loopover-orb Bot added gittensor gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. labels Jul 2, 2026
@JSONbored JSONbored self-assigned this Jul 2, 2026
@JSONbored
JSONbored force-pushed the codex/propose-fix-for-unbounded-regex-cache-vulnerability branch from 8230654 to 0eed7e0 Compare July 2, 2026 07:19
@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 (4101ca6) to head (0eed7e0).
⚠️ Report is 38 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2513      +/-   ##
==========================================
+ Coverage   95.94%   95.95%   +0.01%     
==========================================
  Files         226      226              
  Lines       25361    25438      +77     
  Branches     9229     9246      +17     
==========================================
+ Hits        24333    24410      +77     
  Misses        417      417              
  Partials      611      611              
Files with missing lines Coverage Δ
src/scoring/preview.ts 99.14% <100.00%> (+0.03%) ⬆️
🚀 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 36d01f3 into main Jul 2, 2026
12 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-unbounded-regex-cache-vulnerability branch July 2, 2026 07:44
@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.

1 participant