Merge pull request #43 from jglick/empty-secret-JENKINS-41760

[JENKINS-41760] Suppress masking of blank secrets
jenkinsci · Aug 8, 2017 · 7aa3871 · 7aa3871
2 parents b141c6f + 3ab1e34
commit 7aa3871
Show file tree

Hide file tree

Showing 5 changed files with 41 additions and 7 deletions.
diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java
@@ -188,10 +188,12 @@ public static String getPatternStringForSecrets(Collection<String> secrets) {
         Collections.sort(sortedByLength, stringLengthComparator);
 
         for (String secret : sortedByLength) {
-            if (b.length() > 0) {
-                b.append('|');
+            if (!secret.isEmpty()) {
+                if (b.length() > 0) {
+                    b.append('|');
+                }
+                b.append(Pattern.quote(secret));
             }
-            b.append(Pattern.quote(secret));
         }
         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
@@ -174,9 +174,14 @@ private Object readResolve() throws ObjectStreamException {
             final Pattern p = Pattern.compile(pattern.getPlainText());
             return new LineTransformationOutputStream() {
                 @Override protected void eol(byte[] b, int len) throws IOException {
-                    Matcher m = p.matcher(new String(b, 0, len, charsetName));
-                    if (m.find()) {
-                        logger.write(m.replaceAll("****").getBytes(charsetName));
+                    if (!p.toString().isEmpty()) {
+                        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);

diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java
@@ -149,7 +149,7 @@ private static final class Filter extends ConsoleLogFilter {
                         p = getPatternForBuild(build);
                     }
 
-                    if (p != null) {
+                    if (p != null && !p.toString().isEmpty()) {
                         Matcher m = p.matcher(new String(b, 0, len, charsetName));
                         if (m.find()) {
                             logger.write(m.replaceAll("****").getBytes(charsetName));

diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java
@@ -428,6 +428,20 @@ public void testTrackingOfCredential() {
         });
     }
 
+    @Issue("JENKINS-41760")
+    @Test public void emptyOrBlankCreds() {
+        story.addStep(new Statement() {
+            @Override public void evaluate() throws Throwable {
+                WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p");
+                p.setDefinition(new CpsFlowDefinition("node {withCredentials([]) {echo 'normal output'}}", true));
+                story.j.assertLogContains("normal output", story.j.buildAndAssertSuccess(p));
+                CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), new StringCredentialsImpl(CredentialsScope.GLOBAL, "creds", null, Secret.fromString("")));
+                p.setDefinition(new CpsFlowDefinition("node {withCredentials([string(variable: 'SECRET', credentialsId: 'creds')]) {echo 'normal output'}}", true));
+                story.j.assertLogContains("normal output", story.j.buildAndAssertSuccess(p));
+            }
+        });
+    }
+
     private static Set<String> grep(File dir, String text) throws IOException {
         Set<String> matches = new TreeSet<String>();
         grep(dir, text, "", matches);

diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java
@@ -44,9 +44,12 @@
 import java.util.ArrayList;
 import java.util.Arrays;
 import java.util.Collections;
+import org.junit.ClassRule;
+import org.jvnet.hudson.test.BuildWatcher;
 
 public class SecretBuildWrapperTest {
 
+    @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher();
     @Rule public JenkinsRule r = new JenkinsRule();
 
     @Issue("JENKINS-24805")
@@ -96,4 +99,14 @@ public class SecretBuildWrapperTest {
         FreeStyleBuild b = r.buildAndAssertSuccess(f);
         r.assertLogContains("PASSES", b);
     }
+
+    @Issue("JENKINS-41760")
+    @Test public void emptySecret() throws Exception {
+        CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), new StringCredentialsImpl(CredentialsScope.GLOBAL, "creds", null, Secret.fromString("")));
+        FreeStyleProject p = r.createFreeStyleProject();
+        p.getBuildWrappersList().add(new SecretBuildWrapper(Collections.singletonList(new StringBinding("SECRET", "creds"))));
+        p.getBuildersList().add(Functions.isWindows() ? new BatchFile("echo PASSES") : new Shell("echo PASSES"));
+        r.assertLogContains("PASSES", r.buildAndAssertSuccess(p));
+    }
+
 }