fix(#17): Auto-fix (multi-mode) (multi-mode flow)

[dominic097]

Incident Summary

Requests followed a redirect from https://olb.bsf.net to http://olb.bsf.net/login/ despite the reporter stating the site advertised Strict-Transport-Security. Because the site did not listen on port 80, Requests attempted the downgraded HTTP request and timed out with an urllib3.exceptions.MaxRetryError.

Root Cause Mapping

The RCA identified the likely root cause as redirect handling preserving absolute http:// Location targets without an HSTS cache or upgrade step. The exact original redirect decision line was not verified in the shared flow-memory. The implemented fix was reported in src/requests/sessions.py, but exact changed line numbers were not available in the shared context.

Files Reviewed

• src/requests/sessions.py — changed to record HTTPS hosts that return Strict-Transport-Security and upgrade subsequent same-host http:// redirect targets to https://.
• tests/test_requests.py — changed to add TestRequests::test_hsts_redirect_upgrades_http_location.

Fix Verification

QA did not pass. QA judged that the implementation targets the reported HSTS redirect behavior, but deterministic validation was blocked because the runtime lacked a usable Python/pytest setup.

Test Evidence

Automated verification did not complete successfully.

Commands/results:

• pytest tests/test_requests.py::TestRequests::test_hsts_redirect_upgrades_http_location tests/test_requests.py::TestRequests::test_auth_is_stripped_on_http_downgrade tests/test_requests.py::test_requests_are_updated_each_time -q — failed: bash: pytest: command not found.
• python -m pytest tests/test_requests.py::TestRequests::test_hsts_redirect_upgrades_http_location tests/test_requests.py::TestRequests::test_auth_is_stripped_on_http_downgrade tests/test_requests.py::test_requests_are_updated_each_time -q — exit code 127: /bin/sh: 1: python: not found.
• python3 -m pytest tests/test_requests.py::TestRequests::test_hsts_redirect_upgrades_http_location tests/test_requests.py::TestRequests::test_auth_is_stripped_on_http_downgrade tests/test_requests.py::test_requests_are_updated_each_time -q — exit code 1: /usr/bin/python3: No module named pytest.

Atlas status check reported the server available, local tier, with repository dominic097/requests indexed with 1144 symbols.

Regression Analysis

Reviewer findings:

• QA did not validate the implementation because pytest was unavailable.
• The shared context contained truncated changed-file snippets, so review could not independently confirm the exact final implementation or line-level side effects.

Risk remains that the HSTS redirect behavior or nearby redirect/auth behavior could regress until tests run successfully.

Deployment Considerations

No migrations or configuration changes were reported. Rollout should wait for a passing targeted test run in an environment with Python and pytest installed.

Future Prevention Recommendations

• Ensure CI/test environments include Python and pytest before accepting redirect-behavior changes.
• Keep a deterministic regression test for same-host HSTS http:// redirect upgrades.
• Include nearby redirect/auth tests in verification for future changes to src/requests/sessions.py.

Approval Decision

REQUEST_CHANGES — reviewer requested changes because QA failed, automated tests were not executed successfully, and the final implementation could not be independently confirmed from the truncated shared context.

Fixes #17

---

Opened autonomously by Aziron Pulse (FusionX agent).

[dominic097]

🔗 compatibility · 🟠 Major

Hostname-only HSTS cache can incorrectly rewrite redirects on other ports

_update_hsts_hosts() stores only hostname.lower() in the session cache, and _upgrade_hsts_redirect() later upgrades any http:// redirect whose hostname matches that cached value. Because the rewritten URL preserves the original port, a response from https://example.com that advertises HSTS will also cause a later redirect to http://example.com:8080/... to be rewritten to https://example.com:8080/.... The diff shows no port check or origin scoping in either method (src/requests/sessions.py lines 134-164), so this changes behavior for same-host services on different ports and can break redirects to endpoints that do not serve TLS on that port. Please scope the cache/match to the full origin semantics you intend (for example, include the effective port in the cache key or explicitly gate upgrades to default HTTP/HTTPS ports), and add tests proving the chosen compatibility behavior.

P2 · Impact high (4/5) · Confidence high

🤖 Prompt for AI agents

Adjust the HSTS tracking and redirect-upgrade logic so it does not over-apply to same-host redirects on unrelated ports, then add tests covering default-port and explicit cross-port redirect cases.

_{🔒 Secure confirmation — opens an authenticated page; the fix is attributed to you. Or reply @aziron fix finding:hsts-port-scope-overreach}

[dominic097]

🧪 test · 🟡 Moderate

Redirect rewrite tests do not cover the new cross-port behavior

This new test only covers the happy path https://example.com -> http://example.com/... upgrade. The production change introduces stateful host-based rewriting in SessionRedirectMixin (src/requests/sessions.py lines 134-164, 261-262), but there is no regression test for an explicit-port redirect such as http://example.com:8080/..., nor a negative case where no HSTS header is present. Without those cases, the current suite would not catch the compatibility issue where hostname-only matching upgrades redirects on other ports. Please extend this test area with cross-port and no-HSTS cases so the intended redirect contract is pinned down.

P3 · Impact medium (3/5) · Confidence high

🤖 Prompt for AI agents

Add redirect resolution tests for explicit-port URLs and for responses without Strict-Transport-Security, asserting the intended rewrite/no-rewrite behavior.

_{🔒 Secure confirmation — opens an authenticated page; the fix is attributed to you. Or reply @aziron fix finding:missing-cross-port-test}

[dominic097]

🔍 Aziron Pulse Review

❌ Changes requested · Confidence ⭐ 2/5 · 2 file(s) · 2 finding(s)

This PR adds session-level HSTS tracking so redirects from an HTTPS response to an HTTP Location on the same host are upgraded back to HTTPS. The core mechanism is plausible, but it currently over-applies HSTS to all ports on a host and lacks tests covering that compatibility edge, which can change redirect behavior incorrectly for services running on different ports.

Confidence Score: 2/5

The diff is small and the control flow change is clear, so the main behavioral implications are visible from the provided code. Confidence is not high because only bounded excerpts are available and there is no broader project context or upstream issue statement clarifying the intended HSTS scope.

💬 Pre-merge checks · ✅ 1 · ❌ 3

❌ Failed checks

Check	Status	Explanation	Resolution
Title check	⚠️ Warning	The PR title was not provided in the review context, so it could not be validated.	Verify the PR title clearly describes the HSTS redirect behavior change before merging.
Description check	⚠️ Warning	The PR description/body was not included in the provided context, so there is no evidence of rationale, scope, or rollout notes.	Add or verify a PR description explaining the HSTS redirect semantics and compatibility considerations.
Linked issues check	⚠️ Warning	No linked issue or tracking reference was present in the supplied evidence.	Link the motivating issue or bug report that this redirect behavior is intended to fix.

✅ Passed checks

Check	Status	Explanation
Out of scope changes check	✅ Passed	The diff is narrowly scoped to redirect handling and associated tests, with no unrelated file changes evident.

📝 Walkthrough

Engineering Review

What changed: In src/requests/sessions.py, SessionRedirectMixin now caches hosts that have returned a Strict-Transport-Security header via _hsts_hosts() / _update_hsts_hosts(), then rewrites a redirect target from http to https in _upgrade_hsts_redirect() when the redirect originates from an HTTPS response on the same hostname and that hostname is in the HSTS set. resolve_redirects() was updated to record HSTS state before entering the redirect loop and after each received redirect response, and Session.__attrs__ now persists the new private cache attribute. In tests/test_requests.py, a new unit test verifies that an HTTPS 302 response with Strict-Transport-Security and an http://example.com/... Location yields an upgraded https://example.com/... prepared redirect request.

Correctness: The added flow is internally consistent for the covered case: the response URL is parsed, a host is remembered only when the response scheme is HTTPS and the response includes Strict-Transport-Security (src/requests/sessions.py added methods around lines 134-164), and redirect handling now calls _upgrade_hsts_redirect() before normalizing the next URL (src/requests/sessions.py around lines 233, 261-262, 339). The new test in tests/test_requests.py:1933-1948 demonstrates the intended happy path. However, _update_hsts_hosts() stores only hostname.lower() and _upgrade_hsts_redirect() matches only on hostname, not port, before rewriting the URL. Because the replacement preserves the original port while changing scheme to HTTPS, a host that advertises HSTS on one HTTPS origin can cause an unrelated http://same-host:other-port/... redirect to be upgraded to https://same-host:other-port/..., which is a behavior change not justified by the evidence in this patch and may fail against deployments where only one port serves TLS.

Completeness: The implementation addresses the primary redirect-rewrite scenario but leaves important behavioral gaps untested and unresolved. There is only one new test for the basic same-host upgrade path (tests/test_requests.py:1933-1948), with no coverage for redirects to the same hostname on a different explicit port, redirects to a different hostname, responses without HSTS, or persistence/reset semantics of the per-session cache. Given the new stateful behavior added to Session.__attrs__ (src/requests/sessions.py:475), the absence of those tests makes it hard to conclude the feature is complete.

Risks: Main risk is compatibility/regression in redirect handling: same-host HTTP redirects on non-default ports may now be rewritten to HTTPS unexpectedly because the HSTS cache key ignores port (src/requests/sessions.py lines 134-164, 261-262). There is also statefulness risk because HSTS knowledge is now retained on the Session object via __attrs__ (src/requests/sessions.py:475), so behavior can differ across requests in the same session; that may be intended, but without broader tests it increases the chance of hard-to-diagnose redirect differences. Performance risk is low because the added work is just URL parsing and set lookup per redirect.

Missing tests: Missing tests include: (1) a redirect from https://example.com with HSTS to http://example.com:8080/... to verify whether cross-port upgrades should or should not occur; (2) a same-host HTTP Location without Strict-Transport-Security on the source response to ensure no rewrite; (3) a different-host HTTP Location after an HSTS response to confirm no rewrite occurs; and (4) session persistence behavior showing HSTS state affects later redirects only for intended origins. These are all directly motivated by the new host cache and redirect rewrite code in src/requests/sessions.py.

Follow-up work: If the feature is kept, follow up by documenting the redirect/HSTS behavior change in user-facing release notes and by clarifying the intended scope of the cache key (hostname-only vs origin/port-aware). A second follow-up would be to add a focused test matrix around resolve_redirects() covering scheme, host, and port combinations so future redirect logic changes do not regress this area.

Decision — REQUEST_CHANGES: Requesting changes because there is a concrete compatibility/correctness concern in the shipped behavior: the new HSTS cache is keyed only by hostname, but redirect rewriting preserves arbitrary ports, so the session can now rewrite redirects for the same host on different ports without evidence that those ports should be treated as HSTS-covered. The root feature is partly implemented, but significant edge-case coverage is missing, so the change does not yet meet the criteria for safe approval.

📁 Files changed

Important Files Changed

Filename	Overview
src/requests/sessions.py	Adds session-level HSTS host tracking and rewrites same-host HTTP redirect targets to HTTPS during redirect resolution; main risk is that matching is hostname-only and may over-apply across ports.
tests/test_requests.py	Adds one unit test for upgrading an HTTP Location after an HTTPS HSTS response, but does not cover cross-port or negative cases introduced by the new redirect logic.

Sequence Diagram

sequenceDiagram
    participant Caller
    participant Session
    participant RedirectMixin as SessionRedirectMixin
    participant Response

    Caller->>Session: resolve_redirects(resp, req, yield_requests=True)
    Session->>RedirectMixin: _update_hsts_hosts(resp)
    RedirectMixin->>Response: read resp.url + Strict-Transport-Security
    RedirectMixin-->>Session: store hostname in session HSTS set
    Session->>RedirectMixin: get_redirect_target(resp)
    loop while redirect exists
        Session->>RedirectMixin: _upgrade_hsts_redirect(resp, url)
        alt same hostname, original resp https, redirect url http, host in HSTS set
            RedirectMixin-->>Session: return https-upgraded URL
        else conditions not met
            RedirectMixin-->>Session: return original URL
        end
        Session-->>Caller: yield prepared redirect request/response
        Note over Session,RedirectMixin: New responses update the session HSTS cache before the next redirect step
    end

Loading

Actionable comments posted: 2 (0 suggested change(s), 2 fix-session candidate(s))

Fix all findings: comment @aziron fix

📋 Reviewed against: lint (python): pyproject.toml

_{Aziron Pulse — sandbox + MCP-assisted LLM review with native code intelligence.}

[dominic097]

🪄 Autofix — generate a fix for all Aziron Pulse findings on this PR:

• Create a new PR with the fixes

_{Tick the box above (or comment @aziron fix) to start a fix. Aziron opens the fix as a new PR back to this branch.}