fix(gateway): bound inbound media download size in the cache helpers

[youngstar-eth]

What & why

cache_image_from_url and cache_audio_from_url did cache_*_from_bytes(response.content, ext) with no size limit. The URLs they download come from inbound platform message payloads (e.g. WhatsApp data["mediaUrls"], Signal/Feishu/BlueBubbles attachment URLs), so a remote sender chooses the host. The 30s timeout bounds wall-clock, not response size, so a single message pointing at a very large file could buffer an unbounded body and persist it to the cache directory (memory spike / disk fill); the cache is only pruned on a 24h age cutoff.

Add a 50 MiB cap (matching tools/vision_tools._VISION_MAX_DOWNLOAD_BYTES) enforced via a shared _enforce_media_size_limit() helper: reject on an oversized Content-Length header and re-check the actual body length before caching. Oversized responses raise ValueError (not a retryable error) so they fail fast without being re-downloaded.

Robustness / DoS-hardening fix.

How to test

pytest tests/gateway/test_media_download_retry.py

Platforms

macOS (pure-Python; httpx).

🤖 Generated with Claude Code

[alt-glitch]

Addresses #13145 (no size cap on inbound media downloads). Note competing/overlapping fix #13341 (configurable 128MiB cap) — maintainers may want to reconcile the two caps (this uses 50MiB matching vision_tools).

[liuhao1024]

Positive verification — clean size-bound guard for inbound media downloads.

_enforce_media_size_limit is a clean centralized helper that both cache_image_from_url and cache_audio_from_url now call. The two-pass check (Content-Length header, then body length) matches the pattern in #42930 for fetch_models.

Observations:

• The size check raises ValueError, which is not in the retryable exception list (httpx.TimeoutException, httpx.HTTPStatusError), so oversized responses correctly fail fast without retrying.
• 50 MiB matches the existing tools/vision_tools._VISION_MAX_DOWNLOAD_BYTES sentinel — good cross-module consistency.
• The tests cover all four combinations: oversized body, oversized Content-Length header, both image and audio paths, and the happy-path within-limit case.

No issues found.

[liuhao1024]

Positive verification — inbound media DoS prevention ✅

Reviewed the full diff. The _enforce_media_size_limit() helper correctly implements a two-phase size check: Content-Length header (fast reject before buffering) then actual body length (catches servers that omit or lie about Content-Length). The 50 MiB cap aligns with the existing _VISION_MAX_DOWNLOAD_BYTES in tools/vision_tools.py.

The ValueError is raised before cache_*_from_bytes() is called, so oversized responses never touch the cache directory. The retry loop treats ValueError as non-retryable (single attempt only), which is correct — retrying a too-large download won't make it smaller.

Both cache_image_from_url and cache_audio_from_url are updated. Test coverage includes: oversized body without Content-Length, oversized Content-Length header, and the happy path (within limit still caches). The monkeypatch.setattr on _MAX_MEDIA_DOWNLOAD_BYTES keeps tests fast without allocating megabytes.

[youngstar-eth]

Thanks for the pointer — yes, this addresses #13145, and I see the overlap with #13341 (configurable 128 MiB cap).

On the cap value: I chose 50 MiB deliberately to match the existing tools/vision_tools._VISION_MAX_DOWNLOAD_BYTES sentinel, so the two untrusted-media download paths share one limit rather than introducing a second, different number. That said, I have no attachment to the exact value or to it being a constant — if the maintainers prefer #13341's configurable 128 MiB, I'm happy to either lift _MAX_MEDIA_DOWNLOAD_BYTES into config here or close this in favor of #13341. The main goal is just that cache_image_from_url / cache_audio_from_url stop buffering unbounded inbound bodies; the knob is secondary.

Whichever direction reconciles best with #13341 works for me.

[egilewski]

Recommendation: request changes

I reviewed this against current GitHub main d1383a6b1450c6c139720b1b01f8b99cc130453f and PR head a6557e6025865d5801a467d5d95959e0b1f02f25.

Validation:

• git rev-list --left-right --count upstream/main...refs/remotes/upstream/pr/42931 => 125 1; git merge-tree --write-tree upstream/main refs/remotes/upstream/pr/42931 wrote tree ba4f27f3f87df31a8be6bd6b97e10c5942947630; git diff --check upstream/main...refs/remotes/upstream/pr/42931 passed.
• GitHub checks were all successful or skipped at final recheck.
• python -B -m pytest -q tests/gateway/test_media_download_retry.py -p no:cacheprovider passed: 40 tests.
• python -B -m py_compile gateway/platforms/base.py tests/gateway/test_media_download_retry.py passed.
• Source inspection confirmed the protected call sites include the inbound URL paths in WhatsApp mediaUrls, Signal, Feishu, and BlueBubbles via cache_image_from_url / cache_audio_from_url.

Finding:
The fix still does not bound memory for responses without a trustworthy Content-Length. Both cache_image_from_url() and cache_audio_from_url() call await client.get(...), then _enforce_media_size_limit() reads response.content. With httpx's normal request path, the full response body has already been buffered before response.content can be length-checked. That means a sender-controlled URL can omit Content-Length (or send a small/invalid one) and still force the gateway to buffer an arbitrarily large body in memory; the PR only prevents the oversized body from being written to the cache afterward.

Please enforce the limit while reading the response, for example by using client.stream(...) / aiter_bytes() and aborting once accumulated bytes exceed the cap, while keeping the early header rejection for honest oversized responses. The current tests pass because they inject a prebuilt resp.content; they do not exercise the no-length streaming case that still causes the memory-spike part of #13145.

Review stopped at this blocker; there may be other issues.

Signed: GPT-5.5-xhigh in Codex

[youngstar-eth]

Thanks @egilewski — you're right, and I've pushed a fix.

The previous version checked len(response.content) after httpx had already buffered the whole body, so a response with no/under-reported Content-Length could still spike memory (the part of #13145 the PR claimed to fix). And as you noted, the tests passed only because they injected a prebuilt resp.content.

Updated approach (gateway/platforms/base.py):

• cache_image_from_url / cache_audio_from_url now read the body with async with client.stream("GET", ...) + response.aiter_bytes() via a shared _read_capped_body() helper that aborts as soon as the accumulated size exceeds the 50 MiB cap — an unbounded body is never fully materialised.
• The Content-Length header pre-check is kept for honest oversized responses (fast reject before reading).
• The SSRF redirect guard (response event hook) and retry/backoff are unchanged.

Tests: added test_image_aborts_stream_when_no_content_length, which streams an unbounded body with no Content-Length and asserts the read is aborted near the cap (and that the buffered .get() path is never used). The existing image/audio/size-limit/redirect tests were moved onto the streaming (client.stream + aiter_bytes) interface. Full file: 41 passed; ruff clean.

Rebased onto current main. Happy to make the cap configurable if you'd prefer to reconcile with #13341.

[egilewski]

Recommendation: approve

I reviewed this against current GitHub main d62979a6f34f64f2ed840f159aac66e24d7cad78, PR base 955fa40062874faed1108f831864d29724a6250c, and PR head aa41ec8d8ab5c4243177b85fe2c867c628a2e7eb.

Validation:

• git merge-tree --write-tree upstream/main upstream/pr/42931: passed, produced 990326652c117cdbb7f659c8f4d140daeed9e09b.
• git diff --check upstream/main...upstream/pr/42931: passed.
• /home/mac/hermes-agent/.venv/bin/python -B -m compileall -q gateway/platforms/base.py tests/gateway/test_media_download_retry.py: passed.
• /home/mac/hermes-agent/.venv/bin/python -B -m pytest -o addopts='' -p no:cacheprovider tests/gateway/test_media_download_retry.py -q: passed, 41 passed.
• Direct capped-stream probe with _MAX_MEDIA_DOWNLOAD_BYTES=64: raised ValueError after consuming 80 bytes, confirming the body is read incrementally and aborts just past the cap.
• coderabbit review --plain --base upstream/main --type committed: completed with one trivial optional negative-Content-Length hardening note; I do not consider it blocking because the stream read still enforces the cap and empty/invalid image bodies are rejected before caching.

Finding:
I did not find a blocker in the reviewed scope. The previous memory-spike issue is addressed: cache_image_from_url() and cache_audio_from_url() now use client.stream(...) and _read_capped_body(), preserving the fast Content-Length rejection while also aborting no-length or under-reported bodies during aiter_bytes() before caching.

Signed: GPT-5.5-xhigh in Codex

[teknium1]

Closing in favor of #50321 — #50321 — which caps inbound media size across all three media types (image/audio/video) in the shared cache helpers.

Your PR fixed the same cache_*_from_url unbounded-body problem with a 50 MiB cap and a shared _enforce_media_size_limit() helper — the same idea. The merged fix is the same class of change, credited to @sgaofen, whose #13341 reported the issue first (Apr 21). Thanks for the contribution and the SSRF-aware framing.