refactor(agent-core): unify image extension sniff-failed detection (#974)

Merge the two duplicate image-extension guards in detectFileType into a single mode-independent rule: an image extension without confirming magic is not an image in any mode. The video extension fallback stays before the NUL check so video containers with no magic still win. No behavior change.

Add and align tests: Read rejects an image-extension file with non-image bytes as not readable instead of redirecting to ReadMediaFile; file-type asserts the sniff-failed image case in both media and text modes.
This commit is contained in:
7Sageer 2026-06-22 19:31:48 +08:00 committed by GitHub
parent 27300797f2
commit d434d8f0d8
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
4 changed files with 53 additions and 23 deletions

View file

@ -0,0 +1,5 @@
---
"@moonshot-ai/kimi-code": patch
---
Unify image format detection when sniffing fails.

View file

@ -374,31 +374,27 @@ export function detectFileType(
}
return sniffed;
}
// Sniff failed. In media mode, only video falls back to the extension:
// some video containers (e.g. MPEG-PS `.mpg`) have no magic we recognise,
// so the extension is the only signal. Every image format the model
// accepts (PNG/JPEG/GIF/WebP) has a reliable magic signature, so a failed
// sniff on an image means it is not a supported image — returning the
// extension MIME would only produce a mismatched data URL the model API
// rejects. (Runs before the NUL check so a video extension wins even when
// the header happens to contain a 0x00 byte.)
if (type === 'media') {
if (mediaHint?.kind === 'video') {
return mediaHint;
}
if (mediaHint?.kind === 'image') {
return { kind: 'unknown', mimeType: '' };
}
// Sniff failed.
// An image extension without confirming magic is not an image in any mode.
// Every image format the model accepts (PNG/JPEG/GIF/WebP) has a reliable
// signature, so trusting the extension would only mislead: in media mode it
// builds a mismatched data URL the model API rejects; in text mode it
// redirects the user to ReadMediaFile for a file that is not an image.
if (mediaHint?.kind === 'image') {
return { kind: 'unknown', mimeType: '' };
}
// In media mode, fall back to the extension for video: some containers
// (e.g. MPEG-PS `.mpg`) have no magic we recognise, so the extension is
// the only signal. Runs before the NUL check so a video extension wins
// even when the header happens to contain a 0x00 byte.
if (type === 'media' && mediaHint?.kind === 'video') {
return mediaHint;
}
if (buf.includes(0x00)) {
return { kind: 'unknown', mimeType: '' };
}
// No sniff and no NUL (text mode): still do not trust an image extension
// hint, for the same reason as above. Anything else falls through to the
// No sniff, not an image hint, no NUL: fall through to the
// hint / text / unknown logic.
if (mediaHint?.kind === 'image') {
return { kind: 'unknown', mimeType: '' };
}
}
if (mediaHint) return mediaHint;

View file

@ -214,10 +214,12 @@ describe('detectFileType', () => {
it('returns unknown for an image extension whose bytes fail to sniff', () => {
// A `.png` file with no recognisable image magic and no NUL byte must not
// be reported as `image/png` — otherwise a media reader would ship a
// mismatched data URL that the model API rejects as
// `application/octet-stream`.
// be reported as `image/png` in either mode. In media mode it would build
// a mismatched data URL the model API rejects as
// `application/octet-stream`; in text mode it would redirect the user to
// ReadMediaFile for a file that is not an image.
const garbage = Buffer.from('plain ascii, definitely not a png');
expect(detectFileType('fake.png', garbage, 'media').kind).toBe('unknown');
expect(detectFileType('fake.png', garbage).kind).toBe('unknown');
});

View file

@ -384,6 +384,33 @@ describe('ReadTool', () => {
expect(readText).not.toHaveBeenCalled();
});
it('rejects an image-extension file whose bytes are not an image as not readable', async () => {
// A `.png` file with no recognisable image magic and no NUL byte is not a
// real image; it must fall through to the generic "not readable" error
// rather than being misidentified as an image and sent to ReadMediaFile.
const plainText = Buffer.from('this is plain ascii text, not a png');
const readText = vi
.fn<Kaos['readText']>()
.mockRejectedValue(new Error('readText should not be called for non-image files'));
const tool = new ReadTool(
createFakeKaos({
stat: vi.fn<Kaos['stat']>().mockResolvedValue(REGULAR_FILE_STAT),
readBytes: vi.fn<Kaos['readBytes']>().mockResolvedValue(plainText),
readText,
}),
PERMISSIVE_WORKSPACE,
);
const result = await executeTool(tool, context({ path: '/tmp/fake.png' }));
const output = toolContentString(result);
expect(result.isError).toBe(true);
expect(output).toBe(
'"/tmp/fake.png" is not readable as UTF-8 text. If it is an image or video, use ReadMediaFile. For other binary formats, use Bash or an MCP tool if available.',
);
expect(readText).not.toHaveBeenCalled();
});
it('rejects extensionless image files using magic-byte sniffing', async () => {
const pngHeader = Buffer.from([0x89, 0x50, 0x4e, 0x47, 0x0d, 0x0a, 0x1a, 0x0a]);
const readText = vi