From 113acfed4d3af0e1997b80a207fd972955b671d1 Mon Sep 17 00:00:00 2001 From: merlinsantiago982-cmd Date: Tue, 30 Jun 2026 10:26:52 +0800 Subject: [PATCH 1/2] block artifact download redirects --- src/lib/bundle.test.ts | 16 ++++++++++++++++ src/lib/bundle.ts | 2 +- 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/src/lib/bundle.test.ts b/src/lib/bundle.test.ts index 7319322..c9e363d 100644 --- a/src/lib/bundle.test.ts +++ b/src/lib/bundle.test.ts @@ -667,6 +667,22 @@ describe('streamUrlToFile retry', () => { expect(calls).toBe(1); }); + it('disables automatic redirects so unsafe redirect targets cannot bypass URL validation', async () => { + const dir = mkdtempSync(join(tmpdir(), 'stream-test-')); + const dest = join(dir, 'out.bin'); + const redirects: Array = []; + const fetchImpl = async (_url: Parameters[0], init?: RequestInit) => { + redirects.push(init?.redirect); + return new Response('hello', { status: 200 }); + }; + + await streamUrlToFile('https://example.com/x', dest, fetchImpl as typeof globalThis.fetch, { + sleep: noSleep, + }); + + expect(redirects).toEqual(['error']); + }); + it('sleeps between retries', async () => { const sleepDelays: number[] = []; const fetchImpl = async () => { diff --git a/src/lib/bundle.ts b/src/lib/bundle.ts index ff64f66..1a5c56a 100644 --- a/src/lib/bundle.ts +++ b/src/lib/bundle.ts @@ -715,7 +715,7 @@ export async function streamUrlToFile( for (let attempt = 1; attempt <= STREAM_URL_MAX_RETRIES; attempt++) { let response: Response; try { - response = await fetchImpl(url); + response = await fetchImpl(url, { redirect: 'error' }); } catch (err) { const message = err instanceof Error ? err.message : String(err); if (attempt < STREAM_URL_MAX_RETRIES) { From bded4991b4b0cfacb4af5b16fb88dd9952168b0e Mon Sep 17 00:00:00 2001 From: merlinsantiago982-cmd Date: Tue, 30 Jun 2026 11:10:10 +0800 Subject: [PATCH 2/2] redact artifact download urls --- src/lib/bundle.test.ts | 40 ++++++++++++++++++++++++++++++---------- src/lib/bundle.ts | 18 ++++++++++++++---- 2 files changed, 44 insertions(+), 14 deletions(-) diff --git a/src/lib/bundle.test.ts b/src/lib/bundle.test.ts index c9e363d..99773a4 100644 --- a/src/lib/bundle.test.ts +++ b/src/lib/bundle.test.ts @@ -636,17 +636,24 @@ describe('streamUrlToFile retry', () => { calls++; throw new Error('ENETUNREACH dns lookup failed'); }; - await expect( - streamUrlToFile( - 'https://example.com/x', + let caught: unknown; + + try { + await streamUrlToFile( + 'https://example.com/x?X-Amz-Signature=secret-token', '/tmp/will-not-be-written', fetchImpl as typeof globalThis.fetch, { sleep: noSleep }, - ), - ).rejects.toMatchObject({ + ); + } catch (err) { + caught = err; + } + + expect(caught).toMatchObject({ name: 'TransportError', message: expect.stringContaining('ENETUNREACH'), }); + expect(caught).not.toMatchObject({ message: expect.stringContaining('secret-token') }); expect(calls).toBe(STREAM_URL_MAX_RETRIES); }); @@ -656,14 +663,27 @@ describe('streamUrlToFile retry', () => { calls++; return new Response('Forbidden', { status: 403 }); }; - await expect( - streamUrlToFile( - 'https://example.com/x', + let caught: unknown; + const presignedUrl = 'https://example.com/x?X-Amz-Signature=secret-token#download'; + + try { + await streamUrlToFile( + presignedUrl, '/tmp/will-not-be-written', fetchImpl as typeof globalThis.fetch, { sleep: noSleep }, - ), - ).rejects.toMatchObject({ code: 'UNAVAILABLE' }); + ); + } catch (err) { + caught = err; + } + + expect(caught).toMatchObject({ + code: 'UNAVAILABLE', + details: { status: 403, artifactUrl: 'https://example.com/x' }, + }); + const details = (caught as { details?: Record }).details; + expect(details).not.toHaveProperty('url'); + expect(JSON.stringify(details)).not.toContain('secret-token'); expect(calls).toBe(1); }); diff --git a/src/lib/bundle.ts b/src/lib/bundle.ts index 1a5c56a..44d7661 100644 --- a/src/lib/bundle.ts +++ b/src/lib/bundle.ts @@ -712,6 +712,7 @@ export async function streamUrlToFile( deps?: { sleep?: (ms: number) => Promise }, ): Promise { const sleepFn = deps?.sleep ?? ((ms: number) => new Promise(r => setTimeout(r, ms))); + const artifactUrl = redactArtifactUrlForDetails(url); for (let attempt = 1; attempt <= STREAM_URL_MAX_RETRIES; attempt++) { let response: Response; try { @@ -722,7 +723,7 @@ export async function streamUrlToFile( await sleepFn(STREAM_URL_RETRY_DELAY_MS); continue; } - throw new TransportError(`Failed to download presigned URL ${url}: ${message}`); + throw new TransportError(`Failed to download presigned URL ${artifactUrl}: ${message}`); } if (!response.ok) { // Non-2xx: the URL itself is bad (expired, unauthorized, not found). @@ -734,7 +735,7 @@ export async function streamUrlToFile( nextAction: 'Re-run `testsprite test failure get`. Presigned URLs in the bundle expire after 15 minutes.', requestId: 'local', - details: { status: response.status, url }, + details: { status: response.status, artifactUrl }, }, }); } @@ -754,7 +755,7 @@ export async function streamUrlToFile( await sleepFn(STREAM_URL_RETRY_DELAY_MS); continue; } - throw new TransportError(`Failed to download presigned URL ${url}: ${message}`); + throw new TransportError(`Failed to download presigned URL ${artifactUrl}: ${message}`); } } await mkdir(dirname(filePath), { recursive: true }); @@ -776,11 +777,20 @@ export async function streamUrlToFile( await sleepFn(STREAM_URL_RETRY_DELAY_MS); continue; } - throw new TransportError(`Failed mid-download of ${url}: ${message}`); + throw new TransportError(`Failed mid-download of ${artifactUrl}: ${message}`); } } } +function redactArtifactUrlForDetails(url: string): string { + try { + const parsed = new URL(url); + return `${parsed.origin}${parsed.pathname}`; + } catch { + return ''; + } +} + function isPresignedUrl(value: string): boolean { return value.startsWith('https://'); }