From aa66ffce1e56c4c5ef9637b4f877470d7e00d55c Mon Sep 17 00:00:00 2001 From: Kyle Mistele Date: Fri, 4 Sep 2026 17:32:37 -0700 Subject: [PATCH] Harden tool output against binary data HumanLayer-Session: https://app.dev.codelayer.gg/sessions/01a06e93-42af-7eda-b1df-504cf0399c4a --- packages/fold-agent/src/Tools/BashTool.ts | 53 ++++++++++++-- packages/fold-agent/src/Tools/ReadTool.ts | 71 +++++++++++++++++-- .../fold-agent/test/Tools/BashTool.vi.test.ts | 26 +++++++ .../fold-agent/test/Tools/ReadTool.vi.test.ts | 38 ++++++++++ 4 files changed, 177 insertions(+), 11 deletions(-) diff --git a/packages/fold-agent/src/Tools/BashTool.ts b/packages/fold-agent/src/Tools/BashTool.ts index 0b2a74d..0252c67 100644 --- a/packages/fold-agent/src/Tools/BashTool.ts +++ b/packages/fold-agent/src/Tools/BashTool.ts @@ -27,7 +27,7 @@ import { utf8ByteLength, type FoldTool, } from '@humanlayer/fold-core' -import { Duration, Effect, Fiber, FileSystem, Option, Path, Random, Ref, Schema, Semaphore, Stream } from 'effect' +import { Data, Duration, Effect, Fiber, FileSystem, Option, Path, Random, Ref, Schema, Semaphore, Stream } from 'effect' import { ChildProcess, ChildProcessSpawner } from 'effect/unstable/process' import { resolveToCwd } from '../Fs/PathResolve' @@ -76,6 +76,19 @@ const killGrace = Duration.millis(200) // tail-truncation window while the spill file holds the full output. const inMemoryRetentionBytes = 4 * defaultMaxBytes +class BashOutputNotTextError extends Data.TaggedError('BashOutputNotTextError')<{ + readonly stream: 'stdout' | 'stderr' + readonly cause?: unknown +}> {} + +class BashOutputStreamError extends Data.TaggedError('BashOutputStreamError')<{ + readonly stream: 'stdout' | 'stderr' + readonly cause: unknown +}> {} + +const omittedNonTextOutputMessage = (stream: 'stdout' | 'stderr') => + `\n[${stream} output omitted because it contained binary data or invalid UTF-8]` + /** Options for {@link bashTool}. */ export type BashToolOptions = { /** Working directory for resolving relative paths. Defaults to `process.cwd()` at call time. */ @@ -318,19 +331,45 @@ export const bashTool = (options?: BashToolOptions): FoldTool => name: 'stdout' | 'stderr', ): Effect.Effect => Effect.gen(function* () { - const decoder = new TextDecoder() + const decoder = new TextDecoder('utf-8', { fatal: true }) + let rejectedNonTextOutput = false const push = (text: string): Effect.Effect => text.length === 0 ? Effect.void : accumulator .append(text) .pipe(Effect.andThen(events.emit({ tool: 'bash', stream: name, text }))) - - yield* Stream.runForEach(stream, (bytes) => push(decoder.decode(bytes, { stream: true }))).pipe( - Effect.catch((error) => accumulator.append(`\n[${name} stream error: ${String(error)}]`)), + const decode = (bytes?: Uint8Array): Effect.Effect => + Effect.try({ + try: () => { + const text = decoder.decode(bytes, { stream: bytes !== undefined }) + if (text.includes('\0')) throw new Error('null byte in process output') + return text + }, + catch: (cause) => new BashOutputNotTextError({ stream: name, cause }), + }) + + yield* Stream.runForEach( + stream.pipe(Stream.mapError((cause) => new BashOutputStreamError({ stream: name, cause }))), + (bytes) => decode(bytes).pipe(Effect.flatMap(push)), + ).pipe( + Effect.catchTags({ + BashOutputNotTextError: () => { + rejectedNonTextOutput = true + return push(omittedNonTextOutputMessage(name)) + }, + BashOutputStreamError: (error) => + accumulator.append(`\n[${name} stream error: ${String(error.cause)}]`), + }), ) - // Flush any trailing partial UTF-8 sequence (pi's finish()). - yield* push(decoder.decode()) + if (!rejectedNonTextOutput) { + yield* decode().pipe( + Effect.flatMap(push), + Effect.catchTag('BashOutputNotTextError', () => + push(omittedNonTextOutputMessage(name)), + ), + ) + } }) const run = Effect.gen(function* () { diff --git a/packages/fold-agent/src/Tools/ReadTool.ts b/packages/fold-agent/src/Tools/ReadTool.ts index eed563b..1d90f07 100644 --- a/packages/fold-agent/src/Tools/ReadTool.ts +++ b/packages/fold-agent/src/Tools/ReadTool.ts @@ -3,7 +3,7 @@ * head-truncated lines with right-aligned line-number prefixes and pi's continuation notices and * 1-indexed offset/limit; images (the ticket's hard requirement) are magic-byte sniffed, normalized, * auto-resized, and returned as an image content block that RequestBuilder delivers as a native user - * file part (D3). Errors are typed model-visible failures. + * file part (D3). Binary and malformed UTF-8 files are rejected with typed model-visible failures. */ import { defineTool, @@ -25,6 +25,68 @@ import { resolveReadPath, resolveToCwd } from '../Fs/PathResolve' import { detectSupportedImageMimeType, imageSniffBytes } from './Image/Mime' import { processImage } from './Image/Process' +const binaryFileExtensions = new Set([ + '.zip', + '.tar', + '.gz', + '.exe', + '.dll', + '.so', + '.class', + '.jar', + '.war', + '.7z', + '.doc', + '.docx', + '.xls', + '.xlsx', + '.ppt', + '.pptx', + '.odt', + '.ods', + '.odp', + '.pdf', + '.bin', + '.dat', + '.obj', + '.o', + '.a', + '.lib', + '.wasm', + '.pyc', + '.pyo', +]) + +const binaryDetectionSampleBytes = 4_096 + +/** Match OpenCode's extension and control-byte heuristic for model-facing text reads. */ +const isBinaryFile = (path: string, bytes: Uint8Array): boolean => { + const extensionStart = path.lastIndexOf('.') + if (extensionStart !== -1 && binaryFileExtensions.has(path.slice(extensionStart).toLowerCase())) return true + + const sample = bytes.subarray(0, binaryDetectionSampleBytes) + if (sample.length === 0) return false + + let nonPrintableBytes = 0 + for (const byte of sample) { + if (byte === 0) return true + if (byte < 0x09 || (byte > 0x0d && byte < 0x20)) nonPrintableBytes += 1 + } + + return nonPrintableBytes / sample.length > 0.3 +} + +const decodeTextFile = (path: string, bytes: Uint8Array): Effect.Effect => { + if (isBinaryFile(path, bytes) || bytes.includes(0)) { + return Effect.fail(ToolResultFailure.make({ text: `Cannot read binary file: ${path}` })) + } + + return Effect.try({ + try: () => new TextDecoder('utf-8', { fatal: true }).decode(bytes), + catch: () => ToolResultFailure.make({ text: `Cannot read file because it is not valid UTF-8: ${path}` }), + }) +} + /** Render one platform error as a short, model-actionable failure message. */ export const platformErrorMessage = (action: string, path: string, error: PlatformError.PlatformError): string => { return Match.value(error.reason).pipe( @@ -88,7 +150,8 @@ export const readTool = (options?: { readonly cwd?: string }): FoldTool => }) } - return yield* readTextContent(bytes, params) + const text = yield* decodeTextFile(params.path, bytes) + return yield* readTextContent(text, params) }).pipe( Effect.mapError((error) => Schema.is(ToolResultFailure)(error) ? error : ToolResultFailure.make({ text: error.message }), @@ -107,11 +170,11 @@ const numberTextLines = (content: string, startLine: number, outputLines: number /** Read the text path: offset/limit selection, head truncation, line numbering, and continuation notices. */ const readTextContent = ( - bytes: Uint8Array, + text: string, params: { readonly path: string; readonly offset?: number | undefined; readonly limit?: number | undefined }, ): Effect.Effect => Effect.gen(function* () { - const allLines = new TextDecoder().decode(bytes).split('\n') + const allLines = text.split('\n') const startLine = params.offset !== undefined && params.offset > 0 ? Math.max(0, params.offset - 1) : 0 const startLineDisplay = startLine + 1 diff --git a/packages/fold-agent/test/Tools/BashTool.vi.test.ts b/packages/fold-agent/test/Tools/BashTool.vi.test.ts index 89f5c90..3819ec3 100644 --- a/packages/fold-agent/test/Tools/BashTool.vi.test.ts +++ b/packages/fold-agent/test/Tools/BashTool.vi.test.ts @@ -24,6 +24,32 @@ it.live('captures stdout and reports success on exit 0', () => }), ) +it.live('omits null-containing stdout with a model-actionable notice', () => + Effect.gen(function* () { + const dir = yield* tempDir + const ambient = yield* makeAmbientServices() + const result = yield* handlerOf(bashTool({ cwd: dir }))({ command: `printf 'before\\0after'` }).pipe( + Effect.provide(ambient.layer), + ) + + expect(outputOf(result)).toContain('[stdout output omitted because it contained binary data or invalid UTF-8]') + expect(outputOf(result)).not.toContain('\0') + expect((yield* ambient.emitted).map(decodeBashOutputDelta).every((delta) => !delta?.text.includes('\0'))).toBe( + true, + ) + }), +) + +it.live('omits malformed UTF-8 stderr with a model-actionable notice', () => + Effect.gen(function* () { + const dir = yield* tempDir + const result = yield* runHandler(handlerOf(bashTool({ cwd: dir }))({ command: `printf '\\200' >&2` })) + + expect(outputOf(result)).toContain('[stderr output omitted because it contained binary data or invalid UTF-8]') + expect(outputOf(result)).not.toContain('\uFFFD') + }), +) + it.live('passes host-provided session environment to Bash subprocesses', () => Effect.gen(function* () { const dir = yield* tempDir diff --git a/packages/fold-agent/test/Tools/ReadTool.vi.test.ts b/packages/fold-agent/test/Tools/ReadTool.vi.test.ts index 483a385..81763c9 100644 --- a/packages/fold-agent/test/Tools/ReadTool.vi.test.ts +++ b/packages/fold-agent/test/Tools/ReadTool.vi.test.ts @@ -133,6 +133,44 @@ it.effect('fails with a model-actionable message for missing files', () => }), ) +it.effect('rejects binary files without returning database-invalid null bytes', () => + Effect.gen(function* () { + const dir = yield* tempDir + writeFileSync(join(dir, 'binary.dat'), Buffer.from([0x61, 0x62, 0x00, 0x63])) + + const failure = yield* runHandler(handlerOf(readTool({ cwd: dir }))({ path: 'binary.dat' })).pipe(Effect.flip) + + expect(messageOf(failure)).toBe('Cannot read binary file: binary.dat') + expect(messageOf(failure)).not.toContain('\0') + }), +) + +it.effect('rejects a null byte beyond the binary detection sample', () => + Effect.gen(function* () { + const dir = yield* tempDir + writeFileSync(join(dir, 'late-null.txt'), Buffer.concat([Buffer.alloc(5_000, 0x61), Buffer.from([0x00])])) + + const failure = yield* runHandler(handlerOf(readTool({ cwd: dir }))({ path: 'late-null.txt' })).pipe( + Effect.flip, + ) + + expect(messageOf(failure)).toBe('Cannot read binary file: late-null.txt') + }), +) + +it.effect('rejects malformed UTF-8 with a model-actionable message', () => + Effect.gen(function* () { + const dir = yield* tempDir + writeFileSync(join(dir, 'malformed.txt'), Buffer.from([0x61, 0x80, 0x62])) + + const failure = yield* runHandler(handlerOf(readTool({ cwd: dir }))({ path: 'malformed.txt' })).pipe( + Effect.flip, + ) + + expect(messageOf(failure)).toBe('Cannot read file because it is not valid UTF-8: malformed.txt') + }), +) + it.effect('returns PNG images as an image content block with a note (hard requirement)', () => Effect.gen(function* () { const dir = yield* tempDir