Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 43 additions & 29 deletions src/utils/api.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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'

Expand Down Expand Up @@ -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<string, string> | undefined
Expand All @@ -85,7 +88,7 @@ export type ApiFetchInit = {
function _httpsRequestFetch(
url: string,
init: ApiFetchInit,
agent: HttpsAgent,
agent: HttpsAgent | undefined,
redirectCount: number,
): Promise<Response> {
return new Promise((resolve, reject) => {
Expand Down Expand Up @@ -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<Uint8Array>({
Comment thread
bcomnes marked this conversation as resolved.
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) {
Expand All @@ -186,11 +204,7 @@ export async function apiFetch(
url: string,
init: ApiFetchInit = {},
): Promise<Response> {
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 = {
Expand Down
49 changes: 39 additions & 10 deletions src/utils/api.test.mts
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<string, Function> = {}
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 () => {
Expand Down
Loading