From 85afacfd1344b150119bf90011b5cce213f4ab52 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 11:59:38 -0500 Subject: [PATCH 1/7] feat(auth): support GitHub CLI credentials Add a dedicated github-cli authentication mode with host-aware token resolution, in-memory caching, sanitized failures, tests, and documentation. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 434e2508-0633-4d40-8e0d-5edb92cbc177 --- docs/reference/authentication.md | 37 +++ .../authentication/azure_devops.py | 8 +- src/specify_cli/authentication/base.py | 11 +- src/specify_cli/authentication/bitbucket.py | 8 +- src/specify_cli/authentication/config.py | 2 +- src/specify_cli/authentication/github.py | 112 +++++++- src/specify_cli/authentication/http.py | 14 +- .../authentication/test_authentication.py | 245 +++++++++++++++++- 8 files changed, 422 insertions(+), 15 deletions(-) diff --git a/docs/reference/authentication.md b/docs/reference/authentication.md index 9e0ff21449..be769af819 100644 --- a/docs/reference/authentication.md +++ b/docs/reference/authentication.md @@ -59,6 +59,7 @@ Either `token` or `token_env` must be set for the `bearer`, `basic-pat`, and `ba | Scheme | Header | Use for | |---|---|---| | `bearer` | `Authorization: Bearer ` | PATs, fine-grained PATs, OAuth tokens, GitHub App tokens | +| `github-cli` | `Authorization: Bearer ` | Token acquired on demand from `gh auth token` | **Example — PAT via environment variable:** @@ -71,6 +72,30 @@ Either `token` or `token_env` must be set for the `bearer`, `basic-pat`, and `ba } ``` +**Example — GitHub CLI (recommended for interactive use):** + +```json +{ + "providers": [ + { + "hosts": [ + "github.com", + "api.github.com", + "raw.githubusercontent.com", + "codeload.github.com" + ], + "provider": "github", + "auth": "github-cli" + } + ] +} +``` + +Install [GitHub CLI](https://cli.github.com/) and run `gh auth login` before +using this mode. Specify invokes the fixed command `gh auth token` directly, +without a shell, and caches the resulting token in memory for the current +process. The token is not written to disk by Specify. + ### GitHub Enterprise Server (GHES) To use a private catalog or extension hosted on a GitHub Enterprise Server @@ -98,6 +123,18 @@ and `codeload.` subdomains your catalog/extension URLs use. A `*.ghes.example.com` wildcard matches subdomains but **not** the bare host, so always include the bare host explicitly. +With `auth: "github-cli"`, Specify requests the credential for the exact +enterprise request host by running: + +```bash +gh auth token --hostname ghes.example.com +``` + +The host must already be configured in GitHub CLI. If it is not configured, +or GitHub CLI is not authenticated for it, Specify fails the request and +suggests `gh auth login --hostname ghes.example.com`. GitHub CLI command output +is not included in the error. + ### GitHub Enterprise Cloud with data residency (GHE.com) GHE.com tenants use separate web and REST API hostnames. List both hosts so diff --git a/src/specify_cli/authentication/azure_devops.py b/src/specify_cli/authentication/azure_devops.py index 91578bd418..904a1310c2 100644 --- a/src/specify_cli/authentication/azure_devops.py +++ b/src/specify_cli/authentication/azure_devops.py @@ -58,13 +58,17 @@ def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: f"AzureDevOpsAuth does not support auth scheme {auth_scheme!r}" ) - def resolve_token(self, entry: AuthConfigEntry) -> str | None: + def resolve_token( + self, + entry: AuthConfigEntry, + hostname: str | None = None, + ) -> str | None: """Resolve token, with special handling for azure-cli and azure-ad.""" if entry.auth == "azure-cli": return self._acquire_via_az_cli() if entry.auth == "azure-ad": return self._acquire_via_client_credentials(entry) - return super().resolve_token(entry) + return super().resolve_token(entry, hostname) # -- Token acquisition ------------------------------------------------ diff --git a/src/specify_cli/authentication/base.py b/src/specify_cli/authentication/base.py index d6e0f4b118..a34d266100 100644 --- a/src/specify_cli/authentication/base.py +++ b/src/specify_cli/authentication/base.py @@ -20,7 +20,7 @@ class AuthProvider(ABC): And implement: * ``auth_headers(token, auth_scheme)`` — build headers from a resolved token - * ``resolve_token(entry)`` — obtain the token for a config entry + * ``resolve_token(entry, hostname)`` — obtain the token for a config entry """ key: str = "" @@ -36,13 +36,20 @@ def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: Must return a dict with at least an ``Authorization`` key. """ - def resolve_token(self, entry: AuthConfigEntry) -> str | None: + def resolve_token( + self, + entry: AuthConfigEntry, + hostname: str | None = None, + ) -> str | None: """Resolve the token for *entry*. Default implementation reads from ``entry.token`` directly or from the environment variable named by ``entry.token_env``. Override for schemes that acquire tokens dynamically (e.g. ``azure-cli``, ``azure-ad``). + + ``hostname`` identifies the request target for providers whose + credential helper selects credentials by host. """ import os diff --git a/src/specify_cli/authentication/bitbucket.py b/src/specify_cli/authentication/bitbucket.py index 24474e6b5b..c561535a6a 100644 --- a/src/specify_cli/authentication/bitbucket.py +++ b/src/specify_cli/authentication/bitbucket.py @@ -59,7 +59,11 @@ def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: f"BitbucketAuth does not support auth scheme {auth_scheme!r}" ) - def resolve_token(self, entry: AuthConfigEntry) -> str | None: + def resolve_token( + self, + entry: AuthConfigEntry, + hostname: str | None = None, + ) -> str | None: """Resolve the credential, combining ``username`` for ``basic``. Returns ``None`` when the secret is missing, or — for ``basic`` — @@ -69,7 +73,7 @@ def resolve_token(self, entry: AuthConfigEntry) -> str | None: ``:`` credential or a ``user:name:`` one that the server would parse as user ``user`` (RFC 7617 §2). """ - secret = super().resolve_token(entry) + secret = super().resolve_token(entry, hostname) if entry.auth != "basic": return secret username = (entry.username or "").strip() diff --git a/src/specify_cli/authentication/config.py b/src/specify_cli/authentication/config.py index 296c5c4d39..2f5d6a72d2 100644 --- a/src/specify_cli/authentication/config.py +++ b/src/specify_cli/authentication/config.py @@ -220,7 +220,7 @@ def load_auth_config( raise ValueError( f"providers[{i}]: '{field_name}' must be a non-empty string" ) - # azure-cli needs no extra fields + # azure-cli and github-cli need no extra fields entries.append( AuthConfigEntry( diff --git a/src/specify_cli/authentication/github.py b/src/specify_cli/authentication/github.py index 3797d82926..b2cc1da431 100644 --- a/src/specify_cli/authentication/github.py +++ b/src/specify_cli/authentication/github.py @@ -2,23 +2,127 @@ from __future__ import annotations +import os +import shutil +import subprocess +from typing import TYPE_CHECKING + from .base import AuthProvider +if TYPE_CHECKING: + from .config import AuthConfigEntry + + +_GITHUB_CLI_TIMEOUT_SECONDS = 10 +_GITHUB_PUBLIC_HOSTS = frozenset({ + "github.com", + "api.github.com", + "raw.githubusercontent.com", + "codeload.github.com", +}) + + +class GitHubCliAuthError(RuntimeError): + """Raised when GitHub CLI cannot provide a configured credential.""" + class GitHubAuth(AuthProvider): """GitHub authentication provider. - Supports the ``bearer`` auth scheme, used for PATs, fine-grained PATs, - OAuth tokens, and GitHub App installation tokens. + Supports ``bearer`` for PATs, OAuth tokens, and GitHub App installation + tokens, plus ``github-cli`` for credentials managed by GitHub CLI. """ key = "github" - supported_auth_schemes = ("bearer",) + supported_auth_schemes = ("bearer", "github-cli") + + def __init__(self) -> None: + self._github_cli_cache: dict[str, tuple[bool, str]] = {} def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: """Return ``Authorization: Bearer ``.""" - if auth_scheme != "bearer": + if auth_scheme not in self.supported_auth_schemes: raise ValueError( f"GitHubAuth does not support auth scheme {auth_scheme!r}" ) return {"Authorization": f"Bearer {token}"} + + def resolve_token( + self, + entry: AuthConfigEntry, + hostname: str | None = None, + ) -> str | None: + """Resolve configured tokens or acquire one from GitHub CLI.""" + if entry.auth != "github-cli": + return super().resolve_token(entry, hostname) + + request_host = (hostname or entry.hosts[0]).lower() + credential_host = ( + "github.com" if request_host in _GITHUB_PUBLIC_HOSTS else request_host + ) + cached = self._github_cli_cache.get(credential_host) + if cached is not None: + succeeded, value = cached + if succeeded: + return value + raise GitHubCliAuthError(value) + + try: + token = self._acquire_via_gh_cli(credential_host) + except GitHubCliAuthError as exc: + message = str(exc) + self._github_cli_cache[credential_host] = (False, message) + raise GitHubCliAuthError(message) from None + + self._github_cli_cache[credential_host] = (True, token) + return token + + @staticmethod + def _acquire_via_gh_cli(hostname: str) -> str: + """Run the fixed GitHub CLI token command for *hostname*.""" + resolved = shutil.which("gh") + if not resolved or not os.path.isabs(resolved): + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + "GitHub CLI ('gh') was not found." + ) + + argv = [resolved, "auth", "token"] + if hostname != "github.com": + argv.extend(["--hostname", hostname]) + + try: + result = subprocess.run( # noqa: S603 + argv, + capture_output=True, + text=True, + timeout=_GITHUB_CLI_TIMEOUT_SECONDS, + check=False, + shell=False, + ) + except subprocess.TimeoutExpired: + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + f"GitHub CLI timed out while requesting a token for {hostname!r}." + ) from None + except (OSError, UnicodeDecodeError): + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + f"GitHub CLI could not provide a token for {hostname!r}." + ) from None + + if result.returncode != 0: + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + f"GitHub CLI could not provide a token for {hostname!r}. " + f"Run 'gh auth login --hostname {hostname}'." + ) + + token = result.stdout.strip() + if not token: + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + f"GitHub CLI returned an empty token for {hostname!r}. " + f"Run 'gh auth login --hostname {hostname}'." + ) + return token diff --git a/src/specify_cli/authentication/http.py b/src/specify_cli/authentication/http.py index 7dabc752aa..6e63382357 100644 --- a/src/specify_cli/authentication/http.py +++ b/src/specify_cli/authentication/http.py @@ -69,6 +69,14 @@ class RedirectPolicyError(urllib.error.URLError): """A redirect rejected because it violates the client's security policy.""" +def _hostname_for_url(url: str) -> str | None: + """Return a normalized request hostname, or None for malformed URLs.""" + try: + return (urlparse(url).hostname or "").lower() or None + except ValueError: + return None + + def _validate_strict_redirect(old_url: str, new_url: str) -> None: if not is_safe_download_redirect(old_url, new_url): raise RedirectPolicyError( @@ -140,11 +148,12 @@ def build_request(url: str, extra_headers: dict[str, str] | None = None) -> urll headers.update({k: v for k, v in extra_headers.items() if k.lower() != "authorization"}) # Auth headers applied last — cannot be overridden by extra_headers entries = find_entries_for_url(url, _load_config()) + hostname = _hostname_for_url(url) for entry in entries: provider = get_provider(entry.provider) if provider is None: continue - token = provider.resolve_token(entry) + token = provider.resolve_token(entry, hostname) if token: headers.update(provider.auth_headers(token, entry.auth)) break @@ -190,6 +199,7 @@ def open_url( HTTP between loopback URLs, and rejects remote-to-local redirects. """ entries = find_entries_for_url(url, _load_config()) + hostname = _hostname_for_url(url) def _make_req(auth_headers: dict[str, str]) -> urllib.request.Request: merged = {} @@ -205,7 +215,7 @@ def _make_req(auth_headers: dict[str, str]) -> urllib.request.Request: provider = get_provider(entry.provider) if provider is None: continue - token = provider.resolve_token(entry) + token = provider.resolve_token(entry, hostname) if not token: continue diff --git a/tests/specify_cli/authentication/test_authentication.py b/tests/specify_cli/authentication/test_authentication.py index 45c0a4b833..e9069b47b9 100644 --- a/tests/specify_cli/authentication/test_authentication.py +++ b/tests/specify_cli/authentication/test_authentication.py @@ -30,7 +30,7 @@ find_entries_for_url, load_auth_config, ) -from specify_cli.authentication.github import GitHubAuth +from specify_cli.authentication.github import GitHubAuth, GitHubCliAuthError # --------------------------------------------------------------------------- @@ -49,6 +49,17 @@ def _github_entry(token_env: str = "GH_TOKEN", token: str | None = None) -> Auth ) +def _github_cli_entry( + hosts: tuple[str, ...] = ("github.com",), +) -> AuthConfigEntry: + """Build a GitHub CLI config entry.""" + return AuthConfigEntry( + hosts=hosts, + provider="github", + auth="github-cli", + ) + + def _ado_basic_entry(token_env: str = "AZURE_DEVOPS_PAT") -> AuthConfigEntry: """Build an ADO basic-pat config entry.""" return AuthConfigEntry( @@ -293,6 +304,18 @@ def test_azure_cli_config(self, tmp_path): entries = load_auth_config(cfg) assert entries[0].auth == "azure-cli" + def test_github_cli_config(self, tmp_path): + cfg = tmp_path / "auth.json" + cfg.write_text(json.dumps({ + "providers": [{ + "hosts": ["github.com"], + "provider": "github", + "auth": "github-cli", + }] + })) + entries = load_auth_config(cfg) + assert entries == [_github_cli_entry()] + def test_multiple_entries(self, tmp_path): cfg = tmp_path / "auth.json" cfg.write_text(json.dumps({ @@ -616,7 +639,201 @@ def test_key(self): assert GitHubAuth.key == "github" def test_supported_schemes(self): - assert GitHubAuth.supported_auth_schemes == ("bearer",) + assert GitHubAuth.supported_auth_schemes == ("bearer", "github-cli") + + def test_github_cli_headers_use_bearer(self): + assert GitHubAuth().auth_headers("cli-token", "github-cli") == { + "Authorization": "Bearer cli-token" + } + + def test_resolve_token_github_cli_success_without_shell(self): + from unittest.mock import MagicMock, patch + + provider = GitHubAuth() + result = MagicMock(returncode=0, stdout=" cli-token\n") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ) as run: + assert provider.resolve_token( + _github_cli_entry(), + "github.com", + ) == "cli-token" + + assert run.call_args.args[0] == [executable, "auth", "token"] + assert run.call_args.kwargs == { + "capture_output": True, + "text": True, + "timeout": 10, + "check": False, + "shell": False, + } + + def test_resolve_token_github_cli_uses_enterprise_hostname(self): + from unittest.mock import MagicMock, patch + + provider = GitHubAuth() + result = MagicMock(returncode=0, stdout="enterprise-token") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ) as run: + assert provider.resolve_token( + _github_cli_entry(("ghes.example.com",)), + "ghes.example.com", + ) == "enterprise-token" + + assert run.call_args.args[0] == [ + executable, + "auth", + "token", + "--hostname", + "ghes.example.com", + ] + + def test_resolve_token_github_cli_caches_per_credential_host(self): + from unittest.mock import MagicMock, patch + + provider = GitHubAuth() + result = MagicMock(returncode=0, stdout="cached-token") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + entry = _github_cli_entry(( + "github.com", + "api.github.com", + "ghes.example.com", + )) + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ) as run: + assert provider.resolve_token(entry, "github.com") == "cached-token" + assert provider.resolve_token(entry, "api.github.com") == "cached-token" + assert provider.resolve_token(entry, "ghes.example.com") == "cached-token" + assert provider.resolve_token(entry, "ghes.example.com") == "cached-token" + + assert run.call_count == 2 + + def test_resolve_token_github_cli_missing_raises_clear_error(self): + from unittest.mock import patch + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=None, + ), pytest.raises(GitHubCliAuthError) as exc_info: + GitHubAuth().resolve_token(_github_cli_entry(), "github.com") + + message = str(exc_info.value) + assert "provider 'github'" in message + assert "GitHub CLI ('gh') was not found" in message + + def test_resolve_token_github_cli_unauthenticated_suggests_login(self): + from unittest.mock import MagicMock, patch + + result = MagicMock( + returncode=1, + stdout="", + stderr="not logged into any GitHub hosts", + ) + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ), pytest.raises(GitHubCliAuthError, match="gh auth login --hostname github.com"): + GitHubAuth().resolve_token(_github_cli_entry(), "github.com") + + def test_resolve_token_github_cli_failure_redacts_command_output( + self, + caplog, + ): + from unittest.mock import MagicMock, patch + + secret = "ghp_must_not_leak" + result = MagicMock(returncode=2, stdout=secret, stderr=secret) + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ), pytest.raises(GitHubCliAuthError) as exc_info: + GitHubAuth().resolve_token(_github_cli_entry(), "github.com") + + assert secret not in str(exc_info.value) + assert secret not in caplog.text + + def test_resolve_token_github_cli_empty_output_raises(self): + from unittest.mock import MagicMock, patch + + result = MagicMock(returncode=0, stdout=" \n", stderr="") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ), pytest.raises(GitHubCliAuthError, match="returned an empty token"): + GitHubAuth().resolve_token(_github_cli_entry(), "github.com") + + def test_resolve_token_github_cli_timeout_raises_without_output(self): + import subprocess + from unittest.mock import patch + + secret = b"ghp_timeout_secret" + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + timeout = subprocess.TimeoutExpired( + cmd=[executable, "auth", "token"], + timeout=10, + output=secret, + stderr=secret, + ) + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + side_effect=timeout, + ), pytest.raises(GitHubCliAuthError) as exc_info: + GitHubAuth().resolve_token(_github_cli_entry(), "github.com") + + assert "timed out" in str(exc_info.value) + assert secret.decode() not in str(exc_info.value) + + def test_resolve_token_github_cli_caches_failure(self): + from unittest.mock import patch + + provider = GitHubAuth() + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=None, + ) as which: + for _ in range(2): + with pytest.raises(GitHubCliAuthError): + provider.resolve_token(_github_cli_entry(), "github.com") + + assert which.call_count == 1 # --------------------------------------------------------------------------- @@ -1070,6 +1287,30 @@ def test_build_request_attaches_auth_for_matching_host(self, monkeypatch): req = build_request("https://github.com/org/repo") assert req.get_header("Authorization") == "Bearer my-token" + def test_build_request_resolves_github_cli_for_request_host(self, monkeypatch): + from unittest.mock import MagicMock, patch + + from specify_cli.authentication.http import build_request + + provider = get_provider("github") + assert isinstance(provider, GitHubAuth) + monkeypatch.setattr(provider, "_github_cli_cache", {}) + self._set_config(monkeypatch, [_github_cli_entry(("ghes.example.com",))]) + result = MagicMock(returncode=0, stdout="enterprise-token") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ) as run: + req = build_request("https://ghes.example.com/org/repo") + + assert req.get_header("Authorization") == "Bearer enterprise-token" + assert run.call_args.args[0][-2:] == ["--hostname", "ghes.example.com"] + def test_build_request_attaches_bitbucket_basic_auth(self, monkeypatch): from specify_cli.authentication.http import build_request monkeypatch.setenv("ATLASSIAN_API_TOKEN", "api-tok") From feb98210f31b51401c5fbddced6bccb42d26eebc Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 12:22:08 -0500 Subject: [PATCH 2/7] fix(auth): address GitHub CLI host handling Pass explicit GitHub CLI hostnames, map configured enterprise service hosts to their instance login, and route sanitized credential failures through existing URL error handling. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 434e2508-0633-4d40-8e0d-5edb92cbc177 --- docs/reference/authentication.md | 22 ++-- src/specify_cli/authentication/github.py | 34 ++++-- .../authentication/test_authentication.py | 102 +++++++++++++++++- tests/test_extensions.py | 63 +++++++++++ 4 files changed, 202 insertions(+), 19 deletions(-) diff --git a/docs/reference/authentication.md b/docs/reference/authentication.md index be769af819..6d3887d687 100644 --- a/docs/reference/authentication.md +++ b/docs/reference/authentication.md @@ -92,9 +92,10 @@ Either `token` or `token_env` must be set for the `bearer`, `basic-pat`, and `ba ``` Install [GitHub CLI](https://cli.github.com/) and run `gh auth login` before -using this mode. Specify invokes the fixed command `gh auth token` directly, -without a shell, and caches the resulting token in memory for the current -process. The token is not written to disk by Specify. +using this mode. Specify invokes the fixed command +`gh auth token --hostname github.com` directly, without a shell, and caches +the resulting token in memory for the current process. The token is not +written to disk by Specify. ### GitHub Enterprise Server (GHES) @@ -123,17 +124,20 @@ and `codeload.` subdomains your catalog/extension URLs use. A `*.ghes.example.com` wildcard matches subdomains but **not** the bare host, so always include the bare host explicitly. -With `auth: "github-cli"`, Specify requests the credential for the exact -enterprise request host by running: +With `auth: "github-cli"`, Specify requests the credential for the configured +GitHub instance host: ```bash gh auth token --hostname ghes.example.com ``` -The host must already be configured in GitHub CLI. If it is not configured, -or GitHub CLI is not authenticated for it, Specify fails the request and -suggests `gh auth login --hostname ghes.example.com`. GitHub CLI command output -is not included in the error. +When a request targets an `api.`, `raw.`, or `codeload.` host, Specify uses the +bare instance host only when that exact host is also listed in the same +provider entry. For example, `raw.ghes.example.com` maps to +`ghes.example.com` in the configuration above. Otherwise, the request host is +used unchanged. The selected host must already be configured and authenticated +in GitHub CLI; failures suggest `gh auth login --hostname ` without +including GitHub CLI command output in the error. ### GitHub Enterprise Cloud with data residency (GHE.com) diff --git a/src/specify_cli/authentication/github.py b/src/specify_cli/authentication/github.py index b2cc1da431..b9e1599e5d 100644 --- a/src/specify_cli/authentication/github.py +++ b/src/specify_cli/authentication/github.py @@ -5,6 +5,7 @@ import os import shutil import subprocess +import urllib.error from typing import TYPE_CHECKING from .base import AuthProvider @@ -20,11 +21,16 @@ "raw.githubusercontent.com", "codeload.github.com", }) +_GITHUB_ENTERPRISE_SERVICE_PREFIXES = ("api.", "raw.", "codeload.") -class GitHubCliAuthError(RuntimeError): +class GitHubCliAuthError(urllib.error.URLError): """Raised when GitHub CLI cannot provide a configured credential.""" + def __str__(self) -> str: + """Return the sanitized reason without URLError's wrapper.""" + return str(self.reason) + class GitHubAuth(AuthProvider): """GitHub authentication provider. @@ -57,9 +63,7 @@ def resolve_token( return super().resolve_token(entry, hostname) request_host = (hostname or entry.hosts[0]).lower() - credential_host = ( - "github.com" if request_host in _GITHUB_PUBLIC_HOSTS else request_host - ) + credential_host = self._credential_hostname(entry, request_host) cached = self._github_cli_cache.get(credential_host) if cached is not None: succeeded, value = cached @@ -77,6 +81,24 @@ def resolve_token( self._github_cli_cache[credential_host] = (True, token) return token + @staticmethod + def _credential_hostname(entry: AuthConfigEntry, request_host: str) -> str: + """Map a request host to the GitHub CLI login host.""" + if request_host in _GITHUB_PUBLIC_HOSTS: + return "github.com" + + exact_hosts = { + configured_host.lower() + for configured_host in entry.hosts + if "*" not in configured_host + } + for prefix in _GITHUB_ENTERPRISE_SERVICE_PREFIXES: + if request_host.startswith(prefix): + instance_host = request_host.removeprefix(prefix) + if instance_host in exact_hosts: + return instance_host + return request_host + @staticmethod def _acquire_via_gh_cli(hostname: str) -> str: """Run the fixed GitHub CLI token command for *hostname*.""" @@ -87,9 +109,7 @@ def _acquire_via_gh_cli(hostname: str) -> str: "GitHub CLI ('gh') was not found." ) - argv = [resolved, "auth", "token"] - if hostname != "github.com": - argv.extend(["--hostname", hostname]) + argv = [resolved, "auth", "token", "--hostname", hostname] try: result = subprocess.run( # noqa: S603 diff --git a/tests/specify_cli/authentication/test_authentication.py b/tests/specify_cli/authentication/test_authentication.py index e9069b47b9..63f26d82e4 100644 --- a/tests/specify_cli/authentication/test_authentication.py +++ b/tests/specify_cli/authentication/test_authentication.py @@ -18,6 +18,7 @@ import io import json import os +import urllib.error import pytest @@ -646,9 +647,13 @@ def test_github_cli_headers_use_bearer(self): "Authorization": "Bearer cli-token" } - def test_resolve_token_github_cli_success_without_shell(self): + def test_resolve_token_github_cli_success_without_shell( + self, + monkeypatch, + ): from unittest.mock import MagicMock, patch + monkeypatch.setenv("GH_HOST", "enterprise.example.com") provider = GitHubAuth() result = MagicMock(returncode=0, stdout=" cli-token\n") executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") @@ -665,7 +670,13 @@ def test_resolve_token_github_cli_success_without_shell(self): "github.com", ) == "cli-token" - assert run.call_args.args[0] == [executable, "auth", "token"] + assert run.call_args.args[0] == [ + executable, + "auth", + "token", + "--hostname", + "github.com", + ] assert run.call_args.kwargs == { "capture_output": True, "text": True, @@ -701,6 +712,66 @@ def test_resolve_token_github_cli_uses_enterprise_hostname(self): "ghes.example.com", ] + @pytest.mark.parametrize( + "request_host", + [ + "api.ghes.example.com", + "raw.ghes.example.com", + "codeload.ghes.example.com", + ], + ) + def test_resolve_token_github_cli_maps_enterprise_service_host( + self, + request_host, + ): + from unittest.mock import MagicMock, patch + + provider = GitHubAuth() + result = MagicMock(returncode=0, stdout="enterprise-token") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + entry = _github_cli_entry(( + "ghes.example.com", + "api.ghes.example.com", + "raw.ghes.example.com", + "codeload.ghes.example.com", + )) + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ) as run: + assert provider.resolve_token(entry, request_host) == "enterprise-token" + + assert run.call_args.args[0][-2:] == ["--hostname", "ghes.example.com"] + + def test_resolve_token_github_cli_does_not_infer_unconfigured_instance_host(self): + from unittest.mock import MagicMock, patch + + provider = GitHubAuth() + result = MagicMock(returncode=0, stdout="enterprise-token") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + entry = _github_cli_entry(("raw.ghes.example.com",)) + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ) as run: + assert provider.resolve_token( + entry, + "raw.ghes.example.com", + ) == "enterprise-token" + + assert run.call_args.args[0][-2:] == [ + "--hostname", + "raw.ghes.example.com", + ] + def test_resolve_token_github_cli_caches_per_credential_host(self): from unittest.mock import MagicMock, patch @@ -711,6 +782,7 @@ def test_resolve_token_github_cli_caches_per_credential_host(self): "github.com", "api.github.com", "ghes.example.com", + "raw.ghes.example.com", )) with patch( @@ -723,7 +795,7 @@ def test_resolve_token_github_cli_caches_per_credential_host(self): assert provider.resolve_token(entry, "github.com") == "cached-token" assert provider.resolve_token(entry, "api.github.com") == "cached-token" assert provider.resolve_token(entry, "ghes.example.com") == "cached-token" - assert provider.resolve_token(entry, "ghes.example.com") == "cached-token" + assert provider.resolve_token(entry, "raw.ghes.example.com") == "cached-token" assert run.call_count == 2 @@ -737,6 +809,8 @@ def test_resolve_token_github_cli_missing_raises_clear_error(self): GitHubAuth().resolve_token(_github_cli_entry(), "github.com") message = str(exc_info.value) + assert isinstance(exc_info.value, urllib.error.URLError) + assert not message.startswith(" Date: Wed, 14 Oct 2026 12:45:39 -0500 Subject: [PATCH 3/7] fix(auth): preserve GitHub CLI diagnostics Keep sanitized GitHub CLI credential failures distinct from offline errors in release checks and upgrade output, prove no HTTP fallback occurs, and document terminal failure caching. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 434e2508-0633-4d40-8e0d-5edb92cbc177 --- docs/reference/authentication.md | 8 ++++++++ src/specify_cli/_version.py | 7 +++++++ .../authentication/test_authentication.py | 15 ++++++++++++--- tests/specify_cli/test_version_verification.py | 18 ++++++++++++++++++ 4 files changed, 45 insertions(+), 3 deletions(-) diff --git a/docs/reference/authentication.md b/docs/reference/authentication.md index 6d3887d687..33fd7871db 100644 --- a/docs/reference/authentication.md +++ b/docs/reference/authentication.md @@ -97,6 +97,14 @@ using this mode. Specify invokes the fixed command the resulting token in memory for the current process. The token is not written to disk by Specify. +A configured `github-cli` entry is authoritative for its matching hosts. If +GitHub CLI cannot provide a token, Specify fails that credential lookup before +making an HTTP request; it does not try later matching entries or an +unauthenticated fallback for that request. Successful tokens and sanitized +lookup failures are both cached per selected credential host for the current +process. After correcting a GitHub CLI login, start a new Specify invocation +to retry the lookup. + ### GitHub Enterprise Server (GHES) To use a private catalog or extension hosted on a GitHub Enterprise Server diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index 93b4c6a618..afdf71b8de 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -37,6 +37,9 @@ "rate limited (configure ~/.specify/auth.json with a GitHub token)" ) _RESOLUTION_FAILURE_HTTP_PREFIX = "HTTP " +_RESOLUTION_FAILURE_GITHUB_AUTH_PREFIX = ( + "GitHub authentication failed for provider 'github': " +) _FAILURE_INSTALLER_MISSING = "installer-missing" _FAILURE_INSTALLER_INVALID = "installer-invalid" _FAILURE_TARGET_TAG_UNPARSEABLE = "target-tag-unparseable" @@ -112,6 +115,7 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: On anything else — including a malformed response body — the exception propagates; there is no catch-all (research D-006). """ + from .authentication.github import GitHubCliAuthError from .authentication.http import open_url try: @@ -139,6 +143,8 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: if e.code in (403, 429): return None, _RESOLUTION_FAILURE_RATE_LIMITED return None, f"{_RESOLUTION_FAILURE_HTTP_PREFIX}{e.code}" + except GitHubCliAuthError as e: + return None, str(e) except (urllib.error.URLError, OSError): return None, _RESOLUTION_FAILURE_OFFLINE @@ -1022,6 +1028,7 @@ def _emit_failure( if ( category in _RESOLUTION_FAILURE_CATEGORIES or category.startswith(_RESOLUTION_FAILURE_HTTP_PREFIX) + or category.startswith(_RESOLUTION_FAILURE_GITHUB_AUTH_PREFIX) ): console.print(f"Upgrade aborted: {category}", soft_wrap=True) return diff --git a/tests/specify_cli/authentication/test_authentication.py b/tests/specify_cli/authentication/test_authentication.py index 63f26d82e4..4a59b5f0e5 100644 --- a/tests/specify_cli/authentication/test_authentication.py +++ b/tests/specify_cli/authentication/test_authentication.py @@ -1984,7 +1984,10 @@ def test_accept_header_present(self, monkeypatch): _fetch_latest_release_tag() assert captured["request"].get_header("Accept") == "application/vnd.github+json" - def test_github_cli_failure_maps_to_offline(self, monkeypatch): + def test_github_cli_failure_preserves_message_without_http( + self, + monkeypatch, + ): from unittest.mock import patch from specify_cli._version import _fetch_latest_release_tag @@ -2000,11 +2003,17 @@ def test_github_cli_failure_maps_to_offline(self, monkeypatch): with patch( "specify_cli.authentication.github.shutil.which", return_value=None, - ): + ), patch( + "specify_cli.authentication.http.urllib.request.build_opener", + ) as build_opener: tag, reason = _fetch_latest_release_tag() assert tag is None - assert reason == "offline or timeout" + assert reason == ( + "GitHub authentication failed for provider 'github': " + "GitHub CLI ('gh') was not found." + ) + build_opener.assert_not_called() # --------------------------------------------------------------------------- diff --git a/tests/specify_cli/test_version_verification.py b/tests/specify_cli/test_version_verification.py index f3311bb23f..062fd82201 100644 --- a/tests/specify_cli/test_version_verification.py +++ b/tests/specify_cli/test_version_verification.py @@ -285,6 +285,24 @@ def test_offline_exits_1_with_phase1_string(self, uv_tool_argv0, clean_environ): assert result.exit_code == 1 assert "Upgrade aborted: offline or timeout" in strip_ansi(result.output) + def test_github_cli_auth_failure_preserves_actionable_message( + self, + uv_tool_argv0, + clean_environ, + ): + failure = ( + "GitHub authentication failed for provider 'github': " + "GitHub CLI ('gh') was not found." + ) + with patch( + "specify_cli._version._fetch_latest_release_tag", + return_value=(None, failure), + ): + result = runner.invoke(app, ["self", "upgrade"]) + + assert result.exit_code == 1 + assert f"Upgrade aborted: {failure}" in strip_ansi(result.output) + def test_rate_limited_exits_1(self, uv_tool_argv0, clean_environ): err = urllib.error.HTTPError( url="https://api.github.com", From e07aa6e68d8e744a8583a969063c804e3234202e Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:28:35 -0500 Subject: [PATCH 4/7] fix(auth): scope credentials across redirects Prevent GitHub CLI tokens from following redirects to a different GitHub credential host while preserving authorization across public and configured enterprise aliases. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 434e2508-0633-4d40-8e0d-5edb92cbc177 --- docs/reference/authentication.md | 4 +- src/specify_cli/authentication/base.py | 14 +++ src/specify_cli/authentication/github.py | 16 +++ src/specify_cli/authentication/http.py | 28 ++++- .../authentication/test_authentication.py | 117 ++++++++++++++++++ 5 files changed, 176 insertions(+), 3 deletions(-) diff --git a/docs/reference/authentication.md b/docs/reference/authentication.md index 33fd7871db..045db6e012 100644 --- a/docs/reference/authentication.md +++ b/docs/reference/authentication.md @@ -319,7 +319,9 @@ You can configure multiple entries for different hosts or organizations: is attempted as a final fallback. 5. On redirects, the `Authorization` header is stripped if the redirect target leaves the entry's declared hosts — preventing credential - leakage to CDNs or third-party services. + leakage to CDNs or third-party services. For `github-cli`, the header is + also stripped when the target maps to a different GitHub CLI credential + host, even if both hosts appear in the same provider entry. ## Template diff --git a/src/specify_cli/authentication/base.py b/src/specify_cli/authentication/base.py index a34d266100..e83fdfce83 100644 --- a/src/specify_cli/authentication/base.py +++ b/src/specify_cli/authentication/base.py @@ -62,3 +62,17 @@ def resolve_token( if val: return val return None + + def can_reuse_token_for_host( + self, + entry: AuthConfigEntry, + source_hostname: str | None, + target_hostname: str, + ) -> bool: + """Return whether a resolved token may follow a redirect. + + Static credentials are scoped by the entry's declared hosts, so the + default allows reuse within those hosts. Providers with host-selected + credentials can narrow that scope. + """ + return True diff --git a/src/specify_cli/authentication/github.py b/src/specify_cli/authentication/github.py index b9e1599e5d..40f60df00d 100644 --- a/src/specify_cli/authentication/github.py +++ b/src/specify_cli/authentication/github.py @@ -99,6 +99,22 @@ def _credential_hostname(entry: AuthConfigEntry, request_host: str) -> str: return instance_host return request_host + def can_reuse_token_for_host( + self, + entry: AuthConfigEntry, + source_hostname: str | None, + target_hostname: str, + ) -> bool: + """Allow redirects only within one GitHub CLI credential scope.""" + if entry.auth != "github-cli": + return True + if source_hostname is None: + return False + return self._credential_hostname( + entry, + source_hostname, + ) == self._credential_hostname(entry, target_hostname) + @staticmethod def _acquire_via_gh_cli(hostname: str) -> str: """Run the fixed GitHub CLI token command for *hostname*.""" diff --git a/src/specify_cli/authentication/http.py b/src/specify_cli/authentication/http.py index 6e63382357..84f1700b60 100644 --- a/src/specify_cli/authentication/http.py +++ b/src/specify_cli/authentication/http.py @@ -63,6 +63,7 @@ def _hostname_in_hosts(hostname: str, hosts: tuple[str, ...]) -> bool: RedirectValidator = Callable[[str, str], None] +AuthHostnameValidator = Callable[[str], bool] class RedirectPolicyError(urllib.error.URLError): @@ -99,10 +100,12 @@ def __init__( self, hosts: tuple[str, ...], redirect_validator: RedirectValidator | None = None, + auth_hostname_validator: AuthHostnameValidator | None = None, ) -> None: super().__init__() self._hosts = hosts self._redirect_validator = redirect_validator + self._auth_hostname_validator = auth_hostname_validator def redirect_request(self, req, fp, code, msg, headers, newurl): try: @@ -127,7 +130,15 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): old_scheme = urlparse(req.full_url).scheme hostname = (new_parsed.hostname or "").lower() is_https_downgrade = old_scheme == "https" and new_parsed.scheme != "https" - if _hostname_in_hosts(hostname, self._hosts) and not is_https_downgrade: + is_auth_hostname_allowed = ( + self._auth_hostname_validator is None + or self._auth_hostname_validator(hostname) + ) + if ( + _hostname_in_hosts(hostname, self._hosts) + and not is_https_downgrade + and is_auth_hostname_allowed + ): if original_auth: new_req.add_unredirected_header("Authorization", original_auth) else: @@ -220,7 +231,20 @@ def _make_req(auth_headers: dict[str, str]) -> urllib.request.Request: continue req = _make_req(provider.auth_headers(token, entry.auth)) - opener = urllib.request.build_opener(_StripAuthOnRedirect(entry.hosts, redirect_validator)) + def auth_hostname_validator(target_hostname: str) -> bool: + return provider.can_reuse_token_for_host( + entry, + hostname, + target_hostname, + ) + + opener = urllib.request.build_opener( + _StripAuthOnRedirect( + entry.hosts, + redirect_validator, + auth_hostname_validator, + ) + ) try: return opener.open(req, timeout=timeout) except urllib.error.HTTPError as exc: diff --git a/tests/specify_cli/authentication/test_authentication.py b/tests/specify_cli/authentication/test_authentication.py index 4a59b5f0e5..775d4e0c9e 100644 --- a/tests/specify_cli/authentication/test_authentication.py +++ b/tests/specify_cli/authentication/test_authentication.py @@ -772,6 +772,54 @@ def test_resolve_token_github_cli_does_not_infer_unconfigured_instance_host(self "raw.ghes.example.com", ] + @pytest.mark.parametrize( + ("source", "target"), + [ + ("github.com", "api.github.com"), + ("github.com", "codeload.github.com"), + ("ghes.example.com", "api.ghes.example.com"), + ("ghes.example.com", "raw.ghes.example.com"), + ("ghes.example.com", "codeload.ghes.example.com"), + ], + ) + def test_github_cli_reuses_token_within_credential_host( + self, + source, + target, + ): + provider = GitHubAuth() + entry = _github_cli_entry(( + "github.com", + "api.github.com", + "codeload.github.com", + "ghes.example.com", + "api.ghes.example.com", + "raw.ghes.example.com", + "codeload.ghes.example.com", + )) + + assert provider.can_reuse_token_for_host(entry, source, target) + + def test_github_cli_does_not_reuse_token_across_instances(self): + provider = GitHubAuth() + entry = _github_cli_entry(( + "github.com", + "api.github.com", + "ghes.example.com", + "raw.ghes.example.com", + )) + + assert not provider.can_reuse_token_for_host( + entry, + "github.com", + "ghes.example.com", + ) + assert not provider.can_reuse_token_for_host( + entry, + "raw.ghes.example.com", + "api.github.com", + ) + def test_resolve_token_github_cli_caches_per_credential_host(self): from unittest.mock import MagicMock, patch @@ -1650,6 +1698,75 @@ def test_redirect_outside_hosts_strips_auth(self): assert new_req.headers.get("Authorization") is None assert new_req.unredirected_hdrs.get("Authorization") is None + def test_redirect_across_credential_scopes_strips_auth(self): + from specify_cli.authentication.http import _StripAuthOnRedirect + from urllib.request import Request + import io + + provider = GitHubAuth() + entry = _github_cli_entry(("github.com", "ghes.example.com")) + handler = _StripAuthOnRedirect( + entry.hosts, + auth_hostname_validator=lambda target: provider.can_reuse_token_for_host( + entry, + "github.com", + target, + ), + ) + req = Request( + "https://github.com/org/repo", + headers={"Authorization": "Bearer public-token"}, + ) + + new_req = handler.redirect_request( + req, + io.BytesIO(b""), + 302, + "Found", + {}, + "https://ghes.example.com/org/repo", + ) + + assert new_req is not None + assert new_req.get_header("Authorization") is None + assert new_req.unredirected_hdrs.get("Authorization") is None + + def test_redirect_within_credential_scope_preserves_auth(self): + from specify_cli.authentication.http import _StripAuthOnRedirect + from urllib.request import Request + import io + + provider = GitHubAuth() + entry = _github_cli_entry(("ghes.example.com", "raw.ghes.example.com")) + handler = _StripAuthOnRedirect( + entry.hosts, + auth_hostname_validator=lambda target: provider.can_reuse_token_for_host( + entry, + "ghes.example.com", + target, + ), + ) + req = Request( + "https://ghes.example.com/org/repo", + headers={"Authorization": "Bearer enterprise-token"}, + ) + + new_req = handler.redirect_request( + req, + io.BytesIO(b""), + 302, + "Found", + {}, + "https://raw.ghes.example.com/org/repo/file", + ) + + assert new_req is not None + auth = ( + new_req.get_header("Authorization") + or new_req.unredirected_hdrs.get("Authorization") + ) + assert auth == "Bearer enterprise-token" + @pytest.mark.parametrize( ("hosts", "target", "expected_auth"), [ From 5fafafdd05618b12dd0192534659a62862e78181 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 13:54:25 -0500 Subject: [PATCH 5/7] fix(auth): preserve bundle CLI diagnostics Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 434e2508-0633-4d40-8e0d-5edb92cbc177 --- src/specify_cli/bundles/adapters.py | 3 + src/specify_cli/bundles/component_catalog.py | 3 + .../authentication/test_authentication.py | 81 +++++++++++++++++++ tests/specify_cli/bundles/test_adapters.py | 35 ++++++++ tests/specify_cli/bundles/test_references.py | 30 +++++++ 5 files changed, 152 insertions(+) diff --git a/src/specify_cli/bundles/adapters.py b/src/specify_cli/bundles/adapters.py index 46433cb005..5dd40460ea 100644 --- a/src/specify_cli/bundles/adapters.py +++ b/src/specify_cli/bundles/adapters.py @@ -22,6 +22,7 @@ from .._assets import _locate_core_pack, _repo_root from .._download_security import MAX_JSON_CATALOG_BYTES, read_response_limited +from ..authentication.github import GitHubCliAuthError from . import BundlerError from .catalogs import CatalogSource from .manifest import ComponentRef @@ -276,6 +277,8 @@ def _validate_redirect(_old_url: str, new_url: str) -> None: raise BundlerError( f"Failed to fetch catalog from {url}: HTTP {exc.code} {exc.reason}" ) from exc + except GitHubCliAuthError as exc: + raise BundlerError(f"Failed to fetch catalog from {url}: {exc}") from exc except urllib.error.URLError as exc: # urllib wraps a TLS handshake failure as URLError(reason=); # a certificate-verification failure is a security failure, not a diff --git a/src/specify_cli/bundles/component_catalog.py b/src/specify_cli/bundles/component_catalog.py index d4b7a107c7..931463de00 100644 --- a/src/specify_cli/bundles/component_catalog.py +++ b/src/specify_cli/bundles/component_catalog.py @@ -6,6 +6,7 @@ from http.client import HTTPException from urllib.error import HTTPError, URLError +from ..authentication.github import GitHubCliAuthError from ..authentication.http import RedirectPolicyError from . import BundlerError from .manifest import ComponentRef @@ -19,6 +20,8 @@ def _is_unavailable(exc: BaseException) -> bool: seen: set[int] = set() while exc is not None and id(exc) not in seen: seen.add(id(exc)) + if isinstance(exc, GitHubCliAuthError): + return False if isinstance(exc, RedirectPolicyError): return False if isinstance(exc, HTTPError): diff --git a/tests/specify_cli/authentication/test_authentication.py b/tests/specify_cli/authentication/test_authentication.py index 775d4e0c9e..28b8d41c3f 100644 --- a/tests/specify_cli/authentication/test_authentication.py +++ b/tests/specify_cli/authentication/test_authentication.py @@ -862,6 +862,22 @@ def test_resolve_token_github_cli_missing_raises_clear_error(self): assert "provider 'github'" in message assert "GitHub CLI ('gh') was not found" in message + @pytest.mark.parametrize("executable", [r".\gh.exe", "gh.exe", "./gh"]) + def test_resolve_token_github_cli_rejects_relative_executable( + self, executable + ): + from unittest.mock import patch + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + ) as run, pytest.raises(GitHubCliAuthError, match="was not found"): + GitHubAuth().resolve_token(_github_cli_entry(), "github.com") + + run.assert_not_called() + def test_resolve_token_github_cli_unauthenticated_suggests_login(self): from unittest.mock import MagicMock, patch @@ -1482,6 +1498,71 @@ def fake_open(req, timeout=None): open_url("https://github.com/org/repo/catalog.json") assert captured["req"].get_header("Authorization") == "Bearer my-token" + def test_open_url_wires_github_cli_redirect_credential_scope(self, monkeypatch): + from unittest.mock import MagicMock, patch + + from specify_cli.authentication.http import ( + _StripAuthOnRedirect, + open_url, + ) + + provider = get_provider("github") + assert isinstance(provider, GitHubAuth) + monkeypatch.setattr(provider, "_github_cli_cache", {}) + self._set_config( + monkeypatch, + [ + _github_cli_entry( + ("github.com", "codeload.github.com", "ghes.example.com") + ) + ], + ) + result = MagicMock(returncode=0, stdout="github-token") + executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + response = MagicMock() + mock_opener = MagicMock() + mock_opener.open.return_value = response + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value=executable, + ), patch( + "specify_cli.authentication.github.subprocess.run", + return_value=result, + ), patch( + "specify_cli.authentication.http.urllib.request.build_opener", + return_value=mock_opener, + ) as build_opener: + assert open_url("https://github.com/org/repo/archive.zip") is response + + request = mock_opener.open.call_args.args[0] + handler = next( + argument + for argument in build_opener.call_args.args + if isinstance(argument, _StripAuthOnRedirect) + ) + same_instance = handler.redirect_request( + request, + io.BytesIO(b""), + 302, + "Found", + {}, + "https://codeload.github.com/org/repo/archive.zip", + ) + cross_instance = handler.redirect_request( + request, + io.BytesIO(b""), + 302, + "Found", + {}, + "https://ghes.example.com/org/repo/archive.zip", + ) + + assert same_instance is not None + assert same_instance.get_header("Authorization") == "Bearer github-token" + assert cross_instance is not None + assert cross_instance.get_header("Authorization") is None + def test_open_url_no_auth_for_non_matching_host(self, monkeypatch): from unittest.mock import MagicMock, patch from specify_cli.authentication.http import open_url diff --git a/tests/specify_cli/bundles/test_adapters.py b/tests/specify_cli/bundles/test_adapters.py index 58e05d9706..f4b739a7b7 100644 --- a/tests/specify_cli/bundles/test_adapters.py +++ b/tests/specify_cli/bundles/test_adapters.py @@ -13,6 +13,7 @@ import pytest +from specify_cli.authentication.github import GitHubCliAuthError from specify_cli.authentication.http import _StripAuthOnRedirect from specify_cli.bundler import BundlerError from specify_cli.bundles.catalogs import CatalogSource, InstallPolicy @@ -111,6 +112,40 @@ def fake_open_url(url, timeout=10, extra_headers=None, redirect_validator=None): adapters._http_get_json("team", "https://example.com/c.json") +def test_http_get_json_preserves_github_cli_auth_error(monkeypatch): + def fake_open_url(url, timeout=10, extra_headers=None, redirect_validator=None): + raise GitHubCliAuthError( + "GitHub CLI authentication failed for github.com. " + "Run 'gh auth login --hostname github.com'." + ) + + monkeypatch.setattr("specify_cli.authentication.http.open_url", fake_open_url) + + with pytest.raises(BundlerError, match="gh auth login") as excinfo: + adapters._http_get_json("team", "https://example.com/c.json") + assert not isinstance(excinfo.value, adapters._CatalogUnavailable) + + +def test_builtin_catalog_does_not_fall_back_for_github_cli_auth_error(monkeypatch): + def fake_open_url(url, timeout=10, extra_headers=None, redirect_validator=None): + raise GitHubCliAuthError( + "GitHub CLI authentication failed for github.com. " + "Run 'gh auth login --hostname github.com'." + ) + + monkeypatch.setattr("specify_cli.authentication.http.open_url", fake_open_url) + monkeypatch.setattr( + adapters, + "_load_packaged_catalog", + lambda filename: pytest.fail("authentication errors must not use a snapshot"), + ) + + fetcher = adapters.make_catalog_fetcher(allow_network=True) + with pytest.raises(BundlerError, match="gh auth login") as excinfo: + fetcher(_source("builtin://community")) + assert not isinstance(excinfo.value, adapters._CatalogUnavailable) + + def test_http_get_json_marks_server_error_unavailable(monkeypatch): def fake_open_url(url, timeout=10, extra_headers=None, redirect_validator=None): raise urllib.error.HTTPError(url, 503, "Service Unavailable", {}, None) diff --git a/tests/specify_cli/bundles/test_references.py b/tests/specify_cli/bundles/test_references.py index 261e1b2661..b034593e9c 100644 --- a/tests/specify_cli/bundles/test_references.py +++ b/tests/specify_cli/bundles/test_references.py @@ -492,6 +492,36 @@ def unavailable(self, source, force_refresh=False): assert "unreachable" in warnings[0] +def test_online_validation_preserves_wrapped_github_cli_auth_error( + tmp_path, monkeypatch, +): + from specify_cli.authentication.github import GitHubCliAuthError + from specify_cli.extensions import ExtensionCatalog, ExtensionError + + _mock_catalog(monkeypatch, ExtensionCatalog, "extensions", "requested", {}) + + def auth_failure(self, source, force_refresh=False): + try: + raise GitHubCliAuthError( + "GitHub CLI authentication failed for github.com. " + "Run 'gh auth login --hostname github.com'." + ) + except GitHubCliAuthError as exc: + raise ExtensionError( + "Failed to fetch catalog: " + "Run 'gh auth login --hostname github.com'." + ) from exc + + monkeypatch.setattr(ExtensionCatalog, "_fetch_single_catalog", auth_failure) + warnings = [] + check = make_reference_checker(tmp_path, allow_network=True, warnings=warnings) + + problem = check(_ref("extensions", "requested")) + + assert problem is not None and "gh auth login" in problem + assert warnings == [] + + @pytest.mark.parametrize("kind", ["extensions", "presets", "workflows", "steps"]) @pytest.mark.parametrize( ("unreachable", "has_match"), From 702806c81a7fae43da4753a685495b7d467e6185 Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 14:28:43 -0500 Subject: [PATCH 6/7] fix(auth): validate GitHub CLI hostnames Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 434e2508-0633-4d40-8e0d-5edb92cbc177 --- docs/reference/authentication.md | 4 +++ src/specify_cli/authentication/github.py | 20 ++++++++++++ .../authentication/test_authentication.py | 31 +++++++++++++++++++ 3 files changed, 55 insertions(+) diff --git a/docs/reference/authentication.md b/docs/reference/authentication.md index 045db6e012..7a54a9b27c 100644 --- a/docs/reference/authentication.md +++ b/docs/reference/authentication.md @@ -139,6 +139,10 @@ GitHub instance host: gh auth token --hostname ghes.example.com ``` +Before invoking GitHub CLI, Specify requires the selected credential host to be +an ASCII DNS hostname made of letters, digits, hyphens, and dots. Invalid hosts +are rejected without invoking `gh` or including the rejected value in the error. + When a request targets an `api.`, `raw.`, or `codeload.` host, Specify uses the bare instance host only when that exact host is also listed in the same provider entry. For example, `raw.ghes.example.com` maps to diff --git a/src/specify_cli/authentication/github.py b/src/specify_cli/authentication/github.py index 40f60df00d..f79255af59 100644 --- a/src/specify_cli/authentication/github.py +++ b/src/specify_cli/authentication/github.py @@ -24,6 +24,20 @@ _GITHUB_ENTERPRISE_SERVICE_PREFIXES = ("api.", "raw.", "codeload.") +def _is_safe_cli_hostname(hostname: str) -> bool: + """Return whether *hostname* is a shell-safe DNS hostname.""" + if not hostname or len(hostname) > 253 or not hostname.isascii(): + return False + labels = hostname.split(".") + return all( + 0 < len(label) <= 63 + and label[0].isalnum() + and label[-1].isalnum() + and all(char.isalnum() or char == "-" for char in label) + for label in labels + ) + + class GitHubCliAuthError(urllib.error.URLError): """Raised when GitHub CLI cannot provide a configured credential.""" @@ -118,6 +132,12 @@ def can_reuse_token_for_host( @staticmethod def _acquire_via_gh_cli(hostname: str) -> str: """Run the fixed GitHub CLI token command for *hostname*.""" + if not _is_safe_cli_hostname(hostname): + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + "the request hostname is not valid for GitHub CLI authentication." + ) + resolved = shutil.which("gh") if not resolved or not os.path.isabs(resolved): raise GitHubCliAuthError( diff --git a/tests/specify_cli/authentication/test_authentication.py b/tests/specify_cli/authentication/test_authentication.py index 28b8d41c3f..d0cb4f60f2 100644 --- a/tests/specify_cli/authentication/test_authentication.py +++ b/tests/specify_cli/authentication/test_authentication.py @@ -1581,6 +1581,37 @@ def fake_open(req, timeout=None): open_url("https://example.com/file.json") assert captured["req"].get_header("Authorization") is None + def test_open_url_rejects_shell_unsafe_github_cli_hostname(self, monkeypatch): + from unittest.mock import patch + + from specify_cli.authentication.http import open_url + + attacker_host = "a&whoami&b.ghes.example.com" + provider = get_provider("github") + assert isinstance(provider, GitHubAuth) + monkeypatch.setattr(provider, "_github_cli_cache", {}) + self._set_config( + monkeypatch, + [_github_cli_entry(("*.ghes.example.com",))], + ) + + with patch( + "specify_cli.authentication.github.shutil.which", + return_value="/opt/github-cli/gh.cmd", + ), patch( + "specify_cli.authentication.github.subprocess.run", + ) as run, patch( + "specify_cli.authentication.http.urllib.request.build_opener", + ) as build_opener, pytest.raises( + GitHubCliAuthError, + match="hostname is not valid", + ) as exc_info: + open_url(f"https://{attacker_host}/archive.zip") + + assert attacker_host not in str(exc_info.value) + run.assert_not_called() + build_opener.assert_not_called() + def test_open_url_no_auth_when_no_config(self, monkeypatch): from unittest.mock import MagicMock, patch from specify_cli.authentication.http import open_url From fe472fe172c6b27c53c964223324d6f3db09bdbe Mon Sep 17 00:00:00 2001 From: Manfred Riem <15701806+mnriem@users.noreply.github.com> Date: Wed, 14 Oct 2026 15:14:15 -0500 Subject: [PATCH 7/7] refactor(auth): narrow GitHub CLI provider scope Remove consumer-specific error handling, redirect policy changes, and shared provider API changes. Keep GitHub CLI host selection within the GitHub provider by treating each provider entry as one credential scope. Assisted-by: GitHub Copilot (model: GPT-5.6 Sol, autonomous) Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 434e2508-0633-4d40-8e0d-5edb92cbc177 --- docs/reference/authentication.md | 57 +- src/specify_cli/_version.py | 7 - .../authentication/azure_devops.py | 8 +- src/specify_cli/authentication/base.py | 25 +- src/specify_cli/authentication/bitbucket.py | 8 +- src/specify_cli/authentication/config.py | 2 +- src/specify_cli/authentication/github.py | 93 ++- src/specify_cli/authentication/http.py | 42 +- src/specify_cli/bundles/adapters.py | 3 - src/specify_cli/bundles/component_catalog.py | 3 - .../authentication/test_authentication.py | 605 ++++-------------- tests/specify_cli/bundles/test_adapters.py | 35 - tests/specify_cli/bundles/test_references.py | 30 - .../specify_cli/test_version_verification.py | 18 - tests/test_extensions.py | 63 -- 15 files changed, 193 insertions(+), 806 deletions(-) diff --git a/docs/reference/authentication.md b/docs/reference/authentication.md index 7a54a9b27c..1b20f34305 100644 --- a/docs/reference/authentication.md +++ b/docs/reference/authentication.md @@ -59,7 +59,9 @@ Either `token` or `token_env` must be set for the `bearer`, `basic-pat`, and `ba | Scheme | Header | Use for | |---|---|---| | `bearer` | `Authorization: Bearer ` | PATs, fine-grained PATs, OAuth tokens, GitHub App tokens | -| `github-cli` | `Authorization: Bearer ` | Token acquired on demand from `gh auth token` | + +The `github` provider supports `bearer` tokens and `github-cli` credentials +acquired on demand from GitHub CLI. **Example — PAT via environment variable:** @@ -72,7 +74,7 @@ Either `token` or `token_env` must be set for the `bearer`, `basic-pat`, and `ba } ``` -**Example — GitHub CLI (recommended for interactive use):** +**Example — GitHub CLI:** ```json { @@ -92,18 +94,9 @@ Either `token` or `token_env` must be set for the `bearer`, `basic-pat`, and `ba ``` Install [GitHub CLI](https://cli.github.com/) and run `gh auth login` before -using this mode. Specify invokes the fixed command -`gh auth token --hostname github.com` directly, without a shell, and caches -the resulting token in memory for the current process. The token is not -written to disk by Specify. - -A configured `github-cli` entry is authoritative for its matching hosts. If -GitHub CLI cannot provide a token, Specify fails that credential lookup before -making an HTTP request; it does not try later matching entries or an -unauthenticated fallback for that request. Successful tokens and sanitized -lookup failures are both cached per selected credential host for the current -process. After correcting a GitHub CLI login, start a new Specify invocation -to retry the lookup. +using this mode. Specify runs the fixed command +`gh auth token --hostname github.com` without a shell, caches the result in +memory for the current process, and never writes the token to disk. ### GitHub Enterprise Server (GHES) @@ -132,24 +125,26 @@ and `codeload.` subdomains your catalog/extension URLs use. A `*.ghes.example.com` wildcard matches subdomains but **not** the bare host, so always include the bare host explicitly. -With `auth: "github-cli"`, Specify requests the credential for the configured -GitHub instance host: +With `auth: "github-cli"`, one provider entry represents one GitHub instance. +List exactly one bare enterprise host plus any service aliases that share its +credential: -```bash -gh auth token --hostname ghes.example.com +```json +{ + "hosts": [ + "ghes.example.com", + "raw.ghes.example.com", + "codeload.ghes.example.com" + ], + "provider": "github", + "auth": "github-cli" +} ``` -Before invoking GitHub CLI, Specify requires the selected credential host to be -an ASCII DNS hostname made of letters, digits, hyphens, and dots. Invalid hosts -are rejected without invoking `gh` or including the rejected value in the error. - -When a request targets an `api.`, `raw.`, or `codeload.` host, Specify uses the -bare instance host only when that exact host is also listed in the same -provider entry. For example, `raw.ghes.example.com` maps to -`ghes.example.com` in the configuration above. Otherwise, the request host is -used unchanged. The selected host must already be configured and authenticated -in GitHub CLI; failures suggest `gh auth login --hostname ` without -including GitHub CLI command output in the error. +Specify runs `gh auth token --hostname ghes.example.com`. Use separate provider +entries for separate GitHub instances. The selected host must already be +authenticated in GitHub CLI; failures suggest `gh auth login --hostname ` +without including GitHub CLI command output. ### GitHub Enterprise Cloud with data residency (GHE.com) @@ -323,9 +318,7 @@ You can configure multiple entries for different hosts or organizations: is attempted as a final fallback. 5. On redirects, the `Authorization` header is stripped if the redirect target leaves the entry's declared hosts — preventing credential - leakage to CDNs or third-party services. For `github-cli`, the header is - also stripped when the target maps to a different GitHub CLI credential - host, even if both hosts appear in the same provider entry. + leakage to CDNs or third-party services. ## Template diff --git a/src/specify_cli/_version.py b/src/specify_cli/_version.py index afdf71b8de..93b4c6a618 100644 --- a/src/specify_cli/_version.py +++ b/src/specify_cli/_version.py @@ -37,9 +37,6 @@ "rate limited (configure ~/.specify/auth.json with a GitHub token)" ) _RESOLUTION_FAILURE_HTTP_PREFIX = "HTTP " -_RESOLUTION_FAILURE_GITHUB_AUTH_PREFIX = ( - "GitHub authentication failed for provider 'github': " -) _FAILURE_INSTALLER_MISSING = "installer-missing" _FAILURE_INSTALLER_INVALID = "installer-invalid" _FAILURE_TARGET_TAG_UNPARSEABLE = "target-tag-unparseable" @@ -115,7 +112,6 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: On anything else — including a malformed response body — the exception propagates; there is no catch-all (research D-006). """ - from .authentication.github import GitHubCliAuthError from .authentication.http import open_url try: @@ -143,8 +139,6 @@ def _fetch_latest_release_tag() -> tuple[str | None, str | None]: if e.code in (403, 429): return None, _RESOLUTION_FAILURE_RATE_LIMITED return None, f"{_RESOLUTION_FAILURE_HTTP_PREFIX}{e.code}" - except GitHubCliAuthError as e: - return None, str(e) except (urllib.error.URLError, OSError): return None, _RESOLUTION_FAILURE_OFFLINE @@ -1028,7 +1022,6 @@ def _emit_failure( if ( category in _RESOLUTION_FAILURE_CATEGORIES or category.startswith(_RESOLUTION_FAILURE_HTTP_PREFIX) - or category.startswith(_RESOLUTION_FAILURE_GITHUB_AUTH_PREFIX) ): console.print(f"Upgrade aborted: {category}", soft_wrap=True) return diff --git a/src/specify_cli/authentication/azure_devops.py b/src/specify_cli/authentication/azure_devops.py index 904a1310c2..91578bd418 100644 --- a/src/specify_cli/authentication/azure_devops.py +++ b/src/specify_cli/authentication/azure_devops.py @@ -58,17 +58,13 @@ def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: f"AzureDevOpsAuth does not support auth scheme {auth_scheme!r}" ) - def resolve_token( - self, - entry: AuthConfigEntry, - hostname: str | None = None, - ) -> str | None: + def resolve_token(self, entry: AuthConfigEntry) -> str | None: """Resolve token, with special handling for azure-cli and azure-ad.""" if entry.auth == "azure-cli": return self._acquire_via_az_cli() if entry.auth == "azure-ad": return self._acquire_via_client_credentials(entry) - return super().resolve_token(entry, hostname) + return super().resolve_token(entry) # -- Token acquisition ------------------------------------------------ diff --git a/src/specify_cli/authentication/base.py b/src/specify_cli/authentication/base.py index e83fdfce83..d6e0f4b118 100644 --- a/src/specify_cli/authentication/base.py +++ b/src/specify_cli/authentication/base.py @@ -20,7 +20,7 @@ class AuthProvider(ABC): And implement: * ``auth_headers(token, auth_scheme)`` — build headers from a resolved token - * ``resolve_token(entry, hostname)`` — obtain the token for a config entry + * ``resolve_token(entry)`` — obtain the token for a config entry """ key: str = "" @@ -36,20 +36,13 @@ def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: Must return a dict with at least an ``Authorization`` key. """ - def resolve_token( - self, - entry: AuthConfigEntry, - hostname: str | None = None, - ) -> str | None: + def resolve_token(self, entry: AuthConfigEntry) -> str | None: """Resolve the token for *entry*. Default implementation reads from ``entry.token`` directly or from the environment variable named by ``entry.token_env``. Override for schemes that acquire tokens dynamically (e.g. ``azure-cli``, ``azure-ad``). - - ``hostname`` identifies the request target for providers whose - credential helper selects credentials by host. """ import os @@ -62,17 +55,3 @@ def resolve_token( if val: return val return None - - def can_reuse_token_for_host( - self, - entry: AuthConfigEntry, - source_hostname: str | None, - target_hostname: str, - ) -> bool: - """Return whether a resolved token may follow a redirect. - - Static credentials are scoped by the entry's declared hosts, so the - default allows reuse within those hosts. Providers with host-selected - credentials can narrow that scope. - """ - return True diff --git a/src/specify_cli/authentication/bitbucket.py b/src/specify_cli/authentication/bitbucket.py index c561535a6a..24474e6b5b 100644 --- a/src/specify_cli/authentication/bitbucket.py +++ b/src/specify_cli/authentication/bitbucket.py @@ -59,11 +59,7 @@ def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: f"BitbucketAuth does not support auth scheme {auth_scheme!r}" ) - def resolve_token( - self, - entry: AuthConfigEntry, - hostname: str | None = None, - ) -> str | None: + def resolve_token(self, entry: AuthConfigEntry) -> str | None: """Resolve the credential, combining ``username`` for ``basic``. Returns ``None`` when the secret is missing, or — for ``basic`` — @@ -73,7 +69,7 @@ def resolve_token( ``:`` credential or a ``user:name:`` one that the server would parse as user ``user`` (RFC 7617 §2). """ - secret = super().resolve_token(entry, hostname) + secret = super().resolve_token(entry) if entry.auth != "basic": return secret username = (entry.username or "").strip() diff --git a/src/specify_cli/authentication/config.py b/src/specify_cli/authentication/config.py index 2f5d6a72d2..296c5c4d39 100644 --- a/src/specify_cli/authentication/config.py +++ b/src/specify_cli/authentication/config.py @@ -220,7 +220,7 @@ def load_auth_config( raise ValueError( f"providers[{i}]: '{field_name}' must be a non-empty string" ) - # azure-cli and github-cli need no extra fields + # azure-cli needs no extra fields entries.append( AuthConfigEntry( diff --git a/src/specify_cli/authentication/github.py b/src/specify_cli/authentication/github.py index f79255af59..6f13f349c2 100644 --- a/src/specify_cli/authentication/github.py +++ b/src/specify_cli/authentication/github.py @@ -5,7 +5,6 @@ import os import shutil import subprocess -import urllib.error from typing import TYPE_CHECKING from .base import AuthProvider @@ -21,7 +20,7 @@ "raw.githubusercontent.com", "codeload.github.com", }) -_GITHUB_ENTERPRISE_SERVICE_PREFIXES = ("api.", "raw.", "codeload.") +_GITHUB_SERVICE_PREFIXES = ("api.", "raw.", "codeload.") def _is_safe_cli_hostname(hostname: str) -> bool: @@ -38,20 +37,12 @@ def _is_safe_cli_hostname(hostname: str) -> bool: ) -class GitHubCliAuthError(urllib.error.URLError): +class GitHubCliAuthError(RuntimeError): """Raised when GitHub CLI cannot provide a configured credential.""" - def __str__(self) -> str: - """Return the sanitized reason without URLError's wrapper.""" - return str(self.reason) - class GitHubAuth(AuthProvider): - """GitHub authentication provider. - - Supports ``bearer`` for PATs, OAuth tokens, and GitHub App installation - tokens, plus ``github-cli`` for credentials managed by GitHub CLI. - """ + """GitHub authentication provider.""" key = "github" supported_auth_schemes = ("bearer", "github-cli") @@ -60,24 +51,19 @@ def __init__(self) -> None: self._github_cli_cache: dict[str, tuple[bool, str]] = {} def auth_headers(self, token: str, auth_scheme: str) -> dict[str, str]: - """Return ``Authorization: Bearer ``.""" + """Return a bearer authorization header.""" if auth_scheme not in self.supported_auth_schemes: raise ValueError( f"GitHubAuth does not support auth scheme {auth_scheme!r}" ) - return {"Authorization": f"Bearer {token}"} - - def resolve_token( - self, - entry: AuthConfigEntry, - hostname: str | None = None, - ) -> str | None: - """Resolve configured tokens or acquire one from GitHub CLI.""" + return {"Authorization": "Bearer " + token} + + def resolve_token(self, entry: AuthConfigEntry) -> str | None: + """Resolve a configured token or acquire one from GitHub CLI.""" if entry.auth != "github-cli": - return super().resolve_token(entry, hostname) + return super().resolve_token(entry) - request_host = (hostname or entry.hosts[0]).lower() - credential_host = self._credential_hostname(entry, request_host) + credential_host = self._credential_hostname(entry) cached = self._github_cli_cache.get(credential_host) if cached is not None: succeeded, value = cached @@ -96,38 +82,33 @@ def resolve_token( return token @staticmethod - def _credential_hostname(entry: AuthConfigEntry, request_host: str) -> str: - """Map a request host to the GitHub CLI login host.""" - if request_host in _GITHUB_PUBLIC_HOSTS: + def _credential_hostname(entry: AuthConfigEntry) -> str: + """Select the one GitHub CLI credential host represented by *entry*.""" + exact_hosts = { + host.lower() + for host in entry.hosts + if "*" not in host + } + if exact_hosts & _GITHUB_PUBLIC_HOSTS: + if any(host not in _GITHUB_PUBLIC_HOSTS for host in exact_hosts): + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + "a github-cli entry must describe one GitHub instance. " + "Use separate provider entries for github.com and GitHub Enterprise." + ) return "github.com" - exact_hosts = { - configured_host.lower() - for configured_host in entry.hosts - if "*" not in configured_host + bare_hosts = { + host + for host in exact_hosts + if not host.startswith(_GITHUB_SERVICE_PREFIXES) } - for prefix in _GITHUB_ENTERPRISE_SERVICE_PREFIXES: - if request_host.startswith(prefix): - instance_host = request_host.removeprefix(prefix) - if instance_host in exact_hosts: - return instance_host - return request_host - - def can_reuse_token_for_host( - self, - entry: AuthConfigEntry, - source_hostname: str | None, - target_hostname: str, - ) -> bool: - """Allow redirects only within one GitHub CLI credential scope.""" - if entry.auth != "github-cli": - return True - if source_hostname is None: - return False - return self._credential_hostname( - entry, - source_hostname, - ) == self._credential_hostname(entry, target_hostname) + if len(bare_hosts) != 1: + raise GitHubCliAuthError( + "GitHub authentication failed for provider 'github': " + "a github-cli entry must list exactly one bare GitHub host." + ) + return next(iter(bare_hosts)) @staticmethod def _acquire_via_gh_cli(hostname: str) -> str: @@ -135,7 +116,7 @@ def _acquire_via_gh_cli(hostname: str) -> str: if not _is_safe_cli_hostname(hostname): raise GitHubCliAuthError( "GitHub authentication failed for provider 'github': " - "the request hostname is not valid for GitHub CLI authentication." + "the configured hostname is not valid for GitHub CLI authentication." ) resolved = shutil.which("gh") @@ -145,11 +126,9 @@ def _acquire_via_gh_cli(hostname: str) -> str: "GitHub CLI ('gh') was not found." ) - argv = [resolved, "auth", "token", "--hostname", hostname] - try: result = subprocess.run( # noqa: S603 - argv, + [resolved, "auth", "token", "--hostname", hostname], capture_output=True, text=True, timeout=_GITHUB_CLI_TIMEOUT_SECONDS, diff --git a/src/specify_cli/authentication/http.py b/src/specify_cli/authentication/http.py index 84f1700b60..7dabc752aa 100644 --- a/src/specify_cli/authentication/http.py +++ b/src/specify_cli/authentication/http.py @@ -63,21 +63,12 @@ def _hostname_in_hosts(hostname: str, hosts: tuple[str, ...]) -> bool: RedirectValidator = Callable[[str, str], None] -AuthHostnameValidator = Callable[[str], bool] class RedirectPolicyError(urllib.error.URLError): """A redirect rejected because it violates the client's security policy.""" -def _hostname_for_url(url: str) -> str | None: - """Return a normalized request hostname, or None for malformed URLs.""" - try: - return (urlparse(url).hostname or "").lower() or None - except ValueError: - return None - - def _validate_strict_redirect(old_url: str, new_url: str) -> None: if not is_safe_download_redirect(old_url, new_url): raise RedirectPolicyError( @@ -100,12 +91,10 @@ def __init__( self, hosts: tuple[str, ...], redirect_validator: RedirectValidator | None = None, - auth_hostname_validator: AuthHostnameValidator | None = None, ) -> None: super().__init__() self._hosts = hosts self._redirect_validator = redirect_validator - self._auth_hostname_validator = auth_hostname_validator def redirect_request(self, req, fp, code, msg, headers, newurl): try: @@ -130,15 +119,7 @@ def redirect_request(self, req, fp, code, msg, headers, newurl): old_scheme = urlparse(req.full_url).scheme hostname = (new_parsed.hostname or "").lower() is_https_downgrade = old_scheme == "https" and new_parsed.scheme != "https" - is_auth_hostname_allowed = ( - self._auth_hostname_validator is None - or self._auth_hostname_validator(hostname) - ) - if ( - _hostname_in_hosts(hostname, self._hosts) - and not is_https_downgrade - and is_auth_hostname_allowed - ): + if _hostname_in_hosts(hostname, self._hosts) and not is_https_downgrade: if original_auth: new_req.add_unredirected_header("Authorization", original_auth) else: @@ -159,12 +140,11 @@ def build_request(url: str, extra_headers: dict[str, str] | None = None) -> urll headers.update({k: v for k, v in extra_headers.items() if k.lower() != "authorization"}) # Auth headers applied last — cannot be overridden by extra_headers entries = find_entries_for_url(url, _load_config()) - hostname = _hostname_for_url(url) for entry in entries: provider = get_provider(entry.provider) if provider is None: continue - token = provider.resolve_token(entry, hostname) + token = provider.resolve_token(entry) if token: headers.update(provider.auth_headers(token, entry.auth)) break @@ -210,7 +190,6 @@ def open_url( HTTP between loopback URLs, and rejects remote-to-local redirects. """ entries = find_entries_for_url(url, _load_config()) - hostname = _hostname_for_url(url) def _make_req(auth_headers: dict[str, str]) -> urllib.request.Request: merged = {} @@ -226,25 +205,12 @@ def _make_req(auth_headers: dict[str, str]) -> urllib.request.Request: provider = get_provider(entry.provider) if provider is None: continue - token = provider.resolve_token(entry, hostname) + token = provider.resolve_token(entry) if not token: continue req = _make_req(provider.auth_headers(token, entry.auth)) - def auth_hostname_validator(target_hostname: str) -> bool: - return provider.can_reuse_token_for_host( - entry, - hostname, - target_hostname, - ) - - opener = urllib.request.build_opener( - _StripAuthOnRedirect( - entry.hosts, - redirect_validator, - auth_hostname_validator, - ) - ) + opener = urllib.request.build_opener(_StripAuthOnRedirect(entry.hosts, redirect_validator)) try: return opener.open(req, timeout=timeout) except urllib.error.HTTPError as exc: diff --git a/src/specify_cli/bundles/adapters.py b/src/specify_cli/bundles/adapters.py index 5dd40460ea..46433cb005 100644 --- a/src/specify_cli/bundles/adapters.py +++ b/src/specify_cli/bundles/adapters.py @@ -22,7 +22,6 @@ from .._assets import _locate_core_pack, _repo_root from .._download_security import MAX_JSON_CATALOG_BYTES, read_response_limited -from ..authentication.github import GitHubCliAuthError from . import BundlerError from .catalogs import CatalogSource from .manifest import ComponentRef @@ -277,8 +276,6 @@ def _validate_redirect(_old_url: str, new_url: str) -> None: raise BundlerError( f"Failed to fetch catalog from {url}: HTTP {exc.code} {exc.reason}" ) from exc - except GitHubCliAuthError as exc: - raise BundlerError(f"Failed to fetch catalog from {url}: {exc}") from exc except urllib.error.URLError as exc: # urllib wraps a TLS handshake failure as URLError(reason=); # a certificate-verification failure is a security failure, not a diff --git a/src/specify_cli/bundles/component_catalog.py b/src/specify_cli/bundles/component_catalog.py index 931463de00..d4b7a107c7 100644 --- a/src/specify_cli/bundles/component_catalog.py +++ b/src/specify_cli/bundles/component_catalog.py @@ -6,7 +6,6 @@ from http.client import HTTPException from urllib.error import HTTPError, URLError -from ..authentication.github import GitHubCliAuthError from ..authentication.http import RedirectPolicyError from . import BundlerError from .manifest import ComponentRef @@ -20,8 +19,6 @@ def _is_unavailable(exc: BaseException) -> bool: seen: set[int] = set() while exc is not None and id(exc) not in seen: seen.add(id(exc)) - if isinstance(exc, GitHubCliAuthError): - return False if isinstance(exc, RedirectPolicyError): return False if isinstance(exc, HTTPError): diff --git a/tests/specify_cli/authentication/test_authentication.py b/tests/specify_cli/authentication/test_authentication.py index d0cb4f60f2..c46d4ac5c9 100644 --- a/tests/specify_cli/authentication/test_authentication.py +++ b/tests/specify_cli/authentication/test_authentication.py @@ -18,7 +18,6 @@ import io import json import os -import urllib.error import pytest @@ -51,7 +50,12 @@ def _github_entry(token_env: str = "GH_TOKEN", token: str | None = None) -> Auth def _github_cli_entry( - hosts: tuple[str, ...] = ("github.com",), + hosts: tuple[str, ...] = ( + "github.com", + "api.github.com", + "raw.githubusercontent.com", + "codeload.github.com", + ), ) -> AuthConfigEntry: """Build a GitHub CLI config entry.""" return AuthConfigEntry( @@ -106,6 +110,23 @@ def test_valid_github_config(self, tmp_path): assert entries[0].auth == "bearer" assert entries[0].token_env == "GH_TOKEN" + def test_valid_github_cli_config(self, tmp_path): + cfg = tmp_path / "auth.json" + cfg.write_text(json.dumps({ + "providers": [{ + "hosts": ["github.com", "api.github.com"], + "provider": "github", + "auth": "github-cli", + }] + })) + + entries = load_auth_config(cfg) + + assert len(entries) == 1 + assert entries[0].auth == "github-cli" + assert entries[0].token is None + assert entries[0].token_env is None + def test_padded_token_env_is_normalized_and_resolves(self, tmp_path, monkeypatch): # token_env is validated on its stripped form but was stored raw, so a # padded env-var name passed validation yet broke the verbatim @@ -305,18 +326,6 @@ def test_azure_cli_config(self, tmp_path): entries = load_auth_config(cfg) assert entries[0].auth == "azure-cli" - def test_github_cli_config(self, tmp_path): - cfg = tmp_path / "auth.json" - cfg.write_text(json.dumps({ - "providers": [{ - "hosts": ["github.com"], - "provider": "github", - "auth": "github-cli", - }] - })) - entries = load_auth_config(cfg) - assert entries == [_github_cli_entry()] - def test_multiple_entries(self, tmp_path): cfg = tmp_path / "auth.json" cfg.write_text(json.dumps({ @@ -613,48 +622,14 @@ class TestGitHubAuth: def test_bearer_headers(self): assert GitHubAuth().auth_headers("my-token", "bearer") == {"Authorization": "Bearer my-token"} - def test_unsupported_scheme_raises(self): - with pytest.raises(ValueError, match="basic-pat"): - GitHubAuth().auth_headers("tok", "basic-pat") - - def test_resolve_token_from_env(self, monkeypatch): - monkeypatch.setenv("GH_TOKEN", "env-token") - assert GitHubAuth().resolve_token(_github_entry()) == "env-token" - - def test_resolve_token_inline(self): - assert GitHubAuth().resolve_token(_github_entry(token="inline-tok")) == "inline-tok" - - def test_resolve_token_strips_whitespace(self, monkeypatch): - monkeypatch.setenv("GH_TOKEN", " my-token ") - assert GitHubAuth().resolve_token(_github_entry()) == "my-token" - - def test_resolve_token_empty_env_returns_none(self, monkeypatch): - monkeypatch.setenv("GH_TOKEN", " ") - assert GitHubAuth().resolve_token(_github_entry()) is None - - def test_resolve_token_missing_env_returns_none(self, monkeypatch): - monkeypatch.delenv("GH_TOKEN", raising=False) - assert GitHubAuth().resolve_token(_github_entry()) is None - - def test_key(self): - assert GitHubAuth.key == "github" - - def test_supported_schemes(self): - assert GitHubAuth.supported_auth_schemes == ("bearer", "github-cli") - - def test_github_cli_headers_use_bearer(self): - assert GitHubAuth().auth_headers("cli-token", "github-cli") == { - "Authorization": "Bearer cli-token" + def test_github_cli_headers(self): + assert GitHubAuth().auth_headers("my-token", "github-cli") == { + "Authorization": "Bearer my-token" } - def test_resolve_token_github_cli_success_without_shell( - self, - monkeypatch, - ): + def test_github_cli_resolves_token_with_fixed_command(self): from unittest.mock import MagicMock, patch - monkeypatch.setenv("GH_HOST", "enterprise.example.com") - provider = GitHubAuth() result = MagicMock(returncode=0, stdout=" cli-token\n") executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") @@ -665,95 +640,28 @@ def test_resolve_token_github_cli_success_without_shell( "specify_cli.authentication.github.subprocess.run", return_value=result, ) as run: - assert provider.resolve_token( - _github_cli_entry(), - "github.com", - ) == "cli-token" - - assert run.call_args.args[0] == [ - executable, - "auth", - "token", - "--hostname", - "github.com", - ] - assert run.call_args.kwargs == { - "capture_output": True, - "text": True, - "timeout": 10, - "check": False, - "shell": False, - } - - def test_resolve_token_github_cli_uses_enterprise_hostname(self): - from unittest.mock import MagicMock, patch - - provider = GitHubAuth() - result = MagicMock(returncode=0, stdout="enterprise-token") - executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") + token = GitHubAuth().resolve_token(_github_cli_entry()) - with patch( - "specify_cli.authentication.github.shutil.which", - return_value=executable, - ), patch( - "specify_cli.authentication.github.subprocess.run", - return_value=result, - ) as run: - assert provider.resolve_token( - _github_cli_entry(("ghes.example.com",)), - "ghes.example.com", - ) == "enterprise-token" - - assert run.call_args.args[0] == [ - executable, - "auth", - "token", - "--hostname", - "ghes.example.com", - ] + assert token == "cli-token" + run.assert_called_once_with( + [executable, "auth", "token", "--hostname", "github.com"], + capture_output=True, + text=True, + timeout=10, + check=False, + shell=False, + ) - @pytest.mark.parametrize( - "request_host", - [ - "api.ghes.example.com", - "raw.ghes.example.com", - "codeload.ghes.example.com", - ], - ) - def test_resolve_token_github_cli_maps_enterprise_service_host( - self, - request_host, - ): + def test_github_cli_uses_bare_enterprise_host(self): from unittest.mock import MagicMock, patch - provider = GitHubAuth() - result = MagicMock(returncode=0, stdout="enterprise-token") - executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") entry = _github_cli_entry(( "ghes.example.com", - "api.ghes.example.com", "raw.ghes.example.com", "codeload.ghes.example.com", )) - - with patch( - "specify_cli.authentication.github.shutil.which", - return_value=executable, - ), patch( - "specify_cli.authentication.github.subprocess.run", - return_value=result, - ) as run: - assert provider.resolve_token(entry, request_host) == "enterprise-token" - - assert run.call_args.args[0][-2:] == ["--hostname", "ghes.example.com"] - - def test_resolve_token_github_cli_does_not_infer_unconfigured_instance_host(self): - from unittest.mock import MagicMock, patch - - provider = GitHubAuth() result = MagicMock(returncode=0, stdout="enterprise-token") executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") - entry = _github_cli_entry(("raw.ghes.example.com",)) with patch( "specify_cli.authentication.github.shutil.which", @@ -762,76 +670,22 @@ def test_resolve_token_github_cli_does_not_infer_unconfigured_instance_host(self "specify_cli.authentication.github.subprocess.run", return_value=result, ) as run: - assert provider.resolve_token( - entry, - "raw.ghes.example.com", - ) == "enterprise-token" + assert GitHubAuth().resolve_token(entry) == "enterprise-token" - assert run.call_args.args[0][-2:] == [ - "--hostname", - "raw.ghes.example.com", - ] - - @pytest.mark.parametrize( - ("source", "target"), - [ - ("github.com", "api.github.com"), - ("github.com", "codeload.github.com"), - ("ghes.example.com", "api.ghes.example.com"), - ("ghes.example.com", "raw.ghes.example.com"), - ("ghes.example.com", "codeload.ghes.example.com"), - ], - ) - def test_github_cli_reuses_token_within_credential_host( - self, - source, - target, - ): - provider = GitHubAuth() - entry = _github_cli_entry(( - "github.com", - "api.github.com", - "codeload.github.com", - "ghes.example.com", - "api.ghes.example.com", - "raw.ghes.example.com", - "codeload.ghes.example.com", - )) + assert run.call_args.args[0][-2:] == ["--hostname", "ghes.example.com"] - assert provider.can_reuse_token_for_host(entry, source, target) + def test_github_cli_requires_one_instance_per_entry(self): + entry = _github_cli_entry(("github.com", "ghes.example.com")) - def test_github_cli_does_not_reuse_token_across_instances(self): - provider = GitHubAuth() - entry = _github_cli_entry(( - "github.com", - "api.github.com", - "ghes.example.com", - "raw.ghes.example.com", - )) + with pytest.raises(GitHubCliAuthError, match="separate provider entries"): + GitHubAuth().resolve_token(entry) - assert not provider.can_reuse_token_for_host( - entry, - "github.com", - "ghes.example.com", - ) - assert not provider.can_reuse_token_for_host( - entry, - "raw.ghes.example.com", - "api.github.com", - ) - - def test_resolve_token_github_cli_caches_per_credential_host(self): + def test_github_cli_caches_token_per_process(self): from unittest.mock import MagicMock, patch provider = GitHubAuth() result = MagicMock(returncode=0, stdout="cached-token") executable = os.path.join(os.path.abspath(os.sep), "opt", "gh", "gh") - entry = _github_cli_entry(( - "github.com", - "api.github.com", - "ghes.example.com", - "raw.ghes.example.com", - )) with patch( "specify_cli.authentication.github.shutil.which", @@ -840,32 +694,13 @@ def test_resolve_token_github_cli_caches_per_credential_host(self): "specify_cli.authentication.github.subprocess.run", return_value=result, ) as run: - assert provider.resolve_token(entry, "github.com") == "cached-token" - assert provider.resolve_token(entry, "api.github.com") == "cached-token" - assert provider.resolve_token(entry, "ghes.example.com") == "cached-token" - assert provider.resolve_token(entry, "raw.ghes.example.com") == "cached-token" + assert provider.resolve_token(_github_cli_entry()) == "cached-token" + assert provider.resolve_token(_github_cli_entry()) == "cached-token" - assert run.call_count == 2 + run.assert_called_once() - def test_resolve_token_github_cli_missing_raises_clear_error(self): - from unittest.mock import patch - - with patch( - "specify_cli.authentication.github.shutil.which", - return_value=None, - ), pytest.raises(GitHubCliAuthError) as exc_info: - GitHubAuth().resolve_token(_github_cli_entry(), "github.com") - - message = str(exc_info.value) - assert isinstance(exc_info.value, urllib.error.URLError) - assert not message.startswith("