fix: refuse unsupported image formats instead of poisoning sessions - #1536
Conversation
Images in formats providers reject (AVIF, HEIC, BMP, TIFF, ICO) used to pass through to the API, and the resulting HTTP 400 repeated on every later turn because the image_url stayed in the session history. Add a single format policy (accepted set: PNG/JPEG/GIF/WebP) enforced at every ingestion point: ReadMediaFile refuses with a per-OS conversion command; MCP tool results, REST uploads, and ACP prompts replace the image with a text notice; and turn.prompt/steer gates as the last-funnel backstop so the SDK/RPC path cannot poison a session either. Accepted MIME aliases (image/jpg, case/whitespace) are forwarded in canonical form, and data URLs carrying MIME parameters can no longer slip past the gate. Remote image URLs pass through (no bytes to inspect).
🦋 Changeset detectedLatest commit: 347d6ee The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
commit: |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: b0e26a26ac
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
The format gate compared only the MIME token when deciding whether to rebuild a data URL, so an accepted image carrying MIME parameters (`data:image/jpeg;charset=utf-8;base64,...`) was forwarded with its original header. The Anthropic provider splits the data URL and exact-matches the full header against its whitelist, so the part still poisoned the session. Rebuild to the byte-exact canonical URL whenever the original differs, covering aliases, case/whitespace, and parameters with one comparison. Addresses review feedback on PR #1536.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 46a181d107
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An uppercase `;BASE64,` marker is legal (RFC 2045 encoding names are case-insensitive), but the parser required a lowercase match and returned null, so the gate treated the URL as remote and forwarded it: an unsupported image could still land in the session history, and the Anthropic provider's lowercase-only split then threw on every turn. Match the scheme and marker case-insensitively; the canonical rebuild emits the lowercase form. Addresses review feedback on PR #1536.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f17e188225
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Two more ways an unsupported image could reach the provider are closed: - Bytes, not labels, decide the format. A data-URL image whose declared MIME disagrees with its magic bytes (e.g. AVIF bytes an image search tool labels image/png) is now gated on the sniffed format at every entry point (MCP results, ACP, SDK/RPC prompt, REST inline and file uploads), so a mislabel cannot slip past the gate. - A poisoned image already in the session history no longer kills the session: a server image-format 400 (or kosong's client-side image rejection) now retries once with every media part replaced by a text marker, mirroring the 413 media-degraded recovery. The recovery also fires during compaction, and the transient-retry fallback no longer burns the retry budget on image-format errors before the dedicated recovery can run.
Remote image URLs (MCP resource_link, REST `kind: 'url'`) carry no bytes to sniff, so a link ending in `.avif` (or `.heic`, `.bmp`, `.tiff`, `.ico`) would pass through and be fetched server-side — and rejected. Reject such URLs by their path extension instead (query/fragment ignored, case-insensitive); extensionless or accepted-extension URLs still pass through to the provider and the 400 recovery.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: fbb244413a
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…ry scope Address two review findings on PR #1536: - A declared media type with parameters (e.g. image/jpeg; charset=utf-8) is no longer misread as unsupported: normalizeImageMime now strips parameters, matching the data-URL parser, so an accepted image with parameters is forwarded instead of dropped. - The image-format recovery predicate is narrowed to specific format/data rejection phrases, so a 400 about image count, size, or image-input support no longer triggers a media-stripped resend that would let the model answer blind to the user's images.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 09f63e7da8
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3aba6fc6a0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
- The media_type/mime_type recovery match now requires the message to mention an image, so a video/audio media_type rejection surfaces instead of triggering a blind media-stripped resend. - unsupportedImageMimeFromUrl flags .svg URLs as image/svg+xml without touching the shared suffix map (SVG stays text for the file tools), so remote SVG images get the intended notice instead of a provider rejection. Addresses review feedback on PR #1536.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 8f7c0bdae5
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
An MCP resource_link with an extensionless or signed URL gives the extension gate nothing to work with, and convertMCPContentBlock was discarding the declared mimeType — an honestly-declared AVIF/HEIC link from an image search tool still became an image_url and poisoned the session. Reject on the declared MIME when the server provides one: unsupported declarations become a text notice that keeps the URL so the model can fetch and convert it; accepted declarations pass through as before. Addresses review feedback on PR #1536.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 385c5e141f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
…RLs in notices
- Drop the bare `media` alternative from the image-format recovery
patterns so audio/video media rejections ("unsupported media type",
"invalid media type") can never be misclassified as image errors and
blindly media-stripped; every pattern now mentions "image" literally.
- Remote image URLs rejected by their extension now keep the URL in the
replacement notice (gateImageFormatParts and the REST url path), so the
model can still fetch and convert the image — matching the declared-MIME
resource_link path.
Addresses review feedback on PR #1536.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4e9a5f59ad
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const magick = `magick "${path}" "${converted}"`; | ||
| const linuxDecoder = format?.linuxDecoder; | ||
| switch (osKind) { | ||
| case 'macOS': | ||
| return `On macOS: sips -s format jpeg "${path}" --out "${converted}"`; |
There was a problem hiding this comment.
When an unsupported image path contains shell syntax such as $(), backticks, backslashes, or a double quote (valid filename characters on POSIX), these suggested conversion commands interpolate the raw path inside double quotes. If the model follows the guidance through Bash, double quotes still allow expansions and embedded quotes break out of the argument, so the command can be malformed or execute something other than a conversion; shell-quote both the source and converted paths before composing the sips, heif-convert, and magick commands.
Useful? React with 👍 / 👎.
…ison the session A `data:` URL that fails to parse (missing `;base64,` separator, empty MIME, …) was treated like a remote URL and passed through the format gate; the provider then rejects it on every turn, and the read-side media-stripped recovery keeps paying that round-trip until compaction. Detect unparseable `data:` URLs in gateImageFormatParts and replace them with a (truncated) notice at ingestion, covering the MCP/ACP/SDK/turn paths that share the gate. Addresses review feedback on PR #1536.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 347d6ee6bc
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| name === undefined || name.length === 0 | ||
| ? `unsupported image format ${mimeType}` | ||
| : `"${name}" uses unsupported image format ${mimeType}`; |
There was a problem hiding this comment.
When an MCP tool returns an unsupported remote image with a long URL, such as a signed .avif link, gateImageFormatParts() calls this helper with the URL as name after MCP text budgeting has already run. The full URL is then injected as a text notice and bypasses both MCP_MAX_OUTPUT_CHARS and the later binary URL cap, so a single tool result can still put megabytes of text into the next model request; truncate/budget the notice name or re-run the text cap after these replacements.
Useful? React with 👍 / 👎.
1 participant
Related Issue
No linked issue — the problem is explained below.
Problem
Model providers only accept PNG/JPEG/GIF/WebP image blocks, but kimi-code sniffed other formats (AVIF, HEIC, BMP, TIFF, ICO) and shipped the bytes through, letting "the provider decide". A user uploading an AVIF image, or an image-search MCP tool returning one, makes the API respond with HTTP 400 — and because the
image_urlstays in the session history, every subsequent request in the session fails too, leaving the session unrecoverable.What changed
A single format policy enforced at every ingestion point, so an unsupported image can never land in the session history (closed set: PNG/JPEG/GIF/WebP — matching what Codex CLI and Claude Code do):
image-format-policy): the accepted set, a refusal registry (AVIF, HEIC/HEIF, BMP, TIFF, ICO) with per-OS conversion guidance (macOSsips, Linuxheif-convert/ImageMagick, Windows ImageMagick), and the notice text. Adding a future format is one row in the table.turn.prompt/turn.steer: last-funnel backstop so the SDK/RPC prompt path cannot poison a session either.image/jpg, case/whitespace variants) are forwarded in canonical form, because strict provider whitelists (e.g. Anthropic's) reject the raw alias and would re-create the poisoning; data URLs carrying MIME parameters can no longer slip past the gate.Tests cover AVIF still/animated brands, BMP/TIFF/ICO, HEIC parity, MCP media-only vs text-accompanied results, alias canonicalization, MIME-parameter data URLs, server inline + upload paths, ACP, and the prompt/steer entry gate. A changeset for
@moonshot-ai/kimi-code(patch) is included.Checklist
gen-changesetsskill, or this PR needs no changeset.gen-docsskill, or this PR needs no doc update. (Checked — user docs don't describe format-level tool behavior, so no update needed.)