Skip to content

Commit b276d44

Browse files
feat(spec)!: refuse a check on a select / delete row-level security policy (#20167)
Fixes #19965 Clause-②: no ## What `RowLevelSecurityPolicySchema` now refuses a non-blank `check` on a policy whose `operation` is `select` or `delete`. The refusal is one `custom` issue at the `check` path. Its message names the operation and gives both rewrites: > `check` is never evaluated on a `delete` policy: it validates the new row an insert or an update writes, and a delete writes none. Remove `check` from this policy. To limit which rows the policy lets a caller delete, write that predicate as `using`; to validate rows as they are written, declare the `check` on a policy whose `operation` is `insert`, `update` or `all`. - The `check` docblock and `.describe()` sentence "A `check` on a `select` or `delete` policy is never evaluated" now describe the refusal. The generated reference pages (`content/docs/references/security/{rls,permission}.mdx`) were regenerated with `gen:docs`, not edited by hand. - ADR-0087: registered as the D3 semantic entry `rls-check-on-select-or-delete-policy-refused` under protocol 18. There is no D2 conversion because no lossless rewrite exists: dropping the key discards the predicate, and moving it changes which rows the policy admits. The changeset is `minor` for `@objectstack/spec`, carries a `**BREAKING**` banner and a FROM → TO table, and has the `registered` disposition marker. - A blank `check` (empty or whitespace only) is **not** refused. The runtime reads a blank clause as absent (`policyDeclaresClause`), and the Studio editor leaves `check: ''` behind when an author clears the field. - Governing text: ADR-0049 (declared ≠ enforced ⇒ enforce or remove), NORTH-STAR priority rule 4, and the stage posture of no transition window. ⛔ No lint transition. `packages/lint/src/validate-rls-predicate-enforceability.ts` was not touched. ## Premise, re-measured on `origin/main` `8d1f7ab` - **H1 (runtime evaluates `check` only on insert/update): holds.** - `function writeCheckPolicies(` is at `security-plugin.ts:889`. - Its only caller, `computeWriteCheckFilter`, is reached from step 3.6, which is guarded by `opCtx.operation === 'insert' || opCtx.operation === 'update'` (`:3111-3112`). - The policy set it gets comes from `getApplicablePolicies` (`rls-compiler.ts:733-775`), which admits `policy.operation === rlsOp` or `all` and nothing else. - A repo read of every `check` reader in `plugin-security`, `runtime`, `objectql`, `verify` and `lint` found only `rls-compiler.ts:536` (the `clause: 'check'` compile) and `security-plugin.ts:893`. Both sit behind that gate. - **H2 (no shipped or authored policy carries it): holds for shipped and authored metadata. Two test fixtures carried it.** - Census over objectstack `packages/ examples/ skills/ content/ apps/ scripts/ docs/ .changeset/` at `8d1f7ab`: 9332 files, 264 `operation: select|delete` literals, 2 with a sibling `check`. Both are tests, and both were triaged as "the operation was incidental" (rewritten, not re-declared): - `packages/spec/src/security/rls.test.ts:48` ("complete policy with all fields") is now `operation: 'all'`, where both clauses are read. - `packages/plugins/plugin-security/src/rls-reserved-membership-keys.test.ts:189` (a `compileFilter` check-face fixture; `compileFilter` ignores `operation`) is now `operation: 'insert'`. - A second scan for a non-literal `operation` beside `check` found 1 candidate. It is a false positive: a `.find` predicate in `default-permission-sets.test.ts:127`. - objectui at the pin `f8a9d0fb05`: 8314 files, 17 occurrences, 0 with `check`. objectui at `origin/main` `5c61e52`: 8798 files, 17 occurrences, 0 with `check`. - **H3 (every authoring door reaches the refinement): holds by construction. One door is proven by a pin.** - `PermissionSetSchema.rowLevelSecurity` is `z.array(RowLevelSecurityPolicySchema)` (`permission.zod.ts:920`), and the stack `permissions` is `z.array(PermissionSetSchema)` (`stack.zod.ts:461`). - The metadata `permission` type validates against `PermissionSetSchema` (`metadata-type-schemas.ts:164`), and `os validate` parses `ObjectStackDefinitionSchema` (`validate.ts:283`). - Pinned: `defineStack` refuses with `code: 'STACK_SCHEMA_INVALID'`, `status: 422` and exactly one issue at `permissions.0.rowLevelSecurity.0.check`. The same stack without the `check` is accepted. - `os validate` and a live metadata save were **not measured** by a run (reach established by reading the code only). - **H4 (the prescription must be actionable): measured.** - A check-only `select` / `delete` policy gets exactly this one issue, not the "At least one of using or check" issue. - The rewrite it prescribes (the predicate as `using`) parses. Both are pinned. - **H5 (ADR-0087 owed): registered as described above.** `check-adr-0087-registration` passes with: `✓ check-adr-0087-registration: 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition.` ## Pins (`packages/spec/src/security/rls.test.ts`) These pins assert the issue code, path and message head, never a bare throw: - `select` + `check` and `delete` + `check` are refused. - A check-only policy gets this refusal and not the "at least one" refusal. - The prescribed rewrite parses. - A blank `check` is left to the existing rule. - `insert` / `update` / `all` with `check` parse. - `select` with `using` parses. - The refusal reaches the permission set (`rowLevelSecurity[1].check`, sole issue) and the `defineStack` door (envelope `code` + `status`). ## Evidence, at `f25e34d` | reading | result | | --- | --- | | `@objectstack/spec` test | 540 files, 15809 passed, 2 todo | | `@objectstack/spec` typecheck | exit 0 | | `@objectstack/plugin-security` test / typecheck | 135 files, 2697 passed / exit 0 | | `@objectstack/lint` test | 108 files, 4144 passed | | `@objectstack/runtime` test | 279 files, 3906 passed, 1 skipped | | `@objectstack/verify` test | 15 files, 116 passed | | `@objectstack/spec check:generated` (+ each derived spec gate: api-surface, authorable-surface, docs, liveness, strictness-ledger, spec-changes, upgrade-guide, migration-registry, skill-examples …) | 22 of 22 exit 0; `✓ All 15 generated artifacts are up to date` | | ablation: refusal gate replaced with `false &&` via `scripts/ablation-replace.mjs` (subject imported from `src`, no `dist` in the path) | 6 failed / 52 passed, exactly the six refusal and reach pins; restored blob `6b2ec57ec014` equals HEAD, `git diff HEAD` empty, then 58 / 58 | | eslint, narrowed to the 5 changed `.ts` files | 5 files linted, 0 errors, 0 warnings. Population is the config's `**/*.{ts,…}` block. Invariance: `eslint.config.mjs` enables no type-aware linting, so this diff cannot move a verdict on an untouched file | The consumer readings are five downstream consumers of `@objectstack/spec` (the PM's lead list), not the full `...@objectstack/spec` set. - `packages/cli` has one RLS fixture, `test/migrate-meta.e2e.test.ts:117-124`. It is a `select` policy with `using` and no `check`. That file is `integration` tier and is left to CI. The `unit` tier was NOT MEASURED when this PR opened: its dependency closure was still waiting on the shared verify lock. If that reading is taken, it is in the `os-dev-report` comment on the card. - `packages/qa/dogfood` census: 0 hits. It is left to the Dogfood Regression Gate. - Gate reconciliation (`dispatch-gates --ran`): 115 derived, 38 run (all exit 0), 0 NOT-MEASURED, 77 unrun. The 77 unrun are left to CI as the declared narrowing. ## Acceptance notes - **Studio editor.** The Studio RLS editor (objectui `PermissionAdvancedFacets.tsx`) renders a `check` input for every operation. Its client validation parses `@objectstack/spec/security` `PermissionSetSchema`, so after the next objectui spec bump the refusal also shows in the editor. Hiding the input on `select` / `delete` would be polish. Owner: none. - **JSON Schema.** The published JSON Schema cannot express this cross-field refusal (it is a `superRefine`). The `check` description does now state it. - **Pending changeset.** `.changeset/19953-rls-check-default-composition-text.md` (unreleased) still reads "A `check` on a `select` or `delete` policy is never evaluated". It will ship in the same train as this refusal. It was left untouched because it is another PR's changeset. - **`using` docblock.** It says "For SELECT/UPDATE/DELETE operations, USING is required". With this change that holds for `select` / `delete` (`check` is refused and one predicate is required). It still does not hold for `update`, where a check-only policy is legal and enforced. This is prose drift, not addressed here. --- _Generated by [Claude Code](https://claude.ai/code/session_01QcAS3qiYYZNezaxZxaUdMV)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 0a6024d commit b276d44

8 files changed

Lines changed: 328 additions & 10 deletions

File tree

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
---
2+
'@objectstack/spec': minor
3+
---
4+
5+
**BREAKING for authored metadata** — a row-level security policy (`RowLevelSecurityPolicySchema`, authored as `rowLevelSecurity[]` on a permission set) whose `operation` is `select` or `delete` may no longer declare a `check`. It is refused at parse, at the `check` path, with a message that names the operation and says what to write instead (#19965).
6+
7+
Clause-②: no
8+
9+
## What changed, and why
10+
11+
`check` judges the post-image of a write: the new row of an insert, the changed row of an update. A `select` or `delete` writes no row, and the runtime's write gate only ever collects the policies whose `operation` is the write's own or `all`. So a `check` on a `select` or `delete` policy was accepted, stored and **never evaluated**. It did not guard any write, and beside a USING-only sibling it did not replace that sibling's `using` default the way a `check` on an `insert`, `update` or `all` policy does. An author who wrote a `check` on a `delete` policy believed deletes were guarded by it (ADR-0049: declared is not enforced).
12+
13+
```
14+
FROM RowLevelSecurityPolicySchema.safeParse({
15+
name: 'no_archived', object: 'account', operation: 'delete',
16+
using: 'owner_id == current_user.id', check: "status != 'archived'" })
17+
-> { success: true } // the check never ran
18+
19+
TO -> { success: false,
20+
issues: [{ code: 'custom', path: ['check'],
21+
message: '`check` is never evaluated on a `delete` policy: it validates the new
22+
row an insert or an update writes, and a delete writes none. Remove
23+
`check` from this policy. …' }] }
24+
```
25+
26+
Through a permission set the issue lands at `rowLevelSecurity[N].check`; through `defineStack` it is part of the `STACK_SCHEMA_INVALID` refusal (422).
27+
28+
## Migration — FROM → TO
29+
30+
| You wrote | Write instead |
31+
| --- | --- |
32+
| `operation: 'select'` with a `check` meant to limit which rows can be read | that predicate as the policy's `using` (AND it into an existing `using` with `&&`), and remove `check` |
33+
| `operation: 'delete'` with a `check` meant to limit which rows can be deleted | that predicate as the policy's `using` (AND it into an existing `using` with `&&`), and remove `check` |
34+
| `operation: 'select'` or `'delete'` with a `check` meant to validate written rows | the `check` on a policy whose `operation` is `insert`, `update` or `all` |
35+
36+
**The one-line fix: remove `check` from every `select` / `delete` policy, and write its predicate as that policy's `using` or on an `insert` / `update` / `all` policy, depending on what it was meant to guard.** This cannot be converted automatically: dropping the key would discard the predicate, and moving it would change which rows the policy admits. So it ships as an ADR-0087 D3 structured TODO with **no D2 conversion**.
37+
38+
<!-- adr-0087: registered rls-check-on-select-or-delete-policy-refused -->
39+
40+
## Stored permission sets
41+
42+
Stored rows are not rewritten. A permission set already stored with a `check` on a `select` or `delete` policy is refused the next time it is parsed through `@objectstack/spec`, for example on its next save. Removing the `check` changes nothing at runtime, because it never ran. Moving its predicate into `using` or onto a write policy does change behaviour, so re-check the policy set afterwards.
43+
44+
## What does NOT change
45+
46+
- A `check` on an `insert`, `update` or `all` policy parses and is enforced exactly as before.
47+
- A `select` or `delete` policy with `using` only parses exactly as before.
48+
- A blank `check` (empty or whitespace only) declares nothing, and the runtime reads it as absent. It is not refused by this rule.
49+
- No export is added, removed or renamed.

‎content/docs/references/security/permission.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ const result = AdminScopeSchema.parse(data);
168168
| **object** | `string` | ✅ | Target object name |
169169
| **operation** | `Enum<'select' \| 'insert' \| 'update' \| 'delete' \| 'all'>` | ✅ | Database operation this policy applies to |
170170
| **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. |
171-
| **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. A `check` on a `select` or `delete` policy is never evaluated. |
171+
| **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. |
172172
| **positions** | `string[]` | optional | Positions this policy applies to (omit for all) |
173173
| **enabled** | `boolean` | optional (default: `true`) | Whether this policy is active |
174174
| **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. |

‎content/docs/references/security/rls.mdx‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -171,7 +171,7 @@ const result = RLSEvaluationResultSchema.parse(data);
171171
| **object** | `string` | ✅ | Target object name |
172172
| **operation** | `Enum<'select' \| 'insert' \| 'update' \| 'delete' \| 'all'>` | ✅ | Database operation this policy applies to |
173173
| **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. |
174-
| **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. A `check` on a `select` or `delete` policy is never evaluated. |
174+
| **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. |
175175
| **positions** | `string[]` | optional | Positions this policy applies to (omit for all) |
176176
| **enabled** | `boolean` | optional (default: `true`) | Whether this policy is active |
177177
| **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. |

‎packages/plugins/plugin-security/src/rls-reserved-membership-keys.test.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -183,10 +183,12 @@ describe('RESERVED_RLS_MEMBERSHIP_KEYS — refused BY NAME at the compiler merge
183183
// `compileFilter` is the ONE seam both faces pass through — the read layer
184184
// compiles `using`, the ADR-0058 D4 write gate compiles `check` — so a
185185
// repair that held on only one of them would be half a repair.
186+
// `insert`: the write gate compiles `check` only for a policy that
187+
// writes a row, and the spec refuses a `check` on `select` / `delete`.
186188
const checkPolicy = {
187189
name: 'p',
188190
object: 'o',
189-
operation: 'select',
191+
operation: 'insert',
190192
check: cell.using,
191193
} as never;
192194
const withBag = compiler.compileFilter([checkPolicy], {
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
3+
import type { SemanticMigration } from '../../types.js';
4+
5+
// A row-level policy's `check` is refused where it can never run: on a policy
6+
// whose `operation` is `select` or `delete`. Recorded as a semantic TODO, not a
7+
// D2 conversion, because which rewrite is right depends on what the author meant
8+
// the predicate to guard, and a rewrite made on their behalf would change which
9+
// rows the policy admits.
10+
export const entry: SemanticMigration = {
11+
id: 'rls-check-on-select-or-delete-policy-refused',
12+
// No backticks in `surface` — build-upgrade-guide renders it inside a code
13+
// span already, and a nested backtick would close it.
14+
surface:
15+
'security.PermissionSet rowLevelSecurity[].check (RowLevelSecurityPolicySchema) on a policy whose '
16+
+ 'operation is select or delete. A blank check (empty or whitespace only) declares nothing and is '
17+
+ 'not refused',
18+
replacement:
19+
'what the predicate was meant to guard, written where it runs. To limit which rows a select policy '
20+
+ 'lets a caller read, or which rows a delete policy lets a caller delete, write the predicate as '
21+
+ '`using` on that policy (remove `check`; if the policy already has a `using`, AND the two with &&). '
22+
+ 'To validate rows as they are written, declare the `check` on a policy whose `operation` is '
23+
+ '`insert`, `update` or `all` instead. The refusal lands at rowLevelSecurity[N].check, names the '
24+
+ 'operation, and states both rewrites',
25+
reason:
26+
'ADR-0049 enforce-or-remove and ADR-0058 D4. A `check` judges the post-image of a write: the new '
27+
+ 'row of an insert, the changed row of an update. A select or delete writes no row, and the '
28+
+ 'plugin-security write gate collects only the policies whose operation is the write\'s own or '
29+
+ '`all`, so a `check` on a select or delete policy was accepted, stored and never evaluated. '
30+
+ 'Measured on main before this change: a policy carrying only check record.status != '
31+
+ '\'archived\' on select or delete admitted every insert and update of an archived row, and beside '
32+
+ 'a USING-only `all` sibling it did not replace that sibling\'s `using` default the way a `check` '
33+
+ 'on an insert, update or all policy does. An author (an AI author above all) who wrote a check '
34+
+ 'on a delete policy believed deletes were guarded by it. The refusal is a non-transforming '
35+
+ 'refinement on the policy schema, so it reaches every door that parses a permission set: '
36+
+ 'defineStack, os validate, and the metadata save path, whose permission type validates '
37+
+ 'against PermissionSetSchema. Metadata AT REST is not '
38+
+ 'rewritten and this entry adds no D2 conversion: dropping the key would silently discard the '
39+
+ 'predicate the author wrote, and moving it to `using` would start filtering reads or deletes '
40+
+ 'the policy never filtered before — both change which rows the policy admits, which is the '
41+
+ 'policy author\'s decision. Ships at once, no transition window and no advisory lint phase.',
42+
acceptanceCriteria:
43+
'Search every authored and stored permission set for a rowLevelSecurity policy whose operation '
44+
+ 'is select or delete and whose check is non-blank — metadata files, sys_metadata permission '
45+
+ 'rows, and the row_level_security column of sys_permission_set. For authored metadata the sweep '
46+
+ 'is mechanical: PermissionSetSchema.safeParse answers one custom issue at '
47+
+ 'rowLevelSecurity[N].check whose message begins "`check` is never evaluated on a `select` '
48+
+ 'policy" (or `delete`). For each, decide what the predicate was meant to guard: reads or '
49+
+ 'deletes ⇒ express it in that policy\'s `using`; writes ⇒ move it to an insert, update or all '
50+
+ 'policy. Then re-check the policy set\'s behaviour rather than assuming it is unchanged: the '
51+
+ 'removed check never ran, so dropping it changes nothing, but a predicate moved into `using` '
52+
+ 'now filters rows it never filtered, and a `check` moved onto an insert, update or all policy '
53+
+ 'now replaces the `using` default of its USING-only siblings for that write. A policy with '
54+
+ '`using` only, and a `check` on an insert, update or all policy, parse exactly as before. The '
55+
+ 'repo, its example apps and the pinned console carried no such policy at the time of the '
56+
+ 'change; two test fixtures that used select incidentally were moved to all and insert.',
57+
};

0 commit comments

Comments
 (0)