Merge pull request #31 from amuniz/JENKINS-34853

[JENKINS-34853] Inject required resources environment variable
jenkinsci · Jun 1, 2016 · b4ff2fe · b4ff2fe
2 parents a57c50c + c114882
commit b4ff2fe
Show file tree

Hide file tree

Showing 9 changed files with 117 additions and 11 deletions.
diff --git a/pom.xml b/pom.xml
@@ -13,7 +13,8 @@
 	<parent>
 		<groupId>org.jenkins-ci.plugins</groupId>
 		<artifactId>plugin</artifactId>
-		<version>1.609.1</version>
+		<version>2.5</version>
+		<relativePath />
 	</parent>
 
 	<groupId>org.6wind.jenkins</groupId>
@@ -32,6 +33,9 @@
 		<!-- First version including TailCall is 1.9, and the same baseline is maintained
 		until 1.14, so let's pick up the greatest -->
 		<workflow.version>1.14</workflow.version>
+		<jenkins.version>1.609.1</jenkins.version>
+		<!-- TODO activate this and fix findbugs errors -->
+		<findbugs.failOnError>false</findbugs.failOnError>
 	</properties>
 
 	<licenses>
@@ -80,11 +84,6 @@
 			<artifactId>matrix-project</artifactId>
 			<version>1.4</version>
 		</dependency>
-		<dependency>
-			<groupId>com.google.code.findbugs</groupId>
-			<artifactId>annotations</artifactId>
-			<version>3.0.0</version>
-		</dependency>
 		<dependency>
 			<groupId>com.infradna.tool</groupId>
 			<artifactId>bridge-method-annotation</artifactId>
@@ -112,6 +111,12 @@
 			<version>1.6</version>
 			<scope>test</scope>
 		</dependency>
+		<dependency><!-- Required when testing against core > 1.575 -->
+			<groupId>org.jenkins-ci.plugins</groupId>
+			<artifactId>junit</artifactId>
+			<version>1.13</version>
+			<scope>test</scope>
+		</dependency>
 	</dependencies>
 
 	<build>

diff --git a/src/main/java/org/jenkins/plugins/lockableresources/actions/ResourceVariableNameAction.java b/src/main/java/org/jenkins/plugins/lockableresources/actions/ResourceVariableNameAction.java
@@ -0,0 +1,43 @@
+package org.jenkins.plugins.lockableresources.actions;
+
+import java.io.IOException;
+
+import org.kohsuke.accmod.Restricted;
+import org.kohsuke.accmod.restrictions.NoExternalUse;
+
+import hudson.EnvVars;
+import hudson.Extension;
+import hudson.model.EnvironmentContributor;
+import hudson.model.InvisibleAction;
+import hudson.model.Run;
+import hudson.model.StringParameterValue;
+import hudson.model.TaskListener;
+
+@Restricted(NoExternalUse.class)
+public class ResourceVariableNameAction extends InvisibleAction {
+
+	private final StringParameterValue resourceNameParameter;
+
+	public ResourceVariableNameAction(StringParameterValue r) {
+		this.resourceNameParameter = r;
+	}
+
+	StringParameterValue getParameter() {
+		return resourceNameParameter;
+	}
+
+	@Extension
+	public static final class ResourceVariableNameActionEnvironmentContributor extends EnvironmentContributor {
+
+		@Override
+		public void buildEnvironmentFor(Run r, EnvVars envs, TaskListener listener)
+				throws IOException, InterruptedException {
+			ResourceVariableNameAction a = r.getAction(ResourceVariableNameAction.class);
+			if (a != null && a.getParameter() != null && a.getParameter().getValue() != null) {
+				envs.put(a.getParameter().getName(), String.valueOf(a.getParameter().getValue()));
+			}
+		}
+
+	}
+
+}
diff --git a/src/main/java/org/jenkins/plugins/lockableresources/queue/LockRunListener.java b/src/main/java/org/jenkins/plugins/lockableresources/queue/LockRunListener.java
@@ -25,6 +25,7 @@
 import org.jenkins.plugins.lockableresources.LockableResourcesManager;
 import org.jenkins.plugins.lockableresources.LockableResource;
 import org.jenkins.plugins.lockableresources.actions.LockedResourcesBuildAction;
+import org.jenkins.plugins.lockableresources.actions.ResourceVariableNameAction;
 
 @Extension
 public class LockRunListener extends RunListener<AbstractBuild<?, ?>> {
@@ -59,11 +60,9 @@ public void onStarted(AbstractBuild<?, ?> build, TaskListener listener) {
 					LOGGER.fine(build.getFullDisplayName()
 							+ " acquired lock on " + required);
 					if (resources.requiredVar != null) {
-						List<ParameterValue> params = new ArrayList<ParameterValue>();
-						params.add(new StringParameterValue(
-							resources.requiredVar,
-							required.toString().replaceAll("[\\]\\[]", "")));
-						build.addAction(new ParametersAction(params));
+						build.addAction(new ResourceVariableNameAction(new StringParameterValue(
+								resources.requiredVar,
+								required.toString().replaceAll("[\\]\\[]", ""))));
 					}
 				} else {
 					listener.getLogger().printf("%s failed to lock %s\n",

diff --git a/src/main/resources/index.jelly b/src/main/resources/index.jelly
@@ -1,3 +1,4 @@
+<?jelly escape-by-default='true'?>
 <!--
  * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * Copyright (c) 2013, 6WIND S.A. All rights reserved.                 *

diff --git a/src/main/resources/org/jenkins/plugins/lockableresources/LockStep/config.jelly b/src/main/resources/org/jenkins/plugins/lockableresources/LockStep/config.jelly
@@ -1,3 +1,4 @@
+<?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:d="jelly:define"
 	xmlns:l="/lib/layout" xmlns:t="/lib/hudson" xmlns:f="/lib/form">
 	<f:entry title="${%Resource}" field="resource">

diff --git a/src/main/resources/org/jenkins/plugins/lockableresources/LockableResource/config.jelly b/src/main/resources/org/jenkins/plugins/lockableresources/LockableResource/config.jelly
@@ -1,3 +1,4 @@
+<?jelly escape-by-default='true'?>
 <!--
  * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * Copyright (c) 2013, 6WIND S.A. All rights reserved.                 *

diff --git a/...ain/resources/org/jenkins/plugins/lockableresources/LockableResourcesManager/config.jelly b/...ain/resources/org/jenkins/plugins/lockableresources/LockableResourcesManager/config.jelly
@@ -1,3 +1,4 @@
+<?jelly escape-by-default='true'?>
 <!--
  * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * Copyright (c) 2013, 6WIND S.A. All rights reserved.                 *

diff --git a/...in/resources/org/jenkins/plugins/lockableresources/RequiredResourcesProperty/config.jelly b/...in/resources/org/jenkins/plugins/lockableresources/RequiredResourcesProperty/config.jelly
@@ -1,3 +1,4 @@
+<?jelly escape-by-default='true'?>
 <!--
  * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * * *
  * Copyright (c) 2013, 6WIND S.A. All rights reserved.                 *

diff --git a/src/test/java/org/jenkins/plugins/lockableresources/BasicIntegrationTest.java b/src/test/java/org/jenkins/plugins/lockableresources/BasicIntegrationTest.java
@@ -0,0 +1,54 @@
+package org.jenkins.plugins.lockableresources;
+
+import java.io.IOException;
+import java.util.concurrent.ExecutionException;
+import java.util.concurrent.Semaphore;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.MockBuilder;
+import org.jvnet.hudson.test.TestExtension;
+
+import hudson.Launcher;
+import hudson.model.AbstractBuild;
+import hudson.model.BuildListener;
+import hudson.model.FreeStyleBuild;
+import hudson.model.FreeStyleProject;
+import hudson.model.Result;
+
+public class BasicIntegrationTest {
+
+	@Rule
+	public JenkinsRule j = new JenkinsRule();
+
+	@Test
+	@Issue("JENKINS-34853")
+	public void security170fix() throws Exception {
+		LockableResourcesManager.get().createResource("resource1");
+		FreeStyleProject p = j.createFreeStyleProject("p");
+		p.addProperty(new RequiredResourcesProperty("resource1", "resourceNameVar", null, null));
+		p.getBuildersList().add(new PrinterBuilder());
+
+		FreeStyleBuild b1 = p.scheduleBuild2(0).get();
+		j.assertLogContains("resourceNameVar: resource1", b1);
+		j.assertBuildStatus(Result.SUCCESS, b1);
+	}
+
+	@TestExtension
+	public static class PrinterBuilder extends MockBuilder {
+
+		public PrinterBuilder() {
+			super(Result.SUCCESS);
+		}
+
+		@Override
+		public boolean perform(AbstractBuild<?, ?> build, Launcher launcher, BuildListener listener) throws InterruptedException, IOException {
+			listener.getLogger().println("resourceNameVar: " + build.getEnvironment(listener).get("resourceNameVar"));
+			return true;
+		}
+
+	}
+
+}