Skip to content

Commit 00bee27

Browse files
fix(plugin-security)!: a package-declared position is refused at the data door, as the metadata door already refuses it (#22378)
Fixes #22360 Clause-②: no (narrowing) ## Rework round 1 — supersedes the sections below where they disagree Seat REWORK `6071670550` on #22360. Head `c345f93b73`, which merges `origin/main` `16096e8d7b`. - **What changed from round 1.** `assertSystemRowWriteGate` (`security-plugin.ts`, the gate region only) gains carve-out (d). An `update` of a `sys_position` row stamped `package` (or `config`, its legacy pre-A4 spelling, which `SYSTEM_ROW_PROVENANCE` guards as the same owner) passes when its patch is ONE object naming only `active` and/or `is_default` (`PACKAGE_POSITION_ROW_STATE_COLUMNS`). This matches #4669's carve-out for packaged permission sets and #15196's Q2 = A. - A filtered or multi-row row-state patch passes over package rows. A built-in in the filter still refuses it. - Still refused, with today's code and message: - a definition column (`name`, `label`, `description`, `delegatable`), any other column, a mixed or empty patch; - delete, transfer, restore and purge; - every write to a `platform` row (even a bare `{ active }`); - `sys_capability`; - the provenance-stamp refusal (a). - **H1, this branch:** on a package-declared position, Deactivate, Activate and Set as Default answer `200` and persist across a reboot. A label or `delegatable` edit answers `403`, and so does a delete. - **Pins:** - 13 gate cases in `store-fault-fail-closed.test.ts`, on its ledgered double. The ablation of the carve-out turns exactly the 3 admit cases red. - The dogfood pin is rewritten: 10/10 with the change. Against `main`'s build, 5 are red. The dist ablation turns 3 red. - **Changeset:** definition edits (including `delegatable`) and delete are refused; row state stays switchable; the remedy stays. - **Verification:** - `plugin-security` 188 files / 3915 tests passed; - the position dogfood files 18/18 (171 tests); - gates 70 of 70 derived and run, with `--ran` a derived zero. - Acceptance note 1 ("the narrowing is wider than…") is resolved by this carve-out. ## What this changes The declared-position seeder (`bootstrapDeclaredPositions`, `@objectstack/plugin-security`) now stamps `managed_by: 'package'` on the row of a position a code package holds: - on insert; - on an existing row that carries no managed value: an upgraded deployment's row, stamped `admin` by the object default. That write carries `managed_by` and nothing else, apart from the label and description refresh the seeder always made. "A code package holds the name" is asked of the engine registry's artifact lookup (`getArtifactItem`). That is the lookup the metadata door refuses a save from with `403 NOT_OVERRIDABLE`, so the data door now refuses what the metadata door refuses, and nothing more. `assertSystemRowWriteGate` is unchanged: it already refuses an admin-door update or delete of a `package` row, as it does for the built-ins. A position the environment authored through the metadata door has no package, so its row stays unmanaged. Diff: `bootstrap-declared-positions.ts`, its unit test, one dogfood pin, one changeset. Nothing in `packages/spec`, `packages/core`, the gate, the built-in seeder, the permission-set seeder, or the in-flight position write-through of #15196 stage S7. ## Reproduction (H1), on `origin/main` `b460153912` Showcase, `single` posture, two boots of one database file. Scratch probe, deleted and never committed. | step | `main` | this branch | |---|---|---| | declared rows (10) | `managed_by: admin`, organization null | `managed_by: package` | | catalog entry for each | source `registry`, package `com.example.showcase` (= `getArtifactItem`) | same | | `PUT /meta/position/manager` | 403 `NOT_OVERRIDABLE` | 403 `NOT_OVERRIDABLE` | | `PATCH /data/sys_position/ID` label of `manager` | 200, row relabelled; next boot restores the declared label | 403 `PERMISSION_DENIED`, row unchanged | | same, `active: false` on `contributor` | 200, **persists** across the reboot | 403 `PERMISSION_DENIED` | | same, `delegatable: true` on `exec` | 200, **persists** across the reboot | 403 `PERMISSION_DENIED` | | `DELETE` on `auditor` | 200; the next boot creates the row again | 403 `PERMISSION_DENIED` | | Setup-created position: create, then edit | 201, 200 | 201, 200 | | built-in `everyone` label edit | 403 `PERMISSION_DENIED` | 403 `PERMISSION_DENIED` | `assertSystemRowWriteGate` judged the declared rows as admin-authored before (`managed_by: admin` is not in its managed set) and judges them `package` now. ## The stamp and #2909 T2 (H2) T2's text locks that a re-seed only refreshes `label`/`description` and never touches the authoritative fields (bindings, `delegatable`, and the like). The stamp does not conflict with it: - On a row the seeder creates, the stamp sets provenance. It projects no declaration content, because a position declaration has no `managed_by` key. - On an existing row it overwrites no administrator edit. `managed_by` is `readonly`, and the gate refuses an admin-door payload naming `platform`/`package`, so `admin` on a declared row was only ever the object default. The authoritative columns keep their values (pinned: `active`, `is_default`, `delegatable` survive the re-stamp). Value: `package`, the spelling the gate refuses and the one the permission-set and capability seeders stamp. It needs nothing from `normalize-managed-by.ts`: `package` is canonical and the normalizer only rewrites `system`/`config`/`user`. A row already carrying a gate-managed value (`platform`, `package`, legacy `system`/`config`) is never re-stamped. **Measured: a Setup-created position whose name a package declares later gets locked.** A Setup row (`p22360_taken`, organization-bound) was created and edited (200). A third boot, whose stack declares that name, then ran the seeder. On `main` the seeder relabelled the row and left it `admin`, so the next edit answered 200. On this branch the row is re-stamped `package` and the next edit answers 403 `PERMISSION_DENIED`. The seeder matches by name, and it already took such a row over for its label; the fix was not widened (see Acceptance notes). ## Readers of `sys_position.managed_by` (H3) - `assertSystemRowWriteGate`: the one answer that changes. It refuses update, delete, transfer, restore and purge on the stamped rows, and a filter-scoped write whose filter matches one. - the `reserved_identity_name` validation rule on `sys_position`: it exempts only `platform`/`system`, so its answer is unchanged. The seeder never writes a built-in name. - `normalize-managed-by.ts`: scans legacy values only. Unchanged. - uninstall cleanup (`cleanup-package-permissions.ts`, the only plugin-security entry on the protocol's uninstall seam; the other entry is runtime's package jobs): it selects `sys_permission_set` by `package_id` + `managed_by: 'package'`, and deletes `sys_position_permission_set` by `permission_set_id` and `sys_audience_binding_suggestion` by `package_id`. It never reads or deletes `sys_position`, so **it deletes no row it did not delete before**. - explain engine, delegated-admin gate, `resolve-authz-context`, sharing services: they read `sys_position` (name, `active`, organization) but never `managed_by`. - Setup UI: `managed_by` is in the object's `highlightFields`, so the record header now reads Package. The Activate, Deactivate and Set as Default actions carry no `managed_by` condition and answer 403 on these rows, as they already do on a built-in. objectui at the pinned `.objectui-sha` `a58626c8`: the only row-level `managed_by` reader is `recordDelete`, and it acts on `sys_permission_set` only. ## Pins: red on `main`, green with the change, ablations Unit (`src/bootstrap-declared-positions.test.ts`, source-resolved, real `SchemaRegistry`), 7 new cases: - The seeder source reverted to the `b460153912` blob (on-disk hash checked) gives **4 failed, 16 passed**: a new row stamped; an upgraded admin row corrected with exactly `{ id, managed_by }`; display refresh and stamp in one write; the no-artifact-lookup registry branch. The other three are controls that pass on `main` by design: a second pass writes nothing; a gate-managed value is kept; an environment-authored definition stays unmanaged. - Ablating only the re-stamp line (`node scripts/ablation-replace.mjs`, anchor 1 to 0, blob `85be9d2d` to `cff57413`) gives **2 failed, 18 passed**: exactly the two re-stamp cases. - Both restores were proved by blob equal to HEAD and an empty `git diff HEAD`. The tree with the change gives 20 passed. Dogfood (`declared-position-provenance.dogfood.test.ts`; plugin-security resolves through `dist/`): - Against the pre-change `dist/` (`ablation-dist-preflight --absent` passed on the new marker): **3 failed, 4 passed**. The red cases are the declared row's provenance, the refused edit with the row unchanged, and the cold boot where the upgraded row is re-stamped and refused. The 4 controls pass: the precondition (`NOT_OVERRIDABLE` at the metadata door), the administrator-authored position, the built-in refusal, and the administrator- and environment-authored rows after the boot. - Ablating the re-stamp line, then rebuilding (`dist/` reading: the call is absent) gives **1 failed, 6 passed**: the cold-boot case. - Restore leg: rebuild, the call is present in 2 built files, **7 passed**. ## Changeset `.changeset/22360-declared-position-package-provenance.md`: `@objectstack/plugin-security: minor`. It carries a **BREAKING** banner, `Clause-②: no (narrowing)`, the remedy (change it in the package, or clone it under a new name), and ADR-0087 `not-required (no-migration-prescription)`. The breaking change ships as `minor` under the launch-window convention. Verdict lines: - `check-changeset-no-major --base origin/main`: "✓ This diff introduces no `major` bump." - the same run with a `pull_request` event carrying this body's declaration line: "✓ LEVEL AXIS: this PR declares clause-② `no (narrowing)`, and no package whose `packages/**/src/**` it moves is graded `patch`." - `check-adr-0087-registration --base origin/main`: "✓ 1 declared-breaking changeset(s), each carrying an ADR-0087 disposition." ## Verification (HEAD `8c3e68a2b5`) - `@objectstack/plugin-security` full suite: 186 files, 3891 passed, 45 skipped. `typecheck` passed (tsc on src and scripts, plus `check:test-typecheck`; `--listFiles` counts the edited test). - dogfood, every file that reads or writes positions (18, including the new pin): 168 passed. `@objectstack/dogfood` `typecheck` passed (`--listFiles` counts the pin). - `node scripts/pm/dispatch-gates.mjs --commands --repo objectstack-ai/objectstack`: 69 commands, identical to the claim's reading, all exit 0. `check:dual-build-cjs-loads` first answered PREREQUISITE NOT MET (8 packages had no `dist/`); after building them (turbo, all cached) it answered exit 0. `--ran`: "69 derived, 69 run, 0 NOT-MEASURED, 0 UNRUN". - `pnpm check:error-status-conformance` (by hand): "✓ every derivable runtime status is documented, and every documented status is reachable." - eslint, narrowed to the 3 changed TypeScript files: `--format json` reports 3 files, 0 errors, 0 warnings. `--print-config` shows none ignored and no `parserOptions.project`/`projectService`, so type-aware linting is off and this diff cannot move a verdict on any file it does not touch. - Not merged with `origin/main`: the 2 commits since the branch point touch `service-storage`, the spec error-code ledger and one storage dogfood file, none of this diff's packages or behaviour. ## Acceptance notes - **The narrowing is wider than "accepted, then reverted".** With the gate unchanged, every admin-door update of a package-declared row is refused, not only the label and description. The measured `active` and `delegatable` edits used to persist. So Setup's Activate, Deactivate and Set as Default on such a position now answer 403, as they do on a built-in, and so does a delete. The changeset says so. The triage ruling covers `delegatable` explicitly, and ADR-0131 D6/D3 (managed items read-only, clonable) covers the rest; the report raises it for the seat to confirm. - **A Setup-created position later declared by a package becomes the package's** (measured above). Not widened, per the order. - `objects/sys-position.object.ts` (the comment above `reserved_identity_name`) still says the declared seeder "stamps no provenance at all (the row defaults to `admin`)". The rule's verdict is unchanged, because `package` is not exempt, but that premise is now stale. The file is outside this PR's fence and is left as is. - #15196 S7's branch (`6d127ed8c1`, no PR yet) has a dogfood case, "Q2", that expects `PATCH` of `manager`'s label to answer 200. With this change on `main` it answers 403 `PERMISSION_DENIED`. Whichever lands later reconciles. --- _Generated by [Claude Code](https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ)_ --------- Co-authored-by: Claude <noreply@anthropic.com>
1 parent 9fe7448 commit 00bee27

6 files changed

Lines changed: 746 additions & 18 deletions

File tree

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
---
2+
'@objectstack/plugin-security': minor
3+
---
4+
5+
fix(plugin-security)!: a definition edit or delete of a position a code package declares is refused at the data door, as the metadata door already refuses it; its row now carries package provenance, and its row state stays switchable
6+
7+
Clause-②: no (narrowing)
8+
9+
<!-- adr-0087: not-required (no-migration-prescription) No metadata moves: no spec key, authorable spelling, export, type or stored shape is added, removed, renamed or re-shaped. What changes is one column the boot seeder writes on its own rows (sys_position.managed_by, now package for a position a code package declares) and, through the system-row write gate, a runtime write door: an admin-door update of such a row's definition columns, and its delete, are refused, while a patch of its row state (active, is_default) passes. No stored row is left for `objectstack migrate meta` to convert, because the seeder corrects the stamp itself at the next boot. The other categories are closed on facts: the package publishes (not unpublished); no ADR-0087 id is named or touched (not registered or already-registered); and no TypeScript declaration moves (not runtime-interface-only or type-surface-only). -->
10+
11+
**BREAKING**: an accept-set narrowing of the `sys_position` data door, shipped as `minor` under the launch-window convention for breaking changes. It carries ADR-0131 D6 (no door edits a managed definition) and D3 (managed items are read-only and clonable).
12+
13+
**What was wrong.** The declared-position seeder wrote the row of a package-declared position with no provenance stamp, so the row carried the object default, `managed_by: 'admin'`, and the system-row write gate did not protect it. A Setup or data API edit of its label answered `200`, and the next boot wrote the declared label back over it with no message. The metadata door already refused the same edit with `403 NOT_OVERRIDABLE`.
14+
15+
**What is refused now.** A position is package-declared when the engine registry's artifact lookup answers a code package for its name, the same lookup the metadata door refuses a save from. On such a position's row, the admin door now answers `403 PERMISSION_DENIED`, with the message it already gives for a built-in position, to:
16+
17+
- an update that touches a definition column (`name`, `label`, `description`, `delegatable`) or any column other than the two row-state columns below. That covers single-record edits, `updateMany`, and filtered updates. Until now a label or description edit answered `200` and the next boot wrote the declaration back over it; a `delegatable` edit answered `200` and persisted.
18+
- a delete. Until now it answered `200` and the next boot created the row again. The other lifecycle writes (transfer, restore, purge) are refused as well.
19+
20+
**What stays switchable.** A patch that touches only row state (`active` and/or `is_default`) still passes, so Setup's Activate, Deactivate and Set as Default keep working on a package-declared position and persist across restarts. This is the rule a packaged permission set already follows: switching it off is not an edit of its definition. A filtered update passes on the same terms, as long as its filter reaches no built-in position.
21+
22+
System-context writes are unchanged: the boot seeder still refreshes the row's label and description from the declaration.
23+
24+
**What is unchanged.**
25+
26+
- A position an administrator created in Setup, and a position the environment authored through the metadata door, keep an unmanaged row (`admin`) and stay editable.
27+
- The built-in positions are refused exactly as before, a bare `active` or `is_default` patch included.
28+
- Assigning a position to users and binding permission sets to it are writes on other rows (`sys_user_position`, `sys_position_permission_set`), and this change does not touch them.
29+
30+
**At the first boot after upgrading.** The existing row of each package-declared position is re-stamped `package` in place when it carries `admin`, no value, or the legacy `user`. Only `managed_by` changes: the `active`, `is_default` and `delegatable` values an administrator set before the upgrade are kept. The boot's `declared positions seeded` info line counts these rows as `restampedPackageProvenance`. The seeder matches a row by name, as its label refresh always has, so a position created in Setup whose name a package declares later becomes that package's position: its definition is refused the same way, and its row state stays switchable.
31+
32+
**What to do.** To change a package-declared position's label, description or `delegatable`, change it in the package and publish a new version. To have a position you can edit in Setup, clone it under a new name with the Clone Position action, bind its permission sets, and assign the clone.

‎packages/plugins/plugin-security/src/bootstrap-declared-positions.test.ts‎

Lines changed: 113 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,8 +91,9 @@ describe('bootstrapDeclaredPositions (#2909 T2 — seed-only semantics locked)',
9191
expect(r.seeded).toBe(1);
9292
const row = ql.rows[0];
9393
expect(row).toMatchObject({ name: 'contributor', label: 'Contributor', active: true, is_default: false });
94-
// Provenance is NOT stamped by the declared seeder (bootstrapBuiltinRoles
95-
// owns the built-in anchors; declared positions carry the object default).
94+
// No code package holds this name (the list registry names none), so the
95+
// row gets no provenance stamp and carries the object default. The stamp
96+
// a package-held name gets is pinned in the #22360 block below.
9697
expect(row.managed_by).toBeUndefined();
9798
});
9899

@@ -297,3 +298,113 @@ describe('bootstrapDeclaredPositions — one catalog read, both sources (ADR-013
297298
expect(ql.rows).toEqual([]);
298299
});
299300
});
301+
302+
/**
303+
* [#22360] The row of a position a code package holds carries the package's
304+
* provenance, so the system-row write gate refuses an admin-door edit of it —
305+
* as the metadata door already refuses one (ADR-0131 D6). Before this the row
306+
* carried the object default (`admin`): a data-door label edit answered 200
307+
* and the next boot wrote the declaration back over it.
308+
*
309+
* The registry is the real `SchemaRegistry`, so "a package holds the name" is
310+
* the registry's own artifact lookup — the one the metadata door refuses a
311+
* save from — not a fixture's opinion.
312+
*/
313+
describe('bootstrapDeclaredPositions — package provenance on a package-held name (#22360)', () => {
314+
const PKG = 'com.example.pkg';
315+
const packaged = (...items: Array<Record<string, unknown>>) => {
316+
const registry = new SchemaRegistry();
317+
for (const item of items) registry.registerItem('position', { ...item }, 'name', PKG);
318+
return registry;
319+
};
320+
/** The payloads the pass handed to `update`, as it handed them. */
321+
const recordUpdates = (ql: ReturnType<typeof makeQl>) => {
322+
const sent: any[] = [];
323+
const update = ql.update.bind(ql);
324+
ql.update = async (object: string, data: any) => { sent.push({ object, data: { ...data } }); return update(object, data); };
325+
return sent;
326+
};
327+
328+
it('stamps a new row package-managed', async () => {
329+
const ql = makeQl([], packaged({ name: 'field_rep', label: 'Field Rep' }));
330+
const r = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
331+
expect(ql.rows.map((row) => [row.name, row.managed_by])).toEqual([['field_rep', 'package']]);
332+
expect(r).toEqual({ seeded: 1, updated: 0, unchanged: 0, unreadable: 0 });
333+
});
334+
335+
it('corrects an upgraded deployment\'s admin-stamped row in place: managed_by and nothing else', async () => {
336+
const ql = makeQl([], packaged({ name: 'field_rep', label: 'Field Rep', description: 'In the field' }));
337+
// The row a pre-fix boot wrote, with the columns an administrator set since.
338+
const before = {
339+
id: 'pos_1', name: 'field_rep', label: 'Field Rep', description: 'In the field',
340+
managed_by: 'admin', active: false, is_default: true, delegatable: true,
341+
};
342+
ql.rows.push({ ...before });
343+
const sent = recordUpdates(ql);
344+
const r = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
345+
expect(sent).toEqual([{ object: 'sys_position', data: { id: 'pos_1', managed_by: 'package' } }]);
346+
expect(ql.rows).toEqual([{ ...before, managed_by: 'package' }]);
347+
expect(r).toEqual({ seeded: 0, updated: 1, unchanged: 0, unreadable: 0 });
348+
});
349+
350+
it('refreshes drifted display text and corrects the stamp in one write', async () => {
351+
const ql = makeQl([], packaged({ name: 'field_rep', label: 'Field Rep v2' }));
352+
ql.rows.push({ id: 'pos_1', name: 'field_rep', label: 'Field Rep', description: null, managed_by: 'admin' });
353+
const sent = recordUpdates(ql);
354+
await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
355+
expect(sent.map((s) => s.data)).toEqual([
356+
{ id: 'pos_1', label: 'Field Rep v2', description: null, managed_by: 'package' },
357+
]);
358+
});
359+
360+
it('a stamped row is left alone by the next pass', async () => {
361+
const ql = makeQl([], packaged({ name: 'field_rep', label: 'Field Rep' }));
362+
await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
363+
const sent = recordUpdates(ql);
364+
const again = await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
365+
expect(sent).toEqual([]);
366+
expect(again).toEqual({ seeded: 0, updated: 0, unchanged: 1, unreadable: 0 });
367+
});
368+
369+
it('a row the write gate already treats as managed keeps its value', async () => {
370+
const names = ['kept_platform', 'kept_system', 'kept_config'];
371+
const ql = makeQl([], packaged(...names.map((name) => ({ name, label: name }))));
372+
ql.rows.push(
373+
{ id: 'pos_p', name: 'kept_platform', label: 'kept_platform', description: null, managed_by: 'platform' },
374+
{ id: 'pos_s', name: 'kept_system', label: 'kept_system', description: null, managed_by: 'system' },
375+
{ id: 'pos_c', name: 'kept_config', label: 'kept_config', description: null, managed_by: 'config' },
376+
);
377+
const sent = recordUpdates(ql);
378+
await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
379+
expect(sent).toEqual([]);
380+
expect(ql.rows.map((row) => row.managed_by)).toEqual(['platform', 'system', 'config']);
381+
});
382+
383+
// Control: a position the environment authored through the metadata door is
384+
// hydrated into the registry with no package, and that door lets its author
385+
// edit it — so its row stays unmanaged, exactly as before.
386+
it('control: an environment-authored definition keeps an unmanaged row', async () => {
387+
const registry = new SchemaRegistry();
388+
registry.registerItem('position', { name: 'door_authored', label: 'Door Authored' }, 'name');
389+
registry.registerItem('position', { name: 'door_kept', label: 'Door Kept' }, 'name');
390+
const ql = makeQl([], registry);
391+
ql.rows.push({ id: 'pos_k', name: 'door_kept', label: 'Door Kept', description: null, managed_by: 'admin' });
392+
const sent = recordUpdates(ql);
393+
await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
394+
expect(sent).toEqual([]);
395+
const byName = Object.fromEntries(ql.rows.map((row) => [row.name, row.managed_by]));
396+
expect(byName).toEqual({ door_kept: 'admin', door_authored: undefined });
397+
});
398+
399+
// A registry without the artifact lookup is asked the way the metadata door
400+
// asks one: the definition names a package, and not the rehydration sentinel.
401+
it('a registry without the artifact lookup: a named package stamps, the sys_metadata sentinel does not', async () => {
402+
const ql = makeQl([
403+
{ name: 'shipped', label: 'Shipped', _packageId: PKG },
404+
{ name: 'rehydrated', label: 'Rehydrated', _packageId: 'sys_metadata' },
405+
]);
406+
await bootstrapDeclaredPositions(ql, NO_METADATA_POSITIONS);
407+
const byName = Object.fromEntries(ql.rows.map((row) => [row.name, row.managed_by]));
408+
expect(byName).toEqual({ shipped: 'package', rehydrated: undefined });
409+
});
410+
});

0 commit comments

Comments
 (0)