From ca2cf3e160e57d1ad4e3278350d500fbc87f6f4b Mon Sep 17 00:00:00 2001 From: Neil <4138956+nwparker@users.noreply.github.com> Date: Wed, 28 Oct 2026 01:36:42 -0700 Subject: [PATCH] fix(files): never open a remote binary as text, and never page one in to find out (#34877) A host decided files.read binaries by extension only. A binary with an extension it did not list (.so, .o, .wasm, .class, ...) came back as text: under 512 KiB the editor opened it as text, where a save writes the mangled text back; up to 50 MiB the desktop paged the whole file in before its NUL check called it binary. Applies to WSL and SSH hosts and paired servers alike. - The host checks the prefix it already reads for a NUL byte (git's own heuristic) and answers the existing `binary_file` refusal, which every client handles. - The desktop shows any binary that has nothing to preview (not an image or PDF) as binary at once, without fetching a preview or paging (readRemoteBinaryFile); and treats text from an older host whose first 8 KiB holds a NUL the same way. --- .../files/file-reads-binary-sniff.test.ts | 55 +++++++++++++++++++ src/main/runtime/files/file-reads.ts | 4 ++ .../src/runtime/runtime-file-client.test.ts | 50 +++++++++++++++++ .../src/runtime/runtime-file-read-client.ts | 8 ++- .../src/runtime/runtime-paged-file-read.ts | 10 ++++ 5 files changed, 126 insertions(+), 1 deletion(-) create mode 100644 src/main/runtime/files/file-reads-binary-sniff.test.ts diff --git a/src/main/runtime/files/file-reads-binary-sniff.test.ts b/src/main/runtime/files/file-reads-binary-sniff.test.ts new file mode 100644 index 00000000000..88aa0f91d06 --- /dev/null +++ b/src/main/runtime/files/file-reads-binary-sniff.test.ts @@ -0,0 +1,55 @@ +import { mkdtempSync, rmSync, writeFileSync } from 'node:fs' +import { tmpdir } from 'node:os' +import { join } from 'node:path' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' +import type { RuntimeStore } from '../runtime-store-contract' +import type { ResolvedRuntimeFileTarget } from './file-target' +import { RuntimeFileReads } from './file-reads' + +vi.mock('./chat-scratch-file-root', () => ({ + resolveRuntimeAuthorizedPath: async (path: string) => path +})) + +let root: string + +beforeEach(() => { + root = mkdtempSync(join(tmpdir(), 'orca-file-reads-binary-')) +}) + +afterEach(() => { + rmSync(root, { recursive: true, force: true }) +}) + +function reads(): RuntimeFileReads { + return new RuntimeFileReads({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the store only reaches the mocked path authorization. + requireStore: () => ({}) as RuntimeStore, + resolveRuntimeFileTarget: async () => ({ + // oxlint-disable-next-line typescript/consistent-type-assertions -- SAFETY: the read uses only the worktree's id and path. + worktree: { id: 'wt-1', path: root } as ResolvedRuntimeFileTarget['worktree'], + executionHostId: 'local' + }) + }) +} + +describe('files.read on a host', () => { + it('refuses a binary whose extension the list does not know, before sending it as text', async () => { + writeFileSync(join(root, 'libfoo.so'), Buffer.from([0x7f, 0x45, 0x4c, 0x46, 0x02, 0x00, 0x01])) + await expect(reads().readMobileFile('id:wt-1', 'libfoo.so')).rejects.toThrow('binary_file') + }) + + it('refuses a large one from its first bytes, so the client never pages it in', async () => { + const big = Buffer.alloc(2 * 1024 * 1024, 0x61) + big[100] = 0 + writeFileSync(join(root, 'blob.dat'), big) + await expect(reads().readMobileFile('id:wt-1', 'blob.dat')).rejects.toThrow('binary_file') + }) + + it('still sends text', async () => { + writeFileSync(join(root, 'notes.txt'), 'hello\n') + await expect(reads().readMobileFile('id:wt-1', 'notes.txt')).resolves.toMatchObject({ + content: 'hello\n', + truncated: false + }) + }) +}) diff --git a/src/main/runtime/files/file-reads.ts b/src/main/runtime/files/file-reads.ts index 8acd2d1d75f..29ed3a089bb 100644 --- a/src/main/runtime/files/file-reads.ts +++ b/src/main/runtime/files/file-reads.ts @@ -178,6 +178,10 @@ async function readLocalMobileFile(filePath: string, store: RuntimeStore): Promi store, MOBILE_FILE_READ_MAX_BYTES + 1 ) + // An extension the list does not know can still be binary; as text, a save would corrupt it. + if (isBinaryBuffer(bytes)) { + throw new Error('binary_file') + } return bytes.toString('utf8') } diff --git a/src/renderer/src/runtime/runtime-file-client.test.ts b/src/renderer/src/runtime/runtime-file-client.test.ts index a6607b09ab4..a3bc9d89b84 100644 --- a/src/renderer/src/runtime/runtime-file-client.test.ts +++ b/src/renderer/src/runtime/runtime-file-client.test.ts @@ -168,6 +168,56 @@ describe('runtime file client', () => { }) }) + it('shows a binary the host refused as binary, fetching no preview when it has none', async () => { + runtimeEnvironmentCall.mockImplementation(() => + Promise.resolve({ + id: 'rpc-read', + ok: false, + error: { code: 'runtime_error', message: 'binary_file' }, + _meta: { runtimeId: 'remote-runtime' } + }) + ) + + await expect( + readRuntimeFileContent({ + target: { kind: 'environment' as const, environmentId: 'env-1' }, + filePath: '/remote/repo/libfoo.so', + relativePath: 'libfoo.so', + worktreeId: 'wt-1', + pageOversizedBinary: true + }) + ).resolves.toEqual({ content: '', isBinary: true }) + expect(runtimeEnvironmentCall).toHaveBeenCalledTimes(1) + }) + + it('shows an unlisted binary an older host sent as text as binary, fetching nothing more', async () => { + runtimeEnvironmentCall.mockImplementation((args: { method: string }) => + Promise.resolve( + args.method === 'files.read' + ? { + id: 'rpc-read', + ok: true, + result: { content: '\u007fELF\u0002\u0000\u0001', truncated: true, byteLength: 7 }, + _meta: { runtimeId: 'remote-runtime' } + } + : { id: 'rpc-other', ok: false, error: { code: 'runtime_error', message: 'unexpected' } } + ) + ) + + await expect( + readRuntimeFileContent({ + target: { kind: 'environment' as const, environmentId: 'env-1' }, + filePath: '/remote/repo/libfoo.so', + relativePath: 'libfoo.so', + worktreeId: 'wt-1' + }) + ).resolves.toEqual({ content: '', isBinary: true }) + // Nothing more is fetched: not paged in chunks though truncated, and no preview to show. + for (const method of ['files.readChunk', 'files.readPreview']) { + expect(runtimeEnvironmentCall).not.toHaveBeenCalledWith(expect.objectContaining({ method })) + } + }) + it('does not fall back to files.readPreview for non-binary remote read errors', async () => { runtimeEnvironmentCall.mockImplementation((args: { method: string }) => { if (args.method === 'files.read') { diff --git a/src/renderer/src/runtime/runtime-file-read-client.ts b/src/renderer/src/runtime/runtime-file-read-client.ts index c4f993946a6..bed58815b0e 100644 --- a/src/renderer/src/runtime/runtime-file-read-client.ts +++ b/src/renderer/src/runtime/runtime-file-read-client.ts @@ -5,6 +5,7 @@ import type { } from '../../../shared/runtime-types' import { callRuntimeRpc } from './runtime-rpc-client' import { RuntimeRpcCallError } from './runtime-rpc-result' +import { BINARY_PROBE_BYTES } from '../../../shared/binary-buffer' import type { RuntimeFileDownloadResult, RuntimeFileOperationArgs, @@ -28,6 +29,7 @@ import { import { pagedTextFileResult, pinRemoteFileAddress, + readRemoteBinaryFile, readRemoteFileInChunks, readRemoteFilePreview } from './runtime-paged-file-read' @@ -108,10 +110,14 @@ export async function readRuntimeFileContent({ // Typed binary_file refusal permits base64 preview for PDFs/images; unrelated errors cannot spoof this fallback. if (err instanceof RuntimeRpcCallError && err.message === 'binary_file') { // Why: baseline checks call this too and discard binary content, so paging is opt-in. - return readRemoteFilePreview(remote, { pageOversized: pageOversizedBinary === true }) + return readRemoteBinaryFile(remote, { pageOversized: pageOversizedBinary === true }) } throw err } + // Hosts before content sniffing send an unlisted binary as text; NUL is git's binary signal. + if (result.content.slice(0, BINARY_PROBE_BYTES).includes('\0')) { + return { content: '', isBinary: true } + } if (result.truncated) { // files.read caps replies at 512KiB; page the whole file or later editable saves truncate it. const paged = await readRemoteFileInChunks(remote) diff --git a/src/renderer/src/runtime/runtime-paged-file-read.ts b/src/renderer/src/runtime/runtime-paged-file-read.ts index 63f83ef1b29..627494ba9ed 100644 --- a/src/renderer/src/runtime/runtime-paged-file-read.ts +++ b/src/renderer/src/runtime/runtime-paged-file-read.ts @@ -103,6 +103,16 @@ export async function readFileInChunks(reader: ChunkedFileReader): Promise { + return previewableBinaryMimeType(address.relativePath) + ? readRemoteFilePreview(address, options) + : { content: '', isBinary: true } +} + /** `files.readPreview`, falling back to a chunked read when the file is too large for one reply. */ export async function readRemoteFilePreview( address: RuntimeRemoteFileAddress,