Skip to content

Commit 05be352

Browse files
fix(formula,plugin-security): the cross-class field-comparison refusal leads with its remedy, so REST callers read the fix (#20869) (#20972)
Fixes #20869 Clause-②: no ## What this changes The refusal of a field-to-field comparison across comparison classes (`INVALID_FILTER` / 400) now opens with its remedy, on both of its producers. The REST door bounds a 4xx message by cutting its tail (`CLIENT_MESSAGE_MAX = 500`: 499 characters plus an ellipsis). Both producers wrote the remedy last, so no caller ever read it. The bound is not touched; that is #5423's decision. The producers change. - **`packages/formula/src/matches-filter.ts`, `crossFieldClassError`.** The remedy sentence comes first, byte-identical to the one the message ended with. The diagnosis is shortened so the whole message is 494 characters and reaches the wire whole. Order: the remedy; what is refused (two columns with no shared class, and the classes); why it is refused; why the columns are withheld. It still names no column, operator or policy; the columns travel on the error's symbol key for the server log only. - **`packages/plugins/plugin-security/src/explain-engine.ts`, `crossFieldRefusalForExplain`.** The remedy sentence, also byte-identical, moves before the subject and the diagnostic. Those have no length bound: object, field and policy names declare no maximum (`SnakeCaseIdentifierSchema`, the object `name` and the RLS policy `name` are regex-only), and the subject lists every refused policy. So no subject-first order keeps a trailing remedy for every policy; at index 0 it survives any length. The reason drops one redundant clause ("instead of judging a record": the same sentence already says explain "reports no verdict"). Code, status and trigger are unchanged. Only the text moves and shortens. ## Measured through the real handlers ObjectQL on driver-sql (better-sqlite3), `SecurityPlugin`, `RestServer` route handlers, and a policy `record.status != record.amount`. "Remedy at" is the index where the remedy sentence starts. | door | producer | before (`013f97df93`) | after | |---|---|---|---| | `POST /data/:object` (insert: the RLS write check) | record matcher | thrown 972, remedy at 825; wire 500 with ellipsis, **remedy absent** | thrown 494 = wire 494, no ellipsis, remedy at 0 | | `GET /data/:object` (find) | driver-sql's read refusal | 383, whole, no ellipsis | unchanged | | `POST /security/explain`, short names | explain engine | thrown 601, remedy at 477; wire 500, **remedy absent** | thrown 573, remedy at 0; wire 500, cut in the reason | | `POST /security/explain`, 60-character names | explain engine | thrown 920, remedy at 796; wire 500, **remedy absent** | thrown 892, remedy at 0; wire 500 | | control: `GET /data/:object` with `{ title: { $bogus: 1 } }` | driver-sql, another class | 228, wire equals thrown | unchanged | **A find never carries the matcher's message.** Only the RLS write check (`security-plugin.ts`, `satisfiesCheck`) and explain (`matchUnderDeclaredColumns`) hand `matchesFilterCondition` the declared columns its class rule reads; the other runtime caller (`objectql` `having-filter.ts`) passes none. On `/data` a find answers driver-sql's own read refusal of the same comparison, 383 characters, which already reached the wire whole and states the rule ("compared as the same type class"). The matcher's text reaches `/data` on an insert or an update. So the `/data` pin covers both: the insert carries the matcher's remedy, and the find carries its read refusal whole. ## Pins - `packages/rest/src/cross-class-refusal-remedy-on-the-wire.test.ts` (new, the real stack, both envelopes this family speaks): - `/data` insert: the wire message starts with the matcher's remedy, is under the bound, equals the thrown message, and names neither column; - `/data` find: the read refusal reaches the wire whole and states the same-class rule; - `POST /security/explain`: the wire message starts with the explain remedy, is within the bound, and names the policy; - long names (object, policy and fields about 120 characters each): explain is cut at exactly 500 with the remedy intact, and the matcher's message is identical to the short fixture's; - control: a short refusal of another class (unsupported operator, 228 characters) reaches the wire equal to the thrown message. - `packages/formula/src/matches-filter-cross-field-class.test.ts`: the message starts with the remedy, is under 500, is the same for long column names, and keeps the clause order. - `packages/plugins/plugin-security/src/explain-cross-class-refusal.test.ts`: every refused cell (5 predicates, read / update / delete, SQLite and sqlite-wasm) asserts the message starts with the remedy. - `packages/plugins/plugin-security/src/rls-check-cross-class-field-refused.test.ts`: the pinned opening moves with the text. **Red, then green.** The new REST pin reads the producers through their `dist/` (the `rest` to `plugin-security` pair is unaliased and registered in `check:test-source-alias`). At `2cfaa6522f`, against producer builds from BASE `013f97df93` (new-text markers: 0 in both dists): 3 failed (insert, explain, long names) and 2 passed (find, control). After rebuilding both producers from `2cfaa6522f` (markers: 1 in each): 5 passed. ## Verification (head `dc440bff6b`, after merging `origin/main` at `f6ccca4a44`) - **Package suites at `5fde18e296`** (before the merge): - `@objectstack/formula`: `test` 42 files, 1241 passed; `typecheck` OK, test-layer debt held; - `@objectstack/plugin-security`: 149 files, 3227 passed, 23 skipped (the PostgreSQL legs, no server); `typecheck` OK; - `@objectstack/rest`: `--project local` 245 files, 4875 passed, 114 skipped; `typecheck` OK, 0 test-layer errors. - **At `dc440bff6b`** (the merge brought commits into `rest`): the formula pin file 23 passed; the three plugin-security cross-class files 150 passed, 23 skipped; `@objectstack/rest` `--project local` 246 files, 4890 passed, 114 skipped. - **Gates.** `node scripts/pm/dispatch-gates.mjs --commands` at `dc440bff6b` derives 65 commands; all 65 exit 0. `--ran` reconciliation: "65 derived, 65 run, 0 NOT-MEASURED, 0 UNRUN". Three of them (`check:dual-build-cjs-loads`, `check:i18n`, `check:type-check-debt`) first exited 3 (PREREQUISITE NOT MET: nothing measured). After the full `turbo run build --filter='./packages/*' --filter='./packages/*/*'` (71 tasks), all three exit 0. - **Lint, narrowed, at `dc440bff6b`.** ESLint's own `isPathIgnored` and `calculateConfigForFile` put 6 of the 7 changed paths in its population; its config ignores the changeset `.md`. `lintFiles` over those 6 with `allowInlineConfig: false` (the `lint` script's `--no-inline-config`), JSON formatter: 6 files, 0 errors, 0 warnings. The config enables no type-aware linting (no `parserOptions.project`, no typed rules), so this diff cannot move a verdict on an untouched file. The repository-wide `pnpm lint` belongs to CI. ## Acceptance notes - **driver-mongodb, a sibling refusal of another class** (`fieldReferenceUnsupportedError`: the driver has no field-to-field lowering at all). It is 538 characters from source, so the bound cuts it. Its remedy ("Compare against a literal value instead.") ends at 410 and survives; the cut drops the end of its withholding sentence. Not edited here. Source reading plus the bound's arithmetic; not measured through a REST door, because no MongoDB server was available. - `packages/rest/src/security-explain-envelope.test.ts` still builds its matcher and explain refusals by hand, with the old opening, and says neither `@objectstack/formula` nor `@objectstack/plugin-security` is a dependency of `rest`. `plugin-security` is a devDependency now. The hand-built text is a fixture, not a pin of either producer, and the route reads only its code and status. Left as is. - The explain copy never carried the class list or a withholding sentence (it names the policy and both columns by design, since the explain report publishes the same predicate), so it keeps remedy, subject and diagnostic, and reason. --- _Generated by [Claude Code](https://claude.ai/code/session_01DEvba2nBuD4tWzfq8r8NFY)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 525b813 commit 05be352

7 files changed

Lines changed: 349 additions & 16 deletions
Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
---
2+
'@objectstack/formula': patch
3+
'@objectstack/plugin-security': patch
4+
---
5+
6+
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)
7+
8+
Clause-②: no
9+
10+
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:
11+
12+
- 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.
13+
- 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.
14+
15+
Both messages now start with the remedy. It is the same sentence as before and has only moved:
16+
17+
- 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.
18+
- 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.
19+
20+
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‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -161,6 +161,31 @@ describe('matchesFilterCondition — a field compared with a field of no shared
161161
expect(crossFieldClassRefusalCarriedBy(new Error('x'))).toBeNull();
162162
});
163163

164+
it('leads with its remedy and fits the REST client-message bound whole, however long the column names', () => {
165+
// The REST door cuts a 4xx message of 500 characters or more to 499 plus an
166+
// ellipsis (`CLIENT_MESSAGE_MAX`, `@objectstack/rest`): it keeps the HEAD.
167+
const remedy =
168+
'In a row-level policy, compare a field only with a field of the same class, or fix the declaration of ' +
169+
'the one that is declared with the wrong type.';
170+
const long = (stem: string) => `${stem}_${'x'.repeat(120)}`;
171+
const longFields = { [long('stage')]: { type: 'text' }, [long('amount')]: { type: 'number' } };
172+
const shortErr = refusalOf({ status: { $ne: { $field: 'amount' } } })!;
173+
let longErr: WireBearingError | null = null;
174+
try {
175+
matchesFilterCondition({}, { [long('stage')]: { $ne: { $field: long('amount') } } } as never, { fields: longFields });
176+
} catch (e) {
177+
longErr = e as WireBearingError;
178+
}
179+
expect({ code: longErr?.code, status: longErr?.status }).toEqual({ code: 'INVALID_FILTER', status: 400 });
180+
// It names no column, so its length does not depend on theirs.
181+
expect(longErr?.message).toBe(shortErr.message);
182+
expect(shortErr.message.startsWith(remedy)).toBe(true);
183+
expect(shortErr.message.length).toBeLessThan(500);
184+
// After the remedy: what is refused, why, and why the columns are withheld.
185+
const at = (s: string) => shortErr.message.indexOf(s);
186+
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);
187+
});
188+
164189
it('findCrossFieldClassRefusal answers null for a filter whose comparisons all compare', () => {
165190
expect(findCrossFieldClassRefusal({ $and: [{ status: { $eq: { $field: 'title' } } }, { amount: { $lt: { $field: 'budget' } } }] }, FIELDS)).toBeNull();
166191
expect(findCrossFieldClassRefusal({ amount: { $lt: { $field: 'status' } } }, FIELDS)).toMatchObject({

‎packages/formula/src/matches-filter.ts‎

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

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

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -180,15 +180,26 @@ const ROW = { id: 'r1', status: 'open', title: 'x', amount: 5 };
180180

181181
const rlsRecordOf = (d: ExplainDecision) => d.layers.find((l) => l.layer === 'rls')?.record;
182182

183+
/**
184+
* The remedy explain's refusal leads with. It comes before the policy names and
185+
* the diagnostic, which have no length bound, because the REST door keeps only
186+
* a long message's first 499 characters.
187+
*/
188+
const REMEDY =
189+
'Compare a field only with a field of the same class, or fix the declaration of the one that is declared with ' +
190+
'the wrong type.';
191+
183192
/**
184193
* Explain's answer is the find's refusal: the same envelope, no decision and so
185-
* no record verdict, and a message that names the policy and both columns.
194+
* no record verdict, and a message that leads with the remedy and names the
195+
* policy and both columns.
186196
*/
187197
async function expectExplainRefuses(p: Promise<unknown>, columns: [string, string]): Promise<void> {
188198
const r = await refusalOf(p);
189199
expect(r).not.toBe('answered');
190200
if (r === 'answered') return;
191201
expect({ code: r.code, status: r.status }).toEqual(INVALID);
202+
expect(r.message.startsWith(`${REMEDY} `), r.message).toBe(true);
192203
expect(r.message).toContain(`'${POLICY}'`);
193204
for (const column of columns) expect(r.message).toContain(`"${column}"`);
194205
}

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

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

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

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

210211
const lines = w.refusalLines();

0 commit comments

Comments
 (0)