fix(gateway): validate Slack image downloads before caching#6971
Closed
Tranquil-Flow wants to merge 1 commit into
Closed
fix(gateway): validate Slack image downloads before caching#6971Tranquil-Flow wants to merge 1 commit into
Tranquil-Flow wants to merge 1 commit into
Conversation
Slack may return an HTML sign-in/redirect page (HTTP 200) instead of actual image bytes when the bot token lacks file access or the download URL has expired. Previously these HTML responses were cached with a .png/.jpg extension, causing confusing downstream failures in vision_analyze. Two layers of defense: - Slack adapter: reject text/html Content-Type before caching - cache_image_from_bytes: validate magic bytes so no caller can accidentally cache non-image data regardless of upstream platform Also add defensive ValueError handling in WeCom and Email adapters so the new validation does not propagate as an unrelated error. Closes NousResearch#6829
Contributor
|
Merged via #7125 which cherry-picks your cross-platform approach onto current main. Your centralized magic-byte validation in base.py was the winning architecture — it protects all adapters, not just Slack. Thanks for the thorough fix, @Tranquil-Flow! |
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
vision_analyzefailurescache_image_from_bytesValueErrorhandling in WeCom and Email adapters so the new validation doesn't propagate as unrelated errorsRoot cause
When the bot token lacks file access or the download URL expires, Slack returns HTTP 200 with an HTML sign-in page.
_download_slack_filecached these bytes as.png/.jpgfiles. Later,vision_analyzerejected them with a confusing "Only real image files are supported" error.Changes
gateway/platforms/slack.py: Rejecttext/htmlContent-Type before callingcache_image_from_bytesgateway/platforms/base.py: Add_looks_like_image()magic-byte check (JPEG/PNG/GIF/BMP/WebP) incache_image_from_bytes— raisesValueErroron non-image datagateway/platforms/wecom.py: CatchValueErrorfromcache_image_from_bytesto prevent false WebSocket reconnectsgateway/platforms/email.py: CatchValueErrorfromcache_image_from_bytesto prevent IMAP batch failurestests/gateway/test_media_download_retry.py: 5 newTestCacheImageFromBytestests + 1 Slack HTML rejection test; existing test data updated to use valid image magic bytesTest plan
pytest tests/gateway/test_media_download_retry.py— 30 tests passpytest tests/gateway/test_wecom.py— 15 tests passpytest tests/gateway/test_email.py— 69 tests pass (+ all other callers verified safe viaexcept Exception)Closes #6829