From 696cb63c2d64203d7498c46174d2a690fbe27016 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 05:46:27 +0000 Subject: [PATCH 1/2] fix(plugin-security): a cloned permission set is not an unowned declaration (WIP) The boot loop tells an environment-owned set apart from a package declaration before it judges ownership. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- .../src/bootstrap-declared-permissions.ts | 63 ++++++++++++++++++- 1 file changed, 60 insertions(+), 3 deletions(-) diff --git a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts index 1fb46ec5815..56a5addec78 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts @@ -182,6 +182,48 @@ export function readDeclared(engine: any, type: string): any[] { return []; } +/** + * ADR-0086 two-doors — is this `sys_permission_set` row ENVIRONMENT-owned? + * + * The one spelling of the fact this seeder already acted on: a row the PACKAGE + * door does not own — `managed_by` `admin`/`platform`, the legacy `user`, or + * absent (pre-provenance rows, including `bootstrapPlatformAdmin`'s platform + * defaults) — is environment-authored config that package materialization + * never clobbers. {@link upsertPackagePermissionSet}'s last branch and the boot + * loop's unowned-vs-environment split both ask THIS predicate, so the two can + * never disagree about which rows are the environment's. + */ +function isEnvironmentOwnedRow(row: any): boolean { + return !!row && typeof row === 'object' && row.managed_by !== 'package'; +} + +/** + * [#21669] Does the environment already own a row under this name? + * + * The engine registry the boot loop walks holds MORE than package + * declarations: `loadMetaFromDb` hydrates every env-wide `sys_metadata` + * `permission` row into the same collection, and a set an environment + * authored itself — the Setup Clone action, a Setup "New", a metadata-door + * save — carries no package id there, because none exists. Read as a + * declaration, such an item is "unowned"; it is not a declaration at all. Its + * row stands, the environment door owns it, and Setup lists it. + * + * Answered from the batched oracle the loop already holds — no extra read. + * The row is this pass's own, or — on a per-organization pass — the + * organization-less row the ENVIRONMENT door writes (`permission-set- + * projection.ts` is deliberately not per-organization; see this file's + * header). `unknown` is NOT an answer: a read that failed cannot prove the + * environment owns the name, so the caller falls through to the unowned + * refusal exactly as before. + */ +async function environmentOwnsName(existingByName: ExistingByNameIndex, name: string): Promise { + const lookup = await existingByName.get(name); + const row = lookup.status === 'present' + ? lookup.row + : lookup.status === 'absent' ? lookup.organizationLessResidue : undefined; + return isEnvironmentOwnedRow(row); +} + /** * Upsert ONE declared/published PermissionSet body into `sys_permission_set` * under the owning `packageId`, applying the ADR-0086 provenance rules @@ -297,7 +339,7 @@ export async function upsertPackagePermissionSet( return out; } - if (existing.managed_by === 'package') { + if (!isEnvironmentOwnedRow(existing)) { if (!permissionSetNameIsForeign(existing.package_id, packageId)) { // Our own row — re-seed so the record always reflects the shipped/published // declaration (idempotent; covers version bumps without bookkeeping). @@ -359,8 +401,9 @@ export async function upsertPackagePermissionSet( return out; } - // `platform`/`user` — or absent (legacy rows, incl. bootstrapPlatformAdmin - // defaults): env-authored config. Never clobbered by package materialization. + // {@link isEnvironmentOwnedRow}: `admin`/`platform`/`user` — or absent + // (legacy rows, incl. bootstrapPlatformAdmin defaults): env-authored config. + // Never clobbered by package materialization. out.skippedEnvAuthored += 1; return out; } @@ -417,6 +460,20 @@ export async function bootstrapDeclaredPermissions( // Registry provenance first (ADR-0010 `_packageId`), author-declared // spec `packageId` (ADR-0086 D3) as fallback. const packageId: string | undefined = ps._packageId ?? ps.packageId ?? undefined; + // [#21669] Tell an ENVIRONMENT-owned set apart from a package declaration + // BEFORE judging ownership. A registry item with no package id whose row + // the environment already owns (a Setup clone: `managed_by:'admin'`, no + // `package_id`) is not an unowned declaration — it is not a declaration — + // and the `permission_set_declaration_unowned` warning told the operator, + // on every boot and every `metadata:reloaded`, that Setup could not see a + // set Setup lists and edits. It lands where every package declaration + // over an environment row lands: `skippedEnvAuthored`, never clobbered. + // ⛔ A declaration with no owner AND no environment row still reaches the + // unowned refusal below, unchanged — that warning is true. + if (!packageId && await environmentOwnsName(existingByName, String(ps.name))) { + out.skippedEnvAuthored += 1; + continue; + } const r = await upsertPackagePermissionSet(ql, ps, packageId, options.logger, { existingByName, organizationId, residue, refusals, collisions }); out.seeded += r.seeded; out.updated += r.updated; From 067aa4f58f1434dcd0d1675c5aee76c57b1636f1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 4 Oct 2026 05:51:31 +0000 Subject: [PATCH 2/2] test(plugin-security): pin a cloned set boots without the unowned warning, and a truly unowned one still warns Unit pins at the seeder seam (single and per-organization passes, the package-owned-row and unreadable controls, conservation) and a boot-level pin over the real showcase composition: clone through the shipped action's own payload on boot 1, assert the walk holds the clone and no unowned line names it on boot 2. Plus the patch changeset. Claude-Session: https://claude.ai/code/session_01DiCSbmJrkzNhuEAier4VoJ Co-authored-by: Claude --- ...21669-cloned-permission-set-not-unowned.md | 13 ++ .../bootstrap-declared-permissions.test.ts | 153 ++++++++++++++++ ...clone-boot-unowned-warning.dogfood.test.ts | 166 ++++++++++++++++++ 3 files changed, 332 insertions(+) create mode 100644 .changeset/21669-cloned-permission-set-not-unowned.md create mode 100644 packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts diff --git a/.changeset/21669-cloned-permission-set-not-unowned.md b/.changeset/21669-cloned-permission-set-not-unowned.md new file mode 100644 index 00000000000..5e96402e777 --- /dev/null +++ b/.changeset/21669-cloned-permission-set-not-unowned.md @@ -0,0 +1,13 @@ +--- +'@objectstack/plugin-security': patch +--- + +fix(plugin-security): a permission set the environment cloned no longer logs `permission_set_declaration_unowned` on every boot (#21669) + +Clause-②: no + +The declared-permission seeding pass walks every `permission` item in the engine registry. That registry also holds the permission sets an environment authored itself, which boot hydration loads from their `sys_metadata` rows. A set made with Setup's **Clone** action is one of them, and it carries no package id because it has none. The pass judged "no owning package" before it looked at the set's row, so on every boot, and on every `metadata:reloaded`, it logged one warning per cloned set: `[permission_set_declaration_unowned] declared permission set "…" has no owning package — not materialized … the Setup admin surface reads sys_permission_set and cannot see this set`. That is false for a clone. Its row exists (`managed_by: admin`), and Setup lists it and edits it. + +The pass now checks the row first. A registry item with no package id whose `sys_permission_set` row the environment owns (`managed_by` other than `package`) is the environment's own set. It is counted as `skippedEnvAuthored`, the count a package declaration over an environment row already gets, and no warning is logged. The pass reads that row from the existence read it already makes, so no query is added. On a per-organization pass, the environment door's organization-less row counts too. + +Unchanged: a declaration with no owning package and no environment row still logs `permission_set_declaration_unowned`, with the same text, and still counts as `skippedUnowned`. If the row could not be read, the warning still fires, because an unreadable row does not prove the environment owns the set. The publish-time materializer is unchanged. Nothing is written or granted differently: only the false warning stops, and the clone moves from the `skippedUnowned` count to the `skippedEnvAuthored` count in the pass's summary line. 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 d2e0d9b7593..6eeb04de759 100644 --- a/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts +++ b/packages/plugins/plugin-security/src/bootstrap-declared-permissions.test.ts @@ -500,3 +500,156 @@ describe('[#18571] the unowned permission-set refusal moves a counter', () => { expect(ql.rows.some((r) => r.name === 'crm_orphan')).toBe(false); }); }); + +// ─────────────────────────────────────────────────────────────────────────── +// [#21669] An ENVIRONMENT-owned set in the registry walk is not an unowned +// declaration. The engine registry the boot loop walks also holds every +// env-wide `sys_metadata` `permission` row `loadMetaFromDb` hydrates — a set +// made by Setup's Clone action among them — and such an item carries no +// package id because it has none. The loop judged "unowned" before it read +// the row, so a set whose `managed_by:'admin'` row Setup lists and edits was +// reported, on every boot, as one Setup "cannot see". +// +// The item below is the shape measured on a real showcase cold boot after a +// Clone (the boot-level pin is +// `packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts`): +// no `_packageId`, no `packageId`, `_provenance: 'org'`. +// ─────────────────────────────────────────────────────────────────────────── + +const CLONE_NAME = 'crm_sales_rep_local'; + +/** The registry item a hydrated clone is — measured, not invented. */ +const clonedItem = () => ({ + name: CLONE_NAME, + label: 'Sales Rep (local)', + objects: { crm_lead: { allowRead: true, allowCreate: true } }, + fields: {}, + systemPermissions: [], + rowLevelSecurity: [], + tabPermissions: {}, + _provenance: 'org', +}); + +/** The row the Clone action leaves: the environment's own set. */ +const clonedRow = (over: Record = {}) => ({ + id: 'ps_clone', name: CLONE_NAME, managed_by: 'admin', package_id: null, active: true, + object_permissions: '{"crm_lead":{"allowRead":true,"allowCreate":true}}', + ...over, +}); + +/** The unowned refusal's first sentence — the text an operator reads, unchanged by this card. */ +const unownedFirstSentence = (name: string) => + `declared permission set "${name}" has no owning package — not materialized.`; + +describe('[#21669] an environment-owned set is told apart from an unowned declaration', () => { + it('a CLONED set (row managed_by admin) boots with no warning on any channel, and its row is untouched', async () => { + const ql = makeQl([clonedItem()]); + ql.rows.push(clonedRow()); + const before = JSON.stringify(ql.rows); + + const cap = captureAllConsole(); + let r: Awaited>; + try { + r = await bootstrapDeclaredPermissions(ql, undefined); + } finally { + cap.restore(); + } + + expect(cap.seen.filter((l) => l.includes(PERMISSION_SET_DECLARATION_UNOWNED))).toEqual([]); + expect(cap.seen).toEqual([]); + // Where a package declaration over an environment row already lands — + // never clobbered, and counted, so CONSERVATION still holds. + expect(r).toMatchObject({ skippedEnvAuthored: 1, skippedUnowned: 0, seeded: 0, updated: 0, unchanged: 0 }); + expect(JSON.stringify(ql.rows)).toBe(before); + }); + + it('the same, on a per-organization pass: the environment door\'s organization-less row is the environment\'s', async () => { + // `permission-set-projection.ts` (the ENVIRONMENT door) is deliberately not + // per-organization, so a clone's row carries no organization_id and a + // walled pass sees it only beside its own rows. + const ql = makeQl([clonedItem()]); + ql.rows.push(clonedRow()); + + const warn = vi.fn(); + const r = await bootstrapDeclaredPermissions(ql, undefined, { + logger: { info: () => {}, warn }, organizationId: 'org_a', platformBucketNames: [], + }); + + const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean); + expect(events).not.toContain(PERMISSION_SET_DECLARATION_UNOWNED); + expect(r).toMatchObject({ skippedEnvAuthored: 1, skippedUnowned: 0, seeded: 0 }); + expect(ql.rows).toHaveLength(1); + }); + + it('⛔ a TRULY unowned declaration — no package id, no row — still warns, with the same code and text', async () => { + const ql = makeQl([declaredSet({ name: 'crm_orphan', _packageId: undefined })]); + + const cap = captureAllConsole(); + let r: Awaited>; + try { + r = await bootstrapDeclaredPermissions(ql, undefined); + } finally { + cap.restore(); + } + + expect(cap.seen).toHaveLength(1); + expect(cap.seen[0]).toContain(`[${PERMISSION_SET_DECLARATION_UNOWNED}]`); + expect(cap.seen[0]).toContain(unownedFirstSentence('crm_orphan')); + expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 }); + expect(ql.rows).toHaveLength(0); + }); + + it('⛔ the same on a per-organization pass with no row at all', async () => { + const ql = makeQl([declaredSet({ name: 'crm_orphan', _packageId: undefined })]); + const warn = vi.fn(); + + const r = await bootstrapDeclaredPermissions(ql, undefined, { + logger: { info: () => {}, warn }, organizationId: 'org_a', platformBucketNames: [], + }); + + const unowned = warn.mock.calls.filter((c) => (c[1] as any)?.event === PERMISSION_SET_DECLARATION_UNOWNED); + expect(unowned).toHaveLength(1); + expect(String(unowned[0]![0])).toContain(unownedFirstSentence('crm_orphan')); + expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 }); + }); + + it('⛔ a row the PACKAGE door owns is not the environment\'s: an unowned item over it still warns', async () => { + // The guard asks who owns the ROW. A `managed_by:'package'` row under the + // name is a package's record, so it proves nothing about an item that + // carries no owner of its own. + const ql = makeQl([declaredSet({ _packageId: undefined })]); + ql.rows.push({ id: 'ps_pkg', name: 'crm_sales_rep', managed_by: 'package', package_id: 'com.example.crm' }); + + const warn = vi.fn(); + const r = await bootstrapDeclaredPermissions(ql, undefined, { logger: { info: () => {}, warn } }); + + const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean); + expect(events).toEqual([PERMISSION_SET_DECLARATION_UNOWNED]); + expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 }); + }); + + it('⛔ a read that cannot answer proves nothing: the unowned refusal stands, exactly as before', async () => { + const ql = unreadableQl([clonedItem()]); + const warn = vi.fn(); + + const r = await bootstrapDeclaredPermissions(ql, undefined, { logger: { info: () => {}, warn } }); + + const events = warn.mock.calls.map((c) => (c[1] as any)?.event).filter(Boolean); + expect(events).toContain(PERMISSION_SET_DECLARATION_UNOWNED); + expect(r).toMatchObject({ skippedUnowned: 1, skippedEnvAuthored: 0 }); + }); + + it('⭐ CONSERVATION, with a clone in the walk: every named item lands in exactly one counter', async () => { + const owned = declaredSet({ name: 'crm_new', _packageId: 'com.a' }); + const orphan = declaredSet({ name: 'crm_orphan', _packageId: undefined }); + const ql = makeQl([owned, clonedItem(), orphan]); + ql.rows.push(clonedRow()); + + const out = await bootstrapDeclaredPermissions(ql, undefined, { logger: mute() }); + + expect(out).toMatchObject({ seeded: 1, skippedEnvAuthored: 1, skippedUnowned: 1, unreadable: 0 }); + const counted = out.seeded + out.updated + out.unchanged + + out.skippedEnvAuthored + out.skippedForeign + out.skippedUnowned + out.unreadable; + expect(counted).toBe(3); + }); +}); diff --git a/packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts b/packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts new file mode 100644 index 00000000000..41bcd6563e4 --- /dev/null +++ b/packages/qa/dogfood/test/permission-set-clone-boot-unowned-warning.dogfood.test.ts @@ -0,0 +1,166 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#21669] A permission set the environment CLONED is not reported, on the next +// boot, as a package declaration with no owner — over the real showcase +// composition, across a cold boot on one database file. +// +// ## What was broken +// +// The declared-permission boot loop (`bootstrapDeclaredPermissions`, +// plugin-security) walks every `permission` item in the engine registry. That +// registry holds more than package declarations: `loadMetaFromDb` hydrates every +// env-wide `sys_metadata` `permission` row into the same collection, and a set +// made with Setup's Clone action is one — with no package id, because it has +// none. The loop judged "unowned" before it looked at the row, so every boot +// (and every `metadata:reloaded`) logged +// `[permission_set_declaration_unowned] … the Setup admin surface … cannot see +// this set` for a set whose `managed_by:'admin'` row Setup lists and edits. +// Measured on `origin/main` before the fix through this file's own steps: one +// such line naming the clone, `skippedUnowned: 1`. +// +// ## Why a booted stack, booted twice +// +// The unit pins (`bootstrap-declared-permissions.test.ts`, `[#21669]`) hand the +// loop the item shape this boot produces. What they cannot show is that the +// real boot puts the clone into the walk at all — that is a property of the +// hydration, not of the loop, and only a restart on the same file shows it. So +// this file asserts the walk holds the clone with no package id BEFORE it +// asserts the warning is absent; without that precondition the absence would be +// vacuous. +// +// ## The clone is made the way the Setup dialog makes it +// +// The payload is built from the shipped `clone_permission_set` action's own +// declaration — its `target`, its `bodyExtra` and its `params` (the typed +// `label`/`name`, and every `defaultFromRow` facet copied from the base row) — +// and POSTed to the data door as the signed-in admin. Read, never restated: a +// facet added to the action reaches this pin without an edit here. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { securityObjects } from '@objectstack/plugin-security'; +import { fileURLToPath } from 'node:url'; +import { mkdtempSync, rmSync } from 'node:fs'; +import { tmpdir } from 'node:os'; +import { join } from 'node:path'; + +/** Package-relative refs resolve against the cwd — see the sibling cold-boot files. */ +const SHOWCASE_DIR = fileURLToPath(new URL('../../../../examples/app-showcase/', import.meta.url)); +const SYS = { context: { isSystem: true } } as const; +const TOKEN = 'permission_set_declaration_unowned'; +/** A set the showcase package ships (`com.example.showcase`) — the clone's base. */ +const BASE = 'showcase_contributor'; +/** A name no package ships. */ +const CLONE = 'showcase_contributor_local_21669'; +const API_BASE = '/api/v1'; + +interface CloneParam { name?: string; field?: string; defaultFromRow?: boolean } +interface CloneAction { method: string; target: string; bodyExtra?: Record; params: CloneParam[] } + +/** The shipped Clone action, read off the object the security plugin registers. */ +function cloneAction(): CloneAction { + const object = (securityObjects as any[]).find((o) => o?.name === 'sys_permission_set'); + const action = (object?.actions ?? []).find((a: any) => a?.name === 'clone_permission_set'); + if (!action) throw new Error('clone_permission_set is missing from sys_permission_set.actions'); + return action as CloneAction; +} + +/** Every line the process writes while `run` is in flight — the kernel logger writes to the streams. */ +async function captureOutput(run: () => Promise): Promise<{ value: T; lines: string[] }> { + const lines: string[] = []; + const stdout = process.stdout.write.bind(process.stdout); + const stderr = process.stderr.write.bind(process.stderr); + const warn = console.warn; + (process.stdout as any).write = (chunk: unknown, ...rest: any[]) => { lines.push(String(chunk)); return stdout(chunk as any, ...rest); }; + (process.stderr as any).write = (chunk: unknown, ...rest: any[]) => { lines.push(String(chunk)); return stderr(chunk as any, ...rest); }; + console.warn = (...args: unknown[]) => { lines.push(args.map(String).join(' ')); warn(...args); }; + try { + return { value: await run(), lines }; + } finally { + (process.stdout as any).write = stdout; + (process.stderr as any).write = stderr; + console.warn = warn; + } +} + +describe('[#21669] a cloned permission set boots without the unowned-declaration warning (showcase, cold boot)', () => { + let prevCwd: string; + let dir: string; + let dbFile: string; + let stack: VerifyStack | undefined; + /** Everything boot 2 wrote. */ + let bootLines: string[] = []; + /** Boot 2's engine. */ + let ql: any; + + beforeAll(async () => { + prevCwd = process.cwd(); + process.chdir(SHOWCASE_DIR); + dir = mkdtempSync(join(tmpdir(), 'dogfood-21669-')); + dbFile = join(dir, 'showcase.db'); + + // ── boot 1: clone a packaged set through the Setup dialog's own path ── + stack = await bootStack(showcaseStack, { databaseFile: dbFile }); + const token = await stack.signIn(); + const engine: any = await stack.kernel.getServiceAsync('objectql'); + const [base] = await engine.find('sys_permission_set', { where: { name: BASE }, limit: 1 }, SYS); + expect(base?.managed_by, 'the base is a package-declared set').toBe('package'); + + const read = await stack.apiAs(token, 'GET', `/data/sys_permission_set/${base.id}`); + const served: any = await read.json(); + const row = served?.record ?? served?.data?.record ?? served?.data; + const action = cloneAction(); + const body: Record = { ...(action.bodyExtra ?? {}) }; + for (const p of action.params) { + if (p.name === 'label') body.label = 'Contributor (local)'; + else if (p.name === 'name') body.name = CLONE; + else if (p.field && p.defaultFromRow) body[p.field] = row?.[p.field]; + } + expect(action.target.startsWith(API_BASE), action.target).toBe(true); + const created = await stack.apiAs(token, action.method, action.target.slice(API_BASE.length), body); + expect(created.status, JSON.stringify(await created.clone().json().catch(() => ({})))).toBe(201); + + // Harness health: the clone is the environment's own set — the row + // Setup lists, and the env-wide metadata row the next boot hydrates. + const [clone] = await engine.find('sys_permission_set', { where: { name: CLONE }, limit: 1 }, SYS); + expect({ managed_by: clone?.managed_by, package_id: clone?.package_id ?? null }) + .toEqual({ managed_by: 'admin', package_id: null }); + const stored = await engine.find('sys_metadata', { where: { type: 'permission', name: CLONE } }, SYS); + expect(stored.map((r: any) => ({ state: r.state, organization_id: r.organization_id ?? null }))) + .toEqual([{ state: 'active', organization_id: null }]); + await stack.stop(); + stack = undefined; + + // ── boot 2: same file, nothing authored ─────────────────────────────── + const second = await captureOutput(() => bootStack(showcaseStack, { databaseFile: dbFile })); + stack = second.value; + bootLines = second.lines; + ql = await stack.kernel.getServiceAsync('objectql'); + }, 300_000); + + afterAll(async () => { + await stack?.stop(); + if (prevCwd) process.chdir(prevCwd); + if (dir) rmSync(dir, { recursive: true, force: true }); + }); + + it('precondition: the boot loop walks the clone, with no package id', () => { + // ⛔ Without this the absence below proves nothing: a boot that stopped + // hydrating the clone would also log no warning for it. + const item = (ql.registry.listItems('permission') ?? []).find((i: any) => i?.name === CLONE); + expect(item, `${CLONE} is in the registry the loop walks`).toBeDefined(); + expect(item._packageId ?? item.packageId ?? null).toBeNull(); + }); + + it('control: the capture saw the loop report its pass', () => { + // The seeding pass logs one summary line per run, through the same + // logger the warning used — so an empty capture cannot pass the next case. + expect(bootLines.some((l) => l.includes('declared permission sets seeded into sys_permission_set'))).toBe(true); + }); + + it('the cloned set is not reported as an unowned declaration', () => { + const unowned = bootLines.filter((l) => l.includes(TOKEN) && l.includes(CLONE)); + expect(unowned).toEqual([]); + }); +});