Context
Follow-up from PR #130 (read_file image support, engine/filetools.go),
found during review.
1. PDF/bmp/ico still feed binary garbage as numbered text lines
readFileImageMediaTypes only recognizes image/png, image/jpeg,
image/gif, and image/webp. Any other http.DetectContentType result —
image/bmp, image/x-icon, application/pdf, and so on — falls through to
the ordinary line-numbered text read, which for binary content produces
thousands of lines of mangled bytes.
PDF is the valuable case to close: provider/anthropic and
provider/openai's transcodeBlob already accept application/pdf and
map it to a real wire document block (see imageclamp.go's
emitsAsBlock, which already treats application/pdf as a first-class
block type). read_file could sniff application/pdf and return a
message.Blob{MediaType: "application/pdf", ...} the same way it does
for an image today, closing a real gap with no new wire-format work.
bmp/ico are lower value (rare as agent input) but at minimum
read_file should return a clear text error for a sniffed binary type
outside its allowlist, rather than mangled numbered-line text.
2. Streaming-source TOCTOU regression test
readImageIfDetected (engine/filetools.go) bounds its read via
io.LimitReader(f, budget+1) over the actual open handle, specifically
so a file that grows after os.Open cannot outrun the
readFileMaxImageBytes cap. This was manually verified against a real
FIFO during PR #130's review (a growing pipe never loaded more than the
capped byte count), but there is no committed automated regression test
for it — the existing TestReadFileImageOverCapReturnsTextErrorNoBlob
only shrinks readFileMaxImageBytes against a static file, which does
not exercise the TOCTOU property itself.
A regression test should write to a mkfifo-created pipe (or an
io.Pipe-backed reader for a pure in-process version) from a goroutine
that keeps writing past the cap, and assert readImageIfDetected never
reads more than readFileMaxImageBytes+1 bytes before returning its
over-cap error.
Context
Follow-up from PR #130 (read_file image support, engine/filetools.go),
found during review.
1. PDF/bmp/ico still feed binary garbage as numbered text lines
readFileImageMediaTypesonly recognizes image/png, image/jpeg,image/gif, and image/webp. Any other http.DetectContentType result —
image/bmp, image/x-icon, application/pdf, and so on — falls through to
the ordinary line-numbered text read, which for binary content produces
thousands of lines of mangled bytes.
PDF is the valuable case to close:
provider/anthropicandprovider/openai'stranscodeBlobalready acceptapplication/pdfandmap it to a real wire document block (see imageclamp.go's
emitsAsBlock, which already treatsapplication/pdfas a first-classblock type). read_file could sniff
application/pdfand return amessage.Blob{MediaType: "application/pdf", ...}the same way it doesfor an image today, closing a real gap with no new wire-format work.
bmp/ico are lower value (rare as agent input) but at minimum
read_fileshould return a clear text error for a sniffed binary typeoutside its allowlist, rather than mangled numbered-line text.
2. Streaming-source TOCTOU regression test
readImageIfDetected(engine/filetools.go) bounds its read viaio.LimitReader(f, budget+1)over the actual open handle, specificallyso a file that grows after
os.Opencannot outrun thereadFileMaxImageBytescap. This was manually verified against a realFIFO during PR #130's review (a growing pipe never loaded more than the
capped byte count), but there is no committed automated regression test
for it — the existing
TestReadFileImageOverCapReturnsTextErrorNoBlobonly shrinks
readFileMaxImageBytesagainst a static file, which doesnot exercise the TOCTOU property itself.
A regression test should write to a
mkfifo-created pipe (or anio.Pipe-backed reader for a pure in-process version) from a goroutinethat keeps writing past the cap, and assert
readImageIfDetectedneverreads more than
readFileMaxImageBytes+1bytes before returning itsover-cap error.