diff --git a/README.adoc b/README.adoc index 6ee678403c..0093632588 100644 --- a/README.adoc +++ b/README.adoc @@ -261,6 +261,13 @@ This setting can be helpful with link:https://plugins.jenkins.io/swarm/[Jenkins + Default is `false` so that `setsid` is not used. +maskUrlCredentials:: +When `org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials` is set to `true`, build log messages that contain URLs with credentials will have the first occurrence of credentials masked. URLs that appear in error messages are not masked to aid in troubleshooting. ++ +For example, a URL of `https://foo:bar@baz.quux/git/my-repo.git` will be masked as `https://xxxxx@baz.quux/git/my-repo.git`. ++ +Default is `false` so that credentials in URLs are not masked. + [#changelog] == Changelog in https://github.com/jenkinsci/git-client-plugin/releases[GitHub Releases] diff --git a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java index 5f38704bf5..5319942bd1 100644 --- a/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java +++ b/src/main/java/org/jenkinsci/plugins/gitclient/CliGitAPIImpl.java @@ -305,6 +305,30 @@ private void getGitVersion() { return gitVersion >= requestedVersion; } + private static final Pattern MASK_URL_CREDENTIALS_PATTERN = Pattern.compile("://[/]?[^/@]*@"); + private static final String MASK_URL_CREDENTIALS_REPLACE = "://xxxxx@"; + + /** + * Mask the first occurrence of credentials in the supplied URL. + * + * @param url the URL to mask + * @return the masked URL, or null if null was supplied + */ + public static String maskUrlCredentials(String url) { + return url != null + ? MASK_URL_CREDENTIALS_PATTERN.matcher(url).replaceFirst(MASK_URL_CREDENTIALS_REPLACE) + : null; + } + + /** + * Return the value of the org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials system property. + * + * @return true if maskUrlCredentials is enabled, false otherwise + */ + public static boolean getMaskUrlCredentials() { + return Boolean.parseBoolean(System.getProperty(CliGitAPIImpl.class.getName() + ".maskUrlCredentials", "false")); + } + /** * Compare the current cli git version with the required version. * Finds if the current cli git version is at-least the required version. @@ -343,6 +367,9 @@ protected CliGitAPIImpl(String gitExe, File workspace, TaskListener listener, En } launcher = new LocalLauncher(IGitAPI.verbose ? listener : TaskListener.NULL); + if (this.getMaskUrlCredentials()) { + this.listener.getLogger().println("Masking credentials in URLs"); + } } /** {@inheritDoc} */ @@ -568,7 +595,9 @@ public FetchCommand depth(Integer depth) { @Override public void execute() throws GitException, InterruptedException { - listener.getLogger().println("Fetching upstream changes from " + url); + // Mask credentials in URLs before writing to the build log, if requested + final String displayUrl = getMaskUrlCredentials() ? maskUrlCredentials(url.toString()) : url.toString(); + listener.getLogger().println("Fetching upstream changes from " + displayUrl); ArgumentListBuilder args = new ArgumentListBuilder(); args.add("fetch"); @@ -813,7 +842,10 @@ public void execute() throws GitException, InterruptedException { throw new IllegalArgumentException("Invalid repository " + url, e); } - listener.getLogger().println("Cloning repository " + url); + // Mask credentials in URLs before writing to the build log, if requested + final String displayUrl = + getMaskUrlCredentials() ? maskUrlCredentials(urIish.toString()) : urIish.toString(); + listener.getLogger().println("Cloning repository " + displayUrl); try { Util.deleteContentsRecursive(workspace); @@ -2808,7 +2840,10 @@ private String launchCommandIn(ArgumentListBuilder args, File workDir, EnvVars e args.prepend("setsid"); } int usedTimeout = timeout == null ? TIMEOUT : timeout; - listener.getLogger().println(" > " + command + TIMEOUT_LOG_PREFIX + usedTimeout); + + // Mask credentials in URLs before writing to the build log, if requested + final String displayCommand = getMaskUrlCredentials() ? maskUrlCredentials(command) : command; + listener.getLogger().println(" > " + displayCommand + TIMEOUT_LOG_PREFIX + usedTimeout); Launcher.ProcStarter p = launcher.launch().cmds(args.toCommandArray()).envs(freshEnv); diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientCloneTest.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientCloneTest.java index 281d867677..d71fa9b0d1 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientCloneTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientCloneTest.java @@ -429,6 +429,77 @@ public void test_clone_huge_timeout_logging() throws Exception { assertTimeout(testGitClient, "fetch", expectedValue); } + @Test + public void test_clone_maskUrlCredentials_enabled() throws Exception { + if (!gitImplName.equals("git")) { + return; + } + System.setProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials", "true"); + try { + final CliGitAPIImpl newTestGitClient = + new CliGitAPIImpl("git", repo.getRoot(), listener, new hudson.EnvVars()); + assertThat(newTestGitClient.getMaskUrlCredentials(), is(true)); + newTestGitClient + .clone_() + .url("https://foo:bar@localhost/git/my-repo.git") + .repositoryName("origin") + .execute(); + } catch (Exception e) { + System.out.println("(ignored) clone exception: " + e); + } finally { + System.clearProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials"); + } + assertThat(handler.containsMessageSubstring("Masking credentials in URLs"), is(true)); + assertThat(handler.containsMessageSubstring("https://xxxxx@localhost/git/my-repo.git"), is(true)); + } + + @Test + public void test_clone_maskUrlCredentials_disabled() throws Exception { + if (!gitImplName.equals("git")) { + return; + } + System.setProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials", "false"); + try { + final CliGitAPIImpl newTestGitClient = + new CliGitAPIImpl("git", repo.getRoot(), listener, new hudson.EnvVars()); + assertThat(newTestGitClient.getMaskUrlCredentials(), is(false)); + newTestGitClient + .clone_() + .url("https://foo:bar@localhost/git/my-repo.git") + .repositoryName("origin") + .execute(); + } catch (Exception e) { + System.out.println("(ignored) clone exception: " + e); + } finally { + System.clearProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials"); + } + assertThat(handler.containsMessageSubstring("Masking credentials in URLs"), is(false)); + assertThat(handler.containsMessageSubstring("https://xxxxx@localhost/git/my-repo.git"), is(false)); + assertThat(handler.containsMessageSubstring("https://foo:bar@localhost/git/my-repo.git"), is(true)); + } + + @Test + public void test_clone_maskUrlCredentials_unset() throws Exception { + if (!gitImplName.equals("git")) { + return; + } + System.clearProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials"); + final CliGitAPIImpl newTestGitClient = new CliGitAPIImpl("git", repo.getRoot(), listener, new hudson.EnvVars()); + assertThat(newTestGitClient.getMaskUrlCredentials(), is(false)); + try { + newTestGitClient + .clone_() + .url("https://foo:bar@localhost/git/my-repo.git") + .repositoryName("origin") + .execute(); + } catch (Exception e) { + System.out.println("(ignored) clone exception: " + e); + } + assertThat(handler.containsMessageSubstring("Masking credentials in URLs"), is(false)); + assertThat(handler.containsMessageSubstring("https://xxxxx@localhost/git/my-repo.git"), is(false)); + assertThat(handler.containsMessageSubstring("https://foo:bar@localhost/git/my-repo.git"), is(true)); + } + private void assertAlternatesFileExists() { final String alternates = ".git" + File.separator + "objects" + File.separator + "info" + File.separator + "alternates"; diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientFetchTest.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientFetchTest.java index f8631fef72..353496ee53 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientFetchTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientFetchTest.java @@ -594,6 +594,84 @@ public void test_fetch_noTags() throws Exception { assertThat("Tags have been found : " + tags, tags.isEmpty(), is(true)); } + @Test + public void test_fetch_maskUrlCredentials_enabled() throws Exception { + if (!gitImplName.equals("git")) { + return; + } + System.setProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials", "true"); + try { + final CliGitAPIImpl newTestGitClient = + new CliGitAPIImpl("git", repo.getRoot(), listener, new hudson.EnvVars()); + assertThat(newTestGitClient.getMaskUrlCredentials(), is(true)); + newTestGitClient.setRemoteUrl("origin", "https://foo:bar@localhost/git/my-repo.git"); + newTestGitClient + .fetch_() + .from( + new URIish("origin"), + Collections.singletonList(new RefSpec("refs/heads/*:refs/remotes/origin/*"))) + .execute(); + } catch (Exception e) { + System.out.println("(ignored) fetch exception: " + e); + } finally { + System.clearProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials"); + } + assertThat(handler.containsMessageSubstring("Masking credentials in URLs"), is(true)); + assertThat(handler.containsMessageSubstring("https://xxxxx@localhost/git/my-repo.git"), is(true)); + } + + @Test + public void test_fetch_maskUrlCredentials_disabled() throws Exception { + if (!gitImplName.equals("git")) { + return; + } + System.setProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials", "false"); + try { + final CliGitAPIImpl newTestGitClient = + new CliGitAPIImpl("git", repo.getRoot(), listener, new hudson.EnvVars()); + assertThat(newTestGitClient.getMaskUrlCredentials(), is(false)); + newTestGitClient.setRemoteUrl("origin", "https://foo:bar@localhost/git/my-repo.git"); + newTestGitClient + .fetch_() + .from( + new URIish("origin"), + Collections.singletonList(new RefSpec("refs/heads/*:refs/remotes/origin/*"))) + .execute(); + } catch (Exception e) { + System.out.println("(ignored) fetch exception: " + e); + } finally { + System.clearProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials"); + } + assertThat(handler.containsMessageSubstring("Masking credentials in URLs"), is(false)); + assertThat(handler.containsMessageSubstring("https://xxxxx@localhost/git/my-repo.git"), is(false)); + assertThat(handler.containsMessageSubstring("https://foo:bar@localhost/git/my-repo.git"), is(true)); + } + + @Test + public void test_fetch_maskUrlCredentials_unset() throws Exception { + if (!gitImplName.equals("git")) { + return; + } + System.clearProperty("org.jenkinsci.plugins.gitclient.CliGitAPIImpl.maskUrlCredentials"); + try { + final CliGitAPIImpl newTestGitClient = + new CliGitAPIImpl("git", repo.getRoot(), listener, new hudson.EnvVars()); + assertThat(newTestGitClient.getMaskUrlCredentials(), is(false)); + newTestGitClient.setRemoteUrl("origin", "https://foo:bar@localhost/git/my-repo.git"); + newTestGitClient + .fetch_() + .from( + new URIish("origin"), + Collections.singletonList(new RefSpec("refs/heads/*:refs/remotes/origin/*"))) + .execute(); + } catch (Exception e) { + System.out.println("(ignored) fetch exception: " + e); + } + assertThat(handler.containsMessageSubstring("Masking credentials in URLs"), is(false)); + assertThat(handler.containsMessageSubstring("https://xxxxx@localhost/git/my-repo.git"), is(false)); + assertThat(handler.containsMessageSubstring("https://foo:bar@localhost/git/my-repo.git"), is(true)); + } + /* JENKINS-33258 detected many calls to git rev-parse. This checks * those calls are not being made. The checkoutRandomBranch call * creates a branch with a random name. The later assertion checks that diff --git a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java index 51997bcdcf..314e80738c 100644 --- a/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java +++ b/src/test/java/org/jenkinsci/plugins/gitclient/GitClientTest.java @@ -3343,4 +3343,95 @@ public void test_git_branch_with_line_breaks_and_long_strings() throws Exception private boolean isWindows() { return File.pathSeparatorChar == ';'; } + + @Test + public void testMaskUrlCredentialsNoCredentials() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String url = "https://dev.baz/git/repo.git"; + final String expected = url; + assertThat(cliGitClient.maskUrlCredentials(url), is(expected)); + } + + @Test + public void testMaskUrlCredentialsNoCredentialSeparator() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String url = "https://quux@dev.baz/git/repo.git"; + final String expected = "https://xxxxx@dev.baz/git/repo.git"; + assertThat(cliGitClient.maskUrlCredentials(url), is(expected)); + } + + @Test + public void testMaskUrlCredentialsUsernameAndPassword() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String url = "https://foo:bar@dev.baz/git/repo.git"; + final String expected = "https://xxxxx@dev.baz/git/repo.git"; + assertThat(cliGitClient.maskUrlCredentials(url), is(expected)); + } + + @Test + public void testMaskUrlCredentialsUsernameOnly() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String url = "https://foo:@dev.baz/git/repo.git"; + final String expected = "https://xxxxx@dev.baz/git/repo.git"; + assertThat(cliGitClient.maskUrlCredentials(url), is(expected)); + } + + @Test + public void testMaskUrlCredentialsPasswordOnly() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String url = "https://:bar@dev.baz/git/repo.git"; + final String expected = "https://xxxxx@dev.baz/git/repo.git"; + assertThat(cliGitClient.maskUrlCredentials(url), is(expected)); + } + + @Test + public void testMaskUrlCredentialsNullUrl() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String url = null; + final String expected = null; + assertThat(cliGitClient.maskUrlCredentials(url), is(expected)); + } + + @Test + public void testMaskUrlCredentialsCommand() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String command = + "git fetch --no-tags --force --progress --depth=1 -- https://foo:bar@dev.baz/git/repo.git +refs/heads/main:refs/remotes/origin/main"; + final String expected = + "git fetch --no-tags --force --progress --depth=1 -- https://xxxxx@dev.baz/git/repo.git +refs/heads/main:refs/remotes/origin/main"; + assertThat(cliGitClient.maskUrlCredentials(command), is(expected)); + } + + @Test + public void testMaskUrlCredentialsCommandTwoUrls() throws Exception { + if (!gitImplName.equals("git") || !(gitClient instanceof CliGitAPIImpl)) { + return; + } + CliGitAPIImpl cliGitClient = (CliGitAPIImpl) gitClient; + final String command = "git config url.ssh://git@github.com/foobar.insteadof https://baz:qux@github.com/foobar"; + final String expected = + "git config url.ssh://xxxxx@github.com/foobar.insteadof https://baz:qux@github.com/foobar"; + assertThat(cliGitClient.maskUrlCredentials(command), is(expected)); + } }