Skip to content

Commit 16fe3da

Browse files
committed
fix(plugin-security): a row-state-only patch of a package-provided position passes the system-row write gate
A package position row is locked the way a packaged permission set is: a patch touching only active and/or is_default on a sys_position row stamped package (or its legacy config spelling) passes, single-id or filtered, as permission-set-projection's write door passes a bare { active } patch on a packaged set. Definition columns (name, label, description, delegatable), any other column, delete and the other lifecycle verbs stay refused with the gate's code and message. Platform rows and sys_capability get no carve-out; the provenance-stamp refusal is unchanged. Claude-Session: https://claude.ai/code/session_01WkL6Eijt432S1Y7ekb6ovQ Co-authored-by: Claude <noreply@anthropic.com>
1 parent 73c02fd commit 16fe3da

2 files changed

Lines changed: 295 additions & 2 deletions

File tree

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

Lines changed: 69 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7359,6 +7359,10 @@ export class SecurityPlugin implements Plugin {
73597359
* driver.
73607360
* (c) admin-authored rows (`managed_by` user/∅/admin) are untouched — the
73617361
* admin fully owns those (incl. a delegate's rows in their own subtree).
7362+
* (d) [#22360] ONE carve-out from (b): an `update` of a PACKAGE-provided
7363+
* `sys_position` row whose patch touches only ROW STATE passes — see
7364+
* {@link SecurityPlugin.PACKAGE_POSITION_ROW_STATE_COLUMNS}. Platform
7365+
* rows and `sys_capability` get no carve-out.
73627366
* Fails CLOSED and never depends on the caller's grants, so a superuser with
73637367
* modifyAllRecords cannot delete a platform position either.
73647368
*/
@@ -7395,17 +7399,27 @@ export class SecurityPlugin implements Plugin {
73957399

73967400
if (!this.ql) return;
73977401

7402+
// (d) Whether this write is the row-state-only update the carve-out admits.
7403+
const rowStateOnly = SecurityPlugin.isPackagePositionRowStatePatch(opCtx);
7404+
73987405
const targetId = this.extractSingleId(opCtx);
73997406
if (targetId == null) {
74007407
// Multi-row / filter write with no single id. Deny ONLY if a managed row
74017408
// actually falls within the write's own filter — so a bulk edit that
74027409
// targets only admin-authored rows still succeeds (no over-broad block). A
74037410
// whole-table write (no filter) matches every managed row, so it is denied.
7411+
//
7412+
// (d) A row-state-only patch asks only about the rows the carve-out does
7413+
// not cover: package positions in the filter no longer refuse it, a
7414+
// platform position in the filter still does.
7415+
const guardedValues = rowStateOnly
7416+
? managedValues.filter((v) => !SecurityPlugin.PACKAGE_POSITION_PROVENANCE.has(v))
7417+
: managedValues;
74047418
const writeWhere = opCtx?.options?.where;
74057419
const managedWhere =
74067420
writeWhere && typeof writeWhere === 'object'
7407-
? { $and: [writeWhere, { managed_by: { $in: managedValues } }] }
7408-
: { managed_by: { $in: managedValues } };
7421+
? { $and: [writeWhere, { managed_by: { $in: guardedValues } }] }
7422+
: { managed_by: { $in: guardedValues } };
74097423
// [#7505] No `.catch(() => null)` — see the sibling gate: a swallowed
74107424
// fault made this asset guard answer "nothing managed matched" and admit
74117425
// the write. Fail-closed: propagate.
@@ -7427,6 +7441,8 @@ export class SecurityPlugin implements Plugin {
74277441
? String((existing as Record<string, unknown>).managed_by ?? '')
74287442
: '';
74297443
const ownerLabel = spec.managed[existingManagedBy];
7444+
// (d) The carve-out: a row-state-only patch of a package-provided position.
7445+
if (existing && rowStateOnly && SecurityPlugin.PACKAGE_POSITION_PROVENANCE.has(existingManagedBy)) return;
74307446
if (existing && ownerLabel) {
74317447
const row = existing as Record<string, unknown>;
74327448
const source = ownerLabel === 'the platform' ? 'platform definition' : 'application package';
@@ -7439,6 +7455,57 @@ export class SecurityPlugin implements Plugin {
74397455
}
74407456
}
74417457

7458+
/**
7459+
* [#22360] The ROW-STATE columns of `sys_position`: the one named allowlist
7460+
* the carve-out (d) of {@link SecurityPlugin.assertSystemRowWriteGate} admits
7461+
* on a package-provided position.
7462+
*
7463+
* A package position row is locked the way a packaged permission set is.
7464+
* `permission-set-projection.ts`'s write door (#4669) passes a bare
7465+
* `{ active }` patch on a packaged set straight through: switching a packaged
7466+
* set off is not a customization of it. Under #15196's Q2 = A,
7467+
* `sys_position.active` stays row-authoritative, so switching a package's
7468+
* position off — or making it the default for new users — is the operator's
7469+
* lever on the row, not an edit of the package's definition. Neither column
7470+
* is a `PositionSchema` key.
7471+
*
7472+
* Everything else stays refused, with the gate's own code and message:
7473+
* a definition column (`name`, `label`, `description`, `delegatable`), any
7474+
* column not named here, `delete`, and `transfer` / `restore` / `purge`.
7475+
* `delegatable` is deliberately NOT row state: it is declared on the position
7476+
* definition, and the triage ruling locks it ahead of #15196's S8b reader
7477+
* switch. ⛔ Widening this set is a ruling, not an edit.
7478+
*/
7479+
private static readonly PACKAGE_POSITION_ROW_STATE_COLUMNS: ReadonlySet<string> = new Set(['active', 'is_default']);
7480+
7481+
/**
7482+
* [#22360] The `sys_position` provenance values carve-out (d) applies to:
7483+
* `package`, and `config`, its legacy pre-A4 spelling, which
7484+
* `SYSTEM_ROW_PROVENANCE` guards as the same owner ("an application
7485+
* package") — the two vocabularies are kept in lockstep there, so a legacy
7486+
* row the boot normalizer has not healed yet answers as its canonical twin.
7487+
* ⛔ Never `platform` / `system`: switching off a built-in such as
7488+
* `everyone` is not an operator lever.
7489+
*/
7490+
private static readonly PACKAGE_POSITION_PROVENANCE: ReadonlySet<string> = new Set(['package', 'config']);
7491+
7492+
/**
7493+
* [#22360] Is this the write carve-out (d) admits: an `update` of
7494+
* `sys_position` whose patch is ONE object naming at least one row-state
7495+
* column and nothing else? `id` is the row's identifier (the single-id
7496+
* branch reads it from the payload), not a column the patch changes. An
7497+
* array payload is never admitted here: its rows are judged by the gate
7498+
* exactly as before.
7499+
*/
7500+
private static isPackagePositionRowStatePatch(opCtx: any): boolean {
7501+
if (opCtx?.object !== 'sys_position' || opCtx?.operation !== 'update') return false;
7502+
const patch = opCtx.data;
7503+
if (!patch || typeof patch !== 'object' || Array.isArray(patch)) return false;
7504+
const columns = Object.keys(patch).filter((k) => k !== 'id');
7505+
return columns.length > 0
7506+
&& columns.every((k) => SecurityPlugin.PACKAGE_POSITION_ROW_STATE_COLUMNS.has(k));
7507+
}
7508+
74427509
/**
74437510
* [#8552 / ADR-0066 D1] Curated-name authoring refusal for `sys_capability`.
74447511
*
Lines changed: 226 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,226 @@
1+
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
2+
//
3+
// [#22360] `assertSystemRowWriteGate` on a PACKAGE-provided `sys_position` row:
4+
// its definition is locked, its row state is not.
5+
//
6+
// The declared-position seeder stamps `managed_by: 'package'` on the row of a
7+
// position a code package holds, so this gate now protects that row. A package
8+
// position row is locked the way a packaged permission set is
9+
// (`permission-set-projection.ts`, #4669): a patch touching ONLY row state
10+
// (`active`, `is_default`) passes, because switching a package's position off,
11+
// or making it the default for new users, is not an edit of its definition.
12+
// Everything else stays refused with the gate's own code: a definition column
13+
// (`name`, `label`, `description`, `delegatable`), any other column, a delete
14+
// and the other lifecycle verbs. Platform (built-in) rows and `sys_capability`
15+
// get no carve-out.
16+
//
17+
// Each case runs one write through the plugin's REAL middleware over a store
18+
// double, so "admitted" means every gate after this one admitted it too.
19+
20+
import { describe, it, expect, vi } from 'vitest';
21+
import { SecurityPlugin } from './security-plugin.js';
22+
import type { PermissionSet } from '@objectstack/spec/security';
23+
import { assertEngineFindOnePredicate } from '@objectstack/metadata-core';
24+
25+
type Row = Record<string, unknown>;
26+
27+
const POSITION_FIELDS = Object.fromEntries(
28+
['id', 'name', 'label', 'description', 'active', 'is_default', 'delegatable', 'managed_by'].map((n) => [n, { name: n }]),
29+
);
30+
const SCHEMAS: Record<string, unknown> = {
31+
sys_position: { name: 'sys_position', fields: POSITION_FIELDS },
32+
sys_capability: { name: 'sys_capability', fields: POSITION_FIELDS },
33+
};
34+
35+
/** Plain CRUD on both asset objects — no wildcard, no `modifyAllRecords`. */
36+
const ADMIN_SET: PermissionSet = {
37+
name: 'admin_set',
38+
label: 'Admin Set',
39+
objects: {
40+
sys_position: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
41+
sys_capability: { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true },
42+
},
43+
} as unknown as PermissionSet;
44+
45+
const rows = (): Record<string, Row[]> => ({
46+
sys_position: [
47+
{ id: 'pos_pkg_a', name: 'field_rep', label: 'Field Rep', managed_by: 'package', active: true },
48+
{ id: 'pos_pkg_b', name: 'auditor', label: 'Auditor', managed_by: 'package', active: true },
49+
{ id: 'pos_legacy', name: 'legacy_lead', label: 'Legacy Lead', managed_by: 'config', active: true },
50+
{ id: 'pos_platform', name: 'everyone', label: 'Everyone', managed_by: 'platform', active: true },
51+
{ id: 'pos_admin', name: 'regional_lead', label: 'Regional Lead', managed_by: 'admin', active: true },
52+
],
53+
sys_capability: [
54+
{ id: 'cap_pkg', name: 'pkg_cap', label: 'Package Capability', managed_by: 'package', active: true },
55+
],
56+
});
57+
58+
async function boot() {
59+
const store = rows();
60+
const matches = (row: Row, where: any): boolean => {
61+
if (!where || typeof where !== 'object') return true;
62+
if (Array.isArray(where.$and) && !where.$and.every((w: any) => matches(row, w))) return false;
63+
if (Array.isArray(where.$or) && !where.$or.some((w: any) => matches(row, w))) return false;
64+
return Object.entries(where).every(([k, v]) => {
65+
if (k === '$and' || k === '$or') return true;
66+
if (v && typeof v === 'object' && Array.isArray((v as any).$in)) {
67+
return (v as any).$in.map(String).includes(String(row[k]));
68+
}
69+
return String(row[k]) === String(v);
70+
});
71+
};
72+
73+
let middleware: any;
74+
const ql = {
75+
registerMiddleware: (mw: any) => {
76+
if (!middleware) middleware = mw;
77+
},
78+
getSchema: (object: string) => SCHEMAS[object],
79+
find: vi.fn(async (object: string, o: any = {}) => (store[object] ?? []).filter((r) => matches(r, o?.where))),
80+
findOne: vi.fn(async (object: string, o: any = {}) => {
81+
assertEngineFindOnePredicate(object, o);
82+
return (store[object] ?? []).find((r) => matches(r, o?.where)) ?? null;
83+
}),
84+
};
85+
const services: Record<string, unknown> = {
86+
manifest: { register: vi.fn() },
87+
objectql: ql,
88+
metadata: { get: async (n: string) => SCHEMAS[n], list: async () => [ADMIN_SET] },
89+
};
90+
const ctx: any = {
91+
logger: { info: vi.fn(), warn: vi.fn(), error: vi.fn() },
92+
registerService: vi.fn(),
93+
getService: (name: string) => {
94+
if (!(name in services)) throw new Error(`service not registered: ${name}`);
95+
return services[name];
96+
},
97+
};
98+
const plugin = new SecurityPlugin({ defaultPermissionSets: [ADMIN_SET], fallbackPermissionSet: ADMIN_SET.name });
99+
await plugin.init(ctx);
100+
await plugin.start(ctx);
101+
102+
/** One write through the real middleware: 'admitted', or the refusal's code + status + message. */
103+
const write = async (opCtx: Record<string, unknown>): Promise<'admitted' | { code: unknown; status: unknown; message: string }> => {
104+
let admitted = false;
105+
try {
106+
await middleware(
107+
{ options: {}, context: { userId: 'usr_admin', tenantId: 'org-1', positions: [], permissions: [] }, ...opCtx } as any,
108+
async () => { admitted = true; },
109+
);
110+
} catch (e: any) {
111+
return { code: e?.code, status: e?.status ?? e?.statusCode, message: String(e?.message ?? '') };
112+
}
113+
if (!admitted) throw new Error('middleware resolved without calling next()');
114+
return 'admitted';
115+
};
116+
return { write };
117+
}
118+
119+
const update = (object: string, data: unknown, where: Row = {}) => ({ object, operation: 'update', data, options: { where } });
120+
const byId = (id: string, patch: Row) => update('sys_position', { id, ...patch }, { id });
121+
const REFUSED = { code: 'PERMISSION_DENIED', status: 403 };
122+
123+
describe('[#22360] a package position row: row state switches, the definition stays locked', () => {
124+
it('a row-state-only patch passes: deactivate, activate, Set as Default, both together', async () => {
125+
const h = await boot();
126+
expect(await h.write(byId('pos_pkg_a', { active: false }))).toBe('admitted');
127+
expect(await h.write(byId('pos_pkg_a', { active: true }))).toBe('admitted');
128+
expect(await h.write(byId('pos_pkg_a', { is_default: true }))).toBe('admitted');
129+
expect(await h.write(byId('pos_pkg_a', { active: false, is_default: false }))).toBe('admitted');
130+
// The id carried only by the filter, as the engine's by-id update spells it.
131+
expect(await h.write(update('sys_position', { active: false }, { id: 'pos_pkg_a' }))).toBe('admitted');
132+
});
133+
134+
it('a definition column, any other column, or a mixed patch is refused with the gate\'s code', async () => {
135+
const h = await boot();
136+
for (const patch of [
137+
{ label: 'Field Rep (edited)' },
138+
{ description: 'edited' },
139+
{ delegatable: true },
140+
{ name: 'field_rep_two' },
141+
{ active: false, label: 'Field Rep (edited)' },
142+
{ is_default: true, delegatable: true },
143+
{ active: false, notes: 'not a row-state column' },
144+
]) {
145+
const refused = await h.write(byId('pos_pkg_a', patch));
146+
expect(refused, JSON.stringify(patch)).toMatchObject(REFUSED);
147+
expect((refused as any).message).toContain('provided by an application package');
148+
}
149+
});
150+
151+
it('an empty patch admits nothing new: it is refused as before', async () => {
152+
const h = await boot();
153+
expect(await h.write(byId('pos_pkg_a', {}))).toMatchObject(REFUSED);
154+
});
155+
156+
it('delete and the other lifecycle verbs stay refused', async () => {
157+
const h = await boot();
158+
for (const operation of ['delete', 'transfer', 'restore', 'purge']) {
159+
const refused = await h.write({ object: 'sys_position', operation, options: { where: { id: 'pos_pkg_a' } } });
160+
expect(refused, operation).toMatchObject(REFUSED);
161+
}
162+
});
163+
164+
it('the legacy `config` spelling of package provenance answers as its canonical twin', async () => {
165+
const h = await boot();
166+
expect(await h.write(byId('pos_legacy', { active: false }))).toBe('admitted');
167+
expect(await h.write(byId('pos_legacy', { label: 'edited' }))).toMatchObject(REFUSED);
168+
});
169+
170+
it('control: a built-in\'s bare { active } patch stays refused', async () => {
171+
const h = await boot();
172+
const refused = await h.write(byId('pos_platform', { active: false }));
173+
expect(refused).toMatchObject(REFUSED);
174+
expect((refused as any).message).toContain('provided by the platform');
175+
expect(await h.write(byId('pos_platform', { is_default: true }))).toMatchObject(REFUSED);
176+
});
177+
178+
it('control: sys_capability gets no carve-out', async () => {
179+
const h = await boot();
180+
expect(await h.write(update('sys_capability', { id: 'cap_pkg', active: false }, { id: 'cap_pkg' }))).toMatchObject(REFUSED);
181+
});
182+
183+
it('control: an administrator-authored position stays fully editable', async () => {
184+
const h = await boot();
185+
expect(await h.write(byId('pos_admin', { label: 'Regional Lead (edited)', delegatable: true }))).toBe('admitted');
186+
});
187+
188+
it('control: forging provenance in a row-state patch is still refused (a)', async () => {
189+
const h = await boot();
190+
const refused = await h.write(byId('pos_admin', { active: false, managed_by: 'package' }));
191+
expect(refused).toMatchObject(REFUSED);
192+
expect((refused as any).message).toContain('cannot stamp');
193+
});
194+
});
195+
196+
describe('[#22360] a filtered update over package position rows', () => {
197+
const PKG_NAMES = { name: { $in: ['field_rep', 'auditor'] } };
198+
199+
it('a row-state-only patch over package rows passes', async () => {
200+
const h = await boot();
201+
expect(await h.write(update('sys_position', { active: false }, PKG_NAMES))).toBe('admitted');
202+
expect(await h.write(update('sys_position', { is_default: false }, PKG_NAMES))).toBe('admitted');
203+
});
204+
205+
it('a label patch over a package row is refused', async () => {
206+
const h = await boot();
207+
const refused = await h.write(update('sys_position', { label: 'Renamed' }, PKG_NAMES));
208+
expect(refused).toMatchObject(REFUSED);
209+
expect((refused as any).message).toContain('provided by the platform or an application package');
210+
});
211+
212+
it('a row-state-only patch whose filter also reaches a built-in is refused', async () => {
213+
const h = await boot();
214+
expect(await h.write(update('sys_position', { active: false }, { name: { $in: ['field_rep', 'everyone'] } })))
215+
.toMatchObject(REFUSED);
216+
// A whole-table write matches every built-in.
217+
expect(await h.write({ object: 'sys_position', operation: 'update', data: { active: false }, options: {} }))
218+
.toMatchObject(REFUSED);
219+
});
220+
221+
it('control: a filtered delete over package rows stays refused', async () => {
222+
const h = await boot();
223+
expect(await h.write({ object: 'sys_position', operation: 'delete', options: { where: PKG_NAMES } }))
224+
.toMatchObject(REFUSED);
225+
});
226+
});

0 commit comments

Comments
 (0)