From 128360856d50d7b39473644e6c1c21ba11766195 Mon Sep 17 00:00:00 2001
From: Erik Hatcher <erik.hatcher@lucidworks.com>
Date: Wed, 11 Dec 2019 12:36:14 -0500
Subject: [PATCH] SOLR-14025: VelocityResponseWriter hardening

---
 solr/CHANGES.txt                              |   3 +
 .../response/SolrParamResourceLoader.java     |  73 --------
 .../solr/response/VelocityResponseWriter.java | 133 ++++++++-------
 .../solr/collection1/conf/solrconfig.xml      |   3 -
 .../collection1/conf/velocity/custom_tool.vm  |   1 -
 .../solr/collection1/conf/velocity/foreach.vm |   1 -
 .../conf/velocity/locale_number.vm            |   1 +
 .../conf/velocity/outside_the_box.vm          |   2 +-
 .../conf/velocity/sandbox_intersection.vm     |   2 +-
 solr/contrib/velocity/src/test/custom_tool.vm |  19 +++
 solr/contrib/velocity/src/test/foreach.vm     |  14 ++
 .../velocity/VelocityResponseWriterTest.java  | 157 ++++++++++++------
 .../src/velocity-response-writer.adoc         |  30 ++--
 13 files changed, 228 insertions(+), 211 deletions(-)
 delete mode 100644 solr/contrib/velocity/src/java/org/apache/solr/response/SolrParamResourceLoader.java
 delete mode 100644 solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/custom_tool.vm
 delete mode 100644 solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/foreach.vm
 create mode 100644 solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/locale_number.vm
 create mode 100644 solr/contrib/velocity/src/test/custom_tool.vm
 create mode 100644 solr/contrib/velocity/src/test/foreach.vm

diff --git a/solr/CHANGES.txt b/solr/CHANGES.txt
index 5203372d2ff..f8400da0900 100644
--- a/solr/CHANGES.txt
+++ b/solr/CHANGES.txt
@@ -119,6 +119,9 @@ Upgrade Notes
   third-party components will work the same as before due to type erasure but source code changes may be
   required.
 
+* SOLR-14025: VelocityResponseWriter has been hardened - only trusted configsets can render configset provided
+  templates and rendering templates from request parameters has been removed.
+
 New Features
 ---------------------
 * SOLR-13821: A Package store to store and load package artifacts (noble, Ishan Chattopadhyaya)
diff --git a/solr/contrib/velocity/src/java/org/apache/solr/response/SolrParamResourceLoader.java b/solr/contrib/velocity/src/java/org/apache/solr/response/SolrParamResourceLoader.java
deleted file mode 100644
index dda63f6180b..00000000000
--- a/solr/contrib/velocity/src/java/org/apache/solr/response/SolrParamResourceLoader.java
+++ /dev/null
@@ -1,73 +0,0 @@
-/*
- * Licensed to the Apache Software Foundation (ASF) under one or more
- * contributor license agreements.  See the NOTICE file distributed with
- * this work for additional information regarding copyright ownership.
- * The ASF licenses this file to You under the Apache License, Version 2.0
- * (the "License"); you may not use this file except in compliance with
- * the License.  You may obtain a copy of the License at
- *
- *     http://www.apache.org/licenses/LICENSE-2.0
- *
- * Unless required by applicable law or agreed to in writing, software
- * distributed under the License is distributed on an "AS IS" BASIS,
- * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
- * See the License for the specific language governing permissions and
- * limitations under the License.
- */
-package org.apache.solr.response;
-
-import org.apache.solr.common.params.SolrParams;
-import org.apache.solr.request.SolrQueryRequest;
-import org.apache.velocity.runtime.resource.loader.ResourceLoader;
-import org.apache.velocity.runtime.resource.Resource;
-import org.apache.velocity.exception.ResourceNotFoundException;
-import org.apache.velocity.util.ExtProperties;
-
-import java.io.Reader;
-import java.io.StringReader;
-import java.util.HashMap;
-import java.util.Iterator;
-import java.util.Map;
-
-public class SolrParamResourceLoader extends ResourceLoader {
-  public static final String TEMPLATE_PARAM_PREFIX = VelocityResponseWriter.TEMPLATE + ".";
-
-  private Map<String,String> templates = new HashMap<>();
-  public SolrParamResourceLoader(SolrQueryRequest request) {
-    super();
-
-    // TODO: Consider using content streams, but need a template name associated with each stream
-    // for now, a custom param convention of template.<name>=<template body> is a nice example
-    // of per-request overrides of templates
-
-    SolrParams params = request.getParams();
-    Iterator<String> names = params.getParameterNamesIterator();
-    while (names.hasNext()) {
-      String name = names.next();
-      
-      if (name.startsWith(TEMPLATE_PARAM_PREFIX)) {
-        templates.put(name.substring(TEMPLATE_PARAM_PREFIX.length()) + VelocityResponseWriter.TEMPLATE_EXTENSION,params.get(name));
-      }
-    }
-  }
-
-  @Override
-  public void init(ExtProperties extendedProperties) {
-  }
-
-  @Override
-  public Reader getResourceReader(String source, String encoding) throws ResourceNotFoundException {
-    String template = templates.get(source);
-    return template == null ? null : new StringReader(template);
-  }
-
-  @Override
-  public boolean isSourceModified(Resource resource) {
-    return false;
-  }
-
-  @Override
-  public long getLastModified(Resource resource) {
-    return 0;
-  }
-}
diff --git a/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java b/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java
index 39b0a698fec..8594d500768 100644
--- a/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java
+++ b/solr/contrib/velocity/src/java/org/apache/solr/response/VelocityResponseWriter.java
@@ -73,7 +73,6 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
   public static final String PROPERTIES_FILE = "init.properties.file";
 
   // System property names, these are _only_ loaded at node startup (no per-request control of these)
-  public static final String PARAMS_RESOURCE_LOADER_ENABLED = "velocity.resourceloader.params.enabled";
   public static final String SOLR_RESOURCE_LOADER_ENABLED = "velocity.resourceloader.solr.enabled";
 
   // request param names
@@ -89,8 +88,6 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
   public static final String JSON_CONTENT_TYPE = "application/json;charset=UTF-8";
 
   private File fileResourceLoaderBaseDir;
-  private boolean paramsResourceLoaderEnabled;
-  private boolean solrResourceLoaderEnabled;
   private String initPropertiesFileName;  // used just to hold from init() to inform()
 
   private static final Logger log = LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
@@ -115,12 +112,6 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
       }
     }
 
-    // params resource loader: off by default
-    paramsResourceLoaderEnabled = Boolean.getBoolean(PARAMS_RESOURCE_LOADER_ENABLED);
-
-    // solr resource loader: off by default
-    solrResourceLoaderEnabled = Boolean.getBoolean(SOLR_RESOURCE_LOADER_ENABLED);
-
     initPropertiesFileName = (String) args.get(PROPERTIES_FILE);
 
     NamedList tools = (NamedList)args.get("tools");
@@ -143,7 +134,7 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
         log.warn("Error loading " + PROPERTIES_FILE + " specified property file: " + initPropertiesFileName, e);
       }
     }
-  }
+    }
 
   @Override
   public String getContentType(SolrQueryRequest request, SolrQueryResponse response) {
@@ -267,26 +258,32 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
     resourceTool.configure(toolConfig);
     context.put("resource", resourceTool);
 
-/*
-    // Custom tools, specified in config as:
-        <queryResponseWriter name="velocityWithCustomTools" class="solr.VelocityResponseWriter">
-          <lst name="tools">
-            <str name="mytool">com.example.solr.velocity.MyTool</str>
-          </lst>
-        </queryResponseWriter>
-*/
-    // Custom tools can override any of the built-in tools provided above, by registering one with the same name
-    for(Map.Entry<String, String> entry : customTools.entrySet()) {
-      String name = entry.getKey();
-      Object customTool = SolrCore.createInstance(entry.getValue(), Object.class, "VrW custom tool: " + name, request.getCore(), request.getCore().getResourceLoader());
-      if (customTool instanceof LocaleConfig) {
-        ((LocaleConfig)customTool).configure(toolConfig);
-      }
-      context.put(name, customTool);
-    }
+    if (request.getCore().getCoreDescriptor().isConfigSetTrusted()) {
+      // Load custom tools, only if in a trusted configset
 
-    // custom tools _cannot_ override context objects added below, like $request and $response
-    // TODO: at least log a warning when one of the *fixed* tools classes in name with a custom one, currently silently ignored
+      /*
+          // Custom tools, specified in config as:
+              <queryResponseWriter name="velocityWithCustomTools" class="solr.VelocityResponseWriter">
+                <lst name="tools">
+                  <str name="mytool">com.example.solr.velocity.MyTool</str>
+                </lst>
+              </queryResponseWriter>
+      */
+      // Custom tools can override any of the built-in tools provided above, by registering one with the same name
+      if (request.getCore().getCoreDescriptor().isConfigSetTrusted()) {
+        for (Map.Entry<String, String> entry : customTools.entrySet()) {
+          String name = entry.getKey();
+          // TODO: at least log a warning when one of the *fixed* tools classes is same name with a custom one, currently silently ignored
+          Object customTool = SolrCore.createInstance(entry.getValue(), Object.class, "VrW custom tool: " + name, request.getCore(), request.getCore().getResourceLoader());
+          if (customTool instanceof LocaleConfig) {
+            ((LocaleConfig) customTool).configure(toolConfig);
+          }
+          context.put(name, customTool);
+        }
+      }
+
+      // custom tools _cannot_ override context objects added below, like $request and $response
+    }
 
 
     // Turn the SolrQueryResponse into a SolrResponse.
@@ -320,11 +317,11 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
   }
 
   private VelocityEngine createEngine(SolrQueryRequest request) {
-    VelocityEngine engine = new VelocityEngine();
 
-    // Set some engine properties that improve the experience
-    //   - these could be considered in the future for parameterization, but can also be overridden by using
-    //     the init.properties.file setting.  (TODO: add a test for this properties set here overridden)
+    boolean trustedMode = request.getCore().getCoreDescriptor().isConfigSetTrusted();
+
+
+    VelocityEngine engine = new VelocityEngine();
 
     // load the built-in _macros.vm first, then load VM_global_library.vm for legacy (pre-5.0) support,
     // and finally allow macros.vm to have the final say and override anything defined in the preceding files.
@@ -340,25 +337,25 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
        and there are Velocity template resource loaders.  It's confusing, they overlap: there is a Velocity resource
        loader that loads templates from Solr's resource loader (SolrVelocityResourceLoader).
 
-      The Velocity resource loader order is [params,][file,][solr], intentionally ordered in this manner, and each
-      one optional and individually enable-able.  By default, only "solr" (resource loader) is used, parsing templates
-      from a velocity/ sub-tree in either the classpath or under conf/.
+      The Velocity resource loader order is `[file,][solr],builtin` intentionally ordered in this manner.
+      The "file" resource loader, enabled when the configset is trusted and `template.base.dir` is specified as a
+      response writer init property.
 
-      A common usage would be to enable the file template loader, keeping the solr loader enabled; the Velocity resource
-      loader path would then be "file,solr" (params is disabled by default).  The basic browse templates are built into
+      The "solr" resource loader, enabled when the configset is trusted, and provides templates from a velocity/
+      sub-tree in either the classpath or under conf/.
+
+      By default, only "builtin" resource loader is enabled, providing tenplates from builtin Solr .jar files.
+
+      The basic browse templates are built into
       this plugin, but can be individually overridden by placing a same-named template in the template.base.dir specified
-      directory.
+      directory, or within a trusted configset's velocity/ directory.
      */
     ArrayList<String> loaders = new ArrayList<String>();
-    if (paramsResourceLoaderEnabled) {
-      loaders.add("params");
-      engine.setProperty("params.resource.loader.instance", new SolrParamResourceLoader(request));
-    }
-    if (fileResourceLoaderBaseDir != null) {
+    if ((fileResourceLoaderBaseDir != null) && trustedMode) {
       loaders.add("file");
       engine.setProperty(RuntimeConstants.FILE_RESOURCE_LOADER_PATH, fileResourceLoaderBaseDir.getAbsolutePath());
     }
-    if (solrResourceLoaderEnabled) {
+    if (trustedMode) {
       // The solr resource loader serves templates under a velocity/ subtree from <lib>, conf/,
       // or SolrCloud's configuration tree.  Or rather the other way around, other resource loaders are rooted
       // from the top, whereas this is velocity/ sub-tree rooted.
@@ -374,24 +371,42 @@ public class VelocityResponseWriter implements QueryResponseWriter, SolrCoreAwar
 
     engine.setProperty(RuntimeConstants.RESOURCE_LOADER, String.join(",", loaders));
 
-    engine.setProperty(RuntimeConstants.INPUT_ENCODING, "UTF-8");
 
-    // Work around VELOCITY-908 with Velocity not handling locales properly
-    Object spaceGobblingInitProperty = velocityInitProps.get(RuntimeConstants.SPACE_GOBBLING);
-    if(spaceGobblingInitProperty != null) {
-      // If there is an init property, uppercase it before Velocity.
-      velocityInitProps.put(RuntimeConstants.SPACE_GOBBLING,
-          String.valueOf(spaceGobblingInitProperty).toUpperCase(Locale.ROOT));
-    } else {
-      // Fallback to checking if the engine default property is set and if not make it a reasonable default.
-      Object spaceGobblingEngineProperty = engine.getProperty(RuntimeConstants.SPACE_GOBBLING);
-      if(spaceGobblingEngineProperty == null) {
-        engine.setProperty(RuntimeConstants.SPACE_GOBBLING, RuntimeConstants.SpaceGobbling.LINES.toString());
+    engine.setProperty(RuntimeConstants.INPUT_ENCODING, "UTF-8");
+    engine.setProperty(RuntimeConstants.SPACE_GOBBLING, RuntimeConstants.SpaceGobbling.LINES.toString());
+
+    // install a class/package restricting uberspector
+    engine.setProperty(RuntimeConstants.UBERSPECT_CLASSNAME,"org.apache.velocity.util.introspection.SecureUberspector");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_PACKAGES,"java.lang.reflect");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.Class");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.ClassLoader");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.Compiler");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.InheritableThreadLocal");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.Package");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.Process");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.Runtime");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.RuntimePermission");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.SecurityManager");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.System");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.Thread");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.ThreadGroup");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"java.lang.ThreadLocal");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"org.apache.solr.core.SolrResourceLoader");
+    engine.addProperty(RuntimeConstants.INTROSPECTOR_RESTRICT_CLASSES,"org.apache.solr.core.CoreContainer");
+
+    if (trustedMode) {
+      // Work around VELOCITY-908 with Velocity not handling locales properly
+      Object spaceGobblingInitProperty = velocityInitProps.get(RuntimeConstants.SPACE_GOBBLING);
+      if (spaceGobblingInitProperty != null) {
+        // If there is an init property, uppercase it before Velocity.
+        velocityInitProps.put(RuntimeConstants.SPACE_GOBBLING,
+            String.valueOf(spaceGobblingInitProperty).toUpperCase(Locale.ROOT));
       }
+      // bring in any custom properties too
+      engine.setProperties(velocityInitProps);
     }
 
-    // bring in any custom properties too
-    engine.init(velocityInitProps);
+    engine.init();
 
     return engine;
   }
diff --git a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/solrconfig.xml b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/solrconfig.xml
index f1e0687b052..35ce52b59a5 100644
--- a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/solrconfig.xml
+++ b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/solrconfig.xml
@@ -38,9 +38,6 @@
   </queryResponseWriter>
 
   <queryResponseWriter name="velocityWithCustomTools" class="solr.VelocityResponseWriter">
-    <!-- Enable params resource loader to make tests leaner, no external template needed -->
-    <bool name="params.resource.loader.enabled">true</bool>
-
     <lst name="tools">
       <!-- how someone would typically add a custom tool, with a custom, non-clashing name -->
       <str name="mytool">org.apache.solr.velocity.MockTool</str>
diff --git a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/custom_tool.vm b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/custom_tool.vm
deleted file mode 100644
index 461a27b25dd..00000000000
--- a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/custom_tool.vm
+++ /dev/null
@@ -1 +0,0 @@
-$!mytool.star("foo")
\ No newline at end of file
diff --git a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/foreach.vm b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/foreach.vm
deleted file mode 100644
index 3449dac187c..00000000000
--- a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/foreach.vm
+++ /dev/null
@@ -1 +0,0 @@
-#foreach($x in ["a","b"])$!foreach.index#end
\ No newline at end of file
diff --git a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/locale_number.vm b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/locale_number.vm
new file mode 100644
index 00000000000..9994022e4cd
--- /dev/null
+++ b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/locale_number.vm
@@ -0,0 +1 @@
+$number.format(2112)
\ No newline at end of file
diff --git a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/outside_the_box.vm b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/outside_the_box.vm
index 284282372ba..c52c94bd2f7 100644
--- a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/outside_the_box.vm
+++ b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/outside_the_box.vm
@@ -1,4 +1,4 @@
 #set($x='')
 #set($sys=$x.class.forName('java.lang.System'))
 #set($ex=$sys.getProperty('os.name'))
-$ex
+$ex
\ No newline at end of file
diff --git a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/sandbox_intersection.vm b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/sandbox_intersection.vm
index 75d97a18796..80c7422ffb0 100644
--- a/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/sandbox_intersection.vm
+++ b/solr/contrib/velocity/src/test-files/velocity/solr/collection1/conf/velocity/sandbox_intersection.vm
@@ -2,4 +2,4 @@
 #set($sys=$x.class.forName('java.nio.file.Paths'))
 #set($path=$sys.get('/dumbass/denied_location'))
 #set($ex=$path.resolve($path).toRealPath())
-$ex
+$ex
\ No newline at end of file
diff --git a/solr/contrib/velocity/src/test/custom_tool.vm b/solr/contrib/velocity/src/test/custom_tool.vm
new file mode 100644
index 00000000000..3efff7d43c5
--- /dev/null
+++ b/solr/contrib/velocity/src/test/custom_tool.vm
@@ -0,0 +1,19 @@
+#* Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License. *#
+
+mytool.star=$!mytool.star("LATERALUS")
+mytool.locale=$!mytool.locale
+log.star=$!log.star("log overridden")
+response.star=$!response.star("response overridden??")
diff --git a/solr/contrib/velocity/src/test/foreach.vm b/solr/contrib/velocity/src/test/foreach.vm
new file mode 100644
index 00000000000..5d7d1e91cbc
--- /dev/null
+++ b/solr/contrib/velocity/src/test/foreach.vm
@@ -0,0 +1,14 @@
+#* Licensed to the Apache Software Foundation (ASF) under one or more
+ contributor license agreements.  See the NOTICE file distributed with
+ this work for additional information regarding copyright ownership.
+ The ASF licenses this file to You under the Apache License, Version 2.0
+ (the "License"); you may not use this file except in compliance with
+ the License.  You may obtain a copy of the License at
+
+     http://www.apache.org/licenses/LICENSE-2.0
+
+ Unless required by applicable law or agreed to in writing, software
+ distributed under the License is distributed on an "AS IS" BASIS,
+ WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ See the License for the specific language governing permissions and
+ limitations under the License. *##foreach($x in ["a","b"])$!foreach.index#end
diff --git a/solr/contrib/velocity/src/test/org/apache/solr/velocity/VelocityResponseWriterTest.java b/solr/contrib/velocity/src/test/org/apache/solr/velocity/VelocityResponseWriterTest.java
index c139379fda7..1aa3dfa8168 100644
--- a/solr/contrib/velocity/src/test/org/apache/solr/velocity/VelocityResponseWriterTest.java
+++ b/solr/contrib/velocity/src/test/org/apache/solr/velocity/VelocityResponseWriterTest.java
@@ -17,35 +17,39 @@
 package org.apache.solr.velocity;
 
 import java.io.IOException;
+import java.io.StringReader;
 import java.io.StringWriter;
 import java.security.AccessControlException;
+import java.util.Properties;
 
 import org.apache.solr.SolrTestCaseJ4;
 import org.apache.solr.common.SolrException;
 import org.apache.solr.common.util.NamedList;
 import org.apache.solr.request.SolrQueryRequest;
 import org.apache.solr.response.QueryResponseWriter;
-import org.apache.solr.response.SolrParamResourceLoader;
 import org.apache.solr.response.SolrQueryResponse;
 import org.apache.solr.response.VelocityResponseWriter;
 import org.apache.velocity.exception.MethodInvocationException;
 import org.junit.AfterClass;
 import org.junit.BeforeClass;
+import org.junit.Ignore;
 import org.junit.Test;
 
 public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
   @BeforeClass
   public static void beforeClass() throws Exception {
-    System.setProperty("velocity.resourceloader.params.enabled", "true");
-    System.setProperty("velocity.resourceloader.solr.enabled", "true");
     initCore("solrconfig.xml", "schema.xml", getFile("velocity/solr").getAbsolutePath());
-    System.out.println(getFile("velocity/solr").getAbsolutePath());
   }
 
   @AfterClass
   public static void afterClass() throws Exception {
-    System.clearProperty("velocity.resourceloader.params.enabled");
-    System.clearProperty("velocity.resourceloader.solr.enabled");
+  }
+
+  @Override
+  public void setUp() throws Exception {
+    // This test case toggles the configset used from trusted to untrusted - return to default of trusted for each test
+    h.getCoreContainer().getCoreDescriptor(h.coreName).setConfigSetTrusted(true);
+    super.setUp();
   }
 
   @Test
@@ -55,6 +59,20 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
   }
 
   @Test
+  public void testSecureUberspector() throws Exception {
+    VelocityResponseWriter vrw = new VelocityResponseWriter();
+    NamedList<String> nl = new NamedList<>();
+    nl.add("template.base.dir", getFile("velocity").getAbsolutePath());
+    vrw.init(nl);
+    SolrQueryRequest req = req(VelocityResponseWriter.TEMPLATE,"outside_the_box");
+    SolrQueryResponse rsp = new SolrQueryResponse();
+    StringWriter buf = new StringWriter();
+    vrw.write(buf, req, rsp);
+    assertEquals("$ex",buf.toString());  // $ex rendered literally because it is null, and thus did not succeed to break outside the box
+  }
+
+  @Test
+  @Ignore("SOLR-14025: Velocity's SecureUberspector addresses this")
   public void testTemplateSandbox() throws Exception {
     assumeTrue("This test only works with security manager", System.getSecurityManager() != null);
     VelocityResponseWriter vrw = new VelocityResponseWriter();
@@ -66,7 +84,7 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
     StringWriter buf = new StringWriter();
     try {
       vrw.write(buf, req, rsp);
-      fail("template broke outside the box, retrieved OS: " + buf);
+      fail("template broke outside the box, retrieved: " + buf);
     } catch (MethodInvocationException e) {
       assertNotNull(e.getCause());
       assertEquals(AccessControlException.class, e.getCause().getClass());
@@ -75,6 +93,7 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
   }
 
   @Test
+  @Ignore("SOLR-14025: Velocity's SecureUberspector addresses this")
   public void testSandboxIntersection() throws Exception {
     assumeTrue("This test only works with security manager", System.getSecurityManager() != null);
     VelocityResponseWriter vrw = new VelocityResponseWriter();
@@ -86,7 +105,7 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
     StringWriter buf = new StringWriter();
     try {
       vrw.write(buf, req, rsp);
-      fail("template broke outside the box, retrieved OS: " + buf);
+      fail("template broke outside the box, retrieved: " + buf);
     } catch (MethodInvocationException e) {
       assertNotNull(e.getCause());
       assertEquals(AccessControlException.class, e.getCause().getClass());
@@ -94,32 +113,6 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
     }
   }
 
-  @Test
-  public void testCustomParamTemplate() throws Exception {
-    org.apache.solr.response.VelocityResponseWriter vrw = new VelocityResponseWriter();
-    NamedList<String> nl = new NamedList<>();
-    nl.add(VelocityResponseWriter.PARAMS_RESOURCE_LOADER_ENABLED, "true");
-    vrw.init(nl);
-    SolrQueryRequest req = req(VelocityResponseWriter.TEMPLATE,"custom",
-        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"custom","$response.response.response_data");
-    SolrQueryResponse rsp = new SolrQueryResponse();
-    StringWriter buf = new StringWriter();
-    rsp.add("response_data", "testing");
-    vrw.write(buf, req, rsp);
-    assertEquals("testing", buf.toString());
-  }
-
-  @Test
-  public void testParamResourceLoaderDisabled() {
-    VelocityResponseWriter vrw = new VelocityResponseWriter();
-    // by default param resource loader is disabled, no need to set it here
-    SolrQueryRequest req = req(VelocityResponseWriter.TEMPLATE,"custom",
-        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"custom","$response.response.response_data");
-    SolrQueryResponse rsp = new SolrQueryResponse();
-    StringWriter buf = new StringWriter();
-    expectThrows(IOException.class, () -> vrw.write(buf, req, rsp));
-  }
-
   @Test
   public void testFileResourceLoader() throws Exception {
     VelocityResponseWriter vrw = new VelocityResponseWriter();
@@ -133,6 +126,28 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
     assertEquals("testing", buf.toString());
   }
 
+  @Test
+  public void testTemplateTrust() throws Exception {
+    // Try on trusted configset....
+    assertEquals("0", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"numFound")));
+
+    // Turn off trusted configset, which disables the Solr resource loader
+    h.getCoreContainer().getCoreDescriptor(h.coreName).setConfigSetTrusted(false);
+    assertFalse(h.getCoreContainer().getCoreDescriptor(coreName).isConfigSetTrusted());
+
+    try {
+      assertEquals("0", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"numFound")));
+      fail("template rendering should have failed, from an untrusted configset");
+    } catch (IOException e) {
+      // expected exception
+      assertEquals(IOException.class, e.getClass());
+    }
+
+    // set the harness back to the default of trusted
+    h.getCoreContainer().getCoreDescriptor(h.coreName).setConfigSetTrusted(true);
+  }
+
+
   @Test
   public void testSolrResourceLoaderTemplate() throws Exception {
     assertEquals("0", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"numFound")));
@@ -163,31 +178,69 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
 
     assertEquals("01", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"foreach")));
     assertEquals("", h.query(req("q","*:*", "wt","velocityWithInitProps",VelocityResponseWriter.TEMPLATE,"foreach")));
+
+    // Turn off trusted configset, which disables the init properties
+    h.getCoreContainer().getCoreDescriptor(h.coreName).setConfigSetTrusted(false);
+    assertFalse(h.getCoreContainer().getCoreDescriptor(coreName).isConfigSetTrusted());
+
+    assertEquals("01", h.query(req("q","*:*", "wt","velocityWithInitProps",VelocityResponseWriter.TEMPLATE,"foreach")));
+
+    // set the harness back to the default of trusted
+    h.getCoreContainer().getCoreDescriptor(h.coreName).setConfigSetTrusted(true);
   }
 
   @Test
   public void testCustomTools() throws Exception {
-    // custom_tool.vm responds with $!mytool.star("foo"), but $mytool is not defined (only in velocityWithCustomTools)
-    assertEquals("", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"custom_tool")));
+    // Render this template once without a custom tool defined, and once with it defined.  The tool has a `.star` method.
+    // The tool added as `mytool`, `log`, and `response`.  `log` is designed to be overridable, but not `response`
+    //    mytool.star=$!mytool.star("LATERALUS")
+    //    mytool.locale=$!mytool.locale
+    //    log.star=$!log.star("log overridden")
+    //    response.star=$!response.star("response overridden??")
 
-    assertEquals("** LATERALUS **", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
-            SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t", "$mytool.star(\"LATERALUS\")")));
+    // First without the tool defined, with `$!` turning null object/method references into empty string
+    Properties rendered_props = new Properties();
+    String rsp = h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"custom_tool"));
+    rendered_props.load(new StringReader(rsp));
+    // ignore mytool.locale here, as it will be the random test one
+    assertEquals("",rendered_props.getProperty("mytool.star"));
+    assertEquals("",rendered_props.getProperty("log.star"));
+    assertEquals("",rendered_props.getProperty("response.star"));
 
-    // Does $log get overridden?
-    assertEquals("** log overridden **", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
-            SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t", "$log.star(\"log overridden\")")));
+    // Now with custom tools defined:
+    rsp = h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"custom_tool",VelocityResponseWriter.LOCALE, "de_DE"));
+    rendered_props.clear();
+    rendered_props.load(new StringReader(rsp));
+    assertEquals("** LATERALUS **",rendered_props.getProperty("mytool.star"));
+    assertEquals("** log overridden **",rendered_props.getProperty("log.star"));
+    assertEquals("",rendered_props.getProperty("response.star"));
+    assertEquals("de_DE",rendered_props.getProperty("mytool.locale"));
+
+
+    // Turn off trusted configset, which disables the custom tool injection
+    h.getCoreContainer().getCoreDescriptor(h.coreName).setConfigSetTrusted(false);
+    assertFalse(h.getCoreContainer().getCoreDescriptor(coreName).isConfigSetTrusted());
+
+    rsp = h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"custom_tool",VelocityResponseWriter.LOCALE, "de_DE"));
+    rendered_props.clear();
+    rendered_props.load(new StringReader(rsp));
+    assertEquals("",rendered_props.getProperty("mytool.star"));
+    assertEquals("",rendered_props.getProperty("log.star"));
+    assertEquals("",rendered_props.getProperty("response.star"));
+    assertEquals("",rendered_props.getProperty("mytool.locale"));
+
+    // set the harness back to the default of trusted
+    h.getCoreContainer().getCoreDescriptor(h.coreName).setConfigSetTrusted(true);
 
-    // Does $response get overridden?  actual blank response because of the bang on $! reference that silences bogus $-references
-    assertEquals("", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
-        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t", "$!response.star(\"response overridden??\")")));
 
     // Custom tools can also have a SolrCore-arg constructor because they are instantiated with SolrCore.createInstance
     // TODO: do we really need to support this?  no great loss, as a custom tool could take a SolrCore object as a parameter to
     // TODO: any method, so one could do $mytool.my_method($request.core)
     // I'm currently inclined to make this feature undocumented/unsupported, as we may want to instantiate classes
     // in a different manner that only supports no-arg constructors, commented (passing) test case out
-//    assertEquals("collection1", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
-//        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t", "$mytool.core.name")));
+    //    assertEquals("collection1", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
+    //        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t", "$mytool.core.name")))
+    //           - NOTE: example uses removed inline param; convert to external template as needed
   }
 
   @Test
@@ -201,14 +254,10 @@ public class VelocityResponseWriterTest extends SolrTestCaseJ4 {
     assertEquals("Colour", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"resource_get")));
 
     // Test that $number tool uses the specified locale
-    assertEquals("2,112", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
-        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t","$number.format(2112)", VelocityResponseWriter.LOCALE, "en_US")));
-    assertEquals("2.112", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
-        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t","$number.format(2112)", VelocityResponseWriter.LOCALE, "de_DE")));
-
-    // Test that custom tool extending LocaleConfig gets the right locale
-    assertEquals("de_DE", h.query(req("q","*:*", "wt","velocityWithCustomTools",VelocityResponseWriter.TEMPLATE,"t",
-        SolrParamResourceLoader.TEMPLATE_PARAM_PREFIX+"t","$mytool.locale", VelocityResponseWriter.LOCALE, "de_DE")));
+    assertEquals("2,112", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"locale_number",
+        VelocityResponseWriter.LOCALE, "en_US")));
+    assertEquals("2.112", h.query(req("q","*:*", "wt","velocity",VelocityResponseWriter.TEMPLATE,"locale_number",
+        VelocityResponseWriter.LOCALE, "de_DE")));
   }
 
   @Test
diff --git a/solr/solr-ref-guide/src/velocity-response-writer.adoc b/solr/solr-ref-guide/src/velocity-response-writer.adoc
index 10b75d98322..ef951964752 100644
--- a/solr/solr-ref-guide/src/velocity-response-writer.adoc
+++ b/solr/solr-ref-guide/src/velocity-response-writer.adoc
@@ -16,7 +16,7 @@
 // specific language governing permissions and limitations
 // under the License.
 
-The VelocityResponseWriter is an optional plugin available in the `contrib/velocity` directory. It powers the /browse user interfaces when using configurations such as "_default", "techproducts", and "example/files".
+The VelocityResponseWriter is an optional plugin available in the `contrib/velocity` directory. It powers the /browse user interfaces when using some example configurations such as "techproducts" and "example/files".
 
 Its JAR and dependencies must be added (via `<lib>` or solr/home lib inclusion), and must be registered in `solrconfig.xml` like this:
 
@@ -34,24 +34,13 @@ Its JAR and dependencies must be added (via `<lib>` or solr/home lib inclusion),
 </queryResponseWriter>
 ----
 
-The above example shows the optional initialization and custom tool parameters used by VelocityResponseWriter; these are detailed in the following table. These initialization parameters are only specified in the writer registration in `solrconfig.xml`, not as request-time parameters. In this example, all Solr nodes should be started with `-Dvelocity.resourceloader.params.enabled=true -Dvelocity.resourceloader.enabled=true`. See further below for request-time parameters.
-
 == Configuration & Usage
 
-=== VelocityResponseWriter Startup Parameters
+=== Template Rendering Protections
 
-These are Java system properties to mitigate security risks.
-
-`velocity.resourceloader.params.enabled`::
-The "params" resource loader allows templates to be specified in Solr request parameters. For example:
-+
-[source,bash]
-http://localhost:8983/solr/gettingstarted/select?q=\*:*&wt=velocity&v.template=custom&v.template.custom=CUSTOM%3A%20%23core_name
-+
-where `v.template=custom` says to render a template called "custom" and the value of `v.template.custom` is the custom template. This is `false` by default; it'd be a niche, unusual, use case to need this enabled.
-
-`velocity.resourceloader.solr.enabled`::
-The "solr" resource loader is the only template loader registered by default. Templates are served from resources visible to the SolrResourceLoader under a `velocity/` subdirectory. The VelocityResponseWriter itself has some built-in templates (in its JAR file, under `velocity/`) that are available automatically through this loader. These built-in templates can be overridden when the same template name is in conf/velocity/ or by using the `template.base.dir` option.
+Velocity template rendering is largely controlled by the `trusted` configset flag.  Templates built into (the `/browse` ones) the component library are always available
+with this component.  In a trusted configset, templates in the `velocity/` subdirectory of the configset are renderable.  Also in a trusted configset, when `template.base.dir`
+is specified those templates are renderable.
 
 === VelocityResponseWriter Initialization Parameters
 
@@ -97,8 +86,6 @@ Locale to use with the `$resource` tool and other LocaleConfig implementing tool
 +
 Resource bundles can be added by providing a JAR file visible by the SolrResourceLoader with resource bundles under a velocity sub-directory. Resource bundles are not loadable under `conf/`, as only the class loader aspect of SolrResourceLoader can be used here.
 
-`v.template._template_name_`:: When the "params" resource loader is enabled, templates can be specified as part of the Solr request.
-
 
 === VelocityResponseWriter Context Objects
 
@@ -122,3 +109,10 @@ Resource bundles can be added by providing a JAR file visible by the SolrResourc
 |`content` |The rendered output of the main template, when rendering the layout (`v.layout.enabled=true` and `v.layout=<template>`).
 |[custom tool(s)] |Tools provided by the optional "tools" list of the VelocityResponseWriter registration are available by their specified name.
 |===
+
+=== VelocityResponseWriter Usage
+
+To see results in an HTML user interface on your own collection, try http://localhost:8983/solr/<my collection>/select?q=*:*&wt=velocity&v.template=browse&v.layout=layout
+
+Or try `/browse` in the examples techproducts or example/files.
+