Make setting specific to inline scripts

rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_disallow_scripts.yml

@@ -1,4 +1,4 @@
-# TODO: test also saved scripts
+#TODO: Add option to have 0 allowed scripts; cannot be encoded as `*` as that's a valid script id (and potentially also valid script).
 setup:
   - do:
       indices.create:
@@ -20,205 +20,260 @@ setup:
           - '{"index": {}}'
           - '{"some_value": 3}'
 ---
+#TODO: add stored scripts
+#TODO: add test with some allowed stored scripts + default inline scripts and vice versa
 "all scripts allowed by default":
   - requires:
       cluster_features: ["gte_v8.15.0"]
-      reason: search.aggs.allowed_metric_scripts introduced in 8.15.0
+      reason: scripted metrics agg allow list settings introduced in 8.15.0
   - do:
       search:
         index: test_index
         size: 0
         body:
           aggs:
-            some_of_values:
+            sum_of_values:
               scripted_metric:
                 init_script: 'state.transactions = []'
                 map_script: 'state.transactions.add(doc.some_value.value)'
                 combine_script: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
                 reduce_script: 'long sum = 0; for (a in states) { sum += a } return sum'
-
   - match: { hits.total.value: 3 }
-  - match: { aggregations.some_of_values.value: 6}
+  - match: { aggregations.sum_of_values.value: 6}
 
   - do:
       cluster.put_settings:
         body: >
           {
             "persistent": {
-              "search.aggs.allowed_metric_scripts": []
+              "search.aggs.allowed_inline_metric_scripts": []
             }
           }
-
   - do:
       search:
         index: test_index
         size: 0
         body:
           aggs:
-            some_of_values:
+            sum_of_values:
               scripted_metric:
                 init_script: 'state.transactions = []'
                 map_script: 'state.transactions.add(doc.some_value.value)'
                 combine_script: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
                 reduce_script: 'long sum = 0; for (a in states) { sum += a } return sum'
-
   - match: { hits.total.value: 3 }
-  - match: { aggregations.some_of_values.value: 6}
+  - match: { aggregations.sum_of_values.value: 6}
 ---
-"explicitly allowed scripts work":
+"explicitly allowed inline scripts work":
   - requires:
       cluster_features: ["gte_v8.15.0"]
-      reason: search.aggs.allowed_metric_scripts introduced in 8.15.0
+      reason: scripted metrics agg allow list settings introduced in 8.15.0
   - do:
       cluster.put_settings:
         body: >
           {
             "persistent": {
-              "search.aggs.allowed_metric_scripts": [
+              "search.aggs.allowed_inline_metric_scripts": [
                 "state.transactions = []",
                 "state.transactions.add(doc.some_value.value)",
                 "long sum = 0; for (t in state.transactions) { sum += t } return sum",
                 "long sum = 0; for (a in states) { sum += a } return sum"
               ]
             }
           }
-
   - do:
       search:
         index: test_index
         size: 0
         body:
           aggs:
-            some_of_values:
+            sum_of_values:
               scripted_metric:
                 init_script: 'state.transactions = []'
                 map_script: 'state.transactions.add(doc.some_value.value)'
                 combine_script: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
                 reduce_script: 'long sum = 0; for (a in states) { sum += a } return sum'
-
   - match: { hits.total.value: 3 }
-  - match: { aggregations.some_of_values.value: 6}
+  - match: { aggregations.sum_of_values.value: 6}
 ---
-"init_script must be allowed":
+"inline init_script must be allowed":
   - requires:
       cluster_features: ["gte_v8.15.0"]
-      reason: search.aggs.allowed_metric_scripts introduced in 8.15.0
+      reason: scripted metrics agg allow list settings introduced in 8.15.0
   - do:
       cluster.put_settings:
         body: >
           {
             "persistent": {
-              "search.aggs.allowed_metric_scripts": [
+              "search.aggs.allowed_inline_metric_scripts": [
                 "state.transactions.add(doc.some_value.value)",
                 "long sum = 0; for (t in state.transactions) { sum += t } return sum",
                 "long sum = 0; for (a in states) { sum += a } return sum"
               ]
             }
           }
-
   - do:
-      catch: '/type=illegal_argument_exception, reason=\[init_script\] contains not allowed script: \[some_of_values\]/'
+      catch: '/type=illegal_argument_exception, reason=\[init_script\] contains not allowed script: \[sum_of_values\]/'
       search:
         index: test_index
         size: 0
         body:
           aggs:
-            some_of_values:
+            sum_of_values:
               scripted_metric:
                 init_script: 'state.transactions = []'
                 map_script: 'state.transactions.add(doc.some_value.value)'
                 combine_script: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
                 reduce_script: 'long sum = 0; for (a in states) { sum += a } return sum'
 ---
-"map_script must be allowed":
+"inline map_script must be allowed":
   - requires:
       cluster_features: ["gte_v8.15.0"]
-      reason: search.aggs.allowed_metric_scripts introduced in 8.15.0
+      reason: scripted metrics agg allow list settings introduced in 8.15.0
   - do:
       cluster.put_settings:
         body: >
           {
             "persistent": {
-              "search.aggs.allowed_metric_scripts": [
+              "search.aggs.allowed_inline_metric_scripts": [
                 "state.transactions = []",
                 "long sum = 0; for (t in state.transactions) { sum += t } return sum",
                 "long sum = 0; for (a in states) { sum += a } return sum"
               ]
             }
           }
-
   - do:
-      catch: '/type=illegal_argument_exception, reason=\[map_script\] contains not allowed script: \[some_of_values\]/'
+      catch: '/type=illegal_argument_exception, reason=\[map_script\] contains not allowed script: \[sum_of_values\]/'
       search:
         index: test_index
         size: 0
         body:
           aggs:
-            some_of_values:
+            sum_of_values:
               scripted_metric:
                 init_script: 'state.transactions = []'
                 map_script: 'state.transactions.add(doc.some_value.value)'
                 combine_script: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
                 reduce_script: 'long sum = 0; for (a in states) { sum += a } return sum'
 ---
-"combine_script must be allowed":
+"inline combine_script must be allowed":
   - requires:
       cluster_features: ["gte_v8.15.0"]
-      reason: search.aggs.allowed_metric_scripts introduced in 8.15.0
+      reason: scripted metrics agg allow list settings introduced in 8.15.0
   - do:
       cluster.put_settings:
         body: >
           {
             "persistent": {
-              "search.aggs.allowed_metric_scripts": [
+              "search.aggs.allowed_inline_metric_scripts": [
                 "state.transactions = []",
                 "state.transactions.add(doc.some_value.value)",
                 "long sum = 0; for (a in states) { sum += a } return sum"
               ]
             }
           }
-
   - do:
-      catch: '/type=illegal_argument_exception, reason=\[combine_script\] contains not allowed script: \[some_of_values\]/'
+      catch: '/type=illegal_argument_exception, reason=\[combine_script\] contains not allowed script: \[sum_of_values\]/'
       search:
         index: test_index
         size: 0
         body:
           aggs:
-            some_of_values:
+            sum_of_values:
               scripted_metric:
                 init_script: 'state.transactions = []'
                 map_script: 'state.transactions.add(doc.some_value.value)'
                 combine_script: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
                 reduce_script: 'long sum = 0; for (a in states) { sum += a } return sum'
 ---
-"reduce_script must be allowed":
+"inline reduce_script must be allowed":
   - requires:
       cluster_features: ["gte_v8.15.0"]
-      reason: search.aggs.allowed_metric_scripts introduced in 8.15.0
+      reason: scripted metrics agg allow list settings introduced in 8.15.0
   - do:
       cluster.put_settings:
         body: >
           {
             "persistent": {
-              "search.aggs.allowed_metric_scripts": [
+              "search.aggs.allowed_inline_metric_scripts": [
                 "state.transactions = []",
                 "state.transactions.add(doc.some_value.value)",
                 "long sum = 0; for (t in state.transactions) { sum += t } return sum"
               ]
             }
           }
-
   - do:
-      catch: '/type=illegal_argument_exception, reason=\[reduce_script\] contains not allowed script: \[some_of_values\]/'
+      catch: '/type=illegal_argument_exception, reason=\[reduce_script\] contains not allowed script: \[sum_of_values\]/'
       search:
         index: test_index
         size: 0
         body:
           aggs:
-            some_of_values:
+            sum_of_values:
               scripted_metric:
                 init_script: 'state.transactions = []'
                 map_script: 'state.transactions.add(doc.some_value.value)'
                 combine_script: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
                 reduce_script: 'long sum = 0; for (a in states) { sum += a } return sum'
+---
+"allowed inline scripts do not affect allowed stored scripts":
+  - requires:
+      cluster_features: ["gte_v8.15.0"]
+      reason: scripted metrics agg allow list settings introduced in 8.15.0
+  - do:
+      cluster.put_settings:
+        body: >
+          {
+            "persistent": {
+              "search.aggs.allowed_inline_metric_scripts": [
+                "foobar"
+              ]
+            }
+          }
+  - do:
+      put_script:
+        id: init
+        body:
+          script:
+            source: 'state.transactions = []'
+            lang: painless
+  - do:
+      put_script:
+        id: map
+        body:
+          script:
+            source: 'state.transactions.add(doc.some_value.value)'
+            lang: painless
+  - do:
+      put_script:
+        id: combine
+        body:
+          script:
+            source: 'long sum = 0; for (t in state.transactions) { sum += t } return sum'
+            lang: painless
+  - do:
+      put_script:
+        id: reduce
+        body:
+          script:
+            source: 'long sum = 0; for (a in states) { sum += a } return sum'
+            lang: painless
+
+  - do:
+      search:
+        index: test_index
+        size: 0
+        body:
+          aggs:
+            sum_of_values:
+              scripted_metric:
+                init_script:
+                  id: init
+                map_script:
+                  id: map
+                combine_script:
+                  id: combine
+                reduce_script:
+                  id: reduce
+  - match: { hits.total.value: 3 }
+  - match: { aggregations.sum_of_values.value: 6}

server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java

@@ -436,7 +436,6 @@ public void apply(Settings value, Settings current, Settings previous) {
         ScriptService.SCRIPT_MAX_SIZE_IN_BYTES,
         ScriptService.TYPES_ALLOWED_SETTING,
         ScriptService.CONTEXTS_ALLOWED_SETTING,
-        SearchModule.SCRIPTED_METRICS_AGG_ALLOWED_SCRIPTS_SETTING,
         IndicesService.INDICES_CACHE_CLEAN_INTERVAL_SETTING,
         IndicesFieldDataCache.INDICES_FIELDDATA_CACHE_SIZE_KEY,
         IndicesFieldDataCache.INDICES_FIELDDATA_CACHE_EXPIRE,
@@ -516,6 +515,7 @@ public void apply(Settings value, Settings current, Settings previous) {
         ResourceWatcherService.RELOAD_INTERVAL_LOW,
         SearchModule.INDICES_MAX_CLAUSE_COUNT_SETTING,
         SearchModule.INDICES_MAX_NESTED_DEPTH_SETTING,
+        SearchModule.SCRIPTED_METRICS_AGG_ALLOWED_INLINE_SCRIPTS,
         SearchService.SEARCH_WORKER_THREADS_ENABLED,
         SearchService.QUERY_PHASE_PARALLEL_COLLECTION_ENABLED,
         ThreadPool.ESTIMATED_TIME_INTERVAL_SETTING,

server/src/main/java/org/elasticsearch/search/SearchModule.java

@@ -293,8 +293,8 @@ public class SearchModule {
         Setting.Property.NodeScope
     );
 
-    public static final Setting<List<String>> SCRIPTED_METRICS_AGG_ALLOWED_SCRIPTS_SETTING = Setting.stringListSetting(
-        "search.aggs.allowed_metric_scripts",
+    public static final Setting<List<String>> SCRIPTED_METRICS_AGG_ALLOWED_INLINE_SCRIPTS = Setting.stringListSetting(
+        "search.aggs.allowed_inline_metric_scripts",
         Setting.Property.NodeScope,
         Setting.Property.Dynamic
     );

server/src/main/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricAggregationBuilder.java

@@ -13,6 +13,7 @@
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.common.io.stream.StreamOutput;
 import org.elasticsearch.script.Script;
+import org.elasticsearch.script.ScriptType;
 import org.elasticsearch.script.ScriptedMetricAggContexts;
 import org.elasticsearch.search.SearchModule;
 import org.elasticsearch.search.aggregations.AbstractAggregationBuilder;
@@ -183,12 +184,12 @@ public BucketCardinality bucketCardinality() {
     protected ScriptedMetricAggregatorFactory doBuild(AggregationContext context, AggregatorFactory parent, Builder subfactoriesBuilder)
         throws IOException {
 
-        List<String> allowedScripts = context.getClusterSettings().get(SearchModule.SCRIPTED_METRICS_AGG_ALLOWED_SCRIPTS_SETTING);
+        List<String> allowedInlineScripts = context.getClusterSettings().get(SearchModule.SCRIPTED_METRICS_AGG_ALLOWED_INLINE_SCRIPTS);
 
-        validateScript(INIT_SCRIPT_FIELD.getPreferredName(), name, initScript, allowedScripts);
-        validateScript(MAP_SCRIPT_FIELD.getPreferredName(), name, mapScript, allowedScripts);
-        validateScript(COMBINE_SCRIPT_FIELD.getPreferredName(), name, combineScript, allowedScripts);
-        validateScript(REDUCE_SCRIPT_FIELD.getPreferredName(), name, reduceScript, allowedScripts);
+        validateScript(INIT_SCRIPT_FIELD.getPreferredName(), name, initScript, allowedInlineScripts);
+        validateScript(MAP_SCRIPT_FIELD.getPreferredName(), name, mapScript, allowedInlineScripts);
+        validateScript(COMBINE_SCRIPT_FIELD.getPreferredName(), name, combineScript, allowedInlineScripts);
+        validateScript(REDUCE_SCRIPT_FIELD.getPreferredName(), name, reduceScript, allowedInlineScripts);
 
         if (combineScript == null) {
             throw new IllegalArgumentException("[combineScript] must not be null: [" + name + "]");
@@ -240,9 +241,9 @@ protected ScriptedMetricAggregatorFactory doBuild(AggregationContext context, Ag
         );
     }
 
-    private static void validateScript(String scriptName, String aggName, Script script, List<String> allowedScripts) {
-        if (script != null && allowedScripts.isEmpty() == false) {
-            if (allowedScripts.contains(script.getIdOrCode()) == false) {
+    private static void validateScript(String scriptName, String aggName, Script script, List<String> allowedInlineScripts) {
+        if (script != null && allowedInlineScripts.isEmpty() == false) {
+            if (script.getType().equals(ScriptType.INLINE) && allowedInlineScripts.contains(script.getIdOrCode()) == false) {
                 throw new IllegalArgumentException("[" + scriptName + "] contains not allowed script: [" + aggName + "]");
             }
         }