fix(security): close SSRF redirect-guard bypass across all httpx download hooks - #56160
Merged
Conversation
…load hooks Inside httpx AsyncClient response event hooks, response.next_request is often None even for a genuine redirect, so guards keyed on `if response.is_redirect and response.next_request` silently never fire. A public URL that 302s to http://169.254.169.254/ was followed anyway, defeating the pre-flight is_safe_url() check. Resolve the redirect target from the Location header (via urljoin, so relative Locations work too), falling back to next_request only when no Location is present. Extracted as tools.url_safety.redirect_target_from_response and wired into every SSRF redirect guard: - gateway/platforms/base.py (shared image + audio download for all platforms) - tools/vision_tools.py (two download hooks) - plugins/platforms/slack/adapter.py Original fix by @zapabob (PR #35940), which targeted the since-refactored gateway/platforms/slack.py; reconstructed onto the current shared sites and widened to the whole bug class.
Collaborator
Reconstruction/widening of #35940 by @zapabob (original targeted the since-refactored |
13 tasks
13 tasks
2 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Closes an SSRF redirect-guard bypass that affected every gateway/tool httpx download path, not just Slack.
Inside
httpx.AsyncClientresponse event hooks,response.next_requestis oftenNoneeven for a genuine redirect (it's populated later by the redirect-following machinery). Guards written asif response.is_redirect and response.next_request:therefore never fire — a public URL that302s tohttp://169.254.169.254/gets followed anyway, defeating the pre-flightis_safe_url()check.The fix resolves the redirect target from the
Locationheader (viaurljoin, so relative Locations work too), falling back tonext_requestonly when noLocationis present.Changes
tools/url_safety.py: new shared helperredirect_target_from_response().gateway/platforms/base.py:_ssrf_redirect_guard— shared image and audio download for all platforms.tools/vision_tools.py: both download hooks.plugins/platforms/slack/adapter.py: Slack adapter guard.tests/tools/test_url_safety.py: regression tests for the helper (absolute/relative Location, non-redirect, next_request fallback, empty).Root cause / why the original PR moved
@zapabob's PR #35940 fixed the right bug but targeted
gateway/platforms/slack.py, whose per-URL download loop has since been refactored into the sharedbase.pyhelpers. Reconstructed onto the current sites and widened to the whole bug class (4 guard sites). The vulnerable pattern is now gone from all of them.Note:
tools/mcp_tool.py's cross-origin auth-header-strip hook has the samenext_request-None blind spot but a different mechanism (it mutates the outgoing request's headers, which the Location value can't do). Left out of scope — different fix shape.Validation
Live-reproduced with
httpx.MockTransport(httpx 0.28.1):302 → 169.254.169.254redirect_target_from_responseresolves absolute + relativeLocationwithnext_request=None; returnsNonefor non-redirects.base.pyguard and vision helper path both raise before the private request is issued (verified through a realAsyncClientwith redirect-following on).tests/tools/test_url_safety.py: 127 passing.tests/gateway/test_slack.py+test_send_multiple_images.py: green.Original fix by @zapabob (#35940); authorship preserved.
Infographic