Skip to content

fix(yuanbao): block redirect-based SSRF in download_url - #748

Open
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56927
Open

fix(yuanbao): block redirect-based SSRF in download_url#748
hashbender wants to merge 1 commit into
mainfrom
mirror/pr-56927

Conversation

@hashbender

Copy link
Copy Markdown
Owner

What does this PR do?

gateway/platforms/yuanbao_media.py::download_url fetches model-supplied and inbound URLs server-side. It has an is_safe_url() pre-flight, but its per-hop redirect guard was inert:

async def _redirect_guard(response: httpx.Response) -> None:
    if response.is_redirect and response.next_request:   # <- next_request is None here
        ...

Inside an httpx.AsyncClient response event hook, response.next_request is None even for a genuine 302 (it is populated later by the redirect-following machinery). So the guard condition was always falsy and never validated the redirect target. A public URL that passes the pre-flight check can therefore 302-redirect to http://169.254.169.254/ (cloud metadata / IMDS) or any private address, and httpx (follow_redirects=True) follows it — a redirect-based SSRF.

This is the same class that NousResearch#56160 (merged) fixed across the other httpx download hooks by resolving the hop from the Location header via redirect_target_from_response; yuanbao_media.py was the one path that migration missed. (The broken next_request guard here was introduced by NousResearch#54470.)

Related Issue

No open issue — found while auditing the httpx redirect guards after NousResearch#56160. N/A.

Type of Change

  • 🔒 Security fix

Changes Made

  • gateway/platforms/yuanbao_media.py: _redirect_guard now resolves the redirect target with redirect_target_from_response(response) (reads the Location header, urljoin-resolving relative Locations) instead of the always-None response.next_request, then rejects it with is_safe_url. This mirrors gateway/platforms/base.py::_ssrf_redirect_guard.
  • tests/gateway/test_yuanbao_media_ssrf.py: added test_redirect_to_metadata_blocked — drives a 302 whose Location points at 169.254.169.254 with next_request=None (the real in-hook shape) through the registered guard and asserts ValueError. Fails on the old guard, passes on the fix.

How to Test

$ python -m pytest tests/gateway/test_yuanbao_media_ssrf.py -q
6 passed

The new test reproduces the bug: with the previous next_request-based guard, a 302 to the metadata endpoint (with next_request=None, as httpx leaves it inside the hook) is not blocked; with the fix it raises ValueError: Blocked redirect to private/internal address.

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits (fix(yuanbao): ...)
  • I searched existing PRs/issues to make sure this isn't a duplicate (no open PR/issue for the yuanbao redirect guard; fix(security): close SSRF redirect-guard bypass across all httpx download hooks NousResearch/hermes-agent#56160 fixed the sibling hooks and merged)
  • My PR contains only changes related to this fix
  • I've run the relevant tests (pytest tests/gateway/test_yuanbao_media_ssrf.py -q → 6 passed); the full suite runs in CI
  • I've added a regression test for this fix
  • I've tested on my platform: Windows 10

Documentation & Housekeeping

  • Documentation — N/A
  • cli-config.yaml.example — N/A
  • CONTRIBUTING.md / AGENTS.md — N/A
  • Cross-platform impact — N/A (server-side URL fetch; platform-independent)
  • Tool descriptions/schemas — N/A

Screenshots / Logs

See How to Test above.

🤖 Generated with Claude Code


Mirror-of: NousResearch#56927
NousResearch#56927

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant