Repository navigation
Commit c9be1f1
fix(plugin-security): the permission-set lock reads the row's provenance, so org-owned sets, clones and runtime-package sets edit again (#21857)
Fixes #21789
Clause-②: no
## What this changes
The packaged-permission-set lock in `plugin-security` answers one
question for both write doors: is this set shipped by a code (artifact)
package? It answered it as "does any engine-registry item of this name
carry a package id?". The registry holds stored rows as well as
artifacts, and the metadata list read (`GET /api/v1/meta/permission`,
which every Studio page load issues) stamps a stored row's `package_id`
column onto its body as `_packageId`. So a set saved into a writable
runtime package looked code-shipped after the first list read.
The read the console renders from had the matching defect. The security
plugin keeps a marked copy of every overlay-backed definition in the
metadata manager for the evaluator (the "projection echo"), and the
protocol's layered read serves that copy as the item's `code` layer. The
echo carried no provenance, so an org's own set, a clone and a
runtime-package set all reported a `code` layer with no `provenance`.
objectui's permission-matrix editor reads exactly that as "a code
package ships this" and rendered them locked, while every write door
accepted the save.
Two edits, both in `packages/plugins/plugin-security/src`:
1. `packaged-permission-set-lock.ts`, `declaredPackageIdOf`: a
tenant-authored item (ADR-0010 `_provenance: 'org'`, the stamp the
hydrator writes on every stored row) is never a shipped artifact. It is
read through `isTenantAuthored` from `@objectstack/metadata-core`, the
exclusion `isCodeArtifactBody` and `SchemaRegistry.getArtifactItem`
already apply. No second provenance evaluator. A stored row of a name a
code package ships is hydrated wearing the artifact's envelope
(`_provenance: 'package'`), and the artifact itself is in the same list,
so a code-shipped set stays locked.
2. `permission-set-projection.ts`: the projection echo carries
`_provenance: 'org'` exactly when `classifyPackagedPermissionSet` (the
classifier both write doors ask, fed the same layered probe) answers
`org` for the name. A `packaged` or `unknown` verdict leaves the echo
unstamped, as before. The reported state and the enforced state are one
judgment.
Not touched: `metadata-protocol` (H4 was not needed), `packages/spec`,
any error code, any export, any parameter of an exported function (the
new parameter is on the module-private `syncEvaluatorRegistry`). The
lock-resolution semantics from #21801 are unchanged.
## Measurements, before and after
Driven through the real showcase over HTTP, base `088428fb` (before) and
this branch (after):
| shape | before: door (`PUT /meta` after the list read, `PATCH /data`)
| before: layered read | after: door | after: layered read |
|---|---|---|---|---|
| set in a writable runtime package | 403 `NOT_OVERRIDABLE` / 403
`NOT_OVERRIDABLE` | `code` = echo, no `provenance`, `editable: true` |
200 / 200 | `provenance: 'org'`, `editable: true` |
| org-owned set (data door) | 200 / 200 | `code` = echo, no
`provenance`, `editable: true` | 200 / 200 | `provenance: 'org'`,
`editable: true` |
| clone ("Clone to customize") | 200 / 200 | `code` = echo, no
`provenance`, `editable: true` | 200 / 200 | `provenance: 'org'`,
`editable: true` |
| control: `showcase_contributor` (shipped by `com.example.showcase`) |
403 `NOT_OVERRIDABLE` / 403 `NOT_OVERRIDABLE` | `code._packageId` = the
package, `provenance: 'package'`, `editable: false` | unchanged |
unchanged |
The runtime-package set's registry row after the list read was `{
_packageId: 'com.dogfood.lock21789', _provenance: 'org' }`: the
provenance that tells it apart was on the body all along.
## Mechanism hypotheses, measured
- **H1, holds.** The lock read any non-sentinel `_packageId` as
code-shipped. The three shapes carry, in the registry: runtime-package
set `{ _packageId: PKG, _provenance: 'org' }` (the package id appears
only after a list read; neither the write-through nor the boot hydration
stamps it), org-owned set and clone `{ _provenance: 'org' }`, no package
id. The clone's record has `created_by` and `organization_id` null, but
so do the org-owned set's and the runtime-package set's records: it is
not specific to the clone.
- **H2, holds.** The platform's one answer is `isCodeArtifactBody` /
`isTenantAuthored` in `@objectstack/metadata-core` (already a dependency
of `plugin-security`). The lock reuses `isTenantAuthored`; it keeps its
two documented extensions (the echo-marker skip and the spec `packageId`
fallback).
- **H3, holds, with a refinement.** The org-owned set and the clone were
never refused by the server (both doors 200 before the fix); their
"lock" was report-only. The runtime-package set was refused by both
doors while the server's own `editable` said `true`. So the reported
state and the enforced state were split in both directions, and the fix
pins both.
- **H4, not needed.** No `metadata-protocol` edit: the layered read
already reads `provenance` off the `code` layer, and the echo now states
it.
- **H5, the lock's judgment (the smaller one).** Stamping the clone's
`created_by` / organization would not change anything the lock or the
console reads: the server lock already answered `org` for the clone, and
the console's lock came from the echo's missing provenance.
- **H6, holds.** The code-shipped set is still refused at both doors
with `403 NOT_OVERRIDABLE` (the data door's refusal is the lock's own
sentence naming the clone path), and its layered read still reports
`provenance: 'package'`, its package id and `editable: false`, before
and after a cold boot.
## Pins
- `packaged-permission-set-lock.test.ts`, block `[#21789]`: the
classifier over the bodies the hydrator registers (runtime-package row,
org-owned row, clone; a shipped artifact, alone and beside a legacy
overlay wearing its envelope, in both orders), the layered probe with no
registry, the data door (hatch-open double, so only the lock can
refuse), and the metadata-door gate, with the refusal asserted on `code`
and `status`.
- `permission-set-projection.test.ts`: the echo of a set no code package
ships carries `_provenance: 'org'`; the control shows the echo of a
legacy overlay of a shipped set does not.
-
`packages/qa/dogfood/test/permission-set-lock-row-provenance.dogfood.test.ts`
(new): the showcase, 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), the list read, a precondition that the list read stamped
the package id, then both doors and the layered read for each shape, the
code-shipped control at both doors and on the read, and a cold boot on
the same file that reads the three shapes again (the echo minted by the
boot's reconciliation) and re-checks the control.
## Ablations (each committed first, mutated through
`scripts/ablation-replace.mjs`, rebuilt, dist proven, restored to
`HEAD`)
Both ablations were run at `e9dff47f` (the fix and its pins committed,
pre-merge), each through `node scripts/ablation-replace.mjs` (anchor
hits went from 1 to 0, blob changed), then `pnpm turbo run build
--filter=@objectstack/plugin-security`, then `node
scripts/ablation-dist-preflight.mjs @objectstack/plugin-security MARKER
--absent` (exit 0: marker absent from every built file), because the
dogfood suite resolves `plugin-security` from `dist/`. Each restore was
proven by blob equality with `HEAD` and an empty `git diff HEAD`, then a
rebuild and the preflight without `--absent` (exit 0, marker back in
`dist/index.js`, tree clean).
| ablation | what was put back | unit result | dogfood result |
|---|---|---|---|
| 1 | the lock reads "has a package id" again: `if
(isTenantAuthored(item)) return null;` replaced by a no-op | 5 failed /
87 passed: the four classifier/door pins for the runtime-package shape,
and the echo pin | 2 failed / 12 passed: runtime-package set, metadata
door and data door |
| 2 | the echo states no provenance again: the `_provenance: 'org'`
spread replaced by an empty one | 1 failed / 91 passed: the echo pin | 4
failed / 10 passed: the layered-read pin for each of the three shapes,
and the cold-boot read |
The controls (a code-shipped set refused, and its read reporting
`provenance: 'package'`) stayed green in both directions, as they must.
A first attempt at ablation 1 used a replacement that left the
`isTenantAuthored` import unused, so the DTS step of the build failed
(the JS bundle still carried the ablation and the same pins went red);
it was redone with the import kept in use, and the numbers above are
from the clean run.
## Tests and gates
All on `9e3e32ed` (this branch after merging `origin/main` `8832655a`,
which carries #21812 and touches `plugin-security`), after rebuilding
the dogfood dependency closure:
- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2`: 167 files passed, 3600 tests passed, 45 skipped.
- `pnpm --filter @objectstack/plugin-security typecheck`: exit 0
(including `check:test-typecheck`: 0 errors).
- `pnpm --filter @objectstack/dogfood exec vitest run --maxWorkers=2
test/permission-set-lock-row-provenance.dogfood.test.ts`: 14 passed.
`pnpm --filter @objectstack/dogfood typecheck`: exit 0.
- `node scripts/pm/dispatch-gates.mjs --commands --repo
objectstack-ai/objectstack` derived 71 gate commands; all 71 run, every
exit 0, `--ran` verdict: 71 derived, 71 run, 0 NOT-MEASURED, 0 UNRUN.
`check:dual-build-cjs-loads` first answered exit 3 (PREREQUISITE NOT
MET: eight packages outside the dogfood closure had no `dist/`); those
eight were built and the gate re-run, exit 0.
- Lint, narrowed and proven: `pnpm exec eslint --no-inline-config
--format json` over the five touched TypeScript files (the changeset is
not linted): 5 files in the JSON output, 0 errors, 0 warnings. The
population is the files this diff touches; `eslint.config.mjs` never
enables type-aware linting (no `parserOptions.project`, no typed rules,
stated in its own header), so this diff cannot move the verdict on any
untouched file. The repo-wide `pnpm lint` is CI's.
## Acceptance notes
- **Not in this PR: `metadata-protocol`.** Measured, the layered read
did not need to change for the read and the lock to agree: it reads
`provenance` off its `code` layer, which is the plugin's projection
echo, and all three shapes read `provenance: 'org'` with `protocol.ts`
byte-identical to `main`. No shape is left unfixed without a protocol
edit. This PR's file list is disjoint from #21844's (`item-lock.ts`,
`protocol.ts`, two protocol/objectql tests, the engine-double ledger,
its changeset).
- **Observation, not filed (carrier: the `domain:engine` seat, #21844
holds the region).** For a set no artifact ships, the layered read's
`code` layer is still a non-null body (the echo, read through
`readItemFromMetadataService` in `getMetaItemLayered`), while
`GetMetaItemLayeredResponseSchema.code` says `null` when no artifact
ships the item. The registry fallback right below it already drops a
tenant-authored item (`runtimeOnly`, `isTenantAuthored`); the
MetadataService read does not. After this PR the echo is tenant-stamped,
so a provenance-only filter there would answer `code: null` for these
sets; no client reads a wrong answer today, which is why it is noted
here rather than built.
- **Finding, reported for the seat to file (same family: a package id
read as "shipped by code").** The Discard Overlay action's eligibility
(`permission-set-overlay-discard.ts`, `discardPermissionSetOverlay`)
reads `_packageId ?? packageId` on the registry item, as the lock did.
Measured on `088428fb` and again on this branch: after a list read,
`POST /api/v1/security/permission-sets/ID/discard-overlay` on a set
saved into a writable runtime package answered 200 and deleted the set's
only `sys_metadata` row. The action declares, and
`content/docs/permissions/permission-sets.mdx` repeats, that it refuses
any set that is not currently package-declared.
`permission-set-drift.ts`'s declared filter carries the same reading.
Not fixed here: outside the claimed file surface.
- **Finding, reported for the seat to file.** A data-door edit (`PATCH
/api/v1/data/sys_permission_set/ID`) of a set saved into a writable
runtime package writes a second, package-less active `sys_metadata` row
carrying the edit and leaves the package-bound row unchanged (the
write-through's update leg calls `saveMetaItem` without the row's
package). Measured on this branch: two active rows after one PATCH; the
projected record reads `managed_by: 'admin'`, `package_id: null`. It is
reachable on `main` before any list read, and through a package-less
`PUT /meta`; this PR lets the data door accept the edit after a list
read too.
- **H5.** The clone's record has `created_by` and `organization_id`
null, and so do the other two shapes' records: the projection writes
them in system context. It is not what locked the clone, and is not
changed here.
- **Docs.** No `content/docs` sentence is made false by this change;
`content/docs/concepts/metadata-lifecycle.mdx` (runtime-created sets,
package-bound rows included, keep working) becomes true.
`content/docs/permissions/permission-sets.mdx` still says an edit of a
packaged set through Setup becomes an environment overlay, which the
lock has refused since the clone-to-customize ruling; that is older
drift, not touched here.
- **Report state.** `Clause-②: no` is copied from the claim: the fix
restores the lock's declared population (code-shipped sets) and widens
no accepted input; a code-shipped set is refused exactly as before.
---
_Generated by [Claude
Code](https://claude.ai/code/session_011K3zqE8Pv1Evw5hc8tZCnN)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 969ffba commit c9be1f1
6 files changed
Lines changed: 466 additions & 2 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 | + | |
Lines changed: 100 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
77 | 77 | | |
78 | 78 | | |
79 | 79 | | |
| 80 | + | |
| 81 | + | |
80 | 82 | | |
81 | 83 | | |
82 | 84 | | |
| |||
885 | 887 | | |
886 | 888 | | |
887 | 889 | | |
| 890 | + | |
| 891 | + | |
| 892 | + | |
| 893 | + | |
| 894 | + | |
| 895 | + | |
| 896 | + | |
| 897 | + | |
| 898 | + | |
| 899 | + | |
| 900 | + | |
| 901 | + | |
| 902 | + | |
| 903 | + | |
| 904 | + | |
| 905 | + | |
| 906 | + | |
| 907 | + | |
| 908 | + | |
| 909 | + | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
| 915 | + | |
| 916 | + | |
| 917 | + | |
| 918 | + | |
| 919 | + | |
| 920 | + | |
| 921 | + | |
| 922 | + | |
| 923 | + | |
| 924 | + | |
| 925 | + | |
| 926 | + | |
| 927 | + | |
| 928 | + | |
| 929 | + | |
| 930 | + | |
| 931 | + | |
| 932 | + | |
| 933 | + | |
| 934 | + | |
| 935 | + | |
| 936 | + | |
| 937 | + | |
| 938 | + | |
| 939 | + | |
| 940 | + | |
| 941 | + | |
| 942 | + | |
| 943 | + | |
| 944 | + | |
| 945 | + | |
| 946 | + | |
| 947 | + | |
| 948 | + | |
| 949 | + | |
| 950 | + | |
| 951 | + | |
| 952 | + | |
| 953 | + | |
| 954 | + | |
| 955 | + | |
| 956 | + | |
| 957 | + | |
| 958 | + | |
| 959 | + | |
| 960 | + | |
| 961 | + | |
| 962 | + | |
| 963 | + | |
| 964 | + | |
| 965 | + | |
| 966 | + | |
| 967 | + | |
| 968 | + | |
| 969 | + | |
| 970 | + | |
| 971 | + | |
| 972 | + | |
| 973 | + | |
| 974 | + | |
| 975 | + | |
| 976 | + | |
| 977 | + | |
| 978 | + | |
| 979 | + | |
| 980 | + | |
| 981 | + | |
| 982 | + | |
| 983 | + | |
| 984 | + | |
| 985 | + | |
| 986 | + | |
| 987 | + | |
Lines changed: 23 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
87 | 87 | | |
88 | 88 | | |
89 | 89 | | |
| 90 | + | |
| 91 | + | |
90 | 92 | | |
91 | 93 | | |
92 | 94 | | |
| |||
146 | 148 | | |
147 | 149 | | |
148 | 150 | | |
149 | | - | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
150 | 169 | | |
151 | 170 | | |
152 | 171 | | |
| |||
155 | 174 | | |
156 | 175 | | |
157 | 176 | | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
158 | 180 | | |
159 | 181 | | |
160 | 182 | | |
| |||
Lines changed: 34 additions & 0 deletions
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
419 | 419 | | |
420 | 420 | | |
421 | 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 | + | |
422 | 456 | | |
423 | 457 | | |
424 | 458 | | |
| |||
Lines changed: 27 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
97 | 97 | | |
98 | 98 | | |
99 | 99 | | |
| 100 | + | |
100 | 101 | | |
101 | 102 | | |
102 | 103 | | |
| |||
723 | 724 | | |
724 | 725 | | |
725 | 726 | | |
| 727 | + | |
| 728 | + | |
| 729 | + | |
| 730 | + | |
| 731 | + | |
| 732 | + | |
| 733 | + | |
| 734 | + | |
| 735 | + | |
| 736 | + | |
| 737 | + | |
| 738 | + | |
| 739 | + | |
| 740 | + | |
| 741 | + | |
726 | 742 | | |
727 | 743 | | |
728 | 744 | | |
| |||
731 | 747 | | |
732 | 748 | | |
733 | 749 | | |
| 750 | + | |
734 | 751 | | |
735 | 752 | | |
736 | 753 | | |
737 | 754 | | |
738 | 755 | | |
739 | 756 | | |
| 757 | + | |
740 | 758 | | |
741 | 759 | | |
742 | 760 | | |
| |||
829 | 847 | | |
830 | 848 | | |
831 | 849 | | |
| 850 | + | |
| 851 | + | |
| 852 | + | |
832 | 853 | | |
833 | 854 | | |
834 | 855 | | |
| |||
846 | 867 | | |
847 | 868 | | |
848 | 869 | | |
| 870 | + | |
849 | 871 | | |
850 | 872 | | |
851 | 873 | | |
| |||
885 | 907 | | |
886 | 908 | | |
887 | 909 | | |
888 | | - | |
| 910 | + | |
| 911 | + | |
| 912 | + | |
| 913 | + | |
| 914 | + | |
889 | 915 | | |
890 | 916 | | |
891 | 917 | | |
| |||
0 commit comments