fix: remove unused nexe dep, upgrade mcpb, add overrides for tar/tmp vulnerabilities#386
fix: remove unused nexe dep, upgrade mcpb, add overrides for tar/tmp vulnerabilities#386megmk11 wants to merge 1 commit intowonderwhy-er:mainfrom
Conversation
|
CodeAnt AI is reviewing your PR. Thanks for using CodeAnt! 🎉We're free for open-source projects. if you're enjoying it, help us grow by sharing. Share on X · |
📝 WalkthroughWalkthroughUpdated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested labels
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)
📝 Coding Plan
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 Tip You can make CodeRabbit's review stricter and more nitpicky using the `assertive` profile, if that's what you prefer.Change the |
Sequence DiagramThis PR changes the install process so postinstall only runs tracking checks when built files exist, preventing fresh install crashes. It also hardens dependency resolution by removing an unused package, upgrading mcpb, and forcing safe transitive versions. sequenceDiagram
participant Developer
participant NPM
participant Postinstall
participant DistScripts
participant Resolver
Developer->>NPM: Run install
NPM->>Resolver: Resolve dependencies with overrides
Resolver-->>NPM: Install updated and pinned packages
NPM->>Postinstall: Run postinstall
Postinstall->>Postinstall: Check dist tracking script exists
alt Dist script exists
Postinstall->>DistScripts: Run tracking and ripgrep verification
else Dist script missing
Postinstall-->>NPM: Skip silently and continue
end
Generated by CodeAnt AI |
|
CodeAnt AI finished reviewing your PR. |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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`:
- Line 25: The postinstall script currently swallows failures by wrapping
execSync in a try/catch with an empty catch; update the "postinstall" script so
it first checks for the existence of both dist/track-installation.js and
dist/npm-scripts/verify-ripgrep.js (using existsSync) and only runs execSync for
those files if they exist, but do not catch errors from execSync so any non-zero
exit from track-installation.js or verify-ripgrep.js surfaces as a failing
postinstall; remove the empty catch and ensure the filenames
dist/track-installation.js and dist/npm-scripts/verify-ripgrep.js are the gating
conditions referenced in the script.
- Around line 114-117: The "overrides" section in package.json won't protect
consumers because npm ignores overrides when your package is installed as a
dependency; update the release process to produce and commit an
npm-shrinkwrap.json that pins the transitive packages (tar and tmp) or change
direct dependencies that pull in the vulnerable versions (e.g., the packages
using node-pre-gyp or PDF utilities) to safe versions so the published package's
dependency tree is locked for end-users; ensure the shrinkwrap is generated from
a clean install and included in the release artifact to guarantee consumers get
the fixed transitive versions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: b7e67f31-c11c-43d0-9023-ec03a67bcf3d
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (1)
package.json
| "overrides": { | ||
| "tar": "^7.5.12", | ||
| "tmp": "^0.2.5" | ||
| }, |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
pwd && git ls-files | head -20Repository: wonderwhy-er/DesktopCommanderMCP
Length of output: 400
🏁 Script executed:
head -50 package.jsonRepository: wonderwhy-er/DesktopCommanderMCP
Length of output: 2824
🏁 Script executed:
ls -la | grep -E "(shrinkwrap|lock)"Repository: wonderwhy-er/DesktopCommanderMCP
Length of output: 147
🌐 Web query:
npm overrides scope root package only transitive dependencies
💡 Result:
npm overrides are defined in the root package.json of a project and apply only to that project's dependency tree. They do not cascade to or from installed dependencies or workspaces; overrides in sub-packages are ignored. Overrides primarily target transitive (nested) dependencies but can also affect direct dependencies under restrictions: you cannot override a direct dependency to a conflicting version (throws EOVERRIDE error). To match a direct dep's version, use "$packageName" reference or exact matching spec. The main use case is overriding transitive deps, e.g., for security fixes or version unification. Scoped overrides (for specific packages/scopes): Use nested objects to target deps of particular packages, including scoped ones like "@scope/pkg". Examples: - Global override (all instances): "overrides": { "lodash": "4.17.21" } - Scoped to a package: "overrides": { "react-scripts": { "webpack": "5.74.0" } } - Self + subdeps: "overrides": { "eslint": { ".": "8.56.0", "chalk": "5.2.0" } } - Only when child of specific pkg: "overrides": { "bar": { "foo": "1.0.0" } } Run npm install after changes. Requires npm >=8.3.0. "Scoped" here means targeting specific dependency paths, not limited to npm scopes (@org/pkg), though scoped package names work identically.
Citations:
- 1: https://docs.npmjs.com/cli/v9/configuring-npm/package-json/
- 2: Document that package.json overrides are not used when a package is installed as a dependency. npm/documentation#975
- 3: https://github.com/npm/rfcs/blob/main/accepted/0036-overrides.md
- 4: https://docs.npmjs.com/cli/v8/configuring-npm/package-json/
🏁 Script executed:
cat package.json | grep -A 30 '"dependencies"'Repository: wonderwhy-er/DesktopCommanderMCP
Length of output: 902
🏁 Script executed:
cat package.json | grep -E '(tar|tmp)' | head -10Repository: wonderwhy-er/DesktopCommanderMCP
Length of output: 367
🏁 Script executed:
cat package-lock.json | grep -A 5 -B 2 '"tar"' | head -30Repository: wonderwhy-er/DesktopCommanderMCP
Length of output: 238
🏁 Script executed:
cat package-lock.json | grep -A 5 -B 2 '"tmp"' | head -30Repository: wonderwhy-er/DesktopCommanderMCP
Length of output: 430
🏁 Script executed:
git log --oneline -10 -- package.jsonRepository: wonderwhy-er/DesktopCommanderMCP
Length of output: 168
These overrides won't protect users who install the published package.
npm only applies overrides from the root package.json; they are ignored when the package is installed as a dependency. Since tar and tmp are transitive dependencies (via node-pre-gyp and PDF utilities), the overrides protect your development environment but not end-users who install this CLI from npm.
For published CLI tools, include an npm-shrinkwrap.json in the release to lock the dependency tree for consumers. Alternatively, update the direct dependencies to specify versions that avoid the vulnerable transitive versions.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 114 - 117, The "overrides" section in package.json
won't protect consumers because npm ignores overrides when your package is
installed as a dependency; update the release process to produce and commit an
npm-shrinkwrap.json that pins the transitive packages (tar and tmp) or change
direct dependencies that pull in the vulnerable versions (e.g., the packages
using node-pre-gyp or PDF utilities) to safe versions so the published package's
dependency tree is locked for end-users; ensure the shrinkwrap is generated from
a clean install and included in the release artifact to guarantee consumers get
the fixed transitive versions.
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check for shrinkwrap
ls -la npm-shrinkwrap.json 2>/dev/null || echo "npm-shrinkwrap.json NOT found"
# Check if package-lock.json exists and which packages pull in tar/tmp
echo "=== package-lock.json tar entries ==="
node -e "
const fs = require('fs');
try {
const lock = JSON.parse(fs.readFileSync('package-lock.json', 'utf8'));
const packages = lock.packages || {};
const tarPkgs = Object.entries(packages).filter(([k,v]) => k.includes('/tar') || k === 'node_modules/tar');
const tmpPkgs = Object.entries(packages).filter(([k,v]) => k.includes('/tmp') || k === 'node_modules/tmp');
console.log('TAR packages:', tarPkgs.map(([k,v]) => k + '@' + v.version));
console.log('TMP packages:', tmpPkgs.map(([k,v]) => k + '@' + v.version));
} catch(e) { console.log('Error reading package-lock.json:', e.message); }
" 2>/dev/null || echo "Could not parse lock file"
# Check which top-level runtime deps pull in tar
echo "=== Checking which runtime deps might pull tar/tmp ==="
cat package-lock.json | python3 -c "
import json, sys
data = json.load(sys.stdin)
packages = data.get('packages', {})
# Find packages that depend on tar or tmp
for pkg_path, pkg_info in packages.items():
deps = {**pkg_info.get('dependencies', {}), **pkg_info.get('optionalDependencies', {})}
if 'tar' in deps or 'tmp' in deps:
depth = pkg_path.count('node_modules') - 1
print(f'depth={depth} {pkg_path}: tar={deps.get(\"tar\",\"\")} tmp={deps.get(\"tmp\",\"\")}')
" 2>/dev/null | head -40
# Check if tar/tmp are dev-only in the lock
echo "=== dev-only check for tar/tmp ==="
cat package-lock.json | python3 -c "
import json, sys
data = json.load(sys.stdin)
packages = data.get('packages', {})
for pkg_path, pkg_info in packages.items():
if pkg_path in ('node_modules/tar', 'node_modules/tmp') or pkg_path.endswith('/tar') or pkg_path.endswith('/tmp'):
print(f'{pkg_path}: dev={pkg_info.get(\"dev\", False)}, optional={pkg_info.get(\"optional\", False)}, version={pkg_info.get(\"version\",\"?\")}')
" 2>/dev/null
# Check "files" array in package.json for shrinkwrap
echo "=== files array ==="
node -e "const p=require('./package.json'); console.log(JSON.stringify(p.files,null,2));" 2>/dev/null
Length of output: 712
@megmk11, I've verified the finding against the current code — the concern is valid and a fix is needed. Here's what I found:
Verification results:
| Package | Version pinned | dev flag |
optional |
Pulled in by |
|---|---|---|---|---|
tar |
7.5.12 ✅ |
false |
true |
@mapbox/node-pre-gyp → sharp (direct runtime dep) |
tmp |
0.2.5 ✅ |
false |
false |
exceljs (direct runtime dep) |
The overrides correctly pin the versions locally, but:
npm-shrinkwrap.jsondoes not exist in the repo.- The
"files"array is["dist", "logo.png", "testemonials"]— even if a shrinkwrap were generated, it wouldn't be published to npm. - Both
tarandtmpare runtime transitive deps (not dev-only), so end-users whonpm installthis package will receive un-pinned transitive versions.
Recommended fix:
- Add
npm-shrinkwrap.jsongeneration to the release process so pinned transitive versions are shipped to consumers - Add
npm-shrinkwrap.jsonto the"files"array inpackage.jsonso it's included in the published package
🏁 Script executed:
#!/bin/bash
# Check the release script for existing npm pack/publish steps
fd "publish-release" --type f | xargs cat 2>/dev/null | head -100
Length of output: 2502
🏁 Script executed:
#!/bin/bash
# Get the full release script, focusing on the publish step
fd "publish-release" --type f | xargs cat 2>/dev/null | grep -n -A 5 -B 5 "npm publish\|shrinkwrap\|execSync.*publish\|PUBLISH\|step.*4\|step.*npm"
Length of output: 763
@megmk11, here are the two targeted changes needed. Both are small and surgical:
1. package.json — add npm-shrinkwrap.json to the files array so it gets included in the published package:
"files": [
"dist",
"logo.png",
- "testemonials"
+ "testemonials",
+ "npm-shrinkwrap.json"
],2. scripts/publish-release.cjs — generate the shrinkwrap just before the real npm publish (around line 803):
} else {
const publishCmd = options.alpha ? 'npm publish --tag alpha' : 'npm publish';
+ printStep('Generating npm-shrinkwrap.json to lock transitive deps for consumers...');
+ exec('npm shrinkwrap');
+ printSuccess('npm-shrinkwrap.json generated');
exec(publishCmd);
markStepComplete(state, 'npm');
printSuccess(`Published to NPM${options.alpha ? ' (alpha tag)' : ''}`);Why this is the right approach:
npm shrinkwrapconverts the existingpackage-lock.jsoninto annpm-shrinkwrap.json. Sinceoverridesalready resolvedtarto7.5.12andtmpto0.2.5in your lock file (confirmed by the script output), the generated shrinkwrap will carry those exact pinned versions.- Publishing with
npm-shrinkwrap.jsonin the tarball means npm will respect it when end-users install the package, locking the transitive tree including the patchedtarandtmpversions. - The dry-run path is intentionally left unchanged (no shrinkwrap generated for dry runs).
Note: You should also make sure
.gitignoredoes not ignorenpm-shrinkwrap.jsonand that it gets committed to the repo after each release, so the published tarball always reflects the latest locked tree.
Failed to handle agent chat message. Please try again.
User description
Summary
Fixes install issues and resolves all 26 security vulnerabilities reported by
npm audit.Changes
Fixed
postinstallcrash on fresh install: The postinstall script was trying to rundist/track-installation.jsbeforepreparehad builtdist/, causing aMODULE_NOT_FOUNDerror. Now it checks for the file's existence first and silently skips if not present.Removed unused
nexedevDependency:nexewas listed as a devDependency but not used anywhere in the codebase. Removing it eliminated 2 high-severity vulnerabilities (gotSSRF via Unix socket redirect,http-cache-semanticsReDoS).Upgraded
@anthropic-ai/mcpbfrom1.2.0→2.1.2: Reduced vulnerability surface from the mcpb dependency chain.Added
overridesfortarandtmp: Forces safe versions (tar@^7.5.12,tmp@^0.2.5) to resolve path traversal and symlink vulnerabilities in transitive deps.Result
Summary by CodeRabbit
CodeAnt-AI Description
Fix postinstall crash on fresh installs and remove vulnerable dev deps
What Changed
Impact
✅ No postinstall crash on fresh installs✅ Zero npm audit vulnerabilities from the changed dependency set✅ Safer installs by eliminating known tar/tmp path traversal risks💡 Usage Guide
Checking Your Pull Request
Every time you make a pull request, our system automatically looks through it. We check for security issues, mistakes in how you're setting up your infrastructure, and common code problems. We do this to make sure your changes are solid and won't cause any trouble later.
Talking to CodeAnt AI
Got a question or need a hand with something in your pull request? You can easily get in touch with CodeAnt AI right here. Just type the following in a comment on your pull request, and replace "Your question here" with whatever you want to ask:
This lets you have a chat with CodeAnt AI about your pull request, making it easier to understand and improve your code.
Example
Preserve Org Learnings with CodeAnt
You can record team preferences so CodeAnt AI applies them in future reviews. Reply directly to the specific CodeAnt AI suggestion (in the same thread) and replace "Your feedback here" with your input:
This helps CodeAnt AI learn and adapt to your team's coding style and standards.
Example
Retrigger review
Ask CodeAnt AI to review the PR again, by typing:
Check Your Repository Health
To analyze the health of your code repository, visit our dashboard at https://app.codeant.ai. This tool helps you identify potential issues and areas for improvement in your codebase, ensuring your repository maintains high standards of code health.