From 2100fb25536d8eb82c4341808f920d2da50e1469 Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Fri, 14 Aug 2026 00:24:57 -0700 Subject: [PATCH] fix(runtime): cap remote git.diff and file previews at the transport budget (#14160) * fix(runtime): cap remote git.diff and file previews at the transport budget A remote or mobile user who opens the diff of a large image loses their whole WebSocket, not just that request: the E2EE channel closes with 1013 when a reply exceeds the 4 MiB outbound envelope. Two producers can exceed it unaided. git.diff/branchDiff/commitDiff cap text with MAX_RENDERED_DIFF_COMBINED_CHARACTERS (6M chars) -- a *renderer* budget that sits above the transport limit -- and return base64 for previewable binaries bounded only by MAX_GIT_SHOW_BYTES, so a 10 MiB PNG changed in place is ~26.7 MiB in one envelope. files.readPreview inlines base64 up to 10 MiB, and mobile calls it for every image tab. Both now measure against a budget derived from the outbound limit. The check sits in orca-runtime-git.ts, downstream of the dedupe and of both the SSH-provider and local branches, so a payload forwarded verbatim by an old relay is covered by the same code and src/relay needs no change. Local and in-process callers pass no budget and keep full fidelity. Measuring raw bytes would not work, which is the whole reason this needs a module. JSON escaping turns one control byte into six (\u00XX), and binary-buffer.ts sniffs only for NUL in the first 8 KiB -- so a NUL-free file of 0x01-0x1f bytes is classified as *text*, would pass a raw-byte cap, and would then blow the envelope. The budget is escape-aware, with a three-branch fast path that keeps normal diffs at two native byteLength calls and scans only the ambiguous band. The SSH branch of readFileExplorerPreview had the same raw-vs-escaped gap: its stat gate sizes base64 binaries, but text crossed unbounded. It now honours the same decoded-text limit the local branch already enforced. No wire change: GitDiffResult is untouched -- no third kind, no new field. Old clients see an error for one request instead of a dropped connection. diff_too_large joins the structured passthrough codes and lands on an existing error arm in both mobile consumers and the desktop remote path; file_too_large was already handled on both. Instruments the 1013 close, which nothing measured before, so the incidence this cap is meant to drive to zero is finally observable. `emitter` separates a producer size bug from a wedged link. Known regression: remote image previews between ~3.096 and ~3.146 MB now return file_too_large. They only intermittently worked before -- above ~3.0 MB they killed the socket -- so this trades intermittent connection loss for a consistent error. Test: 10281 passed in src/main/runtime + src/shared + src/main/git; mobile 3427 passed. Each of the six budget-enforcement sites is independently mutation-killed. Escaping fixtures cover newline-dense, control-char, CJK, lone-surrogate and base64 content against native JSON.stringify. tsc clean for node, web and cli; oxlint clean. Co-authored-by: Orca * fix(runtime): harden remote reply transport budgets * test(runtime): cover desktop remote preview budgets * test(runtime): close telemetry review gaps * chore(shared): repoint budget imports after the shared/types barrel removal Upstream #14447 dropped the shared/types barrel; GitDiffResult now lives in git-diff-compare-types and GlobalSettings in global-settings-types. Co-authored-by: Orca * fix(ssh): surface an over-cap preview read as file_too_large The stream reader aborts an over-cap read with StreamProtocolError, whose numeric code falls through mapRuntimeError to a generic runtime_error carrying the raw "Reported totalSize N exceeds client cap M" string. Neither preview client recognizes that: runtime-file-client.ts and mobile-file-preview-response.ts both key on file_too_large. It also made the two file_too_large guards directly below the read unreachable on the streaming path. Gives the cap its own error type so the caller can translate it, keeping the bandwidth saving the cap exists for. A genuine protocol fault still propagates unmasked. Found by the readiness review. Mutation-verified: removing the translation fails exactly the new test. Co-authored-by: Orca --------- Co-authored-by: Orca --- mobile/src/files/mobile-file-tab-doc.test.ts | 11 + .../mobile-diff-review-loaders.test.ts | 59 +++++ .../src/session/mobile-diff-review-loaders.ts | 3 + .../providers/filesystem-provider-contract.ts | 7 +- .../ssh-filesystem-provider-stream.test.ts | 15 ++ src/main/providers/ssh-filesystem-provider.ts | 5 +- src/main/providers/types.ts | 1 + src/main/runtime/orca-runtime-files.test.ts | 239 +++++++++++++++++- src/main/runtime/orca-runtime-files.ts | 188 +++++++++++--- .../orca-runtime-git-diff-budget.test.ts | 198 +++++++++++++++ src/main/runtime/orca-runtime-git.ts | 94 ++++--- src/main/runtime/orca-runtime.test.ts | 5 +- .../e2ee-channel-text-backpressure.test.ts | 23 ++ src/main/runtime/rpc/e2ee-channel.ts | 31 ++- src/main/runtime/rpc/errors.ts | 2 + .../files-preview-transport-budget.test.ts | 65 +++++ src/main/runtime/rpc/methods/files.ts | 40 ++- .../methods/git-diff-transport-budget.test.ts | 176 +++++++++++++ src/main/runtime/rpc/methods/git.test.ts | 9 +- src/main/runtime/rpc/methods/git.ts | 38 ++- src/main/runtime/runtime-rpc.test.ts | 10 +- src/main/ssh/ssh-file-stream-read-cap.ts | 10 + src/main/ssh/ssh-filesystem-stream-reader.ts | 17 +- src/shared/git-diff-transport-budget.test.ts | 193 ++++++++++++++ src/shared/git-diff-transport-budget.ts | 24 ++ src/shared/remote-rpc-content-budget.test.ts | 96 +++++++ src/shared/remote-rpc-content-budget.ts | 151 +++++++++++ src/shared/telemetry-events.test.ts | 20 ++ src/shared/telemetry-events.ts | 4 + 29 files changed, 1615 insertions(+), 119 deletions(-) create mode 100644 mobile/src/session/mobile-diff-review-loaders.test.ts create mode 100644 src/main/runtime/orca-runtime-git-diff-budget.test.ts create mode 100644 src/main/runtime/rpc/methods/files-preview-transport-budget.test.ts create mode 100644 src/main/runtime/rpc/methods/git-diff-transport-budget.test.ts create mode 100644 src/main/ssh/ssh-file-stream-read-cap.ts create mode 100644 src/shared/git-diff-transport-budget.test.ts create mode 100644 src/shared/git-diff-transport-budget.ts create mode 100644 src/shared/remote-rpc-content-budget.test.ts create mode 100644 src/shared/remote-rpc-content-budget.ts diff --git a/mobile/src/files/mobile-file-tab-doc.test.ts b/mobile/src/files/mobile-file-tab-doc.test.ts index 06ee3510c05..6bd153c67bb 100644 --- a/mobile/src/files/mobile-file-tab-doc.test.ts +++ b/mobile/src/files/mobile-file-tab-doc.test.ts @@ -138,6 +138,17 @@ describe('resolveMobileFileTabDoc', () => { }) }) + // Why: the host now caps oversized diffs with an error envelope, and a client that knows nothing + // about the code must still surface the message instead of rendering an empty diff. + it('propagates a host diff_too_large failure instead of rendering an empty diff', async () => { + const client = clientOf({ + 'git.diff': fail('diff_too_large', 'This diff is too large to open over a remote connection.') + }) + await expect( + resolveMobileFileTabDoc(client, { ...WT, relativePath: 'a.ts', diffSource: 'staged' }) + ).rejects.toThrow('This diff is too large to open over a remote connection.') + }) + it('propagates the RPC error message when a read fails', async () => { const client = clientOf({ 'files.read': fail('EIO', 'file_too_large') }) await expect( diff --git a/mobile/src/session/mobile-diff-review-loaders.test.ts b/mobile/src/session/mobile-diff-review-loaders.test.ts new file mode 100644 index 00000000000..a2b350bb395 --- /dev/null +++ b/mobile/src/session/mobile-diff-review-loaders.test.ts @@ -0,0 +1,59 @@ +import { describe, expect, it, vi } from 'vitest' +import type { RpcClient } from '../transport/rpc-client' +import type { RpcResponse } from '../transport/types' +import type { MobileDiffReviewQueueItem } from './mobile-diff-review-queue' +import { loadMobileDiffReviewDiff } from './mobile-diff-review-loaders' + +const DELETED_ITEM: MobileDiffReviewQueueItem = { + key: 'unstaged\0unstaged\0\0deleted.ts', + scope: 'unstaged', + area: 'unstaged', + filePath: 'deleted.ts', + status: 'deleted', + title: 'deleted.ts', + subtitle: 'Unstaged', + canStage: true, + canUnstage: false, + canDiscard: true, + isGeneratedOrLockFile: false, + diffIdentity: 'deleted-diff', + noteCount: 0, + unsentNoteCount: 0, + staleNoteCount: 0, + isReviewed: false, + changedSinceReview: false +} + +function clientWith(response: RpcResponse): RpcClient { + return { + sendRequest: vi.fn().mockResolvedValue(response) + } as unknown as RpcClient +} + +function failure(code: string, message: string): RpcResponse { + return { id: 'rpc-1', ok: false, error: { code, message }, _meta: { runtimeId: 'runtime-1' } } +} + +describe('loadMobileDiffReviewDiff', () => { + it('shows the too-large state for an oversized deleted diff', async () => { + await expect( + loadMobileDiffReviewDiff({ + client: clientWith(failure('diff_too_large', 'Diff too large')), + worktreeId: 'wt-1', + item: DELETED_ITEM, + branchCompare: null + }) + ).resolves.toEqual({ kind: 'too-large', itemKey: DELETED_ITEM.key }) + }) + + it('keeps the deleted fallback for hosts that cannot return deleted content', async () => { + await expect( + loadMobileDiffReviewDiff({ + client: clientWith(failure('internal_error', 'Unable to read deleted file')), + worktreeId: 'wt-1', + item: DELETED_ITEM, + branchCompare: null + }) + ).resolves.toEqual({ kind: 'deleted', itemKey: DELETED_ITEM.key }) + }) +}) diff --git a/mobile/src/session/mobile-diff-review-loaders.ts b/mobile/src/session/mobile-diff-review-loaders.ts index dfdaed2cd7c..0b1a14d156b 100644 --- a/mobile/src/session/mobile-diff-review-loaders.ts +++ b/mobile/src/session/mobile-diff-review-loaders.ts @@ -130,6 +130,9 @@ export async function loadMobileDiffReviewDiff(input: DiffLoadInput): Promise - readFile(filePath: string): Promise + readFile(filePath: string, limits?: FileReadLimits): Promise readTerminalArtifact?( filePath: string, options: TerminalArtifactAccessOptions diff --git a/src/main/providers/ssh-filesystem-provider-stream.test.ts b/src/main/providers/ssh-filesystem-provider-stream.test.ts index baaaed69406..dd3e004b362 100644 --- a/src/main/providers/ssh-filesystem-provider-stream.test.ts +++ b/src/main/providers/ssh-filesystem-provider-stream.test.ts @@ -155,6 +155,21 @@ describe('SshFilesystemProvider readFile streaming', () => { expect(mux.notify).toHaveBeenCalledWith('fs.cancelStream', { streamId: 1 }) }) + it('applies a caller binary cap before allocating the stream buffer', async () => { + mux.request.mockResolvedValue({ + streamId: 2, + totalSize: 2, + isBinary: true, + chunkEncoding: 'base64', + resultEncoding: 'base64' + }) + + await expect(provider.readFile('/home/x.bin', { maxBinaryBytes: 1 })).rejects.toThrow( + /exceeds client cap/i + ) + expect(mux.notify).toHaveBeenCalledWith('fs.cancelStream', { streamId: 2 }) + }) + it('rejects on fs.streamError notification', async () => { const totalSize = 1024 mux.request.mockImplementation(async () => { diff --git a/src/main/providers/ssh-filesystem-provider.ts b/src/main/providers/ssh-filesystem-provider.ts index 7fa2deb85b9..6adda586065 100644 --- a/src/main/providers/ssh-filesystem-provider.ts +++ b/src/main/providers/ssh-filesystem-provider.ts @@ -16,6 +16,7 @@ import { } from './ssh-filesystem-provider-watch' import type { IFilesystemProvider, + FileReadLimits, FileStat, FileReadResult, FileUploadSession, @@ -88,14 +89,14 @@ export class SshFilesystemProvider implements IFilesystemProvider { return (await this.mux.request('fs.readDir', { dirPath })) as DirEntry[] } - async readFile(filePath: string): Promise { + async readFile(filePath: string, limits?: FileReadLimits): Promise { // Why: streaming is the default path so previews above the legacy single- // frame budget (~12 MB after base64) don't hit MAX_MESSAGE_SIZE. Old relays // that don't implement fs.readFileStream surface as MethodNotFound; we fall // back to the legacy single-shot fs.readFile (which retains the old 10 MB // cap on those hosts). try { - return await readFileViaStream(this.mux, filePath) + return await readFileViaStream(this.mux, filePath, limits) } catch (err) { if (isMethodNotFoundError(err)) { if (!this.loggedStreamFallback) { diff --git a/src/main/providers/types.ts b/src/main/providers/types.ts index 23c36859a1f..aae7266ecaa 100644 --- a/src/main/providers/types.ts +++ b/src/main/providers/types.ts @@ -18,6 +18,7 @@ export type { // ─── Filesystem Provider ──────────────────────────────────────────── export type { + FileReadLimits, FileReadResult, FileStat, FileUploadSession, diff --git a/src/main/runtime/orca-runtime-files.test.ts b/src/main/runtime/orca-runtime-files.test.ts index de78d21b10b..608599f4a67 100644 --- a/src/main/runtime/orca-runtime-files.test.ts +++ b/src/main/runtime/orca-runtime-files.test.ts @@ -2,6 +2,7 @@ authorization, and watcher lifecycle fixtures; splitting would duplicate the setup that makes cross-command filesystem behavior comparable. */ import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import { FileReadCapExceededError, StreamProtocolError } from '../ssh/ssh-filesystem-stream-reader' import { EventEmitter } from 'node:events' import { link, mkdtemp, readFile, realpath, rm, symlink, writeFile } from 'node:fs/promises' import { tmpdir } from 'node:os' @@ -104,7 +105,12 @@ vi.mock('../providers/ssh-filesystem-dispatch', () => ({ 'Remote connection dropped. Click Reconnect on the SSH target before retrying.' })) -import { awaitRuntimeFileWatcherUnsubscribes, RuntimeFileCommands } from './orca-runtime-files' +import { + awaitRuntimeFileWatcherUnsubscribes, + RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES, + RuntimeFileCommands +} from './orca-runtime-files' +import { REMOTE_RPC_MAX_CONTENT_BYTES } from '../../shared/remote-rpc-content-budget' import { getSshFilesystemProvider } from '../providers/ssh-filesystem-dispatch' import { resetSshConnectionGenerations, @@ -2326,6 +2332,30 @@ describe('RuntimeFileCommands', () => { ).rejects.toThrow('terminal_file_grant_stale') }) + it('keeps local terminal artifact previews above the remote cap available', async () => { + const size = RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES + 1 + const artifactPath = await tempFile('result.png', 'a'.repeat(size)) + const { commands } = createRuntimeFileCommands({ path: '/repo' }) + resolveAuthorizedPathMock.mockImplementation(async (p: string) => p) + + const result = await resolveTerminalArtifactPath(commands, artifactPath) + const target = absoluteFileTarget(result) + + await expect( + commands.readTerminalArtifactPreview( + 'id:wt-1', + target.grantId, + target.absolutePath, + 'client-a' + ) + ).resolves.toMatchObject({ + content: Buffer.alloc(size, 0x61).toString('base64'), + isBinary: true, + isImage: true, + mimeType: 'image/png' + }) + }) + it('rejects binary-extension terminal artifacts from the editable text path', async () => { const artifactPath = await tempFile('report.pdf', '%PDF text-looking bytes') const { commands } = createRuntimeFileCommands({ path: '/repo' }) @@ -2494,6 +2524,28 @@ describe('RuntimeFileCommands', () => { expect(readTerminalArtifact).not.toHaveBeenCalled() }) + it('rejects additive remote terminal preview fields beyond the request budget', async () => { + const { commands, readTerminalArtifact } = + createRemoteTerminalArtifactGrantFixture('/tmp/result.png') + const result = await resolveTerminalArtifactPath(commands, '/tmp/result.png') + const target = absoluteFileTarget(result) + readTerminalArtifact.mockResolvedValue({ + content: 'a', + isBinary: true, + futureMetadata: 'x'.repeat(128) + }) + + await expect( + commands.readTerminalArtifactPreview( + 'id:wt-1', + target.grantId, + target.absolutePath, + 'client-a', + 128 + ) + ).rejects.toThrow('file_too_large') + }) + it('rejects remote terminal artifact writes when a grant no longer resolves to the granted path', async () => { const { commands, readTerminalArtifact, writeTerminalArtifact, moveArtifactTarget } = createRemoteTerminalArtifactGrantFixture() @@ -2581,4 +2633,189 @@ describe('RuntimeFileCommands', () => { ) }) }) + + // Why: mobile opens every image tab through files.readPreview, so this constant is the most + // reachable way to overflow the outbound envelope and kill the socket. + describe('previewable binary budget', () => { + const previewTempDirs: string[] = [] + + afterEach(async () => { + await Promise.all(previewTempDirs.map((dir) => rm(dir, { recursive: true, force: true }))) + previewTempDirs.length = 0 + }) + + async function previewFixture(size = Buffer.byteLength('fake-png')): Promise { + const dir = await mkdtemp(join(tmpdir(), 'orca-preview-budget-')) + previewTempDirs.push(dir) + await writeFile(join(dir, 'logo.png'), Buffer.alloc(size, 0x61)) + return dir + } + + it('stays inside the transport ceiling once base64-inflated', () => { + const result = { + content: Buffer.alloc(RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES).toString('base64'), + isBinary: true, + isImage: true, + mimeType: 'image/png' + } + + expect(Buffer.byteLength(JSON.stringify(result), 'utf8')).toBeLessThanOrEqual( + REMOTE_RPC_MAX_CONTENT_BYTES + ) + }) + + it('rejects a previewable image one byte above the cap', async () => { + const dir = await previewFixture(RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES + 1) + const { commands } = createRuntimeFileCommands({ path: dir }) + resolveAuthorizedPathMock.mockImplementation(async (p: string) => p) + + await expect( + commands.readFileExplorerPreview('id:wt-1', 'logo.png', REMOTE_RPC_MAX_CONTENT_BYTES) + ).rejects.toThrow('file_too_large') + }) + + it('returns full base64 for a previewable image at the cap', async () => { + const dir = await previewFixture(RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES) + const { commands } = createRuntimeFileCommands({ path: dir }) + resolveAuthorizedPathMock.mockImplementation(async (p: string) => p) + + await expect( + commands.readFileExplorerPreview('id:wt-1', 'logo.png', REMOTE_RPC_MAX_CONTENT_BYTES) + ).resolves.toEqual({ + content: Buffer.alloc(RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES, 0x61).toString('base64'), + isBinary: true, + isImage: true, + mimeType: 'image/png' + }) + }) + + it('keeps local previews above the remote cap available without a request budget', async () => { + const size = RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES + 1 + const dir = await previewFixture(size) + const { commands } = createRuntimeFileCommands({ path: dir }) + resolveAuthorizedPathMock.mockImplementation(async (p: string) => p) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'logo.png')).resolves.toEqual({ + content: Buffer.alloc(size, 0x61).toString('base64'), + isBinary: true, + isImage: true, + mimeType: 'image/png' + }) + }) + + it('rejects an SSH text preview past the decoded text limit the local branch enforces', async () => { + const { commands, store } = createRuntimeFileCommands({ path: '/repo' }) + store.getRepo.mockReturnValue({ connectionId: 'ssh-1' }) + // NUL-free control bytes: sniffed as text, yet each escapes to six JSON bytes. + const content = '\u0001'.repeat(1024 * 1024) + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'file', size: content.length }), + readFile: vi.fn().mockResolvedValue({ content, isBinary: false }) + } as never) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'log.txt')).rejects.toThrow( + 'file_too_large' + ) + }) + + it('still returns an SSH binary preview inside the base64 cap', async () => { + const { commands, store } = createRuntimeFileCommands({ path: '/repo' }) + store.getRepo.mockReturnValue({ connectionId: 'ssh-1' }) + const preview = { content: 'a'.repeat(1024 * 1024), isBinary: true, isImage: true } + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi + .fn() + .mockResolvedValue({ type: 'file', size: RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES }), + readFile: vi.fn().mockResolvedValue(preview) + } as never) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'logo.png')).resolves.toEqual( + preview + ) + }) + + it('rejects an SSH binary result that grew past its request-scoped budget', async () => { + const { commands, store } = createRuntimeFileCommands({ path: '/repo' }) + store.getRepo.mockReturnValue({ connectionId: 'ssh-1' }) + const readFile = vi.fn().mockResolvedValue({ + content: 'a'.repeat(13), + isBinary: true, + isImage: true + }) + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'file', size: 0 }), + readFile + } as never) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'logo.png', 12)).rejects.toThrow( + 'file_too_large' + ) + expect(readFile).toHaveBeenCalledWith('/repo/logo.png', { + maxBinaryBytes: 0, + maxTextBytes: 512 * 1024 + }) + }) + + it('rejects escape-dense SSH text beyond the request-scoped result budget', async () => { + const { commands, store } = createRuntimeFileCommands({ path: '/repo' }) + store.getRepo.mockReturnValue({ connectionId: 'ssh-1' }) + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'file', size: 64 }), + readFile: vi.fn().mockResolvedValue({ content: '\u0001'.repeat(64), isBinary: false }) + } as never) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'log.txt', 128)).rejects.toThrow( + 'file_too_large' + ) + }) + + // Why: without translation the reader's raw "exceeds client cap" string reaches the client as a + // generic runtime_error, which neither the desktop nor the mobile preview arm recognizes. + it('translates an over-cap stream read into file_too_large', async () => { + const { commands, store } = createRuntimeFileCommands({ path: '/repo' }) + store.getRepo.mockReturnValue({ connectionId: 'ssh-1' }) + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'file', size: 1024 }), + readFile: vi + .fn() + .mockRejectedValue( + new FileReadCapExceededError('Reported totalSize 900000 exceeds client cap 524288') + ) + } as never) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'log.txt')).rejects.toThrow( + 'file_too_large' + ) + }) + + it('leaves a genuine stream protocol failure unmasked', async () => { + const { commands, store } = createRuntimeFileCommands({ path: '/repo' }) + store.getRepo.mockReturnValue({ connectionId: 'ssh-1' }) + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'file', size: 1024 }), + readFile: vi.fn().mockRejectedValue(new StreamProtocolError('Malformed chunk for stream 4')) + } as never) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'log.txt')).rejects.toThrow( + 'Malformed chunk' + ) + }) + + it('rejects oversized SSH preview metadata with small content', async () => { + const { commands, store } = createRuntimeFileCommands({ path: '/repo' }) + store.getRepo.mockReturnValue({ connectionId: 'ssh-1' }) + vi.mocked(getSshFilesystemProvider).mockReturnValue({ + stat: vi.fn().mockResolvedValue({ type: 'file', size: 1 }), + readFile: vi.fn().mockResolvedValue({ + content: 'a', + isBinary: true, + mimeType: 'x'.repeat(128) + }) + } as never) + + await expect(commands.readFileExplorerPreview('id:wt-1', 'logo.png', 128)).rejects.toThrow( + 'file_too_large' + ) + }) + }) }) diff --git a/src/main/runtime/orca-runtime-files.ts b/src/main/runtime/orca-runtime-files.ts index 826f8a9e2d3..c99b3e8a5e9 100644 --- a/src/main/runtime/orca-runtime-files.ts +++ b/src/main/runtime/orca-runtime-files.ts @@ -10,7 +10,6 @@ import { lstat, mkdir, open, - readFile, readdir, rename, realpath, @@ -31,6 +30,10 @@ import { relativePathInsideRoot, resolveRuntimePath } from '../../shared/cross-platform-path' +import { + REMOTE_RPC_MAX_CONTENT_BYTES, + remoteRpcResultExceedsContentBudget +} from '../../shared/remote-rpc-content-budget' import { PhysicalExitTracker } from '../../shared/physical-exit-tracker' import { sortDirEntries } from '../../shared/file-name-sort' import type { @@ -76,7 +79,8 @@ import { onSshFilesystemProviderRegistered, SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE } from '../providers/ssh-filesystem-dispatch' -import type { FileStat, IFilesystemProvider } from '../providers/types' +import type { FileReadLimits, FileStat, IFilesystemProvider } from '../providers/types' +import { FileReadCapExceededError } from '../ssh/ssh-filesystem-stream-reader' import { isWatcherProcessFailure, WatcherProcessFailure @@ -90,13 +94,68 @@ import { beginWatcherInstall } from '../ipc/watcher-removal-gate' import { assertSshMutationExpectation } from '../ssh/ssh-connection-generation' import { toSshExecutionHostId, type ExecutionHostId } from '../../shared/execution-host' import { renameLocalPathSerializedByDestination } from '../destination-serialized-local-rename' +import { + NodeFileReadTooLargeError, + readNodeFileWithinLimit +} from '../../shared/node-bounded-file-reader' const MOBILE_FILE_LIST_LIMIT = 5000 const MOBILE_FILE_PATH_SEARCH_CACHE_LIMIT = 20_000 const MOBILE_FILE_PATH_SEARCH_CACHE_ENTRIES = 8 const MOBILE_FILE_PATH_SEARCH_CACHE_TTL_MS = 30_000 const MOBILE_FILE_READ_MAX_BYTES = 512 * 1024 -const RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES = 10 * 1024 * 1024 +const LOCAL_PREVIEWABLE_BINARY_MAX_BYTES = 10 * 1024 * 1024 +const PREVIEWABLE_BINARY_EMPTY_RESULT_BYTES = Buffer.byteLength( + JSON.stringify({ + content: '', + isBinary: true, + isImage: true, + mimeType: 'application/octet-stream' + }), + 'utf8' +) +const PREVIEW_CONTENT_FIELDS = ['content'] as const + +function previewableBinaryByteLimit(maxContentBytes: number): number { + const base64Bytes = Math.max(0, maxContentBytes - PREVIEWABLE_BINARY_EMPTY_RESULT_BYTES) + return Math.floor(base64Bytes / 4) * 3 +} + +// Why: the stream reader aborts an over-cap read with a raw protocol message; clients key on +// `file_too_large`, so translate it here rather than surfacing internal stream wording. +async function readPreviewFileWithinCap( + provider: IFilesystemProvider, + filePath: string, + limits: FileReadLimits +): Promise { + try { + return await provider.readFile(filePath, limits) + } catch (error) { + if (error instanceof FileReadCapExceededError) { + throw new Error('file_too_large') + } + throw error + } +} + +function assertPreviewWithinTransportBudget( + result: RuntimeFilePreviewResult, + maxContentBytes: number | undefined +): RuntimeFilePreviewResult { + if ( + maxContentBytes !== undefined && + remoteRpcResultExceedsContentBudget(result, maxContentBytes, PREVIEW_CONTENT_FIELDS) + ) { + throw new Error('file_too_large') + } + return result +} + +// Why: previews are reachable only over RPC and base64 inflates them 4/3, so derive the cap from the +// transport ceiling — a hardcoded 10 MiB serializes past the outbound envelope and kills the socket. +export const RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES = previewableBinaryByteLimit( + REMOTE_RPC_MAX_CONTENT_BYTES +) const WINDOWS_RUNTIME_FILE_WATCH_DEBOUNCE_MS = 150 export const WINDOWS_RUNTIME_FILE_WATCH_CLOSE_DEADLINE_MS = 10_000 const TERMINAL_FILE_GRANT_TTL_MS = 10 * 60 * 1000 @@ -1159,7 +1218,8 @@ export class RuntimeFileCommands { worktreeSelector: string, grantId: string, absolutePath: string, - clientId?: string + clientId?: string, + maxContentBytes?: number ): Promise { const { grant } = await this.requireTerminalFileGrant( worktreeSelector, @@ -1170,13 +1230,20 @@ export class RuntimeFileCommands { if (grant.connectionId) { const provider = await this.assertRemoteTerminalFileGrantFreshForRead(grant) this.refreshTerminalFileGrant(grant) - return this.readRemoteTerminalArtifactPreview(provider, grant) + return assertPreviewWithinTransportBudget( + await this.readRemoteTerminalArtifactPreview(provider, grant, maxContentBytes), + maxContentBytes + ) } const handle = await openLocalTerminalArtifactGrant(grant, constants.O_RDONLY) try { - const preview = await readLocalTerminalArtifactPreviewFromHandle(handle, grant) + const preview = await readLocalTerminalArtifactPreviewFromHandle( + handle, + grant, + maxContentBytes + ) this.refreshTerminalFileGrant(grant) - return preview + return assertPreviewWithinTransportBudget(preview, maxContentBytes) } finally { await handle.close() } @@ -1273,19 +1340,27 @@ export class RuntimeFileCommands { private async readRemoteTerminalArtifactPreview( provider: IFilesystemProvider, - grant: TerminalFileGrant + grant: TerminalFileGrant, + maxContentBytes: number | undefined ): Promise { - const preview = await this.readRemoteTerminalArtifact( - provider, - grant, - RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES - ) + const binaryMaxBytes = + maxContentBytes === undefined + ? LOCAL_PREVIEWABLE_BINARY_MAX_BYTES + : previewableBinaryByteLimit(maxContentBytes) + const preview = await this.readRemoteTerminalArtifact(provider, grant, binaryMaxBytes) if ( !preview.isBinary && Buffer.byteLength(preview.content, 'utf8') > MOBILE_FILE_READ_MAX_BYTES ) { throw new Error('file_too_large') } + if ( + preview.isBinary && + maxContentBytes !== undefined && + Buffer.byteLength(preview.content, 'utf8') > maxContentBytes + ) { + throw new Error('file_too_large') + } return preview } @@ -1496,8 +1571,13 @@ export class RuntimeFileCommands { async readFileExplorerPreview( worktreeSelector: string, - relativePath: string + relativePath: string, + maxContentBytes?: number ): Promise { + const binaryMaxBytes = + maxContentBytes === undefined + ? LOCAL_PREVIEWABLE_BINARY_MAX_BYTES + : previewableBinaryByteLimit(maxContentBytes) const target = await this.resolveFileExplorerPath(worktreeSelector, relativePath) const provider = target.connectionId ? getSshFilesystemProvider(target.connectionId) : null if (target.connectionId) { @@ -1505,37 +1585,62 @@ export class RuntimeFileCommands { throw new Error(SSH_FILESYSTEM_PROVIDER_UNAVAILABLE_MESSAGE) } const fileStats = await provider.stat(target.path) - if (fileStats.size > RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES) { + if (fileStats.size > binaryMaxBytes) { throw new Error('file_too_large') } - const result = await provider.readFile(target.path) - return result + const result = await readPreviewFileWithinCap(provider, target.path, { + maxBinaryBytes: binaryMaxBytes, + maxTextBytes: MOBILE_FILE_READ_MAX_BYTES + }) + // Why: the stat gate sizes base64 binaries; text crosses the wire JSON-escaped (up to 6x), so + // hold it to the same decoded limit the local branch enforces before reading. + if ( + !result.isBinary && + Buffer.byteLength(result.content, 'utf8') > MOBILE_FILE_READ_MAX_BYTES + ) { + throw new Error('file_too_large') + } + if ( + result.isBinary && + maxContentBytes !== undefined && + Buffer.byteLength(result.content, 'utf8') > maxContentBytes + ) { + throw new Error('file_too_large') + } + return assertPreviewWithinTransportBudget(result, maxContentBytes) } const filePath = await resolveAuthorizedPath(target.path, this.host.requireStore()) - const fileStats = await stat(filePath) const mimeType = RUNTIME_PREVIEWABLE_BINARY_MIME_TYPES[extname(filePath).toLowerCase()] - if (mimeType) { - if (fileStats.size > RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES) { + const maxBytes = mimeType ? binaryMaxBytes : MOBILE_FILE_READ_MAX_BYTES + let buffer: Buffer + try { + buffer = (await readNodeFileWithinLimit(filePath, maxBytes)).buffer + } catch (error) { + if (error instanceof NodeFileReadTooLargeError) { throw new Error('file_too_large') } - const buffer = await readFile(filePath) - return { - content: buffer.toString('base64'), - isBinary: true, - isImage: true, - mimeType - } + throw error + } + if (mimeType) { + return assertPreviewWithinTransportBudget( + { + content: buffer.toString('base64'), + isBinary: true, + isImage: true, + mimeType + }, + maxContentBytes + ) } - if (fileStats.size > MOBILE_FILE_READ_MAX_BYTES) { - throw new Error('file_too_large') - } - const buffer = await readFile(filePath) if (isBinaryBuffer(buffer)) { - return { content: '', isBinary: true } + return assertPreviewWithinTransportBudget({ content: '', isBinary: true }, maxContentBytes) } - return { content: buffer.toString('utf-8'), isBinary: false } + return assertPreviewWithinTransportBudget( + { content: buffer.toString('utf-8'), isBinary: false }, + maxContentBytes + ) } async readFileExplorerChunk( @@ -2386,7 +2491,8 @@ async function readLocalTerminalArtifactFileFromHandle( async function readLocalTerminalArtifactPreviewFromHandle( handle: FileHandle, - grant: TerminalFileGrant + grant: TerminalFileGrant, + maxContentBytes: number | undefined ): Promise { const fileStats = await handle.stat() if (fileStats.isDirectory()) { @@ -2395,13 +2501,17 @@ async function readLocalTerminalArtifactPreviewFromHandle( assertTerminalFileGrantFresh(grant, fileStats) const mimeType = RUNTIME_PREVIEWABLE_BINARY_MIME_TYPES[extname(grant.absolutePath).toLowerCase()] if (mimeType) { - if (fileStats.size > RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES) { + const binaryMaxBytes = + maxContentBytes === undefined + ? LOCAL_PREVIEWABLE_BINARY_MAX_BYTES + : previewableBinaryByteLimit(maxContentBytes) + if (fileStats.size > binaryMaxBytes) { + throw new Error('file_too_large') + } + const buffer = await readFileHandleBufferBounded(handle, binaryMaxBytes + 1) + if (buffer.byteLength > binaryMaxBytes) { throw new Error('file_too_large') } - const buffer = await readFileHandleBufferBounded( - handle, - RUNTIME_PREVIEWABLE_BINARY_MAX_BYTES + 1 - ) return { content: buffer.toString('base64'), isBinary: true, diff --git a/src/main/runtime/orca-runtime-git-diff-budget.test.ts b/src/main/runtime/orca-runtime-git-diff-budget.test.ts new file mode 100644 index 00000000000..2e30f7f931e --- /dev/null +++ b/src/main/runtime/orca-runtime-git-diff-budget.test.ts @@ -0,0 +1,198 @@ +// Why: the cap lives in orca-runtime-git.ts so both branches of all three diff readers are covered — +// an SSH host forwards its provider's payload verbatim, so an older relay cannot be relied on to clamp it. +import { beforeEach, describe, expect, it, vi } from 'vitest' +import { REMOTE_RPC_MAX_CONTENT_BYTES } from '../../shared/remote-rpc-content-budget' +import type { GitDiffResult } from '../../shared/git-diff-compare-types' +import type { GlobalSettings } from '../../shared/global-settings-types' +import type * as GitStatusModule from '../git/status' +import { RuntimeGitCommands, type ResolvedRuntimeGitWorktree } from './orca-runtime-git' + +const mocks = vi.hoisted(() => ({ + getSshGitProvider: vi.fn(), + getDiff: vi.fn(), + getBranchDiff: vi.fn(), + getCommitDiff: vi.fn() +})) + +vi.mock('../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: mocks.getSshGitProvider +})) + +vi.mock('../git/status', async () => ({ + ...(await vi.importActual('../git/status')), + getDiff: mocks.getDiff, + getBranchDiff: mocks.getBranchDiff, + getCommitDiff: mocks.getCommitDiff +})) + +const OVERSIZED_BASE64 = 'A'.repeat(REMOTE_RPC_MAX_CONTENT_BYTES + 1) +const BRANCH_COMPARE = { mergeBase: 'base-oid', headOid: 'head-oid' } +const COMMIT_ARGS = { + commitOid: 'commit-oid', + parentOid: 'parent-oid', + filePath: 'assets/logo.png' +} +const TOO_LARGE = { code: 'diff_too_large', data: { maxBytes: REMOTE_RPC_MAX_CONTENT_BYTES } } + +function oversizedResult(): GitDiffResult { + return { + kind: 'binary', + originalContent: '', + modifiedContent: OVERSIZED_BASE64, + originalIsBinary: false, + modifiedIsBinary: true + } +} + +function commands(connectionId?: string): RuntimeGitCommands { + const worktree = { + id: 'wt-1', + repoId: 'repo-1', + path: '/remote/repo', + git: { path: '/remote/repo', branch: 'main', isBare: false, isMainWorktree: false } + } as unknown as ResolvedRuntimeGitWorktree + return new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ + worktree, + ...(connectionId ? { connectionId } : {}) + }), + getRuntimeSettings: () => ({}) as GlobalSettings + }) +} + +function sshProvider(): { + getDiff: ReturnType + getBranchDiff: ReturnType + getCommitDiff: ReturnType +} { + return { + getDiff: vi.fn().mockResolvedValue(oversizedResult()), + getBranchDiff: vi.fn().mockResolvedValue([oversizedResult()]), + getCommitDiff: vi.fn().mockResolvedValue(oversizedResult()) + } +} + +describe('runtime git diff transport budget', () => { + beforeEach(() => { + mocks.getSshGitProvider.mockReset() + mocks.getDiff.mockReset().mockResolvedValue(oversizedResult()) + mocks.getBranchDiff.mockReset().mockResolvedValue(oversizedResult()) + mocks.getCommitDiff.mockReset().mockResolvedValue(oversizedResult()) + }) + + it('caps an SSH-forwarded diff that exceeds the budget', async () => { + const provider = sshProvider() + mocks.getSshGitProvider.mockReturnValue(provider) + + await expect( + commands('conn-1').getRuntimeGitDiff( + 'id:wt-1', + 'assets/logo.png', + false, + undefined, + REMOTE_RPC_MAX_CONTENT_BYTES + ) + ).rejects.toMatchObject(TOO_LARGE) + expect(provider.getDiff).toHaveBeenCalledWith( + '/remote/repo', + 'assets/logo.png', + false, + undefined + ) + expect(mocks.getDiff).not.toHaveBeenCalled() + }) + + it('leaves an SSH-forwarded diff uncapped when no budget is supplied', async () => { + mocks.getSshGitProvider.mockReturnValue(sshProvider()) + + await expect( + commands('conn-1').getRuntimeGitDiff('id:wt-1', 'assets/logo.png', false) + ).resolves.toMatchObject({ modifiedContent: OVERSIZED_BASE64 }) + }) + + it('caps a local-repo diff that exceeds the budget', async () => { + await expect( + commands().getRuntimeGitDiff( + 'id:wt-1', + 'assets/logo.png', + false, + undefined, + REMOTE_RPC_MAX_CONTENT_BYTES + ) + ).rejects.toMatchObject(TOO_LARGE) + expect(mocks.getDiff).toHaveBeenCalled() + expect(mocks.getSshGitProvider).not.toHaveBeenCalled() + }) + + it('leaves a local-repo diff uncapped when no budget is supplied', async () => { + await expect( + commands().getRuntimeGitDiff('id:wt-1', 'assets/logo.png', false) + ).resolves.toMatchObject({ modifiedContent: OVERSIZED_BASE64 }) + }) + + it('caps an SSH-forwarded branch diff that exceeds the budget', async () => { + const provider = sshProvider() + mocks.getSshGitProvider.mockReturnValue(provider) + + await expect( + commands('conn-1').getRuntimeGitBranchDiff( + 'id:wt-1', + BRANCH_COMPARE, + 'assets/logo.png', + undefined, + REMOTE_RPC_MAX_CONTENT_BYTES + ) + ).rejects.toMatchObject(TOO_LARGE) + expect(provider.getBranchDiff).toHaveBeenCalled() + expect(mocks.getBranchDiff).not.toHaveBeenCalled() + }) + + it('caps a local-repo branch diff that exceeds the budget', async () => { + await expect( + commands().getRuntimeGitBranchDiff( + 'id:wt-1', + BRANCH_COMPARE, + 'assets/logo.png', + undefined, + REMOTE_RPC_MAX_CONTENT_BYTES + ) + ).rejects.toMatchObject(TOO_LARGE) + expect(mocks.getBranchDiff).toHaveBeenCalled() + }) + + it('leaves a local-repo branch diff uncapped when no budget is supplied', async () => { + await expect( + commands().getRuntimeGitBranchDiff('id:wt-1', BRANCH_COMPARE, 'assets/logo.png') + ).resolves.toMatchObject({ modifiedContent: OVERSIZED_BASE64 }) + }) + + it('caps an SSH-forwarded commit diff that exceeds the budget', async () => { + const provider = sshProvider() + mocks.getSshGitProvider.mockReturnValue(provider) + + await expect( + commands('conn-1').getRuntimeGitCommitDiff( + 'id:wt-1', + COMMIT_ARGS, + REMOTE_RPC_MAX_CONTENT_BYTES + ) + ).rejects.toMatchObject(TOO_LARGE) + expect(provider.getCommitDiff).toHaveBeenCalled() + expect(mocks.getCommitDiff).not.toHaveBeenCalled() + }) + + it('caps a local-repo commit diff that exceeds the budget', async () => { + await expect( + commands().getRuntimeGitCommitDiff('id:wt-1', COMMIT_ARGS, REMOTE_RPC_MAX_CONTENT_BYTES) + ).rejects.toMatchObject(TOO_LARGE) + expect(mocks.getCommitDiff).toHaveBeenCalled() + }) + + it('leaves a local-repo commit diff uncapped when no budget is supplied', async () => { + await expect(commands().getRuntimeGitCommitDiff('id:wt-1', COMMIT_ARGS)).resolves.toMatchObject( + { + modifiedContent: OVERSIZED_BASE64 + } + ) + }) +}) diff --git a/src/main/runtime/orca-runtime-git.ts b/src/main/runtime/orca-runtime-git.ts index 3d802d219ea..c90f6cfef27 100644 --- a/src/main/runtime/orca-runtime-git.ts +++ b/src/main/runtime/orca-runtime-git.ts @@ -16,6 +16,7 @@ import type { Repo } from '../../shared/repo-types' import type { TuiAgent } from '../../shared/tui-agent' import type { GitPushTarget, GitWorktreeInfo, Worktree } from '../../shared/worktree/types' import type { CommitMessageDraftContext } from '../../shared/commit-message-generation' +import { assertGitDiffWithinTransportBudget } from '../../shared/git-diff-transport-budget' import { getCommitMessageModelDiscoveryHostKey } from '../../shared/commit-message-host-key' import type { GitHistoryOptions, GitHistoryResult } from '../../shared/git-history' import { @@ -341,11 +342,14 @@ export class RuntimeGitCommands { return listLocalBranches(target.worktree.path, localGitOptionsForTarget(target)) } + // Why: the budget is enforced here, after both branches, so an SSH payload forwarded verbatim from + // an older relay is capped too. async getRuntimeGitDiff( worktreeSelector: string, filePath: string, staged: boolean, - compareAgainstHead?: boolean + compareAgainstHead?: boolean, + maxContentBytes?: number ): Promise { const target = await this.host.resolveRuntimeGitTarget(worktreeSelector) const relativePath = normalizeRuntimeGitRelativePath(filePath) @@ -354,14 +358,20 @@ export class RuntimeGitCommands { if (!provider) { throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) } - return provider.getDiff(target.worktree.path, relativePath, staged, compareAgainstHead) + return assertGitDiffWithinTransportBudget( + await provider.getDiff(target.worktree.path, relativePath, staged, compareAgainstHead), + maxContentBytes + ) } - return getDiff( - target.worktree.path, - relativePath, - staged, - compareAgainstHead, - localGitOptionsForTarget(target) + return assertGitDiffWithinTransportBudget( + await getDiff( + target.worktree.path, + relativePath, + staged, + compareAgainstHead, + localGitOptionsForTarget(target) + ), + maxContentBytes ) } @@ -522,7 +532,8 @@ export class RuntimeGitCommands { worktreeSelector: string, compare: { mergeBase: string; headOid: string }, filePath: string, - oldPath?: string + oldPath?: string, + maxContentBytes?: number ): Promise { const target = await this.host.resolveRuntimeGitTarget(worktreeSelector) const relativePath = normalizeRuntimeGitRelativePath(filePath) @@ -538,31 +549,36 @@ export class RuntimeGitCommands { filePath: relativePath, oldPath: oldRelativePath }) - return ( + return assertGitDiffWithinTransportBudget( results[0] ?? { kind: 'text', originalContent: '', modifiedContent: '', originalIsBinary: false, modifiedIsBinary: false - } + }, + maxContentBytes ) } - return getBranchDiff( - target.worktree.path, - { - mergeBase: compare.mergeBase, - headOid: compare.headOid, - filePath: relativePath, - oldPath: oldRelativePath - }, - localGitOptionsForTarget(target) + return assertGitDiffWithinTransportBudget( + await getBranchDiff( + target.worktree.path, + { + mergeBase: compare.mergeBase, + headOid: compare.headOid, + filePath: relativePath, + oldPath: oldRelativePath + }, + localGitOptionsForTarget(target) + ), + maxContentBytes ) } async getRuntimeGitCommitDiff( worktreeSelector: string, - args: { commitOid: string; parentOid?: string | null; filePath: string; oldPath?: string } + args: { commitOid: string; parentOid?: string | null; filePath: string; oldPath?: string }, + maxContentBytes?: number ): Promise { const target = await this.host.resolveRuntimeGitTarget(worktreeSelector) const relativePath = normalizeRuntimeRelativePath(args.filePath) @@ -572,22 +588,28 @@ export class RuntimeGitCommands { if (!provider) { throw new Error(SSH_GIT_PROVIDER_UNAVAILABLE_MESSAGE) } - return provider.getCommitDiff(target.worktree.path, { - commitOid: args.commitOid, - parentOid: args.parentOid, - filePath: relativePath, - oldPath: oldRelativePath - }) + return assertGitDiffWithinTransportBudget( + await provider.getCommitDiff(target.worktree.path, { + commitOid: args.commitOid, + parentOid: args.parentOid, + filePath: relativePath, + oldPath: oldRelativePath + }), + maxContentBytes + ) } - return getCommitDiff( - target.worktree.path, - { - commitOid: args.commitOid, - parentOid: args.parentOid, - filePath: relativePath, - oldPath: oldRelativePath - }, - localGitOptionsForTarget(target) + return assertGitDiffWithinTransportBudget( + await getCommitDiff( + target.worktree.path, + { + commitOid: args.commitOid, + parentOid: args.parentOid, + filePath: relativePath, + oldPath: oldRelativePath + }, + localGitOptionsForTarget(target) + ), + maxContentBytes ) } diff --git a/src/main/runtime/orca-runtime.test.ts b/src/main/runtime/orca-runtime.test.ts index 49a5a3db094..f651bb7e3fa 100644 --- a/src/main/runtime/orca-runtime.test.ts +++ b/src/main/runtime/orca-runtime.test.ts @@ -3880,7 +3880,10 @@ describe('OrcaRuntimeService', () => { expect(fsProvider.stat).toHaveBeenCalledWith(folderPath) expect(fsProvider.readDir).toHaveBeenCalledWith('/srv/platform/src') expect(fsProvider.stat).toHaveBeenCalledWith('/srv/platform/src/app.ts') - expect(fsProvider.readFile).toHaveBeenCalledWith('/srv/platform/src/app.ts') + expect(fsProvider.readFile).toHaveBeenCalledWith('/srv/platform/src/app.ts', { + maxBinaryBytes: 10 * 1024 * 1024, + maxTextBytes: 512 * 1024 + }) }) it('lists persisted SSH worktrees while the git provider is unavailable', async () => { diff --git a/src/main/runtime/rpc/e2ee-channel-text-backpressure.test.ts b/src/main/runtime/rpc/e2ee-channel-text-backpressure.test.ts index 4e4b5f52ecb..5761ac504aa 100644 --- a/src/main/runtime/rpc/e2ee-channel-text-backpressure.test.ts +++ b/src/main/runtime/rpc/e2ee-channel-text-backpressure.test.ts @@ -3,6 +3,11 @@ import type { WebSocket } from 'ws' import { E2EEChannel, type E2EEChannelOptions } from './e2ee-channel' import { deriveSharedKey, decrypt, encrypt, generateKeyPair } from './e2ee-crypto' import { createMobileE2EEOutboundMemoryBudget } from './mobile-e2ee-outbound-memory-budget' +import { REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES } from '../../../shared/remote-runtime-memory-limits' + +const trackMock = vi.hoisted(() => vi.fn()) + +vi.mock('../../telemetry/client', () => ({ track: trackMock })) // Repro for gap (a): the streaming JSON reply path (encryptedReply) had no // bufferedAmount gate, so a fast producer over a slow link (legacy @@ -64,6 +69,7 @@ function emitReply(ctx: ReturnType, payload: string): void { describe('E2EE text reply backpressure', () => { beforeEach(() => { vi.useFakeTimers() + trackMock.mockReset() }) afterEach(() => { vi.useRealTimers() @@ -100,6 +106,19 @@ describe('E2EE text reply backpressure', () => { expect(decrypt(ctx.ws.sent[baseline]!, ctx.sharedKey)).toBe('{"ok":true}') }) + it('still closes an oversized reply when telemetry throws', () => { + const ctx = setup() + trackMock.mockImplementationOnce(() => { + throw new Error('telemetry unavailable') + }) + + expect(() => + emitReply(ctx, 'x'.repeat(REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES + 1)) + ).not.toThrow() + expect(trackMock).toHaveBeenCalledWith('remote_outbound_budget_close', { emitter: 'size' }) + expect(ctx.onError).toHaveBeenCalledWith(1013, 'Outbound reply buffer overflow') + }) + it('rejects aggregate queue growth across independently backpressured sockets', () => { const outboundMemoryBudget = createMobileE2EEOutboundMemoryBudget({ maxBufferedBytes: 1_000, @@ -116,6 +135,10 @@ describe('E2EE text reply backpressure', () => { expect(first.onError).not.toHaveBeenCalled() expect(second.onError).toHaveBeenCalledWith(1013, 'Outbound reply buffer overflow') + // Why: this close kills the whole remote session, so it has to be countable. + expect(trackMock).toHaveBeenCalledWith('remote_outbound_budget_close', { + emitter: 'queue' + }) first.channel.destroy() expect(outboundMemoryBudget.evidence().queuedBytes).toBe(0) }) diff --git a/src/main/runtime/rpc/e2ee-channel.ts b/src/main/runtime/rpc/e2ee-channel.ts index 1cef6a10213..a5ecbbdde33 100644 --- a/src/main/runtime/rpc/e2ee-channel.ts +++ b/src/main/runtime/rpc/e2ee-channel.ts @@ -18,6 +18,10 @@ import type { MobileE2EEOutboundMemoryBudget } from './mobile-e2ee-outbound-memo import { MobileE2EEDesktopOutboundOwner } from './mobile-e2ee-desktop-outbound-owner' import { parseRuntimeClientCapabilities } from './runtime-client-capabilities' import type { RuntimeCapability } from '../../../shared/protocol-version' +import type { EventProps } from '../../../shared/telemetry-events' +import { track } from '../../telemetry/client' + +type OutboundBudgetEmitter = EventProps<'remote_outbound_budget_close'>['emitter'] const HANDSHAKE_TIMEOUT_MS = 10_000 const MAX_CONSECUTIVE_DECRYPT_FAILURES = 5 @@ -147,13 +151,13 @@ export class E2EEChannel { return } if (!isMobileE2EETextPayloadWithinLimit(response)) { - this.onError(1013, 'Outbound reply buffer overflow') + this.closeForOutboundBudget('size') return } this.outbound.enqueueLegacyText( encrypt(response, this.sharedKey), () => Boolean(this.sharedKey), - () => this.onError(1013, 'Outbound reply buffer overflow') + () => this.closeForOutboundBudget('queue') ) } const encryptedBinaryReply = (response: Uint8Array): boolean => { @@ -161,7 +165,7 @@ export class E2EEChannel { return false } if (!isMobileE2EEBinaryPayloadWithinLimit(response)) { - this.onError(1013, 'Outbound reply buffer overflow') + this.closeForOutboundBudget('size') return false } if (!this.outbound.canSend(response.byteLength + 40)) { @@ -298,12 +302,21 @@ export class E2EEChannel { return false } if (!isMobileE2EEOutboundItemWithinLimit(item)) { - this.onError(1013, 'Outbound reply buffer overflow') + this.closeForOutboundBudget('size') return false } - return this.outbound.enqueueV2(item, this.v2Session, () => - this.onError(1013, 'Outbound reply buffer overflow') - ) + return this.outbound.enqueueV2(item, this.v2Session, () => this.closeForOutboundBudget('queue')) + } + + // Why: this close kills the whole remote session. `size` means a producer emitted something + // too big and should fall to zero once producers cap themselves; `queue` means a backed-up link. + private closeForOutboundBudget(emitter: OutboundBudgetEmitter): void { + try { + track('remote_outbound_budget_close', { emitter }) + } catch { + // Telemetry is best-effort; closing the unsafe socket remains authoritative. + } + this.onError(1013, 'Outbound reply buffer overflow') } private sendEncryptedControl(message: unknown): void { @@ -311,9 +324,7 @@ export class E2EEChannel { this.enqueueV2({ kind: 'text', plaintext: JSON.stringify(message) }) } else if (this.ws.readyState === this.ws.OPEN && this.sharedKey) { const frame = encrypt(JSON.stringify(message), this.sharedKey) - this.outbound.sendLegacyFrame(frame, () => - this.onError(1013, 'Outbound reply buffer overflow') - ) + this.outbound.sendLegacyFrame(frame, () => this.closeForOutboundBudget('queue')) } } diff --git a/src/main/runtime/rpc/errors.ts b/src/main/runtime/rpc/errors.ts index ad5e5f36695..af6fcb6a7b4 100644 --- a/src/main/runtime/rpc/errors.ts +++ b/src/main/runtime/rpc/errors.ts @@ -8,6 +8,7 @@ import { COMPUTER_ERROR_CODES } from '../../../shared/runtime-types' import { LINEAR_ERROR_CODES } from '../../../shared/linear/agent-access' import { AGENT_SESSION_RPC_ERROR_CODES } from '../../../shared/agent-session-host-authority' import { ARTIFACT_SHARING_DISABLED_CODE } from '../../../shared/artifact-sharing-gate' +import { GIT_DIFF_TOO_LARGE_CODE } from '../../../shared/git-diff-transport-budget' export function successResponse(id: string, meta: RpcEnvelopeMeta, result: unknown): RpcSuccess { return { @@ -98,6 +99,7 @@ const STRUCTURED_RUNTIME_PASSTHROUGH_CODES: ReadonlySet = new Set([ 'stale_delivery', 'waiter_exists', 'invalid_argument', + GIT_DIFF_TOO_LARGE_CODE, ARTIFACT_SHARING_DISABLED_CODE ]) diff --git a/src/main/runtime/rpc/methods/files-preview-transport-budget.test.ts b/src/main/runtime/rpc/methods/files-preview-transport-budget.test.ts new file mode 100644 index 00000000000..c36a34983a9 --- /dev/null +++ b/src/main/runtime/rpc/methods/files-preview-transport-budget.test.ts @@ -0,0 +1,65 @@ +import { describe, expect, it, vi } from 'vitest' +import { remoteRpcContentBudget } from '../../../../shared/remote-rpc-content-budget' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { RpcDispatcher } from '../dispatcher' +import type { RpcRequest } from '../core' +import { FILE_METHODS } from './files' + +function makeRequest(method: string, params?: unknown): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +describe('file preview RPC transport budgets', () => { + it.each(['mobile', 'runtime'] as const)( + 'charges the %s request id to both preview content budgets', + async (clientKind) => { + const preview = { + content: 'base64', + isBinary: true, + isImage: true, + mimeType: 'image/png' + } + const readFileExplorerPreview = vi.fn().mockResolvedValue(preview) + const readTerminalArtifactPreview = vi.fn().mockResolvedValue(preview) + const runtime = { + getRuntimeId: () => 'test-runtime', + readFileExplorerPreview, + readTerminalArtifactPreview + } as unknown as OrcaRuntimeService + const dispatcher = new RpcDispatcher({ runtime, methods: FILE_METHODS }) + const id = '\u0001'.repeat(8_192) + const reply = vi.fn() + + await dispatcher.dispatchStreaming( + { + ...makeRequest('files.readPreview', { worktree: 'id:wt-1', relativePath: 'logo.png' }), + id + }, + reply, + { clientKind } + ) + await dispatcher.dispatchStreaming( + { + ...makeRequest('files.readTerminalArtifactPreview', { + worktree: 'id:wt-1', + absolutePath: '/tmp/logo.png', + grantId: 'grant-1' + }), + id + }, + reply, + { clientKind } + ) + + const budget = remoteRpcContentBudget(id) + expect(readFileExplorerPreview).toHaveBeenCalledWith('id:wt-1', 'logo.png', budget) + expect(readTerminalArtifactPreview).toHaveBeenCalledWith( + 'id:wt-1', + 'grant-1', + '/tmp/logo.png', + undefined, + budget + ) + } + ) +}) diff --git a/src/main/runtime/rpc/methods/files.ts b/src/main/runtime/rpc/methods/files.ts index 741fc65635f..486a63feb9e 100644 --- a/src/main/runtime/rpc/methods/files.ts +++ b/src/main/runtime/rpc/methods/files.ts @@ -2,10 +2,18 @@ import { z } from 'zod' import { defineMethod, defineStreamingMethod, type RpcAnyMethod } from '../core' import { runFileWatchStream } from './file-watch-stream-lifecycle' +import { remoteRpcContentBudget } from '../../../../shared/remote-rpc-content-budget' let filesWatchSubscriptionSeq = 0 const RUNTIME_FILE_BASE64_PATTERN = /^[A-Za-z0-9+/]*={0,2}$/ +function remoteFileContentBudget( + clientKind: 'mobile' | 'runtime' | undefined, + requestId: string | undefined +): number | undefined { + return clientKind && requestId ? remoteRpcContentBudget(requestId) : undefined +} + function isValidRuntimeFileBase64(value: unknown): value is string { return ( typeof value === 'string' && value.length % 4 !== 1 && RUNTIME_FILE_BASE64_PATTERN.test(value) @@ -281,13 +289,23 @@ export const FILE_METHODS: RpcAnyMethod[] = [ defineMethod({ name: 'files.readTerminalArtifactPreview', params: TerminalArtifactFile, - handler: async (params, { runtime, clientId }) => - runtime.readTerminalArtifactPreview( - params.worktree, - params.grantId, - params.absolutePath, - clientId - ) + handler: async (params, { runtime, clientId, clientKind, requestId }) => { + const budget = remoteFileContentBudget(clientKind, requestId) + return budget === undefined + ? runtime.readTerminalArtifactPreview( + params.worktree, + params.grantId, + params.absolutePath, + clientId + ) + : runtime.readTerminalArtifactPreview( + params.worktree, + params.grantId, + params.absolutePath, + clientId, + budget + ) + } }), defineMethod({ name: 'files.writeTerminalArtifact', @@ -304,8 +322,12 @@ export const FILE_METHODS: RpcAnyMethod[] = [ defineMethod({ name: 'files.readPreview', params: FileOpen, - handler: async (params, { runtime }) => - runtime.readFileExplorerPreview(params.worktree, params.relativePath) + handler: async (params, { runtime, clientKind, requestId }) => { + const budget = remoteFileContentBudget(clientKind, requestId) + return budget === undefined + ? runtime.readFileExplorerPreview(params.worktree, params.relativePath) + : runtime.readFileExplorerPreview(params.worktree, params.relativePath, budget) + } }), defineMethod({ name: 'files.readChunk', diff --git a/src/main/runtime/rpc/methods/git-diff-transport-budget.test.ts b/src/main/runtime/rpc/methods/git-diff-transport-budget.test.ts new file mode 100644 index 00000000000..a4b22aaa891 --- /dev/null +++ b/src/main/runtime/rpc/methods/git-diff-transport-budget.test.ts @@ -0,0 +1,176 @@ +// Why: git.diff, git.branchDiff and git.commitDiff all return a GitDiffResult, so capping only the +// first would leave the other two able to kill a remote socket. +import { describe, expect, it, vi } from 'vitest' +import { + REMOTE_RPC_MAX_CONTENT_BYTES, + remoteRpcContentBudget +} from '../../../../shared/remote-rpc-content-budget' +import { assertGitDiffWithinTransportBudget } from '../../../../shared/git-diff-transport-budget' +import type { GitDiffResult } from '../../../../shared/git-diff-compare-types' +import type { GlobalSettings } from '../../../../shared/global-settings-types' +import type { OrcaRuntimeService } from '../../orca-runtime' +import { RuntimeGitCommands, type ResolvedRuntimeGitWorktree } from '../../orca-runtime-git' +import type { RpcRequest, RpcResponse } from '../core' +import { RpcDispatcher } from '../dispatcher' +import { GIT_METHODS } from './git' + +const sshMocks = vi.hoisted(() => ({ getSshGitProvider: vi.fn() })) + +vi.mock('../../../providers/ssh-git-dispatch', () => ({ + getSshGitProvider: sshMocks.getSshGitProvider +})) + +const OVERSIZED_BASE64 = 'A'.repeat(REMOTE_RPC_MAX_CONTENT_BYTES + 1024) + +const OVERSIZED_DIFF: GitDiffResult = { + kind: 'binary', + originalContent: '', + modifiedContent: OVERSIZED_BASE64, + isImage: true, + mimeType: 'image/png', + originalIsBinary: false, + modifiedIsBinary: true +} + +const CASES: readonly { method: string; runtimeMethod: string; params: Record }[] = + [ + { + method: 'git.diff', + runtimeMethod: 'getRuntimeGitDiff', + params: { worktree: 'id:wt-1', filePath: 'assets/logo.png', staged: false } + }, + { + method: 'git.branchDiff', + runtimeMethod: 'getRuntimeGitBranchDiff', + params: { + worktree: 'id:wt-1', + compare: { mergeBase: 'a'.repeat(40), headOid: 'b'.repeat(40) }, + filePath: 'assets/logo.png' + } + }, + { + method: 'git.commitDiff', + runtimeMethod: 'getRuntimeGitCommitDiff', + params: { worktree: 'id:wt-1', commitOid: 'c'.repeat(40), filePath: 'assets/logo.png' } + } + ] + +/** Stands in for orca-runtime-git.ts, which enforces the budget it is handed as its last argument. */ +function stubRuntime(runtimeMethod: string): OrcaRuntimeService { + return { + getRuntimeId: () => 'test-runtime', + [runtimeMethod]: vi.fn(async (...args: unknown[]) => { + const maxContentBytes = args.at(-1) + return assertGitDiffWithinTransportBudget( + OVERSIZED_DIFF, + typeof maxContentBytes === 'number' ? maxContentBytes : undefined + ) + }) + } as unknown as OrcaRuntimeService +} + +function budgetArgument(runtime: OrcaRuntimeService, runtimeMethod: string): unknown { + const spy = (runtime as unknown as Record>)[runtimeMethod]! + return spy.mock.calls[0]!.at(-1) +} + +function makeRequest(method: string, params: Record): RpcRequest { + return { id: 'req-1', authToken: 'tok', method, params } +} + +async function dispatchRemote( + runtime: OrcaRuntimeService, + method: string, + params: Record, + clientKind: 'mobile' | 'runtime' +): Promise { + const dispatcher = new RpcDispatcher({ runtime, methods: GIT_METHODS }) + const replies: string[] = [] + await dispatcher.dispatchStreaming(makeRequest(method, params), (reply) => replies.push(reply), { + clientKind + }) + return JSON.parse(replies[0]!) as RpcResponse +} + +describe('remote git diff transport budget', () => { + it.each(CASES)('caps $method for a mobile client', async ({ method, runtimeMethod, params }) => { + const runtime = stubRuntime(runtimeMethod) + + const response = await dispatchRemote(runtime, method, params, 'mobile') + + expect(budgetArgument(runtime, runtimeMethod)).toBe(remoteRpcContentBudget('req-1')) + expect(response).toMatchObject({ + ok: false, + error: { code: 'diff_too_large', data: { maxBytes: remoteRpcContentBudget('req-1') } } + }) + }) + + it.each(CASES)( + 'caps $method for a remote desktop client', + async ({ method, runtimeMethod, params }) => { + const runtime = stubRuntime(runtimeMethod) + + const response = await dispatchRemote(runtime, method, params, 'runtime') + + expect(budgetArgument(runtime, runtimeMethod)).toBe(remoteRpcContentBudget('req-1')) + expect(response).toMatchObject({ ok: false, error: { code: 'diff_too_large' } }) + } + ) + + it('charges a long request id against the remote content budget', async () => { + const runtime = stubRuntime('getRuntimeGitDiff') + const dispatcher = new RpcDispatcher({ runtime, methods: GIT_METHODS }) + const requestId = '\u0001'.repeat(8_192) + const replies: string[] = [] + + await dispatcher.dispatchStreaming( + { ...makeRequest('git.diff', CASES[0]!.params), id: requestId }, + (reply) => replies.push(reply), + { clientKind: 'mobile' } + ) + + expect(budgetArgument(runtime, 'getRuntimeGitDiff')).toBe(remoteRpcContentBudget(requestId)) + expect(Buffer.byteLength(replies[0]!, 'utf8')).toBeLessThanOrEqual( + REMOTE_RPC_MAX_CONTENT_BYTES + 8 * 1024 + ) + }) + + // Why: the in-process/Unix-socket context sets no clientKind, so desktop-local diffs keep full fidelity. + it.each(CASES)( + 'leaves $method uncapped for a local caller', + async ({ method, runtimeMethod, params }) => { + const runtime = stubRuntime(runtimeMethod) + const dispatcher = new RpcDispatcher({ runtime, methods: GIT_METHODS }) + + const response = await dispatcher.dispatch(makeRequest(method, params)) + + expect(budgetArgument(runtime, runtimeMethod)).toBeUndefined() + expect(response).toMatchObject({ + ok: true, + result: { kind: 'binary', modifiedContent: OVERSIZED_BASE64 } + }) + } + ) + + // Why: an SSH host forwards its provider's payload verbatim, so the cap cannot rely on the far + // side clamping — this walks the real RuntimeGitCommands with an unclamped forwarded diff. + it('caps an SSH-forwarded diff a remote client requested', async () => { + sshMocks.getSshGitProvider.mockReturnValue({ + getDiff: vi.fn().mockResolvedValue(OVERSIZED_DIFF) + }) + const commands = new RuntimeGitCommands({ + resolveRuntimeGitTarget: async () => ({ + worktree: { id: 'wt-1', path: '/remote/repo' } as unknown as ResolvedRuntimeGitWorktree, + connectionId: 'conn-1' + }), + getRuntimeSettings: () => ({}) as GlobalSettings + }) + const runtime = Object.assign(commands, { + getRuntimeId: () => 'test-runtime' + }) as unknown as OrcaRuntimeService + + const response = await dispatchRemote(runtime, 'git.diff', CASES[0]!.params, 'mobile') + + expect(response).toMatchObject({ ok: false, error: { code: 'diff_too_large' } }) + }) +}) diff --git a/src/main/runtime/rpc/methods/git.test.ts b/src/main/runtime/rpc/methods/git.test.ts index e6ab9f3340d..92221d4bb33 100644 --- a/src/main/runtime/rpc/methods/git.test.ts +++ b/src/main/runtime/rpc/methods/git.test.ts @@ -179,7 +179,14 @@ describe('git RPC methods', () => { }) ) - expect(runtime.getRuntimeGitDiff).toHaveBeenCalledWith('id:wt-1', 'src/index.ts', false, true) + // A local dispatch sets no clientKind, so the transport budget stays undefined. + expect(runtime.getRuntimeGitDiff).toHaveBeenCalledWith( + 'id:wt-1', + 'src/index.ts', + false, + true, + undefined + ) expect(response).toMatchObject({ ok: true, result: { kind: 'text', modifiedContent: 'hello' } diff --git a/src/main/runtime/rpc/methods/git.ts b/src/main/runtime/rpc/methods/git.ts index 167b82966f5..a11369ff9e0 100644 --- a/src/main/runtime/rpc/methods/git.ts +++ b/src/main/runtime/rpc/methods/git.ts @@ -1,5 +1,6 @@ /* eslint-disable max-lines -- Why: this table is the runtime git RPC contract; splitting it would make method coverage harder to audit. */ import { defineMethod, type RpcMethod } from '../core' +import { remoteRpcContentBudget } from '../../../../shared/remote-rpc-content-budget' import type { GlobalSettings } from '../../../../shared/global-settings-types' import type { ResolvedSourceControlAiGenerationParams } from '../../../../shared/source-control-ai' import { @@ -28,6 +29,15 @@ import { WorktreeSelector } from './git-params' +// Why: clientKind is set only for WebSocket-transported requests, so desktop-local and in-process +// callers keep uncapped full-fidelity diffs. +function remoteDiffContentBudget( + clientKind: 'mobile' | 'runtime' | undefined, + requestId: string | undefined +): number | undefined { + return clientKind && requestId ? remoteRpcContentBudget(requestId) : undefined +} + type CommitMessageGenerationOverride = { commitMessageAi?: GlobalSettings['commitMessageAi'] sourceControlAi?: GlobalSettings['sourceControlAi'] @@ -158,12 +168,13 @@ export const GIT_METHODS: RpcMethod[] = [ defineMethod({ name: 'git.diff', params: GitDiff, - handler: async (params, { runtime }) => + handler: async (params, { runtime, clientKind, requestId }) => runtime.getRuntimeGitDiff( params.worktree, params.filePath, params.staged, - params.compareAgainstHead + params.compareAgainstHead, + remoteDiffContentBudget(clientKind, requestId) ) }), defineMethod({ @@ -236,24 +247,29 @@ export const GIT_METHODS: RpcMethod[] = [ defineMethod({ name: 'git.branchDiff', params: GitBranchDiff, - handler: async (params, { runtime }) => + handler: async (params, { runtime, clientKind, requestId }) => runtime.getRuntimeGitBranchDiff( params.worktree, params.compare, params.filePath, - params.oldPath + params.oldPath, + remoteDiffContentBudget(clientKind, requestId) ) }), defineMethod({ name: 'git.commitDiff', params: GitCommitDiff, - handler: async (params, { runtime }) => - runtime.getRuntimeGitCommitDiff(params.worktree, { - commitOid: params.commitOid, - parentOid: params.parentOid, - filePath: params.filePath, - oldPath: params.oldPath - }) + handler: async (params, { runtime, clientKind, requestId }) => + runtime.getRuntimeGitCommitDiff( + params.worktree, + { + commitOid: params.commitOid, + parentOid: params.parentOid, + filePath: params.filePath, + oldPath: params.oldPath + }, + remoteDiffContentBudget(clientKind, requestId) + ) }), defineMethod({ name: 'git.commit', diff --git a/src/main/runtime/runtime-rpc.test.ts b/src/main/runtime/runtime-rpc.test.ts index 5ccffe24550..9fde32745e5 100644 --- a/src/main/runtime/runtime-rpc.test.ts +++ b/src/main/runtime/runtime-rpc.test.ts @@ -13,6 +13,7 @@ import { OrchestrationDb } from './orchestration/db' import * as runtimeMetadataModule from './runtime-metadata' import { readRuntimeMetadata, writeRuntimeMetadata } from './runtime-metadata' import { createRuntimeTransportMetadata, OrcaRuntimeRpcServer } from './runtime-rpc' +import { remoteRpcContentBudget } from '../../shared/remote-rpc-content-budget' import { parsePairingCode } from '../../shared/pairing' import { subscribeRemoteRuntimeRequest } from '../../shared/remote-runtime-client' import { @@ -3299,7 +3300,14 @@ describe('OrcaRuntimeRpcServer', () => { expect(abortRuntimeGitRebase).toHaveBeenCalledWith('id:wt-1') expect(bulkUnstageRuntimeGitPaths).toHaveBeenCalledWith('id:wt-1', ['c.ts']) expect(openMobileDiff).toHaveBeenCalledWith('id:wt-1', 'docs/readme.md', true) - expect(getRuntimeGitDiff).toHaveBeenCalledWith('id:wt-1', 'docs/readme.md', false, undefined) + // A mobile WebSocket client is transport-capped; a local caller gets undefined here. + expect(getRuntimeGitDiff).toHaveBeenCalledWith( + 'id:wt-1', + 'docs/readme.md', + false, + undefined, + remoteRpcContentBudget('req_git_diff') + ) expect(browserTabCreate).toHaveBeenCalledWith({ worktree: 'id:wt-1', url: 'about:blank' }) expect(browserSetViewport).toHaveBeenCalledWith({ worktree: 'id:wt-1', diff --git a/src/main/ssh/ssh-file-stream-read-cap.ts b/src/main/ssh/ssh-file-stream-read-cap.ts new file mode 100644 index 00000000000..e5bdce97e5a --- /dev/null +++ b/src/main/ssh/ssh-file-stream-read-cap.ts @@ -0,0 +1,10 @@ +import type { FileReadLimits } from '../providers/types' + +const MAX_PREVIEWABLE_BINARY_SIZE = 50 * 1024 * 1024 +const MAX_TEXT_FILE_SIZE = 10 * 1024 * 1024 + +export function sshFileStreamReadCap(isBinary: boolean, limits?: FileReadLimits): number { + const defaultCap = isBinary ? MAX_PREVIEWABLE_BINARY_SIZE : MAX_TEXT_FILE_SIZE + const requestedCap = isBinary ? limits?.maxBinaryBytes : limits?.maxTextBytes + return requestedCap === undefined ? defaultCap : Math.min(defaultCap, requestedCap) +} diff --git a/src/main/ssh/ssh-filesystem-stream-reader.ts b/src/main/ssh/ssh-filesystem-stream-reader.ts index f61939d6c2f..568ab195dc8 100644 --- a/src/main/ssh/ssh-filesystem-stream-reader.ts +++ b/src/main/ssh/ssh-filesystem-stream-reader.ts @@ -1,17 +1,15 @@ import type { SshChannelMultiplexer } from './ssh-channel-multiplexer' import { STREAM_CHUNK_SIZE, JsonRpcErrorCode, RelayErrorCode } from './relay-protocol' -import type { FileReadResult } from '../providers/types' +import type { FileReadLimits, FileReadResult } from '../providers/types' import { createSshFileStreamInactivityDeadline, SSH_FILE_STREAM_INACTIVITY_TIMEOUT_MS } from './ssh-file-stream-inactivity-deadline' +import { sshFileStreamReadCap } from './ssh-file-stream-read-cap' const RESULT_ENCODING_BASE64 = 'base64' const SENTINEL_STREAM_ID = -1 -const MAX_PREVIEWABLE_BINARY_SIZE = 50 * 1024 * 1024 -const MAX_TEXT_FILE_SIZE = 10 * 1024 * 1024 - type StreamMetadataResponse = { streamId?: number totalSize: number @@ -37,9 +35,14 @@ export class StreamProtocolError extends Error { } } +// Why: exceeding a cap the caller itself set is a size verdict, not a protocol fault — callers +// translate it into their own too-large error rather than leaking the raw stream message. +export class FileReadCapExceededError extends StreamProtocolError {} + export async function readFileViaStream( mux: SshChannelMultiplexer, - filePath: string + filePath: string, + limits?: FileReadLimits ): Promise { // Why: subscribe BEFORE awaiting the metadata response so a chunk arriving // immediately after the response cannot beat the listener registration. @@ -307,11 +310,11 @@ export async function readFileViaStream( return } - const cap = metadata.isBinary ? MAX_PREVIEWABLE_BINARY_SIZE : MAX_TEXT_FILE_SIZE + const cap = sshFileStreamReadCap(metadata.isBinary, limits) if (metadata.totalSize < 0 || metadata.totalSize > cap) { streamIdRef.current = metadata.streamId fail( - new StreamProtocolError( + new FileReadCapExceededError( `Reported totalSize ${metadata.totalSize} exceeds client cap ${cap}` ) ) diff --git a/src/shared/git-diff-transport-budget.test.ts b/src/shared/git-diff-transport-budget.test.ts new file mode 100644 index 00000000000..5b4e426df26 --- /dev/null +++ b/src/shared/git-diff-transport-budget.test.ts @@ -0,0 +1,193 @@ +// Why: a raw-byte cap is not enough. JSON escaping expands a control character sixfold and +// binary-buffer.ts sniffs only for NUL, so control-dense content is classified as text; these +// fixtures pin every branch of the measurement to native JSON.stringify. +import { describe, expect, it } from 'vitest' +import type { GitDiffResult } from './git-diff-compare-types' +import { REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES } from './remote-runtime-memory-limits' +import { + assertGitDiffWithinTransportBudget, + gitDiffExceedsTransportBudget +} from './git-diff-transport-budget' +import { REMOTE_RPC_MAX_CONTENT_BYTES, remoteRpcContentBudget } from './remote-rpc-content-budget' + +const BUDGET = REMOTE_RPC_MAX_CONTENT_BYTES + +function referenceResultBytes(result: GitDiffResult): number { + return Buffer.byteLength(JSON.stringify(result), 'utf8') +} + +/** Content whose JSON encoding, quotes included, is exactly `jsonBytes`. */ +function sideOfJsonBytes(unit: string, jsonBytes: number): string { + const unitCost = Buffer.byteLength(JSON.stringify(unit), 'utf8') - 2 + const count = Math.floor((jsonBytes - 2) / unitCost) + return unit.repeat(count) + 'x'.repeat(jsonBytes - 2 - count * unitCost) +} + +function textDiff(modifiedContent: string): GitDiffResult { + return { + kind: 'text', + originalContent: '', + modifiedContent, + originalIsBinary: false, + modifiedIsBinary: false + } +} + +function textDiffOfJsonBytes(unit: string, jsonBytes: number): GitDiffResult { + const empty = textDiff('') + const fixedBytes = referenceResultBytes(empty) - 2 + return textDiff(sideOfJsonBytes(unit, jsonBytes - fixedBytes)) +} + +function binaryDiffOfJsonBytes(unit: string, jsonBytes: number): GitDiffResult { + const empty: GitDiffResult = { + kind: 'binary', + originalContent: '', + modifiedContent: '', + isImage: true, + mimeType: 'image/png', + originalIsBinary: true, + modifiedIsBinary: true + } + const contentBytes = jsonBytes - (referenceResultBytes(empty) - 4) + const firstBytes = Math.floor(contentBytes / 2) + return { + ...empty, + originalContent: sideOfJsonBytes(unit, firstBytes), + modifiedContent: sideOfJsonBytes(unit, contentBytes - firstBytes) + } +} + +function budgetError(result: GitDiffResult): { code?: string; data?: unknown } { + try { + assertGitDiffWithinTransportBudget(result, BUDGET) + } catch (error) { + return error as { code?: string; data?: unknown } + } + throw new Error('expected the transport budget assertion to throw') +} + +const UNITS: readonly { name: string; unit: string; expansion: number }[] = [ + { name: 'ascii', unit: 'a', expansion: 1 }, + { name: 'newline-dense text', unit: '\n', expansion: 2 }, + { name: 'control-char text (0x01)', unit: '\u0001', expansion: 6 }, + { name: 'base64', unit: 'QUJD', expansion: 1 }, + { name: 'cjk', unit: '漢', expansion: 1 }, + { name: 'lone surrogate', unit: '\ud800', expansion: 2 }, + { name: 'surrogate pair', unit: '😀', expansion: 1 }, + { name: 'quotes and backslashes', unit: '"\\', expansion: 2 } +] + +describe('gitDiffExceedsTransportBudget', () => { + it.each(UNITS)('pins the JSON expansion assumed for $name', ({ unit, expansion }) => { + const jsonBytes = Buffer.byteLength(JSON.stringify(unit), 'utf8') - 2 + expect(jsonBytes / Buffer.byteLength(unit, 'utf8')).toBe(expansion) + }) + + it.each(UNITS)('admits $name exactly at the budget', ({ unit }) => { + const result = textDiffOfJsonBytes(unit, BUDGET) + + expect(referenceResultBytes(result)).toBe(BUDGET) + expect(gitDiffExceedsTransportBudget(result, BUDGET)).toBe(false) + expect(assertGitDiffWithinTransportBudget(result, BUDGET)).toBe(result) + }) + + it.each(UNITS)('rejects $name one byte above the budget', ({ unit }) => { + const result = textDiffOfJsonBytes(unit, BUDGET + 1) + + expect(referenceResultBytes(result)).toBe(BUDGET + 1) + expect(gitDiffExceedsTransportBudget(result, BUDGET)).toBe(true) + expect(budgetError(result).code).toBe('diff_too_large') + }) + + it.each(UNITS)('agrees with native JSON.stringify across the boundary for $name', ({ unit }) => { + for (const jsonBytes of [BUDGET - 1, BUDGET, BUDGET + 1]) { + const result = textDiffOfJsonBytes(unit, jsonBytes) + expect(gitDiffExceedsTransportBudget(result, BUDGET)).toBe( + referenceResultBytes(result) > BUDGET + ) + } + }) + + // Why: this is the case a raw-byte budget silently lets through into the 1013 close. + it('rejects control-dense content whose raw bytes are far under the budget', () => { + const result = textDiffOfJsonBytes('\u0001', BUDGET + 1) + + expect(Buffer.byteLength(result.modifiedContent, 'utf8')).toBeLessThan(BUDGET / 5) + expect(budgetError(result).data).toEqual({ maxBytes: BUDGET }) + }) + + it('splits the budget across both sides', () => { + const overBudget = binaryDiffOfJsonBytes('Q', BUDGET + 1) + + expect(referenceResultBytes(overBudget)).toBe(BUDGET + 1) + expect(budgetError(overBudget).code).toBe('diff_too_large') + }) + + it('rejects additive relay metadata beyond the result budget', () => { + const skewed = { + ...textDiff(''), + futureMetadata: 'x'.repeat(BUDGET) + } as unknown as GitDiffResult + + expect(gitDiffExceedsTransportBudget(skewed, BUDGET)).toBe(true) + expect(budgetError(skewed).code).toBe('diff_too_large') + }) + + // Why: the SSH provider casts a relay payload to GitDiffResult without validating it. + it('treats a side missing from a relay payload as empty', () => { + const skewed = { kind: 'text', modifiedContent: 'hi' } as unknown as GitDiffResult + + expect(gitDiffExceedsTransportBudget(skewed, BUDGET)).toBe(false) + expect(assertGitDiffWithinTransportBudget(skewed, BUDGET)).toBe(skewed) + }) + + it('leaves local callers uncapped', () => { + const result = textDiff('x'.repeat(BUDGET + 1)) + + expect(assertGitDiffWithinTransportBudget(result, undefined)).toBe(result) + }) +}) + +// Why: the invariant the cap rests on — a diff that exactly fills the content budget must still +// serialize inside the outbound JSON limit once wrapped in an RPC reply. A base64-only version of +// this passes trivially; the newline and control-char fixtures are the load-bearing ones. +describe('envelope ceiling', () => { + function replyBytes(result: GitDiffResult, requestId = 'req_0123456789abcdef'): number { + return Buffer.byteLength( + JSON.stringify({ + id: requestId, + ok: true, + result, + _meta: { runtimeId: '00000000-0000-4000-8000-000000000000' } + }), + 'utf8' + ) + } + + it.each(UNITS)('keeps a $name diff at the budget inside the outbound limit', ({ unit }) => { + expect(replyBytes(binaryDiffOfJsonBytes(unit, BUDGET))).toBeLessThanOrEqual( + REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES + ) + }) + + it('charges an escape-dense request id instead of overflowing the fixed reserve', () => { + const requestId = '\u0001'.repeat(8_192) + const budget = remoteRpcContentBudget(requestId) + + expect(budget).toBeLessThan(BUDGET) + expect(replyBytes(binaryDiffOfJsonBytes('a', budget), requestId)).toBeLessThanOrEqual( + REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES + ) + }) + + // Why: every reserved byte is content that transferred before this cap existed, so the reserve + // must stay close to the real envelope overhead. Fails if someone inflates it "just to be safe". + it('keeps the envelope reserve within 64x the overhead it covers', () => { + const overhead = replyBytes(binaryDiffOfJsonBytes('a', BUDGET)) - BUDGET + const reserve = REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES - BUDGET + + expect(reserve).toBeGreaterThan(overhead) + expect(reserve).toBeLessThan(overhead * 64) + }) +}) diff --git a/src/shared/git-diff-transport-budget.ts b/src/shared/git-diff-transport-budget.ts new file mode 100644 index 00000000000..ac46333c3d4 --- /dev/null +++ b/src/shared/git-diff-transport-budget.ts @@ -0,0 +1,24 @@ +import type { GitDiffResult } from './git-diff-compare-types' +import { remoteRpcResultExceedsContentBudget } from './remote-rpc-content-budget' + +export const GIT_DIFF_TOO_LARGE_CODE = 'diff_too_large' +const GIT_DIFF_CONTENT_FIELDS = ['originalContent', 'modifiedContent'] as const + +/** Whether the complete diff result exceeds `maxBytes` once JSON-encoded. */ +export function gitDiffExceedsTransportBudget(result: GitDiffResult, maxBytes: number): boolean { + return remoteRpcResultExceedsContentBudget(result, maxBytes, GIT_DIFF_CONTENT_FIELDS) +} + +/** `maxBytes === undefined` means uncapped: local and in-process callers keep full fidelity. */ +export function assertGitDiffWithinTransportBudget( + result: T, + maxBytes: number | undefined +): T { + if (maxBytes === undefined || !gitDiffExceedsTransportBudget(result, maxBytes)) { + return result + } + throw Object.assign(new Error('This diff is too large to open over a remote connection.'), { + code: GIT_DIFF_TOO_LARGE_CODE, + data: { maxBytes } + }) +} diff --git a/src/shared/remote-rpc-content-budget.test.ts b/src/shared/remote-rpc-content-budget.test.ts new file mode 100644 index 00000000000..9d56012f9e2 --- /dev/null +++ b/src/shared/remote-rpc-content-budget.test.ts @@ -0,0 +1,96 @@ +import { describe, expect, it, vi } from 'vitest' +import { REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES } from './remote-runtime-memory-limits' +import { + REMOTE_RPC_MAX_CONTENT_BYTES, + remoteRpcContentBudget, + remoteRpcResultExceedsContentBudget +} from './remote-rpc-content-budget' + +describe('remoteRpcContentBudget', () => { + it('measures the complete serialized result, including additive fields', () => { + const result = { content: '', futureMetadata: 'x'.repeat(128) } + const bytes = Buffer.byteLength(JSON.stringify(result), 'utf8') + + expect(remoteRpcResultExceedsContentBudget(result, bytes, ['content'])).toBe(false) + expect(remoteRpcResultExceedsContentBudget(result, bytes - 1, ['content'])).toBe(true) + }) + + it('reuses exact measurements for a shared result object', () => { + const result = { content: '\n'.repeat(64 * 1024), isBinary: false } + const bytes = Buffer.byteLength(JSON.stringify(result), 'utf8') + const charCodeAt = vi.spyOn(String.prototype, 'charCodeAt') + try { + expect(remoteRpcResultExceedsContentBudget(result, bytes, ['content'])).toBe(false) + const firstCalls = charCodeAt.mock.calls.length + expect(firstCalls).toBeGreaterThan(result.content.length) + + expect(remoteRpcResultExceedsContentBudget(result, bytes - 1, ['content'])).toBe(true) + expect(charCodeAt.mock.calls.length - firstCalls).toBeLessThan(100) + } finally { + charCodeAt.mockRestore() + } + }) + + it('reuses raw measurements for a shared oversized result', () => { + const result = { content: 'A'.repeat(4 * 1024 * 1024), isBinary: false } + const byteLength = vi.spyOn(Buffer, 'byteLength') + const contentMeasurementCalls = () => + byteLength.mock.calls.filter(([value]) => value === result.content).length + try { + expect(remoteRpcResultExceedsContentBudget(result, result.content.length, ['content'])).toBe( + true + ) + expect(contentMeasurementCalls()).toBe(1) + + expect( + remoteRpcResultExceedsContentBudget(result, result.content.length - 128, ['content']) + ).toBe(true) + expect(contentMeasurementCalls()).toBe(1) + } finally { + byteLength.mockRestore() + } + }) + + it('re-evaluates a cached raw measurement under a larger budget', () => { + const result = { content: 'A'.repeat(1_000), isBinary: false } + const bytes = Buffer.byteLength(JSON.stringify(result), 'utf8') + + expect(remoteRpcResultExceedsContentBudget(result, 900, ['content'])).toBe(true) + expect(remoteRpcResultExceedsContentBudget(result, bytes, ['content'])).toBe(false) + }) + + it('does not reuse a truncated measurement for a larger budget', () => { + const result = { content: String.fromCharCode(1).repeat(1_000), isBinary: false } + const bytes = Buffer.byteLength(JSON.stringify(result), 'utf8') + + expect(bytes).toBeGreaterThan(1_150) + expect(remoteRpcResultExceedsContentBudget(result, 1_050, ['content'])).toBe(true) + expect(remoteRpcResultExceedsContentBudget(result, 1_150, ['content'])).toBe(true) + }) + + it('charges an escape-dense echoed request id to a file preview reply', () => { + const id = '\u0001'.repeat(8_192) + const replyBytes = (contentBytes: number) => + Buffer.byteLength( + JSON.stringify({ + id, + ok: true, + result: { + content: 'A'.repeat(contentBytes), + isBinary: true, + isImage: true, + mimeType: 'image/png' + }, + _meta: { runtimeId: '00000000-0000-4000-8000-000000000000' } + }), + 'utf8' + ) + + expect(replyBytes(REMOTE_RPC_MAX_CONTENT_BYTES)).toBeGreaterThan( + REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES + ) + expect(replyBytes(remoteRpcContentBudget(id))).toBeLessThanOrEqual( + REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES + ) + }) +}) diff --git a/src/shared/remote-rpc-content-budget.ts b/src/shared/remote-rpc-content-budget.ts new file mode 100644 index 00000000000..5e1be37901c --- /dev/null +++ b/src/shared/remote-rpc-content-budget.ts @@ -0,0 +1,151 @@ +import { REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES } from './remote-runtime-memory-limits' +import { + JsonStringifyByteLimitError, + stringifyJsonWithinByteLimit +} from './node-bounded-json-stringify' + +// Why: reserve fixed reply fields and future additive metadata; the echoed request id is charged +// separately because the wire contract permits arbitrary strings. +const OUTBOUND_ENVELOPE_RESERVE_BYTES = 4 * 1024 +const MAX_JSON_STRING_ESCAPE_EXPANSION = 6 +type CachedResultBytes = { + skeleton: string + stringFields: readonly { name: string; value: string }[] + rawStringBytes: number + byteLength?: number +} +const cachedResultBytes = new WeakMap() + +/** Ceiling for content in one RPC reply before charging its request id. */ +export const REMOTE_RPC_MAX_CONTENT_BYTES = + REMOTE_RUNTIME_MAX_OUTBOUND_JSON_BYTES - OUTBOUND_ENVELOPE_RESERVE_BYTES + +/** Content budget after charging the JSON-encoded request id echoed by the reply. */ +export function remoteRpcContentBudget(requestId: string): number { + const requestIdBytes = Buffer.byteLength(JSON.stringify(requestId), 'utf8') + return Math.max(0, REMOTE_RPC_MAX_CONTENT_BYTES - requestIdBytes) +} + +function jsonStringContentBytes(value: string): number { + let bytes = 0 + for (let index = 0; index < value.length; index += 1) { + const code = value.charCodeAt(index) + if (code === 0x22 || code === 0x5c) { + bytes += 2 + } else if (code < 0x20) { + bytes += + code === 0x08 || code === 0x09 || code === 0x0a || code === 0x0c || code === 0x0d ? 2 : 6 + } else if (code <= 0x7f) { + bytes += 1 + } else if (code <= 0x7ff) { + bytes += 2 + } else if (code >= 0xd800 && code <= 0xdbff) { + const next = value.charCodeAt(index + 1) + if (next >= 0xdc00 && next <= 0xdfff) { + bytes += 4 + index += 1 + } else { + bytes += 6 + } + } else if (code >= 0xdc00 && code <= 0xdfff) { + bytes += 6 + } else { + bytes += 3 + } + } + return bytes +} + +/** Whether a complete RPC result exceeds its request-scoped content budget. */ +export function remoteRpcResultExceedsContentBudget( + result: unknown, + maxBytes: number, + largeStringFields: readonly string[] = [] +): boolean { + const stringFields: { name: string; value: string }[] = [] + let measuredResult = result + if (result !== null && typeof result === 'object' && largeStringFields.length > 0) { + const skeleton = { ...(result as Record) } + for (const field of largeStringFields) { + const value = skeleton[field] + if (typeof value === 'string') { + stringFields.push({ name: field, value }) + skeleton[field] = '' + } + } + measuredResult = skeleton + } + + let skeletonBytes: number + let serializedSkeleton: string + try { + const measurement = stringifyJsonWithinByteLimit(measuredResult, maxBytes) + skeletonBytes = measurement.byteLength + serializedSkeleton = measurement.serialized + } catch (error) { + if (error instanceof JsonStringifyByteLimitError) { + return true + } + throw error + } + + let rawBytes: number | undefined + if (result !== null && typeof result === 'object') { + const cached = cachedResultBytes.get(result) + if ( + cached?.skeleton === serializedSkeleton && + cached.stringFields.length === stringFields.length && + cached.stringFields.every( + (field, index) => + field.name === stringFields[index]?.name && field.value === stringFields[index]?.value + ) + ) { + if (cached.byteLength !== undefined) { + return cached.byteLength > maxBytes + } + const remainingBytes = maxBytes - skeletonBytes + if (cached.rawStringBytes * MAX_JSON_STRING_ESCAPE_EXPANSION <= remainingBytes) { + return false + } + if (cached.rawStringBytes > remainingBytes) { + return true + } + rawBytes = cached.rawStringBytes + } + } + + const remainingBytes = maxBytes - skeletonBytes + if (rawBytes === undefined) { + rawBytes = 0 + for (const field of stringFields) { + rawBytes += Buffer.byteLength(field.value, 'utf8') + } + if (result !== null && typeof result === 'object') { + cachedResultBytes.set(result, { + skeleton: serializedSkeleton, + stringFields, + rawStringBytes: rawBytes + }) + } + } + if (rawBytes * MAX_JSON_STRING_ESCAPE_EXPANSION <= remainingBytes) { + return false + } + if (rawBytes > remainingBytes) { + return true + } + + let encodedBytes = 0 + for (const field of stringFields) { + encodedBytes += jsonStringContentBytes(field.value) + } + if (result !== null && typeof result === 'object') { + cachedResultBytes.set(result, { + skeleton: serializedSkeleton, + stringFields, + rawStringBytes: rawBytes, + byteLength: skeletonBytes + encodedBytes + }) + } + return encodedBytes > remainingBytes +} diff --git a/src/shared/telemetry-events.test.ts b/src/shared/telemetry-events.test.ts index e172d03a787..687749a5eee 100644 --- a/src/shared/telemetry-events.test.ts +++ b/src/shared/telemetry-events.test.ts @@ -612,3 +612,23 @@ describe('exported enum schemas', () => { } }) }) + +describe('remote_outbound_budget_close schema', () => { + it('round-trips every emitter', () => { + for (const emitter of ['size', 'queue']) { + expect(eventSchemas.remote_outbound_budget_close.safeParse({ emitter }).success).toBe(true) + } + }) + + it('rejects an unknown emitter and any payload-describing extra key', () => { + expect(eventSchemas.remote_outbound_budget_close.safeParse({ emitter: 'other' }).success).toBe( + false + ) + expect( + eventSchemas.remote_outbound_budget_close.safeParse({ + emitter: 'size', + byte_length: 4194305 + }).success + ).toBe(false) + }) +}) diff --git a/src/shared/telemetry-events.ts b/src/shared/telemetry-events.ts index 1d6905c1f8d..f6bbea3c755 100644 --- a/src/shared/telemetry-events.ts +++ b/src/shared/telemetry-events.ts @@ -386,6 +386,9 @@ const runtimeRpcStartFailedSchema = z .object({ error_class: runtimeRpcStartErrorClassSchema }) .strict() +// Why: classify session-killing 1013 closures as producer size failures or queue backpressure. +const remoteOutboundBudgetCloseSchema = z.object({ emitter: z.enum(['size', 'queue']) }).strict() + // Why: a deadlocked main thread never crashes, so it produces no crash report and no user report // beyond "it froze" — incidence has been unmeasurable. `self_recovered` splits stalls that cleared // from ones that never did, which is the number that decides whether auto-recovery is ever safe to @@ -1447,6 +1450,7 @@ export const eventSchemas = { daemon_lifecycle: daemonLifecycleSchema, daemon_audit_eligibility: daemonAuditEligibilitySchema, runtime_rpc_start_failed: runtimeRpcStartFailedSchema, + remote_outbound_budget_close: remoteOutboundBudgetCloseSchema, codex_trust_grant: codexTrustGrantSchema,