From 0b18ac300222d56131a47631ebf03c07c5a550f8 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Wed, 6 Nov 2024 14:37:27 +0100 Subject: [PATCH 01/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators --- pom.xml | 5 +++++ .../java/hudson/slaves/CommandLauncher.java | 21 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/pom.xml b/pom.xml index f9f1f56..281675c 100644 --- a/pom.xml +++ b/pom.xml @@ -51,6 +51,11 @@ pom import + + org.jenkins-ci.plugins + script-security + 1367.vdf2fc45f229c + diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index e7f3d7c..49f480c 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -23,6 +23,7 @@ */ package hudson.slaves; +import edu.umd.cs.findbugs.annotations.CheckForNull; import edu.umd.cs.findbugs.annotations.NonNull; import edu.umd.cs.findbugs.annotations.Nullable; import hudson.AbortException; @@ -31,6 +32,7 @@ import hudson.Functions; import hudson.Util; import hudson.model.Descriptor; +import hudson.model.DescriptorVisibilityFilter; import hudson.model.Slave; import hudson.model.TaskListener; import hudson.remoting.Channel; @@ -250,4 +252,23 @@ public FormValidation doCheckCommand(@QueryParameter String value, @QueryParamet return ScriptApproval.get().checking(value, SystemCommandLanguage.get(), !StringUtils.equals(value, oldCommand)); } } + + @Extension + public static class DescriptorVisibilityFilterForceSandBox extends DescriptorVisibilityFilter { + @Override + public boolean filter(@CheckForNull Object context, @NonNull Descriptor descriptor) { + return (context instanceof DumbSlave && ((DumbSlave) context).getLauncher() instanceof CommandLauncher) + || checkForceSandboxVisibility(descriptor); + } + + @Override + public boolean filterType(@NonNull Class contextClass, @NonNull Descriptor descriptor) { + return checkForceSandboxVisibility(descriptor); + } + + private boolean checkForceSandboxVisibility(@NonNull Descriptor descriptor) + { + return !(descriptor instanceof DescriptorImpl && ScriptApproval.get().isForceSandboxForCurrentUser()); + } + } } From c5ab3e9b34e881dad22eb6f45d997c4d0b23c716 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Fri, 8 Nov 2024 12:09:46 +0100 Subject: [PATCH 02/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators --- .../java/hudson/slaves/CommandLauncher.java | 66 ++++++++++++++++--- 1 file changed, 56 insertions(+), 10 deletions(-) diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index 49f480c..d72bf37 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -31,6 +31,7 @@ import hudson.Extension; import hudson.Functions; import hudson.Util; +import hudson.model.ComputerSet; import hudson.model.Descriptor; import hudson.model.DescriptorVisibilityFilter; import hudson.model.Slave; @@ -41,6 +42,7 @@ import hudson.util.StreamCopyThread; import java.io.IOException; import java.util.Date; +import java.util.List; import java.util.logging.Level; import java.util.logging.Logger; import jenkins.model.Jenkins; @@ -51,6 +53,7 @@ import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.jenkinsci.plugins.scriptsecurity.scripts.UnapprovedUsageException; import org.jenkinsci.plugins.scriptsecurity.scripts.languages.SystemCommandLanguage; +import org.kohsuke.stapler.Ancestor; import org.kohsuke.stapler.DataBoundConstructor; import org.kohsuke.stapler.QueryParameter; import org.kohsuke.stapler.Stapler; @@ -82,7 +85,8 @@ public class CommandLauncher extends ComputerLauncher { * @see #CommandLauncher(String command, EnvVars env) */ @DataBoundConstructor - public CommandLauncher(String command) { + public CommandLauncher(String command) throws Descriptor.FormException { + checkSandbox(); agentCommand = command; env = null; // TODO add withKey if we can determine the Slave.nodeName being configured @@ -96,17 +100,32 @@ public CommandLauncher(String command) { * "sh -c" or write the expression into a script and point to the script) * @param env environment variables for the launcher to include when it runs the command */ - public CommandLauncher(String command, EnvVars env) { - this.agentCommand = command; + public CommandLauncher(String command, EnvVars env) throws Descriptor.FormException{ + checkSandbox(); + this.agentCommand = command; this.env = env; ScriptApproval.get().preapprove(command, SystemCommandLanguage.get()); } - /** Constructor for use from {@link CommandConnector}. Never approves the script. */ + /** Constructor for use from {@link CommandConnector}. Never approves the script. + * We don't execute the {@link #checkSandbox()} for backward compatibility, as this is just for running the Scripts + * */ CommandLauncher(EnvVars env, String command) { this.agentCommand = command; this.env = env; } + + /** + * Check if the current user is forced to use the Sandbox when creating a new instance. + * In this case, we don't allow saving new instances of the CommandLauncher object by throwing a new exception + */ + private void checkSandbox() throws Descriptor.FormException { + if (ScriptApproval.get().isForceSandboxForCurrentUser()) { + throw new Descriptor.FormException( + "This Launch Method requires scripts executions out of the sandbox." + + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", "sandbox"); + } + } private Object readResolve() { ScriptApproval.get().configuring(agentCommand, SystemCommandLanguage.get(), ApprovalContext.create(), true); @@ -253,22 +272,49 @@ public FormValidation doCheckCommand(@QueryParameter String value, @QueryParamet } } + /** + * In case the flag + * {@link ScriptApproval#isForceSandboxForCurrentUser} is true, we don't show the {@link DescriptorImpl descriptor} + * for the current user, except if we are editing a node that already has the launcher {@link CommandLauncher} + */ @Extension public static class DescriptorVisibilityFilterForceSandBox extends DescriptorVisibilityFilter { @Override public boolean filter(@CheckForNull Object context, @NonNull Descriptor descriptor) { - return (context instanceof DumbSlave && ((DumbSlave) context).getLauncher() instanceof CommandLauncher) - || checkForceSandboxVisibility(descriptor); + if(descriptor instanceof DescriptorImpl) + { + return !ScriptApproval.get().isForceSandboxForCurrentUser() || + (context instanceof DumbSlave && ((DumbSlave) context).getLauncher() instanceof CommandLauncher); + } + return true; } @Override public boolean filterType(@NonNull Class contextClass, @NonNull Descriptor descriptor) { - return checkForceSandboxVisibility(descriptor); + if(descriptor instanceof DescriptorImpl) + { + //If we are creating a new object, check ScriptApproval.get().isForceSandboxForCurrentUser() + //If we are NOT creating a new object, return true, and delegate the logic to #filter + return !(isCreatingNewObject() && ScriptApproval.get().isForceSandboxForCurrentUser()); + } + return true; } - private boolean checkForceSandboxVisibility(@NonNull Descriptor descriptor) - { - return !(descriptor instanceof DescriptorImpl && ScriptApproval.get().isForceSandboxForCurrentUser()); + private boolean isCreatingNewObject() { + boolean isCreating = false; + + if (Stapler.getCurrentRequest() != null) { + List ancs = Stapler.getCurrentRequest().getAncestors(); + for (Ancestor anc : ancs) { + if (!isCreating && anc.getObject() instanceof ComputerSet) { + String uri = Stapler.getCurrentRequest().getOriginalRequestURI(); + if (uri.endsWith("createItem")) { + isCreating = true; + } + } + } + } + return isCreating; } } } From 1e1d95d7f6c81e58fca1e9b733242a40c876374c Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Mon, 11 Nov 2024 09:01:11 +0100 Subject: [PATCH 03/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests --- pom.xml | 5 + .../hudson/slaves/CommandLauncherTest.java | 137 ++++++++++++++++++ 2 files changed, 142 insertions(+) diff --git a/pom.xml b/pom.xml index 281675c..aa94127 100644 --- a/pom.xml +++ b/pom.xml @@ -73,5 +73,10 @@ test-harness test + + org.jenkins-ci.plugins + matrix-auth + test + diff --git a/src/test/java/hudson/slaves/CommandLauncherTest.java b/src/test/java/hudson/slaves/CommandLauncherTest.java index 23c9872..2ec76fe 100644 --- a/src/test/java/hudson/slaves/CommandLauncherTest.java +++ b/src/test/java/hudson/slaves/CommandLauncherTest.java @@ -25,6 +25,17 @@ import hudson.EnvVars; import hudson.Functions; +import hudson.model.Descriptor; +import hudson.model.User; +import hudson.security.ACL; +import hudson.security.ACLContext; +import hudson.security.GlobalMatrixAuthorizationStrategy; +import hudson.security.Permission; + +import org.htmlunit.html.HtmlForm; +import org.jenkinsci.plugins.matrixauth.AuthorizationType; +import org.jenkinsci.plugins.matrixauth.PermissionEntry; +import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -37,6 +48,8 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; +import jenkins.model.Jenkins; + import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; @@ -156,4 +169,128 @@ public DumbSlave createAgentTimeout(String command) throws Exception { return agent; } + + @Test + public void commandLauncher_ForceSandbox() throws Exception { + DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/",new CommandLauncher("echo unconfigured")); + DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); + + j.jenkins.addNode(commandLauncherAgent); + j.jenkins.addNode(noCommandLauncherAgent); + + Jenkins.MANAGE.setEnabled(true); + + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + GlobalMatrixAuthorizationStrategy strategy = new GlobalMatrixAuthorizationStrategy(); + + PermissionEntry adminPermission = new PermissionEntry(AuthorizationType.USER, "admin"); + PermissionEntry develPermission = new PermissionEntry(AuthorizationType.USER, "devel"); + + strategy.add(Jenkins.ADMINISTER, adminPermission); + strategy.add(Jenkins.MANAGE, adminPermission); + strategy.add(Jenkins.READ, adminPermission); + strategy.add(Jenkins.MANAGE, develPermission); + strategy.add(Jenkins.READ, develPermission); + + for (Permission p : SlaveComputer.PERMISSIONS.getPermissions()) { + strategy.add(p, develPermission); + } + + j.jenkins.setAuthorizationStrategy(strategy); + + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + //With forceSandbox enabled, nonadmin users should not create agents with Launcher = CommandLauncher + ScriptApproval.get().setForceSandbox(true); + Descriptor.FormException ex = assertThrows(Descriptor.FormException.class, () -> + new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")) + ); + + assertEquals("This Launch Method requires scripts executions out of the sandbox." + + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", + ex.getMessage()); + + //With forceSandbox disabled, nonadmin users can create agents with Launcher = CommandLauncher + ScriptApproval.get().setForceSandbox(false); + new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")); + } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + //admin users can create agents with Launcher = CommandLauncher independently of forceSandbox flag. + ScriptApproval.get().setForceSandbox(true); + new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")); + + ScriptApproval.get().setForceSandbox(false); + new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")); + } + + ScriptApproval.get().setForceSandbox(true); + { + try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { + //Edit noCommandLauncher Agent. + //We are not admin and Sandbox is true, + //We don't have any html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertTrue(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe are not admin and Sandbox is true + // As the agent is already a commandLauncher one we have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Launch CommandLauncher non Approved Script + //We are not admin and Sandbox is true, + //Error message should not show any admin approval reference + //TODO: not sure how to tackle this. + //j.jenkins.addNode(test); + + //TODO: Test the new node page + } + + try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { + //Edit noCommandLauncher Agent. + //We areadmin and Sandbox is true, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe not admin and Sandbox is true + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } + + ScriptApproval.get().setForceSandbox(false); + { + try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { + //Edit noCommandLauncher Agent. + //We are not admin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe are not admin and Sandbox is false + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { + //Edit noCommandLauncher Agent. + //We areadmin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe not admin and Sandbox is false + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } + } } From 230ae87ae0be30630e3c0fb8382eb863a687ba52 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Mon, 11 Nov 2024 12:49:49 +0100 Subject: [PATCH 04/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests --- .../slaves/CommandLauncherForceSandbox.java | 159 ++++++++++++++++++ .../hudson/slaves/CommandLauncherTest.java | 137 --------------- 2 files changed, 159 insertions(+), 137 deletions(-) create mode 100644 src/test/java/hudson/slaves/CommandLauncherForceSandbox.java diff --git a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java new file mode 100644 index 0000000..cb852f7 --- /dev/null +++ b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java @@ -0,0 +1,159 @@ +package hudson.slaves; + +import java.io.IOException; + +import org.htmlunit.html.HtmlForm; +import org.jenkinsci.plugins.matrixauth.AuthorizationType; +import org.jenkinsci.plugins.matrixauth.PermissionEntry; +import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.jvnet.hudson.test.JenkinsRule; + +import hudson.model.Descriptor; +import hudson.model.User; +import hudson.security.ACL; +import hudson.security.ACLContext; +import hudson.security.GlobalMatrixAuthorizationStrategy; +import hudson.security.Permission; +import jenkins.model.Jenkins; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertThrows; +import static org.junit.Assert.assertTrue; + +public class CommandLauncherForceSandbox { + + @Rule + public JenkinsRule j = new JenkinsRule(); + + @Before + public void configureTest() throws IOException { + Jenkins.MANAGE.setEnabled(true); + + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + GlobalMatrixAuthorizationStrategy strategy = new GlobalMatrixAuthorizationStrategy(); + + PermissionEntry adminPermission = new PermissionEntry(AuthorizationType.USER, "admin"); + PermissionEntry develPermission = new PermissionEntry(AuthorizationType.USER, "devel"); + + strategy.add(Jenkins.ADMINISTER, adminPermission); + strategy.add(Jenkins.MANAGE, adminPermission); + strategy.add(Jenkins.READ, adminPermission); + strategy.add(Jenkins.MANAGE, develPermission); + strategy.add(Jenkins.READ, develPermission); + + for (Permission p : SlaveComputer.PERMISSIONS.getPermissions()) { + strategy.add(p, develPermission); + } + + j.jenkins.setAuthorizationStrategy(strategy); + } + + + @Test + public void newCommandLauncher() throws Exception { + try (ACLContext ctx = ACL.as(User.getById("devel", true))) { + //With forceSandbox enabled, nonadmin users should not create agents with Launcher = CommandLauncher + ScriptApproval.get().setForceSandbox(true); + Descriptor.FormException ex = assertThrows(Descriptor.FormException.class, () -> new DumbSlave("s", "/", + new CommandLauncher( + "echo unconfigured"))); + + assertEquals("This Launch Method requires scripts executions out of the sandbox." + + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", + ex.getMessage()); + + //With forceSandbox disabled, nonadmin users can create agents with Launcher = CommandLauncher + ScriptApproval.get().setForceSandbox(false); + new DumbSlave("s", "/", new CommandLauncher("echo unconfigured")); + } + + try (ACLContext ctx = ACL.as(User.getById("admin", true))) { + //admin users can create agents with Launcher = CommandLauncher independently of forceSandbox flag. + ScriptApproval.get().setForceSandbox(true); + new DumbSlave("s", "/", new CommandLauncher("echo unconfigured")); + + ScriptApproval.get().setForceSandbox(false); + new DumbSlave("s", "/", new CommandLauncher("echo unconfigured")); + } + } + + @Test + public void editCommandLauncher_ForceSandboxTrue() throws Exception { + DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/", new CommandLauncher("echo unconfigured")); + DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); + j.jenkins.addNode(commandLauncherAgent); + j.jenkins.addNode(noCommandLauncherAgent); + + ScriptApproval.get().setForceSandbox(true); + + try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { + //Edit noCommandLauncher Agent. + //We are not admin and Sandbox is true, + //We don't have any html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertTrue(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe are not admin and Sandbox is true + // As the agent is already a commandLauncher one we have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { + //Edit noCommandLauncher Agent. + //We areadmin and Sandbox is true, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe not admin and Sandbox is true + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } } + + @Test + public void editCommandLauncher_ForceSandboxFalse() throws Exception { + DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/", + new CommandLauncher("echo unconfigured")); + DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); + j.jenkins.addNode(commandLauncherAgent); + j.jenkins.addNode(noCommandLauncherAgent); + + ScriptApproval.get().setForceSandbox(false); + + try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { + //Edit noCommandLauncher Agent. + //We are not admin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe are not admin and Sandbox is false + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { + //Edit noCommandLauncher Agent. + //We areadmin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + + //Edit CommandLauncher Agent. + //Wwe not admin and Sandbox is false + //We have some html object for CommandLauncher + form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); + assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } +} diff --git a/src/test/java/hudson/slaves/CommandLauncherTest.java b/src/test/java/hudson/slaves/CommandLauncherTest.java index 2ec76fe..23c9872 100644 --- a/src/test/java/hudson/slaves/CommandLauncherTest.java +++ b/src/test/java/hudson/slaves/CommandLauncherTest.java @@ -25,17 +25,6 @@ import hudson.EnvVars; import hudson.Functions; -import hudson.model.Descriptor; -import hudson.model.User; -import hudson.security.ACL; -import hudson.security.ACLContext; -import hudson.security.GlobalMatrixAuthorizationStrategy; -import hudson.security.Permission; - -import org.htmlunit.html.HtmlForm; -import org.jenkinsci.plugins.matrixauth.AuthorizationType; -import org.jenkinsci.plugins.matrixauth.PermissionEntry; -import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.junit.Rule; import org.junit.Test; import org.junit.rules.TemporaryFolder; @@ -48,8 +37,6 @@ import java.util.concurrent.ExecutionException; import java.util.concurrent.TimeUnit; -import jenkins.model.Jenkins; - import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.containsString; import static org.hamcrest.Matchers.not; @@ -169,128 +156,4 @@ public DumbSlave createAgentTimeout(String command) throws Exception { return agent; } - - @Test - public void commandLauncher_ForceSandbox() throws Exception { - DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/",new CommandLauncher("echo unconfigured")); - DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); - - j.jenkins.addNode(commandLauncherAgent); - j.jenkins.addNode(noCommandLauncherAgent); - - Jenkins.MANAGE.setEnabled(true); - - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - GlobalMatrixAuthorizationStrategy strategy = new GlobalMatrixAuthorizationStrategy(); - - PermissionEntry adminPermission = new PermissionEntry(AuthorizationType.USER, "admin"); - PermissionEntry develPermission = new PermissionEntry(AuthorizationType.USER, "devel"); - - strategy.add(Jenkins.ADMINISTER, adminPermission); - strategy.add(Jenkins.MANAGE, adminPermission); - strategy.add(Jenkins.READ, adminPermission); - strategy.add(Jenkins.MANAGE, develPermission); - strategy.add(Jenkins.READ, develPermission); - - for (Permission p : SlaveComputer.PERMISSIONS.getPermissions()) { - strategy.add(p, develPermission); - } - - j.jenkins.setAuthorizationStrategy(strategy); - - try (ACLContext ctx = ACL.as(User.getById("devel", true))) { - //With forceSandbox enabled, nonadmin users should not create agents with Launcher = CommandLauncher - ScriptApproval.get().setForceSandbox(true); - Descriptor.FormException ex = assertThrows(Descriptor.FormException.class, () -> - new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")) - ); - - assertEquals("This Launch Method requires scripts executions out of the sandbox." - + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", - ex.getMessage()); - - //With forceSandbox disabled, nonadmin users can create agents with Launcher = CommandLauncher - ScriptApproval.get().setForceSandbox(false); - new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")); - } - - try (ACLContext ctx = ACL.as(User.getById("admin", true))) { - //admin users can create agents with Launcher = CommandLauncher independently of forceSandbox flag. - ScriptApproval.get().setForceSandbox(true); - new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")); - - ScriptApproval.get().setForceSandbox(false); - new DumbSlave("s", "/",new CommandLauncher("echo unconfigured")); - } - - ScriptApproval.get().setForceSandbox(true); - { - try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { - //Edit noCommandLauncher Agent. - //We are not admin and Sandbox is true, - //We don't have any html object for CommandLauncher - HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); - assertTrue(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - - //Edit CommandLauncher Agent. - //Wwe are not admin and Sandbox is true - // As the agent is already a commandLauncher one we have some html object for CommandLauncher - form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); - assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - - //Launch CommandLauncher non Approved Script - //We are not admin and Sandbox is true, - //Error message should not show any admin approval reference - //TODO: not sure how to tackle this. - //j.jenkins.addNode(test); - - //TODO: Test the new node page - } - - try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { - //Edit noCommandLauncher Agent. - //We areadmin and Sandbox is true, - //We have some html object for CommandLauncher - HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); - assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - - //Edit CommandLauncher Agent. - //Wwe not admin and Sandbox is true - //We have some html object for CommandLauncher - form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); - assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - } - } - - ScriptApproval.get().setForceSandbox(false); - { - try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { - //Edit noCommandLauncher Agent. - //We are not admin and Sandbox is false, - //We have some html object for CommandLauncher - HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); - assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - - //Edit CommandLauncher Agent. - //Wwe are not admin and Sandbox is false - //We have some html object for CommandLauncher - form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); - assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - } - - try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { - //Edit noCommandLauncher Agent. - //We areadmin and Sandbox is false, - //We have some html object for CommandLauncher - HtmlForm form = wc.getPage(noCommandLauncherAgent, "configure").getFormByName("config"); - assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - - //Edit CommandLauncher Agent. - //Wwe not admin and Sandbox is false - //We have some html object for CommandLauncher - form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); - assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); - } - } - } } From ee825348e1d22d0bd8085c0f51a578f90645f474 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Mon, 11 Nov 2024 13:55:45 +0100 Subject: [PATCH 05/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests --- .../slaves/CommandLauncherForceSandbox.java | 87 +++++++++++++++---- 1 file changed, 70 insertions(+), 17 deletions(-) diff --git a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java index cb852f7..e4a4ad4 100644 --- a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java +++ b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java @@ -3,6 +3,7 @@ import java.io.IOException; import org.htmlunit.html.HtmlForm; +import org.htmlunit.html.HtmlRadioButtonInput; import org.jenkinsci.plugins.matrixauth.AuthorizationType; import org.jenkinsci.plugins.matrixauth.PermissionEntry; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; @@ -10,6 +11,7 @@ import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; +import org.jvnet.hudson.test.JenkinsRule.WebClient; import hudson.model.Descriptor; import hudson.model.User; @@ -52,15 +54,13 @@ public void configureTest() throws IOException { j.jenkins.setAuthorizationStrategy(strategy); } - @Test public void newCommandLauncher() throws Exception { try (ACLContext ctx = ACL.as(User.getById("devel", true))) { //With forceSandbox enabled, nonadmin users should not create agents with Launcher = CommandLauncher ScriptApproval.get().setForceSandbox(true); - Descriptor.FormException ex = assertThrows(Descriptor.FormException.class, () -> new DumbSlave("s", "/", - new CommandLauncher( - "echo unconfigured"))); + Descriptor.FormException ex = assertThrows(Descriptor.FormException.class, () -> + new DumbSlave("s", "/",new CommandLauncher("echo unconfigured"))); assertEquals("This Launch Method requires scripts executions out of the sandbox." + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", @@ -82,15 +82,15 @@ public void newCommandLauncher() throws Exception { } @Test - public void editCommandLauncher_ForceSandboxTrue() throws Exception { + public void editCommandLauncherUI_ForceSandboxTrue() throws Exception { + ScriptApproval.get().setForceSandbox(true); + DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/", new CommandLauncher("echo unconfigured")); DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); j.jenkins.addNode(commandLauncherAgent); j.jenkins.addNode(noCommandLauncherAgent); - ScriptApproval.get().setForceSandbox(true); - - try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { + try (WebClient wc = j.createWebClient().login("devel")) { //Edit noCommandLauncher Agent. //We are not admin and Sandbox is true, //We don't have any html object for CommandLauncher @@ -98,13 +98,13 @@ public void editCommandLauncher_ForceSandboxTrue() throws Exception { assertTrue(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); //Edit CommandLauncher Agent. - //Wwe are not admin and Sandbox is true + //We are not admin and Sandbox is true // As the agent is already a commandLauncher one we have some html object for CommandLauncher form = wc.getPage(commandLauncherAgent, "configure").getFormByName("config"); assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); } - try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { + try (WebClient wc = j.createWebClient().login("admin")) { //Edit noCommandLauncher Agent. //We areadmin and Sandbox is true, //We have some html object for CommandLauncher @@ -119,16 +119,15 @@ public void editCommandLauncher_ForceSandboxTrue() throws Exception { } } @Test - public void editCommandLauncher_ForceSandboxFalse() throws Exception { - DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/", - new CommandLauncher("echo unconfigured")); + public void editCommandLauncherUI_ForceSandboxFalse() throws Exception { + ScriptApproval.get().setForceSandbox(false); + + DumbSlave commandLauncherAgent = new DumbSlave("commandLauncherAgent", "/", new CommandLauncher("echo unconfigured")); DumbSlave noCommandLauncherAgent = new DumbSlave("noCommandLauncherAgent", "/", new JNLPLauncher()); j.jenkins.addNode(commandLauncherAgent); j.jenkins.addNode(noCommandLauncherAgent); - ScriptApproval.get().setForceSandbox(false); - - try (JenkinsRule.WebClient wc = j.createWebClient().login("devel")) { + try (WebClient wc = j.createWebClient().login("devel")) { //Edit noCommandLauncher Agent. //We are not admin and Sandbox is false, //We have some html object for CommandLauncher @@ -142,7 +141,7 @@ public void editCommandLauncher_ForceSandboxFalse() throws Exception { assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); } - try (JenkinsRule.WebClient wc = j.createWebClient().login("admin")) { + try (WebClient wc = j.createWebClient().login("admin")) { //Edit noCommandLauncher Agent. //We areadmin and Sandbox is false, //We have some html object for CommandLauncher @@ -156,4 +155,58 @@ public void editCommandLauncher_ForceSandboxFalse() throws Exception { assertFalse(form.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); } } + + @Test + public void createCommandLauncherUI_ForceSandboxTrue() throws Exception { + ScriptApproval.get().setForceSandbox(true); + + try (WebClient wc = j.createWebClient().login("devel")) { + //Create Permanent Agent. + //We are not admin and Sandbox is true, + //We don't have any html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertTrue(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (WebClient wc = j.createWebClient().login("admin")) { + //Create Permanent Agent. + //We are admin and Sandbox is true, + //We have some html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertFalse(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } + + @Test + public void createCommandLauncherUI_ForceSandboxFalse() throws Exception { + ScriptApproval.get().setForceSandbox(false); + + try (WebClient wc = j.createWebClient().login("devel")) { + //Create Permanent Agent. + //We are not admin and Sandbox is false, + //We have some html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertFalse(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + + try (WebClient wc = j.createWebClient().login("admin")) { + //Create Permanent Agent. + //We are admin and Sandbox is true, + //We have some html object for CommandLauncher + HtmlForm form = wc.goTo("computer/new").getFormByName("createItem"); + form.getInputByName("name").setValue("devel_ComandLauncher"); + form.getInputsByValue(DumbSlave.class.getName()).stream().findFirst().get().setChecked(true); + HtmlForm createNodeForm = j.submit(form).getFormByName("config"); + assertFalse(createNodeForm.getInputsByValue(CommandLauncher.class.getName()).isEmpty()); + } + } } From 95404f10c943e2721a4930933931b165d6ceab96 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Mon, 11 Nov 2024 14:04:14 +0100 Subject: [PATCH 06/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests --- .../hudson/slaves/CommandLauncherForceSandbox.java | 11 +++-------- 1 file changed, 3 insertions(+), 8 deletions(-) diff --git a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java index e4a4ad4..df224cf 100644 --- a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java +++ b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java @@ -3,7 +3,6 @@ import java.io.IOException; import org.htmlunit.html.HtmlForm; -import org.htmlunit.html.HtmlRadioButtonInput; import org.jenkinsci.plugins.matrixauth.AuthorizationType; import org.jenkinsci.plugins.matrixauth.PermissionEntry; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; @@ -18,7 +17,6 @@ import hudson.security.ACL; import hudson.security.ACLContext; import hudson.security.GlobalMatrixAuthorizationStrategy; -import hudson.security.Permission; import jenkins.model.Jenkins; import static org.junit.Assert.assertEquals; @@ -35,22 +33,19 @@ public class CommandLauncherForceSandbox { public void configureTest() throws IOException { Jenkins.MANAGE.setEnabled(true); - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - GlobalMatrixAuthorizationStrategy strategy = new GlobalMatrixAuthorizationStrategy(); - PermissionEntry adminPermission = new PermissionEntry(AuthorizationType.USER, "admin"); PermissionEntry develPermission = new PermissionEntry(AuthorizationType.USER, "devel"); + GlobalMatrixAuthorizationStrategy strategy = new GlobalMatrixAuthorizationStrategy(); strategy.add(Jenkins.ADMINISTER, adminPermission); strategy.add(Jenkins.MANAGE, adminPermission); strategy.add(Jenkins.READ, adminPermission); strategy.add(Jenkins.MANAGE, develPermission); strategy.add(Jenkins.READ, develPermission); - for (Permission p : SlaveComputer.PERMISSIONS.getPermissions()) { - strategy.add(p, develPermission); - } + SlaveComputer.PERMISSIONS.getPermissions().forEach(p -> strategy.add(p,develPermission)); + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); j.jenkins.setAuthorizationStrategy(strategy); } From 2945dec029d9f72b3d9e06b1524d1aea6ec97160 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Mon, 11 Nov 2024 14:14:49 +0100 Subject: [PATCH 07/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests --- src/test/java/hudson/slaves/CommandLauncherForceSandbox.java | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java index df224cf..d37da69 100644 --- a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java +++ b/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java @@ -33,6 +33,8 @@ public class CommandLauncherForceSandbox { public void configureTest() throws IOException { Jenkins.MANAGE.setEnabled(true); + j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); + PermissionEntry adminPermission = new PermissionEntry(AuthorizationType.USER, "admin"); PermissionEntry develPermission = new PermissionEntry(AuthorizationType.USER, "devel"); @@ -42,10 +44,7 @@ public void configureTest() throws IOException { strategy.add(Jenkins.READ, adminPermission); strategy.add(Jenkins.MANAGE, develPermission); strategy.add(Jenkins.READ, develPermission); - SlaveComputer.PERMISSIONS.getPermissions().forEach(p -> strategy.add(p,develPermission)); - - j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); j.jenkins.setAuthorizationStrategy(strategy); } From 886fb9e7100cf86c793432db0289d200791a5a2d Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Mon, 11 Nov 2024 14:28:32 +0100 Subject: [PATCH 08/17] BEE-52312 - forceSandBox - Hide command-launcher drop down from non-administrators - Tests --- ...erForceSandbox.java => CommandLauncherForceSandboxTest.java} | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) rename src/test/java/hudson/slaves/{CommandLauncherForceSandbox.java => CommandLauncherForceSandboxTest.java} (99%) diff --git a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java b/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java similarity index 99% rename from src/test/java/hudson/slaves/CommandLauncherForceSandbox.java rename to src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java index d37da69..c116c52 100644 --- a/src/test/java/hudson/slaves/CommandLauncherForceSandbox.java +++ b/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java @@ -24,7 +24,7 @@ import static org.junit.Assert.assertThrows; import static org.junit.Assert.assertTrue; -public class CommandLauncherForceSandbox { +public class CommandLauncherForceSandboxTest { @Rule public JenkinsRule j = new JenkinsRule(); From 7f12293d2e146ce594cd729bfb28d7b055ca930a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa?= <60674455+jgarciacloudbees@users.noreply.github.com> Date: Tue, 12 Nov 2024 14:24:59 +0100 Subject: [PATCH 09/17] JENKINS-74820 - Apply suggestions from code review Co-authored-by: Antonio Muniz --- src/main/java/hudson/slaves/CommandLauncher.java | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index d72bf37..c447df9 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -100,7 +100,7 @@ public CommandLauncher(String command) throws Descriptor.FormException { * "sh -c" or write the expression into a script and point to the script) * @param env environment variables for the launcher to include when it runs the command */ - public CommandLauncher(String command, EnvVars env) throws Descriptor.FormException{ + public CommandLauncher(String command, EnvVars env) throws Descriptor.FormException { checkSandbox(); this.agentCommand = command; this.env = env; @@ -109,7 +109,7 @@ public CommandLauncher(String command, EnvVars env) throws Descriptor.FormExcept /** Constructor for use from {@link CommandConnector}. Never approves the script. * We don't execute the {@link #checkSandbox()} for backward compatibility, as this is just for running the Scripts - * */ + */ CommandLauncher(EnvVars env, String command) { this.agentCommand = command; this.env = env; @@ -281,8 +281,7 @@ public FormValidation doCheckCommand(@QueryParameter String value, @QueryParamet public static class DescriptorVisibilityFilterForceSandBox extends DescriptorVisibilityFilter { @Override public boolean filter(@CheckForNull Object context, @NonNull Descriptor descriptor) { - if(descriptor instanceof DescriptorImpl) - { + if(descriptor instanceof DescriptorImpl) { return !ScriptApproval.get().isForceSandboxForCurrentUser() || (context instanceof DumbSlave && ((DumbSlave) context).getLauncher() instanceof CommandLauncher); } From 905f8e86fbd22ead0820e1490fbd5ec367adb6d3 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 12 Nov 2024 16:07:55 +0100 Subject: [PATCH 10/17] BEE-52312 - change constructor signature to avoid breaking changes. --- .../java/hudson/slaves/CommandLauncher.java | 17 ++++++++++------- 1 file changed, 10 insertions(+), 7 deletions(-) diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index c447df9..76d22f5 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -100,8 +100,13 @@ public CommandLauncher(String command) throws Descriptor.FormException { * "sh -c" or write the expression into a script and point to the script) * @param env environment variables for the launcher to include when it runs the command */ - public CommandLauncher(String command, EnvVars env) throws Descriptor.FormException { - checkSandbox(); + public CommandLauncher(String command, EnvVars env) { + try { + checkSandbox(); + } catch (Descriptor.FormException ex) { + throw new RuntimeException(ex); + } + this.agentCommand = command; this.env = env; ScriptApproval.get().preapprove(command, SystemCommandLanguage.get()); @@ -300,20 +305,18 @@ public boolean filterType(@NonNull Class contextClass, @NonNull Descriptor de } private boolean isCreatingNewObject() { - boolean isCreating = false; - if (Stapler.getCurrentRequest() != null) { List ancs = Stapler.getCurrentRequest().getAncestors(); for (Ancestor anc : ancs) { - if (!isCreating && anc.getObject() instanceof ComputerSet) { + if (anc.getObject() instanceof ComputerSet) { String uri = Stapler.getCurrentRequest().getOriginalRequestURI(); if (uri.endsWith("createItem")) { - isCreating = true; + return true; } } } } - return isCreating; + return false; } } } From 5ba1477a31a3a048c76f73f211e68c28d45bd613 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 12 Nov 2024 17:05:19 +0100 Subject: [PATCH 11/17] BEE-52312 - bump pom version --- pom.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pom.xml b/pom.xml index aa94127..6015bb4 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.jenkins-ci.plugins plugin - 4.86 + 5.2 command-launcher From a70002a7042f0d9c75d36ceb72945e63aa0f76cc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Javier=20Garc=C3=ADa?= <60674455+jgarciacloudbees@users.noreply.github.com> Date: Tue, 12 Nov 2024 17:06:58 +0100 Subject: [PATCH 12/17] JENKINS-74820 - Apply suggestions from code review Co-authored-by: Jesse Glick --- src/main/java/hudson/slaves/CommandLauncher.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index 76d22f5..8190fcf 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -128,7 +128,7 @@ private void checkSandbox() throws Descriptor.FormException { if (ScriptApproval.get().isForceSandboxForCurrentUser()) { throw new Descriptor.FormException( "This Launch Method requires scripts executions out of the sandbox." - + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", "sandbox"); + + " This Jenkins instance has been configured to not allow regular users to disable the sandbox", "command"); } } @@ -288,7 +288,7 @@ public static class DescriptorVisibilityFilterForceSandBox extends DescriptorVis public boolean filter(@CheckForNull Object context, @NonNull Descriptor descriptor) { if(descriptor instanceof DescriptorImpl) { return !ScriptApproval.get().isForceSandboxForCurrentUser() || - (context instanceof DumbSlave && ((DumbSlave) context).getLauncher() instanceof CommandLauncher); + (context instanceof Slave && ((Slave) context).getLauncher() instanceof CommandLauncher); } return true; } From ff5a9dc2614e0fe80bdff804d66a696821cdfbe7 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 12 Nov 2024 17:13:40 +0100 Subject: [PATCH 13/17] BEE-52312 - add pom dependecies --- pom.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/pom.xml b/pom.xml index 6015bb4..0772d5d 100644 --- a/pom.xml +++ b/pom.xml @@ -44,6 +44,11 @@ + + jakarta.servlet + jakarta.servlet-api + 5.0.0 + io.jenkins.tools.bom bom-2.440.x From d5d3cbaa4da6057be669700e7f4e682d91763d4e Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 12 Nov 2024 18:31:43 +0100 Subject: [PATCH 14/17] BEE-52312 - add pom dependecies --- pom.xml | 18 ++++-------------- 1 file changed, 4 insertions(+), 14 deletions(-) diff --git a/pom.xml b/pom.xml index 0772d5d..efcda55 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ org.jenkins-ci.plugins plugin - 5.2 + 4.88 command-launcher @@ -13,7 +13,7 @@ 999999-SNAPSHOT jenkinsci/${project.artifactId}-plugin - 2.440.3 + 2.452.4 Command Agent Launcher Plugin Allows agents to be launched using a specified command. @@ -44,23 +44,13 @@ - - jakarta.servlet - jakarta.servlet-api - 5.0.0 - io.jenkins.tools.bom - bom-2.440.x - 3234.v5ca_5154341ef + bom-2.452.x + 3654.v237e4a_f2d8da_ pom import - - org.jenkins-ci.plugins - script-security - 1367.vdf2fc45f229c - From a1700c5ef49d5dbcc8647bfaa4a4b1c751276ef8 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 12 Nov 2024 18:50:10 +0100 Subject: [PATCH 15/17] BEE-52312 - SuggestedChanges --- src/main/java/hudson/slaves/CommandLauncher.java | 13 ++++--------- 1 file changed, 4 insertions(+), 9 deletions(-) diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index 8190fcf..488e070 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -101,12 +101,6 @@ public CommandLauncher(String command) throws Descriptor.FormException { * @param env environment variables for the launcher to include when it runs the command */ public CommandLauncher(String command, EnvVars env) { - try { - checkSandbox(); - } catch (Descriptor.FormException ex) { - throw new RuntimeException(ex); - } - this.agentCommand = command; this.env = env; ScriptApproval.get().preapprove(command, SystemCommandLanguage.get()); @@ -305,11 +299,12 @@ public boolean filterType(@NonNull Class contextClass, @NonNull Descriptor de } private boolean isCreatingNewObject() { - if (Stapler.getCurrentRequest() != null) { - List ancs = Stapler.getCurrentRequest().getAncestors(); + var req = Stapler.getCurrentRequest(); + if (req != null) { + List ancs = req.getAncestors(); for (Ancestor anc : ancs) { if (anc.getObject() instanceof ComputerSet) { - String uri = Stapler.getCurrentRequest().getOriginalRequestURI(); + String uri = req.getOriginalRequestURI(); if (uri.endsWith("createItem")) { return true; } From f31f604b162321bdd229f5fc02a7b7c4d0cb6161 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 12 Nov 2024 19:07:00 +0100 Subject: [PATCH 16/17] BEE-52312 - SuggestedChanges - Test --- pom.xml | 5 ----- .../CommandLauncherForceSandboxTest.java | 20 ++++++++----------- 2 files changed, 8 insertions(+), 17 deletions(-) diff --git a/pom.xml b/pom.xml index efcda55..2b69c2c 100644 --- a/pom.xml +++ b/pom.xml @@ -68,10 +68,5 @@ test-harness test - - org.jenkins-ci.plugins - matrix-auth - test - diff --git a/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java b/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java index c116c52..63f42d1 100644 --- a/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java +++ b/src/test/java/hudson/slaves/CommandLauncherForceSandboxTest.java @@ -3,20 +3,19 @@ import java.io.IOException; import org.htmlunit.html.HtmlForm; -import org.jenkinsci.plugins.matrixauth.AuthorizationType; -import org.jenkinsci.plugins.matrixauth.PermissionEntry; import org.jenkinsci.plugins.scriptsecurity.scripts.ScriptApproval; import org.junit.Before; import org.junit.Rule; import org.junit.Test; import org.jvnet.hudson.test.JenkinsRule; import org.jvnet.hudson.test.JenkinsRule.WebClient; +import org.jvnet.hudson.test.MockAuthorizationStrategy; +import hudson.model.Computer; import hudson.model.Descriptor; import hudson.model.User; import hudson.security.ACL; import hudson.security.ACLContext; -import hudson.security.GlobalMatrixAuthorizationStrategy; import jenkins.model.Jenkins; import static org.junit.Assert.assertEquals; @@ -35,16 +34,13 @@ public void configureTest() throws IOException { j.jenkins.setSecurityRealm(j.createDummySecurityRealm()); - PermissionEntry adminPermission = new PermissionEntry(AuthorizationType.USER, "admin"); - PermissionEntry develPermission = new PermissionEntry(AuthorizationType.USER, "devel"); + MockAuthorizationStrategy strategy = new MockAuthorizationStrategy(). + grant(Jenkins.ADMINISTER).everywhere().to("admin"). + grant(Jenkins.MANAGE).everywhere().to("devel"). + grant(Jenkins.READ, Computer.CONFIGURE).everywhere().to("devel"); + + SlaveComputer.PERMISSIONS.getPermissions().forEach(p -> strategy.grant(p).everywhere().to("devel")); - GlobalMatrixAuthorizationStrategy strategy = new GlobalMatrixAuthorizationStrategy(); - strategy.add(Jenkins.ADMINISTER, adminPermission); - strategy.add(Jenkins.MANAGE, adminPermission); - strategy.add(Jenkins.READ, adminPermission); - strategy.add(Jenkins.MANAGE, develPermission); - strategy.add(Jenkins.READ, develPermission); - SlaveComputer.PERMISSIONS.getPermissions().forEach(p -> strategy.add(p,develPermission)); j.jenkins.setAuthorizationStrategy(strategy); } From 51eaba7c23442e813dd30756675d2aed9bb96ad0 Mon Sep 17 00:00:00 2001 From: Javier Garcia Date: Tue, 12 Nov 2024 19:11:41 +0100 Subject: [PATCH 17/17] BEE-52312 - final changes --- src/main/java/hudson/slaves/CommandLauncher.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/src/main/java/hudson/slaves/CommandLauncher.java b/src/main/java/hudson/slaves/CommandLauncher.java index 488e070..d8dc9bb 100644 --- a/src/main/java/hudson/slaves/CommandLauncher.java +++ b/src/main/java/hudson/slaves/CommandLauncher.java @@ -101,14 +101,12 @@ public CommandLauncher(String command) throws Descriptor.FormException { * @param env environment variables for the launcher to include when it runs the command */ public CommandLauncher(String command, EnvVars env) { - this.agentCommand = command; + this.agentCommand = command; this.env = env; ScriptApproval.get().preapprove(command, SystemCommandLanguage.get()); } - /** Constructor for use from {@link CommandConnector}. Never approves the script. - * We don't execute the {@link #checkSandbox()} for backward compatibility, as this is just for running the Scripts - */ + /** Constructor for use from {@link CommandConnector}. Never approves the script. */ CommandLauncher(EnvVars env, String command) { this.agentCommand = command; this.env = env;