Repository navigation
Commit e669735
fix(plugin-security): a refused grant holder the caller cannot read is answered as a holder that does not exist (#31244)
security#1 · os-bob · session_012dn2AdqWnb3ndnEacKvJvF
Clause-②: no
Metadata-protocol: none
Skills-docs: none — what a metadata author writes is unchanged; only the
refusal entry one class of write receives changes
## 维护者速读
-
改了什么:岗位分配(`sys_user_position`)与权限集授予(`sys_user_permission_set`)的写入,若点名一个调用者自己读不到的用户,被拒时改为得到与"该用户不存在"完全相同的回答,由引擎自己的引用检查给出。
- 为什么改:安全加固——调用者从一次被拒的授予里,不应得知任何关于自己无权查看的用户的信息。
-
风险与回滚:哪些写入被拒、状态码与错误码都不变,只换掉一类拒绝条目的文案与约束字段;出厂权限下组织管理员看不到组织外的用户,所以对组织外用户的拒绝文案会从"不是该组织成员"变为"不存在该用户记录"。回滚即还原一个文件。
-
先看:`packages/plugins/plugin-security/src/grant-holder-membership-refusal.ts`(`holderLeftToReferenceCheck`
与 `referenceCheckAsksCaller` 两段,风险在后者的放行条件)、同名测试文件末尾新增的 describe
块、`packages/core/src/security/second-object-read-exposure.pin.test.ts`
里新增的一条普查登记、`.changeset/sec-grant-holder-unreadable-answer.md`。
- 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run
src/grant-holder-membership-refusal.test.ts`,预期 31 个测试全部通过。
- 需要你做的:无,席位落地。
## What changes
The rule: **a refused grant holder the caller cannot read is answered as
a holder that does not exist** — on `sys_user_position` and
`sys_user_permission_set`, for every non-system insert, and for every
update that names a different holder than the row stores.
- The grant-holder membership refusal now asks the caller's own read
door about a holder the write names, the same read the engine's
reference check asks of a `user_id` value. A holder that door does not
return is left to the engine's reference check, which answers it with
its own `reference_not_found` entry in the engine's own message context
— the answer every other user reference that caller names receives. The
plugin keeps no second spelling of that sentence.
- A holder the caller CAN read is judged on organization membership
exactly as before, and a non-member keeps the membership refusal and its
remedy.
- The hand-off happens only where the engine's check is certain to ask:
the live object's `user_id` is a non-`readonly` `lookup` / `user`
reference to `sys_user`, and `sys_user` is registered. Anywhere else the
membership predicate is judged in the plugin as before.
- The second-object read census
(`packages/core/src/security/second-object-read-exposure.pin.test.ts`)
classifies the plugin as a caller of the reference-target arbiter that
serves nothing of the target: its one read of a holder is the caller's
own read door, projected to `id`, and its answer only decides whether
the write-path reference check answers the holder.
- Unchanged: which writes are refused, `400`, `VALIDATION_FAILED`,
`fields[].field: user_id`, `fields[].code: reference_not_found`, the
system-write stand-down, global grants, and an update that leaves the
holder as stored (judged on membership, as before). No new error code.
Route choice: the engine's own reference check answers the holder,
rather than the plugin rebuilding the engine's sentence.
`@objectstack/plugin-security` carries no runtime dependency on
`@objectstack/objectql`, where the sentence builder and its message
context (negotiated locale, translated field label) live, and handing
the holder to the engine's own door keeps every stage after the
membership hook identical for any holder the caller cannot read. No
engine file is touched.
## Tests
`packages/plugins/plugin-security/src/grant-holder-membership-refusal.test.ts`,
one new top-level `describe` in the existing harness (a real `ObjectQL`
engine over a real SQL driver with the real `SecurityPlugin`), 10 cases:
- CONTROL — the caller's own `sys_user` read returns its own row and
never the named holder, which exists where the case needs it.
- `sys_user_position` and `sys_user_permission_set`, insert and
update-by-id — the whole answer an HTTP door serves
(`resolveThrownHttpError`, serialized) is byte-identical across three
stores of the same holder id, refused at `user_id` with
`reference_not_found`; nothing is stored, the updated row is untouched
(4 cases).
- A holder the caller cannot read is read only through the caller's own
door, projected to its `id`, and no membership is read for it (every
read naming the holder is spied: non-system, the caller's `userId`,
`fields: ['id']`) — the behaviour pin the census row above names.
- A holder the caller can read keeps the membership refusal
(`constraint.target: sys_member`), and a readable member is admitted.
- The refusal set is unchanged on the path the engine's check does not
judge: an update that moves only the organization is still refused for a
non-member and lands once the holder is a member there.
- Under the shipped organization administrator's `sys_user` read, a
member is admitted, and a user outside the organization gets the same
answer a missing user gets.
- A composition with no `sys_user` object: membership is judged in the
plugin.
The pre-existing 21 cases of the file pass unchanged.
`packages/core/src/security/second-object-read-exposure.pin.test.ts`:
one `CALLERS` row, `kind: 'not-a-served-read'`, naming the pin above.
## Acceptance notes
- Under the shipped `organization_admin` set, a user outside the
caller's organization is unreadable to that caller, so a grant naming
one now reads the engine's `reference_not_found` sentence instead of the
membership sentence. The refusal itself and its codes are unchanged;
this is the platform's answer for every user reference that caller
cannot read.
- The refused entry's `constraint` follows the entry: `{ target:
sys_user }` from the reference check for a holder the caller cannot
read, `{ target: sys_member, targetField, organization }` from the
membership refusal otherwise.
- `Clause-②: no` without an arm: no write accepted before is refused
now, and none refused before is accepted.
## Verification
Every reading below except the first three ablations was taken at head
`367379988` (after the last merge of `main`, at `7c523aa10`).
<details>
<summary>Package: build, typecheck, full suite</summary>
Under the verify lock, after rebuilding the dependency closure
(`--filter '@objectstack/plugin-security^...'`): `pnpm --filter
@objectstack/plugin-security build`, `pnpm --filter @objectstack/spec
check:generated` (14 artifacts up to date), `pnpm --filter
@objectstack/plugin-security typecheck` (incl. `check:test-typecheck`: 0
errors), the core `repo` project (`Test Files 7 passed (7)`, `Tests 96
passed (96)`), then the plugin-security suite: `Test Files 248 passed
(248)`, `Tests 5834 passed | 45 skipped (5879)`, lock VERDICT
command-exit 0.
</details>
<details>
<summary>Ablations: 5 legs, each red then restored</summary>
Taken at `ee912fb9` (the fix commit; the test file has since lost one
type cast and nothing else). Each leg mutated through
`scripts/ablation-replace.mjs`, which proves the mutation landed (anchor
count, blob hash) and that the restore matched HEAD with `git diff HEAD`
empty. The tests import the hook from `src/`, so no `dist/` is involved.
1. The insert-path hand-off removed: 2 of 30 red, exactly the two insert
cases.
2. The update-path hand-off removed: 2 of 30 red, exactly the two update
cases.
3. The `sys_user`-registered condition removed from the hand-off guard:
1 of 30 red, the no-`sys_user` composition case (the write is no longer
refused).
4. At `367379988`, the census row's key renamed: 2 of 4 red in the core
pin (the unclassified-caller case, which names this file, and the
vacuity control).
5. At `367379988`, the insert-path hand-off removed: 3 of 31 red, the
two insert cases and the read-shape pin.
</details>
<details>
<summary>Gates</summary>
`node scripts/pm/dispatch-gates.mjs --commands` derives 69 families at
`367379988` (the core census file adds six). All 69 exit 0 at
`367379988`, `check:type-check-debt` included (run under the verify
lock). `check:dts-closure` and `check:dual-build-cjs-loads` first read
exit 1 while this worktree's own closure build was rewriting
`packages/spec/dist` beside them; re-run after it, both exit 0. `--ran`
reconciliation: 69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN. Repo-wide
scans and every other family are CI's.
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_012dn2AdqWnb3ndnEacKvJvF)_
<!-- signed: security#1 · os-bob · session_012dn2AdqWnb3ndnEacKvJvF -->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 82cccaf commit e669735
4 files changed
Lines changed: 360 additions & 14 deletions
File tree
- .changeset
- packages
- core/src/security
- plugins/plugin-security/src
| 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 | |
|---|---|---|---|
| |||
264 | 264 | | |
265 | 265 | | |
266 | 266 | | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 271 | + | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
267 | 279 | | |
268 | 280 | | |
269 | 281 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
27 | 27 | | |
28 | 28 | | |
29 | 29 | | |
| 30 | + | |
| 31 | + | |
30 | 32 | | |
31 | 33 | | |
32 | 34 | | |
| |||
74 | 76 | | |
75 | 77 | | |
76 | 78 | | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
77 | 101 | | |
78 | 102 | | |
79 | 103 | | |
| |||
119 | 143 | | |
120 | 144 | | |
121 | 145 | | |
| 146 | + | |
| 147 | + | |
122 | 148 | | |
123 | 149 | | |
124 | 150 | | |
| |||
154 | 180 | | |
155 | 181 | | |
156 | 182 | | |
157 | | - | |
158 | | - | |
| 183 | + | |
| 184 | + | |
| 185 | + | |
| 186 | + | |
159 | 187 | | |
160 | 188 | | |
161 | 189 | | |
| |||
167 | 195 | | |
168 | 196 | | |
169 | 197 | | |
170 | | - | |
| 198 | + | |
171 | 199 | | |
172 | 200 | | |
173 | 201 | | |
| |||
446 | 474 | | |
447 | 475 | | |
448 | 476 | | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
| 496 | + | |
| 497 | + | |
| 498 | + | |
| 499 | + | |
| 500 | + | |
| 501 | + | |
| 502 | + | |
| 503 | + | |
| 504 | + | |
| 505 | + | |
| 506 | + | |
| 507 | + | |
| 508 | + | |
| 509 | + | |
| 510 | + | |
| 511 | + | |
| 512 | + | |
| 513 | + | |
| 514 | + | |
| 515 | + | |
| 516 | + | |
| 517 | + | |
| 518 | + | |
| 519 | + | |
| 520 | + | |
| 521 | + | |
| 522 | + | |
| 523 | + | |
| 524 | + | |
| 525 | + | |
| 526 | + | |
| 527 | + | |
| 528 | + | |
| 529 | + | |
| 530 | + | |
| 531 | + | |
| 532 | + | |
| 533 | + | |
| 534 | + | |
| 535 | + | |
| 536 | + | |
| 537 | + | |
| 538 | + | |
| 539 | + | |
| 540 | + | |
| 541 | + | |
| 542 | + | |
| 543 | + | |
| 544 | + | |
| 545 | + | |
| 546 | + | |
| 547 | + | |
| 548 | + | |
| 549 | + | |
| 550 | + | |
| 551 | + | |
| 552 | + | |
| 553 | + | |
| 554 | + | |
| 555 | + | |
| 556 | + | |
| 557 | + | |
| 558 | + | |
| 559 | + | |
| 560 | + | |
| 561 | + | |
| 562 | + | |
| 563 | + | |
| 564 | + | |
| 565 | + | |
| 566 | + | |
| 567 | + | |
| 568 | + | |
| 569 | + | |
| 570 | + | |
| 571 | + | |
| 572 | + | |
| 573 | + | |
| 574 | + | |
| 575 | + | |
| 576 | + | |
| 577 | + | |
| 578 | + | |
| 579 | + | |
| 580 | + | |
| 581 | + | |
| 582 | + | |
| 583 | + | |
| 584 | + | |
| 585 | + | |
| 586 | + | |
| 587 | + | |
| 588 | + | |
| 589 | + | |
| 590 | + | |
| 591 | + | |
| 592 | + | |
| 593 | + | |
| 594 | + | |
| 595 | + | |
| 596 | + | |
| 597 | + | |
| 598 | + | |
| 599 | + | |
| 600 | + | |
| 601 | + | |
| 602 | + | |
| 603 | + | |
| 604 | + | |
| 605 | + | |
| 606 | + | |
| 607 | + | |
| 608 | + | |
| 609 | + | |
| 610 | + | |
| 611 | + | |
| 612 | + | |
| 613 | + | |
| 614 | + | |
| 615 | + | |
| 616 | + | |
| 617 | + | |
| 618 | + | |
| 619 | + | |
| 620 | + | |
| 621 | + | |
| 622 | + | |
| 623 | + | |
| 624 | + | |
| 625 | + | |
| 626 | + | |
| 627 | + | |
| 628 | + | |
| 629 | + | |
| 630 | + | |
| 631 | + | |
| 632 | + | |
| 633 | + | |
| 634 | + | |
| 635 | + | |
| 636 | + | |
| 637 | + | |
| 638 | + | |
| 639 | + | |
| 640 | + | |
| 641 | + | |
| 642 | + | |
| 643 | + | |
| 644 | + | |
| 645 | + | |
| 646 | + | |
| 647 | + | |
| 648 | + | |
| 649 | + | |
| 650 | + | |
| 651 | + | |
| 652 | + | |
| 653 | + | |
| 654 | + | |
| 655 | + | |
| 656 | + | |
| 657 | + | |
| 658 | + | |
| 659 | + | |
| 660 | + | |
| 661 | + | |
| 662 | + | |
| 663 | + | |
| 664 | + | |
| 665 | + | |
| 666 | + | |
| 667 | + | |
| 668 | + | |
| 669 | + | |
449 | 670 | | |
450 | 671 | | |
451 | 672 | | |
| |||
0 commit comments