[FIXED JENKINS-46764] Log a useful message when scriptApproval is mal…

…formed
jenkinsci · Dec 6, 2017 · dee8e78 · dee8e78
1 parent ab44ba3
commit dee8e78
Show file tree

Hide file tree

Showing 4 changed files with 32 additions and 3 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
@@ -668,8 +668,14 @@ public synchronized String[] getAclApprovedSignatures() {
     @Restricted(NoExternalUse.class) // implementation
     @Extension public static final class ApprovedWhitelist extends ProxyWhitelist {
         public ApprovedWhitelist() throws IOException {
-            reconfigure();
+            try {
+                reconfigure();
+            } catch (IOException e) {
+                LOG.log(Level.SEVERE, "Malformed signature entry in scriptApproval.xml: '" + e.getMessage() + "'");
+                throw e;
+            }
         }
+
         String[][] reconfigure() throws IOException {
             ScriptApproval instance = ScriptApproval.get();
             synchronized (instance) {

diff --git a/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java b/src/test/java/org/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest.java
@@ -27,21 +27,29 @@
 import com.gargoylesoftware.htmlunit.html.HtmlPage;
 import com.gargoylesoftware.htmlunit.html.HtmlTextArea;
 import org.hamcrest.Matchers;
+import org.jenkinsci.plugins.scriptsecurity.sandbox.Whitelist;
 import org.jenkinsci.plugins.scriptsecurity.scripts.languages.GroovyLanguage;
+import org.junit.Rule;
 import org.junit.Test;
+import org.jvnet.hudson.test.Issue;
 import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.LoggerRule;
 import org.jvnet.hudson.test.recipes.LocalData;
 import org.xml.sax.SAXException;
 
 import java.io.IOException;
 import java.util.List;
 import java.util.concurrent.atomic.AtomicLong;
+import java.util.logging.Level;
 
 import static org.junit.Assert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.fail;
 
 public class ScriptApprovalTest extends AbstractApprovalTest<ScriptApprovalTest.Script> {
+    @Rule
+    public LoggerRule logging = new LoggerRule();
+
     private static final String CLEAR_ALL_ID = "approvedScripts-clear";
 
     private static final AtomicLong COUNTER = new AtomicLong(0L);
@@ -53,12 +61,27 @@ public class ScriptApprovalTest extends AbstractApprovalTest<ScriptApprovalTest.
         script("").use();
     }
 
-    @Test @LocalData public void dangerousApprovedSignatures() {
+    @Issue("JENKINS-46764")
+    @Test
+    @LocalData("malformedScriptApproval")
+    public void malformedScriptApproval() throws Exception {
+        logging.record(ScriptApproval.class, Level.FINER).capture(100);
+        try {
+            Whitelist w = new ScriptApproval.ApprovedWhitelist();
+        } catch (Exception e) {
+            // ignore - we want to make sure we're logging this properly.
+        }
+        assertThat(logging.getRecords(), Matchers.hasSize(Matchers.equalTo(1)));
+        assertEquals("Malformed signature entry in scriptApproval.xml: ' new java.lang.Exception java.lang.String'",
+                logging.getRecords().get(0).getMessage());
+    }
+
+    @Test @LocalData("dangerousApproved") public void dangerousApprovedSignatures() {
         String[] dangerousSignatures = ScriptApproval.get().getDangerousApprovedSignatures();
         assertEquals(1, dangerousSignatures.length);
     }
 
-    @Test @LocalData public void dangerousApprovedWarnings() throws IOException, SAXException {
+    @Test @LocalData("dangerousApproved") public void dangerousApprovedWarnings() throws IOException, SAXException {
         JenkinsRule.WebClient wc = r.createWebClient();
         HtmlPage managePage = wc.goTo("manage");
 

diff --git a/...ptsecurity/scripts/ScriptApprovalTest.zip → .../ScriptApprovalTest/dangerousApproved.zip b/...ptsecurity/scripts/ScriptApprovalTest.zip → .../ScriptApprovalTest/dangerousApproved.zip
diff --git a/...g/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest/malformedScriptApproval.zip b/...g/jenkinsci/plugins/scriptsecurity/scripts/ScriptApprovalTest/malformedScriptApproval.zip