mirror of
https://github.com/stablyai/orca.git
synced 2026-10-31 09:07:40 +00:00
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.
This commit is contained in:
@@ -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
|
||||
})
|
||||
})
|
||||
})
|
||||
@@ -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')
|
||||
}
|
||||
|
||||
|
||||
@@ -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') {
|
||||
|
||||
@@ -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)
|
||||
|
||||
@@ -103,6 +103,16 @@ export async function readFileInChunks(reader: ChunkedFileReader): Promise<Paged
|
||||
return { kind: 'ok', bytes }
|
||||
}
|
||||
|
||||
/** A binary the host refused as text: only images and PDFs preview, so no other is fetched. */
|
||||
export async function readRemoteBinaryFile(
|
||||
address: RuntimeRemoteFileAddress,
|
||||
options: { pageOversized: boolean }
|
||||
): Promise<RuntimeFilePreviewResult> {
|
||||
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,
|
||||
|
||||
Reference in New Issue
Block a user