fix(security): derive the ACP image mimeType from decoded content

[adiarora06]

Closes #3769.

Problem / Motivation

acp/prompt_blocks.py chose an image's wire mimeType from its path suffix:

suffix = path.suffix.lower()
mime = IMAGE_MEDIA_TYPES.get(suffix)

A suffix is a claim made by whoever named the file — an upload handler that kept a client-supplied name, a channel that renamed an attachment, a screenshot tool with its own convention. Every producer feeding this shared sink therefore needed its own guard to keep that claim honest, which is what #3754's review flagged.

Two concrete failures follow:

1. A renamed image ships mislabelled. An image already inside the dimension cap rides through _downscale_within_limits byte-identical (return raw_bytes, mime), so a JPEG named .png reaches the model as JPEG bytes declared image/png.
2. A non-image named like one is inlined anyway. notes.png becomes an image block the backend cannot decode.

Why it matters

The second failure above is not a one-turn error. kiro-cli replays the full message history every turn, so a rejected block sits at a fixed history index and wedges the session from then on — the same consequence MAX_IMAGE_EDGE_PX and MAX_IMAGE_B64_BYTES already fail closed to avoid. This path was failing open on the one question they do not ask, so a single mislabelled or misnamed file can permanently break a conversation rather than just failing that one turn.

What changed

The suffix now decides only which paths are candidates — it is what _PATH_RE matched on, and re-checking it keeps a .txt from costing a stat. What the file is gets decided from its bytes, in a new _sniff_media_type:

• Pillow's reported format wins when Pillow is installed: it comes from a real decode, and it is header-only (Image.open populates format without decompressing), so this costs nothing on the common path.
• A magic-byte signature table is the no-Pillow fallback. This is the part that matters for the issue's "hand-stripped no-Pillow install retains a suffix-only fallback": degrading to the suffix would leave the sink trusting a filename exactly where it has least help. RIFF is matched together with its form field at offset 8, so a WAV does not read as a WEBP.
• Unsupported or undecodable → left as a path, not inlined. Fail closed, same direction as the size caps.
• A content/suffix mismatch is not a refusal. The image is real, so it still travels — labelled by its content. Producers rename attachments routinely, and dropping a good image to placate a filename would trade a real capability for nothing. The mismatch is logged at INFO.

This also fixes the re-encode, not just the label: _downscale_within_limits picks its Pillow save format from the mime (_PIL_SAVE_FORMAT), so a wrong mime produced a wrongly-encoded payload as well as a wrong declaration.

Scope: Sink only. The issue's follow-on — "remove redundant producer-side image-suffix tables/renames where safe" — is deliberately not here. Those producer guards are defense in depth, and removing them is precisely the part that needs the broader producer-compatibility coverage #3754 declined to take on. Making the sink authoritative is the prerequisite; deleting the now-redundant guards can follow once this has soaked.

Tests

• New TestMediaTypeFromContent (14 tests): content wins over a misleading suffix; a renamed-but-real image is still inlined; fail-closed for a text file named .png, for SVG bytes behind a raster name (SVG is excluded by policy, and excluding it by suffix alone let the same bytes through under another name), and for a truncated image; the signature table agrees with Pillow on every supported format with _HAS_PIL patched off, and refuses a RIFF that is not WEBP; the re-encode follows content; GIF still collapses to a PNG still frame.
• test_every_supported_suffix_maps_to_its_mime was rewritten to write REAL bytes of each format. It previously wrote a PNG behind every suffix — which is exactly why it could pass while the mime was being read off the filename. That test pinned the bug.
• 16 tests fail on main with the source change reverted and the tests kept.
• test_acp_prompt_blocks.py: 86 passed, 1 skipped. Full ACP/image/upload/attachment suites: 2132 passed (5 unrelated failures are worktree-environment artifacts — an AF_UNIX path too long, two bare-interpreter import checks, and an RSS measurement flake).
• flake8 / isort clean; ./scripts/docs-lint.sh passes.
• docs/system-specs/modules/acp-client.md documents the content-derived contract and the fallback.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[dwu96]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[bolichen97]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[NicholasRBowers]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[iamwhatever]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[dwu96]

👋 Hi! This PR's description is missing some required sections from our PR template. Workflow runs won't be auto-approved until the description is updated.

Missing sections:

• ## Problem / Motivation
• ## Why it matters
• ## What changed
• ## Tests

Please update your PR description to include these sections, then push or re-save the description. The workflows will be approved on the next cycle.

[github-actions]

GPT 5.6 Review (fork) — 🔴 changes requested (blocking)

Reviewed 46c27e8122a901865927470c6c9b625452b1e246 via the fork AI-review pipeline; updated in place on each push.

BLOCKING -- src/kiro_crew/acp/prompt_blocks.py:196 -- No-Pillow fallback accepts truncated image headers

return mime
Truncated image upload -> signature sniff -> no-Pillow pass-through -> invalid ACP block poisons replayed session history.
Anchor: residual/crash-data-loss-corruption
Fix: Return None when Pillow is unavailable instead of accepting signature-only matches.
[BLOCK-MERGE] 46c27e8
[GPT-REVIEWED] 46c27e8

[github-actions]

Opus 4.8 Review (fork) — ✅ no blocking findings

Reviewed 46c27e8122a901865927470c6c9b625452b1e246 via the fork AI-review pipeline; updated in place on each push.

Review details

No blocking issues; one advisory finding.

FINDING — src/kiro_crew/acp/prompt_blocks.py:130 — _PIL_FORMAT_MEDIA_TYPE omits "MPO", so a .jpg phone photo that Pillow reports as format "MPO" (multi-picture JPEG, a real and common case) now sniffs to None and is dropped to a text path; the old code took mime from the suffix (image/jpeg) and downscale_image_block re-encoded any decodable format to JPEG/PNG, so it was inlined — a non-tool-capable agent silently loses the image → Fix: add "MPO": "image/jpeg" (and any other JPEG-family Pillow format names you still want inlined) to _PIL_FORMAT_MEDIA_TYPE, or fall through to downscale_image_block when Pillow decodes the bytes but reports an unlisted format.

[OPUS-REVIEWED] 46c27e8

[github-actions]

Design Review (Fable 5, fork) — 🟡 CONCERNS

Design-level review of 46c27e8122a901865927470c6c9b625452b1e246 via the fork AI-review pipeline — updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

Design-Verdict: CONCERNS

Sound fail-closed direction, but the sniff duplicates format identity imaging.py already owns, and its closed Pillow table refuses decodable JPEGs the base inlined.

Watch

• The description's two failures don't reproduce on a Pillow-installed base: _downscale_within_limits (landed in feat(mcp-gateway): hold tool-result image blocks to the inline-image budget (#4010) #4025) already detects, relabels (return raw_bytes, _FORMAT_MIME[detected]), verifies, and fails closed. The real delta is the no-Pillow signature fallback plus re-encode-format fidelity — calibrate the "soak, then delete producer guards" plan to that narrower baseline.
• The Pillow branch maps img.format through a closed five-entry table, so Pillow-decodable variants it reports differently — MPO, the multi-frame JPEG common camera files named .jpg decode as — return None and are refused, where base re-encoded and inlined them. The signature branch calls the same \xff\xd8\xff bytes image/jpeg, so the two branches disagree; test_signature_table_matches_pillow checks only the other direction.
• _PIL_FORMAT_MEDIA_TYPE duplicates imaging._FORMAT_MIME entry-for-entry; adding or dropping a format now touches parallel tables in two modules, and divergence ships as the sniff refusing what the downscaler handles.

Suggestions

• Host the sniffer and signature table in the imaging.py leaf beside _FORMAT_MIME/_PIL_SAVE_FORMAT (prompt_blocks imports it): one owner of format identity, and it fixes imaging's own no-Pillow pass-through for the gateway sink too.
• Let a Pillow-decodable-but-off-table format fall through to the existing re-encode path instead of refusing it.

[DESIGN-REVIEWED] 46c27e8

[github-actions]

First Principles Review (Fable 5, fork) — 🔴 BLOCK

Premise-level review of 46c27e8122a901865927470c6c9b625452b1e246 via the fork AI-review pipeline — why this exists and whether the shipped surface is the smallest honest version. Updated in place on each push. A BLOCK verdict blocks PR readiness; PASS/CONCERNS are advisory.

First-Principles-Verdict: BLOCK

The sink already detects content — imaging.py labels by decoded format and fails closed, raster.py owns the magic table — this PR rebuilds both instead of calling them.

What this change ships

Intent: stop a misnamed or non-image file from shipping a wrong mimeType that wedges the ACP session — a FIX.

1. A fake image named .png stays a text path on no-Pillow installs — justified (issue security: derive ACP image MIME from decoded content #3769)
2. No-Pillow installs label images by bytes, not filename — justified, the actual fix
3. Over-cap renamed image re-encodes in its true format, not the suffix's — rides along
4. Content/suffix mismatch now logs a line — rides along
5. New _IMAGE_SIGNATURES table + sniffer loop — duplicate of messaging/raster.py:sniff_raster_mime
6. New _PIL_FORMAT_MEDIA_TYPE dict — duplicate of imaging._FORMAT_MIME (same 5 entries)
7. New module-level _HAS_PIL flag — duplicate of imaging.pil_available()
8. Spec section in acp-client.md — justified, same-commit doc rule
9. Suffix test rewritten to byte fixtures — rationale contradicted by the removed hunk
10. 14 new tests — justified, though several already pass on base with Pillow

Blockers

_sniff_media_type re-implements two existing mechanisms, on a premise the base contradicts. The magic-byte half is a second spelling of messaging/raster.py:sniff_raster_mime — identical 5-mime set, identical WEBP form-tag check, 7 counted consumers (grep sniff_raster_mime: dashboard/handlers/files.py, messaging/attachments.py, messaging/outbound_files.py, whatsapp/client.py, pptx_maker, mochi), and that module's own docstring warns "a second copy of the magic table is how one direction ends up accepting a type the other rejects." The Pillow half re-does what imaging._downscale_within_limits already does downstream: base imaging.py:209 returns raw_bytes, _FORMAT_MIME[detected] — commented "the caller's claim can lie" — and returns None (fail closed) on undecodable or truncated bytes. So the description's headline failures are absent on any Pillow install (Pillow is a hard dependency per imaging.py:127): "a JPEG named .png reaches the model as JPEG bytes declared image/png" quotes return raw_bytes, mime, which is only the no-Pillow branch; and "It previously wrote a PNG behind every suffix" is contradicted by the removed hunk itself ("Fixtures are format-faithful (real bytes per format, not one PNG renamed)"). The real, derived fix is items 1–2 only. Subtraction: delete _IMAGE_SIGNATURES, _PIL_FORMAT_MEDIA_TYPE, _HAS_PIL, and _sniff_media_type's body; call sniff_raster_mime(raw_bytes[:SNIFF_BYTES]) (hosted in the imaging leaf if an acp→messaging import is unwanted). Every new test still passes: the sniff gates entry, and imaging's existing verify()/fail-closed handles truncation.

[FIRST-PRINCIPLES-REVIEWED] 46c27e8

[bolichen97]

Audit note — part of this has already landed; the rest has not

This PR is not a duplicate and is not finished by anything on main. The audit checked it part by part against main, and some of what it does is already there. Flagging it so a reviewer does not have to rediscover the overlap, and so the PR is not mistaken for fully-covered work.

Which parts main already has

P5 (content-derived wire mimeType for any Pillow-decodable image, in-cap via imaging._downscale_within_limits -> return raw_bytes, _FORMAT_MIME[detected] at imaging.py:209, over-cap via out_mime = _FORMAT_MIME.get(fmt, 'image/png') at imaging.py:240) and P6 (fail CLOSED for a text file / SVG bytes / truncated raster named like an image, via imaging.py:242-251's except Exception: return None and prompt_blocks' existing if downscaled is None: continue). Both are pinned on main by test_mcp_gateway_image_budget.py test_lying_mime_label_is_corrected_on_passthrough / test_unknown_source_mime_relabels_to_the_written_format / test_undecodable_returns_none / test_truncated_image_returns_none. P14 (a format-faithful test_every_supported_suffix_maps_to_its_mime) is already on main verbatim. P2/P3/P4 exist as equivalent INGREDIENTS on main (imaging._FORMAT_MIME, messaging.raster.sniff_raster_mime + _MAGIC, imaging.pil_available) but P3 is not reachable from the ACP prompt path.

What is still genuinely yours

P7 the no-Pillow branch (imaging.py:184-185 if pil is None or max_edge <= 0: return raw_bytes, mime -- a hand-stripped install still labels from the suffix and still inlines a non-image); P8 the same line's max_edge <= 0 escape hatch; P9 the re-encode save format still chosen from the suffix (_PIL_SAVE_FORMAT.get(mime, 'PNG'), imaging.py:227); P10 the mismatch INFO log; P11 the distinct 'not a supported image by content' WARNING; P12 the suffix not in IMAGE_MEDIA_TYPES candidate-only source rewrite; P13 the whole TestMediaTypeFromContent class (~5 of its 11 functions genuinely fail on main, incl. every no-Pillow signature case); P15 the entire acp-client.md content-derived-media-type contract. Plus P1's _sniff_media_type symbol itself.

Suggested action: CONTINUE_DEV — the remainder is real work; rebase onto the landed part rather than closing.

---

_{From a repository-wide duplicate/overlap audit of every pull request open against main (2026-09-02, 330 PRs, one reviewer per PR). Each PR was read as its full merge-base diff plus its description and every comment and review, then compared against each candidate PR's own diff and against origin/main at 1a765b88ceb7. This PR is not being closed — the note is informational. If the reading is wrong, please correct the reasoning rather than just the conclusion.}

[bolichen97]

Open PR relationship audit

This is a consolidated, point-in-time code-level audit note. It compares complete merge-base diffs and current/merged code; it does not treat a shared topic as duplication or partial coverage as completion.

Relationship findings

• PR #6305 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #6305: CONTINUE_DEVELOPMENT. Different security property at the same line; whichever lands second rebases, no work is redundant. Files: src/kiro_crew/acp/prompt_blocks.py.
• PR #8238 is OVERLAPPING relative to this PR. The goals differ or the implementations can complement each other; this is not a duplicate claim. Recommended action for PR #8238: CONTINUE_DEVELOPMENT. Different user goal, disjoint hunks, and a clean merge. The only coupling is the acp-client.md Image Support step that 4079 preserves and 8238 silently invalidates; whichever lands second must reconcile that line. Files: src/kiro_crew/acp/prompt_blocks.py, docs/system-specs/modules/acp-client.md.
• This PR is PARTIALLY_COVERED with PR #4025. Coverage is explicitly incomplete; this finding is not a completion or closure claim. Recommended action for PR #4079: CONTINUE_DEVELOPMENT. PR #4025 landed content-derived labelling and fail-closed verification for every Pillow-present path, which is the majority of what this PR's description claims as new. The no-Pillow branch, the max_edge<=0 escape hatch, the suffix-keyed re-encode format, and the documented sink contract are not covered. Not closable as completed. Files: src/kiro_crew/imaging.py.

No PR, Issue, label, branch, or review state was changed by the relationship-note portion of this audit.

[chenmingwei23]

Kiro Crew [operator: chenmingwei23]: This PR has been inactive for 7+ days. I reviewed the blockers but they require your input:

• First Principles Review is a hard BLOCK on approach, not a bug: it argues the sink already detects content -- imaging.py labels by decoded format and fails closed, messaging/raster.py owns the magic-byte table -- and this PR rebuilds both (a new _IMAGE_SIGNATURES sniffer, a _PIL_FORMAT_MEDIA_TYPE dict duplicating imaging._FORMAT_MIME, a _HAS_PIL flag duplicating imaging.pil_available()). Deciding whether to call the existing modules vs keep the new local copies is a design call that changes the PR's structure.
• GPT 5.6 also blocks on a concrete point: the no-Pillow fallback accepts signature-only (possibly truncated) headers at prompt_blocks.py:196 and returns the mime anyway; it suggests returning None when Pillow is unavailable. Your call on whether that is the intended fail-closed behavior.
• The PR description is missing the required template sections (Problem / Motivation, Why it matters, What changed, Tests), which is also suppressing fork workflow auto-approval.

When you've addressed these, the pipeline will re-assess on its next cycle.

[bolichen97]

@adiarora06 Thanks for this. Most of the headline claim has since landed on main, so the PR needs narrowing rather than a straight merge.

Already on main via merged #4025: imaging.py _downscale_within_limits now calls img.verify() and returns _FORMAT_MIME[detected], so for any Pillow-present install the wire mimeType already tracks the decoded format, and a text file or SVG named .png already fails closed and stays a text block.

Still missing on main, and what this PR uniquely provides: the no-Pillow branch in imaging.py still labels from the suffix and inlines an unverified file; the max_edge <= 0 escape does the same; the re-encode format is still suffix-keyed (_PIL_SAVE_FORMAT.get(mime, "PNG")), so an over-cap JPEG named .png is re-encoded as PNG; acp/prompt_blocks.py still does mime = IMAGE_MEDIA_TYPES.get(suffix) with no mismatch log; and the content-derived media-type contract is absent from docs/system-specs/modules/acp-client.md.

Could you rebase onto current main (this is 1224 commits behind and currently conflicting, since merged #6305 rewrote the same lines) and narrow the diff to that remainder? Please also address the two blocking reviews: reuse messaging/raster.py::sniff_raster_mime and imaging._FORMAT_MIME instead of the new local _IMAGE_SIGNATURES/_PIL_FORMAT_MEDIA_TYPE tables, and decide the truncated-header no-Pillow case.

On overlap with open #8238 (@snandi1603): both touch src/kiro_crew/acp/prompt_blocks.py and test/test_acp_prompt_blocks.py, but the hunks are disjoint and the goals differ, this PR changes which mimeType the image block carries, #8238 deletes the text.replace(raw, f"[image: {path.name}]") fallback line. The one real collision is documentation: this PR renumbers and keeps the acp-client.md Image Support step saying the path is replaced, which #8238 makes false. #8238 is under a design decision first, so please land the code here independently and let whichever merges second fix that spec line in its own commit.

Posted from the 2026-09-08 open-PR relationship audit (read-only, one auditor per PR); reply here if any of this is wrong.