From 83a303806e1cf68167c532a4f7a286cc82115fd0 Mon Sep 17 00:00:00 2001 From: jdalton Date: Thu, 15 Oct 2026 01:04:25 -0400 Subject: [PATCH] fix(cli): apply API request timeout Honor configured timeout for direct CLI API calls and carry it across redirects. Assisted-by: Codex:GPT-6 --- src/utils/api-timeout.integration.test.mts | 325 ++++++++++++++++++ src/utils/api-timeout.test.mts | 20 ++ src/utils/api.mts | 33 +- .../certs/direct-api-timeout-cert.pem | 18 + .../fixtures/certs/direct-api-timeout-key.pem | 28 ++ 5 files changed, 423 insertions(+), 1 deletion(-) create mode 100644 src/utils/api-timeout.integration.test.mts create mode 100644 src/utils/api-timeout.test.mts create mode 100644 test/fixtures/certs/direct-api-timeout-cert.pem create mode 100644 test/fixtures/certs/direct-api-timeout-key.pem diff --git a/src/utils/api-timeout.integration.test.mts b/src/utils/api-timeout.integration.test.mts new file mode 100644 index 000000000..3d2e26a89 --- /dev/null +++ b/src/utils/api-timeout.integration.test.mts @@ -0,0 +1,325 @@ +import { spawn } from 'node:child_process' +import { mkdtempSync, readFileSync, rmSync } from 'node:fs' +import { createServer } from 'node:https' +import { tmpdir } from 'node:os' +import { join } from 'node:path' + +import { afterAll, beforeAll, describe, expect, it, vi } from 'vitest' + +import type { IncomingMessage, ServerResponse } from 'node:http' +import type { Server as HttpsServer } from 'node:https' +import type { AddressInfo } from 'node:net' + +type RequestObservation = { + authorization: string | undefined + body: string + closed: Promise + method: string | undefined + path: string | undefined + resolveClosed: () => void +} + +const certificatePath = new URL( + '../../test/fixtures/certs/direct-api-timeout-cert.pem', + import.meta.url, +) +const keyPath = new URL( + '../../test/fixtures/certs/direct-api-timeout-key.pem', + import.meta.url, +) +const certificate = readFileSync(certificatePath) +const key = readFileSync(keyPath) +const requestWaiters = new Map< + string, + (observation: RequestObservation) => void +>() +const responseTimers = new Set>() +const observations: RequestObservation[] = [] +const server: HttpsServer = createServer( + { cert: certificate, key }, + handleRequest, +) +let apiFetch: typeof import('./api.mts').apiFetch +let queryApi: typeof import('./api.mts').queryApiSafeTextWithStatus +let sendApiRequest: typeof import('./api.mts').sendApiRequest +let baseUrl = '' +let isolatedHome = '' + +function handleRequest( + request: IncomingMessage, + response: ServerResponse, +): void { + const pathname = new URL(request.url ?? '/', 'https://127.0.0.1').pathname + const closeHandler: { resolve?: () => void } = {} + const observation: RequestObservation = { + authorization: request.headers['authorization'], + body: '', + closed: new Promise(resolve => { + closeHandler.resolve = resolve + }), + method: request.method, + path: pathname, + resolveClosed: () => { + closeHandler.resolve?.() + }, + } + observations.push(observation) + requestWaiters.get(pathname)?.(observation) + requestWaiters.delete(pathname) + + const chunks: Buffer[] = [] + request.on('data', (chunk: Buffer | string) => { + chunks.push(Buffer.isBuffer(chunk) ? chunk : Buffer.from(chunk)) + }) + request.on('end', () => { + observation.body = Buffer.concat(chunks).toString('utf8') + if (pathname === '/redirect-302' || pathname === '/redirect-307') { + response.writeHead(pathname === '/redirect-302' ? 302 : 307, { + location: '/delay-headers', + }) + response.end() + return + } + const complete = () => { + responseTimers.delete(timer) + if (!response.destroyed) { + response.end(pathname === '/gap' ? 'firstsecond' : '{"ok":true}') + } + } + let delay = 0 + if (pathname === '/delay-headers') { + delay = 220 + } else if (pathname === '/idle-body') { + response.writeHead(200, { 'content-type': 'application/json' }) + response.flushHeaders() + response.write('{"ok":') + delay = 220 + } else if ( + pathname === '/explicit' || + pathname === '/gap' || + pathname === '/unset-gap' + ) { + delay = 180 + } + const timer = setTimeout(complete, delay) + responseTimers.add(timer) + }) + response.on('close', () => { + observation.resolveClosed() + }) +} + +function waitForRequest(pathname: string): Promise { + return new Promise(resolve => { + requestWaiters.set(pathname, resolve) + }) +} + +async function listen(): Promise { + await new Promise(resolve => { + server.listen(0, '127.0.0.1', resolve) + }) + const address = server.address() + if (address === null || typeof address === 'string') { + throw new Error('Loopback fixture did not receive a TCP address') + } + baseUrl = `https://127.0.0.1:${address.port}` +} + +function runUnsetTimeoutChild(): Promise { + const source = [ + "const { queryApiSafeTextWithStatus } = await import('./src/utils/api.mts')", + "const result = await queryApiSafeTextWithStatus('unset-gap')", + 'process.stdout.write(JSON.stringify({ ok: result.ok }))', + ].join('; ') + return new Promise((resolve, reject) => { + const child = spawn( + process.execPath, + ['--input-type=module', '--eval', source], + { + cwd: process.cwd(), + env: { + PATH: process.env['PATH'] ?? '', + HOME: isolatedHome, + XDG_CONFIG_HOME: join(isolatedHome, 'config'), + XDG_DATA_HOME: join(isolatedHome, 'data'), + SOCKET_CLI_CONFIG: '{}', + SOCKET_CLI_API_BASE_URL: baseUrl, + SOCKET_CLI_API_TOKEN: 'sktsec_abc123abc123abc123abc123', + SOCKET_CLI_API_PROXY: '', + HTTPS_PROXY: '', + https_proxy: '', + HTTP_PROXY: '', + http_proxy: '', + SSL_CERT_FILE: certificatePath.pathname, + NODE_EXTRA_CA_CERTS: '', + }, + }, + ) + let stdout = '' + let stderr = '' + const watchdog = setTimeout(() => { + child.kill() + reject(new Error('Unset-timeout child exceeded its watchdog')) + }, 3_000) + child.stdout.setEncoding('utf8') + child.stderr.setEncoding('utf8') + child.stdout.on('data', (chunk: string) => { + stdout += chunk + }) + child.stderr.on('data', (chunk: string) => { + stderr += chunk + }) + child.on('error', error => { + clearTimeout(watchdog) + reject(error) + }) + child.on('close', code => { + clearTimeout(watchdog) + if (code === 0) { + resolve(stdout) + } else { + reject(new Error(`Unset-timeout child exited ${code}: ${stderr}`)) + } + }) + }) +} + +describe('v1 direct Socket API timeout configuration', () => { + beforeAll(async () => { + isolatedHome = mkdtempSync(join(tmpdir(), 'socket-api-timeout-')) + vi.stubEnv('HOME', isolatedHome) + vi.stubEnv('XDG_CONFIG_HOME', join(isolatedHome, 'config')) + vi.stubEnv('XDG_DATA_HOME', join(isolatedHome, 'data')) + vi.stubEnv('SOCKET_CLI_CONFIG', '{}') + vi.stubEnv('SOCKET_CLI_API_TIMEOUT', '70') + vi.stubEnv('SOCKET_CLI_API_TOKEN', 'sktsec_abc123abc123abc123abc123') + vi.stubEnv('SOCKET_CLI_API_PROXY', '') + vi.stubEnv('HTTPS_PROXY', '') + vi.stubEnv('https_proxy', '') + vi.stubEnv('HTTP_PROXY', '') + vi.stubEnv('http_proxy', '') + vi.stubEnv('SSL_CERT_FILE', certificatePath.pathname) + vi.stubEnv('NODE_EXTRA_CA_CERTS', '') + await listen() + vi.stubEnv('SOCKET_CLI_API_BASE_URL', baseUrl) + const api = await import('./api.mts') + apiFetch = api.apiFetch + queryApi = api.queryApiSafeTextWithStatus + sendApiRequest = api.sendApiRequest + }) + + afterAll(async () => { + for (const timer of responseTimers) { + clearTimeout(timer) + } + server.closeAllConnections() + await new Promise(resolve => { + server.close(() => resolve()) + }) + vi.unstubAllEnvs() + rmSync(isolatedHome, { force: true, recursive: true }) + }) + + it('applies configured timeout to direct GET and POST requests', async () => { + const observedGet = waitForRequest('/delay-headers') + const getResult = await queryApi('delay-headers') + expect(getResult.ok).toBe(false) + if (!getResult.ok) { + expect(getResult.message).toBe('API request failed') + expect(getResult.cause).toContain('timed out after 70 ms') + } + const getRequest = await observedGet + await getRequest.closed + expect(getRequest.method).toBe('GET') + + const observedPost = waitForRequest('/delay-headers') + const postResult = await sendApiRequest('delay-headers', { + method: 'POST', + body: { source: 'synthetic' }, + }) + expect(postResult.ok).toBe(false) + if (!postResult.ok) { + expect(postResult.message).toBe('API request failed') + expect(postResult.cause).toContain('timed out after 70 ms') + } + const postRequest = await observedPost + await postRequest.closed + expect(postRequest.method).toBe('POST') + expect(postRequest.body).toBe('{"source":"synthetic"}') + expect(postRequest.authorization).toBeTruthy() + }) + + it('destroys an idle response body with a typed timeout error', async () => { + const observedRequest = waitForRequest('/idle-body') + const response = await apiFetch(`${baseUrl}/idle-body`, { timeout: 70 }) + await expect(response.text()).rejects.toMatchObject({ code: 'ETIMEDOUT' }) + const observed = await observedRequest + await observed.closed + }) + + it('allows normal completion within the configured timeout', async () => { + const observedGet = waitForRequest('/quick') + const getResult = await queryApi('quick') + expect(getResult.ok).toBe(true) + await observedGet + + const observedPost = waitForRequest('/quick') + const postResult = await sendApiRequest('quick', { + method: 'POST', + body: { source: 'synthetic' }, + }) + expect(postResult).toEqual({ ok: true, data: { ok: true } }) + const postRequest = await observedPost + expect(postRequest.body).toBe('{"source":"synthetic"}') + }) + + it('keeps raw API requests without a timeout when the setting is unset', async () => { + const observedRequest = waitForRequest('/unset-gap') + const childResult = await runUnsetTimeoutChild() + expect(childResult).toBe('{"ok":true}') + const observed = await observedRequest + await observed.closed + }) + + it('does not apply the API setting to raw fetches and preserves explicit timeout overrides', async () => { + const observedGap = waitForRequest('/gap') + const gapResponse = await apiFetch(`${baseUrl}/gap`) + expect(await gapResponse.text()).toBe('firstsecond') + await observedGap + + const observedExplicit = waitForRequest('/explicit') + const explicitResponse = await apiFetch(`${baseUrl}/explicit`, { + timeout: 300, + }) + expect(await explicitResponse.text()).toBe('{"ok":true}') + await observedExplicit + }) + + it.each(['/redirect-302', '/redirect-307'] as const)( + 'propagates timeout through %s redirects', + async path => { + const observedRedirect = waitForRequest(path) + const observedTarget = waitForRequest('/delay-headers') + const options = + path === '/redirect-307' + ? { method: 'POST', body: '{"source":"synthetic"}', timeout: 70 } + : { timeout: 70 } + await expect( + apiFetch(`${baseUrl}${path}`, options), + ).rejects.toMatchObject({ code: 'ETIMEDOUT' }) + const redirectRequest = await observedRedirect + const targetRequest = await observedTarget + await targetRequest.closed + expect(redirectRequest.method).toBe( + path === '/redirect-307' ? 'POST' : 'GET', + ) + expect(targetRequest.method).toBe( + path === '/redirect-307' ? 'POST' : 'GET', + ) + expect(targetRequest.body).toBe( + path === '/redirect-307' ? '{"source":"synthetic"}' : '', + ) + }, + ) +}) diff --git a/src/utils/api-timeout.test.mts b/src/utils/api-timeout.test.mts new file mode 100644 index 000000000..56ed0aebd --- /dev/null +++ b/src/utils/api-timeout.test.mts @@ -0,0 +1,20 @@ +import { describe, expect, it } from 'vitest' + +import { resolveConfiguredSocketApiTimeout } from './api.mts' + +describe('resolveConfiguredSocketApiTimeout', () => { + it('keeps finite positive configured values', () => { + expect(resolveConfiguredSocketApiTimeout(250)).toBe(250) + expect(resolveConfiguredSocketApiTimeout(90_000)).toBe(90_000) + }) + + it.each([ + 0, + -1, + Number.NaN, + Number.POSITIVE_INFINITY, + Number.NEGATIVE_INFINITY, + ])('omits invalid configured value %s', timeout => { + expect(resolveConfiguredSocketApiTimeout(timeout)).toBeUndefined() + }) +}) diff --git a/src/utils/api.mts b/src/utils/api.mts index 0acb94935..4d733c4bd 100644 --- a/src/utils/api.mts +++ b/src/utils/api.mts @@ -97,6 +97,13 @@ export type ApiFetchInit = { body?: string | undefined headers?: Record | undefined method?: string | undefined + timeout?: number | undefined +} + +export function resolveConfiguredSocketApiTimeout( + timeout: number = constants.ENV.SOCKET_CLI_API_TIMEOUT, +): number | undefined { + return Number.isFinite(timeout) && timeout > 0 ? timeout : undefined } // Internal httpsRequest-based fetch with redirect support. @@ -108,6 +115,7 @@ function _httpsRequestFetch( ): Promise { return new Promise((resolve, reject) => { const headers: Record = { ...init.headers } + let timedOutError: (Error & { code: string }) | undefined // Set Content-Length for request bodies to avoid chunked transfer encoding. if (init.body) { headers['content-length'] = String(Buffer.byteLength(init.body)) @@ -118,6 +126,7 @@ function _httpsRequestFetch( method: init.method || 'GET', headers, agent, + ...(init.timeout === undefined ? {} : { timeout: init.timeout }), }, res => { const { statusCode } = res @@ -160,7 +169,11 @@ function _httpsRequestFetch( redirectUrl, preserveMethod ? { ...init, headers: redirectHeaders } - : { headers: redirectHeaders, method: 'GET' }, + : { + headers: redirectHeaders, + method: 'GET', + timeout: init.timeout, + }, agent, redirectCount + 1, ), @@ -193,6 +206,11 @@ function _httpsRequestFetch( res.on('error', (err: Error) => { controller.error(err) }) + res.on('aborted', () => { + controller.error( + timedOutError ?? new Error('Socket API response aborted'), + ) + }) }, cancel() { res.destroy() @@ -207,6 +225,15 @@ function _httpsRequestFetch( ) }, ) + if (init.timeout !== undefined) { + req.on('timeout', () => { + timedOutError = Object.assign( + new Error(`Socket API request timed out after ${init.timeout} ms`), + { code: 'ETIMEDOUT' }, + ) + req.destroy(timedOutError) + }) + } if (init.body) { req.write(init.body) } @@ -506,8 +533,10 @@ async function queryApi(path: string, apiToken: string) { } const url = `${baseUrl}${baseUrl.endsWith('/') ? '' : '/'}${path}` + const timeout = resolveConfiguredSocketApiTimeout() const result = await apiFetch(url, { method: 'GET', + ...(timeout === undefined ? {} : { timeout }), headers: { Authorization: `Basic ${btoa(`${apiToken}:`)}`, 'User-Agent': getCliUserAgent(), @@ -718,8 +747,10 @@ export async function sendApiRequest( let result try { + const timeout = resolveConfiguredSocketApiTimeout() const fetchOptions = { method, + ...(timeout === undefined ? {} : { timeout }), headers: { Authorization: `Basic ${btoa(`${apiToken}:`)}`, 'Content-Type': 'application/json', diff --git a/test/fixtures/certs/direct-api-timeout-cert.pem b/test/fixtures/certs/direct-api-timeout-cert.pem new file mode 100644 index 000000000..c35d87316 --- /dev/null +++ b/test/fixtures/certs/direct-api-timeout-cert.pem @@ -0,0 +1,18 @@ +-----BEGIN CERTIFICATE----- +MIIC2jCCAcKgAwIBAgIJAL4PcDIvciwhMA0GCSqGSIb3DQEBCwUAMBQxEjAQBgNV +BAMMCWxvY2FsaG9zdDAeFw0yNjEwMTUwNDU0MzJaFw0zNjEwMTIwNDU0MzJaMBQx +EjAQBgNVBAMMCWxvY2FsaG9zdDCCASIwDQYJKoZIhvcNAQEBBQADggEPADCCAQoC +ggEBAOLW+uj5QsZ1EMLbMQ3LE3hqjkBjt4W7+JpChSG8kw87TV9Cs2654UMIWuol +YjA/ZMPMd7xoJsh5bOzDwiWlqOUYF2cTyNu37ENSfpba3vFHcCR/B55ocav7auPk +0vbaG4uIEzJMQ+DEjUS3LHX3BWF5ha3lyIwUR2rN3i5/+5NUMdVswLP2rO1eyGC4 +WBwz4uN/Fz5mPr8+F/Vhz0ftuMos1+OxvE5VZhg8dFyj/nBAJOPpj9AkXuVFt2h3 +72E/i4K9qgWTZVp5mnnHvggPt5Gpg0NACZqcPzMi+sFHdS8h3+fH6m7hKAirmBoo +wgqtss/kl+a5NsbpopvbuL6THk8CAwEAAaMvMC0wGgYDVR0RBBMwEYIJbG9jYWxo +b3N0hwR/AAABMA8GA1UdEwEB/wQFMAMBAf8wDQYJKoZIhvcNAQELBQADggEBALCx +UVrglAVZDRpCDU6ZpfI5YABwdNU3uiWjP93renYqfVY+w/iVrktcn3u17VnIMwa5 +RcJ/S2992vJ19Yp/Fg9sNzzkDxBOLCy1WWwwyC4WNY0L0cyUKLKnHZyOx/qh8ZXA +Lh87mrE1MrTce3ZbaKA2UmeHcUb+xM2VvlV3DRmvOyAAdvs6/r6MEEY5aqZLaRqj +sir+/69C7NYPYjrurUWFD7R9m0m482k7Y/UM0eI7D02wr+RB+hWW14HX2jt6DZ12 +uzVJ9l/cyeG1NFwcf46clodYsTzBEwjLvzvleW7RW8y54UjPBUBGYgz5RskIXol6 +g+NDdcm06eMWcmrWce8= +-----END CERTIFICATE----- diff --git a/test/fixtures/certs/direct-api-timeout-key.pem b/test/fixtures/certs/direct-api-timeout-key.pem new file mode 100644 index 000000000..705333073 --- /dev/null +++ b/test/fixtures/certs/direct-api-timeout-key.pem @@ -0,0 +1,28 @@ +-----BEGIN PRIVATE KEY----- +MIIEvQIBADANBgkqhkiG9w0BAQEFAASCBKcwggSjAgEAAoIBAQDi1vro+ULGdRDC +2zENyxN4ao5AY7eFu/iaQoUhvJMPO01fQrNuueFDCFrqJWIwP2TDzHe8aCbIeWzs +w8IlpajlGBdnE8jbt+xDUn6W2t7xR3AkfweeaHGr+2rj5NL22huLiBMyTEPgxI1E +tyx19wVheYWt5ciMFEdqzd4uf/uTVDHVbMCz9qztXshguFgcM+Ljfxc+Zj6/Phf1 +Yc9H7bjKLNfjsbxOVWYYPHRco/5wQCTj6Y/QJF7lRbdod+9hP4uCvaoFk2VaeZp5 +x74ID7eRqYNDQAmanD8zIvrBR3UvId/nx+pu4SgIq5gaKMIKrbLP5JfmuTbG6aKb +27i+kx5PAgMBAAECggEBAISEbsRXickCbq5G+ttB/bflGAFJv9bivxLvNXNGBOtr +C4JRJx3SfWR5pxMOj9P71NQFyRVKCw4m0Zlnqy3vH0Hm1xUim76wLdgOg2GBr6ho +wXywsTS7c1yzXfiQnyYSuPDEp+JEZ4sRiC3SjKjFjGErawryKDydRDOC3P9xSPkB +pxaC35BHVrhkGHxZ/ukUgY/gdIJii4uI9ED/mbhP/xIQsRfNOKhrTOccqBc00NpQ +YycjA0vovTGvLWtJUvODMsMmc/UImxR2Fw4hMX2w0ab2RqTg3Jh2vAbhvI2qS/GO ++o61f7xwymbqhvV6CfO9EX4P3QVBEqrb9jkuF1YSTfECgYEA9bLU7MikFh/gesty +vaWC3D2Cmmq9FAdj8Sqn2iCadwC6dV3Tn3ZsYYNUv24IeRHg91Qj7Y7kf2gem1Fa +RZ4UX/r+vhwV5LPYTl4w/CDnKDjTB6grfHzJE354DHF8+r6QCMznbtTK1rHhSHNe +HNFUDcv4mtHpr4oq2qtEWjoGQaMCgYEA7Fm6+L2pOe3FOk8/WYfF1IKkJ/giqU6J +QmvAiEjJVOWEtieSf5sMwVNvBTzw0FSPoc2fBeC5Aw7uA0sopAvnUDyl6BPTVHWR +6H2Vt4UC/HIZl2MhfInX5sifMXLE5BZxUPZQi75Z8IUs1MRFv316nUBb36Bbjs6P +g3LozrrVc2UCgYAmx5sj5GdBtrZPq1zo4f7NlEfQ55gAWUpE/l6P5yV6Mvs7GxwT +xzVEDt6JeGyN72x6nSUukacvFNf/289Lva+plowupXAvMNOrSuKgaPETQCTOyJRU +ib2S/hskm6UGTp2dRfohXCTHzjuom8z273M82jza8tbXhg9NhEq9kvr50QKBgFuz +5Qul6wcC7kdVgCrotyVhGvg3Gpu+URtoW2eFdjhuZTOX3wuLyUMpcCYEIxtx89i/ +wzewnZhGLqswejBMPg+vuF93ccOPanFeaLyv6xyv2KYYXGHJye9NYBb7hNFKs+TJ +7S6LSLI9uctD7BwEP5LL6dm2ik+6bYx/BT1Q2IjRAoGASMpzEDKvS5G62QnHMzIU +ufepf785EYSL4MpEkIwxnkPxl64kTUv21bJ4hOXv8sygaJl81LnImkJLkH91ALXe +fRBCOuDcJaedslerGCbPSiXUCzgXawrAfkyEpQph46OB0lNaqvAmhQcJ/6Wkq76e +71Ciy5CauImwu7PGz1FvO8c= +-----END PRIVATE KEY-----