Merge remote-tracking branch 'origin/main' into claude/issue-20595-driver-sql-citations

Author: claude

.changeset/21254-rls-write-check-json-column-operator-refusal.md

@@ -0,0 +1,21 @@
+---
+'@objectstack/plugin-security': patch
+---
+
+fix(plugin-security): a row-level `check` refuses an operator the read refuses on a field declared JSON-stored, with the read's `INVALID_FILTER` / 400, so a policy whose read is refused no longer admits writes (#21254)
+
+Clause-②: no
+
+The read a row-level policy scopes refuses a scalar comparison, an ordering or a text operator (`@objectstack/core`'s `JSON_COLUMN_INCOMPATIBLE_OPERATORS`, and implicit equality) on a field the object declares JSON-stored: a structured-JSON type (`json`, `address`, …), or a multi-valued field (`tags`, `multiselect`, `checkboxes`, or a `select` / `lookup` / `user` / `file` / `image` flagged `multiple: true`). The write `check` evaluated the same operators against the stored list instead. Measured through `ObjectQL.insert` with `SecurityPlugin` on two SQLite driver families, as a member resolving a permission set, with the same predicate as `using` and `check`:
+
+| `check` | written | write, before | read |
+|---|---|---|---|
+| `record.tags != 'x'` | `['x']` or `'x'` | admitted, stored `["x"]` | 400 |
+| `!(record.tags in ['x'])` | `['x']` | admitted, stored `["x"]` | 400 |
+| `record.tags == 'x'` / `record.tags in ['x']` | `['x']` | 403 | 400 |
+| `record.tags > 'a'` | `['x']` | 400 | 400 |
+| `record.meta != 'x'` / `record.meta == 'x'` (`meta` is `json`) | a scalar | admitted, stored | 400 |
+
+Now the write check refuses every one of these with the read's answer: `INVALID_FILTER` / 400 and the read's words, which withhold the field and the operator. The refusal reads the object's declaration, never the record, so a policy is refused for every row or for none, on the insert, a by-id update and a predicate update. The diagnostic, which names the field, the operator and the policy, goes to the server log. Rows that already refused still store nothing; their answer is now the read's.
+
+Unchanged: `contains` and its negation (`$contains` / `$notContains`), and the presence checks (`== null`, `!= null`), answer on such a field as before; a field declared neither way keeps every operator; an object whose schema cannot be loaded is judged as before. To repair a refused policy, test membership with `contains` (for example `!record.tags.contains('x')`).

.changeset/21274-driver-fault-boundary-redaction.md

@@ -0,0 +1,14 @@
+---
+'@objectstack/objectql': patch
+---
+
+fix(objectql): a driver error that leaves the engine no longer carries the failing statement or the caller's values
+
+Clause-②: no
+
+The engine has cut the bound statement out of its own log line for a failed driver call for a long time, but it rethrew the driver's raw error. Any in-process code that logged what it caught, such as an auth library's error logger, printed the statement and the row's values. The same cut now runs where the error leaves the engine, so no consumer needs a patch of its own.
+
+- **Where.** Every engine operation that reaches a driver: `find`, `findOne`, `count`, `aggregate`, `insert` (batch included), `update` and `delete` (by id and by predicate), `execute`, `transaction`, `resolveSecretField` and `resolveInternalField`.
+- **What is cut.** The statement and the caller's values, from the error's `message` and `stack`, from the properties drivers attach (mysql2's `sql` and `sqlMessage`; node-postgres' `detail`, `where` and `internalQuery`), and down the `cause` chain. A `DuplicateRecordError` keeps its own fields and carries a cut `cause`.
+- **What stays.** The error's class (`instanceof` still holds), `name`, `code`, `errno`, `sqlState`, Postgres' identifier fields (`constraint`, `table`, `column`, …) and the database's own diagnostic. The message now reads as the statement's kind, a `[statement and bound values redacted]` marker and the diagnostic. A Postgres key-shaped `detail` keeps its column list. Every REST answer keeps its status, code and `field`.
+- **What changes for a caller.** Code that read the statement or a value out of a driver error's message or properties now gets the marker instead. Branch on the class, `code` or `errno` instead. The driver error on a `DuplicateRecordError`'s `cause` is an equivalent copy, no longer the object the driver threw. An import's row report for a value-bearing database error no longer repeats the rejected value.

packages/objectql/src/driver-fault-boundary-redaction.test.ts

@@ -23,7 +23,7 @@
 // the tolerant-fallback direction, not the loud one.
 
 import { describe, it, expect } from 'vitest';
-import { isUniqueViolationError, uniqueViolationColumn } from '@objectstack/types';
+import { isUniqueViolationError, mapDataError, uniqueViolationColumn } from '@objectstack/types';
 import { ObjectQL } from './engine.js';
 import {
   VALUE_BEARING_TEMPLATES,
@@ -853,14 +853,23 @@ describe('#8682 half B — the write-path loggers', () => {
     }
   });
 
-  it('the RETHROWN error is untouched — the caller`s 400 must not move', async () => {
-    // `mapDataError` reads the driver's raw message to extract the failing
-    // field and answer `400 INVALID_FIELD`. Redacting what we THROW would break
-    // that answer; the redaction is one argument at one call site.
+  it('the caller`s 400 does not move — and since #21274 the rethrown error carries no caller value either', async () => {
+    // `mapDataError` reads the thrown message to extract the failing field and
+    // answer `400 INVALID_FIELD`. This pin used to hold that by asserting the
+    // thrown error was the driver's RAW one, statement and values included —
+    // which is the exposure #21274 closed for every in-process logger. What it
+    // protects is the ANSWER, so the answer is what it asserts now: the cut at
+    // the engine boundary keeps the diagnostic the field is read from.
     const { thrown } = await insertAgainstADriftedColumn();
 
-    expect(String(thrown?.message)).toContain('insert into');
-    expect(String(thrown?.message)).toContain(SECRET);
+    expect(String(thrown?.message)).not.toContain(SECRET);
+    expect(String(thrown?.message)).not.toContain(DESCRIPTION);
+    expect(String(thrown?.stack)).not.toContain(SECRET);
+    expect(String(thrown?.message)).toContain('has no column named secret_note');
+    expect(thrown?.code).toBe('SQLITE_ERROR');
+    const answer = mapDataError(thrown, 'crm_account');
+    expect(answer.status).toBe(400);
+    expect(answer.body).toMatchObject({ code: 'INVALID_FIELD', field: 'secret_note' });
   });
 
   /**
@@ -905,10 +914,13 @@ describe('#8682 half B — the write-path loggers', () => {
     }
   });
 
-  it('MySQL duplicate entry — the driver error reaches the caller UNTOUCHED, on `cause`', async () => {
-    // Same boundary as above: the log narrows, and what the driver said is not
-    // rewritten anywhere. `isUniqueViolationError` and `uniqueViolationColumn`
-    // read this text downstream and must keep seeing it.
+  it('MySQL duplicate entry — the driver error reaches the caller on `cause`, cut, with every verdict kept', async () => {
+    // `isUniqueViolationError` and `uniqueViolationColumn` read this error
+    // downstream and must keep answering as they did. This pin used to hold
+    // that by asserting the `cause` was the driver's error UNTOUCHED, statement
+    // and value included; #21274 cuts both where the error leaves the engine,
+    // so what is asserted now is what those readers actually read — the code,
+    // the diagnostic and the index name — and that the value is gone.
     //
     // ⚠️ [#14095] WHERE the caller finds it moved by one step, and only for a
     // recognised unique violation: the insert door now answers the
@@ -924,9 +936,10 @@ describe('#8682 half B — the write-path loggers', () => {
     expect((thrown as any)?.status).toBe(409);
 
     const cause = (thrown as any)?.cause;
-    expect(String(cause?.message)).toContain('insert into');
-    expect(String(cause?.message)).toContain(SECRET);
+    expect(String(cause?.message)).not.toContain(SECRET);
+    expect(String(cause?.message)).not.toContain(DESCRIPTION);
     expect(String(cause?.message)).toContain('Duplicate entry');
+    expect(String(cause?.message)).toContain("for key 'crm_account.secret_note'");
     expect(cause?.code).toBe('ER_DUP_ENTRY');
 
     // The verdicts the downstream consumers ask of it, asked of the envelope.