fix(auth): fix credential helper reliability on Windows interactive login#389
Open
frblondin wants to merge 6 commits intomicrosoft:mainfrom
Open
fix(auth): fix credential helper reliability on Windows interactive login#389frblondin wants to merge 6 commits intomicrosoft:mainfrom
frblondin wants to merge 6 commits intomicrosoft:mainfrom
Conversation
Use a class-level shared cache keyed by host so that multiple GitHubTokenManager instances created within a single command (e.g. validation phase + install phase) only invoke the git credential helper once per host instead of once per instance.
Author
|
@microsoft-github-policy-service agree |
Contributor
There was a problem hiding this comment.
Pull request overview
Improves Windows authentication reliability for apm install by extending git credential fill timeouts and deduplicating credential-helper prompts via a shared cache.
Changes:
- Increase default credential-helper timeout and add an env override with max clamping.
- Introduce a process-wide shared credential cache guarded by a lock to reduce duplicate credential-helper calls.
- Add unit tests and documentation describing the new timeout and caching behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 5 comments.
| File | Description |
|---|---|
| tests/test_token_manager.py | Adds tests for timeout selection and shared-cache behavior; adds an autouse fixture to isolate shared cache between tests. |
| src/apm_cli/core/token_manager.py | Implements configurable git credential fill timeout and shared credential cache guarded by a lock. |
| docs/src/content/docs/getting-started/authentication.md | Documents the new timeout env var and per-command/per-host credential caching behavior. |
Collaborator
|
Great stuff let's get the pr comments addressed @frblondin and get this merged. Thank you! |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Author
|
Great @danielmeppiel , please have a look with new updates. |
danielmeppiel
added a commit
that referenced
this pull request
Mar 21, 2026
- Fix docs conflating EMU with *.ghe.com — EMU orgs can exist on github.com too; *.ghe.com is specifically GHE Cloud Data Residency - Increase git credential fill timeout: 5s → 60s default (configurable via APM_GIT_CREDENTIAL_TIMEOUT, max 180s) — fixes silent auth failures on Windows when credential helper shows interactive account picker - Add 7 timeout tests - CHANGELOG entry for credential timeout fix Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
danielmeppiel
added a commit
that referenced
this pull request
Mar 21, 2026
- Fix docs conflating EMU with *.ghe.com — EMU orgs can exist on github.com too; *.ghe.com is specifically GHE Cloud Data Residency - Increase git credential fill timeout: 5s → 60s default (configurable via APM_GIT_CREDENTIAL_TIMEOUT, max 180s) — fixes silent auth failures on Windows when credential helper shows interactive account picker - Add 7 timeout tests - CHANGELOG entry for credential timeout fix Credit: credential timeout fix based on investigation by @frblondin in #389. Co-authored-by: Thomas Caudal <thomas@caudal.fr> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
On Windows,
apm installcould fail silently or prompt the user twice when authentication relied ongit credential fill(e.g. the GitHub account picker via Windows Credential Manager). This PR fixes both symptoms as part of a single reliability improvement to credential helper handling.Root causes
Silent timeout:
git credential fillwas called with a hardcoded 5-second timeout. The Windows account-selection dialog takes ~25 seconds, so the subprocess raisedTimeoutExpired(caught silently), APM received no token, and reported the package as "not accessible or doesn't exist".Duplicate prompts: Multiple
GitHubPackageDownloaderinstances are created per command (validation phase + install phase). Each owned a freshGitHubTokenManagerwith an empty cache, so the credential helper was invoked once per instance rather than once per command.Changes
DEFAULT_GIT_CREDENTIAL_TIMEOUT_SECONDS: 5 → 60MAX_GIT_CREDENTIAL_TIMEOUT_SECONDS: 180APM_GIT_CREDENTIAL_TIMEOUTenv override (seconds, clamped 1–180)_SHARED_CREDENTIAL_CACHEclass variable (keyed by host) +threading.Lock()onGitHubTokenManager— credential helper called at most once per host across all instances in a single processTests
5 new unit tests in
tests/test_token_manager.pycovering timeout defaults, env override, clamping, and shared-cache deduplication. All 43 token-manager tests pass.Documentation
Updated
docs/src/content/docs/getting-started/authentication.mdwith a note on the configurable timeout and per-command caching behaviour.