Skip to content

Commit 5774a75

Browse files
os-zhuangclaude
andauthored
feat(plugin-security): built-in row-write guardrail for sys_position / sys_capability managed rows (#2930)
Platform/application-published system rows (sys_position `managed_by` system/config, sys_capability `managed_by` platform/package) could be deleted or rewritten directly by a customer admin — the write went straight to the driver with no guardrail, silently breaking an app's authorization baseline. sys_permission_set already had the two-doors `assertPackageManagedWriteGate`; sys_position / sys_capability had no equivalent (ADR-0049: a provenance attribute that exists but is never enforced is exactly the gap to close). Add `assertSystemRowWriteGate`, wired into the data-write hook next to the package gate, as an unconditional data-layer boundary over both asset objects: - Refuse forging platform/package `managed_by` through the admin door (insert or update, single object or array) — closes update-to-forge. - Refuse delete / update / transfer / restore / purge on rows whose existing `managed_by` is platform/package-owned. Unlike sys_permission_set these objects have no ADR-0094 overlay write-through, so the refusal holds here rather than deferring downstream. - Admin-authored rows (`user`/∅ on sys_position, `admin` on sys_capability) are untouched — the admin fully owns those, incl. a delegate's rows in their own subtree. The gate fails closed and never depends on the caller's grants, so a superuser with modifyAllRecords cannot delete a platform position. The two objects' `managed_by` vocabularies differ, so the gate keys off a per-object provenance spec. Deny text is business-message only. No conflict with the delegated-admin gate — GOVERNED_OBJECTS excludes both objects, which govern the RBAC link tables, not the definitions. Tests: non-admin/admin × managed/admin-authored × delete/update matrix, forge + update-to-forge + bulk-array cases, filter-write probe, and the isSystem seeder bypass. Closes #2918 Claude-Session: https://claude.ai/code/session_01D5TEu5LzF1aZdE9rRaF6tV Co-authored-by: Claude <noreply@anthropic.com>
1 parent 891ea81 commit 5774a75

3 files changed

Lines changed: 351 additions & 0 deletions

File tree

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,17 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
内置行写护栏:`sys_position` / `sys_capability` 的平台/应用托管行不再可被客户管理员删改。
6+
7+
`sys_permission_set` 早有两道门写护栏(`assertPackageManagedWriteGate`)拦截对 package 托管行的写入,但 `sys_position` / `sys_capability` 缺失对应保护——平台/应用发布的系统岗位与能力(provenance 记录在 `managed_by`)可被管理员直接 delete / update 直达驱动,静默破坏应用的授权基线(ADR-0049:provenance 字段存在却无强制 = 正是要补的 enforcement gap)。
8+
9+
新增 **`assertSystemRowWriteGate`**`packages/plugins/plugin-security/src/security-plugin.ts`,data-write hook 接线与 package 门同处),对这两个对象的托管行施加一道无条件的数据层边界:
10+
11+
- **禁止伪造托管来源**:管理员门的 insert / update 载荷(单对象或数组)不得把 `managed_by` 盖成平台/应用值——只有携带 `isSystem` 的平台 seeder / 包发布路径可写;同时封堵 update-to-forge(把自建行改badge成托管行)。
12+
- **拒绝改删托管行**:对 `managed_by` 已是平台/应用值的行,`delete` / `update` / `transfer` / `restore` / `purge` 一律拒绝。与 `sys_permission_set` 不同,这两个对象没有 ADR-0094 overlay write-through,故写护栏必须在此层直接拒绝,而非下放给下游翻译。
13+
- **管理员自建行不受限**`managed_by``user`/∅(sys_position)或 `admin`(sys_capability)的行完全归管理员所有(含委派管理员在自己 subtree 内的自建行)。
14+
15+
护栏 fail-closed 且不依赖调用方授权——持 `modifyAllRecords` 的超管也无法删除平台岗位。两对象的 `managed_by` 词表不同(sys_position:`system`/`config` 托管,`user`/∅ 自建;sys_capability:`platform`/`package` 托管,`admin` 自建),网关按对象分别判定。错误信息仅含业务文案("此岗位/能力由 平台|应用包 提供,不可删除/修改")。
16+
17+
与 delegated-admin 边界不冲突:`GOVERNED_OBJECTS` 本就不含这两个对象,委派管理仍治理 RBAC 链接表而非定义对象。

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

Lines changed: 182 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1215,6 +1215,188 @@ describe('SecurityPlugin', () => {
12151215
).resolves.toBeDefined();
12161216
});
12171217
});
1218+
1219+
// ── ADR-0066 / #2918 — built-in row-write guardrail (sys_position / sys_capability) ──
1220+
// Platform/application-managed asset rows are not the admin's to delete or
1221+
// rewrite. Unlike sys_permission_set these objects have no ADR-0094 overlay
1222+
// write-through, so the refusal must hold for update/delete at this gate.
1223+
// Matrix: non-admin/admin × managed/admin-authored × delete/update.
1224+
describe('system-row write gate (sys_position / sys_capability provenance)', () => {
1225+
const adminSet: PermissionSet = {
1226+
name: 'admin_full_access', label: 'Admin',
1227+
objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, modifyAllRecords: true } },
1228+
} as any;
1229+
const adminCtx = { userId: 'admin1', tenantId: 'org-1', positions: [], permissions: ['admin_full_access'] };
1230+
1231+
const runGate = async (opCtx: any, findOneImpl?: (q: any) => any) => {
1232+
const plugin = new SecurityPlugin({ fallbackPermissionSet: 'admin_full_access' });
1233+
const harness = makeMiddlewareCtx({
1234+
permissionSets: [adminSet],
1235+
objectFields: ['id', 'name', 'label', 'managed_by'],
1236+
...(findOneImpl ? { findOneImpl } : {}),
1237+
});
1238+
await plugin.init(harness.ctx);
1239+
await plugin.start(harness.ctx);
1240+
return harness.run(opCtx);
1241+
};
1242+
1243+
it('DENIES deleting a platform-managed position (sys_position managed_by:system)', async () => {
1244+
const opCtx: any = {
1245+
object: 'sys_position', operation: 'delete',
1246+
options: { where: { id: 'pos_admin' } }, context: adminCtx,
1247+
};
1248+
await expect(
1249+
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })),
1250+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1251+
});
1252+
1253+
it('DENIES updating a package-declared position (sys_position managed_by:config)', async () => {
1254+
const opCtx: any = {
1255+
object: 'sys_position', operation: 'update',
1256+
data: { id: 'pos_sales', label: 'renamed' }, options: { where: { id: 'pos_sales' } },
1257+
context: adminCtx,
1258+
};
1259+
await expect(
1260+
runGate(opCtx, () => ({ id: 'pos_sales', name: 'sales_rep', managed_by: 'config' })),
1261+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1262+
});
1263+
1264+
it('DENIES deleting a platform-owned capability (sys_capability managed_by:platform)', async () => {
1265+
const opCtx: any = {
1266+
object: 'sys_capability', operation: 'delete',
1267+
options: { where: { id: 'cap_plat' } }, context: adminCtx,
1268+
};
1269+
await expect(
1270+
runGate(opCtx, () => ({ id: 'cap_plat', name: 'manage_users', managed_by: 'platform' })),
1271+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1272+
});
1273+
1274+
it('DENIES updating a package-owned capability (sys_capability managed_by:package)', async () => {
1275+
const opCtx: any = {
1276+
object: 'sys_capability', operation: 'update',
1277+
data: { id: 'cap_pkg', label: 'renamed' }, options: { where: { id: 'cap_pkg' } },
1278+
context: adminCtx,
1279+
};
1280+
await expect(
1281+
runGate(opCtx, () => ({ id: 'cap_pkg', name: 'crm.export', managed_by: 'package' })),
1282+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1283+
});
1284+
1285+
it('ALLOWS deleting an admin-authored position (sys_position managed_by:user)', async () => {
1286+
const opCtx: any = {
1287+
object: 'sys_position', operation: 'delete',
1288+
options: { where: { id: 'pos_user' } }, context: adminCtx,
1289+
};
1290+
await expect(
1291+
runGate(opCtx, () => ({ id: 'pos_user', name: 'my_team', managed_by: 'user' })),
1292+
).resolves.toBeDefined();
1293+
});
1294+
1295+
it('ALLOWS updating an admin-authored position with NO managed_by (tenant-created)', async () => {
1296+
const opCtx: any = {
1297+
object: 'sys_position', operation: 'update',
1298+
data: { id: 'pos_user', label: 'renamed' }, options: { where: { id: 'pos_user' } },
1299+
context: adminCtx,
1300+
};
1301+
await expect(
1302+
runGate(opCtx, () => ({ id: 'pos_user', name: 'my_team', managed_by: null })),
1303+
).resolves.toBeDefined();
1304+
});
1305+
1306+
it('ALLOWS updating an admin-authored capability (sys_capability managed_by:admin)', async () => {
1307+
const opCtx: any = {
1308+
object: 'sys_capability', operation: 'update',
1309+
data: { id: 'cap_admin', label: 'renamed' }, options: { where: { id: 'cap_admin' } },
1310+
context: adminCtx,
1311+
};
1312+
await expect(
1313+
runGate(opCtx, () => ({ id: 'cap_admin', name: 'my_cap', managed_by: 'admin' })),
1314+
).resolves.toBeDefined();
1315+
});
1316+
1317+
it('DENIES an admin-door insert that forges platform provenance (sys_position managed_by:system)', async () => {
1318+
const opCtx: any = {
1319+
object: 'sys_position', operation: 'insert',
1320+
data: { name: 'forged', managed_by: 'system' }, context: adminCtx,
1321+
};
1322+
await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1323+
});
1324+
1325+
it('DENIES a bulk/ARRAY insert that forges package provenance on any element (sys_capability)', async () => {
1326+
const opCtx: any = {
1327+
object: 'sys_capability', operation: 'insert',
1328+
data: [
1329+
{ name: 'ok_admin_cap' },
1330+
{ name: 'forged', managed_by: 'package' },
1331+
],
1332+
context: adminCtx,
1333+
};
1334+
await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1335+
});
1336+
1337+
it('DENIES an update that RE-BADGES an admin row as package-managed (update-to-forge)', async () => {
1338+
const opCtx: any = {
1339+
object: 'sys_capability', operation: 'update',
1340+
data: { id: 'cap_admin', managed_by: 'package' }, options: { where: { id: 'cap_admin' } },
1341+
context: adminCtx,
1342+
};
1343+
await expect(
1344+
runGate(opCtx, () => ({ id: 'cap_admin', name: 'my_cap', managed_by: 'admin' })),
1345+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1346+
});
1347+
1348+
it('ALLOWS a normal admin-door capability insert (no forged provenance)', async () => {
1349+
const opCtx: any = {
1350+
object: 'sys_capability', operation: 'insert',
1351+
data: { name: 'my_cap', label: 'Custom', managed_by: 'admin' },
1352+
context: adminCtx,
1353+
};
1354+
await expect(runGate(opCtx)).resolves.toBeDefined();
1355+
});
1356+
1357+
it('DENIES a filter DELETE whose filter matches a platform/package-managed row', async () => {
1358+
const opCtx: any = {
1359+
object: 'sys_position', operation: 'delete',
1360+
options: { where: { active: false } }, // no single id → filter path
1361+
context: adminCtx,
1362+
};
1363+
await expect(
1364+
runGate(opCtx, () => ({ id: 'pos_admin', managed_by: 'system' })),
1365+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1366+
});
1367+
1368+
it('ALLOWS a filter delete that matches only admin-authored rows (probe finds none)', async () => {
1369+
const opCtx: any = {
1370+
object: 'sys_position', operation: 'delete',
1371+
options: { where: { managed_by: 'user' } },
1372+
context: adminCtx,
1373+
};
1374+
// The managed-row probe finds nothing → the write proceeds.
1375+
await expect(runGate(opCtx, () => null)).resolves.toBeDefined();
1376+
});
1377+
1378+
it('DENIES even a principal-less delete of a managed row (gate is before the fall-open)', async () => {
1379+
const opCtx: any = {
1380+
object: 'sys_position', operation: 'delete',
1381+
options: { where: { id: 'pos_admin' } },
1382+
context: {}, // no roles, no permissions, no userId, not isSystem
1383+
};
1384+
await expect(
1385+
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })),
1386+
).rejects.toMatchObject({ name: 'PermissionDeniedError' });
1387+
});
1388+
1389+
it('lets system/boot writes through (isSystem bypass) even on a platform-managed position', async () => {
1390+
const opCtx: any = {
1391+
object: 'sys_position', operation: 'update',
1392+
data: { id: 'pos_admin', label: 'reseed' }, options: { where: { id: 'pos_admin' } },
1393+
context: { isSystem: true },
1394+
};
1395+
await expect(
1396+
runGate(opCtx, () => ({ id: 'pos_admin', name: 'platform_admin', managed_by: 'system' })),
1397+
).resolves.toBeDefined();
1398+
});
1399+
});
12181400
});
12191401
// ---------------------------------------------------------------------------
12201402
describe('PermissionEvaluator', () => {

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

Lines changed: 152 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,36 @@ const EMPTY_REQUIRED_PERMISSIONS: NormalizedRequiredPermissions = Object.freeze(
7676
all: [], read: [], create: [], update: [], delete: [],
7777
}) as NormalizedRequiredPermissions;
7878

79+
/**
80+
* [ADR-0066 / #2918] Provenance spec for the platform/application asset objects
81+
* whose managed rows are write-protected by {@link SecurityPlugin.assertSystemRowWriteGate}.
82+
*
83+
* The two objects use DIFFERENT `managed_by` vocabularies but the same ownership
84+
* idea — a row authored by the platform or an application package is not the
85+
* admin's to delete or rewrite:
86+
* • sys_position.managed_by — `system` (platform built-in) / `config`
87+
* (package declared) are managed; `user`/∅ (tenant-authored) are the admin's.
88+
* • sys_capability.managed_by — `platform` / `package` are managed; `admin`
89+
* (created in Setup) is the admin's.
90+
* The map value for each managed `managed_by` is the human owner label used in
91+
* the (business-message-only) deny text.
92+
*/
93+
const SYSTEM_ROW_PROVENANCE: Record<
94+
string,
95+
{ noun: string; pluralNoun: string; managed: Record<string, string> }
96+
> = {
97+
sys_position: {
98+
noun: 'position',
99+
pluralNoun: 'positions',
100+
managed: { system: 'the platform', config: 'an application package' },
101+
},
102+
sys_capability: {
103+
noun: 'capability',
104+
pluralNoun: 'capabilities',
105+
managed: { platform: 'the platform', package: 'an application package' },
106+
},
107+
};
108+
79109
/** Normalize a raw object `requiredPermissions` (string[] | per-op map) into buckets. */
80110
function normalizeRequiredPermissions(raw: unknown): NormalizedRequiredPermissions {
81111
if (Array.isArray(raw)) {
@@ -509,6 +539,20 @@ export class SecurityPlugin implements Plugin {
509539
// and already short-circuited the whole middleware above.
510540
await this.assertPackageManagedWriteGate(opCtx);
511541

542+
// [ADR-0066 / #2918] Built-in row-write guardrail for the platform/app
543+
// ASSET objects sys_position / sys_capability. Like the package gate
544+
// above, an unconditional data-layer boundary: a row authored by the
545+
// platform or an application package (provenance recorded in
546+
// `managed_by`) may not be deleted or rewritten through the admin door.
547+
// Unlike sys_permission_set there is NO ADR-0094 overlay write-through
548+
// for these objects, so the refusal must hold for update/delete here
549+
// rather than deferring to a downstream translation. Runs BEFORE the
550+
// empty-principal fall-open and the CRUD check so the boundary holds even
551+
// for a principal-less context and a superuser with modifyAllRecords.
552+
// System/boot writes carry `isSystem` and already short-circuited above,
553+
// so the seeder and package publish are unaffected.
554+
await this.assertSystemRowWriteGate(opCtx);
555+
512556
// [ADR-0090 D5/D9] Audience-anchor binding guard — like the package
513557
// gate above, an unconditional data-layer boundary: a permission set
514558
// carrying high-privilege bits must never be bound to the `everyone`
@@ -1701,6 +1745,114 @@ export class SecurityPlugin implements Plugin {
17011745
}
17021746
}
17031747

1748+
/**
1749+
* [ADR-0066 / #2918] Built-in row-write guardrail for the platform/application
1750+
* ASSET objects `sys_position` and `sys_capability`.
1751+
*
1752+
* ADR-0066's asset-ownership model splits authoring from assignment: WHAT a
1753+
* position or capability *is* is defined by the platform or an application
1754+
* package developer; a customer admin only decides WHO it is assigned to (via
1755+
* the RBAC link tables, which are governed separately by the delegated-admin
1756+
* gate). The `managed_by` provenance column on each object already records
1757+
* that ownership, but until now nothing ENFORCED it at the data layer — an
1758+
* admin could delete or rewrite a platform/package-managed row and silently
1759+
* break that app's authorization baseline (ADR-0049: a provenance attribute
1760+
* that exists but is never enforced is exactly the gap to close).
1761+
*
1762+
* This gate is an unconditional data-layer boundary, mirroring the
1763+
* `sys_permission_set` two-doors gate above:
1764+
* (a) The admin door may never FORGE managed provenance — stamping
1765+
* `managed_by` to a platform/package value on insert OR update (single
1766+
* object OR array) is refused; only the platform seeder / package
1767+
* publish path (which carries `isSystem` and short-circuited the whole
1768+
* middleware above) may author it. This also closes update-to-forge.
1769+
* (b) delete / update / transfer / restore / purge on a row whose EXISTING
1770+
* `managed_by` is platform/package-owned are refused — unlike
1771+
* `sys_permission_set`, these objects have NO ADR-0094 overlay
1772+
* write-through, so the mutation would otherwise go straight to the
1773+
* driver.
1774+
* (c) admin-authored rows (`managed_by` user/∅/admin) are untouched — the
1775+
* admin fully owns those (incl. a delegate's rows in their own subtree).
1776+
* Fails CLOSED and never depends on the caller's grants, so a superuser with
1777+
* modifyAllRecords cannot delete a platform position either.
1778+
*/
1779+
private async assertSystemRowWriteGate(opCtx: any): Promise<void> {
1780+
const spec = SYSTEM_ROW_PROVENANCE[opCtx?.object as string];
1781+
if (!spec) return;
1782+
const op = opCtx.operation;
1783+
if (!['insert', 'update', 'delete', 'transfer', 'restore', 'purge'].includes(op)) return;
1784+
1785+
const managedValues = Object.keys(spec.managed);
1786+
1787+
// (a) Reject any admin-door PAYLOAD that CLAIMS platform/package provenance,
1788+
// on insert OR update, single object OR array. Only the seeder / publish
1789+
// path (which carries `isSystem` and short-circuited above) may stamp it.
1790+
const payloadRows = Array.isArray(opCtx.data)
1791+
? opCtx.data
1792+
: (opCtx.data && typeof opCtx.data === 'object' ? [opCtx.data] : []);
1793+
if (
1794+
payloadRows.some(
1795+
(r: unknown) =>
1796+
r &&
1797+
typeof r === 'object' &&
1798+
managedValues.includes(String((r as Record<string, unknown>).managed_by ?? '')),
1799+
)
1800+
) {
1801+
throw new PermissionDeniedError(
1802+
`[Security] Access denied: cannot stamp a platform/package 'managed_by' value on a ${spec.noun} ` +
1803+
`through the admin door — ${spec.pluralNoun} provided by the platform or an application package are ` +
1804+
`authored there and land via seeding/publish, not through Setup (ADR-0066 asset ownership).`,
1805+
{ operation: op, object: opCtx.object },
1806+
);
1807+
}
1808+
if (op === 'insert') return; // no existing row to protect
1809+
1810+
if (!this.ql) return;
1811+
1812+
const targetId = this.extractSingleId(opCtx);
1813+
if (targetId == null) {
1814+
// Multi-row / filter write with no single id. Deny ONLY if a managed row
1815+
// actually falls within the write's own filter — so a bulk edit that
1816+
// targets only admin-authored rows still succeeds (no over-broad block). A
1817+
// whole-table write (no filter) matches every managed row, so it is denied.
1818+
const writeWhere = opCtx?.options?.where;
1819+
const managedWhere =
1820+
writeWhere && typeof writeWhere === 'object'
1821+
? { $and: [writeWhere, { managed_by: { $in: managedValues } }] }
1822+
: { managed_by: { $in: managedValues } };
1823+
const hitsManagedRow = await this.ql
1824+
.findOne(opCtx.object, { where: managedWhere, context: { isSystem: true } })
1825+
.catch(() => null);
1826+
if (hitsManagedRow) {
1827+
throw new PermissionDeniedError(
1828+
`[Security] Access denied: this '${op}' on '${opCtx.object}' targets one or more ${spec.pluralNoun} ` +
1829+
`provided by the platform or an application package — those cannot be deleted or modified through ` +
1830+
`the admin door (ADR-0066 asset ownership).`,
1831+
{ operation: op, object: opCtx.object },
1832+
);
1833+
}
1834+
return;
1835+
}
1836+
1837+
const existing = await this.ql
1838+
.findOne(opCtx.object, { where: { id: targetId }, context: { isSystem: true } })
1839+
.catch(() => null);
1840+
const existingManagedBy = existing
1841+
? String((existing as Record<string, unknown>).managed_by ?? '')
1842+
: '';
1843+
const ownerLabel = spec.managed[existingManagedBy];
1844+
if (existing && ownerLabel) {
1845+
const row = existing as Record<string, unknown>;
1846+
const source = ownerLabel === 'the platform' ? 'platform definition' : 'application package';
1847+
throw new PermissionDeniedError(
1848+
`[Security] Access denied: '${String(row.name ?? row.label ?? targetId)}' is a ${spec.noun} provided ` +
1849+
`by ${ownerLabel} — it cannot be deleted or modified through the admin door. Change it by editing ` +
1850+
`its ${source} and re-publishing (ADR-0066 asset ownership).`,
1851+
{ operation: op, object: opCtx.object, recordId: targetId, managedBy: existingManagedBy },
1852+
);
1853+
}
1854+
}
1855+
17041856
private extractSingleId(opCtx: any): string | number | bigint | null {
17051857
const isScalar = (v: unknown): v is string | number | bigint =>
17061858
v !== null && (typeof v === 'string' || typeof v === 'number' || typeof v === 'bigint');

0 commit comments

Comments
 (0)