Skip to content

Conversation

@jtfmumm
Copy link
Contributor

@jtfmumm jtfmumm commented Jun 18, 2025

This PR is a combination of #12920 and #13754. Prior to these changes, following a redirect when searching indexes would bypass our authentication middleware. This PR updates uv to support propagating credentials through our middleware on same-origin redirects and to support netrc credentials for both same- and cross-origin redirects. It does not handle the case described in #11097 where the redirect location itself includes credentials (e.g., https://user:[email protected]). That will be addressed in follow-up work.

This includes unit tests for the new redirect logic and integration tests for credential propagation. The automated external registries test is also passing for AWS CodeArtifact, Azure Artifacts, GCP Artifact Registry, JFrog Artifactory, GitLab, Cloudsmith, and Gemfury.

@jtfmumm jtfmumm added the bug Something isn't working label Jun 18, 2025
@jtfmumm jtfmumm temporarily deployed to uv-test-registries June 18, 2025 12:17 — with GitHub Actions Inactive
@jtfmumm jtfmumm temporarily deployed to uv-test-publish June 18, 2025 12:17 — with GitHub Actions Inactive
@jtfmumm jtfmumm force-pushed the feature/redirect-authentication branch from ad789d9 to a24d727 Compare June 18, 2025 12:26
@jtfmumm jtfmumm temporarily deployed to uv-test-registries June 18, 2025 12:29 — with GitHub Actions Inactive
@jtfmumm jtfmumm temporarily deployed to uv-test-publish June 18, 2025 12:29 — with GitHub Actions Inactive
@jtfmumm jtfmumm force-pushed the feature/redirect-authentication branch from a24d727 to 9be29f6 Compare June 18, 2025 14:39
@jtfmumm jtfmumm temporarily deployed to uv-test-registries June 18, 2025 14:41 — with GitHub Actions Inactive
@jtfmumm jtfmumm temporarily deployed to uv-test-publish June 18, 2025 14:42 — with GitHub Actions Inactive
jtfmumm added 2 commits June 19, 2025 13:43
This is mostly restoring #13215. It also includes a one-line fix for
#13208 (which resulted from that PR). In particular, Azure was returning
303s which were not being correctly handled.

I have also opened another PR (#13754) that refactors and improves the
redirect handling here. It also supersedes the fix here. There are some
tests failing here but they all pass there.

This PR depends on #13615, which adds a script for testing against
registries. The test fails for Azure when running against the restored
#13215 alone and passes with the fix. It also passes for AWS
CodeArtifact, GCP Artifact Registry, JFrog Artifactory, GitLab, and
Gemfury in both cases. I also plan to test against Cloudsmith and Nexus.
…ble (#13754)

This PR factors out and updates the redirect handling logic from #13595.
It handles a few new cases:
* If a 303 redirect is received for any method other than GET or HEAD,
converts it to a GET. Unlike `reqwest`, it does not do this conversion
for 301s or 302s (which is not required by RFC 7231 and was not the
original intention of the spec).
* If the original request did not have a Referer header, does not
include a Referer header in the redirect request.
* If the redirect is a cross-origin request, removes sensitive headers
to avoid leaking credentials to untrusted domains.
* * This change had the side effect of breaking mock server tests that
redirected from `localhost` to `pypi-proxy.fly.dev`. I have added a
`CrossOriginCredentialsPolicy` enum with a `#[cfg(test)]`-only
`Insecure` variant. This allows existing tests to continue to work while
still making it impossible to propagate credentials on cross-origin
requests outside of tests.
* * I've updated the main redirect integration test to check if
cross-origin requests fail (there is, by design, no way to configure an
insecure cross-origin policy from the command line). But critically,
netrc credentials for the new location can still be successfully fetched
on a cross-origin redirect (tested in
`pip_install_redirect_with_netrc_cross_origin`).
* One of the goals of the refactor was to make the redirect handling
logic unit-testable. This PR adds a number of unit tests checking things
like proper propagation of credentials on redirects on the same domain
(and removal on cross-origin) and HTTP 303 POST-to-GET conversion.

The following table illustrates the different behaviors on current
`main`, the initial (reverted) redirect handling PR (#12920), the PR
that restores #12920 and fixes the 303s bug (#13595), and this PR
(#13754). We want to propagate credentials on same-origin but not
cross-origin redirects, and we want to look up netrc credentials on
redirects.

| Behavior | main | reverted #12920 | fix #13595 | update #13754 |

|---------------------------------------|------|--------------|-------------|----------------|
| Propagate credentials on same-origin redirects | No | Yes | Yes | Yes
|
| Propagate credentials on cross-origin redirects | No | Yes | Yes | No
|
| Look up netrc credentials on redirects | No | Yes | Yes | Yes |
| Handle 303s without failing | Yes | No | Yes | Yes |

Depends on #13595.
@jtfmumm jtfmumm force-pushed the feature/redirect-authentication branch from 9be29f6 to 53a0183 Compare June 19, 2025 11:43
@jtfmumm jtfmumm temporarily deployed to uv-test-registries June 19, 2025 11:45 — with GitHub Actions Inactive
@jtfmumm jtfmumm temporarily deployed to uv-test-publish June 19, 2025 11:45 — with GitHub Actions Inactive
@jtfmumm jtfmumm merged commit 62365d4 into main Jun 20, 2025
110 checks passed
@jtfmumm jtfmumm deleted the feature/redirect-authentication branch June 20, 2025 07:21
jtfmumm added a commit that referenced this pull request Jun 21, 2025
tmeijn pushed a commit to tmeijn/dotfiles that referenced this pull request Jun 24, 2025
This MR contains the following updates:

| Package | Update | Change |
|---|---|---|
| [astral-sh/uv](https://github.com/astral-sh/uv) | patch | `0.7.13` -> `0.7.14` |

MR created with the help of [el-capitano/tools/renovate-bot](https://gitlab.com/el-capitano/tools/renovate-bot).

**Proposed changes to behavior should be submitted there as MRs.**

---

### Release Notes

<details>
<summary>astral-sh/uv (astral-sh/uv)</summary>

### [`v0.7.14`](https://github.com/astral-sh/uv/blob/HEAD/CHANGELOG.md#0714)

[Compare Source](astral-sh/uv@0.7.13...0.7.14)

##### Enhancements

- Add XPU to `--torch-backend` ([#&#8203;14172](astral-sh/uv#14172))
- Add ROCm backends to `--torch-backend` ([#&#8203;14120](astral-sh/uv#14120))
- Remove preview label from `--torch-backend` ([#&#8203;14119](astral-sh/uv#14119))
- Add `[tool.uv.dependency-groups].mygroup.requires-python` ([#&#8203;13735](astral-sh/uv#13735))
- Add auto-detection for AMD GPUs ([#&#8203;14176](astral-sh/uv#14176))
- Show retries for HTTP status code errors ([#&#8203;13897](astral-sh/uv#13897))
- Support transparent Python patch version upgrades ([#&#8203;13954](astral-sh/uv#13954))
- Warn on empty index directory ([#&#8203;13940](astral-sh/uv#13940))
- Publish to DockerHub ([#&#8203;14088](astral-sh/uv#14088))

##### Performance

- Make cold resolves about 10% faster ([#&#8203;14035](astral-sh/uv#14035))

##### Bug fixes

- Don't use walrus operator in interpreter query script ([#&#8203;14108](astral-sh/uv#14108))
- Fix handling of changes to `requires-python` ([#&#8203;14076](astral-sh/uv#14076))
- Fix implied `platform_machine` marker for `win_amd64` platform tag ([#&#8203;14041](astral-sh/uv#14041))
- Only update existing symlink directories on preview uninstall ([#&#8203;14179](astral-sh/uv#14179))
- Serialize Python requests for tools as canonicalized strings ([#&#8203;14109](astral-sh/uv#14109))
- Support netrc and same-origin credential propagation on index redirects ([#&#8203;14126](astral-sh/uv#14126))
- Support reading `dependency-groups` from pyproject.tomls with no `[project]` ([#&#8203;13742](astral-sh/uv#13742))
- Handle an existing shebang in `uv init --script` ([#&#8203;14141](astral-sh/uv#14141))
- Prevent concurrent updates of the environment in `uv run` ([#&#8203;14153](astral-sh/uv#14153))
- Filter managed Python distributions by platform before querying when included in request ([#&#8203;13936](astral-sh/uv#13936))

##### Documentation

- Replace cuda124 with cuda128 ([#&#8203;14168](astral-sh/uv#14168))
- Document the way member sources shadow workspace sources ([#&#8203;14136](astral-sh/uv#14136))
- Sync documented PyTorch integration index for CUDA and ROCm versions from PyTorch website ([#&#8203;14100](astral-sh/uv#14100))

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied.

♻ **Rebasing**: Whenever MR becomes conflicted, or you tick the rebase/retry checkbox.

🔕 **Ignore**: Close this MR and you won't be reminded about this update again.

---

 - [ ] <!-- rebase-check -->If you want to rebase/retry this MR, check this box

---

This MR has been generated by [Renovate Bot](https://github.com/renovatebot/renovate).
<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiI0MC42Mi4xIiwidXBkYXRlZEluVmVyIjoiNDAuNjIuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsiUmVub3ZhdGUgQm90Il19-->
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants