fix(sf-core-installer): remove axios and harden dependencies against supply chain attacks#13450
fix(sf-core-installer): remove axios and harden dependencies against supply chain attacks#13450
Conversation
…upply chain risk axios and axios-proxy-builder were used for a single HTTP GET to download the binary installer. Replace with built-in fetch() and undici.ProxyAgent for proxy support. This eliminates 3 third-party dependencies (axios, axios-proxy-builder, tunnel) from the published npm package, reducing exposure to supply chain attacks like the compromised axios@1.14.1. Proxy behavior (HTTPS_PROXY, HTTP_PROXY, NO_PROXY) verified equivalent across 13 e2e test cases against a real local proxy server. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
xml2js was never imported or required anywhere in the installer code. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Pin rimraf and undici to exact versions in sf-core-installer to prevent npm from resolving to a potentially compromised newer version. Add min-release-age=3 to .npmrc to align local npm install with the existing Dependabot cooldown policy (3 days for patches). This prevents npm from resolving to package versions published less than 3 days ago. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
✅ Snyk checks have passed. No issues have been found so far.
💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse. |
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughReplaced Axios-based downloads and external proxy config with undici's ProxyAgent and internal NO_PROXY/HTTP_PROXY handling in the core installer. Binary API signatures simplified (removed fetchOptions); downloads now use fetch(...).arrayBuffer() and synchronous write to disk. Added Changes
Sequence Diagram(s)sequenceDiagram
rect rgba(200,200,255,0.5)
participant CLI as Installer (Binary)
end
rect rgba(200,255,200,0.5)
participant ProxySel as ProxySelector (env NO_PROXY / HTTP(S)_PROXY)
end
rect rgba(255,200,200,0.5)
participant Remote as Remote Server
end
rect rgba(255,255,200,0.5)
participant FS as Filesystem
end
CLI->>ProxySel: determine proxy for download URL
ProxySel-->>CLI: return ProxyAgent or direct
CLI->>Remote: fetch(url) using undici + ProxyAgent
Remote-->>CLI: HTTP response (status, body)
CLI->>CLI: if !res.ok -> throw error
CLI->>FS: writeFileSync(arrayBuffer, mode=0o755)
FS-->>CLI: file persisted
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 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 |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 06752b8923
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/sf-core-installer/binary.js (1)
158-166:⚠️ Potential issue | 🟠 MajorUnreachable cleanup:
removeBinary()never executes aftererror().The
error()function callsprocess.exit(1), so the subsequentthis.removeBinary()calls on lines 160 and 165 are unreachable. The same issue exists in the.catch()block at line 190.🐛 Proposed fix to ensure cleanup runs before exit
process.on('SIGINT', () => { - error('Could not download Serverless') this.removeBinary() + error('Could not download Serverless') }) process.on('SIGTERM', () => { - error('Could not download Serverless') this.removeBinary() + error('Could not download Serverless') })And in the catch block:
.catch((e) => { - error(`Error fetching release: ${e.message}`) this.removeBinary() + error(`Error fetching release: ${e.message}`) })🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/sf-core-installer/binary.js` around lines 158 - 166, The error handler calls error(...) which calls process.exit(1) before this.removeBinary(), making cleanup unreachable; move the cleanup to run before exiting by invoking this.removeBinary() (and await it if removeBinary returns a Promise) inside the process.on('SIGINT') and process.on('SIGTERM') handlers, then call error(...) only after cleanup completes; apply the same change in the .catch(...) block where error(...) is called so that removeBinary() runs (and is awaited if async) prior to exiting.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/sf-core-installer/package.json`:
- Around line 23-26: Package.json currently pins "rimraf":"5.0.10" which pulls
in vulnerable glob@10.4.5 (CVE-2025-64756); update the dependency or add an
override to ensure glob >= 10.5.0. Fix by either bumping rimraf to a release
that depends on glob 10.5.0+ (update the "rimraf" version in package.json) or
add an npm/yarn/pnpm override entry that pins "glob" to ">=10.5.0" (or a
specific safe version) so the transitive dependency is forced to the
non-vulnerable release; reference the "rimraf" entry in package.json and the
project’s package manager override configuration to implement the change.
---
Outside diff comments:
In `@packages/sf-core-installer/binary.js`:
- Around line 158-166: The error handler calls error(...) which calls
process.exit(1) before this.removeBinary(), making cleanup unreachable; move the
cleanup to run before exiting by invoking this.removeBinary() (and await it if
removeBinary returns a Promise) inside the process.on('SIGINT') and
process.on('SIGTERM') handlers, then call error(...) only after cleanup
completes; apply the same change in the .catch(...) block where error(...) is
called so that removeBinary() runs (and is awaited if async) prior to exiting.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 2ac95a30-7e31-42c6-bfa6-16940133f48c
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (3)
.npmrcpackages/sf-core-installer/binary.jspackages/sf-core-installer/package.json
|
@cursor review |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: cda53ece04
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Align with undici@6.24.1 which requires node >=18.17. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Summary
Reduces the
serverlessnpm package (sf-core-installer) supply chain attack surface by removing unnecessary dependencies, pinning versions, and addingmin-release-ageprotection.Changes
1. Replace axios with fetch + undici (
binary.js,package.json)fetch()andundici.ProxyAgentfor proxy support.HTTPS_PROXY,HTTP_PROXY,NO_PROXYwith hostname/suffix/port matching) inlined fromaxios-proxy-builderlogic.axios,axios-proxy-builder,tunnel) from the published dependency tree.res.okcheck added —fetch()doesn't throw on non-2xx responses unlike axios, so without this a 404 would silently write an error page as the binary.Binary.install()— previously therun()code path never applied proxy config. Proxy now works for bothpostInstallandrunentry points.2. Remove unused xml2js (
package.json)xml2jswas listed as a dependency but never imported or required anywhere in the installer.3. Pin dependency versions and add min-release-age (
package.json,.npmrc)rimrafandundicito exact versions (5.0.10,6.24.1) to prevent npm from resolving to a potentially compromised newer version.min-release-age=3to.npmrc— aligns localnpm installwith the existing Dependabot cooldown policy (3 days for patches). Prevents npm from resolving to package versions published less than 3 days ago.Summary by CodeRabbit
Bug Fixes
Chores
Configuration
Documentation
Note
Medium Risk
Changes the
sf-core-installerbinary download and proxy handling path, which could affect installs in proxied or restricted environments. Dependency removals/pinning reduce supply-chain exposure but may surface compatibility issues if the new proxy/HTTP behavior differs.Overview
Reduces the
sf-core-installerpublished dependency surface by removingaxios,axios-proxy-builder,tunnel, and unusedxml2js, and by pinning remaining deps (rimraf,undici) plus aligning the lockfile (also pins@aws-sdk/client-sso-oidc).Reworks binary download in
binary.jsto use Nodefetch()withundici’sProxyAgent, adds explicit non-2xx handling, and inlinesHTTP(S)_PROXY/NO_PROXYparsing so proxy behavior is applied during installation. Adds.npmrcmin-release-age=3to avoid resolving very recently published versions, and makes a small docs formatting tweak.Written by Cursor Bugbot for commit cda53ec. This will update automatically on new commits. Configure here.