masks the whole URL, update tests

packages/artifact/__tests__/artifactTwirpClient.test.ts

@@ -1,9 +1,6 @@
 import {ArtifactHttpClient} from '../src/internal/shared/artifact-twirp-client'
-import {setSecret, debug} from '@actions/core'
-import {
-  CreateArtifactResponse,
-  GetSignedArtifactURLResponse
-} from '../src/generated/results/api/v1/artifact'
+import {setSecret} from '@actions/core'
+import {CreateArtifactResponse} from '../src/generated/results/api/v1/artifact'
 
 jest.mock('@actions/core', () => ({
   setSecret: jest.fn(),
@@ -26,70 +23,101 @@ describe('ArtifactHttpClient', () => {
     delete process.env['ACTIONS_RESULTS_URL']
   })
 
+  describe('maskSigUrl', () => {
+    it('should mask the sig parameter and set it as a secret', () => {
+      const url =
+        'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
+
+      const maskedUrl = client.maskSigUrl(url)
+
+      expect(setSecret).toHaveBeenCalledWith('secret-token')
+      expect(maskedUrl).toBe(
+        'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      )
+    })
+
+    it('should return the original URL if no sig parameter is found', () => {
+      const url = 'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z'
+
+      const maskedUrl = client.maskSigUrl(url)
+
+      expect(setSecret).not.toHaveBeenCalled()
+      expect(maskedUrl).toBe(url)
+    })
+
+    it('should handle sig parameter at the end of the URL', () => {
+      const url = 'https://example.com/upload?param1=value&sig=secret-token'
+
+      const maskedUrl = client.maskSigUrl(url)
+
+      expect(setSecret).toHaveBeenCalledWith('secret-token')
+      expect(maskedUrl).toBe('https://example.com/upload?param1=value&sig=***')
+    })
+
+    it('should handle sig parameter in the middle of the URL', () => {
+      const url = 'https://example.com/upload?sig=secret-token&param2=value'
+
+      const maskedUrl = client.maskSigUrl(url)
+
+      expect(setSecret).toHaveBeenCalledWith('secret-token&param2=value')
+      expect(maskedUrl).toBe('https://example.com/upload?sig=***')
+    })
+  })
+
   describe('maskSecretUrls', () => {
     it('should mask signed_upload_url', () => {
-      const response: CreateArtifactResponse = {
+      const spy = jest.spyOn(client, 'maskSigUrl')
+      const response = {
         ok: true,
-        signedUploadUrl:
+        signed_upload_url:
           'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
       }
 
       client.maskSecretUrls(response)
 
-      expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_upload_url: https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
       )
     })
 
     it('should mask signed_download_url', () => {
-      const response: GetSignedArtifactURLResponse = {
-        signedUrl:
+      const spy = jest.spyOn(client, 'maskSigUrl')
+      const response = {
+        signed_url:
           'https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
       }
 
       client.maskSecretUrls(response)
 
-      expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_url: https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
       )
     })
 
-    it('should not call setSecret if URLs are missing', () => {
+    it('should not call maskSigUrl if URLs are missing', () => {
+      const spy = jest.spyOn(client, 'maskSigUrl')
       const response = {} as CreateArtifactResponse
 
       client.maskSecretUrls(response)
 
-      expect(setSecret).not.toHaveBeenCalled()
+      expect(spy).not.toHaveBeenCalled()
     })
 
-    it('should mask only the sensitive token part of signed_upload_url', () => {
-      const response: CreateArtifactResponse = {
-        ok: true,
-        signedUploadUrl:
-          'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
+    it('should handle both URL types when present', () => {
+      const spy = jest.spyOn(client, 'maskSigUrl')
+      const response = {
+        signed_upload_url: 'https://example.com/upload?sig=secret-token1',
+        signed_url: 'https://example.com/download?sig=secret-token2'
       }
 
       client.maskSecretUrls(response)
 
-      expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_upload_url: https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      expect(spy).toHaveBeenCalledTimes(2)
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/upload?sig=secret-token1'
       )
-    })
-
-    it('should mask only the sensitive token part of signed_download_url', () => {
-      const response: GetSignedArtifactURLResponse = {
-        signedUrl:
-          'https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
-      }
-
-      client.maskSecretUrls(response)
-
-      expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_url: https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/download?sig=secret-token2'
       )
     })
   })

packages/artifact/src/internal/shared/artifact-twirp-client.ts

@@ -5,10 +5,6 @@ import {ArtifactServiceClientJSON} from '../../generated'
 import {getResultsServiceUrl, getRuntimeToken} from './config'
 import {getUserAgentString} from './user-agent'
 import {NetworkError, UsageError} from './errors'
-import {
-  CreateArtifactResponse,
-  GetSignedArtifactURLResponse
-} from '../../generated/results/api/v1/artifact'
 
 // The twirp http client must implement this interface
 interface Rpc {
@@ -77,24 +73,32 @@ export class ArtifactHttpClient implements Rpc {
   /**
    * Masks the `sig` parameter in a URL and sets it as a secret.
    * @param url The URL containing the `sig` parameter.
-   * @param urlType The type of the URL (e.g., 'signed_upload_url', 'signed_download_url').
+   * @returns A masked URL where the sig parameter value is replaced with '***' if found,
+   *          or the original URL if no sig parameter is present.
    */
-  maskSigUrl(url: string, urlType: string): void {
-    const sigMatch = url.match(/[?&]sig=([^&]+)/)
-    if (sigMatch) {
-      setSecret(sigMatch[1])
-      debug(`Masked ${urlType}: ${url.replace(sigMatch[1], '***')}`)
+  maskSigUrl(url: string): string {
+    const sigIndex = url.indexOf('sig=')
+    if (sigIndex !== -1) {
+      const sigValue = url.substring(sigIndex + 4)
+      setSecret(sigValue)
+      return `${url.substring(0, sigIndex + 4)}***`
     }
+    return url
   }
 
-  maskSecretUrls(
-    body: CreateArtifactResponse | GetSignedArtifactURLResponse
-  ): void {
-    if ('signedUploadUrl' in body && body.signedUploadUrl) {
-      this.maskSigUrl(body.signedUploadUrl, 'signed_upload_url')
-    }
-    if ('signedUrl' in body && body.signedUrl) {
-      this.maskSigUrl(body.signedUrl, 'signed_url')
+  maskSecretUrls(body): void {
+    if (typeof body === 'object' && body !== null) {
+      if (
+        'signed_upload_url' in body &&
+        typeof body.signed_upload_url === 'string'
+      ) {
+        this.maskSigUrl(body.signed_upload_url)
+      }
+      if ('signed_url' in body && typeof body.signed_url === 'string') {
+        this.maskSigUrl(body.signed_url)
+      }
+    } else {
+      debug('body is not an object or is null')
     }
   }
 

packages/cache/__tests__/cacheTwirpClient.test.ts

@@ -1,9 +1,5 @@
-import {
-  CreateCacheEntryResponse,
-  GetCacheEntryDownloadURLResponse
-} from '../src/generated/results/api/v1/cache'
 import {CacheServiceClient} from '../src/internal/shared/cacheTwirpClient'
-import {setSecret, debug} from '@actions/core'
+import {setSecret} from '@actions/core'
 
 jest.mock('@actions/core', () => ({
   setSecret: jest.fn(),
@@ -24,75 +20,106 @@ describe('CacheServiceClient', () => {
     delete process.env['ACTIONS_RUNTIME_TOKEN']
   })
 
-  describe('maskSecretUrls', () => {
-    it('should mask signedUploadUrl', () => {
-      const response = {
-        ok: true,
-        signedUploadUrl:
-          'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
-      } as CreateCacheEntryResponse
+  describe('maskSigUrl', () => {
+    it('should mask the sig parameter and set it as a secret', () => {
+      const url =
+        'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
 
-      client.maskSecretUrls(response)
+      const maskedUrl = client.maskSigUrl(url)
 
       expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_upload_url: https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      expect(maskedUrl).toBe(
+        'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=***'
       )
     })
 
-    it('should mask signedDownloadUrl', () => {
-      const response = {
-        ok: true,
-        signedDownloadUrl:
-          'https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=secret-token',
-        matchedKey: 'cache-key'
-      } as GetCacheEntryDownloadURLResponse
+    it('should return the original URL if no sig parameter is found', () => {
+      const url = 'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z'
 
-      client.maskSecretUrls(response)
-
-      expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_download_url: https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=***'
-      )
-    })
-
-    it('should not call setSecret if URLs are missing', () => {
-      const response = {ok: true} as CreateCacheEntryResponse
-
-      client.maskSecretUrls(response)
+      const maskedUrl = client.maskSigUrl(url)
 
       expect(setSecret).not.toHaveBeenCalled()
+      expect(maskedUrl).toBe(url)
     })
 
-    it('should mask only the sensitive token part of signedUploadUrl', () => {
-      const response = {
-        ok: true,
-        signedUploadUrl:
-          'https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=secret-token'
-      } as CreateCacheEntryResponse
+    it('should handle sig parameter at the end of the URL', () => {
+      const url = 'https://example.com/upload?param1=value&sig=secret-token'
 
-      client.maskSecretUrls(response)
+      const maskedUrl = client.maskSigUrl(url)
 
       expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_upload_url: https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      expect(maskedUrl).toBe('https://example.com/upload?param1=value&sig=***')
+    })
+
+    it('should handle sig parameter in the middle of the URL', () => {
+      const url = 'https://example.com/upload?sig=secret-token&param2=value'
+
+      const maskedUrl = client.maskSigUrl(url)
+
+      expect(setSecret).toHaveBeenCalledWith('secret-token&param2=value')
+      expect(maskedUrl).toBe('https://example.com/upload?sig=***')
+    })
+  })
+
+  describe('maskSecretUrls', () => {
+    it('should mask signed_upload_url', () => {
+      const spy = jest.spyOn(client, 'maskSigUrl')
+      const body = {
+        signed_upload_url: 'https://example.com/upload?sig=secret-token',
+        key: 'test-key',
+        version: 'test-version'
+      }
+
+      client.maskSecretUrls(body)
+
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/upload?sig=secret-token'
       )
     })
 
-    it('should mask only the sensitive token part of signedDownloadUrl', () => {
-      const response = {
-        ok: true,
-        signedDownloadUrl:
-          'https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=secret-token',
-        matchedKey: 'cache-key'
-      } as GetCacheEntryDownloadURLResponse
+    it('should mask signed_download_url', () => {
+      const spy = jest.spyOn(client, 'maskSigUrl')
+      const body = {
+        signed_download_url: 'https://example.com/download?sig=secret-token',
+        key: 'test-key',
+        version: 'test-version'
+      }
 
-      client.maskSecretUrls(response)
+      client.maskSecretUrls(body)
 
-      expect(setSecret).toHaveBeenCalledWith('secret-token')
-      expect(debug).toHaveBeenCalledWith(
-        'Masked signed_download_url: https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=***'
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/download?sig=secret-token'
       )
     })
+
+    it('should mask both URLs when both are present', () => {
+      const spy = jest.spyOn(client, 'maskSigUrl')
+      const body = {
+        signed_upload_url: 'https://example.com/upload?sig=secret-token1',
+        signed_download_url: 'https://example.com/download?sig=secret-token2'
+      }
+
+      client.maskSecretUrls(body)
+
+      expect(spy).toHaveBeenCalledTimes(2)
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/upload?sig=secret-token1'
+      )
+      expect(spy).toHaveBeenCalledWith(
+        'https://example.com/download?sig=secret-token2'
+      )
+    })
+
+    it('should not call maskSigUrl when URLs are missing', () => {
+      const spy = jest.spyOn(client, 'maskSigUrl')
+      const body = {
+        key: 'test-key',
+        version: 'test-version'
+      }
+
+      client.maskSecretUrls(body)
+
+      expect(spy).not.toHaveBeenCalled()
+    })
   })
 })

packages/cache/package-lock.json

@@ -21,6 +21,7 @@
         "semver": "^6.3.1"
       },
       "devDependencies": {
+        "@types/node": "^22.13.9",
         "@types/semver": "^6.0.0",
         "typescript": "^5.2.2"
       }
@@ -324,9 +325,13 @@
       }
     },
     "node_modules/@types/node": {
-      "version": "20.4.6",
-      "resolved": "https://registry.npmjs.org/@types/node/-/node-20.4.6.tgz",
-      "integrity": "sha512-q0RkvNgMweWWIvSMDiXhflGUKMdIxBo2M2tYM/0kEGDueQByFzK4KZAgu5YHGFNxziTlppNpTIBcqHQAxlfHdA=="
+      "version": "22.13.9",
+      "resolved": "https://registry.npmjs.org/@types/node/-/node-22.13.9.tgz",
+      "integrity": "sha512-acBjXdRJ3A6Pb3tqnw9HZmyR3Fiol3aGxRCK1x3d+6CDAMjl7I649wpSd+yNURCjbOUGu9tqtLKnTGxmK6CyGw==",
+      "license": "MIT",
+      "dependencies": {
+        "undici-types": "~6.20.0"
+      }
     },
     "node_modules/@types/node-fetch": {
       "version": "2.6.4",
@@ -548,6 +553,12 @@
         "node": ">=14.17"
       }
     },
+    "node_modules/undici-types": {
+      "version": "6.20.0",
+      "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.20.0.tgz",
+      "integrity": "sha512-Ny6QZ2Nju20vw1SRHe3d9jVu6gJ+4e3+MMpqu7pqE5HT6WsTSlce++GQmK5UXS8mzV8DSYHrQH+Xrf2jVcuKNg==",
+      "license": "MIT"
+    },
     "node_modules/webidl-conversions": {
       "version": "3.0.1",
       "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz",
@@ -824,9 +835,12 @@
       }
     },
     "@types/node": {
-      "version": "20.4.6",
-      "resolved": "https://registry.npmjs.org/@types/node/-/node-20.4.6.tgz",
-      "integrity": "sha512-q0RkvNgMweWWIvSMDiXhflGUKMdIxBo2M2tYM/0kEGDueQByFzK4KZAgu5YHGFNxziTlppNpTIBcqHQAxlfHdA=="
+      "version": "22.13.9",
+      "resolved": "https://registry.npmjs.org/@types/node/-/node-22.13.9.tgz",
+      "integrity": "sha512-acBjXdRJ3A6Pb3tqnw9HZmyR3Fiol3aGxRCK1x3d+6CDAMjl7I649wpSd+yNURCjbOUGu9tqtLKnTGxmK6CyGw==",
+      "requires": {
+        "undici-types": "~6.20.0"
+      }
     },
     "@types/node-fetch": {
       "version": "2.6.4",
@@ -993,6 +1007,11 @@
       "integrity": "sha512-mI4WrpHsbCIcwT9cF4FZvr80QUeKvsUsUvKDoR+X/7XHQH98xYD8YHZg7ANtz2GtZt/CBq2QJ0thkGJMHfqc1w==",
       "dev": true
     },
+    "undici-types": {
+      "version": "6.20.0",
+      "resolved": "https://registry.npmjs.org/undici-types/-/undici-types-6.20.0.tgz",
+      "integrity": "sha512-Ny6QZ2Nju20vw1SRHe3d9jVu6gJ+4e3+MMpqu7pqE5HT6WsTSlce++GQmK5UXS8mzV8DSYHrQH+Xrf2jVcuKNg=="
+    },
     "webidl-conversions": {
       "version": "3.0.1",
       "resolved": "https://registry.npmjs.org/webidl-conversions/-/webidl-conversions-3.0.1.tgz",

packages/cache/package.json

@@ -47,10 +47,10 @@
     "@azure/ms-rest-js": "^2.6.0",
     "@azure/storage-blob": "^12.13.0",
     "@protobuf-ts/plugin": "^2.9.4",
-    "@types/node": "^22.13.9",
     "semver": "^6.3.1"
   },
   "devDependencies": {
+    "@types/node": "^22.13.9",
     "@types/semver": "^6.0.0",
     "typescript": "^5.2.2"
   }

packages/cache/src/internal/shared/cacheTwirpClient.ts

@@ -6,10 +6,6 @@ import {getRuntimeToken} from '../cacheUtils'
 import {BearerCredentialHandler} from '@actions/http-client/lib/auth'
 import {HttpClient, HttpClientResponse, HttpCodes} from '@actions/http-client'
 import {CacheServiceClientJSON} from '../../generated/results/api/v1/cache.twirp-client'
-import {
-  CreateCacheEntryResponse,
-  GetCacheEntryDownloadURLResponse
-} from '../../generated/results/api/v1/cache'
 
 // The twirp http client must implement this interface
 interface Rpc {
@@ -156,24 +152,35 @@ export class CacheServiceClient implements Rpc {
   /**
    * Masks the `sig` parameter in a URL and sets it as a secret.
    * @param url The URL containing the `sig` parameter.
-   * @param urlType The type of the URL (e.g., 'signed_upload_url', 'signed_download_url').
+   * @returns A masked URL where the sig parameter value is replaced with '***' if found,
+   *          or the original URL if no sig parameter is present.
    */
-  maskSigUrl(url: string, urlType: string): void {
-    const sigMatch = url.match(/[?&]sig=([^&]+)/)
-    if (sigMatch) {
-      setSecret(sigMatch[1])
-      debug(`Masked ${urlType}: ${url.replace(sigMatch[1], '***')}`)
+  maskSigUrl(url: string): string {
+    const sigIndex = url.indexOf('sig=')
+    if (sigIndex !== -1) {
+      const sigValue = url.substring(sigIndex + 4)
+      setSecret(sigValue)
+      return `${url.substring(0, sigIndex + 4)}***`
     }
+    return url
   }
 
-  maskSecretUrls(
-    body: CreateCacheEntryResponse | GetCacheEntryDownloadURLResponse
-  ): void {
-    if ('signedUploadUrl' in body && body.signedUploadUrl) {
-      this.maskSigUrl(body.signedUploadUrl, 'signed_upload_url')
-    }
-    if ('signedDownloadUrl' in body && body.signedDownloadUrl) {
-      this.maskSigUrl(body.signedDownloadUrl, 'signed_download_url')
+  maskSecretUrls(body): void {
+    if (typeof body === 'object' && body !== null) {
+      if (
+        'signed_upload_url' in body &&
+        typeof body.signed_upload_url === 'string'
+      ) {
+        this.maskSigUrl(body.signed_upload_url)
+      }
+      if (
+        'signed_download_url' in body &&
+        typeof body.signed_download_url === 'string'
+      ) {
+        this.maskSigUrl(body.signed_download_url)
+      }
+    } else {
+      debug('body is not an object or is null')
     }
   }