harden: bound HTTP reads and enforce strict redirects#3140
Open
PascalThuet wants to merge 2 commits into
Open
Conversation
Add a shared _download_security module (read_response_limited, is_https_or_localhost_http, size constants) and route the GitHub release and Azure DevOps token network reads through bounded reads so an oversized response can't exhaust memory. Add a strict_redirects mode to authentication.open_url: the redirect handler now rejects any redirect whose target isn't HTTPS (or HTTP to localhost), composing with the existing per-hop redirect_validator and auth-stripping. The Azure DevOps token POST is routed through that handler so a 307/308 cannot forward the client_secret body to a non-HTTPS host.
Assisted-by: Codex (model: GPT-5, autonomous)
This was referenced Jun 23, 2026
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
1 participant
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.
Part of splitting #2442 into smaller, dedicated PRs. Re-derived against current
main.What
src/specify_cli/_download_security.py:read_response_limited(bounded, chunk-loop read) +is_https_or_localhost_http(shared scheme predicate) + size constants.authentication/http.py:open_url(strict_redirects=...)plus an always-on_validate_strict_redirectinside the redirect handler — rejects redirects whose target isn't HTTPS (except HTTP→localhost), composing with the existingredirect_validatorand auth-stripping rather than replacing them._version.py/_github_http.py: bounded JSON reads (MAX_JSON_METADATA_BYTES) +strict_redirects=Trueon the release-tag fetch.authentication/azure_devops.py: route the OAuth token POST through the redirect handler (empty host list → scheme check only), so a 307/308 cannot forward theclient_secretrequest body to a non-HTTPS host; bound the token read.HTTPResponse.read(size)cursor semantics, so the broader extension/preset/workflow suites cover the bounded-read path.Why
Unbounded
response.read()on remote JSON is a memory-DoS surface; a redirect to plain HTTP can downgrade transport or leak credentials. Both are closed without changing the happy path.Validation
tests/test_download_security.py,test_github_http.py,test_authentication.py,test_upgrade.py,test_self_upgrade_*— 271 passed, 1 skipped. (The 19 warnings are pre-existing auth-fixture file-permission notes.)tests/test_extensions.py tests/test_presets.py tests/test_workflows.py— 927 passed.uvx ruff check src tests— clean.git diff --check— clean.HOME=/private/tmp/hermes-home .venv/bin/python -m pytest tests/integrations/test_integration_hermes.py -q— 34 passed. The full localpytest tests -qrun only fails when Hermes writes to/Users/pascalthuet/.hermes, which is outside this sandbox's writable roots.Foundation note: this PR adds only the read/URL core of
_download_security. The ZIP-extraction + checksum helpers (and the tree-wide unbounded-read gate) land in the follow-up extension/preset and catalog PRs, which extend this module. Independent of PR2/PR3.Disclosure: Updated on behalf of @PascalThuet by Codex (model: GPT-5, autonomous).