fix(formula,plugin-security): the cross-class field-comparison refusal leads with its remedy, so REST callers read the fix (#20869)

.changeset/20869-cross-class-refusal-remedy-first.md

@@ -0,0 +1,20 @@
+---
+'@objectstack/formula': patch
+'@objectstack/plugin-security': patch
+---
+
+fix(formula,plugin-security): the refusal of a field-to-field comparison across comparison classes now leads with its remedy, so the remedy reaches REST callers (#20869)
+
+Clause-②: no
+
+A row-level policy that compares two fields of no shared comparison class (text against a number, or any field against a file field, a formula field, or a field that holds a list or an object) is refused with `INVALID_FILTER` / 400. The REST door keeps a 4xx message under 500 characters by cutting it to its first 499 characters plus an ellipsis. Both messages for this refusal put the remedy last, so the remedy was always cut off, and a caller read the diagnosis but never the fix:
+
+- The record matcher's message (`@objectstack/formula`, raised by the RLS write check on an insert or update through `/data`) was 972 characters, with the remedy starting at character 825.
+- The explain engine's message (`@objectstack/plugin-security`, answered by `GET` / `POST /api/v1/security/explain`) put the remedy after the policy names and the diagnostic. Those have no length limit, so the message was 601 characters with a short policy name and longer with longer names.
+
+Both messages now start with the remedy. It is the same sentence as before and has only moved:
+
+- The record matcher's message is 494 characters and reaches the wire whole. In order it says: the remedy; that the two columns share no class, and which classes exist; why the comparison is refused; and why the columns are not named. It still names no column, operator or policy; the server log names them.
+- The explain engine's message starts with the remedy, then names the policy and both columns, then gives the reason. Whatever the names' length, the remedy sits in the first 125 characters. With long names the REST door may cut the reason at the end.
+
+Unchanged: the error code (`INVALID_FILTER`), the status (400), which comparisons are refused, the refusal a find answers with (driver-sql's read refusal, 383 characters, which already reached the wire whole), and every other refusal.

packages/formula/src/matches-filter-cross-field-class.test.ts

@@ -161,6 +161,31 @@ describe('matchesFilterCondition — a field compared with a field of no shared
     expect(crossFieldClassRefusalCarriedBy(new Error('x'))).toBeNull();
   });
 
+  it('leads with its remedy and fits the REST client-message bound whole, however long the column names', () => {
+    // The REST door cuts a 4xx message of 500 characters or more to 499 plus an
+    // ellipsis (`CLIENT_MESSAGE_MAX`, `@objectstack/rest`): it keeps the HEAD.
+    const remedy =
+      'In a row-level policy, compare a field only with a field of the same class, or fix the declaration of ' +
+      'the one that is declared with the wrong type.';
+    const long = (stem: string) => `${stem}_${'x'.repeat(120)}`;
+    const longFields = { [long('stage')]: { type: 'text' }, [long('amount')]: { type: 'number' } };
+    const shortErr = refusalOf({ status: { $ne: { $field: 'amount' } } })!;
+    let longErr: WireBearingError | null = null;
+    try {
+      matchesFilterCondition({}, { [long('stage')]: { $ne: { $field: long('amount') } } } as never, { fields: longFields });
+    } catch (e) {
+      longErr = e as WireBearingError;
+    }
+    expect({ code: longErr?.code, status: longErr?.status }).toEqual({ code: 'INVALID_FILTER', status: 400 });
+    // It names no column, so its length does not depend on theirs.
+    expect(longErr?.message).toBe(shortErr.message);
+    expect(shortErr.message.startsWith(remedy)).toBe(true);
+    expect(shortErr.message.length).toBeLessThan(500);
+    // After the remedy: what is refused, why, and why the columns are withheld.
+    const at = (s: string) => shortErr.message.indexOf(s);
+    expect([at('share no class'), at('so it is refused'), at('withheld')].every((i, n, a) => i > remedy.length && (n === 0 || i > a[n - 1]))).toBe(true);
+  });
+
   it('findCrossFieldClassRefusal answers null for a filter whose comparisons all compare', () => {
     expect(findCrossFieldClassRefusal({ $and: [{ status: { $eq: { $field: 'title' } } }, { amount: { $lt: { $field: 'budget' } } }] }, FIELDS)).toBeNull();
     expect(findCrossFieldClassRefusal({ amount: { $lt: { $field: 'status' } } }, FIELDS)).toMatchObject({

packages/formula/src/matches-filter.ts

@@ -507,19 +507,23 @@ const CROSS_FIELD_CLASS_REFUSAL = Symbol.for('objectstack.formula.crossFieldClas
  * — driver-sql withholds the same comparison's columns on the read for the
  * same reason (#7929). The columns, the operator and both declarations travel
  * on the error for the server log ({@link crossFieldClassRefusalCarriedBy}).
+ *
+ * The remedy leads, and the whole message stays under the REST door's client
+ * message bound (`CLIENT_MESSAGE_MAX` in `@objectstack/rest`: a 4xx message of
+ * 500 characters or more is cut to 499 plus an ellipsis). The bound cuts the
+ * TAIL, so a remedy written last never reached the wire. The order is: the
+ * remedy; what is refused (two columns with no shared class, and the classes);
+ * why it is refused; why the columns are withheld. The text is fixed, so its
+ * length is too — a sentence added here must be paid for by a shorter one.
  */
 function crossFieldClassError(refusal: CrossFieldClassRefusal): Error {
   const err = new Error(
-    'A field-to-field comparison ({ "$field": … }) in this filter compares two columns that share no ' +
-      'comparison class. Two columns are compared only within one class — a number with a number, text ' +
-      'with text, a boolean with a boolean, a date with a date, a datetime with a datetime, a time of day ' +
-      'with a time of day — and a file field, a formula field, or a column that holds a list or an object ' +
-      'has no class at all, so the platform defines no answer for this comparison. It is refused rather ' +
-      'than evaluated: across classes SQL and this evaluator answer differently, and the read path refuses ' +
-      'the same comparison, so an answer here would give one access policy two meanings. The columns and ' +
-      'the operator are withheld from this message because the filter may be an access policy the caller ' +
-      'did not write; the server log names them. In a row-level policy, compare a field only with a field ' +
-      'of the same class, or fix the declaration of the one that is declared with the wrong type.',
+    'In a row-level policy, compare a field only with a field of the same class, or fix the declaration ' +
+      'of the one that is declared with the wrong type. This filter compares two columns that share no ' +
+      'class (number, text, boolean, date, datetime, time; file, formula, list and object fields have ' +
+      'none). SQL and this evaluator answer it differently, so it is refused, as on the read path. The ' +
+      'columns and operator are withheld, as the caller may not have written the policy; the server log ' +
+      'names them.',
   ) as Error & { code?: string; status?: number };
   err.code = StandardErrorCode.enum.INVALID_FILTER;
   err.status = 400;

packages/plugins/plugin-security/src/explain-cross-class-refusal.test.ts

@@ -180,15 +180,26 @@ const ROW = { id: 'r1', status: 'open', title: 'x', amount: 5 };
 
 const rlsRecordOf = (d: ExplainDecision) => d.layers.find((l) => l.layer === 'rls')?.record;
 
+/**
+ * The remedy explain's refusal leads with. It comes before the policy names and
+ * the diagnostic, which have no length bound, because the REST door keeps only
+ * a long message's first 499 characters.
+ */
+const REMEDY =
+  'Compare a field only with a field of the same class, or fix the declaration of the one that is declared with ' +
+  'the wrong type.';
+
 /**
  * Explain's answer is the find's refusal: the same envelope, no decision and so
- * no record verdict, and a message that names the policy and both columns.
+ * no record verdict, and a message that leads with the remedy and names the
+ * policy and both columns.
  */
 async function expectExplainRefuses(p: Promise<unknown>, columns: [string, string]): Promise<void> {
   const r = await refusalOf(p);
   expect(r).not.toBe('answered');
   if (r === 'answered') return;
   expect({ code: r.code, status: r.status }).toEqual(INVALID);
+  expect(r.message.startsWith(`${REMEDY} `), r.message).toBe(true);
   expect(r.message).toContain(`'${POLICY}'`);
   for (const column of columns) expect(r.message).toContain(`"${column}"`);
 }

packages/plugins/plugin-security/src/explain-engine.ts

@@ -923,6 +923,14 @@ function refusedPolicyNamesOf(
  * same caller for the same object publishes the same predicate: `readFilter`
  * without a `recordId`, the `rls` layer's `rowFilter` with one. So naming the
  * policy and its two columns here discloses nothing that report does not.
+ *
+ * The remedy leads, BEFORE the subject. The REST door bounds a 4xx message
+ * (`CLIENT_MESSAGE_MAX` in `@objectstack/rest`: 500 characters or more is cut
+ * to 499 plus an ellipsis), and the subject and the diagnostic have no length
+ * bound: object, field and policy names declare no maximum, and the subject
+ * lists every refused policy. So no subject-first order can keep a trailing
+ * remedy on the wire for every policy; at index 0 it survives any length. The
+ * reason comes last and is the part a long subject may cut.
  */
 function crossFieldRefusalForExplain(
   cause: unknown,
@@ -939,10 +947,10 @@ function crossFieldRefusalForExplain(
     : `The row-level security ${policies.length === 1 ? 'policy' : 'policies'} ` +
       `${policies.map((p) => `'${p}'`).join(', ')} on '${object}'`;
   const err = new Error(
-    `${subject} cannot be evaluated: ${refusal.diagnostic}. Enforcement refuses every request this filter ` +
-      'scopes instead of judging a record (the find answers INVALID_FILTER / 400), so explain answers with the ' +
-      'same refusal and reports no verdict. Compare a field only with a field of the same class, or fix ' +
-      'the declaration of the one that is declared with the wrong type.',
+    'Compare a field only with a field of the same class, or fix the declaration of the one that is ' +
+      `declared with the wrong type. ${subject} cannot be evaluated: ${refusal.diagnostic}. Enforcement ` +
+      'refuses every request this filter scopes (the find answers INVALID_FILTER / 400), so explain answers ' +
+      'with the same refusal and reports no verdict.',
   );
   const { code, status } = cause as { code?: string; status?: number };
   return Object.assign(err, { code, status, cause });

packages/plugins/plugin-security/src/rls-check-cross-class-field-refused.test.ts

@@ -204,7 +204,8 @@ for (const [driverName, makeDriver, available] of DRIVERS) {
       it(`${c.id} \`${c.predicate}\` — the 400 names neither column; the server log names the policy and both`, async () => {
         const w = await boot(makeDriver, 'check', c.predicate);
         const message = await messageOf(w.engine.insert(w.OBJ, NEW, { context: w.caller } as never));
-        expect(message).toMatch(/^A field-to-field comparison/);
+        // The remedy leads: the REST door cuts a long message's tail, never its head.
+        expect(message).toMatch(/^In a row-level policy, compare a field only with a field of the same class/);
         for (const column of c.columns) expect(message).not.toContain(column);
 
         const lines = w.refusalLines();