Skip to content

Commit d6bfb3d

Browse files
authored
refactor(spec)!: close the three #3980 triage items — remove rls.priority, bind tabPermissions + writeScope proofs (#3990)
The three items #3980 flagged for the v17 window. 1. rls.priority removed — it promised "conflict resolution" that cannot exist: applicable policies OR-combine (most permissive wins, per the schema's own describe), so there is never a conflict to order and nothing ever read the key. Tombstoned via retiredKey (tsc never + parse-time prescription, both directions pinned), stripped mechanically by the permission-rls-priority-removed D2 conversion + D3 chain step, cleaned from factory helpers / the showcase example / the RLS docs, baselines updated. The docs table's enabled row now states the since-#3980-enforced contract. 2. permission.tabPermissions bound to the me-apps proof — after extending the proof to actually AUTHOR the property on the permission-set side (it had only mentioned tabPermissions in its header; binding then would have been the owner-anchor/allowTransfer anti-pattern). The new case pins hidden dropping the app from /me/apps, a more-visible grant winning it back (tabRank most-visible-wins), and unrelated principals untouched. 3. writeScope gets its runtime proof (showcase-scope-depth-write, 9/9, post-state assertions): 'unit' edits a BU co-member's row and does not descend; an ABSENT writeScope stays owner-only even under a 'unit' readScope (the axes gate independently); the hierarchy seam fails closed without the enterprise resolver. Bound as the scope-depth-write class. spec: 6879+ tests green incl. the chain-replay composability gate (union-merged over #3987's protocol-17 conversions); all gates green; dogfood fixture sweep corrected (the invoice-seed-isolation fixture no longer authors the retired key).
1 parent 77fadbf commit d6bfb3d

19 files changed

Lines changed: 453 additions & 46 deletions

File tree

.changeset/rls-priority-removed.md

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
---
2+
"@objectstack/spec": major
3+
"@objectstack/example-showcase": patch
4+
---
5+
6+
refactor(spec)!: remove the RLS-policy `priority` key — it promised conflict resolution that cannot exist (#3896 audit)
7+
8+
`RowLevelSecurityPolicySchema.priority` was documented as *"Policy priority for
9+
conflict resolution"*. The 2026-07-30 security-subset liveness re-verification
10+
found that **nothing ever read it** — and, stronger, that nothing ever could:
11+
applicable policies **OR-combine** (any match allows access, most permissive
12+
wins — the schema's own describe said so), so there is never a conflict to
13+
order and evaluation order cannot change an outcome. A semantically-void knob
14+
on a security policy is worse than dead: an author — very often an AI
15+
(ADR-0033) — reads it as a precedence lever and reasons about policy
16+
interactions that do not exist.
17+
18+
Removed per the `tool.requiresConfirmation` (#3715) / `DynamicLoadingConfig`
19+
(#3950) precedent, inside the v17 breaking window:
20+
21+
- **Tombstoned, not silently stripped** (`retiredKey`, #3855 pattern): an
22+
authored `priority` fails `tsc` (the input type is `never`) and rejects at
23+
parse with the prescription itself — *"policies OR-combine (most permissive
24+
wins), so there is no conflict to order. Delete the key — policy outcomes are
25+
unchanged."*
26+
- **ADR-0087 D2 conversion + D3 chain step** (`permission-rls-priority-removed`):
27+
`os migrate meta` deletes the key from authored sources mechanically — a pure
28+
lossless delete, no semantic residue. spec-changes.json and the protocol
29+
upgrade guide carry the entry.
30+
- The policy factory helpers (`ownerPolicy`, `tenantPolicy`, …), the showcase
31+
example's permission sets, and `content/docs/permissions/rls.mdx` no longer
32+
author it; the docs table's `enabled` row now states the (since-enforced)
33+
contract instead.
34+
- Liveness ledger entry updated to record the removal; the tombstone and entry
35+
age out ~two majors from now.
36+
37+
Dropping the key changes **no policy outcome anywhere** — that impossibility of
38+
effect is the entire reason for the removal.

content/docs/permissions/rls.mdx

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,6 @@ export const ContributorAccess = definePermissionSet({
4848
using: 'assignee == current_user.email',
4949
positions: ['contributor'],
5050
enabled: true,
51-
priority: 10,
5251
},
5352
],
5453
});
@@ -63,8 +62,7 @@ export const ContributorAccess = definePermissionSet({
6362
| `using` | `string` | Predicate for rows the user may **see / act on** (compiled into the query filter) |
6463
| `check` | `string` | Predicate rows must satisfy **after a write**. Omit it and `using` is reused |
6564
| `positions` | `string[]` | Which positions the policy applies to. Omit = everyone |
66-
| `enabled` | `boolean` | Default `true` |
67-
| `priority` | `number` | Default `0` |
65+
| `enabled` | `boolean` | Default `true`. `false` switches the policy off — a disabled policy is not evaluated |
6866

6967
At least one of `using` / `check` is required.
7068

content/docs/references/security/rls.mdx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -221,7 +221,7 @@ const result = RLSEvaluationResult.parse(data);
221221
| **check** | `string` | optional | Validation condition for INSERT/UPDATE (defaults to USING clause if not specified - enforced at application level) |
222222
| **positions** | `string[]` | optional | Positions this policy applies to (omit for all) |
223223
| **enabled** | `boolean` || Whether this policy is active |
224-
| **priority** | `integer` | | Policy evaluation priority (higher = evaluated first) |
224+
| **priority** | `any` | optional | [REMOVED] `rowLevelSecurity[].priority` was removed in @objectstack/spec 17.0.0 (#3896 security audit). It never had an effect and could not: applicable policies OR-combine (most permissive wins), so there is no conflict to order. Delete the key — policy outcomes are unchanged. |
225225
| **tags** | `string[]` | optional | Policy categorization tags |
226226

227227

docs/protocol-upgrade-guide.md

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ Finally it removes agent `tools` (#3894): the legacy inline `{type,name,descript
124124

125125
Beyond those spec-surface removals, it graduates the seven flow-node config key aliases the executors still tolerated (#3796): the CRUD nodes' `object` (use `objectName`) — the last tenant of the `readAliasedConfig` executor shim, which is deleted with it — plus the six open-coded fallbacks that never went through that shim: notify `to`/`subject`/`body`/`url` (use `recipients`/`title`/`message`/`actionUrl`) and script `functionName`/`input` (use `function`/`inputs`). All are pure key renames with unchanged values and replay losslessly. Like the sharing-rule access level above they keep a load-path acceptance window: none carried a prior deprecation warning, and `FlowNodeSchema.config` is an unconstrained record, so no schema tombstone can reject them — the conversion layer is the only seam that can declare, convert, and retire them.
126126

127+
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.
128+
127129
### Mechanical (applied for you)
128130

129131
| Conversion | Surface | Change | Load window |
@@ -136,6 +138,7 @@ Beyond those spec-surface removals, it graduates the seven flow-node config key
136138
| `flow-node-crud-object-alias` | `flow.node.config.objectName` | CRUD flow-node config key 'object' → 'objectName' (#3796`readAliasedConfig` shim graduation) | live — protocol 17 loader accepts the old shape |
137139
| `flow-node-notify-config-aliases` | `flow.node.notify.config` | notify flow-node config keys 'to' → 'recipients', 'subject' → 'title', 'body' → 'message', 'url' → 'actionUrl' (#3796) | live — protocol 17 loader accepts the old shape |
138140
| `flow-node-script-config-aliases` | `flow.node.script.config` | script flow-node config keys 'functionName' → 'function', 'input' → 'inputs' (#3796) | live — protocol 17 loader accepts the old shape |
141+
| `permission-rls-priority-removed` | `permission.rowLevelSecurity.priority` | RLS-policy key 'priority' removed (#3896 audit — policies OR-combine, so the promised conflict-resolution semantics cannot exist; dropping it changes no outcome) | retired — `migrate meta` only |
139142

140143
---
141144

examples/app-showcase/src/security/permission-sets.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ export const ContributorPermissionSet = definePermissionSet({
6464
using: 'assignee == current_user.email',
6565
positions: ['contributor'],
6666
enabled: true,
67-
priority: 10,
6867
},
6968
// Owner RLS on the MASTER invoice. Because `showcase_invoice_line` is
7069
// `controlled_by_parent`, a contributor seeing only their own invoices also
@@ -80,7 +79,6 @@ export const ContributorPermissionSet = definePermissionSet({
8079
using: 'owner == current_user.email',
8180
positions: ['contributor'],
8281
enabled: true,
83-
priority: 10,
8482
},
8583
// [ADR-0058 D4] RLS `check` — write-side post-image validation (NOT a read
8684
// filter). On UPDATE the new row must still be owned by the caller, so a
@@ -97,7 +95,6 @@ export const ContributorPermissionSet = definePermissionSet({
9795
check: 'owner == current_user.email',
9896
positions: ['contributor'],
9997
enabled: true,
100-
priority: 10,
10198
},
10299
],
103100
});

packages/plugins/plugin-security/src/objects/default-permission-sets.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -82,7 +82,7 @@ const denyWritesOnManagedObjects = (): Record<string, {
8282
* permission set name appears in the request `ExecutionContext.permissions[]`.
8383
*
8484
* Each entry is run through `PermissionSetSchema.parse(...)` so Zod fills
85-
* in the boolean/`priority`/`enabled` defaults — keeping the literal
85+
* in the boolean/`enabled` defaults — keeping the literal
8686
* source readable while still satisfying the strict output type.
8787
*
8888
* `objects: { '*': … }` uses the wildcard sentinel honoured by

packages/qa/dogfood/test/me-apps-and-everyone-baseline.dogfood.test.ts

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,65 @@ describe('ADR-0090 D5 closures: /me/apps + anchor-bindable baseline', () => {
6767
expect(body?.apps ?? []).toEqual([]);
6868
});
6969

70+
// ── permission.tabPermissions authored end-to-end ─────────────────────────
71+
// Added by the 2026-07 security-subset liveness re-verification. Until then
72+
// this file only MENTIONED tabPermissions in its header while exercising the
73+
// route and requiredPermissions — and per ADR-0054, binding a ledger entry to
74+
// a proof that does not author the property is the preview-renderer mistake
75+
// in a nicer costume. This authors the property on a permission set, both
76+
// directions of the rank merge included.
77+
it('a permission set hiding an app via tabPermissions drops it from /me/apps — and a more-visible grant wins it back', async () => {
78+
const tabTok = await stack.signUp('tab-member@verify.test');
79+
const tabUser = await ql.findOne('sys_user', { where: { email: 'tab-member@verify.test' }, context: SYS });
80+
expect(tabUser?.id, 'dedicated tab-probe member resolved').toBeTruthy();
81+
82+
// Control: before any tabPermissions grant, the member sees the app.
83+
const before = await stack.apiAs(tabTok, 'GET', '/me/apps');
84+
const beforeNames = (((await before.json()) as any)?.apps ?? []).map((a: any) => a?.name);
85+
expect(beforeNames, 'control: visible before any tab grant').toContain('showcase_app');
86+
87+
// Author the property: a set whose ONLY job is tabPermissions.hidden.
88+
const hideSet = await ql.insert(
89+
'sys_permission_set',
90+
{
91+
name: 'tab_probe_hide',
92+
label: 'Tab probe — hide showcase',
93+
tab_permissions: JSON.stringify({ showcase_app: 'hidden' }),
94+
},
95+
{ context: SYS },
96+
);
97+
const hideSetId = hideSet?.id ?? (await ql.findOne('sys_permission_set', { where: { name: 'tab_probe_hide' }, context: SYS }))?.id;
98+
expect(hideSetId, 'hide set stored').toBeTruthy();
99+
await ql.insert('sys_user_permission_set', { user_id: tabUser.id, permission_set_id: hideSetId }, { context: SYS });
100+
101+
const hidden = await stack.apiAs(tabTok, 'GET', '/me/apps');
102+
const hiddenNames = (((await hidden.json()) as any)?.apps ?? []).map((a: any) => a?.name);
103+
expect(hiddenNames, 'tabPermissions.hidden drops the app for this principal').not.toContain('showcase_app');
104+
105+
// Rank merge: hidden(0) loses to visible(3) from ANY other resolved set —
106+
// most-visible wins across the principal's sets, matching hono's tabRank.
107+
const showSet = await ql.insert(
108+
'sys_permission_set',
109+
{
110+
name: 'tab_probe_show',
111+
label: 'Tab probe — show showcase',
112+
tab_permissions: JSON.stringify({ showcase_app: 'visible' }),
113+
},
114+
{ context: SYS },
115+
);
116+
const showSetId = showSet?.id ?? (await ql.findOne('sys_permission_set', { where: { name: 'tab_probe_show' }, context: SYS }))?.id;
117+
await ql.insert('sys_user_permission_set', { user_id: tabUser.id, permission_set_id: showSetId }, { context: SYS });
118+
119+
const merged = await stack.apiAs(tabTok, 'GET', '/me/apps');
120+
const mergedNames = (((await merged.json()) as any)?.apps ?? []).map((a: any) => a?.name);
121+
expect(mergedNames, 'most-visible wins: a visible grant overrides the hidden one').toContain('showcase_app');
122+
123+
// The probe must not leak into other principals' assertions.
124+
const others = await stack.apiAs(memberTok, 'GET', '/me/apps');
125+
const otherNames = (((await others.json()) as any)?.apps ?? []).map((a: any) => a?.name);
126+
expect(otherNames, 'unrelated principals are untouched by the probe sets').toContain('showcase_app');
127+
});
128+
70129
// ── #2753: the baseline binds to the everyone anchor at bootstrap ────────
71130
it('bootstrap binds member_default to the everyone position (one channel, D5)', async () => {
72131
const everyone = await ql.findOne('sys_position', { where: { name: 'everyone' }, context: SYS });

packages/qa/dogfood/test/showcase-invoice-seed-isolation.dogfood.test.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,6 @@ const demoSet = PermissionSetSchema.parse({
3939
operation: 'select',
4040
using: 'owner = current_user.email',
4141
enabled: true,
42-
priority: 10,
4342
},
4443
],
4544
});

0 commit comments

Comments
 (0)