From fc482662af1c4982242f044418f68bf6275bbfb2 Mon Sep 17 00:00:00 2001 From: Salman Chishti Date: Thu, 13 Mar 2025 04:23:45 -0700 Subject: [PATCH] PR feedback, back to simplified approach, no export on client as well --- packages/artifact/__tests__/util.test.ts | 180 +----------------- .../internal/shared/artifact-twirp-client.ts | 2 +- packages/artifact/src/internal/shared/util.ts | 148 ++------------ packages/cache/__tests__/util.test.ts | 180 +----------------- .../src/internal/shared/cacheTwirpClient.ts | 2 +- packages/cache/src/internal/shared/util.ts | 158 ++------------- 6 files changed, 37 insertions(+), 633 deletions(-) diff --git a/packages/artifact/__tests__/util.test.ts b/packages/artifact/__tests__/util.test.ts index 34535db2..018bfc45 100644 --- a/packages/artifact/__tests__/util.test.ts +++ b/packages/artifact/__tests__/util.test.ts @@ -69,14 +69,6 @@ describe('maskSigUrl', () => { jest.clearAllMocks() }) - it('masks the sig parameter in the URL and sets it as a secret', () => { - const url = 'https://example.com?sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - it('returns the original URL if no sig parameter is present', () => { const url = 'https://example.com' const maskedUrl = maskSigUrl(url) @@ -85,7 +77,7 @@ describe('maskSigUrl', () => { }) it('masks the sig parameter in the middle of the URL and sets it as a secret', () => { - const url = 'https://example.com?param1=value1&sig=12345¶m2=value2' + const url = 'https://example.com/?param1=value1&sig=12345¶m2=value2' const maskedUrl = maskSigUrl(url) expect(maskedUrl).toBe( 'https://example.com/?param1=value1&sig=***¶m2=value2' @@ -101,39 +93,6 @@ describe('maskSigUrl', () => { expect(setSecret).not.toHaveBeenCalled() }) - it('handles URLs with special characters in signature', () => { - const url = 'https://example.com?sig=abc/+=%3D' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***') - - expect(setSecret).toHaveBeenCalledWith('abc/+') - expect(setSecret).toHaveBeenCalledWith('abc/ ==') - expect(setSecret).toHaveBeenCalledWith('abc%2F%20%3D%3D') - }) - - it('handles relative URLs', () => { - const url = '/path?param=value&sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/path?param=value&sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with uppercase SIG parameter', () => { - const url = 'https://example.com?SIG=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?SIG=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles malformed URLs using regex fallback', () => { - const url = 'not:a:valid:url:but:has:sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('not:a:valid:url:but:has:sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - }) - it('handles URLs with fragments', () => { const url = 'https://example.com?sig=12345#fragment' const maskedUrl = maskSigUrl(url) @@ -141,72 +100,6 @@ describe('maskSigUrl', () => { expect(setSecret).toHaveBeenCalledWith('12345') expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) }) - - it('handles URLs with Sig parameter (first letter uppercase)', () => { - const url = 'https://example.com?Sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?Sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with sIg parameter (middle letter uppercase)', () => { - const url = 'https://example.com?sIg=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sIg=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with siG parameter (last letter uppercase)', () => { - const url = 'https://example.com?siG=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?siG=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with mixed case sig parameters in the same URL', () => { - const url = 'https://example.com?sig=123&SIG=456&Sig=789' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***&SIG=***&Sig=***') - expect(setSecret).toHaveBeenCalledWith('123') - expect(setSecret).toHaveBeenCalledWith('456') - expect(setSecret).toHaveBeenCalledWith('789') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('123')) - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('456')) - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('789')) - }) - - it('handles malformed URLs with different sig case variations', () => { - const url = 'not:a:valid:url:but:has:Sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('not:a:valid:url:but:has:Sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - }) - - it('handles malformed URLs with uppercase SIG in irregular formats', () => { - const url = 'something&SIG=12345&other:stuff' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('something&SIG=***&other:stuff') - expect(setSecret).toHaveBeenCalledWith('12345') - }) - - it('handles sig parameter at the start of the query string', () => { - const url = 'https://example.com?sig=12345¶m=value' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***¶m=value') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles sig parameter at the end of the query string', () => { - const url = 'https://example.com?param=value&sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?param=value&sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) }) describe('maskSecretUrls', () => { @@ -262,77 +155,6 @@ describe('maskSecretUrls', () => { expect(setSecret).not.toHaveBeenCalled() }) - it('handles malformed URLs in the body', () => { - const body = { - signed_upload_url: 'not:a:valid:url:but:has:sig=upload123', - signed_url: 'also:not:valid:with:sig=download123' - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('upload123') - expect(setSecret).toHaveBeenCalledWith('download123') - }) - - it('handles URLs with different case variations of sig parameter', () => { - const body = { - signed_upload_url: 'https://upload.com?SIG=upload123', - signed_url: 'https://download.com?Sig=download123' - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('upload123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('upload123')) - expect(setSecret).toHaveBeenCalledWith('download123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('download123')) - }) - - it('handles URLs with special characters in signature', () => { - const specialSig = 'xyz!@#$' - const encodedSpecialSig = encodeURIComponent(specialSig) - - const body = { - signed_upload_url: 'https://upload.com?sig=abc/+=%3D', - signed_url: `https://download.com?sig=${encodedSpecialSig}` - } - maskSecretUrls(body) - - const allCalls = (setSecret as jest.Mock).mock.calls.flat() - - expect(allCalls).toContain('abc/+') - expect(allCalls).toContain('abc/ ==') - expect(allCalls).toContain('abc%2F%20%3D%3D') - - expect(allCalls).toContain(specialSig) - }) - - it('handles deeply nested URLs in the body', () => { - const body = { - data: { - urls: { - signed_upload_url: 'https://upload.com?sig=nested123', - signed_url: 'https://download.com?sig=nested456' - } - } - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('nested123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('nested123')) - expect(setSecret).toHaveBeenCalledWith('nested456') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('nested456')) - }) - - it('handles arrays of URLs in the body', () => { - const body = { - signed_urls: [ - 'https://first.com?sig=first123', - 'https://second.com?sig=second456' - ] - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('first123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('first123')) - expect(setSecret).toHaveBeenCalledWith('second456') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('second456')) - }) - it('does nothing if body is not an object or is null', () => { maskSecretUrls(null) expect(debug).toHaveBeenCalledWith('body is not an object or is null') diff --git a/packages/artifact/src/internal/shared/artifact-twirp-client.ts b/packages/artifact/src/internal/shared/artifact-twirp-client.ts index 84461518..57499125 100644 --- a/packages/artifact/src/internal/shared/artifact-twirp-client.ts +++ b/packages/artifact/src/internal/shared/artifact-twirp-client.ts @@ -17,7 +17,7 @@ interface Rpc { ): Promise } -export class ArtifactHttpClient implements Rpc { +class ArtifactHttpClient implements Rpc { private httpClient: HttpClient private baseUrl: string private maxAttempts = 5 diff --git a/packages/artifact/src/internal/shared/util.ts b/packages/artifact/src/internal/shared/util.ts index 754c6b76..61f2ab6a 100644 --- a/packages/artifact/src/internal/shared/util.ts +++ b/packages/artifact/src/internal/shared/util.ts @@ -81,159 +81,41 @@ export function maskSigUrl(url: string): string { if (!url) return url try { - const rawSigRegex = /[?&](sig)=([^&=#]+)/gi - let match + const parsedUrl = new URL(url) + const signature = parsedUrl.searchParams.get('sig') - while ((match = rawSigRegex.exec(url)) !== null) { - const rawSignature = match[2] - if (rawSignature) { - setSecret(rawSignature) - } - } - - let parsedUrl: URL - try { - parsedUrl = new URL(url) - } catch { - try { - parsedUrl = new URL(url, 'https://example.com') - } catch (error) { - debug(`Failed to parse URL: ${url}`) - return maskSigWithRegex(url) - } - } - - let masked = false - const paramNames = Array.from(parsedUrl.searchParams.keys()) - - for (const paramName of paramNames) { - if (paramName.toLowerCase() === 'sig') { - const signature = parsedUrl.searchParams.get(paramName) - if (signature) { - setSecret(signature) - setSecret(encodeURIComponent(signature)) - parsedUrl.searchParams.set(paramName, '***') - masked = true - } - } - } - if (masked) { + if (signature) { + setSecret(signature) + setSecret(encodeURIComponent(signature)) + parsedUrl.searchParams.set('sig', '***') return parsedUrl.toString() } - - if (/([:?&]|^)(sig)=([^&=#]+)/i.test(url)) { - return maskSigWithRegex(url) - } } catch (error) { debug( - `Error masking URL: ${ + `Failed to parse URL: ${url} ${ error instanceof Error ? error.message : String(error) }` ) - return maskSigWithRegex(url) } - return url } -/** - * Fallback method to mask signatures using regex when URL parsing fails - */ -function maskSigWithRegex(url: string): string { - try { - const regex = /([:?&]|^)(sig)=([^&=#]+)/gi - - return url.replace(regex, (fullMatch, prefix, paramName, value) => { - if (value) { - setSecret(value) - try { - setSecret(decodeURIComponent(value)) - } catch { - // Ignore decoding errors - } - return `${prefix}${paramName}=***` - } - return fullMatch - }) - } catch (error) { - debug( - `Error in maskSigWithRegex: ${ - error instanceof Error ? error.message : String(error) - }` - ) - return url - } -} - /** * Masks any URLs containing signature parameters in the provided object - * Recursively searches through nested objects and arrays */ -export function maskSecretUrls( - body: Record | unknown[] | null -): void { +export function maskSecretUrls(body: Record | null): void { if (typeof body !== 'object' || body === null) { debug('body is not an object or is null') return } - type NestedValue = - | string - | number - | boolean - | null - | undefined - | NestedObject - | NestedArray - interface NestedObject { - [key: string]: NestedValue - signed_upload_url?: string - signed_url?: string + if ( + 'signed_upload_url' in body && + typeof body.signed_upload_url === 'string' + ) { + maskSigUrl(body.signed_upload_url) } - type NestedArray = NestedValue[] - - const processUrl = (url: string): void => { - maskSigUrl(url) + if ('signed_url' in body && typeof body.signed_url === 'string') { + maskSigUrl(body.signed_url) } - - const processObject = ( - obj: Record | NestedValue[] - ): void => { - if (typeof obj !== 'object' || obj === null) { - return - } - - if (Array.isArray(obj)) { - for (const item of obj) { - if (typeof item === 'string') { - processUrl(item) - } else if (typeof item === 'object' && item !== null) { - processObject(item as Record | NestedValue[]) - } - } - return - } - - if ( - 'signed_upload_url' in obj && - typeof obj.signed_upload_url === 'string' - ) { - maskSigUrl(obj.signed_upload_url) - } - if ('signed_url' in obj && typeof obj.signed_url === 'string') { - maskSigUrl(obj.signed_url) - } - - for (const key in obj) { - const value = obj[key] - if (typeof value === 'string') { - if (/([:?&]|^)(sig)=/i.test(value)) { - maskSigUrl(value) - } - } else if (typeof value === 'object' && value !== null) { - processObject(value as Record | NestedValue[]) - } - } - } - processObject(body as Record | NestedValue[]) } diff --git a/packages/cache/__tests__/util.test.ts b/packages/cache/__tests__/util.test.ts index 41c29f4b..12cec07d 100644 --- a/packages/cache/__tests__/util.test.ts +++ b/packages/cache/__tests__/util.test.ts @@ -8,14 +8,6 @@ describe('maskSigUrl', () => { jest.clearAllMocks() }) - it('masks the sig parameter in the URL and sets it as a secret', () => { - const url = 'https://example.com?sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - it('returns the original URL if no sig parameter is present', () => { const url = 'https://example.com' const maskedUrl = maskSigUrl(url) @@ -24,7 +16,7 @@ describe('maskSigUrl', () => { }) it('masks the sig parameter in the middle of the URL and sets it as a secret', () => { - const url = 'https://example.com?param1=value1&sig=12345¶m2=value2' + const url = 'https://example.com/?param1=value1&sig=12345¶m2=value2' const maskedUrl = maskSigUrl(url) expect(maskedUrl).toBe( 'https://example.com/?param1=value1&sig=***¶m2=value2' @@ -40,39 +32,6 @@ describe('maskSigUrl', () => { expect(setSecret).not.toHaveBeenCalled() }) - it('handles URLs with special characters in signature', () => { - const url = 'https://example.com?sig=abc/+=%3D' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***') - - expect(setSecret).toHaveBeenCalledWith('abc/+') - expect(setSecret).toHaveBeenCalledWith('abc/ ==') - expect(setSecret).toHaveBeenCalledWith('abc%2F%20%3D%3D') - }) - - it('handles relative URLs', () => { - const url = '/path?param=value&sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/path?param=value&sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with uppercase SIG parameter', () => { - const url = 'https://example.com?SIG=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?SIG=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles malformed URLs using regex fallback', () => { - const url = 'not:a:valid:url:but:has:sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('not:a:valid:url:but:has:sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - }) - it('handles URLs with fragments', () => { const url = 'https://example.com?sig=12345#fragment' const maskedUrl = maskSigUrl(url) @@ -80,72 +39,6 @@ describe('maskSigUrl', () => { expect(setSecret).toHaveBeenCalledWith('12345') expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) }) - - it('handles URLs with Sig parameter (first letter uppercase)', () => { - const url = 'https://example.com?Sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?Sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with sIg parameter (middle letter uppercase)', () => { - const url = 'https://example.com?sIg=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sIg=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with siG parameter (last letter uppercase)', () => { - const url = 'https://example.com?siG=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?siG=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles URLs with mixed case sig parameters in the same URL', () => { - const url = 'https://example.com?sig=123&SIG=456&Sig=789' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***&SIG=***&Sig=***') - expect(setSecret).toHaveBeenCalledWith('123') - expect(setSecret).toHaveBeenCalledWith('456') - expect(setSecret).toHaveBeenCalledWith('789') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('123')) - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('456')) - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('789')) - }) - - it('handles malformed URLs with different sig case variations', () => { - const url = 'not:a:valid:url:but:has:Sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('not:a:valid:url:but:has:Sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - }) - - it('handles malformed URLs with uppercase SIG in irregular formats', () => { - const url = 'something&SIG=12345&other:stuff' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('something&SIG=***&other:stuff') - expect(setSecret).toHaveBeenCalledWith('12345') - }) - - it('handles sig parameter at the start of the query string', () => { - const url = 'https://example.com?sig=12345¶m=value' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?sig=***¶m=value') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) - - it('handles sig parameter at the end of the query string', () => { - const url = 'https://example.com?param=value&sig=12345' - const maskedUrl = maskSigUrl(url) - expect(maskedUrl).toBe('https://example.com/?param=value&sig=***') - expect(setSecret).toHaveBeenCalledWith('12345') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345')) - }) }) describe('maskSecretUrls', () => { @@ -201,77 +94,6 @@ describe('maskSecretUrls', () => { expect(setSecret).not.toHaveBeenCalled() }) - it('handles malformed URLs in the body', () => { - const body = { - signed_upload_url: 'not:a:valid:url:but:has:sig=upload123', - signed_download_url: 'also:not:valid:with:sig=download123' - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('upload123') - expect(setSecret).toHaveBeenCalledWith('download123') - }) - - it('handles URLs with different case variations of sig parameter', () => { - const body = { - signed_upload_url: 'https://upload.com?SIG=upload123', - signed_download_url: 'https://download.com?Sig=download123' - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('upload123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('upload123')) - expect(setSecret).toHaveBeenCalledWith('download123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('download123')) - }) - - it('handles URLs with special characters in signature', () => { - const specialSig = 'xyz!@#$' - const encodedSpecialSig = encodeURIComponent(specialSig) - - const body = { - signed_upload_url: 'https://upload.com?sig=abc/+=%3D', - signed_download_url: `https://download.com?sig=${encodedSpecialSig}` - } - maskSecretUrls(body) - - const allCalls = (setSecret as jest.Mock).mock.calls.flat() - - expect(allCalls).toContain('abc/+') - expect(allCalls).toContain('abc/ ==') - expect(allCalls).toContain('abc%2F%20%3D%3D') - - expect(allCalls).toContain(specialSig) - }) - - it('handles deeply nested URLs in the body', () => { - const body = { - data: { - urls: { - signed_upload_url: 'https://upload.com?sig=nested123', - signed_download_url: 'https://download.com?sig=nested456' - } - } - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('nested123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('nested123')) - expect(setSecret).toHaveBeenCalledWith('nested456') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('nested456')) - }) - - it('handles arrays of URLs in the body', () => { - const body = { - signed_urls: [ - 'https://first.com?sig=first123', - 'https://second.com?sig=second456' - ] - } - maskSecretUrls(body) - expect(setSecret).toHaveBeenCalledWith('first123') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('first123')) - expect(setSecret).toHaveBeenCalledWith('second456') - expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('second456')) - }) - it('does nothing if body is not an object or is null', () => { maskSecretUrls(null) expect(debug).toHaveBeenCalledWith('body is not an object or is null') diff --git a/packages/cache/src/internal/shared/cacheTwirpClient.ts b/packages/cache/src/internal/shared/cacheTwirpClient.ts index cfd01513..f6c2af61 100644 --- a/packages/cache/src/internal/shared/cacheTwirpClient.ts +++ b/packages/cache/src/internal/shared/cacheTwirpClient.ts @@ -25,7 +25,7 @@ interface Rpc { * * This class is used to interact with cache service v2. */ -export class CacheServiceClient implements Rpc { +class CacheServiceClient implements Rpc { private httpClient: HttpClient private baseUrl: string private maxAttempts = 5 diff --git a/packages/cache/src/internal/shared/util.ts b/packages/cache/src/internal/shared/util.ts index b9212d8d..b69bb18c 100644 --- a/packages/cache/src/internal/shared/util.ts +++ b/packages/cache/src/internal/shared/util.ts @@ -10,166 +10,44 @@ export function maskSigUrl(url: string): string { if (!url) return url try { - const rawSigRegex = /[?&](sig)=([^&=#]+)/gi - let match + const parsedUrl = new URL(url) + const signature = parsedUrl.searchParams.get('sig') - while ((match = rawSigRegex.exec(url)) !== null) { - const rawSignature = match[2] - if (rawSignature) { - setSecret(rawSignature) - } - } - - let parsedUrl: URL - try { - parsedUrl = new URL(url) - } catch { - try { - parsedUrl = new URL(url, 'https://example.com') - } catch (error) { - debug(`Failed to parse URL: ${url}`) - return maskSigWithRegex(url) - } - } - - let masked = false - const paramNames = Array.from(parsedUrl.searchParams.keys()) - - for (const paramName of paramNames) { - if (paramName.toLowerCase() === 'sig') { - const signature = parsedUrl.searchParams.get(paramName) - if (signature) { - setSecret(signature) - setSecret(encodeURIComponent(signature)) - parsedUrl.searchParams.set(paramName, '***') - masked = true - } - } - } - if (masked) { + if (signature) { + setSecret(signature) + setSecret(encodeURIComponent(signature)) + parsedUrl.searchParams.set('sig', '***') return parsedUrl.toString() } - - if (/([:?&]|^)(sig)=([^&=#]+)/i.test(url)) { - return maskSigWithRegex(url) - } } catch (error) { debug( - `Error masking URL: ${ + `Failed to parse URL: ${url} ${ error instanceof Error ? error.message : String(error) }` ) - return maskSigWithRegex(url) } - return url } -/** - * Fallback method to mask signatures using regex when URL parsing fails - */ -function maskSigWithRegex(url: string): string { - try { - const regex = /([:?&]|^)(sig)=([^&=#]+)/gi - - return url.replace(regex, (fullMatch, prefix, paramName, value) => { - if (value) { - setSecret(value) - try { - setSecret(decodeURIComponent(value)) - } catch (error) { - debug( - `Failed to decode URL parameter: ${ - error instanceof Error ? error.message : String(error) - }` - ) - } - return `${prefix}${paramName}=***` - } - return fullMatch - }) - } catch (error) { - debug( - `Error in maskSigWithRegex: ${ - error instanceof Error ? error.message : String(error) - }` - ) - return url - } -} - /** * Masks any URLs containing signature parameters in the provided object - * Recursively searches through nested objects and arrays */ -export function maskSecretUrls( - body: Record | unknown[] | null -): void { +export function maskSecretUrls(body: Record | null): void { if (typeof body !== 'object' || body === null) { debug('body is not an object or is null') return } - type NestedValue = - | string - | number - | boolean - | null - | undefined - | NestedObject - | NestedArray - interface NestedObject { - [key: string]: NestedValue - signed_upload_url?: string - signed_download_url?: string + if ( + 'signed_upload_url' in body && + typeof body.signed_upload_url === 'string' + ) { + maskSigUrl(body.signed_upload_url) } - type NestedArray = NestedValue[] - - const processUrl = (url: string): void => { - maskSigUrl(url) + if ( + 'signed_download_url' in body && + typeof body.signed_download_url === 'string' + ) { + maskSigUrl(body.signed_download_url) } - - const processObject = ( - obj: Record | NestedValue[] - ): void => { - if (typeof obj !== 'object' || obj === null) { - return - } - - if (Array.isArray(obj)) { - for (const item of obj) { - if (typeof item === 'string') { - processUrl(item) - } else if (typeof item === 'object' && item !== null) { - processObject(item as Record | NestedValue[]) - } - } - return - } - - if ( - 'signed_upload_url' in obj && - typeof obj.signed_upload_url === 'string' - ) { - maskSigUrl(obj.signed_upload_url) - } - if ( - 'signed_download_url' in obj && - typeof obj.signed_download_url === 'string' - ) { - maskSigUrl(obj.signed_download_url) - } - - for (const key in obj) { - const value = obj[key] - if (typeof value === 'string') { - if (/([:?&]|^)(sig)=/i.test(value)) { - maskSigUrl(value) - } - } else if (typeof value === 'object' && value !== null) { - processObject(value as Record | NestedValue[]) - } - } - } - processObject(body as Record | NestedValue[]) }