Painless: ensure type "UnmodifiableMap" for params (opensearch-projec…

…t#13885)

Signed-off-by: Rohit Ashiwal <[email protected]>

CHANGELOG.md

@@ -48,6 +48,7 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 - Allow clearing `remote_store.compatibility_mode` setting ([#13646](https://github.com/opensearch-project/OpenSearch/pull/13646))
 - Fix ReplicaShardBatchAllocator to batch shards without duplicates ([#13710](https://github.com/opensearch-project/OpenSearch/pull/13710))
 - Don't return negative scores from `multi_match` query with `cross_fields` type  ([#13829](https://github.com/opensearch-project/OpenSearch/pull/13829))
+- Painless: ensure type "UnmodifiableMap" for params ([#13885](https://github.com/opensearch-project/OpenSearch/pull/13885))
 - Pass parent filter to inner hit query ([#13903](https://github.com/opensearch-project/OpenSearch/pull/13903))
 - Fix NPE on restore searchable snapshot ([#13911](https://github.com/opensearch-project/OpenSearch/pull/13911))
 

modules/lang-painless/src/test/java/org/opensearch/painless/WhenThingsGoWrongTests.java

@@ -354,6 +354,9 @@ public void testInvalidAssignment() {
         assertEquals(iae.getMessage(), "invalid assignment: cannot assign a value to addition operation [+]");
         iae = expectScriptThrows(IllegalArgumentException.class, () -> exec("Double.x() = 1;"));
         assertEquals(iae.getMessage(), "invalid assignment: cannot assign a value to method call [x/0]");
+
+        expectScriptThrows(UnsupportedOperationException.class, () -> exec("params['modifyingParamsMap'] = 2;"));
+        expectScriptThrows(UnsupportedOperationException.class, () -> exec("params.modifyingParamsMap = 2;"));
     }
 
     public void testCannotResolveSymbol() {

modules/lang-painless/src/yamlRestTest/resources/rest-api-spec/test/painless/17_update_error.yml

@@ -13,3 +13,50 @@
   - match: { error.root_cause.0.position.offset: 13 }
   - match: { error.root_cause.0.position.start: 0 }
   - match: { error.root_cause.0.position.end: 38 }
+
+---
+"Test modifying params map from script leads to exception":
+  - skip:
+      features: "node_selector"
+
+  - do:
+      put_script:
+        id: "except"
+        body: {"script": {"lang": "painless", "source": "params.that = 3"}}
+
+  - do:
+      indices.create:
+        index: "test"
+        body:
+          settings:
+            index:
+              number_of_shards: 1
+              number_of_replicas: 0
+          mappings:
+            properties:
+              this:
+                type: "integer"
+              that:
+                type: "integer"
+
+  - do:
+      index:
+        index: "test"
+        id: 1
+        body: {"this": 1, "that": 2}
+
+  - do:
+      catch: /unsupported_operation_exception/
+      node_selector:
+        version: "2.15.0 - "
+      update:
+        index: "test"
+        id: 1
+        body:
+          script:
+            id: "except"
+            params: {"this": 2}
+
+  - match: { error.caused_by.position.offset: 6 }
+  - match: { error.caused_by.position.start: 0 }
+  - match: { error.caused_by.position.end: 15 }

server/src/main/java/org/opensearch/script/Script.java

@@ -589,7 +589,7 @@ public Script(StreamInput in) throws IOException {
         @SuppressWarnings("unchecked")
         Map<String, String> options = (Map<String, String>) (Map) in.readMap();
         this.options = options;
-        this.params = in.readMap();
+        this.params = Collections.unmodifiableMap(in.readMap());
     }
 
     @Override

server/src/main/java/org/opensearch/script/UpdateScript.java

@@ -32,6 +32,7 @@
 
 package org.opensearch.script;
 
+import java.util.Collections;
 import java.util.Map;
 
 /**
@@ -53,7 +54,7 @@ public abstract class UpdateScript {
     private final Map<String, Object> ctx;
 
     public UpdateScript(Map<String, Object> params, Map<String, Object> ctx) {
-        this.params = params;
+        this.params = Collections.unmodifiableMap(params);
         this.ctx = ctx;
     }