Skip to content
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions docs/how-tos/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,22 @@ the first match:
2. `CI_JOB_TOKEN` environment variable
3. `GITLAB_PRIVATE_TOKEN` environment variable

**Other hosts**, including private wheel indexes, use HTTP Basic authentication

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried to confirm the “default entry is never used” and “HTTP URLs do not receive fallback credentials” statements, and I think redirects get around both. requests doesn’t run session.auth again on a redirect. SessionRedirectMixin.rebuild_auth() calls get_netrc_auth(new_url) instead, which falls back to default and doesn’t check the scheme. So if wheels.example redirects a download to some CDN or object-storage host, that host gets the default credentials, and an https → http redirect to a host with a machine entry sends them in cleartext. Was this considered? We could soften the docs, or properly fix it by overriding rebuild_auth in a Session subclass so redirects go through SessionAuth too. I don’t think trust_env = False is an option, because that also drops the proxy environment variables.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You’re right—the docs missed redirects. I’ve fixed redirects to use the same HTTPS-only, exact-host authentication rules, kept proxy support, and added tests

from an exact netrc `machine` entry over HTTPS. The netrc `default` entry is
never used by this fallback, and HTTP URLs do not receive fallback credentials.
Registered authentication callbacks take precedence, including callbacks that
return no credentials. Explicit headers and request-specific authentication
also take precedence.

Fallback results, including missing credentials, are cached per scheme and host
for the lifetime of the session. Create a new session after changing netrc
credentials.

```{versionchanged} 0.104.0
Restore cached netrc Basic authentication for hosts without a registered
authentication callback, limited to exact `machine` entries over HTTPS.
```
Comment on lines +28 to +48

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please update this to match the new rules:

Suggested change
**Other hosts**, including private wheel indexes, use HTTP Basic authentication
from a matching netrc entry. Registered authentication callbacks take precedence;
the netrc fallback does not replace an explicit `Authorization` header or
request-specific authentication.
```{versionchanged} 0.104.0
Restore netrc Basic authentication for hosts without a registered authentication callback.
```
**Other hosts**, including private wheel indexes, use HTTP Basic authentication
from a netrc `machine` entry that matches the host exactly. This only applies to
HTTPS URLs, and the netrc `default` entry is never used. Registered authentication
callbacks take precedence, and the netrc fallback does not replace an explicit
`Authorization` header or request-specific authentication.
```{versionchanged} 0.104.0
Restore netrc Basic authentication for hosts without a registered authentication
callback, limited to exact `machine` entries over HTTPS.
```


## netrc

The [requests](https://requests.readthedocs.io) library, `pip`, and
Expand All @@ -34,6 +50,12 @@ specified by setting the `NETRC` environment variable. Note that
`NETRC` variable since [8.16.0](https://curl.se/ch/8.16.0.html)
(2025-09-10). Older versions only read `$HOME/.netrc`.

For example, to authenticate to a private wheel index:

```text
machine wheels.example login reader password secret
```

For example, to authenticate to a GitLab package registry with a
[personal access token](https://docs.gitlab.com/ee/user/profile/personal_access_tokens.html#create-a-personal-access-token):

Expand Down
2 changes: 1 addition & 1 deletion pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ dependencies = [
"pyproject_hooks>=1.0.0,!=1.1.0",
"PyYAML",
"rich",
"requests",
"requests>=2.32.4",
"resolvelib",
"starlette",
"stevedore",
Expand Down
63 changes: 53 additions & 10 deletions src/fromager/request_session.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
from __future__ import annotations

import logging
import netrc
import os
from typing import TYPE_CHECKING
from urllib.parse import urlparse
Expand Down Expand Up @@ -40,10 +41,12 @@ class SessionAuth(requests.auth.AuthBase):
and does not provide per-host authentication on mounted adapters.
This class fills that gap by mapping ``(scheme, hostname)`` keys to
auth resolver callbacks. On the first request to a given host the
callback is invoked and the result is cached.
callback is invoked and the result is cached. Hosts without a registered
callback use the optional fallback callback.
"""

def __init__(self) -> None:
def __init__(self, fallback: _AuthCallback | None = None) -> None:
self.fallback = fallback
self._callbacks: dict[tuple[str, str], _AuthCallback] = {}
self._cache: dict[tuple[str, str], dict[str, str]] = {}

Expand All @@ -63,23 +66,62 @@ def add(self, url: str, callback: _AuthCallback) -> None:
def get(self, url: str) -> dict[str, str]:
"""Resolve and return the auth headers for *url*.

Invokes the registered callback on first access and caches the
result. Returns an empty dict when no callback is registered.
Invokes the registered callback or fallback on first access and caches
the result. Returns an empty dict when neither applies.
"""
parsed = urlparse(url)
key = (parsed.scheme, parsed.hostname or "")
if key not in self._cache:
callback = self._callbacks.get(key)
callback = self._callbacks.get(key, self.fallback)
self._cache[key] = callback(*key) if callback else {}
return dict(self._cache[key])

def __call__(self, r: requests.PreparedRequest) -> requests.PreparedRequest:
auth_header = self.get(r.url or "")
if auth_header:
r.headers.update(auth_header)
"""Apply resolved headers while preserving headers set by the caller."""
for name, value in self.get(r.url or "").items():
r.headers.setdefault(name, value)
return r


def _find_netrc() -> str | None:
"""Find netrc using NETRC or the standard home-directory locations."""
netrc_env = os.environ.get("NETRC")
candidates = [netrc_env] if netrc_env is not None else ["~/.netrc", "~/_netrc"]
for candidate in candidates:
path = os.path.expanduser(candidate)
if os.path.exists(path):
return path
return None


def _resolve_netrc_auth(scheme: str, hostname: str) -> dict[str, str]:
"""Resolve Basic auth from an exact netrc machine entry over HTTPS."""
if scheme != "https" or not hostname or hostname == "default":
return {}
path = _find_netrc()
if path is None:
return {}
try:
# authenticators() also matches the default entry, which we must ignore.
entry = netrc.netrc(path).hosts.get(hostname)
except (netrc.NetrcParseError, OSError, UnicodeError):
# Parse errors can contain credentials; do not include exception text.
logger.warning("netrc auth: cannot read credentials from %s", path)
return {}
if entry is None or not any(entry):
return {}
login, account, password = entry
request = requests.PreparedRequest()
request.prepare_headers({})
try:
requests.auth.HTTPBasicAuth(login or account or "", password or "")(request)
except UnicodeEncodeError:
logger.warning("netrc auth: cannot encode credentials for %s", hostname)
return {}
logger.debug("netrc auth: using credentials for %s", hostname)
return {"Authorization": str(request.headers["Authorization"])}


def _resolve_github_auth(scheme: str, hostname: str) -> dict[str, str]:
"""Resolve GitHub auth header from netrc or environment."""
url = f"{scheme}://{hostname}"
Expand Down Expand Up @@ -123,7 +165,8 @@ def create_session() -> tuple[requests.Session, SessionAuth]:

Mounts a `RetryHTTPAdapter` on ``http://`` and ``https://``.
Registers lazy auth callbacks for GitHub and GitLab on a
`SessionAuth` handler keyed by ``(scheme, hostname)``.
`SessionAuth` handler keyed by ``(scheme, hostname)``. Other HTTPS hosts
use exact netrc machine entries as a cached fallback.

Returns the session and its `SessionAuth` so callers can
register additional auth callbacks via ``auth.add()``.
Expand All @@ -137,7 +180,7 @@ def create_session() -> tuple[requests.Session, SessionAuth]:
s.mount("http://", adapter)
s.mount("https://", adapter)

auth = SessionAuth()
auth = SessionAuth(fallback=_resolve_netrc_auth)
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Resolved

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Related to the redirect question: is there a test where the index answers with a 302 to another host? A test like that would pin down whatever behavior we decide on. It would also catch the separate, pre-existing issue that GitLab’s PRIVATE-TOKEN/JOB-TOKEN headers are never stripped on a cross-host redirect, since requests only strips Authorization. That second part can definitely be a follow-up.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a test for a 302 redirect to another host. GitLab token headers still need separate handling; I’ve left that for a follow-up as suggested.

auth.add(GITHUB_API_URL, _resolve_github_auth)
auth.add(GITLAB_CI_SERVER_URL, _resolve_gitlab_auth)
s.auth = auth
Expand Down
Loading