Fix GitHub App token expiry on Windows static agents#1516
Open
pruthviraja wants to merge 6 commits intojenkinsci:masterfrom
Open
Fix GitHub App token expiry on Windows static agents#1516pruthviraja wants to merge 6 commits intojenkinsci:masterfrom
pruthviraja wants to merge 6 commits intojenkinsci:masterfrom
Conversation
Author
|
Hi maintainers — could someone please trigger CI and review this? |
adb23b8 to
951c238
Compare
On Windows, git credential helpers (wincred / git-credential-manager) cache GitHub App installation tokens in Windows Credential Manager and serve them directly on subsequent git operations, bypassing GIT_ASKPASS entirely. This causes authentication failures every ~1 hour on permanent Windows nodes even though Jenkins correctly generates a fresh token. Fix: inside DelegatingGitHubAppCredentials.getPassword() (which runs on the agent JVM), detect when running on a Windows agent and call cmdkey /delete:git:https://<host> cmdkey /delete:LegacyGenericCredential:https://<host> immediately after obtaining a (possibly refreshed) token. This evicts the stale cached entry so that git falls through to GIT_ASKPASS and uses the fresh token Jenkins is about to provide, rather than the expired token Windows Credential Manager has cached from a prior build. The clearing is a no-op when the Credential Manager has no entry for that host (cmdkey exits 1, which is silently ignored). It is skipped entirely on non-Windows agents (Linux, macOS) and on ephemeral agents where Windows Credential Manager is empty at startup anyway. Also adds: - deriveGitHostFromApiUri(): maps https://api.github.com -> github.com and passes GHE host through unchanged. - clearWindowsCredentialManagerCache(): package-private for testing, uses a replaceable Consumer<String> so tests never invoke cmdkey. - CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE flag (default true): allows the behaviour to be disabled from the Jenkins script console if needed without redeploying the plugin. - GithubAppCredentialsWindowsAgentTest: unit tests for all helper methods, run on any OS via a recording stub for the cleaner. Verified on GKE Jenkins controller with a permanent Windows agent: git:https://github.com (exit: 0) <- entry evicted LegacyGenericCredential:https://github.com (exit: 1) <- not present (normal) Both checkouts succeeded after the 60s stale threshold was crossed. Fixes: jenkinsci#1515
SpotBugs does not flag Consumer<String> fields as MS_SHOULD_BE_FINAL so the suppression annotation is redundant and triggers US_USELESS_SUPPRESSION_ON_FIELD.
The test verifies a strict log message sequence using contains(). On Windows CI agents our fix correctly fires cmdkey /delete, producing 'Cleared Windows Credential Manager entry' log lines that interleave with the expected sequence and break the assertion. Disable CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE for the duration of this test (save + restore in the existing finally block) since the test is exercising token-refresh logic, not credential manager behaviour. Windows Credential Manager clearing is covered by GithubAppCredentialsWindowsAgentTest.
3bce4c7 to
6490841
Compare
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
Fixes #1515
On Windows, git credential helpers (
wincred/git-credential-manager) cache GitHub App installation tokens in Windows Credential Manager and serve them directly on subsequent Git operations, bypassingGIT_ASKPASSentirely. This causesAuthentication failederrors every ~1 hour on permanent Windows nodes even though Jenkins correctly generates a fresh token.Root cause: PR #291 fixed token refresh for Linux static agents via
DelegatingGitHubAppCredentials. On Linux, Git always callsGIT_ASKPASSto get credentials. On Windows, Git checks Windows Credential Manager first — if a cached entry exists it never callsGIT_ASKPASS, so the expired cached token is used instead of the fresh one Jenkins prepared.Observed pattern from the field:
Fix
Inside
DelegatingGitHubAppCredentials.getPassword()(runs on the agent JVM), after obtaining a fresh token, detect Windows viaos.nameand run:This evicts the stale cached entry so Git falls through to
GIT_ASKPASSand uses the fresh token Jenkins provides. Both the modern (git:) and legacy (LegacyGenericCredential:) key formats are cleared to cover all Windows Git credential helper variants.The clearing is:
cmdkeyexits 1, silently ignored)Changes
GitHubAppCredentials.javaCLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE— public flag (defaulttrue), disableable from script console without redeploymentwindowsCredentialCleaner— replaceableConsumer<String>so tests never invokecmdkeyderiveGitHostFromApiUri()— mapshttps://api.github.com→github.com; passes GHE host through unchangedclearWindowsCredentialManagerCache()— clears both WCM key formatsDelegatingGitHubAppCredentials.apiUri— stored in plaintext (not sensitive) for host derivation on the agentDelegatingGitHubAppCredentials.getPassword()— calls cache clearing after token refresh, outside thesynchronizedblockGithubAppCredentialsWindowsAgentTest.java(new)deriveGitHostFromApiUri()(standard GitHub, GHE, port, malformed URI, empty)clearWindowsCredentialManagerCache()key format and flag behaviourwindowsCredentialCleaner— runs on any OS, never invokescmdkeyVerification
Tested on GKE Jenkins controller with a permanent Windows agent (Git 2.51.0.windows.2). Agent log after triggering token refresh:
Exit 0 = entry evicted. Exit 1 on
LegacyGenericCredential= key not present (normal — this Windows agent uses the moderngit:format only). Both checkouts succeeded after the 60s stale threshold was crossed.Backwards compatibility
os.namecheck fails — nocmdkeycall, zero changecmdkey /deleteis a no-op (exit 1)deriveGitHostFromApiUriextracts GHE host correctlycmdkeynot on PATHWARNING, build continuesGitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE=falsevia system property or script console