You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Commit d0da880
Browse filesBrowse the repository at this point in the historyBrowse files
fix(plugin-security): an organization administrator's assignment removal is judged on the stored row, and its refusal names only values the caller can read (#30248)
Clause-②: no
Metadata-protocol: none
## 维护者速读
- 改了什么:组织管理员按 id
删除授权行(`sys_user_position`、`sys_user_permission_set`)时,只要它自己读得到这一行,委派管理闸门(ADR-0090
D12)的组织分支就按存储行判定,和委派分支一样;拒绝提示只写出调用者自己读得到的值。
- 为什么改:落实维护者裁决(ruling of record 6649064874)「ADR-0090 D12
授权闸门按存储行判定,拒绝提示只写出调用者读得到的值」,该裁决覆盖两个分支;#30165 的契约复核把组织分支与之不一致记为
should-fix。
- 风险与回滚:这是放宽:调用者的字段规则或遮罩规则作用在 `position` / `permission_set`
上时,组织边界内的删除现在放行(18.2.0 即按存储行判定);边界外照旧拒绝;读不到的行与不存在的 id 仍回答
404;修改、插入、租户级管理员、委派分支都不变。回滚即撤销本 PR。
- 先看:`packages/plugins/plugin-security/src/delegated-admin-gate.ts` 的
`organizationTargets`(先以调用者读取判断有没有这一行,读到后取存储行)与
`organizationAdminFailure` 里三处 `mayName` 措辞分支。
- 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run
src/org-admin-assignment-boundary.posture.test.ts`,预期 85/85 通过。
- 需要你做的:确认本 PR 对未发布 changeset
`.changeset/org-admin-removal-caller-read.md` 一句话的改写(见 Acceptance notes
第一条);在你确认之前 `Check Changeset` 按设计保持红。
## What changes
- **The organization branch judges a removal on the stored row.** An
organization administrator (`organization_admin` /
`organization_admin_no_bypass`) removes an assignment row by id. Whether
there is a row is still asked of the caller's own read (`callerRead`,
the read door's answer to this caller). Once that read returns the row,
`organizationTargets` takes the stored row (`storedRow`) and the
organization, position and permission-set checks run on it, as the
delegated branch's `delegatedTargets` does. This follows ruling of
record 6649064874 ("the ADR-0090 D12 assignment gate judges on the
stored row, and its refusal names only what the caller can read"), which
covers both branches. No second reader: the branch reuses `callerRead`,
`storedRow`, `columnsReadAsStored` and `mayName`, and its target type is
now `DelegatedTarget`. No stored read is made for a row the caller's
read does not return.
- **One ruling record for the rule, across the package.** Every citation
of the stored-row verdict and of the rule that a refusal names only what
the caller can read, in this branch's code and in the code it builds on,
now names ruling of record 6649064874; the citations of 5985885287 for
the read door's not-found answer are kept.
- **A refusal names a value only when the caller can read it.** The
row's organization, its position (and the sets that position carries,
which are named only beside it) and its permission set are named only
when the caller's read returned that column at its stored value
(`mayName`). Otherwise the sentence is "the row does not belong to the
organization you administer", "the assignment's position is not inside
the organization boundary" or "the grant's permission set is not inside
the organization boundary", and `details` leaves the value out. A column
the caller does read is still named, so the guidance stays specific
where it can be.
- **Unchanged.** A row the caller cannot read and an id naming no row
keep the read door's `404 RECORD_NOT_FOUND`. A read refusal or a store
fault propagates as raised. Inserts, tenant-level administrators and the
delegated branch are as before. The update path is untouched: an edit in
place is refused before any row is looked up, as #30200 set it, and
nothing here pins it.
- **Clause-② declaration, measured.** This admits removals the published
18.3.0 refuses, so it widens; that is declared here and in the
changeset. It is `Clause-②: no` because it is a runtime permission
behaviour change, which `execution-duties.md` places outside Clause ②
(its negative boundary: runtime permission and security behaviour is the
human-floor security/permission class), and no published contract
surface moves: no spec schema, no export (the package's public API is
unchanged), no error code. For history: 18.2.0's organization branch
judged the stored row (a system read), and 18.3.0's judged the caller's
read of it; this keeps 18.3.0's not-found for a row the caller cannot
read and takes the verdict from the stored row again.
- **Changeset**: `@objectstack/plugin-security` patch, plus a
one-sentence correction of a pending note (Acceptance notes, first
item).
## Tests
- `org-admin-assignment-boundary.posture.test.ts` (real `ObjectQL`
engine, SQL driver and `SecurityPlugin`), new describe "an organization
administrator's removal is judged on the stored row, and its refusal
names only values the caller can read", 11 tests:
- under `group` and `isolated`, for a position or permission set hidden
by a field rule and for one partially masked: the caller reads the row
without the stored value (precondition, off the read door); the removal
of an in-boundary row is admitted and lands; the removal of an
out-of-boundary row is refused `403 PERMISSION_DENIED` by the boundary,
and neither the stored values (the position, the sets behind it, the
permission set) nor the masked value appear anywhere in the serialized
answer;
- under `group` and `isolated`, for the row's organization hidden by a
field rule and partially masked: the plugin's own D12 gate, asked as the
write door asks it, admits the in-boundary removal on the stored
organization, and refuses the out-of-boundary one naming the values the
caller reads (the set) and not the organization, through the gate and
through the door;
- control, `group` and `isolated`: callers whose field rules hide a
judged column still get the read door's not-found, whole, when they
remove a row they cannot read or an id naming no row;
- control, `single`: a caller whose field rules hide the organization
removes its own organization's in-boundary row end to end, and is
refused another organization's row without that organization being
named.
- **Re-pinned #30165's organization-branch pin** in "an administrator's
refused assignment write names a value only when the caller reads it, or
wrote it" (all three postures). That pin asserted only that the refusal
named no value the caller does not read. It now also asserts the
boundary's refusal on the stored row ("is not inside the organization
boundary") and adds the same caller's in-boundary removal, admitted.
- `delegated-admin-gate.test.ts`, new top-level describe on the existing
organization harness, 20 tests: for both administrator sets, each of
`position`, `permission_set` and `organization_id` (both tables) served
hidden and masked, in-boundary removals resolve and out-of-boundary ones
are refused with the rule's sentence and without the stored or masked
value; a column the caller reads keeps its value in the guidance; a row
the caller's read does not return and an id naming no row are the
unreadable refusal, with no stored read of the addressed row.
- Test names state the rule. The posture rig grows from 74 to 85 tests;
its new describe adds about 1.4 s of test time on a loaded shared box,
inside the file's declared 20 s budget.
## Acceptance notes
- **Correcting a pending release note: please confirm.**
`.changeset/org-admin-removal-caller-read.md` has not shipped as a note
(the plugin's `CHANGELOG.md` at 18.3.0 holds none of it). Its sentence
"The delegated-administration gate (ADR-0090 D12) now judges that row as
the administrator itself reads it, in its own context." is false after
this change, so it now reads "now asks the administrator's own read, in
its own context, whether that row is there to remove." Two of its
bullets are corrected too: the not-found bullet now says "the removal of
a row the administrator cannot read" (an edit in place is refused before
any row is looked up, so the 404 is a removal's answer), and "A row the
administrator can read keeps the answer it had, including the guidance…"
now reads "A row the administrator can read is judged on the stored row.
A refusal names the organization boundary, and names the row's
organization, position or permission set only when the administrator's
own read returns that value as stored." Its title, its first paragraph
and its "Unchanged" bullet are true of the tree as written.
`check-empty-changeset` names the file and stays red by design until a
maintainer confirms; restoring the sentence from the base would publish
a false one.
- **The row's organization column.** This gate judges it on the stored
row too. The rig pins that at the gate under `group` and `isolated`, and
end to end under `single`. Under the walled postures a write also
answers the write door's own row-level check, which this PR does not
change; how that check treats a caller who does not read the
organization column is a separate change the seat has ordered.
- **#30200 is merged in.** It reorders the organization branch's update
refusal and edits the same docblock as this PR; the merge conflicted in
that docblock only, and the resolution keeps both texts (its
update-order sentences replace this branch's earlier sentence about
edits). This PR's new pins exercise removals only, and
`update-by-id-refusal-order.dogfood.test.ts` passes on this head
(Verification).
## Verification
Readings at `d33de915d2`, this body's head: the plugin typecheck, the
whole suite, the two edited files, the three HTTP dogfood files and the
derived gates. The ablations ran at `5fd3195845`; since then the gate
file changed only by the merge of #30200 (its docblock and its update
ordering). 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 two
edited files</summary>
```text
pnpm --filter @objectstack/plugin-security typecheck exit 0 (check:test-typecheck OK, 0 errors)
vitest run, whole package, under the shared verify lock Test Files 242 passed (242) · Tests 5318 passed | 45 skipped (5363)
vitest run src/org-admin-assignment-boundary.posture.test.ts Tests 85 passed (85)
vitest run src/delegated-admin-gate.test.ts Tests 157 passed (157)
```
</details>
<details><summary>HTTP dogfood: the update-by-id refusal order and the
two unreadable-row files</summary>
```text
closure build (pnpm --filter '@objectstack/dogfood^...' build) exit 0
ablation-dist-preflight, this change's wording present in plugin-security dist/index.js and dist/index.mjs
update-by-id-refusal-order, org-admin-assignment-removal-unreadable-row,
delegated-admin-assignment-write-unreadable-row Test Files 3 passed (3) · Tests 25 passed (25)
```
</details>
<details><summary>One-shot ablations of the two edited files (their
subject resolves from this package's own source)</summary>
```text
A1 organizationTargets judges the caller's read again (the stored read replaced by the caller's row)
-> Tests 30 failed | 212 passed: every new test except the four not-found controls,
and the re-pinned field-rule test in all three postures
A2 the position refusal names the position whatever the caller reads
-> Tests 11 failed | 231 passed: the position cases of both new describes
(rig: hidden and masked, group and isolated; unit: both sets, hidden and masked),
and the re-pinned field-rule test in all three postures
restore, both: blob == HEAD (fbfc61a) and `git diff HEAD` empty (scripts/ablation-replace.mjs)
```
</details>
<details><summary>Derived gate families (dispatch-gates --commands:
60)</summary>
```text
59 of the 60 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 one pending changeset (Acceptance notes, first item)
dispatch-gates --ran: 60 derived, 60 run, 0 NOT-MEASURED, 0 UNRUN
```
</details>
<details><summary>Contract-review findings commits 0cc6f17 and
0b0d0e2 (comments, describe names and the corrected pending
note)</summary>
```text
pnpm --filter @objectstack/plugin-security typecheck exit 0 (check:test-typecheck OK, 0 errors)
vitest run src/org-admin-assignment-boundary.posture.test.ts Tests 85 passed (85)
vitest run src/delegated-admin-gate.test.ts Tests 157 passed (157)
node scripts/check-empty-changeset.mjs --base origin/main exit 1 (only .changeset/org-admin-removal-caller-read.md, the declared correction)
check-adr-0087-registration, check-changeset-no-major, check:issue-citations,
check:doc-authoring, check:nul-bytes, check:closed-ceremony-text exit 0 each
```
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_01QSeKYn5CmmdShCJifm6WYj)_
<!-- signed: security#1 · os-justin · session_01QSeKYn5CmmdShCJifm6WYj
-->
---------
Co-authored-by: Claude <noreply@anthropic.com>
Copy file name to clipboardExpand all lines: .changeset/org-admin-removal-caller-read.md
+3-3Lines changed: 3 additions & 3 deletions
Display the source diff
Display the rich diff
Original file line number
Diff line number
Diff line change
@@ -7,8 +7,8 @@ fix(plugin-security): an organization administrator's refused assignment write a
7
7
Clause-②: no
8
8
Metadata-protocol: none
9
9
10
-
An organization administrator (`organization_admin` or `organization_admin_no_bypass`) removes an assignment row (`sys_user_position` or `sys_user_permission_set`) by id. The delegated-administration gate (ADR-0090 D12) now judges that row as the administrator itself reads it, in its own context.
10
+
An organization administrator (`organization_admin` or `organization_admin_no_bypass`) removes an assignment row (`sys_user_position` or `sys_user_permission_set`) by id. The delegated-administration gate (ADR-0090 D12) now asks the administrator's own read, in its own context, whether that row is there to remove.
11
11
12
-
-**A row the administrator cannot read** is answered exactly as an id that names no row is: the read door's `404 RECORD_NOT_FOUND` for that object and id.
13
-
-**A row the administrator can read**keeps the answer it had, including the guidance that names the organization boundary.
12
+
-**The removal of a row the administrator cannot read** is answered exactly as the removal of an id that names no row is: the read door's `404 RECORD_NOT_FOUND` for that object and id.
13
+
-**A row the administrator can read**is judged on the stored row. A refusal names the organization boundary, and names the row's organization, position or permission set only when the administrator's own read returns that value as stored.
14
14
-**Unchanged.** An edit in place of a row it reads is refused as before, every removal that was admitted is still admitted, and a tenant-level administrator is judged as before.
fix(plugin-security): an organization administrator's assignment removal is judged on the stored row, and its refusal names only values the caller can read
6
+
7
+
Clause-②: no
8
+
Metadata-protocol: none
9
+
10
+
An organization administrator (`organization_admin` or `organization_admin_no_bypass`) removes an assignment row (`sys_user_position` or `sys_user_permission_set`) by id. Once its own read returns the row, the delegated-administration gate (ADR-0090 D12) judges the removal on the stored row, as it judges a delegated administrator's write.
11
+
12
+
-**The verdict is the stored row's.** A field rule or a masking rule on the row's `position` or `permission_set` does not change it: the removal of a row inside the organization boundary is admitted, and the removal of a row outside it is refused.
13
+
-**A refusal names a value only when the caller can read it.** The row's organization, its position and the sets that position carries, or its permission set are named only when the caller's own read returned that column at its stored value. Otherwise the sentence leaves the value out, for example "the assignment's position is not inside the organization boundary", and `details` leaves it out too.
14
+
-**Unchanged.** A row the caller cannot read, and an id that names no row, still answer the read door's `404 RECORD_NOT_FOUND`. An edit in place is refused as before. Inserts, tenant-level administrators and delegated administrators are judged as before.
describe("[ADR-0090 D12; ruling of record 6649064874] an organization administrator's removal is judged on the stored row, and its refusal names only values the caller reads",()=>{
it(`${principal}: with ${c.object}.${c.column}${how} from its reads, an in-boundary row is removed, and a row outside is refused without the stored value`,async()=>{
expect(whole(err),`${r.id}: not the masked value either`).not.toContain('****');
1458
+
}
1459
+
});
1460
+
}
1461
+
}
1462
+
1463
+
it(`${principal}: a column it does read keeps the stored value in the guidance — the position and its set, beside an organization it does not read`,async()=>{
0 commit comments