From b625994614e81f26eea5e377a4da1a2a54857196 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 4 Jul 2026 11:40:35 +0000 Subject: [PATCH] feat(security): two-doors separation for permission sets (ADR-0086 P2) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Splits who may change a permission set into two non-overlapping doors, enforced at the data layer instead of by convention. 块1 — package door (publish-time materialization): - metadata-protocol: generic publish-time materializer registry (registerPublishMaterializer); publishMetaItem projects a published body into its data-plane row and surfaces materializeApplied (best-effort, never thrown — same contract as seedApplied). publishPackageDrafts aggregates it across the batch so a refused set is surfaced, not swallowed. - promoteDraft now returns the draft's packageId so the materializer stamps the owning package. - plugin-security registers a `permission` materializer that upserts the published set into sys_permission_set with managed_by:'package' + package_id. The single-set upsert (upsertPackagePermissionSet) is shared with bootstrapDeclaredPermissions, so boot and publish apply identical own-row / foreign-package / env-authored rules. A publish that materializes nothing (no owning package / name owned elsewhere) reports success:false. 块2 — admin door (data-layer write gate): - The security middleware refuses any admin-door write to a sys_permission_set row with managed_by:'package', and refuses a payload (insert OR update, single object OR array) that forges managed_by:'package'. - Placed before the empty-principal fall-open and the CRUD check, so it is a real, unconditional boundary — it holds for a principal-less context and for a superuser with modifyAllRecords. A multi-row/filter write is denied only when a package-owned row actually falls within the write's own filter, so env-only bulk edits still succeed. System/boot writes carry isSystem and bypass the middleware, so the seeder and materializer are unaffected. Tested: unit (materialize semantics, gate incl. array-forge / update-to-forge / principal-less / precise-bulk) + a dogfood test booting the real showcase stack that publishes a package permission draft and drives the admin door against the seeded package row over REST. Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_014y5kiH3aPLWtRRRGcVrXcT --- .../adr-0086-two-doors-permission-sets.md | 36 +++++ .../test/two-doors-permission.dogfood.test.ts | 111 +++++++++++++ packages/metadata-protocol/src/protocol.ts | 117 ++++++++++++++ .../src/sys-metadata-repository.ts | 7 +- .../bootstrap-declared-permissions.test.ts | 59 ++++++- .../src/bootstrap-declared-permissions.ts | 125 +++++++++------ .../src/security-plugin.test.ts | 149 ++++++++++++++++++ .../plugin-security/src/security-plugin.ts | 135 +++++++++++++++- 8 files changed, 688 insertions(+), 51 deletions(-) create mode 100644 .changeset/adr-0086-two-doors-permission-sets.md create mode 100644 packages/dogfood/test/two-doors-permission.dogfood.test.ts diff --git a/.changeset/adr-0086-two-doors-permission-sets.md b/.changeset/adr-0086-two-doors-permission-sets.md new file mode 100644 index 0000000000..d45f90192d --- /dev/null +++ b/.changeset/adr-0086-two-doors-permission-sets.md @@ -0,0 +1,36 @@ +--- +"@objectstack/metadata-protocol": minor +"@objectstack/plugin-security": minor +--- + +feat(security): two-doors separation for permission sets (ADR-0086 P2) + +Splits who may change a permission set into two non-overlapping doors, enforced +at the data layer instead of by convention: + +**块1 — the package door (publish-time materialization).** +`ObjectStackProtocolImplementation` gains a generic publish-time materializer +registry (`registerPublishMaterializer(type, fn)`). When a draft of a registered +type is published, its body is projected into a data-plane row and the result is +surfaced on the publish response as `materializeApplied` (best-effort, never +thrown — same contract as `seedApplied`). `promoteDraft` now returns the draft's +`packageId` so the materializer can stamp the owning package. `plugin-security` +registers a `permission` materializer that upserts the published set into +`sys_permission_set` with `managed_by:'package'` + `package_id` — so a set +authored through the studio package door (saved as a `permission` draft, then +published) lands in the admin surface with the exact provenance the boot seeder +already stamps, now on the runtime publish path too. The single-set upsert is +shared with `bootstrapDeclaredPermissions` (`upsertPackagePermissionSet`), so +both paths apply the same own-row / foreign-package / env-authored rules. + +**块2 — the admin door (data-layer write gate).** +The security middleware now refuses any admin-door write +(`update`/`delete`/`transfer`/`restore`/`purge`) to a `sys_permission_set` row +with `managed_by:'package'`, and refuses an `insert` that forges +`managed_by:'package'`. The gate fails closed regardless of the caller's grants +(a platform admin with `modifyAllRecords` is blocked just the same), so it is a +real data-layer boundary rather than a UI hint. System/boot writes carry +`isSystem` and bypass the whole middleware, so the boot seeder and the publish +materializer are unaffected. Env-authored sets (`managed_by` `user`/`platform` +or absent) stay freely editable through the admin door — the two doors never +overwrite each other. diff --git a/packages/dogfood/test/two-doors-permission.dogfood.test.ts b/packages/dogfood/test/two-doors-permission.dogfood.test.ts new file mode 100644 index 0000000000..ff077376a4 --- /dev/null +++ b/packages/dogfood/test/two-doors-permission.dogfood.test.ts @@ -0,0 +1,111 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// ADR-0086 P2 — "two doors" separation for permission sets, proven on the real +// showcase stack (which declares `showcase_contributor` as a package set): +// +// 块1 — the PACKAGE door: a permission set authored as a `permission` metadata +// draft under a package and then PUBLISHED is materialized into +// `sys_permission_set` with `managed_by:'package'` + the owning +// `package_id` (publish-time, not just at boot). A draft alone +// materializes nothing — only publish makes it live. +// +// 块2 — the ADMIN door: the generic data-plane write path +// (`PATCH /data/sys_permission_set/:id`) refuses to mutate a +// package-managed row — even for the platform admin — so the two doors +// never overwrite each other. An env-authored row stays freely editable. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; + +describe('two-doors permission separation (ADR-0086 P2)', () => { + let stack: VerifyStack; + let ql: any; + let protocol: any; + let adminToken: string; + + beforeAll(async () => { + stack = await bootStack(showcaseStack); + adminToken = await stack.signIn(); + ql = await stack.kernel.getServiceAsync('objectql'); + protocol = await stack.kernel.getServiceAsync('protocol'); + }, 60_000); + afterAll(async () => { await stack?.stop(); }); + + const findSet = async (name: string) => + (await ql.find('sys_permission_set', { where: { name } }, { context: { isSystem: true } }))?.[0]; + + // ── 块1 — package door: draft → publish → materialize ──────────────────── + it('块1: publishing a package permission draft materializes a package-managed row', async () => { + // An unregistered authoring-workspace id is a WRITABLE base (isWritablePackage), + // standing in for the package the studio package door edits. + const PKG = 'com.example.twodoors_ws'; + const NAME = 'twodoors_pkgset'; + + await protocol.saveMetaItem({ + type: 'permission', + name: NAME, + mode: 'draft', + packageId: PKG, + item: { + name: NAME, + label: 'Two Doors Set', + objects: { crm_lead: { allowRead: true, allowCreate: true } }, + }, + }); + + // Draft only — enforcement/admin-surface must NOT see it yet. + expect(await findSet(NAME), 'a draft must not materialize a data row').toBeFalsy(); + + const pub = await protocol.publishMetaItem({ type: 'permission', name: NAME }); + expect(pub.materializeApplied, 'publish surfaces the materialize result').toBeTruthy(); + expect(pub.materializeApplied.success).toBe(true); + + const row = await findSet(NAME); + expect(row, 'published set is now a real record').toBeTruthy(); + expect(row.managed_by).toBe('package'); + expect(row.package_id).toBe(PKG); + expect(JSON.parse(row.object_permissions || '{}')).toEqual({ + crm_lead: { allowRead: true, allowCreate: true }, + }); + }); + + // ── 块2 — admin door: write gate on package-managed rows ────────────────── + it('块2: the admin data door CANNOT edit a package-managed set (403), even as platform admin', async () => { + const contributor = await findSet('showcase_contributor'); + expect(contributor?.managed_by, 'showcase_contributor is package-owned').toBe('package'); + + const res = await stack.apiAs(adminToken, 'PATCH', `/data/sys_permission_set/${contributor.id}`, { + label: 'hijacked-through-admin-door', + }); + expect(res.status).toBe(403); + + // And the row is untouched. + const after = await findSet('showcase_contributor'); + expect(after.label).toBe(contributor.label); + }); + + it('块2: the admin door CAN still edit an env-authored set (isolates the gate to package rows)', async () => { + const memberDefault = await findSet('member_default'); + expect(memberDefault?.managed_by ?? null, 'member_default is env-owned').not.toBe('package'); + + const res = await stack.apiAs(adminToken, 'PATCH', `/data/sys_permission_set/${memberDefault.id}`, { + description: 'edited through the admin door', + }); + expect(res.status).toBeLessThan(300); + + const after = await findSet('member_default'); + expect(after.description).toBe('edited through the admin door'); + }); + + it('块2: the admin door cannot forge package provenance on insert', async () => { + const res = await stack.apiAs(adminToken, 'POST', '/data/sys_permission_set', { + name: 'forged_pkg_set', + label: 'Forged', + managed_by: 'package', + package_id: 'com.example.twodoors_ws', + }); + expect(res.status).toBe(403); + expect(await findSet('forged_pkg_set'), 'the forged row must not exist').toBeFalsy(); + }); +}); diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 4dd6b66acd..417cabe10d 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -695,6 +695,30 @@ function detectDestructiveObjectChanges(prev: any, next: any): Array<{ return issues; } +/** + * Result of projecting a published metadata body into its data-plane + * representation. `success:false` with an `error` is the surfaced-not-thrown + * failure contract — publishing the metadata itself always succeeds. + */ +export interface PublishMaterializeResult { + success: boolean; + inserted: number; + updated: number; + error?: string; +} + +/** + * Publish-time materializer (ADR-0086 P2). Receives the just-published body + * plus the draft's package binding and org scope. Registered per metadata type + * via {@link ObjectStackProtocolImplementation.registerPublishMaterializer}. + */ +export type PublishMaterializer = (args: { + body: unknown; + packageId: string | null; + organizationId: string | null; + actor: string; +}) => Promise; + export class ObjectStackProtocolImplementation implements ObjectStackProtocol { private engine: MetadataHostEngine; private getServicesRegistry?: () => Map; @@ -717,6 +741,19 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { */ private overlayRepos = new Map(); + /** + * Publish-time materializers keyed by singular metadata type (ADR-0086 P2). + * When a draft of a registered type is published, its body is projected + * into a data-plane representation the admin surface reads — e.g. a + * `permission` set is upserted into `sys_permission_set` with + * `managed_by:'package'`. Domain plugins own the projection (the generic + * protocol layer must not know `sys_permission_set`'s field shape), so they + * register here at init. Best-effort — a materializer failure is surfaced on + * the publish response, never thrown (publishing metadata always succeeds + * independently; the same contract as `seed` apply). + */ + private publishMaterializers = new Map(); + constructor( engine: IDataEngine, getServicesRegistry?: () => Map, @@ -729,6 +766,18 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { this.environmentId = environmentId; } + /** + * Register a publish-time materializer for a metadata type (ADR-0086 P2). + * Called by domain plugins at init (e.g. plugin-security registers the + * `permission` → `sys_permission_set` projection). The singular type name is + * used — `permissions` and `permission` both resolve here. One materializer + * per type; a second registration replaces the first (idempotent re-init). + */ + registerPublishMaterializer(type: string, materializer: PublishMaterializer): void { + const singular = PLURAL_TO_SINGULAR[type] ?? type; + this.publishMaterializers.set(singular, materializer); + } + /** * Lazily obtain a SysMetadataRepository for the given organization. * Env-wide overlays (organizationId == null) share a singleton under @@ -4042,6 +4091,13 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { error?: string; errors?: unknown[]; }; + /** + * Present when a publish-time materializer is registered for this type + * (ADR-0086 P2 — e.g. `permission` → `sys_permission_set`): the result + * of projecting the published body into its data-plane row. Best-effort, + * same contract as `seedApplied` — surfaced, never thrown. + */ + materializeApplied?: PublishMaterializeResult; }> { const singularType = PLURAL_TO_SINGULAR[request.type] ?? request.type; if (!ObjectStackProtocolImplementation.isOverlayAllowed(singularType) @@ -4098,6 +4154,7 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { seq: number; message?: string; seedApplied?: { success: boolean; inserted: number; updated: number; error?: string; errors?: unknown[] }; + materializeApplied?: PublishMaterializeResult; } = { success: true, version: result.version, @@ -4112,6 +4169,29 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { if (singularType === 'seed' && !request._skipSeedApply) { response.seedApplied = await this.applySeedBodies([result.item.body], orgId); } + // Publish-time materializer (ADR-0086 P2): project the published body + // into its data-plane row (e.g. `permission` → `sys_permission_set` + // with `managed_by:'package'`). Unlike seeds this needs no batch + // ordering — permission sets carry no cross-item references — so it + // runs on every publish path, package-draft batch included. The + // owning `package_id` rides on `result.packageId` (the draft's + // binding), so a package-door set materializes under the right owner. + const materializer = this.publishMaterializers.get(singularType); + if (materializer) { + try { + response.materializeApplied = await materializer({ + body: result.item.body, + packageId: result.packageId, + organizationId: orgId, + actor: request.actor ?? 'system', + }); + } catch (e: any) { + response.materializeApplied = { + success: false, inserted: 0, updated: 0, + error: e?.message ?? 'materialize failed', + }; + } + } return response; } catch (err: any) { if (err instanceof ConflictError) { @@ -4243,6 +4323,20 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { failed: Array<{ type: string; name: string; error: string; code?: string }>; /** Aggregate result of materializing every published `seed` (absent when no seeds). */ seedApplied?: { success: boolean; inserted: number; updated: number; error?: string; errors?: unknown[] }; + /** + * ADR-0086 P2 — aggregate result of publish-time materializers across the + * batch (e.g. `permission` → `sys_permission_set`). Absent when no + * published item had a registered materializer. `failures` names each + * item whose projection did NOT land (e.g. a permission-set name owned by + * the env door or another package) so the caller surfaces it instead of + * reporting a clean publish over a set that never went live. + */ + materializeApplied?: { + success: boolean; + inserted: number; + updated: number; + failures: Array<{ type: string; name: string; error: string }>; + }; /** * ADR-0038 L3 — post-publish runtime probe report (absent when nothing * was publishable). One real read per published artifact: seeded @@ -4295,6 +4389,10 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { } } const publishedSeqs: number[] = []; + // ADR-0086 P2 — accumulate each item's publish-time materialization so a + // batch package publish surfaces a permission set that failed to go live + // (owned by the env door / another package), not just a clean count. + const materialize = { any: false, inserted: 0, updated: 0, failures: [] as Array<{ type: string; name: string; error: string }> }; for (const d of ordered) { try { @@ -4316,6 +4414,17 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { }); published.push({ type: d.type, name: d.name, version: r.version }); if (typeof r.seq === 'number') publishedSeqs.push(r.seq); + if (r.materializeApplied) { + materialize.any = true; + materialize.inserted += r.materializeApplied.inserted; + materialize.updated += r.materializeApplied.updated; + if (!r.materializeApplied.success) { + materialize.failures.push({ + type: d.type, name: d.name, + error: r.materializeApplied.error ?? 'materialize failed', + }); + } + } } catch (e: any) { failed.push({ type: d.type, @@ -4384,6 +4493,14 @@ export class ObjectStackProtocolImplementation implements ObjectStackProtocol { published, failed, ...(seedApplied ? { seedApplied } : {}), + ...(materialize.any + ? { materializeApplied: { + success: materialize.failures.length === 0, + inserted: materialize.inserted, + updated: materialize.updated, + failures: materialize.failures, + } } + : {}), ...(probes ? { probes } : {}), ...(commit ? { commitId: commit.commitId } : {}), }; diff --git a/packages/metadata-protocol/src/sys-metadata-repository.ts b/packages/metadata-protocol/src/sys-metadata-repository.ts index d0bb22f21b..b8fa5339ba 100644 --- a/packages/metadata-protocol/src/sys-metadata-repository.ts +++ b/packages/metadata-protocol/src/sys-metadata-repository.ts @@ -577,7 +577,7 @@ export class SysMetadataRepository implements MetadataRepository { async promoteDraft( ref: MetaRef, opts: { actor: string; source?: string; message?: string; intent?: MetadataWriteIntent }, - ): Promise<{ version: string; seq: number; item: MetadataItem }> { + ): Promise<{ version: string; seq: number; item: MetadataItem; packageId: string | null }> { this.assertOpen(); // Read the RAW draft row (not just the body) so the promotion can carry // the draft's package binding onto the active row. ADR-0048 keys overlay @@ -629,7 +629,10 @@ export class SysMetadataRepository implements MetadataRepository { // best-effort: a concurrent publisher may have already drained // the draft; the active row's authoritative content is intact. } - return result; + // Surface the promoted draft's package binding so publish-time + // materializers (ADR-0086 P2 — package-door permission sets) can stamp + // the data-plane row with the owning `package_id`. + return { ...result, packageId: draftPackageId }; } /** 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 63ee85f37f..3720bb1bc6 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,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; -import { bootstrapDeclaredPermissions } from './bootstrap-declared-permissions.js'; +import { bootstrapDeclaredPermissions, upsertPackagePermissionSet } from './bootstrap-declared-permissions.js'; /** Minimal in-memory ql + registry for sys_permission_set seeding. */ function makeQl(declared: any[] = []) { @@ -111,3 +111,60 @@ describe('bootstrapDeclaredPermissions (ADR-0086 D5)', () => { expect(ql.rows[0].package_id).toBe('com.example.declared'); }); }); + +// ADR-0086 P2 块1 — the publish-time materializer shares this helper. Here the +// packageId is supplied explicitly (the draft's binding), not read off the body. +describe('upsertPackagePermissionSet (ADR-0086 P2 — publish materialization)', () => { + const publishedBody = (over: Record = {}) => ({ + name: 'crm_sales_rep', + label: 'Sales Rep', + objects: { crm_lead: { allowRead: true } }, + ...over, + }); + + it('materializes a published set into a package-managed row under the draft packageId', async () => { + const ql = makeQl(); + const r = await upsertPackagePermissionSet(ql, publishedBody(), 'com.example.crm'); + expect(r.seeded).toBe(1); + expect(ql.rows[0].managed_by).toBe('package'); + expect(ql.rows[0].package_id).toBe('com.example.crm'); + expect(JSON.parse(ql.rows[0].object_permissions)).toEqual({ crm_lead: { allowRead: true } }); + }); + + it('re-publish of its OWN row updates in place (idempotent)', async () => { + const ql = makeQl(); + await upsertPackagePermissionSet(ql, publishedBody(), 'com.example.crm'); + const r2 = await upsertPackagePermissionSet( + ql, publishedBody({ objects: { crm_lead: { allowRead: true, allowEdit: true } } }), 'com.example.crm', + ); + expect(r2.updated).toBe(1); + expect(ql.rows.length).toBe(1); + expect(JSON.parse(ql.rows[0].object_permissions)).toEqual({ crm_lead: { allowRead: true, allowEdit: true } }); + }); + + it('refuses to clobber an env-authored row of the same name (two-doors: env door owns it)', async () => { + const ql = makeQl(); + ql.rows.push({ id: 'ps_env', name: 'crm_sales_rep', managed_by: 'user', object_permissions: '{"kept":true}' }); + const r = await upsertPackagePermissionSet(ql, publishedBody(), 'com.example.crm'); + expect(r.skippedEnvAuthored).toBe(1); + expect(r.seeded + r.updated).toBe(0); + expect(ql.rows[0].object_permissions).toBe('{"kept":true}'); + }); + + it('refuses a name owned by a DIFFERENT package', async () => { + const ql = makeQl(); + ql.rows.push({ id: 'ps_1', name: 'crm_sales_rep', managed_by: 'package', package_id: 'com.example.other', object_permissions: '{}' }); + const r = await upsertPackagePermissionSet(ql, publishedBody(), 'com.example.crm'); + expect(r.skippedForeign).toBe(1); + expect(ql.rows[0].package_id).toBe('com.example.other'); + }); + + it('skips (does not materialize) when the publish carries no packageId', async () => { + const ql = makeQl(); + const warns: string[] = []; + const r = await upsertPackagePermissionSet(ql, publishedBody(), null, { info: () => {}, warn: (m) => warns.push(m) }); + expect(r.seeded).toBe(0); + expect(ql.rows.length).toBe(0); + expect(warns.some((w) => w.includes('no owning package'))).toBe(true); + }); +}); diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index 3947965d0d..a3634df6e2 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -85,12 +85,82 @@ function toRowFields(ps: any): Record { }; } +export interface PermissionSeedOutcome { + seeded: number; + updated: number; + skippedEnvAuthored: number; + skippedForeign: number; +} + +/** + * Upsert ONE declared/published PermissionSet body into `sys_permission_set` + * under the owning `packageId`, applying the ADR-0086 provenance rules + * (own-row re-seed, foreign-package refuse, env-authored never clobbered). + * Shared by the boot seeder (every declared set) and the publish-time + * materializer (ADR-0086 P2 — a package-door set promoted from a draft). Returns + * a one-hot outcome so callers can aggregate. + */ +export async function upsertPackagePermissionSet( + ql: any, + ps: any, + packageId: string | null | undefined, + logger?: SeedOptions['logger'], +): Promise { + const out: PermissionSeedOutcome = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 }; + if (!ps?.name) return out; + // A `managed_by:'package'` row without a `package_id` would make uninstall + // undefined again — the exact ambiguity ADR-0086 D3 exists to remove — so a + // set with no resolvable owner is skipped rather than materialized unowned. + if (!packageId) { + logger?.warn?.('[security] permission set has no owning package — not materialized', { name: ps.name }); + return out; + } + + const existing = (await tryFind(ql, 'sys_permission_set', { name: ps.name }, 1))[0]; + if (!existing?.id) { + const created = await tryInsert(ql, 'sys_permission_set', { + id: genId('ps'), + name: ps.name, + ...toRowFields(ps), + active: true, + package_id: packageId, + managed_by: 'package', + }); + if (created) out.seeded += 1; + return out; + } + + if (existing.managed_by === 'package') { + if (existing.package_id === packageId) { + // Our own row — re-seed so the record always reflects the shipped/published + // declaration (idempotent; covers version bumps without bookkeeping). + if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...toRowFields(ps) })) { + out.updated += 1; + } + } else { + // Package-namespaced object api names make set-name collisions a + // packaging bug, not a merge case — refuse loudly (ADR-0086 D4: + // a package never writes into a foreign record). + out.skippedForeign += 1; + logger?.warn?.('[security] permission set name owned by another package — skipped', { + name: ps.name, declaredBy: packageId, ownedBy: existing.package_id, + }); + } + return out; + } + + // `platform`/`user` — or absent (legacy rows, incl. bootstrapPlatformAdmin + // defaults): env-authored config. Never clobbered by package materialization. + out.skippedEnvAuthored += 1; + return out; +} + export async function bootstrapDeclaredPermissions( ql: any, metadataService: any, options: SeedOptions = {}, -): Promise<{ seeded: number; updated: number; skippedEnvAuthored: number; skippedForeign: number }> { - const out = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 }; +): Promise { + const out: PermissionSeedOutcome = { seeded: 0, updated: 0, skippedEnvAuthored: 0, skippedForeign: 0 }; if (!ql || typeof ql.find !== 'function' || typeof ql.insert !== 'function') return out; let sets: any[] = readDeclared(ql, 'permission'); @@ -105,52 +175,13 @@ export async function bootstrapDeclaredPermissions( for (const ps of sets) { if (!ps?.name) continue; // Registry provenance first (ADR-0010 `_packageId`), author-declared - // spec `packageId` (ADR-0086 D3) as fallback. A declared set with NO - // resolvable owner is skipped: a `managed_by:'package'` row without a - // `package_id` would make uninstall undefined again — the exact - // ambiguity D3 exists to remove. + // spec `packageId` (ADR-0086 D3) as fallback. const packageId: string | undefined = ps._packageId ?? ps.packageId ?? undefined; - if (!packageId) { - options.logger?.warn?.('[security] declared permission set has no owning package — not seeded', { name: ps.name }); - continue; - } - - const existing = (await tryFind(ql, 'sys_permission_set', { name: ps.name }, 1))[0]; - if (!existing?.id) { - const created = await tryInsert(ql, 'sys_permission_set', { - id: genId('ps'), - name: ps.name, - ...toRowFields(ps), - active: true, - package_id: packageId, - managed_by: 'package', - }); - if (created) out.seeded += 1; - continue; - } - - if (existing.managed_by === 'package') { - if (existing.package_id === packageId) { - // Our own row — re-seed so the record always reflects the shipped - // declaration (idempotent; covers version bumps without bookkeeping). - if (await tryUpdate(ql, 'sys_permission_set', { id: existing.id, ...toRowFields(ps) })) { - out.updated += 1; - } - } else { - // Package-namespaced object api names make set-name collisions a - // packaging bug, not a merge case — refuse loudly (ADR-0086 D4: - // a package never writes into a foreign record). - out.skippedForeign += 1; - options.logger?.warn?.('[security] declared permission set name owned by another package — skipped', { - name: ps.name, declaredBy: packageId, ownedBy: existing.package_id, - }); - } - continue; - } - - // `platform`/`user` — or absent (legacy rows, incl. bootstrapPlatformAdmin - // defaults): env-authored config. Never clobbered by package seeding. - out.skippedEnvAuthored += 1; + const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger); + out.seeded += r.seeded; + out.updated += r.updated; + out.skippedEnvAuthored += r.skippedEnvAuthored; + out.skippedForeign += r.skippedForeign; } options.logger?.info?.('[security] declared permission sets seeded into sys_permission_set (ADR-0086 D5)', { diff --git a/packages/plugins/plugin-security/src/security-plugin.test.ts b/packages/plugins/plugin-security/src/security-plugin.test.ts index 7896b58a1a..d65c814c30 100644 --- a/packages/plugins/plugin-security/src/security-plugin.test.ts +++ b/packages/plugins/plugin-security/src/security-plugin.test.ts @@ -942,6 +942,155 @@ describe('SecurityPlugin', () => { await expect(h.run(opCtx)).resolves.toBeDefined(); }); }); + + // ── ADR-0086 P2 (块2) — two-doors data-layer write gate ──────────────── + // A `managed_by:'package'` sys_permission_set row is owned by the package + // door; the admin data-plane write path must refuse to mutate it, even for a + // superuser (modifyAllRecords). System/boot writes carry isSystem and never + // reach the gate. + describe('two-doors write gate (sys_permission_set managed_by:package)', () => { + // Superuser set — proves the gate is NOT grant-gated (blocks even + // modifyAllRecords) and carries no RLS so the pre-image check is a no-op. + const adminSet: PermissionSet = { + name: 'admin_full_access', label: 'Admin', isProfile: true, + objects: { '*': { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true, modifyAllRecords: true } }, + } as any; + const adminCtx = { userId: 'admin1', tenantId: 'org-1', roles: [], permissions: ['admin_full_access'] }; + + const runGate = async (opCtx: any, findOneImpl?: (q: any) => any) => { + const plugin = new SecurityPlugin({ fallbackPermissionSet: 'admin_full_access' }); + const harness = makeMiddlewareCtx({ + permissionSets: [adminSet], + objectFields: ['id', 'name', 'managed_by', 'package_id'], + ...(findOneImpl ? { findOneImpl } : {}), + }); + await plugin.init(harness.ctx); + await plugin.start(harness.ctx); + return harness.run(opCtx); + }; + + it('DENIES an admin update of a package-managed set (even with modifyAllRecords)', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'update', + data: { id: 'ps_pkg', label: 'hijack' }, options: { where: { id: 'ps_pkg' } }, + context: adminCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'ps_pkg', name: 'crm_sales_rep', managed_by: 'package', package_id: 'com.example.crm' })), + ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('DENIES an admin delete of a package-managed set', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'delete', + options: { where: { id: 'ps_pkg' } }, + context: adminCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'ps_pkg', name: 'crm_sales_rep', managed_by: 'package', package_id: 'com.example.crm' })), + ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('ALLOWS an admin update of an env-authored set (managed_by:user)', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'update', + data: { id: 'ps_env', label: 'renamed' }, options: { where: { id: 'ps_env' } }, + context: adminCtx, + }; + await expect( + runGate(opCtx, () => ({ id: 'ps_env', name: 'my_custom', managed_by: 'user' })), + ).resolves.toBeDefined(); + }); + + it('DENIES an admin-door insert that forges package provenance', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'insert', + data: { name: 'forged', managed_by: 'package', package_id: 'com.example.crm' }, + context: adminCtx, + }; + await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('DENIES a bulk/ARRAY insert that forges package provenance on any element', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'insert', + data: [ + { name: 'ok_custom' }, + { name: 'forged', managed_by: 'package', package_id: 'com.example.crm' }, + ], + context: adminCtx, + }; + await expect(runGate(opCtx)).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('DENIES an update that RE-BADGES an env row as package-managed (update-to-forge)', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'update', + data: { id: 'ps_env', managed_by: 'package', package_id: 'com.example.crm' }, + options: { where: { id: 'ps_env' } }, + context: adminCtx, + }; + // Even though the EXISTING row is env-owned, the payload forges provenance. + await expect( + runGate(opCtx, () => ({ id: 'ps_env', name: 'my_custom', managed_by: 'user' })), + ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('DENIES even a principal-less write to a package row (gate is before the fall-open)', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'delete', + options: { where: { id: 'ps_pkg' } }, + context: {}, // no roles, no permissions, no userId, not isSystem + }; + await expect( + runGate(opCtx, () => ({ id: 'ps_pkg', managed_by: 'package', package_id: 'com.example.crm' })), + ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('ALLOWS a normal admin-door insert (no forged provenance)', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'insert', + data: { name: 'my_custom', label: 'Custom' }, + context: adminCtx, + }; + await expect(runGate(opCtx)).resolves.toBeDefined(); + }); + + it('DENIES a filter write whose filter matches a package-managed row', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'delete', + options: { where: { active: true } }, // no single id → filter path + context: adminCtx, + }; + await expect( + // the gate probes for a package row within the filter → found → deny + runGate(opCtx, () => ({ id: 'ps_pkg', managed_by: 'package' })), + ).rejects.toMatchObject({ name: 'PermissionDeniedError' }); + }); + + it('ALLOWS a filter write that matches only env-authored rows (no over-broad block)', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'update', + data: { label: 'bulk-rename' }, options: { where: { managed_by: 'user' } }, + context: adminCtx, + }; + await expect( + // the gate's package-row probe finds nothing within the filter → allow + runGate(opCtx, () => null), + ).resolves.toBeDefined(); + }); + + it('lets system/boot writes through (isSystem bypass) even on a package row', async () => { + const opCtx: any = { + object: 'sys_permission_set', operation: 'update', + data: { id: 'ps_pkg' }, options: { where: { id: 'ps_pkg' } }, + context: { isSystem: true }, + }; + await expect( + runGate(opCtx, () => ({ id: 'ps_pkg', managed_by: 'package', package_id: 'com.example.crm' })), + ).resolves.toBeDefined(); + }); + }); }); // --------------------------------------------------------------------------- describe('PermissionEvaluator', () => { diff --git a/packages/plugins/plugin-security/src/security-plugin.ts b/packages/plugins/plugin-security/src/security-plugin.ts index c89ee14c4f..ab56f9994a 100644 --- a/packages/plugins/plugin-security/src/security-plugin.ts +++ b/packages/plugins/plugin-security/src/security-plugin.ts @@ -4,7 +4,7 @@ import { Plugin, PluginContext } from '@objectstack/core'; import type { PermissionSet, RowLevelSecurityPolicy } from '@objectstack/spec/security'; import { PermissionEvaluator } from './permission-evaluator.js'; import { bootstrapDeclaredRoles } from './bootstrap-declared-roles.js'; -import { bootstrapDeclaredPermissions } from './bootstrap-declared-permissions.js'; +import { bootstrapDeclaredPermissions, upsertPackagePermissionSet } from './bootstrap-declared-permissions.js'; import { bootstrapBuiltinRoles } from './bootstrap-builtin-roles.js'; import { bootstrapSystemCapabilities } from './bootstrap-system-capabilities.js'; import { RLSCompiler, RLS_DENY_FILTER } from './rls-compiler.js'; @@ -336,6 +336,19 @@ export class SecurityPlugin implements Plugin { ); } + // [ADR-0086 P2 — 块2] Two-doors write gate. A permission set stamped + // `managed_by:'package'` is owned by the PACKAGE door: it is authored in + // the package and lands via publish (块1). The ADMIN door (this data-plane + // write path) must NOT edit, delete, or forge that provenance — otherwise + // the next boot re-seed silently reverts the admin's change and the + // provenance axis becomes a lie. Placed BEFORE the empty-principal + // fall-open and the CRUD check so it is a real, unconditional data-layer + // boundary — it holds even for a principal-less context and even for a + // superuser with modifyAllRecords. System/boot writes carry `isSystem` and + // already short-circuited the whole middleware above, so the seeder and + // the publish materializer pass straight through. + await this.assertPackageManagedWriteGate(opCtx); + const roles = opCtx.context?.roles ?? []; const explicitPermissionSets = opCtx.context?.permissions ?? []; @@ -743,6 +756,45 @@ export class SecurityPlugin implements Plugin { } catch (e) { ctx.logger.warn('[security] declared-permission seeding failed', { error: (e as Error).message }); } + // [ADR-0086 P2 — 块1] Register the publish-time materializer so a + // permission set authored/edited through the PACKAGE door (saved as a + // `permission` draft, then published) lands in sys_permission_set with + // managed_by:'package' + package_id — the exact provenance the boot + // seeder stamps, only now on the runtime publish path instead of only at + // boot. Idempotent: registerPublishMaterializer replaces on re-run, and + // upsertPackagePermissionSet refuses to clobber env- or foreign-owned + // rows (ADR-0086 D4), so the two doors never overwrite each other. + try { + const protocol: any = ctx.getService?.('protocol'); + if (protocol && typeof protocol.registerPublishMaterializer === 'function') { + protocol.registerPublishMaterializer( + 'permission', + async (args: { body: unknown; packageId: string | null }) => { + const r = await upsertPackagePermissionSet(ql, args.body, args.packageId, ctx.logger); + const applied = r.seeded + r.updated; + // A publish that materialized nothing did NOT go live — report it + // as a failure with the reason so the package-door UI never shows + // a clean publish over a set the admin surface can't see (ADR-0049 + // honesty). The upsert only lands zero rows when it refused: the + // name is owned by another package, owned by the env door, or the + // publish carried no owning package_id to stamp. + if (applied === 0) { + return { + success: false, inserted: 0, updated: 0, + error: r.skippedForeign > 0 + ? 'permission set name is owned by another package' + : r.skippedEnvAuthored > 0 + ? 'permission set name is owned by the environment (edit it through the admin door)' + : 'permission set was not materialized (publish carried no owning package)', + }; + } + return { success: true, inserted: r.seeded, updated: r.updated }; + }, + ); + } + } catch (e) { + ctx.logger.warn('[security] permission publish-materializer registration failed', { error: (e as Error).message }); + } // [ADR-0068 D2] Seed the framework's reserved built-in identity roles // (platform_admin / org_*) so the role catalog is self-describing. try { @@ -970,6 +1022,87 @@ export class SecurityPlugin implements Plugin { * multi-row predicate and returns `null` (multi-row writes route through the * `*Many` paths, out of scope for the by-id pre-image check). */ + /** + * [ADR-0086 P2 — 块2] Two-doors data-layer write gate for `sys_permission_set`. + * + * A row with `managed_by:'package'` is owned by the package door (authored in + * the package, materialized on publish). The admin door — the generic + * `/api/v1/data/sys_permission_set` write path this middleware guards — must + * not mutate or delete it, nor may it forge that provenance on insert. Fails + * CLOSED and never depends on the caller's grants, so a platform admin with + * `modifyAllRecords` is blocked just the same. System/boot writes never reach + * here (the middleware short-circuits on `isSystem`), so the seeder and the + * publish materializer are unaffected. + */ + private async assertPackageManagedWriteGate(opCtx: any): Promise { + if (opCtx?.object !== 'sys_permission_set') return; + const op = opCtx.operation; + if (!['insert', 'update', 'delete', 'transfer', 'restore', 'purge'].includes(op)) return; + + // (a) Reject any admin-door PAYLOAD that CLAIMS package provenance + // (`managed_by:'package'`), on insert OR update, single object OR array + // (`engine.insert`/`update` both accept `T | T[]` and route arrays + // through this same middleware). Only the package publish path — which + // carries `isSystem` and short-circuited the whole middleware above — + // may stamp package provenance. This also closes update-to-forge: an + // env row cannot be re-badged package-managed through the admin door. + const payloadRows = Array.isArray(opCtx.data) + ? opCtx.data + : (opCtx.data && typeof opCtx.data === 'object' ? [opCtx.data] : []); + if (payloadRows.some((r: unknown) => r && typeof r === 'object' && (r as Record).managed_by === 'package')) { + throw new PermissionDeniedError( + `[Security] Access denied: cannot set 'managed_by:package' on a permission set through the admin door — ` + + `package permission sets are authored in their package and land via publish (ADR-0086 two-doors).`, + { operation: op, object: opCtx.object }, + ); + } + if (op === 'insert') return; // no existing row to protect + + if (!this.ql) return; + + const targetId = this.extractSingleId(opCtx); + if (targetId == null) { + // Multi-row / filter write with no single id. Deny ONLY if a package-owned + // row actually falls within the write's own filter — so a bulk edit that + // targets only env-authored rows still succeeds (no over-broad block). A + // whole-table write (no filter) matches every package row, so it is denied. + const writeWhere = opCtx?.options?.where; + const packageWhere = writeWhere && typeof writeWhere === 'object' + ? { $and: [writeWhere, { managed_by: 'package' }] } + : { managed_by: 'package' }; + const hitsPackageRow = await this.ql + .findOne('sys_permission_set', { where: packageWhere, context: { isSystem: true } }) + .catch(() => null); + if (hitsPackageRow) { + throw new PermissionDeniedError( + `[Security] Access denied: this '${op}' on 'sys_permission_set' targets one or more package-managed ` + + `rows — change those by editing their package and re-publishing, not through the admin door ` + + `(ADR-0086 two-doors separation).`, + { operation: op, object: opCtx.object }, + ); + } + return; + } + + const existing = await this.ql + .findOne('sys_permission_set', { where: { id: targetId }, context: { isSystem: true } }) + .catch(() => null); + if (existing && (existing as Record).managed_by === 'package') { + const row = existing as Record; + throw new PermissionDeniedError( + `[Security] Access denied: '${String(row.name ?? targetId)}' is a package-managed permission set ` + + `(managed_by:'package') — change it by editing its package and re-publishing, not through the ` + + `admin door (ADR-0086 two-doors separation).`, + { + operation: op, + object: opCtx.object, + recordId: targetId, + packageId: (row.package_id as string | null) ?? null, + }, + ); + } + } + private extractSingleId(opCtx: any): string | number | bigint | null { const isScalar = (v: unknown): v is string | number | bigint => v !== null && (typeof v === 'string' || typeof v === 'number' || typeof v === 'bigint');