fix(security): guard provider image URL downloads

[zapabob]

Summary

• guard provider-returned image URLs with the repo's URL safety policy before any host-side fetch
• follow redirects manually with allow_redirects=False, re-checking each Location before requesting it
• fail closed on missing Location or redirect budget exhaustion so redirect bodies are not cached as images
• keep the existing real HTTP save-url tests, with additional direct-private, redirect-private, safe-redirect, and too-many-redirect coverage

Fixes #44728.
Supersedes the remaining redirect-safety review gap on #44743.

Validation

• uv run pytest -q tests\agent\test_save_url_image.py -o addopts= -> 12 passed
• uv run ruff check agent\image_gen_provider.py tests\agent\test_save_url_image.py -> passed
• uv run python -m py_compile agent\image_gen_provider.py tests\agent\test_save_url_image.py -> passed
• git diff --check -> passed

[AIalliAI]

LGTM — this patches the #44728 SSRF gap on provider image-URL downloads (save_url_image), which previously issued an unguarded requests.get on a provider-returned URL. The fix validates with is_safe_url() before the initial fetch and re-validates each redirect target with allow_redirects=False + a bounded redirect budget, discarding redirect bodies — so it closes the redirect-to-internal vector, not just the direct one. Coverage looks complete for private/loopback/link-local/cloud-metadata/CGNAT and IPv6 (incl. ::ffff: mapped). It's IP-based rather than a domain allowlist, so public CDNs aren't over-blocked.

One honest caveat worth a comment in the code: this is still vulnerable to DNS rebinding (TOCTOU between the pre-flight resolve and the socket connect) — that's a known limitation of pre-flight is_safe_url() checks generally and acceptable for this provider trust boundary, but noting it so it's not mistaken for full coverage. Tests (safe-redirect-followed, direct-private-blocked, redirect-to-private-blocked, budget-exhaustion) are solid.

[tonydwb]

Code Review Summary

Verdict: Approved

Security hardening: guards save_url_image() against SSRF via redirect following. Implements redirect loop detection (max 10 hops) and validates each URL via is_safe_url() before each request. Uses allow_redirects=False to intercept redirects manually.

Security: Solid — SSRF defense-in-depth. is_safe_url from tools.url_safety is the right check. Redirect limit prevents infinite loop attacks.

Code Quality: Clean implementation. Minor note: the if response is None check after the loop is belt-and-suspenders (the loop always sets response before break), but harmless.

---

Reviewed by Hermes Agent (cron batch)

[teknium1]

Thanks for addressing the provider image-download SSRF path. The premise remains valid on current main: agent/image_gen_provider.py:293 sends the provider-returned URL directly to requests.get().

Problems

• agent/image_gen_provider.py:249 does not actually handle a malformed redirect without Location. In Requests, Response.is_redirect requires both a redirect status and a Location header. A 302 without that header therefore takes the break, while raise_for_status() accepts 3xx responses, and the body is subsequently written at line 282. The intended failure path at lines 251-254 is unreachable for this case. The added tests at tests/agent/test_save_url_image.py:68-88 cover only redirects with Location.

Suggested changes

• Detect redirect statuses separately from response.is_redirect; close and reject any redirect-status response lacking Location, then add a real-server regression test proving no cache file is created.

Automated hermes-sweeper review.

[teknium1]

Response.is_redirect is false for a 302 without Location, so this branch breaks before the missing-Location check. Requests' raise_for_status() accepts 3xx responses, letting that response body reach the cache writer. Detect redirect status independently, then close and reject a redirect response that lacks Location; add a regression test.