fix(spec,objectql)!: a permission set's body packageId that disagrees with the package shipping it is refused at the build and registration doors; both readers read the binding only

.changeset/23312-permission-set-owner-binding-build-and-registration.md

@@ -0,0 +1,38 @@
+---
+'@objectstack/spec': minor
+'@objectstack/objectql': minor
+'@objectstack/plugin-security': minor
+'@objectstack/metadata-protocol': patch
+'@objectstack/cli': patch
+---
+
+fix(spec,objectql)!: a permission set whose body `packageId` disagrees with the package that ships it is refused at `defineStack` / `os validate` / `os build` and at package registration, and every reader takes a set's owner from its binding only (ADR-0086 D3)
+
+Clause-②: yes (narrowing)
+
+<!-- adr-0087: not-required (already-registered permission-set-package-id-binding-mismatch-refused) this change widens that entry, in this diff, to the build and registration doors: same key, same ruling, same remedy, no stored row rewritten -->
+
+**BREAKING** (an accept-set narrowing on a published authoring surface), shipped as `minor` under the launch-window convention for breaking changes.
+
+ADR-0086 D3 declares `PermissionSetSchema.packageId` as the owning package of a package-shipped set, absent for an environment-authored one. The owner every reader uses is the binding: the package that ships the set or that it is saved into, served as `_packageId`. The metadata save, restore and publish doors already refuse a body `packageId` that disagrees with it. Package source and package registration did not: a set whose body named another package was registered under the package that shipped it, with no refusal and no warning, so an author, an AI build above all, never learned that the owner it wrote was not the owner it got.
+
+**What is refused, and where.** One judge, `judgePermissionSetOwnerBinding` (new export of `@objectstack/spec/security`), serves every door:
+
+- **The stack schema** (`ObjectStackDefinitionSchema`), so `defineStack`, `objectstack validate`, `objectstack build`, the lint scorer and an artifact load all refuse a set whose body `packageId` is not the id of the package that ships it: the stack's `manifest.id`, each `packages[]` body's own `id`, and `manifest.id` for a top-level set no body declares. A stack with no `manifest` ships its sets from no package, so any key there is refused, the empty string included. The issue sits at the set's `packageId` (`permissions.0.packageId`); `defineStack` throws `STACK_SCHEMA_INVALID` (422) carrying it.
+- **Package registration** (`ObjectQL.registerApp`, the manifest and a nested plugin, for a manifest no schema parse judged) throws `400 VALIDATION_ERROR` with `issues[]` at `packageId` before the set is filed, the envelope the same seam throws for a view container whose `name` disagrees with its key.
+
+The message prescribes first, names both values, and offers both remedies: drop the key, or ship the set from the package it names. No error code is minted.
+
+**Readers.** `@objectstack/plugin-security`'s packaged-set lock (`classifyPackagedPermissionSet`) and `@objectstack/cli`'s permission-set name-collision report take a set's package from its binding only. A set filed into no package whose body named one is no longer locked as that package's, and a stack with no `manifest` declares no owner for the collision report.
+
+**Unchanged.** A body `packageId` equal to the package that ships the set is accepted, and an absent one is accepted everywhere. A top-level copy of a set a `packages[]` body declares (an artifact built with both halves) is judged on its body, not against the artifact's own id. The metadata save, restore and publish doors answer in the same words as before (`@objectstack/metadata-protocol` now calls the shared judge). Stored `sys_metadata` rows are not rewritten.
+
+## FROM → TO
+
+| door | FROM | TO |
+|:--|:--|:--|
+| `defineStack` / `objectstack validate` / `objectstack build` | `defineStack({ manifest: { id: 'com.acme.crm', … }, permissions: [{ name: 'crm_member', packageId: 'com.acme.other', … }] })` accepted; the set was owned by `com.acme.crm` | refused at `permissions.0.packageId`, `STACK_SCHEMA_INVALID` 422 |
+| `ObjectQL.registerApp` | the same manifest registered the set under `com.acme.crm`, silently | `400 VALIDATION_ERROR`, `issues[]` at `packageId`, nothing filed |
+| an artifact built before this change | loaded, the set owned by the shipping package | refused at load; rebuild it without the key |
+
+**Remedy, in one line.** Drop `packageId` from the permission set in your stack source (the package that ships a set is its owner), or move the set into the stack of the package it names, then build again.

content/docs/references/security/permission.mdx

@@ -117,7 +117,7 @@ const result = AdminScopeSchema.parse(data);
 | **name** | `string` | ✅ | Permission set name, unique per organization (lowercase snake_case) |
 | **label** | `string` | optional | Display label |
 | **description** | `string` | optional | Human-readable description shown in Setup |
-| **packageId** | `string` | optional | [ADR-0086 D3] Owning package id for a package-shipped set (absent = env-authored); the metadata save, restore and publish doors refuse a value that disagrees with the package the set is saved into |
+| **packageId** | `string` | optional | [ADR-0086 D3] Owning package id for a package-shipped set (absent = env-authored); a value that disagrees with the package that ships the set, or that it is saved into, is refused by the stack schema (defineStack, os validate, os build), the package registration, and the metadata save, restore and publish doors |
 | **managedBy** | `Enum<'package' \| 'platform' \| 'user'>` | optional | [ADR-0086 D3] Record provenance: package (upgrade-owned metadata) vs platform/user (env config) |
 | **isDefault** | `boolean` | optional (default: `false`) | [ADR-0090 D5] App baseline for the everyone position: app-level sets are auto-bound at boot (guarded, idempotent); package-level sets become install-time suggestions an admin confirms |
 | **objects** | `Record<string, { allowCreate?: boolean; allowRead?: boolean; allowEdit?: boolean; allowDelete?: boolean; … }>` | ✅ | Entity permissions |

packages/cli/src/utils/permission-set-name-collisions.test.ts

@@ -270,23 +270,47 @@ describe('#18024 — `os build` reports a permission-set name another package in
     expect(await findPermissionSetNameCollisions(parsed)).toEqual([]);
   });
 
-  it('the author-declared `packageId` answers only where no package ships the set', () => {
-    // The seeder owns a set under `_packageId ?? packageId` — registry
-    // provenance first, ADR-0086 D3's author-declared id as the fallback. Inside
-    // an artifact the package IS the registry provenance, so a set carrying a
-    // different `packageId` is still declared by the package that ships it;
-    // reading the key there would attribute a package's own sets to someone
-    // else and invent a collision with itself.
-    const inArtifact = parsedArtifact([ordersStack([ordersSet(SHARED_NAME, 'com.example.elsewhere')])]);
+  it('[ADR-0086 D3] the owner is the package that ships the set, never the body\'s `packageId`', async () => {
+    // A set is owned by the package that registers it. Inside an artifact the
+    // package IS that registration, so a set whose body names another package
+    // is still declared by the package that ships it. The build door refuses
+    // that body (the stack schema), so the artifact is composed unjudged here
+    // (`strict: false`) and read raw: this pins the READER, not the door.
+    const inArtifact = artifact([ordersStack([ordersSet(SHARED_NAME, 'com.example.elsewhere')])], { strict: false });
     expect(collectPermissionSetDeclarations(inArtifact)).toEqual([
       { name: SHARED_NAME, declaredBy: ORDERS_ID },
     ]);
 
-    // With no package around the declaration at all, the key is the only owner
-    // there is — the shape the seeder's fallback exists for.
-    expect(collectPermissionSetDeclarations({
-      permissions: [ordersSet(SHARED_NAME, 'com.example.elsewhere')],
-    })).toEqual([{ name: SHARED_NAME, declaredBy: 'com.example.elsewhere' }]);
+    // With no package around the declaration, the set has NO owner: the body's
+    // key is not a fallback. Two such sets of one name, naming two different
+    // packages in their bodies, are therefore not a collision; read through the
+    // body key, they were reported as one package colliding with another.
+    const unbound = {
+      permissions: [ordersSet(SHARED_NAME, 'com.example.elsewhere'), ordersSet(SHARED_NAME, 'com.example.yonder')],
+    };
+    expect(collectPermissionSetDeclarations(unbound)).toEqual([]);
+    expect(await findPermissionSetNameCollisions(unbound)).toEqual([]);
+
+    // …and the build door is where such a stack stops: the parse the commands
+    // run refuses each body key at its own `packageId`.
+    const parsed = ObjectStackDefinitionSchema.safeParse(unbound);
+    expect(parsed.success).toBe(false);
+    expect(parsed.error!.issues.map((i) => i.path.join('.'))).toEqual(
+      expect.arrayContaining(['permissions.0.packageId', 'permissions.1.packageId']),
+    );
+  });
+
+  it('CONTROL: a manifest-bound stack declares every set under its manifest id, whatever the body says', async () => {
+    const bound = {
+      manifest: { id: CORE_ID },
+      permissions: [coreSet(SHARED_NAME), ordersSet(SHARED_NAME, 'com.example.elsewhere')],
+    };
+    expect(collectPermissionSetDeclarations(bound)).toEqual([
+      { name: SHARED_NAME, declaredBy: CORE_ID },
+      { name: SHARED_NAME, declaredBy: CORE_ID },
+    ]);
+    // One package declaring its own name twice is a re-seed, not a collision.
+    expect(await findPermissionSetNameCollisions(bound)).toEqual([]);
   });
 
   it('a declaration with no resolvable owner is dropped, never reported as a collision', async () => {

packages/cli/src/utils/permission-set-name-collisions.ts

@@ -62,18 +62,22 @@
  * would narrow what `os build` accepts, which neither this card nor that ruling
  * asked for.
  *
- * ## The provenance rule, mirrored from the seeder rather than invented
- *
- * `bootstrapDeclaredPermissions` (retired with ADR-0131 C3 stage 6b-2) owned a
- * set under `ps._packageId ?? ps.packageId` — registry provenance (ADR-0010) first, the
- * author-declared `packageId` (ADR-0086 D3) as fallback. At compile time the
- * artifact's package id IS that registry provenance: `artifactPackages`' id is
- * "the string the runtime registers a package under", so a set inside package
- * P's assembled body is declared by P, and `packageId` on the set is read only
- * where no package ships it. ⛔ The id rule itself is imported, not re-spelled:
- * `artifact-packages.ts` exists because two readers computing "which package is
- * this" slightly differently is how one door comes to judge a different set of
- * packages than the other while both look right.
+ * ## The provenance rule: the binding, and nothing else
+ *
+ * A set is owned by the package that registers it — registry provenance
+ * (ADR-0010), the `_packageId` the registry stamps. At compile time the
+ * artifact's package id IS that provenance: `artifactPackages`' id is "the
+ * string the runtime registers a package under", so a set inside package P's
+ * assembled body is declared by P. ⛔ The author-declared `packageId` on the set
+ * (ADR-0086 D3) is never read as an owner: every reader of a set's owner reads
+ * the binding, and a body key that disagrees with it is refused at the build
+ * door before this module runs (`judgePermissionSetOwnerBinding`,
+ * `@objectstack/spec/security`). The retired seeder
+ * (`bootstrapDeclaredPermissions`, ADR-0131 C3 stage 6b-2) read it as a
+ * fallback; that fallback is not mirrored. ⛔ The id rule itself is imported,
+ * not re-spelled: `artifact-packages.ts` exists because two readers computing
+ * "which package is this" slightly differently is how one door comes to judge a
+ * different set of packages than the other while both look right.
  *
  * ⚠️ Two bounds, stated because they are where a reading could be wrong rather
  * than merely narrow:
@@ -111,7 +115,7 @@ export interface CompiledPackage {
 export interface DeclaredPermissionSet {
   /** The declared `sys_permission_set.name`. */
   readonly name: string;
-  /** The package that would own the row — the seeder's `_packageId ?? packageId`. */
+  /** The package that would own the set — the package that registers it, never the body's `packageId`. */
   readonly declaredBy: string;
 }
 
@@ -151,10 +155,12 @@ export function collectPermissionSetDeclarations(
     return out;
   }
 
-  // A single-package stack. Its manifest id is the registry provenance the
-  // seeder prefers; `packageId` on the set answers only when there is none.
+  // A single-package stack. Its manifest id is the registry provenance; a stack
+  // with none registers its sets into no package, so they have no owner and are
+  // never a collision (the seeder's `if (!packageId)` arm). ⛔ The body's
+  // `packageId` is not a fallback owner (ADR-0086 D3, the binding only).
   const manifestId = asId(asRec(parsed.manifest)?.id) ?? asId(asRec(parsed.manifest)?.name);
-  add(parsed.permissions, (ps) => manifestId ?? asId(ps.packageId));
+  add(parsed.permissions, () => manifestId);
   return out;
 }
 

packages/metadata-protocol/src/protocol.ts

@@ -9,7 +9,7 @@ import { declaredUserMessage, resolveTenancyPosture, resolveThrownHttpError } fr
 // `resolveMultiOrgEnabled()` is DEMOTED and its own doc comment says answering
 // this question with it is a bug (cloud#1020, #5233) — so the posture, and only
 // the posture, is what the runtime authoring gate is told.
-import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security';
+import { judgePermissionSetOwnerBinding, postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/security';
 // [commit 376c70f98] The derived `version` this file's `getDiscovery()` serves as the
 // `DiscoverySchema` "System Identity" field — never a literal again.
 import { resolveDiscoveryVersion } from './discovery-version.js';
@@ -956,34 +956,22 @@ function runtimeHookWithoutBodyRefusal(
  * {@link ObjectStackProtocolImplementation.publishPackageDrafts} share — the
  * three doors `savedItemNameRefusal` judges a body's `name` at.
  *
- * What disagrees, judged on the body as authored:
- *
- * | body `packageId` | the row's binding | verdict |
- * |:--|:--|:--|
- * | absent | anything | accepted: an absent key declares nothing |
- * | `'P'` | `'P'` | accepted, as before |
- * | `'Q'` | `'P'` | refused |
- * | any string, `''` included | none | refused: a present key declares a package-shipped set, and the set is environment-authored |
- *
- * A value that is not a string is the schema's to refuse (`z.string()`), with
- * its own located `422`. `binding` is the RESOLVED binding of the row the
- * write lands in, never the request's spelling: a draft save that names no
- * package lands in the package of the item's active row (#11087), a direct
- * save that names none lands in the row its address resolves, and a restore
- * writes into the item's active row, whose binding `put` keeps. An empty-string
- * binding is no package, the normalisation both hydration seams apply.
+ * What disagrees, and the words, are the one judge every door of this rule
+ * calls: `judgePermissionSetOwnerBinding` (`@objectstack/spec/security`, whose
+ * header holds the verdict table), which the stack schema (the build door) and
+ * `ObjectQL.registerApp` (the registration door) call too. This function is the
+ * write doors' envelope around it, and its words for these three doors are byte
+ * for byte the words this function built before the judge moved. `binding` is
+ * the RESOLVED binding of the row the write lands in, never the request's
+ * spelling: a draft save that names no package lands in the package of the
+ * item's active row (#11087), a direct save that names none lands in the row
+ * its address resolves, and a restore writes into the item's active row, whose
+ * binding `put` keeps.
  *
  * `VALIDATION_ERROR` / 400, the envelope of the door's other body-against-
  * address refusal: `savedItemNameRefusal`, which judges the body's `name`
  * against the name the row is written under, at the same doors. No code is
- * minted. The message prescribes first, because a 4xx message crosses the REST
- * boundary bounded at 500 characters with its TAIL truncated, names both
- * values, and offers the two remedies the ruling names: drop the key, or write
- * the set into the package it declares. A restore writes a stored version its
- * caller cannot edit in place, so its remedy is the save that fixes it: the
- * item saved again with the key dropped or set to the binding, instead of
- * restoring the version (the name judge's restore wording). `issues[]` locates
- * the key, the shape the Studio designer keys on.
+ * minted. `issues[]` locates the key, the shape the Studio designer keys on.
  *
  * Every writer through the save door is judged, the two server-stated
  * re-savers included (`migrateStoredMetadata`, which records the refusal as
@@ -999,40 +987,12 @@ function permissionSetOwnerBindingRefusal(
     door: 'save' | 'restore' | 'publish',
 ): (Error & { code: 'VALIDATION_ERROR'; status: 400; issues: MetadataIssueEntry[] }) | undefined {
     if (singularType !== 'permission') return undefined;
-    if (!item || typeof item !== 'object' || Array.isArray(item)) return undefined;
-    const declared = (item as { packageId?: unknown }).packageId;
-    if (typeof declared !== 'string') return undefined;
-    const bound = typeof binding === 'string' && binding !== '' ? binding : null;
-    if (declared === bound) return undefined;
-    const into = (pkg: string | null) => (pkg === null ? 'into no package' : `into package '${pkg}'`);
-    const intoDeclared = declared === ''
-        ? ''
-        : `, or ${door === 'save' ? 'save the set' : 'save it'} into package '${declared}' `
-            + `(?package=${declared} on the REST door, packageId in process)`;
-    const owner = bound === null
-        ? 'a set written into no package is authored in this environment, which an absent `packageId` states'
-        : 'the package a set is written into is its owner';
-    const tail = `and ${owner} (ADR-0086 D3): a \`packageId\` that disagrees is refused, never silently overridden.`;
-    const message = door === 'save'
-        ? `Drop \`packageId\` from permission set '${name}'${intoDeclared}, then send it again. Nothing was written. `
-            + `Its body declares \`packageId\` '${declared}', but this save writes it ${into(bound)}, ${tail}`
-        : door === 'restore'
-        ? `Save permission set '${name}' again with \`packageId\` ${bound === null ? 'dropped' : `dropped or set to '${bound}'`}, `
-            + 'instead of restoring this version. Nothing was restored. '
-            + `The version's body declares \`packageId\` '${declared}', but it is restored ${into(bound)}, ${tail}`
-        : `Save the draft of permission set '${name}' again with \`packageId\` dropped${intoDeclared}, then publish it. `
-            + `Nothing was published. Its body declares \`packageId\` '${declared}', but it is published ${into(bound)}, ${tail}`;
-    const err = new Error(message) as Error & { code: 'VALIDATION_ERROR'; status: 400; issues: MetadataIssueEntry[] };
+    const refusal = judgePermissionSetOwnerBinding(item, name, binding, door);
+    if (!refusal) return undefined;
+    const err = new Error(refusal.message) as Error & { code: 'VALIDATION_ERROR'; status: 400; issues: MetadataIssueEntry[] };
     err.code = 'VALIDATION_ERROR';
     err.status = 400;
-    err.issues = [{
-        path: 'packageId',
-        message: `Disagrees with the package the set is written into (${bound === null ? 'none' : `'${bound}'`}): `
-            + (door === 'restore'
-                ? `drop it${bound === null ? '' : `, or set it to '${bound}'`}.`
-                : `drop it${declared === '' ? '' : `, or write the set into package '${declared}'`}.`),
-        code: 'custom',
-    }];
+    err.issues = [{ path: 'packageId', message: refusal.issueMessage, code: 'custom' }];
     return err;
 }