Merge pull request #1078 from daspilker/script-loading

allow import of Groovy code from the workspace

docs/Home.md

@@ -29,6 +29,8 @@ Browse the Jenkins issue tracker to see any [open issues](https://issues.jenkins
 
 ## Release Notes
 * 1.67 (unreleased)
+  * Allow import of Groovy code from the workspace when script security sandbox is enabled
+    ([#1078](https://github.com/jenkinsci/job-dsl-plugin/pull/1078))
   * Enhanced support for the [Groovy Plugin](https://wiki.jenkins-ci.org/display/JENKINS/Groovy+plugin)
     ([JENKINS-44256](https://issues.jenkins-ci.org/browse/JENKINS-44256))
   * Enhanced support for the

docs/Real-World-Examples.md

@@ -66,12 +66,6 @@ REST API calls
 Import other files (i.e. with class definitions) into your script
 -----------------------------------------------------------------
 
-> Importing Groovy classes from the workspace is not possible when script security is enabled since that would undermine
-> the script approval process. As an alternative it is possible to package the classes into a JAR file and add that JAR
-> to the classpath through the _Additional classpath_ option. Classpath entries are subject to the approval process. See
-> [Job DSL Gradle Example](https://github.com/sheehan/job-dsl-gradle-example) or
-> [Job DSL Sample](https://github.com/unguiculus/job-dsl-sample) as starting point for building and packaging classes.
-
 Make a directory at the same level as the DSL called `utilities` and create a file called `MyUtilities.groovy` in the
 `utilities` directory with the following contents:
 
@@ -92,4 +86,4 @@ Then from the DSL, add something like this:
     def myJob = job('example')
     MyUtilities.addMyFeature(myJob)
 
-Note that importing other files is not possible when [[Script Security]] is enabled.
+Note that importing other files is not possible when [[Script Security]] is enabled and not using Groovy Sandbox.

job-dsl-core/src/main/groovy/javaposse/jobdsl/dsl/AbstractDslScriptLoader.groovy

@@ -48,9 +48,8 @@ abstract class AbstractDslScriptLoader<S extends JobParent, G extends GeneratedI
 
                 GroovyShell groovyShell = groovyShellCache[key]
                 if (!groovyShell) {
-                    ClassLoader classLoader = prepareClassLoader(AbstractDslScriptLoader.classLoader)
                     groovyShell = new GroovyShell(
-                            new URLClassLoader(scriptRequest.urlRoots, classLoader),
+                            prepareClassLoader(scriptRequest.urlRoots, AbstractDslScriptLoader.classLoader),
                             new Binding(),
                             config
                     )
@@ -65,8 +64,13 @@ abstract class AbstractDslScriptLoader<S extends JobParent, G extends GeneratedI
             }
         } finally {
             groovyShellCache.values().each { GroovyShell groovyShell ->
-                groovyShell.classLoader.close()
-                groovyShell.classLoader.parent.close()
+                ClassLoader classLoader = groovyShell.classLoader
+                while (classLoader != AbstractDslScriptLoader.classLoader) {
+                    if (classLoader instanceof Closeable) {
+                        ((Closeable) classLoader).close()
+                    }
+                    classLoader = classLoader.parent
+                }
             }
         }
         generatedItems
@@ -112,8 +116,11 @@ abstract class AbstractDslScriptLoader<S extends JobParent, G extends GeneratedI
         }
     }
 
-    protected ClassLoader prepareClassLoader(ClassLoader classLoader) {
-        classLoader
+    /**
+     * @since 1.67
+     */
+    protected ClassLoader prepareClassLoader(URL[] urlRoots, ClassLoader classLoader) {
+        new URLClassLoader(urlRoots, classLoader)
     }
 
     protected GroovyCodeSource createGroovyCodeSource(ScriptRequest scriptRequest) {

job-dsl-plugin/src/main/groovy/javaposse/jobdsl/plugin/SandboxDslScriptLoader.groovy

@@ -4,6 +4,7 @@ import hudson.model.Item
 import hudson.security.ACL
 import javaposse.jobdsl.dsl.DslException
 import javaposse.jobdsl.dsl.JobManagement
+import javaposse.jobdsl.dsl.ScriptRequest
 import jenkins.model.Jenkins
 import org.acegisecurity.AccessDeniedException
 import org.codehaus.groovy.control.CompilerConfiguration
@@ -28,8 +29,15 @@ class SandboxDslScriptLoader extends SecureDslScriptLoader {
     }
 
     @Override
-    protected ClassLoader prepareClassLoader(ClassLoader classLoader) {
-        GroovySandbox.createSecureClassLoader(classLoader)
+    protected ClassLoader prepareClassLoader(URL[] urlRoots, ClassLoader classLoader) {
+        GroovySandbox.createSecureClassLoader(new WorkspaceClassLoader(urlRoots[0], classLoader, seedJob))
+    }
+
+    protected Collection<ScriptRequest> createSecureScriptRequests(Collection<ScriptRequest> scriptRequests) {
+        scriptRequests.collect {
+            // it is not safe to use additional classpath entries
+            new ScriptRequest(it.body, it.urlRoots[0..0] as URL[], it.ignoreExisting, it.scriptPath)
+        }
     }
 
     @Override
@@ -46,4 +54,36 @@ class SandboxDslScriptLoader extends SecureDslScriptLoader {
             throw new DslException(e.message, e)
         }
     }
+
+    private static class WorkspaceClassLoader extends URLClassLoader {
+        private final Item seedJob
+
+        WorkspaceClassLoader(URL workspaceUrl, ClassLoader parent, Item seedJob) {
+            super([workspaceUrl] as URL[], parent)
+            this.seedJob = seedJob
+        }
+
+        @Override
+        protected Class<?> findClass(String name) throws ClassNotFoundException {
+            throw new ClassNotFoundException(name)
+        }
+
+        @Override
+        URL findResource(String name) {
+            if (!seedJob.hasPermission(Item.WORKSPACE)) {
+                return null
+            }
+
+            super.findResource(name)
+        }
+
+        @Override
+        Enumeration<URL> findResources(String name) throws IOException {
+            if (!seedJob.hasPermission(Item.WORKSPACE)) {
+                return Collections.emptyEnumeration()
+            }
+
+            super.findResources(name)
+        }
+    }
 }

job-dsl-plugin/src/main/groovy/javaposse/jobdsl/plugin/ScriptApprovalDslScriptLoader.groovy

@@ -33,14 +33,17 @@ class ScriptApprovalDslScriptLoader extends SecureDslScriptLoader {
     }
 
     protected Collection<ScriptRequest> createSecureScriptRequests(Collection<ScriptRequest> scriptRequests) {
-        super.createSecureScriptRequests(scriptRequests).each {
+        scriptRequests.collect {
             if (it.body) {
                 ScriptApproval.get().configuring(
                         it.body,
                         GroovyLanguage.get(),
                         ApprovalContext.create().withItem(seedJob)
                 )
             }
+
+            // it is not safe to use additional classpath entries
+            new ScriptRequest(it.body, new URL[0], it.ignoreExisting, it.scriptPath)
         }
     }
 }

job-dsl-plugin/src/main/groovy/javaposse/jobdsl/plugin/SecureDslScriptLoader.groovy

@@ -14,10 +14,10 @@ abstract class SecureDslScriptLoader extends JenkinsDslScriptLoader {
         super.runScripts(createSecureScriptRequests(scriptRequests))
     }
 
-    protected Collection<ScriptRequest> createSecureScriptRequests(Collection<ScriptRequest> scriptRequests) {
-        scriptRequests.collect {
-            // it is not safe to use additional classpath entries
-            new ScriptRequest(it.body, new URL[0], it.ignoreExisting, it.scriptPath)
-        }
+    @Override
+    protected ClassLoader prepareClassLoader(URL[] urlRoots, ClassLoader classLoader) {
+        new URLClassLoader([] as URL[], classLoader)
     }
+
+    protected abstract Collection<ScriptRequest> createSecureScriptRequests(Collection<ScriptRequest> scriptRequests)
 }

job-dsl-plugin/src/test/groovy/ScriptHelper.groovy

@@ -0,0 +1,5 @@
+class ScriptHelper {
+    static foo() {
+        'foo'
+    }
+}

job-dsl-plugin/src/test/groovy/javaposse/jobdsl/plugin/ExecuteDslScriptsSpec.groovy

@@ -1442,6 +1442,123 @@ class ExecuteDslScriptsSpec extends Specification {
         ScriptApproval.get().pendingSignatures*.signature == ['staticMethod java.lang.System exit int']
     }
 
+    def 'run script in sandbox with import from workspace'() {
+        setup:
+        String script = 'import Helper\njob(Helper.computeName()) { description("foo") }'
+
+        jenkinsRule.instance.securityRealm = jenkinsRule.createDummySecurityRealm()
+        jenkinsRule.instance.authorizationStrategy = new MockAuthorizationStrategy()
+                .grant(Jenkins.READ, Item.READ, Item.CONFIGURE, Item.CREATE, Computer.BUILD, Item.WORKSPACE)
+                .everywhere().to('dev')
+
+        FreeStyleProject job = jenkinsRule.createFreeStyleProject('seed')
+        FreeStyleBuild build = job.scheduleBuild2(0).get()
+        build.workspace.child('Helper.groovy').write('class Helper { static computeName() { "foo" } }', 'UTF-8')
+        job.buildersList.add(new ExecuteDslScripts(scriptText: script, sandbox: true))
+        setupQIA('dev', job)
+
+        when:
+        jenkinsRule.submit(jenkinsRule.createWebClient().login('dev').getPage(job, 'configure').getFormByName('config'))
+
+        then:
+        assert ScriptApproval.get().pendingScripts*.script == []
+
+        when:
+        build = job.scheduleBuild2(0).get()
+
+        then:
+        build.result == SUCCESS
+        assert ScriptApproval.get().pendingScripts*.script == []
+    }
+
+    def 'cannot run script in sandbox with import from workspace without WORKSPACE permission'() {
+        setup:
+        String script = 'import Helper\njob(Helper.computeName()) { description("foo") }'
+
+        jenkinsRule.instance.securityRealm = jenkinsRule.createDummySecurityRealm()
+        jenkinsRule.instance.authorizationStrategy = new MockAuthorizationStrategy()
+                .grant(Jenkins.READ, Item.READ, Item.CONFIGURE, Item.CREATE, Computer.BUILD).everywhere().to('dev')
+
+        FreeStyleProject job = jenkinsRule.createFreeStyleProject('seed')
+        FreeStyleBuild build = job.scheduleBuild2(0).get()
+        build.workspace.child('Helper.groovy').write('class Helper { static computeName() { "foo" } }', 'UTF-8')
+        job.buildersList.add(new ExecuteDslScripts(scriptText: script, sandbox: true))
+        setupQIA('dev', job)
+
+        when:
+        jenkinsRule.submit(jenkinsRule.createWebClient().login('dev').getPage(job, 'configure').getFormByName('config'))
+
+        then:
+        assert ScriptApproval.get().pendingScripts*.script == []
+
+        when:
+        build = job.scheduleBuild2(0).get()
+
+        then:
+        build.result == FAILURE
+        build.log.contains('unable to resolve class Helper')
+        ScriptApproval.get().pendingSignatures.isEmpty()
+    }
+
+    def 'run script in sandbox with import from workspace with unapproved signature'() {
+        setup:
+        String script = 'import Helper\njob(Helper.boom()) { description("foo") }'
+
+        jenkinsRule.instance.securityRealm = jenkinsRule.createDummySecurityRealm()
+        jenkinsRule.instance.authorizationStrategy = new MockAuthorizationStrategy()
+                .grant(Jenkins.READ, Item.READ, Item.CONFIGURE, Item.CREATE, Computer.BUILD, Item.WORKSPACE)
+                .everywhere().to('dev')
+
+        FreeStyleProject job = jenkinsRule.createFreeStyleProject('seed')
+        FreeStyleBuild build = job.scheduleBuild2(0).get()
+        build.workspace.child('Helper.groovy').write('class Helper { static boom() { System.exit(0) } }', 'UTF-8')
+        job.buildersList.add(new ExecuteDslScripts(scriptText: script, sandbox: true))
+        setupQIA('dev', job)
+
+        when:
+        jenkinsRule.submit(jenkinsRule.createWebClient().login('dev').getPage(job, 'configure').getFormByName('config'))
+
+        then:
+        assert ScriptApproval.get().pendingScripts*.script == []
+
+        when:
+        build = job.scheduleBuild2(0).get()
+
+        then:
+        build.result == FAILURE
+        ScriptApproval.get().pendingSignatures*.signature == ['staticMethod java.lang.System exit int']
+    }
+
+    def 'cannot import compiled class from workspace'() {
+        setup:
+        String script = 'import ScriptHelper\njob(ScriptHelper.foo()) { description("foo") }'
+
+        jenkinsRule.instance.securityRealm = jenkinsRule.createDummySecurityRealm()
+        jenkinsRule.instance.authorizationStrategy = new MockAuthorizationStrategy()
+                .grant(Jenkins.READ, Item.READ, Item.CONFIGURE, Item.CREATE, Computer.BUILD, Item.WORKSPACE)
+                .everywhere().to('dev')
+
+        FreeStyleProject job = jenkinsRule.createFreeStyleProject('seed')
+        FreeStyleBuild build = job.scheduleBuild2(0).get()
+        build.workspace.child('ScriptHelper.class').copyFrom(getClass().getResourceAsStream('/ScriptHelper.class'))
+        job.buildersList.add(new ExecuteDslScripts(scriptText: script, sandbox: true))
+        setupQIA('dev', job)
+
+        when:
+        jenkinsRule.submit(jenkinsRule.createWebClient().login('dev').getPage(job, 'configure').getFormByName('config'))
+
+        then:
+        assert ScriptApproval.get().pendingScripts*.script == []
+
+        when:
+        build = job.scheduleBuild2(0).get()
+
+        then:
+        build.result == FAILURE
+        build.log.contains('Scripts not permitted to use staticMethod ScriptHelper foo')
+        ScriptApproval.get().pendingSignatures*.signature == ['staticMethod ScriptHelper foo']
+    }
+
     def 'cannot run script in sandbox without queue item authentication'() {
         setup:
         String script = 'job("test") { description("foo") }'