diff --git a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_disallow_scripts.yml b/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_disallow_scripts.yml index c8b4f480d45a5..b7859855a6d6e 100644 --- a/rest-api-spec/src/yamlRestTest/resources/rest-api-spec/test/search/330_disallow_scripts.yml +++ b/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,61 +20,60 @@ 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", @@ -82,143 +81,199 @@ setup: ] } } - - 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} diff --git a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java b/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java index 7acea979338eb..b02037f2bd729 100644 --- a/server/src/main/java/org/elasticsearch/common/settings/ClusterSettings.java +++ b/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, diff --git a/server/src/main/java/org/elasticsearch/search/SearchModule.java b/server/src/main/java/org/elasticsearch/search/SearchModule.java index 9cc14b6f78e77..38db8bc1bb107 100644 --- a/server/src/main/java/org/elasticsearch/search/SearchModule.java +++ b/server/src/main/java/org/elasticsearch/search/SearchModule.java @@ -293,8 +293,8 @@ public class SearchModule { Setting.Property.NodeScope ); - public static final Setting> SCRIPTED_METRICS_AGG_ALLOWED_SCRIPTS_SETTING = Setting.stringListSetting( - "search.aggs.allowed_metric_scripts", + public static final Setting> SCRIPTED_METRICS_AGG_ALLOWED_INLINE_SCRIPTS = Setting.stringListSetting( + "search.aggs.allowed_inline_metric_scripts", Setting.Property.NodeScope, Setting.Property.Dynamic ); diff --git a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricAggregationBuilder.java b/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricAggregationBuilder.java index 30947648c166f..e777fbe558442 100644 --- a/server/src/main/java/org/elasticsearch/search/aggregations/metrics/ScriptedMetricAggregationBuilder.java +++ b/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 allowedScripts = context.getClusterSettings().get(SearchModule.SCRIPTED_METRICS_AGG_ALLOWED_SCRIPTS_SETTING); + List 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 allowedScripts) { - if (script != null && allowedScripts.isEmpty() == false) { - if (allowedScripts.contains(script.getIdOrCode()) == false) { + private static void validateScript(String scriptName, String aggName, Script script, List 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 + "]"); } }