Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import java.io.IOException;
import java.io.Serial;
import java.io.Serializable;
import java.net.URI;
import java.security.GeneralSecurityException;
import java.time.Duration;
import java.time.Instant;
Expand All @@ -32,6 +33,7 @@
import java.util.Optional;
import java.util.concurrent.ConcurrentHashMap;
import java.util.concurrent.TimeUnit;
import java.util.function.Consumer;
import java.util.logging.Level;
import java.util.logging.Logger;
import java.util.stream.Collectors;
Expand Down Expand Up @@ -96,6 +98,44 @@
public static boolean ALLOW_UNSAFE_REPOSITORY_INFERENCE =
Boolean.getBoolean(GitHubAppCredentials.class.getName() + ".ALLOW_UNSAFE_REPOSITORY_INFERENCE");

/**
* On Windows agents, clears the Windows Credential Manager cache entry for the GitHub host
* before each Git credential use. This prevents Git from serving an expired GitHub App
* installation token that was cached by a previous build, and ensures Git falls through to
* {@code GIT_ASKPASS} to receive the fresh token Jenkins is about to provide.
*
* <p>Disable only if the {@code cmdkey} invocations cause problems in your environment.
* Non-final so it can be adjusted from the Jenkins script console if needed.
*/
@SuppressFBWarnings(value = "MS_SHOULD_BE_FINAL", justification = "Non-final for script console override")
public static boolean CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = Boolean.parseBoolean(System.getProperty(
GitHubAppCredentials.class.getName() + ".CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE", "true"));

/**
* Replaceable executor for Windows Credential Manager key deletion.
* The string parameter is the credential key (e.g. {@code git:https://github.com}).
* Non-final to allow replacement in tests.
*/
@Restricted(NoExternalUse.class)
static Consumer<String> windowsCredentialCleaner = key -> {
try {
Process process = new ProcessBuilder("cmdkey", "/delete:" + key)
.redirectErrorStream(true)
.start();
boolean finished = process.waitFor(5, TimeUnit.SECONDS);
if (!finished) {
process.destroyForcibly();
LOGGER.log(Level.WARNING, "Timed out clearing Windows Credential Manager entry: {0}", key);
} else {
LOGGER.log(Level.FINE, "Cleared Windows Credential Manager entry: {0} (exit: {1})", new Object[] {
key, process.exitValue()
});
}
} catch (IOException | InterruptedException e) {
LOGGER.log(Level.WARNING, "Failed to clear Windows Credential Manager entry: " + key, e);
}
};

Check warning on line 137 in src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 122-137 are not covered by tests

@NonNull
private final String appID;

Expand Down Expand Up @@ -613,6 +653,45 @@
return this;
}

/**
* Derives the Git repository host from a GitHub API URI.
*
* <p>Returns {@code github.com} for the standard endpoint ({@code https://api.github.com}), or
* the host component of the URI for GitHub Enterprise Server instances.
*
* @param apiUri the GitHub API URI (e.g. {@code https://api.github.com})
* @return the corresponding Git repository host (e.g. {@code github.com})
*/
static String deriveGitHostFromApiUri(String apiUri) {
try {
String host = new URI(apiUri).getHost();
if (host == null) {
return "github.com";
}
return "api.github.com".equals(host) ? "github.com" : host;
} catch (Exception e) {
LOGGER.log(Level.FINE, "Could not parse API URI to derive git host: " + apiUri, e);
return "github.com";
}
}

/**
* Clears cached GitHub credentials from the Windows Credential Manager for the host
* corresponding to {@code apiUri}.
*
* <p>Removes both the modern ({@code git:https://host}) and legacy
* ({@code LegacyGenericCredential:https://host}) key formats used by the Windows git credential
* helpers, so that Git falls through to {@code GIT_ASKPASS} and uses the fresh token Jenkins is
* about to provide.
*
* @param apiUri the GitHub API URI used to derive the Git repository host
*/
static void clearWindowsCredentialManagerCache(String apiUri) {
String httpsUrl = "https://" + deriveGitHostFromApiUri(apiUri);
windowsCredentialCleaner.accept("git:" + httpsUrl);
windowsCredentialCleaner.accept("LegacyGenericCredential:" + httpsUrl);
}

/**
* Ensures that the credentials state as serialized via Remoting to an agent calls back to the
* controller. Benefits:
Expand All @@ -637,6 +716,8 @@
implements StandardUsernamePasswordCredentials {

private final String appID;
/** The GitHub API URI, used to derive the git host for Windows Credential Manager clearing. */
private final String apiUri;
/**
* An encrypted form of all data needed to refresh the token. Used to prevent {@link GetToken}
* from being abused by compromised build agents.
Expand All @@ -651,6 +732,7 @@
super(onMaster.getScope(), onMaster.getId(), onMaster.getDescription());
JenkinsJVM.checkJenkinsJVM();
appID = onMaster.getAppID();
apiUri = onMaster.actualApiUri();
JSONObject j = new JSONObject();
j.put("appID", appID);
j.put("privateKey", onMaster.getPrivateKey().getPlainText());
Expand Down Expand Up @@ -707,6 +789,7 @@
public Secret getPassword() {
JenkinsJVM.checkNotJenkinsJVM();
try {
final Secret token;
synchronized (this) {
try {
if (cachedToken == null || cachedToken.isStale()) {
Expand Down Expand Up @@ -742,10 +825,22 @@
}
}
LOGGER.log(Level.FINEST, "Returned GitHub App Installation Token for app ID {0} on agent", appID);
token = cachedToken.getToken();
}

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

return token;

Check warning on line 843 in src/main/java/org/jenkinsci/plugins/github_branch_source/GitHubAppCredentials.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 828-843 are not covered by tests
} catch (IOException | InterruptedException x) {
throw new RuntimeException(x);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -353,13 +353,19 @@ public void testProviderRefresh() throws Exception {
@Test
public void testAgentRefresh() throws Exception {
final long notStaleSeconds = GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS;
final boolean originalClearWindowsCache = GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE;
try {
appCredentials.setApiUri(githubApi.baseUrl());

// We want to demonstrate successful caching without waiting for a the default 1 minute
// Must set this to a large enough number to avoid flaky test
GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS = 10;

// Disable Windows Credential Manager cache clearing so its log messages do not
// interfere with the strict log-sequence assertions below. The clearing behaviour
// is tested separately in GithubAppCredentialsWindowsAgentTest.
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = false;

// Ensure we are working from sufficiently clean cache state
Thread.sleep(Duration.ofSeconds(GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS + 2)
.toMillis());
Expand Down Expand Up @@ -463,6 +469,7 @@ public void testAgentRefresh() throws Exception {
0, RequestPatternBuilder.newRequestPattern(RequestMethod.GET, urlPathEqualTo("/rate_limit")));
} finally {
GitHubAppCredentials.AppInstallationToken.NOT_STALE_MINIMUM_SECONDS = notStaleSeconds;
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = originalClearWindowsCache;
logRecorder.doClear();
}
}
Expand Down Expand Up @@ -509,11 +516,12 @@ private List<String> getOutputLines() {
result.addAll(agentLogs);
}

// sort the logs into chronological order
// then just format the message.
// sort the logs into chronological order, then just format the message.
// Agent JVM has its own static field copy (defaults true), so filter its wincred messages.
return result.stream()
.sorted(Comparator.comparingLong(LogRecord::getMillis))
.map(formatter::formatMessage)
.filter(msg -> !msg.startsWith("Cleared Windows Credential Manager"))
.collect(Collectors.toList());
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
package org.jenkinsci.plugins.github_branch_source;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.contains;
import static org.hamcrest.Matchers.empty;
import static org.hamcrest.Matchers.is;

import java.util.ArrayList;
import java.util.List;
import java.util.function.Consumer;
import org.junit.After;
import org.junit.Before;
import org.junit.Test;

/**
* Unit tests for the Windows Credential Manager cache-clearing logic added to
* {@link GitHubAppCredentials}.
*
* <p>These tests exercise the static helper methods
* ({@link GitHubAppCredentials#deriveGitHostFromApiUri} and
* {@link GitHubAppCredentials#clearWindowsCredentialManagerCache}) and verify that the right
* credential keys are evicted. They run on any OS because the {@link
* GitHubAppCredentials#windowsCredentialCleaner} field is replaced with a recording stub.
*/
public class GithubAppCredentialsWindowsAgentTest {

private Consumer<String> originalCleaner;
private boolean originalClearFlag;
private List<String> deletedKeys;

@Before
public void setUp() {
originalCleaner = GitHubAppCredentials.windowsCredentialCleaner;
originalClearFlag = GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE;
deletedKeys = new ArrayList<>();
GitHubAppCredentials.windowsCredentialCleaner = deletedKeys::add;
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = true;
}

@After
public void tearDown() {
GitHubAppCredentials.windowsCredentialCleaner = originalCleaner;
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = originalClearFlag;
}

// -------------------------------------------------------------------------
// deriveGitHostFromApiUri
// -------------------------------------------------------------------------

@Test
public void deriveGitHost_standardGitHub() {
assertThat(GitHubAppCredentials.deriveGitHostFromApiUri("https://api.github.com"), is("github.com"));
}

@Test
public void deriveGitHost_githubEnterprise() {
assertThat(
GitHubAppCredentials.deriveGitHostFromApiUri("https://ghe.example.com/api/v3"), is("ghe.example.com"));
}

@Test
public void deriveGitHost_enterpriseWithPort() {
assertThat(
GitHubAppCredentials.deriveGitHostFromApiUri("https://github.corp.example.com:8443/api/v3"),
is("github.corp.example.com"));
}

@Test
public void deriveGitHost_malformedUri_fallsBackToGithubCom() {
assertThat(GitHubAppCredentials.deriveGitHostFromApiUri("not a uri ://???"), is("github.com"));
}

@Test
public void deriveGitHost_emptyString_fallsBackToGithubCom() {
assertThat(GitHubAppCredentials.deriveGitHostFromApiUri(""), is("github.com"));
}

// -------------------------------------------------------------------------
// clearWindowsCredentialManagerCache – key format
// -------------------------------------------------------------------------

@Test
public void clearCache_standardGitHub_deletesExpectedKeys() {
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://api.github.com");

assertThat(deletedKeys, contains("git:https://github.com", "LegacyGenericCredential:https://github.com"));
}

@Test
public void clearCache_githubEnterprise_deletesExpectedKeys() {
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://ghe.example.com/api/v3");

assertThat(
deletedKeys,
contains("git:https://ghe.example.com", "LegacyGenericCredential:https://ghe.example.com"));
}

@Test
public void clearCache_alwaysDeletesBothKeyFormats() {
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://api.github.com");

assertThat("Both wincred and GCM key formats must be cleared", deletedKeys.size(), is(2));
}

// -------------------------------------------------------------------------
// CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE flag
// -------------------------------------------------------------------------

@Test
public void clearCache_flagDisabled_doesNotDeleteKeys() {
GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE = false;

// Simulate what getPassword() does when the flag is false
if (GitHubAppCredentials.CLEAR_WINDOWS_CREDENTIAL_MANAGER_CACHE) {
GitHubAppCredentials.clearWindowsCredentialManagerCache("https://api.github.com");
}

assertThat(deletedKeys, is(empty()));
}
}
Loading