Skip to content

Commit 951c238

Browse files
pdavineni-sympdavineni-sym
committed
Fix GitHub App token expiry on Windows static agents
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: #1515
1 parent 41174c2 commit 951c238

2 files changed

Lines changed: 223 additions & 1 deletion

File tree

‎src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java‎

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@
2323
import java.io.IOException;
2424
import java.io.Serial;
2525
import java.io.Serializable;
26+
import java.net.URI;
2627
import java.security.GeneralSecurityException;
2728
import java.time.Duration;
2829
import java.time.Instant;
@@ -32,6 +33,7 @@
3233
import java.util.Optional;
3334
import java.util.concurrent.ConcurrentHashMap;
3435
import java.util.concurrent.TimeUnit;
36+
import java.util.function.Consumer;
3537
import java.util.logging.Level;
3638
import java.util.logging.Logger;
3739
import java.util.stream.Collectors;
@@ -95,6 +97,46 @@ public class GitHubAppCredentials extends BaseStandardCredentials implements Sta
9597
public static boolean ALLOW_UNSAFE_REPOSITORY_INFERENCE =
9698
Boolean.getBoolean(GitHubAppCredentials.class.getName() + ".ALLOW_UNSAFE_REPOSITORY_INFERENCE");
9799

100+
/**
101+
* On Windows agents, clears the Windows Credential Manager cache entry for the GitHub host
102+
* before each Git credential use. This prevents Git from serving an expired GitHub App
103+
* installation token that was cached by a previous build, and ensures Git falls through to
104+
* {@code GIT_ASKPASS} to receive the fresh token Jenkins is about to provide.
105+
*
106+
* <p>Disable only if the {@code cmdkey} invocations cause problems in your environment.
107+
* Non-final so it can be adjusted from the Jenkins script console if needed.
108+
*/
109+
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Non-final for script console override")
110+
public static boolean CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = Boolean.parseBoolean(
111+
System.getProperty(
112+
GitHubAppCredentials.class.getName() + ".CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE", "true"));
113+
114+
/**
115+
* Replaceable executor for Windows Credential Manager key deletion.
116+
* The string parameter is the credential key (e.g. {@code git:https://github.com}).
117+
* Non-final to allow replacement in tests.
118+
*/
119+
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Non-final for testing purposes")
120+
@Restricted(NoExternalUse.class)
121+
static Consumer<String> windowsCredentialCleaner = key -> {
122+
try {
123+
Process process =
124+
new ProcessBuilder("cmdkey", "/delete:" + key).redirectErrorStream(true).start();
125+
boolean finished = process.waitFor(5, TimeUnit.SECONDS);
126+
if (!finished) {
127+
process.destroyForcibly();
128+
LOGGER.log(Level.WARNING, "Timed out clearing Windows Credential Manager entry: {0}", key);
129+
} else {
130+
LOGGER.log(
131+
Level.FINE,
132+
"Cleared Windows Credential Manager entry: {0} (exit: {1})",
133+
new Object[] {key, process.exitValue()});
134+
}
135+
} catch (IOException | InterruptedException e) {
136+
LOGGER.log(Level.WARNING, "Failed to clear Windows Credential Manager entry: " + key, e);
137+
}
138+
};
139+
98140
@NonNull
99141
private final String appID;
100142

@@ -612,6 +654,45 @@ Object readResolve() {
612654
return this;
613655
}
614656

657+
/**
658+
* Derives the Git repository host from a GitHub API URI.
659+
*
660+
* <p>Returns {@code github.com} for the standard endpoint ({@code https://api.github.com}), or
661+
* the host component of the URI for GitHub Enterprise Server instances.
662+
*
663+
* @param apiUri the GitHub API URI (e.g. {@code https://api.github.com})
664+
* @return the corresponding Git repository host (e.g. {@code github.com})
665+
*/
666+
static String deriveGitHostFromApiUri(String apiUri) {
667+
try {
668+
String host = new URI(apiUri).getHost();
669+
if (host == null) {
670+
return "github.com";
671+
}
672+
return "api.github.com".equals(host) ? "github.com" : host;
673+
} catch (Exception e) {
674+
LOGGER.log(Level.FINE, "Could not parse API URI to derive git host: " + apiUri, e);
675+
return "github.com";
676+
}
677+
}
678+
679+
/**
680+
* Clears cached GitHub credentials from the Windows Credential Manager for the host
681+
* corresponding to {@code apiUri}.
682+
*
683+
* <p>Removes both the modern ({@code git:https://host}) and legacy
684+
* ({@code LegacyGenericCredential:https://host}) key formats used by the Windows git credential
685+
* helpers, so that Git falls through to {@code GIT_ASKPASS} and uses the fresh token Jenkins is
686+
* about to provide.
687+
*
688+
* @param apiUri the GitHub API URI used to derive the Git repository host
689+
*/
690+
static void clearWindowsCredentialManagerCache(String apiUri) {
691+
String httpsUrl = "https://" + deriveGitHostFromApiUri(apiUri);
692+
windowsCredentialCleaner.accept("git:" + httpsUrl);
693+
windowsCredentialCleaner.accept("LegacyGenericCredential:" + httpsUrl);
694+
}
695+
615696
/**
616697
* Ensures that the credentials state as serialized via Remoting to an agent calls back to the
617698
* controller. Benefits:
@@ -636,6 +717,8 @@ private static final class DelegatingGitHubAppCredentials extends BaseStandardCr
636717
implements StandardUsernamePasswordCredentials {
637718

638719
private final String appID;
720+
/** The GitHub API URI, used to derive the git host for Windows Credential Manager clearing. */
721+
private final String apiUri;
639722
/**
640723
* An encrypted form of all data needed to refresh the token. Used to prevent {@link GetToken}
641724
* from being abused by compromised build agents.
@@ -650,6 +733,7 @@ private static final class DelegatingGitHubAppCredentials extends BaseStandardCr
650733
super(onMaster.getScope(), onMaster.getId(), onMaster.getDescription());
651734
JenkinsJVM.checkJenkinsJVM();
652735
appID = onMaster.getAppID();
736+
apiUri = onMaster.actualApiUri();
653737
JSONObject j = new JSONObject();
654738
j.put("appID", appID);
655739
j.put("privateKey", onMaster.getPrivateKey().getPlainText());
@@ -706,6 +790,7 @@ public String getUsername() {
706790
public Secret getPassword() {
707791
JenkinsJVM.checkNotJenkinsJVM();
708792
try {
793+
final Secret token;
709794
synchronized (this) {
710795
try {
711796
if (cachedToken == null || cachedToken.isStale()) {
@@ -741,10 +826,22 @@ public Secret getPassword() {
741826
}
742827
}
743828
LOGGER.log(Level.FINEST, "Returned GitHub App Installation Token for app ID {0} on agent", appID);
829+
token = cachedToken.getToken();
830+
}
744831

745-
return cachedToken.getToken();
832+
// On Windows agents, evict the cached credential from Windows Credential Manager
833+
// so that Git does not serve the previously-cached (possibly expired) token to the
834+
// next Git operation instead of calling GIT_ASKPASS for the fresh token we just
835+
// obtained above. This is the Windows equivalent of the token-refresh fix on
836+
// Linux; see also CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE.
837+
if (CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE
838+
&& System.getProperty("os.name", "")
839+
.toLowerCase(Locale.ROOT)
840+
.startsWith("windows")) {
841+
clearWindowsCredentialManagerCache(apiUri);
746842
}
747843

844+
return token;
748845
} catch (IOException | InterruptedException x) {
749846
throw new RuntimeException(x);
750847
}

‎src/test/java/org/jenkinsci/plugins/github_branch_source/GithubAppCredentialsWindowsAgentTest.java‎

Lines changed: 125 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,125 @@
1+
package org.jenkinsci.plugins.github_branch_source;
2+
3+
import static org.hamcrest.MatcherAssert.assertThat;
4+
import static org.hamcrest.Matchers.contains;
5+
import static org.hamcrest.Matchers.empty;
6+
import static org.hamcrest.Matchers.is;
7+
8+
import java.util.ArrayList;
9+
import java.util.List;
10+
import java.util.function.Consumer;
11+
import org.junit.After;
12+
import org.junit.Before;
13+
import org.junit.Test;
14+
15+
/**
16+
* Unit tests for the Windows Credential Manager cache-clearing logic added to
17+
* {@link GitHubAppCredentials}.
18+
*
19+
* <p>These tests exercise the static helper methods
20+
* ({@link GitHubAppCredentials#deriveGitHostFromApiUri} and
21+
* {@link GitHubAppCredentials#clearWindowsCredentialManagerCache}) and verify that the right
22+
* credential keys are evicted. They run on any OS because the {@link
23+
* GitHubAppCredentials#windowsCredentialCleaner} field is replaced with a recording stub.
24+
*/
25+
public class GithubAppCredentialsWindowsAgentTest {
26+
27+
private Consumer<String> originalCleaner;
28+
private boolean originalClearFlag;
29+
private List<String> deletedKeys;
30+
31+
@Before
32+
public void setUp() {
33+
originalCleaner = GitHubAppCredentials.windowsCredentialCleaner;
34+
originalClearFlag = GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE;
35+
deletedKeys = new ArrayList<>();
36+
GitHubAppCredentials.windowsCredentialCleaner = deletedKeys::add;
37+
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = true;
38+
}
39+
40+
@After
41+
public void tearDown() {
42+
GitHubAppCredentials.windowsCredentialCleaner = originalCleaner;
43+
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = originalClearFlag;
44+
}
45+
46+
// -------------------------------------------------------------------------
47+
// deriveGitHostFromApiUri
48+
// -------------------------------------------------------------------------
49+
50+
@Test
51+
public void deriveGitHost_standardGitHub() {
52+
assertThat(GitHubAppCredentials.deriveGitHostFromApiUri("https://api.github.com"), is("github.com"));
53+
}
54+
55+
@Test
56+
public void deriveGitHost_githubEnterprise() {
57+
assertThat(
58+
GitHubAppCredentials.deriveGitHostFromApiUri("https://ghe.example.com/api/v3"),
59+
is("ghe.example.com"));
60+
}
61+
62+
@Test
63+
public void deriveGitHost_enterpriseWithPort() {
64+
assertThat(
65+
GitHubAppCredentials.deriveGitHostFromApiUri("https://github.corp.example.com:8443/api/v3"),
66+
is("github.corp.example.com"));
67+
}
68+
69+
@Test
70+
public void deriveGitHost_malformedUri_fallsBackToGithubCom() {
71+
assertThat(GitHubAppCredentials.deriveGitHostFromApiUri("not a uri ://???"), is("github.com"));
72+
}
73+
74+
@Test
75+
public void deriveGitHost_emptyString_fallsBackToGithubCom() {
76+
assertThat(GitHubAppCredentials.deriveGitHostFromApiUri(""), is("github.com"));
77+
}
78+
79+
// -------------------------------------------------------------------------
80+
// clearWindowsCredentialManagerCache – key format
81+
// -------------------------------------------------------------------------
82+
83+
@Test
84+
public void clearCache_standardGitHub_deletesExpectedKeys() {
85+
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://api.github.com");
86+
87+
assertThat(
88+
deletedKeys,
89+
contains("git:https://github.com", "LegacyGenericCredential:https://github.com"));
90+
}
91+
92+
@Test
93+
public void clearCache_githubEnterprise_deletesExpectedKeys() {
94+
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://ghe.example.com/api/v3");
95+
96+
assertThat(
97+
deletedKeys,
98+
contains(
99+
"git:https://ghe.example.com",
100+
"LegacyGenericCredential:https://ghe.example.com"));
101+
}
102+
103+
@Test
104+
public void clearCache_alwaysDeletesBothKeyFormats() {
105+
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://api.github.com");
106+
107+
assertThat("Both wincred and GCM key formats must be cleared", deletedKeys.size(), is(2));
108+
}
109+
110+
// -------------------------------------------------------------------------
111+
// CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE flag
112+
// -------------------------------------------------------------------------
113+
114+
@Test
115+
public void clearCache_flagDisabled_doesNotDeleteKeys() {
116+
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = false;
117+
118+
// Simulate what getPassword() does when the flag is false
119+
if (GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE) {
120+
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://api.github.com");
121+
}
122+
123+
assertThat(deletedKeys, is(empty()));
124+
}
125+
}

0 commit comments

Comments
 (0)