Skip to content

Commit 3d3347a

Browse files
committed
fix: auto re-request approval once dismissed [JENKINS-63668]
1 parent 22fb899 commit 3d3347a

File tree

2 files changed

+43
-1
lines changed

2 files changed

+43
-1
lines changed

src/main/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApproval.java

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,10 @@ public synchronized String using(@NonNull String script, @NonNull Language langu
641641
}
642642
ConversionCheckResult result = checkAndConvertApprovedScript(script, language);
643643
if (!result.approved) {
644-
// Probably need not add to pendingScripts, since generally that would have happened already in configuring.
644+
// Usually. this method is called once the job configuration with the script is saved.
645+
// If a script was previously pending and is now deleted, however, it would require to re-configure the job.
646+
// That's why we call it again if it is unapproved in a running job.
647+
this.configuring(script, language, ApprovalContext.create(), false);
645648
throw new UnapprovedUsageException(result.newHash);
646649
}
647650
return script;

src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java

Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,11 @@
5151
import static org.hamcrest.MatcherAssert.assertThat;
5252
import static org.hamcrest.Matchers.nullValue;
5353
import static org.junit.Assert.assertEquals;
54+
import static org.junit.Assert.assertFalse;
55+
import static org.junit.Assert.assertNotNull;
56+
import static org.junit.Assert.assertNull;
5457
import static org.junit.Assert.assertThrows;
58+
import static org.junit.Assert.assertTrue;
5559
import static org.junit.Assert.fail;
5660

5761
public class ScriptApprovalTest extends AbstractApprovalTest<ScriptApprovalTest.Script> {
@@ -65,6 +69,41 @@ public class ScriptApprovalTest extends AbstractApprovalTest<ScriptApprovalTest.
6569
private static final String WHITELISTED_SIGNATURE = "method java.lang.String trim";
6670
private static final String DANGEROUS_SIGNATURE = "staticMethod hudson.model.User current";
6771

72+
private String approveIfExists(String script, ScriptApproval sa) throws IOException {
73+
for (ScriptApproval.PendingScript pending : sa.getPendingScripts()) {
74+
if(pending.script.equals(script)) {
75+
String hash = pending.getHash();
76+
sa.approveScript(hash);
77+
return hash;
78+
}
79+
}
80+
return null;
81+
}
82+
83+
@Test
84+
public void reapprovingShouldFail() throws Exception {
85+
configureSecurity();
86+
final ScriptApproval sa = ScriptApproval.get();
87+
FreeStyleProject p = r.createFreeStyleProject();
88+
String testScript = "jenkins.model.Jenkins.instance";
89+
p.getPublishersList().add(new TestGroovyRecorder(
90+
new SecureGroovyScript(testScript, false, null)));
91+
92+
String approvedHash = approveIfExists(testScript, sa);
93+
assertNotNull(approvedHash);
94+
r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0).get());
95+
96+
sa.denyScript(approvedHash);
97+
98+
/*
99+
* The script is not approved but should be pending.
100+
*/
101+
r.assertBuildStatus(Result.FAILURE, p.scheduleBuild2(0).get());
102+
approvedHash = approveIfExists(testScript, sa);
103+
assertNotNull(approvedHash);
104+
r.assertBuildStatus(Result.SUCCESS, p.scheduleBuild2(0).get());
105+
}
106+
68107
@Test public void emptyScript() throws Exception {
69108
configureSecurity();
70109
script("").use();

0 commit comments

Comments
 (0)