diff --git a/src/main/azure-devops/azure-devops-api-request.ts b/src/main/azure-devops/azure-devops-api-request.ts index 136b21ab0..089975cf5 100644 --- a/src/main/azure-devops/azure-devops-api-request.ts +++ b/src/main/azure-devops/azure-devops-api-request.ts @@ -1,5 +1,6 @@ import { Buffer } from 'node:buffer' import type { AzureDevOpsRepoRef } from './repository-ref' +import { cancelUnreadResponseBody } from '../lib/unread-response-body' const REQUEST_TIMEOUT_MS = 5000 @@ -84,6 +85,7 @@ export async function requestAzureDevOpsJsonAtBase( signal: AbortSignal.timeout(options.timeoutMs ?? REQUEST_TIMEOUT_MS) }) if (!response.ok) { + await cancelUnreadResponseBody(response) return null } return (await response.json()) as T diff --git a/src/main/azure-devops/client.test.ts b/src/main/azure-devops/client.test.ts index 1b3e5fb11..d394962fc 100644 --- a/src/main/azure-devops/client.test.ts +++ b/src/main/azure-devops/client.test.ts @@ -1,4 +1,5 @@ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { cancelTrackingResponse } from '../lib/unread-response-body.test-fixtures' import { getAzureDevOpsAuthStatus, getAzureDevOpsPullRequestForBranch, @@ -176,4 +177,22 @@ describe('Azure DevOps client', () => { headSha: 'newsha' }) }) + + it('cancels unread error-response bodies so bundled undici cannot crash on socket close', async () => { + gitExecFileAsyncMock.mockResolvedValue({ + stdout: 'https://dev.azure.com/acme/Project/_git/repo\n' + }) + let cancelledBodies = 0 + const fetchMock = vi.fn(async () => + cancelTrackingResponse(502, () => { + cancelledBodies += 1 + }) + ) + globalThis.fetch = fetchMock as never + + await getAzureDevOpsPullRequestForBranch('/repo', 'refs/heads/feature/azure') + + expect(fetchMock).toHaveBeenCalled() + expect(cancelledBodies).toBe(fetchMock.mock.calls.length) + }) }) diff --git a/src/main/bitbucket/client.test.ts b/src/main/bitbucket/client.test.ts index 9ab872358..4e552ab59 100644 --- a/src/main/bitbucket/client.test.ts +++ b/src/main/bitbucket/client.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { cancelTrackingResponse } from '../lib/unread-response-body.test-fixtures' const { gitExecFileAsyncMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn() @@ -122,4 +123,19 @@ describe('Bitbucket client', () => { account: 'bitbucket-user' }) }) + + it('cancels unread error-response bodies so bundled undici cannot crash on socket close', async () => { + let cancelledBodies = 0 + const fetchMock = vi.fn(async () => + cancelTrackingResponse(502, () => { + cancelledBodies += 1 + }) + ) + vi.stubGlobal('fetch', fetchMock) + + await getBitbucketPullRequestForBranch('/repo', 'refs/heads/feature/bitbucket') + + expect(fetchMock).toHaveBeenCalled() + expect(cancelledBodies).toBe(fetchMock.mock.calls.length) + }) }) diff --git a/src/main/bitbucket/client.ts b/src/main/bitbucket/client.ts index 5eea09b16..6dbc11f65 100644 --- a/src/main/bitbucket/client.ts +++ b/src/main/bitbucket/client.ts @@ -12,6 +12,7 @@ import { getHostedReviewLocalGitOptions, type HostedReviewExecutionOptions } from '../source-control/hosted-review-git-options' +import { cancelUnreadResponseBody } from '../lib/unread-response-body' const DEFAULT_API_BASE_URL = 'https://api.bitbucket.org/2.0' const REQUEST_TIMEOUT_MS = 5000 @@ -97,6 +98,7 @@ async function requestJson(path: string, options: RequestOptions = {}): Promi signal: AbortSignal.timeout(options.timeoutMs ?? REQUEST_TIMEOUT_MS) }) if (!response.ok) { + await cancelUnreadResponseBody(response) return null } return (await response.json()) as T diff --git a/src/main/gitea/client.test.ts b/src/main/gitea/client.test.ts index bf1fe13ca..a0ae0b3ce 100644 --- a/src/main/gitea/client.test.ts +++ b/src/main/gitea/client.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { cancelTrackingResponse } from '../lib/unread-response-body.test-fixtures' const { gitExecFileAsyncMock } = vi.hoisted(() => ({ gitExecFileAsyncMock: vi.fn() @@ -301,4 +302,19 @@ describe('Gitea client', () => { }) expect(String(fetchMock.mock.calls[0]?.[0])).toBe('https://git.example.com/api/v1/user') }) + + it('cancels unread error-response bodies so bundled undici cannot crash on socket close', async () => { + let cancelledBodies = 0 + const fetchMock = vi.fn(async () => + cancelTrackingResponse(502, () => { + cancelledBodies += 1 + }) + ) + vi.stubGlobal('fetch', fetchMock) + + await getGiteaPullRequestForBranch('/repo', 'refs/heads/feature/gitea') + + expect(fetchMock).toHaveBeenCalled() + expect(cancelledBodies).toBe(fetchMock.mock.calls.length) + }) }) diff --git a/src/main/gitea/client.ts b/src/main/gitea/client.ts index 30545f83a..9889598b6 100644 --- a/src/main/gitea/client.ts +++ b/src/main/gitea/client.ts @@ -11,6 +11,7 @@ import { getHostedReviewLocalGitOptions, type HostedReviewExecutionOptions } from '../source-control/hosted-review-git-options' +import { cancelUnreadResponseBody } from '../lib/unread-response-body' const REQUEST_TIMEOUT_MS = 5000 // Why: self-hosted Forgejo can take ~5s to serve one /pulls page (it loads @@ -89,6 +90,7 @@ async function requestJsonAtBase( signal: AbortSignal.timeout(options.timeoutMs ?? REQUEST_TIMEOUT_MS) }) if (!response.ok) { + await cancelUnreadResponseBody(response) return null } return (await response.json()) as T diff --git a/src/main/global-fetch-call-site-audit.test.ts b/src/main/global-fetch-call-site-audit.test.ts new file mode 100644 index 000000000..52049b16d --- /dev/null +++ b/src/main/global-fetch-call-site-audit.test.ts @@ -0,0 +1,97 @@ +import { readdirSync, readFileSync } from 'node:fs' +import { join, relative, sep } from 'node:path' +import { describe, expect, it } from 'vitest' + +// Global fetch (bare or via globalThis/global — unlike Electron's net.fetch) +// goes through Node's bundled undici, which can crash the whole process when +// an unread response body pauses the HTTP/1 parser and the peer closes the +// socket (nodejs/undici#5360, orca#8695). This applies to every Node process +// we ship: Electron main, the CLI, and the SSH relay. +// +// Each entry below maps an audited file to its expected number of matching +// lines. Real call sites must consume or cancel the body on every path, +// including !response.ok (see main/lib/unread-response-body.ts). A count +// change means a call site was added, removed, or moved: re-audit the file +// and update the count. +const AUDITED_GLOBAL_FETCH_LINES = new Map([ + // HTTP call sites — body consumed or cancelled on every path, including !ok + ['main/azure-devops/azure-devops-api-request.ts', 1], + ['main/bitbucket/client.ts', 1], + ['main/gitea/client.ts', 1], + ['main/orca-profiles/profile-cloud-client.ts', 1], + ['main/orca-profiles/profile-cloud-org-members-client.ts', 1], + ['main/rate-limits/codex-fetcher.ts', 3], + ['main/runtime/relay/relay-http-client.ts', 2], + ['main/source-control/hosted-review-api-request.ts', 1], + ['main/speech/openai-transcription-client.ts', 1], + // fetch appears only inside injected-page script source strings, not as a + // call this process makes + ['main/amp/hook-service.ts', 1], + ['main/opencode/hook-service.ts', 1], + ['main/pi/agent-status-extension-source.ts', 1], + // local identifiers named `fetch` (git fetch), not HTTP + ['main/ipc/worktree-remote.ts', 2], + ['relay/git-handler.ts', 1], + // fetch mentioned only in a comment + ['main/ipc/feedback.ts', 1] +]) + +// A line is a hit when it calls bare `fetch(` or touches `globalThis.fetch` / +// `global.fetch` in any way (call, alias, fallback like `input.fetch ?? +// globalThis.fetch`). `typeof globalThis.fetch` type annotations are exempt. +const GLOBAL_FETCH_LINE = /(^|[^.\w])fetch\(|(? { + const counts = new Map() + for (const root of SCANNED_ROOTS) { + for (const entry of readdirSync(join(srcRoot, root), { + recursive: true, + withFileTypes: true + })) { + if (!entry.isFile() || !entry.name.endsWith('.ts')) { + continue + } + if ( + entry.name.endsWith('.test.ts') || + entry.name.endsWith('.test-fixtures.ts') || + entry.name.endsWith('.d.ts') + ) { + continue + } + const filePath = join(entry.parentPath, entry.name) + const content = readFileSync(filePath, 'utf8') + if (!GLOBAL_FETCH_LINE.test(content)) { + continue + } + const hits = content.split('\n').filter((line) => GLOBAL_FETCH_LINE.test(line)).length + if (hits > 0) { + counts.set(relative(srcRoot, filePath).split(sep).join('/'), hits) + } + } + } + return counts +} + +describe('global fetch call-site audit (main, cli, relay)', () => { + it('keeps every global-fetch line audited with its expected count', () => { + const found = globalFetchLineCounts(join(__dirname, '..')) + + const drifted = [...found] + .filter(([file, count]) => AUDITED_GLOBAL_FETCH_LINES.get(file) !== count) + .map(([file, count]) => `${file}: found ${count} line(s)`) + .sort() + expect( + drifted, + 'Global fetch (bare, globalThis.fetch, or global.fetch) uses undici, ' + + 'where an unread response body can crash the whole process (orca#8695). ' + + 'New or moved call sites must either use Electron net.fetch or consume/' + + 'cancel the response body on ALL paths (cancelUnreadResponseBody in ' + + 'main/lib/unread-response-body.ts), then update AUDITED_GLOBAL_FETCH_LINES.' + ).toEqual([]) + + const stale = [...AUDITED_GLOBAL_FETCH_LINES.keys()].filter((file) => !found.has(file)).sort() + expect(stale, 'Remove audited entries whose global-fetch lines are gone.').toEqual([]) + }) +}) diff --git a/src/main/lib/unread-response-body.test-fixtures.ts b/src/main/lib/unread-response-body.test-fixtures.ts new file mode 100644 index 000000000..c80ece2b3 --- /dev/null +++ b/src/main/lib/unread-response-body.test-fixtures.ts @@ -0,0 +1,13 @@ +/** Response whose body reports cancellation, for asserting that error paths + * cancel unread bodies (see unread-response-body.ts). */ +export function cancelTrackingResponse(status: number, onCancel: () => void): Response { + const body = new ReadableStream({ + start(controller) { + controller.enqueue(new TextEncoder().encode('unread error body')) + }, + cancel() { + onCancel() + } + }) + return new Response(body, { status }) +} diff --git a/src/main/lib/unread-response-body.test.ts b/src/main/lib/unread-response-body.test.ts new file mode 100644 index 000000000..5138de950 --- /dev/null +++ b/src/main/lib/unread-response-body.test.ts @@ -0,0 +1,27 @@ +import { describe, expect, it } from 'vitest' +import { cancelUnreadResponseBody } from './unread-response-body' +import { cancelTrackingResponse } from './unread-response-body.test-fixtures' + +describe('cancelUnreadResponseBody', () => { + it('cancels an unread body stream', async () => { + let cancelled = false + await cancelUnreadResponseBody( + cancelTrackingResponse(500, () => { + cancelled = true + }) + ) + expect(cancelled).toBe(true) + }) + + it('no-ops on a body-less response', async () => { + await expect( + cancelUnreadResponseBody(new Response(null, { status: 500 })) + ).resolves.toBeUndefined() + }) + + it('swallows cancellation failures on a locked stream', async () => { + const response = cancelTrackingResponse(500, () => {}) + response.body?.getReader() + await expect(cancelUnreadResponseBody(response)).resolves.toBeUndefined() + }) +}) diff --git a/src/main/lib/unread-response-body.ts b/src/main/lib/unread-response-body.ts new file mode 100644 index 000000000..c34342482 --- /dev/null +++ b/src/main/lib/unread-response-body.ts @@ -0,0 +1,12 @@ +/** + * Cancel a fetch Response body that no code path will read. Why: leaving it + * unread can crash the whole process from inside Node's bundled undici + * (nodejs/undici#5360, orca#8695); see global-fetch-call-site-audit.test.ts. + */ +export async function cancelUnreadResponseBody(response: Response): Promise { + try { + await response.body?.cancel() + } catch { + // Cancelling an already-errored, locked, or closed stream is harmless. + } +} diff --git a/src/main/orca-profiles/profile-cloud-client.test.ts b/src/main/orca-profiles/profile-cloud-client.test.ts index d57f7a041..5ff6f2e13 100644 --- a/src/main/orca-profiles/profile-cloud-client.test.ts +++ b/src/main/orca-profiles/profile-cloud-client.test.ts @@ -1,4 +1,5 @@ import { beforeEach, describe, expect, it, vi } from 'vitest' +import { cancelTrackingResponse } from '../lib/unread-response-body.test-fixtures' import type { OrcaCloudAuthConfig } from './profile-cloud-auth-config' import type { OrcaCloudSession } from './profile-cloud-session-store' import { @@ -248,4 +249,16 @@ describe('Orca cloud client', () => { } }) }) + + it('cancels the unread error-response body so bundled undici cannot crash on socket close', async () => { + let cancelledBodies = 0 + fetchMock.mockResolvedValue( + cancelTrackingResponse(502, () => { + cancelledBodies += 1 + }) + ) + + await expect(refreshOrcaCloudCapabilities(config, session)).rejects.toThrow() + expect(cancelledBodies).toBe(1) + }) }) diff --git a/src/main/orca-profiles/profile-cloud-client.ts b/src/main/orca-profiles/profile-cloud-client.ts index 81d88f72e..e7657bbdb 100644 --- a/src/main/orca-profiles/profile-cloud-client.ts +++ b/src/main/orca-profiles/profile-cloud-client.ts @@ -6,6 +6,7 @@ import type { import type { OrcaCloudAuthConfig } from './profile-cloud-auth-config' import type { OrcaCloudSession } from './profile-cloud-session-store' import type { OrcaCloudSessionExchangeResponse } from './profile-cloud-session-exchange' +import { cancelUnreadResponseBody } from '../lib/unread-response-body' type ExchangeCodeArgs = { code: string @@ -173,6 +174,7 @@ async function postJson(url: string, body: unknown, accessToken?: string): Pr signal: AbortSignal.timeout(CLOUD_REQUEST_TIMEOUT_MS) }) if (!response.ok) { + await cancelUnreadResponseBody(response) throw new OrcaCloudRequestError(response.status) } return (await response.json()) as T diff --git a/src/main/rate-limits/codex-fetcher-backend.test.ts b/src/main/rate-limits/codex-fetcher-backend.test.ts index 3028bee55..d345d94a5 100644 --- a/src/main/rate-limits/codex-fetcher-backend.test.ts +++ b/src/main/rate-limits/codex-fetcher-backend.test.ts @@ -1,4 +1,5 @@ import { join } from 'node:path' +import { cancelTrackingResponse } from '../lib/unread-response-body.test-fixtures' import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' const { childSpawnMock, readFileMock, ptySpawnMock } = vi.hoisted(() => ({ @@ -193,4 +194,26 @@ describe('Codex backend rate-limit requests', () => { }) ) }) + + it('cancels the unread error-response body so bundled undici cannot crash on socket close', async () => { + readFileMock.mockResolvedValue( + JSON.stringify({ + tokens: { access_token: 'access-token', account_id: 'account-id' } + }) + ) + let cancelledBodies = 0 + vi.mocked(fetch).mockResolvedValue( + cancelTrackingResponse(429, () => { + cancelledBodies += 1 + }) + ) + + await expect( + consumeCodexRateLimitResetCredit({ + codexHomePath: '/managed/codex-home', + idempotencyKey: 'redeem-429' + }) + ).rejects.toThrow('Codex reset failed: HTTP 429') + expect(cancelledBodies).toBe(1) + }) }) diff --git a/src/main/rate-limits/codex-fetcher.ts b/src/main/rate-limits/codex-fetcher.ts index d606a12e8..f7ce2a5db 100644 --- a/src/main/rate-limits/codex-fetcher.ts +++ b/src/main/rate-limits/codex-fetcher.ts @@ -9,6 +9,7 @@ import type { import { spawn } from 'node:child_process' import { readFile } from 'node:fs/promises' import { homedir } from 'node:os' +import { cancelUnreadResponseBody } from '../lib/unread-response-body' import { join } from 'node:path' import { probeCodexAuthPresence } from './codex-auth-presence' import { resolveCodexCommand } from '../codex-cli/command' @@ -381,6 +382,7 @@ async function fetchBackendRateLimitResetCredits( signal }) if (!response.ok) { + await cancelUnreadResponseBody(response) return null } const payload = (await response.json()) as BackendRateLimitResetCreditsResponse @@ -448,6 +450,7 @@ export async function consumeCodexRateLimitResetCredit(options: { } ) if (!response.ok) { + await cancelUnreadResponseBody(response) throw new Error(`Codex reset failed: HTTP ${response.status}`) } const payload = (await response.json()) as BackendConsumeRateLimitResetCreditResponse @@ -531,6 +534,7 @@ async function fetchViaBackend( signal }) if (!response.ok) { + await cancelUnreadResponseBody(response) return null } const payload = (await response.json()) as BackendUsageResponse diff --git a/src/main/runtime/relay/relay-http-client.test.ts b/src/main/runtime/relay/relay-http-client.test.ts index 3a786210b..57beb5274 100644 --- a/src/main/runtime/relay/relay-http-client.test.ts +++ b/src/main/runtime/relay/relay-http-client.test.ts @@ -1,5 +1,6 @@ import { describe, expect, it, vi } from 'vitest' import nacl from 'tweetnacl' +import { cancelTrackingResponse } from '../../lib/unread-response-body.test-fixtures' import { exchangeRelayAuthorization, requestRelayAssignment } from './relay-http-client' describe('relay HTTP client', () => { @@ -72,4 +73,35 @@ describe('relay HTTP client', () => { }) ).rejects.toThrow('relay_assignment_failed_502') }) + + it('cancels unread error-response bodies so bundled undici cannot crash on socket close', async () => { + const keypair = nacl.box.keyPair() + let cancelledBodies = 0 + const fetch = vi.fn(async () => + cancelTrackingResponse(503, () => { + cancelledBodies += 1 + }) + ) + + await expect( + exchangeRelayAuthorization({ + endpoint: 'https://auth.example/v1/desktop/auth/relay-token', + accessToken: 'ordinary-access-token', + keypair: { + ...keypair, + publicKeyB64: Buffer.from(keypair.publicKey).toString('base64') + }, + fetch + }) + ).rejects.toThrow() + await expect( + requestRelayAssignment({ + directorUrl: 'https://relay.example', + relayToken: 'scoped-token', + relayHostId: 'AbCdEf0123_-xyZ9', + fetch + }) + ).rejects.toThrow() + expect(cancelledBodies).toBe(2) + }) }) diff --git a/src/main/runtime/relay/relay-http-client.ts b/src/main/runtime/relay/relay-http-client.ts index ddda205d1..47118b88c 100644 --- a/src/main/runtime/relay/relay-http-client.ts +++ b/src/main/runtime/relay/relay-http-client.ts @@ -1,6 +1,7 @@ import { createHash } from 'node:crypto' import { z } from 'zod' import type { E2EEKeypair } from '../e2ee-keypair' +import { cancelUnreadResponseBody } from '../../lib/unread-response-body' const RelayTokenResponseSchema = z .object({ @@ -69,6 +70,7 @@ export async function exchangeRelayAuthorization(input: { body: JSON.stringify({ relayHostId, hostPublicKeyB64: input.keypair.publicKeyB64 }) }) if (!response.ok) { + await cancelUnreadResponseBody(response) throw new RelayHttpError('token-exchange', response.status) } const parsed = RelayTokenResponseSchema.safeParse(await response.json()) @@ -96,6 +98,7 @@ export async function requestRelayAssignment(input: { body: JSON.stringify({ v: 1, relayHostId: input.relayHostId }) }) if (!response.ok) { + await cancelUnreadResponseBody(response) throw new RelayHttpError('assignment', response.status) } const parsed = AssignmentResponseSchema.safeParse(await response.json())