Handle cache write error due to read-only token

This commit is contained in:
Jason Ginchereau
2026-06-11 17:02:12 -07:00
parent 4b9afa4c89
commit 78e3d71e19
3 changed files with 179 additions and 1 deletions
+50
View File
@@ -221,6 +221,56 @@ test('save with reserve cache failure should fail', async () => {
expect(getCompressionMock).toHaveBeenCalledTimes(1)
})
test('save with reserve cache denied by read-only token logs warning (not info)', async () => {
// V1 path: when the legacy ReserveCache REST call returns an error message
// starting with the stable `cache write denied:` prefix, the toolkit must
// surface it as a `core.warning` (not the usual `core.info` used for the
// generic "another job may be creating this cache" contention case).
const paths = ['node_modules']
const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43'
const deniedMessage =
'cache write denied: read-only token issued for untrusted trigger'
const logInfoMock = jest.spyOn(core, 'info')
const logWarningMock = jest.spyOn(core, 'warning')
const reserveCacheMock = jest
.spyOn(cacheHttpClient, 'reserveCache')
.mockImplementation(async () => {
const response: ITypedResponseWithError<ReserveCacheResponse> = {
statusCode: 403,
result: null,
headers: {},
error: new HttpClientError(deniedMessage, 403)
}
return response
})
const createTarMock = jest.spyOn(tar, 'createTar')
const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache')
const compression = CompressionMethod.Zstd
const getCompressionMock = jest
.spyOn(cacheUtils, 'getCompressionMethod')
.mockReturnValueOnce(Promise.resolve(compression))
const cacheId = await saveCache(paths, primaryKey)
expect(cacheId).toBe(-1)
// The generic "another job may be creating this cache" info log MUST NOT
// fire — this is a policy denial, not a contention case.
expect(logInfoMock).not.toHaveBeenCalledWith(
expect.stringContaining('another job may be creating this cache')
)
// A single warning carrying the stable prefix is what the customer sees.
expect(logWarningMock).toHaveBeenCalledWith(
`Failed to save: Unable to reserve cache with key ${primaryKey}. More details: ${deniedMessage}`
)
expect(reserveCacheMock).toHaveBeenCalledTimes(1)
expect(createTarMock).toHaveBeenCalledTimes(1)
expect(saveCacheMock).toHaveBeenCalledTimes(0)
expect(getCompressionMock).toHaveBeenCalledTimes(1)
})
test('save with server error should fail', async () => {
const filePath = 'node_modules'
const primaryKey = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43'
+64
View File
@@ -100,6 +100,70 @@ test('create cache entry failure on non-ok response', async () => {
expect(saveCacheMock).toHaveBeenCalledTimes(0)
})
test('create cache entry denied by read-only token logs single warning and skips inner warning', async () => {
// When the receiver signals a policy-driven write denial (token was
// downgraded to read-only), the toolkit should:
// 1. Suppress the inner `Cache reservation failed: ...` warning so the
// runner log is not noisy with duplicate messages.
// 2. Emit a single outer `Failed to save: Unable to reserve cache with
// key ${key}. More details: cache write denied: ...` at `warning`
// level (not `info`), so the customer can see why caching stopped.
const paths = ['node_modules']
const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43'
const deniedMessage =
'cache write denied: read-only token issued for untrusted trigger'
const infoLogMock = jest.spyOn(core, 'info')
const warningLogMock = jest.spyOn(core, 'warning')
const createCacheEntryMock = jest
.spyOn(CacheServiceClientJSON.prototype, 'CreateCacheEntry')
.mockResolvedValue({ok: false, signedUploadUrl: '', message: deniedMessage})
const createTarMock = jest.spyOn(tar, 'createTar')
const finalizeCacheEntryMock = jest.spyOn(
CacheServiceClientJSON.prototype,
'FinalizeCacheEntryUpload'
)
const compression = CompressionMethod.Zstd
const getCompressionMock = jest
.spyOn(cacheUtils, 'getCompressionMethod')
.mockResolvedValueOnce(compression)
const archiveFileSize = 1024
jest
.spyOn(cacheUtils, 'getArchiveFileSizeInBytes')
.mockReturnValueOnce(archiveFileSize)
const cacheVersion = cacheUtils.getCacheVersion(paths, compression)
const saveCacheMock = jest.spyOn(cacheHttpClient, 'saveCache')
const cacheId = await saveCache(paths, key)
expect(cacheId).toBe(-1)
// No "another job may be creating this cache" info log: this is the
// policy-denial path, not a contention path.
expect(infoLogMock).not.toHaveBeenCalledWith(
`Failed to save: Unable to reserve cache with key ${key}, another job may be creating this cache.`
)
// No inner "Cache reservation failed:" warning either: the outer arm owns
// the customer-facing message in this scenario.
expect(warningLogMock).not.toHaveBeenCalledWith(
`Cache reservation failed: ${deniedMessage}`
)
// One outer warning that includes the stable "cache write denied:" prefix
// so the runner UI and post-action consumers can dispatch on it.
expect(warningLogMock).toHaveBeenCalledWith(
`Failed to save: Unable to reserve cache with key ${key}. More details: ${deniedMessage}`
)
expect(createCacheEntryMock).toHaveBeenCalledWith({
key,
version: cacheVersion
})
expect(createTarMock).toHaveBeenCalledTimes(1)
expect(getCompressionMock).toHaveBeenCalledTimes(1)
expect(finalizeCacheEntryMock).toHaveBeenCalledTimes(0)
expect(saveCacheMock).toHaveBeenCalledTimes(0)
})
test('create cache entry fails on rejected promise', async () => {
const paths = ['node_modules']
const key = 'Linux-node-bb828da54c148048dd17899ba9fda624811cfb43'
+65 -1
View File
@@ -31,6 +31,36 @@ export class ReserveCacheError extends Error {
}
}
/**
* Stable prefix the receiver writes into the cache reservation response when
* the issuer downgraded the cache token to read-only (for example, because
* the run was triggered by an untrusted event). saveCacheV1 / saveCacheV2
* dispatch on this prefix to re-classify the failure as a
* CacheWriteDeniedError so consumers (and the outer catch arm) can
* distinguish a policy denial from other reservation failures.
*/
export const CACHE_WRITE_DENIED_PREFIX = 'cache write denied:'
/**
* Raised when the cache backend refuses to reserve a writable cache entry
* because the JWT issued for this run was scoped read-only (for example, the
* run was triggered by an event the repository administrator classified as
* untrusted). The error message is forwarded verbatim from the receiver and
* always begins with `cache write denied:`.
*
* Extends ReserveCacheError for source-compatibility: existing
* `instanceof ReserveCacheError` checks and `typedError.name ===
* ReserveCacheError.name` paths keep working, while consumers that want to
* distinguish the policy case can match on this subclass.
*/
export class CacheWriteDeniedError extends ReserveCacheError {
constructor(message: string) {
super(message)
this.name = 'CacheWriteDeniedError'
Object.setPrototypeOf(this, CacheWriteDeniedError.prototype)
}
}
export class FinalizeCacheError extends Error {
constructor(message: string) {
super(message)
@@ -467,6 +497,18 @@ async function saveCacheV1(
)} MB (${archiveFileSize} B) is over the data cap limit, not saving cache.`
)
} else {
// Inspect the receiver's error message before deciding which error to
// throw. A message starting with the stable `cache write denied:`
// prefix indicates the issuer downgraded the token to read-only
// (policy denial), not a contention case, so we surface it as a
// CacheWriteDeniedError which the outer catch arm logs at warning
// level.
const detailMessage = reserveCacheResponse?.error?.message
if (detailMessage?.startsWith(CACHE_WRITE_DENIED_PREFIX)) {
throw new CacheWriteDeniedError(
`Unable to reserve cache with key ${key}. More details: ${detailMessage}`
)
}
throw new ReserveCacheError(
`Unable to reserve cache with key ${key}, another job may be creating this cache. More details: ${reserveCacheResponse?.error?.message}`
)
@@ -478,6 +520,11 @@ async function saveCacheV1(
const typedError = error as Error
if (typedError.name === ValidationError.name) {
throw error
} else if (typedError.name === CacheWriteDeniedError.name) {
// Cache write was denied by policy (read-only token). Surface to the
// customer at warning level so it is visible in the workflow log
// without failing the run.
core.warning(`Failed to save: ${typedError.message}`)
} else if (typedError.name === ReserveCacheError.name) {
core.info(`Failed to save: ${typedError.message}`)
} else {
@@ -578,7 +625,13 @@ async function saveCacheV2(
try {
const response = await twirpClient.CreateCacheEntry(request)
if (!response.ok) {
if (response.message) {
// Skip the redundant inner warning when the receiver signalled a
// policy denial: the outer catch arm below will log a single
// customer-facing warning.
if (
response.message &&
!response.message.startsWith(CACHE_WRITE_DENIED_PREFIX)
) {
core.warning(`Cache reservation failed: ${response.message}`)
}
throw new Error(response.message || 'Response was not ok')
@@ -586,6 +639,12 @@ async function saveCacheV2(
signedUploadUrl = response.signedUploadUrl
} catch (error) {
core.debug(`Failed to reserve cache: ${error}`)
const errorMessage = (error as Error)?.message ?? ''
if (errorMessage.startsWith(CACHE_WRITE_DENIED_PREFIX)) {
throw new CacheWriteDeniedError(
`Unable to reserve cache with key ${key}. More details: ${errorMessage}`
)
}
throw new ReserveCacheError(
`Unable to reserve cache with key ${key}, another job may be creating this cache.`
)
@@ -623,6 +682,11 @@ async function saveCacheV2(
const typedError = error as Error
if (typedError.name === ValidationError.name) {
throw error
} else if (typedError.name === CacheWriteDeniedError.name) {
// Cache write was denied by policy (read-only token). Surface to the
// customer at warning level so it is visible in the workflow log
// without failing the run.
core.warning(`Failed to save: ${typedError.message}`)
} else if (typedError.name === ReserveCacheError.name) {
core.info(`Failed to save: ${typedError.message}`)
} else if (typedError.name === FinalizeCacheError.name) {