fix(gateway): bypass SSRF check for Discord SDK-supplied attachment URLs - #6512
fix(gateway): bypass SSRF check for Discord SDK-supplied attachment URLs#6512rivercrab26 wants to merge 1 commit into
Conversation
Discord attachment uploads silently break for users behind DNS-rewriting proxies (Clash/Mihomo fake-ip mode). The proxy resolves cdn.discordapp.com to a fake 198.18.x.x address from the IETF benchmark range (RFC 6890), so the SSRF guard added in NousResearch#5944 rejects every attachment as "unsafe". Symptom in the gateway log: [Discord] Failed to cache image attachment: Blocked unsafe URL (SSRF protection): https://cdn.discordapp.com/... Root cause: cache_image_from_url / cache_audio_from_url validate the resolved IP, but Discord attachment URLs come straight from discord.py, which has already authenticated with the platform. The IP-based check is both unreliable (DNS rewriting) and unnecessary (URL is already trusted). Fix: add a `trusted_source` parameter that opts out of the SSRF check. Discord's inbound attachment handler passes trusted_source=True. The default remains False so agent-supplied URLs (e.g. send_image with a user-provided link) keep the safety check. Tests: - 4 new tests in TestTrustedSourceBypass covering both helpers - Add an autouse _mock_safe_url fixture to TestCacheImageFromUrl / TestCacheAudioFromUrl so the existing retry tests do not depend on the developer local DNS resolver (which previously failed under fake-ip) Verified: 37/37 media-cache tests pass, no other regressions in tests/gateway/.
|
Same issue here — voice messages and images both fail to process. The SSRF check blocks cdn.discordapp.com because it resolves to a 198.18.x.x address in my environment. Hope this gets merged soon! |
|
Thanks for the careful analysis and the reproduction detail — the This is an automated hermes-sweeper review. The root cause you identified (fake-ip DNS resolving
The |
Summary
Fixes #6511.
Discord image/audio attachments fail to cache under DNS-rewriting proxies (Clash/Mihomo fake-ip mode) because the SSRF guard added in #5944 rejects the resolved fake IP (
198.18.x.x) as private. The agent then cannot see images uploaded by users on Discord. See the linked issue for the full root-cause analysis.Approach
Add an opt-in
trusted_source: bool = Falseparameter tocache_image_from_urlandcache_audio_from_urlingateway/platforms/base.py. WhenTrue, the IP-based SSRF check is skipped.The Discord adapter passes
trusted_source=Truewhen downloading inbounddiscord.AttachmentURLs, because:discord.pyand is already authenticated by Discord — it is not user/agent controllable in the way a free-form URL is.The default remains
False, so all other call sites — including agent-suppliedsend_imageURLs — keep the existing safety check unchanged.Files
gateway/platforms/base.pytrusted_sourceparameter to both helpersgateway/platforms/discord.pytrusted_source=Truefor inbound attachment downloads (image + audio)tests/gateway/test_media_download_retry.pyTestTrustedSourceBypassclass (4 tests) + autouse_mock_safe_urlfixture so existing retry tests do not depend on the developer's local DNSTests
Coverage:
test_image_blocked_by_default_when_unsafe— default behavior unchangedtest_image_trusted_source_bypasses_ssrf— opt-in skips the checktest_audio_blocked_by_default_when_unsafe— same for audio helpertest_audio_trusted_source_bypasses_ssrf— same for audio helperis_safe_urlis not called whentrusted_source=True, so future regressions are caught.Why the existing tests needed a fixture
While debugging I noticed the existing
TestCacheImageFromUrl/TestCacheAudioFromUrltests fail on any developer machine that uses fake-ip DNS — they call out to a realis_safe_url("http://example.com/..."), which under fake-ip resolves to a private 198.18.x.x address and trips the very guard the tests are not meant to exercise. I added a small autouse fixture that pinsis_safe_urltoTruefor these classes. This is a pure test isolation fix and is independent of the production change.Out of scope
cache_*_from_urlentirely and download viaaiohttpdirectly, so they were not affected by fix(security): consolidated security hardening — SSRF, timing attack, tar traversal, credential leakage #5944 and don't need to change here. If desired I can fold them into the same pattern in a follow-up PR.Risk
Minimal. The
trusted_sourceflag defaults toFalse, so all existing call sites behave identically. Only the Discord inbound-attachment path is opted in, and for that path the URL is already authenticated by the Discord API.