Skip to content

Commit

Permalink
Merge pull request #143 from jglick/JENKINS-64631-amendment
Browse files Browse the repository at this point in the history 
[JENKINS-64361] Make fix for JENKINS-44860 apply to Pipeline step arguments as well (take II)
  • Loading branch information
@jglick
jglick authored Jul 12, 2021
2 parents 543b13d + fde8db9 commit 824c0c9
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 14 deletions.
35 changes: 21 additions & 14 deletions src/main/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStep.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,6 @@
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Set;
Expand Down Expand Up @@ -124,9 +123,8 @@ private void doStart() throws Exception {
FilePath workspace = getContext().get(FilePath.class);
Launcher launcher = getContext().get(Launcher.class);

Collection<String> secrets = new HashSet<>();
Collection<String> secretKeys = new LinkedHashSet<>();
Map<String,String> overrides = new LinkedHashMap<>();
Map<String,String> secretOverrides = new LinkedHashMap<>();
Map<String,String> publicOverrides = new LinkedHashMap<>();
List<MultiBinding.Unbinder> unbinders = new ArrayList<>();
for (MultiBinding<?> binding : step.bindings) {
if (binding.getDescriptor().requiresWorkspace() &&
Expand All @@ -135,21 +133,18 @@ private void doStart() throws Exception {
}
MultiBinding.MultiEnvironment environment = binding.bind(run, workspace, launcher, listener);
unbinders.add(environment.getUnbinder());
Map<String, String> secretValues = environment.getSecretValues();
secrets.addAll(secretValues.values());
secretKeys.addAll(secretValues.keySet());
overrides.putAll(secretValues);
overrides.putAll(environment.getPublicValues());
secretOverrides.putAll(environment.getSecretValues());
publicOverrides.putAll(environment.getPublicValues());
}
if (!overrides.isEmpty()) {
if (!secretOverrides.isEmpty()) {
boolean unix = launcher != null ? launcher.isUnix() : true;
listener.getLogger().println("Masking supported pattern matches of " + secretKeys.stream().map(
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(secrets, 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();
}
Expand Down Expand Up @@ -177,22 +172,34 @@ private static final class Overrider extends EnvironmentExpander {
private static final long serialVersionUID = 1;

private final Map<String,Secret> overrides = new HashMap<String,Secret>();
private Map<String, String> publicOverrides;

Overrider(Map<String,String> overrides) {
Overrider(Map<String,String> overrides, Map<String, String> publicOverrides) {
for (Map.Entry<String,String> override : overrides.entrySet()) {
this.overrides.put(override.getKey(), Secret.fromString(override.getValue()));
}
this.publicOverrides = publicOverrides;
}

@Override public void expand(EnvVars env) throws IOException, InterruptedException {
for (Map.Entry<String,Secret> override : overrides.entrySet()) {
env.override(override.getKey(), override.getValue().getPlainText());
}
for (Map.Entry<String, String> override : publicOverrides.entrySet()) {
env.override(override.getKey(), override.getValue());
}
}

@Override public Set<String> getSensitiveVariables() {
return Collections.unmodifiableSet(overrides.keySet());
}

private Object readResolve() {
if (publicOverrides == null) {
publicOverrides = new HashMap<>();
}
return this;
}
}

/** Similar to {@code MaskPasswordsOutputStream}. */
Expand Down
30 changes: 30 additions & 0 deletions src/test/java/org/jenkinsci/plugins/credentialsbinding/impl/BindingStepTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,8 +65,13 @@
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;
Expand All @@ -75,10 +80,12 @@
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.hamcrest.MatcherAssert.assertThat;
import static org.junit.Assert.*;
import org.junit.ClassRule;

Expand Down Expand Up @@ -445,6 +452,29 @@ 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));
});
}

private static Set<String> grep(File dir, String text) throws IOException {
Set<String> matches = new TreeSet<String>();
grep(dir, text, "", matches);
Expand Down

0 comments on commit 824c0c9

Please sign in to comment.