Repository: jsonata-js/jsonata
Pull request #700 (2024-06-08 17:51:17)

Protect `__evaluate_entry` and `__evaluate_exit` callbacks
Fixes https://github.com/jsonata-js/jsonata/issues/695

These two internal/undocumented APIs are very useful in debugging scenarios and for imposing time/depth constraints on queries. However, this internal API should only be accessible when configuring an expression programmatically and not from inside a query. Otherwise, a query can be manipulated to remove such diagnostics or constraints. Addressing this could potentially protect consumers with certain use cases from targeted attacks.

By changing both binding keys to `Symbol`, they can no longer be accessed inside of the query since the Symbol API is not accessible there. Without access to the Symbol, a query is fundamentally unable to address these bindings.

I am building a pluggable and security-minded framework around JSONata (open source), which makes considerable use of these properties, and I want to protect them. But this is generally applicable. Many people processing queries on the server will not want a query to be able to remove these hooks.

At first, I considered increasing the API surface to do this, but then I realized that using `Symbol.for` is a super pragmatic and ergonomic way of solving this problem with minimal changes, which makes sense for something internal like this. Symbols' primary use case is implementing this kind of separation between public and internal naming, so I think this is the way to go.

Attempting to manipulate the old properties inside a query as described in https://github.com/jsonata-js/jsonata/issues/695 is now not possible. There's nothing special anymore about those references, and they work like any other reference.

I have also made the corresponding change to https://github.com/jsonata-js/jsonata-exerciser/pull/36.

I'm pinging @andrew-coleman as this is something we previously discussed over email. Also, see https://github.com/jsonata-js/jsonata/pull/701. These two small changes will unlock a whole realm of possibilities and allow me to do it safely and securely.

Signed-off-by: Adam Thomas <adam@adam-thomas.co.uk>


Patch:
diff --git a/docs/overview.md b/docs/overview.md
index a087678..598974f 100644
--- a/docs/overview.md
+++ b/docs/overview.md
@@ -26,9 +26,10 @@ The following are known implementations of JSONata in addition to the primary im
 |Go|https://github.com/blues/jsonata-go|Native implementation|1.5.4|
 |Go|https://github.com/yxuco/gojsonata|Native implementation| |
 |Java|https://github.com/IBM/JSONata4Java|Native implementation| |
-|Java|https://github.com/dashjoin/jsonata-java|Native port of reference|2.0.3|
+|Java|https://github.com/dashjoin/jsonata-java|Native port of reference|2.0.5|
 |.NET|https://github.com/mikhail-barg/jsonata.net.native|Native implementation|1.8.5|
 |Python|https://github.com/qlyoung/pyjsonata|API bindings based on C bindings|1.8.3|
+|Python|https://github.com/rayokota/jsonata-python|Native port of reference|2.0.5|
 |Rust|https://github.com/johanventer/jsonata-rust|Implementation work in progress| |
 |Rust|https://github.com/Stedi/jsonata-rs|Actively-developed fork of jsonata-rust| |
 
diff --git a/docs/processing.md b/docs/processing.md
index cc3fbdb..878a744 100644
--- a/docs/processing.md
+++ b/docs/processing.md
@@ -50,7 +50,7 @@ Stage | Syntax | Action
  __Filter__ | seq`[`expr`]` | Filter results from previous stage by applying predicate expression between brackets to each item.
  __Sort__ | seq`^(`expr`)` | Sorts (re-orders) the input sequence according to the criteria in parentheses.
  __Index__ | seq`#`$var | Binds a named variable to the current context position (zero offset) in the sequence.
- __Join__ | seq`@`$var | Binds a named variable to the current current context item in the sequence.  Can only be used directly following a map stage.
+ __Join__ | seq`@`$var | Binds a named variable to the current context item in the sequence.  Can only be used directly following a map stage.
 __Reduce__ | seq`{` expr`:`expr`,` expr`:`expr ...`}` | Group and aggregate the input sequence to a single result object as defined by the name/value expressions.  Can only appear as the final stage in a path expression.
 
 In the above table:
diff --git a/jsonata.d.ts b/jsonata.d.ts
index 257aac0..8020743 100644
--- a/jsonata.d.ts
+++ b/jsonata.d.ts
@@ -11,7 +11,26 @@ declare namespace jsonata {
   }
 
   interface ExprNode {
-    type: string;
+    type:
+        | "binary"
+        | "unary"
+        | "function"
+        | "partial"
+        | "lambda"
+        | "condition"
+        | "transform"
+        | "block"
+        | "name"
+        | "parent"
+        | "string"
+        | "number"
+        | "value"
+        | "wildcard"
+        | "descendant"
+        | "variable"
+        | "regexp"
+        | "operator"
+        | "error";
     value?: any;
     position?: number;
     arguments?: ExprNode[];
@@ -20,7 +39,7 @@ declare namespace jsonata {
     steps?: ExprNode[];
     expressions?: ExprNode[];
     stages?: ExprNode[];
-    lhs?: ExprNode[];
+    lhs?: ExprNode | ExprNode[];
     rhs?: ExprNode;
   }
 
@@ -31,8 +50,8 @@ declare namespace jsonata {
   }
 
   interface Environment {
-    bind(name: string, value: any): void;
-    lookup(name: string): any;
+    bind(name: string | symbol, value: any): void;
+    lookup(name: string | symbol): any;
     readonly timestamp: Date;
     readonly async: boolean;
   }
diff --git a/src/jsonata.js b/src/jsonata.js
index b7dfad5..19e7964 100644
--- a/src/jsonata.js
+++ b/src/jsonata.js
@@ -50,7 +50,7 @@ var jsonata = (function() {
     async function evaluate(expr, input, environment) {
         var result;
 
-        var entryCallback = environment.lookup('__evaluate_entry');
+        var entryCallback = environment.lookup(Symbol.for('jsonata.__evaluate_entry'));
         if(entryCallback) {
             await entryCallback(expr, input, environment);
         }
@@ -124,7 +124,7 @@ var jsonata = (function() {
             result = await evaluateGroupExpression(expr.group, result, environment);
         }
 
-        var exitCallback = environment.lookup('__evaluate_exit');
+        var exitCallback = environment.lookup(Symbol.for('jsonata.__evaluate_exit'));
         if(exitCallback) {
             await exitCallback(expr, input, environment, result);
         }
diff --git a/src/parser.js b/src/parser.js
index 629ff82..9cffaf3 100644
--- a/src/parser.js
+++ b/src/parser.js
@@ -1194,6 +1194,7 @@ const parser = (() => {
                             result = {type: 'apply', value: expr.value, position: expr.position};
                             result.lhs = processAST(expr.lhs);
                             result.rhs = processAST(expr.rhs);
+                            result.keepArray = result.lhs.keepArray || result.rhs.keepArray;
                             break;
                         default:
                             result = {type: expr.type, value: expr.value, position: expr.position};


Test patch:
diff --git a/test/implementation-tests.js b/test/implementation-tests.js
index 6df7e67..1e162f6 100644
--- a/test/implementation-tests.js
+++ b/test/implementation-tests.js
@@ -1057,11 +1057,11 @@ function timeboxExpression(expr, timeout, maxDepth) {
     };
 
     // register callbacks
-    expr.assign("__evaluate_entry", function() {
+    expr.assign(Symbol.for('jsonata.__evaluate_entry'), function() {
         depth++;
         checkRunnaway();
     });
-    expr.assign("__evaluate_exit", function() {
+    expr.assign(Symbol.for('jsonata.__evaluate_exit'), function() {
         depth--;
         checkRunnaway();
     });
diff --git a/test/run-test-suite.js b/test/run-test-suite.js
index d851fbf..60beee0 100644
--- a/test/run-test-suite.js
+++ b/test/run-test-suite.js
@@ -178,12 +178,12 @@ function timeboxExpression(expr, timeout, maxDepth) {
     };
 
     // register callbacks
-    expr.assign("__evaluate_entry", function(expr, input, env) {
+    expr.assign(Symbol.for('jsonata.__evaluate_entry'), function(expr, input, env) {
         if (env.isParallelCall) return;
         depth++;
         checkRunnaway();
     });
-    expr.assign("__evaluate_exit", function(expr, input, env) {
+    expr.assign(Symbol.for('jsonata.__evaluate_exit'), function(expr, input, env) {
         if (env.isParallelCall) return;
         depth--;
         checkRunnaway();
diff --git a/test/test-suite/groups/hof-map/case0010.json b/test/test-suite/groups/hof-map/case0010.json
new file mode 100644
index 0000000..8f408c5
--- /dev/null
+++ b/test/test-suite/groups/hof-map/case0010.json
@@ -0,0 +1,6 @@
+{
+    "expr": "( $square := function($x){$x*$x}; $map([1], $square)[] )",
+    "data": null,
+    "bindings": {},
+    "result": [1]
+}
diff --git a/test/test-suite/groups/hof-map/case0011.json b/test/test-suite/groups/hof-map/case0011.json
new file mode 100644
index 0000000..2bd7fa3
--- /dev/null
+++ b/test/test-suite/groups/hof-map/case0011.json
@@ -0,0 +1,6 @@
+{
+    "expr": "( $data := [1]; $square := function($x){$x*$x}; $data ~> $map($square)[] )",
+    "data": null,
+    "bindings": {},
+    "result": [1]
+}