diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java index c831054a..073d14e4 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java @@ -62,6 +62,7 @@ import org.jenkinsci.plugins.scriptsecurity.sandbox.RejectedAccessException; import org.jenkinsci.plugins.scriptsecurity.scripts.ApprovalContext; import org.jenkinsci.plugins.scriptsecurity.scripts.ClasspathEntry; +import org.jenkinsci.plugins.scriptsecurity.scripts.Messages; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedClasspathException; import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException; @@ -92,13 +93,18 @@ public final class SecureGroovyScript extends AbstractDescribableImpl classpath) { + @DataBoundConstructor public SecureGroovyScript(@NonNull String script, boolean sandbox, + @CheckForNull List classpath) + throws Descriptor.FormException { + if (!sandbox && ScriptApproval.get().isForceSandboxForCurrentUser()) { + throw new Descriptor.FormException(Messages.ScriptApproval_SandboxCantBeDisabled(), "sandbox"); + } this.script = script; this.sandbox = sandbox; this.classpath = classpath; } - @Deprecated public SecureGroovyScript(@NonNull String script, boolean sandbox) { + @Deprecated public SecureGroovyScript(@NonNull String script, boolean sandbox) throws Descriptor.FormException { this(script, sandbox, null); } diff --git a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java index 38619827..e8ae4940 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java @@ -818,7 +818,9 @@ public synchronized FormValidation checking(@NonNull String script, @NonNull Lan } final ConversionCheckResult result = checkAndConvertApprovedScript(script, language); if (result.approved) { - return FormValidation.okWithMarkup("The script is already approved"); + return FormValidation.okWithMarkup(isForceSandboxForCurrentUser() ? + Messages.ScriptApproval_ForceSandBoxMessage() : + "The script is already approved"); } if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) { @@ -826,11 +828,16 @@ public synchronized FormValidation checking(@NonNull String script, @NonNull Lan Messages.ScriptApproval_ForceSandBoxMessage() : Messages.ScriptApproval_PipelineMessage()); } else { + String forceSandboxMessage = isForceSandbox() ? + Messages.ScriptApproval_AdminUserAlert() : + ""; + if ((ALLOW_ADMIN_APPROVAL_ENABLED && (willBeApproved || ADMIN_AUTO_APPROVAL_ENABLED)) || !Jenkins.get().isUseSecurity()) { - return FormValidation.okWithMarkup("The script has not yet been approved, but it will be approved on save."); + return FormValidation.okWithMarkup(forceSandboxMessage + "The script has not yet been approved, " + + "but it will be approved on save."); } String approveScript = "Approve script"; - return FormValidation.okWithMarkup("The script is not approved and will not be approved on save. " + + return FormValidation.okWithMarkup(forceSandboxMessage + "The script is not approved and will not be approved on save. " + "Either modify the script to match an already approved script, approve it explicitly on the " + "Script Approval Configuration page after save, or approve this version of the script. " + approveScript); diff --git a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties index dc0dcce1..a7cdd3d0 100644 --- a/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties +++ b/src/main/resources/org/jenkinsci/plugins/scriptsecurity/scripts/Messages.properties @@ -11,3 +11,5 @@ ScriptApproval.PipelineMessage=A Jenkins administrator will need to approve this ScriptApproval.ForceSandBoxMessage=Running scripts out of the sandbox is not allowed in the system UnapprovedUsage.NonApproved=script not yet approved for use UnapprovedUsage.ForceSandBox=Running scripts out of the sandbox is not allowed in the system +ScriptApproval.SandboxCantBeDisabled=Sandbox cannot be disabled. This Jenkins instance has been configured to not allow regular users to disable the sandbox in the system +ScriptApproval.AdminUserAlert=Sandbox is enabled globally in the system.
\ No newline at end of file diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java index 8d99740b..f7fa48c6 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScriptTest.java @@ -39,6 +39,7 @@ import org.htmlunit.html.HtmlFormUtil; import org.htmlunit.html.HtmlPage; import org.htmlunit.html.HtmlTextArea; +import hudson.model.Descriptor; import hudson.model.FreeStyleProject; import hudson.model.FreeStyleBuild; import hudson.model.Item; @@ -948,6 +949,39 @@ private List getAllUpdatedJarFiles() throws URISyntaxException { } } + /** + * When forceSandbox logic is enabled, and nonAdmin users trying to save non Sandbox groovyScripts + * Then the system will fail with Descriptor.FormException. + * @throws Exception + */ + @Test + public void forceSandbox_NonAdminSaveNonSandbox() throws Exception { + r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); + MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy(); + mockStrategy.grant(Jenkins.ADMINISTER).everywhere().to("admin"); + mockStrategy.grant(Jenkins.READ).everywhere().to("devel"); + for (Permission p : Item.PERMISSIONS.getPermissions()) { + mockStrategy.grant(p).everywhere().to("admin"); + mockStrategy.grant(p).everywhere().to("devel"); + } + r.jenkins.setAuthorizationStrategy(mockStrategy); + + ScriptApproval.get().setForceSandbox(true); + + //Nonadmin users creating a nonSandbox SecureGroovyScript should fail. + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + Exception ex = assertThrows (Descriptor.FormException.class, () -> + new SecureGroovyScript("testScript", false, new ArrayList<>())); + + assertEquals(Messages.ScriptApproval_SandboxCantBeDisabled(), ex.getMessage()); + } + + //adminUsers can create a nonSandbox SecureGroovyScript with no issues. + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + new SecureGroovyScript("testScript", false, new ArrayList<>()); + } + } + @Test @Issue("SECURITY-2450") public void testScriptApproval() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java index b8f1f218..f83b59e2 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -301,7 +301,8 @@ public void forceSandboxScriptSignatureException() throws Exception { public void forceSandboxFormValidation() throws Exception { r.jenkins.setSecurityRealm(r.createDummySecurityRealm()); r.jenkins.setAuthorizationStrategy(new MockAuthorizationStrategy(). - grant(Jenkins.READ, Item.READ).everywhere().to("dev")); + grant(Jenkins.READ, Item.READ).everywhere().to("dev"). + grant(Jenkins.ADMINISTER).everywhere().to("admin")); try (ACLContext ctx = ACL.as(User.getById("devel", true))) { ScriptApproval.get().setForceSandbox(true); @@ -318,6 +319,31 @@ public void forceSandboxFormValidation() throws Exception { assertEquals(Messages.ScriptApproval_PipelineMessage(), result.getMessage()); } } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + + ScriptApproval.get().setForceSandbox(true); + { + FormValidation result = ScriptApproval.get().checking("test", GroovyLanguage.get(), false); + assertEquals(FormValidation.Kind.OK, result.kind); + assertTrue(result.getMessage().contains(Messages.ScriptApproval_AdminUserAlert())); + + result = ScriptApproval.get().checking("test", GroovyLanguage.get(), true); + assertEquals(FormValidation.Kind.OK, result.kind); + assertTrue(result.getMessage().contains(Messages.ScriptApproval_AdminUserAlert())); + } + + ScriptApproval.get().setForceSandbox(false); + { + FormValidation result = ScriptApproval.get().checking("test", GroovyLanguage.get(), false); + assertEquals(FormValidation.Kind.OK, result.kind); + assertFalse(result.getMessage().contains(Messages.ScriptApproval_AdminUserAlert())); + + result = ScriptApproval.get().checking("test", GroovyLanguage.get(), true); + assertEquals(FormValidation.Kind.OK, result.kind); + assertFalse(result.getMessage().contains(Messages.ScriptApproval_AdminUserAlert())); + } + } } private Script script(String groovy) {