From f1c2582cb4d1093bcfdbb435e828fdab8f246193 Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Thu, 23 Jul 2026 23:28:29 +0500 Subject: [PATCH 1/2] fix(auth): normalize whitespace in auth-config env-var/id references at store time MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit token_env, client_secret_env, tenant_id, and client_id were VALIDATED on their .strip()ed form but STORED raw, so an accidentally padded value passed validation yet silently broke the downstream verbatim os.environ.get(name) / OAuth-URL lookups — load_auth_config succeeded but resolve_token returned None and the request quietly downgraded to unauthenticated (401/403) with no diagnostic. Normalize these whitespace-insignificant string references with a _norm helper at store time, mirroring how `hosts` is already normalized (h.strip().lower()). `token` is unchanged (already stripped at resolve time). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- src/specify_cli/authentication/config.py | 22 ++++++++++++++++++---- tests/test_authentication.py | 21 +++++++++++++++++++++ 2 files changed, 39 insertions(+), 4 deletions(-) diff --git a/src/specify_cli/authentication/config.py b/src/specify_cli/authentication/config.py index 8d1faf80c3..829940d6f7 100644 --- a/src/specify_cli/authentication/config.py +++ b/src/specify_cli/authentication/config.py @@ -13,6 +13,7 @@ from dataclasses import dataclass from fnmatch import fnmatch from pathlib import Path +from typing import Any from urllib.parse import urlparse @@ -53,6 +54,19 @@ def _is_valid_host_pattern(pattern: str) -> bool: return pattern.startswith("*.") and "*" not in pattern[2:] +def _norm(value: Any) -> Any: + """Strip surrounding whitespace from a whitespace-insignificant string + config reference (env-var names, tenant/client ids) before it is stored. + + These fields are validated on their ``.strip()``ed form, so an accidentally + padded value passes validation but then silently breaks the verbatim + ``os.environ.get(...)`` / URL lookups downstream. Normalizing at store time + mirrors how ``hosts`` is already handled (``h.strip().lower()``). Non-string + values (e.g. ``None``) pass through unchanged. + """ + return value.strip() if isinstance(value, str) else value + + def load_auth_config( path: Path | None = None, ) -> list[AuthConfigEntry]: @@ -182,10 +196,10 @@ def load_auth_config( provider=provider, auth=auth, token=token, - token_env=token_env, - tenant_id=entry_raw.get("tenant_id"), - client_id=entry_raw.get("client_id"), - client_secret_env=entry_raw.get("client_secret_env"), + token_env=_norm(token_env), + tenant_id=_norm(entry_raw.get("tenant_id")), + client_id=_norm(entry_raw.get("client_id")), + client_secret_env=_norm(entry_raw.get("client_secret_env")), ) ) diff --git a/tests/test_authentication.py b/tests/test_authentication.py index 06523c6f26..f88ddbf459 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -92,6 +92,27 @@ def test_valid_github_config(self, tmp_path): assert entries[0].auth == "bearer" assert entries[0].token_env == "GH_TOKEN" + 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 + # os.environ.get() lookup — resolve_token silently returned None. + monkeypatch.setenv("GH_TOKEN", "secret-tok") + cfg = tmp_path / "auth.json" + cfg.write_text(json.dumps({ + "providers": [{ + "hosts": ["github.com"], + "provider": "github", + "auth": "bearer", + "token_env": " GH_TOKEN ", + }] + })) + entries = load_auth_config(cfg) + assert len(entries) == 1 + # Stored normalized (matching how hosts are normalized), ... + assert entries[0].token_env == "GH_TOKEN" + # ... so the env lookup finds the token instead of returning None. + assert GitHubAuth().resolve_token(entries[0]) == "secret-tok" + def test_valid_ado_config(self, tmp_path): cfg = tmp_path / "auth.json" cfg.write_text(json.dumps({ From a910632b7689c4522c7bbb97ff6e1e878e076dec Mon Sep 17 00:00:00 2001 From: jawwad-ali Date: Fri, 24 Jul 2026 12:50:16 +0500 Subject: [PATCH 2/2] test(auth): cover tenant_id/client_id/client_secret_env normalization MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address review: the regression test only covered token_env, but the fix also normalizes tenant_id, client_id, and client_secret_env. Add a padded azure-ad entry asserting all three are stored stripped. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.8 (1M context) --- tests/test_authentication.py | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/tests/test_authentication.py b/tests/test_authentication.py index f88ddbf459..9edbd9860a 100644 --- a/tests/test_authentication.py +++ b/tests/test_authentication.py @@ -157,6 +157,26 @@ def test_azure_ad_config(self, tmp_path): assert entries[0].auth == "azure-ad" assert entries[0].tenant_id == "tid" + def test_padded_azure_ad_refs_are_normalized(self, tmp_path): + # The normalization also covers tenant_id / client_id / client_secret_env + # (used verbatim in the OAuth token URL/body and os.environ.get). Padded + # values were validated on their stripped form but stored raw. + cfg = tmp_path / "auth.json" + cfg.write_text(json.dumps({ + "providers": [{ + "hosts": ["dev.azure.com"], + "provider": "azure-devops", + "auth": "azure-ad", + "tenant_id": " tid ", + "client_id": " cid ", + "client_secret_env": " SECRET ", + }] + })) + entries = load_auth_config(cfg) + assert entries[0].tenant_id == "tid" + assert entries[0].client_id == "cid" + assert entries[0].client_secret_env == "SECRET" + def test_azure_cli_config(self, tmp_path): cfg = tmp_path / "auth.json" cfg.write_text(json.dumps({