| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Add a 'cache-write' input (default: true) that controls whether the cache is saved at the end of the workflow. When set to 'false', the action will restore cached dependencies but skip saving, providing a read-only cache mode. This is useful for preventing cache poisoning attacks from untrusted PR builds while still benefiting from cached dependencies.
There was a problem hiding this comment.
Adds a new action input to support “read-only” cache usage by allowing workflows to restore caches without writing updated caches back, helping mitigate cache poisoning risks for untrusted PRs.
Changes:
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| action.yml | Adds the new cache-write input and documents its purpose. |
| src/cache-save.ts | Implements early return to skip saving cache when cache-write is disabled. |
| dist/cache-save/index.js | Compiled output reflecting the new cache-write behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Sorry, something went wrong.
| const cacheWriteEnabled = core.getInput('cache-write'); | ||
| if (cacheWriteEnabled === 'false') { |
There was a problem hiding this comment.
cache-write is treated as a raw string and only disables saving when the value is exactly 'false'. This is inconsistent with other boolean inputs in this repo (e.g., check-latest uses core.getBooleanInput) and will silently save the cache for values like False/FALSE. Prefer core.getBooleanInput('cache-write') (or normalize/lowercase) and branch on the boolean result.
| const cacheWriteEnabled = core.getInput('cache-write'); | |
| if (cacheWriteEnabled === 'false') { | |
| const cacheWriteInput = core.getInput('cache-write'); | |
| const cacheWriteEnabled = | |
| cacheWriteInput === '' | |
| ? true | |
| : core.getBooleanInput('cache-write'); | |
| if (!cacheWriteEnabled) { |
Sorry, something went wrong.
| const cacheWriteEnabled = core.getInput('cache-write'); | ||
| if (cacheWriteEnabled === 'false') { | ||
| core.info('Cache write is disabled (read-only mode). Skipping cache save.'); | ||
| return; | ||
| } | ||
|
|
||
| const cache = core.getInput('cache'); | ||
| if (cache) { | ||
| await saveCache(cache); |
There was a problem hiding this comment.
New cache-write behavior is not covered by tests. Add a unit test in __tests__/cache-save.test.ts that sets cache-write to false and asserts cache.saveCache is not called (and optionally that the skip message is logged) when cache is enabled.
Sorry, something went wrong.
|
After reviewing the current GitHub Actions cache security model, I recommend closing this PR as unnecessary for the stated cache-poisoning threat.
The suggested cache-write: ${{ github.event_name != 'pull_request' }} pattern disables useful PR-local caching without adding protection for the default branch, and it remains true for pull_request_target, where backend enforcement is what provides protection. Explicit restore-only behavior may still be useful ergonomically, but GitHub already recommends composing actions/cache/restore for that case rather than adding a security-specific setup-action input. References:
|
Sorry, something went wrong.
| Back | FazBrowse Home | New Git URL |
Right now if you use cache: pip (or pipenv/poetry) in a PR workflow, the action restores and saves the cache. There's no way to get read-only mode where you benefit from existing caches without writing back. This matters for cache poisoning — an untrusted PR could plant bad packages in the cache that later get picked up by pushes to main.
This adds a cache-write input (defaults to true, no breaking change). Set it to false to skip the post-step save.
Usage:
What changed:
Same change going into setup-node, setup-go, setup-java, setup-dotnet.