[JENKINS-60866][JENKINS-71513] Apply Stapler update for CSP-compliant…

… st:bind and renderOnDemand (#6865)

* [JENKINS-60866] Apply Stapler update for CSP-compliant st:bind

* [JENKINS-60866] Make renderOnDemand CSP-compliant

* Thanks Spotless

* Make Stapler incrementals work

* Update Stapler to new incremental

* Fixup bad merge

* Update Stapler, add test demonstrating st:bind working

* Address review feedback

* Add test for null bind, update Stapler

* Checkstyle

* More tests

* Use released Stapler

---------

Co-authored-by: Daniel Beck <daniel-beck@users.noreply.github.com>
Co-authored-by: Basil Crow <me@basilcrow.com>

bom/pom.xml

@@ -40,7 +40,7 @@ THE SOFTWARE.
   <properties>
     <asm.version>9.6</asm.version>
     <slf4jVersion>2.0.12</slf4jVersion>
-    <stapler.version>1822.v120278426e1c</stapler.version>
+    <stapler.version>1839.ved17667b_a_eb_5</stapler.version>
     <groovy.version>2.4.21</groovy.version>
   </properties>
 

core/src/main/java/hudson/Functions.java

@@ -2242,10 +2242,19 @@ public static boolean isWipeOutPermissionEnabled() {
         return SystemProperties.getBoolean("hudson.security.WipeOutPermission");
     }
 
+    @Deprecated
     public static String createRenderOnDemandProxy(JellyContext context, String attributesToCapture) {
         return Stapler.getCurrentRequest().createJavaScriptProxy(new RenderOnDemandClosure(context, attributesToCapture));
     }
 
+    /**
+     * Called from renderOnDemand.jelly to generate the parameters for the proxy object generation.
+     */
+    @Restricted(NoExternalUse.class)
+    public static StaplerRequest.RenderOnDemandParameters createRenderOnDemandProxyParameters(JellyContext context, String attributesToCapture) {
+        return Stapler.getCurrentRequest().createJavaScriptProxyParameters(new RenderOnDemandClosure(context, attributesToCapture));
+    }
+
     public static String getCurrentDescriptorByNameUrl() {
         return Descriptor.getCurrentDescriptorByNameUrl();
     }

core/src/main/resources/lib/layout/renderOnDemand.jelly

@@ -36,8 +36,12 @@ THE SOFTWARE.
     </st:attribute>
   </st:documentation>
 
+  <j:set var="parameters" value="${h.createRenderOnDemandProxyParameters(context,attrs.capture)}"/>
   <x:element name="${attrs.tag?:'div'}">
     <x:attribute name="class">render-on-demand ${attrs.clazz}</x:attribute>
-    <x:attribute name="proxy">${h.createRenderOnDemandProxy(context,attrs.capture)}</x:attribute>
+    <x:attribute name="data-proxy-method">${parameters.proxyMethod}</x:attribute>
+    <x:attribute name="data-proxy-url">${parameters.url}</x:attribute>
+    <x:attribute name="data-proxy-crumb">${parameters.crumb}</x:attribute>
+    <x:attribute name="data-proxy-url-names">${parameters.urlNames}</x:attribute>
   </x:element>
 </j:jelly>

test/src/test/java/org/kohsuke/stapler/BindTest.java

@@ -0,0 +1,176 @@
+package org.kohsuke.stapler;
+
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.endsWith;
+import static org.hamcrest.CoreMatchers.is;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.junit.Assert.assertThrows;
+
+import hudson.ExtensionList;
+import hudson.model.InvisibleAction;
+import hudson.model.RootAction;
+import java.util.Arrays;
+import java.util.List;
+import org.apache.commons.lang.StringUtils;
+import org.htmlunit.Page;
+import org.htmlunit.ScriptException;
+import org.htmlunit.html.HtmlPage;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.junit.runners.Parameterized;
+import org.jvnet.hudson.test.JenkinsRule;
+import org.jvnet.hudson.test.TestExtension;
+import org.kohsuke.stapler.bind.JavaScriptMethod;
+import org.kohsuke.stapler.bind.WithWellKnownURL;
+
+@RunWith(Parameterized.class)
+public class BindTest {
+    @Rule
+    public JenkinsRule j = new JenkinsRule();
+
+    @Parameterized.Parameters
+    public static List<String> contexts() {
+        return Arrays.asList("/jenkins", "");
+    }
+
+    public BindTest(String contextPath) {
+        j.contextPath = contextPath;
+    }
+
+    @Test
+    public void bindNormal() throws Exception {
+        final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
+        try (JenkinsRule.WebClient wc = j.createWebClient()) {
+            final HtmlPage htmlPage = wc.goTo(root.getUrlName());
+            final String scriptUrl = htmlPage
+                    .getElementsByTagName("script")
+                    .stream()
+                    .filter(it -> it.getAttribute("src").startsWith(j.contextPath + "/$stapler/bound/script" + j.contextPath + "/$stapler/bound/"))
+                    .findFirst()
+                    .orElseThrow()
+                    .getAttribute("src");
+
+            final Page script = wc.goTo(StringUtils.removeStart(scriptUrl, j.contextPath + "/"), "application/javascript");
+            final String content = script.getWebResponse().getContentAsString();
+            assertThat(content, startsWith("varname = makeStaplerProxy('" + j.contextPath + "/$stapler/bound/"));
+            assertThat(content, endsWith("','test',['annotatedJsMethod1','byName1']);"));
+        }
+        assertThat(root.invocations, is(1));
+    }
+
+    @Test
+    public void bindWithWellKnownURL() throws Exception {
+        final RootActionWithWellKnownURL root = ExtensionList.lookupSingleton(RootActionWithWellKnownURL.class);
+        try (JenkinsRule.WebClient wc = j.createWebClient()) {
+            final HtmlPage htmlPage = wc.goTo(root.getUrlName());
+            final String scriptUrl = htmlPage
+                    .getElementsByTagName("script")
+                    .stream()
+                    .filter(it -> it.getAttribute("src").startsWith(j.contextPath + "/$stapler/bound/script" + j.contextPath + "/theWellKnownRoot?"))
+                    .findFirst()
+                    .orElseThrow()
+                    .getAttribute("src");
+
+            final Page script = wc.goTo(StringUtils.removeStart(scriptUrl, j.contextPath + "/"), "application/javascript");
+            assertThat(script.getWebResponse().getContentAsString(), is("varname = makeStaplerProxy('" + j.contextPath + "/theWellKnownRoot','test',['annotatedJsMethod2','byName2']);"));
+        }
+        assertThat(root.invocations, is(1));
+    }
+
+    @Test
+    public void bindNull() throws Exception {
+        final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
+        try (JenkinsRule.WebClient wc = j.createWebClient()) {
+            final ScriptException exception = assertThrows(ScriptException.class, () -> wc.goTo(root.getUrlName() + "/null"));
+            assertThat(exception.getFailingLineNumber(), is(2));
+            assertThat(exception.getFailingColumnNumber(), is(0));
+            assertThat(exception.getMessage(), containsString("TypeError: Cannot call method \"byName1\" of null"));
+
+            final HtmlPage htmlPage = exception.getPage();
+            final String scriptUrl = htmlPage.getElementsByTagName("script").stream().filter(it -> it.getAttribute("src").equals(j.contextPath + "/$stapler/bound/script/null?var=varname")).findFirst().orElseThrow().getAttribute("src");
+
+            final Page script = wc.goTo(StringUtils.removeStart(scriptUrl, j.contextPath + "/"), "application/javascript");
+            final String content = script.getWebResponse().getContentAsString();
+            assertThat(content, is("varname = null;"));
+        }
+        assertThat(root.invocations, is(0));
+    }
+
+    @Test
+    public void bindUnsafe() throws Exception {
+        final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
+        try (JenkinsRule.WebClient wc = j.createWebClient()) {
+            final HtmlPage htmlPage = wc.goTo(root.getUrlName() + "/unsafe-var");
+            final String content = htmlPage
+                    .getElementsByTagName("script")
+                    .stream()
+                    .filter(it -> it.getTextContent().contains("makeStaplerProxy"))
+                    .findFirst()
+                    .orElseThrow()
+                    .getTextContent();
+
+            assertThat(content, startsWith("window['varname']=makeStaplerProxy('" + j.contextPath + "/$stapler/bound/"));
+            assertThat(content, endsWith("','test',['annotatedJsMethod1','byName1']);"));
+        }
+        assertThat(root.invocations, is(1));
+    }
+
+    @Test
+    public void bindInlineNull() throws Exception {
+        final RootActionImpl root = ExtensionList.lookupSingleton(RootActionImpl.class);
+        try (JenkinsRule.WebClient wc = j.createWebClient()) {
+            final HtmlPage htmlPage = wc.goTo(root.getUrlName() + "/inline-null");
+            final String content = htmlPage
+                    .getElementsByTagName("script")
+                    .stream()
+                    .filter(it -> it.getTextContent().contains("var inline"))
+                    .findFirst()
+                    .orElseThrow()
+                    .getTextContent();
+
+            assertThat(content, containsString("var inline = null"));
+        }
+        assertThat(root.invocations, is(0));
+    }
+
+    @TestExtension
+    public static class RootActionImpl extends InvisibleAction implements RootAction {
+        private int invocations;
+
+        @Override
+        public String getUrlName() {
+            return "theRoot";
+        }
+
+        @JavaScriptMethod
+        public void annotatedJsMethod1(String foo) {}
+
+        public void jsByName1() {
+            invocations++;
+        }
+    }
+
+    @TestExtension
+    public static class RootActionWithWellKnownURL extends InvisibleAction implements RootAction, WithWellKnownURL {
+        private int invocations;
+
+        @Override
+        public String getUrlName() {
+            return "theWellKnownRoot";
+        }
+
+        @Override
+        public String getWellKnownUrl() {
+            return "/" + getUrlName();
+        }
+
+        @JavaScriptMethod
+        public void annotatedJsMethod2(String foo) {}
+
+        public void jsByName2() {
+            invocations++;
+        }
+    }
+}

test/src/test/resources/org/kohsuke/stapler/BindTest/RootActionImpl/adjunct.js

@@ -0,0 +1,2 @@
+// eslint-disable-next-line no-undef
+varname.byName1();

test/src/test/resources/org/kohsuke/stapler/BindTest/RootActionImpl/index.jelly

@@ -0,0 +1,10 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
+    <l:layout title="The Root">
+        <l:main-panel>
+            <h1>Root Action</h1>
+            <st:bind var="varname" value="${it}"/>
+            <st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionImpl.adjunct"/>
+        </l:main-panel>
+    </l:layout>
+</j:jelly>

test/src/test/resources/org/kohsuke/stapler/BindTest/RootActionImpl/inline-null.jelly

@@ -0,0 +1,11 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
+    <l:layout title="The Root">
+        <l:main-panel>
+            <h1>Root Action</h1>
+            <script>
+                var inline = <st:bind value="${null}"/>
+            </script>
+        </l:main-panel>
+    </l:layout>
+</j:jelly>

test/src/test/resources/org/kohsuke/stapler/BindTest/RootActionImpl/null.jelly

@@ -0,0 +1,10 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
+    <l:layout title="The Root">
+        <l:main-panel>
+            <h1>Root Action with null</h1>
+            <st:bind var="varname" value="${null}"/>
+            <st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionImpl.adjunct"/>
+        </l:main-panel>
+    </l:layout>
+</j:jelly>

test/src/test/resources/org/kohsuke/stapler/BindTest/RootActionImpl/unsafe-var.jelly

@@ -0,0 +1,10 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
+    <l:layout title="The Root">
+        <l:main-panel>
+            <h1>Root Action</h1>
+            <st:bind var="window['varname']" value="${it}"/>
+            <st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionImpl.adjunct"/>
+        </l:main-panel>
+    </l:layout>
+</j:jelly>

test/src/test/resources/org/kohsuke/stapler/BindTest/RootActionWithWellKnownURL/adjunct.js

@@ -0,0 +1,2 @@
+// eslint-disable-next-line no-undef
+varname.byName2();

test/src/test/resources/org/kohsuke/stapler/BindTest/RootActionWithWellKnownURL/index.jelly

@@ -0,0 +1,10 @@
+<?jelly escape-by-default='true'?>
+<j:jelly xmlns:j="jelly:core" xmlns:l="/lib/layout" xmlns:st="jelly:stapler">
+    <l:layout title="The Root">
+        <l:main-panel>
+            <h1>Root Action</h1>
+            <st:bind var="varname" value="${it}"/>
+            <st:adjunct includes="org.kohsuke.stapler.BindTest.RootActionWithWellKnownURL.adjunct"/>
+        </l:main-panel>
+    </l:layout>
+</j:jelly>

war/src/main/webapp/scripts/hudson-behavior.js

@@ -997,7 +997,13 @@ function renderOnDemand(e, callback, noBehaviour) {
   if (!e || !e.classList.contains("render-on-demand")) {
     return;
   }
-  var proxy = eval(e.getAttribute("proxy"));
+
+  let proxyMethod = e.getAttribute("data-proxy-method");
+  let proxyUrl = e.getAttribute("data-proxy-url");
+  let proxyCrumb = e.getAttribute("data-proxy-crumb");
+  let proxyUrlNames = e.getAttribute("data-proxy-url-names").split(",");
+
+  var proxy = window[proxyMethod](proxyUrl, proxyCrumb, proxyUrlNames);
   proxy.render(function (t) {
     var contextTagName = e.parentNode.tagName;
     var c;