From a711d1423ee60da48af8ccc09cfc77213ef80412 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 29 Oct 2024 11:41:53 +0100 Subject: [PATCH 1/2] JENKINS-73941 - ForceSandbox - Align preexistant behavior in Workflow-CPS plugin --- .../sandbox/groovy/SecureGroovyScript.java | 10 ++++-- .../scripts/ScriptApproval.java | 13 +++++-- .../scripts/Messages.properties | 1 + .../groovy/SecureGroovyScriptTest.java | 34 +++++++++++++++++++ .../scripts/ScriptApprovalTest.java | 28 ++++++++++++++- 5 files changed, 80 insertions(+), 6 deletions(-) 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..ac1e1e47 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_ForceSandBoxMessage() + System.lineSeparator() : + ""; + 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..a379c95a 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,4 @@ 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 \ 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..55cabc71 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_ForceSandBoxMessage())); + + result = ScriptApproval.get().checking("test", GroovyLanguage.get(), true); + assertEquals(FormValidation.Kind.OK, result.kind); + assertTrue(result.getMessage().contains(Messages.ScriptApproval_ForceSandBoxMessage())); + } + + 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_ForceSandBoxMessage())); + + result = ScriptApproval.get().checking("test", GroovyLanguage.get(), true); + assertEquals(FormValidation.Kind.OK, result.kind); + assertFalse(result.getMessage().contains(Messages.ScriptApproval_ForceSandBoxMessage())); + } + } } private Script script(String groovy) { From 8cad9eed48ba2431299bd5e9c4221ea2e19bba01 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 29 Oct 2024 13:56:59 +0100 Subject: [PATCH 2/2] JENKINS-73941 - ForceSandbox - Changing admin information message --- .../plugins/scriptsecurity/scripts/ScriptApproval.java | 2 +- .../plugins/scriptsecurity/scripts/Messages.properties | 3 ++- .../scriptsecurity/scripts/ScriptApprovalTest.java | 8 ++++---- 3 files changed, 7 insertions(+), 6 deletions(-) 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 ac1e1e47..e8ae4940 100644 --- a/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java +++ b/src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java @@ -829,7 +829,7 @@ public synchronized FormValidation checking(@NonNull String script, @NonNull Lan Messages.ScriptApproval_PipelineMessage()); } else { String forceSandboxMessage = isForceSandbox() ? - Messages.ScriptApproval_ForceSandBoxMessage() + System.lineSeparator() : + Messages.ScriptApproval_AdminUserAlert() : ""; if ((ALLOW_ADMIN_APPROVAL_ENABLED && (willBeApproved || ADMIN_AUTO_APPROVAL_ENABLED)) || !Jenkins.get().isUseSecurity()) { 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 a379c95a..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,4 +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 \ No newline at end of file +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/scripts/ScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java index 55cabc71..f83b59e2 100644 --- a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java +++ b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java @@ -326,22 +326,22 @@ public void forceSandboxFormValidation() throws Exception { { FormValidation result = ScriptApproval.get().checking("test", GroovyLanguage.get(), false); assertEquals(FormValidation.Kind.OK, result.kind); - assertTrue(result.getMessage().contains(Messages.ScriptApproval_ForceSandBoxMessage())); + 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_ForceSandBoxMessage())); + 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_ForceSandBoxMessage())); + 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_ForceSandBoxMessage())); + assertFalse(result.getMessage().contains(Messages.ScriptApproval_AdminUserAlert())); } } }