Repository navigation
Commit 234d1d8
Fixes #21669
Clause-②: no
## What was wrong
`bootstrapDeclaredPermissions` (plugin-security) walks every
`permission` item in the engine SchemaRegistry. That registry also holds
the permission sets an environment authored itself: `loadMetaFromDb`
hydrates every env-wide `sys_metadata` `permission` row into the same
collection. A set made with Setup's **Clone** action is one of them, and
it has no package id there, because it has none. The loop judged "no
owning package" before it looked at the set's row, so every boot, and
every `metadata:reloaded`, logged `[permission_set_declaration_unowned]
declared permission set "…" has no owning package — not materialized …
the Setup admin surface reads sys_permission_set and cannot see this
set` for a set whose `managed_by: admin` row Setup lists and edits.
## Reproduced first, on the real showcase composition
Boot 1 of `bootStack(showcaseStack, { databaseFile })`: the admin signs
in and clones `showcase_contributor` through `POST
/api/v1/data/sys_permission_set`, with the shipped
`clone_permission_set` action's payload. Then a cold boot on the same
file. Measured on `a4fd82a8e6`, with the plugin-security `dist` built
from that tree:
| reading | value |
|:--|:--|
| clone POST | `201` |
| clone row | `managed_by: admin`, `package_id: null`, `organization_id:
null` |
| clone `sys_metadata` row | one row, `state: active`, `organization_id:
null`, `package_id: null` |
| boot 2, registry item for the clone | present; no `_packageId`, no
`packageId`; `_provenance: 'org'` (stamped at hydration) |
| boot 2, the other 17 registry items | every one carries `_packageId`
(`com.example.showcase` or `com.objectstack.plugin-security`) and
`_provenance: 'package'` |
| boot 2, unowned lines | **1**, naming the clone |
| boot 2, seeding summary | `skippedUnowned: 1`, `skippedEnvAuthored:
8`, `total: 18` (boot 1: `skippedUnowned: 0`, `total: 17`) |
The line that reported it is `upsertPackagePermissionSet`'s `if
(!packageId)` branch (`reportPermissionSetDeclarationUnowned`), reached
from the boot loop with `packageId = ps._packageId ?? ps.packageId`,
which is undefined for the clone. The report became visible with
`cf39b83c09`; `git log -S` on the call finds that commit alone.
The same steps with this branch's `dist`: **0** unowned lines, and the
seeding summary reads `skippedUnowned: 0`, `skippedEnvAuthored: 9`.
## The fix
Before the loop judges ownership, it checks whether the environment
already owns a row under the name. A registry item with no package id,
whose `sys_permission_set` row is environment-owned, is the
environment's own set. It lands in `skippedEnvAuthored`, where a package
declaration over an environment row already lands, and nothing is
logged.
**Which signal, and why.** The signal is the row's `managed_by`, read
through the existing env-authored verdict. That is the one definition of
environment ownership this seeder already had: the last branch of
`upsertPackagePermissionSet`, where a row that is not `managed_by:
'package'` is environment-authored and never clobbered (ADR-0086
two-doors). It is now named `isEnvironmentOwnedRow`, and both that
branch and the new check call it, so there is still exactly one
spelling. Two alternatives were measured and not taken:
- **The registry item's `_provenance: 'org'`.** It is on the clone's
SchemaRegistry item. But the metadata service's copy of the same clone
carries no `_provenance`; it carries the projection's `_envProjection`
marker instead. A check keyed on it would depend on which registry copy
the loop reads, and that question is still open on #14491. Nothing in
plugin-security reads `_provenance` today, so it would also be a second
definition.
- **The lock's `declaredPackageIdOf`.** It answers "is this an
artifact?", not "who owns this row?". It would leave the same "unowned"
verdict for the clone.
The check reads from the batched existence read the loop already makes,
so no query is added. On a per-organization pass, it also counts the
organization-less row the environment door writes.
`permission-set-projection.ts` is deliberately not per-organization, so
a clone's row has no `organization_id` there. If the read cannot answer,
that is not proof the environment owns the name, so the unowned refusal
stands, exactly as before. The publish-time materializer (ADR-0086 P2)
is unchanged.
**The warning stays honest.** A declaration with no owning package and
no environment row still logs `permission_set_declaration_unowned`, with
the same text, and still counts as `skippedUnowned`.
`seed-refusal-diagnostics.ts` is untouched.
## #14491 (the third registry copy) was read first
It **shares the registry walk**, and the fix does **not** depend on
which copy the loop reads.
- `readDeclared(ql, 'permission')` reads the one SchemaRegistry
`permission` collection. On boot 2 that collection held 17 items from
`manifest.register` (the unparsed third copy #14491 describes, each with
`_packageId`), plus the clone, which `loadMetaFromDb` hydrated from
`sys_metadata`. So #14491's copy feeds this loop, but the clone does not
come from that copy.
- The fix uses only the item's name and the absence of a package id. It
decides ownership from the `sys_permission_set` row. The metadata
service's copy of the clone also has no package id (measured above), so
the decision would be the same if the loop read that copy instead.
- No `packages/objectql` or `packages/metadata*` file is touched.
## Pins
- `bootstrap-declared-permissions.test.ts`, under `[#21669]`, 7 cases:
- a cloned set (the measured item shape over a `managed_by: admin` row)
logs nothing on any of the five console channels, counts
`skippedEnvAuthored: 1`, and its row is untouched;
- the same on a per-organization pass;
- a truly unowned declaration (no package id, no row) still warns, with
the token and the first sentence `declared permission set "crm_orphan"
has no owning package — not materialized.`; the same on a
per-organization pass;
- controls: an unowned item over a `managed_by: package` row still
warns; an unreadable read still warns;
- conservation with a clone in the walk.
- A boot-level pin,
`packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts`.
The unit seam cannot show that the real boot puts the clone in the walk,
so this pin boots the showcase twice:
- It clones through the shipped action's own declaration: its `target`,
its `bodyExtra`, its typed `label`/`name`, and every `defaultFromRow`
facet.
- It asserts the precondition that boot 2's walk holds the clone with no
package id.
- Control: the capture saw the seeding summary line.
- It asserts no `permission_set_declaration_unowned` line names the
clone.
## Ablations (one-time, nothing left in the tree)
Each leg ran through `scripts/ablation-replace.mjs` with an anchor that
must hit, on top of the committed fix (`067aa4f58f`). Each leg rebuilt
`@objectstack/plugin-security`, and
`scripts/ablation-dist-preflight.mjs` confirmed the marker had reached
`dist/` before any result was read. The direction was predicted before
each run.
| leg | mutation | predicted | observed |
|:--|:--|:--|:--|
| A: guard removed | `environmentOwnsName` returns false | only the
clone rows go red | unit: 3 red, 27 green: the clone (single), the clone
(per-organization), and conservation-with-clone. Boot pin: 1 red, 2
green; the absence case failed with one unowned line. |
| B: guard admits all | `environmentOwnsName` returns true | every input
with no package id and no environment row goes red, i.e. the new
truly-unowned rows, the package-row and unreadable controls, and the
pre-existing #18091 / #18571 unowned pins of the same shape; the clone
rows stay green | unit: 12 red (5 new, 7 pre-existing, all of that
shape), 18 green, both clone rows among them. Boot pin: 3 of 3 green. |
Restore, both legs: `ablation-replace` reported that the file's blob
equals HEAD (`56a5addec78e`) and that `git diff HEAD` is empty. A
rebuild then followed, and `ablation-dist-preflight --absent` reported
both markers absent from all 6 built files and the tree clean.
## Verification, on HEAD `067aa4f58f`
- `pnpm --filter @objectstack/plugin-security test`: `Test Files 164
passed (164)`, `Tests 3534 passed | 45 skipped (3579)`.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0.
`check:test-typecheck` OK, and `tsc -p tsconfig.test.json --listFiles`
includes the edited test file.
- `pnpm --filter @objectstack/dogfood typecheck`: exit 0, and the new
pin is in the program.
- The boot pin: `Tests 3 passed (3)`.
- Gates: `node scripts/pm/dispatch-gates.mjs --repo
objectstack-ai/objectstack --commands` derived 69 commands; all 69
exited 0. Reconciled with `--ran`: `69 run, 0 NOT-MEASURED (a DERIVED
zero — all 69 recorded an exit code and none of them is 3)`.
- `check:dual-build-cjs-loads` first exited 3 (`PREREQUISITE NOT MET`: 8
unrelated packages had no `dist/`). After those were built it exited 0.
- Two families take values only CI supplies and were not run locally:
`check-shard-attestation` (dogfood shard matrix) and
`check-issue-citations --census`.
- Lint: narrowed to the 3 touched TS files, `eslint --no-inline-config
--format json`: 3 file results, 0 errors, 0 warnings.
`eslint.config.mjs` uses no `parserOptions.project` (no type-aware
rules), so this diff cannot move the verdict on any untouched file. The
repo-wide lint is CI's.
- Docs: `content/docs/**` (outside `releases/`) and `skills/**` have no
mention of `permission_set_declaration_unowned` or "no owning package",
so no sentence was made false.
## Acceptance notes
- **Walled posture, at boot level: NOT MEASURED.** The per-organization
case is pinned at the unit seam only. A walled boot here needs the
enterprise organizations package, which no framework package declares.
The reasoning is the environment door's documented organization-less
shape: `permission` is `allowOrgOverride: false`, so the clone's
`sys_metadata` row lands env-wide and is hydrated in every posture.
- **Not measured, noted only: the sibling capability seeder.**
`bootstrap-declared-capabilities.ts` has the same order: "unowned" is
judged before the row is read (`capability_declaration_unowned`).
Whether an environment-authored capability ever reaches that registry
walk without a package id was not measured. #15204 (ADR-0131 C3) retires
that seeder and will touch the file.
- **Not measured, noted only: metadata-door package sets.** A permission
set published through the metadata door with a package binding
(ADR-0070, materialized by the ADR-0086 P2 path) is hydrated from
`sys_metadata`. The write door strips derived provenance from it, so its
registry item would also have no `_packageId`, while its row is
`managed_by: package`. This fix deliberately does not treat such a row
as environment-owned, so that set would still be reported as unowned.
That population was not measured.
- #14491 is not addressed here; that card remains open on its own
surviving question.
---
_Generated by [Claude
Code](https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ)_
---------
Co-authored-by: Claude <noreply@anthropic.com>
1 parent 8d4fc63 commit 234d1d8
4 files changed
Lines changed: 392 additions & 3 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
500 | 500 | | |
501 | 501 | | |
502 | 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
182 | 182 | | |
183 | 183 | | |
184 | 184 | | |
| 185 | + | |
| 186 | + | |
| 187 | + | |
| 188 | + | |
| 189 | + | |
| 190 | + | |
| 191 | + | |
| 192 | + | |
| 193 | + | |
| 194 | + | |
| 195 | + | |
| 196 | + | |
| 197 | + | |
| 198 | + | |
| 199 | + | |
| 200 | + | |
| 201 | + | |
| 202 | + | |
| 203 | + | |
| 204 | + | |
| 205 | + | |
| 206 | + | |
| 207 | + | |
| 208 | + | |
| 209 | + | |
| 210 | + | |
| 211 | + | |
| 212 | + | |
| 213 | + | |
| 214 | + | |
| 215 | + | |
| 216 | + | |
| 217 | + | |
| 218 | + | |
| 219 | + | |
| 220 | + | |
| 221 | + | |
| 222 | + | |
| 223 | + | |
| 224 | + | |
| 225 | + | |
| 226 | + | |
185 | 227 | | |
186 | 228 | | |
187 | 229 | | |
| |||
297 | 339 | | |
298 | 340 | | |
299 | 341 | | |
300 | | - | |
| 342 | + | |
301 | 343 | | |
302 | 344 | | |
303 | 345 | | |
| |||
359 | 401 | | |
360 | 402 | | |
361 | 403 | | |
362 | | - | |
363 | | - | |
| 404 | + | |
| 405 | + | |
| 406 | + | |
364 | 407 | | |
365 | 408 | | |
366 | 409 | | |
| |||
417 | 460 | | |
418 | 461 | | |
419 | 462 | | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
420 | 477 | | |
421 | 478 | | |
422 | 479 | | |
| |||
0 commit comments