Skip to content

Commit 6e2b8ae

Browse files
os-zhuangclaude
andcommitted
fix(plugin-security): project env-scope permission-set edits onto sys_permission_set (#2857)
An environment-scope save('permission', ...) writes only the sys_metadata overlay; the metadata->sys_permission_set projection ran only at boot / on publish (and publish refuses env-authored rows), so the queryable record the admin/Setup surface reads went stale while the layered read showed the edit. Adds the environment door: subscribeEnvPermissionProjection hooks the protocol's post-persistence onMetadataMutation choke point; on an active permission save it re-reads the fresh effective body via the layered read and upsertEnvPermissionSet projects the six facet columns onto the record. Ownership is decided by the RECORD's managed_by (env-authored rows only; package records keep their declared baseline). Mirrors authored-translation-sync's mutation-listener pattern. Tested: 21 unit tests (projection for all six facets, env/package guards, the _packageId-provenance + direct-body-shape regressions, and the mutation wiring via a mock protocol); no regression (153 plugin-security tests). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 7b4d2e4 commit 6e2b8ae

4 files changed

Lines changed: 285 additions & 2 deletions

File tree

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
'@objectstack/plugin-security': patch
3+
---
4+
5+
**Project env-scope permission-set edits onto the `sys_permission_set` record (#2857).**
6+
7+
A `sys_permission_set` has two representations: the authoritative **metadata** the
8+
structured editor writes, and the queryable **data record** (snake_case
9+
JSON-string columns) the admin/Setup surface reads. The metadata→record
10+
projection (`toRowFields` / `upsertPackagePermissionSet`) ran only at **boot** and
11+
on **publish** (package door), and the publish path refuses env-authored rows —
12+
so an environment-scope `save('permission', …)` updated the `sys_metadata`
13+
overlay (and the layered read) but left the `sys_permission_set` record **stale**
14+
(split-brain). Enforcement reads the authoritative metadata so access stayed
15+
correct, but the admin surface showed old values.
16+
17+
Adds the **environment door**: `subscribeEnvPermissionProjection` hooks the
18+
protocol's post-persistence `onMetadataMutation` choke point; on an active
19+
(non-draft) `permission` save it re-reads the fresh effective body via the
20+
layered read (the boot-cached metadata registry would return a stale declared
21+
body) and `upsertEnvPermissionSet` projects the six facets onto the record.
22+
Ownership is decided by the **record's** `managed_by` — env-authored rows
23+
(platform/user/absent) are projected; a package-owned record's baseline is left
24+
to boot re-seed / publish, so the two doors never fight. Mirrors the existing
25+
`authored-translation-sync` mutation-listener pattern.

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

Lines changed: 138 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,13 @@
11
// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license.
22

33
import { describe, it, expect } from 'vitest';
4-
import { bootstrapDeclaredPermissions, upsertPackagePermissionSet } from './bootstrap-declared-permissions.js';
4+
import {
5+
bootstrapDeclaredPermissions,
6+
upsertPackagePermissionSet,
7+
upsertEnvPermissionSet,
8+
projectEnvPermissionOnMutation,
9+
subscribeEnvPermissionProjection,
10+
} from './bootstrap-declared-permissions.js';
511

612
/** Minimal in-memory ql + registry for sys_permission_set seeding. */
713
function makeQl(declared: any[] = []) {
@@ -168,3 +174,134 @@ describe('upsertPackagePermissionSet (ADR-0086 P2 — publish materialization)',
168174
expect(warns.some((w) => w.includes('no owning package'))).toBe(true);
169175
});
170176
});
177+
178+
// framework#2857 — the environment door. An env-scope `save('permission', …)`
179+
// writes only the sys_metadata overlay; upsertEnvPermissionSet projects the
180+
// saved facets onto the queryable sys_permission_set record so the admin/Setup
181+
// surface stops going stale. Mirror of upsertPackagePermissionSet (env rows
182+
// only; refuses package-owned records).
183+
describe('upsertEnvPermissionSet (framework#2857 — env-door projection)', () => {
184+
const envBody = (over: Record<string, any> = {}) => ({
185+
name: 'organization_admin',
186+
label: 'Organization Administrator',
187+
objects: { crm_lead: { allowRead: true, allowEdit: true } },
188+
fields: { 'crm_lead.amount': { readable: true, editable: false } },
189+
systemPermissions: ['setup.access', 'manage_org_users'],
190+
rowLevelSecurity: [{ name: 'tenant', object: '*', operation: 'all', using: 'org == current_user.org', enabled: true }],
191+
tabPermissions: { crm_leads: 'visible' },
192+
adminScope: { businessUnit: 'Sales', includeSubtree: true, assignablePermissionSets: ['member_default'] },
193+
...over,
194+
});
195+
196+
it('projects all six facets onto an existing env-authored row (update)', async () => {
197+
const ql = makeQl();
198+
ql.rows.push({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]' });
199+
const r = await upsertEnvPermissionSet(ql, envBody());
200+
expect(r.updated).toBe(1);
201+
const row = ql.rows[0];
202+
expect(JSON.parse(row.object_permissions)).toEqual({ crm_lead: { allowRead: true, allowEdit: true } });
203+
expect(JSON.parse(row.field_permissions)).toEqual({ 'crm_lead.amount': { readable: true, editable: false } });
204+
expect(JSON.parse(row.system_permissions)).toEqual(['setup.access', 'manage_org_users']);
205+
expect(JSON.parse(row.row_level_security)[0].using).toBe('org == current_user.org');
206+
expect(JSON.parse(row.tab_permissions)).toEqual({ crm_leads: 'visible' });
207+
expect(JSON.parse(row.admin_scope).businessUnit).toBe('Sales');
208+
});
209+
210+
it('projects onto a legacy row with ABSENT provenance (platform default)', async () => {
211+
const ql = makeQl();
212+
ql.rows.push({ id: 'ps_legacy', name: 'organization_admin', system_permissions: '[]' });
213+
const r = await upsertEnvPermissionSet(ql, envBody());
214+
expect(r.updated).toBe(1);
215+
expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_org_users']);
216+
});
217+
218+
it('does nothing when no data record exists (creation goes through the data API)', async () => {
219+
const ql = makeQl();
220+
const r = await upsertEnvPermissionSet(ql, envBody());
221+
expect(r.seeded + r.updated).toBe(0);
222+
expect(ql.rows.length).toBe(0);
223+
});
224+
225+
it('refuses to touch a package-owned row (record stays the package baseline)', async () => {
226+
const ql = makeQl();
227+
ql.rows.push({ id: 'ps_pkg', name: 'organization_admin', managed_by: 'package', package_id: 'com.example.crm', system_permissions: '["pkg"]' });
228+
const warns: string[] = [];
229+
const r = await upsertEnvPermissionSet(ql, envBody(), { info: () => {}, warn: (m) => warns.push(m) });
230+
expect(r.skippedForeign).toBe(1);
231+
expect(r.seeded + r.updated).toBe(0);
232+
expect(ql.rows[0].system_permissions).toBe('["pkg"]');
233+
expect(warns.some((w) => w.includes('package-owned'))).toBe(true);
234+
});
235+
236+
it('projects an env record even when the layered body carries _packageId provenance (record decides, not the body)', async () => {
237+
// Regression for framework#2857: the layered read stamps `_packageId` on
238+
// env-authored sets too, so the body must NOT gate projection — the record's
239+
// managed_by does.
240+
const ql = makeQl();
241+
ql.rows.push({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]' });
242+
const r = await upsertEnvPermissionSet(ql, envBody({ _packageId: 'com.example.app', _provenance: 'x' }));
243+
expect(r.updated).toBe(1);
244+
expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_org_users']);
245+
});
246+
});
247+
248+
// framework#2857 — the mutation-driven wiring (onMetadataMutation → layered
249+
// re-read → project). Exercised without the dev server via a mock protocol.
250+
describe('projectEnvPermissionOnMutation / subscribeEnvPermissionProjection', () => {
251+
const body = () => ({
252+
name: 'organization_admin',
253+
// the layered read stamps provenance on env sets too — must not gate.
254+
_packageId: 'com.objectstack.showcase',
255+
_provenance: { source: 'env' },
256+
objects: { crm_lead: { allowRead: true } },
257+
systemPermissions: ['setup.access', 'manage_metadata'],
258+
});
259+
const evt = (over = {}) => ({ type: 'permission', state: 'active', name: 'organization_admin', ...over });
260+
const envRow = () => ({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]' });
261+
262+
it('re-reads the fresh body via getMetaItemLayered (ENVELOPE shape) and projects it', async () => {
263+
const ql = makeQl(); ql.rows.push(envRow());
264+
const protocol = { getMetaItemLayered: async () => ({ effective: body(), code: null }) };
265+
const r = await projectEnvPermissionOnMutation(protocol, ql, evt());
266+
expect(r?.updated).toBe(1);
267+
expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_metadata']);
268+
});
269+
270+
it('accepts a DIRECT body from getMetaItemLayered (no effective/code envelope)', async () => {
271+
// Regression: the layered read can return the effective body directly; a
272+
// `?? null` fallback would drop it and silently skip the projection.
273+
const ql = makeQl(); ql.rows.push(envRow());
274+
const protocol = { getMetaItemLayered: async () => body() };
275+
const r = await projectEnvPermissionOnMutation(protocol, ql, evt());
276+
expect(r?.updated).toBe(1);
277+
expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_metadata']);
278+
});
279+
280+
it('skips draft saves and non-permission events', async () => {
281+
const ql = makeQl(); ql.rows.push(envRow());
282+
const protocol = { getMetaItemLayered: async () => body() };
283+
expect(await projectEnvPermissionOnMutation(protocol, ql, evt({ state: 'draft' }))).toBeNull();
284+
expect(await projectEnvPermissionOnMutation(protocol, ql, evt({ type: 'object' }))).toBeNull();
285+
expect(ql.rows[0].system_permissions).toBe('[]');
286+
});
287+
288+
it('subscribeEnvPermissionProjection wires a listener that projects on an active permission save', async () => {
289+
const ql = makeQl(); ql.rows.push(envRow());
290+
let listener: any = null;
291+
const protocol = {
292+
onMetadataMutation: (fn: any) => { listener = fn; return () => {}; },
293+
getMetaItemLayered: async () => body(),
294+
};
295+
const unsub = subscribeEnvPermissionProjection(protocol, ql);
296+
expect(typeof unsub).toBe('function');
297+
expect(typeof listener).toBe('function');
298+
listener(evt());
299+
await new Promise((r) => setTimeout(r, 0)); // let the fire-and-forget settle
300+
expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_metadata']);
301+
});
302+
303+
it('returns null (no wiring) when the protocol lacks onMetadataMutation', () => {
304+
expect(subscribeEnvPermissionProjection({}, makeQl())).toBeNull();
305+
expect(subscribeEnvPermissionProjection(null, makeQl())).toBeNull();
306+
});
307+
});

packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -158,6 +158,108 @@ export async function upsertPackagePermissionSet(
158158
return out;
159159
}
160160

161+
/**
162+
* Project an ENVIRONMENT-authored PermissionSet body onto its
163+
* `sys_permission_set` row — the mirror image of {@link upsertPackagePermissionSet}
164+
* for the environment door (ADR-0086 two-doors; framework#2857).
165+
*
166+
* An env-scope `save('permission', name, body)` writes only the `sys_metadata`
167+
* overlay; nothing projected the six facet columns onto the queryable
168+
* `sys_permission_set` record, so the admin/Setup surface (which reads the
169+
* record) went stale while the layered read showed the edit — split-brain.
170+
* This closes that gap for env-authored sets: it owns rows whose `managed_by`
171+
* is NOT `'package'` (i.e. `platform`/`user`/absent) and REFUSES to touch a
172+
* package-owned row — a package's record mirrors its declaration and changes
173+
* only via boot re-seed / publish, never through an env override.
174+
*/
175+
export async function upsertEnvPermissionSet(
176+
ql: any,
177+
ps: any,
178+
logger?: SeedOptions['logger'],
179+
): Promise<PermissionSeedOutcome> {
180+
const out: PermissionSeedOutcome = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 };
181+
if (!ql || typeof ql.find !== 'function' || !ps?.name) return out;
182+
183+
// Ownership is decided by the EXISTING RECORD's `managed_by`, never the body:
184+
// the layered read stamps `_packageId` provenance on env-authored sets too
185+
// (a declared-then-env-overridden set), so the body cannot tell the two doors
186+
// apart — only the record's provenance can.
187+
const existing = (await tryFind(ql, 'sys_permission_set', { name: ps.name }, 1))[0];
188+
if (!existing?.id) {
189+
// No data record. A set's admin-surface row is created through the data API
190+
// (the Setup "New" flow), not the metadata door, so there is nothing to
191+
// project here — leave creation to that path / the boot seeder.
192+
return out;
193+
}
194+
195+
// A package-owned record is the package's declared baseline (re-seeded at
196+
// boot / on publish); an env override lives in the overlay/effective layer,
197+
// not this row. Refusing here keeps the two doors from fighting.
198+
if (existing.managed_by === 'package') {
199+
out.skippedForeign += 1;
200+
logger?.warn?.('[security] env permission save targets a package-owned set — record left at package baseline', { name: ps.name });
201+
return out;
202+
}
203+
204+
// Env-authored row (platform / user / absent provenance): project the saved
205+
// facets so the record matches the layered read the editor shows.
206+
if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...toRowFields(ps) })) {
207+
out.updated += 1;
208+
}
209+
return out;
210+
}
211+
212+
/**
213+
* Handle one `permission` metadata-mutation event (framework#2857): re-read the
214+
* FRESH effective body via the protocol's layered read — the boot-time metadata
215+
* registry would hand back a stale declared body — and project it onto the env
216+
* record. Exported (and Promise-returning) so the wiring is unit-testable
217+
* without the dev server. Returns the projection outcome, or `null` when the
218+
* event is skipped (draft, non-permission, or no readable body).
219+
*/
220+
export async function projectEnvPermissionOnMutation(
221+
protocol: any,
222+
ql: any,
223+
evt: { type?: string; name?: string; state?: string; organizationId?: string | null } | null | undefined,
224+
logger?: SeedOptions['logger'],
225+
): Promise<PermissionSeedOutcome | null> {
226+
if (evt?.type !== 'permission' || evt.state === 'draft' || !evt.name) return null;
227+
let body: any = null;
228+
if (protocol && typeof protocol.getMetaItemLayered === 'function') {
229+
const layered = await protocol.getMetaItemLayered({
230+
type: 'permission',
231+
name: evt.name,
232+
...(evt.organizationId ? { environmentId: evt.organizationId } : {}),
233+
});
234+
// `getMetaItemLayered` may return a layered envelope (`{ effective | code }`)
235+
// OR the effective body directly (top-level `name`/`systemPermissions`) —
236+
// accept both so a body isn't silently dropped.
237+
body = layered?.effective ?? layered?.code ?? layered ?? null;
238+
}
239+
if (!body?.name) return null;
240+
return upsertEnvPermissionSet(ql, body, logger);
241+
}
242+
243+
/**
244+
* Subscribe env-permission projection to the protocol's post-persistence
245+
* mutation choke point (framework#2857). Returns the unsubscribe fn, or `null`
246+
* when the protocol doesn't expose `onMetadataMutation`.
247+
*/
248+
export function subscribeEnvPermissionProjection(
249+
protocol: any,
250+
ql: any,
251+
logger?: SeedOptions['logger'],
252+
): (() => void) | null {
253+
if (!protocol || typeof protocol.onMetadataMutation !== 'function') return null;
254+
return protocol.onMetadataMutation((evt: any) => {
255+
void projectEnvPermissionOnMutation(protocol, ql, evt, logger).catch((err: any) => {
256+
logger?.warn?.('[security] env permission projection after save failed', {
257+
name: evt?.name, error: err?.message,
258+
});
259+
});
260+
});
261+
}
262+
161263
export async function bootstrapDeclaredPermissions(
162264
ql: any,
163265
metadataService: any,

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

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
} from './explain-engine.js';
1515
import type { ExplainDecision, ExplainOperation } from '@objectstack/spec/security';
1616
import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js';
17-
import { bootstrapDeclaredPermissions, upsertPackagePermissionSet } from './bootstrap-declared-permissions.js';
17+
import { bootstrapDeclaredPermissions, upsertPackagePermissionSet, subscribeEnvPermissionProjection } from './bootstrap-declared-permissions.js';
1818
import {
1919
syncAudienceBindingSuggestions,
2020
listAudienceBindingSuggestions,
@@ -1044,6 +1044,11 @@ export class SecurityPlugin implements Plugin {
10441044
// insert seed rows. Falls back to immediate execution when the
10451045
// kernel does not expose `hook` (test stubs).
10461046
let bootstrapRanOnce = false;
1047+
// [framework#2857] Guard so the env-projection mutation subscriber is wired
1048+
// exactly once even though runBootstrap re-runs (e.g. after the first user
1049+
// insert) — onMetadataMutation appends listeners, so re-wiring would project
1050+
// each save N times.
1051+
let envProjectionWired = false;
10471052
const runBootstrap = async () => {
10481053
try {
10491054
const report = await bootstrapPlatformAdmin(ql, this.bootstrapPermissionSets, {
@@ -1165,6 +1170,20 @@ export class SecurityPlugin implements Plugin {
11651170
},
11661171
);
11671172
}
1173+
// [framework#2857] Environment door — project an env-scope permission
1174+
// metadata save onto its sys_permission_set record. saveMetaItem writes
1175+
// only the sys_metadata overlay, so without this the admin/Setup surface
1176+
// (which reads the record) went stale while the layered read showed the
1177+
// edit. onMetadataMutation fires post-persistence for active (non-draft)
1178+
// saves; the subscriber re-reads the FRESH effective body via the layered
1179+
// read (the MetadataManager registry would hand back the stale
1180+
// boot-declared body for a seeded set) and projects it. Env-authored
1181+
// rows only — a package record's baseline is its declaration, owned by
1182+
// boot re-seed / publish, never an env override.
1183+
if (!envProjectionWired) {
1184+
const unsub = subscribeEnvPermissionProjection(protocol, ql, ctx.logger);
1185+
if (unsub) envProjectionWired = true;
1186+
}
11681187
} catch (e) {
11691188
ctx.logger.warn('[security] permission publish-materializer registration failed', { error: (e as Error).message });
11701189
}

0 commit comments

Comments
 (0)