diff --git a/.changeset/olive-donuts-invent.md b/.changeset/olive-donuts-invent.md new file mode 100644 index 0000000000..4db59c46d1 --- /dev/null +++ b/.changeset/olive-donuts-invent.md @@ -0,0 +1,11 @@ +--- +'@objectstack/runtime': minor +--- + +**`AppPlugin` now names the manifest-stage `permissions` value its ADR-0057 security registrar cannot read, instead of dropping it in silence.** + +The registrar flattens the manifest under the stack's own collections (`{ ...manifest, ...collections }`), so `manifest.permissions` is read whenever the stack declares no `permissions` collection of its own. That key is the ADR-0025 §3.2 capability grant a package *requests* — a flat list of permission strings, or `{ services, hooks, network, fs }` — while the registrar wants ADR-0090 `PermissionSet[]`. Both arms were skipped with nothing logged: the structured arm is not an array, so the whole value never entered the loop; every member of the flat list carries no `name`, so all of them were dropped. An author who wrote `manifest: { permissions: ['sales_rep'] }` meaning a permission set got no set registered, no `sys_audience_binding_suggestion`, and no line anywhere saying why — the "absence must be loud" rule in AGENTS.md → Route & surface ownership §3. + +It now warns once per boot, naming the field, how many entries were lost, both readings of the key, and where permission sets belong (`defineStack({ permissions: [ … ] })`). The report is written per `SECURITY_FIELDS` entry, so a hand-built bundle carrying `positions` / `capabilities` / `sharingRules` on its manifest is named too. + +**Nothing else moves.** Which items register is byte-for-byte unchanged — the registrar is deliberately *not* made tolerant of the grant reading (widening the key was rejected by name, #14242 road C, maintainer 2026-09-02). The line is `warn`, not `error`: nothing here claimed to persist anything. It stays silent on every shape where nothing was lost — a stack declaring its own `permissions` collection, a manifest with no such key, a manifest whose entries the registrar really can read, and the `securityMetadataRegistrar: 'artifact-door'` composition that owns the route. diff --git a/packages/runtime/src/app-plugin.manifest-security-collision.test.ts b/packages/runtime/src/app-plugin.manifest-security-collision.test.ts new file mode 100644 index 0000000000..5c963bcd3e --- /dev/null +++ b/packages/runtime/src/app-plugin.manifest-security-collision.test.ts @@ -0,0 +1,202 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * `AppPlugin`'s ADR-0057 security-metadata registrar flattens the manifest + * under the stack's own collections — `{ ...manifest, ...collections }` — so a + * manifest key the stack does not also declare reaches the registrar. Exactly + * ONE key can arrive that way, and it arrives meaning something else. + * + * Measured against `ManifestSchema` (`packages/spec/src/kernel/manifest.zod.ts`) + * for all four `SECURITY_FIELDS`, because the card that opened this asked for + * that measurement first: + * + * | field | on `ManifestSchema` | can collide | + * |----------------|-------------------------------------|-------------| + * | `permissions` | yes — `ManifestPermissionsSchema` | YES | + * | `capabilities` | yes, but a `retiredKey()` tombstone | no | + * | `positions` | not declared (`strictObject`) | no | + * | `sharingRules` | not declared (`strictObject`) | no | + * + * `manifest.permissions` is the ADR-0025 §3.2 capability grant a package + * REQUESTS — the legacy flat `string[]`, or the structured + * `{ services, hooks, network, fs }` block. The registrar wants ADR-0090 + * `PermissionSet[]`. Skipping the grant is the right OUTCOME; doing it without + * a word is what this file pins, both directions: + * + * - it SPEAKS when the manifest's value reached the registrar and nothing + * came of it (both arms), and + * - it STAYS SILENT on every shape where nothing was lost — the stack + * declaring its own collection, a manifest with no such key, a manifest + * whose entries the registrar really can read, and the artifact-door + * composition that owns the route. + * + * The second half is the point: a guard that fires on every boot is the same + * silence with extra noise. + * + * ⛔ The registrar is NOT made tolerant of either arm — the two readings are + * incompatible and widening the key was rejected by name (#14242 road C, + * maintainer 2026-09-02). Nothing below asserts that a grant registers. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { AppPlugin } from './app-plugin.js'; + +/** The substring every line this file is about carries. */ +const MARKER = 'cannot read it'; + +type Registration = { type: string; name: string; item: any }; + +interface Driven { + registrations: Registration[]; + warns: string[]; + infos: string[]; +} + +function fakeCtx(metadataService: unknown, sink: { warns: string[]; infos: string[] }) { + return { + logger: { + info: vi.fn((msg: string) => { sink.infos.push(String(msg)); }), + warn: vi.fn((msg: string) => { sink.warns.push(String(msg)); }), + error: vi.fn(), + debug: vi.fn(), + }, + registerService: vi.fn(), + getService: vi.fn((name: string) => { + if (name === 'metadata') return metadataService; + if (name === 'objectql') return {} as any; + return undefined; + }), + getServices: vi.fn(() => []), + hook: vi.fn(), + trigger: vi.fn(), + } as any; +} + +/** Drive the real ADR-0057 block over `bundle`, capturing its writes and its log. */ +async function drive(bundle: unknown, opts: ConstructorParameters[2] = {}): Promise { + const registrations: Registration[] = []; + const sink = { warns: [] as string[], infos: [] as string[] }; + const plugin = new AppPlugin(bundle, undefined, opts); + await plugin.start!( + fakeCtx( + { + registerInMemory: (type: string, name: string, item: unknown) => { + registrations.push({ type, name, item }); + }, + }, + sink, + ), + ); + return { registrations, warns: sink.warns, infos: sink.infos }; +} + +const MANIFEST = { + id: 'com.test.issue-18034', + name: 'Collision Probe', + type: 'app', + version: '1.0.0', +} as const; + +/** A stack whose manifest carries `permissions` and whose top level does not. */ +const stackWithManifestPermissions = (permissions: unknown): any => ({ + manifest: { ...MANIFEST, permissions }, + objects: [], +}); + +/** One readable ADR-0090 permission set, for the stack's own collection. */ +const PERMISSION_SET = { name: 'support_agent', label: 'Support Agent' }; + +const marked = (lines: string[]): string[] => lines.filter((l) => l.includes(MARKER)); + +describe('#18034 — the manifest-stage `permissions` grant reaching the ADR-0090 registrar is named, not dropped in silence', () => { + it('legacy ADR-0025 arm (flat string list): registers nothing, and says so once, naming the arm and the remedy', async () => { + const { registrations, warns } = await drive( + stackWithManifestPermissions(['system.user.read', 'system.data.write']), + ); + + // The OUTCOME is unchanged — a capability grant is not a permission set. + expect(registrations.filter((r) => r.type === 'permission')).toEqual([]); + + const lines = marked(warns); + expect(lines, 'the drop must be audible exactly once').toHaveLength(1); + expect(lines[0]).toContain('manifest.permissions'); + expect(lines[0]).toContain('2 of 2'); // both members named as lost + expect(lines[0]).toContain('ADR-0025'); // which reading was found + expect(lines[0]).toContain('defineStack'); // where the sets belong + }); + + it('structured ADR-0025 arm (`{ services, hooks, … }`): registers nothing, and says so once', async () => { + const { registrations, warns } = await drive( + stackWithManifestPermissions({ services: ['object', 'http'], hooks: ['record.beforeInsert'] }), + ); + + expect(registrations.filter((r) => r.type === 'permission')).toEqual([]); + + const lines = marked(warns); + expect(lines).toHaveLength(1); + expect(lines[0]).toContain('manifest.permissions'); + expect(lines[0]).toContain('defineStack'); + }); + + // ── Discrimination: the shapes that must stay silent ────────────────── + // + // Without these, the assertions above are satisfied by a line that fires on + // every boot, which reports nothing at all. + + it('stays silent when the stack declares its own `permissions` collection — the manifest key never reaches the registrar', async () => { + const { registrations, warns } = await drive({ + manifest: { ...MANIFEST, permissions: ['system.user.read'] }, + permissions: [PERMISSION_SET], + objects: [], + }); + + expect(registrations.filter((r) => r.type === 'permission').map((r) => r.name)).toEqual(['support_agent']); + expect(marked(warns)).toEqual([]); + }); + + it('stays silent on a manifest that declares no `permissions` at all', async () => { + const { registrations, warns } = await drive({ + manifest: { ...MANIFEST }, + permissions: [PERMISSION_SET], + objects: [], + }); + + expect(registrations.filter((r) => r.type === 'permission').map((r) => r.name)).toEqual(['support_agent']); + expect(marked(warns)).toEqual([]); + }); + + it('stays silent when the manifest value IS readable — nothing was lost, so there is nothing to report', async () => { + // Off-spec as authored (`defineStack` strict refuses it with + // `invalid_union` on `manifest.permissions`), but reachable through a + // non-strict or hand-built bundle — and on that path the registrar + // reads the entries today. The guard must not turn a working shape into + // a warning. + const { registrations, warns } = await drive(stackWithManifestPermissions([PERMISSION_SET])); + + expect(registrations.filter((r) => r.type === 'permission').map((r) => r.name)).toEqual(['support_agent']); + expect(marked(warns)).toEqual([]); + }); + + it('stays silent under the artifact-door registrar — that composition owns the route and prints its own summary', async () => { + const { registrations, warns } = await drive( + stackWithManifestPermissions(['system.user.read']), + { securityMetadataRegistrar: 'artifact-door' }, + ); + + expect(registrations).toEqual([]); + expect(marked(warns)).toEqual([]); + }); + + it('a partially readable list still registers what it can, and names only the members it lost', async () => { + const { registrations, warns } = await drive( + stackWithManifestPermissions([PERMISSION_SET, 'system.user.read']), + ); + + expect(registrations.filter((r) => r.type === 'permission').map((r) => r.name)).toEqual(['support_agent']); + + const lines = marked(warns); + expect(lines).toHaveLength(1); + // One member lost, not two — the count is measured, not the array length. + expect(lines[0]).toContain('1 of 2'); + }); +}); diff --git a/packages/runtime/src/app-plugin.ts b/packages/runtime/src/app-plugin.ts index ece2a7fc95..37fce89a82 100644 --- a/packages/runtime/src/app-plugin.ts +++ b/packages/runtime/src/app-plugin.ts @@ -886,8 +886,9 @@ export class AppPlugin implements Plugin { }, ); } else { - const rawSecurityBundle: any = this.collections.manifest - ? { ...this.collections.manifest, ...this.collections } + const manifestSource: any = this.collections.manifest; + const rawSecurityBundle: any = manifestSource + ? { ...manifestSource, ...this.collections } : this.collections; // [#12844] Same bytes, same conversion policy — one funnel. // @@ -923,11 +924,71 @@ export class AppPlugin implements Plugin { let count = 0; for (const [field, type] of SECURITY_FIELDS) { const arr = securityBundle?.[field]; - if (!Array.isArray(arr)) continue; - for (const item of arr) { - if (!item?.name) continue; - metadata.registerInMemory(type, item.name, item); - count += 1; + // [#18034] Which SOURCE the value above came from, decided + // on the RAW inputs rather than on the flattened copy: the + // manifest contributed this key only when the stack's own + // collections do not declare it, because `{ ...manifest, + // ...collections }` lets the stack win. Computed here and + // not from `securityBundle` because the ADR-0087 pass can + // rewrite a COLLECTION KEY (`roles` -> `positions`), and a + // key the conversion produced came from the stack. + const fromManifest = manifestSource !== undefined + && manifestSource !== null + && manifestSource[field] !== undefined + && (this.collections as any)[field] === undefined; + let dropped = 0; + let members = 0; + if (Array.isArray(arr)) { + members = arr.length; + for (const item of arr) { + if (!item?.name) { dropped += 1; continue; } + metadata.registerInMemory(type, item.name, item); + count += 1; + } + } + // The registrar wants ADR-0090 `PermissionSet[]`; the key + // it just read off the MANIFEST means something else, and + // skipping it is the right outcome. Saying nothing is not + // (AGENTS.md, Route & surface ownership §3 — absence must + // be loud). ⛔ The loop is NOT made tolerant of the other + // reading: widening the key was rejected by name (#14242 + // road C, maintainer 2026-09-02). + // + // Measured against `ManifestSchema`, `permissions` is the + // one `SECURITY_FIELDS` key that can arrive this way at + // all: `capabilities` is a `retiredKey()` tombstone that + // refuses any value at parse, and `positions` / + // `sharingRules` are undeclared on a `strictObject`. The + // report is written per field anyway, because a bundle + // that never reached that parse can still carry them and + // this block is the last reader before the value is gone. + // + // `warn`, not `error`: nothing here claimed to persist + // anything — a permission set is simply not registered and + // the next person to look for it finds out. Once per boot, + // because `start()` runs once per app per kernel. + if (fromManifest && (dropped > 0 || !Array.isArray(arr))) { + ctx.logger.warn( + `[AppPlugin] \`manifest.${field}\` reached the stack-declared \`${type}\` ` + + `registrar, which cannot read it: ${Array.isArray(arr) + ? `it dropped ${dropped} of ${members} entr${members === 1 ? 'y' : 'ies'}, ` + + `because a \`${type}\` is identified by its \`name\` and ` + + `${dropped === 1 ? 'that entry carries' : 'those entries carry'} none` + : `the value is ${arr === null ? '`null`' : `${/^[aeiou]/i.test(typeof arr) ? 'an' : 'a'} ${typeof arr}`}, ` + + 'not a list, so the whole of it was skipped'}` + + `. ${field === 'permissions' + ? 'Nothing is lost if an ADR-0025 §3.2 capability GRANT was meant — ' + + '`manifest.permissions` is the manifest-stage grant a package requests ' + + '(a flat list of permission strings, or `{ services, hooks, network, fs }`), ' + + 'and this registrar only reads ADR-0090 permission sets. But if permission ' + + 'sets were meant, none is registered, no audience-binding suggestion is ' + + 'offered, and the boot goes on looking healthy' + : `\`${field}\` is not a key \`ManifestSchema\` declares, so this value reached ` + + 'the runtime without passing an authoring parse'}` + + `. Declare the collection at the stack's own top level — ` + + `\`defineStack({ ${field}: [ … ] })\` — not on the manifest.`, + { appId, field, type, dropped, members, registrar: this.securityMetadataRegistrar }, + ); } } if (count > 0) {