Add cache-write input for read-only cache mode#1508
Add cache-write input for read-only cache mode#1508salmanmkc wants to merge 2 commits intoactions:mainfrom
Conversation
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.
Pull request overview
Adds a new cache-write input to support “read-only” cache usage (restore-only) so untrusted runs (e.g., pull_request) can benefit from caches without writing back and risking cache poisoning.
Changes:
- Adds
cache-writeinput (defaulttrue) toaction.yml. - Skips the post-step cache save when
cache-writeisfalse. - Regenerates the
dist/cache-savebundle to include the new behavior.
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
action.yml |
Declares the new cache-write input and documents its purpose. |
src/cache-save.ts |
Adds an early return to skip saving cache when cache-write is disabled. |
dist/cache-save/index.js |
Rebuilt compiled bundle reflecting the 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.
| const cacheWriteEnabled = core.getInput('cache-write'); | ||
| if (cacheWriteEnabled === 'false') { |
There was a problem hiding this comment.
cache-write is being treated as a raw string and only disables saving when the value is exactly 'false'. This is inconsistent with how other boolean-like inputs in this repo are parsed (case-insensitive), and it also misses values like FALSE / False / whitespace. Consider normalizing the input (e.g., (core.getInput('cache-write') || 'true').trim().toUpperCase() === 'TRUE') or switching to core.getBooleanInput so the behavior is reliably boolean.
| const cacheWriteEnabled = core.getInput('cache-write'); | |
| if (cacheWriteEnabled === 'false') { | |
| const cacheWriteInput = core.getInput('cache-write'); | |
| const cacheWriteEnabled = | |
| (cacheWriteInput || 'true').trim().toUpperCase() === 'TRUE'; | |
| if (!cacheWriteEnabled) { |
| const cacheWriteEnabled = core.getInput('cache-write'); | ||
| if (cacheWriteEnabled === 'false') { | ||
| core.info('Cache write is disabled (read-only mode). Skipping cache save.'); | ||
| return; | ||
| } |
There was a problem hiding this comment.
This new core.getInput('cache-write') call changes run()'s observable behavior and will break existing unit tests that assert getInput is never called in cache-save (e.g. __tests__/cache-save.test.ts). Please update the tests accordingly and add a test that verifies setting cache-write: false skips cache.saveCache (and logs the info message).
When using
cachein workflows triggered bypull_request, there's no way to benefit from cached dependencies without also writing back to the cache at the end. This opens the door to cache poisoning, a malicious PR could push tainted packages into the cache that then get used by trusted runs on main.This adds a
cache-writeinput (defaults totrue, fully backward compatible). When set tofalse, the post-step cache save is skipped so the action still restores deps from cache but never writes.Usage:
What changed:
action.yml— newcache-writeinputsrc/cache-save.ts— early return whencache-writeisfalsedist/— rebuiltPairs with similar PRs across setup-python, setup-go, setup-java, setup-dotnet for a consistent experience.