From a47f4b8529fd216c3c797566aa969191834abd7a Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 31 Oct 2016 11:19:31 -0700 Subject: [PATCH 1/4] [JENKINS-24805] First work on masking secrets in freestyle logs. This still needs tests - I just want to make sure the approach is right. --- .../credentialsbinding/MultiBinding.java | 18 ++++ .../credentialsbinding/impl/BindingStep.java | 9 +- .../impl/SecretBuildWrapper.java | 97 ++++++++++++++++++- 3 files changed, 114 insertions(+), 10 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java index 73d37d05..51c6110b 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java @@ -38,12 +38,15 @@ import java.io.FileNotFoundException; import java.io.IOException; import java.io.Serializable; +import java.util.Collection; import java.util.Collections; import java.util.HashMap; import java.util.Map; import java.util.Set; +import java.util.regex.Pattern; import javax.annotation.Nonnull; +import hudson.util.Secret; import jenkins.model.Jenkins; import org.jenkinsci.plugins.credentialsbinding.impl.CredentialNotFoundException; import org.kohsuke.stapler.DataBoundConstructor; @@ -138,4 +141,19 @@ protected static final class NullUnbinder implements Unbinder { return (BindingDescriptor) super.getDescriptor(); } + /** + * Utility method for turning a collection of secret strings into a {@link Secret}. + * @param secrets A collection of secret strings + * @return A {@link Secret} generated from that collection. + */ + public static Secret getSecretForStrings(Collection secrets) { + StringBuilder b = new StringBuilder(); + for (String secret : secrets) { + if (b.length() > 0) { + b.append('|'); + } + b.append(Pattern.quote(secret)); + } + return Secret.fromString(b.toString()); + } } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java index ce9d3205..2a991953 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java @@ -136,14 +136,7 @@ private static final class Filter extends ConsoleLogFilter implements Serializab private String charsetName; Filter(Collection secrets, String charsetName) { - StringBuilder b = new StringBuilder(); - for (String secret : secrets) { - if (b.length() > 0) { - b.append('|'); - } - b.append(Pattern.quote(secret)); - } - pattern = Secret.fromString(b.toString()); + pattern = MultiBinding.getSecretForStrings(secrets); this.charsetName = charsetName; } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java index 921546ea..058ec0e0 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java @@ -26,48 +26,95 @@ import hudson.Extension; import hudson.Launcher; +import hudson.console.ConsoleLogFilter; +import hudson.console.LineTransformationOutputStream; import hudson.model.AbstractBuild; import hudson.model.AbstractProject; import hudson.model.BuildListener; +import hudson.model.Run; import hudson.tasks.BuildWrapper; import hudson.tasks.BuildWrapperDescriptor; import java.io.IOException; +import java.io.ObjectStreamException; +import java.io.OutputStream; +import java.io.Serializable; import java.util.ArrayList; +import java.util.Collection; import java.util.Collections; +import java.util.HashSet; import java.util.List; import java.util.Map; import java.util.Set; +import java.util.WeakHashMap; +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import hudson.util.Secret; +import org.apache.commons.codec.Charsets; import org.jenkinsci.plugins.credentialsbinding.MultiBinding; import org.kohsuke.stapler.DataBoundConstructor; +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + @SuppressWarnings({"rawtypes", "unchecked"}) // inherited from BuildWrapper public class SecretBuildWrapper extends BuildWrapper { private /*almost final*/ List> bindings; + private static Map, Collection> secretsForBuild = new WeakHashMap, Collection>(); + + /** + * Gets the {@link Pattern} for the secret values for a given build, if that build has secrets defined. If not, return + * null. + * @param build A non-null build. + * @return A compiled {@link Pattern} from the build's secret values, if the build has any. + */ + public static @Nullable Pattern getPatternForBuild(@Nonnull AbstractBuild build) { + if (secretsForBuild.containsKey(build)) { + return Pattern.compile(MultiBinding.getSecretForStrings(secretsForBuild.get(build)).getPlainText()); + } else { + return null; + } + } + @DataBoundConstructor public SecretBuildWrapper(List> bindings) { this.bindings = bindings == null ? Collections.>emptyList() : bindings; } - + public List> getBindings() { return bindings; } - @Override public Environment setUp(AbstractBuild build, final Launcher launcher, BuildListener listener) throws IOException, InterruptedException { + @Override + public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) throws IOException, InterruptedException, Run.RunnerAbortedException { + if (!bindings.isEmpty()) { + return new Filter(build.getCharset().name()).decorateLogger(build, logger); + } else { + return logger; + } + } + + @Override public Environment setUp(final AbstractBuild build, final Launcher launcher, BuildListener listener) throws IOException, InterruptedException { final List m = new ArrayList(); for (MultiBinding binding : bindings) { m.add(binding.bind(build, build.getWorkspace(), launcher, listener)); } + + secretsForBuild.put(build, new HashSet()); + return new Environment() { @Override public void buildEnvVars(Map env) { for (MultiBinding.MultiEnvironment e : m) { env.putAll(e.getValues()); + secretsForBuild.get(build).addAll(e.getValues().values()); } } @Override public boolean tearDown(AbstractBuild build, BuildListener listener) throws IOException, InterruptedException { for (MultiBinding.MultiEnvironment e : m) { e.getUnbinder().unbind(build, build.getWorkspace(), launcher, listener); } + secretsForBuild.remove(build); return true; } }; @@ -86,6 +133,52 @@ protected Object readResolve() { return this; } + /** Similar to {@code MaskPasswordsOutputStream}. */ + private static final class Filter extends ConsoleLogFilter implements Serializable { + + private static final long serialVersionUID = 1; + + private String charsetName; + + Filter(String charsetName) { + this.charsetName = charsetName; + } + + // To avoid de-serialization issues with newly added field (charsetName) + private Object readResolve() throws ObjectStreamException { + if (this.charsetName == null) { + this.charsetName = Charsets.UTF_8.name(); + } + return this; + } + + @Override public OutputStream decorateLogger(final AbstractBuild build, final OutputStream logger) throws IOException, InterruptedException { + return new LineTransformationOutputStream() { + Pattern p; + + @Override protected void eol(byte[] b, int len) throws IOException { + if (p == null) { + p = getPatternForBuild(build); + } + + if (p != null) { + Matcher m = p.matcher(new String(b, 0, len, charsetName)); + if (m.find()) { + logger.write(m.replaceAll("****").getBytes(charsetName)); + } else { + // Avoid byte → char → byte conversion unless we are actually doing something. + logger.write(b, 0, len); + } + } else { + // Avoid byte → char → byte conversion unless we are actually doing something. + logger.write(b, 0, len); + } + } + }; + } + + } + @Extension public static class DescriptorImpl extends BuildWrapperDescriptor { @Override public boolean isApplicable(AbstractProject item) { From 969f56f5cef7052c8860da163312c8f834a9bb1c Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 31 Oct 2016 11:46:06 -0700 Subject: [PATCH 2/4] Review comments, test --- .../impl/SecretBuildWrapper.java | 36 ++++----- .../impl/SecretBuildWrapperTest.java | 81 +++++++++++++++++++ 2 files changed, 96 insertions(+), 21 deletions(-) create mode 100644 src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java index 058ec0e0..0fcab4cd 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java @@ -49,20 +49,18 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; -import hudson.util.Secret; -import org.apache.commons.codec.Charsets; import org.jenkinsci.plugins.credentialsbinding.MultiBinding; import org.kohsuke.stapler.DataBoundConstructor; +import javax.annotation.CheckForNull; import javax.annotation.Nonnull; -import javax.annotation.Nullable; @SuppressWarnings({"rawtypes", "unchecked"}) // inherited from BuildWrapper public class SecretBuildWrapper extends BuildWrapper { private /*almost final*/ List> bindings; - private static Map, Collection> secretsForBuild = new WeakHashMap, Collection>(); + private final static Map, Collection> secretsForBuild = new WeakHashMap, Collection>(); /** * Gets the {@link Pattern} for the secret values for a given build, if that build has secrets defined. If not, return @@ -70,7 +68,7 @@ public class SecretBuildWrapper extends BuildWrapper { * @param build A non-null build. * @return A compiled {@link Pattern} from the build's secret values, if the build has any. */ - public static @Nullable Pattern getPatternForBuild(@Nonnull AbstractBuild build) { + public static @CheckForNull Pattern getPatternForBuild(@Nonnull AbstractBuild build) { if (secretsForBuild.containsKey(build)) { return Pattern.compile(MultiBinding.getSecretForStrings(secretsForBuild.get(build)).getPlainText()); } else { @@ -95,19 +93,25 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) thr } } - @Override public Environment setUp(final AbstractBuild build, final Launcher launcher, BuildListener listener) throws IOException, InterruptedException { + @Override public Environment setUp(AbstractBuild build, final Launcher launcher, BuildListener listener) throws IOException, InterruptedException { final List m = new ArrayList(); + + Set secrets = new HashSet(); + for (MultiBinding binding : bindings) { - m.add(binding.bind(build, build.getWorkspace(), launcher, listener)); + MultiBinding.MultiEnvironment e = binding.bind(build, build.getWorkspace(), launcher, listener); + m.add(e); + secrets.addAll(e.getValues().values()); } - secretsForBuild.put(build, new HashSet()); + if (!secrets.isEmpty()) { + secretsForBuild.put(build, secrets); + } return new Environment() { @Override public void buildEnvVars(Map env) { for (MultiBinding.MultiEnvironment e : m) { env.putAll(e.getValues()); - secretsForBuild.get(build).addAll(e.getValues().values()); } } @Override public boolean tearDown(AbstractBuild build, BuildListener listener) throws IOException, InterruptedException { @@ -134,24 +138,14 @@ protected Object readResolve() { } /** Similar to {@code MaskPasswordsOutputStream}. */ - private static final class Filter extends ConsoleLogFilter implements Serializable { - - private static final long serialVersionUID = 1; + private static final class Filter extends ConsoleLogFilter { - private String charsetName; + private final String charsetName; Filter(String charsetName) { this.charsetName = charsetName; } - // To avoid de-serialization issues with newly added field (charsetName) - private Object readResolve() throws ObjectStreamException { - if (this.charsetName == null) { - this.charsetName = Charsets.UTF_8.name(); - } - return this; - } - @Override public OutputStream decorateLogger(final AbstractBuild build, final OutputStream logger) throws IOException, InterruptedException { return new LineTransformationOutputStream() { Pattern p; diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java new file mode 100644 index 00000000..0142272e --- /dev/null +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java @@ -0,0 +1,81 @@ +/* + * The MIT License + * + * Copyright 2016 CloudBees inc. + * + * Permission is hereby granted, free of charge, to any person obtaining a copy + * of this software and associated documentation files (the "Software"), to deal + * in the Software without restriction, including without limitation the rights + * to use, copy, modify, merge, publish, distribute, sublicense, and/or sell + * copies of the Software, and to permit persons to whom the Software is + * furnished to do so, subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in + * all copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, + * FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE + * AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER + * LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, + * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN + * THE SOFTWARE. + */ + +package org.jenkinsci.plugins.credentialsbinding.impl; + +import com.cloudbees.plugins.credentials.CredentialsProvider; +import com.cloudbees.plugins.credentials.CredentialsScope; +import com.cloudbees.plugins.credentials.domains.Domain; +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; +import hudson.model.Item; +import hudson.tasks.Shell; +import hudson.util.Secret; +import org.jenkinsci.plugins.credentialsbinding.MultiBinding; +import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsRule; + +import java.util.Collections; + +public class SecretBuildWrapperTest { + + @Rule public JenkinsRule r = new JenkinsRule(); + + @Issue("JENKINS-24805") + @Test public void maskingFreeStyleSecrets() throws Exception { + String credentialsId_1 = "creds_1"; + String username_1 = "s3cr3t"; + String password_1 = "p4ss"; + StringCredentialsImpl c_1 = new StringCredentialsImpl(CredentialsScope.GLOBAL, credentialsId_1, "sample1", Secret.fromString(password_1)); + String credentialsId_2 = "creds_2"; + String username_2 = "s3cr3t0"; + String password_2 = "p4ss" + "EvenLonger"; + StringCredentialsImpl c_2 = new StringCredentialsImpl(CredentialsScope.GLOBAL, credentialsId_2, "sample2", Secret.fromString(password_2)); + + CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), c_1); + CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), c_2); + + SecretBuildWrapper bw_1 = new SecretBuildWrapper(Collections.>singletonList(new StringBinding("PASS_1", credentialsId_1))); + SecretBuildWrapper bw_2 = new SecretBuildWrapper(Collections.>singletonList(new StringBinding("PASS_2", credentialsId_2))); + + FreeStyleProject f = r.createFreeStyleProject(); + + f.setConcurrentBuild(true); + f.getBuildersList().add(new Shell("echo $PASS_1")); + f.getBuildersList().add(new Shell("echo $PASS_2")); + f.getBuildWrappersList().add(bw_1); + f.getBuildWrappersList().add(bw_2); + + r.configRoundtrip((Item)f); + + FreeStyleBuild b = r.buildAndAssertSuccess(f); + r.assertLogNotContains(password_1, b); + r.assertLogNotContains(password_2, b); + r.assertLogContains("echo ****", b); + } + +} From 1d4f80a4fb20b4cc3ef9d98e3ce031420d23fae7 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 31 Oct 2016 12:20:52 -0700 Subject: [PATCH 3/4] Review comments, test cleanup --- .../credentialsbinding/MultiBinding.java | 11 +++++--- .../credentialsbinding/impl/BindingStep.java | 2 +- .../impl/SecretBuildWrapper.java | 14 +++-------- .../impl/SecretBuildWrapperTest.java | 25 ++++++------------- 4 files changed, 18 insertions(+), 34 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java index 51c6110b..acb792a5 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java @@ -49,6 +49,8 @@ import hudson.util.Secret; import jenkins.model.Jenkins; import org.jenkinsci.plugins.credentialsbinding.impl.CredentialNotFoundException; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; import org.kohsuke.stapler.DataBoundConstructor; /** @@ -142,11 +144,12 @@ protected static final class NullUnbinder implements Unbinder { } /** - * Utility method for turning a collection of secret strings into a {@link Secret}. + * Utility method for turning a collection of secret strings into a single {@link String} for pattern compilation. * @param secrets A collection of secret strings - * @return A {@link Secret} generated from that collection. + * @return A {@link String} generated from that collection. */ - public static Secret getSecretForStrings(Collection secrets) { + @Restricted(NoExternalUse.class) + public static String getPatternStringForSecrets(Collection secrets) { StringBuilder b = new StringBuilder(); for (String secret : secrets) { if (b.length() > 0) { @@ -154,6 +157,6 @@ public static Secret getSecretForStrings(Collection secrets) { } b.append(Pattern.quote(secret)); } - return Secret.fromString(b.toString()); + return b.toString(); } } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java index 2a991953..882c6dba 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java @@ -136,7 +136,7 @@ private static final class Filter extends ConsoleLogFilter implements Serializab private String charsetName; Filter(Collection secrets, String charsetName) { - pattern = MultiBinding.getSecretForStrings(secrets); + pattern = Secret.fromString(MultiBinding.getPatternStringForSecrets(secrets)); this.charsetName = charsetName; } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java index 0fcab4cd..1d161a75 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java @@ -35,9 +35,7 @@ import hudson.tasks.BuildWrapper; import hudson.tasks.BuildWrapperDescriptor; import java.io.IOException; -import java.io.ObjectStreamException; import java.io.OutputStream; -import java.io.Serializable; import java.util.ArrayList; import java.util.Collection; import java.util.Collections; @@ -70,7 +68,7 @@ public class SecretBuildWrapper extends BuildWrapper { */ public static @CheckForNull Pattern getPatternForBuild(@Nonnull AbstractBuild build) { if (secretsForBuild.containsKey(build)) { - return Pattern.compile(MultiBinding.getSecretForStrings(secretsForBuild.get(build)).getPlainText()); + return Pattern.compile(MultiBinding.getPatternStringForSecrets(secretsForBuild.get(build))); } else { return null; } @@ -86,11 +84,7 @@ public List> getBindings() { @Override public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) throws IOException, InterruptedException, Run.RunnerAbortedException { - if (!bindings.isEmpty()) { - return new Filter(build.getCharset().name()).decorateLogger(build, logger); - } else { - return logger; - } + return new Filter(build.getCharset().name()).decorateLogger(build, logger); } @Override public Environment setUp(AbstractBuild build, final Launcher launcher, BuildListener listener) throws IOException, InterruptedException { @@ -104,9 +98,7 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) thr secrets.addAll(e.getValues().values()); } - if (!secrets.isEmpty()) { - secretsForBuild.put(build, secrets); - } + secretsForBuild.put(build, secrets); return new Environment() { @Override public void buildEnvVars(Map env) { diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java index 0142272e..93b3a678 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java @@ -32,7 +32,6 @@ import hudson.model.Item; import hudson.tasks.Shell; import hudson.util.Secret; -import org.jenkinsci.plugins.credentialsbinding.MultiBinding; import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; import org.junit.Rule; import org.junit.Test; @@ -47,34 +46,24 @@ public class SecretBuildWrapperTest { @Issue("JENKINS-24805") @Test public void maskingFreeStyleSecrets() throws Exception { - String credentialsId_1 = "creds_1"; - String username_1 = "s3cr3t"; - String password_1 = "p4ss"; - StringCredentialsImpl c_1 = new StringCredentialsImpl(CredentialsScope.GLOBAL, credentialsId_1, "sample1", Secret.fromString(password_1)); - String credentialsId_2 = "creds_2"; - String username_2 = "s3cr3t0"; - String password_2 = "p4ss" + "EvenLonger"; - StringCredentialsImpl c_2 = new StringCredentialsImpl(CredentialsScope.GLOBAL, credentialsId_2, "sample2", Secret.fromString(password_2)); + String firstCredentialsId = "creds_1"; + String firstPassword = "p4ss"; + StringCredentialsImpl firstCreds = new StringCredentialsImpl(CredentialsScope.GLOBAL, firstCredentialsId, "sample1", Secret.fromString(firstPassword)); - CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), c_1); - CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), c_2); + CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), firstCreds); - SecretBuildWrapper bw_1 = new SecretBuildWrapper(Collections.>singletonList(new StringBinding("PASS_1", credentialsId_1))); - SecretBuildWrapper bw_2 = new SecretBuildWrapper(Collections.>singletonList(new StringBinding("PASS_2", credentialsId_2))); + SecretBuildWrapper wrapper = new SecretBuildWrapper(Collections.singletonList(new StringBinding("PASS_1", firstCredentialsId))); FreeStyleProject f = r.createFreeStyleProject(); f.setConcurrentBuild(true); f.getBuildersList().add(new Shell("echo $PASS_1")); - f.getBuildersList().add(new Shell("echo $PASS_2")); - f.getBuildWrappersList().add(bw_1); - f.getBuildWrappersList().add(bw_2); + f.getBuildWrappersList().add(wrapper); r.configRoundtrip((Item)f); FreeStyleBuild b = r.buildAndAssertSuccess(f); - r.assertLogNotContains(password_1, b); - r.assertLogNotContains(password_2, b); + r.assertLogNotContains(firstPassword, b); r.assertLogContains("echo ****", b); } From 45487813c1acc730dd42ac903760489c45f1d821 Mon Sep 17 00:00:00 2001 From: Andrew Bayer Date: Mon, 31 Oct 2016 12:33:12 -0700 Subject: [PATCH 4/4] Make sure we handle the degenerate case of no bindings properly. --- .../impl/SecretBuildWrapper.java | 4 +++- .../impl/SecretBuildWrapperTest.java | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 1 deletion(-) diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java index 1d161a75..b74084c7 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java @@ -98,7 +98,9 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) thr secrets.addAll(e.getValues().values()); } - secretsForBuild.put(build, secrets); + if (!secrets.isEmpty()) { + secretsForBuild.put(build, secrets); + } return new Environment() { @Override public void buildEnvVars(Map env) { diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java index 93b3a678..938d0452 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java @@ -32,12 +32,14 @@ import hudson.model.Item; import hudson.tasks.Shell; import hudson.util.Secret; +import org.jenkinsci.plugins.credentialsbinding.MultiBinding; import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.Issue; import org.jvnet.hudson.test.JenkinsRule; +import java.util.ArrayList; import java.util.Collections; public class SecretBuildWrapperTest { @@ -67,4 +69,19 @@ public class SecretBuildWrapperTest { r.assertLogContains("echo ****", b); } + @Issue("JENKINS-24805") + @Test public void emptySecretsList() throws Exception { + SecretBuildWrapper wrapper = new SecretBuildWrapper(new ArrayList>()); + + FreeStyleProject f = r.createFreeStyleProject(); + + f.setConcurrentBuild(true); + f.getBuildersList().add(new Shell("echo PASSES")); + f.getBuildWrappersList().add(wrapper); + + r.configRoundtrip((Item)f); + + FreeStyleBuild b = r.buildAndAssertSuccess(f); + r.assertLogContains("PASSES", b); + } }