Add cache-write input for read-only cache mode#987
Add cache-write input for read-only cache mode#987salmanmkc wants to merge 1 commit 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
This PR adds a new cache-write input to the setup-java action, allowing users to disable cache saving while still restoring the cache. This enables a read-only cache mode that's useful for preventing cache poisoning from untrusted PR builds.
Changes:
- New
cache-writeinput inaction.ymlwith a default oftrue(backward compatible) - Early return in
saveCache()insrc/cleanup-java.tswhencache-writeisfalse - Rebuilt
dist/cleanup/index.jsto reflect the source change
Reviewed changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| action.yml | Adds new cache-write input definition with default true |
| src/cleanup-java.ts | Adds guard clause in saveCache() to skip save when cache-write is false |
| dist/cleanup/index.js | Compiled output reflecting the source change |
💡 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.
The codebase has an established getBooleanInput() utility in src/util.ts that handles case-insensitive comparison (via .toUpperCase() === 'TRUE'). It's used for all other boolean inputs: check-latest in setup-java.ts:30, overwrite-settings in auth.ts:20 and toolchains.ts:30.
Using raw string comparison === 'false' here is fragile — it won't match 'False' or 'FALSE', which users might reasonably pass. This should use getBooleanInput from src/util.ts instead, with the logic inverted (skip save when the result is false).
Additionally, the input name 'cache-write' is hardcoded as a string literal, whereas the codebase convention is to define input name constants in src/constants.ts (e.g., INPUT_CACHE, INPUT_CACHE_DEPENDENCY_PATH at line 20-21). A constant like INPUT_CACHE_WRITE should be added there and used here.
| const cacheWriteEnabled = core.getInput('cache-write'); | ||
| if (cacheWriteEnabled === 'false') { | ||
| core.info('Cache write is disabled (read-only mode). Skipping cache save.'); | ||
| return Promise.resolve(); | ||
| } |
There was a problem hiding this comment.
The existing test file has tests for cache save behavior (error handling, successful saves), but no test was added for the new cache-write: false code path. A test should verify that when cache-write input is 'false', saveCache returns early without calling cache.saveCache. This is important because this is the core behavior being added by this PR.
When
cacheis set to maven/gradle/sbt, this action restores and saves the dependency cache. For PR workflows there's no way to get restore-only — a malicious PR can poison the cache and have it picked up by trusted runs on main.This adds a
cache-writeinput (defaults totrue, backward compatible). Whenfalse, the cleanup step skips cache saving entirely.Usage:
What changed:
action.yml— newcache-writeinputsrc/cleanup-java.ts— early return insaveCache()whencache-writeisfalsedist/— rebuiltSame change going into setup-node, setup-python, setup-go, setup-dotnet.