security: harden CI/CD pipeline against supply chain attacks#20
security: harden CI/CD pipeline against supply chain attacks#20ajag408 wants to merge 6 commits into
Conversation
ajag408
requested review from
Philippoes,
jdomingos,
petar-omni and
raiseerco
as code owners
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughAdds CODEOWNERS for manifests, updates npm/pnpm configs, and hardens CI and release workflows by disabling persisted checkout credentials, using Corepack + Socket Firewall for installs, making critical audit non-fatal, and adding a PR-only dependency-review job. ChangesSupply Chain Security Hardening
Sequence DiagramsequenceDiagram
participant PR as "Pull Request"
participant CI as "CI Job"
participant SFW as "Socket Firewall"
participant Corepack
participant pnpm
participant Registry as "npm Registry"
PR->>CI: trigger test/security/publish job
CI->>Corepack: corepack prepare pnpm
CI->>SFW: setup socket firewall
CI->>SFW: sfw pnpm install --frozen-lockfile
SFW->>pnpm: allow install
pnpm->>Registry: fetch packages / publish artifacts
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify 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/ci.yml:
- Around line 42-48: The Socket Firewall steps ("Set up Socket Firewall" action
socketdev/action) and the "Install dependencies" run step must explicitly expose
the API key by adding the secret as an environment variable (e.g., add an env
mapping SOCKET_SECURITY_API_KEY: ${{ secrets.SOCKET_SECURITY_API_KEY }} on the
Socket step and the run step that invokes `sfw pnpm install`) so the action and
`sfw` CLI can read it at runtime; replicate the same change for the security job
steps that run the firewall and `sfw` install. Ensure each step that invokes the
socket action or `sfw` commands has the env entry so the secret is available
during execution (and consider using workflow-level or job-level env if multiple
steps need it).
🪄 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: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 97609dfa-cad3-4dfe-959f-c25b5110aee0
📒 Files selected for processing (2)
.github/workflows/ci.yml.github/workflows/release.yml
🚧 Files skipped from review as they are similar to previous changes (1)
- .github/workflows/release.yml
| shell: bash | ||
| run: | | ||
| echo "STORE_PATH=$(pnpm store path --silent)" >> $GITHUB_ENV | ||
| npm install -g corepack@latest |
There was a problem hiding this comment.
maybe there is still a hardening gap here: npm install -g corepack@latest performs a registry fetch/execution before socketdev/action, so the pipeline is not protected from the first external package step...in addition, corepack prepare pnpm@10.12.2 --activate can also require fetching the package manager before the firewall is active
could we keep pnpm but move its bootstrap to a pinned path behind the firewall?
There was a problem hiding this comment.
Good catch. The @latest on corepack was an unpinned fetch before the firewall — fixed in the latest commit, now pinned to corepack@0.35.0. Also pinned npm@11.15.0 in the publish job for the same reason.
Worth noting this is a fundamental bootstrapping constraint: Socket Firewall wraps pnpm install (dependency resolution), not npm install -g or corepack prepare. I don't think moving the bootstrap behind the firewall is possible, I don't think sfw doesn't intercept those commands. The monorepo has the same ordering (corepack before Socket in initial_setup/action.yml).
The remaining surface after pinning:
corepackis Node.js-maintained (high-trust, high-scrutiny)corepack preparevalidates pnpm's signature against known keys before activation- All dependency installation (
sfw pnpm install --frozen-lockfile) is fully protected
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/release.yml (1)
133-136:⚠️ Potential issue | 🔴 Critical | ⚡ Quick winCritical: npm publish step missing npm authentication token
.github/workflows/release.ymlpublish step sets onlyNPM_CONFIG_PROVENANCEand does not pass any npm auth token (noNODE_AUTH_TOKEN/NPM_TOKENsecret is referenced anywhere in workflows).registry-urlalone is insufficient—pnpm publishneeds an auth token env/secret to publish.- Fix: add the repo’s npm publish token to the publish step (typically
env.NODE_AUTH_TOKEN: ${{ secrets.<npm token secret> }}).🤖 Prompt for AI Agents
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/release.yml around lines 133 - 136, The Publish to NPM step currently runs "pnpm publish --access public --no-git-checks" and only sets NPM_CONFIG_PROVENANCE; add the npm authentication token to the step by setting the environment variable used by pnpm (e.g., NODE_AUTH_TOKEN or NPM_TOKEN) to the repository secret (secrets.<YOUR_NPM_SECRET>) so pnpm can authenticate; update the publish step's env to include NODE_AUTH_TOKEN: ${{ secrets.<npm token secret> }} alongside NPM_CONFIG_PROVENANCE to enable successful publishing.
🧹 Nitpick comments (2)
.github/workflows/ci.yml (1)
36-48: ⚖️ Poor tradeoffPre-firewall fetch gap remains unaddressed.
As noted in a previous review,
npm install -g corepack@0.35.0andcorepack prepare pnpm@10.12.2 --activateexecute registry fetches before Socket Firewall is active. While pinning versions mitigates risk, it doesn't eliminate the window.Consider either:
- Move Socket Firewall setup earlier (after checkout, before any npm/corepack commands)
- Use a pre-cached corepack/pnpm in the runner image
- Accept the risk given pinned versions and document this gap in the PR
🤖 Prompt for AI Agents
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 36 - 48, The workflow currently runs the "Install pnpm" step (commands npm install -g corepack@0.35.0 and corepack prepare pnpm@10.12.2 --activate) before the "Set up Socket Firewall" action, leaving a pre-firewall registry fetch window; to fix, move the Socket Firewall step so it runs immediately after checkout and before any npm/corepack commands (i.e., place the uses: socketdev/action@... step earlier), or alternatively document/accept the residual risk in the PR and add a comment explaining why pinned versions are considered sufficient; ensure you update or keep the step names "Install pnpm" and "Set up Socket Firewall" so reviewers can verify the change..github/workflows/release.yml (1)
114-115: ⚖️ Poor tradeoffAdditional pre-firewall registry fetch.
npm install -g npm@11.15.0executes before Socket Firewall is active (line 117), adding another unprotected registry fetch to the publish job beyond the corepack installation.🤖 Prompt for AI Agents
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/release.yml around lines 114 - 115, The "Update npm" step (named "Update npm" which runs npm install -g npm@11.15.0) performs an unprotected registry fetch before the Socket Firewall step is activated; move or reorder this step so it runs after the Socket Firewall activation step in the publish job (or remove it if corepack already supplies the needed npm), ensuring the "Update npm" step executes only once the firewall is active to avoid extra unprotected registry access.
🤖 Prompt for all review comments with AI agents
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/release.yml:
- Around line 133-136: The Publish to NPM step currently runs "pnpm publish
--access public --no-git-checks" and only sets NPM_CONFIG_PROVENANCE; add the
npm authentication token to the step by setting the environment variable used by
pnpm (e.g., NODE_AUTH_TOKEN or NPM_TOKEN) to the repository secret
(secrets.<YOUR_NPM_SECRET>) so pnpm can authenticate; update the publish step's
env to include NODE_AUTH_TOKEN: ${{ secrets.<npm token secret> }} alongside
NPM_CONFIG_PROVENANCE to enable successful publishing.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 36-48: The workflow currently runs the "Install pnpm" step
(commands npm install -g corepack@0.35.0 and corepack prepare pnpm@10.12.2
--activate) before the "Set up Socket Firewall" action, leaving a pre-firewall
registry fetch window; to fix, move the Socket Firewall step so it runs
immediately after checkout and before any npm/corepack commands (i.e., place the
uses: socketdev/action@... step earlier), or alternatively document/accept the
residual risk in the PR and add a comment explaining why pinned versions are
considered sufficient; ensure you update or keep the step names "Install pnpm"
and "Set up Socket Firewall" so reviewers can verify the change.
In @.github/workflows/release.yml:
- Around line 114-115: The "Update npm" step (named "Update npm" which runs npm
install -g npm@11.15.0) performs an unprotected registry fetch before the Socket
Firewall step is activated; move or reorder this step so it runs after the
Socket Firewall activation step in the publish job (or remove it if corepack
already supplies the needed npm), ensuring the "Update npm" step executes only
once the firewall is active to avoid extra unprotected registry access.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: ace7a99b-3c93-42e5-97ce-2a1fdc206ad4
📒 Files selected for processing (2)
.github/workflows/ci.yml.github/workflows/release.yml
3 participants
Motivation
The TanStack/Mini Shai-Hulud supply chain attack (May 2026) compromised 169 npm packages by poisoning GitHub Actions caches via PR CI runs, then publishing malicious packages through trusted release workflows.
Shield's
build-binariesjob shared a pnpm store cache between PR CI runs and release builds — the same vector TanStack was exploited through. Since Trust will consume the SEA binary directly, the entire build pipeline must be hardened before the next release tag.The monorepo already has most of these layers (Socket Firewall, harden-runner, persist-credentials). This PR brings Shield to parity and adds install-time controls the monorepo should also adopt.
Changes
Install-time hardening
enable-pre-post-scripts=true.npmrcpreinstall,preparehooks). Removing it blocks all lifecycle scripts by default.onlyBuiltDependencies: ["esbuild"]package.jsonesbuildneeds it (downloads platform binary). Everything else blocked.minimum-release-age=4320.npmrcRelease-path hardening
actions/cachefrombuild-binariesrelease.ymlpublishjob was already cache-free. ~30s slower, eliminates shared cache as attack vector.NPM_CONFIG_PROVENANCE=trueon publishrelease.ymlattest-build-provenancealready done for binaries. Consumers can verify the package was built by our GitHub Actions workflow.Detection
step-security/harden-runner(audit mode)ci.yml+release.ymlv2.16.1).persist-credentials: falseon all checkoutsci.yml+release.ymlGITHUB_TOKENfrom being persisted to.git/config. Matches monorepo.socketdev/action) on all installsci.yml+release.ymlSOCKET_SECURITY_API_KEYrepo secret (see below).actions/dependency-review-actionon PRsci.ymlpull_requestonly.Governance
.github/CODEOWNERS.npmrc,package.json,pnpm-lock.yamlto require review for supply-chain-critical files.What's NOT changed
Admin actions required
SOCKET_SECURITY_API_KEYas a repo secret (Settings → Secrets → Actions). Get the key from the monorepo's secrets or Socket dashboard.mainand tag protection onv*(see Slack thread / PR comments for details).Test plan
sfwvisible in logs)esbuildpostinstall still runs (build succeeds despiteonlyBuiltDependencies)build-binariesruns clean without cacheFollow-up (separate PRs / settings)
main(require PR review + CI, no force-push)v*release.ymltoegress-policy: blockSummary by CodeRabbit