diff --git a/.github/dependabot.yml b/.github/dependabot.yml index 8f9b151a..953356a4 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -7,3 +7,7 @@ updates: open-pull-requests-limit: 10 reviewers: - jglick +- package-ecosystem: github-actions + directory: / + schedule: + interval: weekly diff --git a/.github/release-drafter.yml b/.github/release-drafter.yml index f3f91103..0d0b1c99 100644 --- a/.github/release-drafter.yml +++ b/.github/release-drafter.yml @@ -1,2 +1 @@ _extends: .github -tag-template: credentials-binding-$NEXT_MINOR_VERSION diff --git a/.github/workflows/cd.yaml b/.github/workflows/cd.yaml new file mode 100644 index 00000000..0279984d --- /dev/null +++ b/.github/workflows/cd.yaml @@ -0,0 +1,15 @@ +# Note: additional setup is required, see https://www.jenkins.io/redirect/continuous-delivery-of-plugins + +name: cd +on: + workflow_dispatch: + check_run: + types: + - completed + +jobs: + maven-cd: + uses: jenkins-infra/github-reusable-workflows/.github/workflows/maven-cd.yml@v1 + secrets: + MAVEN_USERNAME: ${{ secrets.MAVEN_USERNAME }} + MAVEN_TOKEN: ${{ secrets.MAVEN_TOKEN }} diff --git a/.gitignore b/.gitignore index 0888f439..c52c86f9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,5 +1,5 @@ target/ -work*/ +work/ # IntelliJ project files *.iml diff --git a/.mvn/extensions.xml b/.mvn/extensions.xml index 94863e60..9ac2968b 100644 --- a/.mvn/extensions.xml +++ b/.mvn/extensions.xml @@ -2,6 +2,6 @@ io.jenkins.tools.incrementals git-changelist-maven-extension - 1.0-beta-7 + 1.4 diff --git a/.mvn/maven.config b/.mvn/maven.config index 2a0299c4..f7daf60d 100644 --- a/.mvn/maven.config +++ b/.mvn/maven.config @@ -1,2 +1,3 @@ -Pconsume-incrementals -Pmight-produce-incrementals +-Dchangelist.format=%d.v%s diff --git a/Jenkinsfile b/Jenkinsfile index 39b42fc8..651206a5 100644 --- a/Jenkinsfile +++ b/Jenkinsfile @@ -1 +1,4 @@ -buildPlugin(useAci: true) +buildPlugin(useContainerAgent: true, configurations: [ + [platform: 'linux', jdk: '17', jenkins: '2.343'], + [platform: 'windows', jdk: '8'], +]) diff --git a/README.md b/README.md index 07dec3bf..a3765734 100644 --- a/README.md +++ b/README.md @@ -46,14 +46,14 @@ withCredentials([usernamePassword(credentialsId: 'amazon', usernameVariable: 'US You should use a single quote (`'`) instead of a double quote (`"`) whenever you can. This is particularly important in Pipelines where a statement may be interpreted by both the Pipeline engine and an external interpreter, such as a Unix shell (`sh`) or Windows Command (`bat`) or Powershell (`ps`). This reduces complications with password masking and command processing. -The first step in the above example properly demonstrates this. -The next two steps use the basic Pipeline `echo` step. -The first one references the Groovy variable and needs no quotes. -The second one needs to use double quotes, so that the interpolation is performed in Groovy. +The first step in the above example properly demonstrates this. +It references an environment variable, so the single-quoted string passes its value unprocessed to the `sh` step, and the shell interprets `$PASSWORD`. +The next two steps use the basic Pipeline `echo` step. +The last one needs to use double quotes, so that the [string interpolation](https://en.wikipedia.org/wiki/String_interpolation) is performed by the Pipeline DSL. For more information, see the Pipeline step reference for [Credentials Binding Plugin](https://www.jenkins.io/doc/pipeline/steps/credentials-binding/). ## Changelog -See [GitHub Releases](https://github.com/jenkinsci/credentials-binding-plugin/releases) for new releases, -or the [old changelog](old-changelog.md) for history. +See [GitHub Releases](https://github.com/jenkinsci/credentials-binding-plugin/releases) for new releases (version 1.20 and newer), +or the [old changelog](old-changelog.md) for history (version 1.19 and earlier). diff --git a/pom.xml b/pom.xml index 44ac70fe..2e734a0b 100644 --- a/pom.xml +++ b/pom.xml @@ -5,24 +5,20 @@ org.jenkins-ci.plugins plugin - 4.15 + 4.48 credentials-binding - ${revision}${changelist} + ${changelist} hpi Credentials Binding Plugin - Allows credentials to be bound to environment variables for use from miscellaneous build steps. - - https://github.com/jenkinsci/credentials-binding-plugin + https://github.com/jenkinsci/${project.artifactId}-plugin - 1.25 - -SNAPSHOT - 2.176.4 - 8 - 2.23 + 999999-SNAPSHOT + 2.319.3 + jenkinsci/${project.artifactId}-plugin @@ -31,10 +27,10 @@ - - scm:git:git://github.com/jenkinsci/${project.artifactId}-plugin.git - scm:git:git@github.com:jenkinsci/${project.artifactId}-plugin.git - https://github.com/jenkinsci/${project.artifactId}-plugin + + scm:git:https://github.com/${gitHubRepo} + scm:git:https://github.com/${gitHubRepo} + https://github.com/${gitHubRepo} ${scmTag} @@ -55,8 +51,8 @@ io.jenkins.tools.bom - bom-2.176.x - 16 + bom-2.319.x + 1643.v1cffef51df73 import pom @@ -74,7 +70,6 @@ org.jenkins-ci.plugins.workflow workflow-step-api - ${workflow-step-api.version} org.jenkins-ci.plugins @@ -113,7 +108,6 @@ org.jenkins-ci.plugins.workflow workflow-step-api - ${workflow-step-api.version} tests test @@ -131,13 +125,13 @@ org.jenkins-ci.plugins authorize-project - 1.3.0 + 1.4.0 test org.xmlunit xmlunit-matchers - 2.8.2 + 2.9.1 test diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/Binding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/Binding.java index e5776bf8..8f720e65 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/Binding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/Binding.java @@ -26,6 +26,8 @@ import com.cloudbees.plugins.credentials.common.StandardCredentials; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import edu.umd.cs.findbugs.annotations.SuppressFBWarnings; import hudson.FilePath; import hudson.Launcher; @@ -34,9 +36,6 @@ import hudson.model.BuildListener; import java.io.IOException; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - import hudson.model.Run; import hudson.model.TaskListener; import java.util.Collections; @@ -93,7 +92,7 @@ public interface Environment { @Deprecated @SuppressWarnings("rawtypes") - public Environment bind(@Nonnull final AbstractBuild build, final Launcher launcher, final BuildListener listener) throws IOException, InterruptedException { + public Environment bind(@NonNull final AbstractBuild build, final Launcher launcher, final BuildListener listener) throws IOException, InterruptedException { final SingleEnvironment e = bindSingle(build, build.getWorkspace(), launcher, listener); return new Environment() { @Override public String value() { @@ -113,10 +112,10 @@ public Environment bind(@Nonnull final AbstractBuild build, final Launcher launc * @param listener The task listener. Cannot be null. * @return The configured {@link SingleEnvironment} */ - public /* abstract */SingleEnvironment bindSingle(@Nonnull Run build, + public /* abstract */SingleEnvironment bindSingle(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException, InterruptedException { if (Util.isOverridden(Binding.class, getClass(), "bind", AbstractBuild.class, Launcher.class, BuildListener.class) && build instanceof AbstractBuild && listener instanceof BuildListener) { Environment e = bind((AbstractBuild) build, launcher, (BuildListener) listener); return new SingleEnvironment(e.value(), new UnbinderWrapper(e)); @@ -134,29 +133,29 @@ private static class UnbinderWrapper implements Unbinder { UnbinderWrapper(Environment e) { this.e = e; } - @Override public void unbind(@Nonnull Run build, + @Override public void unbind(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException, InterruptedException { e.unbind(); } } - @Override public final MultiEnvironment bind(@Nonnull Run build, + @Override public final MultiEnvironment bind(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException, InterruptedException { SingleEnvironment single = bindSingle(build, workspace, launcher, listener); return new MultiEnvironment(Collections.singletonMap(variable, single.value), single.unbinder); } - @Override public final Set variables() { + @Override public final Set variables(@NonNull Run build) { return Collections.singleton(variable); } @Deprecated - protected final @Nonnull C getCredentials(@Nonnull AbstractBuild build) throws IOException { + protected final @NonNull C getCredentials(@NonNull AbstractBuild build) throws IOException { return super.getCredentials(build); } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/BindingDescriptor.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/BindingDescriptor.java index d89bdd1a..8bf3c80d 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/BindingDescriptor.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/BindingDescriptor.java @@ -28,7 +28,7 @@ import com.cloudbees.plugins.credentials.CredentialsProvider; import com.cloudbees.plugins.credentials.common.AbstractIdCredentialsListBoxModel; import com.cloudbees.plugins.credentials.common.StandardCredentials; -import com.cloudbees.plugins.credentials.domains.DomainRequirement; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.model.Descriptor; import hudson.model.Item; import hudson.security.ACL; @@ -60,12 +60,14 @@ public ListBoxModel doFillCredentialsIdItems(@AncestorInPath Item owner) { // when configuring the job, you only want those credentials that are available to ACL.SYSTEM selectable // as we cannot select from a user's credentials unless they are the only user submitting the build // (which we cannot assume) thus ACL.SYSTEM is correct here. - return new Model().withAll(CredentialsProvider.lookupCredentials(type(), owner, ACL.SYSTEM, Collections.emptyList())); + return new Model().withAll(CredentialsProvider.lookupCredentials(type(), owner, ACL.SYSTEM, Collections.emptyList())); } private final class Model extends AbstractIdCredentialsListBoxModel { - @Override protected String describe(C c) { + @NonNull + @Override + protected String describe(@NonNull C c) { return CredentialsNameProvider.name(c); } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java index b4f1f6d7..16411e23 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/MultiBinding.java @@ -27,24 +27,25 @@ import com.cloudbees.plugins.credentials.CredentialsProvider; import com.cloudbees.plugins.credentials.common.IdCredentials; import com.cloudbees.plugins.credentials.common.StandardCredentials; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import hudson.ExtensionPoint; import hudson.FilePath; import hudson.Launcher; +import hudson.Util; import hudson.model.AbstractDescribableImpl; import hudson.model.Descriptor; import hudson.model.Run; import hudson.model.TaskListener; -import java.io.FileNotFoundException; import java.io.IOException; import java.io.Serializable; import java.util.Collections; import java.util.LinkedHashMap; import java.util.Map; import java.util.Set; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; import jenkins.model.Jenkins; +import org.apache.commons.collections.CollectionUtils; import org.jenkinsci.plugins.credentialsbinding.impl.CredentialNotFoundException; import org.kohsuke.stapler.DataBoundConstructor; @@ -72,20 +73,57 @@ public final String getCredentialsId() { /** Result of {@link #bind}. */ public static final class MultiEnvironment implements Serializable { - private final Map values; + private static final long serialVersionUID = 1; + + @Deprecated + private transient Map values; + private Map secretValues; + private Map publicValues; private final Unbinder unbinder; - public MultiEnvironment(Map values) { - this(values, new NullUnbinder()); + public MultiEnvironment(Map secretValues) { + this(secretValues, Collections.emptyMap()); } - public MultiEnvironment(Map values, Unbinder unbinder) { - this.values = new LinkedHashMap<>(values); + public MultiEnvironment(Map secretValues, Map publicValues) { + this(secretValues, publicValues, new NullUnbinder()); + } + + public MultiEnvironment(Map secretValues, Unbinder unbinder) { + this(secretValues, Collections.emptyMap(), unbinder); + } + + public MultiEnvironment(Map secretValues, Map publicValues, Unbinder unbinder) { + this.values = null; + this.secretValues = new LinkedHashMap<>(secretValues); + this.publicValues = new LinkedHashMap<>(publicValues); + if (!CollectionUtils.intersection(secretValues.keySet(), publicValues.keySet()).isEmpty()) { + throw new IllegalArgumentException("Cannot use the same key in both secretValues and publicValues"); + } this.unbinder = unbinder; } + // To avoid de-serialization issues with newly added field (secretValues, publicValues) + private Object readResolve() { + if (values != null) { + secretValues = values; + publicValues = Collections.emptyMap(); + values = null; + } + return this; + } + + @Deprecated public Map getValues() { - return Collections.unmodifiableMap(values); + return Collections.unmodifiableMap(secretValues); + } + + public Map getSecretValues() { + return Collections.unmodifiableMap(secretValues); + } + + public Map getPublicValues() { + return Collections.unmodifiableMap(publicValues); } public Unbinder getUnbinder() { @@ -103,19 +141,19 @@ public interface Unbinder extends Serializable { * @param launcher The launcher - can be null if {@link BindingDescriptor#requiresWorkspace()} is false. * @param listener The task listener. Cannot be null. */ - void unbind(@Nonnull Run build, + void unbind(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException; + @NonNull TaskListener listener) throws IOException, InterruptedException; } /** No-op callback. */ protected static final class NullUnbinder implements Unbinder { private static final long serialVersionUID = 1; - @Override public void unbind(@Nonnull Run build, + @Override public void unbind(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException {} + @NonNull TaskListener listener) {} } /** @@ -126,21 +164,34 @@ protected static final class NullUnbinder implements Unbinder { * @param listener The task listener. Cannot be null. * @return The configured {@link MultiEnvironment} */ - public abstract MultiEnvironment bind(@Nonnull Run build, + public abstract MultiEnvironment bind(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException; + @NonNull TaskListener listener) throws IOException, InterruptedException; - /** Defines keys expected to be set in {@link MultiEnvironment#getValues}, particularly any that might be sensitive. */ - public abstract Set variables(); + /** + * @deprecated override {@link #variables(Run)} + */ + public Set variables() { + return Collections.emptySet(); + } + + /** Defines keys expected to be set in {@link MultiEnvironment#getSecretValues()}, particularly any that might be sensitive. */ + public /*abstract*/ Set variables(@NonNull Run build) throws CredentialNotFoundException { + if (Util.isOverridden(MultiBinding.class, getClass(), "variables")) { + return variables(); + } else { + throw new AbstractMethodError("Implement variables"); + } + } /** * Looks up the actual credentials. * @param build the build. * @return the credentials - * @throws FileNotFoundException if the credentials could not be found (for convenience, rather than returning null) + * @throws CredentialNotFoundException if the credentials could not be found (for convenience, rather than returning null) */ - protected final @Nonnull C getCredentials(@Nonnull Run build) throws IOException { + protected final @NonNull C getCredentials(@NonNull Run build) throws CredentialNotFoundException { IdCredentials cred = CredentialsProvider.findCredentialById(credentialsId, IdCredentials.class, build); if (cred==null) throw new CredentialNotFoundException("Could not find credentials entry with ID '" + credentialsId + "'"); @@ -151,7 +202,7 @@ public abstract MultiEnvironment bind(@Nonnull Run build, } - Descriptor expected = Jenkins.getActiveInstance().getDescriptor(type()); + Descriptor expected = Jenkins.get().getDescriptor(type()); throw new CredentialNotFoundException("Credentials '"+credentialsId+"' is of type '"+ cred.getDescriptor().getDisplayName()+"' where '"+ (expected!=null ? expected.getDisplayName() : type().getName())+ diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/AbstractOnDiskBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/AbstractOnDiskBinding.java index cccaee1d..65dfac9c 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/AbstractOnDiskBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/AbstractOnDiskBinding.java @@ -2,8 +2,7 @@ import java.io.IOException; -import javax.annotation.Nonnull; - +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.credentialsbinding.Binding; import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor; @@ -28,10 +27,10 @@ protected AbstractOnDiskBinding(String variable, String credentialsId) { } @Override - public final SingleEnvironment bindSingle(@Nonnull Run build, + public final SingleEnvironment bindSingle(@NonNull Run build, FilePath workspace, Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException, InterruptedException { if (workspace == null) { throw new IllegalArgumentException("This Binding implementation requires a non-null workspace"); } 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 b2120403..d57589a5 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java @@ -24,20 +24,20 @@ package org.jenkinsci.plugins.credentialsbinding.impl; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.EnvVars; import hudson.Extension; 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; import hudson.util.Secret; -import java.io.IOException; import java.io.ObjectStreamException; import java.io.OutputStream; import java.io.Serializable; +import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.Arrays; import java.util.Collection; @@ -48,11 +48,9 @@ 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; -import org.apache.commons.codec.Charsets; import org.jenkinsci.plugins.credentialsbinding.MultiBinding; import org.jenkinsci.plugins.credentialsbinding.masking.SecretPatterns; import org.jenkinsci.plugins.workflow.steps.AbstractStepExecutionImpl; @@ -67,8 +65,6 @@ import org.jenkinsci.plugins.workflow.steps.StepExecution; import org.kohsuke.stapler.DataBoundConstructor; -import javax.annotation.Nonnull; - /** * Workflow step to bind credentials. */ @@ -86,7 +82,7 @@ public List getBindings() { } @Override - public StepExecution start(StepContext context) throws Exception { + public StepExecution start(StepContext context) { return new Execution2(this, context); } @@ -96,7 +92,7 @@ private static final class Execution extends AbstractStepExecutionImpl { private static final long serialVersionUID = 1; - @Override public boolean start() throws Exception { + @Override public boolean start() { throw new AssertionError(); } @@ -108,12 +104,12 @@ private static final class Execution2 extends GeneralNonBlockingStepExecution { private transient BindingStep step; - Execution2(@Nonnull BindingStep step, StepContext context) { + Execution2(@NonNull BindingStep step, StepContext context) { super(context); this.step = step; } - @Override public boolean start() throws Exception { + @Override public boolean start() { run(this::doStart); return false; } @@ -125,7 +121,8 @@ private void doStart() throws Exception { FilePath workspace = getContext().get(FilePath.class); Launcher launcher = getContext().get(Launcher.class); - Map overrides = new LinkedHashMap<>(); + Map secretOverrides = new LinkedHashMap<>(); + Map publicOverrides = new LinkedHashMap<>(); List unbinders = new ArrayList<>(); for (MultiBinding binding : step.bindings) { if (binding.getDescriptor().requiresWorkspace() && @@ -134,17 +131,18 @@ private void doStart() throws Exception { } MultiBinding.MultiEnvironment environment = binding.bind(run, workspace, launcher, listener); unbinders.add(environment.getUnbinder()); - overrides.putAll(environment.getValues()); + secretOverrides.putAll(environment.getSecretValues()); + publicOverrides.putAll(environment.getPublicValues()); } - if (!overrides.isEmpty()) { - boolean unix = launcher != null ? launcher.isUnix() : true; - listener.getLogger().println("Masking supported pattern matches of " + overrides.keySet().stream().map( + if (!secretOverrides.isEmpty()) { + boolean unix = launcher == null || launcher.isUnix(); + listener.getLogger().println("Masking supported pattern matches of " + secretOverrides.keySet().stream().map( v -> unix ? "$" + v : "%" + v + "%" ).collect(Collectors.joining(" or "))); } getContext().newBodyInvoker(). - withContext(EnvironmentExpander.merge(getContext().get(EnvironmentExpander.class), new Overrider(overrides))). - withContext(BodyInvoker.mergeConsoleLogFilters(getContext().get(ConsoleLogFilter.class), new Filter(overrides.values(), run.getCharset().name()))). + withContext(EnvironmentExpander.merge(getContext().get(EnvironmentExpander.class), new Overrider(secretOverrides, publicOverrides))). + withContext(BodyInvoker.mergeConsoleLogFilters(getContext().get(ConsoleLogFilter.class), new Filter(secretOverrides.values(), run.getCharset().name()))). withCallback(new Callback2(unbinders)). start(); } @@ -171,23 +169,37 @@ private static final class Overrider extends EnvironmentExpander { private static final long serialVersionUID = 1; - private final Map overrides = new HashMap(); + private final Map overrides = new HashMap<>(); + private Map publicOverrides; - Overrider(Map overrides) { + Overrider(Map overrides, Map publicOverrides) { for (Map.Entry override : overrides.entrySet()) { this.overrides.put(override.getKey(), Secret.fromString(override.getValue())); } + this.publicOverrides = publicOverrides; } - @Override public void expand(EnvVars env) throws IOException, InterruptedException { + @Override public void expand(@NonNull EnvVars env) { for (Map.Entry override : overrides.entrySet()) { env.override(override.getKey(), override.getValue().getPlainText()); } + for (Map.Entry override : publicOverrides.entrySet()) { + env.override(override.getKey(), override.getValue()); + } } - @Override public Set getSensitiveVariables() { + @NonNull + @Override + public Set getSensitiveVariables() { return Collections.unmodifiableSet(overrides.keySet()); } + + private Object readResolve() { + if (publicOverrides == null) { + publicOverrides = new HashMap<>(); + } + return this; + } } /** Similar to {@code MaskPasswordsOutputStream}. */ @@ -206,36 +218,13 @@ private static final class Filter extends ConsoleLogFilter implements Serializab // 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(); + this.charsetName = StandardCharsets.UTF_8.name(); } return this; } - @Override public OutputStream decorateLogger(AbstractBuild _ignore, final OutputStream logger) throws IOException, InterruptedException { - final Pattern p = Pattern.compile(pattern.getPlainText()); - return new LineTransformationOutputStream() { - @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()) { - 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); - } - } - @Override public void flush() throws IOException { - logger.flush(); - } - @Override public void close() throws IOException { - super.close(); - logger.close(); - } - }; + @Override public OutputStream decorateLogger(AbstractBuild _ignore, OutputStream logger) { + return new SecretPatterns.MaskingOutputStream(logger, () -> Pattern.compile(pattern.getPlainText()), charsetName); } } @@ -277,7 +266,9 @@ private static final class Callback extends BodyExecutionCallback.TailCall { return "withCredentials"; } - @Override public String getDisplayName() { + @NonNull + @Override + public String getDisplayName() { return "Bind credentials to variables"; } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding.java index e2cd2b80..cd2093e9 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding.java @@ -9,9 +9,8 @@ import java.util.Map; import java.util.Set; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; - +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import org.jenkinsci.Symbol; import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor; import org.jenkinsci.plugins.credentialsbinding.MultiBinding; @@ -59,8 +58,7 @@ public void setAliasVariable(String aliasVariable) { private String aliasVariable; @DataBoundConstructor - @CheckForNull - public CertificateMultiBinding(@Nonnull String keystoreVariable, String credentialsId) { + public CertificateMultiBinding(@NonNull String keystoreVariable, String credentialsId) { super(credentialsId); this.keystoreVariable = keystoreVariable; } @@ -71,8 +69,11 @@ protected Class type() { } @Override - public org.jenkinsci.plugins.credentialsbinding.MultiBinding.MultiEnvironment bind(Run build, - FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { + public org.jenkinsci.plugins.credentialsbinding.MultiBinding.MultiEnvironment bind(@NonNull Run build, + @Nullable FilePath workspace, + @Nullable Launcher launcher, + @NonNull TaskListener listener) + throws IOException, InterruptedException { StandardCertificateCredentials credentials = getCredentials(build); final String storePassword = credentials.getPassword().getPlainText(); Map m = new LinkedHashMap<>(); @@ -87,11 +88,7 @@ public org.jenkinsci.plugins.credentialsbinding.MultiBinding.MultiEnvironment bi OutputStream out = secret.write(); try { credentials.getKeyStore().store(out, storePassword.toCharArray()); - } catch (KeyStoreException e) { - throw new IOException(e); - } catch (NoSuchAlgorithmException e) { - throw new IOException(e); - } catch (CertificateException e) { + } catch (KeyStoreException | NoSuchAlgorithmException | CertificateException e) { throw new IOException(e); } finally { org.apache.commons.io.IOUtils.closeQuietly(out); @@ -105,7 +102,7 @@ public org.jenkinsci.plugins.credentialsbinding.MultiBinding.MultiEnvironment bi } @Override - public Set variables() { + public Set variables(@NonNull Run build) { Set set = new HashSet<>(); set.add(keystoreVariable); if (aliasVariable != null && !aliasVariable.isEmpty()) { @@ -126,6 +123,7 @@ protected Class type() { return StandardCertificateCredentials.class; } + @NonNull @Override public String getDisplayName() { return Messages.CertificateMultiBinding_certificate_keystore(); diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding.java index 56d7ec3d..711305dc 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding.java @@ -24,6 +24,8 @@ package org.jenkinsci.plugins.credentialsbinding.impl; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import hudson.Extension; import hudson.FilePath; import hudson.Launcher; @@ -37,8 +39,6 @@ import org.jenkinsci.plugins.plaincredentials.FileCredentials; import org.kohsuke.stapler.DataBoundConstructor; -import javax.annotation.Nonnull; - public class FileBinding extends AbstractOnDiskBinding { @DataBoundConstructor public FileBinding(String variable, String credentialsId) { @@ -71,10 +71,10 @@ protected Object readResolve() { } @Override - public void unbind(@Nonnull Run build, - FilePath workspace, - Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + public void unbind(@NonNull Run build, + @Nullable FilePath workspace, + @Nullable Launcher launcher, + @NonNull TaskListener listener) { // replaced by the UnbindableDir.UnbinderImpl implementation } } @@ -86,7 +86,9 @@ public void unbind(@Nonnull Run build, return FileCredentials.class; } - @Override public String getDisplayName() { + @NonNull + @Override + public String getDisplayName() { return Messages.FileBinding_secret_file(); } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding.java index b9ff8e8f..59d63c8d 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding.java @@ -24,9 +24,12 @@ import com.cloudbees.jenkins.plugins.sshcredentials.SSHUserPrivateKey; import com.google.common.collect.ImmutableSet; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import hudson.FilePath; import hudson.Launcher; +import hudson.Util; import hudson.model.Run; import hudson.model.TaskListener; import hudson.util.Secret; @@ -36,10 +39,11 @@ import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.DataBoundSetter; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; import java.io.IOException; -import java.util.*; +import java.util.HashSet; +import java.util.LinkedHashMap; +import java.util.Map; +import java.util.Set; public class SSHUserPrivateKeyBinding extends MultiBinding { @@ -47,14 +51,14 @@ public class SSHUserPrivateKeyBinding extends MultiBinding { public String usernameVariable; public String passphraseVariable; - @DataBoundConstructor public SSHUserPrivateKeyBinding(@Nonnull String keyFileVariable, String credentialsId) { + @DataBoundConstructor public SSHUserPrivateKeyBinding(@NonNull String keyFileVariable, String credentialsId) { super(credentialsId); this.keyFileVariable = keyFileVariable; } @DataBoundSetter - public void setUsernameVariable(@Nonnull final String usernameVariable) { - this.usernameVariable = usernameVariable; + public void setUsernameVariable(@CheckForNull String usernameVariable) { + this.usernameVariable = Util.fixEmptyAndTrim(usernameVariable); } @CheckForNull @@ -63,8 +67,8 @@ public String getUsernameVariable() { } @DataBoundSetter - public void setPassphraseVariable(@Nonnull final String passphraseVariable) { - this.passphraseVariable = passphraseVariable; + public void setPassphraseVariable(@CheckForNull String passphraseVariable) { + this.passphraseVariable = Util.fixEmptyAndTrim(passphraseVariable); } @CheckForNull @@ -76,10 +80,11 @@ public String getPassphraseVariable() { return SSHUserPrivateKey.class; } - @Override public Set variables() { + @Override public Set variables(@NonNull Run build) throws CredentialNotFoundException { + SSHUserPrivateKey sshKey = getCredentials(build); Set set = new HashSet<>(); set.add(keyFileVariable); - if (usernameVariable != null) { + if (usernameVariable != null && sshKey.isUsernameSecret()) { set.add(usernameVariable); } if (passphraseVariable != null) { @@ -88,7 +93,10 @@ public String getPassphraseVariable() { return ImmutableSet.copyOf(set); } - @Override public MultiEnvironment bind(Run build, FilePath workspace, Launcher launcher, TaskListener listener) throws IOException, InterruptedException { + @Override public MultiEnvironment bind(@NonNull Run build, + FilePath workspace, + Launcher launcher, + @NonNull TaskListener listener) throws IOException, InterruptedException { SSHUserPrivateKey sshKey = getCredentials(build); UnbindableDir keyDir = UnbindableDir.create(workspace); FilePath keyFile = keyDir.getDirPath().child("ssh-key-" + keyFileVariable); @@ -101,21 +109,22 @@ public String getPassphraseVariable() { keyFile.write(contents.toString(), "UTF-8"); keyFile.chmod(0400); - Map map = new LinkedHashMap<>(); - map.put(keyFileVariable, keyFile.getRemote()); + Map secretValues = new LinkedHashMap<>(); + Map publicValues = new LinkedHashMap<>(); + secretValues.put(keyFileVariable, keyFile.getRemote()); if (passphraseVariable != null) { Secret passphrase = sshKey.getPassphrase(); if (passphrase != null) { - map.put(passphraseVariable, passphrase.getPlainText()); + secretValues.put(passphraseVariable, passphrase.getPlainText()); } else { - map.put(passphraseVariable, ""); + secretValues.put(passphraseVariable, ""); } } if (usernameVariable != null) { - map.put(usernameVariable, sshKey.getUsername()); + (sshKey.isUsernameSecret() ? secretValues : publicValues).put(usernameVariable, sshKey.getUsername()); } - return new MultiEnvironment(map, keyDir.getUnbinder()); + return new MultiEnvironment(secretValues, publicValues, keyDir.getUnbinder()); } @Symbol("sshUserPrivateKey") @@ -125,7 +134,9 @@ public String getPassphraseVariable() { return SSHUserPrivateKey.class; } - @Override public String getDisplayName() { + @NonNull + @Override + public String getDisplayName() { return Messages.SSHUserPrivateKeyBinding_ssh_user_private_key(); } 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 de4adcff..8f52ccfc 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapper.java @@ -24,10 +24,11 @@ package org.jenkinsci.plugins.credentialsbinding.impl; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; 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; @@ -38,12 +39,16 @@ import org.jenkinsci.plugins.credentialsbinding.masking.SecretPatterns; import org.kohsuke.stapler.DataBoundConstructor; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; import java.io.IOException; import java.io.OutputStream; -import java.util.*; -import java.util.regex.Matcher; +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.Pattern; @SuppressWarnings({"rawtypes", "unchecked"}) // inherited from BuildWrapper @@ -51,7 +56,7 @@ public class SecretBuildWrapper extends BuildWrapper { private /*almost final*/ List> bindings; - private final static Map, Collection> secretsForBuild = new WeakHashMap, Collection>(); + private final static Map, Collection> secretsForBuild = new WeakHashMap<>(); /** * Gets the {@link Pattern} for the secret values for a given build, if that build has secrets defined. If not, return @@ -59,7 +64,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 @CheckForNull Pattern getPatternForBuild(@Nonnull AbstractBuild build) { + public static @CheckForNull Pattern getPatternForBuild(@NonNull AbstractBuild build) { if (secretsForBuild.containsKey(build)) { return SecretPatterns.getAggregateSecretPattern(secretsForBuild.get(build)); } else { @@ -68,7 +73,7 @@ public class SecretBuildWrapper extends BuildWrapper { } @DataBoundConstructor public SecretBuildWrapper(List> bindings) { - this.bindings = bindings == null ? Collections.>emptyList() : bindings; + this.bindings = bindings == null ? Collections.emptyList() : bindings; } public List> getBindings() { @@ -76,19 +81,19 @@ public List> getBindings() { } @Override - public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) throws IOException, InterruptedException, Run.RunnerAbortedException { + public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) throws Run.RunnerAbortedException { return new Filter(build.getCharset().name()).decorateLogger(build, logger); } @Override public Environment setUp(AbstractBuild build, final Launcher launcher, BuildListener listener) throws IOException, InterruptedException { - final List m = new ArrayList(); + final List m = new ArrayList<>(); - Set secrets = new HashSet(); + Set secrets = new HashSet<>(); for (MultiBinding binding : bindings) { MultiBinding.MultiEnvironment e = binding.bind(build, build.getWorkspace(), launcher, listener); m.add(e); - secrets.addAll(e.getValues().values()); + secrets.addAll(e.getSecretValues().values()); } if (!secrets.isEmpty()) { @@ -98,7 +103,10 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) thr return new Environment() { @Override public void buildEnvVars(Map env) { for (MultiBinding.MultiEnvironment e : m) { - for (Map.Entry pair : e.getValues().entrySet()) { + for (Map.Entry pair : e.getSecretValues().entrySet()) { + env.put(pair.getKey(), pair.getValue()./* SECURITY-698 */replace("$", "$$$$")); + } + for (Map.Entry pair : e.getPublicValues().entrySet()) { env.put(pair.getKey(), pair.getValue()./* SECURITY-698 */replace("$", "$$$$")); } } @@ -114,7 +122,11 @@ public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) thr @Override public void makeSensitiveBuildVariables(AbstractBuild build, Set sensitiveVariables) { for (MultiBinding binding : bindings) { - sensitiveVariables.addAll(binding.variables()); + try { + sensitiveVariables.addAll(binding.variables(build)); + } catch (CredentialNotFoundException x) { + // ignore here (will throw an error later anyway) + } } } @@ -134,36 +146,10 @@ 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 LineTransformationOutputStream() { - 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()) { - 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); - } - } - - @Override public void flush() throws IOException { - logger.flush(); - } - + @Override public OutputStream decorateLogger(AbstractBuild build, OutputStream logger) { + return new SecretPatterns.MaskingOutputStream(logger, () -> getPatternForBuild(build), charsetName) { @Override public void close() throws IOException { super.close(); - logger.close(); secretsForBuild.remove(build); } }; @@ -177,7 +163,9 @@ private static final class Filter extends ConsoleLogFilter { return true; } - @Override public String getDisplayName() { + @NonNull + @Override + public String getDisplayName() { return Messages.SecretBuildWrapper_use_secret_text_s_or_file_s_(); } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding.java index 93402175..4f4fd60d 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding.java @@ -24,16 +24,14 @@ package org.jenkinsci.plugins.credentialsbinding.impl; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import hudson.Extension; import hudson.FilePath; import hudson.Launcher; import hudson.model.Run; import hudson.model.TaskListener; import java.io.IOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; import org.jenkinsci.Symbol; @@ -42,9 +40,6 @@ import org.jenkinsci.plugins.plaincredentials.StringCredentials; import org.kohsuke.stapler.DataBoundConstructor; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - public class StringBinding extends Binding { @DataBoundConstructor public StringBinding(String variable, String credentialsId) { @@ -55,10 +50,10 @@ public class StringBinding extends Binding { return StringCredentials.class; } - @Override public SingleEnvironment bindSingle(@Nonnull Run build, + @Override public SingleEnvironment bindSingle(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException { return new SingleEnvironment(getCredentials(build).getSecret().getPlainText()); } @@ -73,7 +68,9 @@ public class StringBinding extends Binding { return StringCredentials.class; } - @Override public String getDisplayName() { + @NonNull + @Override + public String getDisplayName() { return Messages.StringBinding_secret_text(); } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UnbindableDir.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UnbindableDir.java index ccfa9b79..a11427ef 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UnbindableDir.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UnbindableDir.java @@ -3,8 +3,7 @@ import java.io.IOException; import java.util.UUID; -import javax.annotation.Nonnull; - +import edu.umd.cs.findbugs.annotations.NonNull; import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor; import org.jenkinsci.plugins.credentialsbinding.MultiBinding.Unbinder; import org.kohsuke.accmod.Restricted; @@ -47,7 +46,7 @@ public FilePath getDirPath() { * @throws IOException * @throws InterruptedException */ - public static UnbindableDir create(@Nonnull FilePath workspace) + public static UnbindableDir create(@NonNull FilePath workspace) throws IOException, InterruptedException { final FilePath secrets = secretsDir(workspace); final String dirName = UUID.randomUUID().toString(); @@ -58,14 +57,14 @@ public static UnbindableDir create(@Nonnull FilePath workspace) return new UnbindableDir(dir); } - private static FilePath secretsDir(FilePath workspace) { - return tempDir(workspace).child("secretFiles"); + private static FilePath secretsDir(FilePath workspace) throws IOException { + final FilePath path = WorkspaceList.tempDir(workspace); + if (path == null) { + throw new IOException("Failed to create tempDir"); + } + return path.child("secretFiles"); } - // TODO 1.652 use WorkspaceList.tempDir - private static FilePath tempDir(FilePath ws) { - return ws.sibling(ws.getName() + System.getProperty(WorkspaceList.class.getName(), "@") + "tmp"); - } @Restricted(NoExternalUse.class) protected static class UnbinderImpl implements Unbinder { @@ -77,10 +76,10 @@ protected UnbinderImpl(String dirName) { } @Override - public void unbind(@Nonnull Run build, + public void unbind(@NonNull Run build, FilePath workspace, Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException, InterruptedException { secretsDir(workspace).child(dirName).deleteRecursive(); } } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding.java index bf7e71d8..ea018028 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding.java @@ -25,16 +25,14 @@ package org.jenkinsci.plugins.credentialsbinding.impl; import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials; +import edu.umd.cs.findbugs.annotations.NonNull; +import edu.umd.cs.findbugs.annotations.Nullable; import hudson.Extension; import hudson.FilePath; import hudson.Launcher; import hudson.model.Run; import hudson.model.TaskListener; import java.io.IOException; -import java.util.Arrays; -import java.util.Collections; -import java.util.HashSet; -import java.util.Set; import org.jenkinsci.Symbol; @@ -42,9 +40,6 @@ import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor; import org.kohsuke.stapler.DataBoundConstructor; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - public class UsernamePasswordBinding extends Binding { @DataBoundConstructor public UsernamePasswordBinding(String variable, String credentialsId) { @@ -55,10 +50,10 @@ public class UsernamePasswordBinding extends Binding build, + @Override public SingleEnvironment bindSingle(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException { StandardUsernamePasswordCredentials credentials = getCredentials(build); return new SingleEnvironment(credentials.getUsername() + ':' + credentials.getPassword().getPlainText()); } @@ -70,7 +65,9 @@ public class UsernamePasswordBinding extends Binding { private final String usernameVariable; @@ -68,19 +66,26 @@ public String getPasswordVariable() { return StandardUsernamePasswordCredentials.class; } - @Override public MultiEnvironment bind(@Nonnull Run build, + @Override public MultiEnvironment bind(@NonNull Run build, @Nullable FilePath workspace, @Nullable Launcher launcher, - @Nonnull TaskListener listener) throws IOException, InterruptedException { + @NonNull TaskListener listener) throws IOException, InterruptedException { StandardUsernamePasswordCredentials credentials = getCredentials(build); - Map m = new LinkedHashMap<>(); - m.put(usernameVariable, credentials.getUsername()); - m.put(passwordVariable, credentials.getPassword().getPlainText()); - return new MultiEnvironment(m); + Map secretValues = new LinkedHashMap<>(); + Map publicValues = new LinkedHashMap<>(); + (credentials.isUsernameSecret() ? secretValues : publicValues).put(usernameVariable, credentials.getUsername()); + secretValues.put(passwordVariable, credentials.getPassword().getPlainText()); + return new MultiEnvironment(secretValues, publicValues); } - @Override public Set variables() { - return new HashSet(Arrays.asList(usernameVariable, passwordVariable)); + @Override public Set variables(@NonNull Run build) throws CredentialNotFoundException { + StandardUsernamePasswordCredentials credentials = getCredentials(build); + Set vars = new LinkedHashSet<>(); + if (credentials.isUsernameSecret()) { + vars.add(usernameVariable); + } + vars.add(passwordVariable); + return vars; } @Symbol("usernamePassword") @@ -90,7 +95,9 @@ public String getPasswordVariable() { return StandardUsernamePasswordCredentials.class; } - @Override public String getDisplayName() { + @NonNull + @Override + public String getDisplayName() { return Messages.UsernamePasswordMultiBinding_username_and_password(); } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding.java index 28d391dd..83d535d8 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding.java @@ -25,8 +25,8 @@ package org.jenkinsci.plugins.credentialsbinding.impl; import com.cloudbees.plugins.credentials.CredentialsProvider; -import com.cloudbees.plugins.credentials.domains.DomainRequirement; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import hudson.FilePath; import hudson.model.Item; @@ -36,6 +36,7 @@ import java.io.InputStream; import java.util.Collections; +import jenkins.model.Jenkins; import org.apache.commons.io.IOUtils; import org.jenkinsci.Symbol; import org.jenkinsci.plugins.credentialsbinding.BindingDescriptor; @@ -43,6 +44,7 @@ import org.kohsuke.stapler.AncestorInPath; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; +import org.kohsuke.stapler.StaplerRequest; public class ZipFileBinding extends AbstractOnDiskBinding { @@ -68,12 +70,29 @@ public class ZipFileBinding extends AbstractOnDiskBinding { return FileCredentials.class; } - @Override public String getDisplayName() { + @NonNull + @Override + public String getDisplayName() { return Messages.ZipFileBinding_secret_zip_file(); } - public FormValidation doCheckCredentialsId(@AncestorInPath Item owner, @QueryParameter String value) { - for (FileCredentials c : CredentialsProvider.lookupCredentials(FileCredentials.class, owner, null, Collections.emptyList())) { + // @RequirePOST + public FormValidation doCheckCredentialsId(StaplerRequest req, @AncestorInPath Item owner, @QueryParameter String value) { + //TODO due to weird behavior in c:select, there are initial calls using GET + // so using this approach will prevent 405 errors + if (!req.getMethod().equals("POST")) { + return FormValidation.ok(); + } + if (owner == null) { + if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) { + return FormValidation.ok(); + } + } else { + if (!owner.hasPermission(Item.EXTENDED_READ) && !owner.hasPermission(CredentialsProvider.USE_ITEM)) { + return FormValidation.ok(); + } + } + for (FileCredentials c : CredentialsProvider.lookupCredentials(FileCredentials.class, owner, null, Collections.emptyList())) { if (c.getId().equals(value)) { InputStream is = null; try { diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/AlmquistShellSecretPatternFactory.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/AlmquistShellSecretPatternFactory.java index 3437415e..1772c8ae 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/AlmquistShellSecretPatternFactory.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/AlmquistShellSecretPatternFactory.java @@ -24,11 +24,11 @@ package org.jenkinsci.plugins.credentialsbinding.masking; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import javax.annotation.Nonnull; import java.util.Collection; import java.util.Collections; @@ -43,7 +43,7 @@ public class AlmquistShellSecretPatternFactory implements SecretPatternFactory { private static final String START_FRAGMENT = "'\""; private static final String END_FRAGMENT = "\"'"; - private @Nonnull String getQuotedForm(@Nonnull String input) { + private @NonNull String getQuotedForm(@NonNull String input) { StringBuilder sb = new StringBuilder(input.length()); for (int i = 0; i < input.length(); i++) { char c = input.charAt(i); @@ -66,8 +66,9 @@ public class AlmquistShellSecretPatternFactory implements SecretPatternFactory { return sb.toString(); } + @NonNull @Override - public @Nonnull Collection getEncodedForms(@Nonnull String input) { + public Collection getEncodedForms(@NonNull String input) { return Collections.singleton(getQuotedForm(input)); } } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BashSecretPatternFactory.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BashSecretPatternFactory.java index 66cad78a..67be4304 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BashSecretPatternFactory.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BashSecretPatternFactory.java @@ -24,11 +24,11 @@ package org.jenkinsci.plugins.credentialsbinding.masking; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import javax.annotation.Nonnull; import java.util.Collection; import java.util.HashSet; import java.util.regex.Pattern; @@ -39,7 +39,7 @@ public class BashSecretPatternFactory implements SecretPatternFactory { private static final Pattern QUOTED_CHARS = Pattern.compile("(\\\\)(\\\\?)"); - private @Nonnull String getQuotedForm(@Nonnull String input) { + private @NonNull String getQuotedForm(@NonNull String input) { StringBuilder sb = new StringBuilder(input.length()); for (int i = 0; i < input.length(); i++) { char c = input.charAt(i); @@ -52,16 +52,17 @@ public class BashSecretPatternFactory implements SecretPatternFactory { return sb.toString(); } - private @Nonnull String surroundWithQuotes(@Nonnull String input) { + private @NonNull String surroundWithQuotes(@NonNull String input) { return "'" + input + "'"; } - private @Nonnull String getUnquotedForm(@Nonnull String input) { + private @NonNull String getUnquotedForm(@NonNull String input) { return QUOTED_CHARS.matcher(input).replaceAll("$2"); } + @NonNull @Override - public @Nonnull Collection getEncodedForms(@Nonnull String input) { + public Collection getEncodedForms(@NonNull String input) { Collection patterns = new HashSet<>(); String quotedForm = getQuotedForm(input); patterns.add(quotedForm); diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactory.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactory.java index d200f5f2..68542388 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactory.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactory.java @@ -24,11 +24,11 @@ package org.jenkinsci.plugins.credentialsbinding.masking; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import javax.annotation.Nonnull; import java.util.Collection; import java.util.Collections; import java.util.regex.Pattern; @@ -38,8 +38,9 @@ public class BatchSecretPatternFactory implements SecretPatternFactory { private static final Pattern QUOTED_CHARS = Pattern.compile("(\\^)(\\^?)"); + @NonNull @Override - public @Nonnull Collection getEncodedForms(@Nonnull String input) { + public Collection getEncodedForms(@NonNull String input) { return input.contains("^") ? Collections.singleton(QUOTED_CHARS.matcher(input).replaceAll("$2")) : Collections.emptySet(); diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/DollarSecretPatternFactory.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/DollarSecretPatternFactory.java index 988bc3a2..20eeb4d5 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/DollarSecretPatternFactory.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/DollarSecretPatternFactory.java @@ -24,20 +24,20 @@ package org.jenkinsci.plugins.credentialsbinding.masking; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import javax.annotation.Nonnull; import java.util.Collection; import java.util.Collections; -import java.util.regex.Pattern; @Extension @Restricted(NoExternalUse.class) public class DollarSecretPatternFactory implements SecretPatternFactory { + @NonNull @Override - public @Nonnull Collection getEncodedForms(@Nonnull String input) { + public Collection getEncodedForms(@NonNull String input) { return input.contains("$") ? Collections.singleton(input.replace("$", "$$")) : Collections.emptySet(); diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/LiteralSecretPatternFactory.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/LiteralSecretPatternFactory.java index f961f510..1d039772 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/LiteralSecretPatternFactory.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/LiteralSecretPatternFactory.java @@ -24,11 +24,11 @@ package org.jenkinsci.plugins.credentialsbinding.masking; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Extension; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import javax.annotation.Nonnull; import java.util.Collection; import java.util.Collections; @@ -38,9 +38,9 @@ @Extension @Restricted(NoExternalUse.class) public class LiteralSecretPatternFactory implements SecretPatternFactory { - @Nonnull + @NonNull @Override - public Collection getEncodedForms(@Nonnull String input) { + public Collection getEncodedForms(@NonNull String input) { return Collections.singleton(input); } } diff --git a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatternFactory.java b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatternFactory.java index 3c54920d..a692cee7 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatternFactory.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatternFactory.java @@ -24,12 +24,12 @@ package org.jenkinsci.plugins.credentialsbinding.masking; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.ExtensionList; import hudson.ExtensionPoint; import org.kohsuke.accmod.Restricted; import org.kohsuke.accmod.restrictions.NoExternalUse; -import javax.annotation.Nonnull; import java.util.Collection; /** @@ -43,12 +43,12 @@ public interface SecretPatternFactory extends ExtensionPoint { /** * Returns a collection of alternative forms the given input may be encoded as in logs. */ - @Nonnull Collection getEncodedForms(@Nonnull String input); + @NonNull Collection getEncodedForms(@NonNull String input); /** * Returns all SecretPatternFactory extensions known at runtime. */ - static @Nonnull ExtensionList all() { + static @NonNull ExtensionList all() { return ExtensionList.lookup(SecretPatternFactory.class); } 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..f45a772f 100644 --- a/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java +++ b/src/main/java/org/jenkinsci/plugins/credentialsbinding/masking/SecretPatterns.java @@ -24,16 +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.function.Supplier; +import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; -@Restricted(NoExternalUse.class) public class SecretPatterns { private static final Comparator BY_LENGTH_DESCENDING = @@ -48,7 +50,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 +62,47 @@ 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 @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 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, @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 (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 { + // As above. + out.write(b, 0, len); + } + } + } + + } + + private SecretPatterns() {} + } diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/MultiBinding/config.jelly b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/MultiBinding/config.jelly index dec100f8..e049197f 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/MultiBinding/config.jelly +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/MultiBinding/config.jelly @@ -26,7 +26,7 @@ THE SOFTWARE. - + diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding/help.html b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding/help.html index a0b9f253..f8a28306 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding/help.html +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBinding/help.html @@ -2,7 +2,7 @@ Sets one variable to the username and one variable to the password given in the credentials.
- Warning: if the master or agent node has multiple executors, + Warning: if the Jenkins controller or agent node has multiple executors, any other build running concurrently on the same node will be able to read the text of the secret, for example on Linux using ps e.
diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding/help.html b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding/help.html index a507aa23..bc7fb8fe 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding/help.html +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/FileBinding/help.html @@ -3,7 +3,7 @@ (The file is deleted when the build completes.)
- Warning: if the master or agent node has multiple executors, + Warning: if the Jenkins controller or agent node has multiple executors, any other build running concurrently on the same node will be able to read the contents of this file.
diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding/help.html b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding/help.html index 0a748220..bd474fba 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding/help.html +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBinding/help.html @@ -3,7 +3,7 @@ (The file is deleted when the build completes.) Also optionally sets variables for the SSH key's username and passphrase.
- Warning: if the master or agent node has multiple executors, + Warning: if the Jenkins controller or agent node has multiple executors, any other build running concurrently on the same node will be able to read the contents of this file.
diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding/help.html b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding/help.html index 6206be09..ec9fb7e7 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding/help.html +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/StringBinding/help.html @@ -2,7 +2,7 @@ Sets a variable to the text given in the credentials.
- Warning: if the master or agent node has multiple executors, + Warning: if the Jenkins controller or agent node has multiple executors, any other build running concurrently on the same node will be able to read the text of the secret, for example on Linux using ps e.
diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding/help.html b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding/help.html index 89615c5f..c7e0307f 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding/help.html +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBinding/help.html @@ -2,7 +2,7 @@ Sets a variable to the username and password given in the credentials, separated by a colon (:).
- Warning: if the master or agent node has multiple executors, + Warning: if the Jenkins controller or agent node has multiple executors, any other build running concurrently on the same node will be able to read the text of the secret, for example on Linux using ps e.
diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBinding/help.html b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBinding/help.html index a0b9f253..f8a28306 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBinding/help.html +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBinding/help.html @@ -2,7 +2,7 @@ Sets one variable to the username and one variable to the password given in the credentials.
- Warning: if the master or agent node has multiple executors, + Warning: if the Jenkins controller or agent node has multiple executors, any other build running concurrently on the same node will be able to read the text of the secret, for example on Linux using ps e.
diff --git a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding/help.html b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding/help.html index df6a1c4a..b6e0969b 100644 --- a/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding/help.html +++ b/src/main/resources/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBinding/help.html @@ -3,7 +3,7 @@ (The directory is deleted when the build completes.)
- Warning: if the master or agent node has multiple executors, + Warning: if the Jenkins controller or agent node has multiple executors, any other build running concurrently on the same node will be able to read the contents of this directory.
diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java index 03b89024..250691ce 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java @@ -24,11 +24,13 @@ package org.jenkinsci.plugins.credentialsbinding.impl; +import com.cloudbees.plugins.credentials.Credentials; import com.cloudbees.plugins.credentials.CredentialsProvider; import com.cloudbees.plugins.credentials.CredentialsScope; import com.cloudbees.plugins.credentials.SecretBytes; import com.cloudbees.plugins.credentials.common.StandardUsernamePasswordCredentials; import com.cloudbees.plugins.credentials.domains.Domain; +import com.cloudbees.plugins.credentials.impl.BaseStandardCredentials; import com.cloudbees.plugins.credentials.impl.UsernamePasswordCredentialsImpl; import hudson.model.Fingerprint; @@ -37,17 +39,15 @@ import hudson.FilePath; import hudson.Functions; -import hudson.model.Node; import hudson.model.Result; +import hudson.model.Run; import hudson.security.FullControlOnceLoggedInAuthorizationStrategy; -import hudson.slaves.DumbSlave; -import hudson.slaves.NodeProperty; -import hudson.slaves.RetentionStrategy; import hudson.slaves.WorkspaceList; import hudson.util.Secret; import java.io.File; import java.io.IOException; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.HashMap; import java.util.Map; @@ -60,13 +60,18 @@ import org.jenkinsci.plugins.authorizeproject.AuthorizeProjectProperty; import org.jenkinsci.plugins.authorizeproject.ProjectQueueItemAuthenticator; import org.jenkinsci.plugins.authorizeproject.strategy.SpecificUsersAuthorizationStrategy; -import org.jenkinsci.plugins.credentialsbinding.MultiBinding; +import org.jenkinsci.plugins.plaincredentials.StringCredentials; import org.jenkinsci.plugins.plaincredentials.impl.FileCredentialsImpl; import org.jenkinsci.plugins.plaincredentials.impl.StringCredentialsImpl; import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist; import org.jenkinsci.plugins.scriptsecurity.sandbox.whitelists.BlanketWhitelist; +import org.jenkinsci.plugins.workflow.actions.ArgumentsAction; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; import org.jenkinsci.plugins.workflow.cps.SnippetizerTester; +import org.jenkinsci.plugins.workflow.flow.FlowExecution; +import org.jenkinsci.plugins.workflow.graph.FlowNode; +import org.jenkinsci.plugins.workflow.graphanalysis.DepthFirstScanner; +import org.jenkinsci.plugins.workflow.graphanalysis.NodeStepTypePredicate; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.steps.AbstractStepDescriptorImpl; @@ -75,19 +80,24 @@ import org.jenkinsci.plugins.workflow.steps.StepConfigTester; import org.jenkinsci.plugins.workflow.test.steps.SemaphoreStep; +import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasItem; import static org.hamcrest.Matchers.is; import static org.hamcrest.Matchers.notNullValue; import static org.hamcrest.Matchers.nullValue; -import static org.junit.Assert.*; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + import org.junit.ClassRule; import org.junit.Rule; import org.junit.Test; -import org.junit.rules.TemporaryFolder; import org.junit.runners.model.Statement; import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.Issue; +import org.jvnet.hudson.test.JenkinsSessionRule; import org.jvnet.hudson.test.RestartableJenkinsRule; import org.jvnet.hudson.test.TestExtension; import org.kohsuke.stapler.DataBoundConstructor; @@ -95,20 +105,20 @@ public class BindingStepTest { @ClassRule public static BuildWatcher buildWatcher = new BuildWatcher(); - @Rule public RestartableJenkinsRule story = new RestartableJenkinsRule(); - @Rule public TemporaryFolder tmp = new TemporaryFolder(); + /*@Rule*/ public RestartableJenkinsRule story = new RestartableJenkinsRule(); + @Rule public JenkinsSessionRule rr = new JenkinsSessionRule(); - @Test public void configRoundTrip() throws Exception { + @Test public void configRoundTrip() { story.addStep(new Statement() { @SuppressWarnings("rawtypes") @Override public void evaluate() throws Throwable { UsernamePasswordCredentialsImpl c = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", "bob", "s3cr3t"); CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), c); - BindingStep s = new StepConfigTester(story.j).configRoundTrip(new BindingStep(Collections.singletonList(new UsernamePasswordBinding("userpass", "creds")))); + BindingStep s = new StepConfigTester(story.j).configRoundTrip(new BindingStep(Collections.singletonList(new UsernamePasswordBinding("userpass", "creds")))); story.j.assertEqualDataBoundBeans(s.getBindings(), Collections.singletonList(new UsernamePasswordBinding("userpass", "creds"))); CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), new FileCredentialsImpl(CredentialsScope.GLOBAL, "secrets", "sample", "secrets.zip", SecretBytes.fromBytes(new byte[] {0x50,0x4B,0x05,0x06,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00,0x00}))); // https://en.wikipedia.org/wiki/Zip_(file_format)#Limits - new SnippetizerTester(story.j).assertRoundTrip(new BindingStep(Collections.singletonList(new ZipFileBinding("file", "secrets"))), + new SnippetizerTester(story.j).assertRoundTrip(new BindingStep(Collections.singletonList(new ZipFileBinding("file", "secrets"))), "withCredentials([[$class: 'ZipFileBinding', credentialsId: 'secrets', variable: 'file']]) {\n // some block\n}"); } }); @@ -120,11 +130,11 @@ public static class ZipStep extends AbstractStepImpl { @Override public String getFunctionName() {return "zip";} } public static class Execution extends AbstractSynchronousStepExecution { - @Override protected Void run() throws Exception {return null;} + @Override protected Void run() {return null;} } } - @Test public void basics() throws Exception { + @Test public void basics() { final String credentialsId = "creds"; final String username = "bob"; final String password = "s$$cr3t"; @@ -146,7 +156,7 @@ public static class Execution extends AbstractSynchronousStepExecution { + "}", true)); WorkflowRun b = p.scheduleBuild2(0).waitForStart(); SemaphoreStep.waitForStart("basics/1", b); - story.j.assertLogContains(Functions.isWindows() ? "Masking supported pattern matches of %USERNAME% or %PASSWORD%" : "Masking supported pattern matches of $USERNAME or $PASSWORD", b); + story.j.assertLogContains(Functions.isWindows() ? "Masking supported pattern matches of %PASSWORD%" : "Masking supported pattern matches of $PASSWORD", b); } }); story.addStep(new Statement() { @@ -170,7 +180,7 @@ public static class Execution extends AbstractSynchronousStepExecution { @Issue("JENKINS-42999") @Test - public void limitedRequiredContext() throws Exception { + public void limitedRequiredContext() { final String credentialsId = "creds"; final String username = "bob"; final String password = "s3cr3t"; @@ -206,7 +216,7 @@ public void limitedRequiredContext() throws Exception { @Issue("JENKINS-42999") @Test - public void widerRequiredContext() throws Exception { + public void widerRequiredContext() { final String credentialsId = "creds"; final String credsFile = "credsFile"; final String credsContent = "s3cr3t"; @@ -230,7 +240,7 @@ public void widerRequiredContext() throws Exception { @Inject StringCredentialsImpl.DescriptorImpl stringCredentialsDescriptor; - @Test public void incorrectType() throws Exception { + @Test public void incorrectType() { story.addStep(new Statement() { @Override public void evaluate() throws Throwable { StringCredentialsImpl c = new StringCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", Secret.fromString("s3cr3t")); @@ -252,53 +262,43 @@ public void widerRequiredContext() throws Exception { }); } - @Test public void cleanupAfterRestart() throws Exception { + @Test public void cleanupAfterRestart() throws Throwable { final String secret = "s3cr3t"; - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - FileCredentialsImpl c = new FileCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", "secret.txt", SecretBytes.fromBytes(secret.getBytes())); - CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), c); - // TODO JENKINS-26398: story.j.createSlave("myslave", null, null) does not work since the slave root is deleted after restart. - story.j.jenkins.addNode(new DumbSlave("myslave", "", tmp.newFolder().getAbsolutePath(), "1", Node.Mode.NORMAL, "", story.j.createComputerLauncher(null), RetentionStrategy.NOOP, Collections.>emptyList())); - WorkflowJob p = story.j.jenkins.createProject(WorkflowJob.class, "p"); - p.setDefinition(new CpsFlowDefinition("" - + "node('myslave') {" - + " withCredentials([file(variable: 'SECRET', credentialsId: 'creds')]) {\n" - + " semaphore 'cleanupAfterRestart'\n" - + " if (isUnix()) {sh 'cp $SECRET key'} else {bat 'copy %SECRET% key'}\n" - + " }\n" - + "}", true)); - WorkflowRun b = p.scheduleBuild2(0).waitForStart(); - SemaphoreStep.waitForStart("cleanupAfterRestart/1", b); - } + rr.then(r -> { + FileCredentialsImpl c = new FileCredentialsImpl(CredentialsScope.GLOBAL, "creds", "sample", "secret.txt", SecretBytes.fromBytes(secret.getBytes())); + CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), c); + r.createSlave("myslave", null, null); + WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition("" + + "node('myslave') {" + + " withCredentials([file(variable: 'SECRET', credentialsId: 'creds')]) {\n" + + " semaphore 'cleanupAfterRestart'\n" + + " if (isUnix()) {sh 'cp \"$SECRET\" key'} else {bat 'copy \"%SECRET%\" key'}\n" + + " }\n" + + "}", true)); + WorkflowRun b = p.scheduleBuild2(0).waitForStart(); + SemaphoreStep.waitForStart("cleanupAfterRestart/1", b); }); - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - WorkflowJob p = story.j.jenkins.getItemByFullName("p", WorkflowJob.class); - assertNotNull(p); - WorkflowRun b = p.getBuildByNumber(1); - assertNotNull(b); - assertEquals(Collections.emptySet(), grep(b.getRootDir(), secret)); - SemaphoreStep.success("cleanupAfterRestart/1", null); - story.j.assertBuildStatusSuccess(story.j.waitForCompletion(b)); - story.j.assertLogNotContains(secret, b); - FilePath ws = story.j.jenkins.getNode("myslave").getWorkspaceFor(p); - FilePath key = ws.child("key"); - assertTrue(key.exists()); - assertEquals(secret, key.readToString()); - FilePath secretFiles = tempDir(ws).child("secretFiles"); - assertTrue(secretFiles.isDirectory()); - assertEquals(Collections.emptyList(), secretFiles.list()); - assertEquals(Collections.emptySet(), grep(b.getRootDir(), secret)); - } + rr.then(r -> { + WorkflowJob p = r.jenkins.getItemByFullName("p", WorkflowJob.class); + assertNotNull(p); + WorkflowRun b = p.getBuildByNumber(1); + assertNotNull(b); + assertEquals(Collections.emptySet(), grep(b.getRootDir(), secret)); + SemaphoreStep.success("cleanupAfterRestart/1", null); + r.assertBuildStatusSuccess(r.waitForCompletion(b)); + r.assertLogNotContains(secret, b); + FilePath ws = r.jenkins.getNode("myslave").getWorkspaceFor(p); + FilePath key = ws.child("key"); + assertTrue(key.exists()); + assertEquals(secret, key.readToString()); + FilePath secretFiles = WorkspaceList.tempDir(ws).child("secretFiles"); + assertTrue(secretFiles.isDirectory()); + assertEquals(Collections.emptyList(), secretFiles.list()); + assertEquals(Collections.emptySet(), grep(b.getRootDir(), secret)); }); } - // TODO 1.652 use WorkspaceList.tempDir - private static FilePath tempDir(FilePath ws) { - return ws.sibling(ws.getName() + System.getProperty(WorkspaceList.class.getName(), "@") + "tmp"); - } - @Issue("JENKINS-27389") @Test public void grabEnv() { story.addStep(new Statement() { @@ -358,7 +358,7 @@ public void testGlobalBindingWithAuthorization() { User.get("dummy", true); // enable the run as user strategy for the AuthorizeProject plugin - Map strategies = new HashMap(); + Map strategies = new HashMap<>(); strategies.put(story.j.jenkins.getDescriptor(SpecificUsersAuthorizationStrategy.class).getId(), true); QueueItemAuthenticatorConfiguration.get().getAuthenticators().add(new ProjectQueueItemAuthenticator(strategies)); @@ -445,8 +445,55 @@ public void testTrackingOfCredential() { }); } + @Issue("JENKINS-64631") + @Test + public void usernameUnmaskedInStepArguments() { + story.then(r -> { + String credentialsId = "my-credentials"; + String username = "alice"; + // UsernamePasswordCredentialsImpl.isUsernameSecret defaults to false for new credentials. + CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), + new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, credentialsId, null, username, "s3cr3t")); + WorkflowJob p = r.createProject(WorkflowJob.class); + p.setDefinition(new CpsFlowDefinition( + "node {\n" + + " withCredentials([usernamePassword(credentialsId: '" + credentialsId + "', usernameVariable: 'username', passwordVariable: 'password')]) {\n" + + " echo(/Username is ${username}/)\n" + + " }\n" + + "}", true)); + WorkflowRun b = r.buildAndAssertSuccess(p); + FlowExecution exec = b.asFlowExecutionOwner().get(); + FlowNode echoNode = new DepthFirstScanner().findFirstMatch(exec, new NodeStepTypePredicate("echo")); + assertThat(ArgumentsAction.getArguments(echoNode).get("message"), equalTo("Username is " + username)); + }); + } + + @Issue("https://github.com/jenkinsci/credentials-plugin/pull/293") + @Test public void forRun() throws Exception { + story.then(r -> { + CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), new SpecialCredentials(CredentialsScope.GLOBAL, "test", null)); + WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); + p.setDefinition(new CpsFlowDefinition("withCredentials([string(variable: 'SECRET', credentialsId: 'test')]) {echo(/got: ${SECRET.toUpperCase()}/)}", true)); + r.assertLogContains("got: P#1", r.assertBuildStatusSuccess(p.scheduleBuild2(0).get())); + }); + } + private static final class SpecialCredentials extends BaseStandardCredentials implements StringCredentials { + private Run build; + SpecialCredentials(CredentialsScope scope, String id, String description) { + super(scope, id, description); + } + @Override public Secret getSecret() { + return Secret.fromString(build != null ? build.getExternalizableId() : "unknown"); + } + @Override public Credentials forRun(Run context) { + SpecialCredentials clone = new SpecialCredentials(getScope(), getId(), getDescription()); + clone.build = context; + return clone; + } + } + private static Set grep(File dir, String text) throws IOException { - Set matches = new TreeSet(); + Set matches = new TreeSet<>(); grep(dir, text, "", matches); return matches; } @@ -459,7 +506,7 @@ private static void grep(File dir, String text, String prefix, Set match String qualifiedName = prefix + kid.getName(); if (kid.isDirectory()) { grep(kid, text, qualifiedName + "/", matches); - } else if (kid.isFile() && FileUtils.readFileToString(kid).contains(text)) { + } else if (kid.isFile() && FileUtils.readFileToString(kid, StandardCharsets.UTF_8).contains(text)) { matches.add(qualifiedName); } } diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BuildWrapperOrderCredentialsBindingTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BuildWrapperOrderCredentialsBindingTest.java index d25da58e..d43478ea 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BuildWrapperOrderCredentialsBindingTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BuildWrapperOrderCredentialsBindingTest.java @@ -27,10 +27,15 @@ import com.cloudbees.plugins.credentials.CredentialsProvider; import com.cloudbees.plugins.credentials.CredentialsScope; import com.cloudbees.plugins.credentials.domains.Domain; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.EnvVars; import hudson.Functions; import hudson.Launcher; -import hudson.model.*; +import hudson.model.AbstractBuild; +import hudson.model.AbstractProject; +import hudson.model.BuildListener; +import hudson.model.FreeStyleBuild; +import hudson.model.FreeStyleProject; import hudson.tasks.BatchFile; import hudson.tasks.BuildWrapper; import hudson.tasks.BuildWrapperDescriptor; @@ -45,12 +50,9 @@ import org.jvnet.hudson.test.TestExtension; import org.kohsuke.stapler.StaplerRequest; -import javax.annotation.Nonnull; import java.io.IOException; -import java.util.Arrays; +import java.util.Collections; import java.util.Map; -import java.util.function.Predicate; -import java.util.stream.Collectors; public class BuildWrapperOrderCredentialsBindingTest { @@ -66,7 +68,7 @@ public class BuildWrapperOrderCredentialsBindingTest { CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), firstCreds); - SecretBuildWrapper wrapper = new SecretBuildWrapper(Arrays.asList(new StringBinding(bindingKey, credentialsId))); + SecretBuildWrapper wrapper = new SecretBuildWrapper(Collections.singletonList(new StringBinding(bindingKey, credentialsId))); FreeStyleProject f = r.createFreeStyleProject("buildWrapperOrder"); @@ -112,7 +114,7 @@ public boolean isApplicable(AbstractProject item) { } @Override - public BuildWrapper newInstance(StaplerRequest req, @Nonnull JSONObject formData) throws FormException { + public BuildWrapper newInstance(StaplerRequest req, @NonNull JSONObject formData) { return new BuildWrapperOrder(); } diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest.java index bf7225eb..3701c507 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest.java @@ -27,14 +27,16 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.CoreMatchers.not; import static org.hamcrest.CoreMatchers.nullValue; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsInAnyOrder; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; -import static org.junit.Assert.assertThat; import java.io.File; import java.io.IOException; import java.io.InputStream; +import java.net.URL; +import java.nio.charset.StandardCharsets; import java.util.Collections; import java.util.List; @@ -82,17 +84,14 @@ public class CertificateMultiBindingTest { @Before public void setUp() throws IOException { /* do the dance to get a simple zip file into jenkins */ - InputStream stream = this.getClass().getResourceAsStream("certificate.p12"); - try { - assertThat(stream, is(not(nullValue()))); - certificate = tmp.newFile("a.certificate"); - FileUtils.copyInputStreamToFile(stream, certificate); - } finally { - IOUtils.closeQuietly(stream); - stream = null; - } + certificate = tmp.newFile("a.certificate"); + final URL resource = this.getClass().getResource("certificate.p12"); + assertThat(resource, is(not(nullValue()))); + FileUtils.copyURLToFile(resource, certificate); } + // TODO configRoundtrip to test form, null hygiene on @DataBoundSetter + @Test public void basics() throws Exception { String alias = "androiddebugkey"; @@ -152,7 +151,7 @@ public void basicsPipeline() throws Exception { CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), c); // create the Pipeline job WorkflowJob p = r.jenkins.createProject(WorkflowJob.class, "p"); - String pipelineScript = IOUtils.toString(getTestResourceInputStream("basicsPipeline-Jenkinsfile")); + String pipelineScript = IOUtils.toString(getTestResourceInputStream("basicsPipeline-Jenkinsfile"), StandardCharsets.UTF_8); p.setDefinition(new CpsFlowDefinition(pipelineScript, true)); // copy resources into workspace FilePath workspace = r.jenkins.getWorkspaceFor(p); diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBindingTest.java similarity index 76% rename from src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyTest.java rename to src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBindingTest.java index ff55b832..99d5feda 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SSHUserPrivateKeyBindingTest.java @@ -30,9 +30,10 @@ import edu.umd.cs.findbugs.annotations.NonNull; import hudson.FilePath; import hudson.Functions; +import hudson.security.ACL; import hudson.util.Secret; -import org.jenkinsci.plugins.credentialsbinding.MultiBinding; import org.jenkinsci.plugins.workflow.cps.CpsFlowDefinition; +import org.jenkinsci.plugins.workflow.cps.SnippetizerTester; import org.jenkinsci.plugins.workflow.job.WorkflowJob; import org.jenkinsci.plugins.workflow.job.WorkflowRun; import org.jenkinsci.plugins.workflow.steps.StepConfigTester; @@ -44,19 +45,27 @@ import org.jvnet.hudson.test.RestartableJenkinsRule; import java.io.Serializable; -import java.util.*; +import java.util.Collections; +import java.util.List; -import static org.junit.Assert.*; +import org.junit.ClassRule; +import org.jvnet.hudson.test.BuildWatcher; -public class SSHUserPrivateKeyTest { +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; +import static org.junit.Assert.assertTrue; + +public class SSHUserPrivateKeyBindingTest { @Rule public RestartableJenkinsRule story = new RestartableJenkinsRule(); + @ClassRule public static BuildWatcher bw = new BuildWatcher(); @Rule public TemporaryFolder tmp = new TemporaryFolder(); private static class DummyPrivateKey extends BaseCredentials implements SSHUserPrivateKey, Serializable { private final String id; private final String user; + boolean usernameSecret = true; private final Secret passphrase; private final String keyContent; @@ -87,7 +96,7 @@ public Secret getPassphrase() { @NonNull @Override public List getPrivateKeys() { - return Arrays.asList(keyContent); + return Collections.singletonList(keyContent); } @NonNull @@ -96,6 +105,11 @@ public String getUsername() { return user; } + @Override + public boolean isUsernameSecret() { + return usernameSecret; + } + @NonNull @Override public String getDescription() { @@ -108,23 +122,24 @@ public CredentialsScope getScope() { } } - @Test public void configRoundTrip() throws Exception { - story.addStep(new Statement() { - @Override public void evaluate() throws Throwable { - SSHUserPrivateKey c = new DummyPrivateKey("creds", "bob", "secret", "the-key"); - CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), c); - SSHUserPrivateKeyBinding binding = new SSHUserPrivateKeyBinding("keyFile", "creds"); - binding.setPassphraseVariable("passphrase"); - binding.setUsernameVariable("user"); - BindingStep s = new StepConfigTester(story.j).configRoundTrip(new BindingStep( - Collections.singletonList(binding))); - story.j.assertEqualDataBoundBeans(s.getBindings(), Collections.singletonList(binding)); - } + @Test public void configRoundTrip() { + story.then(r -> { + SnippetizerTester st = new SnippetizerTester(r); + SSHUserPrivateKey c = new DummyPrivateKey("creds", "bob", "secret", "the-key"); + CredentialsProvider.lookupStores(story.j.jenkins).iterator().next().addCredentials(Domain.global(), c); + SSHUserPrivateKeyBinding binding = new SSHUserPrivateKeyBinding("keyFile", "creds"); + BindingStep s = new StepConfigTester(story.j).configRoundTrip(new BindingStep(Collections.singletonList(binding))); + st.assertRoundTrip(s, "withCredentials([sshUserPrivateKey(credentialsId: 'creds', keyFileVariable: 'keyFile')]) {\n // some block\n}"); + r.assertEqualDataBoundBeans(s.getBindings(), Collections.singletonList(binding)); + binding.setPassphraseVariable("passphrase"); + binding.setUsernameVariable("user"); + s = new StepConfigTester(story.j).configRoundTrip(new BindingStep(Collections.singletonList(binding))); + st.assertRoundTrip(s, "withCredentials([sshUserPrivateKey(credentialsId: 'creds', keyFileVariable: 'keyFile', passphraseVariable: 'passphrase', usernameVariable: 'user')]) {\n // some block\n}"); + r.assertEqualDataBoundBeans(s.getBindings(), Collections.singletonList(binding)); }); } - - @Test public void basics() throws Exception { + @Test public void basics() { final String credentialsId = "creds"; final String username = "bob"; final String passphrase = "s3cr3t"; @@ -138,7 +153,6 @@ public CredentialsScope getScope() { if (Functions.isWindows()) { script = " bat '''\n" - + " @echo off\n" + " echo %THEUSER%:%THEPASS% > out.txt\n" + " type \"%THEKEY%\" > key.txt" + " '''\n"; @@ -169,6 +183,7 @@ public CredentialsScope getScope() { SemaphoreStep.success("basics/1", null); story.j.waitForCompletion(b); story.j.assertBuildStatusSuccess(b); + story.j.assertLogNotContains(username, b); story.j.assertLogNotContains(passphrase, b); FilePath out = story.j.jenkins.getWorkspaceFor(p).child("out.txt"); assertTrue(out.exists()); @@ -177,11 +192,17 @@ public CredentialsScope getScope() { FilePath key = story.j.jenkins.getWorkspaceFor(p).child("key.txt"); assertTrue(key.exists()); assertEquals(keyContent, key.readToString().trim()); + + ((DummyPrivateKey) CredentialsProvider.lookupCredentials(SSHUserPrivateKey.class, story.j.jenkins, ACL.SYSTEM, Collections.emptyList()).get(0)).usernameSecret = false; + SemaphoreStep.success("basics/2", null); + b = story.j.buildAndAssertSuccess(p); + story.j.assertLogContains(username, b); + story.j.assertLogNotContains(passphrase, b); } }); } - @Test public void noUsernameOrPassphrase() throws Exception { + @Test public void noUsernameOrPassphrase() { final String credentialsId = "creds"; final String keyContent = "the-key"; story.addStep(new Statement() { @@ -193,7 +214,6 @@ public CredentialsScope getScope() { if (Functions.isWindows()) { script = " bat '''\n" - + " @echo off\n" + " type \"%THEKEY%\" > key.txt" + " '''\n"; } else { 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 dde2f3e4..a8f51565 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/SecretBuildWrapperTest.java @@ -40,7 +40,6 @@ import hudson.tasks.Recorder; 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; @@ -92,7 +91,7 @@ public class SecretBuildWrapperTest { @Issue("JENKINS-24805") @Test public void emptySecretsList() throws Exception { - SecretBuildWrapper wrapper = new SecretBuildWrapper(new ArrayList>()); + SecretBuildWrapper wrapper = new SecretBuildWrapper(new ArrayList<>()); FreeStyleProject f = r.createFreeStyleProject(); diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBindingTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBindingTest.java index b606a8f7..013414c0 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBindingTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordBindingTest.java @@ -48,15 +48,21 @@ import java.util.Collections; import java.util.List; import jenkins.model.Jenkins; -import static org.hamcrest.Matchers.*; import org.jenkinsci.plugins.credentialsbinding.Binding; import org.jenkinsci.plugins.credentialsbinding.MultiBinding; -import static org.junit.Assert.*; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; import org.xmlunit.matchers.CompareMatcher; +import static org.hamcrest.MatcherAssert.assertThat; +import static org.hamcrest.Matchers.hasItem; +import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.notNullValue; +import static org.hamcrest.Matchers.nullValue; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + public class UsernamePasswordBindingTest { @Rule public JenkinsRule r = new JenkinsRule(); @@ -88,8 +94,8 @@ public class UsernamePasswordBindingTest { @Test public void theSecretBuildWrapperTracksUsage() throws Exception { SystemCredentialsProvider.getInstance().setDomainCredentialsMap( - Collections.singletonMap(Domain.global(), Collections.emptyList())); - for (CredentialsStore s : CredentialsProvider.lookupStores(Jenkins.getInstance())) { + Collections.singletonMap(Domain.global(), Collections.emptyList())); + for (CredentialsStore s : CredentialsProvider.lookupStores(Jenkins.get())) { if (s.getProvider() instanceof SystemCredentialsProvider.ProviderImpl) { store = s; break; diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBindingTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBindingTest.java index 60b380b3..8ebf35f6 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBindingTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/UsernamePasswordMultiBindingTest.java @@ -43,26 +43,32 @@ import org.jenkinsci.plugins.credentialsbinding.MultiBinding; import org.junit.Test; -import static org.junit.Assert.*; +import org.junit.ClassRule; import org.junit.Rule; +import org.jvnet.hudson.test.BuildWatcher; import org.jvnet.hudson.test.JenkinsRule; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNotNull; + public class UsernamePasswordMultiBindingTest { @Rule public JenkinsRule r = new JenkinsRule(); + @ClassRule public static BuildWatcher bw = new BuildWatcher(); @Test public void basics() throws Exception { String username = "bob"; String password = "s3cr3t"; UsernamePasswordCredentialsImpl c = new UsernamePasswordCredentialsImpl(CredentialsScope.GLOBAL, null, "sample", username, password); + c.setUsernameSecret(true); CredentialsProvider.lookupStores(r.jenkins).iterator().next().addCredentials(Domain.global(), c); FreeStyleProject p = r.createFreeStyleProject(); p.getBuildWrappersList().add(new SecretBuildWrapper(Collections.>singletonList(new UsernamePasswordMultiBinding("userid", "pass", c.getId())))); if (Functions.isWindows()) { - p.getBuildersList().add(new BatchFile("@echo off\necho %userid%/%pass% > auth.txt")); + p.getBuildersList().add(new BatchFile("echo %userid%/%pass% > auth.txt")); } else { - p.getBuildersList().add(new Shell("set +x\necho $userid/$pass > auth.txt")); + p.getBuildersList().add(new Shell("echo $userid/$pass > auth.txt")); } r.configRoundtrip((Item)p); SecretBuildWrapper wrapper = p.getBuildWrappersList().get(SecretBuildWrapper.class); @@ -76,8 +82,15 @@ public class UsernamePasswordMultiBindingTest { assertEquals("pass", ((UsernamePasswordMultiBinding) binding).getPasswordVariable()); FreeStyleBuild b = r.buildAndAssertSuccess(p); r.assertLogNotContains(password, b); + r.assertLogNotContains(username, b); + assertEquals(username + '/' + password, b.getWorkspace().child("auth.txt").readToString().trim()); + assertEquals("[pass, userid]", new TreeSet<>(b.getSensitiveBuildVariables()).toString()); + c.setUsernameSecret(false); + b = r.buildAndAssertSuccess(p); + r.assertLogNotContains(password, b); + r.assertLogContains(username, b); assertEquals(username + '/' + password, b.getWorkspace().child("auth.txt").readToString().trim()); - assertEquals("[pass, userid]", new TreeSet(b.getSensitiveBuildVariables()).toString()); + assertEquals("[pass]", new TreeSet<>(b.getSensitiveBuildVariables()).toString()); } } diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBindingTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBindingTest.java index d9361036..331c3ad9 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBindingTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/ZipFileBindingTest.java @@ -45,7 +45,7 @@ public class ZipFileBindingTest { @Issue("JENKINS-30941") @Test public void cleanUpSucceeds() throws Exception { - /** Issue was just present on Linux not windows - but the test will run on both */ + /* Issue was just present on Linux not windows - but the test will run on both */ final String credentialsId = "zipfile"; diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactoryTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactoryTest.java index 7a8a7211..754e72cc 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactoryTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/BatchSecretPatternFactoryTest.java @@ -38,7 +38,7 @@ import java.io.IOException; import static org.hamcrest.CoreMatchers.containsString; -import static org.junit.Assert.assertThat; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assume.assumeTrue; public class BatchSecretPatternFactoryTest { @@ -67,7 +67,7 @@ public class BatchSecretPatternFactoryTest { private String credentialId; @Before - public void assumeWindowsForBatch() throws Exception { + public void assumeWindowsForBatch() { assumeTrue(Functions.isWindows()); } @@ -348,13 +348,12 @@ private WorkflowRun runDelayedAllQuotes() throws Exception { private void assertStringPresentInOrder(WorkflowRun run, String... values) throws Exception { String fullLog = run.getLog(); int currentIndex = 0; - for (int i = 0; i < values.length; i++) { - String currentValue = values[i]; + for (String currentValue : values) { int nextIndex = fullLog.indexOf(currentValue, currentIndex); - if(nextIndex == -1){ + if (nextIndex == -1) { // use assertThat to have better output assertThat(fullLog.substring(currentIndex), containsString(currentValue)); - }else{ + } else { currentIndex = nextIndex + currentValue.length(); } } diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/PowerShellMaskerProviderTest.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/PowerShellMaskerProviderTest.java index 9fedb87f..4f65c41b 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/PowerShellMaskerProviderTest.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/masking/PowerShellMaskerProviderTest.java @@ -36,9 +36,9 @@ import java.io.IOException; import static org.hamcrest.CoreMatchers.containsString; +import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; import static org.jenkinsci.plugins.credentialsbinding.test.Executables.executable; -import static org.junit.Assert.assertThat; import static org.junit.Assume.assumeThat; public class PowerShellMaskerProviderTest { @@ -56,7 +56,7 @@ public class PowerShellMaskerProviderTest { private String credentialId; @Before - public void assumeWindowsForBatch() throws Exception { + public void assumeWindowsForBatch() { // TODO: pwsh is also a valid executable name // https://github.com/jenkinsci/durable-task-plugin/pull/88 assumeThat("powershell", is(executable())); @@ -95,13 +95,12 @@ private void assertDirectNoPlainTextButStars(WorkflowRun run) throws Exception { private void assertStringPresentInOrder(WorkflowRun run, String... values) throws Exception { String fullLog = run.getLog(); int currentIndex = 0; - for (int i = 0; i < values.length; i++) { - String currentValue = values[i]; + for (String currentValue : values) { int nextIndex = fullLog.indexOf(currentValue, currentIndex); - if(nextIndex == -1){ + if (nextIndex == -1) { // use assertThat to have better output assertThat(fullLog.substring(currentIndex), containsString(currentValue)); - }else{ + } else { currentIndex = nextIndex + currentValue.length(); } } diff --git a/src/test/java/org/jenkinsci/plugins/credentialsbinding/test/Executables.java b/src/test/java/org/jenkinsci/plugins/credentialsbinding/test/Executables.java index 483b63e2..34ddbfd8 100644 --- a/src/test/java/org/jenkinsci/plugins/credentialsbinding/test/Executables.java +++ b/src/test/java/org/jenkinsci/plugins/credentialsbinding/test/Executables.java @@ -24,23 +24,25 @@ package org.jenkinsci.plugins.credentialsbinding.test; +import edu.umd.cs.findbugs.annotations.CheckForNull; +import edu.umd.cs.findbugs.annotations.NonNull; import hudson.Functions; import org.apache.commons.io.IOUtils; import org.hamcrest.CustomTypeSafeMatcher; import org.hamcrest.Matcher; -import javax.annotation.CheckForNull; -import javax.annotation.Nonnull; import java.io.IOException; +import java.nio.charset.Charset; import java.util.List; public class Executables { private static final String LOCATOR = Functions.isWindows() ? "where.exe" : "which"; - public static @CheckForNull String getPathToExecutable(@Nonnull String executable) { + public static @CheckForNull + String getPathToExecutable(@NonNull String executable) { try { Process process = new ProcessBuilder(LOCATOR, executable).start(); - List output = IOUtils.readLines(process.getInputStream()); + List output = IOUtils.readLines(process.getInputStream(), Charset.defaultCharset()); if (process.waitFor() != 0) { return null; } diff --git a/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step1.bat b/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step1.bat index b9eda2e6..97696b27 100644 --- a/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step1.bat +++ b/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step1.bat @@ -1,6 +1,6 @@ REM check existence of the keystore file if not defined MY_KEYSTORE exit /B 1 -if not exist %MY_KEYSTORE% exit /B 1 +if not exist "%MY_KEYSTORE%" exit /B 1 REM check the other variables if not defined KEYSTORE_PASSWORD exit /B 1 @@ -8,4 +8,3 @@ if not defined KEYSTORE_ALIAS exit /B 1 REM keep location of the keystore file for the next step echo %MY_KEYSTORE% > keystore-path - diff --git a/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step2.bat b/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step2.bat index 564e1f7d..3ff34a14 100644 --- a/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step2.bat +++ b/src/test/resources/org/jenkinsci/plugins/credentialsbinding/impl/CertificateMultiBindingTest/basicsPipeline-step2.bat @@ -3,4 +3,4 @@ if not exist keystore-path exit /B 1 set /p keystore_path=