Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Prototyping integration with Script Security plugin #28

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
<parent>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>plugin</artifactId>
<version>1.447</version><!-- which version of Jenkins is this plugin built against? -->
<version>1.509.4</version><!-- which version of Jenkins is this plugin built against? -->
</parent>

<groupId>com.seitenbau.jenkins.plugins</groupId>
Expand Down Expand Up @@ -138,6 +138,11 @@
<artifactId>scriptler</artifactId>
<version>2.2</version>
</dependency>
<dependency>
<groupId>org.jenkins-ci.plugins</groupId>
<artifactId>script-security</artifactId>
<version>1.0-beta-5</version>
</dependency>
</dependencies>

</project>
Original file line number Diff line number Diff line change
Expand Up @@ -47,15 +47,16 @@ public class CascadingChoiceParameterDefinition extends ChoiceParameterDefinitio
* @param name parameter name
* @param parentPropertyName the name of the parent property
* @param script script, which generates the parameter value
* @param sandbox whether to execute the script in the security sandbox or not
* @param description parameter description
* @param uuid identifier (optional)
* @param remote execute the script on a remote node
*/
@DataBoundConstructor
public CascadingChoiceParameterDefinition(String name, String parentPropertyName, String script, String description, String uuid,
public CascadingChoiceParameterDefinition(String name, String parentPropertyName, String script, boolean sandbox, String description, String uuid,
Boolean remote, Boolean readonlyInputField, String classPath)
{
super(name, script, description, uuid, remote, readonlyInputField, classPath, null);
super(name, script, sandbox, description, uuid, remote, readonlyInputField, classPath, null);
_parentPropertyName = parentPropertyName;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
#Friday July 28 02:39:00 EDT 2013
constructor=name,parentPropertyName,script,description,uuid,remote,readonlyInputField,classPath
constructor=name,parentPropertyName,script,sandbox,description,uuid,remote,readonlyInputField,classPath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This file should never have been placed into source control to begin with. *.stapler files are generated as a build product.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, I had never seen *.stapler files in plug-ins before so I assumed I should try to modify it accordingly.

Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@

import org.apache.commons.lang.ObjectUtils;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext;
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
import org.jvnet.localizer.ResourceBundleHolder;
import org.kohsuke.stapler.DataBoundConstructor;

Expand All @@ -48,31 +51,46 @@ public class ChoiceParameterDefinition extends ScriptParameterDefinition
private final Boolean readonlyInputField;

private String choiceType;

@Deprecated
public ChoiceParameterDefinition(String name, String script, String description, String uuid,
Boolean remote, String classPath)
{
this(name, script, description, uuid, remote, false, classPath, PARAMETER_TYPE_SINGLE_SELECT);
}

@Deprecated
public ChoiceParameterDefinition(String name, String script, String description, String uuid,
Boolean remote, Boolean readonlyInputField, String classPath, String choiceType)
{
this(name, script, /* sandbox */ false, description, uuid, remote, readonlyInputField, classPath, choiceType);
}

/**
* Constructor.
* @param name parameter name
* @param script script, which generates the parameter value
* @param sandbox whether to execute the script in the security sandbox or not
* @param description parameter description
* @param uuid identifier (optional)
* @param remote execute the script on a remote node
* @param choiceType type of the choice (single, multi, etc.) to display
*/
@DataBoundConstructor
public ChoiceParameterDefinition(String name, String script, String description, String uuid,
public ChoiceParameterDefinition(String name, String script, boolean sandbox, String description, String uuid,
Boolean remote, Boolean readonlyInputField, String classPath, String choiceType)
{
super(name, script, description, uuid, remote, classPath);
super(name, script, sandbox, description, uuid, remote, classPath);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If !sandbox then you will need to call configuring, or an approval request will not get enqueued.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the super is calling

configuring
, but without checking for
!sandbox

this.readonlyInputField = readonlyInputField;
this.choiceType = choiceType;
}

private Object readResolve() {
if (!getSandbox()) {
ScriptApproval.get().configuring(getScript(), GroovyLanguage.get(), ApprovalContext.create());
}
return this;
}

/**
* Return default parameter value - used by trigger mechanism.
Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
#Sun Sep 19 00:45:21 PDT 2010
constructor=name,script,description,uuid,remote,readonlyInputField,classPath
constructor=name,script,sandbox,description,uuid,remote,readonlyInputField,classPath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto here

Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,17 @@
import hudson.model.AutoCompletionCandidates;
import hudson.remoting.Callable;
import hudson.remoting.VirtualChannel;
import hudson.util.FormValidation;

import java.io.File;
import java.io.IOException;
import java.util.Collections;
import java.util.Map;

import org.apache.commons.lang.ArrayUtils;
import org.apache.commons.lang.StringUtils;
import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext;
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
import org.kohsuke.stapler.QueryParameter;

import com.seitenbau.jenkins.plugins.dynamicparameter.config.DynamicParameterConfiguration;
Expand All @@ -50,6 +53,9 @@ public abstract class ScriptParameterDefinition extends BaseParameterDefinition
/** Script, which generates the parameter value. */
private final String _script;

/** Security sandbox. */
private final boolean _sandbox;

/** Local class path. */
private final FilePath _localBaseDirectory;

Expand All @@ -58,24 +64,26 @@ public abstract class ScriptParameterDefinition extends BaseParameterDefinition

/** Class path. */
private final String _classPath;

/**
* Constructor.
* @param name parameter name
* @param script script, which generates the parameter value
* @param whether to use the security sandbox or not
* @param description parameter description
* @param uuid identifier (optional)
* @param remote execute the script on a remote node
*/
protected ScriptParameterDefinition(String name, String script, String description, String uuid,
protected ScriptParameterDefinition(String name, String script, boolean sandbox, String description, String uuid,
Boolean remote, String classPath)
{
super(name, description, uuid, remote);

_localBaseDirectory = new FilePath(DynamicParameterConfiguration.INSTANCE.getBaseDirectoryFile());
_remoteBaseDirectory = DEFAULT_REMOTE_CLASSPATH;
_classPath = classPath;
_script = script;
_script = ScriptApproval.get().configuring(script, GroovyLanguage.get(), ApprovalContext.create());
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should pass in currentUser and possibly more context such as an Item. Also should not call configuring when sandbox.

_sandbox = sandbox;
}

/**
Expand Down Expand Up @@ -122,19 +130,24 @@ public final String[] getClassPathList()
{
return _classPath.split(CLASSPATH_SPLITTER);
}

public final boolean getSandbox()
{
return _sandbox;
}

@Override
protected ClasspathScriptCall prepareLocalCall(Map<String, String> parameters)
{
ClasspathScriptCall call = new ClasspathScriptCall(getScript(), parameters, setupLocalClassPaths());
ClasspathScriptCall call = new ClasspathScriptCall(getScript(), parameters, setupLocalClassPaths(), getSandbox());
return call;
}

@Override
protected ClasspathScriptCall prepareRemoteCall(VirtualChannel channel, Map<String, String> parameters) throws IOException,
InterruptedException
{
ClasspathScriptCall call = new ClasspathScriptCall(getScript(), parameters, setupRemoteClassPaths(channel));
ClasspathScriptCall call = new ClasspathScriptCall(getScript(), parameters, setupRemoteClassPaths(channel), getSandbox());
return call;
}

Expand Down Expand Up @@ -244,6 +257,10 @@ public static String[] splitClassPaths(String value)
return value.toLowerCase().split(CLASSPATH_SPLITTER);
}
}

public FormValidation doCheckGroovyScript(@QueryParameter String value, @QueryParameter boolean sandbox) {
return sandbox ? FormValidation.ok() : ScriptApproval.get().checking(value, GroovyLanguage.get());
}

}

Expand All @@ -259,23 +276,26 @@ public static final class ClasspathScriptCall implements Callable<Object, Throwa
private final Map<String, String> _parameters;

private final FilePath[] _classPaths;

private final boolean _sandbox;

/**
* Constructor.
* @param script script to execute
* @param classPaths class paths
*/
public ClasspathScriptCall(String script, Map<String, String> parameters, FilePath[] classPaths)
public ClasspathScriptCall(String script, Map<String, String> parameters, FilePath[] classPaths, boolean sandbox)
{
_remoteScript = script;
_parameters = parameters;
_classPaths = classPaths;
_sandbox = sandbox;
}

@Override
public Object call()
{
return JenkinsUtils.execute(_remoteScript, _parameters, _classPaths);
return JenkinsUtils.execute(_remoteScript, _parameters, _classPaths, _sandbox);
}

}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,30 @@ public StringParameterDefinition(String name, String script, String description,
this(name, script, description, uuid, remote, false, classPath);
}

@Deprecated
public StringParameterDefinition(String name, String script, String description, String uuid,
Boolean remote, Boolean readonlyInputField, String classPath)
{
this(name, script, /* sandbox*/ false, description, uuid, remote, false, classPath);
}

/**
* Constructor with the parameter which are injected by the jenkins runtime.
*
* @param name parameter name
* @param script script, which generates the parameter value
* @param sandbox whether to execute the script in the security sandbox or not
* @param description parameter description
* @param uuid identifier (optional)
* @param remote execute the script on a remote node
* @param readonlyInputField should the input field marked as read only true / false
* @param classPath the class path description
*/
@DataBoundConstructor
public StringParameterDefinition(String name, String script, String description, String uuid,
public StringParameterDefinition(String name, String script, boolean sandbox, String description, String uuid,
Boolean remote, Boolean readonlyInputField, String classPath)
{
super(name, script, description, uuid, remote, classPath);
super(name, script, sandbox, description, uuid, remote, classPath);
this.readonlyInputField = readonlyInputField;
}

Expand Down
Original file line number Diff line number Diff line change
@@ -1,2 +1,2 @@
#Sun Sep 19 00:45:21 PDT 2010
constructor=name,script,description,remote,readonlyInputField,classPath
constructor=name,script,sandbox,description,remote,readonlyInputField,classPath
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
*/
package com.seitenbau.jenkins.plugins.dynamicparameter.util;

import groovy.lang.Binding;
import groovy.lang.GroovyShell;
import hudson.FilePath;
import hudson.Plugin;
Expand All @@ -41,6 +42,12 @@
import org.codehaus.groovy.control.CompilerConfiguration;
import org.jenkinsci.plugins.scriptler.config.Script;
import org.jenkinsci.plugins.scriptler.config.ScriptlerConfiguration;
import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException;
import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist;
import org.jenkinsci.plugins.scriptsecurity.sandbox.groovy.GroovySandbox;
import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext;
import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;

import com.seitenbau.jenkins.plugins.dynamicparameter.BaseParameterDefinition;

Expand Down Expand Up @@ -77,7 +84,7 @@ public static Object execute(String script)
public static Object execute(String script, Map<String, String> parameters)
{
FilePath[] emptyClassPaths = new FilePath[]{};
return execute(script, parameters, emptyClassPaths);
return execute(script, parameters, emptyClassPaths, /* sandbox */ false);
}

/**
Expand All @@ -87,11 +94,18 @@ public static Object execute(String script, Map<String, String> parameters)
* @param classPaths class paths
* @return result from the script
*/
public static Object execute(String script, Map<String, String> parameters, FilePath[] classPaths)
public static Object execute(final String script, Map<String, String> parameters, FilePath[] classPaths, boolean sandbox)
{
try
{
CompilerConfiguration config = new CompilerConfiguration();
CompilerConfiguration config;

if (sandbox) {
config = GroovySandbox.createSecureCompilerConfiguration();
}
else {
config = new CompilerConfiguration();
}

// set class path
ArrayList<String> classPathList = new ArrayList<String>(classPaths.length);
Expand All @@ -113,25 +127,57 @@ public static Object execute(String script, Map<String, String> parameters, File
logger.log(Level.INFO, "Cannot access path", exp);
}
config.setClasspathList(classPathList);
GroovyShell groovyShell = new GroovyShell(config);

Binding binding = new Binding();
for (Entry<String, String> parameter : parameters.entrySet())
{
groovyShell.setVariable(parameter.getKey(), parameter.getValue());
binding.setVariable(parameter.getKey(), parameter.getValue());
}

// execute script
Object evaluate = groovyShell.evaluate(script);
final Object evaluate;
if (sandbox) {
final GroovyShell groovyShell = new GroovyShell(binding, config);
MyRunnable r = new MyRunnable(groovyShell, script);
GroovySandbox.runInSandbox(r, Whitelist.all());
evaluate = r.getEvaluated();
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Adding a runInSandbox overload taking Callable for the next version. Or, today:

final AtomicReference<Object> ref = new AtomicReference<Object>();
GroovySandbox.runInSandbox(new Runnable() {
    @Override public void run() {
        ref.set(groovyShell.evaluate(script);
    }
}, Whitelist.all());
evaluate = ref.get();

} else {
evaluate = new GroovyShell(binding, config).evaluate(ScriptApproval.get().using(script, GroovyLanguage.get()));
}

return evaluate;
}
catch (Exception e)
{
logger.log(Level.SEVERE, "Cannot access class path", e);
if (e instanceof RejectedAccessException) {
ScriptApproval.get().accessRejected((RejectedAccessException) e, ApprovalContext.create());
}
return null;
}
}

private static final class MyRunnable implements Runnable {

private Object evaluated;
private GroovyShell groovyShell;
private String script;

public MyRunnable(GroovyShell groovyShell, String script) {
this.groovyShell = groovyShell;
this.script = script;
}

@Override
public void run() {
evaluated = groovyShell.evaluate(script);
}

public Object getEvaluated() {
return evaluated;
}
}

/**
* Check if a plugin is available.
* @param shortName plugin short name
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ limitations under the License.
<f:entry title="${%Choices Script}" field="choicesScript">
<f:textarea name="parameter.script" value="${instance.script}" />
</f:entry>
<f:entry field="sandbox" title="${%Use Groovy Sandbox}">
<f:checkbox/>
</f:entry>
<f:entry title="${%Class Paths}" field="classPath">
<f:textbox name="parameter.classPath" value="${instance.classPath}" />
</f:entry>
Expand Down
Loading