diff --git a/packages/artifact/__tests__/artifactTwirpClient.test.ts b/packages/artifact/__tests__/artifactTwirpClient.test.ts new file mode 100644 index 00000000..035031e1 --- /dev/null +++ b/packages/artifact/__tests__/artifactTwirpClient.test.ts @@ -0,0 +1,84 @@ +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' + +jest.mock('@actions/core', () => ({ + setSecret: jest.fn(), + info: jest.fn(), + debug: jest.fn() +})) + +describe('ArtifactHttpClient', () => { + let client: ArtifactHttpClient + + beforeEach(() => { + jest.clearAllMocks() + process.env['ACTIONS_RUNTIME_TOKEN'] = 'test-token' + process.env['ACTIONS_RESULTS_URL'] = 'https://example.com' + client = new ArtifactHttpClient('test-agent') + }) + + afterEach(() => { + delete process.env['ACTIONS_RUNTIME_TOKEN'] + delete process.env['ACTIONS_RESULTS_URL'] + }) + + describe('maskSecretUrls', () => { + it('should mask signed_upload_url', () => { + const response: CreateArtifactResponse = { + ok: true, + signedUploadUrl: '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=***') + }) + + it('should mask 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_download_url: https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=***') + }) + + it('should not call setSecret if URLs are missing', () => { + const response = {} as CreateArtifactResponse + + client.maskSecretUrls(response) + + expect(setSecret).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' + } + + 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=***') + }) + + 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_download_url: https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=***') + }) + }) +}) diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts index 00c65bc7..1d3e8c69 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -1,10 +1,14 @@ import {HttpClient, HttpClientResponse, HttpCodes} from '@actions/http-client' import {BearerCredentialHandler} from '@actions/http-client/lib/auth' -import {info, debug} from '@actions/core' +import {setSecret, info, debug, maskSigUrl} from '@actions/core' 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 { @@ -16,7 +20,7 @@ interface Rpc { ): Promise } -class ArtifactHttpClient implements Rpc { +export class ArtifactHttpClient implements Rpc { private httpClient: HttpClient private baseUrl: string private maxAttempts = 5 @@ -70,6 +74,17 @@ class ArtifactHttpClient implements Rpc { } } + maskSecretUrls( + body: CreateArtifactResponse | GetSignedArtifactURLResponse + ): void { + if ('signedUploadUrl' in body && body.signedUploadUrl) { + maskSigUrl(body.signedUploadUrl, 'signed_upload_url') + } + if ('signedUrl' in body && body.signedUrl) { + maskSigUrl(body.signedUrl, 'signed_url') + } + } + async retryableRequest( operation: () => Promise ): Promise<{response: HttpClientResponse; body: object}> { @@ -86,6 +101,7 @@ class ArtifactHttpClient implements Rpc { debug(`[Response] - ${response.message.statusCode}`) debug(`Headers: ${JSON.stringify(response.message.headers, null, 2)}`) const body = JSON.parse(rawBody) + this.maskSecretUrls(body) debug(`Body: ${JSON.stringify(body, null, 2)}`) if (this.isSuccessStatusCode(statusCode)) { return {response, body} diff --git a/packages/cache/__tests__/cacheTwirpClient.test.ts b/packages/cache/__tests__/cacheTwirpClient.test.ts new file mode 100644 index 00000000..dd38e8fe --- /dev/null +++ b/packages/cache/__tests__/cacheTwirpClient.test.ts @@ -0,0 +1,98 @@ +import { + CreateCacheEntryResponse, + GetCacheEntryDownloadURLResponse +} from '../src/generated/results/api/v1/cache' +import {CacheServiceClient} from '../src/internal/shared/cacheTwirpClient' +import {setSecret, debug} from '@actions/core' + +jest.mock('@actions/core', () => ({ + setSecret: jest.fn(), + info: jest.fn(), + debug: jest.fn() +})) + +describe('CacheServiceClient', () => { + let client: CacheServiceClient + + beforeEach(() => { + jest.clearAllMocks() + process.env['ACTIONS_RUNTIME_TOKEN'] = 'test-token' // <-- set the required env variable + client = new CacheServiceClient('test-agent') + }) + + afterEach(() => { + delete process.env['ACTIONS_RUNTIME_TOKEN'] // <-- clean up after tests + }) + + 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 + + client.maskSecretUrls(response) + + expect(setSecret).toHaveBeenCalledWith('secret-token') + expect(debug).toHaveBeenCalledWith( + 'Masked signedUploadUrl: 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 + + client.maskSecretUrls(response) + + expect(setSecret).toHaveBeenCalledWith('secret-token') + expect(debug).toHaveBeenCalledWith( + 'Masked signedDownloadUrl: 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) + + expect(setSecret).not.toHaveBeenCalled() + }) + + 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 + + client.maskSecretUrls(response) + + expect(setSecret).toHaveBeenCalledWith('secret-token') + expect(debug).toHaveBeenCalledWith( + 'Masked signedUploadUrl: https://example.com/upload?se=2025-03-05T16%3A47%3A23Z&sig=***' + ) + }) + + 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 + + client.maskSecretUrls(response) + + expect(setSecret).toHaveBeenCalledWith('secret-token') + expect(debug).toHaveBeenCalledWith( + 'Masked signedDownloadUrl: https://example.com/download?se=2025-03-05T16%3A47%3A23Z&sig=***' + ) + }) + }) +}) diff --git a/packages/cache/__tests__/saveCacheV2.test.ts b/packages/cache/__tests__/saveCacheV2.test.ts index e96c2ac9..fed2c2a4 100644 --- a/packages/cache/__tests__/saveCacheV2.test.ts +++ b/packages/cache/__tests__/saveCacheV2.test.ts @@ -8,10 +8,16 @@ import * as tar from '../src/internal/tar' import {CacheServiceClientJSON} from '../src/generated/results/api/v1/cache.twirp-client' import * as cacheHttpClient from '../src/internal/cacheHttpClient' import {UploadOptions} from '../src/options' +import { + CreateCacheEntryResponse, + GetCacheEntryDownloadURLResponse +} from '../src/generated/results/api/v1/cache' +import {CacheServiceClient} from '../src/internal/shared/cacheTwirpClient' let logDebugMock: jest.SpyInstance jest.mock('../src/internal/tar') +jest.mock('@actions/core') const uploadFileMock = jest.fn() const blockBlobClientMock = jest.fn().mockImplementation(() => ({ diff --git a/packages/cache/src/internal/shared/cacheTwirpClient.ts b/packages/cache/src/internal/shared/cacheTwirpClient.ts index 69d9a8fc..9c121121 100644 --- a/packages/cache/src/internal/shared/cacheTwirpClient.ts +++ b/packages/cache/src/internal/shared/cacheTwirpClient.ts @@ -1,4 +1,4 @@ -import {info, debug} from '@actions/core' +import {info, debug, maskSigUrl} from '@actions/core' import {getUserAgentString} from './user-agent' import {NetworkError, UsageError} from './errors' import {getCacheServiceURL} from '../config' @@ -6,6 +6,10 @@ 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 { @@ -24,7 +28,7 @@ interface Rpc { * * This class is used to interact with cache service v2. */ -class CacheServiceClient implements Rpc { +export class CacheServiceClient implements Rpc { private httpClient: HttpClient private baseUrl: string private maxAttempts = 5 @@ -94,6 +98,7 @@ class CacheServiceClient implements Rpc { debug(`[Response] - ${response.message.statusCode}`) debug(`Headers: ${JSON.stringify(response.message.headers, null, 2)}`) const body = JSON.parse(rawBody) + this.maskSecretUrls(body) debug(`Body: ${JSON.stringify(body, null, 2)}`) if (this.isSuccessStatusCode(statusCode)) { return {response, body} @@ -148,6 +153,17 @@ class CacheServiceClient implements Rpc { throw new Error(`Request failed`) } + maskSecretUrls( + body: CreateCacheEntryResponse | GetCacheEntryDownloadURLResponse + ): void { + if ('signedUploadUrl' in body && body.signedUploadUrl) { + maskSigUrl(body.signedUploadUrl, 'signedUploadUrl') + } + if ('signedDownloadUrl' in body && body.signedDownloadUrl) { + maskSigUrl(body.signedDownloadUrl, 'signedDownloadUrl') + } + } + isSuccessStatusCode(statusCode?: number): boolean { if (!statusCode) return false return statusCode >= 200 && statusCode < 300 diff --git a/packages/core/src/core.ts b/packages/core/src/core.ts index 0a141693..12359d0a 100644 --- a/packages/core/src/core.ts +++ b/packages/core/src/core.ts @@ -391,3 +391,19 @@ export {toPosixPath, toWin32Path, toPlatformPath} from './path-utils' * Platform utilities exports */ export * as platform from './platform' + +/** + * 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 The URL with the `sig` parameter masked. + */ +export function maskSigUrl(url: string, urlType: string): string { + const sigMatch = url.match(/[?&]sig=([^&]+)/) + if (sigMatch) { + setSecret(sigMatch[1]) + debug(`Masked ${urlType}: ${url.replace(sigMatch[1], '***')}`) + return url.replace(sigMatch[1], '***') + } + return url +}