JENKINS-48014 Allow sandboxing for Groovy scripts

jenkinsci · Feb 11, 2018 · 4575c24 · 4575c24
1 parent e8414f1
commit 4575c24
Show file tree

Hide file tree

Showing 14 changed files with 110 additions and 38 deletions.
diff --git a/README.md b/README.md
@@ -89,6 +89,12 @@ changes without needing to run to `package` phase.
 
 ## Release Notes
 
+### Version 2.5.0
+
+This version introduces the ability to run Groovy scripts in a sandbox when activated using the checkbox in the configuration view.
+
+* JENKINS-48014 - Allow sandboxing for Groovy scripts
+
 ### Version 2.4.0
 
 The old option to build only on success or on failure will now be migrated differently: If both is not selected, the

diff --git a/pom.xml b/pom.xml
@@ -10,7 +10,7 @@
     <artifactId>postbuildscript</artifactId>
     <packaging>hpi</packaging>
     <name>Jenkins PostBuildScript Plugin</name>
-    <version>2.4.1-SNAPSHOT</version>
+    <version>2.5.0-SNAPSHOT</version>
     <url>http://wiki.jenkins-ci.org/display/JENKINS/PostBuildScript+Plugin</url>
     <inceptionYear>2011</inceptionYear>
 

diff --git a/src/main/java/org/jenkinsci/plugins/postbuildscript/Processor.java b/src/main/java/org/jenkinsci/plugins/postbuildscript/Processor.java
@@ -131,7 +131,7 @@ private boolean processScriptFiles() throws PostBuildScriptException {
                         return false;
                     }
                 } else {
-                    if (!scriptPreparer.evaluateCommand(command)) {
+                    if (!scriptPreparer.evaluateCommand(scriptFile, command)) {
                         return false;
                     }
                 }
@@ -161,7 +161,7 @@ private boolean processGroovyScripts() {
 
             String content = script.getContent();
             if (content != null) {
-                if (!executor.evaluateScript(content)) {
+                if (!executor.evaluateScript(script)) {
                     return false;
                 }
             }

diff --git a/src/main/java/org/jenkinsci/plugins/postbuildscript/model/Script.java b/src/main/java/org/jenkinsci/plugins/postbuildscript/model/Script.java
@@ -2,13 +2,16 @@
 
 import hudson.Util;
 import org.kohsuke.stapler.DataBoundConstructor;
+import org.kohsuke.stapler.DataBoundSetter;
 
 import java.util.Collection;
 
 public class Script extends PostBuildItem {
 
     private final String content;
 
+    private boolean sandboxed;
+
     @DataBoundConstructor
     public Script(Collection<String> results, String content) {
         super(results);
@@ -19,4 +22,13 @@ public String getContent() {
         return content;
     }
 
+    @DataBoundSetter
+    public void setSandboxed(boolean sandboxed) {
+        this.sandboxed = sandboxed;
+    }
+
+    public boolean isSandboxed() {
+        return sandboxed;
+    }
+
 }
diff --git a/src/main/java/org/jenkinsci/plugins/postbuildscript/model/ScriptFile.java b/src/main/java/org/jenkinsci/plugins/postbuildscript/model/ScriptFile.java
@@ -10,6 +10,7 @@ public class ScriptFile extends PostBuildItem {
     private final String filePath;
 
     private ScriptType scriptType;
+    private boolean sandboxed;
 
     @DataBoundConstructor
     public ScriptFile(Collection<String> results, String filePath) {
@@ -29,4 +30,13 @@ public ScriptType getScriptType() {
     public void setScriptType(ScriptType scriptType) {
         this.scriptType = scriptType;
     }
+
+    public boolean isSandboxed() {
+        return sandboxed;
+    }
+
+    @DataBoundSetter
+    public void setSandboxed(boolean sandboxed) {
+        this.sandboxed = sandboxed;
+    }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutor.java b/src/main/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutor.java
@@ -7,6 +7,7 @@
 import hudson.model.AbstractBuild;
 import jenkins.security.MasterToSlaveCallable;
 import org.jenkinsci.plugins.postbuildscript.Logger;
+import org.jenkinsci.plugins.postbuildscript.model.Script;
 import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.SecureGroovyScript;
 
 import java.io.File;
@@ -16,13 +17,15 @@
 public class GroovyScriptExecutor extends MasterToSlaveCallable<Boolean, Exception> {
 
     private static final long serialVersionUID = 3874477459736242748L;
-    private final String scriptContent;
+    private final String script;
+    private final boolean sandbox;
     private final List<String> arguments;
     private final transient AbstractBuild<?, ?> build;
     private final Logger log;
 
-    public GroovyScriptExecutor(String scriptContent, List<String> arguments, AbstractBuild<?, ?> build, Logger log) {
-        this.scriptContent = scriptContent;
+    public GroovyScriptExecutor(Script script, List<String> arguments, AbstractBuild<?, ?> build, Logger log) {
+        this.script = script.getContent();
+        this.sandbox = script.isSandboxed();
         this.arguments = new ArrayList<>(arguments);
         this.build = build;
         this.log = log;
@@ -31,8 +34,6 @@ public GroovyScriptExecutor(String scriptContent, List<String> arguments, Abstra
     @Override
     public Boolean call() throws Exception {
 
-        String script = Util.replaceMacro(scriptContent, EnvVars.masterEnvVars);
-
         Binding binding = new Binding();
         if (build != null) {
             FilePath workspace = build.getWorkspace();
@@ -42,15 +43,16 @@ public Boolean call() throws Exception {
             binding.setVariable("build", build); //NON-NLS
         }
 
-        binding.setVariable("log", log);
+        binding.setVariable("log", log); //NON-NLS
         binding.setVariable("out", log.getListener().getLogger()); //NON-NLS
-        binding.setVariable("args", arguments);
-
+        binding.setVariable("args", arguments); //NON-NLS
 
         ClassLoader classLoader = getClass().getClassLoader();
 
-        SecureGroovyScript groovyScript = new SecureGroovyScript(script, false, null);
+        String enrichedScript = Util.replaceMacro(script, EnvVars.masterEnvVars);
+        SecureGroovyScript groovyScript = new SecureGroovyScript(enrichedScript, sandbox, null);
         groovyScript.configuringWithNonKeyItem();
+
         groovyScript.evaluate(classLoader, binding);
 
         return true;

diff --git a/src/main/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutorFactory.java b/src/main/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutorFactory.java
@@ -2,6 +2,7 @@
 
 import hudson.model.AbstractBuild;
 import org.jenkinsci.plugins.postbuildscript.Logger;
+import org.jenkinsci.plugins.postbuildscript.model.Script;
 
 import java.util.List;
 
@@ -16,7 +17,7 @@ public GroovyScriptExecutorFactory(AbstractBuild<?, ?> build, Logger logger) {
         this.logger = logger;
     }
 
-    public GroovyScriptExecutor create(String scriptContent, List<String> arguments) {
-        return new GroovyScriptExecutor(scriptContent, arguments, build, logger);
+    public GroovyScriptExecutor create(Script script, List<String> arguments) {
+        return new GroovyScriptExecutor(script, arguments, build, logger);
     }
 }
diff --git a/src/main/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptPreparer.java b/src/main/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptPreparer.java
@@ -4,6 +4,8 @@
 import org.jenkinsci.plugins.postbuildscript.Logger;
 import org.jenkinsci.plugins.postbuildscript.Messages;
 import org.jenkinsci.plugins.postbuildscript.PostBuildScriptException;
+import org.jenkinsci.plugins.postbuildscript.model.Script;
+import org.jenkinsci.plugins.postbuildscript.model.ScriptFile;
 
 import java.io.PrintWriter;
 import java.io.Serializable;
@@ -34,13 +36,13 @@ public GroovyScriptPreparer(
         this.groovyScriptExecutorFactory = groovyScriptExecutorFactory;
     }
 
-    public boolean evaluateScript(String scriptContent) {
-        return evaluateScript(scriptContent, Collections.emptyList());
+    public boolean evaluateScript(Script script) {
+        return evaluateScript(script, Collections.emptyList());
     }
 
-    public boolean evaluateScript(String scriptContent, List<String> arguments) {
+    public boolean evaluateScript(Script script, List<String> arguments) {
 
-        if (scriptContent == null) {
+        if (script == null || script.getContent() == null) {
             throw new IllegalArgumentException("The script content object must be set.");
         }
 
@@ -49,7 +51,7 @@ public boolean evaluateScript(String scriptContent, List<String> arguments) {
         }
 
         try {
-            return workspace.act(groovyScriptExecutorFactory.create(scriptContent, arguments));
+            return workspace.act(groovyScriptExecutorFactory.create(script, arguments));
         } catch (Exception exception) {
             StringWriter stringWriter = new StringWriter();
             PrintWriter printWriter = new PrintWriter(stringWriter);
@@ -68,7 +70,7 @@ private boolean ensureWorkspaceNotNull(FilePath workspace) {
         return false;
     }
 
-    public boolean evaluateCommand(Command command) throws PostBuildScriptException {
+    public boolean evaluateCommand(ScriptFile scriptFile, Command command) throws PostBuildScriptException {
 
         if (ensureWorkspaceNotNull(workspace)) {
             return false;
@@ -77,7 +79,10 @@ public boolean evaluateCommand(Command command) throws PostBuildScriptException
         FilePath filePath = new ScriptFilePath(workspace).resolve(command.getScriptPath());
         LoadScriptContentCallable callable = new LoadScriptContentCallable();
         String scriptContent = new Content(callable).resolve(filePath);
-        return evaluateScript(scriptContent, command.getParameters());
+        Script script = new Script(scriptFile.getResults(), scriptContent);
+        script.setSandboxed(scriptFile.isSandboxed());
+        return evaluateScript(script, command.getParameters());
+
     }
 
 }
diff --git a/src/main/resources/lib/postbuildscript/form.jelly b/src/main/resources/lib/postbuildscript/form.jelly
@@ -42,6 +42,10 @@
 
                 <pbs:role item="${groovyScriptFiles}"/>
 
+                <f:entry title="" field="sandboxed" help="/plugin/postbuildscript/help/sandbox.html">
+                    <f:checkbox checked="${groovyScriptFiles.sandboxed}" title="${%Run in Groovy Sandbox}"/>
+                </f:entry>
+
                 <f:block>
                     <div align="right">
                         <f:repeatableDeleteButton/>
@@ -69,6 +73,10 @@
 
                 <pbs:role item="${groovyScripts}"/>
 
+                <f:entry title="" field="sandboxed" help="/plugin/postbuildscript/help/sandbox.html">
+                    <f:checkbox checked="${groovyScripts.sandboxed}" title="${%Run in Groovy Sandbox}" />
+                </f:entry>
+
                 <f:block>
                     <div align="right">
                         <f:repeatableDeleteButton/>

diff --git a/src/main/resources/lib/postbuildscript/form_de.properties b/src/main/resources/lib/postbuildscript/form_de.properties
@@ -7,3 +7,4 @@ Add\ Groovy\ script=Groovy-Skript hinzuf\u00fcgen
 Add\ post\ build\ step=Post-Build-Schritt hinzuf\u00fcgen
 Add\ build\ step=Build-Schritt hinzuf\u00fcgen
 Mark\ build\ unstable=Build bei Fehlschlag als instabil markieren
+Run\ in\ Groovy\ Sandbox=In Groovy Sandbox ausf\u00fchren
diff --git a/src/main/webapp/help/sandbox.html b/src/main/webapp/help/sandbox.html
@@ -0,0 +1,6 @@
+<div>
+    To reduce manual interventions by Administrators, most scripts should run in a Groovy Sandbox by default.
+    The sandbox only allows a subset of Groovy’s methods deemed sufficiently safe for "untrusted" access to be
+    executed without prior approval. When a script attempts to use features or methods unauthorized by the sandbox,
+    a script is halted immediately.
+</div>
diff --git a/...t/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutorFactoryTest.java b/...t/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutorFactoryTest.java
@@ -2,6 +2,7 @@
 
 import hudson.model.AbstractBuild;
 import org.jenkinsci.plugins.postbuildscript.Logger;
+import org.jenkinsci.plugins.postbuildscript.model.Script;
 import org.junit.Test;
 import org.junit.runner.RunWith;
 import org.mockito.InjectMocks;
@@ -23,13 +24,16 @@ public class GroovyScriptExecutorFactoryTest {
     @Mock
     private AbstractBuild<?, ?> build;
 
+    @Mock
+    private Script script;
+
     @InjectMocks
     private GroovyScriptExecutorFactory executorFactory;
 
     @Test
     public void createsExecutor() throws Exception {
 
-        GroovyScriptExecutor executor = executorFactory.create("scriptContent", Collections.emptyList());
+        GroovyScriptExecutor executor = executorFactory.create(script, Collections.emptyList());
 
         assertThat(executor, is(notNullValue()));
 

diff --git a/src/test/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutorIT.java b/src/test/java/org/jenkinsci/plugins/postbuildscript/service/GroovyScriptExecutorIT.java
@@ -5,6 +5,7 @@
 import hudson.model.FreeStyleProject;
 import hudson.model.TaskListener;
 import org.jenkinsci.plugins.postbuildscript.Logger;
+import org.jenkinsci.plugins.postbuildscript.model.Script;
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.runner.RunWith;
@@ -35,8 +36,12 @@ public class GroovyScriptExecutorIT {
     @Mock
     private PrintStream printStream;
 
+    @Mock
+    private Script script;
+
     @Test
     public void runsGroovyScriptWithVariablesAndBindings() throws Exception {
+        given(script.getContent()).willReturn("log.info('hello $envVar1'); out.println(build.id)");
 
         FreeStyleProject freeStyleProject = jenkinsRule.createFreeStyleProject();
         FreeStyleBuild executable = freeStyleProject.createExecutable();
@@ -46,7 +51,7 @@ public void runsGroovyScriptWithVariablesAndBindings() throws Exception {
         EnvVars.masterEnvVars.put("envVar2", "jenkins");
 
         GroovyScriptExecutor callable = new GroovyScriptExecutor(
-            "log.info('hello $envVar1'); out.println(build.id)", Collections.emptyList(), executable, log);
+            script, Collections.emptyList(), executable, log);
         Boolean result = callable.call();
 
         assertThat(result, is(true));