Skip to content
Permalink
Browse files
Merge pull request #2353 from rsandell/safe-parameters
[JENKINS-34858] - Listed Parameters should reflect what was used when the build ran
(cherry picked from commit 74d0412)
  • Loading branch information
rsandell authored and olivergondza committed May 25, 2016
1 parent baf831f commit 68a88a1e4229749df85799a91bb739f6a7d6e5a1
Showing with 140 additions and 13 deletions.
  1. +44 −13 core/src/main/java/hudson/model/ParametersAction.java
  2. +96 −0 test/src/test/java/hudson/model/ParametersActionTest2.java
@@ -46,6 +46,7 @@
import java.util.Iterator;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;
import java.util.logging.Level;
import java.util.logging.Logger;

@@ -73,7 +74,7 @@
public static final String SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME = ParametersAction.class.getName() +
".safeParameters";

private transient List<String> safeParameters;
private Set<String> safeParameters;

private final List<ParameterValue> parameters;

@@ -89,6 +90,29 @@

public ParametersAction(List<ParameterValue> parameters) {
this.parameters = parameters;
String paramNames = System.getProperty(SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME);
safeParameters = new TreeSet<>();
if (paramNames != null) {
safeParameters.addAll(Arrays.asList(paramNames.split(",")));
}
}

/**
* Constructs a new action with additional safe parameters.
* The additional safe parameters should be only those considered safe to override the environment
* and what is declared in the project config in addition to those specified by the user in
* {@link #SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME}.
* See <a href="https://issues.jenkins-ci.org/browse/SECURITY-170">SECURITY-170</a>
*
* @param parameters the parameters
* @param additionalSafeParameters additional safe parameters
* @since TODO
*/
public ParametersAction(List<ParameterValue> parameters, Collection<String> additionalSafeParameters) {
this(parameters);
if (additionalSafeParameters != null) {
safeParameters.addAll(additionalSafeParameters);
}
}

public ParametersAction(ParameterValue... parameters) {
@@ -202,7 +226,9 @@ public boolean shouldSchedule(List<Action> actions) {
@Nonnull
public ParametersAction createUpdated(Collection<? extends ParameterValue> overrides) {
if(overrides == null) {
return new ParametersAction(parameters);
ParametersAction parametersAction = new ParametersAction(parameters);
parametersAction.safeParameters = this.safeParameters;
return parametersAction;
}
List<ParameterValue> combinedParameters = newArrayList(overrides);
Set<String> names = newHashSet();
@@ -219,7 +245,7 @@ public ParametersAction createUpdated(Collection<? extends ParameterValue> overr
}
}

return new ParametersAction(combinedParameters);
return new ParametersAction(combinedParameters, this.safeParameters);
}

/*
@@ -230,14 +256,27 @@ public ParametersAction createUpdated(Collection<? extends ParameterValue> overr
@Nonnull
public ParametersAction merge(@CheckForNull ParametersAction overrides) {
if (overrides == null) {
return new ParametersAction(parameters);
ParametersAction parametersAction = new ParametersAction(parameters, this.safeParameters);
return parametersAction;
}
return createUpdated(overrides.parameters);
ParametersAction parametersAction = createUpdated(overrides.parameters);
Set<String> safe = new TreeSet<>();
if (parametersAction.safeParameters != null && this.safeParameters != null) {
safe.addAll(this.safeParameters);
}
if (overrides.safeParameters != null) {
safe.addAll(overrides.safeParameters);
}
parametersAction.safeParameters = safe;
return parametersAction;
}

private Object readResolve() {
if (build != null)
OldDataMonitor.report(build, "1.283");
if (safeParameters == null) {
safeParameters = Collections.emptySet();
}
return this;
}

@@ -301,14 +340,6 @@ public void onLoad(Run<?, ?> r) {
}

private boolean isSafeParameter(String name) {
if (safeParameters == null) {
String paramNames = System.getProperty(SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME);
if (paramNames != null) {
safeParameters = Arrays.asList(paramNames.split(","));
} else {
safeParameters = Collections.emptyList();
}
}
return safeParameters.contains(name);
}

@@ -10,6 +10,7 @@

import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
@@ -149,6 +150,100 @@ public void whitelistedParameter() throws Exception {
}
}

@Test
@Issue("SECURITY-170")
public void whitelistedParameterByOverride() throws Exception {
FreeStyleProject p = j.createFreeStyleProject();
String name = p.getFullName();
p.addProperty(new ParametersDefinitionProperty(Arrays.<ParameterDefinition>asList(
new StringParameterDefinition("foo", "foo"),
new StringParameterDefinition("bar", "bar"))));

try {
ParametersAction action = new ParametersAction(
Arrays.<ParameterValue>asList(
new StringParameterValue("foo", "baz"),
new StringParameterValue("bar", "bar"),
new StringParameterValue("whitelisted1", "x"),
new StringParameterValue("whitelisted2", "y"),
new StringParameterValue("whitelisted3", "y")
),
Arrays.asList("whitelisted1", "whitelisted2"));
FreeStyleBuild build = j.assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause(), action));

assertTrue("whitelisted1 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted1"));
assertTrue("whitelisted2 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted2"));
assertFalse("whitelisted3 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted3"));
p = null;
build = null;
j.jenkins.reload();
//Test again after reload
p = j.jenkins.getItemByFullName(name, FreeStyleProject.class);
build = p.getLastBuild();
assertTrue("whitelisted1 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted1"));
assertTrue("whitelisted2 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted2"));
assertFalse("whitelisted3 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted3"));
} finally {
System.clearProperty(ParametersAction.SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME);
}
}

@Test
@Issue("SECURITY-170")
public void whitelistedParameterSameAfterChange() throws Exception {
FreeStyleProject p = j.createFreeStyleProject();
String name = p.getFullName();
p.addProperty(new ParametersDefinitionProperty(Arrays.<ParameterDefinition>asList(
new StringParameterDefinition("foo", "foo"),
new StringParameterDefinition("bar", "bar"))));

try {
System.setProperty(ParametersAction.SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME, "whitelisted1,whitelisted2");
FreeStyleBuild build = j.assertBuildStatusSuccess(p.scheduleBuild2(0, new Cause.UserIdCause(), new ParametersAction(
new StringParameterValue("foo", "baz"),
new StringParameterValue("bar", "bar"),
new StringParameterValue("whitelisted1", "x"),
new StringParameterValue("whitelisted2", "y"),
new StringParameterValue("whitelisted3", "z"),
new StringParameterValue("whitelisted4", "w")
)));
assertTrue("whitelisted1 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted1"));
assertTrue("whitelisted2 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted2"));
assertFalse("whitelisted3 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted3"));
assertFalse("whitelisted4 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted4"));

System.setProperty(ParametersAction.SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME, "whitelisted3,whitelisted4");
p = null;
build = null;
j.jenkins.reload();
p = j.jenkins.getItemByFullName(name, FreeStyleProject.class);
build = p.getLastBuild();
assertTrue("whitelisted1 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted1"));
assertTrue("whitelisted2 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted2"));
assertFalse("whitelisted3 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted3"));
assertFalse("whitelisted4 parameter is listed in getParameters",
hasParameterWithName(build.getAction(ParametersAction.class), "whitelisted4"));

} finally {
System.clearProperty(ParametersAction.SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME);
}
}



@Test
@Issue("SECURITY-170")
public void nonParameterizedJob() throws Exception {
@@ -194,6 +289,7 @@ public static boolean hasParameterWithName(Iterable<ParameterValue> values, Stri
return false;
}


public static class ParametersCheckBuilder extends Builder {

private final boolean expectLegacyBehavior;

0 comments on commit 68a88a1

Please sign in to comment.