Repository navigation
Commit eb96480
fix(objectql)!: a non-system write may name, in a lookup or user field, only a record the caller can read (#31031)
security#1 · os-tesla · session_01L9qibvynRbCtMndW8EHNGc
Fixes #29029
Clause-②: yes (narrowing) — the write's reference check narrows: a
non-system `lookup` or `user` value naming a record the caller cannot
read is refused, and an update's forgiveness of a value the row already
stores compares only stored values the caller may read. The
business-unit read widens: a delegated admin reads the business units
inside the admin scope it holds, and no other.
Metadata-protocol: none
Skills-docs: none — no file under `skills/**` teaches a reference to a
record its writer cannot read, a delegate's business-unit read, or an
update's carried-back reference (a grep of `skills/` for
`reference_not_found`, `already stores` and `not judged again` finds
only the permission-set catalog refusal, unchanged); `content/docs`
states the rule on four pages (below).
## 维护者速读
- 改了什么:非系统写在 `lookup` / `user` 字段里只能引用调用方自己读得到的记录,读不到的一律答
`reference_not_found`(与 `master_detail` 同一条规则),且先于记录校验规则;委派管理员可读其
adminScope 覆盖的业务单元。本 PR 在 #30747
之上再加一条:更新时「行里已存的值不再判」这条宽免,只比对调用方读得到的已存值,调用方可改不可读的列不宽免任何值。
- 为什么改:裁决 6663849489(A 重申 + 委派读);宽免一条由安全席位要求与本规则同时落地。
- 风险与回滚:BREAKING
收窄。依赖「引用读不到的记录」的非系统写会被拒;可改不可读的引用列上,更新带回原值也按新值判。读得到该列的调用方照旧宽免,系统写(种子、安装、启动)不变。回滚即撤本
PR。
- 先看:`packages/objectql/src/engine-referential-integrity.ts` 的
`storedReferencesCallerReads` 与 `assertReferencesResolve`
末段;`packages/objectql/src/engine.ts`(三条写路径的顺序);`packages/plugins/plugin-security/src/delegated-admin-gate.ts`
的 `delegatedUnitReadScope`。
- 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run
src/reference-forgiveness-readable-column.test.ts`,预期 26/26 绿。
- 需要你做的:无,席位落地。
> [!WARNING]
> **BREAKING** — accept-set narrowing at the write's reference check
(`@objectstack/objectql`), served by every write door. After the
upgrade, a write that is not a system write is refused `400
VALIDATION_FAILED` with the field's localized `reference_not_found` when
a `lookup` or `user` value it supplies names a record the caller cannot
read; and an update's forgiveness of a value the row already stores
compares only stored values the caller may read.
**Supersedes #30747** (closed with a pointer to this PR). A continuation
by the security seat (security#1, session_01L9qibvynRbCtMndW8EHNGc):
#30747's head `0e54448624` on a new branch, `main` merged three times,
and the forgiveness rule below added at the same time, as #30747 itself
continued #30672. #30747 carries a contract review PASS at `0e54448624`
and the maintainer's per-PR BREAKING word; the seat retires #30747 with
a pointer here.
## What changes
**The rule.** A write that is not a system write may name, in a
`lookup`, `user` or `master_detail` field, only a record its caller's
own read door returns. An unreadable reference target always answers
`reference_not_found`, before the record's validation rules. A record
the caller cannot read answers exactly as a record that exists nowhere,
and so does one whose read by the caller fails. System writes
(`isSystem`) stay exempt, keep the order they had and give the answer
they gave; a flow run's elevated write keeps its existence check. A
reference to an object the registry does not hold is not judged. `tree`
keeps the existence check alone. A `lookup` or `user` value naming a
user the caller cannot read answers `reference_not_found` and its ban is
not asked; a user the caller can read is judged for its ban as before
(#30205).
**The forgiveness (new on this PR).** An update that carries back a
`master_detail`, `lookup` or `user` value the row already stores is
still not judged again (#29062), by id and in bulk ("every matched row
stores it") alike, but the update's reference forgiveness compares only
stored values the caller may read: the stored rows as the caller's own
read door serves them, each column kept only where it is served exactly
as stored. A column the caller may edit but not read (a field permission
`readable: false, editable: true`) holds no stored value for it, so it
forgives nothing: a value named there is judged like any other, and the
answer is the same whatever the column holds. A caller who can read the
column keeps the forgiveness. A system write compares the rows as
stored, as before. A failed read of the stored row forgives nothing.
**The delegate read.** An admin scope over a business-unit subtree
(ADR-0090 D12) grants read on the business units inside that subtree,
and on no unit outside it. It applies to a read of `sys_business_unit`
that the caller's own permission sets do not grant, on the caller's own
behalf, bounded to the union of the subtrees the caller's held scopes
cover. A caller with a `sys_business_unit` grant reads as that grant
says, as before.
- `packages/objectql/src/engine-referential-integrity.ts` — `lookup` and
`user` join `master_detail` on the caller's read door;
`referencesCheckedBeforeRules`, the one predicate every write door and
the validate preview read for the order; **new:**
`storedReferencesCallerReads`, the stored rows the forgiveness compares
against, read once per refusing write through the caller's read door and
kept per column by `servedAsStored` (the comparison the `update`-mode
validate preview already makes for its stored row).
- `packages/objectql/src/engine.ts`, `engine-validate.ts` — insert,
update by id, bulk update and the validate preview check references
before the record's validation rules unless the write is a system write;
the reference check's host gains `find`.
- `packages/objectql/src/engine-references-and-master-detail.ts`,
`engine-master-detail-related.ts` — the caller-readable reference
helpers (`callerReadableReferenceExists`, and
`callerReadableReferenceRow` for the user reference whose ban is also
asked), and the #18682 docblock now states the rule.
- `packages/plugins/plugin-security/src/delegated-admin-gate.ts`,
`security-plugin-authz-middleware.ts` — `scopedBusinessUnitIds` and
`delegatedUnitReadScope`; the middleware admits the read and narrows it
to `id IN` the scoped units.
- `packages/verify/src/parent-write-reach.ts` — the parent-write-reach
verifier's personas read every record of each object a probed detail's
required reference names outside its master chain, so the create probe
names records its writer can read.
- `docs/qa/platform-checklist/areas/records-forms.json` —
`records-forms.predicate-relationship-traversal` takes revision 7,
written to the rule.
- `content/docs/api/error-catalog.mdx`,
`content/docs/data-modeling/fields.mdx`,
`content/docs/permissions/delegated-administration.mdx` — each states
the rule where a sentence would otherwise have gone false; the first two
now also say the forgiveness compares only stored values the caller may
read.
- `content/docs/permissions/system-context.mdx` — row 23 says where a
system write's reference check runs and that a system write compares an
update's stored rows as stored; its Symbol cell and the generated site
register name `referencesCheckedBeforeRules` and
`storedReferencesCallerReads`.
- `.changeset/29029-lookup-user-caller-read.md` —
`@objectstack/objectql`, `@objectstack/plugin-security` and
`@objectstack/verify`, patch, BREAKING; amended with the forgiveness
paragraph and the widened `Clause-②:` line.
## Tests
- **New:**
`packages/plugins/plugin-security/src/reference-forgiveness-readable-column.test.ts`
(26) — the real engine and the real security plugin. For `lookup`,
`user` and `master_detail`, by id and in bulk over two rows: in a column
the caller may edit but not read, a value the rows store and one they do
not answer byte-identically (status, code, field finding and sentence,
with the sent id written out) and the rows are untouched; controls — the
same grant edits another column of the same rows; a caller who can read
the column has the stored value carried back land and a new unreadable
value refused; a system write keeps its exemption.
- **New block:**
`packages/objectql/src/engine-unreadable-reference-target.test.ts` (now
113) — a top-level describe on the same rig: a middleware withholds the
reference columns from the caller's reads of the row; `lookup` and
`user`, by id and in bulk, the stored value answers as another
unreadable value; a failed read of the stored row forgives nothing;
controls — a column the caller reads keeps the forgiveness; a flow run's
system write compares the rows as stored.
- `packages/objectql/src/engine-unreadable-reference-target.test.ts`
(#30747's 99): `lookup` and `user` × insert / batch insert / update by
id / bulk update — an unreadable target is refused as one that exists
nowhere; a refused read of the target object answers the same; a
multi-valued field with one unreadable member; a system write; a stored
value carried back; the read door before the elevated probe. On all four
doors and for `lookup`, `user` and `master_detail`, an unreadable
reference answers `reference_not_found` before the record's validation
rules while a readable one reaches them; the validate preview agrees;
the order controls for system and flow-run writes; a read of the target
that fails is refused; a reference to an unregistered object is
admitted.
-
`packages/plugins/plugin-security/src/delegated-admin-own-name-set.posture.test.ts`
(33), three postures: a delegate reads exactly the units its scope
covers (`find`, `findOne`, `count`); assigning a unit in scope is
admitted; a unit outside the scope it cannot read is refused
`reference_not_found`; controls — a writer with no scope, and an
organization admin, unchanged.
-
`packages/plugins/plugin-security/src/write-preview-field-gate-parity.test.ts`:
the controls' callers read the related account; a new block drives the
editor who cannot, and both doors answer the lookup's
`reference_not_found`.
- Forced edits, rule-preserving (from #30747): six plugin-security
suites provision `sys_user` with a row per user they name
(`__tests__/rig-users.testkit.ts`); `agent-ceiling-grant-refusal`
provisions the three users its grants name;
`predicate-related-read-tenant-scope` counts the rule's related reads
alone; four plugin-audit suites leave the write's id-only
reference-check read out of their read counters.
- `packages/objectql/src/engine-banned-user-reference.test.ts` (11),
`packages/verify/src/parent-write-reach.test.ts`,
`packages/qa/dogfood/test/predicate-relationship-traversal.dogfood.test.ts`:
as on #30747.
<details><summary>Ablations of the forgiveness rule (this PR): through
scripts/ablation-replace.mjs, the comparison handed the rows as stored
instead of as the caller reads them; restored blob equal to HEAD, git
diff HEAD empty</summary>
The objectql pins import `./engine.js` from source and plugin-security's
vitest aliases `@objectstack/objectql` to source, so the mutation is
read from `src/`. Anchor `: await storedReferencesCallerReads.call(…)`
x1 → x0, replacement `: storedAsStored;` x0 → x1, blob `6fc9b11c36b0` →
`9bf670a3ec08`; restore blob `6fc9b11c36b0` == HEAD blob, `git diff
HEAD` empty, both legs.
1. `reference-forgiveness-readable-column.test.ts`: 6 failed | 20 passed
(26) — exactly the identical-answer legs, `lookup` / `user` /
`master_detail` × by id / bulk; every control green, the readable-column
forgiveness among them.
2. `engine-unreadable-reference-target.test.ts`: 6 failed | 107 passed
(113) — the hidden-column legs (`lookup`, `user` × by id / bulk) and the
failed-read legs (`lookup`, `user`); the readable-column forgiveness and
the flow-run controls green.
#30747's seven ablations stand as recorded there (on #30672 at
`75a7c44d96`).
</details>
## Deviations
- Main merged three times on this branch (`06cccd32e1` → `1aeeafd9d3`,
`af1d59b692`, `a11ef71e93`): no conflict in any. The third auto-merged
`docs/qa/platform-checklist/areas/records-forms.json` (this PR's item
revision 7 beside main's position-name renames in three other items,
disjoint hunks).
- `content/docs/permissions/system-context.mdx` gains
`storedReferencesCallerReads` in row 23 because the system-context
census counts the forgiveness's one `isSystem` read; the register is
regenerated by `check-system-context-census.mjs --write`.
- The rest as on #30747 (#30546 overlap and the two Test Core fixes on
#30672).
## Acceptance notes
-
`packages/services/service-automation/src/runas-system-reference-check.integration.test.ts`
still names `masterDetailHeaderExists` in a comment, outside this card's
surface.
- The delegate read covers a caller's own read; a read on another's
behalf (`delegatorSets`) is not widened. Explain and `canReadObject` do
not reflect the delegate read.
- The forgiveness's read of the stored rows runs only once a reference
has been refused, one `find` for the write's rows, under the caller's
context.
## Verification
Final head `2ae4d980c2`: #30747's head `0e54448624` with `origin/main`
merged three times and the forgiveness rule added. The third merge (main
`a11ef71e93`) was followed by `pnpm install --frozen-lockfile`, a
whole-workspace build (`turbo run build`, 119 of 119 tasks) and the
suites below; the last commit after it (`2ae4d980c2`) edits one verify
test file, and the verify suite, the typecheck, the dogfood battery and
the full gate set ran on it. Delta against `main`: 40 files, +1739 /
-248 (1987 changed lines; #30747 was 1563).
<details><summary>Suites and typecheck</summary>
- `@objectstack/objectql` (at `dc1dbb4db7`, same objectql source as the
head): Test Files 560 passed (560), Tests 11314 passed (11314).
- `@objectstack/plugin-security` (at `dc1dbb4db7`, same plugin-security
source as the head): Test Files 247 passed (247), Tests 5731 passed, 45
skipped (5776).
- `@objectstack/verify` at `2ae4d980c2`: Test Files 48 passed (48),
Tests 408 passed (408).
- Typecheck at `2ae4d980c2`, under the verify lock: `turbo run
typecheck` over the four packages the diff touches (objectql,
plugin-security, verify, plugin-audit) and their 37 direct consumers
with a typecheck script (downstream: every workspace package declaring
one of the four as a dependency), 112 of 112 tasks successful. The
forgiveness adds nothing to any emitted declaration: neither
`storedReferencesCallerReads` nor the reference-check host type appears
in `packages/objectql/dist/*.d.ts`.
</details>
<details><summary>Gates</summary>
`node scripts/pm/dispatch-gates.mjs --commands` at `2ae4d980c2` (40
changed paths): 98 families, each run with stdin from `/dev/null` and
its exit code captured before any pipe; 97 exited 0 on the pass, and
`pnpm check:type-check-debt` exited 3 there only because its closure
build queued behind this branch's own locked dogfood run; re-run alone
under the lock, exit 0. `--ran` with every exit code recorded: 98
derived, 98 run, 0 NOT-MEASURED, 0 UNRUN (a derived zero).
Two families were red on an earlier head and are fixed on this one:
`check:system-context-census` (the forgiveness's one system-write read
needed an owning row: row 23's Symbol cell, register regenerated) and
`check:test-source-alias` (the third merge brought in its source-reach
half, which judges the changed
`packages/verify/src/parent-write-reach.test.ts`: the two lazy service
loads are now paid at module top, as it prescribes).
`node scripts/pm/check-governed-merges.mjs --branch
origin/claude/sec-hardening-r12`: 0 of 40 paths governed; 1987 changed
lines, under 3000.
</details>
Full dogfood battery on `2ae4d980c2` under the verify lock: `VERDICT
command-exit 0`, Test Files 428 passed, 3 skipped (431), Tests 3560
passed, 41 skipped (3601).
`main` moved 23 commits while these ran; a local merge-tree is clean,
`servedAsStored` is unchanged there and none of them touches
`engine-referential-integrity.ts`. The seat's owed merge of `main`
before queueing covers it.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01L9qibvynRbCtMndW8EHNGc)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e396095 commit eb96480
40 files changed
Lines changed: 1739 additions & 248 deletions
File tree
- .changeset
- content/docs
- api
- data-modeling
- permissions
- docs/qa/platform-checklist/areas
- packages
- objectql/src
- plugins
- plugin-audit/src
- plugin-security/src
- __tests__
- qa/dogfood/test
- verify/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 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
316 | 316 | | |
317 | 317 | | |
318 | 318 | | |
319 | | - | |
320 | | - | |
321 | | - | |
322 | | - | |
323 | | - | |
324 | | - | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
325 | 329 | | |
326 | 330 | | |
327 | | - | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
328 | 334 | | |
329 | 335 | | |
330 | 336 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
283 | 283 | | |
284 | 284 | | |
285 | 285 | | |
286 | | - | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
287 | 290 | | |
288 | 291 | | |
289 | 292 | | |
| 293 | + | |
| 294 | + | |
290 | 295 | | |
291 | 296 | | |
292 | 297 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
56 | 56 | | |
57 | 57 | | |
58 | 58 | | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
59 | 64 | | |
60 | 65 | | |
61 | 66 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
134 | 134 | | |
135 | 135 | | |
136 | 136 | | |
137 | | - | |
| 137 | + | |
138 | 138 | | |
139 | 139 | | |
140 | 140 | | |
| |||
541 | 541 | | |
542 | 542 | | |
543 | 543 | | |
| 544 | + | |
| 545 | + | |
544 | 546 | | |
545 | 547 | | |
546 | 548 | | |
| |||
0 commit comments