Repository navigation
Commit 09ef77c
fix(plugin-security): the permission-set echo names its runtime package, so an isDefault set records its everyone suggestion (#23258)
Refs #23233 (the environment-kernel echo; the host-kernel producer
remains)
Clause-②: no — the projection keeps the package owner the protocol read
already serves (ADR-0090 D5: packages suggest, admins confirm); no
published schema or export moves.
## What this does
A permission set marked `isDefault: true` and published into a writable
runtime package with no `packageId` in its body now records its
`everyone` suggestion on an environment kernel. The security catalog and
the protocol's by-name read now name the same owner for it.
ADR-0090 D5, verbatim:
> **Packages suggest, admins confirm.** `isDefault: true` on a package
permission set produces an install-time prompt ("CRM suggests adding
`crm_readonly` to Everyone — accept?"). It is **never** auto-bound:
installing a package must not silently widen every tenant user's access.
- **`permission-set-projection.ts`.** The projection's in-memory echo of
a tenant-authored set (lock verdict `org`) now carries `_packageId`, the
package its stored row is bound to. The value is the `_packageId` the
protocol's by-name read (`getMetaItem`) serves, and nothing here derives
an owner. A `packaged` or `unknown` verdict carries none, as before.
- **One read of the binding.** `readServedPackageBinding` is a new
module-level function. The data door's update leg (#21861) already made
this exact read inline, and it now calls the same function. Its
behaviour is unchanged.
- **A failed read** registers the echo unowned, which is today's
behaviour. One `warn` names what is missing and what restores it.
- **Untouched:** the suggestion collector and `security-catalog.ts`. The
catalog takes its owner from `_packageId`, and the collector reads the
catalog.
### Hypotheses, as measured
- **H1 is half right.** `stripDecorations` does remove `_packageId`, but
the body it is handed (the layered read's `overlay` layer, the stored
document) never carried one. Keeping `_packageId` through the strip
would change nothing. The owner has to be read from the protocol, which
is what the by-name read serves.
- **H2 holds on an environment kernel only.** There, the protocol
hydrates no stored row into the engine registry
(`applyRegistryWriteThrough` returns before the hydrator when the kernel
has an `environmentId`). So the catalog reads the metadata service,
whose only body for the set is the echo. On a host kernel, the catalog
reads the engine registry's hydrated copy first. See the first
acceptance note.
- **H3:** the publish door's resolved binding reaches the projection
only through the protocol: the projector event carries no package.
`getMetaItem` stamps it from the served row's `package_id`, the same row
`getMetaItemLayered` serves as `overlay`.
- **H5 holds.** A code package's set has no overlay row, so it has no
echo. The catalog serves its artifact from the engine registry with the
artifact's owner. This is pinned below.
## H4 — the row keeps `managed_by: 'admin'` and no `package_id`
This deviates from triage's direction text, which carries the binding
"into `package_id` and the managed-by marker". The suggestion pin and
the owner-agreement pin do not need the row, and writing the row moves
edit doors. Read from the tree at `06dbc4e59d`:
- **`assertPackageManagedWriteGate` (`security-plugin.ts`)** refuses
`restore`, `transfer` and `purge` on a `managed_by: 'package'` row, by
id or by filter. Today those succeed on a runtime-package set's admin
row. Writing the marker would narrow them.
- **`retirePermissionSetRecord`** leaves a package-owned record in place
on a metadata delete ("left to the package door"). Deleting a set from a
runtime package through the metadata door would leave a record with no
definition behind it, until the package is uninstalled. Today that
record is retired.
- **`upsertEnvPermissionSet`** stamps `customized: true` on a package
row backed by an overlay. A runtime-package set's definition IS its
stored row, so Setup would badge it "customized" and offer a reset.
- **The C3 stage 6b-2 landing (`c8a9783eca`)** left no writer of
`managed_by: 'package'` rows. Writing the marker here would bring that
writer class back.
So the edit surface is unchanged, and the pins measure it: the layered
read still reports `lock: 'none'`, `editable: true` and `deletable:
true`; the lock classifier still answers `org` with the owner-carrying
echo as its code layer (the echo marker keeps it from reading as an
artifact); an edit through the metadata door still lands; and the row is
still `managed_by: 'admin'` with a null `package_id`.
## Measured on a booted showcase, before and after
This was a scratch probe, not committed. It boots the showcase, saves an
`isDefault: true` draft into `app.probe_NAME` with no body `packageId`,
runs `publishPackageDrafts`, then reads. The environment kernel was
emulated by setting the protocol's `environmentId` before the publish
(the tenant shape). `main` is `06dbc4e59d`.
| read | `main` | this branch |
|---|---|---|
| `getMetaItem` item `_packageId` | the package | the package |
| `getMetaItemLayered` `packageId` | absent | the package |
| catalog, environment kernel | source `metadata`, no owner | source
`metadata`, the package |
| suggestions, environment kernel | none | one, `pending`, under the
package (written by the post-publish reconcile) |
| catalog, host kernel, right after the publish | source `registry`, no
owner | unchanged (first acceptance note) |
| `sys_permission_set` row | `managed_by: admin`, `package_id: null` |
unchanged |
| `editable` / `deletable` / `lock` | `true` / `true` / `none` |
unchanged |
## The pins: `permission-set-echo-package-owner.test.ts`
The harness is a REAL `ObjectStackProtocolImplementation` (aliased to
source) over a fake engine, constructed with an `environmentId`. The
package door is real (`saveMetaItem` draft, then
`publishPackageDrafts`), as are `createSecurityCatalogReader` and
`syncAudienceBindingSuggestions`.
- Publish a runtime package's `isDefault` set without a body
`packageId`: one pending `everyone` suggestion, under that package.
Anti-vacuity: the engine registry holds no copy of the set.
- The catalog (`resolve` and `list`, source `metadata`) and
`getMetaItem` name the same owner.
- The edit surface does not move (H4).
- **Controls:** a code package's set is unchanged; a set saved into no
package stays unowned and records no suggestion; a failed binding read
leaves the echo unowned and warns once.
## Negative-pin ablation (at `2f35a44309`)
The owner carry was replaced by a comment through
`scripts/ablation-replace.mjs`, which traps the restore. The anchor went
1 → 0 and the blob went `251b44ecf505` → `5c7759d56da9`. Then the pin
file ran:
```text
× records ONE pending everyone suggestion, under the package the publish door bound it to
× the security catalog and the protocol's by-name read name the same owner
× the owner-carrying echo is still not a shipped artifact: the lock answers org, the item stays editable, and an edit lands
AssertionError: expected [] to deeply equal [ ObjectContaining{…} ]
AssertionError: expected { type: 'permission', …(3) } to match object { source: 'metadata', …(1) }
AssertionError: expected { …(6) } to match object { …(2) }
Tests 3 failed | 3 passed (6)
ablation-replace: ok restored: blob == HEAD (251b44e) and `git diff HEAD` is empty
```
The three controls stayed green. The direction is the expected one: red.
## What remains on #23233
A host kernel (no `environmentId`) still serves a runtime package's
freshly published `isDefault` set as unowned until a `getMetaItems` list
read. The cause is a second producer of the same unowned read, in
`metadata-protocol`: `applyRegistryWriteThrough` →
`hydrateOverlayIntoRegistry` registers the published body in the engine
registry without stamping its row's package binding as `_packageId`,
while `getMetaItems` stamps it before hydrating. The security catalog
reads the engine registry first. That half lands in another package and
lane, so the seat routes it as the card's remainder. This PR covers the
measured topology of the card's report: the environment kernel's echo.
## Acceptance notes
- **Host kernel: a second producer drops the binding (finding, not fixed
here).** On a kernel with no `environmentId`, the publish's registry
write-through (`applyRegistryWriteThrough`,
`@objectstack/metadata-protocol`) hydrates the stored row into the
engine registry WITHOUT stamping its `package_id` as `_packageId`. The
list read (`getMetaItems`) does stamp it before it hydrates. Until the
next `permission` list read, the catalog (registry first) serves the set
unowned and no suggestion is recorded. The probe measured `created: 0`
right after the publish and `created: 1` after one `getMetaItems`. That
file is outside this card's surface and is reported for filing.
- **A body `packageId` that disagrees with the binding.** The door
accepts it: the probe published `packageId: 'app.probe_other'` into
`app.probe_bound` with outcome `published`. Before this change, on an
environment kernel, the suggestion went to the body's package while
`getMetaItem` named the binding. With this change it goes to the
binding, which the host kernel already did after a list read. A
suggestion that was recorded under the body's package is pruned while
pending, and withheld from `everyone`, with its reason, if it was
accepted. That follows from the collector's and the derivation's code
and was not measured end to end. The door accepting the disagreement is
reported as a finding.
- **Boot cost.** Reconciliation now makes one `getMetaItem` per
env-authored overlay name, beside the `getMetaItemLayered` it already
made. A live save or publish makes one per mutation. NOT MEASURED
against a remote database.
- **A disabled runtime package.** The catalog does not serve an owned
item whose package the engine registry reports disabled. On an
environment kernel that rule now reaches runtime-package sets, as it
already did on a host kernel after a list read. NOT MEASURED: whether a
runtime package can be disabled in the registry's package map.
## Verification
- `pnpm --filter @objectstack/plugin-security exec vitest run
--maxWorkers=2`: 189 files, 3869 passed, 45 skipped.
- `pnpm --filter @objectstack/plugin-security typecheck` exits 0, and
the test layer compiles under `tsconfig.test.json` with an empty debt
ledger.
- **Gate union at `2f35a44309`:** 75 families derived by `node
scripts/pm/dispatch-gates.mjs --commands` (no paths), all 75 run, each
`:: exit 0`; `--ran` reconciliation: "75 run, 0 NOT-MEASURED (a DERIVED
zero)". `check:dual-build-cjs-loads` first answered PREREQUISITE NOT MET
(eight unrelated packages had no `dist/`); they were restored from the
turbo cache and it then passed. The derivation warns the tree is behind
`origin/main` on `scripts/pm/ci-failure.mjs`, which this diff does not
touch.
- `pnpm exec eslint --no-inline-config --format json` over the two
changed TypeScript files: 2 files, 0 errors, 0 warnings. Both resolve a
config (`--print-config`), and `eslint.config.mjs` enables no type-aware
linting (no `parserOptions.project`), so this diff cannot move any
untouched file's verdict. The repo-wide `pnpm lint` is CI's.
- `check:engine-double-contract` asked for the new pin file's three
pinned doubles to be recorded.
`scripts/engine-double-contract.pinned.json` gains those three rows and
nothing else (`--write`: "0 added or grown, 0 lost").
---
_Generated by [Claude
Code](https://claude.ai/code/session_01FzQy26KznSa5j2gnf3AzsE)_
---------
Co-authored-by: Claude <noreply@anthropic.com>1 parent 7958bdf commit 09ef77c
4 files changed
Lines changed: 388 additions & 7 deletions
File tree
- .changeset
- packages/plugins/plugin-security/src
- scripts
| 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 | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
| 22 | + | |
| 23 | + | |
| 24 | + | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
| 35 | + | |
| 36 | + | |
| 37 | + | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
| 41 | + | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
| 80 | + | |
| 81 | + | |
| 82 | + | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
| 105 | + | |
| 106 | + | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
| 122 | + | |
| 123 | + | |
| 124 | + | |
| 125 | + | |
| 126 | + | |
| 127 | + | |
| 128 | + | |
| 129 | + | |
| 130 | + | |
| 131 | + | |
| 132 | + | |
| 133 | + | |
| 134 | + | |
| 135 | + | |
| 136 | + | |
| 137 | + | |
| 138 | + | |
| 139 | + | |
| 140 | + | |
| 141 | + | |
| 142 | + | |
| 143 | + | |
| 144 | + | |
| 145 | + | |
| 146 | + | |
| 147 | + | |
| 148 | + | |
| 149 | + | |
| 150 | + | |
| 151 | + | |
| 152 | + | |
| 153 | + | |
| 154 | + | |
| 155 | + | |
| 156 | + | |
| 157 | + | |
| 158 | + | |
| 159 | + | |
| 160 | + | |
| 161 | + | |
| 162 | + | |
| 163 | + | |
| 164 | + | |
| 165 | + | |
| 166 | + | |
| 167 | + | |
| 168 | + | |
| 169 | + | |
| 170 | + | |
| 171 | + | |
| 172 | + | |
| 173 | + | |
| 174 | + | |
| 175 | + | |
| 176 | + | |
| 177 | + | |
| 178 | + | |
| 179 | + | |
| 180 | + | |
| 181 | + | |
| 182 | + | |
| 183 | + | |
| 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 | + | |
| 227 | + | |
| 228 | + | |
| 229 | + | |
| 230 | + | |
| 231 | + | |
| 232 | + | |
| 233 | + | |
| 234 | + | |
| 235 | + | |
| 236 | + | |
| 237 | + | |
| 238 | + | |
| 239 | + | |
| 240 | + | |
| 241 | + | |
| 242 | + | |
| 243 | + | |
| 244 | + | |
| 245 | + | |
| 246 | + | |
| 247 | + | |
| 248 | + | |
| 249 | + | |
| 250 | + | |
| 251 | + | |
| 252 | + | |
| 253 | + | |
| 254 | + | |
| 255 | + | |
| 256 | + | |
| 257 | + | |
| 258 | + | |
| 259 | + | |
| 260 | + | |
| 261 | + | |
| 262 | + | |
| 263 | + | |
| 264 | + | |
| 265 | + | |
| 266 | + | |
| 267 | + | |
| 268 | + | |
| 269 | + | |
| 270 | + | |
| 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 | + | |
0 commit comments