Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Failed to install a package from a public url on 0.1.33 #3123

Closed
mgaitan opened this issue Apr 18, 2024 · 17 comments · Fixed by #3130
Closed

Failed to install a package from a public url on 0.1.33 #3123

mgaitan opened this issue Apr 18, 2024 · 17 comments · Fixed by #3130
Assignees
Labels
bug Something isn't working

Comments

@mgaitan
Copy link

mgaitan commented Apr 18, 2024

since yesterday, we are experimented this issue

 > [worker  7/13] RUN uv pip install --system --no-cache -r /tmp/shipping/reqs/requirements-dev.txt:
0.416 error: Failed to download: pyrovider @ https://github.com/Shiphero/pyrovider/releases/download/1.2.4/pyrovider-1.2.4-py3-none-any.whl
0.416   Caused by: HTTP status client error (401 Unauthorized) for url (https://objects.githubusercontent.com/github-production-release-asset-2e65be/144867083/d56d51b0-fe2d-4eda-a5f2-8f9d6503ce92?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAVCODYLSA53PQK4ZA%2F20240418%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240418T144100Z&X-Amz-Expires=300&X-Amz-Signature=6b628fe5fc427071d8bc8bc84688ecda750075f74d01e4fc236ee4a1cc9eb9af&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=144867083&response-content-disposition=attachment%3B%20filename%3Dpyrovider-1.2.4-py3-none-any.whl&response-content-type=application%2Foctet-stream)

As you can see, the package is public
https://github.com/Shiphero/pyrovider/releases/download/1.2.4/pyrovider-1.2.4-py3-none-any.whl

Pinned uv to 0.1.32 worked for now, but is it related to the breaking changes introduced in the 0.1.33 releases? Do we need to upgrade something in our requirement file?

@zanieb
Copy link
Member

zanieb commented Apr 18, 2024

Hi! Sorry about that, this looks like a bug.

Can you provide logs with RUST_LOG=trace uv pip install -v ... and/or a minimal example I could run?

@zanieb zanieb added the bug Something isn't working label Apr 18, 2024
@zanieb zanieb self-assigned this Apr 18, 2024
@ChannyClaus
Copy link
Contributor

fwiw it seems to work okay on my machine

$ uv --version
uv 0.1.33 (2cee7525c 2024-04-17)
chan.kang@Chans-MBP-4123 ~/test/uv - 
$ rm -rf .venv && uv venv && (echo 'pyrovider @ https://github.com/Shiphero/pyrovider/releases/download/1.2.4/pyrovider-1.2.4-py3-none-any.whl' | uv pip compile  - > requirements.txt) && uv pip install -r requirements.txt --no-cache
Using Python 3.12.1 interpreter at: /Users/chan.kang/.pyenv/versions/3.12.1/bin/python3
Creating virtualenv at: .venv
Activate with: source .venv/bin/activate
Resolved 3 packages in 52ms
Resolved 3 packages in 675ms
Downloaded 3 packages in 84ms
Installed 3 packages in 8ms
 + pyrovider==1.2.4 (from https://github.com/Shiphero/pyrovider/releases/download/1.2.4/pyrovider-1.2.4-py3-none-any.whl)
 + python-dotenv==1.0.1
 + pyyaml==6.0.1

@mgaitan
Copy link
Author

mgaitan commented Apr 18, 2024

my teammate @romeroyonatan just discovered that it does not happen when installing the public package alone but in conjunction with another package from an url that requires authentication. He will share an example in a moment.

@mgaitan
Copy link
Author

mgaitan commented Apr 18, 2024

btw, I just realized that this misbehavior looks a lot like #3122 and is probably related to the same root cause.

@romeroyonatan
Copy link

romeroyonatan commented Apr 18, 2024

Hi team, I found an easy way to reproduce the issue

uv pip install 'uv-3123-bug @ https://romeroyonatan:a@github.com/romeroyonatan/uv-3123-bug/releases/download/v0.1.4/uv_3123_bug-0.0.1-py3-none-any.whl'

This is the output

$ uv pip install 'uv-3123-bug @ https://romeroyonatan:a@github.com/romeroyonatan/uv-3123-bug/releases/download/v0.1.4/uv_3123_bug-0.0.1-py3-none-any.whl'
error: Failed to download: uv-3123-bug @ https://romeroyonatan:a@github.com/romeroyonatan/uv-3123-bug/releases/download/v0.1.4/uv_3123_bug-0.0.1-py3-none-any.whl
  Caused by: HTTP status client error (401 Unauthorized) for url (https://objects.githubusercontent.com/github-production-release-asset-2e65be/788539373/37fcd109-909c-4ed1-928c-2354c9a79a48?X-Amz-Algorithm=AWS4-HMAC-SHA256&X-Amz-Credential=AKIAVCODYLSA53PQK4ZA%2F20240418%2Fus-east-1%2Fs3%2Faws4_request&X-Amz-Date=20240418T165731Z&X-Amz-Expires=300&X-Amz-Signature=01a9bf253fb9083fd65e5b37c8b366e6349ca078644a32bac2e309e7a7ea69b2&X-Amz-SignedHeaders=host&actor_id=0&key_id=0&repo_id=788539373&response-content-disposition=attachment%3B%20filename%3Duv_3123_bug-0.0.1-py3-none-any.whl&response-content-type=application%2Foctet-stream)

This is the pip behavior

$ pip install 'uv-3123-bug @ https://romeroyonatan:a@github.com/romeroyonatan/uv-3123-bug/releases/download/v0.1.4/uv_3123_bug-0.0.1-py3-none-any.whl'
Collecting uv-3123-bug@ https://romeroyonatan:a@github.com/romeroyonatan/uv-3123-bug/releases/download/v0.1.4/uv_3123_bug-0.0.1-py3-none-any.whl
  Downloading https://romeroyonatan:****@github.com/romeroyonatan/uv-3123-bug/releases/download/v0.1.4/uv_3123_bug-0.0.1-py3-none-any.whl (2.7 kB)

[notice] A new release of pip is available: 23.2.1 -> 24.0
[notice] To update, run: pip install --upgrade pip

I tried to reproduce the bug using a private repo, but I found that the issue happens even with public repositories with valid or invalid credentials.

I suspect the credentials are cached and we use the private credentials when we shouldn't (public repositories for example)

Edit: I found the auth cache https://github.com/astral-sh/uv/blob/0.1.33/crates/uv-auth/src/cache.rs#L98-L107
Edit 2: This is the tracelog with the real instalation. I masked some sensitive data from the urls, sorry for that I did not reproduce the bug
Edit 3: New logs bug.log

@zanieb
Copy link
Member

zanieb commented Apr 18, 2024

Thanks a bunch ya'll! Will investigate.

@inoa-jboliveira
Copy link

There is a regression in 0.1.33 where user/pass combo on custom repos no longer work. It is also affecting us here.

@zanieb
Copy link
Member

zanieb commented Apr 18, 2024

@inoa-jboliveira can you be more explicit? Providing a username and password doesn't work for a private repository anymore? This issue is for applying credentials from private repositories to public repositories.

@inoa-jboliveira
Copy link

@zanieb Yes, the credentials are being ignored. I tested 0.1.31 and 0.1.32 and they work.

This and UV_INDEX_URL no longer work with user/pass combo
uv pip install --index-url https://myuser:mypass@example.com.br/repository/pypi-all/simple/

@romeroyonatan
Copy link

I found in the logs that the cached credentials are used when it shouldn't (public repositories)

TRACE Sending fresh HEAD request for https://github.com/Shiphero/pyrovider/releases/download/1.2.4/pyrovider-1.2.4-py3-none-any.whl
TRACE Handling request for https://github.com/Shiphero/pyrovider/releases/download/1.2.4/pyrovider-1.2.4-py3-none-any.whl
DEBUG resolving host="github.com"
TRACE No credentials on request, checking cache...
TRACE Found cached credentials.
TRACE checkout waiting for idle connection: ("https", github.com)

@inoa-jboliveira
Copy link

inoa-jboliveira commented Apr 18, 2024

Maybe this is a slightly different issue I'm having, but they seem to be caused by the same regression on v0.1.33

error: Failed to download: django==2.2.28
  Caused by: HTTP status client error (401 Unauthorized) for url  (https://example.com.br/repository/pypi-all/packages/django/2.2.28/Django-2.2.28-py3-none-any.whl#sha256=365429d07c1336eb42ba15aa79f45e1c13a0b04d5c21569e7d596696418a6a45)```
  

@zanieb
Copy link
Member

zanieb commented Apr 18, 2024

I think there are two things going on here:

  1. We fixed our caching to actually apply per network location as intended in the original implementation, but this means we can apply credentials to requests that don't need it.
  2. We stopped pre-seeding the cache with the URLs passed via the CLI, I think this is causing some sort of race condition where we don't apply credentials to some in flight requests because they're not cached yet.

@zanieb
Copy link
Member

zanieb commented Apr 19, 2024

If anyone is interested in trying out #3130, it should resolve these issues but coming up with integration test cases is quite challenging and will take a bit longer.

We're also considering reverting the problematic change at #3131, though that will cause different regressions.

zanieb added a commit that referenced this issue Apr 19, 2024
Roughly reverts
f7820ce
to reduce possible race conditions for pre-authenticated index URLs

Part of:

- #3123
- #3122
@romeroyonatan
Copy link

Hi @zanieb, I tested the changes and it works. Great job!

@inoa-jboliveira
Copy link

inoa-jboliveira commented Apr 19, 2024

Seems to be working on my issue as well, thank you

@zanieb
Copy link
Member

zanieb commented Apr 19, 2024

Thanks for giving it a try! I'll get that cleaned up and try to release today.

@zanieb
Copy link
Member

zanieb commented Apr 19, 2024

This may go out on Monday instead.

zanieb added a commit that referenced this issue Apr 22, 2024
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 #3123
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels