Skip to content

Add SSRF-safe aiohttp downloader with size limits and use it in Matrix/Mattermost adapters#691

Open
badMade wants to merge 7 commits into
mainfrom
badmade/fix-ssrf-vulnerability-in-image-uploads-pqznw2
Open

Add SSRF-safe aiohttp downloader with size limits and use it in Matrix/Mattermost adapters#691
badMade wants to merge 7 commits into
mainfrom
badmade/fix-ssrf-vulnerability-in-image-uploads-pqznw2

Conversation

@badMade
Copy link
Copy Markdown
Owner

@badMade badMade commented Jun 2, 2026

Motivation

  • Introduce a single SSRF-safe downloader with redirect validation and a hard size cap to prevent unsafe redirects and large downloads when fetching remote media.
  • Replace ad-hoc download logic in platform adapters with the shared downloader to unify behavior and error handling.

Description

  • Add download_public_url_bytes_aiohttp to gateway/platforms/base.py with redirect-based SSRF protection, configurable max_bytes, redirect handling, streaming-aware size enforcement, and a PublicUrlDownloadHTTPError type; introduce MAX_GATEWAY_MEDIA_DOWNLOAD_BYTES constant and import urljoin.
  • Update the Matrix adapter to use download_public_url_bytes_aiohttp for send_image, including proxy kwargs and using the final resolved URL for filenames and sensible content-type fallback.
  • Update the Mattermost adapter to use download_public_url_bytes_aiohttp in _send_url_as_file and image batch download paths, add handling for PublicUrlDownloadHTTPError to implement retry behavior on 429/5xx, normalize content-type fallback to image/png, and derive filenames from the final URL.
  • Small formatting fixes and reorder imports to accommodate the new helper.

Testing

  • Ran the new unit tests in tests/gateway/test_platform_base.py covering download_public_url_bytes_aiohttp behavior (unsafe redirect blocking and streaming size limit); the tests passed when run with pytest tests/gateway/test_platform_base.py -q.

Codex Task

Copilot AI review requested due to automatic review settings June 2, 2026 03:23
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

🔎 Lint report: badmade/fix-ssrf-vulnerability-in-image-uploads-pqznw2 vs origin/main

ruff

Total: 0 on HEAD, 0 on base (➖ 0)

🆕 New issues: none

✅ Fixed issues: none

Unchanged: 0 pre-existing issues carried over.

ty (type checker)

Total: 8252 on HEAD, 8253 on base (✅ -1)

🆕 New issues: none

✅ Fixed issues (1):

Rule Count
unresolved-import 1
First entries
gateway/platforms/matrix.py:1089: [unresolved-import] unresolved-import: Cannot resolve imported module `httpx`

Unchanged: 4356 pre-existing issues carried over.

Diagnostics are surfaced as warnings — this check never fails the build.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request introduces a centralized, SSRF-safe utility function download_public_url_bytes_aiohttp in gateway/platforms/base.py to handle public URL downloads with size limits and redirect protection. It refactors the Matrix and Mattermost platform adapters to use this new utility. A review comment suggests catching ValueError explicitly in Mattermost's retry loop to avoid retrying permanent validation failures like SSRF blocks or size limit violations.

Important

The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.

Comment thread gateway/platforms/mattermost.py Outdated
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a shared, SSRF-aware media downloader to the gateway platform base layer and migrates Matrix/Mattermost adapters to use it, aiming to centralize redirect validation, enforce hard download size limits, and unify error handling for public URL media fetches.

Changes:

  • Introduces download_public_url_bytes_aiohttp (with redirect SSRF protection + max-bytes enforcement) and PublicUrlDownloadHTTPError in gateway/platforms/base.py.
  • Updates Matrix send_image and Mattermost URL/image download paths to use the shared downloader and derive filenames from the resolved final URL.
  • Adds unit tests for unsafe redirect blocking and streaming size limit enforcement.

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 3 comments.

File Description
gateway/platforms/base.py Adds SSRF-safe aiohttp downloader with redirect handling and byte caps.
gateway/platforms/matrix.py Switches image download to shared aiohttp downloader (proxy-aware), uses final URL for filename.
gateway/platforms/mattermost.py Replaces ad-hoc downloads with shared downloader; adds HTTP-status-based retry behavior.
tests/gateway/test_platform_base.py Adds tests covering unsafe redirects and streaming byte-limit enforcement for the new downloader.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread gateway/platforms/base.py Outdated
Comment thread tests/gateway/test_platform_base.py
Comment thread gateway/platforms/mattermost.py Outdated
badMade and others added 5 commits June 1, 2026 23:35
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented Jun 2, 2026

Auto-merge: checks failing

The following checks did not pass:

  • test (failure)

Please fix the failing checks before this PR can be merged.

View workflow run

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants