From 691b170c35976a48cb3030cfb4f23952a1681024 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 10:16:28 +0000 Subject: [PATCH 1/2] fix(security)!: an RLS predicate naming an undeclared column denies in every position A predicate naming a column the object does not declare could not narrow, and in a negation-carrying position it did not deny either -- it WIDENED the policy to every row inside the tenant wall (read face) and PERMITTED the write the policy was authored to refuse (write face). Read face: `extractTargetField` is a LEADING `==`/`=`/`in` shape match, so `nope != "x"`, `!(nope == 1)`, `!(nope in [...])` and any arm after the first returned `null`, `if (!targetField) return true` KEPT the policy, `dropped` never incremented and the deny sentinel never armed. Write face: `computeWriteCheckFilter` compiled `check` clauses with no field-existence net at all. The repair is one seam, not two: `RLSCompiler.compileFilter` -- which both the read layer and the ADR-0058 D4 write gate already pass through -- now takes the object's declared-column set and judges every column the policy names on the COMPILED FilterCondition tree. That is positional-agnostic by construction: the pushdown compiler lowers `!` to `$not`, `||` to `$or` and `&&` to `$and`, so a column lands as a plain object key whatever position it was authored in, and there is no spelling of negation left for a shape match to miss. The matcher's include-direction ruling (`noValueSatisfiesNegation`, driver-memory / driver-mongodb) is deliberately UNTOUCHED -- it is correct for an ordinary user query. The defect was that the policy compiler lowered an undeclared column into a filter at all; the matcher now never sees a phantom. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- .../plugin-security/src/rls-compiler.ts | 168 +++++++++++++++++- .../plugin-security/src/security-plugin.ts | 72 +++++++- 2 files changed, 232 insertions(+), 8 deletions(-) diff --git a/packages/plugins/plugin-security/src/rls-compiler.ts b/packages/plugins/plugin-security/src/rls-compiler.ts index 3c54ac68dc..c121a2067f 100644 --- a/packages/plugins/plugin-security/src/rls-compiler.ts +++ b/packages/plugins/plugin-security/src/rls-compiler.ts @@ -22,7 +22,7 @@ import type { CelFilterFailReason } from '@objectstack/formula'; * {@link isEmptyMembershipFilter} then refuses — same silent denial, so it * joins the same vocabulary rather than staying unnamed. */ -type RlsDropReason = CelFilterFailReason | 'empty-membership'; +type RlsDropReason = CelFilterFailReason | 'empty-membership' | 'unknown-field'; /** A dropped policy's cause: the compiler's reason plus its human `detail`. */ interface RlsDropCause { @@ -72,6 +72,149 @@ interface RLSUserContext { [key: string]: unknown; } +/** + * The declared-column set a compiled policy is judged against. + * + * Supplied by the caller because only the caller can resolve it (the plugin's + * `getObjectFieldNames`, which prefers ObjectQL's live SchemaRegistry over the + * boot-time metadata artifact). `undefined` means "not resolvable here", and a + * compile with no guard behaves exactly as it did before the guard existed — + * a schema that cannot be loaded must not manufacture denials. + */ +export interface RlsFieldGuard { + /** Every column the object declares, exactly as `getObjectFieldNames` builds it. */ + declared: ReadonlySet; +} + +/** {@link judgeCompiledFields}' answer. */ +type RlsFieldVerdict = + | { ok: true } + | { ok: false; detail: string }; + +/** + * Every column a compiled {@link https://www.mongodb.com/docs Mongo-style} + * FilterCondition NAMES — at any depth, in any polarity. `null` means the tree + * contains a shape this walker does not model, which is a REFUSAL, not an + * empty answer. + * + * ## Why the compiled filter and not the predicate text + * + * This is the whole point of the seam. A source-text matcher has to enumerate + * the spellings of negation (`!=`, `!(…)`, `not in`, an arm after the first), + * and the one it has not been told about is the one that gets through — which + * is the defect one level over, not its repair. The compiled tree has no + * spellings: `cel-to-filter.ts` lowers `!` to `$not`, `||` to `$or`, `&&` to + * `$and`, and every column it names lands as a plain object key whatever + * position the author wrote it in. Walking that is positional-agnostic by + * construction, and it stays correct when the pushdown compiler learns a new + * source form, because a new form still has to lower into this same shape. + * + * ## The rules, and where they differ from the ingress collector + * + * Two collectors already answer "which columns does this filter name" — + * `collectFilterFieldKeys` (`@objectstack/metadata-protocol`) and + * `collectFilterFieldNames` (`@objectstack/objectql`). This one keeps their + * first rule verbatim and INVERTS their second, deliberately: + * + * - **`$and` / `$or` / `$not` are combinators, recursed into; a non-`$` key is + * a column.** Same as both. + * - ⚠️ **Any OTHER `$`-prefixed key at NODE level REFUSES here** (`null`), + * where the ingress collectors skip it without descending. Their direction is + * right for a gate that must not invent 400s on caller input; it is wrong + * here, because the input is not caller input — it is THIS compiler's own + * output, which emits `$and` / `$or` / `$not` and nothing else at node level. + * An unmodelled combinator therefore means the tree grew a shape this guard + * has not been taught, and leaving the columns beneath it unexamined is + * precisely the fail-open this guard exists to close. + * - **A field key's value is scanned only for `{ $field: … }` references** — + * the field-to-field comparison `cel-to-filter.ts` emits — which name a + * second real column. Other nested keys are a cross-object condition the RLS + * compiler refuses at lowering time (`classify` throws `unsupported` on any + * `.`-chain), so they cannot appear; they are ignored rather than refused so + * this walker never denies on a shape it merely does not produce. + */ +function collectRlsFilterColumns( + node: unknown, + out: Set = new Set(), + depth = 0, +): Set | null { + // A self-referential filter must not hang the read path. Unlike the ingress + // collector — which returns what it has — overrunning the bound REFUSES, + // because an unexamined subtree is an unguarded column. + if (depth > 32) return null; + if (node === null || typeof node !== 'object' || Array.isArray(node)) return null; + for (const [key, value] of Object.entries(node as Record)) { + if (key === '$and' || key === '$or') { + if (!Array.isArray(value)) return null; + for (const arm of value) { + if (collectRlsFilterColumns(arm, out, depth + 1) === null) return null; + } + continue; + } + if (key === '$not') { + if (collectRlsFilterColumns(value, out, depth + 1) === null) return null; + continue; + } + if (key.startsWith('$')) return null; + out.add(key); + collectFieldReferences(value, out, depth + 1); + } + return out; +} + +/** Collect `{ $field: 'other_column' }` right-hand references out of an operator bag. */ +function collectFieldReferences(spec: unknown, out: Set, depth: number): void { + if (depth > 32 || spec === null || typeof spec !== 'object') return; + if (Array.isArray(spec)) { + for (const item of spec) collectFieldReferences(item, out, depth + 1); + return; + } + for (const [key, value] of Object.entries(spec as Record)) { + if (key === '$field') { + if (typeof value === 'string' && value !== '') out.add(value); + continue; + } + collectFieldReferences(value, out, depth + 1); + } +} + +/** + * Does every column this compiled policy names exist on the object? + * + * The security question underneath: a predicate naming a column the object does + * not declare cannot narrow anything, and in a NEGATION-carrying position it + * does not merely fail to narrow — it WIDENS, because a row that has no such + * column satisfies "column != x" under the settled include-direction ruling + * (`noValueSatisfiesNegation`, driver-memory / driver-mongodb). That ruling is + * correct for an ordinary user query and is deliberately untouched; what is + * wrong is lowering an undeclared column into a filter AT ALL from a policy + * compiler. So the phantom is stopped here, before any matcher sees it. + */ +function judgeCompiledFields( + filter: Record, + guard: RlsFieldGuard, +): RlsFieldVerdict { + const named = collectRlsFilterColumns(filter); + if (named === null) { + return { + ok: false, + detail: + 'the compiled predicate contains a filter shape this guard does not model, so the columns it ' + + `names could not be enumerated and the policy was refused rather than trusted (compiled to ${JSON.stringify(filter)})`, + }; + } + const missing = [...named].filter((column) => !guard.declared.has(column)); + if (missing.length === 0) return { ok: true }; + return { + ok: false, + detail: + `the predicate names ${missing.length === 1 ? 'a column' : 'columns'} the object does not declare ` + + `(${missing.map((m) => `"${m}"`).join(', ')}), so it cannot narrow anything; in a negation-carrying ` + + 'position it would instead WIDEN the policy to every row the tenant wall admits, so the policy was ' + + 'refused', + }; +} + /** * Sentinel filter used when applicable RLS policies exist but none can * be compiled against the current execution context (typically because a @@ -261,11 +404,22 @@ export class RLSCompiler { * active organization). The caller must treat this as "deny by * default" — its `id` comparison naturally yields zero rows on * select/update/delete, which is the safe fail-closed answer. + * + * `fieldGuard` is the object's declared-column set. When supplied, a policy + * whose compiled predicate names a column the object does not declare is + * DROPPED — in every position and every polarity, judged on the compiled + * tree rather than on the predicate's text ({@link judgeCompiledFields}) — + * and joins the same fail-closed path as any other dropped policy. It is the + * ONE seam both faces pass through: the read layer compiles `using` here and + * the ADR-0058 D4 write gate compiles `check` here, so the two can no longer + * disagree about what a phantom column means. Omit it and the compile behaves + * exactly as it did before the guard existed. */ compileFilter( policies: RowLevelSecurityPolicy[], executionContext?: ExecutionContext, clause: 'using' | 'check' = 'using', + fieldGuard?: RlsFieldGuard, ): Record | null { if (policies.length === 0) return null; @@ -314,7 +468,17 @@ export class RLSCompiler { applicable++; const outcome = this.compileExpressionOutcome(predicate, userCtx); if (outcome.filter) { - filters.push(outcome.filter); + // Field existence, judged on the COMPILED tree so no negation spelling + // can route around it. A policy naming an undeclared column joins + // `deniedBy` exactly as an unresolved variable does — same collection, + // same fail-closed sentinel below, same WARN line — because it is the + // same class of fault: an applicable policy that cannot enforce. + const verdict = fieldGuard ? judgeCompiledFields(outcome.filter, fieldGuard) : null; + if (verdict && !verdict.ok) { + deniedBy.push({ policy, cause: { reason: 'unknown-field', detail: verdict.detail } }); + } else { + filters.push(outcome.filter); + } } else if (!isSupportedRlsExpression(predicate)) { // ADR-0056 D4: an UNSUPPORTED-SHAPE predicate (e.g. arithmetic, functions, // subqueries) compiles to nothing and would silently vanish, leaving the diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 29b0eb04a5..b6dc708a7c 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -6016,9 +6016,32 @@ export class SecurityPlugin implements Plugin { ? collected.filter((p) => !isPlatformOwnershipFloorPolicy(p)) : collected; if (allRlsPolicies.length > 0) { - // Field-existence safety: a wildcard policy targeting a column the object - // lacks is a *deny* contribution (fail-closed), unless the object opted - // out of tenancy (skip). Schema-lookup failure keeps all policies. + // Field-existence safety, in TWO passes that answer two different + // questions. Schema-lookup failure runs neither: a schema that cannot + // be loaded must not manufacture denials. + // + // Pass 1, HERE — the ADR-0095 delta c carve-out, and only that. A + // wildcard `organization_id`-leading policy on an object that opted OUT + // of tenancy is NOT APPLICABLE to it, so it is skipped WITHOUT + // contributing to the deny sentinel; a leading miss on any other column + // is a deny contribution. `extractTargetField`'s leading `==` / `=` / + // `in` shape match is exactly the right instrument for that carve-out, + // because the shape it recognises is the shape the platform's own + // tenant policy is authored in. + // + // Pass 2, in the COMPILER — every column named ANYWHERE in the + // predicate, in any position and any polarity. ⚠️ This pass is what + // pass 1 cannot do and must not be widened to do: a predicate naming an + // undeclared column in a NEGATION-carrying position (`nope != "x"`, + // `!(nope == 1)`, `!(nope in […])`, or a trailing `||` arm) returned + // `null` from `extractTargetField`, so the policy was KEPT, `dropped` + // never incremented, the sentinel never armed — and a row that has no + // such column SATISFIES the negation, so the authored narrowing widened + // to every row inside the tenant wall instead of denying. Teaching the + // regex more spellings would only move the same defect one level over; + // the compiler judges the compiled tree, where a negation is a `$not` + // node and has no spelling left to hide behind. The matcher's + // include-direction ruling is untouched — it never sees the phantom. let dropped = 0; const compilable = objectFields ? allRlsPolicies.filter((p) => { @@ -6032,7 +6055,12 @@ export class SecurityPlugin implements Plugin { return false; }) : allRlsPolicies; - layer1 = this.rlsCompiler.compileFilter(compilable, context); + layer1 = this.rlsCompiler.compileFilter( + compilable, + context, + 'using', + objectFields ? { declared: objectFields } : undefined, + ); // Every applicable policy dropped for a missing field → deny sentinel. if (layer1 == null && dropped > 0) { layer1 = { ...RLS_DENY_FILTER }; @@ -6266,7 +6294,26 @@ export class SecurityPlugin implements Plugin { // throwing resolver or an unresolved key still drop the policy and still // fail closed, on this path as on the read path. await this.stageRlsMembership(context); - return this.rlsCompiler.compileFilter(withCheck, context, 'check'); + // [#17042] The write face's field-existence net — which this path had NONE + // of. `computeWriteCheckFilter` compiled `check` clauses with no column + // check at all, so a phantom column in a negated position + // (`nope != "x"`, `!(nope == 1)`, `!(nope in […])`, a trailing `||` arm) + // was satisfied VACUOUSLY by the post-image and PERMITTED the write the + // policy was authored to refuse — measured on both SQL drivers, the write + // being driver-independent because the check is evaluated in-process + // against the post-image (`matchesFilterCondition`, step 3.6). Only a + // POSITIVE phantom refused, by accident of `looseEq(undefined, value)` + // being false, which is why the hole was invisible to a suite that tested + // the positive shape. The guard is the SAME one the read layer passes, so + // the two faces can never again disagree about what an undeclared column + // means. `null` (schema not loadable) passes no guard and behaves as before. + const objectFields = await this.getObjectFieldNames(this.metadata, object, this.ql); + return this.rlsCompiler.compileFilter( + withCheck, + context, + 'check', + objectFields ? { declared: objectFields } : undefined, + ); } /** @@ -7650,7 +7697,20 @@ export class SecurityPlugin implements Plugin { /** * Extract the left-hand field name from a simple RLS expression like * `field = current_user.x` or `field IN (current_user.y)`. Returns - * `null` for unsupported shapes (in which case we keep the policy). + * `null` for unsupported shapes. + * + * ⚠️ [#17042] What a `null` MEANS here changed, and the change is the whole + * point: this is no longer the field-existence net, it is the ADR-0095 delta + * c carve-out's instrument. `null` still means "this policy is not the + * leading-`organization_id` shape the carve-out is about", so the caller + * keeps it — but the policy is now judged for column existence a second time + * inside `RLSCompiler.compileFilter`, on the COMPILED tree, where position + * and polarity have been normalised away. ⛔ Do not widen this regex to + * recognise `!=` / `!` / `not in`: a shape match that must enumerate every + * spelling of negation is the same "recognises only what it was told about" + * defect the compiler-side pass exists to end, and widening it here would + * ALSO turn the carve-out into a denial for the tenancy-disabled case it + * deliberately skips. */ private extractTargetField(using?: string): string | null { if (!using) return null; From ef4511306d4f1afe2de715e684e82527b8e97dd5 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 9 Sep 2026 10:24:44 +0000 Subject: [PATCH 2/2] test(security): pin the phantom-column negation shapes on both faces, and correct the linter's consequence prose The regression suite runs the four negation shapes through the compiler seam and end-to-end through a real ObjectQL + SecurityPlugin on both SQL drivers, on the read face and the write face, each against the two controls that make a reading a reading: a real column must still narrow, and the same phantom column in a positive position must still refuse. Ablated against the pre-fix source: 36 of 50 cells fail, and the 14 that hold are exactly the controls. It also pins the include-direction ruling as UNCHANGED -- the raw matcher still admits 3 of 3 rows for the same filter -- so a later reader can see that what moved is that the policy compiler stopped producing the filter, not what the matcher does with one. The linter's detection is untouched. Its consequence text was stale in one half and misattributed in the other: it described the field miss as having two directions decided by position, and it credited the write leg's fail-closed to a safety net `computeWriteCheckFilter` never had. It now states one direction for both clauses and records the older runtime's fail-open write behaviour explicitly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_012zTkyNHJ7TkuN2oXtP5x37 --- ...eclared-column-denies-in-every-position.md | 29 ++ ...idate-rls-predicate-enforceability.test.ts | 80 ++-- .../validate-rls-predicate-enforceability.ts | 78 ++-- .../src/rls-phantom-column-negation.test.ts | 376 ++++++++++++++++++ 4 files changed, 496 insertions(+), 67 deletions(-) create mode 100644 .changeset/rls-undeclared-column-denies-in-every-position.md create mode 100644 packages/plugins/plugin-security/src/rls-phantom-column-negation.test.ts diff --git a/.changeset/rls-undeclared-column-denies-in-every-position.md b/.changeset/rls-undeclared-column-denies-in-every-position.md new file mode 100644 index 0000000000..3f6752cbab --- /dev/null +++ b/.changeset/rls-undeclared-column-denies-in-every-position.md @@ -0,0 +1,29 @@ +--- +"@objectstack/plugin-security": minor +"@objectstack/lint": patch +--- + +fix(plugin-security)!: an RLS predicate naming an undeclared column now denies in EVERY position and polarity, on the read face and the write face alike (#17042) + + + +**BREAKING** — a fail-open-to-fail-closed narrowing on row-level security. A policy that widened yesterday denies today. Shipped as `minor` under the launch-window convention, the same grading the insert-side `check` post-image narrowing used. + +A predicate naming a column the object does **not declare** could not narrow, and in a **negation-carrying position** it did not deny either — it **widened** the policy to every row inside the tenant wall, and on the write path it **permitted** the write the policy was authored to refuse. + +⛔ It is **not** a cross-tenant leak. Tenancy is a separate layer and it holds. What was defeated is the narrowing the policy author wrote *inside* the wall — an owner-only or private-record policy silently becoming "every row". + +Two independent sites, each with its own reason, each measured against the same two controls (a real column must still narrow; the *same* phantom column in a **positive** position must still refuse): + +- **Read face.** `extractTargetField` is a **leading-only** `==` / `=` / `in` shape match, so `nope != "x"`, `!(nope == 1)`, `!(nope in ['a'])` and any arm after the first returned `null`; the policy was **kept**, the drop counter never incremented and the deny sentinel never armed. The kept filter then met the settled include-direction ruling — a row that *has* no such column satisfies "column != x". Measured on the matcher: **3 of 3** rows for each negated shape, against **1 of 3** for the real narrowing and **0 of 3** for the same phantom column in a positive position. +- **Write face — the worse one.** `computeWriteCheckFilter` compiled `check` clauses with **no field-existence check at all**, and the ADR-0058 D4 post-image gate evaluates that filter in-process. Measured end to end on both SQL drivers: every negated phantom **permitted** the insert, in both post-image polarities, while a positive phantom refused (by accident of an absent value comparing unequal) — which is why a suite that only ever exercised the positive shape stayed green over the hole. + +**The repair is one seam, not two.** `RLSCompiler.compileFilter` — the single choke point both the read layer and the write gate already pass through — now takes the object's declared-column set and judges every column the policy names on the **compiled** `FilterCondition` tree. That is positional-agnostic by construction: the pushdown compiler lowers `!` to `$not`, `||` to `$or` and `&&` to `$and`, so a column lands as a plain object key whatever position it was authored in, and there is no spelling of negation left for a shape match to miss. Widening the regex instead was rejected: a matcher that must enumerate every spelling of negation is the same "recognises only what it was told about" defect one level over, and it would additionally have broken the ADR-0095 carve-out that *depends* on the regex recognising only the leading shape. A policy dropped this way joins the existing fail-closed path — same deny sentinel, same WARN line — rather than growing a parallel mechanism. + +⛔ **The matcher's include-direction ruling is untouched.** A row lacking a column *does* satisfy "column != x" for an ordinary user query, and re-semanticing every filter in the repo to fix one caller is not the trade. The defect was that a policy compiler lowered an undeclared column into a filter at all; the matcher now never sees a phantom, and a regression test pins the raw matcher still answering 3 of 3 for the same filter so a later reader can see which half moved. + +**Who is affected.** Only a permission set carrying an RLS policy whose predicate names a column its object does not declare — an authoring mistake `@objectstack/lint` already reports on all of these shapes. For such a policy the object now returns **zero rows** for every holder of the set (read) and refuses every governed insert / update (write), where before a negated spelling returned everything and permitted everything. ⚠️ **An installation relying on such a policy to grant access will lose that access at the upgrade, and that is the intended direction**: what it was "granting" was the absence of enforcement. Correct the column name; the linter names the miss and offers the object's real field list. + +**driver-sql, previously unmeasured, is now measured, and it refines the picture.** On the **read** face `driver-sql` and `driver-sqlite-wasm` never widened — they failed closed by **raising** `INVALID_FILTER` / 400 when the phantom column reached the statement builder, so the read-face defect was driver-dependent (in-process matchers widened; SQL raised). On the **write** face they failed open exactly like every other driver, because the `check` is evaluated in-process and never reaches SQL. After this change both faces answer uniformly on both drivers. `driver-mongodb` remains inferred from the shared ruling rather than measured. + +`@objectstack/lint`'s diagnostic for this miss is corrected in the same change. Its **detection is unchanged** — all the negated shapes were already reported. Its consequence text was stale in one half and misattributed in the other: it described the field miss as having two directions decided by position, and it credited the write leg's fail-closed to a safety net that path never had. It now states one direction for both clauses, and records the older runtime's fail-open write behaviour explicitly so an operator reading it against a deployment that predates this guard is not told the wrong thing. diff --git a/packages/lint/src/validate-rls-predicate-enforceability.test.ts b/packages/lint/src/validate-rls-predicate-enforceability.test.ts index 2600e7bc5f..322837d689 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.test.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.test.ts @@ -623,10 +623,20 @@ describe('validateRlsPredicateEnforceability — the messages name the COST, not expect(f.where).toBe('permission set "sales_manager" policy "opportunity_private_owner_only" on object "crm_opportunity"'); expect(f.path).toBe('permissions[0].rowLevelSecurity[0].using'); expect(f.severity).toBe('error'); - // ⚠️ BOTH directions, because they are not the same and the fail-OPEN one is - // the dangerous half: an author told "this denies everything" about a - // predicate that in fact matches everything hardens the wrong thing. - expect(f.message).toMatch(/one of the two directions is fail-OPEN/); + // ⚠️ ONE direction, and the pin says so on purpose. This block used to + // assert the opposite — that the field half had a fail-OPEN leg decided by + // position — which was true of the runtime at the time and is now false: + // column existence is judged on the COMPILED predicate, so position and + // polarity are normalised away before the check runs. An author told "one + // of these directions is fail-OPEN" would now harden against a hole that + // no longer exists, and would not fix the name. + expect(f.message).not.toMatch(/fail-OPEN/); + expect(f.message).toMatch(/judges column existence on the COMPILED predicate/); + expect(f.message).toMatch(/the position and the polarity you wrote it in make no difference/); + // …and every shape the old text split across two directions is named in + // the one direction, so an author recognises their own predicate in it. + expect(f.message).toMatch(/`field != x`/); + expect(f.message).toMatch(/any arm after the first/); // ⛔ …and NOT by citing a tracker id. This string reaches authors, // operators and generated surfaces, none of whom can resolve `#NNNN` // (`check:doc-authoring`); the id lives in the adjacent `//` comment, which @@ -634,17 +644,19 @@ describe('validateRlsPredicateEnforceability — the messages name the COST, not // author does not re-add it and learn this from CI instead. expect(f.message).not.toMatch(/#\d{3,}/); expect(f.hint).not.toMatch(/#\d{3,}/); - // closed leg — the LEADING position the safety net recognises - expect(f.message).toMatch(/fails CLOSED/); + // the cost, which is the same cost the variable half carries + expect(f.message).toMatch(/DROP the policy at request time/); expect(f.message).toMatch(/RLS_DENY_FILTER/); expect(f.message).toMatch(/ZERO rows/); - // open leg — a negation or any arm after the first - expect(f.message).toMatch(/leaves the policy KEPT/); - expect(f.message).toMatch(/SATISFIES the negated constraint/); - expect(f.message).toMatch(/DEFEATED/); - // …and the limits, stated rather than overstated - expect(f.message).toMatch(/NOT a cross-tenant leak/); - expect(f.message).toMatch(/driver-sql is NOT MEASURED/); + expect(f.message).toMatch(/DISAPPEARS for every holder of this permission set/); + // ⛔ …and NOT the three claims the rewritten text retired. The + // cross-tenant sentence went with them: it was there to bound a leak + // reading that the message no longer makes, and a denial needs no such + // disclaimer. Overstating the old defect was the risk; restating a bound + // on a defect the text does not describe is just noise. + expect(f.message).not.toMatch(/leaves the policy KEPT/); + expect(f.message).not.toMatch(/DEFEATED/); + expect(f.message).not.toMatch(/driver-sql is NOT MEASURED/); // …and the miss itself, with the platform's own "did you mean". expect(f.message).toMatch(/"is_private_nope" is not a field on object "crm_opportunity"/); expect(f.message).toMatch(/Did you mean "is_private"\?/); @@ -673,10 +685,15 @@ describe('validateRlsPredicateEnforceability — the messages name the COST, not expect(f.rule).toBe(RLS_PREDICATE_UNKNOWN_FIELD); expect(f.path).toBe('permissions[0].rowLevelSecurity[0].check'); expect(f.message).toMatch(/PermissionDeniedError/); - // The write path has the SAME asymmetry, measured against the same controls: - // a positive phantom constraint refuses the post-image, a negated one is - // satisfied vacuously and permits the write the policy was written to refuse. - expect(f.message).toMatch(/permits exactly the writes it was written to refuse/); + // ⚠️ The write leg says the SAME thing the read leg does — one direction — + // and it is the leg whose old text was not merely stale but misattributed: + // it credited a fail-closed to the `extractTargetField` safety net, and + // `computeWriteCheckFilter` never had one. The vacuous-permit sentence + // survives as an explicit statement about an OLDER runtime, so an operator + // reading this against a deployment that predates the guard is not told the + // wrong thing. + expect(f.message).toMatch(/On a runtime older than that guard this clause failed OPEN/); + expect(f.message).toMatch(/PERMITTED exactly the writes the policy was written to refuse/); expect(f.message).not.toMatch(/select \/ update \/ delete matches ZERO/); }); }); @@ -803,23 +820,28 @@ describe('validateRlsPredicateEnforceability — the reference pass never throws }); }); -describe('validateRlsPredicateEnforceability — the fail-OPEN field shapes are reported too', () => { +describe('validateRlsPredicateEnforceability — the once-fail-OPEN field shapes are reported too', () => { /** * The half the card's escalation clause did not name. It asked for a * fail-OPEN *variable*, and the compiler refuses those in every position; the - * hole is field-shaped instead. + * hole was field-shaped instead. * - * `extractTargetField` matches only a LEADING `field ==` / `=` / `in`, so for - * each shape below the safety net returns `null`, the policy is KEPT, and the - * phantom column lowers to a negated constraint that a row without that - * column satisfies (`noValueSatisfiesNegation`). Measured: 3 of 3 rows, - * against 1 of 3 for the real narrowing and 0 of 3 for the same phantom - * column in a positive position — read path and write path alike. + * `extractTargetField` matched only a LEADING `field ==` / `=` / `in`, so for + * each shape below the safety net returned `null`, the policy was KEPT, and + * the phantom column lowered to a negated constraint that a row without that + * column satisfies (`noValueSatisfiesNegation`). Measured on the runtime of + * the day: 3 of 3 rows, against 1 of 3 for the real narrowing and 0 of 3 for + * the same phantom column in a positive position — read path and write path + * alike, the write path being the worse one because it had no + * field-existence net at all. * - * The runtime repair is #17042 and is deliberately NOT attempted here. What - * this rule owes is that the miss is REPORTED in these positions too, which - * is what these cases pin: a rule that only caught the leading position would - * satisfy the card and miss the dangerous half entirely. + * ⚠️ The runtime has since been repaired: `RLSCompiler.compileFilter` judges + * column existence on the COMPILED predicate, which both faces pass through, + * so all five shapes now fail CLOSED. That does NOT retire these cases — + * `noValueSatisfiesNegation` is deliberately unchanged, so the shapes are + * still exactly the ones whose miss used to invert, and DETECTING them is + * still this rule's job. A rule that only caught the leading position would + * satisfy the card and miss the half that was dangerous. */ it.each([ ['a bare negation', 'nope_a != "x"'], diff --git a/packages/lint/src/validate-rls-predicate-enforceability.ts b/packages/lint/src/validate-rls-predicate-enforceability.ts index e1b5efaf0b..03439c52c9 100644 --- a/packages/lint/src/validate-rls-predicate-enforceability.ts +++ b/packages/lint/src/validate-rls-predicate-enforceability.ts @@ -502,12 +502,22 @@ function filterFieldPaths(filter: Record | null): Set { /** * What a reference miss costs at request time, per clause. Measured, not inferred. * - * ⚠️ The two KINDS do not have the same failure direction, and the field half - * does not have ONE direction. An unresolved variable is refused by the compiler - * in every position, so it always fails closed. A missing FIELD fails closed or - * fails OPEN depending on where in the predicate it sits, and the message says - * which — an author told "this denies everything" about a predicate that in fact - * matches everything would harden exactly the wrong thing. + * ⚠️ This text was rewritten once, and the reason it was wrong is worth keeping: + * it said the field half had TWO directions — fail-closed in the leading + * position `extractTargetField` recognises, fail-OPEN everywhere else — and it + * attributed the WRITE leg's fail-closed to that same safety net. Both halves + * were wrong to leave standing. The runtime now judges column existence on the + * COMPILED predicate, inside `RLSCompiler.compileFilter`, which both the read + * layer and the ADR-0058 D4 write gate pass through: position and polarity are + * normalised away before the check runs, so a miss fails CLOSED everywhere, on + * both clauses. And the write path never had an `extractTargetField` net to + * credit — `computeWriteCheckFilter` compiled `check` with no field-existence + * check at all, which is why a negated miss there PERMITTED the write until the + * compiler-side guard landed. + * + * ⇒ Both KINDS now have one direction each, and it is the same direction. What + * this text still owes an author is that the miss is not a harmless typo: it + * turns the policy into a blanket refusal for every holder of the set. */ function referenceConsequence(clause: 'using' | 'check', kind: 'field' | 'variable'): string { if (kind === 'variable') { @@ -529,39 +539,31 @@ function referenceConsequence(clause: 'using' | 'check', kind: 'field' | 'variab 'and behaves as a blanket refusal for every holder of this permission set.'; } - // ── The FIELD half. Which direction it takes is decided by position, and one - // of the two is fail-OPEN. The runtime repair is tracked in #17042; the id - // stays HERE and never in the returned string, because that string reaches - // authors, operators and generated surfaces, none of whom can resolve + // ── The FIELD half. ONE direction, in every position and every polarity, + // since the runtime moved the column check onto the COMPILED predicate. The + // tracker id stays HERE and never in the returned string, because that string + // reaches authors, operators and generated surfaces, none of whom can resolve // `#NNNN` (`check:doc-authoring`, maintainer ruling 2026-08-12). - const closed = - clause === 'using' - ? 'the field-existence safety net in `SecurityPlugin` DROPS the policy and, when it was the only ' + - 'applicable one, arms the `RLS_DENY_FILTER` sentinel — every select / update / delete matches ZERO ' + - 'rows and the object disappears for every holder of this permission set' - : 'the post-image can never satisfy the constraint, so every insert / update the policy governs ' + - 'fails with `PermissionDeniedError` — a blanket refusal for every holder of this permission set'; - const open = - clause === 'using' - ? 'every row inside the tenant wall SATISFIES the negated constraint, so the policy stops narrowing ' + - 'and matches everything the wall admits' - : 'the post-image SATISFIES the negated constraint vacuously, so the check permits exactly the ' + - 'writes it was written to refuse'; - return ( - 'What it costs depends on WHERE the miss sits, and one of the two directions is fail-OPEN. ' + - 'The safety net recognises only a LEADING `field ==` / `=` / `in` — `extractTargetField` is that ' + - `shape match — so a miss THERE fails CLOSED: ${closed}. A miss the net does NOT recognise — a ` + - 'negation (`field != x`, `!(field == x)`, `!(field in [...])`), or any arm after the first — leaves ' + - 'the policy KEPT, and a row that has no such column satisfies a negation: ' + - `${open}. The authored narrowing is then DEFEATED rather than enforced. ` + - 'Measured on the driver-memory matcher and on `matchesFilterCondition` (the write path), each against ' + - 'two controls: the real narrowing selects 1 of 3 rows, the SAME phantom column in a positive position ' + - 'selects 0 of 3, and each negation shape selects 3 of 3. ⛔ It is NOT a cross-tenant leak — tenancy is ' + - 'a separate layer and holds; what is defeated is the narrowing authored INSIDE the wall. ' + - 'driver-mongodb follows the same shared ruling; driver-sql is NOT MEASURED and is expected to fail ' + - 'closed by raising `no such column`. Repairing that is the runtime\'s job, not this rule\'s — what ' + - 'this diagnostic owes you is WHICH direction your predicate is in.' - ); + const dropped = + '`RLSCompiler` judges column existence on the COMPILED predicate, so the position and the polarity ' + + 'you wrote it in make no difference — a leading `field ==`, a negation (`field != x`, ' + + '`!(field == x)`, `!(field in [...])`) and any arm after the first all lower to the same tree and ' + + 'all DROP the policy at request time, with one WARN line as the only signal. '; + return clause === 'using' + ? dropped + + 'When it is the only applicable policy for that object and operation the layer falls back to the ' + + '`RLS_DENY_FILTER` sentinel: every select / update / delete matches ZERO rows, so the object ' + + 'DISAPPEARS for every holder of this permission set — not because they were denied, but because ' + + 'the narrowing they were granted names a column that is not there. When other policies also ' + + 'apply, this one vanishes from the OR and grants none of the access it appears to.' + : dropped + + 'On the ADR-0058 D4 write path that leaves the post-image `check` unsatisfiable: every insert / ' + + 'update the policy governs fails with `PermissionDeniedError`. The policy reads as a write rule ' + + 'and behaves as a blanket refusal for every holder of this permission set. ⚠️ On a runtime older ' + + 'than that guard this clause failed OPEN rather than closed — the write path had no ' + + 'field-existence check at all, so a negated miss was satisfied VACUOUSLY by the post-image and ' + + 'PERMITTED exactly the writes the policy was written to refuse, on every driver. Fix the name ' + + 'rather than relying on either behaviour.'; } /** diff --git a/packages/plugins/plugin-security/src/rls-phantom-column-negation.test.ts b/packages/plugins/plugin-security/src/rls-phantom-column-negation.test.ts new file mode 100644 index 0000000000..e59deb8cb7 --- /dev/null +++ b/packages/plugins/plugin-security/src/rls-phantom-column-negation.test.ts @@ -0,0 +1,376 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#17042] An RLS predicate naming an UNDECLARED column must deny — in every + * position, in every polarity, on BOTH faces. + * + * ## What was measured on `origin/main` @ `91f65c4ea`, before the fix + * + * A predicate naming a column the object does not declare cannot narrow. In a + * NEGATION-carrying position it did not deny either: it WIDENED. Two + * independent sites, each with its own reason, each measured against the same + * two controls — a real column must still narrow, and the SAME phantom column + * in a POSITIVE position must still refuse. A cell whose controls do not + * discriminate is not a reading: the first run of this measurement "refused" + * every write because the driver had failed to boot, and only the controls + * said so. + * + * READ face — `extractTargetField` is a LEADING `==` / `=` / `in` shape match, + * so a negated predicate returned `null`, `if (!targetField) return true` KEPT + * the policy, `dropped` never incremented and the deny sentinel never armed. + * The kept filter then met the settled include-direction ruling + * (`noValueSatisfiesNegation`, driver-memory / driver-mongodb, #13166): a row + * that HAS no such column satisfies "column != x". + * + * `nope != "x"` → `{nope:{$ne:'x'}}` 3 / 3 + * `!(nope == 1)` → `{$not:{nope:1}}` 3 / 3 + * `!(nope in ['a'])` → `{$not:{nope:{$in:['a']}}}` 3 / 3 + * `is_private == false || nope != "x"` → `{$or:[…]}` 3 / 3 + * control `is_private == false` 1 / 3 + * control `nope == false` (positive phantom) 0 / 3 + * + * WRITE face — `computeWriteCheckFilter` compiled `check` clauses with NO + * field-existence net at all, and step 3.6 (ADR-0058 D4) evaluates that filter + * against the post-image. Every negated phantom PERMITTED the write the policy + * was authored to refuse, on both drivers, in both post-image polarities; the + * positive phantom refused, by accident of `looseEq(undefined, value)` being + * false — which is why a suite that only ever tested the positive shape stayed + * green over the hole. + * + * ⚠️ driver-sql and driver-sqlite-wasm were NOT MEASURED when the card was + * filed. They are now: on the READ face they do NOT widen — they fail closed by + * RAISING `INVALID_FILTER` / 400 — so the read face is driver-DEPENDENT. On the + * WRITE face they fail open exactly like every other driver, because the + * `check` is evaluated in-process and never reaches SQL. The write face is the + * worse one and no driver choice mitigates it. + * + * ## What this file pins + * + * The repair is ONE seam: `RLSCompiler.compileFilter` — the single choke point + * both faces already pass through — judges every column the policy names on the + * COMPILED FilterCondition tree. That is positional-agnostic by construction: + * the pushdown compiler lowers `!` to `$not`, `||` to `$or` and `&&` to `$and`, + * so a column lands as a plain object key whatever position it was authored in, + * and there is no spelling of negation left for a shape match to miss. + * + * ⛔ `noValueSatisfiesNegation` is NOT touched and must not be: it is correct + * for an ordinary user query, and re-semanticing every filter in the repo to + * fix one caller is the blast radius this card must not take. The first + * describe below pins that directly — the raw matcher STILL admits 3 of 3 rows + * for the same filter — so a future reader can see that what changed is that + * the policy compiler stopped PRODUCING the filter, not what the matcher does + * with one. + * + * ⭐ The load-bearing control is "a real column still narrows". A fix that made + * every policy deny would pass a naive red→green and break every install, so it + * is asserted on both faces and in both directions on the write face. + */ + +import { describe, it, expect, afterEach, vi } from 'vitest'; +import { matchesFilterCondition } from '@objectstack/formula'; +import { ObjectQL } from '@objectstack/objectql'; +import { SqlDriver } from '@objectstack/driver-sql'; +import { SqliteWasmDriver } from '@objectstack/driver-sqlite-wasm'; +import { PermissionSetSchema } from '@objectstack/spec/security'; +import type { PermissionSet } from '@objectstack/spec/security'; +import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; +import { SecurityPlugin } from './security-plugin.js'; +import { defaultPermissionSets } from './objects/default-permission-sets.js'; + +// ── the fixture, transcribed from the card: three rows, one of them public ── + +const ROWS: Array> = [ + { id: 'r1', title: 'one', is_private: false, owner: 'a@e.example' }, + { id: 'r2', title: 'two', is_private: true, owner: 'b@e.example' }, + { id: 'r3', title: 'three', is_private: true, owner: 'c@e.example' }, +]; + +/** Exactly the columns `qa_doc` declares. `nope` is deliberately absent. */ +const DECLARED: ReadonlySet = new Set(['id', 'title', 'is_private', 'owner']); + +const OBJECTS = [ + { + name: 'qa_doc', + label: 'Doc', + sharingModel: 'public_read_write', + fields: { + id: { name: 'id', type: 'text', primaryKey: true }, + title: { name: 'title', type: 'text' }, + is_private: { name: 'is_private', type: 'boolean' }, + owner: { name: 'owner', type: 'text' }, + }, + }, +]; + +/** + * The four shapes the card measured. Each names `nope`, which `qa_doc` does not + * declare, in a position `extractTargetField` does not recognise. + */ +const PHANTOM_NEGATIONS: Array<[label: string, cel: string]> = [ + ['a bare `!=`', 'nope != "x"'], + ['`!` over an equality', '!(nope == 1)'], + ['`!` over a membership', '!(nope in ["a"])'], + ['a trailing `||` arm, past the leading shape match', 'is_private == false || nope != "x"'], +]; + +/** The narrowing an author actually meant — the over-fix control. */ +const REAL_COLUMN = 'is_private == false'; +/** The same phantom column in a POSITIVE position: refused before and after. */ +const PHANTOM_POSITIVE = 'nope == false'; + +const policy = (clause: 'using' | 'check', cel: string): never => + ({ object: 'qa_doc', operation: 'select', [clause]: cel }) as never; + +const CTX = { userId: 'usr_a', tenantId: 'org1', positions: ['reader'] } as never; + +// ── 1. the matcher ruling, pinned as UNCHANGED ───────────────────────────── + +describe('[#17042] the include-direction ruling is untouched — the compiler stopped producing the filter', () => { + it('a row with no such column STILL satisfies a negation at the matcher (3 of 3)', () => { + // ⛔ This is `noValueSatisfiesNegation`'s reading (#13166), shared with + // driver-mongodb, and it is deliberately preserved: it is correct for an + // ordinary user query. If this cell ever goes to 0 the fix was applied in + // the wrong place and every filter in the repo has been re-semanticed. + for (const filter of [ + { nope: { $ne: 'x' } }, + { $not: { nope: 1 } }, + { $not: { nope: { $in: ['a'] } } }, + { $or: [{ is_private: false }, { nope: { $ne: 'x' } }] }, + ]) { + expect(ROWS.filter((r) => matchesFilterCondition(r as never, filter as never)).length).toBe(3); + } + // …and the two controls the 3/3 depends on to be a reading. + expect(ROWS.filter((r) => matchesFilterCondition(r as never, { is_private: false } as never)).length).toBe(1); + expect(ROWS.filter((r) => matchesFilterCondition(r as never, { nope: false } as never)).length).toBe(0); + }); +}); + +// ── 2. the seam itself ───────────────────────────────────────────────────── + +describe('[#17042] RLSCompiler.compileFilter — a phantom column denies in every position', () => { + const compiler = new RLSCompiler(); + const guard = { declared: DECLARED }; + + for (const clause of ['using', 'check'] as const) { + for (const [label, cel] of PHANTOM_NEGATIONS) { + it(`${clause}: ${label} → RLS_DENY_FILTER`, () => { + const filter = compiler.compileFilter([policy(clause, cel)], CTX, clause, guard); + expect(filter).toEqual(RLS_DENY_FILTER); + // The sentinel is a REFUSAL, and a refusal admits nothing. + expect(ROWS.filter((r) => matchesFilterCondition(r as never, filter as never)).length).toBe(0); + }); + } + + it(`${clause}: the SAME phantom in a positive position also denies`, () => { + expect(compiler.compileFilter([policy(clause, PHANTOM_POSITIVE)], CTX, clause, guard)).toEqual(RLS_DENY_FILTER); + }); + + it(`${clause}: ⭐ a REAL column still narrows — the over-fix control`, () => { + const filter = compiler.compileFilter([policy(clause, REAL_COLUMN)], CTX, clause, guard); + expect(filter).toEqual({ is_private: false }); + expect(ROWS.filter((r) => matchesFilterCondition(r as never, filter as never)).length).toBe(1); + }); + } + + it('a phantom named on the RIGHT of a field-to-field comparison denies too', () => { + // `cel-to-filter` emits `{ title: { $eq: { $field: 'nope' } } }` here, so + // the column is not a key at all — the walker has to find it inside the + // operator bag. A guard that only read node keys would miss this. + expect(compiler.compileFilter([policy('using', 'title == nope')], CTX, 'using', guard)).toEqual(RLS_DENY_FILTER); + // control: the same shape between two REAL columns compiles. + expect(compiler.compileFilter([policy('using', 'title == owner')], CTX, 'using', guard)).not.toEqual(RLS_DENY_FILTER); + }); + + it('a policy naming a phantom does not survive by riding alongside a valid sibling', () => { + // The valid sibling still grants, so this is NOT a deny — but the phantom + // arm must contribute nothing rather than OR-ing in an allow-all. + const filter = compiler.compileFilter( + [policy('using', REAL_COLUMN), policy('using', 'nope != "x"')], + CTX, + 'using', + guard, + ); + expect(filter).toEqual({ is_private: false }); + expect(ROWS.filter((r) => matchesFilterCondition(r as never, filter as never)).length).toBe(1); + }); + + it('⛔ WITHOUT a guard the compile is byte-identical to before — a schema that will not load makes no denials', () => { + // `getObjectFieldNames` answers `null` at boot before the registry is + // populated. That must keep every policy, exactly as it did, rather than + // manufacturing a deny for an object whose columns are simply not known yet. + for (const [, cel] of PHANTOM_NEGATIONS) { + expect(compiler.compileFilter([policy('using', cel)], CTX)).not.toEqual(RLS_DENY_FILTER); + } + }); +}); + +// ── 3. both faces, end to end, on both drivers ───────────────────────────── + +const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!; +const SYS_CTX = { isSystem: true, userId: 'usr_system' }; +const CALLER = { + userId: 'usr_a', + email: 'a@e.example', + positions: ['reader'], + permissions: ['qa_reader'], + posture: 'MEMBER', +}; + +const engines: ObjectQL[] = []; +afterEach(async () => { + while (engines.length) { + try { await engines.pop()?.destroy(); } catch { /* noop */ } + } +}); + +function permissionSet(using: string, check?: string): PermissionSet { + return PermissionSetSchema.parse({ + name: 'qa_reader', + objects: { qa_doc: { allowRead: true, allowCreate: true, allowEdit: true } }, + rowLevelSecurity: [ + { name: 'qa_doc_policy', object: 'qa_doc', operation: 'all', using, ...(check ? { check } : {}) }, + ], + }); +} + +async function boot(makeDriver: () => unknown, ps: PermissionSet): Promise { + const engine = new ObjectQL(); + engine.registerDriver(makeDriver() as never, true); + await engine.init(); + engine.registerApp({ + id: 'com.objectstack.qa.rls-phantom-column-negation-17042', + name: 'RLS phantom column negation', + version: '1.0.0', + type: 'plugin', + scope: 'system', + objects: OBJECTS, + } as never); + await engine.syncSchemas(); + engines.push(engine); + const services: Record = { + manifest: { register: vi.fn() }, + objectql: engine, + metadata: { + get: async (_type: string, name: string) => engine.getSchema(name) ?? null, + list: async () => [MEMBER_DEFAULT, ps], + }, + }; + const ctx = { + logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn() }, + registerService: vi.fn(), + getService: (name: string) => { + if (!(name in services)) throw new Error(`service not registered: ${name}`); + return services[name]; + }, + }; + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' }); + await plugin.init(ctx as never); + await plugin.start(ctx as never); + // The expected refusals log at WARN through the engine's own logger. + vi.spyOn((engine as unknown as { logger: { warn: () => void } }).logger, 'warn').mockImplementation(() => undefined); + await engine.insert('qa_doc', ROWS as never, { context: SYS_CTX } as never); + return engine; +} + +const DRIVERS: Array<[string, () => unknown]> = [ + ['driver-sql (better-sqlite3 :memory:)', + () => new SqlDriver({ client: 'better-sqlite3', connection: { filename: ':memory:' }, useNullAsDefault: true } as never)], + ['driver-sqlite-wasm (:memory:)', () => new SqliteWasmDriver({ filename: ':memory:' } as never)], +]; + +interface Outcome { ok: boolean; code?: string; status?: number } + +const attempt = async (run: () => Promise): Promise => { + try { + await run(); + return { ok: true }; + } catch (e) { + const err = e as { code?: string; statusCode?: number; status?: number }; + return { ok: false, code: err.code, status: err.statusCode ?? err.status }; + } +}; + +for (const [driverName, makeDriver] of DRIVERS) { + describe(`[#17042] READ face, end to end — ${driverName}`, () => { + for (const [label, cel] of PHANTOM_NEGATIONS) { + it(`${label} → zero rows, and no raise`, async () => { + const engine = await boot(makeDriver, permissionSet(cel)); + // ⚠️ Before the fix this THREW `INVALID_FILTER` / 400 on both SQL + // drivers — the phantom column reached the statement builder. A zero + // here is the deny sentinel doing its job, and the controls below are + // what separate it from "the harness returns nothing". + const rows = (await engine.find('qa_doc', { context: CALLER } as never)) as unknown[]; + expect(rows).toHaveLength(0); + }); + } + + it('⭐ a REAL column still narrows to 1 of 3 — the over-fix control', async () => { + const engine = await boot(makeDriver, permissionSet(REAL_COLUMN)); + const rows = (await engine.find('qa_doc', { context: CALLER } as never)) as Array>; + expect(rows).toHaveLength(1); + expect(rows[0]!.id).toBe('r1'); + }); + + it('the same phantom in a positive position is still zero rows', async () => { + const engine = await boot(makeDriver, permissionSet(PHANTOM_POSITIVE)); + expect((await engine.find('qa_doc', { context: CALLER } as never)) as unknown[]).toHaveLength(0); + }); + }); + + describe(`[#17042] WRITE face, end to end — ${driverName}`, () => { + /** + * ⚠️ A SINGLE object, never an array. Step 3.6 is guarded by + * `!Array.isArray(opCtx.data)`, so a bulk payload skips the check gate + * entirely and every cell below would read "permitted" for a reason that + * has nothing to do with this card. + */ + const insert = (engine: ObjectQL, isPrivate: boolean) => + engine.insert( + 'qa_doc', + { id: 'w1', title: 'w', is_private: isPrivate, owner: 'a@e.example' } as never, + { context: CALLER } as never, + ); + + const stored = async (engine: ObjectQL) => + ((await engine.find('qa_doc', { where: { id: 'w1' }, context: SYS_CTX } as never)) as unknown[]).length; + + for (const [label, cel] of PHANTOM_NEGATIONS) { + for (const isPrivate of [false, true]) { + it(`check ${label}, post-image is_private=${isPrivate} → refused, nothing stored`, async () => { + const engine = await boot(makeDriver, permissionSet(REAL_COLUMN, cel)); + const outcome = await attempt(() => insert(engine, isPrivate)); + // Asserted on the ADR-0112 envelope, never on a bare throw: a driver + // raising a raw `Error` would satisfy `toThrow()` and prove nothing. + expect(outcome.ok).toBe(false); + expect(outcome.code).toBe('PERMISSION_DENIED'); + expect(outcome.status).toBe(403); + // "the gate refused" and "nothing landed" are separate facts. + expect(await stored(engine)).toBe(0); + }); + } + } + + it('⭐ a REAL column check still ADMITS the post-image that satisfies it', async () => { + const engine = await boot(makeDriver, permissionSet(REAL_COLUMN, REAL_COLUMN)); + const outcome = await attempt(() => insert(engine, false)); + expect(outcome.ok).toBe(true); + expect(await stored(engine)).toBe(1); + }); + + it('⭐ …and still REFUSES the post-image that violates it', async () => { + const engine = await boot(makeDriver, permissionSet(REAL_COLUMN, REAL_COLUMN)); + const outcome = await attempt(() => insert(engine, true)); + expect(outcome.ok).toBe(false); + expect(outcome.code).toBe('PERMISSION_DENIED'); + expect(await stored(engine)).toBe(0); + }); + + it('the same phantom in a positive position is still refused', async () => { + const engine = await boot(makeDriver, permissionSet(REAL_COLUMN, PHANTOM_POSITIVE)); + const outcome = await attempt(() => insert(engine, false)); + expect(outcome.ok).toBe(false); + expect(outcome.code).toBe('PERMISSION_DENIED'); + expect(await stored(engine)).toBe(0); + }); + }); +}