Merge pull request #948 from jgarciacloudbees/JENKINS-73941

JENKINS-73941 - Option to hide "Use Groovy Sandbox" for users without Administer permission globally in the system

Author: jglick

plugin/pom.xml

@@ -54,6 +54,11 @@
                 <scope>import</scope>
                 <type>pom</type>
             </dependency>
+            <dependency>
+                <groupId>org.jenkins-ci.plugins</groupId>
+                <artifactId>script-security</artifactId>
+                <version>1367.vdf2fc45f229c</version>
+            </dependency>
         </dependencies>
     </dependencyManagement>
     <dependencies>
@@ -246,6 +251,16 @@
             <version>4.2.2</version>
             <scope>test</scope>
         </dependency>
+        <dependency>
+            <groupId>io.jenkins</groupId>
+            <artifactId>configuration-as-code</artifactId>
+            <scope>test</scope>
+        </dependency>
+        <dependency>
+            <groupId>io.jenkins.configuration-as-code</groupId>
+            <artifactId>test-harness</artifactId>
+            <scope>test</scope>
+        </dependency>
     </dependencies>
     <build>
         <resources>

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/CpsFlowDefinition.java

@@ -41,7 +41,6 @@
 import jenkins.model.Jenkins;
 import net.sf.json.JSONObject;
 import org.apache.commons.lang.StringUtils;
-import org.jenkinsci.plugins.workflow.cps.config.CPSConfiguration;
 import org.jenkinsci.plugins.workflow.cps.persistence.PersistIn;
 import org.jenkinsci.plugins.workflow.flow.DurabilityHintProvider;
 import org.jenkinsci.plugins.workflow.flow.FlowDefinition;
@@ -89,7 +88,7 @@ public CpsFlowDefinition(String script) throws Descriptor.FormException {
 
     @DataBoundConstructor
     public CpsFlowDefinition(String script, boolean sandbox) throws Descriptor.FormException {
-        if (CPSConfiguration.get().isHideSandbox() && !sandbox && !Jenkins.get().hasPermission(Jenkins.ADMINISTER)) {
+        if (!sandbox && ScriptApproval.get().isForceSandboxForCurrentUser()) {
             // this will end up in the /oops page until https://github.com/jenkinsci/jenkins/pull/9495 is picked up
             throw new Descriptor.FormException("Sandbox cannot be disabled. This Jenkins instance has been configured to not " +
                     "allow regular users to disable the sandbox in pipelines", "sandbox");
@@ -98,7 +97,6 @@ public CpsFlowDefinition(String script, boolean sandbox) throws Descriptor.FormE
         this.script = sandbox ? script : ScriptApproval.get().configuring(script, GroovyLanguage.get(),
                 ApprovalContext.create().withCurrentUser().withItemAsKey(req != null ? req.findAncestorObject(Item.class) : null), req == null);
         this.sandbox = sandbox;
-
     }
 
     private Object readResolve() {
@@ -196,7 +194,7 @@ public JSON doCheckScriptCompile(@AncestorInPath Item job, @QueryParameter Strin
         public boolean shouldHideSandbox(@CheckForNull CpsFlowDefinition instance) {
             // sandbox checkbox is shown to admins even if the global configuration says otherwise
             // it's also shown when sandbox == false, so regular users can enable it
-            return CPSConfiguration.get().isHideSandbox() && !Jenkins.get().hasPermission(Jenkins.ADMINISTER)
+            return ScriptApproval.get().isForceSandboxForCurrentUser()
                     && (instance == null || instance.sandbox);
         }
 

plugin/src/main/java/org/jenkinsci/plugins/workflow/cps/config/CPSConfiguration.java

@@ -24,33 +24,55 @@
 
 package org.jenkinsci.plugins.workflow.cps.config;
 
+import java.io.IOException;
+import java.io.UncheckedIOException;
+
 import edu.umd.cs.findbugs.annotations.NonNull;
 import hudson.Extension;
 import hudson.ExtensionList;
 import jenkins.model.GlobalConfiguration;
 import jenkins.model.GlobalConfigurationCategory;
+import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
 import org.jenkinsci.Symbol;
 
+/**
+ * @deprecated
+ * This class has been deprecated and its only configuration value is ignored. Do not rely on it or use it in any way.
+ * In order to force using the system sandbox for pipelines, please use {@link ScriptApproval#isForceSandbox} or {@link ScriptApproval#isForceSandboxForCurrentUser}
+ **/
 @Symbol("cps")
 @Extension
+@Deprecated
 public class CPSConfiguration extends GlobalConfiguration {
 
     /**
      * Whether to show the sandbox checkbox in jobs to users without Jenkins.ADMINISTER
      */
-    private boolean hideSandbox;
+    private transient boolean hideSandbox;
 
     public CPSConfiguration() {
+
         load();
+        if (hideSandbox) {
+            ScriptApproval.get().setForceSandbox(hideSandbox);
+        }
+
+        // Data migration from this configuration to ScriptApproval should be done only once,
+        // so removing the config file after the previous migration
+        try {
+            this.getConfigFile().delete();
+        } catch (IOException e) {
+            throw new UncheckedIOException(e);
+        }
     }
 
     public boolean isHideSandbox() {
-        return hideSandbox;
+        return ScriptApproval.get().isForceSandbox();
     }
 
     public void setHideSandbox(boolean hideSandbox) {
         this.hideSandbox = hideSandbox;
-        save();
+        ScriptApproval.get().setForceSandbox(hideSandbox);
     }
 
     @NonNull

plugin/src/main/resources/org/jenkinsci/plugins/workflow/cps/config/CPSConfiguration/config.jelly

@@ -25,9 +25,4 @@ THE SOFTWARE.
 
 <?jelly escape-by-default='true'?>
 <j:jelly xmlns:j="jelly:core" xmlns:f="/lib/form">
-    <f:section title="${%Pipeline Sandbox}">
-        <f:entry field="hideSandbox" title="${%Hide Sandbox checkbox in pipeline jobs}">
-            <f:checkbox/>
-        </f:entry>
-    </f:section>
 </j:jelly>

plugin/src/main/resources/org/jenkinsci/plugins/workflow/cps/config/CPSConfiguration/help-hideSandbox.html

@@ -24,6 +24,7 @@
 
 package org.jenkinsci.plugins.workflow.cps;
 
+import hudson.model.Result;
 import hudson.util.VersionNumber;
 import org.htmlunit.FailingHttpStatusCodeException;
 import org.htmlunit.HttpMethod;
@@ -45,12 +46,15 @@
 import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
 import org.jenkinsci.plugins.workflow.cps.config.CPSConfiguration;
 import org.jenkinsci.plugins.workflow.job.WorkflowJob;
+import org.jenkinsci.plugins.workflow.job.WorkflowRun;
 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.MockAuthorizationStrategy;
+import org.jvnet.hudson.test.recipes.LocalData;
 
+import java.io.File;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 
@@ -59,8 +63,6 @@
 import static org.hamcrest.Matchers.equalTo;
 import static org.hamcrest.Matchers.hasSize;
 import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.nullValue;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertTrue;
@@ -316,7 +318,7 @@ public void cpsScriptSandboxHide() throws Exception {
         }
 
         // non-admins cannot see the sandbox checkbox in jobs if hideSandbox is On globally
-        CPSConfiguration.get().setHideSandbox(true);
+        ScriptApproval.get().setForceSandbox(true);
         {
             HtmlForm config = wcDevel.getPage(p, "configure").getFormByName("config");
             assertThat(config.getVisibleText(), not(containsStringIgnoringCase("Use Groovy Sandbox")));
@@ -328,15 +330,15 @@ public void cpsScriptSandboxHide() throws Exception {
         }
 
         // admins can always see the sandbox checkbox
-        CPSConfiguration.get().setHideSandbox(false);
+        ScriptApproval.get().setForceSandbox(false);
         wcDevel.login("admin");
         {
             HtmlForm config = wcDevel.getPage(p, "configure").getFormByName("config");
             assertThat(config.getVisibleText(), containsStringIgnoringCase("Use Groovy Sandbox"));
         }
 
         // even when set to hide globally
-        CPSConfiguration.get().setHideSandbox(true);
+        ScriptApproval.get().setForceSandbox(true);
         {
             HtmlForm config = wcDevel.getPage(p, "configure").getFormByName("config");
             assertThat(config.getVisibleText(), containsStringIgnoringCase("Use Groovy Sandbox"));
@@ -370,4 +372,67 @@ public void cpsScriptSandboxHide() throws Exception {
 
         }
     }
+
+    @Test
+    public void cpsConfigurationSandboxToScriptApprovalSandbox() throws Exception{
+        //Deprecated CPSConfiguration should update ScriptApproval forceSandbox logic to keep casc compatibility
+        ScriptApproval.get().setForceSandbox(false);
+
+        CPSConfiguration.get().setHideSandbox(true);
+        assertTrue(ScriptApproval.get().isForceSandbox());
+
+        ScriptApproval.get().setForceSandbox(false);
+        assertFalse(CPSConfiguration.get().isHideSandbox());
+    }
+
+    @Test
+    public void cpsScriptSignatureException() throws Exception {
+        ScriptApproval.get().setForceSandbox(false);
+        WorkflowJob p = jenkins.createProject(WorkflowJob.class);
+        String script = "jenkins.model.Jenkins.instance";
+        p.setDefinition(new CpsFlowDefinition(script, true));
+        WorkflowRun b = jenkins.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+        jenkins.assertLogContains("Scripts not permitted to use staticMethod jenkins.model.Jenkins getInstance. "
+                                  + "Administrators can decide whether to approve or reject this signature.", b);
+
+        ScriptApproval.get().setForceSandbox(true);
+        b = jenkins.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
+        jenkins.assertLogContains("Scripts not permitted to use staticMethod jenkins.model.Jenkins getInstance. "
+                                  + "Script signature is not in the default whitelist.", b);
+    }
+
+    @Test
+    @LocalData
+    public void cpsLoadConfiguration() throws Exception {
+        //CPSConfiguration file containing <hideSandbox>true</hideSandbox>
+        // should be promoted to ScriptApproval.get().isForceSandbox()
+        assertTrue(ScriptApproval.get().isForceSandbox());
+
+        //Once the info is promoted, we are removing the config file, so should no longer exist.
+        //We are checking the injected localData is removed
+        assertFalse(new File(jenkins.jenkins.getRootDir(),
+                             "org.jenkinsci.plugins.workflow.cps.config.CPSConfiguration.xml").exists());
+    }
+
+    @Test
+    public void cpsRoundTrip() throws Exception {
+        jenkins.jenkins.setSecurityRealm(jenkins.createDummySecurityRealm());
+
+        MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy();
+        mockStrategy.grant(Jenkins.READ).everywhere().to("dev");
+        for (Permission p : Item.PERMISSIONS.getPermissions()) {
+            mockStrategy.grant(p).everywhere().to("dev");
+        }
+
+        WorkflowJob p = jenkins.createProject(WorkflowJob.class);
+        p.setDefinition(new CpsFlowDefinition("echo 'Hello'", true));
+
+        WorkflowJob roundTrip =  jenkins.configRoundtrip(p);
+
+        assertEquals(((CpsFlowDefinition)p.getDefinition()).isSandbox(),
+                     ((CpsFlowDefinition)roundTrip.getDefinition()).isSandbox());
+
+        assertEquals(((CpsFlowDefinition)p.getDefinition()).getScript(),
+                     ((CpsFlowDefinition)roundTrip.getDefinition()).getScript());
+    }
 }

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/CpsFlowDefinitionTest.java

@@ -0,0 +1,42 @@
+package org.jenkinsci.plugins.workflow.cps;
+
+import io.jenkins.plugins.casc.ConfigurationContext;
+import io.jenkins.plugins.casc.ConfiguratorRegistry;
+import io.jenkins.plugins.casc.misc.ConfiguredWithCode;
+import io.jenkins.plugins.casc.misc.JenkinsConfiguredWithCodeRule;
+import io.jenkins.plugins.casc.model.CNode;
+import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval;
+import org.junit.ClassRule;
+import org.junit.Test;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertTrue;
+import static io.jenkins.plugins.casc.misc.Util.getSecurityRoot;
+import static io.jenkins.plugins.casc.misc.Util.toStringFromYamlFile;
+import static io.jenkins.plugins.casc.misc.Util.toYamlString;
+
+public class JcascTest {
+
+    @ClassRule(order = 1)
+    @ConfiguredWithCode("casc_test.yaml")
+    public static JenkinsConfiguredWithCodeRule j = new JenkinsConfiguredWithCodeRule();
+
+    /**
+     * Check that CASC for security.cps.hideSandbox is sending the value to ScriptApproval.get().isForceSandbox()
+     * @throws Exception
+     */
+    @Test
+    public void cascHideSandBox() throws Exception {
+        assertTrue(ScriptApproval.get().isForceSandbox());
+    }
+
+    @Test
+    public void cascExport() throws Exception {
+        ConfiguratorRegistry registry = ConfiguratorRegistry.get();
+        ConfigurationContext context = new ConfigurationContext(registry);
+        CNode yourAttribute = getSecurityRoot(context).get("cps");
+        String exported = toYamlString(yourAttribute);
+        String expected = toStringFromYamlFile(this, "casc_test_expected.yaml");
+        assertEquals(exported, expected);
+    }
+}

plugin/src/test/java/org/jenkinsci/plugins/workflow/cps/JcascTest.java

@@ -0,0 +1,4 @@
+<?xml version='1.1' encoding='UTF-8'?>
+<org.jenkinsci.plugins.workflow.cps.config.CPSConfiguration plugin="workflow-cps">
+    <hideSandbox>true</hideSandbox>
+</org.jenkinsci.plugins.workflow.cps.config.CPSConfiguration>

plugin/src/test/resources/org/jenkinsci/plugins/workflow/cps/CpsFlowDefinitionTest/cpsLoadConfiguration/org.jenkinsci.plugins.workflow.cps.config.CPSConfiguration.xml

@@ -0,0 +1,3 @@
+security:
+  cps:
+    hideSandbox: true

plugin/src/test/resources/org/jenkinsci/plugins/workflow/cps/casc_test.yaml

@@ -0,0 +1 @@
+hideSandbox: true