fix: patch critical vulnerabilities (simple-git, property-expr, minimist, form-data)#25
Conversation
📝 WalkthroughWalkthroughAdds a new ADR documenting five critical vulnerabilities and mitigation steps; updates Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @aman-wednesdaysol, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's security by addressing several critical vulnerabilities found in third-party dependencies. It achieves this by updating key packages to their patched versions and explicitly enforcing these versions using Yarn resolutions. Additionally, a new architectural decision record has been added to document these vulnerabilities and their mitigation strategies, ensuring transparency and future reference for maintaining a secure codebase. Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request aims to patch several critical vulnerabilities by updating dependencies and adding Yarn resolutions, which is a great step towards improving security. The inclusion of an ADR to document these changes is also excellent practice. However, I've found a critical issue with the proposed fix for the form-data package; the resolution version is insufficient to patch the described vulnerability. Additionally, the ADR contains a few minor issues like a future date in the filename and incorrect advisory links that should be corrected for accuracy.
package.json
Outdated
| "simple-git": ">=3.16.0", | ||
| "property-expr": ">=2.0.3", | ||
| "minimist": ">=1.2.6", | ||
| "form-data": ">=2.5.4" |
There was a problem hiding this comment.
The resolution for form-data to ">=2.5.4" is insufficient to fix the "Unsafe Random Function" vulnerability (CVE-2024-29041), which is only patched in version 4.0.0 and later. This leaves the application vulnerable. To correctly mitigate this critical issue, please update the resolution to ">=4.0.0".
| "form-data": ">=2.5.4" | |
| "form-data": ">=4.0.0" |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (1)
package.json (1)
124-129: Consider pinning resolution versions more tightly.Using
>=in resolutions (e.g.,"simple-git": ">=3.16.0") is unbounded — a futureyarn installcould pull in any newer major version, potentially introducing breaking changes in transitive dependencies. Consider using caret ranges (e.g.,"simple-git": "^3.16.0") to stay within the same major version while still getting patches.Proposed fix
"resolutions": { - "simple-git": ">=3.16.0", - "property-expr": ">=2.0.3", - "minimist": ">=1.2.6", - "form-data": ">=2.5.4" + "simple-git": "^3.16.0", + "property-expr": "^2.0.3", + "minimist": "^1.2.6", + "form-data": "^2.5.4" }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@package.json` around lines 124 - 129, The resolutions section in package.json currently uses open-ended version specifiers like "simple-git": ">=3.16.0" which can allow breaking major upgrades; change each resolution entry (e.g., the "resolutions" map keys simple-git, property-expr, minimist, form-data) to use a tighter semver range such as caret ranges (for example "^3.16.0" for simple-git) or exact patch pins if you want stricter control, update each value accordingly, and run an install to verify the lockfile updates and tests pass.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/adr/2026-02-17-fix-critical-vulnerabilities.md`:
- Around line 71-79: The JSON code block containing the "resolutions" object is
missing its closing triple-backtick fence; add a closing ``` on the line
immediately after the JSON block so the code fence is properly terminated and
the rest of the document renders correctly (locate the block showing the
"resolutions" keys: "simple-git", "property-expr", "minimist", "form-data" and
append the closing fence).
- Line 13: Update the deprecated npm advisory links (e.g., the "NPM Advisory
1103707" link using https://www.npmjs.com/advisories/<id>) to GitHub Security
Advisory (GHSA) URLs or direct CVE references; locate each occurrence of the
pattern "NPM Advisory <id>" (seen at lines with "NPM Advisory 1103707" and the
similar entries at the other noted positions) and replace the
npmjs.com/advisories/<id> link with the corresponding GHSA URL or a CVE link so
the document points to stable, current advisory pages.
In `@package.json`:
- Line 52: Move the "coveralls" package out of production dependencies and into
devDependencies in package.json: open package.json, remove the "coveralls":
"^3.1.1" entry from the top-level "dependencies" object and add the same entry
under "devDependencies" (ensuring JSON remains valid and any trailing commas are
corrected), then run npm install/npm ci to update lockfile; reference the
"coveralls" entry and the "coveralls" npm script to confirm it remains available
for CI.
---
Nitpick comments:
In `@package.json`:
- Around line 124-129: The resolutions section in package.json currently uses
open-ended version specifiers like "simple-git": ">=3.16.0" which can allow
breaking major upgrades; change each resolution entry (e.g., the "resolutions"
map keys simple-git, property-expr, minimist, form-data) to use a tighter semver
range such as caret ranges (for example "^3.16.0" for simple-git) or exact patch
pins if you want stricter control, update each value accordingly, and run an
install to verify the lockfile updates and tests pass.
|
Title: Critical Security Vulnerabilities Fixed Comment:
|
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@package.json`:
- Around line 124-134: In the package.json "resolutions" block there is
inconsistent indentation and a stray trailing comma after the "form-data" entry;
normalize the block to use consistent 2-space indentation for all keys (e.g.,
"simple-git", "property-expr", "minimist", "form-data", "next", "json",
"braces", "cross-spawn", "qs") and remove the trailing comma after the
"form-data" value so the JSON is valid and styling is consistent.
- Line 130: Remove the orphaned resolution entry "json": ">=10.0.0" from the
package.json resolutions block (or replace it with the correct package name if
you intended to target a different package such as "json5"); specifically delete
the line containing "json": ">=10.0.0" (or update the key to the intended
package) so the resolutions object no longer contains an unused/incorrect entry.
- Line 129: The resolutions block is forcing Next.js >=15.0.8 which conflicts
with the pinned dependency next: "14.2.35" and would cause a breaking major
upgrade; change the resolutions entry for "next" to match the pinned version (or
remove the resolution so the declared dependency "next" at 14.2.35 is used) and
ensure you reference the resolutions key for "next" (resolutions.next); also
update the "json" resolution from ">=10.0.0" to ">=11.0.0" (resolutions.json) to
actually mitigate the vulnerability; finally normalize indentation in the
resolutions block to 4 spaces for lines currently using 2-space indentation so
the file style is consistent.
---
Duplicate comments:
In `@package.json`:
- Line 72: The package.json currently has "coveralls" moved to devDependencies
but confirm and ensure "coveralls" is removed from the top-level dependencies
and present under "devDependencies" (symbol: "coveralls" in package.json),
update package-lock.json / yarn.lock by running the appropriate install command
(npm install or yarn) to reflect the change, and run the test/CI step that uses
coveralls to verify the tool is picked up from devDependencies.
| "resolutions": { | ||
| "simple-git": ">=3.16.0", | ||
| "property-expr": ">=2.0.3", | ||
| "minimist": ">=1.2.6", | ||
| "form-data": ">=2.5.4", | ||
| "next": ">=15.0.8", | ||
| "json": ">=10.0.0", | ||
| "braces": ">=3.0.3", | ||
| "cross-spawn": ">=6.0.6", | ||
| "qs": ">=6.14.1" | ||
| } |
There was a problem hiding this comment.
Inconsistent indentation in the resolutions block.
Lines 125–128 use 4-space indentation, while lines 129–133 drop to 2-space indentation. This is cosmetically inconsistent and suggests a merge or copy-paste error. It also correlates with the entries that look like scope creep (the non-critical-vuln resolutions).
Proposed fix
"resolutions": {
"simple-git": ">=3.16.0",
"property-expr": ">=2.0.3",
"minimist": ">=1.2.6",
"form-data": ">=2.5.4",
- "next": ">=15.0.8",
- "json": ">=10.0.0",
- "braces": ">=3.0.3",
- "cross-spawn": ">=6.0.6",
- "qs": ">=6.14.1"
+ "next": ">=15.0.8",
+ "json": ">=10.0.0",
+ "braces": ">=3.0.3",
+ "cross-spawn": ">=6.0.6",
+ "qs": ">=6.14.1"
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 124 - 134, In the package.json "resolutions" block
there is inconsistent indentation and a stray trailing comma after the
"form-data" entry; normalize the block to use consistent 2-space indentation for
all keys (e.g., "simple-git", "property-expr", "minimist", "form-data", "next",
"json", "braces", "cross-spawn", "qs") and remove the trailing comma after the
"form-data" value so the JSON is valid and styling is consistent.
| "property-expr": ">=2.0.3", | ||
| "minimist": ">=1.2.6", | ||
| "form-data": ">=2.5.4", | ||
| "next": ">=15.0.8", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n package.json | sed -n '50,60p; 120,135p'Repository: wednesday-solutions/nextjs-template
Length of output: 986
🌐 Web query:
Next.js 15 breaking changes from Next.js 14
💡 Result:
Key breaking changes in Next.js 15 vs 14 (per the official upgrade guide / release notes):
- React 19 is now required (minimum
react+react-domis 19). Also note React 19 form-hook API shifts (e.g.,useFormState→useActionState). [1] - Async Request APIs (Dynamic APIs) are now async: you must
awaitcookies(),headers(),draftMode(), and alsoparams/searchParamsin the listed App Router entry points (e.g.,page.js,layout.js, route handlers, metadata image routes). [1][2] runtime: 'experimental-edge'now errors; useruntime: 'edge'instead. [1]- Caching behavior changed (opt-in caching):
fetch()is no longer cached by default (explicitly setcache: 'force-cache'orexport const fetchCache = 'default-cache'). [1]GETRoute Handlers are no longer cached by default (opt into caching via route config likeexport const dynamic = 'force-static'). [1]- Client-side navigation: page segments are no longer reused from the client-side router cache by default (opt in via
staleTimes). [1]
@next/fontpackage removed; migrate imports to built-innext/font. [1]- Config renames (experimental → stable):
experimental.bundlePagesExternals→bundlePagesRouterDependencies. [1]experimental.serverComponentsExternalPackages→serverExternalPackages. [1]
- Vercel Speed Insights auto-instrumentation removed (you must integrate separately if you use it). [1]
NextRequest.geoandNextRequest.ipremoved (use hosting-provider equivalents; Vercel suggests@vercel/functions). [1]
Sources:
[1] Next.js “Upgrading: Version 15” guide (last updated Feb 11, 2026)
[2] Next.js “Next.js 15” release post (Async Request APIs breaking change)
🏁 Script executed:
#!/bin/bash
# Check if these specific versions exist in npm registry
# Query npm for next.js 14.2.35 and other versions
npm view next@14.2.35 version 2>/dev/null || echo "next@14.2.35 NOT FOUND"
npm view next@15.0.8 version 2>/dev/null || echo "next@15.0.8 NOT FOUND"
npm view json@10.0.0 version 2>/dev/null || echo "json@10.0.0 NOT FOUND"
npm view lint-staged@16.2.7 version 2>/dev/null || echo "lint-staged@16.2.7 NOT FOUND"
npm view eslint-config-next@16.1.6 version 2>/dev/null || echo "eslint-config-next@16.1.6 NOT FOUND"Repository: wednesday-solutions/nextjs-template
Length of output: 114
🌐 Web query:
npm package "json" 10.0.0 what is this package
💡 Result:
json@10.0.0 is a command-line tool (and optional Node module) for working with JSON—pretty-printing, extracting values (e.g. json foo.bar), filtering/editing JSON via expressions (-e, -c), streaming mode, validation, and in-place file editing. It’s implemented as a single-file Node.js script with no external dependencies (beyond Node itself). [1]
Security note: versions < 11.0.0 are reported vulnerable to arbitrary code injection via the -d argument; upgrading to 11.0.0+ is recommended. [2]
Sources: [1] npm package page for json@10.0.0 https://www.npmjs.com/package/json/v/10.0.0, [2] Snyk advisory page for json@10.0.0 https://security.snyk.io/package/npm/json/10.0.0
next resolution >=15.0.8 conflicts with the next dependency pinned at 14.2.35 (line 57).
Yarn resolutions override all dependency versions globally. This forces a major version upgrade to Next.js 15, which introduces significant breaking changes: React 19 becomes required, async Request APIs (cookies(), headers(), params, searchParams) must now be awaited throughout the codebase, fetch() and GET Route Handlers are no longer cached by default, and several APIs (@next/font, NextRequest.geo, NextRequest.ip) are removed. This contradicts the PR's goal of patching critical vulnerabilities without breaking changes.
Additionally, the json resolution at line 130 pins to >=10.0.0, but versions below 11.0.0 are vulnerable to arbitrary code injection. Update to ">=11.0.0" to actually fix the vulnerability.
Finally, lines 129–133 use 2-space indentation while lines 125–128 use 4-space indentation. Standardize to 4 spaces for consistency.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 129, The resolutions block is forcing Next.js >=15.0.8
which conflicts with the pinned dependency next: "14.2.35" and would cause a
breaking major upgrade; change the resolutions entry for "next" to match the
pinned version (or remove the resolution so the declared dependency "next" at
14.2.35 is used) and ensure you reference the resolutions key for "next"
(resolutions.next); also update the "json" resolution from ">=10.0.0" to
">=11.0.0" (resolutions.json) to actually mitigate the vulnerability; finally
normalize indentation in the resolutions block to 4 spaces for lines currently
using 2-space indentation so the file style is consistent.
| "minimist": ">=1.2.6", | ||
| "form-data": ">=2.5.4", | ||
| "next": ">=15.0.8", | ||
| "json": ">=10.0.0", |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cd . && find . -name 'yarn.lock' -o -name 'package.json' | head -20Repository: wednesday-solutions/nextjs-template
Length of output: 105
🏁 Script executed:
cat -n package.json | sed -n '120,140p'Repository: wednesday-solutions/nextjs-template
Length of output: 598
🏁 Script executed:
if [ -f yarn.lock ]; then grep -A 2 -B 2 '"json@' yarn.lock | head -30; fiRepository: wednesday-solutions/nextjs-template
Length of output: 61
🏁 Script executed:
grep -i '"json' yarn.lock | head -20Repository: wednesday-solutions/nextjs-template
Length of output: 61
🏁 Script executed:
cat -n package.json | sed -n '1,123p' | grep -i jsonRepository: wednesday-solutions/nextjs-template
Length of output: 101
🏁 Script executed:
# Check package.json for direct "json" dependency
cat package.json | jq '.dependencies.json, .devDependencies.json, .peerDependencies.json' 2>/dev/null || echo "Check manually"Repository: wednesday-solutions/nextjs-template
Length of output: 93
🏁 Script executed:
# Check if there are any other json-like packages in yarn.lock
grep -o '"[^"]*json[^"]*@' yarn.lock | sort -u | head -20Repository: wednesday-solutions/nextjs-template
Length of output: 209
🏁 Script executed:
# Check the complete resolutions section and look for context
cat -n package.json | sed -n '124,135p'Repository: wednesday-solutions/nextjs-template
Length of output: 418
Remove the orphaned "json": ">=10.0.0" resolution — it has no effect.
No dependency in yarn.lock actually pulls in a package named json, and no direct dependency on json exists in package.json. This resolution is unused and won't affect the lock file. If this was meant to address a vulnerability in a transitive dependency, that dependency may no longer exist. If it was intended for a different package (e.g., json5), the name is incorrect. Either remove it or clarify its purpose.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` at line 130, Remove the orphaned resolution entry "json":
">=10.0.0" from the package.json resolutions block (or replace it with the
correct package name if you intended to target a different package such as
"json5"); specifically delete the line containing "json": ">=10.0.0" (or update
the key to the intended package) so the resolutions object no longer contains an
unused/incorrect entry.
1 participant
Summary by CodeRabbit
Chores
Documentation