Repository navigation
Commit 710c2da
harden(metadata): a permission set's name is unique across packages (#27535)
domain:devx#1 · sunhaolin · session_017xRBcVy4VXmkQjn2MGeaWk
harden(metadata): a permission set's name is unique across packages
Dispatched by the W3-C3b QA run security path (detail withheld).
Reworked on the contract-tier review FAIL (PR comment 6497768595): the
must-fix and all five should-fixes are taken, each listed under What
changes.
Clause-②: no (narrowing)
Metadata-protocol: none
Skills-docs: none — the published skills teach that a grant references a
permission set by its machine `name`
(`skills/objectstack-data/rules/security.md`); none claims a name may be
reused across owners, so this hardening contradicts nothing, and
`Metadata-protocol: none` means there is no new authorable capability to
teach.
This narrows a published accept set (a metadata-door write that answered
`200` is now refused), so it owes one contract-review-tier review before
the queue.
## 维护者速读
- 改了什么:权限集(permission set)在元数据的保存门(草稿/直接发布)、"草稿→发布"提升门、以及恢复门(回滚 rollback
/ 提交回退
revert)上,若同名权限集已被另一个所有者(另一个包,或环境本身)持有,一律拒绝写入,且什么都不落库;错误信封与注册表在安装、冷启动时对同一规则的答复一致:`NAMESPACE_CONFLICT`
/ 422。
- 为什么改:授权按名字引用权限集(一个名字只能对应一套权限,ADR-0131 D4;裁决 #15196 Q4=A、#22307
A:"一个名字一个持有者,每条路径都如此")。安装与冷启动早已拒绝第二个持有者,元数据入口没有,留下一个与已声明契约不符的缺口。本次返工修正了复审指出的问题:所属包自己用"不带包名的草稿保存"编辑自家权限集时被误拒。属安全加固。
-
风险与回滚:只拦"用别人已占用的名字再写一份"这类本就非法的写入;同一所有者改自己的集照常(包括不带包名的草稿保存、以及历史遗留的"包行旁的无包行");已有两个所有者同名的部署,在改名或删除其中一份之前,对该名字的写入(含
`os migrate meta --stored --apply` 重存、包复制 duplicate 中未带前缀的集)会报失败。回滚 =
还原本 PR。
- 先看:`packages/metadata-protocol/src/protocol-save-door-refusals.ts`(判据
`permissionSetNameCollisionRefusal` 与注册表读法
`registryPackageHoldersOf`);`packages/metadata-protocol/src/protocol.ts`
保存门里计算 `landsIn`
的那段(按写入真正落入的包判定,风险最高);`packages/metadata-protocol/src/protocol-authoring-gate.ts`
恢复门的一行调用。
- 两分钟验证:`pnpm --filter @objectstack/metadata-protocol exec vitest run
src/protocol.permission-set-name-cross-package.test.ts` → 预期 17/17
绿(三道门的拒绝 + 各自对照,含"所属包不带包名的草稿编辑可保存"一条)。
- 需要你做的:无 — 席位落地;入队前需一次契约复审档(contract-review)复核,因为本卡 `Clause-②: no
(narrowing)`。
## What changes
A permission set's name is its identity across the whole deployment: a
grant (`sys_user_permission_set.permission_set`) resolves its set by
that name alone (ADR-0131 D4), and positions, permission sets and
capabilities each hold one name per deployment (ruling Q4 = A on #15196,
record 6050490870). The registry already refuses a second holder at
install and at cold boot with `NAMESPACE_CONFLICT` / 422 (ruling A on
#22307, record 6063176077: one name, one holder, on every path). The
metadata doors keyed a permission-set write by `(type, name, package)`
and so let a second owner write a name already held. This PR closes
those doors, pulling them back to the already-declared contract
(ADR-0086 D4: a package never writes into a record another owns).
`permissionSetNameCollisionRefusal` refuses a `permission` write whose
name a DIFFERENT owner already holds, nothing written. It is asked at
every door a body reaches `active` through:
- **the save door** (`saveMetaItem`, draft and publish mode), right
after the sibling owner judge;
- **the draft→active promotion** (`promoteDraftForPublish`, shared by
the per-item publish and `publishPackageDrafts`), on `promoteInto` — a
batch publish aborts on it;
- **the restore doors** (review should-fix 2): first in
`refuseRestoredBodyTheSaveDoorRefuses`, under `rollbackMetaItem` and
`revertCommit`, on the binding the restore lands in (the active row's
`package_id`, which `put` keeps; `null` when no active row exists and
the package-unbound row is written). A version of a deleted set cannot
come back as a second holder of a name a package has taken since.
The save-door check runs before the create precondition, so
`If-None-Match: *` cannot smuggle a cross-owner create.
**The owner is the binding the write LANDS IN (review must-fix 1).** The
first cut judged the owner judge's binding (`saveWriteBinding`), which
reads the store only when the body carries a `packageId`, so the owning
package's own plain `?mode=draft` edit (body without `packageId`, no
package named) was judged as the environment and refused, although the
repository writes that draft into the inherited package
(`resolveWriteHead`). The save door now hands the predicate: the named
package (for a publish-mode save naming none, the package the
package-less address resolved, already on the request); for a draft save
naming none, the `into` of the draft address `packagelessDraftAddress`
names — the address the door already reads, read here too for the
server-stated rewrites it is not read for — and an ambiguous address is
refused with the refusal the repository answers it with
(`RESOURCE_CONFLICT` / 409, "name the package"); otherwise the
package-unbound row (`null`).
**The holders: two readings, met.** The stored active rows by
`package_id` (a set published through the metadata door is stored and
registered in the registry's bare slot without a package, so only the
row says whose it is), and the registry's OWN package-held reading for a
set a package ships in code or declares in its install claim (review
should-fix 5; ruling letter B on #22371, record 6074838935: one reading
of who holds a name). `@objectstack/objectql` exports that reading as
`findPackageHeldSecurityCatalogNames(registry)`; metadata-protocol
cannot import objectql (objectql depends on it), so the predicate asks
the same registry member the export asks, as it already asks the
registry's `defaultRecordPageHoldersOf` for the default-record-page
rule. A new objectql suite drives the doors over the real `ObjectQL` +
`SchemaRegistry` composition, so a rename of that member reds there. The
first cut's `getItem` reading (one item, no install claims, a bare-slot
stamp read as a package) is gone.
**A package-less row beside exactly one package's row is that package's
(review should-fix 6).** #23165's rule for the write address, applied to
ownership: such a row is the residue of the direct save before it was
re-addressed, so it is not a second owner. The owning package's edit and
a write landing in the residue row are accepted; another package is
still refused, and its refusal names the package alone.
**Envelope (review should-fix 4).** `NAMESPACE_CONFLICT` / 422, worded
as the registry's `SecurityCatalogNameConflictError` ("Security catalog
name conflict: … is already held by …", the same rationale sentence),
prescription first because a 4xx message is truncated at 500 characters
at the REST boundary. The rulings fix this rule's envelope at that code
and status on every path, so a client reads one code for the rule
whichever door it met. The code is `@objectstack/objectql`'s;
`@objectstack/spec`'s `ERROR_CODE_LEDGER` now lists it under
`@objectstack/metadata-protocol` too (the `DEFAULT_RECORD_PAGE_CONFLICT`
precedent); the `ErrorCode` union is unchanged and no code is minted.
**Faces declared (review should-fix 3).** The changeset's "Who sees it"
now names `POST /api/v1/packages/:id/duplicate` (a copied set whose name
lacks the source namespace prefix is reported in `failed[]`; remedy:
prefix it with your own module prefix) and `os migrate meta --stored
--apply` (a re-saved row of a name two owners hold is reported failed
until one set is renamed or deleted).
A `patch` changeset ships it for `@objectstack/metadata-protocol` and
`@objectstack/spec`.
## Tests
-
`packages/metadata-protocol/src/protocol.permission-set-name-cross-package.test.ts`
— 17 cases on the storing engine double (doubles recorded in
`scripts/engine-double-contract.pinned.json`), each refusal paired with
a control through the same door: the save door (both modes, the
environment author, the create precondition, an install-claimed name);
the owner as the binding the write lands in (the must-fix pin: owner B
holds the active set, body without `packageId`, `mode: 'draft'`, no
package on the request → `success: true`, draft bound to B; the
publish-mode control; the ambiguous draft address); the package-less
residue; the promotion (per-item and batch, `failed[].code` =
`NAMESPACE_CONFLICT`); the restore door through `rollbackMetaItem` and
its control. Shapes a deployment can only hold from before this
hardening are staged through the real write path with the predicate
switched off for the staging writes alone.
-
`packages/objectql/src/protocol-permission-set-name-registry-reading.test.ts`
— 4 cases over the real composition (`ObjectQLPlugin` binding the real
protocol to a real `ObjectQL` and its `SchemaRegistry`, an in-memory
driver): the exported reading names the claimant; another installed
package and the environment are refused `NAMESPACE_CONFLICT` / 422
naming it, nothing stored; control.
<details><summary>Touched packages — typecheck and full suites
(verify-lock, exit codes captured before any pipe)</summary>
```
pnpm --filter @objectstack/metadata-protocol typecheck && build -> VERDICT command-exit 0
pnpm --filter @objectstack/metadata-protocol test -> Test Files 280 passed | 4 skipped (284) · Tests 21798 passed | 26 skipped (21824) · VERDICT command-exit 0
pnpm --filter @objectstack/objectql typecheck (incl. check:test-typecheck) && test (project local)
-> Test Files 487 passed (487) · Tests 9132 passed (9132) · VERDICT command-exit 0
pnpm --filter @objectstack/spec typecheck && test -> Test Files 752 passed (752) · Tests 22343 passed | 1 todo (22344) · VERDICT command-exit 0
new files alone: metadata-protocol 17/17 · objectql 4/4
```
`tsc --listFilesOnly` counts the metadata-protocol test in that
package's program (1) and the objectql test in `tsconfig.test.json`'s
(1), which `check:test-typecheck` compiles.
</details>
<details><summary>Ablation of must-fix 1 (the draft-address owner branch
disabled) — src-resolved, no rebuild</summary>
The subject resolves through the package's own `src` (the test imports
`./protocol.js`), so the mutation is seen without a rebuild. The fix was
committed first; `scripts/ablation-replace.mjs` under a script with
`trap` restore against `HEAD`.
```
before: anchor=1 marker=0
ablation-replace: anchor x1 -> x0 · replace x0 -> x1 · blob 00b7328 -> 9092f29eaeb8 (mutation landed)
during: anchor=0 marker=1
× a draft save naming no package, body without packageId, over package B's active set is B's own edit
Error: Security catalog name conflict: this environment cannot save the permission set "project_editor"
— it is already held by package "app.owner_b". … (the reviewed defect, reproduced)
× a draft save naming no package over a name two packages already hold … (422 instead of the repository's 409)
Test Files 1 failed (1) · Tests 2 failed | 15 passed (17)
ablation-replace: ok restored: blob == HEAD (00b7328) and `git diff HEAD` is empty
trap: blob 00b7328… == HEAD, git diff HEAD empty
```
Direction observed: red (the expected one); the other 15 cases, controls
included, stay green.
</details>
## Acceptance notes
- `Metadata-protocol: none`: no authorable key, value or type changes; a
cross-owner second set was never an authorable capability (the install
and cold-boot doors and the Studio dialog already refuse it). The
accept-set narrowing is at the metadata HTTP doors, which is why
`Clause-②` carries the `(narrowing)` arm while the kind stays `none` and
the level `patch`. Re-checked after the envelope change: relative to
`main` the doors still go from accepting to refusing, and the ledger row
adds no member to the `ErrorCode` union, so the declaration is
unchanged.
- The registry seam is structural by necessity (the dependency runs
objectql → metadata-protocol). A host registry without the reading
member answers no package-held name, as a registry without `getItem` did
before; on the one production host the objectql suite above holds it.
- `security-catalog-namespace.ts` (objectql) states the registry does
not judge "an environment-catalog save over a package-held name — that
path keeps its own answers". Still true: the metadata doors now give
that answer, with the registry's envelope. Its header could point at the
metadata-protocol predicate; not touched here (no behaviour, outside
this diff's files).
- `os migrate security-catalog-overlays` and the cold boot read the bare
slot as the environment's; this door reads stored rows by binding. The
two meet on the package half (one reading) and differ only on the stored
half, which the cold boot cannot see by construction.
## Verification
Local gate subset on the diff (`node scripts/pm/dispatch-gates.mjs
--repo objectstack-ai/objectstack --commands`, no paths; change set from
the merge base, 8 paths): 93 derived, every one run with its exit code
captured before any pipe; `--ran` reconciliation: 93 accounted — 92 run,
1 NOT-MEASURED (`check:dual-build-cjs-loads`, exit 3, PREREQUISITE NOT
MET: it reads the built output of 72 packages outside this diff's
closure; Build Core answers it in CI). `check:lean-entry-closure` first
exited 3 for the same reason; after `pnpm --filter @objectstack/objectql
build` it measured `exit 0` (2 published conditions from a real load,
admitted set held exactly). Every family CI runs is read from the PR's
check-runs, not re-run here; the repo-wide `pnpm lint` and the
full-build gates are CI's. On head `585e6198` CI reads 49 check-runs,
all completed: 39 success, 10 skipped (path-filtered or opt-in), 0
failure — `Lint & Repo Gates`, `TypeScript Type Check`, `Test Core` (8/8
shards), `Build Core`, `Governed Surface Queue Guard`, `Check
Changeset`, `Dogfood Regression Gate`, `Type Check UI`, `Test UI`,
`Build & E2E UI` and `Live E2E UI` success; `Lint UI` skipped.
<details><summary>Key gate verdict lines</summary>
```
check-adr-0087-registration --base origin/main -> exit 0 · 1 declared-breaking changeset, ADR-0087 exemption (no-migration-prescription)
check-changeset-no-major --base origin/main -> exit 0 · no major bump
check-empty-changeset --base origin/main -> exit 0 · 1 declaring changeset added; none modified or deleted
check:engine-double-contract -> exit 0 · OK — 1123 pinned, 120 in the DEBT ledger, 3 exempt
check:dispatcher-error-vocabulary -> exit 0 · OK — 56 unregistered code-stamping sites, all classified
check:error-status-conformance -> exit 0 · every derivable runtime status is documented, and every documented status is reachable
check:error-code-casing · check:doc-authoring · check:issue-citations · check:nul-bytes -> exit 0
check:test-source-alias · check:cross-package-test-inputs · check:durability-log-level -> exit 0
spec: check:api-surface (unchanged) · check:docs (232 generated files in sync) · check:authorable-surface · 14 more spec families -> exit 0
local-only: check:skill-frame-freshness -> exit 0 · check:dev-prereqs -> exit 1 (this worktree is built only for the diff's closure: 35 of 105 packages have no dist; a local-state reading, not a finding)
```
</details>
<details><summary>Dogfood battery (Clause-② carries the narrowing arm) —
final head</summary>
```
head 585e619 · closure built first: pnpm --workspace-concurrency=2 --filter "@objectstack/dogfood^..." build -> VERDICT command-exit 0
pnpm --filter @objectstack/dogfood test (no OS_TEST_SHARD, no path) -> VERDICT command-exit 0
Test Files 350 passed | 3 skipped (353) · Tests 2817 passed | 20 skipped (2837) · no failure
```
</details>
---
_Generated by [Claude
Code](https://claude.ai/code/session_017xRBcVy4VXmkQjn2MGeaWk)_
<sub>Body refreshed by the PM seat 2026-10-29T15:22Z from the os-dev
rework report.</sub>
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 85663a5 commit 710c2da
8 files changed
Lines changed: 1098 additions & 1 deletion
File tree
- .changeset
- packages
- metadata-protocol/src
- objectql/src
- spec/src/api
- scripts
| 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 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| 80 | + | |
80 | 81 | | |
81 | 82 | | |
82 | 83 | | |
| |||
112 | 113 | | |
113 | 114 | | |
114 | 115 | | |
115 | | - | |
| 116 | + | |
116 | 117 | | |
117 | 118 | | |
118 | 119 | | |
119 | 120 | | |
120 | 121 | | |
121 | 122 | | |
| 123 | + | |
| 124 | + | |
122 | 125 | | |
123 | 126 | | |
124 | 127 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
269 | 269 | | |
270 | 270 | | |
271 | 271 | | |
| 272 | + | |
| 273 | + | |
| 274 | + | |
| 275 | + | |
| 276 | + | |
| 277 | + | |
| 278 | + | |
| 279 | + | |
| 280 | + | |
| 281 | + | |
| 282 | + | |
| 283 | + | |
| 284 | + | |
| 285 | + | |
| 286 | + | |
| 287 | + | |
| 288 | + | |
| 289 | + | |
| 290 | + | |
| 291 | + | |
| 292 | + | |
| 293 | + | |
| 294 | + | |
| 295 | + | |
| 296 | + | |
| 297 | + | |
| 298 | + | |
| 299 | + | |
| 300 | + | |
| 301 | + | |
| 302 | + | |
| 303 | + | |
| 304 | + | |
| 305 | + | |
| 306 | + | |
| 307 | + | |
| 308 | + | |
| 309 | + | |
| 310 | + | |
| 311 | + | |
| 312 | + | |
| 313 | + | |
| 314 | + | |
| 315 | + | |
| 316 | + | |
| 317 | + | |
| 318 | + | |
| 319 | + | |
| 320 | + | |
| 321 | + | |
| 322 | + | |
| 323 | + | |
| 324 | + | |
| 325 | + | |
| 326 | + | |
| 327 | + | |
| 328 | + | |
| 329 | + | |
| 330 | + | |
| 331 | + | |
| 332 | + | |
| 333 | + | |
| 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 | + | |
272 | 427 | | |
273 | 428 | | |
274 | 429 | | |
| |||
0 commit comments