fix(security): block SSRF in yuanbao download_url

[zapabob]

Summary

gateway/platforms/yuanbao_media.py::download_url() fetched arbitrary URLs
with no SSRF protection. The URL is attacker-influenceable: it originates
from model/agent output (e.g. an ![alt](url) image link in a reply that the
Yuanbao gateway resolves and downloads to re-upload). A crafted URL could make
the gateway issue requests to:

• the cloud metadata endpoint (http://169.254.169.254/…) to steal IAM creds,
• loopback/admin services (http://127.0.0.1:…),
• private RFC1918 ranges (http://10.0.0.0/8, etc.).

Because follow_redirects=True was set with no per-hop check, even a public
URL could 30x-redirect to an internal target and bypass any naive front check.

Fix

• Reject private/internal/metadata targets up front via the existing
  tools.url_safety.is_safe_url helper (the same one used elsewhere in the
  gateway, e.g. gateway/platforms/base.py).
• Re-validate every redirect hop with an httpx response event hook
  (_ssrf_redirect_guard) so a redirect to an internal address is blocked
  mid-flight.
• Truncate URLs in error messages to avoid leaking long query strings.

No behavior change for legitimate public media URLs.

Test plan

• tests/gateway/platforms/test_yuanbao_media_ssrf.py — new regression tests:
  • private/internal/metadata targets are rejected (169.254.169.254,
    127.0.0.1, localhost, ::1, 10.0.0.5)
  • redirect guard blocks an unsafe redirect, allows a safe one, and is a
    no-op on non-redirect responses
• All 8 tests pass locally.

[liuhao1024]

Verification review — reviewed the diff for SSRF protection quality.

The implementation is solid:

1. Pre-flight is_safe_url() check blocks known-bad targets (private IPs, localhost, metadata endpoint) before any HTTP request is made.
2. The _ssrf_redirect_guard event hook re-validates each redirect hop — this correctly addresses the redirect-based SSRF bypass where a public URL 30x-redirects to 169.254.169.254.
3. The hook raises ValueError from inside httpx's redirect chain, which correctly propagates to the caller.
4. Test coverage is appropriate: private IP parametrization, redirect guard (safe/unsafe/noop), and monkeypatched is_safe_url for deterministic testing.

One note: the guard only applies to the download_url codepath (yuanbao media). If other modules have similar model-output-originated HTTP fetches, they would need the same protection. This PR is correct for its scope.

[alt-glitch]

Duplicate of #22340 — both add an is_safe_url pre-flight check plus an _ssrf_redirect_guard httpx response hook to yuanbao_media.py::download_url(). Same approach, #22340 is the earlier/canonical open PR. (Note: closed #34591 was a prior attempt.)

[tonydwb]

Code Review Summary

Verdict: Approved

Security: Block SSRF in yuanbao download_url

• Problem: download_url had no SSRF protection. The URL originates from model/agent output (e.g. ![alt](url) image link in a reply), so it is attacker-influenceable. A malicious URL could redirect to 169.254.169.254 (cloud metadata) or 127.0.0.1.
• Fix: Two-layer defense:
  1. is_safe_url() check before making the request (blocks private/internal targets outright).
  2. _ssrf_redirect_guard hook re-validates every redirect hop — prevents bypass of step 1 via a public URL that 30x-redirects to a private target.
• Logging safety: _safe_url_for_log() truncates long URLs to avoid leaking sensitive query strings in logs.
• Tests: 5 cases blocking private targets, redirect guard blocks unsafe redirect, allows safe redirect, no-op on non-redirect.

---

Reviewed by Hermes Agent

[teknium1]

Thanks for the focused Yuanbao SSRF hardening. The pre-flight URL check is already on current main (gateway/platforms/yuanbao_media.py:220-226), but the redirect portion cannot be salvaged unchanged.

Problems

• The proposed guard at gateway/platforms/yuanbao_media.py:48 relies on response.next_request. tools/url_safety.py:479-503 documents that this is frequently None inside HTTPX response hooks, so an unsafe Location redirect can still be followed.
• The synthetic redirect tests always populate next_request (tests/gateway/platforms/test_yuanbao_media_ssrf.py:29-35), and therefore miss that runtime case.

Suggested changes

• Reuse gateway.platforms.base._ssrf_redirect_guard (gateway/platforms/base.py:541-554), which calls redirect_target_from_response() and validates Location targets.
• Add an HTTPX MockTransport regression for a public URL redirecting via Location to a private address, asserting the private request is not issued.

This is an automated hermes-sweeper review.

[teknium1]

response.next_request is frequently None inside HTTPX response event hooks, so this condition can skip a real Location redirect entirely. Use the shared redirect_target_from_response() path (via gateway.platforms.base._ssrf_redirect_guard) so relative and absolute Location targets are checked before HTTPX follows them.

[GottZ]

This was generated by AI during triage.

Summary

Four PRs address or reference the Yuanbao SSRF chain: #43938 and merged #54470 add the initial URL check plus a redirect hook, merged #56160 fixes the shared response.next_request redirect-guard failure across other download paths, and #56927 applies that proven fix to the Yuanbao path missed by #56160.

Related pull requests

• #43938 related — (+109/-1) — close as superseded duplicate: Like #54470, this adds a Yuanbao pre-flight check and a redirect hook, but its hook relies on response.next_request and its synthetic tests always populate that field, missing the actual HTTPX hook behavior. Despite the keep_open review on #43938, the diff retains the documented bypass and current main already contains the pre-flight protection from #54470; #56927 supplies the correct Location-based follow-up.
• #54470 [merged] duplicate — (+113/-1) — merged foundation: This remains relevant as the merged implementation that introduced Yuanbao pre-flight SSRF validation, but its response.next_request-based redirect hook is ineffective inside HTTPX response hooks and is corrected by #56927.
• #56160 [merged] related — (+111/-26) — merged reference implementation: This fixes the same redirect-hook root cause across the shared gateway, Slack, and vision download paths by introducing redirect_target_from_response(), but its diff does not update yuanbao_media.py; #56927 completes that missed site.
• #56927 related — (+80/-7) — merge: It replaces Yuanbao's dead response.next_request condition with the merged #56160 Location-aware helper, preserves the newer DNS-pinned SSRF-safe client, and adds a regression for a metadata redirect with next_request=None. This matches the keep_open review on #56927; after DNS pinning it restores layered application-level rejection rather than closing a currently exploitable connect-time bypass.

Duplicates

#43938 and #54470 substantially implement the same initial Yuanbao pre-flight check and response.next_request redirect hook; #54470 is already merged. #56927 is not a duplicate of #56160 but the Yuanbao completion of its root-cause fix.

Suggested consolidation

Merge #56927 because it applies the established #56160 redirect-target helper to the remaining Yuanbao hook while retaining DNS pinning. Close #43938 as superseded by merged #54470 plus #56927; keep merged #54470 and #56160 as the foundation and reference implementation, respectively.

Complex graph

flowchart LR
    classDef open fill:#dbeafe,stroke:#1d4ed8,color:#1e3a8a
    classDef merged fill:#dcfce7,stroke:#15803d,color:#14532d
    classDef closed fill:#e5e7eb,stroke:#6b7280,color:#1f2937
    classDef unverified fill:#f3f4f6,stroke:#9ca3af,color:#374151
    classDef best stroke-width:3px,stroke:#b45309
    classDef target stroke-width:3px,stroke:#4338ca
    subgraph Dup43938 ["PRs duplicating each other"]
        P43938["PR #43938 (open)"]
        P54470["PR #54470 (merged)"]
    end
    class P43938 open
    class P54470 merged
    class P43938 target
    click P43938 "https://github.com/NousResearch/hermes-agent/pull/43938"
    click P54470 "https://github.com/NousResearch/hermes-agent/pull/54470"

Loading

Graph: solid arrow = fixes / best fix, dashed arrow = partial or unverified (see edge label); boxed group = PRs duplicating each other; amber border = best fix; indigo border = target; gray node = closed or no verify verdict yet (state tag in the node label).

Cross-PR triage: Reviewed 4 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 23 kB of PR diffs, 10 kB of issue/PR text, 7 kB of discussion (9 comments), 2 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.

[egilewski]

@GottZ isn't this a duplicate of #22340, same as #43938?

EDIT: Never mind, wrong tab, this is 43938.

EDIT2: Actually I can't comment on #56927, so I'll have to ask here after all: isn't #56927 a duplicate of #22340, same as this PR?