Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 27 commits into from
Oct 25, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
a988f0a
JENKINS-73941 - Option to hide "Use Groovy Sandbox" for users without…
jgarciacloudbees Oct 16, 2024
c090d1c
JENKINS-73941 - Readme
jgarciacloudbees Oct 16, 2024
1671669
JENKINS-73941 - Renaming objects + Readme
jgarciacloudbees Oct 16, 2024
a42afca
JENKINS-73941 - Block saving Script Approval when Force Sandbox is en…
jgarciacloudbees Oct 16, 2024
b6b3290
JENKINS-73941 - Testing
jgarciacloudbees Oct 17, 2024
83b33c0
JENKINS-73941 - Text Formatting
jgarciacloudbees Oct 17, 2024
7beb61e
JENKINS-73941 - HideSandbox help improvement
jgarciacloudbees Oct 17, 2024
c621bd4
JENKINS-73941 - Avoid disabling the ScriptApproval screen with forceS…
jgarciacloudbees Oct 17, 2024
7b0e0ef
JENKINS-73941 - Fix the new option 'Force to use the Sandbox globally…
jgarciacloudbees Oct 17, 2024
e7215e6
JENKINS-73941 - New forceSandbox logic does not apply for admin users…
jgarciacloudbees Oct 18, 2024
9676a9b
JENKINS-73941 - New forceSandbox logic does not apply for admin user …
jgarciacloudbees Oct 21, 2024
2d9c65e
JENKINS-73941 - Additional messages for Sandbox mode
jgarciacloudbees Oct 21, 2024
7469530
JENKINS-73941 - New forceSandbox logic - Testing fixing.
jgarciacloudbees Oct 21, 2024
16ccea1
JENKINS-73941 - New forceSandbox logic - Messages changing.
jgarciacloudbees Oct 21, 2024
2ff801c
JENKINS-73941 - New forceSandbox logic - test improvement
jgarciacloudbees Oct 21, 2024
93722aa
JENKINS-73941 - New forceSandbox logic - Messages
jgarciacloudbees Oct 21, 2024
dc58b05
JENKINS-73941 - New forceSandbox logic - Messages + tests
jgarciacloudbees Oct 21, 2024
661356d
JENKINS-73941 - New forceSandbox logic - Messages + tests
jgarciacloudbees Oct 22, 2024
4cbf832
JENKINS-73941 - New forceSandbox logic - Add CASC support + tests
jgarciacloudbees Oct 22, 2024
af7eee1
JENKINS-73941 - Apply suggestions from code review
jgarciacloudbees Oct 24, 2024
fec6912
JENKINS-73941 - Apply suggestions from code review
jgarciacloudbees Oct 24, 2024
e7d0edd
JENKINS-73941 - Additional changes required from suggestions
jgarciacloudbees Oct 24, 2024
9907110
JENKINS-73941 - Additional changes required from suggestions
jgarciacloudbees Oct 24, 2024
2a9fe7c
JENKINS-73941 - Apply suggestions from code review
jgarciacloudbees Oct 25, 2024
4e97712
JENKINS-73941 - Add forceSandbox logic to SecureGroovyScript
jgarciacloudbees Oct 25, 2024
aad4d51
JENKINS-73941 - Apply suggestions from code review
jgarciacloudbees Oct 25, 2024
64f4584
JENKINS-73941 - test changes after suggestions
jgarciacloudbees Oct 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -463,6 +463,14 @@
return sandbox ? FormValidation.ok() : ScriptApproval.get().checking(value, GroovyLanguage.get(), !StringUtils.equals(oldScript, value));
}

@Restricted(NoExternalUse.class) // stapler
public boolean shouldHideSandbox(@CheckForNull SecureGroovyScript 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 ScriptApproval.get().isForceSandboxForCurrentUser()

Check warning on line 470 in src/main/java/org/jenkinsci/plugins/scriptsecurity/sandbox/groovy/SecureGroovyScript.java

View check run for this annotation

ci.jenkins.io / Code Coverage

Partially covered line

Line 470 is only partially covered, 3 branches are missing
&& (instance == null || instance.sandbox);
}

}

}
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ THE SOFTWARE.
<!-- TODO https://github.com/stapler/stapler-adjunct-codemirror/issues/1 means no true Groovy support -->
<f:textarea checkMethod="post"/> <!-- TODO codemirror-mode="clike" codemirror-config="'onBlur': cmChange" -->
</f:entry>
<f:entry field="sandbox">
<f:checkbox title="${%Use Groovy Sandbox}" default="${!h.hasPermission(app.ADMINISTER)}" />
<f:entry field="sandbox" class="${descriptor.shouldHideSandbox(instance) ? 'jenkins-hidden' : ''}">
<f:checkbox title="${%Use Groovy Sandbox}"
default="${!h.hasPermission(app.ADMINISTER) || descriptor.shouldHideSandbox(instance)}" />
jgarciacloudbees marked this conversation as resolved.
Show resolved Hide resolved
</f:entry>
<f:entry title="${%Additional classpath}" field="classpath">
<f:entry title="${%Additional classpath}" field="classpath"
class="${descriptor.shouldHideSandbox(instance) ?'jenkins-hidden' : ''}">
<f:repeatableProperty add="${%Add entry}" header="${%Classpath entry}" field="classpath"/>
</f:entry>
</j:jelly>
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
<!-- Just ti satisfy GlobalConfiguration.getGlobalConfigPage() -->
<?jelly escape-by-default='true'?>
<j:jelly xmlns:j="jelly:core" xmlns:st="jelly:stapler" xmlns:l="/lib/layout" xmlns:f="/lib/form">
<f:section title="${%Sandbox Configuration}">
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
<div>
<p>Controls whether the "Use Groovy Sandbox" is shown in the system to users without Overall/Administer permission.</p>
<p>This can be used to simplify the UX in highly secured environments where all pipelines and any other groovy execution are
required to run in the sandbox (ie. running arbitrary code is never approved)</p>
<p>This can be used to simplify the UX in highly secured environments where all Pipelines and any other Groovy execution are
required to run in the sandbox (i.e., running arbitrary code is never approved).</p>
</div>
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,51 @@ private void addPostBuildAction(HtmlPage page) throws IOException {
assertEquals("P#3", r.assertBuildStatusSuccess(p.scheduleBuild2(0)).getDescription());
}

/**
* Basic approval test where the user doesn't have ADMINISTER privs and forceSandbox is enabled
* Sandbox checkbox should not be visible, but set to true by default
*/
@Test public void basicApproval_ForceSandbox() throws Exception {
ScriptApproval.get().setForceSandbox(true);

r.jenkins.setSecurityRealm(r.createDummySecurityRealm());

MockAuthorizationStrategy mockStrategy = new MockAuthorizationStrategy();
mockStrategy.grant(Jenkins.READ).everywhere().to("devel");
for (Permission p : Item.PERMISSIONS.getPermissions()) {
mockStrategy.grant(p).everywhere().to("devel");
}
r.jenkins.setAuthorizationStrategy(mockStrategy);

FreeStyleProject p = r.createFreeStyleProject("p");
JenkinsRule.WebClient wc = r.createWebClient();
wc.login("devel");
HtmlPage page = wc.getPage(p, "configure");
HtmlForm config = page.getFormByName("config");
HtmlFormUtil.getButtonByCaption(config, "Add post-build action").click(); // lib/hudson/project/config-publishers2.jelly
addPostBuildAction(page);
wc.waitForBackgroundJavaScript(10000);
List<HtmlTextArea> scripts = config.getTextAreasByName("_.script");
// Get the last one, because previous ones might be from Lockable Resources during PCT.
HtmlTextArea script = scripts.get(scripts.size() - 1);
String groovy = "build.externalizableId";
script.setText(groovy);

//As the user is not admin and we are forcing Sandbox use,
// the Sandbox checkbox should be hidden and enabled by default.
List<HtmlInput> sandboxes = config.getInputsByName("_.sandbox");
HtmlCheckBoxInput sandboxcb = (HtmlCheckBoxInput) sandboxes.get(sandboxes.size() - 1);
assertTrue(sandboxcb.isChecked());

r.submit(config);

List<Publisher> publishers = p.getPublishersList();
assertEquals(1, publishers.size());
TestGroovyRecorder publisher = (TestGroovyRecorder) publishers.get(0);
assertEquals(groovy, publisher.getScript().getScript());
assertTrue(publisher.getScript().isSandbox());
}


/**
* Test where the user has ADMINISTER privs, default to non sandbox mode, but require approval
Expand Down Expand Up @@ -227,12 +272,20 @@ private void addPostBuildAction(HtmlPage page) throws IOException {
// Test that the script is executable. If it's not, we will get an UnapprovedUsageException
Exception ex = assertThrows(UnapprovedUsageException.class,
() -> ScriptApproval.get().using(groovy,GroovyLanguage.get()));
assertEquals(Messages.UnapprovedUsage_NonApproved(), ex.getMessage());
assertEquals(ScriptApproval.get().isForceSandbox()
? Messages.UnapprovedUsage_ForceSandBox()
: Messages.UnapprovedUsage_NonApproved()
, ex.getMessage());
}

/**
* Test where the user has ADMINISTER privs + forceSandboxEnabled
* logic should not change to the default admin behavior
* Except for the messages
*/
@Test public void testSandboxDefault_with_ADMINISTER_privs_ForceSandbox() throws Exception {
ScriptApproval.get().setForceSandbox(true);
ex = assertThrows(UnapprovedUsageException.class,
() -> ScriptApproval.get().using(groovy,GroovyLanguage.get()));
assertEquals(Messages.UnapprovedUsage_ForceSandBox(), ex.getMessage());
testSandboxDefault_with_ADMINISTER_privs();
}

/**
Expand Down
Loading