From e99a3af709fa772530381c1ddf2738bde1b31179 Mon Sep 17 00:00:00 2001 From: Claude Date: Fri, 4 Sep 2026 05:50:53 +0000 Subject: [PATCH] fix(plugin-security): the app default permission set resolves from `packages[]` (#15007) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Reader card 4/4 of the ADR-0130 D4 option-B program ruled on #14512. This is the site both earlier enumerations missed: `appSecurityPluginOptions` read `config.permissions` top-level and nothing else, so a multi-package artifact in the option-B shape resolves no `isDefault` profile, boots clean, and leaves every member of the app on the platform baseline alone. `declaredPermissionSets` now reads the flattened top level FIRST — so every artifact emitted today answers bit-identically — and then each package body in `resolveArtifactPackageOrder` order (`@objectstack/core`, ADR-0130 D4+D5). The singular `manifest` is still not consulted (#7001), and a malformed `packages` raises the loader's own ADR-0112 envelope rather than being skipped. The #15004 acceptance pin's ledger shrinks by exactly one row, the B2 from-source one. Its B1 sibling stays: that row reads `appSecurityPluginOptions(createStandaloneStack(...))`, and the standalone result surfaces neither `permissions` nor `packages` under option B — it is downstream of card #15005, not of this reader. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m --- ...ecurity-default-profile-packages-reader.md | 50 +++++ .../option-b-reader-acceptance.pin.test.ts | 1 - .../src/app-default-permission-set.test.ts | 195 ++++++++++++++++++ .../src/app-default-permission-set.ts | 88 +++++++- 4 files changed, 328 insertions(+), 6 deletions(-) create mode 100644 .changeset/plugin-security-default-profile-packages-reader.md diff --git a/.changeset/plugin-security-default-profile-packages-reader.md b/.changeset/plugin-security-default-profile-packages-reader.md new file mode 100644 index 0000000000..d9f6241fb3 --- /dev/null +++ b/.changeset/plugin-security-default-profile-packages-reader.md @@ -0,0 +1,50 @@ +--- +"@objectstack/plugin-security": patch +--- + +fix(plugin-security): the app default permission set resolves from `packages[]`, not only the flattened top level (#15007) + +`appSecurityPluginOptions(config)` read `config.permissions` and nothing else. +For a multi-package artifact under the ADR-0130 D4 option-B shape — where +`packages[]` carries each definition exactly once and the flattened top-level +copy is gone — that read returns `undefined`, the reader concludes "this app +declared no default profile", and the boot continues. Nothing throws and +nothing logs. + +That silence has a security posture attached. The name this resolves becomes +the `SecurityPlugin`'s `fallbackPermissionSet`, i.e. the app's half of every +authenticated human principal's additive baseline +(`composeHumanBaselinePermissionSets`, ADR-0090 D5). Losing it does not deny +anyone the boot — the deployment simply runs on the platform floor alone, and +every member of a multi-package app quietly holds less access than the app +declared for them. #7555 measured what that looks like from the outside: nav +entries served, 403 behind them. + +The resolution now reads the flattened top level FIRST and then each package +body, in the order `resolveArtifactPackageOrder` (`@objectstack/core`, +ADR-0130 D4+D5) registers them: + +- **Every artifact the platform emits today answers bit-identically.** The + flattened level still answers first, so the `packages[]` pass can only supply + a set where the top level had none. This is the reader half of the ruled + order (readers first, emitter last, artifact stays additive throughout), so + it lands with no change to what any command emits. +- **Order is the platform's one package order, not the array's.** + `appDefaultPermissionSetName` resolves the FIRST `isDefault` set, so with two + packages declaring one, "first" has to mean here what it means at every other + artifact reader: dependency-topological, so a package that extends another is + read after it whichever array slot it occupies. +- **The singular `manifest` is still not consulted** (#7001 — the harness must + not honour a declaration `serve.ts` ignores). That is not a special case: an + artifact carrying no `packages` key makes `resolveArtifactPackageOrder` + return the caller's own object as the single package body, so that branch + reads `permissions` from exactly where the old code read it. +- **A malformed `packages` is refused, not skipped.** A non-array `packages`, + an entry inlined instead of wrapped under `manifest:`, or a duplicate package + id raises the same ADR-0112 envelope (`code` + `status: 422`) the manifest + service raises when it registers that artifact. Catching it would resolve a + permission surface out of an artifact the loader refuses to load. + +Every boot path that already funnelled through this one function picks the fix +up unchanged: `objectstack serve`'s artifact and from-source paths, and +`@objectstack/verify`'s `bootStack` / RLS harness. diff --git a/packages/cli/test/option-b-reader-acceptance.pin.test.ts b/packages/cli/test/option-b-reader-acceptance.pin.test.ts index 62ae5777d1..b46213586e 100644 --- a/packages/cli/test/option-b-reader-acceptance.pin.test.ts +++ b/packages/cli/test/option-b-reader-acceptance.pin.test.ts @@ -143,7 +143,6 @@ const OPTION_B_LOSSES: readonly string[] = [ 'B2 · AppPlugin ql.setDatasourceMapping (object routing) (from source) · datasourceMapping', 'B2 · AppPlugin seed datasets merged (from source) · data', 'B2 · AppPlugin translation loading into the i18n service (from source) · translations', - 'B2 · plugin-security appSecurityPluginOptions over the from-source config (default permission set) · permissions', 'B2 · runtime collectBundleActions over the from-source config · actions + objects[].actions', 'B2 · runtime collectBundleFunctionEntries over the from-source config · functions', 'B2 · runtime collectBundleHooks over the from-source config · hooks', diff --git a/packages/plugins/plugin-security/src/app-default-permission-set.test.ts b/packages/plugins/plugin-security/src/app-default-permission-set.test.ts index e626ee53ae..4142f46557 100644 --- a/packages/plugins/plugin-security/src/app-default-permission-set.test.ts +++ b/packages/plugins/plugin-security/src/app-default-permission-set.test.ts @@ -1,5 +1,11 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. import { describe, it, expect } from 'vitest'; +import { + AssembledPackageBodySchema, + ObjectStackDefinitionSchema, + composeStacks, + defineStack, +} from '@objectstack/spec'; import { appDefaultPermissionSetName, appSecurityPluginOptions } from './app-default-permission-set'; import { SecurityPlugin } from './security-plugin'; @@ -100,3 +106,192 @@ describe('the resolved options reach the constructed plugin (#7001)', () => { .resolves.toBe('member_default'); }); }); + +/** + * [ADR-0130 D4, #15007] The reader resolves `packages[]`. + * + * Reader card 4/4 of the option-B program ruled on #14512. A multi-package + * artifact carries each definition twice today — flattened at the top level and + * again under `packages[]` — and option B removes the flattened copy. Every + * assertion below is about the SAME declaration read out of both shapes, which + * is what "the artifact stays additive while the readers learn" means. + * + * The two shapes are built by the REAL composer (`composeStacks`, the one + * `examples/app-multi-package` uses) rather than hand-written, so a package + * entry that stopped looking the way this file assumes fails here instead of + * passing against a shape the platform never emits. The option-B shape is + * derived from it by stripping the package-owned keys — and that key set is + * read off the two schemas, never transcribed, so a collection family added to + * the stack schema next month is stripped too. + */ +describe('appSecurityPluginOptions over `packages[]` (ADR-0130 D4, #15007)', () => { + const CORE_ID = 'com.example.security.core'; + const ADDON_ID = 'com.example.security.addon'; + const CORE_PROFILE = 'core_member_default'; + const ADDON_PROFILE = 'addon_member_default'; + + const shapeKeys = (schema: unknown): string[] => + Object.keys((schema as { shape: Record }).shape); + + /** Exactly the keys an option-B artifact no longer carries at the top level. */ + const PACKAGE_OWNED_KEYS: readonly string[] = (() => { + const body = new Set(shapeKeys(AssembledPackageBodySchema)); + return shapeKeys(ObjectStackDefinitionSchema).filter((k) => body.has(k)); + })(); + + const permissionSet = (name: string) => ({ + name, + label: name, + isDefault: true, + objects: {}, + }); + + const coreStack = () => + defineStack({ + manifest: { + id: CORE_ID, name: 'Security Probe Core', namespace: 'secprobe', + version: '1.0.0', type: 'app', + }, + permissions: [permissionSet(CORE_PROFILE)], + }); + + /** Declared SECOND in composition order, and depends on the app package. */ + const addonStack = () => + defineStack({ + manifest: { + id: ADDON_ID, name: 'Security Probe Addon', namespace: 'secprobe', + version: '1.0.0', type: 'module', + dependencies: { [CORE_ID]: '^1.0.0' }, + }, + }); + + /** Today's emitted shape: flattened top level PLUS `packages[]`. */ + const additive = () => composeStacks([addonStack(), coreStack()], { manifest: 'preserve' }); + + /** The ruled shape: `packages[]` only. */ + const optionB = () => { + const composed = additive() as unknown as Record; + const owned = new Set(PACKAGE_OWNED_KEYS); + const out: Record = {}; + for (const [key, value] of Object.entries(composed)) if (!owned.has(key)) out[key] = value; + return out; + }; + + it('CONTROL — the additive shape really does carry the flattened copy', () => { + // Without this, the option-B case below could pass because the fixture + // never had a flattened level to lose. + const composed = additive() as unknown as Record; + expect(Array.isArray(composed.permissions)).toBe(true); + expect((composed.permissions as unknown[]).length).toBeGreaterThan(0); + expect((composed.packages as unknown[]).length).toBe(2); + expect(PACKAGE_OWNED_KEYS).toContain('permissions'); + }); + + it('the additive shape answers exactly what it answered before this card', () => { + expect(appSecurityPluginOptions(additive())).toEqual({ fallbackPermissionSet: CORE_PROFILE }); + }); + + it('OPTION B — the flattened level is gone and the packaged declaration is still resolved', () => { + const stripped = optionB(); + expect(stripped.permissions).toBeUndefined(); + expect((stripped.packages as unknown[]).length).toBe(2); + + // The pre-#15007 reader returned `undefined` here — no throw, no log, and + // every member of the app silently down to the platform floor alone. + expect(appSecurityPluginOptions(stripped)).toEqual({ fallbackPermissionSet: CORE_PROFILE }); + }); + + it('the flattened level still answers FIRST when both shapes carry a set', () => { + // The reader half lands while the artifact is still additive, so this + // function must be a superset of the old read and never a replacement: + // whatever the top level said, it still says. + expect( + appSecurityPluginOptions({ + permissions: [permissionSet('flattened_wins')], + packages: [{ manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } }], + }), + ).toEqual({ fallbackPermissionSet: 'flattened_wins' }); + }); + + it('package order is `resolveArtifactPackageOrder`\'s, not the array\'s', () => { + // Both packages declare an `isDefault` set and the DEPENDENT one is listed + // first. "The first isDefault set" has to mean the same thing here as at + // every other artifact reader, so the depended-upon package answers — + // dependency-topological order (ADR-0130 D5), not authoring accident. + expect( + appSecurityPluginOptions({ + packages: [ + { manifest: { id: ADDON_ID, name: 'Addon', version: '1.0.0', type: 'module', dependencies: { [CORE_ID]: '^1.0.0' }, permissions: [permissionSet(ADDON_PROFILE)] } }, + { manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } }, + ], + }), + ).toEqual({ fallbackPermissionSet: CORE_PROFILE }); + + // …and with the dependency edge removed, declared order is what is left. + expect( + appSecurityPluginOptions({ + packages: [ + { manifest: { id: ADDON_ID, name: 'Addon', version: '1.0.0', type: 'module', permissions: [permissionSet(ADDON_PROFILE)] } }, + { manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } }, + ], + }), + ).toEqual({ fallbackPermissionSet: ADDON_PROFILE }); + }); + + it('a package that declares no default does not shadow one that does', () => { + expect( + appSecurityPluginOptions({ + packages: [ + { manifest: { id: ADDON_ID, name: 'Addon', version: '1.0.0', type: 'module', permissions: [{ name: 'addon_read_only', label: 'RO', objects: {} }] } }, + { manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } }, + ], + }), + ).toEqual({ fallbackPermissionSet: CORE_PROFILE }); + }); + + it('an artifact with no `packages` key still reads the top level and NOTHING else', () => { + // D4's second branch hands `resolveArtifactPackageOrder` the caller's own + // object back as the single package body, so this path is the pre-#15007 + // read exactly — including its refusal to look inside the singular + // `manifest` (#7001, pinned above). + expect(appSecurityPluginOptions({ manifest: { permissions: [permissionSet('buried')] } })).toBeUndefined(); + expect(appSecurityPluginOptions({ packages: [] })).toBeUndefined(); + expect(appSecurityPluginOptions({ permissions: [permissionSet('top')] })).toEqual({ fallbackPermissionSet: 'top' }); + }); + + /** + * The gate travels with the read: `resolveArtifactPackageOrder` refuses a + * malformed `packages` with an ADR-0112 envelope, and this reader does not + * catch it. Swallowing it would resolve a permission surface out of an + * artifact the loader refuses to load. + */ + describe('a malformed `packages` is refused, not silently skipped', () => { + const refusalOf = (config: unknown): { code?: string; status?: number; message?: string } => { + try { + appSecurityPluginOptions(config); + return {}; + } catch (e) { + return e as { code?: string; status?: number; message?: string }; + } + }; + + it('`packages` that is not an array', () => { + const err = refusalOf({ packages: 'nope' }); + expect(err.code).toBe('INVALID_ARTIFACT_PACKAGES'); + expect(err.status).toBe(422); + }); + + it('an entry inlined instead of wrapped under `manifest:`', () => { + const err = refusalOf({ packages: [{ id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] }] }); + expect(err.code).toBe('INVALID_ARTIFACT_PACKAGE_ENTRY'); + expect(err.status).toBe(422); + }); + + it('the same package id twice', () => { + const entry = { manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } }; + const err = refusalOf({ packages: [entry, entry] }); + expect(err.code).toBe('DUPLICATE_ARTIFACT_PACKAGE'); + expect(err.status).toBe(422); + }); + }); +}); diff --git a/packages/plugins/plugin-security/src/app-default-permission-set.ts b/packages/plugins/plugin-security/src/app-default-permission-set.ts index 1dd5cde900..727c845520 100644 --- a/packages/plugins/plugin-security/src/app-default-permission-set.ts +++ b/packages/plugins/plugin-security/src/app-default-permission-set.ts @@ -1,5 +1,7 @@ // Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. +import { resolveArtifactPackageOrder } from '@objectstack/core'; + /** * [ADR-0090 D5, #7555] The PLATFORM's own human baseline permission set. * @@ -83,6 +85,83 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde return undefined; } +/** + * [ADR-0130 D4, #15007] Every permission set a stack config DECLARES — from the + * flattened top level, and from `packages[]`. + * + * ## What this exists to stop + * + * A multi-package artifact carries each definition twice today: flattened onto + * the top level, and again inside `packages[i]`. Option B (the ADR-0130 D4 + * ruling on #14512) removes the flattened copy, so `packages[]` carries it + * once. A reader that only ever looked at the top level does not fail when that + * happens — it reads `undefined`, finds no `isDefault` set, and answers "the app + * declared no default profile". + * + * For THIS reader that silence has a security posture attached. The name it + * resolves becomes the `SecurityPlugin`'s `fallbackPermissionSet`, i.e. the + * app's half of every authenticated human's additive baseline + * ({@link composeHumanBaselinePermissionSets}). Losing it does not deny the + * boot and does not log: the deployment simply runs on the platform floor + * alone, and every member of a multi-package app quietly holds less than the + * app declared they should. #7555 measured what that looks like from the + * outside — nav entries served, 403 behind them — and could only measure it + * because someone went looking. + * + * ## Top level FIRST, `packages[]` second — and why that order is the contract + * + * The reader half of the program lands while the artifact is still ADDITIVE, so + * this function has to be a superset of the old read rather than a replacement + * for it: for every artifact the platform emits today the flattened level + * answers first and this returns exactly what it returned before. The + * `packages[]` pass only supplies a set where the top level had none — which is + * precisely the option-B artifact. That is what makes this card revertible on + * its own and safe to land before the emitter half (#14512). + * + * ## The order is `resolveArtifactPackageOrder`'s, not the array's + * + * `appDefaultPermissionSetName` resolves the FIRST `isDefault` set, so with more + * than one package declaring one, "first" has to mean the same thing here as it + * does everywhere else the artifact is read. `resolveArtifactPackageOrder` + * (`@objectstack/core`, ADR-0130 D4+D5, #14643) is the ONE place that turns an + * artifact into its ordered package list — dependency-topological, so a package + * that extends another is read after it regardless of which array slot it + * occupies. ⛔ Do not iterate `config.packages` directly here; a second + * traversal is a second ordering, and the depended-upon package would win or + * lose by authoring accident. + * + * ## Two things it deliberately does NOT do + * + * • It does not look inside the SINGULAR `manifest`. That constraint is + * #7001's and it still holds — the harness must not honour a declaration + * `serve.ts` ignores. Note this is not a special case bolted on: an + * artifact carrying no `packages` key makes `resolveArtifactPackageOrder` + * return the caller's own object as the single package body (D4's second + * branch, D7's compatibility term), so that branch reads `permissions` from + * exactly where the old code read it and nowhere else. + * • It does not catch `resolveArtifactPackageOrder`'s refusals. A malformed + * `packages` (not an array, an unwrapped entry, a duplicate package id) + * raises an ADR-0112 envelope here, the same one the manifest service + * raises when it registers that artifact moments later. Swallowing it would + * resolve a permission surface out of an artifact the loader refuses to + * load — the gate travels with the read. + */ +function declaredPermissionSets(config: unknown): unknown[] { + const sets: unknown[] = []; + + const flattened = (config as { permissions?: unknown } | null | undefined)?.permissions; + if (Array.isArray(flattened)) sets.push(...flattened); + + const packages = (config as { packages?: unknown } | null | undefined)?.packages; + if (packages === undefined || packages === null) return sets; + + for (const body of resolveArtifactPackageOrder(config)) { + const declared = (body as { permissions?: unknown } | null | undefined)?.permissions; + if (Array.isArray(declared)) sets.push(...declared); + } + return sets; +} + /** * [#7001] The `SecurityPlugin` options a stack config implies — ONE resolution * for EVERY boot path. @@ -112,14 +191,13 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde * the result straight through — `new SecurityPlugin(appSecurityPluginOptions(config))` * — and a caller cannot get the undefined case subtly wrong. * - * Reads `config.permissions`, top-level, exactly as `serve.ts` always has. - * Being cleverer here (also looking inside `manifest`) would re-open the gap it - * closes, in the other direction. + * Reads the sets through {@link declaredPermissionSets} — the flattened top + * level `serve.ts` has always read, and, for a multi-package artifact, the + * `packages[]` bodies that carry the same declaration under ADR-0130 D4. */ export function appSecurityPluginOptions( config: unknown, ): { fallbackPermissionSet: string } | undefined { - const permissions = (config as { permissions?: unknown } | null | undefined)?.permissions; - const name = appDefaultPermissionSetName(permissions); + const name = appDefaultPermissionSetName(declaredPermissionSets(config)); return name ? { fallbackPermissionSet: name } : undefined; }