Skip to content

fix(credential_pool): exact-match copilot source name instead of substring - #76349

Open
wangyunyou wants to merge 1 commit into
NousResearch:mainfrom
wangyunyou:fix/copilot-source-name-mapping
Open

fix(credential_pool): exact-match copilot source name instead of substring#76349
wangyunyou wants to merge 1 commit into
NousResearch:mainfrom
wangyunyou:fix/copilot-source-name-mapping

Conversation

@wangyunyou

Copy link
Copy Markdown

Problem

The copilot source-name mapping in _seed_from_singletons used a substring test to decide between the gh CLI and env-var sources:

source_name = "gh_cli" if "gh" in source.lower() else f"env:{source}"

Every copilot env var name — GH_TOKEN, GITHUB_TOKEN, COPILOT_GITHUB_TOKENcontains the substring "gh", so env-seeded copilot credentials were tagged as gh_cli instead of env:<VAR>.

That mislabeling breaks per-source suppression:

  • A user who suppressed only the gh CLI path (hermes auth remove copilot gh_cli suppresses all sources, but a manual/partial suppression of just gh_cli) also lost their env-var token — the env entry was filed under gh_cli and tripped the same suppression check.
  • hermes auth list mislabeled env-seeded entries as gh_cli.

Verified against the pre-fix code: with suppressed_sources.copilot = ["gh_cli"] and a live GH_TOKEN env var, the pool seeds 0 entries (env token wrongly suppressed). After the fix it seeds 1 entry with source = "env:GH_TOKEN".

Fix

resolve_copilot_token() returns exactly "gh auth token" for the gh CLI path and the raw env var name otherwise, so the mapping is now an exact match:

source_name = "gh_cli" if source == "gh auth token" else f"env:{source}"

The gh CLI path still maps to gh_cli (unchanged); env sources now map to their true env:<VAR> names, so suppression and display are per-source correct.

Test

Two new regression tests in tests/agent/test_credential_pool.py:

  • test_load_pool_tags_env_copilot_source_as_env_var — env source seeds with source == "env:GH_TOKEN".
  • test_load_pool_env_copilot_ignores_gh_cli_suppression — suppressing gh_cli leaves env:GH_TOKEN seedable.

Full suite: 77 tests passed (credential_pool 55 + copilot auth/exchange 22).

…tring

The copilot source-name mapping used "gh" in source.lower() to decide
between the gh CLI and env-var sources. Every env var name — GH_TOKEN,
GITHUB_TOKEN, COPILOT_GITHUB_TOKEN — contains the substring "gh", so
env-seeded copilot entries were tagged as gh_cli instead of env:<VAR>.

That mislabeling breaks per-source suppression: a user who suppressed
only the gh CLI path (hermes auth remove copilot gh_cli) also lost their
env-var token, because the env entry was filed under gh_cli and tripped
the same suppression check. It also mislabeled entries in hermes auth
list.

resolve_copilot_token() returns exactly "gh auth token" for the gh CLI
path and the raw env var name otherwise, so the mapping can be an exact
match. Add regression tests covering env tagging and gh_cli-suppression
independence.
@alt-glitch alt-glitch added type/bug Something isn't working P2 Medium — degraded but workaround exists comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint provider/copilot GitHub Copilot (ACP + Chat) area/auth Authentication, OAuth, credential pools sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data labels Aug 1, 2026
@alt-glitch

Copy link
Copy Markdown
Collaborator

This was generated by AI during triage.

Related: #76341 repairs when suppression is checked; this PR repairs how the resolved Copilot source is named. Both change the same credential-pool flow and are complementary rather than duplicates.

@teknium1

teknium1 commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Thanks for the focused credential-source fix. The premise is confirmed on current main: agent/credential_pool.py:2384 classifies every source containing "gh" as gh_cli, while hermes_cli/copilot_auth.py:81-101 returns raw environment-variable names for env resolution and exactly "gh auth token" for the CLI fallback. The exact comparison therefore preserves the CLI mapping while correctly retaining env:GH_TOKEN-style source identities for suppression and display.

The new tests cover both the corrected source label and the resulting per-source suppression behavior. The related #76341 change remains complementary: its current diff moves suppression earlier but retains the substring mapping, so any combined salvage should preserve this exact-match mapping.

Automated hermes-sweeper review.

@teknium1 teknium1 added sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit labels Aug 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area/auth Authentication, OAuth, credential pools comp/agent Core agent runtime: loop, agent_init, prompt builder, context-compression, responses endpoint P2 Medium — degraded but workaround exists provider/copilot GitHub Copilot (ACP + Chat) sweeper:blast-broad Sweeper blast radius: broad — a core path most sessions hit sweeper:risk-compatibility Sweeper risk: may break existing users, config, migrations, defaults, or upgrades sweeper:risk-security-boundary Sweeper risk: may affect sandboxing, auth, credentials, or sensitive data type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants