Repository navigation
Commit 449c858
fix(plugin-security): canReadObject, getReadFilter and security/explain answer the delegated business-unit read as the read door does (#31142)
<!-- claude-projects-attribution: {"github_login":"marchtian"} -->
_Requested by **田騏** · [project
thread](https://claude.ai/code/project/chan_01VmuUhXU9SSUzqJYyXSmYSi?thread=cmsg_01VmuUhXU9SSUzqJYyXSmYSi9Hju7DThD7eoLKoJU9hnHj)_
Fixes #30758
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:委托管理员(只持有某个业务单元子树的 adminScope)能读到的业务单元,现在
`canReadObject`、`getReadFilter` 和 `security/explain`
都如实回答“能读,且只能读这几个”,与读门(find/findOne/count/aggregate)一致。
- 为什么改:读门早已按裁决放行这类读取,但三处“回答”仍说不能读,单元选择器和 explain
会拒绝委托管理员实际能读的单元(多拒,不多放)。
- 风险与回滚:三处都复用读门同一个判断函数,`canReadObject` 变 true 时 `getReadFilter`
同步收窄到所覆盖单元,绕过中间件的门不会读到范围外单元;回滚即还原本 PR 一个提交。
-
先看:`security-plugin-read-filter.ts`(收窄,最关键)、`explain-engine.ts`(object_crud
与记录级判定)、`security-plugin-object-admission.ts`(第 5 臂的补充)。
- 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run
src/delegated-admin-own-name-set.posture.test.ts`,预期 57/57 通过,其中 #30758
新增的 24 个用例覆盖三种租户姿态。
- 需要你做的:无,席位落地。
## What changes
A delegated administrator reads the business units its admin scopes
cover (ADR-0090 D12, ruling of record 6663849489). The read door admits
that read beside the CRUD grant (`delegatedUnitReadScope`, which reached
`main` in eb96480, the squash of #31031; #30747 itself closed
unmerged). The answers outside the door did not reflect it. This PR
makes them ask the door's own judgment; it does not change what the door
admits.
- `delegated-admin-gate.ts`: the judgment is split out as
`delegatedUnitReadAdmission(gate, object, operation, context, sets,
delegatorSets)`. `delegatedUnitReadScope` (the door's call) now checks
only that the read carries a query and hands the rest to it, so there is
one judgment and no second copy. Conditions are unchanged: a read
(`find`, `findOne`, `count`, `aggregate`) of `sys_business_unit`, own
behalf, gate composed, at least one covered unit, fail closed.
- `canReadObject`: arm 5 (the `find` grant) accepts, where no set
grants, the same admission the middleware makes at step 2. Arms 3, 4 and
6 are unchanged and still run. Object-level "some rows" follows the
plugin's existing precedent for every row-restricted read:
`canReadObject` answers `true` and `getReadFilter` carries the rows (its
docblock: "`true` never means unrestricted — the row scope is still
getReadFilter's").
- `getReadFilter`: where no set grants `find`, the same admission's
confinement (an `id` `$in` filter over the covered units) is ANDed in
beside RLS and controlled-by-parent, as step 3 ANDs it. Without this
half, flipping `canReadObject` alone would have let a bypassing door
read every unit of the organization (measured below).
- `security/explain`: a new optional dep `delegatedUnitReadScope` (wired
to the same judgment). `object_crud` grants through it only where the
principal's sets do not; the `rls` layer's `readFilter` carries the
confinement; the record pass reports a unit outside the scope as not
visible (`decidedBy: object_crud`), which matters when an organization
administrator explains the delegate. `export`, writes and on-behalf-of
principals are not admitted, as at their doors.
Before, measured on this branch's base 5f00374 with the new posture
tests, identical in all three postures (`single`, `group`, `isolated`),
delegate scope over `bu_east` + subtree, `bu_hq` outside:
| question | read door | before | after |
|:--|:--|:--|:--|
| `find` / `findOne` / `count` / `aggregate` | `bu_east`,
`bu_east_field` | same | same |
| `canReadObject('sys_business_unit')` | | `false` | `true` |
| units under `getReadFilter` | | all three, `bu_hq` included |
`bu_east`, `bu_east_field` |
| `explain` read: `allowed` / `object_crud` | | `false` / `denies` |
`true` / `grants` |
| units under `explain`'s `readFilter` | | all three | `bu_east`,
`bu_east_field` |
| `explain` record `bu_east` / `bu_hq` | | not visible / not visible |
visible / not visible |
## Tests
One new top-level `describe` in the existing shared harness
`delegated-admin-own-name-set.posture.test.ts` (real `ObjectQL` over
SQLite with the real `SecurityPlugin`), eight cases per posture:
- `canReadObject` admits the delegate, and `getReadFilter` returns
exactly the read door's units.
- `security/explain` read: allowed, `object_crud` grants, `readFilter`
returns exactly the door's units.
- `security/explain` of one unit, asked by the delegate and by its
organization administrator: covered units visible, `bu_hq` not.
- `aggregate`: inside the scope counts the covered units, a `where` on
`bu_hq` counts none, and a `groupBy` on an FLS-unreadable field is
refused by step 2.5b (`forbiddenFields: ['external_ref']`).
- CONTROLs: a caller with no scope is refused by door, `canReadObject`
and explain alike; an organization administrator's answers are
unchanged; acting on behalf of another user (even one who reads every
unit) admits nothing anywhere; explain still denies `create` and
`export`, and `canExport` stays `false`.
The `east_admin` fixture set gains one field rule
(`sys_business_unit.external_ref` unreadable) so 2.5b has a field to
refuse.
<details><summary>Ablations: every negative pin turned red, then
restored by hash</summary>
Each leg ran through `scripts/ablation-replace.mjs` (anchor hit x1 → x0,
replacement x0 → x1), restored with `git checkout HEAD -- PATH`, then
checked by `git hash-object` against the HEAD blob and an empty `git
diff HEAD`. The test imports the plugin from `src`, so no build sits
between the mutation and the run.
| leg | mutation | red |
|:--|:--|:--|
| A1 | `getReadFilter` drops the unit scope | 3/57: units read `bu_east,
bu_east_field, bu_hq` |
| A2 | explain `filterParts` drops the unit scope | 3/57: explain read
`readFilter` |
| A3 | explain record pass drops the unit-scope check | 3/57:
`organization administrator · bu_hq: expected true to be false` |
| A4 | middleware step 3 drops `extra.push(delegatedUnitScope)` | 15/57,
including the aggregate pin: `['east:1','east_field:1','hq:1']` |
| A5 | step 2.5b condition `aggregate` renamed away | 3/57:
`forbiddenFields` `undefined` (step 2.9 still refuses, without naming
the field) |
| A6 | `canReadObject` passes `null` for the delegator sets | 3/57:
on-behalf-of control, `expected true to be false` |
| A6b | explain wiring passes `null` for the delegator sets | 3/57:
on-behalf-of control |
| A7 | explain wiring asks `find` instead of the explained operation |
3/57: `create: expected true to be false` |
| A8 | `canReadObject` treats a null admission as admitted | 6/57:
no-scope control and on-behalf-of control |
A first A6 run against a delegator holding no read of
`sys_business_unit` stayed green: arm 6 (the delegator's own grant)
already refuses it. The control now names a delegator who reads every
unit, which is the case only the judgment's own on-behalf-of condition
refuses; A6 and A6b are red against that version (commit 7792e80).
</details>
## Acceptance notes
- `canReadObject` is also asked by the engine's `ReadableIds`
composition over a master (ADR-0139). An app object that is
`controlled_by_parent` with `sys_business_unit` as its master would now
let a delegate read that object's rows under the covered units, where it
read none before. No platform object is such a detail (searched
`packages/platform-objects/src`), and ADR-0055 makes reading a detail
follow reading its master, so this follows the ruled read rather than
widening past it. The seat should confirm this reading.
- `objectReadAdmission`'s other asker is unchanged: it hands no
fallback, so only `canReadObject` gains the delegated read.
- Out of scope per the order and untouched: the ADR-0090 D12 note and
the `reference_not_found` comment in
`packages/spec/src/api/errors.zod.ts`.
## Verification
<details><summary>Local runs on the final head</summary>
- `pnpm --filter @objectstack/plugin-security test` :: exit 0 — Test
Files 248 passed, Tests 5820 passed, 45 skipped (before and after the
merge).
- `pnpm --filter @objectstack/plugin-security typecheck` :: exit 0 (test
layer: 0 files, 0 errors in the debt ledger).
- eslint (repo binary, `--no-inline-config`) on the 7 touched source
files :: exit 0.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 64 families for this diff; all 64
ran on head 2d9ed89 and exited 0. `dispatch-gates --ran` with the
exit-coded record :: exit 0 — "64 derived, 64 run, 0 NOT-MEASURED, 0
UNRUN" (a derived zero). A first pass on an earlier head had four
non-zero exits, all resolved before the run above: three exit 3 for
missing prerequisites (a shallow clone for `check-plugin-teardown-shape
--self-test`, the CLI build closure for `check:i18n`, a full `pnpm
build` for `check:dual-build-cjs-loads`), and
`check:query-options-erasure` exit 1 because the new test helpers cast
three query-option bags `as any`; they are typed now (commit
2d9ed89).
- After merging `origin/main` (two commits, neither in plugin-security;
head 40c2c8a): formula, platform-objects, plugin-auth and
service-settings rebuilt, then `pnpm --filter
@objectstack/plugin-security typecheck` and `test` :: exit 0 (248 files,
5820 passed, 45 skipped), and the ratchet and changeset families again
on 40c2c8a:
```text
pnpm check:query-options-erasure :: exit 0
pnpm check:type-check-coverage :: exit 0
pnpm check:type-check-debt :: exit 0
pnpm check:engine-double-contract :: exit 0
pnpm check:test-source-alias :: exit 0
pnpm check:cross-package-test-inputs :: exit 0
pnpm check:i18n :: exit 0
pnpm check:nul-bytes :: exit 0
node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0
node scripts/check-empty-changeset.mjs --base origin/main :: exit 0
node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0
```
- The 64 on 2d9ed89:
```text
node scripts/check-adr-0087-registration.mjs --base origin/main :: exit 0
node scripts/check-adr-0087-registration.mjs --self-test :: exit 0
node scripts/check-changeset-no-major.mjs --base origin/main :: exit 0
node scripts/check-closing-keyword-parity.mjs :: exit 0
node scripts/check-closing-keyword-parity.mjs --self-test :: exit 0
node scripts/check-comment-mask-adoption.mjs :: exit 0
node scripts/check-comment-mask-adoption.mjs --self-test :: exit 0
node scripts/check-comment-mask-corpus.mjs :: exit 0
node scripts/check-dts-emitted.mjs --self-test :: exit 0
node scripts/check-empty-changeset.mjs --base origin/main :: exit 0
node scripts/check-empty-changeset.mjs --self-test :: exit 0
node scripts/check-issue-citations.mjs :: exit 0
node scripts/check-plugin-teardown-shape.mjs :: exit 0
node scripts/check-plugin-teardown-shape.mjs --self-test :: exit 0
node scripts/check-tenant-audit-census.mjs :: exit 0
node scripts/check-tenant-audit-census.mjs --self-test :: exit 0
node scripts/ci/assert-turbo-remote-cache.mjs --self-test :: exit 0
node scripts/ci/plan-test-core-shards.mjs --self-test :: exit 0
node scripts/ci/prune-turbo-cache.mjs --self-test :: exit 0
node scripts/ci/snapshot-next.mjs --self-test :: exit 0
node scripts/docs-audit/check-affected-docs.mjs :: exit 0
node scripts/docs-audit/check-drift-comment.mjs :: exit 0
node scripts/pm/release-rehearsal-clone.mjs --self-test :: exit 0
node scripts/release-pending-publish.mjs --self-test :: exit 0
pnpm --filter @objectstack/spec run check:duration-unit-keys :: exit 0
pnpm check:authz-resolver :: exit 0
pnpm check:changeset-gate-self-tests :: exit 0
pnpm check:ci-filter-parity :: exit 0
pnpm check:closed-ceremony-text :: exit 0
pnpm check:cross-package-test-inputs :: exit 0
pnpm check:doc-authoring :: exit 0
pnpm check:driver-memory-census :: exit 0
pnpm check:dts-closure :: exit 0
pnpm check:dual-build-cjs-loads :: exit 0
pnpm check:engine-double-contract :: exit 0
pnpm check:error-status-conformance :: exit 0
pnpm check:gitlink-declared :: exit 0
pnpm check:i18n :: exit 0
pnpm check:i18n-stale-fill :: exit 0
pnpm check:issue-citations :: exit 0
pnpm check:keyed-text-bounds :: exit 0
pnpm check:lean-entry-closure :: exit 0
pnpm check:logger-receiver-detach :: exit 0
pnpm check:nul-bytes :: exit 0
pnpm check:objectql-double-limit :: exit 0
pnpm check:org-identifier :: exit 0
pnpm check:page-declaration-shape :: exit 0
pnpm check:platform-object-tenancy-census :: exit 0
pnpm check:published-files :: exit 0
pnpm check:query-options-erasure :: exit 0
pnpm check:refd-timer-probe :: exit 0
pnpm check:registry-log-declared :: exit 0
pnpm check:rest-log-spy-declared :: exit 0
pnpm check:slot-lookup :: exit 0
pnpm check:sourcemap-no-sources-content :: exit 0
pnpm check:system-context-census :: exit 0
pnpm check:test-source-alias :: exit 0
pnpm check:tier-file-adoption :: exit 0
pnpm check:type-check-coverage :: exit 0
pnpm check:type-check-debt :: exit 0
pnpm check:undeclared-dep-imports :: exit 0
pnpm check:vouch-for-reference-callers :: exit 0
pnpm check:watch-hint-literal :: exit 0
pnpm check:where-matcher :: exit 0
```
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_017ihL2bzfJoq2gdSaX2sVcL)_
🤖 Generated with [Claude Code](https://claude.com/claude-code)
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 56baf78 commit 449c858
8 files changed
Lines changed: 287 additions & 29 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
437 | 437 | | |
438 | 438 | | |
439 | 439 | | |
440 | | - | |
441 | | - | |
442 | | - | |
443 | | - | |
444 | | - | |
445 | | - | |
446 | | - | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
447 | 443 | | |
448 | 444 | | |
449 | 445 | | |
450 | 446 | | |
451 | 447 | | |
452 | 448 | | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
453 | 478 | | |
454 | 479 | | |
455 | | - | |
456 | | - | |
457 | | - | |
| 480 | + | |
458 | 481 | | |
459 | 482 | | |
460 | | - | |
| 483 | + | |
461 | 484 | | |
462 | 485 | | |
463 | 486 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
25 | 25 | | |
26 | 26 | | |
27 | 27 | | |
| 28 | + | |
28 | 29 | | |
29 | 30 | | |
30 | 31 | | |
| |||
63 | 64 | | |
64 | 65 | | |
65 | 66 | | |
66 | | - | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
67 | 72 | | |
68 | 73 | | |
69 | 74 | | |
| |||
327 | 332 | | |
328 | 333 | | |
329 | 334 | | |
| 335 | + | |
| 336 | + | |
| 337 | + | |
| 338 | + | |
| 339 | + | |
| 340 | + | |
| 341 | + | |
| 342 | + | |
| 343 | + | |
| 344 | + | |
| 345 | + | |
| 346 | + | |
| 347 | + | |
| 348 | + | |
| 349 | + | |
| 350 | + | |
| 351 | + | |
| 352 | + | |
| 353 | + | |
| 354 | + | |
| 355 | + | |
| 356 | + | |
| 357 | + | |
| 358 | + | |
| 359 | + | |
| 360 | + | |
| 361 | + | |
| 362 | + | |
| 363 | + | |
| 364 | + | |
| 365 | + | |
| 366 | + | |
| 367 | + | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
| 372 | + | |
| 373 | + | |
| 374 | + | |
| 375 | + | |
| 376 | + | |
| 377 | + | |
| 378 | + | |
| 379 | + | |
| 380 | + | |
| 381 | + | |
| 382 | + | |
| 383 | + | |
| 384 | + | |
| 385 | + | |
| 386 | + | |
| 387 | + | |
| 388 | + | |
| 389 | + | |
| 390 | + | |
| 391 | + | |
| 392 | + | |
| 393 | + | |
| 394 | + | |
| 395 | + | |
| 396 | + | |
| 397 | + | |
| 398 | + | |
| 399 | + | |
| 400 | + | |
| 401 | + | |
| 402 | + | |
| 403 | + | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
| 407 | + | |
| 408 | + | |
| 409 | + | |
| 410 | + | |
| 411 | + | |
| 412 | + | |
| 413 | + | |
| 414 | + | |
| 415 | + | |
| 416 | + | |
| 417 | + | |
| 418 | + | |
| 419 | + | |
| 420 | + | |
| 421 | + | |
| 422 | + | |
| 423 | + | |
| 424 | + | |
| 425 | + | |
| 426 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
0 commit comments