Skip to content

fix(mcp): avoid regex slash trimming in remote parsing - #1947

Merged
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-regex-dos-in-parsegitremote
Jul 1, 2026
Merged

fix(mcp): avoid regex slash trimming in remote parsing#1947
JSONbored merged 1 commit into
mainfrom
codex/propose-fix-for-regex-dos-in-parsegitremote

Conversation

@JSONbored

Copy link
Copy Markdown
Owner

Motivation

  • The previous trailing-slash normalization used replace(/\/+$/, ""), which can trigger quadratic backtracking on attacker-controlled long runs of slashes and hang local MCP/CLI runs.
  • The change hardens local MCP availability by removing the backtracking-prone regex while preserving existing remote parsing semantics.

Description

  • Replaced the regex-based trailing-slash removal in parseGitRemote with a linear stripTrailingSlashes scanner to avoid regex backtracking (file: packages/gittensory-mcp/lib/local-branch.js).
  • Kept the original GitHub remote matching patterns and return shape (owner/repo) intact so behavior is preserved for valid remotes.
  • Added regression assertions to test/unit/local-branch.test.ts that cover multiple trailing slashes and a crafted long slash-run followed by a non-slash to exercise the previously-vulnerable path.

Testing

  • Ran the focused unit test npx vitest run test/unit/local-branch.test.ts -t "parses remotes" and it passed.
  • npm run build:mcp, npm run test:mcp-pack, and npm run typecheck completed successfully.
  • Full npm run test:coverage / npm run test:ci could not complete in this environment because actionlint setup failed due to network/DNS issues and the audit endpoint returned 403 Forbidden, and a scoped coverage run showed the focused test passed but global coverage thresholds were not met when only the single-file coverage was measured.

Codex Task

@cloudflare-workers-and-pages

cloudflare-workers-and-pages Bot commented Jul 1, 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 b91d628 Commit Preview URL

Branch Preview URL
Jul 01 2026, 12:57 AM

@loopover-orb

loopover-orb Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-01 04:30:35 UTC

2 files · 1 AI reviewer · no blockers · readiness 86/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The change replaces the trailing-slash regex in `parseGitRemote` with a simple backward scanner, preserving the existing trim-before-normalize behavior and the GitHub remote patterns. The scanner is linear, handles already-normalized inputs without allocation, and the added assertions cover both semantic preservation for repeated trailing slashes and the long slash-run non-match case. I do not see a reachable correctness regression in the visible diff.

Nits — 3 non-blocking
  • nit: `test/unit/local-branch.test.ts:1748` uses a 32,000-character case, which is fine for a regression test but could be made more self-documenting by naming the constructed remote before the assertion.
  • `test/unit/local-branch.test.ts:1748`: Consider assigning the crafted input to a `const pathologicalRemote = ...` so the regression intent is visible without parsing the template literal inline.
  • Code changes lack test evidence — Add focused regression tests or explain why existing coverage is sufficient.
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:XS; no linked issue context).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 68 registered-repo PR(s), 58 merged, 274 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 68 PR(s), 274 issue(s).
Gate result ✅ Passing No configured blocker found.
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: 68 PR(s), 274 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 1, 2026
@codecov

codecov Bot commented Jul 1, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.56%. Comparing base (725831b) to head (b91d628).
⚠️ Report is 6 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #1947   +/-   ##
=======================================
  Coverage   95.56%   95.56%           
=======================================
  Files         217      217           
  Lines       24139    24139           
  Branches     8749     8749           
=======================================
  Hits        23068    23068           
  Misses        438      438           
  Partials      633      633           
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@JSONbored JSONbored self-assigned this Jul 1, 2026
@JSONbored
JSONbored merged commit fa09b39 into main Jul 1, 2026
14 checks passed
@JSONbored
JSONbored deleted the codex/propose-fix-for-regex-dos-in-parsegitremote branch July 1, 2026 05:01
@github-project-automation github-project-automation Bot moved this from Todo to Done in gittensory - v1 roadmap Jul 1, 2026
@github-actions github-actions Bot mentioned this pull request Jul 1, 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