Use SecretPatterns API (#219)

README.md

@@ -1,6 +1,10 @@
-[![Build Status](https://ci.jenkins.io/buildStatus/icon?job=Plugins/hashicorp-vault-plugin/master)](https://ci.jenkins.io/job/Plugins/job/hashicorp-vault-plugin/job/master/)
 # Jenkins Vault Plugin
 
+[![Build Status](https://ci.jenkins.io/buildStatus/icon?job=Plugins/hashicorp-vault-plugin/master)](https://ci.jenkins.io/job/Plugins/job/hashicorp-vault-plugin/job/master/)
+[![Jenkins Plugin](https://img.shields.io/jenkins/plugin/v/hashicorp-vault-plugin.svg)](https://plugins.jenkins.io/hashicorp-vault-plugin)
+[![GitHub release](https://img.shields.io/github/release/jenkinsci/hashicorp-vault-plugin.svg?label=release)](https://github.com/jenkinsci/hashicorp-vault-plugin/releases/latest)
+[![Jenkins Plugin Installs](https://img.shields.io/jenkins/plugin/i/hashicorp-vault-plugin.svg?color=blue)](https://plugins.jenkins.io/hashicorp-vault-plugin)
+
 This plugin adds a build wrapper to set environment variables from a HashiCorp [Vault](https://www.vaultproject.io/) secret. Secrets are generally masked in the build log, so you can't accidentally print them.  
 It also has the ability to inject Vault credentials into a build pipeline or freestyle job for fine-grained vault interactions.
 

pom.xml

@@ -181,6 +181,11 @@
       <artifactId>mockito-core</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.mockito</groupId>
+      <artifactId>mockito-inline</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.jenkins-ci.plugins.workflow</groupId>
       <artifactId>workflow-job</artifactId>

src/main/java/com/datapipe/jenkins/vault/credentials/VaultTokenCredentialBinding.java

@@ -101,7 +101,10 @@ public MultiEnvironment bind(@NonNull Run<?, ?> build, FilePath workspace, Launc
         Map<String, String> m = new HashMap<>();
         m.put(addrVariable, vaultAddr);
         m.put(namespaceVariable, vaultNamespace);
-        m.put(tokenVariable, getToken(credentials));
+        String token = getToken(credentials);
+        // don't add null token variable, can cause NPE in places where credential bindings impls
+        // are not expecting null env var values.
+        m.put(tokenVariable, StringUtils.defaultString(token));
         return new MultiEnvironment(m);
     }
 

src/main/java/com/datapipe/jenkins/vault/log/MaskingConsoleLogFilter.java

@@ -1,17 +1,14 @@
 package com.datapipe.jenkins.vault.log;
 
 import hudson.console.ConsoleLogFilter;
-import hudson.console.LineTransformationOutputStream;
 import hudson.model.Run;
 import java.io.IOException;
 import java.io.OutputStream;
 import java.io.Serializable;
-import java.util.ArrayList;
-import java.util.Collection;
 import java.util.List;
-import java.util.regex.Matcher;
-import java.util.regex.Pattern;
-import org.apache.commons.lang.StringUtils;
+import java.util.Objects;
+import java.util.stream.Collectors;
+import org.jenkinsci.plugins.credentialsbinding.masking.SecretPatterns;
 
 /*The logic in this class is borrowed from https://github.com/jenkinsci/credentials-binding-plugin/*/
 public class MaskingConsoleLogFilter extends ConsoleLogFilter
@@ -20,7 +17,7 @@ public class MaskingConsoleLogFilter extends ConsoleLogFilter
     private static final long serialVersionUID = 1L;
 
     private final String charsetName;
-    private List<String> valuesToMask;
+    private final List<String> valuesToMask;
 
 
     public MaskingConsoleLogFilter(final String charsetName,
@@ -32,54 +29,23 @@ public MaskingConsoleLogFilter(final String charsetName,
     @Override
     public OutputStream decorateLogger(Run run,
         final OutputStream logger) throws IOException, InterruptedException {
-        return new LineTransformationOutputStream() {
-            Pattern p;
-
-            @Override
-            protected void eol(byte[] b, int len) throws IOException {
-                p = Pattern.compile(getPatternStringForSecrets(valuesToMask));
-                if (StringUtils.isBlank(p.pattern())) {
-                    logger.write(b, 0, len);
-                    return;
-                }
-                Matcher m = p.matcher(new String(b, 0, len, charsetName));
-                if (m.find()) {
-                    logger.write(m.replaceAll("****").getBytes(charsetName));
+        return new SecretPatterns.MaskingOutputStream(logger,
+            () -> {
+                // Only return a non-null pattern once there are secrets to mask. When a non-null
+                // pattern is returned it is cached and not supplied again. In cases like
+                // VaultBuildWrapper the secrets are added to the "valuesToMasker" list AFTER
+                // construction AND AFTER the decorateLogger method is initially called, therefore
+                // the Pattern should only be returned once the secrets have been made available.
+                // Not to mention it is also a slight optimization when there is are no secrets
+                // to mask.
+                List<String> values = valuesToMask.stream().filter(Objects::nonNull).collect(Collectors.toList());
+                if (!values.isEmpty()) {
+                    return SecretPatterns.getAggregateSecretPattern(values);
                 } else {
-                    // Avoid byte → char → byte conversion unless we are actually doing something.
-                    logger.write(b, 0, len);
+                    return null;
                 }
-            }
-        };
-    }
-
-    /**
-     * 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 String} generated from that collection.
-     */
-    public static String getPatternStringForSecrets(Collection<String> secrets) {
-        if (secrets == null) {
-            return "";
-        }
-        StringBuilder b = new StringBuilder();
-        List<String> sortedByLength = new ArrayList<>(secrets.size());
-        for (String secret : secrets) {
-            if (secret != null) {
-                sortedByLength.add(secret);
-            }
-        }
-        sortedByLength.sort((o1, o2) -> o2.length() - o1.length());
-
-        for (String secret : sortedByLength) {
-            if (b.length() > 0) {
-                b.append('|');
-            }
-            b.append(Pattern.quote(secret));
-        }
-        return b.toString();
+            },
+            charsetName);
     }
 
 }

src/test/java/com/datapipe/jenkins/vault/log/MaskingConsoleLogFilterSpec.java

@@ -1,5 +1,6 @@
 package com.datapipe.jenkins.vault.log;
 
+import hudson.ExtensionList;
 import hudson.model.Run;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
@@ -8,18 +9,33 @@
 import java.util.Arrays;
 import java.util.Collections;
 import java.util.List;
+import jenkins.model.Jenkins;
+import org.jenkinsci.plugins.credentialsbinding.masking.LiteralSecretPatternFactory;
+import org.jenkinsci.plugins.credentialsbinding.masking.SecretPatternFactory;
+import org.junit.Before;
 import org.junit.Test;
 import org.junit.runner.RunWith;
+import org.mockito.Answers;
+import org.mockito.Mock;
+import org.mockito.MockedStatic;
 import org.mockito.junit.MockitoJUnitRunner;
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.core.Is.is;
-import static org.junit.Assert.fail;
 import static org.mockito.Mockito.mock;
 
 @RunWith(MockitoJUnitRunner.class)
 public class MaskingConsoleLogFilterSpec {
 
+    private @Mock(answer = Answers.CALLS_REAL_METHODS) MockedStatic<SecretPatternFactory> secretPatternFactoryMockedStatic;
+
+    @Before
+    public void setup() {
+        ExtensionList<SecretPatternFactory> factories = ExtensionList.create((Jenkins) null, SecretPatternFactory.class);
+        factories.addAll(Collections.singletonList(new LiteralSecretPatternFactory()));
+        secretPatternFactoryMockedStatic.when(SecretPatternFactory::all).thenReturn(factories);
+    }
+
     @Test
     public void shouldCorrectlyMask() throws IOException, InterruptedException {
         MaskingConsoleLogFilter filter = new MaskingConsoleLogFilter(StandardCharsets.UTF_8.name(),
@@ -75,13 +91,17 @@ public void shouldCorrectlyHandleEmptyList() throws Exception {
 
     @Test
     public void shouldFilterNullSecrets() throws Exception {
-        List<String> secrets = Arrays.asList("secret", null, "another", null, "last");
-
-        try {
-            String pattern = MaskingConsoleLogFilter.getPatternStringForSecrets(secrets);
-            assertThat(pattern, is("\\Qanother\\E|\\Qsecret\\E|\\Qlast\\E"));
-        } catch (NullPointerException npe) {
-            fail("NullPointerException thrown");
-        }
+        List<String> secrets = Arrays.asList("secret", null, "another", null, "test");
+        MaskingConsoleLogFilter filter = new MaskingConsoleLogFilter(StandardCharsets.UTF_8.name(),
+            secrets);
+
+        ByteArrayOutputStream resultingLog = new ByteArrayOutputStream();
+
+        OutputStream maskingLogger = filter.decorateLogger(mock(Run.class), resultingLog);
+        maskingLogger.write("This is a test.\n".getBytes(StandardCharsets.UTF_8));
+        String[] resultingLines = resultingLog.toString(StandardCharsets.UTF_8.name()).split("\\n");
+
+        assertThat(resultingLines[0], is("This is a ****."));
     }
+
 }