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,