Skip to content

fix(security): patch picomatch ReDoS/method injection - #679

Open
birme wants to merge 2 commits into
mainfrom
security/641-picomatch-redos
Open

birme wants to merge 2 commits into
mainfrom
security/641-picomatch-redos

Conversation

@birme

@birme birme commented Sep 3, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Patched picomatch transitive vulnerabilities (GHSA-c2c7-rcm5-vvqj ReDoS, GHSA-3v7f-55p6-f55p method injection). Vulnerable range: <=2.3.1 || 4.0.0 - 4.0.3.
  • Two majors were in use; both bumped within their own major (no breaking major forced):
    • v2 tree (lint-staged -> micromatch): 2.3.1 -> 2.3.2
    • v4 trees (vite, vitest, vite-plugin-svgr, tinyglobby, fdir): 4.0.3 -> 4.0.7
  • Method: npm audit fix only — lockfile-only change, no overrides needed. It also patch-bumped vite 7.3.1 -> 7.3.6.

Test plan

  • Tests pass (npm test) — 147 tests across 18 files
  • TypeScript compiles (npm run typecheck)
  • Lint clean (npm run lint)
  • npm ls picomatch shows only safe versions (2.3.2 / 4.0.7)

Closes #641

🤖 Generated with Claude Code

Co-Authored-By: Claude Sonnet 4.6 noreply@anthropic.com

…ction)

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

@birme birme left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

Verdict: LGTM

Summary: Lockfile-only security patch for picomatch (GHSA-c2c7-rcm5-vvqj ReDoS, GHSA-3v7f-55p6-f55p method injection; vulnerable range <=2.3.1 || 4.0.0-4.0.3). Verified all five picomatch entries in package-lock.json now resolve to safe versions — one v2 copy at 2.3.2 (node_modules/picomatch:6719) and four v4 copies at 4.0.7 (@rollup/pluginutils:1454, tinyglobby:7860, vite:8214, vitest:8317). Grep for the vulnerable versions (2.3.0/2.3.1, 4.0.0–4.0.3) returns zero matches, so there are no leftover nested vulnerable copies. package.json is untouched (no direct-dep major forced); the extra transitive bumps (babel, browserslist, ajv, minimatch, vite 7.3.1→7.3.6) are the expected npm audit fix re-resolution cascade, not sneaked-in unrelated changes.

npm hygiene clean: lockfileVersion: 3, no yarn.lock, no overrides block introduced. No secrets/eval (lockfile-only). All 6 CI checks (lint, prettier, tests, builds, e2e x2) are green; mergeable.

Blocking

  • None

Warnings

  • None

Suggestions

  • The PR body understates the diff scope: it lists only picomatch + vite, but npm audit fix also re-resolved ~75 transitive entries. Harmless, but a note in the description would aid future auditors.

Do NOT merge/approve on my behalf — this is an automated review comment only.

@birme

birme commented Sep 14, 2026

Copy link
Copy Markdown
Contributor Author

daily-backlog-pr (Phase 3): code-reviewer verdict is LGTM and CI is green, but this run cannot merge it. This PR is authored by the same account the automation runs as (birme), and the repo requires an approving review — GitHub blocks self-approval, and this run will not use an admin bypass of branch protection. Action needed: a different maintainer should approve and merge. Leaving the board item in "In review".

@birme

birme commented Sep 15, 2026

Copy link
Copy Markdown
Contributor Author

Automated housekeeping (Phase 3.5): linked issue #641 was closed as COMPLETED (2026-09-14), but this PR is still open. Verify whether the picomatch patch actually landed on main (it is a transitive dep) before closing this as a duplicate — flagging for a human since the pipeline account authored it and cannot self-approve/merge.

@birme

birme commented Sep 16, 2026

Copy link
Copy Markdown
Contributor Author

Flagging for a maintainer (not closing). Target issue #641 is CLOSED (COMPLETED), but picomatch is still 2.3.1 on main (the vulnerable version; the issue asked for >=2.3.2 / >=4.0.4) and there is no overrides entry in package.json. The ReDoS fix never landed, so this PR is not a stale duplicate. It is also currently CONFLICTING and needs a rebase. A maintainer should reopen #641, rebase this branch, and land the picomatch bump. Leaving open.

@birme

birme commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Housekeeping note: the target issue #641 (picomatch ReDoS) is already closed as completed, so this PR looked like a stale duplicate — but on checking main, node_modules/picomatch is still pinned at 2.3.1, which is the exact version the advisory flags (fix requires >=2.3.2 or >=4.0.4). So the vulnerability does not appear to actually be resolved on main, and this PR is also now conflicting (DIRTY). This is an inconsistency between the closed issue and the shipped lockfile that needs a human decision: either #641 should be reopened and this PR rebased/merged, or the closure should be justified. Leaving open and flagging rather than closing as duplicate.

@birme

birme commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Flagging for human triage (daily-backlog-pr, Phase 3.5): the target issue #641 is already CLOSED, and this PR is currently CONFLICTING with main. That combination is ambiguous — I can't confirm whether the picomatch patch already landed via another change or whether #641 was closed without the fix applied. Because this is a security fix, I'm not auto-closing it. A maintainer should confirm whether the picomatch ReDoS/method-injection patch is present on main; if it is, this PR can be closed as superseded, otherwise it needs a rebase to resolve conflicts and #641 reopened.

@birme

birme commented Sep 17, 2026

Copy link
Copy Markdown
Contributor Author

Flagged by daily-backlog-pr Phase 3.5 housekeeping: this PR has merge conflicts with the base branch (mergeable=CONFLICTING). It needs a rebase / conflict resolution from the author before it can be reviewed for merge. Not auto-handling since resolving conflicts requires author intent.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Security: picomatch CRITICAL ReDoS/Method Injection — update to >=2.3.2 or >=4.0.4

1 participant