Picks up the work from
- #14559
- https://github.com/astral-sh/uv/pull/14896
There are some high-level changes from those pull requests
1. We do not stash seen credentials in the keyring automatically
2. We use `auth login` and `auth logout` (for future consistency)
3. We add a `token` command for showing the credential that will be used
As well as many smaller changes to API, messaging, testing, etc.
---------
Co-authored-by: John Mumm <jtfmumm@gmail.com>
## Summary
Make the use of `Self` consistent. Mostly done by running `cargo clippy
--fix -- -A clippy::all -W clippy::use_self`.
## Test Plan
<!-- How was it tested? -->
No need.
## Summary
If `HF_TOKEN` is set, we'll automatically wire it up to authenticate
requests when hitting private `huggingface.co` URLs in `uv run`.
## Test Plan
An unauthenticated request:
```
> cargo run -- run https://huggingface.co/datasets/cmarsh/test/resolve/main/main.py
File "/var/folders/nt/6gf2v7_s3k13zq_t3944rwz40000gn/T/mainYadr5M.py", line 1
Invalid username or password.
^^^^^^^^
SyntaxError: invalid syntax
```
An authenticated request:
```
> HF_TOKEN=hf_... cargo run run https://huggingface.co/datasets/cmarsh/test/resolve/main/main.py
Hello from main.py!
```
By default, Rust does not support safe cast from `&U` to `&T` for
`#[repr(transparent)] T(U)` even if the newtype opts in. The dtolnay
ref-cast crate fills this gap, allowing to remove `DisplaySafeUrlRef`.
This PR implements a few review follow-ups from #13560. In particular,
it
* Makes `DisplaySafeUrlRef` implement `Copy` so that it can be passed by
value.
* Updates `to_string_with_credentials` methods with
`displayable_with_credentials`, returning an `impl Display` instead of
`String` for greater flexibility.
* Updates the `DisplaySafeUrl` and `DisplaySafeUrlRef` `Debug`
implementations to match the underlying `Url` `Debug` implementation
(with the exception that credentials are masked).
* Replaces an unnecessary `DisplaySafeUrl::from(Url::from_file_path`
with `DisplaySafeUrl::from_file_path`
Prior to this PR, there were numerous places where uv would leak
credentials in logs. We had a way to mask credentials by calling methods
or a recently-added `redact_url` function, but this was not secure by
default. There were a number of other types (like `GitUrl`) that would
leak credentials on display.
This PR adds a `DisplaySafeUrl` newtype to prevent leaking credentials
when logging by default. It takes a maximalist approach, replacing the
use of `Url` almost everywhere. This includes when first parsing config
files, when storing URLs in types like `GitUrl`, and also when storing
URLs in types that in practice will never contain credentials (like
`DirectorySourceUrl`). The idea is to make it easy for developers to do
the right thing and for the compiler to support this (and to minimize
ever having to manually convert back and forth). Displaying credentials
now requires an active step. Note that despite this maximalist approach,
the use of the newtype should be zero cost.
One conspicuous place this PR does not use `DisplaySafeUrl` is in the
`uv-auth` crate. That would require new clones since there are calls to
`request.url()` that return a `&Url`. One option would have been to make
`DisplaySafeUrl` wrap a `Cow`, but this would lead to lifetime
annotations all over the codebase. I've created a separate PR based on
this one (#13576) that updates `uv-auth` to use `DisplaySafeUrl` with
one new clone. We can discuss the tradeoffs there.
Most of this PR just replaces `Url` with `DisplaySafeUrl`. The core is
`uv_redacted/lib.rs`, where the newtype is implemented. To make it
easier to review the rest, here are some points of note:
* `DisplaySafeUrl` has a `Display` implementation that masks
credentials. Currently, it will still display the username when there is
both a username and password. If we think is the wrong choice, it can
now be changed in one place.
* `DisplaySafeUrl` has a `remove_credentials()` method and also a
`.to_string_with_credentials()` method. This allows us to use it in a
variety of scenarios.
* `IndexUrl::redacted()` was renamed to
`IndexUrl::removed_credentials()` to make it clearer that we are not
masking.
* We convert from a `DisplaySafeUrl` to a `Url` when calling `reqwest`
methods like `.get()` and `.head()`.
* We convert from a `DisplaySafeUrl` to a `Url` when creating a
`uv_auth::Index`. That is because, as mentioned above, I will be
updating the `uv_auth` crate to use this newtype in a separate PR.
* A number of tests (e.g., in `pip_install.rs`) that formerly used
filters to mask tokens in the test output no longer need those filters
since tokens in URLs are now masked automatically.
* The one place we are still knowingly writing credentials to
`pyproject.toml` is when a URL with credentials is passed to `uv add`
with `--raw`. Since displaying credentials is no longer automatic, I
have added a `to_string_with_credentials()` method to the `Pep508Url`
trait. This is used when `--raw` is passed. Adding it to that trait is a
bit weird, but it's the simplest way to achieve the goal. I'm open to
suggestions on how to improve this, but note that because of the way
we're using generic bounds, it's not as simple as just creating a
separate trait for that method.
We were not correctly falling back to cached realm credentials when an
index URL was provided with only a username. This came up in a [later
comment](https://github.com/astral-sh/uv/issues/13443#issuecomment-2881115301)
on #13443 where credentials in a pip extra index in `uv.toml` were being
ignored when the same URL (but with only a username) was used at the
command line for `--extra-index-url`. I've added a test to catch this
case.
Closes#13443
This PR redacts credentials in displayed URLs.
It mostly relies on a `redacted_url` function (and where possible
`IndexUrl::redacted`). This is a quick way to prevent leaked credentials
but it's prone to programmer error when adding new trace statements. A
better follow-on would use a `RedactedUrl` type with the appropriate
`Display` implementation. This would allow us to still extract
credentials from the URL while displaying it securely. On the plus side,
the sites where the `redacted_url` function are used serve as easy
signposts for where to use the new type in a future PR.
Closes#1714.
uv’s default index strategy was designed with dependency confusion
attacks in mind. [According to the
docs](https://docs.astral.sh/uv/configuration/indexes/#searching-across-multiple-indexes),
“if a package exists on an internal index, it should always be installed
from the internal index, and never from PyPI”. Unfortunately, this is
not true in the case where authentication fails on that internal index.
In that case, uv will simply try the next index (even on the
`first-index` strategy). This means that uv is not secure by default in
this common scenario.
This PR causes uv to stop searching for a package if it encounters an
authentication failure at an index. It is possible to opt out of this
behavior for an index with a new `pyproject.toml` option
`ignore-error-codes`. For example:
```
[[tool.uv.index]]
name = "my-index"
url = "<index-url>"
ignore-error-codes = [401, 403]
```
This will also enable users to handle idiosyncratic registries in a more
fine-grained way. For example, PyTorch registries return a 403 when a
package is not found. In this PR, we special-case PyTorch registries to
ignore 403s, but users can use `ignore-error-codes` to handle similar
behaviors if they encounter them on internal registries.
Depends on #12651Closes#9429Closes#12362
Some registries (like Azure Artifact) can require you to authenticate
separately for every package URL if you do not authenticate for the
/simple endpoint. These changes make the auth middleware aware of index
URL endpoints and attempts to fetch keyring credentials for such an
index URL when making a request to any URL it's a prefix of.
The current uv behavior is to cache credentials either at the request
URL or realm level. But with these changes, we also need to cache
credentials at the index level. Note that when uv does not detect an
index URL for a request URL, it will continue to apply the old behavior.
Addresses part of #4056Closes#4583Closes#11236Closes#11391Closes#11507
I noticed in the trace output that we weren't obfuscating the
`Credentials` password in a trace message. This PR creates a `Password`
newtype with a custom `Debug` implementation.
## Summary
I noticed that these only support Basic credentials, but we may want to
allow users to provide Bearer tokens? This PR just generalizes the type.
We were not applying the `authenticate = "always"` behavior to `uv pip`
commands (related to #12362). This PR addresses that, applying
authentication policies wherever we set up a registry client.
## Summary
This PR modifies the requirement source entities to store a (new)
container struct that wraps `IndexUrl`. This will allow us to store
user-defined metadata alongside `IndexUrl`, and propagate that metadata
throughout resolution.
Specifically, I need to store the "kind" of the index (Simple API vs.
`--find-links`), but I also ran into this problem when I tried to add
support for overriding `Cache-Control` headers on a per-index basis: at
present, we have no way to passing around metadata alongside an
`IndexUrl`.
Previously, we required a username to perform a fetch from the keyring
because the `keyring` CLI only supported fetching password for a given
service and username. Unfortunately, this is different from the keyring
Python API which supported fetching a username _and_ password for a
given service. We can't (easily) use the Python API because we don't
expect `keyring` to be installed in a specific environment during
network requests. This means that we did not have parity with `pip`.
Way back in https://github.com/jaraco/keyring/pull/678 we got a `--mode
creds` flag added to `keyring`'s CLI which supports parity with the
Python API. Since `keyring` is expensive to invoke and we cannot be
certain that users are on the latest version of keyring, we've not added
support for invoking keyring with this flag. However, now that we have a
mode that says authentication is _required_ for an index (#11896), we
might as well _try_ to invoke keyring with `--mode creds` when there is
no username. This will address use-cases where the username is
non-constant and move us closer to `pip` parity.
This addresses a small part of #12280, namely when you have
`authenticate` set to `always`, it will output a distinct error message
for the case where you have a username but are missing a password.
Adds a new optional key `auth-policy` to `[tool.uv.index]` that sets the
authentication policy for the index URL.
The default is `"auto"`, which attempts to authenticate when necessary.
`"always"` always attempts to authenticate and fails if the endpoint is
unauthenticated. `"never"` never attempts to authenticate.
These policy address two kinds of cases:
* Some indexes don’t fail on unauthenticated requests; instead they just
forward to the public PyPI. This can leave the user confused as to why
their package is missing. The "always" policy prevents this.
* "never" allows users to ensure their credentials couldn't be leaked to
an unexpected index, though it will only allow for successful requests
on an index that doesn't require credentials.
Closes#11600
## Summary
This lets us drop a dependency entirely. `percent-encoding` is used by
`url` and so is already in the graph, whereas `urlencoding` isn't used
by anything else.
When trying to upload without a password but with the keyring, check
that the keyring has a password for the upload URL and username and warn
if it doesn't.
Fixes#8781
## Summary
These were moved as part of a broader refactor to create a single
integration test module. That "single integration test module" did
indeed have a big impact on compile times, which is great! But we aren't
seeing any benefit from moving these tests into their own files (despite
the claim in [this blog
post](https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html),
I see the same compilation pattern regardless of where the tests are
located). Plus, we don't have many of these, and same-file tests is such
a strong Rust convention.
## Summary
Resolves#7685
## Test Plan
```console
$ echo "this is an invalid netrc" > .netrc
$ NETRC=.netrc cargo run -- pip install anyio --index-url https://pypi-proxy.fly.dev/basic-auth/simple --strict -v
DEBUG uv 0.4.24 (f4d5fba61 2024-10-19)
DEBUG Searching for default Python interpreter in system path or `py` launcher
DEBUG Found `cpython-3.11.2-windows-x86_64-none` at `D:\Projects\Rust\uv\.venv\Scripts\python.exe` (virtual environment)
DEBUG Using Python 3.11.2 environment at .venv
DEBUG Acquired lock for `.venv`
DEBUG At least one requirement is not satisfied: anyio
DEBUG Using request timeout of 30s
DEBUG Solving with installed Python version: 3.11.2
DEBUG Solving with target Python version: >=3.11.2
DEBUG Adding direct dependency: anyio*
DEBUG No cache entry for: https://pypi-proxy.fly.dev/basic-auth/simple/anyio/
WARN Error reading netrc file: parsing error: bad toplevel token 'this' (line 1) in the file '.netrc'
DEBUG Searching for a compatible version of anyio (*)
DEBUG No compatible version found for: anyio
× No solution found when resolving dependencies:
╰─▶ Because anyio was not found in the package registry and you require anyio, we can conclude that your
requirements are unsatisfiable.
hint: An index URL (https://pypi-proxy.fly.dev/basic-auth/simple) could not be queried due to a lack of valid
authentication credentials (401 Unauthorized).
DEBUG Released lock at `D:\Projects\Rust\uv\.venv\.lock`
error: process didn't exit successfully: `target\debug\uv.exe pip install anyio --index-url https://pypi-proxy.fly.dev/basic-auth/simple --strict -v` (exit code: 1)
```
Basically, if username-only authentication came from the _cache_ instead
of being present on the _request URL_ to start, we'd end up ignoring it
during password lookups which breaks keyring.
Includes some cosmetic changes to the logging and commentary in the
middleware, because I was confused when reading the code and logs.
The docs reference `UV_INDEX_`, but the code actually uses
UV_HTTP_BASIC_ as the prefix for environment variable credentials.
See PR #7741
Code is at
https://github.com/astral-sh/uv/blob/main/crates/uv-static/src/env_vars.rs#L163
```rust
/// Generates the environment variable key for the HTTP Basic authentication username.
pub fn http_basic_username(name: &str) -> String {
format!("UV_HTTP_BASIC_{name}_USERNAME")
}
/// Generates the environment variable key for the HTTP Basic authentication password.
pub fn http_basic_password(name: &str) -> String {
format!("UV_HTTP_BASIC_{name}_PASSWORD")
}
```
---------
Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
## Summary
This PR enables users to provide index credentials via named environment
variables.
For example, given an index named `internal` that requires a username
(`public`) and password
(`koala`), you can define the index (without credentials) in your
`pyproject.toml`:
```toml
[[tool.uv.index]]
name = "internal"
url = "https://pypi-proxy.corp.dev/simple"
```
Then set the `UV_INDEX_INTERNAL_USERNAME` and
`UV_INDEX_INTERNAL_PASSWORD`
environment variables, where `INTERNAL` is the uppercase version of the
index name:
```sh
export UV_INDEX_INTERNAL_USERNAME=public
export UV_INDEX_INTERNAL_PASSWORD=koala
```
As per
https://matklad.github.io/2021/02/27/delete-cargo-integration-tests.html
Before that, there were 91 separate integration tests binary.
(As discussed on Discord — I've done the `uv` crate, there's still a few
more commits coming before this is mergeable, and I want to see how it
performs in CI and locally).
While working on https://github.com/astral-sh/uv/pull/6389 I discovered
we never checked `cache.get_url` here, which is wrong — though I don't
think it had much effect in practice since the realm would typically
match first. The main problem is that when we call `get_url` later we
hard-code the username to `None` because we assume we checked up here
with the username if present.
## Summary
We retain them if you use `--raw-sources`, but otherwise they're
removed. We still respect them in the subsequent `uv.lock` via an
in-process store.
Closes#6056.
## Summary
Closes https://github.com/astral-sh/uv/issues/4162
Changes keyring subprocess to allow display of stderr.
This aligns with pip's behavior since pip 23.1.
## Test Plan
* Tested using gnome-keyring-backend on a self-hosted private registry
as well as the keyring script described in #4162 to confirm both
existing functionality and the new stderr display.
* Existing tests using `scripts/packages/keyring_test_plugin` are now
showing its stderr output as well.
Partially addresses https://github.com/astral-sh/uv/issues/4056
We were incorrectly omitting the port from requests to `keyring` when
falling back to a realm/host query, e.g. `localhost` was used instead of
`localhost:1234`. We still won't include "standard" ports like `80` for
an HTTP request.
The only thing a `OnceMap` really needs to be able to do with the value
is to clone it. All extant uses benefited from having this done for them
by automatically wrapping values in an `Arc`. But this isn't necessarily
true for all things. For example, a value might have an `Arc` internally
to making cloning cheap in other contexts, and it doesn't make sense to
re-wrap it in an `Arc` just to use it with a `OnceMap`. Or
alternatively, cloning might just be cheap enough on its own that an
`Arc` isn't worth it.
Fixes the failure to lookup credentials in
https://github.com/astral-sh/uv/issues/3205
The issue is that we seed the cache with the index URL which includes a
username but no password. We did not ensure that a password was present
in the cached credentials before attempting a request with them. Now,
the cache will not return credentials when a username is provided and
the cached credentials have no password — the cached credentials are
useless in that case.
Tested with a Google Artifact Registry and keyring
```
RUST_LOG=uv=trace cargo run -q -- pip install requests --index-url https://oauth2accesstoken@us-central1-python.pkg.dev/<project>/pypi/simple/ --no-cache --keyring-provider subprocess -v
```
In #2976 I made some changes that led to regressions:
- We stopped tracking URLs that we had not seen credentials for in the
cache
- This means the cache no longer returns a value to indicate we've seen
a realm before
- We stopped seeding the cache with URLs
- Combined with the above, this means we no longer had a list of
locations that we would never attempt to fetch credentials for
- We added caching of credentials found on requests
- Previously the cache was only populated from the seed or credentials
found in the netrc or keyring
- This meant that the cache was populated for locations that we
previously did not cache, i.e. GitHub artifacts(?)
Unfortunately this unveiled problems with the granularity of our cache.
We cache credentials per realm (roughly the hostname) but some realms
have mixed authentication modes i.e. different credentials per URL or
URLs that do not require credentials. Applying credentials to a URL that
does not require it can lead to a failed request, as seen in #3123 where
GitHub throws a 401 when receiving credentials.
To resolve this, the cache is expanded to supporting caching at two
levels:
- URL, cached URL must be a prefix of the request URL
- Realm, exact match required
When we don't have URL-level credentials cached, we attempt the request
without authentication first. On failure, we'll search for realm-level
credentials or fetch credentials from external services. This avoids
providing credentials to new URLs unless we know we need them.
Closes https://github.com/astral-sh/uv/issues/3123
## Summary
This PR adds basic struct definitions along with a "workspace" concept
for discovering settings. (The "workspace" terminology is used to match
Ruff; I did not invent it.)
A few notes:
- We discover any `pyproject.toml` or `uv.toml` file in any parent
directory of the current working directory. (We could adjust this to
look at the directories of the input files.)
- We don't actually do anything with the configuration yet; but those
PRs are large and I want this to be reviewed in isolation.
Closes#2566
We were storing the username e.g. `charlie@astral.sh` as a
percent-encoded string `charlie%40astral.sh` which resulted in different
headers and broke JFrog's artifactory which apparently does not decode
usernames.
Tested with a JFrog artifactory and AWS CodeArtifact although it is
worth noting that AWS does _not_ have a username with an `@` — it'd be
nice to test another artifactory with percent-encoded characters in the
username and/or password.
## Summary
Right now, the middleware doesn't apply credentials that were
_originally_ sourced from a URL. This requires that we call
`with_url_encoded_auth` whenever we create a request to ensure that any
credentials that were passed in as part of an index URL (for example)
are respected.
This PR modifies `uv-auth` to instead apply those credentials in the
middleware itself. This seems preferable to me. As far as I can tell, we
can _only_ add in-URL credentials to the store ourselves (since in-URL
credentials are converted to headers by the time they reach the
middleware). And if we ever _didn't_ apply those credentials to new
URLs, it'd be a bug in the logic that precedes the middleware (i.e., us
forgetting to call `with_url_encoded_auth`).
## Test Plan
`cargo run pip install` with an authenticated index.
## Summary
The authentication middleware extracts in-URL credentials from URLs that
pass through it; however, by the time a request reaches the store, the
credentials will have already been removed, and relocated to the header.
So we were never propagating in-URL credentials.
This PR adds an explicit pass wherein we pass in-URL credentials to the
store prior to doing any work.
Closes https://github.com/astral-sh/uv/issues/2444.
## Test Plan
`cargo run pip install` against an authenticated AWS registry.
<!--
Thank you for contributing to uv! To help us out with reviewing, please
consider the following:
- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->
## Summary
<!-- What's the purpose of the change? What does it do, and why? -->
Adds basic keyring auth support for `uv` commands. Adds clone of `pip`'s
`--keyring-provider subprocess` argument (using CLI `keyring` tool).
See issue: https://github.com/astral-sh/uv/issues/1520
## Test Plan
<!-- How was it tested? -->
Hard to write full-suite unit tests due to reliance on
`process::Command` for `keyring` cli
Manually tested end-to-end in a project with GCP artifact registry using
keyring password:
```bash
➜ uv pip uninstall watchdog
Uninstalled 1 package in 46ms
- watchdog==4.0.0
➜ cargo run -- pip install --index-url https://<redacted>/python/simple/ --extra-index-url https://<redacted>/pypi-mirror/simple/ watchdog
Finished dev [unoptimized + debuginfo] target(s) in 0.18s
Running `target/debug/uv pip install --index-url 'https://<redacted>/python/simple/' --extra-index-url 'https://<redacted>/pypi-mirror/simple/' watchdog`
error: HTTP status client error (401 Unauthorized) for url (https://<redacted>/pypi-mirror/simple/watchdog/)
➜ cargo run -- pip install --keyring-provider subprocess --index-url https://<redacted>/python/simple/ --extra-index-url https://<redacted>/pypi-mirror/simple/ watchdog
Finished dev [unoptimized + debuginfo] target(s) in 0.17s
Running `target/debug/uv pip install --keyring-provider subprocess --index-url 'https://<redacted>/python/simple/' --extra-index-url 'https://<redacted>/pypi-mirror/simple/' watchdog`
Resolved 1 package in 2.34s
Installed 1 package in 27ms
+ watchdog==4.0.0
```
`requirements.txt`
```
#
# This file is autogenerated by pip-compile with Python 3.10
# by the following command:
#
# .bin/generate-requirements
#
--index-url https://<redacted>/python/simple/
--extra-index-url https://<redacted>/pypi-mirror/simple/
...
```
```bash
➜ cargo run -- pip install --keyring-provider subprocess -r requirements.txt
Finished dev [unoptimized + debuginfo] target(s) in 0.19s
Running `target/debug/uv pip install --keyring-provider subprocess -r requirements.txt`
Resolved 205 packages in 23.52s
Built <redacted>
...
Downloaded 47 packages in 19.32s
Installed 195 packages in 276ms
+ <redacted>
...
```
---------
Co-authored-by: Thomas Gilgenast <thomas@vant.ai>
Co-authored-by: Zanie Blue <contact@zanie.dev>
Address a few pedantic lints
lints are separated into separate commits so they can be reviewed
individually.
I've not added enforcement for any of these lints, but that could be
added if desirable.
Closes https://github.com/astral-sh/uv/issues/1709
Closes https://github.com/astral-sh/uv/issues/1371
Tested with the reproduction provided in #1709 which gets past the HTTP
401.
Reuses the same copying logic we introduced in
https://github.com/astral-sh/uv/pull/1874 to ensure authentication is
attached to file URLs with a realm that matches that of the index. I had
to move the authentication logic into a new crate so it could be used in
`distribution-types`.
We will want to something more robust in the future, like track all
realms with authentication in a central store and perform lookups there.
That's what `pip` does and it allows consolidation of logic like netrc
lookups. That refactor feels significant though, and I'd like to get
this fixed ASAP so this is a minimal fix.
Zanie Blue
adamnemecek
Charlie Marsh
konsti
John Mumm
JackDyre
samypr100
Jo
David Bern
Amos Wenger
Ed Morley
renovate[bot]
Andrew Gallant
哇呜哇呜呀咦耶
John
Hans Baker
danieleades