brew-rs: default to anonymous GHCR bearer token in fetch#21889
brew-rs: default to anonymous GHCR bearer token in fetch#21889cachebag wants to merge 1 commit intoHomebrew:mainfrom
Conversation
|
Holding off merge for Copilot. |
There was a problem hiding this comment.
Pull request overview
This PR improves brew-rs bottle downloads from GHCR when HOMEBREW_GITHUB_PACKAGES_AUTH is not set by adding an OCI/Docker-style Bearer token negotiation fallback, enabling anonymous access to public GHCR blobs during direct binary invocation and tests.
Changes:
- Refactors HTTP auth header resolution into
resolve_http_auth. - Adds GHCR OCI blob detection and
WWW-AuthenticateBearer challenge parsing. - Introduces token negotiation logic to obtain an anonymous bearer token and use it for downloads, plus unit tests for the parsing/detection helpers.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| fn negotiate_oci_token(client: &Client, url: &Url) -> BrewResult<Option<String>> { | ||
| let probe = client | ||
| .head(url.as_str()) | ||
| .send() | ||
| .with_context(|| format!("Failed to probe {url}"))?; | ||
|
|
||
| if probe.status() != reqwest::StatusCode::UNAUTHORIZED { | ||
| return Ok(None); | ||
| } |
There was a problem hiding this comment.
negotiate_oci_token always performs a separate HEAD probe before the actual GET, which adds an extra round trip per GHCR blob download (and will also fail to negotiate if the registry/proxy doesn’t support HEAD). Consider attempting the GET first and, only if it returns 401 with a WWW-Authenticate Bearer challenge, fetching the token and retrying the GET with Authorization: Bearer .... This avoids doubling requests on the common path and is more robust than relying on HEAD behavior.
There was a problem hiding this comment.
not sure i understand this...i'm pretty sure a HEAD is cheaper than starting a full GET of a bottle blob just to get a 401 and throw the body away
There was a problem hiding this comment.
Ideally we avoid this extra HEAD. Goal here is to just emulate the Ruby code here as closely as possible.
There was a problem hiding this comment.
If the Ruby code does a HEAD: fine, if not, let's just copy it. If a GET gives you a 401 the content size will be tiny.
There was a problem hiding this comment.
yep I just have it fall back to Bearer QQ== like brew.sh does.
| fn parse_bearer_challenge(header: &str) -> Option<BearerChallenge> { | ||
| let params = header.strip_prefix("Bearer ")?; | ||
| let realm = extract_challenge_param(params, "realm")?; | ||
| let service = extract_challenge_param(params, "service")?; | ||
| let scope = extract_challenge_param(params, "scope")?; | ||
| Some(BearerChallenge { |
There was a problem hiding this comment.
parse_bearer_challenge matches only the exact prefix "Bearer ". HTTP auth schemes are case-insensitive, so a server sending bearer/BEARER would cause negotiation to be skipped and the download to fail. Consider parsing the scheme in a case-insensitive way (e.g., split once on whitespace and compare with eq_ignore_ascii_case("bearer")).
| if should_send_github_packages_auth(url) { | ||
| if let Some(auth) = env_value("HOMEBREW_GITHUB_PACKAGES_AUTH") { |
There was a problem hiding this comment.
In resolve_http_auth, the outer if should_send_github_packages_auth(url) guard is redundant because should_send_github_packages_auth already depends on HOMEBREW_GITHUB_PACKAGES_AUTH being present, and then the code checks that env var again. This can be simplified to reduce duplicated env lookups and make the control flow clearer (e.g., check env_value("HOMEBREW_GITHUB_PACKAGES_AUTH") first, then apply the host/other gating logic).
| if should_send_github_packages_auth(url) { | |
| if let Some(auth) = env_value("HOMEBREW_GITHUB_PACKAGES_AUTH") { | |
| if let Some(auth) = env_value("HOMEBREW_GITHUB_PACKAGES_AUTH") { | |
| if should_send_github_packages_auth(url) { |
cachebag
force-pushed
the
main
branch
2 times, most recently
from
3256cd6 to
8458c29
Compare
cachebag
changed the title
brew lgtm(style, typechecking and tests) with your changes locally?I initally thought there was something going on with the redirect/Accept header interaction, but it just looks like the rust downloader depends on
brew.shsettingHOMEBREW_GITHUB_PACKAGES_AUTHfor GHCR bottle downloads. so when this variable isn't present (direct binary invocation, testing), GHCR returns 401 and the download fails silently.this PR
adds OCI bearer token negotiation as a fallback. essentially, when a GHCR blob request gets a 401 with a www-authenticate challenge, we now just fetch an anonymous bearer token from the token endpoint and retry. this falls in line with how standard OCI/Docker clients authenticate with public registries.falls back toBearer QQ==when the env var is not set.the existing
HOMEBREW_GITHUB_PACKAGES_AUTHpath is still of course, preferred when available.