Skip to content

fix(deps): resolve brace-expansion DoS CVE (#907) - #915

Closed
LucasSantana-Dev wants to merge 2 commits into
release/v2.12.0from
fix/brace-expansion-cve
Closed

LucasSantana-Dev wants to merge 2 commits into
release/v2.12.0from
fix/brace-expansion-cve

Conversation

@LucasSantana-Dev

@LucasSantana-Dev LucasSantana-Dev commented May 21, 2026 •

Copy link
Copy Markdown
Owner

Summary

Resolves CVE-2024-45049 (GHSA-jxxr-4gwj-5jf2) in brace-expansion and GHSA-58qx-3vcg-4xpx in ws.

Changes

  • brace-expansion: 5.0.5 → 5.0.6 (DoS protection; large numeric range fix)
  • ws: 8.20.0 → 8.20.1 (memory disclosure fix)

Verification

  • npm audit confirms zero moderate+ vulnerabilities
  • Lockfile updated via npm audit fix
  • No unrelated package bumps

Closes #907

Summary by CodeRabbit

  • Chores
    • Updated dependency version constraints to improve stability and compatibility.

Review Change Stack

@greptile-apps

greptile-apps Bot commented May 21, 2026

Copy link
Copy Markdown

No reviewable files after applying ignore patterns.

@vercel

vercel Bot commented May 21, 2026 •

Copy link
Copy Markdown
Contributor

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
lucky Ready Ready Preview, Comment May 21, 2026 5:31am

Request Review

@github-actions github-actions Bot added dependencies Pull requests that update a dependency file size/xl labels May 21, 2026
@github-actions

Copy link
Copy Markdown

PR Code Suggestions ✨

No code suggestions found for the PR.

@LucasSantana-Dev
LucasSantana-Dev force-pushed the fix/brace-expansion-cve branch from a117b61 to 21320fe Compare May 21, 2026 05:20

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

Resolves two moderate CVEs:
- brace-expansion 5.0.2-5.0.5 → DoS (CVE-2024-45049 / GHSA-jxxr-4gwj-5jf2)
- ws 8.0.0 - <8.20.1 → uninitialized memory disclosure (GHSA-58qx-3vcg-4xpx)

Uses package.json `overrides` (existing pattern, brace-expansion was already at >=5.0.5; bumped to >=5.0.6; ws pinned at 8.20.1 to escape the vulnerable range).
Regenerates lockfile from scratch to avoid `npm audit fix`'s optional-dep
shuffling bug (npm/cli#4828) that broke
@rolldown/binding-linux-x64-gnu resolution.

npm audit: 0 vulnerabilities.
@LucasSantana-Dev
LucasSantana-Dev force-pushed the fix/brace-expansion-cve branch from 21320fe to e351e5d Compare May 21, 2026 05:30

@greptile-apps greptile-apps Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Your free trial has ended. If you'd like to continue receiving code reviews, you can add a payment method here.

@coderabbitai

coderabbitai Bot commented May 21, 2026

Copy link
Copy Markdown

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8d5e9a49-2d75-477b-a441-76f56e507569

📥 Commits

Reviewing files that changed from the base of the PR and between 148adbb and e351e5d.

⛔ Files ignored due to path filters (2)
  • CHANGELOG.md is excluded by !**/CHANGELOG.md
  • package-lock.json is excluded by !**/package-lock.json, !**/package-lock.json
📒 Files selected for processing (1)
  • package.json

📝 Walkthrough

Walkthrough

The PR updates dependency override constraints in package.json to resolve a security vulnerability. The brace-expansion lower bound is increased to 5.0.6 to patch CVE-2024-45049, and a new ws override is added pinning version 8.20.1.

Changes

Security Dependency Overrides

Layer / File(s) Summary
brace-expansion and ws overrides
package.json
brace-expansion override minimum version is bumped from >=5.0.5 to >=5.0.6 to fix DoS vulnerability, and ws is explicitly set to 8.20.1 in overrides.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested labels

size/m

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately summarizes the main change: fixing a security vulnerability in brace-expansion by resolving CVE-2024-45049, which directly matches the content of the changeset.
Linked Issues check ✅ Passed The PR successfully addresses all coding requirements: bumps brace-expansion to >=5.0.6 [#907] and ws to 8.20.1, updates package.json overrides, regenerates lockfile, and achieves zero moderate+ vulnerabilities post-patch.
Out of Scope Changes check ✅ Passed All changes are narrowly scoped to the security fix objectives: dependency version bumps for CVE remediation and corresponding lockfile updates, with no unrelated modifications.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/brace-expansion-cve

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@sonarqubecloud

Copy link
Copy Markdown

@LucasSantana-Dev

Copy link
Copy Markdown
Owner Author

Parking — Lucky's release/v2.12.0 lockfile is fragile to regen.

The CVE patches work in isolation, but every approach that touches the lockfile exposes pre-existing hoisting brittleness:

Cycle 1 — npm audit fix: 5244-line lockfile shuffle dropped @rolldown/binding-linux-x64-gnu (npm/cli#4828 optional-deps bug). compressed-size fails to build frontend.

Cycle 2 — package.json overrides + clean lockfile regen: Zod hoists from 3.25.76 → 4.4.3 because frontend + shared pin ^4.4.3 and npm chose to hoist v4 this time. Backend code (src/middleware/validate.ts:4, src/schemas/autoMessages.ts:9,37) still uses Zod 3 API (ZodTypeDef, required_error) — TypeScript build fails.

Cycle 3 (not attempted, would be): manual lockfile surgery — bump only brace-expansion + ws version strings + integrity hashes by hand. Brittle; one mistake = drift.

Root cause is wider than the CVE

The lockfile happens to hoist Zod 3 at root only because of a specific historical resolution order. Any regen — npm audit fix, npm install --package-lock-only, even an unrelated dep bump — risks flipping it. The real fix is migrating validate.ts + autoMessages.ts to Zod 4 API so release/v2.12.0 actually matches its ^4.4.3 pin.

Severity calibration

Both CVEs are moderate, not critical:

  • brace-expansion 5.0.2-5.0.5: DoS via numeric range parsing (server-side, requires malicious caller-controlled brace string)
  • ws <8.20.1: uninitialized memory disclosure (low-impact info leak)

No live exploitation; can wait for the broader Zod migration.

What I'm leaving open

Reopening issue #907 with the Zod hoisting root cause added — once the backend's Zod 4 migration lands, this CVE patch is a 3-line overrides change away.

LucasSantana-Dev added a commit that referenced this pull request May 21, 2026
Lucky's `frontend` and `shared` workspaces pin `zod: ^4.4.3`, but the
backend code used Zod 3 API:
  - validate.ts:4   used z.ZodTypeDef (removed in Zod 4)
  - autoMessages.ts:9,37  used { required_error: '...' } (removed in 4)

The root lockfile happened to hoist Zod 3.25.76 (from `@infisical/sdk`'s
nested dep, hoisted by historical npm install ordering), masking the drift.
Any lockfile regen could flip the hoist and break the backend build —
which is what tripped PR #915 twice when trying to land the brace-expansion
CVE patch.

Changes:
  validate.ts    z.ZodType<T, z.ZodTypeDef, unknown> → z.ZodType<T, unknown>
                 (Zod 4 dropped the middle Def type parameter)
  autoMessages   { required_error: '...' } → { error: () => '...' }   (x2)
  backend pkg    declare 'zod': '^4.4.3' as a direct dep, so npm resolves
                 backend's Zod to 4 (nested at packages/backend/node_modules/zod
                 if root continues to hoist a transitive Zod 3, leaving the
                 rest of the lockfile untouched)

Lockfile diff: 186 lines (zod entries + `requires` updates only). The
@rolldown/* native bindings, undici, vitest, jsdom et al stay exactly as
they were on the base — no transitive bumps, no cascade.

Verified:
  ✓ npm ci --legacy-peer-deps --ignore-scripts succeeds
  ✓ backend resolves zod 4.4.3 at packages/backend/node_modules/zod
  ✓ npm run build:shared
  ✓ npm run type:check --workspace=packages/backend
  ✓ npm test --workspace=packages/backend → 66 suites / 832 tests pass

Unblocks the CVE work in issue #907.

Decision record:
docs/decisions/2026-05-21-backend-zod-3-to-4-migration.md.

This branch was successfully deployed

1 active deployment
Preview — e351e5de Deployed May 21, 2026 by vercel[bot]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

dependencies Pull requests that update a dependency file size/xl

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant