From d414e1b625cc5c4419f84cba2cbe87787a88be51 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:22:04 -0700 Subject: [PATCH 1/5] fix(cli): bound, trace, and explain the requests the CLI makes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Four transport gaps, all of which failed silently. A request had no timeout, so a connection that was accepted and then never answered hung the terminal indefinitely. `SIM_TIMEOUT_SECONDS` now bounds one, defaulting to 3600s — deliberately above every timeout the server itself applies, since a synchronous workflow run is allowed 3000s on a paid plan and a tighter default would abort real work and report it as a transport failure. `0` removes the bound, for a self-hosted deployment that runs executions without one of its own. The caller's abort signal is composed with the timeout rather than replaced, so neither masks the other. Node ignores HTTP(S)_PROXY unless NODE_USE_ENV_PROXY opts in, and only from v22.21 and v24.5, so on a network that reaches the API only through a proxy every command failed to connect while the variable that would have fixed it was already set. The CLI cannot enable that from inside the process — Node reads it at startup — so it says what to do rather than bundling an HTTP stack for a setting the platform now owns. An API key was sent to any http:// endpoint with no signal. Now a warning, not a refusal: http is the documented way to reach a local dev server, and a deployment terminating TLS at a gateway is real. Loopback stays silent. `SIM_DEBUG=1` traces method, URL, status and duration. Bodies and headers are deliberately absent — the request carries the API key, and `secrets set` carries the secret itself. All four write to stderr, so a piped stdout stays parseable. --- .../content/docs/en/cli/configuration.mdx | 6 + packages/sim-cli/README.md | 5 + packages/sim-cli/src/http/client.test.ts | 106 +++++++++++++++++- packages/sim-cli/src/http/client.ts | 78 ++++++++++++- packages/sim-cli/src/http/environment.test.ts | 73 ++++++++++++ packages/sim-cli/src/http/environment.ts | 101 +++++++++++++++++ 6 files changed, 366 insertions(+), 3 deletions(-) create mode 100644 packages/sim-cli/src/http/environment.test.ts create mode 100644 packages/sim-cli/src/http/environment.ts diff --git a/apps/docs/content/docs/en/cli/configuration.mdx b/apps/docs/content/docs/en/cli/configuration.mdx index 564bc07cef6..a24bae70a1b 100644 --- a/apps/docs/content/docs/en/cli/configuration.mdx +++ b/apps/docs/content/docs/en/cli/configuration.mdx @@ -101,6 +101,12 @@ credentials. The `default` profile is `[default]` in both. | `SIM_CONFIG_DIR` | Relocate both files away from `~/.sim` | | `SIM_CONFIG_FILE` | Relocate only the config file | | `SIM_CREDENTIALS_FILE` | Relocate only the credentials file | +| `SIM_TIMEOUT_SECONDS` | Per-request timeout; `0` waits indefinitely. Defaults to `3600`, above every timeout the server itself applies | +| `SIM_DEBUG` | Trace each request's method, URL, status and duration to stderr | + +Node ignores `HTTPS_PROXY` unless you also set `NODE_USE_ENV_PROXY=1`, and only +from Node 22.21 and 24.5. The CLI warns when a proxy is configured but will not +be used. For CI, set `SIM_API_KEY` and `SIM_WORKSPACE` and nothing needs to touch the filesystem at all. diff --git a/packages/sim-cli/README.md b/packages/sim-cli/README.md index f6c44366817..6f308483843 100644 --- a/packages/sim-cli/README.md +++ b/packages/sim-cli/README.md @@ -57,6 +57,11 @@ Each setting resolves independently, first match wins: | --- | --- | | 1 | Command-line flag (`--endpoint`, `--workspace`, `--output`) | | 2 | Environment (`SIM_ENDPOINT`, `SIM_API_KEY`, `SIM_WORKSPACE`, `SIM_OUTPUT`) | + +`SIM_TIMEOUT_SECONDS` bounds each request (default `3600`, `0` waits +indefinitely) and `SIM_DEBUG=1` traces requests to stderr. Node ignores +`HTTPS_PROXY` unless `NODE_USE_ENV_PROXY=1` is also set, on Node 22.21+ or +24.5+; the CLI warns when a proxy is configured but will not be used. | 3 | `~/.sim/config` / `~/.sim/credentials` for the selected profile | | 4 | Built-in default (`https://www.sim.ai`, `table`) | diff --git a/packages/sim-cli/src/http/client.test.ts b/packages/sim-cli/src/http/client.test.ts index dc965b2e9e0..4ef2b7a1318 100644 --- a/packages/sim-cli/src/http/client.test.ts +++ b/packages/sim-cli/src/http/client.test.ts @@ -1,6 +1,7 @@ import { afterEach, describe, expect, it, vi } from 'vitest' import { CLI_CONTRACT } from '../contract/commands' import { V2_OPERATIONS, type V2OperationName } from '../generated/v2-api' +import { sleep } from '../helpers' import { USER_AGENT } from '../version' import { formatApiErrorDetails, @@ -314,6 +315,102 @@ describe('non-JSON responses', () => { }) }) +describe('a request that never answers', () => { + it('bounds a request by default, above every timeout the server itself applies', async () => { + // A synchronous workflow run is allowed 3000s on a paid plan, so a tighter + // default would abort real work and report it as a transport failure. What + // this catches is a connection that is accepted and then never answers. + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', fetchMock) + + await client().request('/api/v2/workflows') + + const signal = fetchMock.mock.calls[0][1].signal as AbortSignal + expect(signal).toBeInstanceOf(AbortSignal) + expect(signal.aborted).toBe(false) + }) + + it('sends no signal at all when the bound is switched off', async () => { + // A self-hosted deployment can run executions without a timeout of its own, + // and there the client must not invent one. + vi.stubEnv('SIM_TIMEOUT_SECONDS', '0') + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', fetchMock) + + await client().request('/api/v2/workflows') + expect(fetchMock.mock.calls[0][1].signal).toBeUndefined() + }) + + it('refuses a timeout that is not a number, naming the variable', async () => { + vi.stubEnv('SIM_TIMEOUT_SECONDS', 'soon') + vi.stubGlobal('fetch', vi.fn()) + + await expect(client().request('/api/v2/workflows')).rejects.toThrow( + /Invalid SIM_TIMEOUT_SECONDS "soon"/ + ) + }) + + it('explains a timeout as a timeout, not as an unreachable endpoint', async () => { + vi.stubEnv('SIM_TIMEOUT_SECONDS', '0.001') + vi.stubGlobal( + 'fetch', + vi.fn().mockImplementation(async (_url: string, init: RequestInit) => { + await sleep(20) + init.signal?.throwIfAborted() + return new Response('{}') + }) + ) + + await expect(client().request('/api/v2/workflows')).rejects.toThrow(/did not answer within/) + }) +}) + +describe('tracing a request', () => { + it('traces method, url, status and duration when asked, and nothing otherwise', async () => { + const response = () => + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + + const quiet = stubStderr(false) + try { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(response())) + await client().request('/api/v2/workflows') + } finally { + quiet.restore() + } + expect(quiet.writes).toEqual([]) + + vi.stubEnv('SIM_DEBUG', '1') + const traced = stubStderr(false) + try { + vi.stubGlobal('fetch', vi.fn().mockResolvedValue(response())) + await client().request('/api/v2/workflows') + } finally { + traced.restore() + } + + const line = traced.writes.join('') + expect(line).toContain('GET https://sim.example/api/v2/workflows') + expect(line).toContain('200') + expect(line).toMatch(/\d+ms/) + // The request carries the API key, and `secrets set` carries the secret + // itself, so a trace must never include headers or bodies. + expect(line).not.toContain('key') + }) +}) + describe('request identity', () => { it('identifies the CLI, its version and its runtime to the API', async () => { // Without a User-Agent a CLI request is indistinguishable from any other @@ -597,7 +694,6 @@ describe('raw requests', () => { 'https://sim.example/api/v2/chat', expect.objectContaining({ method: 'POST', - signal: controller.signal, headers: expect.objectContaining({ accept: 'text/event-stream', 'content-type': 'application/json', @@ -605,6 +701,14 @@ describe('raw requests', () => { }), }) ) + + // The signal is composed with the request timeout, so it is no longer the + // caller's object. What has to hold is the behaviour: aborting the + // caller's controller still aborts the request. + const sent = fetch.mock.calls[0][1].signal as AbortSignal + expect(sent.aborted).toBe(false) + controller.abort() + expect(sent.aborted).toBe(true) }) it('turns an aborted fetch into a clean CLI error', async () => { diff --git a/packages/sim-cli/src/http/client.ts b/packages/sim-cli/src/http/client.ts index 09cb52ad26e..42388e51999 100644 --- a/packages/sim-cli/src/http/client.ts +++ b/packages/sim-cli/src/http/client.ts @@ -1,6 +1,7 @@ import chalk from 'chalk' import type { ResolvedProfile } from '../config/index' import { USER_AGENT } from '../version' +import { warnIfKeyOverCleartext, warnIfProxyIgnored } from './environment' /** * A failure the CLI can explain. Anything thrown as a `SimApiError` is printed @@ -249,6 +250,54 @@ function dropUnionBranchNoise(issues: DetailIssue[]): DetailIssue[] { return kept.length > 0 ? kept : issues } +/** + * How long a single request may take before the CLI gives up, in seconds. + * + * The default is deliberately above every bound the server itself applies: a + * synchronous workflow run is allowed 3000s on a paid plan, so a tighter + * default would abort real work and report it as a transport failure. What it + * catches is the case the server cannot — a connection that is accepted and + * then never answers, which otherwise hangs the terminal indefinitely. + * + * `SIM_TIMEOUT_SECONDS=0` removes the bound, for a self-hosted deployment that + * runs executions without one of its own. + */ +const DEFAULT_TIMEOUT_SECONDS = 3600 + +function resolveTimeoutMs(env: NodeJS.ProcessEnv = process.env): number { + const raw = env.SIM_TIMEOUT_SECONDS + if (raw === undefined || raw.trim() === '') return DEFAULT_TIMEOUT_SECONDS * 1000 + + const seconds = Number(raw) + if (!Number.isFinite(seconds) || seconds < 0) { + throw new SimApiError( + `Invalid SIM_TIMEOUT_SECONDS "${raw}". Use a non-negative number of seconds, or 0 to disable.`, + 0 + ) + } + return seconds * 1000 +} + +/** Whether to trace requests to stderr. */ +function debugEnabled(env: NodeJS.ProcessEnv = process.env): boolean { + const raw = env.SIM_DEBUG + return raw !== undefined && raw !== '' && raw !== '0' && raw.toLowerCase() !== 'false' +} + +/** + * Traces one request on stderr. + * + * Method, URL, status and duration only. Bodies and headers are deliberately + * absent: the request carries the API key, and `secrets set` carries the secret + * itself, so a trace that included either would write credentials into whatever + * log the user pasted it into. + */ +function traceRequest(method: string, url: string, status: number | string, startedAt: number) { + process.stderr.write( + `${chalk.dim(`[sim] ${method} ${url} → ${status} ${Math.round(performance.now() - startedAt)}ms`)}\n` + ) +} + /** Formats nested validation issues as readable, path-aware lines. */ export function formatApiErrorDetails(details: unknown): string[] { const issues: DetailIssue[] = [] @@ -366,11 +415,27 @@ export class SimClient { const url = buildUrl(this.profile.endpoint, path, options.query) const hasBody = options.body !== undefined + const method = options.method ?? 'GET' + + warnIfProxyIgnored() + warnIfKeyOverCleartext(this.profile.endpoint, Boolean(apiKey)) + + // The caller's signal still cancels; the timeout only adds a second reason + // to abort, so neither can mask the other. + const timeoutMs = resolveTimeoutMs() + const timeout = timeoutMs > 0 ? AbortSignal.timeout(timeoutMs) : undefined + const signal = + options.signal && timeout + ? AbortSignal.any([options.signal, timeout]) + : (options.signal ?? timeout) + + const trace = debugEnabled() + const startedAt = performance.now() let response: Response try { response = await fetch(url, { - method: options.method ?? 'GET', + method, headers: { ...(apiKey ? { 'x-api-key': apiKey } : {}), accept: 'application/json', @@ -379,19 +444,28 @@ export class SimClient { ...options.headers, }, body: hasBody ? JSON.stringify(options.body) : undefined, - signal: options.signal, + signal, redirect: 'manual', }) } catch (cause) { + if (trace) traceRequest(method, url, 'failed', startedAt) if (options.signal?.aborted) { throw new SimApiError('Request cancelled.', 0) } + if (timeout?.aborted) { + throw new SimApiError( + `${url} did not answer within ${timeoutMs / 1000}s. Raise SIM_TIMEOUT_SECONDS, or set it to 0 to wait indefinitely.`, + 0 + ) + } throw new SimApiError( `Could not reach ${this.profile.endpoint}: ${(cause as Error).message}`, 0 ) } + if (trace) traceRequest(method, url, response.status, startedAt) + if (REDIRECT_STATUSES.has(response.status)) throw this.toRedirectError(url, path, response) if (!response.ok) { diff --git a/packages/sim-cli/src/http/environment.test.ts b/packages/sim-cli/src/http/environment.test.ts new file mode 100644 index 00000000000..ac59499035c --- /dev/null +++ b/packages/sim-cli/src/http/environment.test.ts @@ -0,0 +1,73 @@ +/** + * @vitest-environment node + */ +import { afterEach, beforeEach, describe, expect, it } from 'vitest' +import { resetEnvironmentNotices, warnIfKeyOverCleartext, warnIfProxyIgnored } from './environment' + +let writes: string[] +let originalWrite: typeof process.stderr.write + +beforeEach(() => { + resetEnvironmentNotices() + writes = [] + originalWrite = process.stderr.write + process.stderr.write = ((chunk: string) => { + writes.push(String(chunk)) + return true + }) as typeof process.stderr.write +}) + +afterEach(() => { + process.stderr.write = originalWrite +}) + +describe('a proxy the request will not go through', () => { + it('reports a proxy the runtime is capable of but was not opted into', () => { + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080' }, 'v22.21.0') + expect(writes.join('')).toContain('NODE_USE_ENV_PROXY=1') + }) + + it('reports a runtime that cannot honour it at all, naming the version', () => { + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v22.19.0') + expect(writes.join('')).toContain('Node v22.19.0 cannot use it') + }) + + it('stays silent once the runtime supports it and the caller opted in', () => { + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v24.5.0') + warnIfProxyIgnored({ HTTP_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v22.21.0') + expect(writes).toEqual([]) + }) + + it('stays silent when no proxy is configured', () => { + warnIfProxyIgnored({}, 'v22.19.0') + expect(writes).toEqual([]) + }) + + it('says it once, not once per request', () => { + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080' }, 'v24.5.0') + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080' }, 'v24.5.0') + expect(writes).toHaveLength(1) + }) +}) + +describe('an API key crossing the network in the clear', () => { + it('reports a key sent to a remote host over http', () => { + warnIfKeyOverCleartext('http://sim.internal.example', true) + expect(writes.join('')).toContain('sim.internal.example') + expect(writes.join('')).toContain('over http') + }) + + it('stays silent for the documented local development case', () => { + // `http://localhost:3000` is what the README and the login example use. + for (const host of ['http://localhost:3000', 'http://127.0.0.1:3000', 'http://api.localhost']) { + warnIfKeyOverCleartext(host, true) + } + expect(writes).toEqual([]) + }) + + it('stays silent over https, and when there is no key to leak', () => { + warnIfKeyOverCleartext('https://sim.example', true) + warnIfKeyOverCleartext('http://sim.internal.example', false) + expect(writes).toEqual([]) + }) +}) diff --git a/packages/sim-cli/src/http/environment.ts b/packages/sim-cli/src/http/environment.ts new file mode 100644 index 00000000000..0602c90fde5 --- /dev/null +++ b/packages/sim-cli/src/http/environment.ts @@ -0,0 +1,101 @@ +/** + * One-time notices about the environment a request is about to be made in. + * + * Both conditions here are silent failures rather than errors: the request goes + * out and something the caller expected to happen simply did not. They are + * reported once per process, on stderr, so a loop over many rows says it once + * and a piped stdout stays parseable. + */ + +const reported = new Set() + +function once(key: string, message: string): void { + if (reported.has(key)) return + reported.add(key) + process.stderr.write(`warning: ${message}\n`) +} + +/** Test seam: notices are once-per-process, and each test needs a clean slate. */ +export function resetEnvironmentNotices(): void { + reported.clear() +} + +const PROXY_VARIABLES = ['HTTPS_PROXY', 'https_proxy', 'HTTP_PROXY', 'http_proxy'] as const + +/** The first Node releases whose built-in proxy support honours the env vars. */ +const PROXY_SUPPORT = { major22: 22, minor22: 21, major24: 24, minor24: 5 } as const + +/** + * Whether this runtime can act on `HTTP(S)_PROXY` at all. + * + * Node reads them only from v22.21 and v24.5, and only when opted into. Before + * that the variables are inert no matter what is set. + */ +function runtimeCanProxy(version: string): boolean { + const [major, minor] = version.replace(/^v/, '').split('.').map(Number) + if (!Number.isFinite(major) || !Number.isFinite(minor)) return false + if (major > PROXY_SUPPORT.major24) return true + if (major === PROXY_SUPPORT.major24) return minor >= PROXY_SUPPORT.minor24 + if (major === PROXY_SUPPORT.major22) return minor >= PROXY_SUPPORT.minor22 + return major > PROXY_SUPPORT.major22 && major < PROXY_SUPPORT.major24 +} + +/** + * Reports a proxy the request will not actually go through. + * + * Node's `fetch` ignores `HTTP(S)_PROXY` unless `NODE_USE_ENV_PROXY` opts in, + * and older releases ignore them outright — so on a network that only reaches + * the API through a proxy, every command fails to connect while the variable + * that would have fixed it is already set. The CLI cannot enable the support + * from inside the process (Node reads the flag at startup), so it says what to + * do instead of proxying itself, which would mean bundling an HTTP stack for a + * setting the platform now owns. + */ +export function warnIfProxyIgnored( + env: NodeJS.ProcessEnv = process.env, + version: string = process.version +): void { + const variable = PROXY_VARIABLES.find((name) => env[name]) + if (!variable) return + if (env.NODE_USE_ENV_PROXY && runtimeCanProxy(version)) return + + once( + 'proxy', + runtimeCanProxy(version) + ? `${variable} is set but Node only uses it when NODE_USE_ENV_PROXY=1. Re-run with NODE_USE_ENV_PROXY=1 to route through the proxy.` + : `${variable} is set but Node ${version} cannot use it. Upgrade to Node 22.21 or 24.5 and set NODE_USE_ENV_PROXY=1 to route through the proxy.` + ) +} + +/** Hosts where cleartext is the normal case rather than a mistake. */ +const LOOPBACK = new Set(['localhost', '127.0.0.1', '[::1]', '::1', '0.0.0.0']) + +function isLoopback(hostname: string): boolean { + return LOOPBACK.has(hostname) || hostname.endsWith('.localhost') +} + +/** + * Reports an API key about to cross the network in cleartext. + * + * A warning rather than a refusal: `http://` is the documented way to reach a + * local dev server, and an internal deployment terminating TLS at a gateway is + * a real deployment, not a mistake to block. Loopback is silent because that is + * the documented case; anything else means the key is on the wire in the clear, + * which is worth one line. + */ +export function warnIfKeyOverCleartext(endpoint: string, hasApiKey: boolean): void { + if (!hasApiKey) return + + let url: URL + try { + url = new URL(endpoint) + } catch { + return + } + if (url.protocol !== 'http:' || isLoopback(url.hostname)) return + + once( + 'cleartext', + `sending your API key to ${url.host} over http. Anything on the path can read it — use https unless this network is trusted.` + ) +} From de9adc8a701c3ab98833facd1d6ae714deee1e77 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:27:14 -0700 Subject: [PATCH 2/5] fix(cli): make the request bound safe on every runtime it supports MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two ways the new timeout could fail before the request was made. `AbortSignal.any` arrived in Node 20.3 and this package supports Node 20, so composing a caller's abort signal with the timeout threw a bare TypeError on the earliest 20.x releases. It is now used when present and composed through an AbortController when not. `AbortSignal.timeout` rejects a fractional millisecond outright, and past 2^31-1 ms it does not fail at all — it clamps to 1ms, so the longest timeout anyone asked for became the shortest. The value is now rounded and refused above what Node can actually wait, pointing at 0 for an unbounded wait. Also unstubs env vars between tests: `stubEnv` is not undone by `unstubAllGlobals`, so a SIM_TIMEOUT_SECONDS set for one test configured every test after it. --- packages/sim-cli/src/http/client.test.ts | 53 ++++++++++++++++++++++++ packages/sim-cli/src/http/client.ts | 52 ++++++++++++++++++++--- 2 files changed, 100 insertions(+), 5 deletions(-) diff --git a/packages/sim-cli/src/http/client.test.ts b/packages/sim-cli/src/http/client.test.ts index 4ef2b7a1318..e94868118d1 100644 --- a/packages/sim-cli/src/http/client.test.ts +++ b/packages/sim-cli/src/http/client.test.ts @@ -14,6 +14,9 @@ import { afterEach(() => { vi.unstubAllGlobals() + // `stubEnv` is not undone by `unstubAllGlobals`, so a SIM_TIMEOUT_SECONDS or + // SIM_DEBUG set for one test would otherwise configure every test after it. + vi.unstubAllEnvs() }) function client(options: { apiKey?: string } = { apiKey: 'key' }): SimClient { @@ -360,6 +363,56 @@ describe('a request that never answers', () => { ) }) + it('rounds a fractional millisecond rather than letting the timer reject it', async () => { + // `AbortSignal.timeout` rejects a non-integer delay outright, so an + // unrounded 0.0005s threw ERR_OUT_OF_RANGE before the request was made. + vi.stubEnv('SIM_TIMEOUT_SECONDS', '0.0005') + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', fetchMock) + + await expect(client().request('/api/v2/workflows')).resolves.toBeDefined() + }) + + it('refuses a delay longer than Node can wait, which would silently become 1ms', async () => { + // Past 2^31-1 ms Node does not fail — it clamps to 1ms, so the request the + // caller asked to wait longest for would be the first one aborted. + vi.stubEnv('SIM_TIMEOUT_SECONDS', String(2 ** 31)) + vi.stubGlobal('fetch', vi.fn()) + + await expect(client().request('/api/v2/workflows')).rejects.toThrow(/longer than Node can wait/) + }) + + it('composes the caller signal with the timeout without AbortSignal.any', async () => { + // `AbortSignal.any` arrived in Node 20.3 and this package supports Node 20, + // so the earliest 20.x releases would have thrown a bare TypeError here. + const original = AbortSignal.any + // biome-ignore lint/performance/noDelete: restoring the property is the point + delete (AbortSignal as { any?: unknown }).any + const controller = new AbortController() + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', fetchMock) + + try { + await client().request('/api/v2/workflows', { signal: controller.signal }) + const sent = fetchMock.mock.calls[0][1].signal as AbortSignal + expect(sent.aborted).toBe(false) + controller.abort() + expect(sent.aborted).toBe(true) + } finally { + ;(AbortSignal as { any?: unknown }).any = original + } + }) + it('explains a timeout as a timeout, not as an unreachable endpoint', async () => { vi.stubEnv('SIM_TIMEOUT_SECONDS', '0.001') vi.stubGlobal( diff --git a/packages/sim-cli/src/http/client.ts b/packages/sim-cli/src/http/client.ts index 42388e51999..1097227a35a 100644 --- a/packages/sim-cli/src/http/client.ts +++ b/packages/sim-cli/src/http/client.ts @@ -264,6 +264,14 @@ function dropUnionBranchNoise(issues: DetailIssue[]): DetailIssue[] { */ const DEFAULT_TIMEOUT_SECONDS = 3600 +/** + * The longest delay Node's timers accept. + * + * Past it a timeout does not fail — it silently becomes 1ms, so the request the + * caller asked to wait longest for would be the first one aborted. + */ +const MAX_TIMEOUT_MS = 2 ** 31 - 1 + function resolveTimeoutMs(env: NodeJS.ProcessEnv = process.env): number { const raw = env.SIM_TIMEOUT_SECONDS if (raw === undefined || raw.trim() === '') return DEFAULT_TIMEOUT_SECONDS * 1000 @@ -275,7 +283,44 @@ function resolveTimeoutMs(env: NodeJS.ProcessEnv = process.env): number { 0 ) } - return seconds * 1000 + + // Rounded because a fractional millisecond is rejected outright by + // `AbortSignal.timeout`, and a sub-millisecond timeout is not a distinction + // anyone is drawing. + const ms = Math.round(seconds * 1000) + if (ms > MAX_TIMEOUT_MS) { + throw new SimApiError( + `SIM_TIMEOUT_SECONDS "${raw}" is longer than Node can wait (${Math.floor(MAX_TIMEOUT_MS / 1000)}s). Use 0 to wait indefinitely.`, + 0 + ) + } + return ms +} + +/** + * Aborts when either signal does. + * + * `AbortSignal.any` arrived in Node 20.3 and this package supports Node 20, so + * on the earliest 20.x releases calling it would throw a bare `TypeError` + * before the request was ever made — turning a supported runtime into a crash. + */ +function combineSignals( + caller: AbortSignal | undefined, + timeout: AbortSignal | undefined +): AbortSignal | undefined { + if (!caller) return timeout + if (!timeout) return caller + if (typeof AbortSignal.any === 'function') return AbortSignal.any([caller, timeout]) + + const controller = new AbortController() + for (const signal of [caller, timeout]) { + if (signal.aborted) { + controller.abort(signal.reason) + break + } + signal.addEventListener('abort', () => controller.abort(signal.reason), { once: true }) + } + return controller.signal } /** Whether to trace requests to stderr. */ @@ -424,10 +469,7 @@ export class SimClient { // to abort, so neither can mask the other. const timeoutMs = resolveTimeoutMs() const timeout = timeoutMs > 0 ? AbortSignal.timeout(timeoutMs) : undefined - const signal = - options.signal && timeout - ? AbortSignal.any([options.signal, timeout]) - : (options.signal ?? timeout) + const signal = combineSignals(options.signal, timeout) const trace = debugEnabled() const startedAt = performance.now() From 44971ebc0896bcd523345dd3dfc70ef73a44d816 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:36:25 -0700 Subject: [PATCH 3/5] fix(cli): correct the proxy version table, and classify a timeout mid-body MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `runtimeCanProxy` treated any release between 22 and 24 as capable, so on Node 23 — which reached end of life before the backport — a configured proxy was ignored and the CLI stayed silent about it, which is the exact failure the warning exists to report. The table is now the two lines that shipped the support, and anything after them. `AbortSignal.timeout` keeps firing after `fetch` resolves, so a bound that elapsed while the body was still being read — a large `files get` — escaped the client's own handling and printed a raw TimeoutError stack. The top-level handler now names it, which covers the streaming path as well as the JSON one. A user's own Ctrl-C raises AbortError and is deliberately left alone. --- packages/sim-cli/src/http/environment.test.ts | 18 +++++++++++++++++ packages/sim-cli/src/http/environment.ts | 20 +++++++++++++------ packages/sim-cli/src/index.ts | 12 +++++++++++ 3 files changed, 44 insertions(+), 6 deletions(-) diff --git a/packages/sim-cli/src/http/environment.test.ts b/packages/sim-cli/src/http/environment.test.ts index ac59499035c..4b4c2006f56 100644 --- a/packages/sim-cli/src/http/environment.test.ts +++ b/packages/sim-cli/src/http/environment.test.ts @@ -32,6 +32,24 @@ describe('a proxy the request will not go through', () => { expect(writes.join('')).toContain('Node v22.19.0 cannot use it') }) + it('does not credit a Node line that never got the backport', () => { + // 23.x sits between two supported lines but reached end of life before the + // backport, so treating it as "between" silenced the warning on the very + // line that still needs it. + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v23.11.0') + expect(writes.join('')).toContain('Node v23.11.0 cannot use it') + }) + + it('does not credit a line before its own first supported release', () => { + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v24.0.0') + expect(writes.join('')).toContain('Node v24.0.0 cannot use it') + }) + + it('credits every line after the first that shipped it', () => { + warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v25.1.0') + expect(writes).toEqual([]) + }) + it('stays silent once the runtime supports it and the caller opted in', () => { warnIfProxyIgnored({ HTTPS_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v24.5.0') warnIfProxyIgnored({ HTTP_PROXY: 'http://proxy:8080', NODE_USE_ENV_PROXY: '1' }, 'v22.21.0') diff --git a/packages/sim-cli/src/http/environment.ts b/packages/sim-cli/src/http/environment.ts index 0602c90fde5..f8c30b05cff 100644 --- a/packages/sim-cli/src/http/environment.ts +++ b/packages/sim-cli/src/http/environment.ts @@ -22,8 +22,16 @@ export function resetEnvironmentNotices(): void { const PROXY_VARIABLES = ['HTTPS_PROXY', 'https_proxy', 'HTTP_PROXY', 'http_proxy'] as const -/** The first Node releases whose built-in proxy support honours the env vars. */ -const PROXY_SUPPORT = { major22: 22, minor22: 21, major24: 24, minor24: 5 } as const +/** + * The first release of each Node line whose built-in proxy support reads the + * environment variables. Lines absent here never got it: 23 reached end of life + * before the backport, so it is not "between two supported versions" — treating + * it as capable would silence the warning on the one line that most needs it. + */ +const PROXY_SUPPORT: Record = { 22: 21, 24: 5 } + +/** The first Node line to ship the support, so every later line has it. */ +const FIRST_SUPPORTED_MAJOR = 24 /** * Whether this runtime can act on `HTTP(S)_PROXY` at all. @@ -34,10 +42,10 @@ const PROXY_SUPPORT = { major22: 22, minor22: 21, major24: 24, minor24: 5 } as c function runtimeCanProxy(version: string): boolean { const [major, minor] = version.replace(/^v/, '').split('.').map(Number) if (!Number.isFinite(major) || !Number.isFinite(minor)) return false - if (major > PROXY_SUPPORT.major24) return true - if (major === PROXY_SUPPORT.major24) return minor >= PROXY_SUPPORT.minor24 - if (major === PROXY_SUPPORT.major22) return minor >= PROXY_SUPPORT.minor22 - return major > PROXY_SUPPORT.major22 && major < PROXY_SUPPORT.major24 + + const firstSupportedMinor = PROXY_SUPPORT[major] + if (firstSupportedMinor !== undefined) return minor >= firstSupportedMinor + return major > FIRST_SUPPORTED_MAJOR } /** diff --git a/packages/sim-cli/src/index.ts b/packages/sim-cli/src/index.ts index 04198f56d8c..5147bb90310 100644 --- a/packages/sim-cli/src/index.ts +++ b/packages/sim-cli/src/index.ts @@ -19,6 +19,18 @@ async function main() { console.error(chalk.red(`Error: ${sanitize(error.message)}`)) process.exit(1) } + // `AbortSignal.timeout` keeps firing after `fetch` resolves, so a bound that + // elapses while the body is still being read — a large `files get`, say — + // surfaces here rather than inside the client. A user's own Ctrl-C raises + // `AbortError` instead, which is deliberately left alone. + if (error instanceof DOMException && error.name === 'TimeoutError') { + console.error( + chalk.red( + 'Error: the request timed out. Raise SIM_TIMEOUT_SECONDS, or set it to 0 to wait indefinitely.' + ) + ) + process.exit(1) + } if (error instanceof SimApiError) { console.error(chalk.red(`Error: ${sanitize(error.message)}`)) if (error.code) console.error(chalk.dim(` code: ${sanitize(error.code)}`)) From 6f2fdc4a7bbbb46d8f43a98748f1d1647f244e00 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:47:13 -0700 Subject: [PATCH 4/5] fix(cli): report a timed-out download as a timeout `files get --output-file` streams the body to disk, and `streamToFile` converted anything the stream threw into a write failure. So a request bound elapsing mid-download read as `Could not write : ...`, sending the reader to check permissions and free space for a timeout they can raise, and hiding the one instruction that resolves it. The predicate and that instruction now live beside the timeout that raises them, so the client, the top-level handler and the download path all say the same thing. The wrapping stays where it is: the staged-download cleanup runs off that failure, and rethrowing past it would leak the temporary directory. --- .../src/commands/protocol/files-get.test.ts | 27 +++++++++++++++++++ .../src/commands/protocol/files-get.ts | 9 ++++++- packages/sim-cli/src/http/client.ts | 16 ++++++++++- packages/sim-cli/src/index.ts | 15 ++++++----- 4 files changed, 58 insertions(+), 9 deletions(-) diff --git a/packages/sim-cli/src/commands/protocol/files-get.test.ts b/packages/sim-cli/src/commands/protocol/files-get.test.ts index fe948283df8..bc0fc9a586a 100644 --- a/packages/sim-cli/src/commands/protocol/files-get.test.ts +++ b/packages/sim-cli/src/commands/protocol/files-get.test.ts @@ -67,6 +67,16 @@ function failingBody(): ReadableStream { }) } +/** What `fetch` does to a body when the request's own timeout elapses. */ +function timedOutBody(): ReadableStream { + return new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('partial')) + controller.error(new DOMException('The operation was aborted due to timeout', 'TimeoutError')) + }, + }) +} + function program(): Command { const root = new Command('sim').exitOverride() for (const group of buildGeneratedCommands()) root.addCommand(group) @@ -81,6 +91,23 @@ describe('streamToFile', () => { expect(existsSync(target)).toBe(true) }) + it('reports an elapsed request bound as a timeout, not as a failed write', async () => { + // The stream is torn down by the request's own timeout, which is not a + // disk problem: calling it "could not write" sent the reader to check + // permissions and free space for a bound they can raise. + const target = join(dir, 'out.txt') + await expect( + streamToFile(timedOutBody(), createWriteStream(target, { flags: 'wx' })) + ).rejects.toThrow(/SIM_TIMEOUT_SECONDS/) + }) + + it('still reports a genuine write failure as one', async () => { + const target = join(dir, 'out.txt') + await expect( + streamToFile(failingBody(), createWriteStream(target, { flags: 'wx' })) + ).rejects.toThrow(/Could not write/) + }) + it('refuses to clobber an existing file, naming --force', async () => { const target = join(dir, 'out.txt') writeFileSync(target, 'precious') diff --git a/packages/sim-cli/src/commands/protocol/files-get.ts b/packages/sim-cli/src/commands/protocol/files-get.ts index 494a30908a1..cf11edb714d 100644 --- a/packages/sim-cli/src/commands/protocol/files-get.ts +++ b/packages/sim-cli/src/commands/protocol/files-get.ts @@ -7,10 +7,17 @@ import { pipeline } from 'node:stream/promises' import type { Command } from 'commander' import { clientFrom } from '../../context' import { V2_OPERATIONS } from '../../generated/v2-api' -import { resolvePath, SimApiError } from '../../http/client' +import { isRequestTimeout, RAISE_TIMEOUT_HINT, resolvePath, SimApiError } from '../../http/client' import { printProtocolResult } from './result' function writeFailure(path: WriteStream['path'], error: unknown): SimApiError { + // A body torn down by the request's own bound is not a disk problem. Calling + // it "could not write" sent the reader to check permissions and free space + // for a timeout they can raise, and hid the one instruction that resolves it. + if (isRequestTimeout(error)) { + return new SimApiError(`Downloading ${path} timed out. ${RAISE_TIMEOUT_HINT}`, 0) + } + const code = (error as NodeJS.ErrnoException).code if (code === 'EEXIST') { return new SimApiError( diff --git a/packages/sim-cli/src/http/client.ts b/packages/sim-cli/src/http/client.ts index 1097227a35a..87da033a17b 100644 --- a/packages/sim-cli/src/http/client.ts +++ b/packages/sim-cli/src/http/client.ts @@ -272,6 +272,20 @@ const DEFAULT_TIMEOUT_SECONDS = 3600 */ const MAX_TIMEOUT_MS = 2 ** 31 - 1 +/** The one instruction that resolves an elapsed request bound, wherever it surfaces. */ +export const RAISE_TIMEOUT_HINT = 'Raise SIM_TIMEOUT_SECONDS, or set it to 0 to wait indefinitely.' + +/** + * Whether this is the CLI's own request bound elapsing. + * + * `AbortSignal.timeout` raises `TimeoutError`, while a caller's cancel raises + * `AbortError` — so this distinguishes a bound the user can raise from a stop + * the user asked for, which must keep reading as a cancellation. + */ +export function isRequestTimeout(error: unknown): boolean { + return error instanceof DOMException && error.name === 'TimeoutError' +} + function resolveTimeoutMs(env: NodeJS.ProcessEnv = process.env): number { const raw = env.SIM_TIMEOUT_SECONDS if (raw === undefined || raw.trim() === '') return DEFAULT_TIMEOUT_SECONDS * 1000 @@ -496,7 +510,7 @@ export class SimClient { } if (timeout?.aborted) { throw new SimApiError( - `${url} did not answer within ${timeoutMs / 1000}s. Raise SIM_TIMEOUT_SECONDS, or set it to 0 to wait indefinitely.`, + `${url} did not answer within ${timeoutMs / 1000}s. ${RAISE_TIMEOUT_HINT}`, 0 ) } diff --git a/packages/sim-cli/src/index.ts b/packages/sim-cli/src/index.ts index 5147bb90310..251e53ad064 100644 --- a/packages/sim-cli/src/index.ts +++ b/packages/sim-cli/src/index.ts @@ -2,7 +2,12 @@ import chalk from 'chalk' import { ProfileConfigError } from './config/index' -import { formatApiErrorDetails, SimApiError } from './http/client' +import { + formatApiErrorDetails, + isRequestTimeout, + RAISE_TIMEOUT_HINT, + SimApiError, +} from './http/client' import { sanitize } from './output/render' import { buildProgram } from './program' @@ -23,12 +28,8 @@ async function main() { // elapses while the body is still being read — a large `files get`, say — // surfaces here rather than inside the client. A user's own Ctrl-C raises // `AbortError` instead, which is deliberately left alone. - if (error instanceof DOMException && error.name === 'TimeoutError') { - console.error( - chalk.red( - 'Error: the request timed out. Raise SIM_TIMEOUT_SECONDS, or set it to 0 to wait indefinitely.' - ) - ) + if (isRequestTimeout(error)) { + console.error(chalk.red(`Error: the request timed out. ${RAISE_TIMEOUT_HINT}`)) process.exit(1) } if (error instanceof SimApiError) { From 09ff9c0a782ee30249f74eb33b59d672d8d7f180 Mon Sep 17 00:00:00 2001 From: Waleed Latif Date: Mon, 17 Aug 2026 17:54:47 -0700 Subject: [PATCH 5/5] fix(cli): keep a sub-millisecond timeout bounded Zero is how this function says "no bound", so rounding a positive SIM_TIMEOUT_SECONDS down to zero inverted the request: anything under 0.0005s asked for the shortest possible timeout and got none at all, leaving a stalled request to hang. Introduced by the rounding that fixed the fractional-millisecond rejection. Floored at 1ms for every positive value; only a literal 0 still disables. --- packages/sim-cli/src/http/client.test.ts | 16 ++++++++++++++++ packages/sim-cli/src/http/client.ts | 6 ++++-- 2 files changed, 20 insertions(+), 2 deletions(-) diff --git a/packages/sim-cli/src/http/client.test.ts b/packages/sim-cli/src/http/client.test.ts index e94868118d1..de0825e7517 100644 --- a/packages/sim-cli/src/http/client.test.ts +++ b/packages/sim-cli/src/http/client.test.ts @@ -378,6 +378,22 @@ describe('a request that never answers', () => { await expect(client().request('/api/v2/workflows')).resolves.toBeDefined() }) + it('keeps a bound below half a millisecond bounded, rather than disabling it', async () => { + // Zero means "no bound", so rounding a positive value down to zero inverted + // the request: the shortest timeout anyone could ask for became none. + vi.stubEnv('SIM_TIMEOUT_SECONDS', '0.0004') + const fetchMock = vi.fn().mockResolvedValue( + new Response(JSON.stringify({ data: [] }), { + status: 200, + headers: { 'content-type': 'application/json' }, + }) + ) + vi.stubGlobal('fetch', fetchMock) + + await client().request('/api/v2/workflows') + expect(fetchMock.mock.calls[0][1].signal).toBeInstanceOf(AbortSignal) + }) + it('refuses a delay longer than Node can wait, which would silently become 1ms', async () => { // Past 2^31-1 ms Node does not fail — it clamps to 1ms, so the request the // caller asked to wait longest for would be the first one aborted. diff --git a/packages/sim-cli/src/http/client.ts b/packages/sim-cli/src/http/client.ts index 87da033a17b..6268dd379ad 100644 --- a/packages/sim-cli/src/http/client.ts +++ b/packages/sim-cli/src/http/client.ts @@ -300,8 +300,10 @@ function resolveTimeoutMs(env: NodeJS.ProcessEnv = process.env): number { // Rounded because a fractional millisecond is rejected outright by // `AbortSignal.timeout`, and a sub-millisecond timeout is not a distinction - // anyone is drawing. - const ms = Math.round(seconds * 1000) + // anyone is drawing. Floored at 1ms for anything above zero: rounding alone + // sent a bound under 0.0005s to 0, which this function reserves for "no + // bound at all", so asking for the shortest possible timeout produced none. + const ms = seconds === 0 ? 0 : Math.max(1, Math.round(seconds * 1000)) if (ms > MAX_TIMEOUT_MS) { throw new SimApiError( `SIM_TIMEOUT_SECONDS "${raw}" is longer than Node can wait (${Math.floor(MAX_TIMEOUT_MS / 1000)}s). Use 0 to wait indefinitely.`,