Repository navigation
Commit 5e0b489
fix(plugin-security): Discard Overlay refuses every permission set no code package ships, so a runtime-package set's only stored row is no longer deleted (#21873)
Fixes #21860
Clause-②: no
## What this changes
The Discard Overlay action (`POST
/api/v1/security/permission-sets/:id/discard-overlay`) is declared to
refuse any permission set that is not package-declared, so that it can
never destroy a set the environment authored (its docblock, and the
permission-sets docs page). It decided "package-declared" by asking
whether any engine-registry item of the set's name carried a package id
(`_packageId ?? packageId`). The registry holds stored rows as well as
artifacts, and the metadata list read (`GET /api/v1/meta/permission`,
issued by every Studio page load) stamps a stored row's `package_id`
onto its body as `_packageId`. So a set saved into a writable runtime
package passed the check after the first list read: the action answered
`200` and deleted the set's only `sys_metadata` row. The drift
diagnostics (`computePermissionSetDriftDiagnostics`) read the package id
the same way, and their `overlay_shadow` detail names Discard Overlay as
the remedy.
Two edits, both in `packages/plugins/plugin-security/src`:
1. `permission-set-overlay-discard.ts`: eligibility is
`classifyPackagedPermissionSet`'s verdict for the set's name. That is
the classifier the lock's two write doors and the overlay detection
reading already ask, over the same engine registry. Only `packaged`
proceeds. `org` and `unknown` are both refused with the action's
existing refusal (`PermissionDeniedError`, `403 PERMISSION_DENIED`),
because the safe direction for a destructive action is the reading's: a
set this environment cannot prove a code package ships is never deleted.
The refusal's sentence now says the set is not shipped by any installed
code package (for `unknown`: that this could not be determined, with the
classifier's reason).
2. `permission-set-drift.ts`: the declared population is gated on the
same verdict, per name.
No second evaluator. In both files the per-item test that remains below
the verdict (`_packageId ?? packageId`) no longer decides anything: it
only picks, among the registry items of a name the classifier has
already called code-shipped, which body is used (the degraded-kernel
resync body in discard, the compared bodies in drift), by the same
selector as before. So for every code-shipped set the accepted
population, the bodies and the audited or reported package are what they
were.
This builds on PR #21857 (landed as c9be1f1, merged into this branch):
there the classifier learned to skip a tenant-authored registry item
(`_provenance: 'org'`), so a runtime-package set's stored row reads
`org`. It already excluded the `'sys_metadata'` runtime-shadow sentinel.
Not touched: `packaged-permission-set-lock.ts`,
`permission-set-projection.ts`, `packages/spec`, any error code, route,
export or exported signature.
Docs: the permission-sets page's Discard Overlay sentence now names the
sets the action refuses (a set created in the environment, a clone, a
set saved into a writable runtime package). "Package-declared" on its
own reads as covering a runtime package's set.
## Measurements, at the door (showcase, booted through
`@objectstack/verify`, two boots on one database file)
| shape | old reading (ablation A below, and this branch before #21857
landed) | this PR (62143b0) |
|---|---|---|
| set saved into a writable runtime package, after the list read |
`200`, `overlaysDiscarded: 1`: its only `sys_metadata` row deleted |
`403 PERMISSION_DENIED`, every row intact |
| org-owned set (data door) | `403 PERMISSION_DENIED` (no package id on
its item) | `403 PERMISSION_DENIED`, every row intact |
| clone ("Clone to customize") | `403 PERMISSION_DENIED` (no package id
on its item) | `403 PERMISSION_DENIED`, every row intact |
| control: `showcase_contributor` (shipped by `com.example.showcase`)
with a legacy overlay | `200`, overlay discarded, record healed to the
artifact | unchanged |
The runtime-package set's registry row after the list read is `{
_packageId: 'com.dogfood.discard21860', _provenance: 'org' }` (asserted
as a precondition before any refusal is read). The control's
precondition: after the cold boot, its record enforces the legacy
overlay's grants and the boot's drift pass wrote `drift_status:
'overlay_shadow'`.
## Mechanism hypotheses, measured
- **H1 holds.** Reproduced at both levels before the fix took effect:
the unit pin read "promise resolved instead of rejecting", and the door
answered `200` with `overlaysDiscarded: 1` (the set's only stored
definition deleted).
- **H2 holds, with one refinement.** On `main` at dispatch (`5b2d189e`)
the classifier alone did NOT exclude the runtime-package set: it read
any non-sentinel package id. That is exactly what PR #21857 changed in
the lock (`isTenantAuthored`), and why this PR waited for it and is
built on it. Measured on this branch before that merge: the three
runtime-package legs (two unit, one door) stayed red with the classifier
swap alone, and everything else was green. Only the `packaged` verdict
may discard. `unknown` is refused, mirroring the overlay detection
reading's direction rather than the write door's.
- **H3 holds.** The drift filter had the same reading. It is gated on
the same verdict, and a pin checks that the report and the action agree,
shape by shape: reported exactly when eligible.
- **H4 holds.** Every existing pin in both test files is unedited and
green. The door control still discards a shipped set's overlay and heals
its record.
## Pins
- `permission-set-overlay-discard.test.ts`, new block: each
environment-authored registry body is refused with `code`
`PERMISSION_DENIED` and `status` `403`, and both tables are counted and
compared whole before and after. The bodies are the runtime-package row
(`_packageId` plus `_provenance: 'org'`), the org-owned set, the clone,
and the lock's documented `'sys_metadata'` runtime shadow. A
code-shipped set sits beside them in the registry. Further cases: a
registry read that throws (`unknown`) is refused with the rows intact,
and a control discards a code-shipped set's overlay beside a hydrated
overlay item that wears the artifact's envelope.
- `permission-set-drift.test.ts`, new block: a drifted
environment-authored set with a stored definition is never diagnosed
(for each of the four shapes). The agreement pin runs the drift report
and Discard Overlay over one registry: the code-shipped set is reported
and eligible (`409 INVALID_STATE`, nothing to discard), each other shape
is unreported and refused (`403 PERMISSION_DENIED`), and no stored
definition is deleted.
-
`packages/qa/dogfood/test/permission-set-discard-overlay-eligibility.dogfood.test.ts`
(new): the three shapes made through their real doors (`POST /packages`
then `PUT /meta/permission/NAME?package=PKG`; `POST
/data/sys_permission_set`; the shipped `clone_permission_set` action's
own payload). Then a legacy overlay row for the shipped control, a cold
boot, and the list read. Then a precondition for the stamp, then each
shape's refusal with its record ids and stored rows compared before and
after. The drift report run over the booted registry judges the shipped
set and none of the three shapes. The control's discard returns `200`,
its overlay row is gone, its record is kept, and its grants equal the
shipped artifact's again.
## Ablations
The fix and its pins were committed first. Each ablation went through
`node scripts/ablation-replace.mjs` (anchor 1 to 0, blob changed), then
`pnpm --filter @objectstack/plugin-security build` and `node
scripts/ablation-dist-preflight.mjs @objectstack/plugin-security MARKER`
(exit 0, marker in `dist/index.js` and `dist/index.mjs`). The dogfood
suite resolves `plugin-security` from `dist/`. Each restore was proven:
blob equal to `HEAD`, `git diff HEAD` empty, a rebuild, `--absent`
preflight exit 0, and an empty `git status --porcelain`.
They ran before #21857 landed, on a local, never-pushed composition:
this branch at a4d82d3 plus #21857's two source files, blob-identical
to 9e3e32e. The plugin-security source in that composition is
byte-identical to this PR's head. Only #21857's two test files differ,
and they were not in the composition.
| ablation | what was put back | unit result | dogfood result |
|---|---|---|---|
| A | discard's eligibility reads "an item of this name carries a
package id" again (the verdict rebuilt as: some `readDeclared` item has
the row's name and a truthy `_packageId ?? packageId`) | 3 failed / 29
passed: runtime-package and sentinel refusals, and the agreement pin | 1
failed / 6 passed: runtime-package set, `200` with `overlaysDiscarded:
1` |
| B | drift's population reads the package id again (the classifier gate
made always-true) | 3 failed / 29 passed: runtime-package and sentinel
population pins, and the agreement pin | 1 failed / 6 passed: the
drift-population leg |
The org-owned, clone and control legs stayed green under both, as they
must: the old reading already refused the first two and accepted the
third. A first attempt at ablation A used a typed replacement that
narrowed the verdict's type, so the DTS step failed. The JS bundle still
carried the marker and the same three unit pins and one door pin went
red. It was redone with a cast that keeps the declared type, and the
numbers above are from the clean run.
## Tests and gates (head 62143b0 unless noted)
- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2`: 167 files, 3611 passed, 45 skipped (on 50f93c1, the
merge of `origin/main` at c9be1f1; the only later commit is a comment
in the dogfood file).
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0,
including `check:test-typecheck` (0 errors). `pnpm --filter
@objectstack/dogfood typecheck`: exit 0 (its `tsconfig.json` includes
`test/**`).
- `pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2
test/permission-set-discard-overlay-eligibility.dogfood.test.ts`: 7
passed on 62143b0. Together with #21857's
`permission-set-lock-row-provenance.dogfood.test.ts`: 21 passed on
50f93c1.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 97 commands on 62143b0; all 97 ran,
every exit 0. `--ran`: 97 derived, 97 run, 0 NOT-MEASURED, 0 UNRUN. The
first battery, on 50f93c1, found one real red:
`check:cross-package-test-inputs` read a docs path spelled in the
dogfood file's header comment as a test input. The comment now names the
page in words (62143b0). It also hit two PREREQUISITE NOT MET (exit 3:
`check:skill-examples`, `check:dual-build-cjs-loads`), cleared by
building the eight packages they named before the second battery. The
derivation printed a stale-tree note: `origin/main` moved two commits
(#21858, #21862) after the merge, and one of them edits
`scripts/engine-double-contract.pinned.json`. Those commits touch
neither this diff's packages nor its files, and `git merge-tree` against
them is clean.
- Lint, narrowed and proven: `pnpm exec eslint --no-inline-config
--format json` over the five touched TypeScript files gives 5 files in
the JSON output, 0 errors, 0 warnings, on 62143b0. The population is
the files this diff touches. The `.md` and `.mdx` files are outside the
lint globs in `eslint.config.mjs`, which never enables type-aware
linting (no `parserOptions.project`, no typed rules, stated in its own
comment), so this diff cannot move the verdict on any untouched file.
The repo-wide `pnpm lint` is CI's.
## Acceptance notes
- **Route.** The suggested route was to replace both `_packageId ??
packageId` readings with the verdict. Measured, the eligibility half is
replaced. The per-item selector is kept below the verdict, because the
two alternatives both change something for code-shipped sets. Keying it
on the verdict's package id changes which bodies are compared when two
code packages ship one name (ADR-0048 §3.4 lets them coexist under
distinct registry keys). Dropping it would let a projection echo with no
package id into the drift comparison.
- **The `'sys_metadata'` sentinel shape** is pinned at the unit level
only. It is the runtime shadow the lock module's docs describe; no door
producing it was measured here. The old reading accepted it, and the
classifier already refused it on `main`.
- **Drift's reach.** The boot's drift pass runs before any list read,
and the boot's hydration does not stamp the package id (measured by
#21857). So on the showcase the drift misjudgment shows only in a later
run of the diagnostics after a list read. The door pin runs
`computePermissionSetDriftDiagnostics` over the booted stack's registry
after the list read.
- **Observation, not filed (carrier: none).** In the degraded-kernel
branch (no metadata protocol), discard's resync body is the first
registry item of the name that carries any package id. If a hydrated
overlay of a code-shipped name preceded its artifact in registry order,
that body would be the overlay's. Nothing measured shows that order. It
is unchanged here, by H4.
- **Changeset.** `.changeset/21860-discard-overlay-eligibility.md`,
`patch` for `@objectstack/plugin-security`, with `Clause-②: no` copied
from the claim. The action's declared population (sets a code package
ships) is restored, and no accepted input widens.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 2e78046 commit 5e0b489
7 files changed
Lines changed: 582 additions & 27 deletions
File tree
- .changeset
- content/docs/permissions
- 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 | |
|---|---|---|---|
| |||
365 | 365 | | |
366 | 366 | | |
367 | 367 | | |
368 | | - | |
369 | | - | |
370 | | - | |
| 368 | + | |
| 369 | + | |
| 370 | + | |
| 371 | + | |
371 | 372 | | |
372 | 373 | | |
373 | 374 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
23 | 23 | | |
24 | 24 | | |
25 | 25 | | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
26 | 30 | | |
27 | 31 | | |
28 | 32 | | |
| |||
324 | 328 | | |
325 | 329 | | |
326 | 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 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
67 | 67 | | |
68 | 68 | | |
69 | 69 | | |
| 70 | + | |
70 | 71 | | |
71 | 72 | | |
72 | 73 | | |
| |||
125 | 126 | | |
126 | 127 | | |
127 | 128 | | |
128 | | - | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
129 | 150 | | |
130 | 151 | | |
131 | 152 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
356 | 356 | | |
357 | 357 | | |
358 | 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 | + | |
| 427 | + | |
| 428 | + | |
| 429 | + | |
| 430 | + | |
| 431 | + | |
| 432 | + | |
| 433 | + | |
| 434 | + | |
| 435 | + | |
| 436 | + | |
| 437 | + | |
| 438 | + | |
| 439 | + | |
| 440 | + | |
| 441 | + | |
| 442 | + | |
| 443 | + | |
| 444 | + | |
| 445 | + | |
| 446 | + | |
| 447 | + | |
| 448 | + | |
| 449 | + | |
| 450 | + | |
| 451 | + | |
| 452 | + | |
| 453 | + | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| 460 | + | |
| 461 | + | |
| 462 | + | |
| 463 | + | |
| 464 | + | |
| 465 | + | |
| 466 | + | |
| 467 | + | |
| 468 | + | |
| 469 | + | |
| 470 | + | |
| 471 | + | |
| 472 | + | |
| 473 | + | |
| 474 | + | |
| 475 | + | |
| 476 | + | |
| 477 | + | |
| 478 | + | |
| 479 | + | |
| 480 | + | |
| 481 | + | |
| 482 | + | |
| 483 | + | |
| 484 | + | |
| 485 | + | |
| 486 | + | |
| 487 | + | |
| 488 | + | |
| 489 | + | |
| 490 | + | |
| 491 | + | |
| 492 | + | |
| 493 | + | |
| 494 | + | |
| 495 | + | |
0 commit comments