Skip to content
Open
Show file tree
Hide file tree
Changes from all 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
5 changes: 5 additions & 0 deletions packages/cache/RELEASES.md
Original file line number Diff line number Diff line change
@@ -1,5 +1,10 @@
# @actions/cache Releases

### 5.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).
- Honor the `ACTIONS_CACHE_MODE` environment 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-fatal `core.info` line. When `ACTIONS_CACHE_MODE` is unset or unrecognized, behavior is unchanged.

### 5.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 server'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
34 changes: 34 additions & 0 deletions packages/cache/__tests__/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,3 +23,37 @@ test('isGhes returns false for ghe.localhost', () => {
process.env.GITHUB_SERVER_URL = 'https://my.domain.ghe.localhost'
expect(config.isGhes()).toBe(false)
})

describe('cache-mode helpers', () => {
const original = process.env.ACTIONS_CACHE_MODE

afterEach(() => {
if (original === undefined) {
delete process.env.ACTIONS_CACHE_MODE
} else {
process.env.ACTIONS_CACHE_MODE = original
}
})

test('getCacheMode normalizes whitespace and case', () => {
process.env.ACTIONS_CACHE_MODE = ' Write-Only '
expect(config.getCacheMode()).toBe('write-only')
})

test('getCacheMode returns empty string when unset', () => {
delete process.env.ACTIONS_CACHE_MODE
expect(config.getCacheMode()).toBe('')
})

test.each([
['', true, true],
['read', true, false],
['write', true, true],
['write-only', false, true],
['none', false, false],
['garbage', true, true]
])("mode '%s' -> readable=%s writable=%s", (mode, readable, writable) => {
expect(config.isCacheReadable(mode)).toBe(readable)
expect(config.isCacheWritable(mode)).toBe(writable)
})
})
87 changes: 87 additions & 0 deletions packages/cache/__tests__/restoreCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,93 @@ test('restore with server error should fail', async () => {
delete process.env['ACTIONS_RESULTS_URL']
})

test('restore surfaces a getCacheEntry failure as a warning and reports a cache miss', async () => {
// restoreCache treats any getCacheEntry failure (read-denied or otherwise)
// as a non-fatal warning and a cache miss so the workflow continues. The
// read-denied prefix detection itself is covered in cacheHttpClient.test.ts.
const paths = ['node_modules']
const key = 'node-test'
const logErrorMock = jest.spyOn(core, 'error')
const logWarningMock = jest.spyOn(core, 'warning')
const message = 'cache read denied: token has no readable scopes'

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

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

describe('restore cache-mode gating', () => {
const originalMode = process.env.ACTIONS_CACHE_MODE
const originalV2 = process.env.ACTIONS_CACHE_SERVICE_V2

const restoreEnv = (key: string, value: string | undefined): void => {
if (value === undefined) {
delete process.env[key]
} else {
process.env[key] = value
}
}

afterEach(() => {
restoreEnv('ACTIONS_CACHE_MODE', originalMode)
restoreEnv('ACTIONS_CACHE_SERVICE_V2', originalV2)
})

// The skip short-circuits before v1/v2 dispatch, so it applies regardless of
// the ACTIONS_CACHE_SERVICE_V2 feature flag.
test.each([
['none', undefined],
['none', 'true'],
['write-only', undefined],
['write-only', 'true']
])(
"mode '%s' skips restore with ACTIONS_CACHE_SERVICE_V2=%s",
async (mode, v2) => {
process.env.ACTIONS_CACHE_MODE = mode
restoreEnv('ACTIONS_CACHE_SERVICE_V2', v2)
const logInfoMock = jest.spyOn(core, 'info')
const getCacheEntryMock = jest.spyOn(cacheHttpClient, 'getCacheEntry')

const cacheKey = await restoreCache(['node_modules'], 'node-test')

expect(cacheKey).toBe(undefined)
expect(getCacheEntryMock).not.toHaveBeenCalled()
expect(logInfoMock).toHaveBeenCalledTimes(1)
expect(logInfoMock).toHaveBeenCalledWith(
`Cache restore skipped: the effective cache-mode '${mode}' does not permit reads.`
)
}
)

test.each(['read', 'write', '', 'garbage'])(
"mode '%s' does not skip restore",
async mode => {
if (mode === '') {
delete process.env.ACTIONS_CACHE_MODE
} else {
process.env.ACTIONS_CACHE_MODE = mode
}
const logInfoMock = jest.spyOn(core, 'info')
const getCacheEntryMock = jest
.spyOn(cacheHttpClient, 'getCacheEntry')
.mockResolvedValue(null as never)

await restoreCache(['node_modules'], 'node-test')

expect(getCacheEntryMock).toHaveBeenCalledTimes(1)
expect(logInfoMock).not.toHaveBeenCalledWith(
expect.stringContaining('Cache restore skipped')
)
}
)
})

test('restore with restore keys and no cache found', async () => {
const paths = ['node_modules']
const key = 'node-test'
Expand Down
40 changes: 40 additions & 0 deletions packages/cache/__tests__/restoreCacheV2.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,19 @@ beforeAll(() => {
// Ensure that we're using v2 for these tests
jest.spyOn(config, 'getCacheServiceVersion').mockReturnValue('v2')

// 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
.mockImplementation(actualConfig.getCacheMode)
jest
.spyOn(config, 'isCacheReadable')
.mockImplementation(actualConfig.isCacheReadable)
jest
.spyOn(config, 'isCacheWritable')
.mockImplementation(actualConfig.isCacheWritable)

logDebugMock = jest.spyOn(core, 'debug')
logInfoMock = jest.spyOn(core, 'info')
})
Expand Down Expand Up @@ -112,6 +125,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
82 changes: 82 additions & 0 deletions packages/cache/__tests__/saveCache.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,18 @@ beforeAll(() => {
jest.spyOn(cacheUtils, 'createTempDirectory').mockImplementation(async () => {
return Promise.resolve('/foo/bar')
})
// 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 +38 to +42
.mockImplementation(actualConfig.getCacheMode)
jest
.spyOn(config, 'isCacheReadable')
.mockImplementation(actualConfig.isCacheReadable)
jest
.spyOn(config, 'isCacheWritable')
.mockImplementation(actualConfig.isCacheWritable)
})

test('save with missing input should fail', async () => {
Expand All @@ -45,6 +57,75 @@ test('save with missing input should fail', async () => {
)
})

describe('save cache-mode gating', () => {
const originalMode = process.env.ACTIONS_CACHE_MODE
const originalV2 = process.env.ACTIONS_CACHE_SERVICE_V2

const restoreEnv = (key: string, value: string | undefined): void => {
if (value === undefined) {
delete process.env[key]
} else {
process.env[key] = value
}
}

afterEach(() => {
restoreEnv('ACTIONS_CACHE_MODE', originalMode)
restoreEnv('ACTIONS_CACHE_SERVICE_V2', originalV2)
})

// The skip short-circuits before v1/v2 dispatch, so it applies regardless of
// the ACTIONS_CACHE_SERVICE_V2 feature flag.
test.each([
['read', undefined],
['read', 'true'],
['none', undefined],
['none', 'true']
])(
"mode '%s' skips save with ACTIONS_CACHE_SERVICE_V2=%s",
async (mode, v2) => {
process.env.ACTIONS_CACHE_MODE = mode
restoreEnv('ACTIONS_CACHE_SERVICE_V2', v2)
const logInfoMock = jest.spyOn(core, 'info')
const resolvePathsMock = jest.spyOn(cacheUtils, 'resolvePaths')

const cacheId = await saveCache(['node_modules'], 'node-test')

expect(cacheId).toBe(-1)
expect(resolvePathsMock).not.toHaveBeenCalled()
expect(logInfoMock).toHaveBeenCalledTimes(1)
expect(logInfoMock).toHaveBeenCalledWith(
`Cache save skipped: the effective cache-mode '${mode}' does not permit writes.`
)
}
)

test.each(['write', 'write-only', '', 'garbage'])(
"mode '%s' does not skip save",
async mode => {
if (mode === '') {
delete process.env.ACTIONS_CACHE_MODE
} else {
process.env.ACTIONS_CACHE_MODE = mode
}
const logInfoMock = jest.spyOn(core, 'info')
const resolvePathsMock = jest.spyOn(cacheUtils, 'resolvePaths')

try {
await saveCache(['node_modules'], 'node-test')
} catch {
// Downstream client is not fully mocked here; we only assert the guard
// let execution proceed past it.
}

expect(resolvePathsMock).toHaveBeenCalled()
expect(logInfoMock).not.toHaveBeenCalledWith(
expect.stringContaining('Cache save skipped')
)
}
)
})

test('save with large cache outputs should fail', async () => {
const filePath = 'node_modules'
const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43'
Expand Down Expand Up @@ -265,6 +346,7 @@ test('save with reserve cache denied by read-only token logs warning (not info)'
`Failed to save: Unable to reserve cache with key ${primaryKey}. More details: ${deniedMessage}`
)

expect(logWarningMock).toHaveBeenCalledTimes(1)
expect(reserveCacheMock).toHaveBeenCalledTimes(1)
expect(createTarMock).toHaveBeenCalledTimes(1)
expect(saveCacheMock).toHaveBeenCalledTimes(0)
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": "5.1.0",
"version": "5.2.0",
"preview": true,
"description": "Actions cache lib",
"keywords": [
Expand Down
Loading
Loading