fix(presets): harden preset URL installs against unsafe redirects#2911
Open
darion-yaphet wants to merge 4 commits into
Open
fix(presets): harden preset URL installs against unsafe redirects#2911darion-yaphet wants to merge 4 commits into
darion-yaphet wants to merge 4 commits into
Conversation
Preset URL installs already rejected non-HTTPS source URLs, but the authenticated opener follows redirects. Validate the final response URL before writing the ZIP, preserve GitHub release asset URL resolution after the preset command module split, stream the response to disk, and keep catalog config serialization on safe YAML output. Constraint: open_url follows redirects, so source URL validation alone does not constrain the downloaded target Rejected: Keep response.read() for simplicity | large preset downloads should not be buffered entirely in memory Confidence: high Scope-risk: narrow Directive: Keep preset URL policy aligned with workflow installer redirect validation Tested: uvx ruff check src/specify_cli/__init__.py src/specify_cli/presets/__init__.py src/specify_cli/presets/_commands.py tests/test_presets.py Tested: uv run pytest tests/test_presets.py -q Not-tested: Real network redirect integration against a live HTTP server Co-authored-by: OmX <omx@oh-my-codex.dev>
Preset downloads should fail early when a URL lacks a hostname, even if the scheme is HTTPS. The redirect error now describes any disallowed target instead of implying that only non-HTTPS redirects are blocked.
Preset URL downloads already checked the final URL after urllib followed redirects, but that was too late for authenticated requests because same-host redirects could preserve Authorization during the redirect itself. The authenticated HTTP helper now supports an opt-in redirect validator, and preset downloads use it to reject disallowed redirect targets before following them. The redirect auth handlers also stop preserving credentials across HTTPS to non-HTTPS downgrades as defense in depth.
重定向安全加固为 open_url 新增 redirect_validator 参数， 两处 fake_open_url mock 签名未同步导致 TypeError。 补齐参数后全部 3717 个测试通过。
5 tasks
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.
Summary
Hardens
specify preset add --from <url>downloads against redirect-based attacks. Extracted from #2826 so the security fix can be reviewed and merged independently of the refactor.redirect_validatorhook is added toauthentication.http.open_urlso every hop is checked, not just the final response._StripAuthOnRedirectno longer allows a redirect to carry the request to a weaker transport when credentials were attached.ipaddress.ip_address(...).is_loopbackinstead of a hardcoded host list.Relationship to #2826
These commits originally lived on the
refactor/split-init-pr6branch, mixed with the preset command refactor. They are rebased here ontomain(applied to__init__.py, where the preset handlers currently live) so they carry no refactor dependency. #2826 will be rebased to drop them and return to a pure code-move.Test plan
tests/test_presets.py,tests/test_authentication.py)