diff --git a/__tests__/artifactTwirpClient.test.ts b/__tests__/artifactTwirpClient.test.ts index 882c7ef..18c7218 100644 --- a/__tests__/artifactTwirpClient.test.ts +++ b/__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¶m2=value' + + const maskedUrl = client.maskSigUrl(url) + + expect(setSecret).toHaveBeenCalledWith('secret-token¶m2=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' ) }) }) diff --git a/src/internal/shared/artifact-twirp-client.ts b/src/internal/shared/artifact-twirp-client.ts index 2d38911..57a73ad 100644 --- a/src/internal/shared/artifact-twirp-client.ts +++ b/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') } }