Skip to content
Permalink
Browse files
Merge pull request #285 from jenkinsci/JENKINS-34753
[JENKINS-34753] Provide safe parameters to ParametersAction
  • Loading branch information
rsandell committed May 30, 2016
2 parents 644d75d + d8878ba commit 8b4652554b63ec92768c4b8ad840cae0522d4c7d
@@ -54,6 +54,7 @@

<properties>
<jenkins.version>1.609.3</jenkins.version>
<!--<jenkins.version>2.5-SNAPSHOT</jenkins.version>-->
<java.level>6</java.level>
<project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
<findbugs.failOnError>false</findbugs.failOnError>
@@ -40,14 +40,14 @@
import hudson.model.Result;
import hudson.model.Run;
import hudson.model.TaskListener;
import hudson.model.Hudson;

import java.util.Arrays;
import java.util.Collections;
import java.util.Comparator;
import java.util.HashMap;
import java.util.Map;

import jenkins.model.Jenkins;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

@@ -72,24 +72,24 @@

private static final Logger logger = LoggerFactory.getLogger(ParameterExpander.class);
private IGerritHudsonTriggerConfig config;
private Hudson hudson;
private Jenkins jenkins;

/**
* Constructor.
* @param config the global config.
* @param hudson the Hudson instance.
* @param jenkins the Hudson instance.
*/
public ParameterExpander(IGerritHudsonTriggerConfig config, Hudson hudson) {
public ParameterExpander(IGerritHudsonTriggerConfig config, Jenkins jenkins) {
this.config = config;
this.hudson = hudson;
this.jenkins = jenkins;
}

/**
* Constructor.
* @param config the global config.
*/
public ParameterExpander(IGerritHudsonTriggerConfig config) {
this(config, Hudson.getInstance());
this(config, Jenkins.getInstance());
}

/**
@@ -240,7 +240,7 @@ private Integer getBuildStartedCodeReviewValue(Run r) {
}
}
if (r != null) {
map.put("BUILDURL", hudson.getRootUrl() + r.getUrl());
map.put("BUILDURL", jenkins.getRootUrl() + r.getUrl());
}
map.put("VERIFIED", String.valueOf(verified));
map.put("CODE_REVIEW", String.valueOf(codeReview));
@@ -554,7 +554,7 @@ public String getBuildCompletedCommand(MemoryImprint memoryImprint, TaskListener
private String createBuildsStats(MemoryImprint memoryImprint, TaskListener listener,
Map<String, String> parameters) {
StringBuilder str = new StringBuilder("");
final String rootUrl = hudson.getRootUrl();
final String rootUrl = jenkins.getRootUrl();

String unsuccessfulMessage = null;

@@ -43,13 +43,19 @@
import hudson.model.ParametersDefinitionProperty;
import jenkins.model.Jenkins;
import jenkins.model.ParameterizedJobMixIn;
import org.apache.commons.lang3.StringUtils;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import javax.annotation.CheckForNull;
import javax.annotation.Nonnull;
import java.lang.reflect.Constructor;
import java.lang.reflect.Field;
import java.lang.reflect.Modifier;
import java.util.ArrayList;
import java.util.Collection;
import java.util.List;
import java.util.Locale;
import java.util.concurrent.Future;

import static com.sonyericsson.hudson.plugins.gerrit.trigger.PluginImpl.getServerConfig;
@@ -243,6 +249,10 @@ protected Job asJob() {

/**
* Creates a ParameterAction and fills it with the project's default parameters + the Standard Gerrit parameters.
* If running on a core version that let's us specify safeParameters for the ParameterAction
* the Gerrit specific parameters will be specified in the safeParameters list in addition to anything the admin
* might have set.
* A warning will be printed to the log if that is not possible but SECURITY-170 appears to be in effect.
*
* @param event the event.
* @param project the project.
@@ -251,6 +261,42 @@ protected Job asJob() {
protected ParametersAction createParameters(GerritTriggeredEvent event, Job project) {
List<ParameterValue> parameters = getDefaultParametersValues(project);
setOrCreateParameters(event, project, parameters);
try {
Constructor<ParametersAction> constructor = ParametersAction.class.getConstructor(List.class,
Collection.class);
return constructor.newInstance(parameters, GerritTriggerParameters.getNamesSet());
} catch (NoSuchMethodException e) {
ParametersActionInspection inspection = getParametersInspection();
if (inspection.isInspectionFailure()) {
logger.warn("Failed to inspect ParametersAction to determine "
+ "if we can behave normally around SECURITY-170.\nSee "
+ "https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2016-05-11"
+ " for information.");
} else if (inspection.isHasSafeParameterConfig()) {
StringBuilder txt = new StringBuilder(
"Running on a core with SECURITY-170 fixed but no direct way for Gerrit Trigger"
+ " to self-specify safe parameters.");
txt.append(" You should consider upgrading to a new Jenkins core version.\n");
if (inspection.isKeepUndefinedParameters()) {
txt.append(".keepUndefinedParameters is set so the trigger should behave normally.");
} else if (inspection.isSafeParametersSet()) {
txt.append("All Gerrit related parameters are set in .safeParameters");
txt.append(" so the trigger should behave normally.");
} else {
txt.append("No overriding system properties appears to be set,");
txt.append(" your builds might not work as expected.\n");
txt.append("See https://wiki.jenkins-ci.org/display/SECURITY/Jenkins+Security+Advisory+2016-05-11");
txt.append(" for information.");
}
logger.warn(txt.toString());
} else {
logger.debug("Running on an old core before safe parameters, we should be safe.", e);
}
} catch (IllegalAccessException e) {
logger.warn("Running on a core with safe parameters fix available, but not allowed to specify them", e);
} catch (Exception e) {
logger.warn("Running on a core with safe parameters fix available, but failed to provide them", e);
}
return new ParametersAction(parameters);
}

@@ -332,4 +378,113 @@ public int hashCode() {
public boolean equals(Object obj) {
return obj instanceof EventListener && ((EventListener)obj).job.equals(job);
}

/**
* Inspects {@link ParametersAction} to see what kind of capabilities it has in regards to SECURITY-170.
* Assuming the safeParameters constructor could not be found.
*
* @return the inspection result
* @see #createParameters(GerritTriggeredEvent, Job)
*/
private static synchronized ParametersActionInspection getParametersInspection() {
if (parametersInspectionCache == null) {
parametersInspectionCache = new ParametersActionInspection();
}
return parametersInspectionCache;
}

/**
* Stored cache of the inspection.
* @see #getParametersInspection()
*/
private static volatile ParametersActionInspection parametersInspectionCache = null;

/**
* Data structure with information regarding what kind of capabilities {@link ParametersAction} has.
* @see #getParametersInspection()
* @see #createParameters(GerritTriggeredEvent, Job)
*/
private static class ParametersActionInspection {
private static final Class<ParametersAction> KLASS = ParametersAction.class;
private boolean inspectionFailure;
private boolean safeParametersSet = false;
private boolean keepUndefinedParameters = false;
private boolean hasSafeParameterConfig = false;

/**
* Constructor that performs the inspection.
*/
ParametersActionInspection() {
try {
for (Field field : KLASS.getDeclaredFields()) {
if (Modifier.isStatic(field.getModifiers())
&& (
field.getName().equals("KEEP_UNDEFINED_PARAMETERS_SYSTEM_PROPERTY_NAME")
|| field.getName().equals("SAFE_PARAMETERS_SYSTEM_PROPERTY_NAME")
)
) {
this.hasSafeParameterConfig = true;
break;
}
}
if (hasSafeParameterConfig) {
if (Boolean.getBoolean(KLASS.getName() + ".keepUndefinedParameters")) {
this.keepUndefinedParameters = true;
}
String safeParameters = System.getProperty(KLASS.getName() + ".safeParameters");
if (!StringUtils.isBlank(safeParameters)) {
safeParameters = safeParameters.toUpperCase(Locale.ENGLISH);
boolean declared = true;
for (GerritTriggerParameters parameter : GerritTriggerParameters.values()) {
if (!safeParameters.contains(parameter.name())) {
declared = false;
break;
}
}
this.safeParametersSet = declared;
} else {
this.safeParametersSet = false;
}
}
this.inspectionFailure = false;
} catch (Exception e) {
this.inspectionFailure = true;
}
}

/**
* If the system property .safeParameters is set and contains all Gerrit related parameters.
* @return true if so.
*/
boolean isSafeParametersSet() {
return safeParametersSet;
}

/**
* If the system property .keepUndefinedParameters is set and set to true.
*
* @return true if so.
*/
boolean isKeepUndefinedParameters() {
return keepUndefinedParameters;
}

/**
* If any of the constant fields regarding safeParameters are declared in {@link ParametersAction}.
*
* @return true if so.
*/
boolean isHasSafeParameterConfig() {
return hasSafeParameterConfig;
}

/**
* If there was an exception when inspecting the class.
*
* @return true if so.
*/
public boolean isInspectionFailure() {
return inspectionFailure;
}
}
}
@@ -233,7 +233,10 @@ public boolean equals(Object o) {

@Override
public int hashCode() {
int result = tEvent.hashCode();
int result = 1;
if (tEvent != null) {
result = tEvent.hashCode();
}
result = 31 * result + (silentMode ? 1 : 0);
return result;
}
@@ -51,6 +51,8 @@
import java.lang.reflect.Constructor;
import java.nio.charset.Charset;
import java.util.List;
import java.util.Set;
import java.util.TreeSet;

/**
* The parameters to add to a build.
@@ -218,6 +220,20 @@

private static final Logger logger = LoggerFactory.getLogger(GerritTriggerParameters.class);

/**
* A set of all the declared parameter names.
* @return the names of the parameters
* @see #values()
* @see #name()
*/
public static Set<String> getNamesSet() {
Set<String> names = new TreeSet<String>();
for (GerritTriggerParameters p : GerritTriggerParameters.values()) {
names.add(p.name());
}
return names;
}

/**
* Creates a {@link hudson.model.ParameterValue} and adds it to the provided list.
* If the parameter with the same name already exists in the list it will be replaced by the new parameter,
@@ -30,10 +30,16 @@

import hudson.model.Result;

import jenkins.model.Jenkins;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.junit.runners.Parameterized.Parameters;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.modules.junit4.PowerMockRunnerDelegate;

import java.util.Collection;
import java.util.LinkedList;
@@ -51,7 +57,9 @@
* and {@link ParameterExpander#getVerifiedValue(hudson.model.Result, GerritTrigger)}
* @author Robert Sandell &lt;robert.sandell@sonyericsson.com&gt;
*/
@RunWith(Parameterized.class)
@RunWith(PowerMockRunner.class)
@PowerMockRunnerDelegate(Parameterized.class)
@PrepareForTest(Jenkins.class)
public class ParameterExpanderParameterizedTest {

private TestParameters parameters;
@@ -64,6 +72,16 @@ public ParameterExpanderParameterizedTest(TestParameters parameters) {
this.parameters = parameters;
}

/**
* Mock Jenkins.
*/
@Before
public void setup() {
PowerMockito.mockStatic(Jenkins.class);
Jenkins jenkins = PowerMockito.mock(Jenkins.class);
PowerMockito.when(Jenkins.getInstance()).thenReturn(jenkins);
}

/**
* test.
*/
@@ -30,9 +30,15 @@

import hudson.model.Result;

import jenkins.model.Jenkins;
import org.junit.Before;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.Parameterized;
import org.powermock.api.mockito.PowerMockito;
import org.powermock.core.classloader.annotations.PrepareForTest;
import org.powermock.modules.junit4.PowerMockRunner;
import org.powermock.modules.junit4.PowerMockRunnerDelegate;

import java.util.Collection;
import java.util.LinkedList;
@@ -51,7 +57,9 @@
*
* @author Robert Sandell &lt;robert.sandell@sonymobile.com&gt;
*/
@RunWith(Parameterized.class)
@RunWith(PowerMockRunner.class)
@PowerMockRunnerDelegate(Parameterized.class)
@PrepareForTest(Jenkins.class)
public class ParameterExpanderSkipVoteParameterTest {

private TestParameter parameter;
@@ -65,6 +73,16 @@ public ParameterExpanderSkipVoteParameterTest(TestParameter parameter) {
this.parameter = parameter;
}

/**
* Mock Jenkins.
*/
@Before
public void setup() {
PowerMockito.mockStatic(Jenkins.class);
Jenkins jenkins = PowerMockito.mock(Jenkins.class);
PowerMockito.when(Jenkins.getInstance()).thenReturn(jenkins);
}

/**
* Tests that {@link ParameterExpander#getMinimumCodeReviewValue(BuildMemory.MemoryImprint, boolean)}
* returns {@link TestParameter#expectedCodeReview}.

0 comments on commit 8b46525

Please sign in to comment.