Skip to content

fix(signals): preserve segment boundaries for globstar paths - #1426

Merged
JSONbored merged 3 commits into
mainfrom
codex/propose-fix-for-overbroad-globs-issue
Jun 26, 2026
Merged

fix(signals): preserve segment boundaries for globstar paths#1426
JSONbored merged 3 commits into
mainfrom
codex/propose-fix-for-overbroad-globs-issue

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • Prevent **/ rewrite from removing the path-separator and turning segment globs (e.g. **/safe.ts) into broad suffix globs that match attacker-controlled filenames like unsafe.ts.
  • Retain the linear-time matching and root-level **/ semantics while avoiding ReDoS and avoiding overbroad exclusions used by review.exclude_paths.

Description

  • Replaced the previous global **/* rewrite with an expandGlobstarSlash routine that produces explicit alternatives that preserve the / segment when appropriate. (src/signals/focus-manifest.ts)
  • Compile-time matchers are now built as a set of alternatives and test each alternative with the existing linear matcher; exact alternatives are tested with a simple equality predicate for efficiency. (src/signals/focus-manifest.ts)
  • Added a protective cap (MAX_GLOBSTAR_SLASH_ALTERNATIVES) to bound repeated **/ expansion so matcher expansion remains safe and performant. (src/signals/focus-manifest.ts)
  • Added regression tests that assert correct **/ semantics (basename-only matches do not match suffix collisions), ensure excludeReviewPaths no longer drops attacker-named files, and exercise bounded repeated **/ expansion. (test/unit/focus-manifest.test.ts)

Testing

  • Ran the focused unit suite with npm test -- --run test/unit/focus-manifest.test.ts, and all tests in that file passed.
  • Ran static type checks with npm run typecheck, and tsc --noEmit passed.
  • Ran git diff --check and it reported no issues.
  • Attempted coverage with npm run test:coverage -- --run test/unit/focus-manifest.test.ts; the tests passed but coverage remapping failed with TypeError: jsTokens is not a function during coverage conversion (investigation required); this prevented completing the full test:coverage remap step in CI locally.
  • npm run test:ci could not be completed locally due to the coverage remapping issue and external-network-dependent steps; npm audit --audit-level=moderate returned a 403 from the registry in this environment.

Codex Task

@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@codecov

codecov Bot commented Jun 26, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.44%. Comparing base (eb4bfcb) to head (e376c52).
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1426   +/-   ##
=======================================
  Coverage   95.44%   95.44%           
=======================================
  Files         194      194           
  Lines       21054    21070   +16     
  Branches     7619     7621    +2     
=======================================
+ Hits        20094    20110   +16     
  Misses        383      383           
  Partials      577      577           
Files with missing lines Coverage Δ
src/signals/focus-manifest.ts 98.18% <100.00%> (+0.05%) ⬆️
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jun 26, 2026

Copy link
Copy Markdown

Deploying with  Cloudflare Workers  Cloudflare Workers

The latest updates on your project. Learn more about integrating Git with Workers.

Status Name Latest Commit Preview URL Updated (UTC)
✅ Deployment successful!
View logs
gittensory-ui e376c52 Commit Preview URL

Branch Preview URL
Jun 26 2026, 09:46 AM

@JSONbored JSONbored self-assigned this Jun 26, 2026
@JSONbored JSONbored added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jun 26, 2026
@JSONbored

Copy link
Copy Markdown
Owner Author

Verdict: MERGE · gittensor:bug (security). **/safe.ts was matching unsafe.ts in the manifest matcher (backs review.exclude_paths); boundary preserved, blow-up bounded, both-branch tested, green.

@JSONbored
JSONbored merged commit c732bea into main Jun 26, 2026
19 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-overbroad-globs-issue branch June 26, 2026 20:38
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

None yet

Development

Successfully merging this pull request may close these issues.

1 participant