From 56e920c98ce072196dcbc11b7fb998af640ccaff Mon Sep 17 00:00:00 2001 From: os-zhuang Date: Mon, 13 Jul 2026 10:38:37 +0800 Subject: [PATCH] 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 --- .changeset/perm-set-env-projection.md | 25 ++++ .../bootstrap-declared-permissions.test.ts | 139 +++++++++++++++++- .../src/bootstrap-declared-permissions.ts | 102 +++++++++++++ .../plugin-security/src/security-plugin.ts | 21 ++- 4 files changed, 285 insertions(+), 2 deletions(-) create mode 100644 .changeset/perm-set-env-projection.md diff --git a/.changeset/perm-set-env-projection.md b/.changeset/perm-set-env-projection.md new file mode 100644 index 000000000..03c113b4c --- /dev/null +++ b/.changeset/perm-set-env-projection.md @@ -0,0 +1,25 @@ +--- +'@objectstack/plugin-security': patch +--- + +**Project env-scope permission-set edits onto the `sys_permission_set` record (#2857).** + +A `sys_permission_set` has two representations: the authoritative **metadata** the +structured editor writes, and the queryable **data record** (snake_case +JSON-string columns) the admin/Setup surface reads. The metadata→record +projection (`toRowFields` / `upsertPackagePermissionSet`) ran only at **boot** and +on **publish** (package door), and the publish path refuses env-authored rows — +so an environment-scope `save('permission', …)` updated the `sys_metadata` +overlay (and the layered read) but left the `sys_permission_set` record **stale** +(split-brain). Enforcement reads the authoritative metadata so access stayed +correct, but the admin surface showed old values. + +Adds the **environment door**: `subscribeEnvPermissionProjection` hooks the +protocol's post-persistence `onMetadataMutation` choke point; on an active +(non-draft) `permission` save it re-reads the fresh effective body via the +layered read (the boot-cached metadata registry would return a stale declared +body) and `upsertEnvPermissionSet` projects the six facets onto the record. +Ownership is decided by the **record's** `managed_by` — env-authored rows +(platform/user/absent) are projected; a package-owned record's baseline is left +to boot re-seed / publish, so the two doors never fight. Mirrors the existing +`authored-translation-sync` mutation-listener pattern. diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts index 3720bb1bc..909cbd89a 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts @@ -1,7 +1,13 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { bootstrapDeclaredPermissions, upsertPackagePermissionSet } from './bootstrap-declared-permissions.js'; +import { + bootstrapDeclaredPermissions, + upsertPackagePermissionSet, + upsertEnvPermissionSet, + projectEnvPermissionOnMutation, + subscribeEnvPermissionProjection, +} from './bootstrap-declared-permissions.js'; /** Minimal in-memory ql + registry for sys_permission_set seeding. */ function makeQl(declared: any[] = []) { @@ -168,3 +174,134 @@ describe('upsertPackagePermissionSet (ADR-0086 P2 — publish materialization)', expect(warns.some((w) => w.includes('no owning package'))).toBe(true); }); }); + +// framework#2857 — the environment door. An env-scope `save('permission', …)` +// writes only the sys_metadata overlay; upsertEnvPermissionSet projects the +// saved facets onto the queryable sys_permission_set record so the admin/Setup +// surface stops going stale. Mirror of upsertPackagePermissionSet (env rows +// only; refuses package-owned records). +describe('upsertEnvPermissionSet (framework#2857 — env-door projection)', () => { + const envBody = (over: Record = {}) => ({ + name: 'organization_admin', + label: 'Organization Administrator', + objects: { crm_lead: { allowRead: true, allowEdit: true } }, + fields: { 'crm_lead.amount': { readable: true, editable: false } }, + systemPermissions: ['setup.access', 'manage_org_users'], + rowLevelSecurity: [{ name: 'tenant', object: '*', operation: 'all', using: 'org == current_user.org', enabled: true }], + tabPermissions: { crm_leads: 'visible' }, + adminScope: { businessUnit: 'Sales', includeSubtree: true, assignablePermissionSets: ['member_default'] }, + ...over, + }); + + it('projects all six facets onto an existing env-authored row (update)', async () => { + const ql = makeQl(); + ql.rows.push({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]' }); + const r = await upsertEnvPermissionSet(ql, envBody()); + expect(r.updated).toBe(1); + const row = ql.rows[0]; + expect(JSON.parse(row.object_permissions)).toEqual({ crm_lead: { allowRead: true, allowEdit: true } }); + expect(JSON.parse(row.field_permissions)).toEqual({ 'crm_lead.amount': { readable: true, editable: false } }); + expect(JSON.parse(row.system_permissions)).toEqual(['setup.access', 'manage_org_users']); + expect(JSON.parse(row.row_level_security)[0].using).toBe('org == current_user.org'); + expect(JSON.parse(row.tab_permissions)).toEqual({ crm_leads: 'visible' }); + expect(JSON.parse(row.admin_scope).businessUnit).toBe('Sales'); + }); + + it('projects onto a legacy row with ABSENT provenance (platform default)', async () => { + const ql = makeQl(); + ql.rows.push({ id: 'ps_legacy', name: 'organization_admin', system_permissions: '[]' }); + const r = await upsertEnvPermissionSet(ql, envBody()); + expect(r.updated).toBe(1); + expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_org_users']); + }); + + it('does nothing when no data record exists (creation goes through the data API)', async () => { + const ql = makeQl(); + const r = await upsertEnvPermissionSet(ql, envBody()); + expect(r.seeded + r.updated).toBe(0); + expect(ql.rows.length).toBe(0); + }); + + it('refuses to touch a package-owned row (record stays the package baseline)', async () => { + const ql = makeQl(); + ql.rows.push({ id: 'ps_pkg', name: 'organization_admin', managed_by: 'package', package_id: 'com.example.crm', system_permissions: '["pkg"]' }); + const warns: string[] = []; + const r = await upsertEnvPermissionSet(ql, envBody(), { info: () => {}, warn: (m) => warns.push(m) }); + expect(r.skippedForeign).toBe(1); + expect(r.seeded + r.updated).toBe(0); + expect(ql.rows[0].system_permissions).toBe('["pkg"]'); + expect(warns.some((w) => w.includes('package-owned'))).toBe(true); + }); + + it('projects an env record even when the layered body carries _packageId provenance (record decides, not the body)', async () => { + // Regression for framework#2857: the layered read stamps `_packageId` on + // env-authored sets too, so the body must NOT gate projection — the record's + // managed_by does. + const ql = makeQl(); + ql.rows.push({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]' }); + const r = await upsertEnvPermissionSet(ql, envBody({ _packageId: 'com.example.app', _provenance: 'x' })); + expect(r.updated).toBe(1); + expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_org_users']); + }); +}); + +// framework#2857 — the mutation-driven wiring (onMetadataMutation → layered +// re-read → project). Exercised without the dev server via a mock protocol. +describe('projectEnvPermissionOnMutation / subscribeEnvPermissionProjection', () => { + const body = () => ({ + name: 'organization_admin', + // the layered read stamps provenance on env sets too — must not gate. + _packageId: 'com.objectstack.showcase', + _provenance: { source: 'env' }, + objects: { crm_lead: { allowRead: true } }, + systemPermissions: ['setup.access', 'manage_metadata'], + }); + const evt = (over = {}) => ({ type: 'permission', state: 'active', name: 'organization_admin', ...over }); + const envRow = () => ({ id: 'ps_env', name: 'organization_admin', managed_by: 'user', system_permissions: '[]' }); + + it('re-reads the fresh body via getMetaItemLayered (ENVELOPE shape) and projects it', async () => { + const ql = makeQl(); ql.rows.push(envRow()); + const protocol = { getMetaItemLayered: async () => ({ effective: body(), code: null }) }; + const r = await projectEnvPermissionOnMutation(protocol, ql, evt()); + expect(r?.updated).toBe(1); + expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_metadata']); + }); + + it('accepts a DIRECT body from getMetaItemLayered (no effective/code envelope)', async () => { + // Regression: the layered read can return the effective body directly; a + // `?? null` fallback would drop it and silently skip the projection. + const ql = makeQl(); ql.rows.push(envRow()); + const protocol = { getMetaItemLayered: async () => body() }; + const r = await projectEnvPermissionOnMutation(protocol, ql, evt()); + expect(r?.updated).toBe(1); + expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_metadata']); + }); + + it('skips draft saves and non-permission events', async () => { + const ql = makeQl(); ql.rows.push(envRow()); + const protocol = { getMetaItemLayered: async () => body() }; + expect(await projectEnvPermissionOnMutation(protocol, ql, evt({ state: 'draft' }))).toBeNull(); + expect(await projectEnvPermissionOnMutation(protocol, ql, evt({ type: 'object' }))).toBeNull(); + expect(ql.rows[0].system_permissions).toBe('[]'); + }); + + it('subscribeEnvPermissionProjection wires a listener that projects on an active permission save', async () => { + const ql = makeQl(); ql.rows.push(envRow()); + let listener: any = null; + const protocol = { + onMetadataMutation: (fn: any) => { listener = fn; return () => {}; }, + getMetaItemLayered: async () => body(), + }; + const unsub = subscribeEnvPermissionProjection(protocol, ql); + expect(typeof unsub).toBe('function'); + expect(typeof listener).toBe('function'); + listener(evt()); + await new Promise((r) => setTimeout(r, 0)); // let the fire-and-forget settle + expect(JSON.parse(ql.rows[0].system_permissions)).toEqual(['setup.access', 'manage_metadata']); + }); + + it('returns null (no wiring) when the protocol lacks onMetadataMutation', () => { + expect(subscribeEnvPermissionProjection({}, makeQl())).toBeNull(); + expect(subscribeEnvPermissionProjection(null, makeQl())).toBeNull(); + }); +}); diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index 8b51f757f..83eb1e584 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -158,6 +158,108 @@ export async function upsertPackagePermissionSet( return out; } +/** + * Project an ENVIRONMENT-authored PermissionSet body onto its + * `sys_permission_set` row — the mirror image of {@link upsertPackagePermissionSet} + * for the environment door (ADR-0086 two-doors; framework#2857). + * + * An env-scope `save('permission', name, body)` writes only the `sys_metadata` + * overlay; nothing projected the six facet columns onto the queryable + * `sys_permission_set` record, so the admin/Setup surface (which reads the + * record) went stale while the layered read showed the edit — split-brain. + * This closes that gap for env-authored sets: it owns rows whose `managed_by` + * is NOT `'package'` (i.e. `platform`/`user`/absent) and REFUSES to touch a + * package-owned row — a package's record mirrors its declaration and changes + * only via boot re-seed / publish, never through an env override. + */ +export async function upsertEnvPermissionSet( + ql: any, + ps: any, + logger?: SeedOptions['logger'], +): Promise { + const out: PermissionSeedOutcome = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 }; + if (!ql || typeof ql.find !== 'function' || !ps?.name) return out; + + // Ownership is decided by the EXISTING RECORD's `managed_by`, never the body: + // the layered read stamps `_packageId` provenance on env-authored sets too + // (a declared-then-env-overridden set), so the body cannot tell the two doors + // apart — only the record's provenance can. + const existing = (await tryFind(ql, 'sys_permission_set', { name: ps.name }, 1))[0]; + if (!existing?.id) { + // No data record. A set's admin-surface row is created through the data API + // (the Setup "New" flow), not the metadata door, so there is nothing to + // project here — leave creation to that path / the boot seeder. + return out; + } + + // A package-owned record is the package's declared baseline (re-seeded at + // boot / on publish); an env override lives in the overlay/effective layer, + // not this row. Refusing here keeps the two doors from fighting. + if (existing.managed_by === 'package') { + out.skippedForeign += 1; + logger?.warn?.('[security] env permission save targets a package-owned set — record left at package baseline', { name: ps.name }); + return out; + } + + // Env-authored row (platform / user / absent provenance): project the saved + // facets so the record matches the layered read the editor shows. + if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...toRowFields(ps) })) { + out.updated += 1; + } + return out; +} + +/** + * Handle one `permission` metadata-mutation event (framework#2857): re-read the + * FRESH effective body via the protocol's layered read — the boot-time metadata + * registry would hand back a stale declared body — and project it onto the env + * record. Exported (and Promise-returning) so the wiring is unit-testable + * without the dev server. Returns the projection outcome, or `null` when the + * event is skipped (draft, non-permission, or no readable body). + */ +export async function projectEnvPermissionOnMutation( + protocol: any, + ql: any, + evt: { type?: string; name?: string; state?: string; organizationId?: string | null } | null | undefined, + logger?: SeedOptions['logger'], +): Promise { + if (evt?.type !== 'permission' || evt.state === 'draft' || !evt.name) return null; + let body: any = null; + if (protocol && typeof protocol.getMetaItemLayered === 'function') { + const layered = await protocol.getMetaItemLayered({ + type: 'permission', + name: evt.name, + ...(evt.organizationId ? { environmentId: evt.organizationId } : {}), + }); + // `getMetaItemLayered` may return a layered envelope (`{ effective | code }`) + // OR the effective body directly (top-level `name`/`systemPermissions`) — + // accept both so a body isn't silently dropped. + body = layered?.effective ?? layered?.code ?? layered ?? null; + } + if (!body?.name) return null; + return upsertEnvPermissionSet(ql, body, logger); +} + +/** + * Subscribe env-permission projection to the protocol's post-persistence + * mutation choke point (framework#2857). Returns the unsubscribe fn, or `null` + * when the protocol doesn't expose `onMetadataMutation`. + */ +export function subscribeEnvPermissionProjection( + protocol: any, + ql: any, + logger?: SeedOptions['logger'], +): (() => void) | null { + if (!protocol || typeof protocol.onMetadataMutation !== 'function') return null; + return protocol.onMetadataMutation((evt: any) => { + void projectEnvPermissionOnMutation(protocol, ql, evt, logger).catch((err: any) => { + logger?.warn?.('[security] env permission projection after save failed', { + name: evt?.name, error: err?.message, + }); + }); + }); +} + export async function bootstrapDeclaredPermissions( ql: any, metadataService: any, diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index 051bae638..1e89b8697 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -14,7 +14,7 @@ import { } from './explain-engine.js'; import type { ExplainDecision, ExplainOperation } from '@objectstack/spec/security'; import { bootstrapDeclaredPositions } from './bootstrap-declared-positions.js'; -import { bootstrapDeclaredPermissions, upsertPackagePermissionSet } from './bootstrap-declared-permissions.js'; +import { bootstrapDeclaredPermissions, upsertPackagePermissionSet, subscribeEnvPermissionProjection } from './bootstrap-declared-permissions.js'; import { syncAudienceBindingSuggestions, listAudienceBindingSuggestions, @@ -1044,6 +1044,11 @@ export class SecurityPlugin implements Plugin { // insert seed rows. Falls back to immediate execution when the // kernel does not expose `hook` (test stubs). let bootstrapRanOnce = false; + // [framework#2857] Guard so the env-projection mutation subscriber is wired + // exactly once even though runBootstrap re-runs (e.g. after the first user + // insert) — onMetadataMutation appends listeners, so re-wiring would project + // each save N times. + let envProjectionWired = false; const runBootstrap = async () => { try { const report = await bootstrapPlatformAdmin(ql, this.bootstrapPermissionSets, { @@ -1165,6 +1170,20 @@ export class SecurityPlugin implements Plugin { }, ); } + // [framework#2857] Environment door — project an env-scope permission + // metadata save onto its sys_permission_set record. saveMetaItem writes + // only the sys_metadata overlay, so without this the admin/Setup surface + // (which reads the record) went stale while the layered read showed the + // edit. onMetadataMutation fires post-persistence for active (non-draft) + // saves; the subscriber re-reads the FRESH effective body via the layered + // read (the MetadataManager registry would hand back the stale + // boot-declared body for a seeded set) and projects it. Env-authored + // rows only — a package record's baseline is its declaration, owned by + // boot re-seed / publish, never an env override. + if (!envProjectionWired) { + const unsub = subscribeEnvPermissionProjection(protocol, ql, ctx.logger); + if (unsub) envProjectionWired = true; + } } catch (e) { ctx.logger.warn('[security] permission publish-materializer registration failed', { error: (e as Error).message }); }