Skip to content

Commit fc48266

Browse files
salmanmkcsalmanmkc
committed
PR feedback, back to simplified approach, no export on client as well
1 parent abd9054 commit fc48266

File tree

6 files changed

+37
-633
lines changed

6 files changed

+37
-633
lines changed

‎packages/artifact/__tests__/util.test.ts

Lines changed: 1 addition & 179 deletions
Original file line numberDiff line numberDiff line change
@@ -69,14 +69,6 @@ describe('maskSigUrl', () => {
6969
jest.clearAllMocks()
7070
})
7171

72-
it('masks the sig parameter in the URL and sets it as a secret', () => {
73-
const url = 'https://example.com?sig=12345'
74-
const maskedUrl = maskSigUrl(url)
75-
expect(maskedUrl).toBe('https://example.com/?sig=***')
76-
expect(setSecret).toHaveBeenCalledWith('12345')
77-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
78-
})
79-
8072
it('returns the original URL if no sig parameter is present', () => {
8173
const url = 'https://example.com'
8274
const maskedUrl = maskSigUrl(url)
@@ -85,7 +77,7 @@ describe('maskSigUrl', () => {
8577
})
8678

8779
it('masks the sig parameter in the middle of the URL and sets it as a secret', () => {
88-
const url = 'https://example.com?param1=value1&sig=12345&param2=value2'
80+
const url = 'https://example.com/?param1=value1&sig=12345&param2=value2'
8981
const maskedUrl = maskSigUrl(url)
9082
expect(maskedUrl).toBe(
9183
'https://example.com/?param1=value1&sig=***&param2=value2'
@@ -101,112 +93,13 @@ describe('maskSigUrl', () => {
10193
expect(setSecret).not.toHaveBeenCalled()
10294
})
10395

104-
it('handles URLs with special characters in signature', () => {
105-
const url = 'https://example.com?sig=abc/+=%3D'
106-
const maskedUrl = maskSigUrl(url)
107-
expect(maskedUrl).toBe('https://example.com/?sig=***')
108-
109-
expect(setSecret).toHaveBeenCalledWith('abc/+')
110-
expect(setSecret).toHaveBeenCalledWith('abc/ ==')
111-
expect(setSecret).toHaveBeenCalledWith('abc%2F%20%3D%3D')
112-
})
113-
114-
it('handles relative URLs', () => {
115-
const url = '/path?param=value&sig=12345'
116-
const maskedUrl = maskSigUrl(url)
117-
expect(maskedUrl).toBe('https://example.com/path?param=value&sig=***')
118-
expect(setSecret).toHaveBeenCalledWith('12345')
119-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
120-
})
121-
122-
it('handles URLs with uppercase SIG parameter', () => {
123-
const url = 'https://example.com?SIG=12345'
124-
const maskedUrl = maskSigUrl(url)
125-
expect(maskedUrl).toBe('https://example.com/?SIG=***')
126-
expect(setSecret).toHaveBeenCalledWith('12345')
127-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
128-
})
129-
130-
it('handles malformed URLs using regex fallback', () => {
131-
const url = 'not:a:valid:url:but:has:sig=12345'
132-
const maskedUrl = maskSigUrl(url)
133-
expect(maskedUrl).toBe('not:a:valid:url:but:has:sig=***')
134-
expect(setSecret).toHaveBeenCalledWith('12345')
135-
})
136-
13796
it('handles URLs with fragments', () => {
13897
const url = 'https://example.com?sig=12345#fragment'
13998
const maskedUrl = maskSigUrl(url)
14099
expect(maskedUrl).toBe('https://example.com/?sig=***#fragment')
141100
expect(setSecret).toHaveBeenCalledWith('12345')
142101
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
143102
})
144-
145-
it('handles URLs with Sig parameter (first letter uppercase)', () => {
146-
const url = 'https://example.com?Sig=12345'
147-
const maskedUrl = maskSigUrl(url)
148-
expect(maskedUrl).toBe('https://example.com/?Sig=***')
149-
expect(setSecret).toHaveBeenCalledWith('12345')
150-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
151-
})
152-
153-
it('handles URLs with sIg parameter (middle letter uppercase)', () => {
154-
const url = 'https://example.com?sIg=12345'
155-
const maskedUrl = maskSigUrl(url)
156-
expect(maskedUrl).toBe('https://example.com/?sIg=***')
157-
expect(setSecret).toHaveBeenCalledWith('12345')
158-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
159-
})
160-
161-
it('handles URLs with siG parameter (last letter uppercase)', () => {
162-
const url = 'https://example.com?siG=12345'
163-
const maskedUrl = maskSigUrl(url)
164-
expect(maskedUrl).toBe('https://example.com/?siG=***')
165-
expect(setSecret).toHaveBeenCalledWith('12345')
166-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
167-
})
168-
169-
it('handles URLs with mixed case sig parameters in the same URL', () => {
170-
const url = 'https://example.com?sig=123&SIG=456&Sig=789'
171-
const maskedUrl = maskSigUrl(url)
172-
expect(maskedUrl).toBe('https://example.com/?sig=***&SIG=***&Sig=***')
173-
expect(setSecret).toHaveBeenCalledWith('123')
174-
expect(setSecret).toHaveBeenCalledWith('456')
175-
expect(setSecret).toHaveBeenCalledWith('789')
176-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('123'))
177-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('456'))
178-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('789'))
179-
})
180-
181-
it('handles malformed URLs with different sig case variations', () => {
182-
const url = 'not:a:valid:url:but:has:Sig=12345'
183-
const maskedUrl = maskSigUrl(url)
184-
expect(maskedUrl).toBe('not:a:valid:url:but:has:Sig=***')
185-
expect(setSecret).toHaveBeenCalledWith('12345')
186-
})
187-
188-
it('handles malformed URLs with uppercase SIG in irregular formats', () => {
189-
const url = 'something&SIG=12345&other:stuff'
190-
const maskedUrl = maskSigUrl(url)
191-
expect(maskedUrl).toBe('something&SIG=***&other:stuff')
192-
expect(setSecret).toHaveBeenCalledWith('12345')
193-
})
194-
195-
it('handles sig parameter at the start of the query string', () => {
196-
const url = 'https://example.com?sig=12345&param=value'
197-
const maskedUrl = maskSigUrl(url)
198-
expect(maskedUrl).toBe('https://example.com/?sig=***&param=value')
199-
expect(setSecret).toHaveBeenCalledWith('12345')
200-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
201-
})
202-
203-
it('handles sig parameter at the end of the query string', () => {
204-
const url = 'https://example.com?param=value&sig=12345'
205-
const maskedUrl = maskSigUrl(url)
206-
expect(maskedUrl).toBe('https://example.com/?param=value&sig=***')
207-
expect(setSecret).toHaveBeenCalledWith('12345')
208-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('12345'))
209-
})
210103
})
211104

212105
describe('maskSecretUrls', () => {
@@ -262,77 +155,6 @@ describe('maskSecretUrls', () => {
262155
expect(setSecret).not.toHaveBeenCalled()
263156
})
264157

265-
it('handles malformed URLs in the body', () => {
266-
const body = {
267-
signed_upload_url: 'not:a:valid:url:but:has:sig=upload123',
268-
signed_url: 'also:not:valid:with:sig=download123'
269-
}
270-
maskSecretUrls(body)
271-
expect(setSecret).toHaveBeenCalledWith('upload123')
272-
expect(setSecret).toHaveBeenCalledWith('download123')
273-
})
274-
275-
it('handles URLs with different case variations of sig parameter', () => {
276-
const body = {
277-
signed_upload_url: 'https://upload.com?SIG=upload123',
278-
signed_url: 'https://download.com?Sig=download123'
279-
}
280-
maskSecretUrls(body)
281-
expect(setSecret).toHaveBeenCalledWith('upload123')
282-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('upload123'))
283-
expect(setSecret).toHaveBeenCalledWith('download123')
284-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('download123'))
285-
})
286-
287-
it('handles URLs with special characters in signature', () => {
288-
const specialSig = 'xyz!@#$'
289-
const encodedSpecialSig = encodeURIComponent(specialSig)
290-
291-
const body = {
292-
signed_upload_url: 'https://upload.com?sig=abc/+=%3D',
293-
signed_url: `https://download.com?sig=${encodedSpecialSig}`
294-
}
295-
maskSecretUrls(body)
296-
297-
const allCalls = (setSecret as jest.Mock).mock.calls.flat()
298-
299-
expect(allCalls).toContain('abc/+')
300-
expect(allCalls).toContain('abc/ ==')
301-
expect(allCalls).toContain('abc%2F%20%3D%3D')
302-
303-
expect(allCalls).toContain(specialSig)
304-
})
305-
306-
it('handles deeply nested URLs in the body', () => {
307-
const body = {
308-
data: {
309-
urls: {
310-
signed_upload_url: 'https://upload.com?sig=nested123',
311-
signed_url: 'https://download.com?sig=nested456'
312-
}
313-
}
314-
}
315-
maskSecretUrls(body)
316-
expect(setSecret).toHaveBeenCalledWith('nested123')
317-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('nested123'))
318-
expect(setSecret).toHaveBeenCalledWith('nested456')
319-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('nested456'))
320-
})
321-
322-
it('handles arrays of URLs in the body', () => {
323-
const body = {
324-
signed_urls: [
325-
'https://first.com?sig=first123',
326-
'https://second.com?sig=second456'
327-
]
328-
}
329-
maskSecretUrls(body)
330-
expect(setSecret).toHaveBeenCalledWith('first123')
331-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('first123'))
332-
expect(setSecret).toHaveBeenCalledWith('second456')
333-
expect(setSecret).toHaveBeenCalledWith(encodeURIComponent('second456'))
334-
})
335-
336158
it('does nothing if body is not an object or is null', () => {
337159
maskSecretUrls(null)
338160
expect(debug).toHaveBeenCalledWith('body is not an object or is null')

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@ interface Rpc {
1717
): Promise<object | Uint8Array>
1818
}
1919

20-
export class ArtifactHttpClient implements Rpc {
20+
class ArtifactHttpClient implements Rpc {
2121
private httpClient: HttpClient
2222
private baseUrl: string
2323
private maxAttempts = 5

‎packages/artifact/src/internal/shared/util.ts

Lines changed: 15 additions & 133 deletions
Original file line numberDiff line numberDiff line change
@@ -81,159 +81,41 @@ export function maskSigUrl(url: string): string {
8181
if (!url) return url
8282

8383
try {
84-
const rawSigRegex = /[?&](sig)=([^&=#]+)/gi
85-
let match
86-
87-
while ((match = rawSigRegex.exec(url)) !== null) {
88-
const rawSignature = match[2]
89-
if (rawSignature) {
90-
setSecret(rawSignature)
91-
}
92-
}
93-
94-
let parsedUrl: URL
95-
try {
96-
parsedUrl = new URL(url)
97-
} catch {
98-
try {
99-
parsedUrl = new URL(url, 'https://example.com')
100-
} catch (error) {
101-
debug(`Failed to parse URL: ${url}`)
102-
return maskSigWithRegex(url)
103-
}
104-
}
84+
const parsedUrl = new URL(url)
85+
const signature = parsedUrl.searchParams.get('sig')
10586

106-
let masked = false
107-
const paramNames = Array.from(parsedUrl.searchParams.keys())
108-
109-
for (const paramName of paramNames) {
110-
if (paramName.toLowerCase() === 'sig') {
111-
const signature = parsedUrl.searchParams.get(paramName)
112-
if (signature) {
113-
setSecret(signature)
114-
setSecret(encodeURIComponent(signature))
115-
parsedUrl.searchParams.set(paramName, '***')
116-
masked = true
117-
}
118-
}
119-
}
120-
if (masked) {
87+
if (signature) {
88+
setSecret(signature)
89+
setSecret(encodeURIComponent(signature))
90+
parsedUrl.searchParams.set('sig', '***')
12191
return parsedUrl.toString()
12292
}
123-
124-
if (/([:?&]|^)(sig)=([^&=#]+)/i.test(url)) {
125-
return maskSigWithRegex(url)
126-
}
12793
} catch (error) {
12894
debug(
129-
`Error masking URL: ${
95+
`Failed to parse URL: ${url} ${
13096
error instanceof Error ? error.message : String(error)
13197
}`
13298
)
133-
return maskSigWithRegex(url)
13499
}
135-
136100
return url
137101
}
138102

139-
/**
140-
* Fallback method to mask signatures using regex when URL parsing fails
141-
*/
142-
function maskSigWithRegex(url: string): string {
143-
try {
144-
const regex = /([:?&]|^)(sig)=([^&=#]+)/gi
145-
146-
return url.replace(regex, (fullMatch, prefix, paramName, value) => {
147-
if (value) {
148-
setSecret(value)
149-
try {
150-
setSecret(decodeURIComponent(value))
151-
} catch {
152-
// Ignore decoding errors
153-
}
154-
return `${prefix}${paramName}=***`
155-
}
156-
return fullMatch
157-
})
158-
} catch (error) {
159-
debug(
160-
`Error in maskSigWithRegex: ${
161-
error instanceof Error ? error.message : String(error)
162-
}`
163-
)
164-
return url
165-
}
166-
}
167-
168103
/**
169104
* Masks any URLs containing signature parameters in the provided object
170-
* Recursively searches through nested objects and arrays
171105
*/
172-
export function maskSecretUrls(
173-
body: Record<string, unknown> | unknown[] | null
174-
): void {
106+
export function maskSecretUrls(body: Record<string, unknown> | null): void {
175107
if (typeof body !== 'object' || body === null) {
176108
debug('body is not an object or is null')
177109
return
178110
}
179111

180-
type NestedValue =
181-
| string
182-
| number
183-
| boolean
184-
| null
185-
| undefined
186-
| NestedObject
187-
| NestedArray
188-
interface NestedObject {
189-
[key: string]: NestedValue
190-
signed_upload_url?: string
191-
signed_url?: string
112+
if (
113+
'signed_upload_url' in body &&
114+
typeof body.signed_upload_url === 'string'
115+
) {
116+
maskSigUrl(body.signed_upload_url)
192117
}
193-
type NestedArray = NestedValue[]
194-
195-
const processUrl = (url: string): void => {
196-
maskSigUrl(url)
197-
}
198-
199-
const processObject = (
200-
obj: Record<string, NestedValue> | NestedValue[]
201-
): void => {
202-
if (typeof obj !== 'object' || obj === null) {
203-
return
204-
}
205-
206-
if (Array.isArray(obj)) {
207-
for (const item of obj) {
208-
if (typeof item === 'string') {
209-
processUrl(item)
210-
} else if (typeof item === 'object' && item !== null) {
211-
processObject(item as Record<string, NestedValue> | NestedValue[])
212-
}
213-
}
214-
return
215-
}
216-
217-
if (
218-
'signed_upload_url' in obj &&
219-
typeof obj.signed_upload_url === 'string'
220-
) {
221-
maskSigUrl(obj.signed_upload_url)
222-
}
223-
if ('signed_url' in obj && typeof obj.signed_url === 'string') {
224-
maskSigUrl(obj.signed_url)
225-
}
226-
227-
for (const key in obj) {
228-
const value = obj[key]
229-
if (typeof value === 'string') {
230-
if (/([:?&]|^)(sig)=/i.test(value)) {
231-
maskSigUrl(value)
232-
}
233-
} else if (typeof value === 'object' && value !== null) {
234-
processObject(value as Record<string, NestedValue> | NestedValue[])
235-
}
236-
}
118+
if ('signed_url' in body && typeof body.signed_url === 'string') {
119+
maskSigUrl(body.signed_url)
237120
}
238-
processObject(body as Record<string, NestedValue> | NestedValue[])
239121
}

0 commit comments

Comments
 (0)