diff --git a/.changeset/master-detail-controlled-by-parent.md b/.changeset/master-detail-controlled-by-parent.md new file mode 100644 index 0000000000..6602611a28 --- /dev/null +++ b/.changeset/master-detail-controlled-by-parent.md @@ -0,0 +1,15 @@ +--- +"@objectstack/plugin-security": minor +"@objectstack/spec": minor +"@objectstack/verify": minor +--- + +Master-detail "controlled by parent" permissions (ADR-0055). + +A detail object can now declare `sharingModel: 'controlled_by_parent'`: its read/write access is derived from its master record, with no authored RLS. + +- `@objectstack/spec`: `controlled_by_parent` added to the authorable `object.sharingModel` enum. +- `@objectstack/plugin-security`: reads inject `masterFK IN (accessible master ids)` (resolved from the master's own RLS, reusing the existing filter machinery — zero RLS-compiler changes); by-id writes (insert/update/delete) to a detail now require edit access to its master, closing the #1994-class by-id hole for derived access. +- `@objectstack/verify`: related-record **topological synthesis** — `deriveCrudCases` no longer skips objects with required relations; it builds the object dependency graph, orders it topologically, and threads real target ids, so relationship-dense objects (and the master-detail RLS proof) are verifiable. Honest `blocked` verdicts remain for required-reference cycles and external/missing targets. + +v1 limits (per ADR-0055): the accessible-master id set is unbounded (large-tenant scale is a documented future limit), and master-detail chains are single-level (not transitively traversed). diff --git a/docs/adr/0055-master-detail-controlled-by-parent.md b/docs/adr/0055-master-detail-controlled-by-parent.md new file mode 100644 index 0000000000..5c798494d1 --- /dev/null +++ b/docs/adr/0055-master-detail-controlled-by-parent.md @@ -0,0 +1,94 @@ +# ADR-0055: Master-detail "controlled by parent" permissions — derived access via pre-resolved master-id membership + +**Status**: Accepted (2026-06-19) — implemented in this PR (P0–P2) +**Deciders**: ObjectStack Protocol Architects +**Builds on**: [ADR-0049](./0049-no-unenforced-security-properties.md) (enforce-or-remove), [ADR-0054](./0054-runtime-proof-for-authorable-surface.md) (prove-it-runs) +**Surfaced by**: an audit of master-detail permission semantics — `OWDModel.controlled_by_parent` is **declared but unenforced** (zero runtime consumers; not reachable through the object's `sharingModel` enum; the RLS compiler is relationship-blind). This is *false compliance* (ADR-0049) and *unproven liveness* (ADR-0054). + +> **Framing, not the thesis.** The authorization model is already the mainstream (Salesforce-shaped) model — permission sets + FLS + predicate RLS + ownership + sharing rules + hierarchy. So this is **gap-closure, not a rewrite**: we close `controlled_by_parent` with the existing engine and gates. This ADR is the *concrete landing plan* for that one capability; cleanup items it touches (the `OWDModel`↔`sharingModel` inconsistency, removing dead `contextVariables`) are PR-level tasks, not decisions, and are noted, not belaboured. + +--- + +## TL;DR + +A **master-detail detail** record today is access-controlled entirely on its own fields — its master record's access is **not** inherited. Salesforce's "Controlled by Parent" (detail visibility/edit derived from the master) is declared in the spec yet does nothing. + +**Decision.** Implement `controlled_by_parent` by **auto-deriving the set of master records the user can access and constraining the detail's master-FK to that set** — reusing the engine's existing pre-resolved-membership mechanism (`ExecutionContext.rlsMembership` + the compiler's `field IN (current_user.)` form), with **zero compiler changes**. Reads inject `masterFK IN (accessible_master_ids)`; by-id writes extend the #1994 pre-image check to require master **edit** access. The guarantee is proven by a `@objectstack/dogfood` RLS proof (ADR-0054), which depends on the verifier's related-record topological synthesis landing first. + +--- + +## Context — the mechanisms this builds on (verified) + +1. **Read-path RLS injection.** `security-plugin.ts:481-495` AND-s an RLS filter into the query AST (`opCtx.ast.where = { $and: [where, rlsFilter] }`) before the driver runs. The builder `computeRlsFilter` (`security-plugin.ts:753-788`) is **async** and shared by the engine find-path and the analytics raw-SQL path (`getReadFilter`). +2. **Pre-resolved membership (§7.3.1) already exists.** The RLS compiler recognizes `field IN (current_user.)` and resolves `` against `ExecutionContext.rlsMembership` — "the runtime resolves set-membership that would otherwise need a subquery … and stages each set here under a stable key" (`execution-context.zod.ts:74-91`; merge at `rls-compiler.ts:79-99`). **This is the seam controlled_by_parent plugs into — no new compiler form.** +3. **By-id write pre-image check (#1994).** `security-plugin.ts:341-400` already re-reads the target row under the write-op RLS filter before an update/delete and denies if invisible. This is the exact hook to extend with a master-access check. +4. **`sharingModel` enforcement seam.** `sharing-service.ts:53-62` reads `object.sharingModel`; `buildReadFilter` (`:101-143`) gates on it (`effectiveSharingModel(schema) !== 'private'`). A `controlled_by_parent` baseline plugs in here / in the security middleware. +5. **Master-detail storage.** A `master_detail` field's **key is the FK column**; its `reference` (`field.zod.ts:386-400`) names the master object. Given a detail row, the master id is `row[masterFieldKey]`. +6. **Spec inconsistency (to fix either way).** `OWDModel` (`sharing.zod.ts`) includes `controlled_by_parent`; the object's authorable `sharingModel` (`object.zod.ts`) is a different enum `['private','read','read_write','full']` that omits it. + +## Decision + +### 1. Spec contract + +- Add `controlled_by_parent` to the **authorable** `object.sharingModel` enum (converging it with `OWDModel`; resolves the inconsistency). +- An object with `sharingModel: 'controlled_by_parent'` **must declare exactly one required `master_detail` field**; its `reference` identifies the master object and its field key is the master FK. Validation error otherwise (fail closed — an unsatisfiable "controlled by parent" must not silently fall open). +- The author writes **no RLS policy** for this — "controlled by parent" is *derived automatically* from the relationship (Salesforce-like OWD), which is the whole point. + +### 2. Read mechanism — pre-resolved accessible-master-id set (chosen) + +For a `controlled_by_parent` object, the security layer, per request: +1. resolves the **master object's** read filter for this user (`computeRlsFilter` on the master — the same machinery, reused), runs it to get the accessible master ids, and +2. stages them in `ExecutionContext.rlsMembership` under a per-relationship key (e.g. `cbp__`), then +3. the detail's derived policy `" IN (current_user.cbp__)"` compiles via the **existing IN-form** and AND-s onto the read (step 1 in Context). An **empty** set fails closed (the compiler already returns `null` → deny) — correct: no accessible master ⇒ no detail. + +This composes with the detail's own tenant/owner RLS (all AND-ed) and flows to analytics via `getReadFilter` unchanged. + +**Why this mechanism (trade-offs):** + +| Option | Verdict | Why | +|---|---|---| +| (a) query-time subquery join (`masterFK IN (SELECT id FROM master WHERE …)`) | ✗ rejected | the RLS compiler **deliberately has no subquery support** (`rls-compiler.ts:129-138`); the query AST has no EXISTS/sub-select form. Would require extending both. | +| **(b) pre-resolved accessible-master-id set** | ✓ **chosen** | reuses the `rlsMembership` + IN-form path with **zero compiler changes**; resolution is one async pre-query per request, composes with existing RLS, reaches analytics. | +| (c) materialized accessible-ids column | ✗ rejected | dual-write maintenance; goes stale the moment access rules/shares change mid-session — a correctness hazard for a security primitive. | + +### 3. Write mechanism — extend the #1994 pre-image check + +In the pre-image block (`security-plugin.ts:341-400`), for a `controlled_by_parent` detail `update`/`delete`/`create`: +- resolve the target's master id (`row[masterFieldKey]`; for `create`, the master id in the incoming body), and +- re-read the master under its **edit** write-filter (`findOne(master, { where: { $and: [{id: masterId}, masterWriteFilter] } })`); a `null` result ⇒ deny. +- **Rule:** editing/deleting/creating a detail requires **edit** access to its master (Salesforce master-detail semantics). Reading a detail requires **read** access to its master (§2). + +This reuses the existing re-read-and-deny pattern; no new enforcement layer. + +### 4. Proof (ADR-0054) + +A `@objectstack/dogfood` RLS proof, bound in the liveness ledger on `object.sharingModel` (and/or the master-detail field): a member who **cannot read master M** can **neither read nor by-id-write a detail D under M**, and **can** once granted master access (red/green, revert-provable). This is the runtime guard that flips `controlled_by_parent` from declared to *live*. + +**Prerequisite:** the proof must create a master + a detail under it. Today `@objectstack/verify`'s `deriveCrudCases` **skips objects with required relations**. So the **related-record topological synthesis** capability (build the object dependency graph, synthesize in topo order threading real ids) lands **first** — it is P0 here. + +## Consequences + +- **Positive.** Master-detail finally carries permission inheritance (not just cascade/expand); a declared-but-dead OWD value becomes enforced with a permanent runtime guard; zero RLS-compiler changes (lowest-risk path through a security-critical subsystem); composes with tenant/owner/role RLS and analytics; the `OWDModel`↔`sharingModel` inconsistency is resolved. +- **Negative / limits (honest).** + - **Set-size ceiling.** A user with very many accessible masters produces a large `IN (...)`. Acceptable for typical cardinalities; **large-tenant scale (≫ thousands of masters) is a known limit** — a future share-table/join mechanism would replace the id list. v1 documents this, not solves it. + - **Per-request resolution cost.** One extra master-id query per controlled_by_parent object per request (cached within the request). + - **Single-level only in v1.** Nested master-detail chains (a detail whose master is itself a detail) are **not** traversed transitively in v1. + +## Phasing + +- **P0** — related-record topological synthesis in `@objectstack/verify` (prerequisite for the proof; independently valuable — widens auto-derive coverage). +- **P1** — spec contract (§1) + read derivation (§2) behind the `controlled_by_parent` sharingModel. +- **P2** — write pre-image extension (§3) + the dogfood RLS proof (§4); bind in the liveness ledger; flip the ledger entry to `live` with its `proof`. + +## Non-goals + +- **Large-scale share-table/join** for huge master sets (v1 uses the id-set; flagged as a future limit). +- **Transitive nested master-detail chains** (v1 is single-level). +- **A permission-model rewrite** — explicitly rejected; this closes one gap on the existing engine. +- **ServiceNow-style scripted/per-row ACL scripts** — over-engineering for an AI-authored platform; the four-form fail-closed compiler is the deliberate ceiling. +- **Client-side enforcement** — authorization is server-enforced; UI affordances are presentation (ADR-0054 §Non-goals). + +## Alternatives considered + +- **Rewrite the permission model against a mainstream blueprint.** Rejected: the model is already Salesforce-shaped; a rewrite re-opens hard-won invariants (the #1994 by-id-write fix, org-scoping stripping, the fail-closed compiler) for no foundational gain. Gaps are enumerable and closable individually. +- **(a) / (c) access-resolution mechanisms** — see §2 trade-off table. diff --git a/examples/app-showcase/src/objects/invoice.object.ts b/examples/app-showcase/src/objects/invoice.object.ts index a903c76f2f..84712c8fc1 100644 --- a/examples/app-showcase/src/objects/invoice.object.ts +++ b/examples/app-showcase/src/objects/invoice.object.ts @@ -103,6 +103,13 @@ export const InvoiceLine = ObjectSchema.create({ icon: 'list', description: 'A single billable line on an invoice.', + // ADR-0055: a line's access is CONTROLLED BY ITS PARENT invoice — a user sees + // and edits a line only if they can see/edit its `invoice` master. No RLS is + // authored here; the security layer derives it from the required master_detail + // relationship (`invoice`). This is the canonical master-detail use of + // controlled_by_parent (a line is meaningless apart from its invoice). + sharingModel: 'controlled_by_parent', + fields: { invoice: Field.masterDetail('showcase_invoice', { label: 'Invoice', diff --git a/examples/app-showcase/src/pages/task-triage.page.ts b/examples/app-showcase/src/pages/task-triage.page.ts index aa7c006111..724e1b1262 100644 --- a/examples/app-showcase/src/pages/task-triage.page.ts +++ b/examples/app-showcase/src/pages/task-triage.page.ts @@ -33,10 +33,10 @@ export const TaskTriagePage: Page = { element: 'tabs', showAllRecords: true, tabs: [ - { name: 'in_progress', label: 'In Progress', filter: [{ field: 'status', operator: 'equals', value: 'in_progress' }] }, - { name: 'urgent', label: 'Urgent', icon: 'flame', filter: [{ field: 'priority', operator: 'equals', value: 'urgent' }] }, - { name: 'in_review', label: 'In Review', filter: [{ field: 'status', operator: 'equals', value: 'in_review' }] }, - { name: 'done', label: 'Done', filter: [{ field: 'status', operator: 'equals', value: 'done' }] }, + { name: 'in_progress', label: 'In Progress', pinned: false, isDefault: false, visible: true, filter: [{ field: 'status', operator: 'equals', value: 'in_progress' }] }, + { name: 'urgent', label: 'Urgent', icon: 'flame', pinned: false, isDefault: false, visible: true, filter: [{ field: 'priority', operator: 'equals', value: 'urgent' }] }, + { name: 'in_review', label: 'In Review', pinned: false, isDefault: false, visible: true, filter: [{ field: 'status', operator: 'equals', value: 'in_review' }] }, + { name: 'done', label: 'Done', pinned: false, isDefault: false, visible: true, filter: [{ field: 'status', operator: 'equals', value: 'done' }] }, ], }, diff --git a/packages/dogfood/test/controlled-by-parent.dogfood.test.ts b/packages/dogfood/test/controlled-by-parent.dogfood.test.ts new file mode 100644 index 0000000000..479bb06a50 --- /dev/null +++ b/packages/dogfood/test/controlled-by-parent.dogfood.test.ts @@ -0,0 +1,101 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Master-detail "controlled by parent" RLS proof (ADR-0055 P2), end-to-end +// through the real HTTP + security stack. +// +// @proof: cbp-controlled-by-parent +// ADR-0055 runtime proof for derived master-detail access. Referenced by the +// liveness ledger entry `object.sharingModel` (packages/spec/liveness/object.json); +// the spec liveness gate fails if this tag is removed. See proof-registry.mts. +// +// The detail (`cbp_note`) carries NO authored RLS — access is DERIVED from the +// master (`cbp_account`, owner-scoped). The proof asserts both directions: +// • a member who cannot read the admin's account can neither READ nor by-id +// WRITE notes under it (the derived guard), and +// • the member CAN read/write notes under an account they own (not over-blocked). + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { cbpStack, cbpSecurity } from './fixtures/cbp-fixture.js'; + +describe('objectstack verify: master-detail controlled-by-parent (#cbp)', () => { + let stack: VerifyStack; + let adminToken: string; + let memberToken: string; + + // Admin-owned graph (member must NOT reach it). + let adminAccountId: string; + let adminNoteId: string; + + beforeAll(async () => { + stack = await bootStack(cbpStack, { security: cbpSecurity() }); + adminToken = await stack.signIn(); + memberToken = await stack.signUp('cbp-member@verify.test'); + + const acc = await stack.apiAs(adminToken, 'POST', '/data/cbp_account', { name: 'admin account' }); + expect(acc.status, `admin account create: ${acc.status} ${await acc.clone().text()}`).toBeLessThan(300); + adminAccountId = idOf(await acc.json()); + + const note = await stack.apiAs(adminToken, 'POST', '/data/cbp_note', { + name: 'admin note', + body: 'admin-only', + account: adminAccountId, + }); + expect(note.status, `admin note create: ${note.status} ${await note.clone().text()}`).toBeLessThan(300); + adminNoteId = idOf(await note.json()); + }, 60_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('precondition: the member cannot read the admin master account (owner RLS)', async () => { + const r = await stack.apiAs(memberToken, 'GET', `/data/cbp_account/${adminAccountId}`); + expect(r.status, 'member must not read the admin account').not.toBe(200); + }); + + it('DERIVED READ: member cannot read a note under an account they cannot read', async () => { + const r = await stack.apiAs(memberToken, 'GET', `/data/cbp_note/${adminNoteId}`); + expect(r.status, 'member must not read the controlled-by-parent note').not.toBe(200); + }); + + it('DERIVED WRITE: member cannot by-id mutate a note whose master they cannot edit', async () => { + const w = await stack.apiAs(memberToken, 'PATCH', `/data/cbp_note/${adminNoteId}`, { body: 'hacked' }); + expect(w.status, 'member by-id write should be denied').not.toBeLessThan(300); + // Ground truth: admin re-reads — the row must be unchanged. + const after = await stack.apiAs(adminToken, 'GET', `/data/cbp_note/${adminNoteId}`); + expect(after.status).toBe(200); + expect(((await after.json()) as any).record?.body).toBe('admin-only'); + }); + + it('NOT over-blocked: member CAN read + write a note under an account they own', async () => { + // Member owns this account → derived access grants the note under it. + const acc = await stack.apiAs(memberToken, 'POST', '/data/cbp_account', { name: 'member account' }); + expect(acc.status).toBeLessThan(300); + const memberAccountId = idOf(await acc.json()); + + const note = await stack.apiAs(memberToken, 'POST', '/data/cbp_note', { + name: 'member note', + body: 'mine', + account: memberAccountId, + }); + expect(note.status, `member note create: ${note.status} ${await note.clone().text()}`).toBeLessThan(300); + const memberNoteId = idOf(await note.json()); + + // Read it back (derived: account IN [memberAccountId]). + const read = await stack.apiAs(memberToken, 'GET', `/data/cbp_note/${memberNoteId}`); + expect(read.status, 'member should read their own note').toBe(200); + + // Edit it (master is member-owned → editable). + const edit = await stack.apiAs(memberToken, 'PATCH', `/data/cbp_note/${memberNoteId}`, { body: 'updated' }); + expect(edit.status, 'member should edit their own note').toBeLessThan(300); + const after = await stack.apiAs(memberToken, 'GET', `/data/cbp_note/${memberNoteId}`); + expect(((await after.json()) as any).record?.body).toBe('updated'); + }); +}); + +function idOf(j: any): string { + const id = j?.id ?? j?.record?.id; + expect(id, 'expected an id from create').toBeTruthy(); + return id as string; +} diff --git a/packages/dogfood/test/derive-topology.test.ts b/packages/dogfood/test/derive-topology.test.ts new file mode 100644 index 0000000000..202cd5689f --- /dev/null +++ b/packages/dogfood/test/derive-topology.test.ts @@ -0,0 +1,154 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Unit proof of ADR-0055 P0 — related-record topological synthesis in +// `deriveCrudCases`. Pure-function tests over synthetic configs (no stack boot): +// dependency ordering, optional vs required relations, and the honest `blocked` +// verdicts (external/missing target, cascade, required-reference cycle). + +import { describe, it, expect } from 'vitest'; +import { deriveCrudCases, fillRelationalRefs, type CrudCase } from '@objectstack/verify'; + +const obj = (name: string, fields: Record) => ({ name, fields }); +const cfg = (...objects: any[]) => ({ manifest: { id: 'fixture' }, objects }); + +function byName(cases: CrudCase[]): Map { + return new Map(cases.map((c) => [c.object, c])); +} +function liveOrder(cases: CrudCase[]): string[] { + return cases.filter((c) => !c.blocked).map((c) => c.object); +} + +describe('deriveCrudCases — topological synthesis (ADR-0055 P0)', () => { + it('orders a required master_detail chain master-before-detail', () => { + const cases = deriveCrudCases( + cfg( + obj('line', { + name: { type: 'text', required: true }, + order: { type: 'master_detail', reference: 'order', required: true }, + }), + obj('order', { + name: { type: 'text', required: true }, + account: { type: 'lookup', reference: 'account', required: true }, + }), + obj('account', { name: { type: 'text', required: true } }), + ), + ); + // account (no deps) → order (needs account) → line (needs order) + expect(liveOrder(cases)).toEqual(['account', 'order', 'line']); + const line = byName(cases).get('line')!; + expect(line.blocked).toBeUndefined(); + expect(line.relationalRefs).toEqual([ + { field: 'order', target: 'order', required: true, multiple: false }, + ]); + }); + + it('records an optional relation as a ref but never blocks on it', () => { + const cases = deriveCrudCases( + cfg( + obj('note', { + name: { type: 'text', required: true }, + related: { type: 'lookup', reference: 'account' }, // optional + }), + obj('account', { name: { type: 'text', required: true } }), + ), + ); + const note = byName(cases).get('note')!; + expect(note.blocked).toBeUndefined(); + expect(note.relationalRefs?.[0]).toMatchObject({ field: 'related', target: 'account', required: false }); + }); + + it('blocks an object whose REQUIRED relation target is external/missing', () => { + const cases = deriveCrudCases( + cfg( + obj('thing', { + name: { type: 'text', required: true }, + ext: { type: 'lookup', reference: 'not_in_app', required: true }, + }), + ), + ); + expect(byName(cases).get('thing')!.blocked).toMatch(/not in app config/); + }); + + it('skips (does not block) an OPTIONAL relation to an external target', () => { + const cases = deriveCrudCases( + cfg( + obj('thing', { + name: { type: 'text', required: true }, + ext: { type: 'lookup', reference: 'not_in_app' }, // optional + }), + ), + ); + const t = byName(cases).get('thing')!; + expect(t.blocked).toBeUndefined(); + expect(t.skippedFields?.some((s) => s.reason.includes('external'))).toBe(true); + }); + + it('cascade-blocks a dependent when its required target is itself blocked', () => { + const cases = deriveCrudCases( + cfg( + obj('detail', { + name: { type: 'text', required: true }, + parent: { type: 'master_detail', reference: 'parent', required: true }, + }), + obj('parent', { + name: { type: 'text', required: true }, + ext: { type: 'lookup', reference: 'missing', required: true }, // blocks parent + }), + ), + ); + const m = byName(cases); + expect(m.get('parent')!.blocked).toMatch(/not in app config/); + expect(m.get('detail')!.blocked).toMatch(/required relational target "parent"/); + }); + + it('blocks a required-reference cycle (incl. required self-reference)', () => { + const cases = deriveCrudCases( + cfg( + obj('a', { name: { type: 'text', required: true }, b: { type: 'lookup', reference: 'b', required: true } }), + obj('b', { name: { type: 'text', required: true }, a: { type: 'lookup', reference: 'a', required: true } }), + obj('tree', { name: { type: 'text', required: true }, parent: { type: 'tree', reference: 'tree', required: true } }), + ), + ); + const m = byName(cases); + expect(m.get('a')!.blocked).toMatch(/cycle/); + expect(m.get('b')!.blocked).toMatch(/cycle/); + expect(m.get('tree')!.blocked).toMatch(/cycle/); + }); + + it('allows an OPTIONAL self-reference (a tree root with null parent)', () => { + const cases = deriveCrudCases( + cfg(obj('cat', { name: { type: 'text', required: true }, parent: { type: 'tree', reference: 'cat' } })), + ); + expect(byName(cases).get('cat')!.blocked).toBeUndefined(); + }); +}); + +describe('fillRelationalRefs — id threading', () => { + const c: CrudCase = { + object: 'line', + body: { name: 'verify-sample' }, + relationalRefs: [ + { field: 'order', target: 'order', required: true, multiple: false }, + { field: 'tags', target: 'tag', required: false, multiple: true }, + ], + }; + + it('fills required + multiple refs from the created registry', () => { + const created = new Map([['order', 'o1'], ['tag', 't1']]); + const { body, missing } = fillRelationalRefs(c, created); + expect(missing).toBeUndefined(); + expect(body).toEqual({ name: 'verify-sample', order: 'o1', tags: ['t1'] }); + }); + + it('reports missing when a REQUIRED target was not created', () => { + const { missing } = fillRelationalRefs(c, new Map()); + expect(missing).toMatch(/required relation "order"/); + }); + + it('leaves an OPTIONAL ref unset when its target was not created', () => { + const created = new Map([['order', 'o1']]); + const { body, missing } = fillRelationalRefs(c, created); + expect(missing).toBeUndefined(); + expect(body).toEqual({ name: 'verify-sample', order: 'o1' }); // no tags + }); +}); diff --git a/packages/dogfood/test/fixtures/cbp-fixture.ts b/packages/dogfood/test/fixtures/cbp-fixture.ts new file mode 100644 index 0000000000..b4f4d686bc --- /dev/null +++ b/packages/dogfood/test/fixtures/cbp-fixture.ts @@ -0,0 +1,80 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// Master-detail "controlled by parent" fixture (ADR-0055 P2). +// +// A two-object app exercising derived access: `cbp_account` (the MASTER) is +// owner-scoped via RLS on `created_by`; `cbp_note` (the DETAIL) declares +// `sharingModel: 'controlled_by_parent'` with a required master_detail field +// pointing at the account. The detail carries NO authored RLS — its access is +// derived from the master by the security layer (read: `account IN (accessible +// account ids)`; write: requires master edit access). +// +// The proof then asserts both directions: a member who cannot read the admin's +// account cannot read OR by-id-write notes under it, but CAN read/write notes +// under an account they own. + +import { defineStack } from '@objectstack/spec'; +import { ObjectSchema, Field } from '@objectstack/spec/data'; +import { PermissionSetSchema, RLS, type PermissionSet } from '@objectstack/spec/security'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; + +/** MASTER — owner-scoped account. */ +export const CbpAccount = ObjectSchema.create({ + name: 'cbp_account', + label: 'CBP Account', + pluralLabel: 'CBP Accounts', + fields: { + name: Field.text({ label: 'Name', required: true }), + }, +}); + +/** DETAIL — note whose access is controlled by its parent account. */ +export const CbpNote = ObjectSchema.create({ + name: 'cbp_note', + label: 'CBP Note', + pluralLabel: 'CBP Notes', + sharingModel: 'controlled_by_parent', + fields: { + name: Field.text({ label: 'Name', required: true }), + body: Field.text({ label: 'Body' }), + account: Field.masterDetail('cbp_account', { label: 'Account', required: true }), + }, +}); + +export const cbpStack = defineStack({ + manifest: { + id: 'com.dogfood.cbp_fixture', + namespace: 'cbp', + version: '0.0.0', + type: 'app', + name: 'Controlled-by-Parent Fixture', + description: 'Master-detail app exercising controlled_by_parent derived access (ADR-0055).', + }, + objects: [CbpAccount, CbpNote], +}); + +const FIXTURE_MEMBER_SET = 'cbp_fixture_member'; + +/** + * The fallback permission set for a fresh member: full CRUD on both objects (so + * requests reach the RLS layer rather than being denied by RBAC) and an OWNER + * policy on the MASTER account. The detail `cbp_note` gets NO RLS — its scoping + * is derived from the master by `sharingModel: 'controlled_by_parent'`. + */ +export const cbpMemberSet: PermissionSet = PermissionSetSchema.parse({ + name: FIXTURE_MEMBER_SET, + label: 'CBP Fixture Member — owner-scoped account, controlled-by-parent notes', + isProfile: true, + objects: { + cbp_account: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + cbp_note: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, + rowLevelSecurity: [RLS.ownerPolicy('cbp_account', 'created_by')], +}); + +export function cbpSecurity(): SecurityPlugin { + return new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, cbpMemberSet], + fallbackPermissionSet: FIXTURE_MEMBER_SET, + }); +} diff --git a/packages/dogfood/test/showcase-invoice-cbp.dogfood.test.ts b/packages/dogfood/test/showcase-invoice-cbp.dogfood.test.ts new file mode 100644 index 0000000000..d4df578e79 --- /dev/null +++ b/packages/dogfood/test/showcase-invoice-cbp.dogfood.test.ts @@ -0,0 +1,116 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// SHOWCASE scenario proof for ADR-0055 — `showcase_invoice_line` is declared +// `sharingModel: 'controlled_by_parent'`, so a line's access is derived from its +// parent `showcase_invoice`. This exercises the feature on the REAL showcase +// metadata, end-to-end through the real HTTP stack (the same requests a browser +// would issue; the security boundary is server-side). +// +// Setup uses system inserts (to sidestep the showcase's authoring-validation +// rules — not what's under test); the controlled-by-parent READ/WRITE behavior +// is then exercised as a real member over HTTP. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { SecurityPlugin, securityDefaultPermissionSets } from '@objectstack/plugin-security'; +import { PermissionSetSchema, RLS } from '@objectstack/spec/security'; + +const MEMBER_EMAIL = 'showcase-cbp-member@verify.test'; +const ADMIN_EMAIL = 'admin@objectos.ai'; + +// Member fallback set: full CRUD on the invoice graph (so requests reach the RLS +// layer) + an OWNER policy on the MASTER invoice. The detail line carries no RLS +// — its scoping is derived from the master by `controlled_by_parent`. +const memberSet = PermissionSetSchema.parse({ + name: 'showcase_cbp_member', + label: 'Showcase CBP Member', + isProfile: true, + objects: { + showcase_account: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + showcase_product: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + showcase_invoice: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + showcase_invoice_line: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }, + }, + rowLevelSecurity: [RLS.ownerPolicy('showcase_invoice', 'created_by')], +}); + +describe('showcase: invoice-line controlled-by-parent (ADR-0055)', () => { + let stack: VerifyStack; + let memberToken: string; + // eslint-disable-next-line @typescript-eslint/no-explicit-any + let ql: any; + let adminInvoiceId: string; + let adminLineId: string; + let memberLineId: string; + + beforeAll(async () => { + stack = await bootStack(showcaseStack, { + security: new SecurityPlugin({ + defaultPermissionSets: [...securityDefaultPermissionSets, memberSet], + fallbackPermissionSet: 'showcase_cbp_member', + }), + }); + await stack.signIn(); // seed dev admin + memberToken = await stack.signUp(MEMBER_EMAIL); + + // eslint-disable-next-line @typescript-eslint/no-explicit-any + ql = await stack.kernel.getServiceAsync('objectql'); + const sys = { context: { isSystem: true } }; + const idOf = (r: any) => r?.id ?? r?.record?.id ?? r; + + const users = (await ql.find('sys_user', { + where: { email: { $in: [ADMIN_EMAIL, MEMBER_EMAIL] } }, + context: { isSystem: true }, + })) as Array<{ id: string; email: string }>; + const adminId = users.find((u) => u.email === ADMIN_EMAIL)!.id; + const memberId = users.find((u) => u.email === MEMBER_EMAIL)!.id; + + // Shared (non-owner-scoped) account + product. + const acc = idOf(await ql.insert('showcase_account', { name: 'CBP Co', status: 'prospect' }, sys)); + const prod = idOf(await ql.insert('showcase_product', { name: 'Widget' }, sys)); + + // Two invoices: one owned by the admin, one by the member. + adminInvoiceId = idOf(await ql.insert('showcase_invoice', { name: 'INV-ADMIN', account: acc, status: 'draft', created_by: adminId }, sys)); + const memberInvoiceId = idOf(await ql.insert('showcase_invoice', { name: 'INV-MEMBER', account: acc, status: 'draft', created_by: memberId }, sys)); + + // A line under each invoice (the detail under test). + adminLineId = idOf(await ql.insert('showcase_invoice_line', { invoice: adminInvoiceId, product: prod, quantity: 1, unit_price: 10, description: 'admin line' }, sys)); + memberLineId = idOf(await ql.insert('showcase_invoice_line', { invoice: memberInvoiceId, product: prod, quantity: 1, unit_price: 20, description: 'member line' }, sys)); + + // Premise sanity: the admin invoice really is admin-owned (else the proof is void). + const adminInv = await ql.findOne('showcase_invoice', { where: { id: adminInvoiceId }, context: { isSystem: true } }); + expect(adminInv?.created_by, 'admin invoice must be admin-owned').toBe(adminId); + }, 60_000); + + afterAll(async () => { + await stack?.stop(); + }); + + it('precondition: member cannot read the admin-owned master invoice (owner RLS)', async () => { + const r = await stack.apiAs(memberToken, 'GET', `/data/showcase_invoice/${adminInvoiceId}`); + expect(r.status).not.toBe(200); + }); + + it('DERIVED READ: member cannot read a line under an invoice they cannot read', async () => { + const r = await stack.apiAs(memberToken, 'GET', `/data/showcase_invoice_line/${adminLineId}`); + expect(r.status, 'line under unreadable invoice must be hidden').not.toBe(200); + }); + + it('DERIVED WRITE: member cannot by-id mutate a line whose master they cannot edit', async () => { + const w = await stack.apiAs(memberToken, 'PATCH', `/data/showcase_invoice_line/${adminLineId}`, { description: 'hacked' }); + expect(w.status).not.toBeLessThan(300); + const after = await ql.findOne('showcase_invoice_line', { where: { id: adminLineId }, context: { isSystem: true } }); + expect(after?.description, 'admin line must be unchanged').toBe('admin line'); + }); + + it('NOT over-blocked: member CAN read + edit a line under an invoice they own', async () => { + const read = await stack.apiAs(memberToken, 'GET', `/data/showcase_invoice_line/${memberLineId}`); + expect(read.status, 'member should read their own line').toBe(200); + + const edit = await stack.apiAs(memberToken, 'PATCH', `/data/showcase_invoice_line/${memberLineId}`, { description: 'updated' }); + expect(edit.status, 'member should edit their own line').toBeLessThan(300); + const after = await ql.findOne('showcase_invoice_line', { where: { id: memberLineId }, context: { isSystem: true } }); + expect(after?.description).toBe('updated'); + }); +}); diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 02ab26d53f..63410c19ec 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -106,6 +106,8 @@ export class SecurityPlugin implements Plugin { */ private metadata: any = null; private ql: any = null; + /** ADR-0055: cache the resolved master-detail relation per controlled_by_parent object. */ + private cbpRelCache = new Map(); private dbLoader?: (names: string[]) => Promise; private logger: { info?: (...a: any[]) => void; warn?: (...a: any[]) => void; error?: (...a: any[]) => void } = {}; @@ -399,6 +401,25 @@ export class SecurityPlugin implements Plugin { } } + // 2.8. ADR-0055 — controlled-by-parent WRITE: a detail write (insert/update/ + // delete) requires edit access to its master. The detail itself carries no + // authored RLS, so the #1994 pre-image check above is a no-op for it; this + // closes the by-id write path by checking the master instead. + if ( + (opCtx.operation === 'insert' || opCtx.operation === 'update' || opCtx.operation === 'delete') && + permissionSets.length > 0 && + !!opCtx.context?.userId && + this.ql + ) { + await this.assertControlledByParentWrite( + permissionSets, + opCtx.object, + opCtx.operation, + opCtx, + opCtx.context, + ); + } + // 2.5. Field-Level Security write enforcement. // // The client-side masker (ObjectForm / inline grid) already hides @@ -479,18 +500,28 @@ export class SecurityPlugin implements Plugin { // the public getReadFilter service via computeRlsFilter, so the engine // find-path and the analytics raw-SQL path enforce identical scoping. if (opCtx.ast) { + const extra: Record[] = []; const rlsFilter = await this.computeRlsFilter( permissionSets, opCtx.object, opCtx.operation, opCtx.context, ); - if (rlsFilter) { - if (opCtx.ast.where) { - opCtx.ast.where = { $and: [opCtx.ast.where, rlsFilter] }; - } else { - opCtx.ast.where = rlsFilter; - } + if (rlsFilter) extra.push(rlsFilter); + // ADR-0055: a controlled_by_parent object derives its read scope from the + // master record — `masterFK IN (accessible master ids)`, AND-ed in. + const cbpFilter = await this.computeControlledByParentFilter( + permissionSets, + opCtx.object, + opCtx.context, + ); + if (cbpFilter) extra.push(cbpFilter); + if (extra.length) { + opCtx.ast.where = opCtx.ast.where + ? { $and: [opCtx.ast.where, ...extra] } + : extra.length === 1 + ? extra[0] + : { $and: extra }; } } @@ -787,6 +818,154 @@ export class SecurityPlugin implements Plugin { return rlsFilter; } + /** + * Resolve a controlled_by_parent object's master-detail relation (the FK field + * key + the master object name), or null. Prefers a required `master_detail` + * field; falls back to any `master_detail`, then a required `lookup`. Cached. + */ + private resolveCbpRelation(object: string): { fk: string; master: string } | null { + if (this.cbpRelCache.has(object)) return this.cbpRelCache.get(object) ?? null; + let rel: { fk: string; master: string } | null = null; + const schema = typeof this.ql?.getSchema === 'function' ? this.ql.getSchema(object) : null; + const fields = schema?.fields; + const entries: Array<[string, any]> = Array.isArray(fields) + ? fields.map((f: any) => [f?.name, f] as [string, any]) + : fields && typeof fields === 'object' + ? (Object.entries(fields) as Array<[string, any]>) + : []; + const ref = (f: any) => f?.reference ?? f?.reference_to ?? f?.referenceTo; + const pick = (pred: (f: any) => boolean) => entries.find(([, f]) => pred(f) && ref(f)); + const found = + pick((f) => f?.type === 'master_detail' && f?.required) ?? + pick((f) => f?.type === 'master_detail') ?? + pick((f) => f?.type === 'lookup' && f?.required); + if (found) rel = { fk: String(found[0]), master: String(ref(found[1])) }; + this.cbpRelCache.set(object, rel); + return rel; + } + + /** + * ADR-0055 — master-detail "controlled by parent" READ derivation. + * + * For an object whose `sharingModel` is `controlled_by_parent`, access is + * derived from the master: return a filter `masterFK IN ()`. The id set is resolved by running the MASTER's own read RLS + * (reused via `computeRlsFilter`) under a system context — no middleware + * re-entry, so no recursion. An empty set yields `{ masterFK: { $in: [] } }`, + * which matches no rows (fail closed). A misconfigured object (no + * master_detail/lookup to derive from) denies all reads (defense-in-depth; + * spec validation should prevent authoring it). Returns null when the object is + * not controlled_by_parent. + * + * v1 scope (ADR-0055): single level — the master's OWN controlled_by_parent is + * not traversed transitively; master accessibility is the master's RLS filter + * (sharing-service grants on the master are not folded in). + */ + private async computeControlledByParentFilter( + permissionSets: PermissionSet[], + object: string, + context: any, + ): Promise | null> { + if (!this.ql || !context?.userId) return null; + const schema = typeof this.ql.getSchema === 'function' ? this.ql.getSchema(object) : null; + const sharingModel = schema?.sharingModel ?? schema?.security?.sharingModel; + if (sharingModel !== 'controlled_by_parent') return null; + + const rel = this.resolveCbpRelation(object); + if (!rel) return { ...RLS_DENY_FILTER }; + + const masterFilter = await this.computeRlsFilter(permissionSets, rel.master, 'find', context); + let masterIds: string[] = []; + try { + const rows = await this.ql.find(rel.master, { + where: masterFilter ?? {}, + fields: ['id'], + context: { isSystem: true }, + }); + masterIds = (Array.isArray(rows) ? rows : []) + .map((r: any) => r?.id) + .filter((id: any) => id != null); + } catch { + masterIds = []; + } + return { [rel.fk]: { $in: masterIds } }; + } + + /** + * ADR-0055 — master-detail "controlled by parent" WRITE enforcement. + * + * A by-id write (insert/update/delete) to a controlled_by_parent detail + * requires EDIT access to its master: the caller must hold CRUD `update` on the + * master object AND the master row must be visible under the master's write RLS. + * This is the write-side companion to the read derivation — the RLS read filter + * never applies to a by-id write (the #1994 class), so without this a member + * could mutate a detail under a master they cannot edit. Throws on denial; + * no-op when the object is not controlled_by_parent. + * + * v1 scope: single-id writes. Bulk writes flow through the AST and are already + * scoped by the controlled-by-parent READ filter (to readable masters). + */ + private async assertControlledByParentWrite( + permissionSets: PermissionSet[], + object: string, + operation: string, + opCtx: any, + context: any, + ): Promise { + const schema = typeof this.ql?.getSchema === 'function' ? this.ql.getSchema(object) : null; + const sharingModel = schema?.sharingModel ?? schema?.security?.sharingModel; + if (sharingModel !== 'controlled_by_parent') return; + + const deny = (reason: string, recordId?: unknown) => { + throw new PermissionDeniedError( + `[Security] Access denied: ${operation} on '${object}' requires edit access to its master record (${reason})`, + { operation, object, recordId }, + ); + }; + + const rel = this.resolveCbpRelation(object); + if (!rel) deny('controlled_by_parent declared but no master_detail relation'); + + // Resolve the master id: from the incoming body on insert, else from the + // target row (read as system — we only need its FK value). + let masterId: unknown; + if (operation === 'insert') { + const data = opCtx.data; + masterId = data && typeof data === 'object' && !Array.isArray(data) ? (data as any)[rel!.fk] : undefined; + } else { + const targetId = this.extractSingleId(opCtx); + if (targetId == null) return; // bulk write — scoped by the read filter on the AST + let row: any = null; + try { + row = await this.ql.findOne(object, { where: { id: targetId }, context: { isSystem: true } }); + } catch { + row = null; + } + if (!row) deny('target record not found', targetId); + masterId = row[rel!.fk]; + } + if (masterId == null) deny('detail record has no master reference'); + + // Master edit access = CRUD update on the master AND master row visible under + // the master's write RLS. + if (!this.permissionEvaluator.checkObjectPermission('update', rel!.master, permissionSets)) { + deny(`no edit permission on master '${rel!.master}'`, masterId); + } + const masterWriteFilter = await this.computeRlsFilter(permissionSets, rel!.master, 'update', context); + if (masterWriteFilter) { + let visible: unknown = null; + try { + visible = await this.ql.findOne(rel!.master, { + where: { $and: [{ id: masterId }, masterWriteFilter] }, + context, + }); + } catch { + visible = null; + } + if (!visible) deny(`master '${rel!.master}' not editable by this user (row-level security)`, masterId); + } + } + /** * Collect all RLS policies from permission sets applicable to the given object/operation. */ diff --git a/packages/spec/liveness/README.md b/packages/spec/liveness/README.md index 3177793a41..0d648fa1ee 100644 --- a/packages/spec/liveness/README.md +++ b/packages/spec/liveness/README.md @@ -77,6 +77,7 @@ binding lands one class at a time (ADR-0054 §3), never as a big-bang backfill. |---|---|---|---| | Field types | ✅ enforced | `field.type` | `field-zoo-roundtrip.dogfood.test.ts#field-type-roundtrip` | | RLS / sharing | ✅ enforced | `permission.rowLevelSecurity.using` | `rls-fixture.dogfood.test.ts#rls-by-id-write` | +| Master-detail controlled-by-parent | ✅ enforced | `object.sharingModel` | `controlled-by-parent.dogfood.test.ts#cbp-controlled-by-parent` | | Flow nodes | ✅ enforced | `flow.nodes.type` | `flow-node.dogfood.test.ts#flow-node-execution` | | Analytics dims/measures | ✅ enforced | `dataset.dimensions.dateGranularity` | `analytics-timezone.dogfood.test.ts#analytics-tz-bucketing` | | Form layout/section/widget | ⛔ pending | — | none yet (form surface not yet governed) | diff --git a/packages/spec/liveness/object.json b/packages/spec/liveness/object.json index 13b6bf2e3c..3c33d11ab8 100644 --- a/packages/spec/liveness/object.json +++ b/packages/spec/liveness/object.json @@ -98,7 +98,9 @@ }, "sharingModel": { "status": "live", - "evidence": "packages/plugins/plugin-sharing/src/sharing-service.ts:54" + "evidence": "packages/plugins/plugin-sharing/src/sharing-service.ts:54", + "proof": "packages/dogfood/test/controlled-by-parent.dogfood.test.ts#cbp-controlled-by-parent", + "note": "ADR-0055 high-risk class (sharing): the `controlled_by_parent` value derives a detail object's access from its master — the security layer injects `masterFK IN (accessible master ids)` on reads and requires master edit-access on by-id writes. The proof asserts a member who cannot read the master can neither read nor by-id-write the detail, and is not over-blocked on a master they own." }, "publicSharing": { "status": "live", diff --git a/packages/spec/scripts/liveness/proof-registry.mts b/packages/spec/scripts/liveness/proof-registry.mts index df694931bd..014891bd7f 100644 --- a/packages/spec/scripts/liveness/proof-registry.mts +++ b/packages/spec/scripts/liveness/proof-registry.mts @@ -76,6 +76,15 @@ export const HIGH_RISK_CLASSES: HighRiskClass[] = [ // pre-image check — the property whose end-to-end correctness the proof guards. ledgerBindings: [{ type: 'permission', path: 'rowLevelSecurity.using' }], }, + { + id: 'sharing-controlled-by-parent', + label: 'Master-detail controlled-by-parent', + summary: "a detail record's read/write access derived from its master record (ADR-0055).", + proofId: 'cbp-controlled-by-parent', + proofRef: 'packages/dogfood/test/controlled-by-parent.dogfood.test.ts#cbp-controlled-by-parent', + bound: true, + ledgerBindings: [{ type: 'object', path: 'sharingModel' }], + }, { id: 'analytics', label: 'Analytics dimensions / measures', diff --git a/packages/spec/scripts/liveness/proof-registry.test.ts b/packages/spec/scripts/liveness/proof-registry.test.ts index e35c054a37..ce796dbeaa 100644 --- a/packages/spec/scripts/liveness/proof-registry.test.ts +++ b/packages/spec/scripts/liveness/proof-registry.test.ts @@ -107,6 +107,7 @@ describe('registry invariants', () => { 'flow/nodes.type', 'permission/rowLevelSecurity.using', 'dataset/dimensions.dateGranularity', + 'object/sharingModel', ].sort(), ); }); @@ -126,6 +127,7 @@ describe('real proof wiring resolves', () => { permission: 'packages/spec/liveness/permission.json', flow: 'packages/spec/liveness/flow.json', dataset: 'packages/spec/liveness/dataset.json', + object: 'packages/spec/liveness/object.json', }; function ledgerEntry(type: string, path: string): any { diff --git a/packages/spec/src/data/object.zod.ts b/packages/spec/src/data/object.zod.ts index 1a88cb83d3..4dc31cdcfe 100644 --- a/packages/spec/src/data/object.zod.ts +++ b/packages/spec/src/data/object.zod.ts @@ -587,8 +587,18 @@ const ObjectSchemaBase = z.object({ */ enable: ObjectCapabilities.optional().describe('Enabled system features modules'), - /** Sharing Model */ - sharingModel: z.enum(['private', 'read', 'read_write', 'full']).optional().describe('Default sharing model'), + /** + * Sharing Model (org-wide default). + * + * `controlled_by_parent` (ADR-0055) makes this a DETAIL object in a + * master-detail relationship: its access is *derived* from the master record + * — a user sees/edits a detail only if they can see/edit its master. The + * object must declare exactly one required `master_detail` field identifying + * the master; the security layer auto-injects `masterFK IN (accessible master + * ids)` on reads and requires master edit-access on by-id writes. No RLS policy + * is authored — the inheritance is derived from the relationship. + */ + sharingModel: z.enum(['private', 'read', 'read_write', 'full', 'controlled_by_parent']).optional().describe('Default sharing model'), /** * Public Share-Link Policy diff --git a/packages/verify/src/derive.ts b/packages/verify/src/derive.ts index 25d93922e8..c50952c9d7 100644 --- a/packages/verify/src/derive.ts +++ b/packages/verify/src/derive.ts @@ -9,8 +9,14 @@ // write it → read it back → assert type fidelity" for every object, then runs // it against the real in-process stack. // -// v0 derives per-object CRUD round-trip cases. RLS cross-owner denial (the -// #1994 invariant) is v1 — it needs the multi-user harness + sharing service. +// v0 derived per-object CRUD round-trip cases and SKIPPED any object with a +// required relation (lookup / master_detail) — it had no target id to write. +// v1 (ADR-0055 P0) closes that gap with RELATED-RECORD TOPOLOGICAL SYNTHESIS: +// build the object dependency graph from required relational fields, topologically +// order it (targets before dependents), and have the runner thread real ids — so +// relationship-dense objects (the core of real apps) are verified, not skipped. +// What it still can't satisfy (required-reference cycles, external/missing targets) +// is reported `blocked` with a precise reason — the gate stays honest. /* eslint-disable @typescript-eslint/no-explicit-any */ @@ -30,12 +36,20 @@ export interface DerivedAssert { value: unknown; kind: AssertKind; } +/** A relational field to fill with a real target id at run time (threaded by the runner). */ +export interface RelationalRef { + field: string; // the FK field key on this object + target: string; // the referenced object name + required: boolean; + multiple: boolean; // store as an array of ids +} export interface CrudCase { object: string; - blocked?: string; // why this object can't be auto-CRUD'd (e.g. required lookup) + blocked?: string; // why this object can't be auto-CRUD'd (e.g. required-reference cycle) body?: Record; asserts?: DerivedAssert[]; skippedFields?: Array<{ name: string; type: string; reason: string }>; + relationalRefs?: RelationalRef[]; // resolved against created-record ids by the runner } function clampNum(f: any, fallback: number): number { @@ -84,44 +98,166 @@ function synth(type: string, f: any): { value: unknown; kind: AssertKind } | nul } } +/** The target object a relational field references (snake_case object name), or null. */ +function relationTarget(f: any): string | null { + const ref = f?.reference ?? f?.reference_to ?? f?.referenceTo; + return typeof ref === 'string' && ref.length > 0 ? ref : null; +} + +interface Draft { + name: string; + body: Record; + asserts: DerivedAssert[]; + skippedFields: Array<{ name: string; type: string; reason: string }>; + relationalRefs: RelationalRef[]; + requiredTargets: string[]; // referenced objects that MUST exist + be ordered-before + blocked?: string; +} + /** - * Derive one CRUD round-trip case per authorable object in the config. - * An object whose REQUIRED fields can't be synthesized (e.g. a required lookup - * needing a target record) is reported `blocked` rather than silently skipped. + * Derive one CRUD round-trip case per authorable object, in DEPENDENCY ORDER. + * + * Required relational fields no longer block the object outright: the referenced + * target is created first (topological order) and the runner threads its real id + * in. Only genuinely unsatisfiable shapes are `blocked`: + * - a required relation whose target is missing from the app config (external), or + * - a required-reference cycle (incl. a required self-reference), or + * - a required relation whose target is itself blocked (cascade), or + * - a required non-relational field that can't be synthesized (unchanged from v0). */ export function deriveCrudCases(config: any): CrudCase[] { - const cases: CrudCase[] = []; - for (const obj of config?.objects ?? []) { + const objects: any[] = config?.objects ?? []; + const byName = new Map(); + for (const o of objects) if (o?.name) byName.set(o.name, o); + + const drafts = new Map(); + + // ── Pass 1: per-object field classification ─────────────────────────────── + for (const obj of objects) { + if (!obj?.name) continue; const fields: Record = obj?.fields ?? {}; - const body: Record = {}; - const asserts: DerivedAssert[] = []; - const skippedFields: Array<{ name: string; type: string; reason: string }> = []; - let blocked: string | undefined; + const d: Draft = { + name: obj.name, body: {}, asserts: [], skippedFields: [], relationalRefs: [], requiredTargets: [], + }; for (const [name, f] of Object.entries(fields)) { const type = String((f as any)?.type ?? '').toLowerCase(); + const isRequired = !!(f as any)?.required; if (SYSTEM_NAMES.has(name) || (f as any)?.system || (f as any)?.readonly) continue; if (COMPUTED.has(type)) continue; - if (RELATIONAL.has(type) || STRUCTURED.has(type) || MEDIA.has(type)) { - if ((f as any)?.required) { blocked = `required ${type} field "${name}" (needs target/structured value)`; break; } - skippedFields.push({ name, type, reason: 'unsynthesizable-optional' }); + if (RELATIONAL.has(type)) { + const target = relationTarget(f); + if (!target) { + if (isRequired) { d.blocked = `required ${type} field "${name}" has no \`reference\` target`; break; } + d.skippedFields.push({ name, type, reason: 'relation-missing-reference' }); + continue; + } + if (!byName.has(target)) { + // External / cross-app target — we can't synthesize a record for it. + if (isRequired) { d.blocked = `required ${type} field "${name}" → target "${target}" not in app config`; break; } + d.skippedFields.push({ name, type, reason: `relation-target-external:${target}` }); + continue; + } + d.relationalRefs.push({ field: name, target, required: isRequired, multiple: !!(f as any)?.multiple }); + if (isRequired) d.requiredTargets.push(target); + continue; + } + + if (STRUCTURED.has(type) || MEDIA.has(type)) { + if (isRequired) { d.blocked = `required ${type} field "${name}" (needs structured/media value)`; break; } + d.skippedFields.push({ name, type, reason: 'unsynthesizable-optional' }); continue; } const s = synth(type, f); if (!s) { - if ((f as any)?.required) { blocked = `required field "${name}" of type "${type}" is not synthesizable`; break; } - skippedFields.push({ name, type, reason: 'no-synth' }); + if (isRequired) { d.blocked = `required field "${name}" of type "${type}" is not synthesizable`; break; } + d.skippedFields.push({ name, type, reason: 'no-synth' }); continue; } - body[name] = s.value; - if (s.kind !== 'none') asserts.push({ field: name, type, value: s.value, kind: s.kind }); + d.body[name] = s.value; + if (s.kind !== 'none') d.asserts.push({ field: name, type, value: s.value, kind: s.kind }); + } + + drafts.set(obj.name, d); + } + + // ── Pass 2: cascade-block on missing/blocked required targets (to fixpoint) ─ + let changed = true; + while (changed) { + changed = false; + for (const d of drafts.values()) { + if (d.blocked) continue; + for (const t of d.requiredTargets) { + const td = drafts.get(t); + if (!td || td.blocked) { + d.blocked = `required relational target "${t}" is ${!td ? 'missing' : 'not synthesizable'}`; + changed = true; + break; + } + } + } + } + + // ── Pass 3: topological order (targets before dependents) over required edges ─ + const emitted = new Set(); + const order: string[] = []; + const live = [...drafts.values()].filter((d) => !d.blocked).map((d) => d.name); + let progress = true; + while (progress) { + progress = false; + for (const name of live) { + if (emitted.has(name)) continue; + const d = drafts.get(name)!; + if (d.requiredTargets.every((t) => emitted.has(t))) { + emitted.add(name); + order.push(name); + progress = true; + } } + } + // Residue = a required-reference cycle (incl. required self-reference). + for (const name of live) { + if (!emitted.has(name)) drafts.get(name)!.blocked = 'unsatisfiable required-reference cycle'; + } - // Every object needs a name-ish required text; synth already covers `name`. - if (blocked) cases.push({ object: obj.name, blocked }); - else cases.push({ object: obj.name, body, asserts, skippedFields }); + // ── Assemble: ordered live cases first, then blocked ones ────────────────── + const cases: CrudCase[] = []; + for (const name of order) { + const d = drafts.get(name)!; + cases.push({ + object: d.name, + body: d.body, + asserts: d.asserts, + skippedFields: d.skippedFields, + ...(d.relationalRefs.length ? { relationalRefs: d.relationalRefs } : {}), + }); + } + for (const d of drafts.values()) { + if (d.blocked) cases.push({ object: d.name, blocked: d.blocked }); } return cases; } + +/** + * Resolve a case's relational fields against the registry of already-created + * record ids, returning the body to POST. When a REQUIRED target has no created + * record (its own creation failed at run time), returns a `missing` reason so the + * caller can skip rather than POST an invalid body. + */ +export function fillRelationalRefs( + c: CrudCase, + created: Map, +): { body: Record; missing?: string } { + const body: Record = { ...(c.body ?? {}) }; + for (const ref of c.relationalRefs ?? []) { + const id = created.get(ref.target); + if (id == null) { + if (ref.required) return { body, missing: `required relation "${ref.field}" → no created "${ref.target}" record` }; + continue; // optional: leave unset + } + body[ref.field] = ref.multiple ? [id] : id; + } + return { body }; +} diff --git a/packages/verify/src/index.ts b/packages/verify/src/index.ts index 64b12436eb..6874babcce 100644 --- a/packages/verify/src/index.ts +++ b/packages/verify/src/index.ts @@ -10,8 +10,8 @@ export { bootStack } from './harness.js'; export type { VerifyStack, BootOptions } from './harness.js'; -export { deriveCrudCases } from './derive.js'; -export type { CrudCase, DerivedAssert, AssertKind } from './derive.js'; +export { deriveCrudCases, fillRelationalRefs } from './derive.js'; +export type { CrudCase, DerivedAssert, AssertKind, RelationalRef } from './derive.js'; export { runCrudVerification, formatReport } from './verify.js'; export type { VerifyReport, ObjectVerifyResult } from './verify.js'; diff --git a/packages/verify/src/rls.ts b/packages/verify/src/rls.ts index 44bc1b435a..57156176f8 100644 --- a/packages/verify/src/rls.ts +++ b/packages/verify/src/rls.ts @@ -18,7 +18,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import type { VerifyStack } from './harness.js'; -import { deriveCrudCases } from './derive.js'; +import { deriveCrudCases, fillRelationalRefs } from './derive.js'; const PROBE_TYPES = new Set(['text', 'textarea', 'string']); const MUTATION = 'rls-mutated-by-B'; @@ -43,6 +43,10 @@ export async function runRlsProofs( ): Promise { const cases = deriveCrudCases(config); const results: RlsResult[] = []; + // Admin-created ids, threaded so a detail's required relation points at a real + // master (topological order created it first) — lets the #1994 invariant reach + // relationship-dense objects, not just the leaves. + const createdIds = new Map(); for (const c of cases) { if (c.blocked) { results.push({ object: c.object, status: 'skipped', detail: c.blocked }); continue; } @@ -52,8 +56,11 @@ export async function runRlsProofs( const probe = (c.asserts ?? []).find((a) => PROBE_TYPES.has(a.type)); if (!probe) { results.push({ object: c.object, status: 'skipped', detail: 'no plain-text probe field' }); continue; } + const { body, missing } = fillRelationalRefs(c, createdIds); + if (missing) { results.push({ object: c.object, status: 'skipped', detail: missing }); continue; } + // Admin (owner) creates the record. - const created = await stack.apiAs(adminToken, 'POST', `/data/${c.object}`, c.body); + const created = await stack.apiAs(adminToken, 'POST', `/data/${c.object}`, body); if (created.status >= 300) { results.push({ object: c.object, status: 'skipped', detail: `admin create failed (${created.status})` }); continue; @@ -61,6 +68,7 @@ export async function runRlsProofs( const cj = (await created.json()) as any; const id = cj?.id ?? cj?.record?.id; if (!id) { results.push({ object: c.object, status: 'skipped', detail: 'no id from create' }); continue; } + createdIds.set(c.object, String(id)); // Member B: can they SEE it? const bRead = await stack.apiAs(memberToken, 'GET', `/data/${c.object}/${id}`); diff --git a/packages/verify/src/verify.ts b/packages/verify/src/verify.ts index c660a039dd..67400c1a58 100644 --- a/packages/verify/src/verify.ts +++ b/packages/verify/src/verify.ts @@ -10,7 +10,7 @@ /* eslint-disable @typescript-eslint/no-explicit-any */ import type { VerifyStack } from './harness.js'; -import { deriveCrudCases, type CrudCase } from './derive.js'; +import { deriveCrudCases, fillRelationalRefs, type CrudCase } from './derive.js'; export interface ObjectVerifyResult { object: string; @@ -57,15 +57,24 @@ export async function runCrudVerification( ): Promise { const cases = deriveCrudCases(config); const results: ObjectVerifyResult[] = []; + // Registry of created record ids, threaded so a dependent object's required + // relation is filled with a real target id (topological order guarantees the + // target was created first). One instance per object — reused across refs. + const createdIds = new Map(); for (const c of cases as CrudCase[]) { if (c.blocked) { results.push({ object: c.object, status: 'skipped', reason: c.blocked }); continue; } + const { body, missing } = fillRelationalRefs(c, createdIds); + if (missing) { + results.push({ object: c.object, status: 'skipped', reason: missing }); + continue; + } let created: Response; try { - created = await stack.apiAs(token, 'POST', `/data/${c.object}`, c.body); + created = await stack.apiAs(token, 'POST', `/data/${c.object}`, body); } catch (e: any) { results.push({ object: c.object, status: 'create-failed', detail: String(e?.message ?? e).slice(0, 200) }); continue; @@ -91,6 +100,7 @@ export async function runCrudVerification( results.push({ object: c.object, status: 'create-failed', detail: 'no id returned' }); continue; } + createdIds.set(c.object, String(id)); const got = await stack.apiAs(token, 'GET', `/data/${c.object}/${id}`); if (got.status !== 200) {