Repository navigation
Commit 9532e27
fix(plugin-security): a controlled_by_parent detail's read scope requires object-level read on its master (#24985)
Fixes #24962
Clause-②: no
## What this changes
ADR-0055 §3 says "Reading a detail requires **read** access to its
master". §4 says a member who cannot read master M "can **neither read
nor by-id-write a detail D under M**".
`computeControlledByParentFilter` builds a `controlled_by_parent`
object's read scope from the master ids the principal may read. It
scoped the master's **rows**: RLS, the sharing half, and the master's
own derivation on a chain. It never asked the master's **object-level**
read admission. Now it asks that first, and a principal the master's own
read door refuses at the object gate gets the empty master set.
- **One predicate, two askers.** Arms 3 to 6 of `canReadObject` move
into a private `objectReadAdmission(object, sets, delegatorSets)`, which
works over sets the caller has already resolved. The arms are the
posture, the ADR-0066 D3 capabilities, the `allowRead` grant, and the
ADR-0090 D10 delegator's same questions. `canReadObject` resolves the
context and asks it, as before. The derivation asks it of the master,
for the principal whose sets it was handed. There is no second copy of
the rule.
- **Every call site goes through it.** These are the caller half, the
D10 delegator half and `getReadFilter` (`security-plugin.ts` near
`:4390`, `:4408` and `:5943` at `f32d795ce5`). The recursion asks the
same question at every hop of a chain.
- **The write doors follow without a change of their own.** A by-id
update or delete asks the read door's question first: a row the caller
cannot read answers the read door's not-found. A predicate write is
scoped by the read scope.
- **No widening.** The diff only adds an empty-set exit ahead of the
existing scan. A principal the master object admits runs the same code
as before.
- Not touched: `packages/objectql/**` and `packages/spec/**`.
## The two spellings now agree
The engine's ADR-0139 `ReadableIds` leaf asks `canReadObject(parent)`
before it scopes a parent's rows (`engine.ts`, `readableIdsScopeOf`).
That call now runs the same predicate the id-list derivation asks.
Measured on the real stack with a probe read through the leaf, for a
principal without object-level read on the master and for its control:
- before this change: the leaf answered no master and the id list
answered every master;
- after it: both answer no master, and for the control both answer every
master.
The probe was a one-off measurement and is not committed.
## Verification (head `3dd33f7b78`)
**Pins.** `src/controlled-by-parent-master-object-read.test.ts` runs on
the real stack: `ObjectQL` over better-sqlite3, the real
`SecurityPlugin`, and on-behalf-of contexts assembled by
`resolveAuthzContext` and `assembleExecutionContext`. It has 23 cases,
named by the rule:
- for two principals the master's read door refuses, `find`, `findOne`
and `count` read no detail, the `getReadFilter` scope admits none, and a
detail two hops down is not read;
- a detail whose master is itself `controlled_by_parent` is not read
without object-level read on that master;
- on an on-behalf-of read, the delegator's object-level read on the
master is required too;
- a by-id update or delete of such a detail answers `RECORD_NOT_FOUND` /
404 and writes nothing, an insert is refused, and a predicate update
matches no detail;
- the controls keep their answers: master read with rows scoped, master
read with no master row in scope, master read unscoped, a super-user
grant, a system context, and an on-behalf-of read whose delegator reads
the master. So do the write controls: a by-id update, an insert and a
predicate update by a principal that reads the master.
**Package.** At `438ef2dfde`, `pnpm --filter
@objectstack/plugin-security exec vitest run --maxWorkers=2` gave 198
files passed, 4100 passed and 45 skipped, `VERDICT command-exit 0`. The
one commit since then renames the new file's labels, and its 23 cases
pass at `3dd33f7b78`. `pnpm --filter @objectstack/plugin-security
typecheck` gave `VERDICT command-exit 0` at `3dd33f7b78`.
`tsconfig.test.json` includes `src/**/*`, so the new file is compiled.
**Ablation.** The fix was committed first. Each leg mutated
`security-plugin.ts` through `scripts/ablation-replace.mjs` (anchor 1 to
0, blob changed) and ran the pin file. Each was then restored with `git
checkout HEAD -- PATH` and proven by blob equal to HEAD (`a774fe4e1ddf`)
and an empty `git diff HEAD`. The subject is imported from `src/` by a
relative path, so no `dist/` leg applies.
- **A: the new guard removed.** 12 failed, 11 passed. Red: every
"without object-level read" read, scope, chain, on-behalf-of,
by-id-write and predicate-write pin. Green: the two preconditions, the
two insert pins and every control. The insert pin is refused by gates
ahead of the read scope and stands either way, so it pins §4's third
write rather than this change.
- **B: the guard skipped on recursive hops.** 3 failed: the two
two-hops-down pins and the on-behalf-of pin, whose chain read goes
through the recursion.
- **C: the delegator half handed the caller's sets and context.** 1
failed: the on-behalf-of pin. That pin measures the delegator half, not
the caller half.
**Gates.** `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 69 families at `3dd33f7b78`. All 69
ran one at a time, each exit captured before any pipe. `--ran` reports
"69 derived famil(ies) accounted for — 69 run, 0 NOT-MEASURED (a DERIVED
zero — all 69 recorded an exit code and none of them is 3)".
- An earlier run at `438ef2dfde` had `check:dual-build-cjs-loads` and
`check:i18n` exit 3, because the `dist/` they read was missing. The
trees were present at the second run, and both exited 0 there.
## Acceptance notes
- **Error envelope.** For some principals without object-level read on
the master, a by-id update or delete of a detail now answers
`RECORD_NOT_FOUND` / 404 where it answered `PERMISSION_DENIED` / 403
from the master-edit leg. The 404 is the write doors' existing rule for
a row the caller cannot read, and that row is now unreadable. It is
pinned.
- **#24932** works in this same function and is in the decision box. No
ruling had landed on it at this head. This change does not move the
id-list mechanism, so a later switch to the leaf keeps these rows by
construction.
- **Administrators.** A super-user grant is admitted to every master
object, so it is unchanged, and that is pinned. The ADR-0090 D12
delegated-administration gate judges RBAC link-table writes only and is
not on this path.
- **Base.** `origin/main` was merged at `b811c73696` (3 commits:
service-datasource, docs, spec liveness data). This branch changes
nothing under `packages/spec`. Main has since moved to `ba69b168be`
(plugin-approvals labels, disjoint), and that is not merged.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011TdZ8GjixNZFLz1xJD9zzK)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7adfaec commit 9532e27
3 files changed
Lines changed: 563 additions & 35 deletions
File tree
- .changeset
- packages/plugins/plugin-security/src
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
0 commit comments