feat: backport cache read-denied + ACTIONS_CACHE_MODE handling (5.2.0)#2451
Open
philip-gai wants to merge 1 commit into
Open
feat: backport cache read-denied + ACTIONS_CACHE_MODE handling (5.2.0)#2451philip-gai wants to merge 1 commit into
philip-gai wants to merge 1 commit into
Conversation
Backport of the read-denied and ACTIONS_CACHE_MODE cache-mode gating from the ESM v6.2.0 line (#2447) to the CommonJS v5 line, released as 5.2.0. Mirrors the earlier write-denied backport (#2435, 5.1.0). - Detect the `cache read denied:` prefix on download failures (v2 twirp path and v1 `_apis/artifactcache` path) and surface it as a core.warning without failing the run. - Honor ACTIONS_CACHE_MODE: skip restore when the effective cache-mode does not permit reads (none, write-only) and skip save when it does not permit writes (none, read), logging a single non-fatal core.info line. Unset or unrecognized modes are unchanged. - Add read-denied and cache-mode tests; bump to 5.2.0 with RELEASES entry. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: e96deec1-716e-4e14-acdf-a230139420a2
Contributor
There was a problem hiding this comment.
Pull request overview
This PR backports cache read-denied handling and ACTIONS_CACHE_MODE read/write gating from the v6 line into the v5 (CommonJS) @actions/cache package to ship as 5.2.0, ensuring cache operations remain best-effort while respecting runner-provided cache-mode policy.
Changes:
- Add detection of
cache read denied:failures (v1_apis/artifactcacheand v2 twirp) and treat them as a cache miss surfaced via warning. - Add cache-mode helpers (
getCacheMode,isCacheReadable,isCacheWritable) and skip restore/save when the effective mode forbids the operation. - Add/adjust tests and bump
@actions/cacheversion + release notes for 5.2.0.
Show a summary per file
| File | Description |
|---|---|
| packages/cache/src/internal/constants.ts | Adds shared cache read denied: prefix constant for consistent matching. |
| packages/cache/src/internal/config.ts | Introduces cache-mode parsing + read/write permission helpers. |
| packages/cache/src/internal/cacheHttpClient.ts | Surfaces receiver body only for read-denied 403s to enable caller dispatch. |
| packages/cache/src/cache.ts | Adds CacheReadDeniedError, re-exports read-denied prefix, and applies cache-mode gating to restore/save. |
| packages/cache/RELEASES.md | Documents 5.2.0 behavior changes. |
| packages/cache/package.json | Bumps package version to 5.2.0. |
| packages/cache/package-lock.json | Updates lockfile version metadata to 5.2.0. |
| packages/cache/tests/saveCache.test.ts | Adds save cache-mode gating coverage and tightens warning assertion. |
| packages/cache/tests/restoreCacheV2.test.ts | Adds v2 read-denied warning behavior coverage. |
| packages/cache/tests/restoreCache.test.ts | Adds restore cache-mode gating coverage and ensures getCacheEntry failures are non-fatal. |
| packages/cache/tests/config.test.ts | Adds unit tests for cache-mode helpers. |
| packages/cache/tests/cacheHttpClient.test.ts | Adds regression tests for when to surface (or not surface) v1 receiver body messages. |
Review details
Tip
Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Files not reviewed (1)
- packages/cache/package-lock.json: Generated file
- Files reviewed: 11/12 changed files
- Comments generated: 4
- Review effort level: Low
Comment on lines
+27
to
+40
| export function getCacheMode(): string { | ||
| return (process.env['ACTIONS_CACHE_MODE'] || '').trim().toLowerCase() | ||
| } | ||
|
|
||
| // Unset or unrecognized modes are permissive so behavior matches today. | ||
| export function isCacheReadable(mode: string): boolean { | ||
| if (!KNOWN_CACHE_MODES.includes(mode)) return true | ||
| return mode === 'read' || mode === 'write' | ||
| } | ||
|
|
||
| export function isCacheWritable(mode: string): boolean { | ||
| if (!KNOWN_CACHE_MODES.includes(mode)) return true | ||
| return mode === 'write' || mode === 'write-only' | ||
| } |
Comment on lines
+38
to
+42
| // config is auto-mocked; use the real cache-mode helpers so gating reflects | ||
| // ACTIONS_CACHE_MODE and unset stays permissive. | ||
| const actualConfig = jest.requireActual('../src/internal/config') | ||
| jest | ||
| .spyOn(config, 'getCacheMode') |
Comment on lines
+36
to
+40
| // config is auto-mocked; use the real cache-mode helpers so gating reflects | ||
| // ACTIONS_CACHE_MODE and unset stays permissive. | ||
| const actualConfig = jest.requireActual('../src/internal/config') | ||
| jest | ||
| .spyOn(config, 'getCacheMode') |
| delete process.env['ACTIONS_RESULTS_URL'] | ||
| }) | ||
|
|
||
| test('restore surfaces a getCacheEntry failure as a warning and reports a cache miss', async () => { |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backports the cache read-denied handling and
ACTIONS_CACHE_MODEgating from the ESM v6.2.0 line (#2447) to the CommonJS v5 line, to ship as@actions/cache5.2.0. This mirrors the earlier write-denied backport (#2435, shipped as 5.1.0) and targets thecache-v5.2.0-releasebranch.Changes
cache read denied:prefix on cache download failures on both the v2 twirp path (GetCacheEntryDownloadURL) and the v1_apis/artifactcachepath, and surface it as acore.warningwithout failing the run (treated as a cache miss).CacheReadDeniedErrorand re-exportCACHE_READ_DENIED_PREFIXfrom the sharedCacheReadDeniedMessagePrefixconstant. Read-denied matching usesincludes(prefix)(the twirp 403 wraps the prefix mid-message); write-denied continues to usestartsWith.ACTIONS_CACHE_MODEenvironment variable: skip restore when the effective cache-mode does not permit reads (none,write-only) and skip save when it does not permit writes (none,read), logging a single non-fatalcore.infoline. Unset or unrecognized modes are unchanged.getCacheMode(),isCacheReadable(), andisCacheWritable()helpers toconfig.ts.getCacheEntry(v1 path) only for read-denied policy denials, keeping the generic status-code error otherwise.@actions/cacheto5.2.0and add the correspondingRELEASES.mdentry.Notes
.jsextensions), since the v5 line predates the ESM refactor.packages/cache.Validation
tsc: cleanjest: 150/150 passingprettier --check: cleaneslint: clean