fix(plugin-security): the app default permission set resolves from the first level that NAMES one (#15298)

`declaredPermissionSets`' docblock stated a short-circuit the code did not
have: it pushed the flattened top level and then every package body
unconditionally, collecting each permission set twice on today's additive
artifact. The duplication was unobservable at the sole (private) call site,
which takes the first `isDefault` set, so this corrects a false written
contract on a security-path reader rather than a live defect.

The reader now walks the discipline its docblock claims — start from the
expression the card replaced (`appDefaultPermissionSetName(config.permissions)`)
and consult `packages[]` only where it came back `undefined`.

The condition is the resolved NAME, never the `permissions` container:
branching on the container re-creates the silent loss the card removed one
shape further along, since a flattened level that carries sets but marks none
of them `isDefault` is legal and hand-authorable.

`resolveArtifactPackageOrder` is called before the top level is consulted, so
its ADR-0112 refusal of a malformed `packages` stays unconditional instead of
depending on which level happened to answer first.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01UHvF5hyiZjnCyExFnfQB8m

Author: claude

packages/plugins/plugin-security/src/app-default-permission-set.test.ts

@@ -259,6 +259,56 @@ describe('appSecurityPluginOptions over `packages[]` (ADR-0130 D4, #15007)', ()
     expect(appSecurityPluginOptions({ permissions: [permissionSet('top')] })).toEqual({ fallbackPermissionSet: 'top' });
   });
 
+  /**
+   * [#15007 follow-up] "The top level had none" is the resolved NAME coming
+   * back `undefined` — never the `permissions` CONTAINER being absent or empty.
+   *
+   * Branching on the container re-creates the silent loss this card removed,
+   * one shape further along. A flattened level that carries permission sets but
+   * marks none of them `isDefault` is legal today and hand-authorable in any
+   * `objectstack.config.ts`; a container-shaped condition shorts it past the
+   * whole `packages[]` pass and answers `undefined` — nothing thrown, nothing
+   * logged, every member of the app back down to the platform floor alone.
+   */
+  describe('the `packages[]` pass runs wherever the top level named no default', () => {
+    const corePackage = {
+      manifest: {
+        id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app',
+        permissions: [permissionSet(CORE_PROFILE)],
+      },
+    };
+
+    it('an EMPTY flattened array does not short-circuit it', () => {
+      expect(appSecurityPluginOptions({ permissions: [], packages: [corePackage] }))
+        .toEqual({ fallbackPermissionSet: CORE_PROFILE });
+    });
+
+    it('a NON-EMPTY flattened array that marks no default does not either', () => {
+      // A `permissions.length > 0` guard passes the case above and fails this
+      // one — which is the whole reason the condition is the resolved name.
+      expect(
+        appSecurityPluginOptions({
+          permissions: [{ name: 'core_read_only', label: 'Read only', objects: {} }],
+          packages: [corePackage],
+        }),
+      ).toEqual({ fallbackPermissionSet: CORE_PROFILE });
+    });
+
+    it('a `permissions` key that is not an array at all does not either', () => {
+      expect(appSecurityPluginOptions({ permissions: null, packages: [corePackage] }))
+        .toEqual({ fallbackPermissionSet: CORE_PROFILE });
+    });
+
+    it('and once the top level DOES name one, the packages pass cannot change the answer', () => {
+      expect(
+        appSecurityPluginOptions({
+          permissions: [permissionSet('flattened_wins')],
+          packages: [corePackage],
+        }),
+      ).toEqual({ fallbackPermissionSet: 'flattened_wins' });
+    });
+  });
+
   /**
    * The gate travels with the read: `resolveArtifactPackageOrder` refuses a
    * malformed `packages` with an ADR-0112 envelope, and this reader does not
@@ -293,5 +343,21 @@ describe('appSecurityPluginOptions over `packages[]` (ADR-0130 D4, #15007)', ()
       expect(err.code).toBe('DUPLICATE_ARTIFACT_PACKAGE');
       expect(err.status).toBe(422);
     });
+
+    it('…and refused just the same when the flattened top level already named a default', () => {
+      // The package order is resolved BEFORE the top level is consulted, so an
+      // artifact is either loadable or refused independently of which level
+      // happens to answer. Move that resolution below the early return and this
+      // pair turns into a silent accept: a permission surface resolved out of an
+      // artifact the manifest service refuses moments later.
+      const notAnArray = refusalOf({ permissions: [permissionSet('flattened_wins')], packages: 'nope' });
+      expect(notAnArray.code).toBe('INVALID_ARTIFACT_PACKAGES');
+      expect(notAnArray.status).toBe(422);
+
+      const entry = { manifest: { id: CORE_ID, name: 'Core', version: '1.0.0', type: 'app', permissions: [permissionSet(CORE_PROFILE)] } };
+      const duplicate = refusalOf({ permissions: [permissionSet('flattened_wins')], packages: [entry, entry] });
+      expect(duplicate.code).toBe('DUPLICATE_ARTIFACT_PACKAGE');
+      expect(duplicate.status).toBe(422);
+    });
   });
 });

packages/plugins/plugin-security/src/app-default-permission-set.ts

@@ -86,8 +86,9 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde
 }
 
 /**
- * [ADR-0130 D4, #15007] Every permission set a stack config DECLARES — from the
- * flattened top level, and from `packages[]`.
+ * [ADR-0130 D4, #15007] The app-declared default permission-set NAME, resolved
+ * from wherever the artifact carries the declaration — the flattened top
+ * level, or `packages[]`.
  *
  * ## What this exists to stop
  *
@@ -114,52 +115,70 @@ export function appDefaultPermissionSetName(permissions: unknown): string | unde
  * 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).
+ * `packages[]` pass is consulted ONLY where the top level named no default —
+ * 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 condition is the ANSWER, never the container
+ *
+ * "The top level had none" is spelled as `appDefaultPermissionSetName` coming
+ * back `undefined`, and deliberately NOT as the `permissions` array being
+ * absent or empty. Branching on the container re-creates the silent loss this
+ * card exists to remove, one shape further along: a config whose flattened
+ * level carries permission sets but marks none of them `isDefault` — legal
+ * today, and expressible by hand in any `objectstack.config.ts` — would
+ * short-circuit the whole `packages[]` pass and resolve `undefined`, with
+ * nothing thrown and nothing logged. Reading the container also hands back the
+ * `[]`-is-truthy trap for free. The answer is the only condition that cannot
+ * be wrong in either direction, so the answer is what this branches on.
  *
  * ## 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`
+ * The first package body that names a default wins, 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.
+ * ## The package order is resolved BEFORE the top level is consulted
+ *
+ * Reading that line as a misplaced statement is the expected mistake, so: it is
+ * placed there on purpose, and moving it below the early return is a behaviour
+ * change. `resolveArtifactPackageOrder` REFUSES a malformed `packages` (not an
+ * array, an unwrapped entry, a duplicate package id) 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. Resolving the order
+ * first is what keeps that refusal unconditional: an artifact is either
+ * loadable or refused, and which answer this reader gives about it must not
+ * depend on whether its flattened level happened to name a default first.
+ *
+ * ## One thing 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 never reaches the package pass at all, so that branch reads
+ * `permissions` from exactly where the old code read it and nowhere else.
  */
-function declaredPermissionSets(config: unknown): unknown[] {
-  const sets: unknown[] = [];
+function declaredDefaultPermissionSetName(config: unknown): string | undefined {
+  const packages = (config as { packages?: unknown } | null | undefined)?.packages;
+  const bodies =
+    packages === undefined || packages === null ? [] : resolveArtifactPackageOrder(config);
 
   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;
+  const fromFlattened = appDefaultPermissionSetName(flattened);
+  if (fromFlattened !== undefined) return fromFlattened;
 
-  for (const body of resolveArtifactPackageOrder(config)) {
+  for (const body of bodies) {
     const declared = (body as { permissions?: unknown } | null | undefined)?.permissions;
-    if (Array.isArray(declared)) sets.push(...declared);
+    const fromPackage = appDefaultPermissionSetName(declared);
+    if (fromPackage !== undefined) return fromPackage;
   }
-  return sets;
+  return undefined;
 }
 
 /**
@@ -191,13 +210,14 @@ function declaredPermissionSets(config: unknown): unknown[] {
  * the result straight through — `new SecurityPlugin(appSecurityPluginOptions(config))`
  * — and a caller cannot get the undefined case subtly wrong.
  *
- * 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.
+ * Resolves the name through {@link declaredDefaultPermissionSetName} — 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 name = appDefaultPermissionSetName(declaredPermissionSets(config));
+  const name = declaredDefaultPermissionSetName(config);
   return name ? { fallbackPermissionSet: name } : undefined;
 }