diff --git a/src/utils/api.mts b/src/utils/api.mts index 26f1085ca..7582b54a0 100644 --- a/src/utils/api.mts +++ b/src/utils/api.mts @@ -20,6 +20,7 @@ */ import { Agent as HttpsAgent, request as httpsRequest } from 'node:https' +import { ReadableStream } from 'node:stream/web' import { messageWithCauses } from 'pony-cause' @@ -72,9 +73,11 @@ function getHttpsAgent(): HttpsAgent | undefined { return _httpsAgent } -// Wrapper around fetch that supports extra CA certificates via SSL_CERT_FILE. -// Uses node:https.request with a custom agent when extra CA certs are needed, -// falling back to regular fetch() otherwise. Follows redirects like fetch(). +// All outbound API requests use node:https.request rather than global fetch. +// This ensures no body timeout is applied — large streaming ND-JSON responses +// (e.g. full scan results) can transfer without a hard deadline. When +// SSL_CERT_FILE is configured, a custom HttpsAgent carrying the extra CA +// certificates is passed; otherwise the default agent is used. export type ApiFetchInit = { body?: string | undefined headers?: Record | undefined @@ -85,7 +88,7 @@ export type ApiFetchInit = { function _httpsRequestFetch( url: string, init: ApiFetchInit, - agent: HttpsAgent, + agent: HttpsAgent | undefined, redirectCount: number, ): Promise { return new Promise((resolve, reject) => { @@ -149,29 +152,44 @@ function _httpsRequestFetch( ) return } - const chunks: Buffer[] = [] - res.on('data', (chunk: Buffer) => chunks.push(chunk)) - res.on('end', () => { - const body = Buffer.concat(chunks) - const responseHeaders = new Headers() - for (const [key, value] of Object.entries(res.headers)) { - if (typeof value === 'string') { - responseHeaders.set(key, value) - } else if (Array.isArray(value)) { - for (const v of value) { - responseHeaders.append(key, v) - } + // Build response headers immediately on receipt. + const responseHeaders = new Headers() + for (const [key, value] of Object.entries(res.headers)) { + if (typeof value === 'string') { + responseHeaders.set(key, value) + } else if (Array.isArray(value)) { + for (const v of value) { + responseHeaders.append(key, v) } } - resolve( - new Response(body, { - status: statusCode ?? 0, - statusText: res.statusMessage ?? '', - headers: responseHeaders, - }), - ) + } + // Resolve with a streaming body as soon as headers are available, + // matching fetch() semantics. Callers that pipe response.body (e.g. + // streamDownloadWithFetch) receive a live ReadableStream rather than + // a fully-buffered Buffer. + const body = new ReadableStream({ + start(controller) { + res.on('data', (chunk: Buffer) => { + controller.enqueue(chunk) + }) + res.on('end', () => { + controller.close() + }) + res.on('error', (err: Error) => { + controller.error(err) + }) + }, + cancel() { + res.destroy() + }, }) - res.on('error', reject) + resolve( + new Response(body, { + status: statusCode ?? 0, + statusText: res.statusMessage ?? '', + headers: responseHeaders, + }), + ) }, ) if (init.body) { @@ -186,11 +204,7 @@ export async function apiFetch( url: string, init: ApiFetchInit = {}, ): Promise { - const agent = getHttpsAgent() - if (!agent) { - return await fetch(url, init as globalThis.RequestInit) - } - return await _httpsRequestFetch(url, init, agent, 0) + return await _httpsRequestFetch(url, init, getHttpsAgent(), 0) } export type CommandRequirements = { diff --git a/src/utils/api.test.mts b/src/utils/api.test.mts index 22146a2df..222960940 100644 --- a/src/utils/api.test.mts +++ b/src/utils/api.test.mts @@ -6,8 +6,9 @@ * direct API calls when NODE_EXTRA_CA_CERTS is not set at process startup. * * Test Coverage: - * - apiFetch falls back to regular fetch when no extra CA certs are needed. - * - apiFetch uses node:https.request with custom agent when CA certs are set. + * - apiFetch always uses node:https.request (no undici body timeout). + * - apiFetch passes a custom HttpsAgent when CA certs are set via SSL_CERT_FILE. + * - apiFetch passes no agent (undefined) when no CA certs are configured. * - Response object construction from https.request output. * - POST requests with JSON body through https.request path. * - Error propagation from https.request failures. @@ -113,18 +114,46 @@ describe('apiFetch with extra CA certificates', () => { globalThis.fetch = originalFetch }) - it('should use regular fetch when no extra CA certs are needed', async () => { - const mockResponse = new Response(JSON.stringify({ ok: true }), { - status: 200, - statusText: 'OK', - }) - globalThis.fetch = vi.fn().mockResolvedValue(mockResponse) + it('should use https.request with no agent when no extra CA certs are needed', async () => { + const mockReq = { + end: vi.fn(), + on: vi.fn(), + write: vi.fn(), + } + + mockHttpsRequest.mockImplementation( + (_url: string, _opts: unknown, callback: RequestCallback) => { + setTimeout(() => { + const mockRes = { + headers: { 'content-type': 'text/plain' }, + on: vi.fn(), + statusCode: 200, + statusMessage: 'OK', + } + const handlers: Record = {} + mockRes.on.mockImplementation((event: string, handler: Function) => { + handlers[event] = handler + return mockRes + }) + callback(mockRes) + handlers['data']?.(Buffer.from('response body')) + handlers['end']?.() + }, 0) + return mockReq + }, + ) const { queryApiSafeText } = await import('./api.mts') const result = await queryApiSafeText('test/path', 'test request') - expect(globalThis.fetch).toHaveBeenCalled() - expect(mockHttpsRequest).not.toHaveBeenCalled() + // Always uses https.request — no undici body timeout. + expect(mockHttpsRequest).toHaveBeenCalled() + // No custom HttpsAgent created when CA certs are not configured. + expect(MockHttpsAgent).not.toHaveBeenCalled() + // agent is undefined when no CA certs are configured. + const callArgs = mockHttpsRequest.mock.calls[0] + expect(callArgs[1]).toEqual(expect.objectContaining({ agent: undefined })) + expect(result.ok).toBe(true) }) it('should use https.request when extra CA certs are available', async () => {