diff --git a/.changeset/19953-rls-check-default-composition-text.md b/.changeset/19953-rls-check-default-composition-text.md index 3255e28b15f..982c5e6cee6 100644 --- a/.changeset/19953-rls-check-default-composition-text.md +++ b/.changeset/19953-rls-check-default-composition-text.md @@ -16,7 +16,7 @@ Text only. No schema shape, accepted value or runtime behaviour changes. A policy is applicable when it is not `enabled: false`, its `object` is the written object or `'*'`, its `operation` is the write's own or `'all'`, and the caller holds one of its `positions` when it lists any. A `check` on a `select` or `delete` policy is never evaluated. -The check runs on the new row of a single-record insert and of a by-id update. An array insert and a `multi: true` update are not post-image checked; those are tracked in #19964 and #19950, and the texts now say so instead of implying every insert and update is checked. +When this text change was written, the check ran on the new row of a single-record insert and of a by-id update only, and the texts said so. The same release extends it: every row of an array insert and of a `multi: true` update is judged (#19964, #19950), and a by-id update is also judged on the row its `beforeUpdate` hooks leave (#19989). The texts now state that every row an insert or an update writes is judged (#19967). - **`@objectstack/spec`**: the `check` describe and TSDoc state this composition and that scope. The `rowLevelSecurity[].priority` refusal no longer gives "applicable policies OR-combine (most permissive wins)" as its reason, which is not true of the write check, and the file overview limits "OR-combine" to reads. The generated reference pages (`references/security/rls`, `references/security/permission`) are regenerated from the describe. - **`@objectstack/lint`**: the `rls-predicate-*` findings on a `using` now also say what the dropped `using` does to an insert. On an `insert` or `all` policy, when no applicable policy for the insert declares a `check`, that `using` is also the single-record insert check. If nothing else in that set compiles, every single-record insert the policy governs is refused with `PermissionDeniedError`. The findings on a `check` now say the refusal is a blanket one only when no other applicable policy declares a `check` that compiles. diff --git a/.changeset/19967-rls-using-check-texts.md b/.changeset/19967-rls-using-check-texts.md new file mode 100644 index 00000000000..eae1a908a3e --- /dev/null +++ b/.changeset/19967-rls-using-check-texts.md @@ -0,0 +1,16 @@ +--- +"@objectstack/spec": patch +--- + +fix(spec): the RLS `using` / `check` texts say what the write gate enforces today — every row an insert or an update writes is checked, a check-only `update` policy is legal, and an `insert` policy's `using` is its check when none is declared (#19967) + +Clause-②: no + +Text only. No schema shape, accepted value or runtime behaviour changes. + +- **`RowLevelSecurityPolicySchema.check`**: the describe and TSDoc said the check ran on the new row of a single-record insert or a by-id update, and that an array insert and a `multi: true` update were not post-image checked. Every insert and update shape is now judged: each row of an insert, an array insert included, as its `beforeInsert` hooks leave it, and each row an update changes, by id or `multi: true`, as the prior row merged with the final payload after its `beforeUpdate` hooks. One failing row refuses the whole write. A by-id update is also judged, before its hooks, on the change set as sent. +- **`RowLevelSecurityPolicySchema.using`**: the describe called it a filter for SELECT/UPDATE/DELETE and "optional for INSERT-only policies", and the TSDoc said UPDATE requires it. It now says, per operation, which rows it admits, that it stands in as the check on an insert or update when no applicable policy declares `check` (on an `insert` policy that is its only effect), that a `select` or `delete` policy needs it, and that an `insert`, `update` or `all` policy may declare `check` alone. +- **The "at least one of `using` or `check`" refusal**: its head is unchanged. It no longer says an UPDATE policy must provide `using` or that an INSERT policy must provide `check`. It names what each operation takes. +- **OR-combination**: the schema overview, the `priority` tombstone notes and the `os migrate meta --from 16` prose for the `priority` removal no longer say applicable policies OR-combine with "most permissive wins" without qualification. That holds on reads. On a write, the check is chosen per operation across the applicable policies first, and the chosen predicates then OR-combine. +- **Default deny**: the overview now limits "default deny" to the policies that apply. A caller to whom no policy applies is not restricted by the policies; the tenant wall still applies. +- The generated reference pages (`references/security/rls`, `references/security/permission`) and `docs/protocol-upgrade-guide.md` are regenerated from these sources. diff --git a/.changeset/rls-check-defaults-to-using.md b/.changeset/rls-check-defaults-to-using.md index 6069f15eceb..e42de6c8fb3 100644 --- a/.changeset/rls-check-defaults-to-using.md +++ b/.changeset/rls-check-defaults-to-using.md @@ -10,7 +10,7 @@ Clause-②: no (narrowing) **BREAKING**: this narrows the set of writes the write gate accepts. A write that is admitted today can be refused after this change. It ships as `minor` under the launch-window convention, the same way the insert-side `check` reorder did (#16805). -The published contract has always said this. `RowLevelSecurityPolicySchema.check` reads "defaults to USING clause if not specified", and PostgreSQL treats a policy without `WITH CHECK` the same way. The write gate did not do it. It compiled only the policies that declared `check`, so a policy with only a `using` never checked a write. With `using: "record.status != 'closed'"`, a caller could INSERT a closed row. The row was stored even though the same caller could not read it afterwards. +The published contract has always said this. `RowLevelSecurityPolicySchema.check` read "defaults to USING clause if not specified" (it now states the default per operation across the applicable policies, #19953), and PostgreSQL treats a policy without `WITH CHECK` the same way. The write gate did not do it. It compiled only the policies that declared `check`, so a policy with only a `using` never checked a write. With `using: "record.status != 'closed'"`, a caller could INSERT a closed row. The row was stored even though the same caller could not read it afterwards. **Writes that are now refused.** Each refusal is the existing row-level CHECK denial, `403 PERMISSION_DENIED`, and nothing is stored. There is no transition switch. diff --git a/content/docs/permissions/authorization.mdx b/content/docs/permissions/authorization.mdx index a6b1f43be4b..564126bf39e 100644 --- a/content/docs/permissions/authorization.mdx +++ b/content/docs/permissions/authorization.mdx @@ -106,8 +106,14 @@ implementation detail: Keys never collide across packages because object api names are package-namespaced. 3. **RLS: OR within an object, AND with tenant-global.** Multiple row policies - for the same object/operation OR-combine; the tenant wall (Layer 0, ADR-0095 - D1) is a separate always-first AND conjunct, not an OR-mergeable policy. + for the same object/operation OR-combine their `using` on reads and on the + rows a write may target. The `check` on the rows an insert or update writes + is chosen per operation first: when any applicable policy declares `check`, + only the declared checks decide (OR-combined) and a USING-only policy adds + nothing; otherwise each `using` stands in (OR-combined) — see + [the fail-closed contract](/docs/permissions/rls#the-fail-closed-contract). + The tenant wall (Layer 0, ADR-0095 D1) is a separate always-first AND + conjunct, not an OR-mergeable policy. `viewAllRecords` / `modifyAllRecords` (super-user bypass, posture-gated) short-circuit the object's *business* RLS. Crossing the **tenant wall**, though, requires the **`PLATFORM_ADMIN` posture** (ADR-0099 D1). That posture diff --git a/content/docs/permissions/rls.mdx b/content/docs/permissions/rls.mdx index 04b68f2bd23..9196964ffb1 100644 --- a/content/docs/permissions/rls.mdx +++ b/content/docs/permissions/rls.mdx @@ -60,7 +60,7 @@ export const ContributorAccess = definePermissionSet({ | `object` | `string` | Target object — or `'*'` to apply to every object | | `operation` | `'select' \| 'insert' \| 'update' \| 'delete' \| 'all'` | Which operation the policy guards. A `select` policy also bounds writes when no write-class policy applies — see below | | `using` | `string` | Predicate for rows the user may **see / act on** (compiled into the query filter) | -| `check` | `string` | Predicate the new row must satisfy **after a single-record insert or a by-id update**; an array insert and a `multi: true` update are not checked. Omit it and `using` stands in, unless another applicable policy for the same operation declares a `check` — see [the fail-closed contract](#the-fail-closed-contract) | +| `check` | `string` | Predicate **every row an insert or an update writes** must satisfy: each row of an insert (an array insert included) as its `beforeInsert` hooks leave it, and each row an update changes (by id or `multi: true`) after its `beforeUpdate` hooks. One failing row refuses the whole write. Omit it and `using` stands in, unless another applicable policy for the same operation declares a `check` — see [the fail-closed contract](#the-fail-closed-contract) | | `positions` | `string[]` | Which positions the policy applies to. Omit = everyone | | `enabled` | `boolean` | Default `true`. `false` switches the policy off — a disabled policy is not evaluated | diff --git a/content/docs/protocol/objectql/security.mdx b/content/docs/protocol/objectql/security.mdx index 28eabc88197..8259705cd41 100644 --- a/content/docs/protocol/objectql/security.mdx +++ b/content/docs/protocol/objectql/security.mdx @@ -141,7 +141,7 @@ Return filtered result ## 2. Row-Level Security -Row-level filtering is expressed as **RLS policies** (`RowLevelSecurityPolicySchema` in `packages/spec/src/security/rls.zod.ts`). Policies carry a **CEL** `using` clause (for SELECT/UPDATE/DELETE) and/or a `check` clause (for the new row of a single-record INSERT or a by-id UPDATE; an array insert and a `multi: true` update are not checked) — canonical CEL since ADR-0058; a legacy SQL-style `=`/`IN (...)` predicate still compiles via a **deprecated bridge** (warns). On reads, multiple policies for one object are combined with OR; for the write `check`, see `RowLevelSecurityPolicySchema.check`. Available context variables are the **unique identifiers and membership sets** the runtime pre-resolves: equality predicates may use `current_user.id`, `current_user.email` (the unique, seedable owner anchor), or `current_user.organization_id`; set-membership predicates may use `id in current_user.org_user_ids`, `'manager' in current_user.positions`, or any §7.3.1 set staged in `ExecutionContext.rlsMembership`. Display `name` and arbitrary user fields are **intentionally not** resolvable — only unique identifiers, so an ownership predicate can never leak access through a name collision. +Row-level filtering is expressed as **RLS policies** (`RowLevelSecurityPolicySchema` in `packages/spec/src/security/rls.zod.ts`). Policies carry a **CEL** `using` clause (the rows a SELECT reads and an UPDATE or DELETE may target) and/or a `check` clause (judged on every row an INSERT or UPDATE writes, an array insert and a `multi: true` update included; when no applicable policy declares `check`, each `using` stands in) — canonical CEL since ADR-0058; a legacy SQL-style `=`/`IN (...)` predicate still compiles via a **deprecated bridge** (warns). On reads, multiple policies for one object are combined with OR; for the write `check`, see `RowLevelSecurityPolicySchema.check`. Available context variables are the **unique identifiers and membership sets** the runtime pre-resolves: equality predicates may use `current_user.id`, `current_user.email` (the unique, seedable owner anchor), or `current_user.organization_id`; set-membership predicates may use `id in current_user.org_user_ids`, `'manager' in current_user.positions`, or any §7.3.1 set staged in `ExecutionContext.rlsMembership`. Display `name` and arbitrary user fields are **intentionally not** resolvable — only unique identifiers, so an ownership predicate can never leak access through a name collision. Policies can be attached to a permission set via its `rowLevelSecurity` array, or registered as standalone metadata. diff --git a/content/docs/references/security/permission.mdx b/content/docs/references/security/permission.mdx index bfafecaf663..f2a73a2c7c9 100644 --- a/content/docs/references/security/permission.mdx +++ b/content/docs/references/security/permission.mdx @@ -167,8 +167,8 @@ const result = AdminScopeSchema.parse(data); | **description** | `string` | optional | Policy description and business justification | | **object** | `string` | ✅ | Target object name | | **operation** | `Enum<'select' \| 'insert' \| 'update' \| 'delete' \| 'all'>` | ✅ | Database operation this policy applies to | -| **using** | `string` | optional | Filter condition for SELECT/UPDATE/DELETE, authored in canonical CEL (ADR-0058 D1). It enforces when the predicate lowers to an ObjectQL filter: a field compared against a literal or a `current_user.*` context value using `==`, `!=`, `<`, `<=`, `>` or `>=`; `in` against a `current_user.*` array or an inline literal list (e.g. status in ['draft', 'pending']); these combined with `&&` / `\|\|`; or the bare allow-all `true`. Anything that does not lower fails closed — the policy matches zero rows. The legacy SQL-ish spellings are still accepted through a transitional bridge that rewrites `=` to `==` and `IN` to `in` (deprecated under ADR-0058 D1); SQL `AND` / `OR` / `NOT IN` / `IS NULL` / `LIKE` are NOT bridged and fail closed. Optional for INSERT-only policies. | -| **check** | `string` | optional | Validation condition matched against the new row of a single-record INSERT or a by-id UPDATE (enforced at application level); an array insert and a `multi: true` update are not post-image checked. The default to `using` is decided per operation across the applicable policies, not per policy: when any applicable policy for that operation declares `check`, only the declared checks decide (OR-combined) and a USING-only sibling adds nothing; only when none declares `check` does each applicable policy's `using` stand in as its check (OR-combined). Applicable = not `enabled: false`, `object` matches or is '*', `operation` matches or is 'all', and the caller holds one of its `positions` when it lists any. Refused on a policy whose `operation` is `select` or `delete`, which write no new row to check: limit the rows such a policy admits with `using`, and declare the `check` on an `insert`, `update` or `all` policy. | +| **using** | `string` | optional | Row predicate, authored in canonical CEL (ADR-0058 D1): the rows a `select` policy lets a caller read, and the existing rows an `update` or `delete` policy lets a caller change or remove. On an insert or an update, when no applicable policy for that operation declares `check`, each applicable policy's `using` also stands in as its check on every row written (see `check`); on an `insert` policy that is its only effect. It enforces when the predicate lowers to an ObjectQL filter: a field compared against a literal or a `current_user.*` context value using `==`, `!=`, `<`, `<=`, `>` or `>=`; `in` against a `current_user.*` array or an inline literal list (e.g. status in ['draft', 'pending']); these combined with `&&` / `\|\|`; or the bare allow-all `true`. Anything that does not lower fails closed — the policy matches zero rows. The legacy SQL-ish spellings are still accepted through a transitional bridge that rewrites `=` to `==` and `IN` to `in` (deprecated under ADR-0058 D1); SQL `AND` / `OR` / `NOT IN` / `IS NULL` / `LIKE` are NOT bridged and fail closed. Needed on a `select` or `delete` policy (a `check` there is refused); optional on an `insert`, `update` or `all` policy that declares `check`. | +| **check** | `string` | optional | Validation condition judged on every row an insert or an update writes (enforced at application level): each row of an insert, an array insert included, as its `beforeInsert` hooks leave it, and each row an update changes, by id or `multi: true`, as the prior row merged with the final payload after its `beforeUpdate` hooks. One failing row refuses the whole write. A by-id update is also judged, before its hooks, on the prior row merged with the change set as sent. The default to `using` is decided per operation across the applicable policies, not per policy: when any applicable policy for that operation declares `check`, only the declared checks decide (OR-combined) and a USING-only sibling adds nothing; only when none declares `check` does each applicable policy's `using` stand in as its check (OR-combined). Applicable = not `enabled: false`, `object` matches or is '*', `operation` matches or is 'all', and the caller holds one of its `positions` when it lists any. Refused on a policy whose `operation` is `select` or `delete`, which write no new row to check: limit the rows such a policy admits with `using`, and declare the `check` on an `insert`, `update` or `all` policy. | | **positions** | `string[]` | optional | Positions this policy applies to (omit for all) | | **enabled** | `boolean` | optional (default: `true`) | Whether this policy is active | | **priority** | `never` | optional | [REMOVED] `rowLevelSecurity[].priority` was removed in @objectstack/spec 17.0.0. It never had an effect. Delete the key — policy outcomes are unchanged. Run `os migrate meta --from 16` to list the mechanical edits for existing sources; apply them by hand. | diff --git a/content/docs/references/security/rls.mdx b/content/docs/references/security/rls.mdx index e0b989c21c5..2d5c4fbc1ae 100644 --- a/content/docs/references/security/rls.mdx +++ b/content/docs/references/security/rls.mdx @@ -91,7 +91,12 @@ ObjectStack RLS: ### Security Considerations 1. **Defense in Depth**: RLS is one layer; use with object permissions -2. **Default Deny**: If no policy matches, access is denied +2. **Default Deny, among the policies that apply**: once a policy applies to + the caller and the operation, a row that none of them admits is denied, + and a policy that cannot be evaluated fails closed. When no policy applies, + the policies restrict nothing (the tenant wall, a separate layer, still + applies): object permissions, not RLS policies, stop an un-permissioned + user 3. **Context Variables**: Ensure current_user context is always set See also: https://www.postgresql.org/docs/current/ddl-rowsecurity.html @@ -170,8 +175,8 @@ const result = RLSEvaluationResultSchema.parse(data); | **description** | `string` | optional | Policy description and business justification | | **object** | `string` | ✅ | Target object name | | **operation** | `Enum<'select' \| 'insert' \| 'update' \| 'delete' \| 'all'>` | ✅ | Database operation this policy applies to | -| **using** | `string` | optional | Filter condition for SELECT/UPDATE/DELETE, authored in canonical CEL (ADR-0058 D1). It enforces when the predicate lowers to an ObjectQL filter: a field compared against a literal or a `current_user.*` context value using `==`, `!=`, `<`, `<=`, `>` or `>=`; `in` against a `current_user.*` array or an inline literal list (e.g. status in ['draft', 'pending']); these combined with `&&` / `\|\|`; or the bare allow-all `true`. Anything that does not lower fails closed — the policy matches zero rows. The legacy SQL-ish spellings are still accepted through a transitional bridge that rewrites `=` to `==` and `IN` to `in` (deprecated under ADR-0058 D1); SQL `AND` / `OR` / `NOT IN` / `IS NULL` / `LIKE` are NOT bridged and fail closed. Optional for INSERT-only policies. | -| **check** | `string` | optional | Validation condition matched against the new row of a single-record INSERT or a by-id UPDATE (enforced at application level); an array insert and a `multi: true` update are not post-image checked. The default to `using` is decided per operation across the applicable policies, not per policy: when any applicable policy for that operation declares `check`, only the declared checks decide (OR-combined) and a USING-only sibling adds nothing; only when none declares `check` does each applicable policy's `using` stand in as its check (OR-combined). Applicable = not `enabled: false`, `object` matches or is '*', `operation` matches or is 'all', and the caller holds one of its `positions` when it lists any. Refused on a policy whose `operation` is `select` or `delete`, which write no new row to check: limit the rows such a policy admits with `using`, and declare the `check` on an `insert`, `update` or `all` policy. | +| **using** | `string` | optional | Row predicate, authored in canonical CEL (ADR-0058 D1): the rows a `select` policy lets a caller read, and the existing rows an `update` or `delete` policy lets a caller change or remove. On an insert or an update, when no applicable policy for that operation declares `check`, each applicable policy's `using` also stands in as its check on every row written (see `check`); on an `insert` policy that is its only effect. It enforces when the predicate lowers to an ObjectQL filter: a field compared against a literal or a `current_user.*` context value using `==`, `!=`, `<`, `<=`, `>` or `>=`; `in` against a `current_user.*` array or an inline literal list (e.g. status in ['draft', 'pending']); these combined with `&&` / `\|\|`; or the bare allow-all `true`. Anything that does not lower fails closed — the policy matches zero rows. The legacy SQL-ish spellings are still accepted through a transitional bridge that rewrites `=` to `==` and `IN` to `in` (deprecated under ADR-0058 D1); SQL `AND` / `OR` / `NOT IN` / `IS NULL` / `LIKE` are NOT bridged and fail closed. Needed on a `select` or `delete` policy (a `check` there is refused); optional on an `insert`, `update` or `all` policy that declares `check`. | +| **check** | `string` | optional | Validation condition judged on every row an insert or an update writes (enforced at application level): each row of an insert, an array insert included, as its `beforeInsert` hooks leave it, and each row an update changes, by id or `multi: true`, as the prior row merged with the final payload after its `beforeUpdate` hooks. One failing row refuses the whole write. A by-id update is also judged, before its hooks, on the prior row merged with the change set as sent. The default to `using` is decided per operation across the applicable policies, not per policy: when any applicable policy for that operation declares `check`, only the declared checks decide (OR-combined) and a USING-only sibling adds nothing; only when none declares `check` does each applicable policy's `using` stand in as its check (OR-combined). Applicable = not `enabled: false`, `object` matches or is '*', `operation` matches or is 'all', and the caller holds one of its `positions` when it lists any. Refused on a policy whose `operation` is `select` or `delete`, which write no new row to check: limit the rows such a policy admits with `using`, and declare the `check` on an `insert`, `update` or `all` policy. | | **positions** | `string[]` | optional | Positions this policy applies to (omit for all) | | **enabled** | `boolean` | optional (default: `true`) | Whether this policy is active | | **priority** | `never` | optional | [REMOVED] `rowLevelSecurity[].priority` was removed in @objectstack/spec 17.0.0. It never had an effect. Delete the key — policy outcomes are unchanged. Run `os migrate meta --from 16` to list the mechanical edits for existing sources; apply them by hand. | diff --git a/docs/protocol-upgrade-guide.md b/docs/protocol-upgrade-guide.md index 8fc7260a589..25866bdd6ec 100644 --- a/docs/protocol-upgrade-guide.md +++ b/docs/protocol-upgrade-guide.md @@ -33,7 +33,7 @@ The same graduation covers `wait`, whose fallback was not a config-to-config ren The reconciliation that found those also found `map`, whose executor read a bare `cfg.flowName ?? cfg.flow` for an undeclared `flow` spelling no schema ever described (#4045). A pure rename, graduated the same way, so the executor reads only the canonical `flowName`. -And it removes the RLS-policy key `priority` (#3896 security audit): promised "conflict resolution" that cannot exist, because applicable policies OR-combine (most permissive wins) — there is never a conflict to order, and nothing ever read the key (call graph closed across the collection site, the projection round-trip and the compiler). A pure lossless delete: outcomes are identical with or without it; the schema tombstones the key with the same prescription. +And it removes the RLS-policy key `priority` (#3896 security audit): promised "conflict resolution" that cannot exist, because no outcome depends on an order: applicable policies OR-combine on reads, and a write's check is chosen once per operation across the applicable policies, then OR-combined (see `RowLevelSecurityPolicySchema.check`) — there is never a conflict to order, and nothing ever read the key (call graph closed across the collection site, the projection round-trip and the compiler). A pure lossless delete: outcomes are identical with or without it; the schema tombstones the key with the same prescription. The same close-out retires the four inert tool authoring keys (`category`, `permissions`, `active`, `builtIn`): none is part of AIToolDefinition and no execution path read them. Two were misleading in the dangerous direction — `permissions` promised an invocation gate nothing enforced, and `active: false` read as "withdrawn" while the tool kept reaching the LLM tool set. Lossless deletes; the strict ToolSchema rejects each with its prescription. diff --git a/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts b/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts index f68228572d4..1dc2877baec 100644 --- a/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts +++ b/packages/plugins/plugin-security/src/rls-check-defaults-to-using.test.ts @@ -1,10 +1,13 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. /** - * [ADR-0058 D4] A policy that declares no `check` holds the write post-image to - * its `using`, which is what `RowLevelSecurityPolicySchema.check` publishes: - * "defaults to USING clause if not specified". It is also PostgreSQL's rule for - * a policy without `WITH CHECK`. + * [ADR-0058 D4] When no applicable policy for a write declares `check`, each + * applicable policy's `using` stands in as its check on the written row. The + * default is decided once per write operation across the applicable policies, + * not per policy, as `RowLevelSecurityPolicySchema.check` states (read it + * there; a quote here would go stale). Its starting point is PostgreSQL's rule + * for a policy without `WITH CHECK`; the composition across policies is not + * (see `writeCheckPolicies` in `security-plugin.ts`). * * ## What was measured before the change * diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 844f8f7923c..b0869bf98d2 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -843,9 +843,10 @@ function permissionSetPageOrRefuse(rows: unknown, names: readonly string[]): any * compiles, out of the policies that apply to this principal, object and write * operation (`insert` / `update`, `all` included). * - * The published contract is `RowLevelSecurityPolicySchema.check`: "defaults to - * USING clause if not specified". That is also PostgreSQL's rule: a policy - * without `WITH CHECK` holds new rows to its `USING`. Before this selector the + * The published contract is `RowLevelSecurityPolicySchema.check`, which states + * the per-operation composition below; read it there rather than from a quote + * here. Its starting point is PostgreSQL's rule: a policy without `WITH CHECK` + * holds new rows to its `USING`. Before this selector the * runtime compiled only policies that declared `check`, so a USING-only policy * never gated an INSERT or an UPDATE's new row. An author writing * `record.status != 'closed'` could store a closed row they could then not see. diff --git a/packages/spec/liveness/permission.json b/packages/spec/liveness/permission.json index b82288eb87d..bb0d3a60937 100644 --- a/packages/spec/liveness/permission.json +++ b/packages/spec/liveness/permission.json @@ -192,17 +192,17 @@ }, "using": { "status": "live", - "verifiedAt": "2026-08-28", - "evidence": "packages/plugins/plugin-security/src/rls-compiler.ts#compileFilter (`policy.using` is the predicate on the read pass, and the fallback when a `check` pass finds no `check`)", + "verifiedAt": "2026-09-27", + "evidence": "packages/plugins/plugin-security/src/rls-compiler.ts#compileFilter (`policy.using` is the predicate on the read pass, and on a `check` pass for a policy that declares no `check`); packages/plugins/plugin-security/src/security-plugin.ts#writeCheckPolicies (hands USING-only policies to that `check` pass only when no applicable policy for the write declares `check`)", "proof": "packages/qa/dogfood/test/rls-fixture.dogfood.test.ts#rls-by-id-write", - "note": "compiled into find + analytics SQL. ADR-0054 high-risk class (RLS): the proof boots an owner-isolated fixture so a fresh member cannot read an admin-created row, then asserts the runner's verdict in both directions — `rls-consistent` when the owner predicate also gates the by-id write (#1994 pre-image check) and `rls-hole` when it doesn't. Guards read AND by-id-write enforcement, not just the read predicate. 2026-08-28: RE-ANCHORED (#13003) — path-only citation upgraded to an anchor. Re-closed by hand against c459da6bc." + "note": "compiled into find + analytics SQL. ADR-0054 high-risk class (RLS): the proof boots an owner-isolated fixture so a fresh member cannot read an admin-created row, then asserts the runner's verdict in both directions — `rls-consistent` when the owner predicate also gates the by-id write (#1994 pre-image check) and `rls-hole` when it doesn't. Guards read AND by-id-write enforcement, not just the read predicate. 2026-08-28: RE-ANCHORED (#13003) — path-only citation upgraded to an anchor. Re-closed by hand against c459da6bc. 2026-09-27: the evidence now also names `writeCheckPolicies`, which decides when a `using` stands in as the check (#19967); re-read against 0d3ec47137." }, "check": { "status": "live", - "verifiedAt": "2026-08-28", + "verifiedAt": "2026-09-27", "proof": "packages/qa/dogfood/test/showcase-d3-d4-capabilities.dogfood.test.ts#showcase-d3-d4-capabilities", - "evidence": "packages/plugins/plugin-security/src/rls-compiler.ts#compileFilter (`(policy as { check?: string }).check ?? policy.using` — the post-image pass prefers `check` and falls back to `using`)", - "note": "2026-08-28: RE-ANCHORED (#13003) — path-only citation upgraded to an anchor. Re-closed by hand against c459da6bc." + "evidence": "packages/plugins/plugin-security/src/security-plugin.ts#writeCheckPolicies (the policies that declare `check` when any applicable policy for the write does, otherwise each applicable policy's `using`); packages/plugins/plugin-security/src/rls-compiler.ts#compileFilter (`policyDeclaresClause(policy, 'check') ? check : policy.using` on the `check` pass, OR-combined)", + "note": "2026-08-28: RE-ANCHORED (#13003) — path-only citation upgraded to an anchor. Re-closed by hand against c459da6bc. 2026-09-27: RE-ANCHORED (#19967) — the cited `check ?? policy.using` expression left compileFilter when the per-operation selector `writeCheckPolicies` took over the choice (#19952); both halves re-read against 0d3ec47137." }, "positions": { "status": "live", diff --git a/packages/spec/src/conversions/registry.ts b/packages/spec/src/conversions/registry.ts index cee304a07e5..65b50b25250 100644 --- a/packages/spec/src/conversions/registry.ts +++ b/packages/spec/src/conversions/registry.ts @@ -1919,9 +1919,11 @@ const appAreaFailOpenGatesRemoved: MetadataConversion = { * RLS-policy `priority` removed (protocol 17, #3896 security audit). * * A pure DELETE with no rename target, because the promised semantics never - * existed: applicable policies OR-combine (any match allows access — most - * permissive wins), so there is no conflict for a priority to resolve and - * evaluation order cannot change an outcome. The 2026-07-30 security-subset + * existed: no outcome depends on an order. Applicable policies OR-combine on + * reads (any match allows access), and a write's check is chosen once per + * operation across the applicable policies, then OR-combined + * (`RowLevelSecurityPolicySchema.check`), so there is no conflict for a + * priority to resolve and evaluation order cannot change an outcome. The 2026-07-30 security-subset * liveness re-verification closed the call graph — collection site, projection * round-trip, compiler — and found NO reader, ever. Dropping the key is * therefore strictly lossless: outcomes are identical with or without it. diff --git a/packages/spec/src/migrations/registry.ts b/packages/spec/src/migrations/registry.ts index daae679d9ff..d733956ea4e 100644 --- a/packages/spec/src/migrations/registry.ts +++ b/packages/spec/src/migrations/registry.ts @@ -157,8 +157,10 @@ const step17: MigrationStep = { 'ever described (#4045). A pure rename, graduated the same way, so the ', 'executor reads only the canonical `flowName`.\n\n', 'And it removes the RLS-policy key `priority` (#3896 security audit): promised ', - '"conflict resolution" that cannot exist, because applicable policies OR-combine ', - '(most permissive wins) — there is never a conflict to order, and nothing ever ', + '"conflict resolution" that cannot exist, because no outcome depends on an order: ', + 'applicable policies OR-combine on reads, and a write\'s check is chosen once per ', + 'operation across the applicable policies, then OR-combined (see ', + '`RowLevelSecurityPolicySchema.check`) — there is never a conflict to order, and nothing ever ', 'read the key (call graph closed across the collection site, the projection ', 'round-trip and the compiler). A pure lossless delete: outcomes are identical ', 'with or without it; the schema tombstones the key with the same prescription.\n\n', diff --git a/packages/spec/src/security/rls.test.ts b/packages/spec/src/security/rls.test.ts index 4e31e78b5ad..ac8be4ad73b 100644 --- a/packages/spec/src/security/rls.test.ts +++ b/packages/spec/src/security/rls.test.ts @@ -122,8 +122,9 @@ describe('Row-Level Security (RLS) Protocol', () => { }); it('priority is RETIRED: absent parses clean, authored rejects with the prescription', () => { - // Removed by the 2026-07-30 #3896 security audit: policies OR-combine - // (most permissive wins), so the promised "conflict resolution" cannot + // Removed by the 2026-07-30 #3896 security audit: no outcome depends on + // an order (reads OR-combine; a write's check is chosen per operation, + // then OR-combined), so the promised "conflict resolution" cannot // exist and nothing ever read the key. The tombstone keeps the removal // audible instead of silently stripping an authored value. const policy = { @@ -726,3 +727,70 @@ describe('RowLevelSecurityPolicySchema.using — the published description (#676 expect(description).not.toMatch(/\b(one|two|three|four|five)\s+(compiler-supported\s+)?forms\b/i); }); }); + +// --------------------------------------------------------------------------- +// The "at least one of using / check" refusal tells each operation what it +// takes, and every prescription it gives is one the schema accepts. +// +// The message used to say "For SELECT/UPDATE/DELETE operations, provide +// "using"". False for `update`: a policy that declares only `check` is legal +// and enforced (the showcase's `invoice_owner_immutable` is one). It also +// said an insert takes "check" and nothing else, while a USING-only `insert` +// policy is accepted and its `using` IS the insert check when no applicable +// policy declares one. The pins hold the message's claims to the schema's own +// answers: each operation is named, and each clause the message offers for +// it parses on that operation. +// --------------------------------------------------------------------------- +describe('RowLevelSecurityPolicySchema — the "at least one" refusal', () => { + const base = { name: 'p', object: 'account' }; + const PREDICATE = "status != 'archived'"; + const HEAD = 'At least one of "using" or "check" must be specified.'; + + const refusalOf = (operation: string) => { + const result = RowLevelSecurityPolicySchema.safeParse({ ...base, operation }); + expect(result.success).toBe(false); + const issues = result.success ? [] : result.error.issues; + expect(issues).toHaveLength(1); + expect(issues[0].code).toBe('custom'); + expect(issues[0].path).toEqual([]); + return issues[0].message; + }; + + const parses = (policy: Record) => + RowLevelSecurityPolicySchema.safeParse({ ...base, ...policy }).success; + + it('keeps its head and names every operation, for every operation', () => { + for (const operation of ['select', 'insert', 'update', 'delete', 'all']) { + const message = refusalOf(operation); + expect(message.startsWith(HEAD), operation).toBe(true); + for (const named of ['select', 'insert', 'update', 'delete', 'all']) { + expect(message, `${operation}: names \`${named}\``).toContain(`\`${named}\``); + } + expect(message).not.toContain('For SELECT/UPDATE/DELETE operations, provide "using"'); + } + }); + + it('select / delete take "using", and that prescription parses', () => { + expect(refusalOf('select')).toContain('A `select` or `delete` policy takes "using"'); + for (const operation of ['select', 'delete']) { + expect(parses({ operation, using: PREDICATE }), operation).toBe(true); + } + }); + + it('insert takes "check", or "using" alone as its stand-in check; both parse', () => { + const message = refusalOf('insert'); + expect(message).toContain('An `insert` policy takes "check"'); + expect(message).toContain('a "using" alone is that check when no applicable insert policy declares one'); + expect(parses({ operation: 'insert', check: PREDICATE })).toBe(true); + expect(parses({ operation: 'insert', using: PREDICATE })).toBe(true); + }); + + it('update / all take either or both: a check-only update policy is accepted', () => { + expect(refusalOf('update')).toContain('An `update` or `all` policy takes either or both'); + for (const operation of ['update', 'all']) { + expect(parses({ operation, check: PREDICATE }), `${operation} check-only`).toBe(true); + expect(parses({ operation, using: PREDICATE }), `${operation} using-only`).toBe(true); + expect(parses({ operation, using: PREDICATE, check: PREDICATE }), `${operation} both`).toBe(true); + } + }); +}); diff --git a/packages/spec/src/security/rls.zod.ts b/packages/spec/src/security/rls.zod.ts index 6b2ec57ec01..b61e58636d4 100644 --- a/packages/spec/src/security/rls.zod.ts +++ b/packages/spec/src/security/rls.zod.ts @@ -91,7 +91,12 @@ import { strictObject } from '../shared/strict-object'; * ## Security Considerations * * 1. **Defense in Depth**: RLS is one layer; use with object permissions - * 2. **Default Deny**: If no policy matches, access is denied + * 2. **Default Deny, among the policies that apply**: once a policy applies to + * the caller and the operation, a row that none of them admits is denied, + * and a policy that cannot be evaluated fails closed. When no policy applies, + * the policies restrict nothing (the tenant wall, a separate layer, still + * applies): object permissions, not RLS policies, stop an un-permissioned + * user * 3. **Context Variables**: Ensure current_user context is always set * * @see https://www.postgresql.org/docs/current/ddl-rowsecurity.html @@ -117,8 +122,10 @@ export type RLSOperation = z.input; * Row-Level Security Policy Schema * * Defines a single RLS policy that filters records based on conditions. - * Multiple policies can be defined for the same object, and they are - * combined with OR logic (union of results). + * Multiple policies can be defined for the same object. On a read, their + * `using` clauses are combined with OR logic (union of results); on an insert + * or an update, which predicates judge the written rows is decided per + * operation across the applicable policies (see `check`). * * @example Multi-Tenant Isolation * ```typescript @@ -292,14 +299,34 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( .describe('Database operation this policy applies to'), /** - * USING clause - Filter condition for SELECT/UPDATE/DELETE. + * USING clause - The existing rows the policy admits, and the stand-in + * write check when no applicable policy declares `check`. * * A constrained CEL predicate (ADR-0058 D1) compiled into an ObjectQL * filter (see the supported grammar below). Only rows the compiled filter * matches are accessible. * - * **Note**: For INSERT-only policies, USING is not required (only CHECK is needed). - * For SELECT/UPDATE/DELETE operations, USING is required. + * **What it does, per `operation`**: + * - `select`: the rows a read returns. `delete`: the rows a delete may + * remove. `using` is the only predicate such a policy can carry (a + * non-blank `check` there is refused), so it needs one. + * - `update`: the existing rows an update may change, by id or + * `multi: true`. It is not required: a policy that declares only `check` + * is accepted and enforced. Its `check` judges the rows the update writes, + * and the rows the caller may change then come from the other applicable + * `update` / `all` policies' `using`, or, when none carries one, from the + * caller's `select` policies. + * - `insert`: an insert has no existing row, so `using` filters nothing. + * It is not required either (a `check` alone is enough), but it is not + * inert: it is the insert's check whenever no applicable policy for that + * insert declares `check`. + * - `all`: each of the above for its operation. + * + * **The stand-in check**: on an insert or an update, when no applicable + * policy for that operation declares `check`, each applicable policy's + * `using` is its check, OR-combined, on every row written. When + * any applicable policy declares `check`, only the declared checks decide + * and a USING-only sibling adds nothing (see `check`). * * **Security Note**: the compiler lowers each predicate to a structured * filter and binds context values as parameters at the driver layer — @@ -390,13 +417,27 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( */ using: z.string() .optional() - .describe('Filter condition for SELECT/UPDATE/DELETE, authored in canonical CEL (ADR-0058 D1). It enforces when the predicate lowers to an ObjectQL filter: a field compared against a literal or a `current_user.*` context value using `==`, `!=`, `<`, `<=`, `>` or `>=`; `in` against a `current_user.*` array or an inline literal list (e.g. status in [\'draft\', \'pending\']); these combined with `&&` / `||`; or the bare allow-all `true`. Anything that does not lower fails closed — the policy matches zero rows. The legacy SQL-ish spellings are still accepted through a transitional bridge that rewrites `=` to `==` and `IN` to `in` (deprecated under ADR-0058 D1); SQL `AND` / `OR` / `NOT IN` / `IS NULL` / `LIKE` are NOT bridged and fail closed. Optional for INSERT-only policies.'), + .describe('Row predicate, authored in canonical CEL (ADR-0058 D1): the rows a `select` policy lets a caller read, and the existing rows an `update` or `delete` policy lets a caller change or remove. On an insert or an update, when no applicable policy for that operation declares `check`, each applicable policy\'s `using` also stands in as its check on every row written (see `check`); on an `insert` policy that is its only effect. It enforces when the predicate lowers to an ObjectQL filter: a field compared against a literal or a `current_user.*` context value using `==`, `!=`, `<`, `<=`, `>` or `>=`; `in` against a `current_user.*` array or an inline literal list (e.g. status in [\'draft\', \'pending\']); these combined with `&&` / `||`; or the bare allow-all `true`. Anything that does not lower fails closed — the policy matches zero rows. The legacy SQL-ish spellings are still accepted through a transitional bridge that rewrites `=` to `==` and `IN` to `in` (deprecated under ADR-0058 D1); SQL `AND` / `OR` / `NOT IN` / `IS NULL` / `LIKE` are NOT bridged and fail closed. Needed on a `select` or `delete` policy (a `check` there is refused); optional on an `insert`, `update` or `all` policy that declares `check`.'), /** - * CHECK clause - Validation of the new row of a single-record INSERT or a - * by-id UPDATE. An array insert and a `multi: true` update are not - * post-image checked (#19964, #19950). - * + * CHECK clause - Validation of every row an insert or an update writes, + * enforced at application level through the engine's + * `OperationContext.postHookWriteImageCheck` seam (introduced for inserts + * by commit a016f08b8a, whose original card no longer resolves; extended by + * #19950, #19964 and #19989): + * - an insert, an array insert included: each row as its `beforeInsert` + * hooks leave it. Values the engine fills in after that point (an + * autonumber, a `secret` field's stored reference, an absent tenant + * column) are not on the judged image; + * - an update, by id or `multi: true`: each row it changes, as the prior + * row merged with the final payload once its `beforeUpdate` hooks have + * run. + * + * One failing row refuses the whole write. A by-id update is judged a + * second time, before its hooks, on the prior row merged with the change + * set as sent, so a change set the check refuses is refused even when a + * hook would have overwritten the refused value. + * * **Default Behavior**: the `using` → `check` default is decided per write * operation across the applicable policies, not policy by policy. A policy * is applicable to a write when it is not `enabled: false`, its `object` is @@ -409,8 +450,8 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( * not part of the check. * - When none declares `check`, each applicable policy's `using` stands in as * its check, OR-combined. The platform's own ownership floor - * (`owner_only_writes`) takes part only where the by-id pre-image gate - * kept it. + * (`owner_only_writes`) takes part only where the write's own row gate + * kept it (plugin-security `writeCheckPolicies`). * * So declaring `check` on one policy replaces, for the callers that policy * applies to, the `using` its USING-only siblings would otherwise have @@ -439,7 +480,7 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( */ check: z.string() .optional() - .describe('Validation condition matched against the new row of a single-record INSERT or a by-id UPDATE (enforced at application level); an array insert and a `multi: true` update are not post-image checked. The default to `using` is decided per operation across the applicable policies, not per policy: when any applicable policy for that operation declares `check`, only the declared checks decide (OR-combined) and a USING-only sibling adds nothing; only when none declares `check` does each applicable policy\'s `using` stand in as its check (OR-combined). Applicable = not `enabled: false`, `object` matches or is \'*\', `operation` matches or is \'all\', and the caller holds one of its `positions` when it lists any. Refused on a policy whose `operation` is `select` or `delete`, which write no new row to check: limit the rows such a policy admits with `using`, and declare the `check` on an `insert`, `update` or `all` policy.'), + .describe('Validation condition judged on every row an insert or an update writes (enforced at application level): each row of an insert, an array insert included, as its `beforeInsert` hooks leave it, and each row an update changes, by id or `multi: true`, as the prior row merged with the final payload after its `beforeUpdate` hooks. One failing row refuses the whole write. A by-id update is also judged, before its hooks, on the prior row merged with the change set as sent. The default to `using` is decided per operation across the applicable policies, not per policy: when any applicable policy for that operation declares `check`, only the declared checks decide (OR-combined) and a USING-only sibling adds nothing; only when none declares `check` does each applicable policy\'s `using` stand in as its check (OR-combined). Applicable = not `enabled: false`, `object` matches or is \'*\', `operation` matches or is \'all\', and the caller holds one of its `positions` when it lists any. Refused on a policy whose `operation` is `select` or `delete`, which write no new row to check: limit the rows such a policy admits with `using`, and declare the `check` on an `insert`, `update` or `all` policy.'), /** * Restrict this policy to specific positions (ADR-0090 D3; formerly @@ -470,10 +511,13 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( /** * REMOVED — `priority` promised "conflict resolution" that cannot exist. * - * Applicable policies OR-combine (any match allows access — the doc above - * `RLSCompiler.compileFilter` and this schema's own former describe both say - * most-permissive-wins), so there is never a conflict to order and evaluation - * order cannot change an outcome. Nothing ever read the key: the 2026-07-30 + * No outcome depends on an order. On a read, the applicable policies' `using` + * clauses OR-combine (any match allows access). On an insert or an update, + * which predicates judge the written rows is decided once per operation + * across the applicable policies (the declared `check`s when any exists, + * otherwise each `using`; see `check`), and the chosen ones OR-combine. So + * there is never a conflict to order and evaluation order cannot change an + * outcome. Nothing ever read the key: the 2026-07-30 * security-subset liveness re-verification (#3896 follow-up) closed the call * graph across the collection site, the projection round-trip and the * compiler, and found no consumer. A semantically-void knob on a SECURITY @@ -504,7 +548,12 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( if (!data.using && !data.check) { ctx.addIssue({ code: z.ZodIssueCode.custom, - message: 'At least one of "using" or "check" must be specified. For SELECT/UPDATE/DELETE operations, provide "using". For INSERT operations, provide "check".', + message: + 'At least one of "using" or "check" must be specified. A `select` or `delete` policy takes "using": ' + + 'the rows it lets a caller read or delete. An `insert` policy takes "check", which judges each row ' + + 'an insert writes; a "using" alone is that check when no applicable insert policy declares one. ' + + 'An `update` or `all` policy takes either or both: "using" limits the existing rows it admits, and ' + + '"check" judges each row written.', }); } @@ -536,9 +585,10 @@ export const RowLevelSecurityPolicySchema = lazySchema(() => strictObject( }); } - // For non-insert operations, USING should typically be present - // This is a soft warning through documentation, not enforced here - // since 'all' and mixed operation types are valid + // With both rules above, a non-blank `using` is the only predicate a + // `select` or `delete` policy can carry. An `insert`, `update` or `all` + // policy may carry either clause or both; what each does per operation is + // on the `using` and `check` docs. })); // RLSAuditEventSchema / RLSAuditConfigSchema / RLSConfigSchema were REMOVED