feat(ci): add and test supply-chain hardening in release workflows#452
Conversation
|
|
Warning Rate limit exceeded
You’ve run out of usage credits. Purchase more in the billing tab. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (13)
📝 WalkthroughWalkthroughThis PR implements GitHub Actions cache poisoning prevention by adding a lint script that enforces cache disabling in release workflows, creating comprehensive test fixtures to validate the enforcement, updating the release workflow with explicit cache-disabling configuration, and adding a new supply-chain verification workflow that runs the enforcement checks. ChangesSupply-Chain Cache Poisoning Prevention
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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 |
auxesis
force-pushed
the
ci/ensure-no-caching-on-release-workflows
branch
3 times, most recently
from
d0d8d08 to
f2b3d7b
Compare
auxesis
changed the title
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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/tests-supply-chain.yml:
- Around line 50-51: The workflow currently checks out PR code using the
actions/checkout step (uses: actions/checkout@v6) before executing the mutable
gate pnpm run lint:workflow-cache, which allows a PR to modify the policy;
change the gate to run from an immutable source instead—either run the policy
check in a pull_request_target trigger or invoke a pinned reusable
workflow/action that cannot be altered by the PR (instead of using the repo
checkout + pnpm run lint:workflow-cache in the PR context); update the steps
referencing actions/checkout@v6 and the pnpm run lint:workflow-cache invocation
(also at the repeated block around lines 79-80) to call the immutable policy
executor so PR changes cannot weaken the check.
In `@SECURITY.md`:
- Around line 149-151: Update the confusing guidance in SECURITY.md so the
policy is explicit: state that `pnpm/action-setup` must be invoked with `cache:
false` and `actions/setup-node` must use `package-manager-cache: false`, and
reword the later sentence to forbid any `actions/cache` steps or any
cache-enabling values (e.g., `cache: true` or omitting the
`cache:`/`package-manager-cache:` inputs), while allowing the explicit `false`
flags required by those actions (`cache: false` and `package-manager-cache:
false`).
🪄 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: 9b202d52-da73-4f92-8950-8935f9d6f4ed
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (13)
.github/workflows/release.yml.github/workflows/tests-supply-chain.ymlSECURITY.mdpackage.jsonscripts/__tests__/fixtures/lint-no-workflow-caching/actions-cache-restore.ymlscripts/__tests__/fixtures/lint-no-workflow-caching/actions-cache-save.ymlscripts/__tests__/fixtures/lint-no-workflow-caching/actions-cache.ymlscripts/__tests__/fixtures/lint-no-workflow-caching/clean.ymlscripts/__tests__/fixtures/lint-no-workflow-caching/missing-explicit-false.ymlscripts/__tests__/fixtures/lint-no-workflow-caching/no-actions-cache.ymlscripts/__tests__/fixtures/lint-no-workflow-caching/setup-node-cache.ymlscripts/__tests__/lint-no-workflow-caching.test.mjsscripts/lint-no-workflow-caching.mjs
| - name: Checkout Repo | ||
| uses: actions/checkout@v6 |
There was a problem hiding this comment.
Policy enforcement is mutable by the PR under test.
On pull_request, this workflow checks out PR code and then runs pnpm run lint:workflow-cache, so the same PR can weaken the lint script/tests and still pass while reintroducing cache usage. Move this gate to an immutable source (for example: base-branch pull_request_target policy execution, or a pinned reusable workflow/action that is not editable in the same PR).
Also applies to: 79-80
🤖 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/tests-supply-chain.yml around lines 50 - 51, The workflow
currently checks out PR code using the actions/checkout step (uses:
actions/checkout@v6) before executing the mutable gate pnpm run
lint:workflow-cache, which allows a PR to modify the policy; change the gate to
run from an immutable source instead—either run the policy check in a
pull_request_target trigger or invoke a pinned reusable workflow/action that
cannot be altered by the PR (instead of using the repo checkout + pnpm run
lint:workflow-cache in the PR context); update the steps referencing
actions/checkout@v6 and the pnpm run lint:workflow-cache invocation (also at the
repeated block around lines 79-80) to call the immutable policy executor so PR
changes cannot weaken the check.
There was a problem hiding this comment.
This is a valid concern, but will be addressed in a follow up PR, by splitting this check out into a re-usable GitHub Action.
There was a problem hiding this comment.
@auxesis, understood — splitting it into a reusable GitHub Action is a solid approach since the action can be pinned to a ref that the PR cannot modify. Would you like me to open a GitHub issue to track that follow-up work?
🧠 Learnings used
Learnt from: auxesis
Repo: cipherstash/stack PR: 431
File: .github/workflows/tests-bench.yml:0-0
Timestamp: 2026-05-06T11:35:47.341Z
Learning: Do not use continue-on-error: true in GitHub Actions workflows to mask test failures. Masking failures hides root causes and makes diagnosis harder. Keep failing steps blocking so genuine issues surface. If a test is known to fail or flaky, isolate it in a separate workflow or job (e.g., a dedicated flaky-tests job) or mark it as flaky in the test suite with clear reporting, rather than masking in the main workflow. Apply this rule to all YAML workflow files under .github/workflows, regardless of PR descriptions.
f2b3d7b to
8c41e7c
Compare
GitHub Actions cache poisoning is a known attack[^1] against credential-bearing workflows: - a lower-privileged run plants a malicious entry under a deterministic cache key - a privileged workflow restores a cache from that cache key and executes it - secrets are exfiltrated The `release.yml` workflow publishes packages to npm using OIDC trusted publishing and an `NPM_TOKEN`. Before this change, `release.yml` was vulnerable to this class of attack. Mitigate this by explicitly disabling all build caching in `release.yml`: - `pnpm/action-setup` sets `cache: false` - `actions/setup-node` sets `package-manager-cache: false` - `actions/setup-node` does not set `cache:` - no `actions/cache` step is used Also add a check to the repo (`tests-supply-chain.yml`) to test that high-risk workflows (`release.yml`) are not using caching, and it is not added back in. [^1]: https://adnanthekhan.com/2024/05/06/the-monsters-in-your-build-cache-github-actions-cache-poisoning/
auxesis
force-pushed
the
ci/ensure-no-caching-on-release-workflows
branch
from
8c41e7c to
5da89fe
Compare
| const TARGET_WORKFLOWS = [ | ||
| '.github/workflows/release.yml', | ||
| '.github/workflows/tests-supply-chain.yml', | ||
| ] |
There was a problem hiding this comment.
Could this be a problem if there is ever a slightly different name used for a release workflow? Possibly not an immediate concern. You may already have a plan for that anyway given the intent to split this out.
There was a problem hiding this comment.
Oh this is just the test of the lint, not the lint itself?
There was a problem hiding this comment.
Correct, this is the test of the lint.
coderdan
left a comment
coderdan
left a comment
There was a problem hiding this comment.
Nice. Took me a moment to fully understand but its a good solution. Thank you!
2 participants
GitHub Actions cache poisoning is a known attack1 against credential-bearing workflows. The mechanism is:
The
release.ymlworkflow publishes packages to npm using OIDC trusted publishing and anNPM_TOKEN. Before this change,release.ymlwas vulnerable to this class of attack.Mitigate this by explicitly disabling all build caching in
release.yml:pnpm/action-setupsetscache: falseactions/setup-nodesetspackage-manager-cache: falseactions/setup-nodedoes not setcache:actions/cachestep is usedAlso add a check to the repo (
tests-supply-chain.yml) to test that high-risk workflows (release.yml) are not using caching, and it is not added back in.Summary by CodeRabbit
Documentation
Chores
Footnotes
The Monsters in Your Build Cache - GitHub Actions Cache Poisoning ↩