Backport: Handle cache write error due to RO token (#2435)

packages/cache/RELEASES.md

@@ -1,5 +1,9 @@
 # @actions/cache Releases
 
+### 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).
+
 ### 5.0.5
 
 - Bump `@actions/glob` to `0.5.1`
@@ -247,11 +251,11 @@ Read more about the change & access the migration guide: [reference to the annou
 ### 0.2.1
 
 - Fix to await async function getCompressionMethod
-  
+
 ### 0.2.0
 
 - Fixes issues with the zstd compression algorithm on Windows and Ubuntu 16.04 [#469](https://github.com/actions/toolkit/pull/469)
-  
+
 ### 0.1.0
 
 - Initial release

packages/cache/__tests__/saveCache.test.ts

@@ -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'

packages/cache/__tests__/saveCacheV2.test.ts

@@ -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'

packages/cache/package-lock.json

@@ -1,12 +1,12 @@
 {
   "name": "@actions/cache",
-  "version": "5.0.5",
+  "version": "5.1.0",
   "lockfileVersion": 3,
   "requires": true,
   "packages": {
     "": {
       "name": "@actions/cache",
-      "version": "5.0.5",
+      "version": "5.1.0",
       "license": "MIT",
       "dependencies": {
         "@actions/core": "^2.0.0",

packages/cache/package.json

@@ -1,6 +1,6 @@
 {
   "name": "@actions/cache",
-  "version": "5.0.5",
+  "version": "5.1.0",
   "preview": true,
   "description": "Actions cache lib",
   "keywords": [
@@ -58,4 +58,4 @@
     "uri-js": "npm:uri-js-replace@^1.0.1",
     "node-fetch": "^3.3.2"
   }
-}
+}

packages/cache/src/cache.ts

@@ -29,6 +29,27 @@ export class ReserveCacheError extends Error {
   }
 }
 
+/**
+ * Stable prefix used by the cache receiver to signal that the token has
+ * no writable scopes (read-only cache policy). Consumers can match on
+ * this prefix to distinguish policy denials from ordinary contention.
+ */
+export const CACHE_WRITE_DENIED_PREFIX = '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 a policy denial can check for CacheWriteDeniedError.name.
+ */
+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)
@@ -465,8 +486,14 @@ async function saveCacheV1(
           )} MB (${archiveFileSize} B) is over the data cap limit, not saving cache.`
       )
     } else {
+      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}`
+        `Unable to reserve cache with key ${key}, another job may be creating this cache. More details: ${detailMessage}`
       )
     }
 
@@ -476,6 +503,8 @@ async function saveCacheV1(
     const typedError = error as Error
     if (typedError.name === ValidationError.name) {
       throw error
+    } else if (typedError.name === CacheWriteDeniedError.name) {
+      core.warning(`Failed to save: ${typedError.message}`)
     } else if (typedError.name === ReserveCacheError.name) {
       core.info(`Failed to save: ${typedError.message}`)
     } else {
@@ -576,7 +605,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')
@@ -584,6 +619,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.`
       )
@@ -621,6 +662,8 @@ async function saveCacheV2(
     const typedError = error as Error
     if (typedError.name === ValidationError.name) {
       throw error
+    } else if (typedError.name === CacheWriteDeniedError.name) {
+      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) {