Skip to content

fix(bench): Check backup URLs before the agent downloads them (backport #654) - #655

Merged
ssiyad merged 2 commits into
masterfrom
mergify/bp/master/pr-654
Oct 27, 2026
Merged

ssiyad merged 2 commits into
masterfrom
mergify/bp/master/pr-654

Conversation

@mergify

@mergify mergify Bot commented Oct 26, 2026

Copy link
Copy Markdown
Contributor

The agent downloads the database and files backups for a restore from the URLs that Press sends. This change checks each URL before the download, with the same checks that webhooks and Remote File use in Press.

Changes

  • New agent/remote_url.py with open_remote_url. It accepts only http and https URLs, resolves the host, and connects only when every address is public. The connection uses the address that was checked. The Host header and the TLS certificate check keep the hostname.
  • Redirects are followed by hand, up to 3. Each target goes through the same check.
  • download_file uses open_remote_url in place of requests.get.

Before merge

All restore downloads go through the check, including the S3 links that Press presigns. If a Backup Bucket endpoint_url resolves to an address that is not public, restores from that bucket fail. Please confirm the endpoints in use.

Tests

agent/tests/test_remote_url.py, 9 tests on Python 3.8. Two tests send real requests to a local server, where only the check can resolve the hostname. Over HTTPS with a test CA, a certificate for the hostname is accepted, and a certificate for another name or for the pinned IP is rejected.

python -m unittest agent.tests.test_remote_url

This is an automatic backport of pull request #654 done by [Mergify](https://mergify.com).

The agent downloads the database and files backups for a restore from
the URLs that Press sends. Each request now goes only to a public
address, and the connection uses the address that was checked. The
download follows up to 3 redirects, and each target goes through the
same check. Webhooks and Remote File in Press use the same checks.

(cherry picked from commit d6263ad)
The adapter test replaced the request, so it never ran a TLS
handshake. The new tests serve HTTPS on 127.0.0.1 with a test CA. A
certificate for the hostname is accepted, and the SNI is the hostname.
A certificate for another name, or for the pinned IP, is rejected.

urllib3 sends server_hostname for SNI and checks the certificate
against it, so the adapter no longer sets assert_hostname.

(cherry picked from commit 31c5827)
@mergify mergify Bot assigned ssiyad Oct 26, 2026
@mergify

mergify Bot commented Oct 26, 2026 •

Copy link
Copy Markdown
Contributor Author

This pull request does not currently match the merge queue conditions, so it cannot be queued from here. The box comes back if it matches again.

@ssiyad
ssiyad merged commit 9908706 into master Oct 27, 2026
5 checks passed
@ssiyad
ssiyad deleted the mergify/bp/master/pr-654 branch October 27, 2026 17:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant