diff --git a/.changeset/30319-copy-doors-inherited-grants.md b/.changeset/30319-copy-doors-inherited-grants.md new file mode 100644 index 00000000000..a38cdfcc2af --- /dev/null +++ b/.changeset/30319-copy-doors-inherited-grants.md @@ -0,0 +1,12 @@ +--- +'@objectstack/metadata-protocol': patch +--- + +fix(metadata-protocol): duplicating a package copies a permission set that grants on an optional plugin's object not installed here, as the permission-set clone door does + +Clause-②: no +Metadata-protocol: none + +- **Now.** `duplicatePackage` saves a copied permission set with the grant keys of its SOURCE row as stored passed as `inheritedGrantObjects`, so the save door's unknown-object refusal does not judge them (#30255's rule for the clone door). A platform set granting on `sys_comment` / `sys_activity` on a deployment without `@objectstack/plugin-audit` is copied; before, it was left in `failed[]` with the 422 finding and the duplicate answered `success: false`. +- **Still refused.** A grant the copy's rewrite renames onto the copy's own object (`SOURCE_NS_x` → `TARGET_NS_x`) is not the source's key and is judged as before: when that object did not copy, the set lands in `failed[]` naming that grant alone. The keys come from the stored row, never from the request. +- **Unchanged, by ruling.** `importUnmanagedPackage` still refuses such a set `422 INVALID_METADATA` and rolls the import back: the package is the request's body, not a stored row, so its permission sets are new writes and every grant is judged. diff --git a/packages/metadata-protocol/src/protocol-package-commits.ts b/packages/metadata-protocol/src/protocol-package-commits.ts index 35ffb243b5c..011ea1702c6 100644 --- a/packages/metadata-protocol/src/protocol-package-commits.ts +++ b/packages/metadata-protocol/src/protocol-package-commits.ts @@ -69,6 +69,18 @@ import { isNonCanonicalStoredType } from './protocol-stored-overlay-types.js'; import type { RollbackMetaItemRequest } from './protocol-door-requests.js'; import type { ProtocolHost } from './protocol-host.js'; +/** + * [#30319] The grant keys of a stored permission set's `objects` map: what a + * copy of that row holds because the source does (the save request's + * `inheritedGrantObjects`). The same derivation as the permission-set clone + * door's `inheritedGrantObjectsOf` (`@objectstack/runtime`, which depends on + * this package, so it is restated, not imported). + */ +function storedGrantObjectKeys(stored: unknown): string[] { + const objects = stored && typeof stored === 'object' ? (stored as { objects?: unknown }).objects : undefined; + return objects && typeof objects === 'object' && !Array.isArray(objects) ? Object.keys(objects) : []; +} + export class ProtocolPackageCommitsFacet { /** The body of `ObjectStackProtocolImplementation#revertStoredPackage`; the method's docblock there is its contract. */ static async revertStoredPackage(host: Pick, request: { @@ -779,6 +791,15 @@ export class ProtocolPackageCommitsFacet { // duplicate-AGAIN workflow, and its `?force=true` default // would name a parameter this door does not accept. writeFace: 'package-duplicate', + // [#30319] A copied permission set's grants are the SOURCE + // row's, so the unknown-object refusal does not judge the + // keys that row holds as stored — the clone door's rule + // (#30255) for this door's copy. A platform set may grant on + // an optional plugin's object this deployment does not + // install (`sys_comment`, `@objectstack/plugin-audit`). + // Derived from the stored row, never from the request; a key + // the rewrite above renamed is not the source's, and is judged. + ...(singular === 'permission' ? { inheritedGrantObjects: storedGrantObjectKeys(body) } : {}), ...(request.actor ? { actor: request.actor } : {}), }); copied.push({ type: row.type, name: newName }); @@ -939,6 +960,9 @@ export class ProtocolPackageCommitsFacet { packageId: targetPackageId, // Server-stated, as on every face: no body reaches this field. writeFace: 'package-unmanaged-import', + // [#30319] No `inheritedGrantObjects`, deliberately: the + // package is the request's body, not a stored row, so its + // permission sets are new writes and every grant is judged. ...(actor ? { actor } : {}), }); copied.push({ type: item.type, name: item.name }); diff --git a/packages/metadata-protocol/src/protocol.copy-doors-inherited-grants.test.ts b/packages/metadata-protocol/src/protocol.copy-doors-inherited-grants.test.ts new file mode 100644 index 00000000000..281968ebc86 --- /dev/null +++ b/packages/metadata-protocol/src/protocol.copy-doors-inherited-grants.test.ts @@ -0,0 +1,257 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#30319] The two doors that copy a permission set into a new package, on a + * deployment without an optional plugin whose object a platform set grants on + * (`sys_comment`, which `@objectstack/plugin-audit` registers). + * + * Measured on `origin/main` (870ebce3) before this file, on the harness below: + * - `duplicatePackage` answered `success: false`, the object copied and the + * set in `failed[]` with the unknown-object refusal at + * `objects.sys_comment` — the same 422 the permission-set clone door had + * (#30255, fixed by PR #30310 through `inheritedGrantObjects`); + * - `importUnmanagedPackage` answered `422 INVALID_METADATA` at + * `objects.sys_comment` and rolled the whole import back. + * + * Pinned here: + * - the DUPLICATE door copies a stored set's grants the source row holds: + * the copy is saved, granting on `sys_comment`. Keys come from the stored + * row, never the request. A key the copy's rewrite renames (the source's + * own namespace) is not the source's, so a renamed grant on an object that + * does not exist is still refused — and only that one; + * - the UNMANAGED IMPORT keeps refusing, by ruling (seat 2 on #30319, option + * A): the package is the request's body, not a stored row, so its sets are + * new writes and every grant is judged (triage 6651966154: the inherited + * keys are derived from the source set as stored, never from request input). + * + * Harness: a stateful `sys_metadata` stub and a registry that keeps packages + * and namespaces, after `package-unmanaged-install.test.ts`. The registry + * registers no object, so an object is known only when it is stored here. + */ +// Pays the lint root's first load at collection, never inside a clocked case. +import './lint-root-preload.testkit.js'; +import { describe, expect, it } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineFindOnePredicate, assertEngineUpdateDispatch } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +type Row = Record; + +function matches(row: Row, where: Record = {}): boolean { + for (const [key, want] of Object.entries(where)) { + if (want === undefined) continue; + if (key === '$or') { + if (!(want as Array>).some((c) => matches(row, c))) return false; + continue; + } + if (key.startsWith('$')) throw new Error(`stub engine: unsupported operator ${key}`); + const have = row[key] ?? null; + if (want !== null && typeof want === 'object' && !Array.isArray(want)) { + const ops = Object.keys(want); + if (ops.length !== 1 || ops[0] !== '$in') throw new Error(`stub engine: unsupported operator ${ops.join(',')}`); + if (!(want as { $in: unknown[] }).$in.includes(have)) return false; + continue; + } + if (have !== want) return false; + } + return true; +} + +function makeProtocol(seed: Row[] = [], seedPackages: any[] = []) { + const tables = new Map([['sys_metadata', seed.map((r) => ({ ...r }))]]); + const table = (name: string) => { + if (!tables.has(name)) tables.set(name, []); + return tables.get(name)!; + }; + let nextId = 0; + const packages = new Map(seedPackages.map((m) => [m.id, { manifest: m, status: 'installed', enabled: true }])); + const namespaces = new Map>(); + for (const m of seedPackages) if (m.namespace) namespaces.set(m.namespace, new Set([m.id])); + const engine: any = { + async syncObjectSchema() { return true; }, + async dropObjectSchema() { return true; }, + async find(t: string, q?: { where?: Record; limit?: number }) { + const rows = table(t).filter((r) => matches(r, q?.where)); + return typeof q?.limit === 'number' ? rows.slice(0, q.limit) : rows; + }, + async findOne(t: string, q: { where: Record }) { + assertEngineFindOnePredicate(t, q); + return table(t).find((r) => matches(r, q.where)) ?? null; + }, + async insert(t: string, data: Row) { + nextId += 1; + const row = { id: `r_${nextId}`, ...data }; + table(t).push(row); + return { ...row }; + }, + async update(t: string, data: Row, q?: Record) { + assertEngineUpdateDispatch(data, q); + const where = (q as { where?: Record } | undefined)?.where; + const hit = table(t).filter((r) => matches(r, where)); + for (const r of hit) Object.assign(r, data); + return { id: hit[0]?.id ?? null }; + }, + async delete(t: string, q?: Record) { + assertEngineDeleteDispatch(q); + const where = (q as { where?: Record } | undefined)?.where; + const keep = table(t).filter((r) => !matches(r, where)); + const deleted = table(t).length - keep.length; + tables.set(t, keep); + return { deleted }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + registry: { + registerItem: () => {}, + registerObject: () => {}, + unregisterItem: () => {}, + listItems: () => [], + getItem: () => undefined, + getAllObjects: () => [], + // No plugin-audit: `sys_comment` is registered by nothing here. + getObject: () => undefined, + getArtifactItem: () => undefined, + isPackageDisabled: () => false, + getPackage: (id: string) => packages.get(id), + installPackage: (manifest: any) => { + const pkg = { manifest, status: 'installed', enabled: true }; + packages.set(manifest.id, pkg); + if (manifest.namespace) { + if (!namespaces.has(manifest.namespace)) namespaces.set(manifest.namespace, new Set()); + namespaces.get(manifest.namespace)!.add(manifest.id); + } + return pkg; + }, + uninstallPackage: (id: string) => { + const pkg = packages.get(id); + packages.delete(id); + if (pkg?.manifest?.namespace) namespaces.get(pkg.manifest.namespace)?.delete(id); + }, + unregisterNamespace: () => {}, + getNamespaceOwners: (ns: string) => [...(namespaces.get(ns) ?? [])], + }, + }; + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + return { protocol, table, packages }; +} + +const SOURCE = 'com.example.leave'; +const TARGET = 'com.example.leave-copy'; +const SOURCE_MANIFEST = { id: SOURCE, name: 'Leave', type: 'app', namespace: 'leave', version: '1.0.0' }; + +const ticket = { + name: 'leave_ticket', label: 'Ticket', sharingModel: 'private', fields: { title: { type: 'text', label: 'Title' } }, +}; +/** A schema-valid permission-set body granting read on each named object. */ +const set = (name: string, objectNames: string[]) => ({ + name, + label: 'Leave staff', + objects: Object.fromEntries(objectNames.map((o) => [o, { allowRead: true }])), +}); +/** A row of the source package as it sits at rest. */ +const sourceRow = (type: string, body: { name: string }) => ({ + type, name: body.name, organization_id: null, package_id: SOURCE, state: 'active', metadata: JSON.stringify(body), +}); + +/** The finding's first sentence, spelled out (`unknownGrantObjectSaveRefusal`). */ +const unknownFinding = (setName: string, objectName: string) => + `Permission '${setName}' grants on object '${objectName}' which is not defined in objects: no package registers ` + + 'an object of that name and none is saved here, so the grant applies to nothing.'; + +/** The stored grant keys of one permission set of the target package. */ +const copiedGrants = (rows: Row[], name: string) => + rows.filter((r) => r.type === 'permission' && r.name === name && r.package_id === TARGET) + .map((r) => Object.keys(JSON.parse(r.metadata).objects ?? {})); + +describe('[#30319] duplicatePackage — a copied permission set keeps the grants its source row holds', () => { + it('a set granting on an optional plugin\'s object not installed here is copied, the grant with it', async () => { + const { protocol, table } = makeProtocol( + [sourceRow('object', ticket), sourceRow('permission', set('leave_staff', ['leave_ticket', 'sys_comment']))], + [SOURCE_MANIFEST], + ); + + const res = await protocol.duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET }); + + expect(res.failed).toEqual([]); + expect(res.success).toBe(true); + expect(res.copied).toEqual([ + { type: 'object', name: 'leave_copy_ticket' }, + { type: 'permission', name: 'leave_copy_staff' }, + ]); + // The source's own object is rewritten to the copy's; the platform grant is kept. + expect(copiedGrants(table('sys_metadata'), 'leave_copy_staff')).toEqual([['leave_copy_ticket', 'sys_comment']]); + }); + + it('a grant the copy renames onto an object that does not exist is still refused — that grant alone', async () => { + // The copy's rewrite renames a grant on the source's own object to the + // copy's (`leave_ticket` → `leave_copy_ticket`): a key the source never + // held. Here that object's row does not copy (unparseable), so the + // renamed grant names nothing and is judged; `sys_comment` is not. + const { protocol, table } = makeProtocol( + [ + { ...sourceRow('object', ticket), metadata: '{not json' }, + sourceRow('permission', set('leave_staff', ['leave_ticket', 'sys_comment'])), + ], + [SOURCE_MANIFEST], + ); + + const res = await protocol.duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET }); + + expect(res.success).toBe(false); + expect(res.copied).toEqual([]); + expect(res.failed).toEqual([ + { type: 'object', name: 'leave_ticket', error: 'unparseable metadata' }, + { + type: 'permission', + name: 'leave_staff', + error: 'permission/leave_copy_staff failed spec validation: objects.leave_copy_ticket: ' + + `${unknownFinding('leave_copy_staff', 'leave_copy_ticket')} ` + + "Correct the name to the object's full name (e.g. 'crm_lead', not 'lead'), create the object first, or remove the grant.", + }, + ]); + expect(copiedGrants(table('sys_metadata'), 'leave_copy_staff')).toEqual([]); + }); + + it('a grant the source holds on an object outside its namespace that does not exist is the source\'s, and is copied', async () => { + // Not renamed (no object row of the source carries it), so it is the + // source's grant as stored — the clone door's rule for a copy. + const { protocol, table } = makeProtocol( + [sourceRow('object', ticket), sourceRow('permission', set('leave_staff', ['leave_ticket', 'other_gone']))], + [SOURCE_MANIFEST], + ); + + const res = await protocol.duplicatePackage({ sourcePackageId: SOURCE, targetPackageId: TARGET }); + + expect(res.failed).toEqual([]); + expect(copiedGrants(table('sys_metadata'), 'leave_copy_staff')).toEqual([['leave_copy_ticket', 'other_gone']]); + }); +}); + +describe('[#30319] importUnmanagedPackage — the package is the request\'s body, so every grant is judged', () => { + it('a set granting on an object not installed here is refused 422 at `objects.`, and the import is rolled back', async () => { + // By ruling (seat 2 on #30319, option A): no `inheritedGrantObjects` + // here. Deriving them from the body would be reading request input, + // which turns the unknown-object refusal off for this door. + const { protocol, table, packages } = makeProtocol(); + + let err: any; + try { + await protocol.importUnmanagedPackage({ + manifest: { + id: 'com.example.crm', namespace: 'crm', version: '4.0.0', type: 'app', name: 'CRM', + objects: [{ name: 'crm_lead', label: 'Lead', sharingModel: 'private', fields: { title: { type: 'text', label: 'Title' } } }], + permissions: [set('crm_rep', ['crm_lead', 'sys_comment'])], + }, + }); + } catch (e) { + err = e; + } + + expect({ code: err?.code, status: err?.status }, String(err?.message)).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(err.issues.map((i: any) => i.path)).toEqual(['objects.sys_comment']); + expect(String(err.issues[0].message).startsWith(unknownFinding('crm_rep', 'sys_comment')), err.issues[0].message).toBe(true); + // Rolled back whole: no row, no base. + expect(table('sys_metadata')).toEqual([]); + expect(packages.has('com.example.crm-copy')).toBe(false); + }); +}); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 0c0fd772117..171fb7a76e7 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -611,6 +611,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.copy-doors-inherited-grants.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.copy-doors-inherited-grants.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.copy-doors-inherited-grants.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.credential-channel.test.ts", "verb": "delete",