Skip to content

fix: prevent copilot from leaking into model picker via GH_TOKEN - #46427

Closed
Budescu-Eftimie wants to merge 2 commits into
NousResearch:mainfrom
Budescu-Eftimie:fix/copilot-picker-leak
Closed

fix: prevent copilot from leaking into model picker via GH_TOKEN#46427
Budescu-Eftimie wants to merge 2 commits into
NousResearch:mainfrom
Budescu-Eftimie:fix/copilot-picker-leak

Conversation

@Budescu-Eftimie

@Budescu-Eftimie Budescu-Eftimie commented Jun 15, 2026

Copy link
Copy Markdown

GH_TOKEN or COPILOT_GITHUB_TOKEN set for gh CLI or from stale Docker
env was causing GitHub Copilot to appear in the TUI model picker dropdown
even when the user never configured copilot as their provider.

Three-layer fix:

  • providers.py: empty extra_env_vars for github-copilot overlay so the
    HERMES_OVERLAYS env-var short-circuit doesn't fire
  • model_switch.py: gate copilot in Section 1 (PROVIDER_REGISTRY path)
    behind active_provider / model.provider check — not env vars, since
    the env var IS the leak vector
  • credential_pool.py: gate _seed_from_singletons behind
    is_provider_explicitly_configured, mirroring the existing anthropic
    guard from PR fix: gate Claude Code credentials behind explicit Hermes config in wizard trigger #4210

How to Test

  1. Set GH_TOKEN or COPILOT_GITHUB_TOKEN in the environment
  2. Ensure model.provider is NOT copilot and active_provider is NOT copilot
  3. Open the model picker (TUI /model, desktop app dropdown, or
    hermes model CLI) — GitHub Copilot should NOT appear
  4. Switch model.provider to copilot — it SHOULD appear in the picker

Checklist

Code

  • I've read the Contributing Guide
  • My commit messages follow Conventional Commits
  • I searched for existing PRs to make sure this isn't a duplicate
  • My PR contains only changes related to this fix (no unrelated commits)
  • I've run pytest tests/ -q and all tests pass
  • I've added tests for my changes
  • I've tested on my platform: Windows/Docker (desktop app + TUI)

Documentation & Housekeeping

  • N/A — no config keys or architecture changes

GH_TOKEN or COPILOT_GITHUB_TOKEN set for gh CLI or from stale Docker
env was causing GitHub Copilot to appear in the TUI model picker dropdown
even when the user never configured copilot as their provider.

Three-layer fix:
- providers.py: empty extra_env_vars for github-copilot overlay so the
  HERMES_OVERLAYS env-var short-circuit doesn't fire
- model_switch.py: gate copilot in Section 1 (PROVIDER_REGISTRY path)
  behind active_provider / model.provider check — not env vars, since
  the env var IS the leak vector
- credential_pool.py: gate _seed_from_singletons behind
  is_provider_explicitly_configured, mirroring the existing anthropic
  guard from PR NousResearch#4210
The new explicit-configuration gate in _seed_from_singletons requires
copilot to appear configured before auto-seeding. Set active_provider
in the test auth store so the pool has entries for auth_remove_command
to operate on.
@alt-glitch alt-glitch added type/bug Something isn't working comp/cli CLI entry point, hermes_cli/, setup wizard provider/copilot GitHub Copilot (ACP + Chat) P2 Medium — degraded but workaround exists labels Jun 15, 2026
@liuhao1024

Copy link
Copy Markdown
Contributor

✅ Code Review — Verified Clean

Reviewed: Prevent Copilot from leaking into model picker via GH_TOKEN

Checks applied:

  1. Belt-and-suspenders: Three independent guards — (a) credential_pool.py gates seeding behind is_provider_explicitly_configured("copilot"), (b) model_switch.py gates picker listing behind active_provider or model.provider check, (c) providers.py removes extra_env_vars entirely so env-var detection path is disabled.
  2. Existing pattern match: The is_provider_explicitly_configured guard in credential_pool.py mirrors the existing anthropic guard above it — same function, same ImportError fallback. Confirmed the function exists in hermes_cli/auth.py.
  3. Config-as-dict guard: model_switch.py correctly uses isinstance(model_cfg, dict) before calling .get("provider") — avoids AttributeError when model is a string shorthand.
  4. Auth store fallback still works: The if not has_creds: block below the guard checks store.get("credential_pool", {}).get("copilot") — so previously-configured copilot credentials still surface correctly.
  5. Test coverage: Two tests — one verifying seeding when explicitly configured (active_provider: "copilot"), one verifying NO seeding when not configured. The existing test_auth_remove_copilot_suppresses_all_variants test is updated to set active_provider.

Design note: Removing COPILOT_GITHUB_TOKEN from extra_env_vars means users who intentionally set that env var (without config) will also not see copilot in the picker. This is consistent with the PR's explicit-configuration gate but may surprise users who relied on env-only setup.

No bugs found. LGTM.

@teknium1

Copy link
Copy Markdown
Contributor

Closed as superseded for the reported picker pollution by #60514. Desktop configured-only picker payloads now exclude ambient gh_cli credentials because they are not explicit provider configuration. The lower-level ambient Copilot discovery remains intentional for onboarding/full-universe surfaces, so that broader behavior was not adopted.

@teknium1 teknium1 closed this Jul 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

comp/cli CLI entry point, hermes_cli/, setup wizard P2 Medium — degraded but workaround exists provider/copilot GitHub Copilot (ACP + Chat) type/bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants