Repository navigation
Commit 18cdcb2
fix(plugin-security)!: a delegate can neither capture nor evict a direct grant beyond its scope (#31634)
Clause-②: no (narrowing) — an edit that points a direct permission-set
grant at another user is judged on both holders it touches, the user it
takes the grant from and the user it gives the grant to, and an edit
that re-names a stored grant's set is judged on the set it takes away as
well as the set it grants, so a delegated administrator no longer moves
a grant away from a holder outside its subtree or from a holder it
cannot read, nor re-names a stored grant of a set outside its allowlist.
Nothing is widened: no export, error code or status is added.
Metadata-protocol: none
Skills-docs: updated here —
`content/docs/permissions/delegated-administration.mdx` states it in two
rows ("Subtree anchoring" for the holders, "Allowlist, no
self-escalation" for the set); no `skills/**` file teaches editing a
direct grant (`skills/objectstack-data/rules/security.md` says a
delegate writes a grant only "for that set and user", which stays true).
## 维护者速读
-
改了什么:委派管理员(adminScope)编辑一条已存的直接授权(`sys_user_permission_set`)时,委派门现在判断这次编辑碰到的每个持有人和每个权限集:改指向另一个用户时,原持有人和新持有人两端都要在委派子树内且调用者读得到;改名为另一个权限集时,被拿走的原权限集也按「删除这条授权」的标准判断(白名单、安全目录、严格包含)。任一不过即拒。
- 为什么改:落实规则「a delegate can neither capture nor evict a direct grant
beyond its scope」(ADR-0090 D12),与同级的 `sys_user_position`
同时判断前像与新值的做法一致:删不掉的授权,也不能靠编辑挪走。属于安全加固,细节归 security#1 席。
- 风险与回滚:收窄(BREAKING,patch
级,不新增错误码或状态):委派管理员不能再把子树外或读不到的持有人的授权改指给别人,也不能把白名单外权限集的授权改名为白名单内的;拒绝与对应的删除/越界答案相同(`403
DELEGATION_SCOPE_VIOLATION`)。租户级管理员照旧可改;组织管理员的权利本来就只有新建和删除、不含原地编辑,答案不变。回滚
= revert 本 PR。
- 先看:① `packages/plugins/plugin-security/src/delegated-admin-gate.ts` 的
`assertDirectGrantWrite`(`fromHolder` 与 `stored` 两处判断及 `refusedSet`
的细节选取,风险 hunk)、`directGrantSet` / `grantOwnSet`;②
`delegated-admin-own-name-set.posture.test.ts` 末尾两个三形态 describe;③
`delegated-admin-gate.test.ts` 末尾两个 describe。
- 两分钟验证:`pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2 src/delegated-admin-gate.test.ts
src/delegated-admin-own-name-set.posture.test.ts`,预期 `Test Files 2
passed (2)`、`Tests 383 passed (383)`。
- 需要你做的:无,席位落地。
## What changes
**Rule.** A delegate can neither capture nor evict a direct grant beyond
its scope. A delegated write on a direct permission-set grant is judged
on every holder and every set it touches (ADR-0090 D12, which scopes
direct set grants to "inside the subtree" and allowlists what a delegate
may hand out).
- **Holders** — `assertDirectGrantWrite`
(`packages/plugins/plugin-security/src/delegated-admin-gate.ts`): when
an update changes the grant's holder, the stored holder is judged
exactly as the holder the write names — through the caller's own read
(`targetUserUnits`), inside the subtree only through one of its business
units — with the same reason, class, sentence and details. Mirrors the
sibling door, `assertAssignmentWrite`, whose docblock reads "Pre-image
(update/delete) must also lie inside the subtree — a delegate can
neither capture nor evict assignments beyond it." A stored grant that
names no holder is moved only by a tenant-level administrator, as an
unanchored assignment is modified only by one.
- **Sets** — `directGrantSet` / the new `grantOwnSet`: when an update
re-names the grant's `permission_set`, the set it takes away is judged
exactly as the removal of that grant judges it — the allowlist, the
security catalog and strict containment — with the same reasons,
sentences and details (the refusal's `permissionSet` is the set it is
about), beside the set it re-names the grant to, judged as before.
`directGrantSet`'s docblock says the stored set is judged too.
- Each stored value is named in a refusal only when the caller's own
read returned it at its stored value (ruling of record 6649064874, kept
as written).
- Unchanged: the judgment of the set an edit points a grant at; an edit
that keeps both the holder and the set; an insert and a removal;
tenant-level and organization administrators.
- `content/docs/permissions/delegated-administration.mdx`: the "Subtree
anchoring" row names both holders of a moved grant; the "Allowlist, no
self-escalation" row names the set a re-name takes away.
- `.changeset/delegated-grant-move-judges-both-holders.md`: `patch`,
`fix(plugin-security)!: a delegate can neither capture nor evict a
direct grant beyond its scope`, a rule-level BREAKING paragraph whose
"Who moves" reads "A delegate can no longer move a direct grant away
from a holder outside its subtree, nor re-name a stored grant of a set
outside its allowlist.", and the ADR-0087 marker `not-required
(no-migration-prescription)`, spelled as in
`.changeset/delegated-unreadable-user-outside-subtree.md`.
**Docblocks of record touched — quoted.** One sentence of
`judgesTargetUserThroughCallerRead`'s docblock (`[ADR-0090 D12; ruling
on #26059, Q3]`) gains the new holder; `directGrantSet`'s `[ADR-0131
D4]` docblock gains the stored set; the rest of every docblock is kept
as written.
> `judgesTargetUserThroughCallerRead` — before: "it holds for every
target user the gate judges: the holder a direct-grant write names (an
insert, an update pointing the grant at another user), the holder a
stored grant keeps (a delete, an update that leaves `user_id` as
stored), and the user the explain doors ask about"
>
> After: "it holds for every target user the gate judges: the holder a
direct-grant write names (an insert, an update pointing the grant at
another user), the holder a stored grant keeps (a delete, an update that
leaves `user_id` as stored), the holder an update pointing the grant at
another user takes it from, and the user the explain doors ask about"
> `directGrantSet` — added bullet: "**The set an update re-names the
grant AWAY from** (`stored`) — the stored grant's own set, judged too,
exactly as the removal of that grant judges it ({@link grantOwnSet}).
[ADR-0090 D12] A re-name takes that set away from its holder, so a
delegate can neither capture nor evict a direct grant beyond its scope:
a grant it could not remove it cannot re-name either."
>
> `setNamed` paragraph — before: "the grant's own name only when the
caller's read returned it." After: "the grant's own name — the one a
keep-set update or a removal judges, and the one a re-name takes away —
only when the caller's read returned it."
Untouched and still governing the new branches: the inline `[ADR-0090
D12; ruling on #26059, Q3]` comment ("the refusal is one reason, one
class, one sentence and one set of details for all of them") and
`[ruling of record 6649064874] Named only when the caller wrote or reads
it`.
**Ruling #26059 Q3, the scope-violation class** (ruling of record
6416233738, item 3, verbatim):
> 3. server p2 — item 3, server half: the delegated-admin gate
(`delegated-admin-gate.ts:403–503`, reasons `:1136–1179`) answers its
own code (e.g. `DELEGATION_SCOPE_VIOLATION`, status 403, ADR-0112 ledger
row), distinct from `PERMISSION_DENIED`.
A holder outside the subtree at the end an update takes the grant from
is a target user outside the subtree, and a set an update takes away
outside the allowlist is a set not in the allowlist, so both answer that
class; a stored grant that names no holder, a stored set the catalog
does not hold, and a stored set the caller's scope does not strictly
contain are not the caller's own scope and keep `PERMISSION_DENIED`, as
their removal does.
## Tests
Named by the rule, on both rigs:
- `delegated-admin-gate.test.ts` (fake engine), two new top-level
describes. Holders (8 pins): a move FROM a holder outside TO one inside
is refused for each of the four target states (no user, no membership, a
unit outside, a unit inside the caller cannot read) with one answer;
FROM an unreadable in-subtree holder; TO a holder outside, as before;
the same answer whichever end is outside (class, code, status, message,
details and the REST envelope); CONTROL: a move between two readable
in-subtree holders is admitted at either end; both holders are asked of
the caller's own read under the write's own context; a holderless stored
grant is moved only by a tenant admin; CONTROL: a tenant-level
administrator moves the grant whichever end is outside and asks the read
nothing. Sets (6 pins): a re-name FROM a set outside the allowlist TO an
allowlisted one is refused with the answer the removal of that grant
gets; the removal's other refusals hold for a re-name (a set not
strictly contained, a name the catalog does not hold); a caller that
cannot read the stored set gets the same refusal without its name, as
the removal does; a re-name TO a set outside, as before; CONTROL: a
re-name between two allowlisted sets is admitted; CONTROL: a
tenant-level administrator re-names freely.
- `delegated-admin-own-name-set.posture.test.ts` (real `ObjectQL` + SQL
driver + `SecurityPlugin`), in each of `single`, `group`, `isolated`.
Holders (24 pins per posture): per subject state, on one rig the state
boots once (every pin there is a refusal and re-reads both grants to
show nothing moved) — a move FROM the subject is refused with the one
literal answer, whole (status, code, class, message and details), at the
gate and through the write door, and the grant stays; a move TO the
subject, as before; the same answer whichever end; a delegate that
cannot read the stored grant gets the same refusal without the stored
holder's name (ruling of record 6649064874); CONTROL: an organization
administrator gets the answer it always got at either end (its right
creates and removes a grant, never edits one in place). Then, booting
their own rigs: CONTROL moves between two readable in-subtree holders
land at either end; CONTROL: a tenant-level administrator still moves
the grant FROM and TO a holder outside, through the write door. Sets (5
pins per posture): a re-name FROM a set outside the allowlist is refused
with the removal's answer, at the gate and through the write door, and
the grant keeps its set; a delegate that cannot read the stored grant
gets the same refusal without the set's name, as the removal does; a
re-name TO a set outside, as before; CONTROL: a re-name between two
allowlisted sets lands; CONTROL: a tenant-level administrator re-names
FROM and TO a set outside the allowlist.
No posture pin boots more than two rigs; the heaviest one (eight boots)
had timed out under CI load (`Test timed out in 5000ms`, Test Core 7/8
on `0dcdbb4712`). No assertion was dropped: the cross-state "one answer"
is each state's answer against the one literal answer, and the
organization-administrator CONTROL now runs in all four states. The
file's local wall fell from `tests 37.93s` to `tests 26.32s` with the
set pins added.
**Ablations** (one behaviour each, committed first, `node
scripts/ablation-replace.mjs` inside a trap restore, under the verify
lock, on the final head `3890ac0699`):
- the stored-holder judgment switched off on disk (anchor 1 → 0, blob
`3793552a` → `1ff05ea9`) → `Tests 41 failed | 342 passed (383)`: exactly
this rule's holder pins — 5 on the fake engine (FROM-end, unreadable
holder, identity, both holders read, holderless) and 36 on the posture
rig (FROM-end, identity, unnamed holder × 4 states × 3 postures); every
CONTROL and every TO-end pin stayed green. Restored: blob == HEAD
(`3793552a`), `git diff HEAD` empty.
- the stored-set judgment switched off on disk (anchor 1 → 0, blob
`3793552a` → `1b94cfd1`) → `Tests 9 failed | 374 passed (383)`: exactly
this rule's set pins — 3 on the fake engine (FROM-end, the removal's
other refusals, unnamed set) and 6 on the posture rig (FROM-end, unnamed
set × 3 postures); every CONTROL and every TO-end pin stayed green.
Restored: blob == HEAD (`3793552a`), `git diff HEAD` empty.
Both suites reach the gate by relative imports from `src/` (no `dist/`
on the path), so no rebuild sits between mutation and run.
## Acceptance notes
- **Organization administrator control.** The first-round order asked
that an organization administrator can still re-point the grant.
Measured: an organization administrator's right under ADR-0090 D12 (as
amended 2026-10-17) creates and removes a grant and never edits one in
place, so its update is refused before and after this change (`403
PERMISSION_DENIED`, "'update' is not part of the right"). The pins hold
it unchanged; the tenant-level administrator moves and re-names as
before.
- **Identity across ends.** The answer is identical whichever end is
outside when the caller reads the stored grant. A caller that cannot
read it is not told the stored holder's id or the stored set's name
(ruling of record 6649064874): class, code, status and details still
match the other end or the removal; the sentence uses the unnamed form,
as the keep-holder edit and the removal already do.
- **The set a re-name takes away is judged on all the removal judges,
not the allowlist alone.** "Exactly as the removal judges it" includes
the catalog check (a name the catalog does not hold: only a tenant admin
may change it) and strict containment (a set carrying an adminScope the
caller's own does not strictly contain); both are pinned.
- **Holderless stored grant.** `user_id` is required on
`sys_user_permission_set`, so no write door stores one; the branch fails
closed for a row a system write left so.
- **Concurrent work on `plugin-security`.** #31443 (the
organization-boundary helper in `delegated-admin-gate.ts`) landed on
`main` while this branch was in flight; this branch merges it cleanly.
## Verification
Every reading below is on the final head `3890ac0699` (this branch
merged with `origin/main` `69244406b7`), unless named.
<details><summary>plugin-security test and typecheck</summary>
- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2` (whole package, under the verify lock): `VERDICT
command-exit 0`, `Test Files 250 passed (250)`, `Tests 6025 passed | 45
skipped (6070)`.
- `pnpm --filter @objectstack/plugin-security run typecheck`: `VERDICT
command-exit 0`; `check:test-typecheck: OK — … 0 file(s) / 0 error(s)`.
- The two pinned files: `delegated-admin-gate.test.ts` `Tests 190 passed
(190)`; `delegated-admin-own-name-set.posture.test.ts` `Tests 193 passed
(193)` (`tests 30.33s` on `3890ac0699`, `tests 26.32s` on `38577d9ec8`;
it was `tests 37.93s` before the split).
</details>
<details><summary>Derived gate families (dispatch-gates --commands,
reconciled with --ran)</summary>
- 97 derived on `3890ac0699`, after a full `pnpm build` ("Tasks: 119
successful, 119 total"); all 97 run, each exit code captured before any
pipe: 97 × exit 0. `check:type-check-debt` first refused (exit 3: its
closure build did not get a turn on the verify lock) and was re-run
under the lock: "128/131 workspace packages type-checked",
"--re-measure: OK — … 0 raw tsc error(s) total".
- Changeset gates, all exit 0: `check-adr-0087-registration --base
origin/main` (+ `--self-test`), `check-changeset-no-major --base
origin/main`, `check-empty-changeset --base origin/main` (+
`--self-test`), `check:changeset-gate-self-tests`.
- `dispatch-gates --ran`: "97 derived famil(ies) accounted for — 97 run,
0 NOT-MEASURED (a DERIVED zero — all 97 recorded an exit code and none
of them is 3)", 0 UNRUN.
</details>
<details><summary>Dogfood battery (the narrowing arm's kit), on the
final head</summary>
- Full `pnpm build`, then `pnpm --filter @objectstack/dogfood test`
under the verify lock, no shard and no path, head `3890ac0699`: `VERDICT
command-exit 1`, `Test Files 1 failed | 439 passed | 3 skipped (443)`,
`Tests 1 failed | 3640 passed | 41 skipped (3682)`. The one failure:
`test/package-commit-revert-path-package.dogfood.test.ts` › "an
uninstalled package's commit is refused 404 through its own revert and
rollback path, and writes nothing" — `Test timed out in 5000ms` (7127
ms), on a box running this union and other seats' builds beside it. Not
this change's: the file exercises the package commit revert path
(ADR-0067), which no line here reaches; it passed in the full battery on
`38577d9ec8` (`Test Files 440 passed | 3 skipped (443)`, `Tests 3641
passed | 41 skipped (3682)`), whose runtime source is byte-identical to
`3890ac0699` (they differ in one plugin-security test file); and the
same file re-run alone on `3890ac0699` under the lock passes, `Tests 12
passed (12)`. CI's `Dogfood Regression Gate` on `3890ac0699` was
satisfied by its path filter — no dogfood shard ran on this PR
("expected attestations: 0") — so the battery above is the only dogfood
run on this head.
</details>
<details><summary>CI on the final head</summary>
- `3890ac0699`: 50 check-runs, 38 success, 12 skipped, none other —
`Lint & Repo Gates`, `TypeScript Type Check`, `Test Core` (8/8 shards),
`Build Core`, `Governed Surface Queue Guard`, `Test UI`, `Live E2E UI`,
`Dogfood Regression Gate` (filter-satisfied; no shard ran), `Check
Changeset` all success; the UI lint, type-check and build lanes skipped
by their path filters.
</details>
<!-- signed: security#1 · sunhaolin · session_017xRBcT1w3zpekijraHJE4f
-->
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent e81636c commit 18cdcb2
5 files changed
Lines changed: 688 additions & 12 deletions
File tree
- .changeset
- content/docs/permissions
- packages/plugins/plugin-security/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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
72 | 72 | | |
73 | 73 | | |
74 | 74 | | |
75 | | - | |
76 | | - | |
| 75 | + | |
| 76 | + | |
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
2138 | 2138 | | |
2139 | 2139 | | |
2140 | 2140 | | |
| 2141 | + | |
| 2142 | + | |
| 2143 | + | |
| 2144 | + | |
| 2145 | + | |
| 2146 | + | |
| 2147 | + | |
| 2148 | + | |
| 2149 | + | |
| 2150 | + | |
| 2151 | + | |
| 2152 | + | |
| 2153 | + | |
| 2154 | + | |
| 2155 | + | |
| 2156 | + | |
| 2157 | + | |
| 2158 | + | |
| 2159 | + | |
| 2160 | + | |
| 2161 | + | |
| 2162 | + | |
| 2163 | + | |
| 2164 | + | |
| 2165 | + | |
| 2166 | + | |
| 2167 | + | |
| 2168 | + | |
| 2169 | + | |
| 2170 | + | |
| 2171 | + | |
| 2172 | + | |
| 2173 | + | |
| 2174 | + | |
| 2175 | + | |
| 2176 | + | |
| 2177 | + | |
| 2178 | + | |
| 2179 | + | |
| 2180 | + | |
| 2181 | + | |
| 2182 | + | |
| 2183 | + | |
| 2184 | + | |
| 2185 | + | |
| 2186 | + | |
| 2187 | + | |
| 2188 | + | |
| 2189 | + | |
| 2190 | + | |
| 2191 | + | |
| 2192 | + | |
| 2193 | + | |
| 2194 | + | |
| 2195 | + | |
| 2196 | + | |
| 2197 | + | |
| 2198 | + | |
| 2199 | + | |
| 2200 | + | |
| 2201 | + | |
| 2202 | + | |
| 2203 | + | |
| 2204 | + | |
| 2205 | + | |
| 2206 | + | |
| 2207 | + | |
| 2208 | + | |
| 2209 | + | |
| 2210 | + | |
| 2211 | + | |
| 2212 | + | |
| 2213 | + | |
| 2214 | + | |
| 2215 | + | |
| 2216 | + | |
| 2217 | + | |
| 2218 | + | |
| 2219 | + | |
| 2220 | + | |
| 2221 | + | |
| 2222 | + | |
| 2223 | + | |
| 2224 | + | |
| 2225 | + | |
| 2226 | + | |
| 2227 | + | |
| 2228 | + | |
| 2229 | + | |
| 2230 | + | |
| 2231 | + | |
| 2232 | + | |
| 2233 | + | |
| 2234 | + | |
| 2235 | + | |
| 2236 | + | |
| 2237 | + | |
| 2238 | + | |
| 2239 | + | |
| 2240 | + | |
| 2241 | + | |
| 2242 | + | |
| 2243 | + | |
| 2244 | + | |
| 2245 | + | |
| 2246 | + | |
| 2247 | + | |
| 2248 | + | |
| 2249 | + | |
| 2250 | + | |
| 2251 | + | |
| 2252 | + | |
| 2253 | + | |
| 2254 | + | |
| 2255 | + | |
| 2256 | + | |
| 2257 | + | |
| 2258 | + | |
| 2259 | + | |
| 2260 | + | |
| 2261 | + | |
| 2262 | + | |
| 2263 | + | |
| 2264 | + | |
| 2265 | + | |
| 2266 | + | |
| 2267 | + | |
| 2268 | + | |
| 2269 | + | |
| 2270 | + | |
| 2271 | + | |
| 2272 | + | |
| 2273 | + | |
| 2274 | + | |
| 2275 | + | |
| 2276 | + | |
| 2277 | + | |
| 2278 | + | |
| 2279 | + | |
| 2280 | + | |
| 2281 | + | |
| 2282 | + | |
| 2283 | + | |
| 2284 | + | |
| 2285 | + | |
| 2286 | + | |
| 2287 | + | |
| 2288 | + | |
| 2289 | + | |
| 2290 | + | |
| 2291 | + | |
| 2292 | + | |
| 2293 | + | |
| 2294 | + | |
| 2295 | + | |
| 2296 | + | |
| 2297 | + | |
| 2298 | + | |
| 2299 | + | |
| 2300 | + | |
| 2301 | + | |
| 2302 | + | |
| 2303 | + | |
| 2304 | + | |
| 2305 | + | |
| 2306 | + | |
| 2307 | + | |
| 2308 | + | |
| 2309 | + | |
| 2310 | + | |
| 2311 | + | |
| 2312 | + | |
| 2313 | + | |
| 2314 | + | |
| 2315 | + | |
| 2316 | + | |
| 2317 | + | |
| 2318 | + | |
| 2319 | + | |
| 2320 | + | |
| 2321 | + | |
| 2322 | + | |
| 2323 | + | |
| 2324 | + | |
| 2325 | + | |
| 2326 | + | |
| 2327 | + | |
| 2328 | + | |
| 2329 | + | |
| 2330 | + | |
| 2331 | + | |
| 2332 | + | |
| 2333 | + | |
| 2334 | + | |
| 2335 | + | |
| 2336 | + | |
| 2337 | + | |
| 2338 | + | |
| 2339 | + | |
| 2340 | + | |
| 2341 | + | |
| 2342 | + | |
| 2343 | + | |
| 2344 | + | |
| 2345 | + | |
| 2346 | + | |
| 2347 | + | |
| 2348 | + | |
| 2349 | + | |
0 commit comments