Skip to content

Commit

Permalink
JENKINS-73941 - ForceSandbox - Align Script-Security plugin with pree…
Browse files Browse the repository at this point in the history
…xisting behavior in Workflow-CPS plugin (#585)

* JENKINS-73941 - ForceSandbox - Align preexistant behavior in Workflow-CPS plugin

* JENKINS-73941 - ForceSandbox - Changing admin information message
  • Loading branch information
jgarciacloudbees authored Oct 29, 2024
1 parent d44b49a commit df2fc45
Show file tree
Hide file tree
Showing 5 changed files with 81 additions and 6 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -92,13 +93,18 @@ public final class SecureGroovyScript extends AbstractDescribableImpl<SecureGroo

static final Logger LOGGER = Logger.getLogger(SecureGroovyScript.class.getName());

@DataBoundConstructor public SecureGroovyScript(@NonNull String script, boolean sandbox, @CheckForNull List<ClasspathEntry> classpath) {
@DataBoundConstructor public SecureGroovyScript(@NonNull String script, boolean sandbox,
@CheckForNull List<ClasspathEntry> 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);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -818,19 +818,26 @@ 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");

Check warning on line 823 in src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Not covered lines

Lines 821-823 are not covered by tests
}

if (!Jenkins.get().hasPermission(Jenkins.ADMINISTER)) {
return FormValidation.warningWithMarkup(isForceSandboxForCurrentUser() ?
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 = "<a class='jenkins-button script-approval-approve-link' data-base-url='" + Jenkins.get().getRootUrl() + ScriptApproval.get().getUrlName() + "' data-hash='" + result.newHash + "'>Approve script</a>";
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 " +
"<a target='blank' href='"+ Jenkins.get().getRootUrl() + ScriptApproval.get().getUrlName() + "'>Script Approval Configuration</a> page after save, or approve this version of the script. " +
approveScript);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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=<b>Sandbox is enabled globally in the system.</b><br />
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -948,6 +949,39 @@ private List<File> 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());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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) {
Expand Down

0 comments on commit df2fc45

Please sign in to comment.