feat(runtime): reject inbound file/media parts loudly (#255 Phase 0)

[initializ-mk]

Part of #255 (multimodal input/output). This is Phase 0 — the footgun fix, shippable on its own ahead of full multimodal support.

Problem

The A2A envelope models file parts (images/documents) and POST /tasks/send accepts them fine — but the runtime projects only text/data parts into the prompt (a2a.Message.PromptText() omits file parts). So a client attaching an image or PDF gets a 200 and a plausible answer that silently ignored the attachment, with no error and no signal. The issue calls this out as a correctness footgun to fix regardless of the full feature.

What this does

Make the drop loud. No model consumes media yet — full multimodal input/output lands in later slices (see the phase plan in the issue).

• a2a.Message.FileParts() — surfaces file parts (index/mime/name) for gating.
• Runner.checkInboundMedia — rejects a message carrying file parts with a clear 4xx (JSON-RPC invalid-params / HTTP 400) that names the offending mime types, and emits a new input_media_rejected audit event (dropped/count/reason). Wired into all four send handlers (JSON-RPC + REST tasks/send & sendSubscribe), before executeTask and before SSE headers commit.
• http.MaxBytesReader (32 MiB) on the REST decode paths — bounds an otherwise unbounded json.Decode on req.Body, and sets the ceiling for inline media later.

Text-only and data-part messages are unaffected.

Why reject (not describe-as-text)

Per the design decision on #255: fail loudly so the client immediately knows their attachment was not considered, rather than degrading to a placeholder. Later phases flip specific (capable model + supported mime) combinations from reject → inline consumption.

Tests

• TestMessageFileParts — unit matrix (text/data ignored; file parts reported with correct index/mime/name; nil FileContent still reported).
• TestRunner_RejectsInboundFileParts — end-to-end across JSON-RPC and REST: a file part is rejected with the mime named; a text-only message still returns 200 (gate must not affect text). Verified the input_media_rejected audit event fires.
• gofmt, go build/go test on forge-core + forge-cli, golangci-lint (0 issues), make sync-knowledge all green.

Docs

• docs/security/audit-logging.md — new input_media_rejected event row.
• .claude/skills/forge.md knowledge skill (synced copy included).

Follow-ups (later #255 phases)

• JSON-RPC transport-layer body cap (this PR caps the REST paths; the JSON-RPC envelope is read upstream as rawParams).
• ChatMessage.Parts plumbing + history references → image input (Anthropic/OpenAI) → document input → persist-inbound-to-.forge/files → model-generated image output.

[initializ-mk]

Self-review — verified against source ✅

Reviewed the gate, the wiring completeness, and the body cap; posting as a COMMENT (own PR) with two LOW notes.

Verified good:

• FileParts() reports file parts correctly (index/mime/name, nil File handled); checkInboundMedia centralizes one contract, emits input_media_rejected, names the mimes.
• Wiring complete — I traced it: executeTask has exactly two callers (JSON-RPC + REST tasks/send), both gated; the two streaming paths (ExecuteStream) are gated at their handlers. All four inbound paths covered, no un-gated caller, no scheduled/in-process ingest bypass. Response shapes are right per path, and REST sendSubscribe rejects before the SSE Content-Type commits (clean 400 still possible).
• MaxBytesReader(32 MiB) bounds both REST decode paths.
• Tests genuine (FileParts matrix; e2e reject across JSON-RPC & REST with mime named; text-only still 200; audit event fires).

Two LOW notes inline (one already on your follow-up list).

Nit: commit authored as MK vs initializ-mk.

[initializ-mk]

LOW (future-proofing): per-handler gating, no single choke point. This gate is invoked from 4 handlers; current coverage is complete (verified: 2 executeTask callers + 2 ExecuteStream handlers, all gated). But there's no one shared point below them in the runtime — the non-streaming paths funnel through executeTask, the streaming ones through ExecuteStream — so a future 5th ingest path (a new endpoint, an in-process channel→executor call, a batch/replay) would silently reintroduce the footgun unless it remembers this gate. Consider a structural backstop at the executor boundary (forge-core Execute/ExecuteStream) in a later #255 phase so media rejection is guaranteed regardless of caller. Not needed for Phase 0 — just flagging the maintenance risk of a 4-call-site invariant.

[initializ-mk]

LOW (already on your follow-up list): the 32 MiB cap is REST-only. MaxBytesReader is applied on the two REST decode paths, but the JSON-RPC send/sendSubscribe bodies are read upstream as rawParams, so they remain unbounded — the memory-exhaustion bound is incomplete on the JSON-RPC transport until the follow-up cap lands. Fine to defer (you've noted it), just making the current asymmetry explicit for anyone reading this as "the body is capped."

[initializ-mk]

Thanks — addressed both LOW notes.

LOW #2 (body cap) — fixed the real issue. The premise that JSON-RPC bodies are unbounded isn't quite right: server.handleJSONRPC already applies http.MaxBytesReader(w, r.Body, 2<<20) (2 MiB) with a proper 413 (a2a_server.go:267). What my PR did introduce, though, was an asymmetry — a new 32 MiB cap on the REST paths vs. the established 2 MiB on the JSON-RPC transport. Fixed in 0e8ed5f by aligning the REST cap to the same 2 MiB bound, with a comment cross-referencing handleJSONRPC so the two can't silently drift. Net: REST goes from unbounded (pre-PR) to a uniform 2 MiB, and the inline-media phases raise both transports together deliberately.

LOW #1 (no single choke point) — deferred as you suggested, now tracked concretely. Agreed the 4-call-site invariant is a maintenance risk. Per your note it's not needed for Phase 0, and a proper backstop belongs at the executor boundary with a typed error the handlers map to a 4xx (a half-measure in executeTask would only cover the non-streaming paths and would degrade the clean 4xx to a 500). Captured as a follow-up for a later #255 phase: a typed ErrUnsupportedInboundMedia guard at both choke points (executeTask + ExecuteStream) so rejection is guaranteed regardless of caller.

Nit (commit author MK vs initializ-mk): local git config user.name; noted.

Full sweep re-run green (build, go test ./runtime/..., golangci-lint 0 issues).

[initializ-mk]

Follow-up on the body-cap fix — requesting one change

Correction to my earlier review first: I'd said the JSON-RPC path was unbounded — it was actually already capped at 2 MiB in server.handleJSONRPC. Thanks; the commits surfaced that.

0e8ed5f9b correctly aligned REST down to 2 MiB (both 2 MiB). But 0c1e29b7f then raised both to 32 MiB, which raises the JSON-RPC bound 16× — and Phase 0 rejects all media, so that headroom admits nothing usable today (a 32 MiB body is decoded into memory and then rejected by checkInboundMedia, or is 32 MiB of text). Net effect during a footgun-hardening PR is a looser DoS posture.

Requesting change (inline): revert the cap to 2 MiB on both transports (keep the parity from 0e8ed5f9b, drop 0c1e29b7f's raise); revert the TestHandleJSONRPC_OversizedBody threshold accordingly. Raise to 32 MiB in the inline-media phase, landed together with the controls that make a large cap safe (per-part count/size limits, image decode-dimension bounds, a concurrency semaphore) — a flat MaxBytesReader alone is not media DoS protection. Full checklist posted on #255.

[initializ-mk]

Requesting change (MEDIUM): keep this at 2 MiB for Phase 0 (and match server.handleJSONRPC back to 2 MiB). Media is rejected here, so 32 MiB buys nothing but a 16× larger memory-exhaustion surface on the JSON-RPC path (concurrent 32 MiB decodes). The intermediate commit 0e8ed5f9b (both at 2 MiB, in parity) was the right Phase-0 state. Raise to 32 MiB in the phase that actually consumes inline media, alongside per-part limits + decode-dimension bounds + a concurrency semaphore (see the #255 checklist). A comment here noting the raise is deferred to the media phase would prevent it drifting back.

[initializ-mk]

Done — reverted to 2 MiB on both transports in 92c4675.

You're right: Phase 0 rejects all media, so the 32 MiB raise admitted nothing usable and only widened the JSON-RPC memory-exhaustion surface during a hardening PR. Both transports are back to the parity state from 0e8ed5f (REST runtime.maxRequestBodyBytes + server.handleJSONRPC, both 2 MiB), and TestHandleJSONRPC_OversizedBody is back to the 3 MiB threshold.

Added a comment on maxRequestBodyBytes recording that the raise is deliberately deferred to the media-consuming phase, where 32 MiB lands together with the controls that make a large cap safe — per-part count/size limits, image decode-dimension bounds, and a concurrency semaphore (a flat MaxBytesReader alone isn't media DoS protection) — so it can't quietly drift back.

Remaining LOW #1 (executor-boundary backstop) stays tracked as a follow-up for a later #255 phase. Re-ran build + server/runtime tests + golangci-lint (0 issues).

[initializ-mk]

Closing re-review — commit 92c46754 ✅ LGTM

Verified against source. The requested change is resolved.

• DoS bound — reverted to 2 MiB on both transports. runner.go maxRequestBodyBytes and server.handleJSONRPC are both back to 2 MiB and in parity; the 16× JSON-RPC loosening is undone. Since Phase 0 rejects all media, this is the right ceiling.
• Drift-guarded. The comment now states the cap is deliberately not raised yet and that the media-consuming phase raises it with per-part limits + image decode-dimension bounds + a concurrency semaphore (a flat MaxBytesReader alone isn't media DoS protection). Checklist co-located with the code and mirrored on #255.
• Test reverted — TestHandleJSONRPC_OversizedBody back to 3 MiB-vs-2 MiB.

Everything on the PR now checks out:

• Media-reject gate wired into all four inbound paths (2 executeTask + 2 ExecuteStream, no un-gated caller), correct per-path response shapes, REST-subscribe rejects before SSE headers commit.
• FileParts() / checkInboundMedia sound; input_media_rejected audit event; genuine tests (FileParts matrix, e2e reject across JSON-RPC & REST, text-only still 200, audit fires).
• DoS bound at the correct 2 MiB with cross-transport parity.

LOW #1 (executor-boundary backstop) was future-proofing, correctly deferred to a later #255 phase. (Correction from earlier in this thread: the JSON-RPC path was already 2 MiB-capped, not unbounded.)

Verdict: LGTM. Needs a second reviewer to merge (own PR). Nit: commit authored as MK vs initializ-mk.