Skip to content

Commit 4e0d6b2

Browse files
committed
fix(plugin-security): protect platform/package-stamped sys_position rows (#2926 ①)
The system-row write gate (#2918/#2930) keyed sys_position provenance on the legacy managed_by values (system/config), but the A4 vocab unification (#2934) stamps and boot-normalizes rows to platform/package — so the gate silently stopped firing for positions, letting admins physically delete the everyone/ guest audience anchors once their bindings were removed. Guard both the canonical and legacy vocabularies, update the gate tests to the canonical values, add legacy-row regressions, and correct the stale 'no runtime path branches on legacy values' safety comments. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017bJWtKmZ2mFpRFAmmmoeqe
1 parent b96ddab commit 4e0d6b2

5 files changed

Lines changed: 88 additions & 27 deletions

File tree

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
fix(plugin-security): re-arm the sys_position system-row write gate after the A4 managed_by rename (#2926 ①). The gate's provenance map still keyed on the legacy `system`/`config` values while rows are now stamped (and boot-normalized to) `platform`/`package`, so platform/package-managed positions — including the `everyone`/`guest` audience anchors — could be physically deleted through the data API once their bindings were removed. The map now guards both the canonical and legacy vocabularies, and the misleading "no runtime path branches on legacy values" safety notes were corrected.

packages/plugins/plugin-security/src/normalize-managed-by.ts

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,12 @@
1717
* `'config'` / `'user'`. Built-in position rows and declared package sets
1818
* self-heal on their own bootstrap upsert, so this only mops up the rest.
1919
*
20-
* Safe by construction: NO runtime path branches on the legacy values (every
21-
* read keys on `'package'` or `'platform'`, both unchanged by the rename), so
22-
* this is a pure display-vocabulary migration — it never changes an access
23-
* decision. Idempotent: canonical rows are skipped, so a re-run is a no-op.
20+
* NOT purely cosmetic: the system-row write gate's provenance map
21+
* (SYSTEM_ROW_PROVENANCE in security-plugin.ts) branches on `managed_by`
22+
* values, so it must recognize BOTH the canonical and the legacy vocabulary —
23+
* renaming a stored value without updating that map silently disarms the gate
24+
* (#2926 ①). Keep the two in lockstep whenever this vocabulary changes.
25+
* Idempotent: canonical rows are skipped, so a re-run is a no-op.
2426
* Best-effort and non-fatal, like the sibling boot reconcilers.
2527
*
2628
* Runs on `kernel:ready` after the seeders, as `isSystem` (the field is

packages/plugins/plugin-security/src/objects/sys-position.object.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -224,9 +224,10 @@ export const SysPosition = ObjectSchema.create({
224224
// built-in identity position (seeded by bootstrapBuiltinRoles, read-only);
225225
// `package` = stack/package-declared; `admin` = tenant-created in Setup.
226226
// Back-compat: legacy rows may carry system (== platform) / config (== package)
227-
// / user (== admin); no runtime path branches on those (every read keys on
228-
// 'package' / 'platform'), and the boot normalizer heals them to the canonical
229-
// vocab. Built-in (`platform`) rows self-heal on the next bootstrap upsert.
227+
// / user (== admin); the boot normalizer heals them to the canonical vocab, and
228+
// the system-row write gate (SYSTEM_ROW_PROVENANCE, security-plugin.ts) guards
229+
// both vocabularies — keep it in lockstep with any change here (#2926 ①).
230+
// Built-in (`platform`) rows self-heal on the next bootstrap upsert.
230231
managed_by: Field.select({
231232
label: 'Managed By',
232233
readonly: true,

packages/plugins/plugin-security/src/security-plugin.test.ts

Lines changed: 54 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -1428,24 +1428,48 @@ describe('SecurityPlugin', () => {
14281428
return harness.run(opCtx);
14291429
};
14301430

1431-
it('DENIES deleting a platform-managed position (sys_position managed_by:system)', async () => {
1431+
it('DENIES deleting a platform-managed position (sys_position managed_by:platform)', async () => {
14321432
const opCtx: any = {
14331433
object: 'sys_position', operation: 'delete',
14341434
options: { where: { id: 'pos_admin' } }, context: adminCtx,
14351435
};
14361436
await expect(
1437-
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })),
1437+
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'platform' })),
14381438
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
14391439
});
14401440

1441-
it('DENIES updating a package-declared position (sys_position managed_by:config)', async () => {
1441+
it('DENIES updating a package-declared position (sys_position managed_by:package)', async () => {
14421442
const opCtx: any = {
14431443
object: 'sys_position', operation: 'update',
14441444
data: { id: 'pos_sales', label: 'renamed' }, options: { where: { id: 'pos_sales' } },
14451445
context: adminCtx,
14461446
};
14471447
await expect(
1448-
runGate(opCtx, () => ({ id: 'pos_sales', name: 'sales_rep', managed_by: 'config' })),
1448+
runGate(opCtx, () => ({ id: 'pos_sales', name: 'sales_rep', managed_by: 'package' })),
1449+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1450+
});
1451+
1452+
// [#2926 ①] Regression: the A4 rename (system→platform, config→package) once
1453+
// disarmed this gate because the provenance map only knew the legacy values.
1454+
// Rows the boot normalizer has not healed yet must STAY protected too.
1455+
it('KEEPS denying legacy-vocab managed positions (managed_by:system, pre-normalizer rows)', async () => {
1456+
const opCtx: any = {
1457+
object: 'sys_position', operation: 'delete',
1458+
options: { where: { id: 'pos_legacy' } }, context: adminCtx,
1459+
};
1460+
await expect(
1461+
runGate(opCtx, () => ({ id: 'pos_legacy', name: 'everyone', managed_by: 'system' })),
1462+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1463+
});
1464+
1465+
it('KEEPS denying legacy-vocab managed positions (managed_by:config, pre-normalizer rows)', async () => {
1466+
const opCtx: any = {
1467+
object: 'sys_position', operation: 'update',
1468+
data: { id: 'pos_legacy2', label: 'renamed' }, options: { where: { id: 'pos_legacy2' } },
1469+
context: adminCtx,
1470+
};
1471+
await expect(
1472+
runGate(opCtx, () => ({ id: 'pos_legacy2', name: 'sales_rep', managed_by: 'config' })),
14491473
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
14501474
});
14511475

@@ -1470,13 +1494,23 @@ describe('SecurityPlugin', () => {
14701494
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
14711495
});
14721496

1473-
it('ALLOWS deleting an admin-authored position (sys_position managed_by:user)', async () => {
1497+
it('ALLOWS deleting an admin-authored position (sys_position managed_by:admin)', async () => {
14741498
const opCtx: any = {
14751499
object: 'sys_position', operation: 'delete',
14761500
options: { where: { id: 'pos_user' } }, context: adminCtx,
14771501
};
14781502
await expect(
1479-
runGate(opCtx, () => ({ id: 'pos_user', name: 'my_team', managed_by: 'user' })),
1503+
runGate(opCtx, () => ({ id: 'pos_user', name: 'my_team', managed_by: 'admin' })),
1504+
).resolves.toBeDefined();
1505+
});
1506+
1507+
it('ALLOWS deleting a legacy admin-authored position (managed_by:user, pre-normalizer)', async () => {
1508+
const opCtx: any = {
1509+
object: 'sys_position', operation: 'delete',
1510+
options: { where: { id: 'pos_user_legacy' } }, context: adminCtx,
1511+
};
1512+
await expect(
1513+
runGate(opCtx, () => ({ id: 'pos_user_legacy', name: 'my_team', managed_by: 'user' })),
14801514
).resolves.toBeDefined();
14811515
});
14821516

@@ -1502,10 +1536,18 @@ describe('SecurityPlugin', () => {
15021536
).resolves.toBeDefined();
15031537
});
15041538

1505-
it('DENIES an admin-door insert that forges platform provenance (sys_position managed_by:system)', async () => {
1539+
it('DENIES an admin-door insert that forges platform provenance (sys_position managed_by:platform)', async () => {
1540+
const opCtx: any = {
1541+
object: 'sys_position', operation: 'insert',
1542+
data: { name: 'forged', managed_by: 'platform' }, context: adminCtx,
1543+
};
1544+
await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1545+
});
1546+
1547+
it('DENIES an admin-door insert that forges LEGACY platform provenance (managed_by:system)', async () => {
15061548
const opCtx: any = {
15071549
object: 'sys_position', operation: 'insert',
1508-
data: { name: 'forged', managed_by: 'system' }, context: adminCtx,
1550+
data: { name: 'forged_legacy', managed_by: 'system' }, context: adminCtx,
15091551
};
15101552
await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' });
15111553
});
@@ -1549,14 +1591,14 @@ describe('SecurityPlugin', () => {
15491591
context: adminCtx,
15501592
};
15511593
await expect(
1552-
runGate(opCtx, () => ({ id: 'pos_admin', managed_by: 'system' })),
1594+
runGate(opCtx, () => ({ id: 'pos_admin', managed_by: 'platform' })),
15531595
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
15541596
});
15551597

15561598
it('ALLOWS a filter delete that matches only admin-authored rows (probe finds none)', async () => {
15571599
const opCtx: any = {
15581600
object: 'sys_position', operation: 'delete',
1559-
options: { where: { managed_by: 'user' } },
1601+
options: { where: { managed_by: 'admin' } },
15601602
context: adminCtx,
15611603
};
15621604
// The managed-row probe finds nothing → the write proceeds.
@@ -1570,7 +1612,7 @@ describe('SecurityPlugin', () => {
15701612
context: {}, // no roles, no permissions, no userId, not isSystem
15711613
};
15721614
await expect(
1573-
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })),
1615+
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'platform' })),
15741616
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
15751617
});
15761618

@@ -1581,7 +1623,7 @@ describe('SecurityPlugin', () => {
15811623
context: { isSystem: true },
15821624
};
15831625
await expect(
1584-
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })),
1626+
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'platform' })),
15851627
).resolves.toBeDefined();
15861628
});
15871629
});

packages/plugins/plugin-security/src/security-plugin.ts

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -113,13 +113,18 @@ const EMPTY_REQUIRED_PERMISSIONS: NormalizedRequiredPermissions = Object.freeze(
113113
* [ADR-0066 / #2918] Provenance spec for the platform/application asset objects
114114
* whose managed rows are write-protected by {@link SecurityPlugin.assertSystemRowWriteGate}.
115115
*
116-
* The two objects use DIFFERENT `managed_by` vocabularies but the same ownership
117-
* idea — a row authored by the platform or an application package is not the
118-
* admin's to delete or rewrite:
119-
* • sys_position.managed_by — `system` (platform built-in) / `config`
120-
* (package declared) are managed; `user`/∅ (tenant-authored) are the admin's.
121-
* • sys_capability.managed_by — `platform` / `package` are managed; `admin`
122-
* (created in Setup) is the admin's.
116+
* Both objects share the unified A4 (#2920) `managed_by` vocabulary — a row
117+
* authored by the platform or an application package is not the admin's to
118+
* delete or rewrite:
119+
* • `platform` / `package` are managed; `admin`/∅ (tenant-authored) rows are
120+
* the admin's.
121+
* • sys_position additionally keeps its LEGACY values `system` (→ platform)
122+
* and `config` (→ package) in the managed map: the boot normalizer
123+
* (normalize-managed-by.ts) heals stored rows to the canonical vocabulary,
124+
* but rows written before the normalizer runs — or in a store it has not
125+
* touched yet — must not lose protection in the interim. Dropping the
126+
* legacy keys here is what silently disarmed this gate for sys_position
127+
* after the A4 rename (#2926 ①).
123128
* The map value for each managed `managed_by` is the human owner label used in
124129
* the (business-message-only) deny text.
125130
*/
@@ -130,7 +135,13 @@ const SYSTEM_ROW_PROVENANCE: Record<
130135
sys_position: {
131136
noun: 'position',
132137
pluralNoun: 'positions',
133-
managed: { system: 'the platform', config: 'an application package' },
138+
managed: {
139+
platform: 'the platform',
140+
package: 'an application package',
141+
// Legacy pre-A4 values — keep guarded until every store is normalized.
142+
system: 'the platform',
143+
config: 'an application package',
144+
},
134145
},
135146
sys_capability: {
136147
noun: 'capability',

0 commit comments

Comments
 (0)