Skip to content
Draft
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions packages/cache/RELEASES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,9 @@
# @actions/cache Releases

## 6.2.0

- Handle cache read error due to read-only token: detect the `cache read denied:` prefix on cache download failures (both the v2 twirp path and the v1 `_apis/artifactcache` path) and surface it as a `core.warning` (without failing the run).

## 6.1.0

- Handle cache write error due to read-only token: detect the `cache write denied:` prefix on cache reservation failures and surface it as a `core.warning` (without failing the run).
Expand Down
35 changes: 34 additions & 1 deletion packages/cache/__tests__/cacheHttpClient.test.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import {downloadCache} from '../src/internal/cacheHttpClient'
import {downloadCache, getCacheEntry} from '../src/internal/cacheHttpClient'
import {getCacheVersion} from '../src/internal/cacheUtils'
import {CompressionMethod} from '../src/internal/constants'
import * as downloadUtils from '../src/internal/downloadUtils'
import * as requestUtils from '../src/internal/requestUtils'
import {DownloadOptions, getDownloadOptions} from '../src/options'
import {HttpClientError} from '@actions/http-client'

jest.mock('../src/internal/downloadUtils')

Expand Down Expand Up @@ -57,6 +59,37 @@ test('getCacheVersion with enableCrossOsArchive as false returns version on wind
}
})

test('getCacheEntry throws a generic status-code error for non-read-denied failures', async () => {
// Regression: a non read-denied failure must NOT leak the receiver's body
// message; it should surface the generic status-code error.
jest.spyOn(requestUtils, 'retryTypedResponse').mockResolvedValue({
statusCode: 403,
result: null,
headers: {},
error: new HttpClientError('some other server detail', 403)
})

await expect(getCacheEntry(['key'], ['node_modules'])).rejects.toThrow(
'Cache service responded with 403'
)
})

test('getCacheEntry surfaces the body message for a cache read denial', async () => {
jest.spyOn(requestUtils, 'retryTypedResponse').mockResolvedValue({
statusCode: 403,
result: null,
headers: {},
error: new HttpClientError(
'cache read denied: token has no readable scopes',
403
)
})

await expect(getCacheEntry(['key'], ['node_modules'])).rejects.toThrow(
'cache read denied: token has no readable scopes'
)
})

test('downloadCache uses http-client for non-Azure URLs', async () => {
const downloadCacheHttpClientMock = jest.spyOn(
downloadUtils,
Expand Down
47 changes: 47 additions & 0 deletions packages/cache/__tests__/restoreCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,53 @@ test('restore with server error should fail', async () => {
delete process.env['ACTIONS_RESULTS_URL']
})

test('restore denied by read-only token logs warning and reports cache miss', async () => {
// The GHES v1 artifact cache service returns HTTP 403 with a
// `cache read denied:` body when the token has no readable scopes.
// getCacheEntry surfaces that message; expect a single warning (not error)
// and a cache miss.
const paths = ['node_modules']
const key = 'node-test'
const logErrorMock = jest.spyOn(core, 'error')
const logWarningMock = jest.spyOn(core, 'warning')
const deniedMessage = 'cache read denied: token has no readable scopes'

jest.spyOn(cacheHttpClient, 'getCacheEntry').mockImplementation(async () => {
throw new Error(deniedMessage)
})

const cacheKey = await restoreCache(paths, key)
expect(cacheKey).toBe(undefined)
expect(logErrorMock).not.toHaveBeenCalled()
expect(logWarningMock).toHaveBeenCalledWith(
`Failed to restore: ${deniedMessage}`
)
expect(logWarningMock).toHaveBeenCalledTimes(1)
})

test('restore surfaces a non-read-denied getCacheEntry error as a normal warning', async () => {
// Guards the inner catch in restoreCacheV1: only `cache read denied:` errors
// are re-classified; every other getCacheEntry failure must pass through and
// be logged normally (not swallowed or mislabeled as a read denial).
const paths = ['node_modules']
const key = 'node-test'
const logErrorMock = jest.spyOn(core, 'error')
const logWarningMock = jest.spyOn(core, 'warning')
const genericMessage = 'Cache service responded with 400'

jest.spyOn(cacheHttpClient, 'getCacheEntry').mockImplementation(async () => {
throw new Error(genericMessage)
})

const cacheKey = await restoreCache(paths, key)
expect(cacheKey).toBe(undefined)
expect(logErrorMock).not.toHaveBeenCalled()
expect(logWarningMock).toHaveBeenCalledWith(
`Failed to restore: ${genericMessage}`
)
expect(logWarningMock).toHaveBeenCalledTimes(1)
})

test('restore with restore keys and no cache found', async () => {
const paths = ['node_modules']
const key = 'node-test'
Expand Down
27 changes: 27 additions & 0 deletions packages/cache/__tests__/restoreCacheV2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,33 @@ test('restore with server error should fail', async () => {
)
})

test('restore denied by read-only token logs warning and reports cache miss', async () => {
// The receiver returns twirp PermissionDenied (403) when the run's token has
// no readable cache scopes; the client wraps it so the `cache read denied:`
// prefix arrives embedded. Expect a single warning (not error) and a miss.
const paths = ['node_modules']
const key = 'node-test'
const logErrorMock = jest.spyOn(core, 'error')
const logWarningMock = jest.spyOn(core, 'warning')
const wrappedDeniedMessage =
'Failed to GetCacheEntryDownloadURL: Received non-retryable error: ' +
'Failed request: (403) Forbidden: cache read denied: token has no readable scopes'

jest
.spyOn(CacheServiceClientJSON.prototype, 'GetCacheEntryDownloadURL')
.mockImplementation(() => {
throw new Error(wrappedDeniedMessage)
})

const cacheKey = await restoreCache(paths, key)
expect(cacheKey).toBe(undefined)
expect(logErrorMock).not.toHaveBeenCalled()
expect(logWarningMock).toHaveBeenCalledWith(
`Failed to restore: ${wrappedDeniedMessage}`
)
expect(logWarningMock).toHaveBeenCalledTimes(1)
})

test('restore with restore keys and no cache found', async () => {
const paths = ['node_modules']
const key = 'node-test'
Expand Down
4 changes: 2 additions & 2 deletions packages/cache/package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/cache/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "@actions/cache",
"version": "6.1.0",
"version": "6.2.0",
"description": "Actions cache lib",
"keywords": [
"github",
Expand Down
60 changes: 55 additions & 5 deletions packages/cache/src/cache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,23 @@ export class CacheWriteDeniedError extends ReserveCacheError {
}
}

// Prefix the receiver embeds in a cache read denial: the v2 twirp
// GetCacheEntryDownloadURL error, or the GHES v1 `_apis/artifactcache` 403
// body. Match with `includes`, not `startsWith`: the message is wrapped by
// the transport, so the prefix ends up embedded rather than leading.
export const CACHE_READ_DENIED_PREFIX = 'cache read denied:'
Comment thread
philip-gai marked this conversation as resolved.
Outdated

// Raised when the cache backend denies a download URL because the run's token
// has no readable cache scopes. Caching is best-effort, so restoreCache logs a
// warning and reports a cache miss rather than rethrowing this.
export class CacheReadDeniedError extends Error {
constructor(message: string) {
super(message)
this.name = 'CacheReadDeniedError'
Object.setPrototypeOf(this, CacheReadDeniedError.prototype)
}
}

export class FinalizeCacheError extends Error {
constructor(message: string) {
super(message)
Expand Down Expand Up @@ -191,10 +208,23 @@ async function restoreCacheV1(
let archivePath = ''
try {
// path are needed to compute version
const cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, {
compressionMethod,
enableCrossOsArchive
})
let cacheEntry
try {
cacheEntry = await cacheHttpClient.getCacheEntry(keys, paths, {
compressionMethod,
enableCrossOsArchive
})
} catch (error) {
// The GHES v1 artifact cache service returns HTTP 403 with a
// `cache read denied:` body when the run's token has no readable cache
// scopes. getCacheEntry surfaces that body message, so re-classify it
// here to mirror the read-denied handling on the v2 path.
const errorMessage = (error as Error)?.message ?? ''
if (errorMessage.includes(CACHE_READ_DENIED_PREFIX)) {
throw new CacheReadDeniedError(errorMessage)
}
throw error
}
if (!cacheEntry?.archiveLocation) {
// Cache not found
return undefined
Expand Down Expand Up @@ -237,6 +267,10 @@ async function restoreCacheV1(
const typedError = error as Error
if (typedError.name === ValidationError.name) {
throw error
} else if (typedError.name === CacheReadDeniedError.name) {
// Read denied by policy (token has no readable cache scopes). Warn and
// treat as a cache miss so the workflow continues.
core.warning(`Failed to restore: ${typedError.message}`)
} else {
// warn on cache restore failure and continue build
// Log server errors (5xx) as errors, all other errors as warnings
Expand Down Expand Up @@ -314,7 +348,19 @@ async function restoreCacheV2(
)
}

const response = await twirpClient.GetCacheEntryDownloadURL(request)
let response
try {
response = await twirpClient.GetCacheEntryDownloadURL(request)
} catch (error) {
// The receiver returns twirp PermissionDenied (403) when the run's token
// has no readable cache scopes. The client wraps that 403, so the stable
// prefix is embedded in the message rather than leading it.
const errorMessage = (error as Error)?.message ?? ''
if (errorMessage.includes(CACHE_READ_DENIED_PREFIX)) {
throw new CacheReadDeniedError(errorMessage)
}
throw error
}

if (!response.ok) {
core.debug(
Expand Down Expand Up @@ -369,6 +415,10 @@ async function restoreCacheV2(
const typedError = error as Error
if (typedError.name === ValidationError.name) {
throw error
} else if (typedError.name === CacheReadDeniedError.name) {
// Read denied by policy (token has no readable cache scopes). Warn and
// treat as a cache miss so the workflow continues.
core.warning(`Failed to restore: ${typedError.message}`)
} else {
// Supress all non-validation cache related errors because caching should be optional
Comment thread
Copilot marked this conversation as resolved.
Outdated
// Log server errors (5xx) as errors, all other errors as warnings
Expand Down
6 changes: 6 additions & 0 deletions packages/cache/src/internal/cacheHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,12 @@ export async function getCacheEntry(
return null
}
if (!isSuccessStatusCode(response.statusCode)) {
// Only surface the receiver's body for a `cache read denied:` policy denial
// so callers can dispatch on it; keep the generic message otherwise.
const errorMessage = response.error?.message
if (errorMessage?.includes('cache read denied:')) {
throw new Error(errorMessage)
}
throw new Error(`Cache service responded with ${response.statusCode}`)
}

Expand Down
Loading