Repository navigation
Commit 474c030
fix(plugin-security): an assignment write the administrator cannot address answers not-found, as a read does; a refusal names only values the caller can read (#30165)
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:组织管理员和委派管理员按 id
修改或删除授权行(`sys_user_position`、`sys_user_permission_set`)时,如果这一行自己读不到,或这个
id 根本没有对应的行,现在都回答读取门的 `404 RECORD_NOT_FOUND`,和读取这个 id、以及其他任何对象按 id
写入时的回答完全一样。委派管理员被拒时,提示和 details 里只出现调用者自己读得到、或自己写进去的值。
- 为什么改:落实维护者裁决:按 id 写入时,读不到的行按读取门的回答处理;拒绝提示不写出调用者读不到的字段值,判定照旧按存储行做。
- 风险与回滚:只是同一类拒绝的状态码从 403 换成
404,原来放行的写入一律照旧放行;读得到的行判定和提示不变;插入、租户级管理员、读取被整体拒绝的调用者都不受影响。回滚即撤销本 PR。
- 先看:`security-plugin-authz-middleware.ts` 里
`readDoorAnswerForGateRefusal` 新增的 not-found
分支;`delegated-admin-gate.ts` 里的
`UnreadableAddressedRowRefusal`、`delegatedTargets` 和 `mayName`;组织管理员分支
`organizationAdminFailure` 对 update 先问一次读取。
- 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run
src/org-admin-assignment-boundary.posture.test.ts`,预期 74/74 通过。
- 需要你做的:确认本 PR 对两条未发布 changeset 的改写(见 Acceptance notes
第一条);`check-empty-changeset` 在你确认之前按设计保持红。
## What changes
- **An assignment write the administrator cannot address answers
not-found, as a read does.** For the by-id update or delete the caller
addressed, when the delegated-administration gate (ADR-0090 D12) refused
it because the caller's own read of the row returned none, the security
middleware now answers `recordNotFoundError` for that object and id: the
read door's `404 RECORD_NOT_FOUND`, which step 2.7 already answers for
every other object. This covers the organization administrator's branch
and the delegated branch (a delegate holding an `adminScope`, and an
organization administrator who holds one too). A row the caller cannot
read and an id naming no row get the same answer, and it is the same
answer a `GET` of that id gets.
- The gate marks that refusal with its own class,
`UnreadableAddressedRowRefusal` (a `PermissionDeniedError`). Only
`readDoorAnswerForGateRefusal` turns it into the not-found, and only for
the by-id write the caller addressed (`addressedByIdWriteId`, with step
2.7's exclusions). A nested write, a referential clear and the
write-admission preview keep the gate's `403`, which names no row and no
id.
- A caller the read door refuses outright still gets that read refusal,
unchanged.
- An organization administrator's edit in place is asked the read
question first, so an edit of a row it reads is still refused as an edit
(`403`).
- **A refusal names a value only when the caller can read it.** In the
delegated walk, each value its guidance names — the position, the sets
it distributes, the business unit, the holder, the permission set — is
named only when the caller wrote it (an insert's row, a column its
update payload carries), or when the caller's own read of the row
returned that column at its stored value (`mayName`). Otherwise the
sentence is the same without the value, for example "the assignment's
position distributes a permission set which is not in the scope's
allowlist", and `details` leaves the value out. The verdict does not
change: it is still taken on the stored row.
- "Can read" is the read door's own answer, not a second evaluator. The
gate already holds the caller's read of the row (`readAsCaller`, the
operation's memoised pre-image read). A column a field rule hides is
absent from that read, and a masked column differs from the stored
value, so neither is named.
- The organization administrator's branch already words its refusal of a
removal from the caller's own read, so it names nothing that read hides.
It is pinned here and not changed.
- **Two unreleased changesets corrected.**
`.changeset/org-admin-removal-caller-read.md` and
`.changeset/delegated-admin-unreadable-row.md` said the answer is `403
PERMISSION_DENIED`. This change makes that false, so each now names the
not-found.
## Tests
- `org-admin-assignment-boundary.posture.test.ts` (real ObjectQL engine
and SQL driver, real `SecurityPlugin`):
- Under `group` and `isolated`, for both tables, both verbs, and a row
the caller cannot read as well as an id naming no row:
- the organization administrator (both sets), the delegate and the
organization administrator who holds a scope each answer the read door's
not-found, compared whole (status, code, name, message, every own field)
with `recordNotFoundError` for that id;
- the tenant-level administrator, whom step 2.7 answers, gets the same
whole answer;
- a delegate the read door refuses gets that read refusal, whole.
- In every posture, for each of the four columns a refusal could name, a
delegate whose field rules hide the column is refused
`DELEGATION_SCOPE_VIOLATION`, and its value appears nowhere in the
answer. The delegate who reads the column gets the same verdict, with
the value named. A value the caller wrote is named. An organization
administrator whose field rules hide the column is refused without the
value.
- A partially masked column (a masking rule on `position`, not
`readable: false`): the delegate reads the masked value, and neither
that value nor the stored one appears anywhere in the serialized
refusal. The verdict still comes from the stored row: refused for a
stored position off the allowlist, admitted for one inside the scope.
- The field-hidden-position control from the delegated describe is
re-pinned to the verdict and code, plus no hidden value in the answer.
- Controls kept: in-scope rows the caller reads are admitted;
out-of-scope rows it reads keep their guidance with the values; the
tenant-level administrator is answered as before; in `single`, rows the
caller reads keep their guidance and an id naming no row answers the
not-found.
- HTTP dogfood (showcase, `isolated`):
`org-admin-assignment-removal-unreadable-row` and
`delegated-admin-assignment-write-unreadable-row` now pin `PATCH` and
`DELETE`, of the row and of an id naming none, to equal exactly what a
`GET` of that id answers (`404 RECORD_NOT_FOUND`, status and body). Each
has a measured red leg, below.
## Acceptance notes
- **Correcting two pending release notes: please confirm.** This PR
rewrites the first bullet of
`.changeset/org-admin-removal-caller-read.md` and of
`.changeset/delegated-admin-unreadable-row.md`. Neither has shipped (the
plugin's `CHANGELOG.md` holds neither, and npm has `18.3.0`). Both said
a row the caller cannot read answers `403 PERMISSION_DENIED`, which this
change makes false. Each now says the read door's `404
RECORD_NOT_FOUND`. `check-empty-changeset` names both files and stays
red by design until a maintainer confirms. Restoring them from the base
would republish the false sentence.
- The Setup screens that remove an assignment (Assigned users on a
permission set, Position holders on a position) show the server's
sentence for any refused removal, whatever the status, and fall back to
their localized "Failed to remove" line. Nothing under `ui/` treats a
`403` and a `404` from these writes differently. They list only rows the
caller reads, so the new answer reaches them only when a row was removed
in between. That answer now reads "Record … not found in …" where it
used to read as an access refusal.
- A caller with no administrator authority over these tables is answered
as before.
## Verification
Readings at `da7d298fd2`, this body's head: the plugin typecheck, the
posture rig, ablation A3 and the derived gates. Readings at
`f29ee6a49d`: the whole suite and the dogfood legs. Since then only the
posture rig's test file and one pending changeset's wording have
changed. Readings at `ddb6927721`: ablations A1 and A2. The one commit
between `ddb6927721` and `f29ee6a49d` types a test's assertion label as
a string. CI's families are this PR's own check-runs; `test:repo` and
the whole-tree lint are CI's.
<details><summary>plugin-security: typecheck, the whole suite, the
posture rig</summary>
```text
pnpm typecheck (tsc + tsconfig.scripts.json + check:test-typecheck) exit 0
vitest run, whole package, under the shared verify lock Test Files 242 passed (242) · Tests 5269 passed | 45 skipped (5314)
vitest run src/org-admin-assignment-boundary.posture.test.ts Tests 74 passed (74)
```
</details>
<details><summary>One-shot ablations of the posture rig (its subject
resolves from this package's own source)</summary>
```text
A1 the not-found arm of readDoorAnswerForGateRefusal disabled
-> Tests 5 failed | 68 passed: the organization administrator's not-found test (group, isolated),
the 'single' control's no-row leg, the delegated not-found test (group, isolated)
A2 mayName answers true for every value
-> Tests 5 failed | 68 passed: the field-rule test (single, group, isolated) and the delegated
control's hidden-position leg (group, isolated)
A3 a column counts as read when present at any value, not at its stored value
-> Tests 1 failed | 73 passed: the partially masked column case
(the refusal named position 'qa_out_pos')
restore, every file: blob == HEAD and `git diff HEAD` empty (scripts/ablation-replace.mjs)
```
</details>
<details><summary>HTTP dogfood: the two re-pinned files, with a measured
red leg</summary>
```text
closure build (turbo, @objectstack/dogfood^...) exit 0 · 69 tasks
RED not-found arm disabled; @objectstack/plugin-security rebuilt
ablation-dist-preflight: marker present in dist/index.js and dist/index.mjs
-> Test Files 2 failed (2) · Tests 2 failed | 4 passed: exactly the two re-pinned tests;
each received 403 PERMISSION_DENIED where GET of the same id answers 404 RECORD_NOT_FOUND
restore: source blob == HEAD; rebuilt; preflight --absent: marker absent from all 6 built files; tree clean
GREEN Test Files 2 passed (2) · Tests 6 passed (6)
```
</details>
<details><summary>Derived gate families (dispatch-gates --commands:
64)</summary>
```text
62 of the 64 commands :: exit 0, each run once at this head with its exit code captured before any pipe
node scripts/check-empty-changeset.mjs --base origin/main :: exit 1
the deliberate correction of two pending changesets (Acceptance notes, first item)
pnpm check:dual-build-cjs-loads :: exit 3
NOT MEASURED: the gate reads every package's dist/, and this worktree builds only the closures
it needs; @objectstack/plugin-security's rebuilt dist loads as CommonJS and as ESM (exit 0 each)
--ran: 64 derived, 63 run, 1 NOT-MEASURED (derived from the recorded exit 3), 0 UNRUN
```
</details>
Full dogfood battery: not owed (no `!` in the title, no narrowing arm).
<!-- signed: security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj
-->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 4851668 commit 474c030
8 files changed
Lines changed: 488 additions & 115 deletions
File tree
- .changeset
- packages
- plugins/plugin-security/src
- qa/dogfood/test
| 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
| 12 | + | |
13 | 13 | | |
14 | 14 | | |
15 | 15 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
9 | 9 | | |
10 | 10 | | |
11 | 11 | | |
12 | | - | |
| 12 | + | |
13 | 13 | | |
14 | | - | |
| 14 | + | |
Large diffs are not rendered by default.
0 commit comments