ci: block compromised npm packages with Socket Firewall Free - #2889
Conversation
Sorry, something went wrong.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Sorry, something went wrong.
Summary by CodeRabbit
WalkthroughMultiple GitHub Actions workflow files are updated to use voidzero-dev/setup-vp v1.12.0 instead of v1.10.0. Most jobs add sfw: true configuration. Dependency installation is refactored in lint, i18n, and release-tag jobs to use run-install configuration with explicit arguments. Chromatic workflow gains explicit build step parameters. Deploy-canary workflow adds SocketDev/action configuration and updates vercel installation. A linter configuration reference is adjusted. ChangesCI Workflow Modernisation
Possibly related PRs
Suggested reviewers
✏️ 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. ❤️ ShareComment @coderabbitai help to get the list of available commands and usage tips. |
Sorry, something went wrong.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
Sorry, something went wrong.
See https://docs.socket.dev/docs/socket-firewall-free. Using `sfw pnpm i` instead of `pnpm i` prevents any known malicious package versions from being installed at all, and will cause the install to fail if any are encountered. As we've seen these last few months, the npm ecosystem is unfortunately frequently victim to supply chain attacks, and this is a simple way to add a layer of protection against such. Socket has built a reputation for detecting these malicious packages within ~5 mins of them being published to the npm registry, so this is a pretty reliable solution for the ~3-36 hour period before these get unpublished by npm. Socket Firewall Free is 100% free to use, and does not require a licence, account, or attribution: https://github.com/SocketDev/sfw-free/blob/164c32dcbe3bbebcc0b091fd6a31ff93883545b2/README.md#license. Originally, this wasn't going to be possible with vite-plus `vp install`, but I chatted with the maintainers and they added built-in support for this in v1.12.0: voidzero-dev/setup-vp#72.
| - name: 🟧 Install pnpm globally | ||
| run: vp install -g pnpm |
There was a problem hiding this comment.
This was a workaround for chromatic needing access to the package manager that it detects we use (pnpm), but that's just needed so it can detect our command to build storybook, so we can just specify that command instead!
Sorry, something went wrong.
| uses: chromaui/action@8a2b82547aef5a3efc8ec3c7905f4ab09a76ed0b # v16.1.0 | ||
| with: | ||
| buildCommand: vp run build-storybook | ||
| outputDir: storybook-static |
There was a problem hiding this comment.
required when you pass buildCommand. this value is the default.
Sorry, something went wrong.
There was a problem hiding this comment.
Isn't the default a temporary directory?
In docs, I can only see that "storybook-static" is default for TurboSnap, but it says temporary dir for CLI and therefore GitHub action: https://www.chromatic.com/docs/configure/#outputdir
But I guess we have to set it to something since it is indeed required, and since our .gitignore already has this entry, I approve ✅
Sorry, something went wrong.
There was a problem hiding this comment.
Oh, heh, it's an example provided, not the default 🤦🏼.
Sorry, something went wrong.
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1).github/workflows/deploy-canary.yml (1)25-30: ⚠️ Potential issue | 🟠 Major
Enable Socket Firewall Free (sfw) for the global vercel install in deploy-canary.
In .github/workflows/deploy-canary.yml, voidzero-dev/setup-vp is configured without sfw: true, and the workflow then runs vp install -g vercel. Other workflows in this repo do set sfw: true. Since Socket Firewall Free blocks malicious packages at download time (preventing them from being written/executed, including via lifecycle/postinstall), the current global install is a avoidable supply-chain risk—especially because vp install -g vercel doesn’t pin a version.
- uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1 # v1.12.0 with: node-version: lts/* run-install: false - run: vp install -g vercel🤖 Prompt for AI Agents
- Add sfw: true to the setup-vp step in deploy-canary (to wrap/guard the vp install -g vercel step).
- Prefer pinning vercel (e.g., vp install -g vercel@<version>).
- If enabling sfw, ensure the sfw version used is patched against GHSA-6c5p-vqrh-h6fp (fixed in Socket Firewall Free >= 0.15.5).
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/deploy-canary.yml around lines 25 - 30, The deploy-canary workflow uses the voidzero-dev/setup-vp step but omits sfw, then runs an unpinned global install via the vp install -g vercel step; update the setup-vp step to include sfw: true (ensuring the action invocation specifies a Socket Firewall Free version >= 0.15.5), and change the subsequent vp install command to pin a concrete vercel version (e.g., vp install -g vercel@<version>) so the global install is guarded by SFW and not pulling an unpinned package.
.github/workflows/ci.yml (2)🤖 Prompt for all review comments with AI agents.github/workflows/release-tag.yml (1)33-39: ⚡ Quick win
Consider adding cache: true for consistency and performance.
The lint job is missing cache: true, while all other jobs that install dependencies (types, unit, test, browser, a11y, knip) retain it. Caching dependencies significantly improves CI performance by avoiding redundant downloads.
⚡ Proposed fix to add caching🤖 Prompt for AI Agents- uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1 # v1.12.0 with: node-version: lts/* + cache: true sfw: true # root only, no scripts run-install: | - args: ['--filter', '.', '--ignore-scripts']Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 33 - 39, The lint job's setup-vp action is missing caching; update the lint job's step that uses voidzero-dev/setup-vp (the action block with node-version, sfw, run-install) to include cache: true so it matches other jobs (types, unit, test, browser, a11y, knip) and improves CI performance by reusing dependency caches.
237-243: ⚡ Quick win
Consider adding cache: true for consistency and performance.
The i18n job is missing cache: true, while all other jobs that install dependencies retain it. This could slow down the CI run unnecessarily.
⚡ Proposed fix to add caching🤖 Prompt for AI Agents- uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1 # v1.12.0 with: node-version: lts/* + cache: true sfw: true # root only, no scripts run-install: | - args: ['--filter', '.', '--ignore-scripts']Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 237 - 243, Add caching to the i18n job's setup step: in the .github/workflows/ci.yml where the i18n job uses the voidzero-dev/setup-vp action (the block with "uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1" and "with: node-version: lts/*"), add the key "cache: true" alongside the existing inputs so this job matches the other dependency-installing jobs and benefits from improved CI performance.67-73: Optimise voidzero-dev/setup-vp step usage in .github/workflows/release-tag.yml
voidzero-dev/setup-vp is called 4 times total: twice in jobs.tag (steps at ~31 and ~69) and twice in jobs.publish-connector (steps at ~108 and ~115), not three times in a single job. In each job, the first call uses run-install: false, and the second call is only to perform a filtered dependency install with sfw: true. If the second invocation is purely for that install, consider running the filtered install in a shell step (e.g., vp install --filter ... --ignore-scripts, keeping any required sfw handling) so setup-vp is invoked once per job.
🤖 Prompt for AI AgentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/release-tag.yml around lines 67 - 73, The workflow currently calls the action voidzero-dev/setup-vp twice per job (see the uses of setup-vp with run-install: false and again with run-install: | plus sfw: true) — instead, keep a single setup-vp invocation per job and move the filtered install into a normal shell step; update the jobs.tag and jobs.publish-connector flows to remove the second setup-vp call and replace it with a run step that executes the filtered install command (e.g., vp install --filter . --ignore-scripts) while preserving any sfw handling required by the environment so you only call setup-vp once per job.
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Outside diff comments: In @.github/workflows/deploy-canary.yml: - Around line 25-30: The deploy-canary workflow uses the voidzero-dev/setup-vp step but omits sfw, then runs an unpinned global install via the vp install -g vercel step; update the setup-vp step to include sfw: true (ensuring the action invocation specifies a Socket Firewall Free version >= 0.15.5), and change the subsequent vp install command to pin a concrete vercel version (e.g., vp install -g vercel@<version>) so the global install is guarded by SFW and not pulling an unpinned package. --- Nitpick comments: In @.github/workflows/ci.yml: - Around line 33-39: The lint job's setup-vp action is missing caching; update the lint job's step that uses voidzero-dev/setup-vp (the action block with node-version, sfw, run-install) to include cache: true so it matches other jobs (types, unit, test, browser, a11y, knip) and improves CI performance by reusing dependency caches. - Around line 237-243: Add caching to the i18n job's setup step: in the .github/workflows/ci.yml where the i18n job uses the voidzero-dev/setup-vp action (the block with "uses: voidzero-dev/setup-vp@2dec1e33f4ab2c6d5bce1b0c4607961bb1a3f7a1" and "with: node-version: lts/*"), add the key "cache: true" alongside the existing inputs so this job matches the other dependency-installing jobs and benefits from improved CI performance. In @.github/workflows/release-tag.yml: - Around line 67-73: The workflow currently calls the action voidzero-dev/setup-vp twice per job (see the uses of setup-vp with run-install: false and again with run-install: | plus sfw: true) — instead, keep a single setup-vp invocation per job and move the filtered install into a normal shell step; update the jobs.tag and jobs.publish-connector flows to remove the second setup-vp call and replace it with a run step that executes the filtered install command (e.g., vp install --filter . --ignore-scripts) while preserving any sfw handling required by the environment so you only call setup-vp once per job.
ℹ️ Review info ⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: a3e2af4b-8894-4325-bb3d-49d183aec8bf
📥 CommitsReviewing files that changed from the base of the PR and between 1c48d0c and fe080d6.
📒 Files selected for processing (8)- .github/workflows/autofix.yml
- .github/workflows/chromatic.yml
- .github/workflows/ci.yml
- .github/workflows/deploy-canary.yml
- .github/workflows/lunaria.yml
- .github/workflows/release-pr.yml
- .github/workflows/release-tag.yml
- .github/zizmor.yml
Sorry, something went wrong.
trueberryless
left a comment
There was a problem hiding this comment.
I really like this security change!
Technically, support for sfw was added in v1.11.0, but I would also definitely use v1.12.0 here. Also fun details: I like how the voidzero action and sfw-free versions are currently in sync (both v1.12.0). Coincidence? I think it's a signal from shai hulud. 😆
Generally, the changes look good to me, I just left small thoughts 💭
Sorry, something went wrong.
| uses: chromaui/action@8a2b82547aef5a3efc8ec3c7905f4ab09a76ed0b # v16.1.0 | ||
| with: | ||
| buildCommand: vp run build-storybook | ||
| outputDir: storybook-static |
There was a problem hiding this comment.
Isn't the default a temporary directory?
In docs, I can only see that "storybook-static" is default for TurboSnap, but it says temporary dir for CLI and therefore GitHub action: https://www.chromatic.com/docs/configure/#outputdir
But I guess we have to set it to something since it is indeed required, and since our .gitignore already has this entry, I approve ✅
Sorry, something went wrong.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agentsVerify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. Inline comments: In @.github/workflows/deploy-canary.yml: - Around line 30-33: The SocketDev/action step in the deploy-canary workflow has an invalid mode parameter value of firewall-free. Change the mode parameter from firewall-free to firewall, which is the correct valid value for using Socket Firewall functionality. The pinned SHA and firewall-version parameters are already correct and do not require changes.
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: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 21a49a34-b651-410d-a202-f20bced2a0b8
📥 CommitsReviewing files that changed from the base of the PR and between fe080d6 and f49fe8b.
📒 Files selected for processing (1)- .github/workflows/deploy-canary.yml
Sorry, something went wrong.
trueberryless
left a comment
There was a problem hiding this comment.
LGTM, Shai-Hulud-proofed!
Sorry, something went wrong.
🔗 Linked issue
N/A
🧭 Context
See https://docs.socket.dev/docs/socket-firewall-free.
Using sfw pnpm i instead of pnpm i prevents any known malicious package versions from being installed at all, and will cause the install to fail if any are encountered.
As we've seen these last few months, the npm ecosystem is unfortunately frequently victim to supply chain attacks, and this is a simple way to add a layer of protection against such. Socket has built a reputation for detecting these malicious packages within ~5 mins of them being published to the npm registry, so this is a pretty reliable solution for the ~3-36 hour period before these get unpublished by npm.
Socket Firewall Free is 100% free to use, and does not require a licence, account, or attribution:
https://github.com/SocketDev/sfw-free/blob/164c32dcbe3bbebcc0b091fd6a31ff93883545b2/README.md#license.
Originally, this wasn't going to be possible with vite-plus vp install, but I chatted with the maintainers and they added built-in support for this in v1.12.0:
voidzero-dev/setup-vp#72.
📚 Description
Bump to v1.12.0 of the action and use sfw: true to opt in to Socket Firewall Free.