From 887717bb15124ec14c38b39123922274a4188fe5 Mon Sep 17 00:00:00 2001
From: Alex Hall <alex.mojaki@gmail.com>
Date: Mon, 27 Nov 2023 21:01:41 +0200
Subject: [PATCH] (core) Decode cell values to prevent working around rule
 using 'in' on lists

Summary:
Fixes bug described in https://grist.slack.com/archives/C069RUP71/p1699643458649019

Decodes cell values obtained from `InfoView.get` when evaluating user-defined ACL formulas, i.e. the result of `rec.foo` in such a formula. In particular this is so that `rec.some_list` loses the leading `L` type code and behaves sensibly in an expression like `thing in rec.some_list`.

`InfoView.get` is called in many places, but for every usage I found other than here, leaving the cell values encoded was best.

Test Plan: Added two unit server tests. The first is for the main bug involving lists. The second checks the only other plausible way I could think of that this change affects behaviour, and it seems to be for the better since both tests failed before. Most operations involving non-primitive cell values don't do anything sensible with or without decoding, so behaviour shouldn't change meaningfully in those cases.

Reviewers: georgegevoian, paulfitz

Reviewed By: georgegevoian, paulfitz

Subscribers: paulfitz

Differential Revision: https://phab.getgrist.com/D4123
---
 app/server/lib/ACLFormula.ts  |  3 ++-
 test/server/lib/ACLFormula.ts | 45 +++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+), 1 deletion(-)

diff --git a/app/server/lib/ACLFormula.ts b/app/server/lib/ACLFormula.ts
index d1dd13ea..41a8deca 100644
--- a/app/server/lib/ACLFormula.ts
+++ b/app/server/lib/ACLFormula.ts
@@ -11,6 +11,7 @@
 import {CellValue} from 'app/common/DocActions';
 import {ErrorWithCode} from 'app/common/ErrorWithCode';
 import {AclMatchFunc, AclMatchInput, ParsedAclFormula} from 'app/common/GranularAccessClause';
+import {decodeObject} from "app/plugin/objtypes";
 import constant = require('lodash/constant');
 
 const GRIST_CONSTANTS: Record<string, string> = {
@@ -94,7 +95,7 @@ function getAttr(value: any, attrName: string, valueNode: ParsedAclFormula): any
     }
     throw new Error(`No value for '${describeNode(valueNode)}'`);
   }
-  return (typeof value.get === 'function' ? value.get(attrName) : // InfoView
+  return (typeof value.get === 'function' ? decodeObject(value.get(attrName)) : // InfoView
           value[attrName]);
 }
 
diff --git a/test/server/lib/ACLFormula.ts b/test/server/lib/ACLFormula.ts
index 1f913276..4b4ac634 100644
--- a/test/server/lib/ACLFormula.ts
+++ b/test/server/lib/ACLFormula.ts
@@ -1,5 +1,6 @@
 import {CellValue} from 'app/common/DocActions';
 import {AclMatchFunc, InfoView} from 'app/common/GranularAccessClause';
+import {GristObjCode} from 'app/plugin/GristData';
 import {compileAclFormula} from 'app/server/lib/ACLFormula';
 import {makeExceptionalDocSession} from 'app/server/lib/DocSession';
 import {User} from 'app/server/lib/GranularAccess';
@@ -193,4 +194,48 @@ describe('ACLFormula', function() {
     assert.equal(compiled({user, rec: V({emails: null})}), true);
     assert.equal(compiled({user, rec: V({emails: 'X@'})}), false);
   });
+
+  it('should decode cell values so that "in" is safe to use with lists', async function () {
+    const user = new User({Email: 'L'});
+
+    // A previous bug meant that the above user would always pass this formula,
+    // because an encoded list always starts with the 'L' type code,
+    // and encoded cell values were used in evaluating formulas.
+    let compiled = await setAndCompile('user.Email in rec.emails');
+    assert.equal(compiled({user, rec: V({emails: [GristObjCode.List]})}), false);
+    assert.equal(compiled({user, rec: V({emails: [GristObjCode.List, "X"]})}), false);
+    assert.equal(compiled({user, rec: V({emails: [GristObjCode.List, "L"]})}), true);
+
+    // This should never happen (nothing should be encoded as an empty list),
+    // this just shows what would happen.
+    assert.throws(() => compiled({user, rec: V({emails: [] as any})}),
+      /\.includes is not a function/);
+
+    // List literals aren't decoded and work as expected.
+    compiled = await setAndCompile('user.Email in []');
+    assert.equal(compiled({user, rec: V({})}), false);
+
+    compiled = await setAndCompile('user.Email in ["X"]');
+    assert.equal(compiled({user, rec: V({})}), false);
+
+    compiled = await setAndCompile('user.Email in ["L"]');
+    assert.equal(compiled({user, rec: V({})}), true);
+  });
+
+  it('should allow comparing dates', async function () {
+    const user = new User({});
+
+    const compiled = await setAndCompile('rec.date1 < rec.date2');
+    for (let i = 0; i < 150; i++) {
+      const date1 = i * 10000000000;
+      for (let j = 0; j < 150; j++) {
+        const date2 = j * 10000000000;
+        const rec = V({
+          date1: [GristObjCode.Date, date1],
+          date2: [GristObjCode.Date, date2],
+        });
+        assert.equal(compiled({user, rec}), date1 < date2);
+      }
+    }
+  });
 });