From 9cc0b870d2baf3260ad8bb4e49791f4a16761d2e Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 23 Jun 2021 12:38:43 -0400 Subject: [PATCH 1/3] Introduced `MaskingOutputStream` to DRY between Pipeline & freestyle --- .../credentialsbinding/impl/BindingStep.java | 20 +------- .../impl/SecretBuildWrapper.java | 25 +-------- .../masking/SecretPatterns.java | 51 +++++++++++++++++-- 3 files changed, 48 insertions(+), 48 deletions(-) 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 3a5da075..767a96f5 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java @@ -29,7 +29,6 @@ import hudson.FilePath; import hudson.Launcher; import hudson.console.ConsoleLogFilter; -import hudson.console.LineTransformationOutputStream; import hudson.model.AbstractBuild; import hudson.model.Run; import hudson.model.TaskListener; @@ -49,7 +48,6 @@ import java.util.List; import java.util.Map; import java.util.Set; -import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -219,23 +217,7 @@ private Object readResolve() throws ObjectStreamException { } @Override public OutputStream decorateLogger(AbstractBuild _ignore, final OutputStream logger) throws IOException, InterruptedException { - final Pattern p = Pattern.compile(pattern.getPlainText()); - return new LineTransformationOutputStream.Delegating(logger) { - @Override protected void eol(byte[] b, int len) throws IOException { - if (!p.toString().isEmpty()) { - Matcher m = p.matcher(new String(b, 0, len, charsetName)); - if (m.find()) { - out.write(m.replaceAll("****").getBytes(charsetName)); - } else { - // Avoid byte → char → byte conversion unless we are actually doing something. - out.write(b, 0, len); - } - } else { - // Avoid byte → char → byte conversion unless we are actually doing something. - out.write(b, 0, len); - } - } - }; + return new SecretPatterns.MaskingOutputStream(logger, Pattern.compile(pattern.getPlainText()), 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 68f0ff40..98400855 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java @@ -27,7 +27,6 @@ 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; @@ -43,7 +42,6 @@ import java.io.IOException; import java.io.OutputStream; import java.util.*; -import java.util.regex.Matcher; import java.util.regex.Pattern; @SuppressWarnings({"rawtypes", "unchecked"}) // inherited from BuildWrapper @@ -142,28 +140,7 @@ private static final class Filter extends ConsoleLogFilter { } @Override public OutputStream decorateLogger(final AbstractBuild build, final OutputStream logger) throws IOException, InterruptedException { - return new LineTransformationOutputStream.Delegating(logger) { - Pattern p; - - @Override protected void eol(byte[] b, int len) throws IOException { - if (p == null) { - p = getPatternForBuild(build); - } - - if (p != null && !p.toString().isEmpty()) { - Matcher m = p.matcher(new String(b, 0, len, charsetName)); - if (m.find()) { - out.write(m.replaceAll("****").getBytes(charsetName)); - } else { - // Avoid byte → char → byte conversion unless we are actually doing something. - out.write(b, 0, len); - } - } else { - // Avoid byte → char → byte conversion unless we are actually doing something. - out.write(b, 0, len); - } - } - + return new SecretPatterns.MaskingOutputStream(logger, getPatternForBuild(build), charsetName) { @Override public void close() throws IOException { super.close(); secretsForBuild.remove(build); diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java index 632ba9e3..f10a6753 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java @@ -24,14 +24,18 @@ package org.jenkinsci.plugins.credentialsbinding.masking; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; - -import javax.annotation.Nonnull; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; +import hudson.console.LineTransformationOutputStream; +import java.io.IOException; +import java.io.OutputStream; import java.util.Collection; import java.util.Comparator; +import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import org.kohsuke.accmod.Restricted; +import org.kohsuke.accmod.restrictions.NoExternalUse; @Restricted(NoExternalUse.class) public class SecretPatterns { @@ -48,7 +52,7 @@ public class SecretPatterns { * For example, {@code bash -x} will only quote arguments echoed when necessary. To avoid leaking the presence or * absence of quoting, the longer form is masked. */ - public static @Nonnull Pattern getAggregateSecretPattern(@Nonnull Collection inputs) { + public static @NonNull Pattern getAggregateSecretPattern(@NonNull Collection inputs) { String pattern = inputs.stream() .filter(input -> !input.isEmpty()) .flatMap(input -> @@ -60,4 +64,41 @@ public class SecretPatterns { .collect(Collectors.joining("|")); return Pattern.compile(pattern); } + + /** + * Delegating output stream that masks occurrences of a set of secrets. + */ + public static class MaskingOutputStream extends LineTransformationOutputStream.Delegating { + + private final @CheckForNull Pattern secretPattern; + private final @NonNull String charsetName; + + /** + * @param out the base output stream which will not be sent secrets + * @param secretPattern the result of {@link #getAggregateSecretPattern}, or null to just skip masking + * @param charsetName the character set to detect strings + */ + public MaskingOutputStream(@NonNull OutputStream out, @CheckForNull Pattern secretPattern, @NonNull String charsetName) { + super(out); + this.secretPattern = secretPattern; + this.charsetName = charsetName; + } + + @Override protected void eol(byte[] b, int len) throws IOException { + if (secretPattern != null && !secretPattern.toString().isEmpty()) { + Matcher m = secretPattern.matcher(new String(b, 0, len, charsetName)); + if (m.find()) { + out.write(m.replaceAll("****").getBytes(charsetName)); + } else { + // Avoid byte → char → byte conversion unless we are actually doing something. + out.write(b, 0, len); + } + } else { + // Avoid byte → char → byte conversion unless we are actually doing something. + out.write(b, 0, len); + } + } + + } + } From 958f3958526a9896580d75993514ba1ff200eaae Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 23 Jun 2021 12:43:34 -0400 Subject: [PATCH 2/3] Expose `SecretPatterns` as an API --- .../plugins/credentialsbinding/masking/SecretPatterns.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java index f10a6753..edc0d881 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java @@ -34,10 +34,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -import org.kohsuke.accmod.Restricted; -import org.kohsuke.accmod.restrictions.NoExternalUse; -@Restricted(NoExternalUse.class) public class SecretPatterns { private static final Comparator BY_LENGTH_DESCENDING = @@ -101,4 +98,6 @@ public MaskingOutputStream(@NonNull OutputStream out, @CheckForNull Pattern secr } + private SecretPatterns() {} + } From 6dc28f0f87353e53273bad9a3557d30461c51c0c Mon Sep 17 00:00:00 2001 From: Jesse Glick Date: Wed, 23 Jun 2021 13:09:11 -0400 Subject: [PATCH 3/3] `SecretBuildWrapper` needs to initialize its pattern lazily --- .../credentialsbinding/impl/BindingStep.java | 4 ++-- .../impl/SecretBuildWrapper.java | 4 ++-- .../masking/SecretPatterns.java | 23 +++++++++++-------- 3 files changed, 18 insertions(+), 13 deletions(-) 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 767a96f5..16f78107 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java @@ -216,8 +216,8 @@ private Object readResolve() throws ObjectStreamException { return this; } - @Override public OutputStream decorateLogger(AbstractBuild _ignore, final OutputStream logger) throws IOException, InterruptedException { - return new SecretPatterns.MaskingOutputStream(logger, Pattern.compile(pattern.getPlainText()), charsetName); + @Override public OutputStream decorateLogger(AbstractBuild _ignore, OutputStream logger) throws IOException, InterruptedException { + return new SecretPatterns.MaskingOutputStream(logger, () -> Pattern.compile(pattern.getPlainText()), 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 98400855..03dd53d9 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java @@ -139,8 +139,8 @@ private static final class Filter extends ConsoleLogFilter { this.charsetName = charsetName; } - @Override public OutputStream decorateLogger(final AbstractBuild build, final OutputStream logger) throws IOException, InterruptedException { - return new SecretPatterns.MaskingOutputStream(logger, getPatternForBuild(build), charsetName) { + @Override public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) throws IOException, InterruptedException { + return new SecretPatterns.MaskingOutputStream(logger, () -> getPatternForBuild(build), charsetName) { @Override public void close() throws IOException { super.close(); secretsForBuild.remove(build); diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java index edc0d881..f45a772f 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java @@ -31,6 +31,7 @@ import java.io.OutputStream; import java.util.Collection; import java.util.Comparator; +import java.util.function.Supplier; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; @@ -67,32 +68,36 @@ public class SecretPatterns { */ public static class MaskingOutputStream extends LineTransformationOutputStream.Delegating { - private final @CheckForNull Pattern secretPattern; + private final @NonNull Supplier secretPattern; private final @NonNull String charsetName; + private @CheckForNull Pattern p; /** * @param out the base output stream which will not be sent secrets - * @param secretPattern the result of {@link #getAggregateSecretPattern}, or null to just skip masking + * @param secretPattern a lazy computation of either the result of {@link #getAggregateSecretPattern}, or null to just skip masking * @param charsetName the character set to detect strings */ - public MaskingOutputStream(@NonNull OutputStream out, @CheckForNull Pattern secretPattern, @NonNull String charsetName) { + public MaskingOutputStream(@NonNull OutputStream out, @NonNull Supplier secretPattern, @NonNull String charsetName) { super(out); this.secretPattern = secretPattern; this.charsetName = charsetName; } @Override protected void eol(byte[] b, int len) throws IOException { - if (secretPattern != null && !secretPattern.toString().isEmpty()) { - Matcher m = secretPattern.matcher(new String(b, 0, len, charsetName)); + if (p == null) { + p = secretPattern.get(); + } + if (p == null || p.toString().isEmpty()) { + // Avoid byte → char → byte conversion unless we are actually doing something. + out.write(b, 0, len); + } else { + Matcher m = p.matcher(new String(b, 0, len, charsetName)); if (m.find()) { out.write(m.replaceAll("****").getBytes(charsetName)); } else { - // Avoid byte → char → byte conversion unless we are actually doing something. + // As above. out.write(b, 0, len); } - } else { - // Avoid byte → char → byte conversion unless we are actually doing something. - out.write(b, 0, len); } }