From 01218c6364b9274385067689fb89c90ffa266882 Mon Sep 17 00:00:00 2001 From: Claude Date: Thu, 6 Aug 2026 13:02:15 +0000 Subject: [PATCH] fix(objectql,spec): app-declared capabilities reach the registry with package provenance (#5870) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ObjectQL.registerApp()` is the only seam that stamps ADR-0010 provenance (`registerItem` -> `applyProtection` -> `_packageId` / `_provenance`), and the `metadataArrayKeys` list driving it — twice in engine.ts, the manifest seam and the nested-plugin seam — carried every Security-Protocol collection except `capabilities`. `bootstrapDeclaredCapabilities` resolves the owner as `cap._packageId ?? cap.packageId` and reads `readDeclared(ql, 'capability')`, which was therefore always empty: the author-side `packageId` documented as the ADR-0086 D3 fallback was in fact mandatory, and omitting it produced one boot warn plus one authorization declaration that never took effect. Register `capabilities` through the same seam as `permissions` at both sites, and map `capabilities` -> `capability` in `PLURAL_TO_SINGULAR` so the items land under the singular type the seeder reads back (the mapping AppPlugin's security bridge already used). Part of objectstack-ai/objectstack#4967 (Part 2). Co-Authored-By: Claude Fable 5 Claude-Session: https://claude.ai/code/session_019Q7oc7ASjh8yxyS3Yz78We --- .../capabilities-registry-provenance-seam.md | 54 ++++++ .../app-showcase/src/security/capabilities.ts | 21 ++- .../src/engine-capability-provenance.test.ts | 169 ++++++++++++++++++ packages/objectql/src/engine.ts | 19 +- .../src/shared/metadata-collection.zod.ts | 6 + 5 files changed, 255 insertions(+), 14 deletions(-) create mode 100644 .changeset/capabilities-registry-provenance-seam.md create mode 100644 packages/objectql/src/engine-capability-provenance.test.ts diff --git a/.changeset/capabilities-registry-provenance-seam.md b/.changeset/capabilities-registry-provenance-seam.md new file mode 100644 index 0000000000..0877657610 --- /dev/null +++ b/.changeset/capabilities-registry-provenance-seam.md @@ -0,0 +1,54 @@ +--- +"@objectstack/objectql": patch +"@objectstack/spec": patch +--- + +fix(objectql,spec): app-declared `capabilities` reach the registry with package provenance (#5870, #4967 Part 2) + +An authorization capability a package DEFINES (`defineCapability` → +`stack.capabilities`, ADR-0066 D1) never acquired registry provenance, so a +declaration that did not repeat its owner by hand was **declared but never +enforced**. + +**FROM.** `ObjectQL.registerApp()` decomposes a manifest's metadata arrays and +calls `SchemaRegistry.registerItem(type, item, 'name', packageId)` — the only +seam that runs `applyProtection(item, { packageId })` and stamps `_packageId` / +`_provenance`. The key list driving that decomposition (`metadataArrayKeys`, +present twice in `packages/objectql/src/engine.ts`: the manifest seam and the +nested-plugin seam) carried every other Security-Protocol collection — +`permissions`, `sharingRules`, `roles`, `profiles`, `policies` — but not +`capabilities`. The other path a stack's security metadata travels, +`AppPlugin` → `MetadataManager.registerInMemory`, stamps nothing by design. So +`readDeclared(ql, 'capability')` always returned `[]`, and +`bootstrapDeclaredCapabilities` — which resolves the owner as +`cap._packageId ?? cap.packageId` — could never satisfy the first half. + +The consequence was a contract that lied in the direction that costs the most: +`CapabilityDeclarationSchema.packageId` documents itself as the ADR-0086 D3 +*fallback* used "when the registry has not stamped `_packageId`", and the field +is `.optional()`, but in practice it was **mandatory**. Omitting it produced one +boot `warn` and one authorization grant that silently never took effect — +permission sets have rows and materialize, capabilities did not. + +**TO.** `capabilities` is registered through the same provenance seam as +`permissions`, at both `metadataArrayKeys` sites, and `capabilities` → +`capability` joins `PLURAL_TO_SINGULAR` in `@objectstack/spec` so the items land +under the singular type name the seeder reads back (the same mapping +`AppPlugin`'s security bridge already used). A capability declared by a package +— or by a package's nested plugin — now carries `_packageId` / +`_provenance: 'package'`, is listable via `registry.listItems('capability')`, +and is seeded into `sys_capability` with `managed_by:'package'` and a real +`package_id`. + +**What authors should do.** Nothing is required and nothing breaks: an authored +`packageId` still parses and still wins where the registry has not stamped +(items registered outside a package context). It is now genuinely optional — +drop it and the platform supplies the owner. Where both are present the registry +stamp takes precedence, as the schema always said; the showcase's +`showcase.export_data` keeps its authored `packageId` and names the same id the +stack does, so the two agree. + +This does not change what may be created at runtime: `capability` has no entry +in `DEFAULT_METADATA_TYPE_REGISTRY`, and the runtime-create gate reads that +registry, not the item store — its verdict for the type is the same before and +after. diff --git a/examples/app-showcase/src/security/capabilities.ts b/examples/app-showcase/src/security/capabilities.ts index fe1c9bb5fb..f83713fdc0 100644 --- a/examples/app-showcase/src/security/capabilities.ts +++ b/examples/app-showcase/src/security/capabilities.ts @@ -37,17 +37,16 @@ import { defineCapability } from '@objectstack/spec'; * capability that never existed — one boot `warn` and an inert security * declaration. * - * Why it is authored here rather than inferred: the registry stamp never - * reaches an app-declared capability, because `capabilities` is missing from - * the metadata-array key list the ObjectQL engine registers a stack's - * collections through — so the "fallback" is in practice mandatory. That is a - * PLATFORM gap, filed as #4967 (together with the sharper half: a refused - * declaration is still counted as declared, which suppresses the back-compat - * derivation and leaves the capability existing nowhere), not something the - * showcase should work around. Declaring `packageId` is the spec's own - * sanctioned entry point, so this both fixes the app and demonstrates the - * field; when the platform stamps `_packageId` that takes precedence and this - * stays consistent with it. + * Why it is still authored here now that the platform stamps: it used to be + * MANDATORY, because `capabilities` was missing from the metadata-array key + * list the ObjectQL engine registers a stack's collections through, so the + * registry stamp never reached an app-declared capability and the documented + * "fallback" was the only provenance there was. #5870 (#4967 Part 2) closed + * that seam — `registerApp` now stamps `_packageId` on capabilities exactly as + * it does on permission sets — so this line is once again what the spec says it + * is: the fallback, kept because it also DEMONSTRATES the field. It names the + * same id as the stack (`com.example.showcase`), so the stamp that now takes + * precedence agrees with it. */ export const ExportDataCapability = defineCapability({ name: 'showcase.export_data', diff --git a/packages/objectql/src/engine-capability-provenance.test.ts b/packages/objectql/src/engine-capability-provenance.test.ts new file mode 100644 index 0000000000..0cae1e32e0 --- /dev/null +++ b/packages/objectql/src/engine-capability-provenance.test.ts @@ -0,0 +1,169 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#5870, #4967 Part 2] `capabilities` in the provenance-stamping seam. + * + * ADR-0066 D1 says a package DEFINES its authorization capabilities and the + * registry attributes them to that package. The attribution only happens on ONE + * path: `ObjectQL.registerApp()` decomposes a manifest's metadata arrays and + * calls `SchemaRegistry.registerItem(type, item, 'name', packageId)`, which runs + * `applyProtection(item, { packageId })` and stamps `_packageId` / + * `_provenance`. The key list that drives that decomposition + * (`metadataArrayKeys`, twice in `engine.ts`: the manifest seam and the nested + * `registerPlugin` seam) carried every other Security-Protocol collection — + * `permissions`, `sharingRules`, `roles`, `profiles`, `policies` — but not + * `capabilities`. + * + * Consequence before the fix: `plugin-security`'s `bootstrapDeclaredCapabilities` + * resolves the owner as `cap._packageId ?? cap.packageId` and reads its input + * with `readDeclared(ql, 'capability')`, which is + * `engine.registry.listItems('capability')` mapped through `i?.content ?? i` + * (`packages/plugins/plugin-security/src/bootstrap-declared-permissions.ts:61-69`). + * With `capabilities` outside the seam that list was ALWAYS empty, so the + * `_packageId` half of that `??` could never be satisfied and the author-side + * `packageId` — documented in `CapabilityDeclarationSchema` as the *fallback* + * for "registry-stamped absent" — was in fact mandatory. Omitting it produced a + * boot `warn` and one authorization grant that never took effect: declared ≠ + * enforced. + * + * These tests assert the registry state `readDeclared` consumes rather than + * importing it — `@objectstack/plugin-security` does not depend on + * `@objectstack/objectql` (nor the reverse), and a test-only edge between them + * would invent a package dependency the runtime does not have. The expression + * under test is quoted from that file above and pinned by + * {@link readDeclaredShape} below. + */ + +import { describe, it, expect } from 'vitest'; +import { pluralToSingular } from '@objectstack/spec/shared'; +import { ObjectQL } from './engine'; + +/** + * The exact read `bootstrapDeclaredCapabilities` performs on the engine, kept + * in one place so the shape this suite depends on is stated once: + * `readDeclared(ql, type)` → `registry.listItems(type)` unwrapped by + * `content ?? item`. + */ +function readDeclaredShape(engine: ObjectQL, type: string): any[] { + return (engine.registry.listItems(type) ?? []).map((i: any) => i?.content ?? i).filter(Boolean); +} + +const PKG = 'com.acme.exporter'; + +/** A declaration with NO author-side `packageId` — the registry must supply it. */ +function manifestWithCapability() { + return { + id: PKG, + name: 'exporter', + capabilities: [ + { name: 'export_data', label: 'Export Data', description: 'Bulk-export records.', scope: 'org' }, + ], + permissions: [ + { name: 'ops', label: 'Ops', systemPermissions: ['export_data'] }, + ], + }; +} + +describe('registerApp — declared capabilities carry registry provenance (#5870)', () => { + it('registers a manifest capability under the singular `capability` type', () => { + const engine = new ObjectQL(); + engine.registerApp(manifestWithCapability()); + + const caps = readDeclaredShape(engine, 'capability'); + expect(caps.map((c) => c.name)).toEqual(['export_data']); + }); + + it('stamps `_packageId` so the seeder resolves an owner without an author-side packageId', () => { + const engine = new ObjectQL(); + engine.registerApp(manifestWithCapability()); + + const cap = readDeclaredShape(engine, 'capability')[0]; + // The seeder's own expression: registry provenance first (ADR-0010), + // author-declared `packageId` (ADR-0086 D3) only as fallback. + expect(cap._packageId ?? cap.packageId).toBe(PKG); + // …and the fallback really is a fallback: nothing authored `packageId`. + expect(cap.packageId).toBeUndefined(); + expect(cap._packageId).toBe(PKG); + expect(cap._provenance).toBe('package'); + }); + + it('reaches the registry on exactly the same terms as its `permissions` sibling', () => { + const engine = new ObjectQL(); + engine.registerApp(manifestWithCapability()); + + const cap = readDeclaredShape(engine, 'capability')[0]; + const permission = readDeclaredShape(engine, 'permission')[0]; + + expect(permission?._packageId).toBe(PKG); + expect(cap?._packageId).toBe(permission?._packageId); + expect(cap?._provenance).toBe(permission?._provenance); + }); + + it('keeps two packages\' same-named capabilities attributed to their own owner', () => { + const engine = new ObjectQL(); + engine.registerApp({ id: 'com.acme.crm', capabilities: [{ name: 'export_data', label: 'CRM Export' }] }); + engine.registerApp({ id: 'com.acme.hr', capabilities: [{ name: 'export_data', label: 'HR Export' }] }); + + expect(engine.registry.getItem('capability', 'export_data', 'com.acme.crm')?.label).toBe('CRM Export'); + expect(engine.registry.getItem('capability', 'export_data', 'com.acme.hr')?.label).toBe('HR Export'); + expect(readDeclaredShape(engine, 'capability')).toHaveLength(2); + }); + + it('stamps capabilities declared by a NESTED plugin too (the second seam)', () => { + // `engine.ts` carries `metadataArrayKeys` twice — the manifest seam and the + // `registerPlugin` seam reached via `manifest.plugins[]`. A fix applied to + // only one leaves a package's nested plugin declaring capabilities that + // still never get stamped, which is the same defect one level down. + const engine = new ObjectQL(); + engine.registerApp({ + id: PKG, + plugins: [ + { name: 'billing', capabilities: [{ name: 'billing.refund', label: 'Refund' }] }, + ], + }); + + const caps = readDeclaredShape(engine, 'capability'); + expect(caps.map((c) => c.name)).toEqual(['billing.refund']); + // Nested plugins contribute UNDER the parent package's ownership. + expect(caps[0]._packageId).toBe(PKG); + }); +}); + +describe('metadataArrayKeys ↔ stamping consistency pin (#5870)', () => { + /** + * The three Security-Protocol collections a boot seeder reads back by + * SINGULAR type name: `bootstrapDeclaredPermissions` (`permission`), + * `bootstrapDeclaredCapabilities` (`capability`) and + * `bootstrapDeclaredSharingRules` (`sharing_rule`). Membership in + * `metadataArrayKeys` alone is not enough — the plural key must also map to + * the singular type the seeder asks for, or the items land in a store nobody + * reads. This pin holds both halves at once. + */ + const SEEDED_SECURITY_COLLECTIONS: ReadonlyArray = [ + ['permissions', 'permission'], + ['capabilities', 'capability'], + ['sharingRules', 'sharing_rule'], + ]; + + it('maps each seeded security collection to the singular type its seeder reads', () => { + for (const [plural, singular] of SEEDED_SECURITY_COLLECTIONS) { + expect(pluralToSingular(plural), `${plural} must register as '${singular}'`).toBe(singular); + } + }); + + it('stamps every seeded security collection through the manifest seam', () => { + const engine = new ObjectQL(); + engine.registerApp({ + id: PKG, + permissions: [{ name: 'ops', label: 'Ops' }], + capabilities: [{ name: 'export_data', label: 'Export Data' }], + sharingRules: [{ name: 'share_all', object: 'account', accessLevel: 'read' }], + }); + + for (const [, singular] of SEEDED_SECURITY_COLLECTIONS) { + const items = readDeclaredShape(engine, singular); + expect(items, `nothing registered under '${singular}'`).toHaveLength(1); + expect(items[0]._packageId, `'${singular}' reached the registry unstamped`).toBe(PKG); + } + }); +}); diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 7b53d08ed3..d326e157d6 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -2209,8 +2209,17 @@ export class ObjectQL implements IObjectQLEngine { // Automation Protocol 'flows', 'workflows', 'approvals', 'webhooks', 'jobs', - // Security Protocol - 'roles', 'permissions', 'profiles', 'sharingRules', 'policies', + // Security Protocol — `capabilities` is here for the same reason as + // `permissions` (#5870, #4967 Part 2): the ONLY seam that stamps + // ADR-0010 provenance is `registerItem` → `applyProtection`, so a + // collection missing from this list reaches no registry with a + // `_packageId`. `bootstrapDeclaredCapabilities` resolves the owning + // package as `cap._packageId ?? cap.packageId`; while `capabilities` + // sat outside this list the first half could never be satisfied and + // `readDeclared(ql, 'capability')` returned nothing, which made the + // author-side `packageId` — documented as the FALLBACK — mandatory, + // and its omission a silent, unenforced authorization declaration. + 'roles', 'permissions', 'capabilities', 'profiles', 'sharingRules', 'policies', // AI Protocol 'agents', 'tools', 'skills', 'ragPipelines', // API Protocol @@ -2374,7 +2383,11 @@ export class ObjectQL implements IObjectQLEngine { const metadataArrayKeys = [ 'actions', 'views', 'pages', 'dashboards', 'reports', 'datasets', 'themes', 'flows', 'workflows', 'approvals', 'webhooks', - 'roles', 'permissions', 'profiles', 'sharingRules', 'policies', + // `capabilities` per #5870 — same stamping seam, one level down: a + // nested plugin's declarations must carry the parent package's + // provenance too, or the same declared-≠-enforced hole reopens for + // packages that ship their capabilities from a nested plugin. + 'roles', 'permissions', 'capabilities', 'profiles', 'sharingRules', 'policies', 'agents', 'ragPipelines', 'apis', 'hooks', 'mappings', 'analyticsCubes', 'connectors', 'docs', 'books', diff --git a/packages/spec/src/shared/metadata-collection.zod.ts b/packages/spec/src/shared/metadata-collection.zod.ts index faae132b18..d3e5b1274d 100644 --- a/packages/spec/src/shared/metadata-collection.zod.ts +++ b/packages/spec/src/shared/metadata-collection.zod.ts @@ -121,6 +121,12 @@ export const PLURAL_TO_SINGULAR: Record = { jobs: 'job', positions: 'position', permissions: 'permission', + // [ADR-0066 D1, #5870] Package-declared authorization capabilities. The + // singular form is what `bootstrapDeclaredCapabilities` reads back + // (`readDeclared(ql, 'capability')`) and what `AppPlugin` already registers + // under; without the mapping the registration seam stored them as + // `'capabilities'`, a store nothing reads. + capabilities: 'capability', sharingRules: 'sharing_rule', apis: 'api', webhooks: 'webhook',