diff --git a/.changeset/28990-permission-grant-unknown-object.md b/.changeset/28990-permission-grant-unknown-object.md new file mode 100644 index 00000000000..bde58826e71 --- /dev/null +++ b/.changeset/28990-permission-grant-unknown-object.md @@ -0,0 +1,18 @@ +--- +'@objectstack/metadata-protocol': minor +--- + +fix(metadata-protocol)!: a runtime permission-set save refuses a grant on an object that does not exist + +Clause-②: no (narrowing) +Metadata-protocol: narrowed + +**BREAKING** for one shape only: a `permission` set whose `objects` map grants on an object this runtime does not know. It was stored and published (200), and the grant applied to nothing, because the permission evaluator keys a grant by the object's name: a short `lead` where the object is `crm_lead` silently granted nothing. `defineStack` already refuses the same grant at build; the metadata save doors now refuse it too. + +- **Where.** `PUT /api/v1/meta/permission/NAME`, live and `?mode=draft`; the publish of a draft (`POST /meta/permission/NAME/publish`, `POST /packages/ID/publish-drafts`); and the restore doors (`POST /meta/permission/NAME/rollback`, a commit revert). +- **The refusal.** `422 INVALID_METADATA`, the save door's existing envelope, with one finding per unknown object at `objects.NAME`: "Permission 'SET' grants on object 'NAME' which is not defined in objects: …". Nothing is stored; a package publish aborts the batch. +- **What counts as known.** The `'*'` wildcard; an object the runtime registers, from any package, platform objects (`sys_*`, the RBAC link tables) included; an object stored in this environment or defined in code; on a draft save, any object draft; on a package publish, an object drafted in the same batch. +- **What is not judged.** A grant the set already holds: a re-save, `os migrate meta --stored` or a restore that keeps it is not refused, even if the object has gone since. Editing a shipped set is therefore never refused for a grant it ships with. +- **What to do.** Correct the object name to the object's full name, create the object first (or draft it in the same package), or remove the grant. + + diff --git a/packages/metadata-protocol/src/protocol-authoring-gate.ts b/packages/metadata-protocol/src/protocol-authoring-gate.ts index 73b029ba085..8c873d3e7bc 100644 --- a/packages/metadata-protocol/src/protocol-authoring-gate.ts +++ b/packages/metadata-protocol/src/protocol-authoring-gate.ts @@ -90,6 +90,7 @@ export interface AuthoringGateHost { runAuthoringGate: ObjectStackProtocolImplementation['runAuthoringGate']; tenancyPostureInForce: ObjectStackProtocolImplementation['tenancyPostureInForce']; unknownDatasourceSaveRefusal: ObjectStackProtocolImplementation['unknownDatasourceSaveRefusal']; + unknownGrantObjectSaveRefusal: ObjectStackProtocolImplementation['unknownGrantObjectSaveRefusal']; seedExternalIdSaveRefusal: ObjectStackProtocolImplementation['seedExternalIdSaveRefusal']; } @@ -124,7 +125,7 @@ export function restoredBodyWriter( /** The body of `ObjectStackProtocolImplementation#refuseRestoredBodyTheSaveDoorRefuses`; the method's docblock there is its contract. */ export async function refuseRestoredBodyTheSaveDoorRefuses( - this: Pick, + this: Pick, type: string, name: string, body: unknown, @@ -141,6 +142,8 @@ export async function refuseRestoredBodyTheSaveDoorRefuses( if (bindingRefusal) throw bindingRefusal; const datasourceRefusal = await this.unknownDatasourceSaveRefusal(type, body, name); if (datasourceRefusal) throw datasourceRefusal; + const grantRefusal = await this.unknownGrantObjectSaveRefusal(type, body, name); + if (grantRefusal) throw grantRefusal; const seedKeyRefusal = await this.seedExternalIdSaveRefusal(type, body, name); if (seedKeyRefusal) throw seedKeyRefusal; const repo = this.getOverlayRepo(); diff --git a/packages/metadata-protocol/src/protocol-save-door-refusals.ts b/packages/metadata-protocol/src/protocol-save-door-refusals.ts index 9104f0f7818..d3afe993a50 100644 --- a/packages/metadata-protocol/src/protocol-save-door-refusals.ts +++ b/packages/metadata-protocol/src/protocol-save-door-refusals.ts @@ -527,6 +527,82 @@ export async function unknownDatasourceSaveRefusal( return err; } +/** + * The body of `ObjectStackProtocolImplementation#unknownGrantObjectSaveRefusal`; + * the method's docblock there is its contract. [#28990] Written here beside + * {@link unknownDatasourceSaveRefusal}, whose shape it follows: the engine's + * registered objects, then the by-name read (`readMetaItem`), then the pending + * drafts, then the definition the set already holds. + */ +export async function unknownGrantObjectSaveRefusal( + this: Pick, + singularType: string, + item: unknown, + name: string, + pending?: 'drafts' | ReadonlySet, + writeFace?: MetadataWriteFace, +): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> { + if (singularType !== 'permission') return undefined; + if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined; + const grants = (item as { objects?: unknown }).objects; + if (!grants || typeof grants !== 'object' || Array.isArray(grants)) return undefined; + // A registry that cannot look an object up by name cannot say a name is + // unknown, so nothing is refused on it. + const registry = this.engine?.registry as { getObject?: unknown } | undefined; + const getObject = registry?.getObject; + if (typeof getObject !== 'function') return undefined; + // `'*'` is the wildcard grant the permission evaluator falls back to for + // every object with no entry of its own: it names no object. + let unknown = Object.keys(grants).filter((key) => { + if (key === '*') return false; + // The object's own name, exactly: the registry's legacy `pkg__name` + // lookup answers an object the evaluator never keys a grant by. + const registered = getObject.call(registry, key) as { name?: unknown } | undefined; + return registered?.name !== key; + }); + if (unknown.length === 0) return undefined; + // An object the environment stores, or the code layer defines, that the + // registry has not (yet) registered: known here, and judged where it is used. + const declared = await Promise.all(unknown.map(async (key) => + Boolean((await this.readMetaItem({ type: 'object', name: key }))?.item))); + unknown = unknown.filter((_, index) => !declared[index]); + if (unknown.length === 0) return undefined; + // An object still pending: any object draft, for a draft save; the batch's + // own object drafts, for a package publish. + if (pending instanceof Set) { + unknown = unknown.filter((key) => !pending.has(key)); + } else if (pending === 'drafts') { + const drafted = new Set((await this.listDrafts({ type: 'object' })).drafts.map((d) => d.name)); + unknown = unknown.filter((key) => !drafted.has(key)); + } + if (unknown.length === 0) return undefined; + // A grant the set already holds is not this save's: a re-save, a + // stored-row migration or a restore that keeps it is judged at the save + // that introduced it, and an object that has gone since is not the + // re-saving author's to answer for. (A declaration boot never runs + // `onEnable`, so what a plugin registers there is absent from it.) + const current = (await this.readMetaItem({ type: 'permission', name }))?.item as { objects?: unknown } | undefined; + const held = current?.objects && typeof current.objects === 'object' && !Array.isArray(current.objects) + ? new Set(Object.keys(current.objects)) + : new Set(); + unknown = unknown.filter((key) => !held.has(key)); + if (unknown.length === 0) return undefined; + const issues: MetadataIssueEntry[] = unknown.map((key) => ({ + path: `objects.${key}`, + message: `Permission '${name}' grants on object '${key}' 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. Correct the name to the ' + + `object's full name (e.g. 'crm_lead', not 'lead'), create the object first, or remove the grant.`, + code: 'custom', + })); + const err = new Error( + `${singularType}/${name} failed spec validation: ${specValidationFindings(writeFace, issues)}`, + ) as Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }; + err.code = 'INVALID_METADATA'; + err.status = 422; + err.issues = issues; + return err; +} + /** The body of `ObjectStackProtocolImplementation#objectsBoundExternallyTo`; the method's docblock there is its contract. */ export async function objectsBoundExternallyTo( this: Pick, diff --git a/packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts b/packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts index ca05dca987e..792fe2c25f3 100644 --- a/packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts +++ b/packages/metadata-protocol/src/protocol.restore-doors-save-refusal-family.test.ts @@ -8,7 +8,8 @@ * family of shapes after its schema gate: a hook with no `body`, a second * default record page of an object, an `external` object binding on a managed * datasource (both halves of the pair), an object `datasource` that names no - * datasource (#26724), a keyed seed whose `externalId` names no field of its + * datasource (#26724), a permission-set grant on an object that does not + * exist (#28990), a keyed seed whose `externalId` names no field of its * object (#28952), a `runAs: 'system'` flow an anonymous * endpoint names, the shared runtime authoring rules, and the authoring gate a * domain plugin registers. Measured on `origin/main` `5801a4f2c` before this @@ -506,6 +507,28 @@ pinBothDoors({ }, }); +pinBothDoors({ + title: 'a permission set restored with a grant on an object that does not exist now (unknownGrantObjectSaveRefusal)', + type: 'permission', + name: 'qa_widget_editor', + code: 'INVALID_METADATA', + status: 422, + async stage(h) { + h.objects.set('qa_widget', { name: 'qa_widget' }); + const set = (objects: Record, label = 'Widget editor') => ({ name: 'qa_widget_editor', label, objects }); + await h.protocol.saveMetaItem({ type: 'permission', name: 'qa_widget_editor', item: set({ qa_widget: { allowRead: true } }) }); + await h.protocol.saveMetaItem({ type: 'permission', name: 'qa_widget_editor', item: set({}) }); + await h.protocol.saveMetaItem({ type: 'permission', name: 'qa_widget_editor', item: set({}, 'Widget editor v3') }); + // The object v1 granted on is gone, and the set holds no grant on it now. + h.objects.delete('qa_widget'); + return { refused: 1, accepted: 2 }; + }, + expectDetail: (error) => { + expect(error.issues).toEqual([expect.objectContaining({ path: 'objects.qa_widget', code: 'custom' })]); + expect(error.issues[0].message.startsWith("Permission 'qa_widget_editor' grants on object 'qa_widget' which is not defined in objects")).toBe(true); + }, +}); + pinBothDoors({ title: 'a seed restored keyed on a field its object no longer has (seedExternalIdSaveRefusal)', type: 'seed', diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 8c383a7cc58..0bfc00c3773 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/packages/metadata-protocol/src/protocol.ts @@ -350,6 +350,7 @@ import { saveWriteBinding, seedExternalIdSaveRefusal, unknownDatasourceSaveRefusal, + unknownGrantObjectSaveRefusal, } from './protocol-save-door-refusals.js'; // [#25467 cut 7] Package install and update. import { @@ -7482,6 +7483,9 @@ export class ObjectStackProtocolImplementation implements * - [#26724] an object restored with a `datasource` that names no * datasource here, when that is not the binding it holds now * ({@link unknownDatasourceSaveRefusal}); + * - [#28990] a permission set restored with a grant on an object that + * does not exist here, when the set does not hold that grant now + * ({@link unknownGrantObjectSaveRefusal}); * - [#27818] a datasource restored with an `external.credentialsRef` the * active row it is restored into does not hold now * ({@link datasourceCredentialsRefSaveRefusal}, in its restore words); @@ -7542,6 +7546,7 @@ export class ObjectStackProtocolImplementation implements permissionSetNameCollisionRefusal: (...a) => this.permissionSetNameCollisionRefusal(...a), runAuthoringGate: (...a) => this.runAuthoringGate(...a), unknownDatasourceSaveRefusal: (...a) => this.unknownDatasourceSaveRefusal(...a), + unknownGrantObjectSaveRefusal: (...a) => this.unknownGrantObjectSaveRefusal(...a), seedExternalIdSaveRefusal: (...a) => this.seedExternalIdSaveRefusal(...a), }; return refuseRestoredBodyTheSaveDoorRefuses.call(host, type, name, body, binding, source); @@ -20887,6 +20892,57 @@ export class ObjectStackProtocolImplementation implements return unknownDatasourceSaveRefusal.call(host, singularType, item, name, pending, writeFace); } + /** + * [#28990] The save doors refuse a `permission` set whose `objects` map + * grants on an object that does not exist here. Before this, such a set + * was stored and published (200), and the grant applied to nothing: the + * permission evaluator keys a grant by the object's name, so a short `lead` + * where the object is `crm_lead` silently grants nothing — the case + * `defineStack`'s build rule refuses for a stack's own sets ("Fail loudly + * at build time"); this is the same judgement at the runtime door. + * + * A grant key is known when any of these holds, and the refusal fires on + * none: + * - it is `'*'`, the wildcard the evaluator falls back to; + * - the engine's registry registers an object of exactly that name — any + * package's, the platform's (`sys_*`, the RBAC link tables) included, + * since they are registered, so the build rule's platform-prefix + * allowance needs no restating here. A registry that cannot look an + * object up refuses nothing; + * - an object of that name resolves through this door's by-name read + * ({@link readMetaItem}: the environment's stored row, else the code + * layer's definition); + * - it is still pending: on a draft save, any object draft of that name; + * on a package publish, an object draft of the same batch (`pending` as + * a set), since the batch promotes as one unit in no type order; + * - the set's current definition already grants on it: the grant is not + * this write's. A re-save, a stored-row migration (`os migrate meta + * --stored`) or a restore that keeps it is not judged, and neither is + * an object that has gone since the grant was made. + * + * Asked in draft and in publish mode by {@link saveMetaItem} after the + * schema gate, again by {@link promoteDraftForPublish} on the draft body + * (the object can go between a draft's save and its publish), and by the + * restore doors ({@link refuseRestoredBodyTheSaveDoorRefuses}). Answered + * as the schema gate answers: `422 INVALID_METADATA`, one finding per + * unknown object at `objects.`, naming it. + */ + private unknownGrantObjectSaveRefusal( + singularType: string, + item: unknown, + name: string, + pending?: 'drafts' | ReadonlySet, + writeFace?: MetadataWriteFace, + ): Promise<(Error & { code: 'INVALID_METADATA'; status: 422; issues: MetadataIssueEntry[] }) | undefined> { + const readEngine = () => this.engine; + const host: ThisParameterType = { + get engine() { return readEngine(); }, + listDrafts: (...a) => this.listDrafts(...a), + readMetaItem: (...a) => this.readMetaItem(...a), + }; + return unknownGrantObjectSaveRefusal.call(host, singularType, item, name, pending, writeFace); + } + /** * [#28952] The refusal of a `seed` whose `externalId` names no field of * the seed's object: a load in a keyed mode (`upsert`, `update`, @@ -22615,6 +22671,18 @@ export class ObjectStackProtocolImplementation implements if (datasourceRefusal) throw datasourceRefusal; } + // [#28990] A `permission` set granting on an object that does not exist + // here is refused, in draft and in publish mode, after the schema and + // before the authoring gate and every write: stored, the grant applies + // to nothing. A draft may grant on an object that is itself still a + // draft. See {@link unknownGrantObjectSaveRefusal}. + { + const grantRefusal = await this.unknownGrantObjectSaveRefusal( + singularType, request.item, request.name, mode === 'draft' ? 'drafts' : undefined, request.writeFace, + ); + if (grantRefusal) throw grantRefusal; + } + // [#28952] A keyed `seed` whose `externalId` names no field of its // object is refused, in draft and in publish mode, after the schema and // before the authoring gate and every write: every load of it would @@ -24153,6 +24221,14 @@ export class ObjectStackProtocolImplementation implements * on the `publishMetaItem` path, where the datasource must be live. */ batchDatasources?: ReadonlySet; + /** + * [#28990] The names of the object drafts this promotion's batch + * publishes with it, for {@link unknownGrantObjectSaveRefusal}: a + * permission-set draft may grant on one. Stated by + * `publishPackageDrafts`; absent on the `publishMetaItem` path, where + * the object must exist already. + */ + batchObjects?: ReadonlySet; /** * [#22877] The bundle member whose draft to promote; omitted, the * canonical one. The draft read, the gate's read and the promotion @@ -24479,6 +24555,19 @@ export class ObjectStackProtocolImplementation implements ); if (datasourceRefusal) throw datasourceRefusal; } + // [#28990] …and a permission-set draft granting on an object that does + // not exist by the time it is published, or never did: a draft save + // accepts an object that is itself a draft, and that draft may not be + // published with it. Judged on the draft body, before + // `repo.promoteDraft` writes, so a batch publish aborts on it too; an + // object draft of the same batch counts. See + // {@link unknownGrantObjectSaveRefusal}. + if (draftForGate) { + const grantRefusal = await this.unknownGrantObjectSaveRefusal( + singularType, draftForGate.body, request.name, request.batchObjects, + ); + if (grantRefusal) throw grantRefusal; + } // [#28952] …and a seed draft whose `externalId` names no field of its // object by the time it is published: a draft saved before this // refusal, one whose object was still a draft at its save, or one whose @@ -25430,6 +25519,9 @@ export class ObjectStackProtocolImplementation implements const batchDatasources: ReadonlySet = new Set( drafts.filter((d) => canonicalMetaType(d.type) === 'datasource').map((d) => d.name), ); + const batchObjects: ReadonlySet = new Set( + drafts.filter((d) => canonicalMetaType(d.type) === 'object').map((d) => d.name), + ); // ADR-0067 — capture each artifact's PRE-publish state so this turn can // be recorded as ONE revertible commit. existedBefore=false → the commit @@ -25621,6 +25713,9 @@ export class ObjectStackProtocolImplementation implements // [#26724] …and the datasources it drafts, which // an object draft beside them may name. batchDatasources, + // [#28990] …and the objects it drafts, which a + // permission-set draft beside them may grant on. + batchObjects, ...(request.actor ? { actor: request.actor } : {}), // [ADR-0146 D1] The author's change note, when the // publish carries one, is what every history row diff --git a/packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts b/packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts new file mode 100644 index 00000000000..8f0dd295afc --- /dev/null +++ b/packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts @@ -0,0 +1,333 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#28990] The metadata save doors refuse a `permission` set whose `objects` + * map grants on an object that does not exist here. + * + * Measured on `origin/main` before this file: a permission set saved with a + * grant keyed `qa_no_such_object` was stored in draft and in publish mode, and + * `publishPackageDrafts` answered `outcome: 'published'`. The permission + * evaluator keys a grant by the object's name, so the grant applied to + * nothing. `defineStack` refuses the same grant at build + * (`collectPermissionGrantObjectErrors`, #18202). + * + * Pinned here, through `saveMetaItem`, `publishMetaItem` and + * `publishPackageDrafts` (the restore doors are pinned in + * `protocol.restore-doors-save-refusal-family.test.ts`): + * - the refusal: `422 INVALID_METADATA`, one finding per unknown object at + * `objects.` naming it, nothing stored (save) and nothing promoted + * (publish of a draft stored before the rule; the batch aborts); + * - the per-item publish of a set draft whose object is still only a draft + * is refused: the set would go live granting on nothing; + * - controls, each saved, published and promoted: a registered object, a + * platform object (an RBAC link table), the `'*'` wildcard, an object + * stored in this environment, an object drafted in the same package and + * published in the same batch (promoted after the set), and a re-save + * that keeps a grant the set already holds; a registry that cannot look + * an object up refuses nothing. + * + * Each refusal is asserted by its code, its status, its path and the first + * sentence of its finding, spelled out here. + * + * Harness: the stateful `sys_metadata` double of + * `protocol.unknown-datasource-refusal.test.ts`, kept local, with a registry + * that answers `getObject` over a mutable set of registered objects. + */ +import { describe, expect, it } from 'vitest'; +import { assertEngineDeleteDispatch, assertEngineUpdateDispatch, assertEngineFindOnePredicate } from '@objectstack/metadata-core'; +import { ObjectStackProtocolImplementation } from './protocol.js'; + +interface Row { + id: string; + type: string; + name: string; + organization_id: string | null; + package_id: string | null; + state: string; + metadata: string; +} + +const PKG = 'app.grantprobe'; + +/** ADR-0048 overlay key — (type, name, org, state, package_id). */ +const keyOf = (w: Record) => + `${w.type}|${w.name}|${w.organization_id ?? '__env__'}|${w.state ?? 'active'}|${w.package_id ?? '__nopkg__'}`; + +/** Top-level eq + `$or` + `$in` + explicit-NULL, the subset these paths emit. */ +function matchesWhere(r: Row, where: Record): boolean { + for (const [k, v] of Object.entries(where)) { + if (k === '$or') { + if (!(v as Array>).some((c) => matchesWhere(r, c))) return false; + continue; + } + if (v === undefined) continue; + const actual = (r as unknown as Record)[k]; + if (v !== null && typeof v === 'object' && Array.isArray((v as { $in?: unknown }).$in)) { + if (!(v as { $in: unknown[] }).$in.includes(actual)) return false; + continue; + } + if (v === null ? actual !== null && actual !== undefined : actual !== v) return false; + } + return true; +} + +function makeSession(opts: { objects?: string[]; looksUpObjects?: boolean } = {}) { + const rows = new Map(); + const history: Array> = []; + const objects = new Set(opts.objects ?? []); + let nextId = 0; + const findRow = (w: Record): { key: string; row: Row } | null => { + if (w.id !== undefined) { + for (const [k, r] of rows) if (r.id === w.id) return { key: k, row: r }; + return null; + } + for (const [k, r] of rows) if (matchesWhere(r, w)) return { key: k, row: r }; + return null; + }; + const registry: any = { + registerItem: () => {}, + registerObject: () => {}, + listItems: () => [], + getItem: () => undefined, + getAllObjects: () => [], + // An installed authored base that declares no namespace. + getPackage: (id: string) => (id === PKG ? { manifest: { id } } : undefined), + getArtifactItem: () => undefined, + isPackageDisabled: () => false, + }; + if (opts.looksUpObjects !== false) { + registry.getObject = (name: string) => (objects.has(name) ? { name } : undefined); + } + const engine: any = { + async findOne(table: string, o: { where: Record }) { + assertEngineFindOnePredicate(table, o); + if (table === 'sys_metadata_history') { + return history.find((h) => Object.entries(o.where).every(([k, v]) => { + if (k.startsWith('$')) throw new Error(`history double: unsupported operator ${k}`); + return v === undefined || h[k] === v; + })) ?? null; + } + if (table !== 'sys_metadata') return null; + return findRow(o.where)?.row ?? null; + }, + async find(table: string, o?: { where?: Record; limit?: number }) { + if (table !== 'sys_metadata') return []; + // `check:objectql-double-limit` — the caller's bound, applied after the filter. + const matched = Array.from(rows.values()).filter((r) => matchesWhere(r, o?.where ?? {})); + return typeof o?.limit === 'number' ? matched.slice(0, o.limit) : matched; + }, + async insert(table: string, data: Record) { + nextId += 1; + if (table === 'sys_metadata_history') { + history.push({ ...data, id: `h_${nextId}` }); + return { id: `h_${nextId}` }; + } + if (table !== 'sys_metadata') return { id: 'side_effect_skip' }; + const row = { ...(data as unknown as Row), id: `r_${nextId}` }; + rows.set(keyOf(data), row); + return { id: row.id }; + }, + async update(_t: string, data: Record, o: { where: Record }) { + assertEngineUpdateDispatch(data, o); + const found = findRow(o.where); + if (!found) return { id: null }; + const merged = { ...found.row, ...(data as unknown as Row) }; + rows.delete(found.key); + rows.set(keyOf(merged), merged); + return { id: found.row.id }; + }, + async delete(_t: string, o: { where: Record }) { + assertEngineDeleteDispatch(o); + const found = findRow(o.where); + if (!found) return { deleted: 0 }; + rows.delete(found.key); + return { deleted: 1 }; + }, + async transaction(cb: (ctx: any, info: { owned: boolean }) => Promise): Promise { + return cb(undefined, { owned: true }); + }, + async syncObjectSchema() { return true; }, + registry, + }; + const protocol = new ObjectStackProtocolImplementation(engine, () => new Map()) as any; + return { protocol, rows, objects }; +} + +const refusalOf = (run: Promise) => run.then(() => null, (e: any) => e); + +/** The stored rows of one item, as `state|package|grant keys`. */ +const stored = (rows: Map, type: string, name: string) => + [...rows.values()] + .filter((r) => r.type === type && r.name === name) + .map((r) => `${r.state}|${r.package_id ?? '-'}|${Object.keys(JSON.parse(r.metadata).objects ?? {}).join(',')}`) + .sort(); + +/** A schema-valid permission-set body granting read on each named object. */ +const set = (name: string, objectNames: string[], extra: Record = {}) => ({ + name, + label: 'Grant probe', + objects: Object.fromEntries(objectNames.map((o) => [o, { allowRead: true }])), + ...extra, +}); + +/** A schema-valid object body. */ +const object = (name: string) => ({ + name, + label: 'Widget', + sharingModel: 'private', + fields: { name: { type: 'text', label: 'Name' } }, +}); + +/** Plant a row as a row stored before the rule sits at rest. */ +function plant(rows: Map, body: Record, state: 'draft' | 'active', packageId: string | null = PKG) { + const data = { + type: 'permission', name: String(body.name), organization_id: null, package_id: packageId, + state, metadata: JSON.stringify(body), + }; + rows.set(keyOf(data), { ...data, id: `planted_${state}_${String(body.name)}` }); +} + +/** The finding's first sentence, spelled out. */ +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.'; + +const expectRefusal = (err: any, setName: string, objectNames: string[]) => { + expect({ code: err?.code, status: err?.status }, String(err?.message)).toEqual({ code: 'INVALID_METADATA', status: 422 }); + expect(String(err.message).startsWith(`permission/${setName} failed spec validation: `), String(err.message)).toBe(true); + expect(err.issues.map((i: any) => i.path)).toEqual(objectNames.map((o) => `objects.${o}`)); + for (const [index, objectName] of objectNames.entries()) { + expect(err.issues[index].code).toBe('custom'); + expect(String(err.issues[index].message).startsWith(unknownFinding(setName, objectName)), err.issues[index].message).toBe(true); + } +}; + +describe('[#28990] the permission-set save door — a grant on an object that does not exist', () => { + it.each([ + ['a draft save into a package', { mode: 'draft', packageId: PKG }], + ['a draft save naming no package', { mode: 'draft' }], + ['a publish save', {}], + ['a publish save under `force`', { force: true }], + ])('%s is refused 422 at `objects.`, naming it, and nothing is stored', async (_label, saveOpts) => { + const { protocol, rows } = makeSession({ objects: ['qa_widget'] }); + + const err = await refusalOf(protocol.saveMetaItem({ + type: 'permission', name: 'qa_probe', item: set('qa_probe', ['qa_widget', 'qa_no_such_object']), ...saveOpts, + })); + + expectRefusal(err, 'qa_probe', ['qa_no_such_object']); + expect(stored(rows, 'permission', 'qa_probe')).toEqual([]); + }); + + it('one finding per unknown object, in the order the set names them; a short name is not the object', async () => { + const { protocol } = makeSession({ objects: ['crm_lead'] }); + + const err = await refusalOf(protocol.saveMetaItem({ + type: 'permission', name: 'qa_probe', item: set('qa_probe', ['lead', 'crm_lead', 'qa_no_such_object']), + })); + + expectRefusal(err, 'qa_probe', ['lead', 'qa_no_such_object']); + }); + + it('a draft save does not count an object draft that is gone by its publish: the per-item publish is refused', async () => { + const { protocol, rows } = makeSession(); + await protocol.saveMetaItem({ type: 'object', name: 'qa_widget', item: object('qa_widget'), mode: 'draft' }); + await protocol.saveMetaItem({ type: 'permission', name: 'qa_probe', item: set('qa_probe', ['qa_widget']), mode: 'draft' }); + + // The set alone goes live; its object stays a draft. + const err = await refusalOf(protocol.publishMetaItem({ type: 'permission', name: 'qa_probe' })); + + expectRefusal(err, 'qa_probe', ['qa_widget']); + expect(stored(rows, 'permission', 'qa_probe')).toEqual(['draft|-|qa_widget']); + }); +}); + +describe('[#28990] the publish doors — a set draft stored before the rule', () => { + it('publishMetaItem refuses it, and the draft stays a draft', async () => { + const { protocol, rows } = makeSession(); + plant(rows, set('qa_probe', ['qa_no_such_object']), 'draft', null); + + const err = await refusalOf(protocol.publishMetaItem({ type: 'permission', name: 'qa_probe' })); + + expectRefusal(err, 'qa_probe', ['qa_no_such_object']); + expect(stored(rows, 'permission', 'qa_probe')).toEqual(['draft|-|qa_no_such_object']); + }); + + it('publishPackageDrafts answers `refused`, names it, and promotes nothing of the batch', async () => { + const { protocol, rows } = makeSession({ objects: ['qa_widget'] }); + await protocol.saveMetaItem({ type: 'permission', name: 'qa_good', item: set('qa_good', ['qa_widget']), mode: 'draft', packageId: PKG }); + plant(rows, set('qa_probe', ['qa_no_such_object']), 'draft'); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res).toMatchObject({ outcome: 'refused', success: false, publishedCount: 0, published: [] }); + const causal = res.failed.find((f: any) => f.name === 'qa_probe'); + expect(causal, JSON.stringify(res.failed)).toMatchObject({ type: 'permission', code: 'INVALID_METADATA' }); + expect(JSON.stringify(causal)).toContain("grants on object 'qa_no_such_object'"); + // (The double's `transaction` does not roll back, so the sibling's + // promotion is not read here: `publishedCount: 0` is the batch's word.) + expect(stored(rows, 'permission', 'qa_probe')).toEqual([`draft|${PKG}|qa_no_such_object`]); + }); +}); + +describe('[#28990] controls — a grant on an object that exists is not refused', () => { + it.each([ + ['an object the registry registers', ['qa_widget']], + ['a platform object (an RBAC link table)', ['sys_user_position']], + ["the `'*'` wildcard", ['*']], + ['no grant at all', []], + ])('%s: saves as a draft, publishes, and saves live', async (_label, grants) => { + const { protocol, rows } = makeSession({ objects: ['qa_widget', 'sys_user_position'] }); + await protocol.saveMetaItem({ type: 'permission', name: 'qa_ok', item: set('qa_ok', grants), mode: 'draft', packageId: PKG }); + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + expect(res.outcome, JSON.stringify(res.failed)).toBe('published'); + expect((await protocol.saveMetaItem({ type: 'permission', name: 'qa_ok2', item: set('qa_ok2', grants) })).success).toBe(true); + expect(stored(rows, 'permission', 'qa_ok')).toEqual([`active|${PKG}|${grants.join(',')}`]); + }); + + it('an object stored in this environment that the registry has not registered', async () => { + const { protocol, rows } = makeSession(); + await protocol.saveMetaItem({ type: 'object', name: 'qa_widget', item: object('qa_widget') }); + await protocol.saveMetaItem({ type: 'permission', name: 'qa_ok', item: set('qa_ok', ['qa_widget']), mode: 'draft' }); + expect((await protocol.publishMetaItem({ type: 'permission', name: 'qa_ok' })).success).toBe(true); + expect(stored(rows, 'permission', 'qa_ok')).toEqual(['active|-|qa_widget']); + }); + + it('an object drafted in the same package publishes with the set, even promoted after it', async () => { + const { protocol, rows } = makeSession(); + await protocol.saveMetaItem({ type: 'object', name: 'qa_widget', item: object('qa_widget'), mode: 'draft', packageId: PKG }); + await protocol.saveMetaItem({ type: 'permission', name: 'qa_ok', item: set('qa_ok', ['qa_widget']), mode: 'draft', packageId: PKG }); + // Re-saving the object draft lists it after the set, so the batch + // promotes the set first, while the object is a draft. + await protocol.saveMetaItem({ type: 'object', name: 'qa_widget', item: { ...object('qa_widget'), label: 'Widget v2' }, mode: 'draft', packageId: PKG }); + const order = (await protocol.listDrafts({ packageId: PKG })).drafts.map((d: any) => `${d.type}/${d.name}`); + expect(order).toEqual(['permission/qa_ok', 'object/qa_widget']); + + const res = await protocol.publishPackageDrafts({ packageId: PKG }); + + expect(res, JSON.stringify(res.failed)).toMatchObject({ outcome: 'published', publishedCount: 2 }); + expect(stored(rows, 'permission', 'qa_ok')).toEqual([`active|${PKG}|qa_widget`]); + }); + + it('a re-save that keeps a grant the set already holds on an object gone since (a row stored before the rule)', async () => { + const { protocol, rows } = makeSession({ objects: ['qa_widget'] }); + plant(rows, set('qa_old', ['qa_gone']), 'active', null); + + const result = await protocol.saveMetaItem({ + type: 'permission', name: 'qa_old', item: set('qa_old', ['qa_gone', 'qa_widget'], { label: 'Renamed' }), + }); + + expect(result.success).toBe(true); + // …while adding another unknown object is this save's grant, and refused. + const err = await refusalOf(protocol.saveMetaItem({ + type: 'permission', name: 'qa_old', item: set('qa_old', ['qa_gone', 'qa_typo']), + })); + expectRefusal(err, 'qa_old', ['qa_typo']); + }); + + it('a registry that cannot look an object up refuses nothing', async () => { + const { protocol, rows } = makeSession({ looksUpObjects: false }); + expect((await protocol.saveMetaItem({ type: 'permission', name: 'qa_probe', item: set('qa_probe', ['qa_no_such_object']) })).success).toBe(true); + expect(stored(rows, 'permission', 'qa_probe')).toEqual(['active|-|qa_no_such_object']); + }); +}); diff --git a/packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts b/packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts index e0076613d47..ed94264c8d2 100644 --- a/packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts +++ b/packages/objectql/src/protocol-duplicate-package-objects-key-collision.test.ts @@ -167,12 +167,30 @@ describe('ADR-0070 D4 — duplicatePackage refuses a renamed reference that meet await save(type, item); } + /** + * A permission set stored before the save door refused a grant on an + * object that does not exist: the door now refuses `dupdst_customer` + * (no package registers it, none is saved), so the set is saved with its + * one known grant and its stored row is then given the full `objects` map, + * in the given key order — the at-rest shape this D4 branch still guards. + */ + async function seedStoredPermission(name: string, objects: Record): Promise { + await seed('permission', { name, label: 'Objects', objects: { dupsrc_customer: NARROW } } as { name: string }); + const row = Array.from(stores.get('sys_metadata')?.values() ?? []) + .find((r) => r.type === 'permission' && r.name === name && r.package_id === P); + expect(row).toBeDefined(); + const body = typeof row!.metadata === 'string' ? JSON.parse(row!.metadata) : row!.metadata; + const stored = { ...body, objects }; + row!.metadata = typeof row!.metadata === 'string' ? JSON.stringify(stored) : stored; + expect(storedBody('permission', name, P).objects).toEqual(objects); + } + for (const [order, objects] of [ ['the renamed key first', { dupsrc_customer: NARROW, dupdst_customer: WIDE }], ['the key it meets first', { dupdst_customer: WIDE, dupsrc_customer: NARROW }], ] as const) { it(`a permission set whose renamed \`objects\` key meets one it holds is refused, never merged (${order})`, async () => { - await seed('permission', { name: 'dupsrc_objs', label: 'Objects', objects } as { name: string }); + await seedStoredPermission('dupsrc_objs', objects); const res = await protocol.duplicatePackage({ sourcePackageId: P, targetPackageId: Q }); diff --git a/packages/qa/dogfood/test/permission-unknown-grant-object-refusal.dogfood.test.ts b/packages/qa/dogfood/test/permission-unknown-grant-object-refusal.dogfood.test.ts new file mode 100644 index 00000000000..68334a6235a --- /dev/null +++ b/packages/qa/dogfood/test/permission-unknown-grant-object-refusal.dogfood.test.ts @@ -0,0 +1,136 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +// +// [#28990] A permission set whose `objects` map grants on an object that does +// not exist is refused at the metadata save door, over the real showcase +// composition and through the REST doors the card measured. +// +// ## What was broken +// +// `PUT /api/v1/meta/permission/NAME` with a grant keyed `qa_no_such_object` +// answered 200 (live save), and the same body through `?mode=draft` then +// publish answered 200 too. The permission evaluator keys a grant by the +// object's name, so the grant applied to nothing; `defineStack` refuses the +// same grant at build (#18202). +// +// ## What these cases pin +// +// The save door refuses it (live and draft), `422 INVALID_METADATA` with the +// finding at `objects.` naming the object, and nothing is stored; a +// rollback to a version granting on an object deleted since is refused the +// same way. Controls: a grant on a showcase object, on a platform object (an +// RBAC link table) and the `'*'` wildcard save live; an object and a set +// granting on it, drafted together in one package, publish in one batch. +// The publish door's refusal of a draft stored before this rule is pinned +// in-process (`packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts`): +// no door can store such a draft any more. + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import { type VerifyStack } from '@objectstack/verify'; +import { bootShowcase } from './showcase-boot.js'; + +const PKG = 'app.grantprobe28990'; +/** The batch control's own package, so no other case's draft rides in its batch. */ +const BATCH_PKG = 'app.grantbatch28990'; + +const set = (name: string, objectNames: string[], label = 'Grant probe') => ({ + name, + label, + objects: Object.fromEntries(objectNames.map((o) => [o, { allowRead: true }])), +}); + +const object = (name: string) => ({ + name, + label: 'Grant probe widget', + sharingModel: 'public_read_write', + fields: { name: { type: 'text', label: 'Name' } }, +}); + +describe('dogfood: a permission-set grant on an object that does not exist is refused at save (#28990)', () => { + let stack: VerifyStack; + let token: string; + + const call = async (method: string, path: string, body?: unknown) => { + const res = await stack.apiAs(token, method, path, body); + const json: any = await res.json().catch(() => ({})); + return { status: res.status, json }; + }; + + beforeAll(async () => { + stack = await bootShowcase(); + token = await stack.signIn(); + const created = await call('POST', '/packages', { + manifest: { id: PKG, name: 'Grant probe', version: '0.1.0', type: 'app', namespace: 'grantprobe' }, + }); + expect(created.status, JSON.stringify(created.json)).toBe(201); + const batch = await call('POST', '/packages', { + manifest: { id: BATCH_PKG, name: 'Grant batch probe', version: '0.1.0', type: 'app', namespace: 'grantbatch' }, + }); + expect(batch.status, JSON.stringify(batch.json)).toBe(201); + }, 180_000); + + afterAll(async () => { + await stack?.stop(); + }); + + /** The refusal's envelope, wherever the REST door nests it. */ + const expectRefused = (res: { status: number; json: any }, setName: string, objectName: string) => { + expect(res.status, JSON.stringify(res.json)).toBe(422); + const wire = JSON.stringify(res.json); + expect(wire).toContain('INVALID_METADATA'); + expect(wire).toContain(`Permission '${setName}' grants on object '${objectName}' which is not defined in objects`); + expect(wire).toContain(`"path":"objects.${objectName}"`); + }; + + it('the card: a live save granting on an unknown object is refused, naming it, and nothing is stored', async () => { + const saved = await call('PUT', '/meta/permission/grantprobe_live', set('grantprobe_live', ['showcase_task', 'qa_no_such_object'])); + expectRefused(saved, 'grantprobe_live', 'qa_no_such_object'); + expect((await call('GET', '/meta/permission/grantprobe_live')).status).toBe(404); + }); + + it('the card: the same body as a draft is refused too, and nothing reaches the drafts', async () => { + const saved = await call('PUT', `/meta/permission/grantprobe_draft?mode=draft&package=${PKG}`, set('grantprobe_draft', ['qa_no_such_object'])); + expectRefused(saved, 'grantprobe_draft', 'qa_no_such_object'); + + const drafts = await call('GET', `/meta/_drafts?packageId=${PKG}`); + expect(drafts.status, JSON.stringify(drafts.json)).toBe(200); + expect(JSON.stringify(drafts.json)).not.toContain('grantprobe_draft'); + }); + + it.each([ + ['a showcase object', 'showcase_task'], + ['a platform object (an RBAC link table)', 'sys_user_position'], + ["the `'*'` wildcard", '*'], + ])('control: a live save granting on %s saves', async (_label, objectName) => { + const name = `grantprobe_ok_${objectName === '*' ? 'wildcard' : objectName}`; + const saved = await call('PUT', `/meta/permission/${name}`, set(name, [objectName])); + expect(saved.status, JSON.stringify(saved.json)).toBe(200); + }); + + it('control: an object and a set granting on it, drafted in one package, publish in one batch', async () => { + const obj = await call('PUT', `/meta/object/grantbatch_widget?mode=draft&package=${BATCH_PKG}`, object('grantbatch_widget')); + expect(obj.status, JSON.stringify(obj.json)).toBe(200); + const perm = await call('PUT', `/meta/permission/grantbatch_set?mode=draft&package=${BATCH_PKG}`, set('grantbatch_set', ['grantbatch_widget'])); + expect(perm.status, JSON.stringify(perm.json)).toBe(200); + + const published = await call('POST', `/packages/${BATCH_PKG}/publish-drafts`, {}); + expect(published.status, JSON.stringify(published.json)).toBe(200); + expect(published.json?.data, JSON.stringify(published.json)).toMatchObject({ outcome: 'published', publishedCount: 2 }); + }); + + it('a rollback to a version granting on an object deleted since is refused the same way', async () => { + const obj = await call('PUT', `/meta/object/grantprobe_gone?package=${PKG}`, object('grantprobe_gone')); + expect(obj.status, JSON.stringify(obj.json)).toBe(200); + for (const body of [set('grantprobe_restore', ['grantprobe_gone']), set('grantprobe_restore', ['showcase_task'])]) { + const saved = await call('PUT', `/meta/permission/grantprobe_restore?package=${PKG}`, body); + expect(saved.status, JSON.stringify(saved.json)).toBe(200); + } + const removed = await call('DELETE', `/meta/object/grantprobe_gone?package=${PKG}`); + expect(removed.status, JSON.stringify(removed.json)).toBe(200); + + const rollback = await call('POST', '/meta/permission/grantprobe_restore/rollback', { toVersion: 1 }); + + expectRefused(rollback, 'grantprobe_restore', 'grantprobe_gone'); + const current = await call('GET', '/meta/permission/grantprobe_restore'); + expect(JSON.stringify(current.json)).not.toContain('grantprobe_gone'); + }); +}); diff --git a/packages/qa/dogfood/test/wall-catalog-refusal.dogfood.test.ts b/packages/qa/dogfood/test/wall-catalog-refusal.dogfood.test.ts index 823b0bad08f..704e92839cd 100644 --- a/packages/qa/dogfood/test/wall-catalog-refusal.dogfood.test.ts +++ b/packages/qa/dogfood/test/wall-catalog-refusal.dogfood.test.ts @@ -101,15 +101,18 @@ for (const posture of ['isolated', 'group'] as const) { userB = await idOf(`w-b-${posture}@wall.test`); // The operator authors the catalog, and grants inside each organization: - // A holds `manage_metadata` and CRUD on the catalog and assignment tables - // in org A only (the stock `organization_admin` denies those tables, so - // without this grant the data door would refuse A on CRUD alone); + // A holds `manage_metadata` and CRUD on the assignment tables in org A + // only (the stock `organization_admin` denies those tables, so without + // this grant the data door would refuse A on CRUD alone). No grant names + // `sys_permission_set` / `sys_position`: C3 stage 8 retired those row + // objects, and the save door refuses a grant on an object that does not + // exist; // B's member holds the shared set in org B. const rbac = { allowRead: true, allowCreate: true, allowEdit: true, allowDelete: true }; for (const body of [ { name: META_AUTHOR, label: 'Meta author', objects: {}, systemPermissions: ['manage_metadata'] }, { name: SHARED, label: 'Shared', objects: {}, systemPermissions: [] }, - { name: RBAC_WRITER, label: 'Catalog table writer', systemPermissions: [], objects: { sys_permission_set: rbac, sys_position: rbac, sys_user_permission_set: rbac, sys_user_position: rbac } }, + { name: RBAC_WRITER, label: 'Catalog table writer', systemPermissions: [], objects: { sys_user_permission_set: rbac, sys_user_position: rbac } }, ]) { const saved = await stack.apiAs(operator, 'PUT', `/meta/permission/${body.name}`, body); expect(saved.status, await saved.clone().text()).toBe(200); diff --git a/scripts/engine-double-contract.pinned.json b/scripts/engine-double-contract.pinned.json index 0dd27312307..17e30636e16 100644 --- a/scripts/engine-double-contract.pinned.json +++ b/scripts/engine-double-contract.pinned.json @@ -2026,6 +2026,21 @@ "verb": "update", "pinned": 1 }, + { + "file": "packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts", + "verb": "delete", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts", + "verb": "findOne", + "pinned": 1 + }, + { + "file": "packages/metadata-protocol/src/protocol.unknown-grant-object-refusal.test.ts", + "verb": "update", + "pinned": 1 + }, { "file": "packages/metadata-protocol/src/protocol.unrecognised-meta-type.test.ts", "verb": "delete",