fix(security): block redirect-based SSRF in Slack image uploads + base.py cache helpers#7151
Merged
Conversation
…y cache helpers Follow-up to Dusk1e's PR #7120 (Slack send_image redirect guard): - Rename _safe_url_for_log -> safe_url_for_log (drop underscore) since it is now imported cross-module by the Slack adapter - Add _ssrf_redirect_guard httpx event hook to cache_image_from_url() and cache_audio_from_url() in base.py — same pattern as vision_tools and the Slack adapter fix - Update url_safety.py docstring to reflect broader coverage - Add regression tests for image/audio redirect blocking + safe passthrough
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
2 participants
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
Salvage of PR #7120 by @Dusk1e, plus follow-up hardening.
From #7120 (Dusk1e): Adds an httpx
event_hooksredirect guard to Slacksend_image()that re-validates each redirect target viais_safe_url(), preventing redirect-based SSRF where a public URL 302s to a private/internal address (e.g.169.254.169.254).Nit fix: Renamed
_safe_url_for_log→safe_url_for_log(dropped underscore) since the PR imports it cross-module into the Slack adapter.Follow-up: Applied the same redirect guard pattern to
cache_image_from_url()andcache_audio_from_url()inbase.py— these had the same pre-flight-onlyis_safe_url()check with unguardedfollow_redirects=True. Updatedurl_safety.pydocstring to reflect broader coverage.Files changed
gateway/platforms/base.py—safe_url_for_logrename,_ssrf_redirect_guardhelper, wired into both cache download functionsgateway/platforms/slack.py— updated import to use public nametests/gateway/test_media_download_retry.py— 3 new SSRF redirect guard tests (image block, audio block, safe passthrough)tests/gateway/test_platform_base.py— updated to use public nametools/url_safety.py— docstring updateTest plan
Closes #7120. Credit to @Dusk1e for the original fix.