Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 16 additions & 0 deletions .changeset/24932-cbp-leaf-switch.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
---
'@objectstack/plugin-security': patch
'@objectstack/service-analytics': patch
'@objectstack/driver-turso': patch
'@objectstack/spec': patch
---

fix(plugin-security): a `controlled_by_parent` detail's read scope is the engine's `ReadableIds` leaf over its master — one statement per read, no master id list, 200 past the dialect's bind limit

Clause-②: no
Metadata-protocol: none

- **`@objectstack/plugin-security`.** The read derivation for a `controlled_by_parent` object answers `{ masterFK: { $in: ReadableIds.of(master) } }` (ADR-0139 D6, the leaf's seventh consumer) in place of a master id list it pre-scanned under a system context and bound into `$in`. The engine composes the leaf at its executor boundary from this service's own `canReadObject` / `getReadFilter` of the master — recursively down a master chain — and the driver compiles one statement with the master inside a subquery. Measured before: 2 statements per find and 4 per find + count on a detail, and `DATABASE_ERROR` / 500 past ~32,800 masters (`too many SQL variables`). Now: 1 and 2, no scan of the master, and 200 with the right rows at 33,000 masters. Every row set is unchanged: plain, restricted, unrestricted (`viewAllRecords`), delegated (the ADR-0090 D10 intersection), two-level chains, null and dangling master pointers (still excluded), and a caller the master object does not admit (still nothing). A metadata cycle and a chain past the depth bound still answer zero rows, now from a registry-only walk that pre-empts the engine's own refusal. A write verb's predicate carries no leaf (ADR-0139 D1): a predicate write of a detail is narrowed by the read door's answer and judged per master, as before.
- **`@objectstack/service-analytics`.** The engine-bound merges (`ObjectQLStrategy.withReadScope`, its FK attribute map, and the plugin's record-label pass) admit the leaf and hand the scope to the engine, which composes it as a find does. `NativeSQLStrategy.canHandle` declines a query whose base or joined object's read scope carries the leaf, so a `controlled_by_parent` object's analytics query routes to the ObjectQL strategy and serves the find path's rows. `compileScopedFilterToSql` refuses a scope carrying the leaf by name, in its existing `READ_SCOPE_COMPILE_FAILED` / 500 envelope: the `/api/v1/analytics/sql` dry-run echo of a `controlled_by_parent` object now refuses where it rendered the bound id list — it runs no composition and shows no statement it would not run. A filter an author writes keeps its judgement on every face.
- **`@objectstack/driver-turso`.** The remote face resolves a composed leaf nested inside a composed leaf's scope through `toRemoteWhere` with the outer read's options, so a two-level `controlled_by_parent` chain on a remote Turso datasource answers rows instead of `INTERNAL_ERROR` / 500.
- **`@objectstack/spec`.** One nested-leaf row in `READABLE_IDS_CASES`, so every driver's conformance suite measures the shape, and the comparand-shape door's docblock names both callers that pass `admitReadableIds`. No schema, accept set or export of a metadata key moves.
5 changes: 5 additions & 0 deletions docs/adr/0055-master-detail-controlled-by-parent.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

**Status**: Accepted (2026-06-19) — implemented in this PR (P0–P2) · **Amended** (2026-09-07, #11082 / PR #11183 — the "Single-level only in v1" limit under Consequences and the "Transitive nested master-detail chains" **Non-goal** are both **reversed**: since `@objectstack/plugin-security` 17.3.0 (merged 2026-08-23) the read derivation and the write gate compose `controlled_by_parent` across a chain, bounded by a cycle guard and a cost ceiling that both fail **closed**. Nothing else in this record changes — §1–§4, the mechanism choice, the set-size limit and the share-table non-goal all stand. See **"Amendment (2026-09-07, #11082): transitive chains compose — the single-level non-goal is reversed"** at the end.)
· **Revisited in part** (2026-10-10, [ADR-0139](./0139-engine-internal-semi-join-leaf-readable-parent-ids.md), Proposed) — §2's alternatives row (a) is reversed for ONE engine-internal, non-authorable leaf the engine composes from the compiler's output; the RLS compiler still has no subquery form, and §2's chosen mechanism (b), its `rlsMembership` IN-form and the set-size limit all stand.
· **Amended** (2026-10-22, #24932, ruling B `6303721609` and Q1 C / Q2 A `6329027469`) — **§2's chosen mechanism (b) is retired for `controlled_by_parent`, and the set-size limit with it**: the read derivation (`packages/plugins/plugin-security/src/security-plugin.ts#computeControlledByParentFilter`) answers ADR-0139's `ReadableIds` leaf over the master, the engine composes it from this plugin's own `canReadObject` / `getReadFilter` of the master, and the driver compiles one statement with the master inside a subquery — the seventh consumer of that leaf, in the end-state form. No master id set is pre-resolved any more. The 2026-10-10 line above reads as of its date. What still stands: §1 (the one required `master_detail`), §3 (the write gate), the cycle guard and the depth bound (now the engine's own cut, judged first over the registry), and the RLS compiler's `rlsMembership` IN-form, which serves `current_user.<key>` memberships and never served this derivation. The limit that replaces the set-size ceiling is ADR-0139 D5's two master shapes, which the boot refuses (#25144).
**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).
Expand Down Expand Up @@ -72,7 +73,9 @@ A `@objectstack/dogfood` RLS proof, bound in the liveness ledger on `object.shar
- **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.
⚠️ **Amended 2026-10-22 (#24932) — retired.** There is no id list and no `IN (...)` of master ids any more: the read derivation answers the engine's `ReadableIds` leaf and the driver compiles `masterFK IN (SELECT id FROM master WHERE …)`, so nothing is bound per master and the statement does not grow with the master table. The ceiling this bullet booked was measured before it went (`DATABASE_ERROR` / 500, `too many SQL variables`, past ~32,800 masters on SQLite; 200 with the right rows at 33,000 after). The bullet is kept because it was true for mechanism (b); ⛔ it is no longer a statement of a limit.
- **Per-request resolution cost.** One extra master-id query per controlled_by_parent object per request (cached within the request).
⚠️ **Amended 2026-10-22 (#24932) — retired, and corrected.** There is no extra query: the master is a subquery inside the detail's one statement (one per find, one per count). Before the switch the extra query ran **twice** per request — once for `find`, once more for `count` — and was never cached within the request; the parenthesis described an intent the code did not implement.
- **Single-level only in v1.** Nested master-detail chains (a detail whose master is itself a detail) are **not** traversed transitively in v1.
⚠️ **Amended 2026-09-07 — this limit was lifted on 2026-08-23** by #11082 (PR #11183), released in `@objectstack/plugin-security` 17.3.0. The bullet is kept rather than deleted because it was true, and correctly recorded, **for v1**; what changed is the version it describes. ⛔ It is no longer a statement of the **enforced boundary** — both halves walk the chain now. Read the Amendment at the end before deriving any invariant from this line.

Expand Down Expand Up @@ -118,9 +121,11 @@ So a two-level chain was enforced at level one and open at level two, under meta
### What both halves do now

- **Read.** `packages/plugins/plugin-security/src/security-plugin.ts#computeControlledByParentFilter` AND-composes the master's **own** `controlled_by_parent` derivation as a third layer, resolved through **that same method**, so the recursive answer cannot drift from the top-level one. The master set is therefore point-for-point equal to what a direct read of the master returns **at every level** — the equality §2 established for one.
⚠️ **Amended 2026-10-22 (#24932).** The derivation no longer composes the master's layers itself: it answers the `ReadableIds` leaf over the master, and the **engine** composes the master's scope by asking this plugin's public doors — `canReadObject(master)` and `getReadFilter(master)`, the latter being this same derivation for a master that is itself `controlled_by_parent`, so the chain composes recursively, one nested subquery per hop, inside one statement (ADR-0139 D6 as amended). The equality holds for the same reason, one level up: the scope the engine composes IS the scope a direct read of the master runs under.
- **Write.** `packages/plugins/plugin-security/src/security-plugin.ts#assertControlledByParentWrite` walks the chain hop by hop, running the same three master-edit legs — extracted verbatim as `packages/plugins/plugin-security/src/security-plugin.ts#assertMasterRowEditable` — on every hop until it reaches a master that governs its own rows. Each added refusal keeps the `403 PERMISSION_DENIED` envelope named for the **caller's own** object and operation, never for an ancestor the caller never asked about.

**Cost.** The per-request resolution cost booked under Consequences is now paid **per hop** — one extra master-id query per `controlled_by_parent` object on the chain, bounded as below.
⚠️ **Amended 2026-10-22 (#24932).** No extra query per hop: each hop is one nested subquery inside the detail's one statement, and the depth bound caps the statement's nesting, not a count of queries. The write gate's per-hop master-row probe is unchanged.

### The two guards, and what the depth bound is NOT

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,21 @@ chosen up front, never reached by catching a refusal.
(ADR-0057 §3.6, `packages/objectql/src/engine.ts#LIFECYCLE_DATASOURCE`): there the leaf serves consumer 1 nothing,
and letter C's loud bound is what that deployment class has. Recorded as a limit, not solved.

**The seventh consumer routes nothing to a probe (2026-10-22, #24932 — the plugin-security switch, ruling Q1 C / Q2 A
[`6329027469`](https://github.com/objectstack-ai/objectstack/issues/24932#issuecomment-6329027469)).**
`controlled_by_parent`'s read derivation (`packages/plugins/plugin-security/src/security-plugin.ts#computeControlledByParentFilter`)
answers the leaf for EVERY `controlled_by_parent` object, with no up-front routing to a bounded id list, because the two
classes above cannot occur for its master: the boot **refuses** a `controlled_by_parent` detail whose master is bound to
another datasource (D5.3) or narrowed by an object-scoped read middleware (D5.2) — `ObjectQLPlugin`, on
`kernel:bootstrapped`, `INVALID_METADATA` / 422 ([#25144](https://github.com/objectstack-ai/objectstack/issues/25144),
PR [#25166](https://github.com/objectstack-ai/objectstack/pull/25166)). "Refuses at boot" is the accurate form: the
refusal judges the artifacts present at boot, and an artifact registered later (the `manifest` service,
`metadata:reloaded`) is admitted, its detail then answering this record's D5 refusal at read, never a wrong row set; no
public entry reaches that path today, and the engine lane re-measures it now that the leaf is written on every read. The
plugin's own two guards stay: a metadata cycle and a chain past `CBP_MAX_CHAIN_DEPTH` answer zero rows from a
registry-only walk, made before the leaf is written, so this record's cycle and depth refusals are never reached for a
declared chain; the two bounds are one cut.

### D6 — What ADR-0055 and ADR-0056 now say

ADR-0055's row (a) is reversed **for this one leaf and nothing else**: the leaf is composed by the engine from the
Expand All @@ -249,7 +264,9 @@ IN-form and deliberate lack of a subquery form are unchanged
(`packages/plugins/plugin-security/src/rls-compiler.ts#RLSCompiler`, `#compileExpression`: a subquery shape compiles to
`null` and fails closed). §2's mechanism (b) for
`controlled_by_parent` stands, set-size ceiling included; whether that id set becomes a seventh consumer was ⛔ not
decided here. ADR-0056's non-goal stands as written for the compiler and is revisited only as the engine-side leaf.
decided here (as accepted; decided 2026-10-21 by ruling B, and landed 2026-10-22 by the plugin-security switch,
#24932 — ADR-0055's own amendment of that date retires its mechanism (b) and the ceiling; D5's seventh-consumer note
records the routing). ADR-0056's non-goal stands as written for the compiler and is revisited only as the engine-side leaf.
Each record carries one pointer to this one, landed with this record — ADR-0055's status line, ADR-0056's non-goal
bullet; nothing else in either moves.

Expand Down
12 changes: 11 additions & 1 deletion packages/drivers/driver-turso/src/turso-driver.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3383,14 +3383,24 @@ export class TursoDriver extends SqlDriver {
* TENANT)` and decides nothing about the wall, so the two faces answer one
* row set. An uncomposed leaf, or a composed one outside a `$in`, is refused
* first — INTERNAL_ERROR / 500, logged at `error` once per driver.
*
* [ADR-0139 D6 as amended — the nested master chain] A composed leaf's
* scope may itself carry a composed leaf (a `controlled_by_parent` master
* that is `controlled_by_parent` too), so the scope is resolved through
* THIS method, with the OUTER read's options: the nested parent's semi-join
* and its tenant wall are compiled the same way, one level down, and the
* transport nests the subqueries. `replaceComposedReadableIds` returns a
* composed leaf unentered, so without the recursion the nested leaf reached
* the transport unresolved and the remote face refused it (500) where the
* local face answered.
*/
private toRemoteWhere(object: string, where: unknown, options: DriverOptions | undefined): unknown {
const misplaced = findMisplacedReadableIds(where, true);
if (misplaced) throw refuseReadableIdsOnce(this.readableIdsDriverName, misplaced, this.logger, this.readableIdsRefusalLog);
const resolved = replaceComposedReadableIds(where, (leaf) => remoteReadableIdsSemiJoin({
object: leaf.object,
table: this.remoteTableFor(leaf.object, 'ReadableIds'),
scope: leaf.scope === undefined ? undefined : (this.toRemoteFilter(leaf.object, leaf.scope) as FilterCondition),
scope: leaf.scope === undefined ? undefined : (this.toRemoteWhere(leaf.object, leaf.scope, options) as FilterCondition),
tenant: this.remoteTenantScope(leaf.object, options),
}));
return this.toRemoteFilter(object, resolved);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ import type { PermissionSet } from '@objectstack/spec/security';
import { BUILTIN_OPERATION_MESSAGES } from '@objectstack/spec/system';
import { SecurityPlugin } from './security-plugin.js';
import { defaultPermissionSets } from './objects/default-permission-sets.js';
import { composeReadableIdsInMemory } from './readable-ids-in-memory.testkit.js';

// ── metadata: the showcase invoice graph, exactly as it ships ──────────────

Expand Down Expand Up @@ -446,7 +447,15 @@ async function makeStack(opts: { orgScoping?: boolean } = {}): Promise<Stack> {
let result: any[] = [];
await securityMw(opCtx, async () => {
await sharingMw(opCtx, async () => {
result = await engine.find(object, { where: opCtx.ast.where });
// [ADR-0139 D6] A controlled_by_parent read carries the engine's
// leaf over the master; this double is no engine, so the composition
// is done for it (`readable-ids-in-memory.testkit.ts`).
const where = await composeReadableIdsInMemory(opCtx.ast.where, opCtx.context, {
rows: (o) => engine._tables[o],
readAdmission: (o, c) => security.canReadObject(o, c),
readScope: (o, c) => security.getReadFilter(o, c),
});
result = await engine.find(object, { where });
});
});
return result;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ import { defaultPermissionSets } from './objects/default-permission-sets.js';
import { SysUserPosition } from './objects/sys-user-position.object.js';
import { SysUserPermissionSet } from './objects/sys-user-permission-set.object.js';
import { relaxEveryRegisteredTenantColumn } from './pre-18-tenant-column.testkit.js';
import { wireEngineSecuritySource } from './engine-security-source.testkit.js';

const SYS = { context: { isSystem: true } } as never;
const MEMBER_DEFAULT = defaultPermissionSets.find((p) => p.name === 'member_default')!;
Expand Down Expand Up @@ -281,6 +282,8 @@ beforeAll(async () => {
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'member_default' });
await plugin.init(ctx as never);
await plugin.start(ctx as never);
// The engine's security source, as `ObjectQLPlugin.init` wires it on a kernel (ADR-0139 D2).
wireEngineSecuritySource(engine, () => plugin);
sharing = new SharingService({ engine: engine as never, securityService: () => security as never });
engine.registerMiddleware(buildSharingMiddleware(sharing, ctx.logger) as never, { object: '*' });
});
Expand Down
Loading
Loading