Skip to content

Disambiguate GitHub auth fallback by repository context#630

Open
awakecoding wants to merge 4 commits intomicrosoft:mainfrom
awakecoding:fix/disambiguate-git-credential-fill
Open

Disambiguate GitHub auth fallback by repository context#630
awakecoding wants to merge 4 commits intomicrosoft:mainfrom
awakecoding:fix/disambiguate-git-credential-fill

Conversation

@awakecoding
Copy link
Copy Markdown

Summary

This PR is intended to disambiguate GitHub authentication based on the repository URL when APM resolves credentials for package fetches.

The motivating issue is Git Credential Manager account-selection prompts on machines that use more than one GitHub account. In my case, APM was attempting authentication in a way that was not specific enough to the target repository, which caused GCM to ask which account to use.

With these changes, I was able to configure different default user accounts for different GitHub repository URLs and use APM to fetch from private repositories without getting the account-selection prompt.

What changed

  • threads repository path context through APM auth resolution
  • includes repo-path context in git credential fill lookups when available
  • adds a GitHub CLI fallback before the generic credential-helper path for GitHub hosts
  • updates tests and auth documentation accordingly

Why this exists

The goal is to avoid ambiguous host-only credential resolution for GitHub package fetches, especially on developer machines that are signed into multiple GitHub accounts.

The key idea is that the repository URL contains enough context to help select the correct identity, and in practice these changes worked for me.

Notes for reviewers

These patches were vibe coded while investigating the issue end to end. Please feel free to close this PR in favor of a different implementation if the same behavior should be achieved another way.

If the changes are acceptable as-is, great. If they require adjustments, I can make them.

Copilot AI review requested due to automatic review settings April 8, 2026 19:46
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR refines APM’s GitHub authentication resolution by threading repository path context through auth lookups to reduce ambiguous multi-account credential selection, and introduces an explicit GitHub CLI (gh) fallback before invoking OS credential helpers.

Changes:

  • Thread repo_path through AuthResolver and pass it into git credential fill requests to disambiguate credential-helper lookups.
  • Add gh auth token --hostname <host> as an earlier fallback in the GitHub token chain.
  • Update unit tests and authentication documentation to reflect the new resolution chain.

Reviewed changes

Copilot reviewed 8 out of 8 changed files in this pull request and generated 11 comments.

Show a summary per file
File Description
src/apm_cli/core/auth.py Extends auth resolution/cache key with repo_path and adds gh/git fallback behavior.
src/apm_cli/core/token_manager.py Adds repo-path aware git credential fill requests and a gh auth token fallback.
src/apm_cli/commands/install.py Threads repo_path into try_with_fallback() during repo validation.
src/apm_cli/marketplace/client.py Passes repo_path for marketplace fetches to improve credential selection.
tests/unit/test_auth.py Adds a dep-aware test asserting repo-path is passed to credential fill.
tests/test_token_manager.py Adds tests for gh fallback, path/username in credential fill, and cache key separation.
packages/apm-guide/.apm/skills/apm-usage/authentication.md Updates the skill doc token precedence chain with gh + repo-path context.
docs/src/content/docs/getting-started/authentication.md Updates public docs to describe repo-path context and the gh fallback step.

Copy link
Copy Markdown
Collaborator

@sergio-sisternes-epam sergio-sisternes-epam left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for tackling this — multi-account credential ambiguity is a real pain point, and the gh auth token fallback is a genuinely great idea that we'd love to see land.

That said, we have concerns about the current scope. Auth resolution is our most security-sensitive code path, and this PR changes 8 files, alters cache key semantics from (host, org) to (host, org, repo_path), and threads a new input (repo_path) into git credential fill stdin — which is a protocol that interprets newlines as field delimiters. That's a large review surface for a security-critical area, and we need to be extra cautious here.

Our recommendation: land this incrementally.

The gh auth token --hostname <host> fallback alone solves ~90% of multi-account cases in a fraction of the code. It doesn't require repo_path threading, cache key changes, or new subprocess input — just a clean new step in the resolution chain.

We'd suggest:

  1. Slim this PR to just the gh auth token fallback (before git credential fill in the chain). That's a focused, reviewable change we can merge with confidence.
  2. If users later report issues in the "no gh CLI, multiple GCM credentials" scenario, we can add repo_path pass-through to git credential fill as a follow-up PR — with proper input sanitization via our path_security utilities.

Would you be open to scoping down to just the gh fallback? We're happy to help define the boundaries if useful.

Thanks again for the contribution — the core insight here is solid.

@awakecoding
Copy link
Copy Markdown
Author

Thanks for taking the time to review the pull request and suggest modifications to be made. I'll look into reducing the scope of changes in this pull request this week

@awakecoding
Copy link
Copy Markdown
Author

Reduced the PR scope as requested in follow-up commit bcf42a4. This version keeps the GitHub CLI fallback (gh auth token --hostname ) ahead of git credential fill, and removes the repo-path threading, cache key expansion, and repo-path-specific tests/docs so the first batch stays focused. Focused auth validation still passes locally: ests/test_token_manager.py and ests/unit/test_auth.py.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants