diff --git a/agent/credential_pool.py b/agent/credential_pool.py index 715f8b8b174c7..b5d11db4d0ff0 100644 --- a/agent/credential_pool.py +++ b/agent/credential_pool.py @@ -2381,7 +2381,16 @@ def _env_val(key: str) -> str: "unavailable); enterprise-only models may 400 with " "model_not_available_for_integrator until exchange recovers." ) - source_name = "gh_cli" if "gh" in source.lower() else f"env:{source}" + # Map the token source to its pool source name. The ONLY + # value resolve_copilot_token() returns for the gh CLI path + # is exactly "gh auth token"; env sources return the raw env + # var name (COPILOT_GITHUB_TOKEN / GH_TOKEN / GITHUB_TOKEN). + # A substring test like `"gh" in source.lower()` misfires on + # every env name (they all contain "gh"), tagging env-seeded + # entries as gh_cli — which makes `hermes auth remove + # copilot gh_cli`-style per-source suppression unable to + # distinguish a suppressed gh CLI from a live env var. + source_name = "gh_cli" if source == "gh auth token" else f"env:{source}" if not _is_suppressed(provider, source_name): active_sources.add(source_name) pconfig = PROVIDER_REGISTRY.get(provider) diff --git a/tests/agent/test_credential_pool.py b/tests/agent/test_credential_pool.py index fa3d2b2111451..51df62107a131 100644 --- a/tests/agent/test_credential_pool.py +++ b/tests/agent/test_credential_pool.py @@ -1352,6 +1352,74 @@ def test_load_pool_seeds_copilot_via_gh_auth_token(tmp_path, monkeypatch): assert entries[0].base_url == "https://api.githubcopilot.com" +def test_load_pool_tags_env_copilot_source_as_env_var(tmp_path, monkeypatch): + """An env-var copilot token must be tagged env:, not gh_cli. + + Regression test: the source-name mapping used ``"gh" in + source.lower()``, which matches EVERY env var name — COPILOT_GITHUB_TOKEN, + GH_TOKEN and GITHUB_TOKEN all contain the substring "gh" — so env-seeded + entries were mislabelled as gh_cli. Only the gh CLI path returns the + literal "gh auth token", so the mapping must be an exact match. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store(tmp_path, {"version": 1, "credential_pool": {}}) + + monkeypatch.setattr( + "hermes_cli.copilot_auth.resolve_copilot_token", + lambda: ("gho_fake_token_abc123", "GH_TOKEN"), + ) + monkeypatch.setattr( + "hermes_cli.copilot_auth.get_copilot_api_token", + lambda token: ("exchanged_api_token", None), + ) + + from agent.credential_pool import load_pool + pool = load_pool("copilot") + + assert pool.has_credentials() + entries = pool.entries() + assert len(entries) == 1 + assert entries[0].source == "env:GH_TOKEN" + assert entries[0].access_token == "exchanged_api_token" + + +def test_load_pool_env_copilot_ignores_gh_cli_suppression(tmp_path, monkeypatch): + """Suppressing gh_cli must not suppress an env-var copilot source. + + With the old substring mapping both sources collapsed to "gh_cli", so a + user who suppressed the gh CLI path (hermes auth remove copilot gh_cli) + lost their env-var token too. With the exact mapping the env source stays + independent: suppressing gh_cli leaves env:GH_TOKEN seedable. + """ + monkeypatch.setenv("HERMES_HOME", str(tmp_path / "hermes")) + _write_auth_store( + tmp_path, + { + "version": 1, + "credential_pool": {}, + "suppressed_sources": {"copilot": ["gh_cli"]}, + }, + ) + + monkeypatch.setattr( + "hermes_cli.copilot_auth.resolve_copilot_token", + lambda: ("gho_fake_token_abc123", "GH_TOKEN"), + ) + monkeypatch.setattr( + "hermes_cli.copilot_auth.get_copilot_api_token", + lambda token: ("exchanged_api_token", None), + ) + + from agent.credential_pool import load_pool + pool = load_pool("copilot") + + # gh_cli suppressed, but the env source is a different source — it must seed. + assert pool.has_credentials() + entries = pool.entries() + assert len(entries) == 1 + assert entries[0].source == "env:GH_TOKEN" + + def test_load_pool_seeds_qwen_oauth_via_cli_tokens(tmp_path, monkeypatch):