diff --git a/CHANGELOG.md b/CHANGELOG.md index 7b624b8f5..354791932 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Changed - **Bazel diagnostics** — `socket manifest bazel --verbose` now emits bounded subprocess traces with argv, cwd, duration, exit status, output sizes, and failure stderr tails to make customer log-only triage safer and faster. +## [1.1.110](https://github.com/SocketDev/socket-cli/releases/tag/v1.1.110) - 2026-05-29 + +### Fixed +- Resolved intermittent ~5-second timeouts affecting manifest uploads for reachability analysis and `socket fix`, along with other long-running API requests. Socket CLI now uses an explicit HTTP agent for all API traffic, so slow uploads and large streaming responses are no longer dropped prematurely. + ## [1.1.109](https://github.com/SocketDev/socket-cli/releases/tag/v1.1.109) - 2026-05-28 ### Added diff --git a/package.json b/package.json index 5f83f65df..982b09fcd 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "socket", - "version": "1.1.109", + "version": "1.1.110", "description": "CLI for Socket.dev", "homepage": "https://github.com/SocketDev/socket-cli", "license": "MIT AND OFL-1.1", diff --git a/src/utils/api.mts b/src/utils/api.mts index 7582b54a0..99ff1f247 100644 --- a/src/utils/api.mts +++ b/src/utils/api.mts @@ -54,30 +54,34 @@ import type { const MAX_REDIRECTS = 20 const NO_ERROR_MESSAGE = 'No error message returned' -// Cached HTTPS agent for extra CA certificate support in direct API calls. +// Cached HTTPS agent for direct API calls. Undefined only until the first +// getHttpsAgent() call lazily creates it. let _httpsAgent: HttpsAgent | undefined -let _httpsAgentResolved = false -// Returns an HTTPS agent configured with extra CA certificates when -// SSL_CERT_FILE is set but NODE_EXTRA_CA_CERTS is not. -function getHttpsAgent(): HttpsAgent | undefined { - if (_httpsAgentResolved) { +// Returns an explicit HTTPS agent for direct API calls, carrying extra CA +// certificates when SSL_CERT_FILE is set but NODE_EXTRA_CA_CERTS is not. An +// explicit agent is always returned. Node >=19's global agent enables keepAlive +// with a 5s socket timeout that Node applies as a per-socket inactivity +// timeout. A request made without an explicit agent inherits it and is torn +// down after 5s of socket inactivity, prematurely dropping slow or idle-gapped +// requests (e.g. streaming full-scan responses, large downloads) even when no +// timeout was requested. A fresh Agent carries no timeout. +function getHttpsAgent(): HttpsAgent { + if (_httpsAgent) { return _httpsAgent } - _httpsAgentResolved = true const ca = getExtraCaCerts() - if (!ca) { - return undefined - } - _httpsAgent = new HttpsAgent({ ca }) - return _httpsAgent + const agent = ca ? new HttpsAgent({ ca }) : new HttpsAgent() + _httpsAgent = agent + return agent } // 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. +// (e.g. full scan results) can transfer without a hard deadline. An explicit +// HttpsAgent is always passed (carrying extra CA certificates when +// SSL_CERT_FILE is configured) so requests do not inherit Node's global-agent +// keepAlive socket timeout. export type ApiFetchInit = { body?: string | undefined headers?: Record | undefined @@ -88,7 +92,7 @@ export type ApiFetchInit = { function _httpsRequestFetch( url: string, init: ApiFetchInit, - agent: HttpsAgent | undefined, + agent: HttpsAgent, redirectCount: number, ): Promise { return new Promise((resolve, reject) => { diff --git a/src/utils/api.test.mts b/src/utils/api.test.mts index 222960940..2533d0d75 100644 --- a/src/utils/api.test.mts +++ b/src/utils/api.test.mts @@ -8,7 +8,7 @@ * Test Coverage: * - 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. + * - apiFetch passes an explicit HttpsAgent (no timeout) 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. @@ -114,7 +114,7 @@ describe('apiFetch with extra CA certificates', () => { globalThis.fetch = originalFetch }) - it('should use https.request with no agent when no extra CA certs are needed', async () => { + it('should use https.request with an explicit no-timeout agent when no extra CA certs are needed', async () => { const mockReq = { end: vi.fn(), on: vi.fn(), @@ -148,11 +148,15 @@ describe('apiFetch with extra CA certificates', () => { // 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. + // An explicit HttpsAgent is created so the request does not inherit Node's + // global agent (keepAlive plus a 5s socket timeout). + expect(MockHttpsAgent).toHaveBeenCalledTimes(1) + // A fresh agent carries no timeout, so no per-socket inactivity timeout. + const agentOpts = MockHttpsAgent.mock.calls[0]?.[0] + expect(agentOpts?.timeout).toBeUndefined() + // The request is made with that explicit agent. const callArgs = mockHttpsRequest.mock.calls[0] - expect(callArgs[1]).toEqual(expect.objectContaining({ agent: undefined })) + expect(callArgs[1].agent).toMatchObject({ _isHttpsAgent: true }) expect(result.ok).toBe(true) }) diff --git a/src/utils/sdk.mts b/src/utils/sdk.mts index ac72b0dc2..9d551e669 100644 --- a/src/utils/sdk.mts +++ b/src/utils/sdk.mts @@ -25,6 +25,7 @@ */ import { readFileSync } from 'node:fs' +import { Agent as HttpAgent } from 'node:http' import { Agent as HttpsAgent } from 'node:https' import { rootCertificates } from 'node:tls' @@ -182,25 +183,31 @@ export async function setupSdk( // Usage of HttpProxyAgent vs. HttpsProxyAgent based on the chart at: // https://github.com/delvedor/hpagent?tab=readme-ov-file#usage - const ProxyAgent = apiBaseUrl?.startsWith('http:') - ? HttpProxyAgent - : HttpsProxyAgent + const isHttp = apiBaseUrl?.startsWith('http:') + const ProxyAgent = isHttp ? HttpProxyAgent : HttpsProxyAgent // Load extra CA certificates for SSL_CERT_FILE support when // NODE_EXTRA_CA_CERTS was not set at process startup. const ca = getExtraCaCerts() + // Always pass an explicit agent. Node >=19's global agent enables keepAlive + // with a 5s socket timeout that Node applies as a per-socket inactivity + // timeout. A request made without an explicit agent inherits it and is torn + // down after 5s of socket inactivity, even when SOCKET_CLI_API_TIMEOUT is + // unset. This breaks slow endpoints like upload-manifest-files, which streams + // a chunked multipart body while the server parses auth/multipart before + // sending any response byte. A fresh Agent carries no timeout, so a request + // is bounded only by a real SOCKET_CLI_API_TIMEOUT (applied below via the + // SDK's timeout option) or until interrupted. const sdkOptions = { - ...(apiProxy - ? { - agent: new ProxyAgent({ - proxy: apiProxy, - ...(ca ? { ca, proxyConnectOptions: { ca } } : {}), - }), - } - : ca - ? { agent: new HttpsAgent({ ca }) } - : {}), + agent: apiProxy + ? new ProxyAgent({ + proxy: apiProxy, + ...(ca ? { ca, proxyConnectOptions: { ca } } : {}), + }) + : isHttp + ? new HttpAgent() + : new HttpsAgent(ca ? { ca } : undefined), ...(apiBaseUrl ? { baseUrl: apiBaseUrl } : {}), timeout: constants.ENV.SOCKET_CLI_API_TIMEOUT, userAgent: createUserAgentFromPkgJson({ diff --git a/src/utils/sdk.test.mts b/src/utils/sdk.test.mts index ea8cd7e08..644e79497 100644 --- a/src/utils/sdk.test.mts +++ b/src/utils/sdk.test.mts @@ -453,6 +453,24 @@ describe('SDK setup with telemetry hooks', () => { expect(result.data.options.userAgent).toBe('socket-cli/1.1.34') } }) + + it('should pass an explicit agent with no idle timeout by default', async () => { + // Regression: Node >=19's global agent applies a 5s socket inactivity + // timeout. Requests made without an explicit agent inherit it, so + // uploadManifestFiles (which streams a chunked multipart body while the + // server parses auth/multipart before responding) was torn down at ~5s + // with no response. setupSdk must always supply an explicit agent that + // has no such timeout. + const result = await setupSdk({ apiToken: 'test-token' }) + + expect(result.ok).toBe(true) + if (result.ok) { + expect(result.data.options.agent).toBeDefined() + expect(MockHttpsAgent).toHaveBeenCalled() + const agentOpts = MockHttpsAgent.mock.calls.at(-1)?.[0] + expect(agentOpts?.timeout).toBeUndefined() + } + }) }) describe('hook integration', () => { @@ -703,14 +721,19 @@ describe('setupSdk with extra CA certificates', () => { } }) - it('should not create agent when no extra CA certs are needed', async () => { + it('should create a default HttpsAgent with no timeout when no proxy or CA certs are configured', async () => { const { setupSdk: fn } = await import('./sdk.mts') const result = await fn({ apiToken: 'test-token' }) expect(result.ok).toBe(true) if (result.ok) { - expect(result.data.options.agent).toBeUndefined() - expect(MockHttpsAgent).not.toHaveBeenCalled() + // The default branch must still pass an explicit agent so requests do + // not inherit Node's global agent (keepAlive plus a 5s socket timeout). + expect(result.data.options.agent).toBeDefined() + expect(MockHttpsAgent).toHaveBeenCalledTimes(1) + // A fresh agent carries no timeout, so no per-socket inactivity timeout. + const agentOpts = MockHttpsAgent.mock.calls[0]?.[0] + expect(agentOpts?.timeout).toBeUndefined() } }) })