From 1276b4e077a01f4e160087e610f65b5fed8eaa07 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 00:58:57 +0000 Subject: [PATCH 1/4] fix(spec,objectql,metadata-protocol)!: a permission set's body packageId is judged against its binding at the build and registration doors, by one judge Moves the owner-binding judge (comparison and words) into @objectstack/spec/security, runs it from the stack schema (build door) and ObjectQL.registerApp (registration door), and makes the plugin-security lock reader and the cli collision reader read the binding only. Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- .../utils/permission-set-name-collisions.ts | 38 +++-- packages/metadata-protocol/src/protocol.ts | 74 ++------- packages/objectql/src/engine.ts | 48 ++++++ .../src/packaged-permission-set-lock.ts | 2 +- ...set-package-id-binding-mismatch-refused.ts | 45 ++++-- packages/spec/src/security/index.ts | 1 + .../security/permission-set-owner-binding.ts | 148 ++++++++++++++++++ packages/spec/src/security/permission.zod.ts | 17 +- packages/spec/src/stack.zod.ts | 64 +++++++- 9 files changed, 339 insertions(+), 98 deletions(-) create mode 100644 packages/spec/src/security/permission-set-owner-binding.ts diff --git a/packages/cli/src/utils/permission-set-name-collisions.ts b/packages/cli/src/utils/permission-set-name-collisions.ts index eab626f6252..8de6c91be26 100644 --- a/packages/cli/src/utils/permission-set-name-collisions.ts +++ b/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; } diff --git a/packages/metadata-protocol/src/protocol.ts b/packages/metadata-protocol/src/protocol.ts index 28e4be99988..c8d9a11a151 100644 --- a/packages/metadata-protocol/src/protocol.ts +++ b/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; } diff --git a/packages/objectql/src/engine.ts b/packages/objectql/src/engine.ts index 42b13d8193a..a8fde20ca13 100644 --- a/packages/objectql/src/engine.ts +++ b/packages/objectql/src/engine.ts @@ -221,6 +221,7 @@ import { type TenancyPosture, type TenantLayer0Verdict, postureUsesUnionScope, + judgePermissionSetOwnerBinding, } from '@objectstack/spec/security'; /** @@ -4140,6 +4141,42 @@ function stampPackage(item: T, packageId: string, packageVersion: string | un return item; } +/** + * [ADR-0086 D3] The registration door's refusal of a permission set whose body + * `packageId` disagrees with the package {@link ObjectQL.registerApp} registers + * it under (`ownerId`; none for a manifest that carries no id). + * + * The judgement and its words are the one judge every door of the rule calls, + * `judgePermissionSetOwnerBinding` (`@objectstack/spec/security`, whose header + * holds the verdict table): the stack schema runs it at the build door + * (`defineStack`, `os validate`, `os build`, the artifact door's parse), so this + * seam is the door for a manifest no schema parse judged — a programmatic + * `registerApp`, a nested plugin, a `strict: false` stack, a package installed + * from a stored manifest. ⛔ Never a second comparison here. + * + * `VALIDATION_ERROR` / 400, thrown before the set is filed: the envelope of + * this seam's other body-against-key refusal, the divergent view-container + * `name` (`viewContainerNameRefusal`), which the same loop throws one statement + * earlier. `issues[]` locates the key, as the metadata write doors' refusal of + * the same rule does. No code is minted. + */ +function permissionSetOwnerRegistrationRefusal( + item: unknown, + name: string, + ownerId: string | undefined, +): (Error & { code: 'VALIDATION_ERROR'; status: 400; httpStatus: 400; issues: Array<{ path: string; message: string; code: string }> }) | undefined { + const refusal = judgePermissionSetOwnerBinding(item, name, ownerId, 'register'); + if (!refusal) return undefined; + const err = new Error(refusal.message) as Error & { + code: 'VALIDATION_ERROR'; status: 400; httpStatus: 400; issues: Array<{ path: string; message: string; code: string }>; + }; + err.code = 'VALIDATION_ERROR'; + err.status = 400; + err.httpStatus = 400; + err.issues = [{ path: 'packageId', message: refusal.issueMessage, code: 'custom' }]; + return err; +} + /** * {@link stampPackage} for an authored OBJECT definition, on a shallow copy. * The registry stamps an object schema on its own copy (`applySystemFields` @@ -8047,6 +8084,17 @@ export class ObjectQL implements IObjectQLEngine { const refusal = viewContainerNameRefusal(item, sourceLabel, ownerId); if (refusal) throw refusal; } + // [ADR-0086 D3] A permission set whose body `packageId` disagrees + // with the package this seam registers it under is refused, in + // the same place and envelope as the container refusal above: + // `registerItem` stamps `ownerId` as the set's `_packageId`, the + // owner every reader reads, so the body's key used to be + // registered over in silence. See + // {@link permissionSetOwnerRegistrationRefusal}. + if (key === 'permissions') { + const refusal = permissionSetOwnerRegistrationRefusal(item, itemName, ownerId); + if (refusal) throw refusal; + } const toRegister = item.name === itemName ? item : { ...item, name: itemName }; // [#5320] The `views:` tighten — containers ONLY, the contract the // stack schema has always declared (`stack.zod.ts`, diff --git a/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts b/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts index b433e436bbf..e31b226f69d 100644 --- a/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts +++ b/packages/plugins/plugin-security/src/packaged-permission-set-lock.ts @@ -175,7 +175,7 @@ function declaredPackageIdOf(item: any, name: string): string | null { // A stored (tenant-authored) row is never a shipped artifact, whatever // package it is bound to. See the doc comment. if (isTenantAuthored(item)) return null; - const packageId = item._packageId ?? item.packageId; + const packageId = item._packageId; // [ADR-0086 D3] the binding only: never the body's `packageId` if (typeof packageId !== 'string' || packageId === '') return null; if (packageId === RUNTIME_SHADOW_PACKAGE_ID) return null; return packageId; diff --git a/packages/spec/src/migrations/entries/semantic/18.permission-set-package-id-binding-mismatch-refused.ts b/packages/spec/src/migrations/entries/semantic/18.permission-set-package-id-binding-mismatch-refused.ts index a65da6b251f..d9d15791031 100644 --- a/packages/spec/src/migrations/entries/semantic/18.permission-set-package-id-binding-mismatch-refused.ts +++ b/packages/spec/src/migrations/entries/semantic/18.permission-set-package-id-binding-mismatch-refused.ts @@ -3,23 +3,27 @@ import type { SemanticMigration } from '../../types.js'; // A permission set's body `packageId` that disagrees with the package the set -// is saved into is refused at the metadata save, restore and publish doors. Stored rows -// are deliberately not rewritten: every reader already takes the owner from -// the row's binding, so a stored disagreement reads as its binding and is -// refused on its next write. +// is shipped from or saved into is refused at every door that files it under a +// package: the stack schema (defineStack, os validate, os build, an artifact +// load), the package registration (ObjectQL.registerApp), and the metadata save, +// restore and publish doors. Stored rows are deliberately not rewritten: every +// reader already takes the owner from the row's binding, so a stored +// disagreement reads as its binding and is refused on its next write. export const entry: SemanticMigration = { id: 'permission-set-package-id-binding-mismatch-refused', // No backticks in `surface` — build-upgrade-guide.ts renders it inside a // code span already, and a nested backtick would close it. surface: - 'security.permission.packageId (PermissionSetSchema, ADR-0086 D3) saved or published through the ' - + 'metadata API with a value that disagrees with the package the set is saved into: another ' - + 'package\'s id, or any value (the empty string included) on a set saved into no package', + 'security.permission.packageId (PermissionSetSchema, ADR-0086 D3) with a value that disagrees with ' + + 'the package the set is shipped from or saved into: another package\'s id, or any value (the empty ' + + 'string included) on a set that no package ships or that is saved into no package. Judged in ' + + 'package source (the stack schema: defineStack, os validate, os build, an artifact load), at ' + + 'package registration (ObjectQL.registerApp), and at the metadata API\'s save, restore and publish', replacement: - 'no `packageId` in the body: the package the set is saved into is its owner, and every reader of a ' - + 'set\'s owner reads that binding. To make the declared package the owner, save the set into it ' - + '(`?package=` on the REST door, `packageId` in process) instead. A value equal to the package ' - + 'the set is saved into is accepted as before.', + 'no `packageId` in the body: the package that ships the set, or that it is saved into, is its ' + + 'owner, and every reader of a set\'s owner reads that binding. To make the declared package the ' + + 'owner, ship the set from that package\'s stack, or save the set into it (`?package=` on the ' + + 'REST door, `packageId` in process) instead. A value equal to that package is accepted as before.', reason: 'The seat\'s ruling on the card (refuse, not retire; ADR-0086 D3 declares the key as "the owning ' + 'package for a package-shipped set (absent ⇒ env-authored)", and retiring it would amend an ' @@ -31,7 +35,12 @@ export const entry: SemanticMigration = { + 'promotion behind the per-item publish and the package publish now refuse it, ' + '400 VALIDATION_ERROR with issues[] at packageId, naming both values and the two remedies; a ' + 'package publish that meets one aborts the whole batch, as it does on a draft whose name ' - + 'disagrees with its row, and a commit revert reports it in failed[]. The binding judged is the one the write resolves: a draft save that ' + + 'disagrees with its row, and a commit revert reports it in failed[]. Package source and package ' + + 'registration refuse it too, where a set used to be registered under the package that ships it ' + + 'whatever its body said: the stack schema raises an issue at permissions[i].packageId (each ' + + 'packages[] body\'s sets judged against that body\'s id), so defineStack throws STACK_SCHEMA_INVALID ' + + 'and os validate and os build fail, and ObjectQL.registerApp throws 400 VALIDATION_ERROR before the ' + + 'set is filed. The binding judged is the one the write resolves: a draft save that ' + 'names no package lands in the package of the item\'s active row and is judged against it. ' + 'Package duplication copies a set that declares its source package as declaring the target. ' + 'The audience-binding suggestion reader stops reading the body key as a fallback owner for a set ' @@ -57,9 +66,11 @@ export const entry: SemanticMigration = { + 'For a set bound to no package whose body named one, check the audience-binding suggestions in ' + 'Setup: a suggestion that set produced under the named package is no longer offered, and an ' + 'accepted one is withheld from everyone with a warn line naming the set; bind the set to a ' - + 'position directly, or save it into the package it named. Package source (defineStack ' - + 'permissions) is not judged by this entry: an installed package\'s sets are owned by the ' - + 'package that ships them whatever the body says. The repo and example apps carried zero ' - + 'permission sets with a body packageId at the time of the change, so no fixture had to be ' - + 'rewritten.', + + 'position directly, or save it into the package it named. Package source is judged too: search ' + + 'every stack\'s permissions, and each packages[] body\'s, for a body packageId that is not the ' + + 'id of the package that ships it (the stack\'s manifest.id, or the body\'s id; any value on a ' + + 'stack with no manifest), and drop it; os validate names each one at its packageId. A package ' + + 'artifact built before the change that carries one is refused when it is loaded or registered: ' + + 'rebuild it without the key. The repo and example apps carried zero permission sets with a body ' + + 'packageId at the time of the change, so no fixture had to be rewritten.', }; diff --git a/packages/spec/src/security/index.ts b/packages/spec/src/security/index.ts index 544fef7241a..4fa3140b545 100644 --- a/packages/spec/src/security/index.ts +++ b/packages/spec/src/security/index.ts @@ -15,6 +15,7 @@ */ export * from './permission.zod'; +export * from './permission-set-owner-binding'; export * from './permission.form'; export * from './capabilities'; export * from './high-privilege'; diff --git a/packages/spec/src/security/permission-set-owner-binding.ts b/packages/spec/src/security/permission-set-owner-binding.ts new file mode 100644 index 00000000000..59b4d78e55e --- /dev/null +++ b/packages/spec/src/security/permission-set-owner-binding.ts @@ -0,0 +1,148 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0086 D3] The owner-binding judge for a permission set's body + * `packageId`: ONE comparison and ONE set of words, called by every door that + * files a permission set under a package. + * + * ## The rule + * + * D3 declares the key as "the owning package for a package-shipped set + * (absent ⇒ env-authored)", and every reader of a set's owner reads the + * BINDING: the package the set is shipped from or saved into (the registry's + * `_packageId`, a stored row's `package_id`). A body that declared another + * owner used to be accepted and its declaration silently overridden, so an + * author (an AI build above all) was never told that the owner it wrote is not + * the owner it got. The seat's ruling is to refuse it, with a prescription. + * + * | body `packageId` | the binding | verdict | + * |:--|:--|:--| + * | absent | anything | accepted: an absent key declares nothing | + * | `'P'` | `'P'` | accepted | + * | `'Q'` | `'P'` | refused | + * | any string, `''` included | none | refused: a present key declares a package-shipped set, and nothing ships it as one | + * + * A value that is not a string is the schema's to refuse (`z.string()`), with + * its own located issue, so it is not judged here. An empty-string binding is + * no package, the normalisation both hydration seams apply. + * + * ## The doors, and why the judge lives in this package + * + * | door | caller | the binding it passes | + * |:--|:--|:--| + * | `build` | the stack schema's refinement (`ObjectStackDefinitionSchema`, `stack.zod.ts`): `defineStack`, `os validate`, `os build`, the artifact door's parse | the stack's `manifest.id` (none without a manifest); each `packages[]` body's own `id` | + * | `register` | `ObjectQL.registerApp`, for a manifest no schema parse judged (`@objectstack/objectql`) | the package id it registers under | + * | `save` / `restore` / `publish` | the metadata write doors (`@objectstack/metadata-protocol`) | the resolved binding of the row the write lands in | + * + * `@objectstack/spec` is the one layer every one of those callers already + * depends on, the stack schema included, so the judge lives here and nowhere + * else. ⛔ Do not write a second comparison at a door: two doors spelling one + * rule differently is how one comes to accept what the other refuses. + * + * ## The words + * + * Pure: it builds no error and throws nothing. Each door wraps + * {@link PermissionSetOwnerBindingRefusal} in its own envelope: the build door + * as a schema issue at the set's `packageId`, the others as ADR-0112's + * `VALIDATION_ERROR` / 400 with `issues[]` at `packageId`. 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 put the set where it declares. + * A restore writes a stored version its caller cannot edit in place, so its + * remedy is the save that fixes it. The words of the three metadata write doors + * are byte for byte the words those doors printed before the judge moved here. + * Runtime words carry no tracker number. + */ + +/** The door judging the set: where its binding comes from, and what fixes it there. */ +export type PermissionSetOwnerBindingDoor = 'save' | 'restore' | 'publish' | 'build' | 'register'; + +/** A disagreement, in the words every door shows it with. */ +export interface PermissionSetOwnerBindingRefusal { + /** The body's `packageId`, as authored. */ + readonly declared: string; + /** The binding judged against, normalised: `null` is no package. */ + readonly bound: string | null; + /** The refusal sentence: the prescription first, then both values, then the rule. */ + readonly message: string; + /** The located sentence a door's `issues[]` entry at `packageId` carries. */ + readonly issueMessage: string; +} + +/** The binding as the words show it: `null` is no package. */ +const boundOf = (binding: string | null | undefined): string | null => + typeof binding === 'string' && binding !== '' ? binding : null; + +/** + * Judge one permission set's body `packageId` against the binding of the door + * that files it. + * + * @param item The permission set's body, as authored (anything: a non-object + * or a non-string `packageId` is not judged here). + * @param name The set's name, as the words show it. + * @param binding The package the door files the set under: `null`, `undefined` + * or `''` for none. + * @param door Which door is asking (see the module header). + * @returns The refusal when the key disagrees with the binding, else `undefined`. + */ +export function judgePermissionSetOwnerBinding( + item: unknown, + name: string, + binding: string | null | undefined, + door: PermissionSetOwnerBindingDoor, +): PermissionSetOwnerBindingRefusal | 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 = boundOf(binding); + if (declared === bound) return undefined; + const shown = bound === null ? 'none' : `'${bound}'`; + if (door === 'build' || door === 'register') { + const shipDeclared = declared === '' ? '' : `, or ship the set from package '${declared}'`; + const owner = bound === null + ? 'a set no package ships is environment-authored, which an absent `packageId` states' + : 'the package that ships a set is its owner'; + const tail = `and ${owner} (ADR-0086 D3): a \`packageId\` that disagrees is refused, never silently overridden.`; + const message = door === 'build' + ? `Drop \`packageId\` from permission set '${name}'${shipDeclared}` + + `${declared === '' ? '' : ` (the stack whose \`manifest.id\` is '${declared}')`}. ` + + `Its body declares \`packageId\` '${declared}', but this stack ships it ` + + `${bound === null ? 'from no package (the stack declares no `manifest`)' : `from package '${bound}'`}, ${tail}` + : `Drop \`packageId\` from permission set '${name}'${shipDeclared}, then build and install the package again. ` + + `Its body declares \`packageId\` '${declared}', but it is registered ` + + `${bound === null ? 'into no package' : `under package '${bound}'`}, ${tail}`; + return { + declared, + bound, + message, + issueMessage: `Disagrees with the package that ships the set (${shown}): drop it${shipDeclared}.`, + }; + } + 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}`; + return { + declared, + bound, + message, + issueMessage: `Disagrees with the package the set is written into (${shown}): ` + + (door === 'restore' + ? `drop it${bound === null ? '' : `, or set it to '${bound}'`}.` + : `drop it${declared === '' ? '' : `, or write the set into package '${declared}'`}.`), + }; +} diff --git a/packages/spec/src/security/permission.zod.ts b/packages/spec/src/security/permission.zod.ts index 79b64774401..dd2382b0148 100644 --- a/packages/spec/src/security/permission.zod.ts +++ b/packages/spec/src/security/permission.zod.ts @@ -836,13 +836,18 @@ export const PermissionSetSchema = lazySchema(() => strictObject( * what makes the metadata↔config boundary machine-checkable and package * uninstall well-defined (remove the package's own sets). * - * The owner is the package the set is saved into: every reader of a set's - * owner reads that binding. So the metadata save, restore and publish doors - * refuse a body `packageId` that disagrees with it — another package's id, - * or any value on a set saved into no package — rather than accept it and - * override it silently; a value equal to the binding is accepted. + * The owner is the package that ships the set or that it is saved into: + * every reader of a set's owner reads that binding. So every door that files + * a set under a package refuses a body `packageId` that disagrees with it — + * another package's id, or any value on a set no package ships or saves — + * rather than accept it and override it silently; a value equal to the + * binding is accepted. The doors: the stack schema (`defineStack`, + * `os validate`, `os build`, the artifact load), the package registration + * (`ObjectQL.registerApp`), and the metadata save, restore and publish doors. + * One judge serves them all: `judgePermissionSetOwnerBinding` + * (`permission-set-owner-binding.ts`). */ - packageId: z.string().optional().describe('[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: z.string().optional().describe('[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'), /** * [ADR-0086 D3] Per-record provenance on the existing diff --git a/packages/spec/src/stack.zod.ts b/packages/spec/src/stack.zod.ts index 286556d2e38..4d9297f3978 100644 --- a/packages/spec/src/stack.zod.ts +++ b/packages/spec/src/stack.zod.ts @@ -58,6 +58,7 @@ import { JobSchema } from './system/job.zod'; // Security Protocol import { PositionSchema } from './identity/position.zod'; import { PermissionSetSchema } from './security/permission.zod'; +import { judgePermissionSetOwnerBinding } from './security/permission-set-owner-binding'; import { CapabilityDeclarationSchema } from './security/capabilities'; import { SharingRuleSchema } from './security/sharing.zod'; @@ -178,6 +179,67 @@ function applyApiEndpointGates( } } +/** + * [ADR-0086 D3] Raise every permission set whose body `packageId` disagrees + * with the package that ships it as a Zod issue at that key: the BUILD door of + * the owner-binding judge ({@link judgePermissionSetOwnerBinding}, + * `security/permission-set-owner-binding.ts`, whose header holds the rule and + * every other door). A set's owner is the package it is registered under, so a + * key naming another package (or any key, on a set no package ships) was + * registered over in silence; refused here instead, before anything is built. + * + * On the SCHEMA rather than inside `defineStack`, for the reason + * {@link applyApiEndpointGates} gives: `defineStack`, `os validate`, `os build`, + * the lint scorer and the metadata plugin's artifact ingestion all run this one + * parse, so no build or load path can skip the judgement. The binding is the id + * each set is registered under, and nothing else: + * + * - **no `packages[]`**: the stack IS its one package, so every top-level set + * ships from `manifest.id`, and from no package when there is no `manifest`; + * - **`packages[]`** (ADR-0130 D4): a body's sets ship from that body's own + * `id`. A top-level set that no body declares is the stack's residual, + * registered under `manifest.id` (the residual rule is `unclaimedTopLevel` in + * `@objectstack/metadata`; a permission set's slot there is its name). A + * top-level set that a body declares is a copy of that body's set and is never + * registered from the top level, so the top level does not judge it. + */ +function applyPermissionSetOwnerBindingGate( + config: { manifest?: { id?: unknown } | undefined; permissions?: unknown; packages?: unknown }, + ctx: z.RefinementCtx, +): void { + const nameOf = (set: unknown): unknown => (set as { name?: unknown } | null | undefined)?.name; + const judge = (set: unknown, binding: unknown, path: (string | number)[]): void => { + const name = nameOf(set); + const refusal = judgePermissionSetOwnerBinding( + set, + typeof name === 'string' ? name : String(name), + typeof binding === 'string' ? binding : undefined, + 'build', + ); + if (refusal) ctx.addIssue({ code: 'custom', path: [...path, 'packageId'], message: refusal.message }); + }; + const topLevel = Array.isArray(config.permissions) ? config.permissions : []; + if (!Array.isArray(config.packages)) { + topLevel.forEach((set, i) => judge(set, config.manifest?.id, ['permissions', i])); + return; + } + const claimed = new Set(); + config.packages.forEach((entry: unknown, j: number) => { + const body = (entry as { manifest?: { id?: unknown; permissions?: unknown } } | null | undefined)?.manifest; + if (!body || !Array.isArray(body.permissions)) return; + body.permissions.forEach((set: unknown, i: number) => { + const name = nameOf(set); + if (typeof name === 'string') claimed.add(name); + judge(set, body.id, ['packages', j, 'manifest', 'permissions', i]); + }); + }); + topLevel.forEach((set, i) => { + const name = nameOf(set); + if (typeof name === 'string' && claimed.has(name)) return; + judge(set, config.manifest?.id, ['permissions', i]); + }); +} + /** * One package entry as an AUTHOR writes it — manifest only (ADR-0130 D4). * @@ -1768,7 +1830,7 @@ export const ObjectStackDefinitionSchema = lazySchema(() => strictObject({ .describe('Assembled package bodies carried by this release artifact (ADR-0130 D4)'), ...STACK_DEFINITION_COLLECTIONS_SHAPE, -}).superRefine(applyApiEndpointGates)); +}).superRefine(applyApiEndpointGates).superRefine(applyPermissionSetOwnerBindingGate)); export type ObjectStackDefinition = z.input; /** Post-parse shape of {@link ObjectStackDefinition} — defaults applied, transforms run (ADR-0122). */ From 49130777c1f827fc7c4c7a94c77ac6b667474756 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 01:10:30 +0000 Subject: [PATCH 2/4] test(spec,objectql,plugin-security,cli): pin the owner-binding rule at the build and registration doors and both readers Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- .../permission-set-name-collisions.test.ts | 50 +++++-- .../permission-set-owner-registration.test.ts | 88 ++++++++++++ .../src/packaged-permission-set-lock.test.ts | 34 +++++ .../permission-set-owner-binding.test.ts | 78 +++++++++++ ...stack-permission-set-owner-binding.test.ts | 125 ++++++++++++++++++ 5 files changed, 362 insertions(+), 13 deletions(-) create mode 100644 packages/objectql/src/permission-set-owner-registration.test.ts create mode 100644 packages/spec/src/security/permission-set-owner-binding.test.ts create mode 100644 packages/spec/src/stack-permission-set-owner-binding.test.ts diff --git a/packages/cli/src/utils/permission-set-name-collisions.test.ts b/packages/cli/src/utils/permission-set-name-collisions.test.ts index 68cab7f874f..044040ff9e7 100644 --- a/packages/cli/src/utils/permission-set-name-collisions.test.ts +++ b/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 () => { diff --git a/packages/objectql/src/permission-set-owner-registration.test.ts b/packages/objectql/src/permission-set-owner-registration.test.ts new file mode 100644 index 00000000000..b74c6ae97ba --- /dev/null +++ b/packages/objectql/src/permission-set-owner-registration.test.ts @@ -0,0 +1,88 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0086 D3] The REGISTRATION door of the permission-set owner-binding + * rule: `ObjectQL.registerApp` refuses a set whose body `packageId` disagrees + * with the package it registers the set under, instead of filing it there in + * silence. + * + * ## The defect, measured before the fix + * + * A manifest `com.probe.pkg` whose set declared `packageId: 'com.probe.other'` + * registered the set as `{ _packageId: 'com.probe.pkg', packageId: + * 'com.probe.other' }`, with no refusal and no warning line; the same through + * a nested plugin. A manifest with no id registered it with no `_packageId` + * at all, so the body's key was the only owner a fallback reader could find. + * + * The judge is the one every door of the rule calls + * (`judgePermissionSetOwnerBinding`, `@objectstack/spec/security`). This file + * pins the door: the envelope (`VALIDATION_ERROR` / 400, `issues[]` at + * `packageId`, the divergent-container refusal's envelope at the same seam), + * that nothing is filed, and the controls the door must keep accepting. + */ + +import { describe, it, expect } from 'vitest'; +import { ObjectQL } from './engine'; + +const PKG = 'com.probe.pkg'; +const OTHER = 'com.probe.other'; + +const set = (packageId?: string) => ({ + name: 'probe_set', + label: 'Probe', + objects: {}, + ...(packageId === undefined ? {} : { packageId }), +}); + +/** Register once; hand back what was thrown (if anything) and what was filed. */ +function register(manifest: Record): { error: any; filed: any } { + const engine = new ObjectQL(); + let error: any; + try { + engine.registerApp(manifest as any); + } catch (e) { + error = e; + } + const filed = engine.registry.listItems('permission').find((i: any) => i?.name === 'probe_set'); + return { error, filed }; +} + +describe('[ADR-0086 D3] registerApp refuses a permission set whose body `packageId` is not its package', () => { + it('a manifest set naming another package: VALIDATION_ERROR / 400 at `packageId`, and nothing filed', () => { + const { error, filed } = register({ id: PKG, name: 'probe', permissions: [set(OTHER)] }); + expect(error, 'the registration must be refused').toBeInstanceOf(Error); + expect(error).toMatchObject({ code: 'VALIDATION_ERROR', status: 400, httpStatus: 400 }); + expect(error.issues).toEqual([expect.objectContaining({ path: 'packageId', code: 'custom' })]); + expect(error.message.indexOf("Drop `packageId` from permission set 'probe_set'")).toBe(0); + expect(error.message).toContain(`under package '${PKG}'`); + expect(filed, 'refused before the set is filed').toBeUndefined(); + }); + + it('a nested plugin\'s set is judged against the parent package it registers under', () => { + const { error, filed } = register({ id: PKG, name: 'probe', plugins: [{ name: 'nested', permissions: [set(OTHER)] }] }); + expect(error).toMatchObject({ code: 'VALIDATION_ERROR', status: 400, httpStatus: 400 }); + expect(filed).toBeUndefined(); + }); + + it('a manifest with no id registers its sets into no package: any key is refused, the empty string included', () => { + for (const declared of [OTHER, '']) { + const { error, filed } = register({ permissions: [set(declared)] }); + expect(error, `packageId '${declared}'`).toMatchObject({ code: 'VALIDATION_ERROR', status: 400 }); + expect(error.message).toContain('registered into no package'); + expect(filed).toBeUndefined(); + } + }); + + it('CONTROL: a key equal to the package, and an absent key, register under the package as before', () => { + for (const declared of [PKG, undefined]) { + const { error, filed } = register({ id: PKG, name: 'probe', permissions: [set(declared)] }); + expect(error, `packageId ${String(declared)}`).toBeUndefined(); + expect(filed).toMatchObject({ name: 'probe_set', _packageId: PKG }); + } + // An unbound manifest's set with no key is an ordinary unowned set. + const unbound = register({ permissions: [set()] }); + expect(unbound.error).toBeUndefined(); + expect(unbound.filed).toMatchObject({ name: 'probe_set' }); + expect(unbound.filed._packageId).toBeUndefined(); + }); +}); diff --git a/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts b/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts index 5c91a12dd0e..a16a029681e 100644 --- a/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts +++ b/packages/plugins/plugin-security/src/packaged-permission-set-lock.test.ts @@ -746,6 +746,40 @@ describe('[#21789] the lock judges "shipped by a code package" from the row\'s p }); }); +// ───────────────────────────────────────────────────────────────────────────── +// [ADR-0086 D3] The lock reads a set's package from its BINDING only +// ───────────────────────────────────────────────────────────────────────────── + +/** + * A set's owner is the package it is registered under (`_packageId`); its + * body's `packageId` is an author's declaration, refused at every door where it + * disagrees with that binding. The lock used to fall back to the body key when + * the binding was absent, so a set filed into no package whose body named one + * was locked as that package's: the one reader still treating the declaration + * as provenance. + */ +describe('[ADR-0086 D3] the lock never reads a set\'s body `packageId` as its package', () => { + /** Filed with no binding, its body naming a package: the shape the fallback locked. */ + const unboundNamingAPackage = () => orgSet({ name: 'ehr_quality_inspector', packageId: 'com.example.ehr' }); + + it('a registry item with no binding is NOT packaged, whatever its body declares', () => { + expect(classifyPackagedPermissionSet('ehr_quality_inspector', makeQl([unboundNamingAPackage()]))) + .toEqual({ status: 'org' }); + }); + + it('the layered read\'s code layer, the same', () => { + const read = { status: 'read' as const, envelope: { code: unboundNamingAPackage(), overlay: null, effective: null } }; + expect(classifyPackagedPermissionSet('ehr_quality_inspector', makeQl(null), read)).toEqual({ status: 'org' }); + }); + + it('CONTROL: the binding decides, and a body key naming another package does not move it', () => { + expect(classifyPackagedPermissionSet('ehr_quality_inspector', makeQl([packagedSet()]))) + .toEqual({ status: 'packaged', packageId: 'com.example.ehr' }); + expect(classifyPackagedPermissionSet('ehr_quality_inspector', makeQl([packagedSet({ packageId: 'com.example.other' })]))) + .toEqual({ status: 'packaged', packageId: 'com.example.ehr' }); + }); +}); + // ───────────────────────────────────────────────────────────────────────────── // The refusal's guidance reaches the END USER — `userMessage` on the wire // ───────────────────────────────────────────────────────────────────────────── diff --git a/packages/spec/src/security/permission-set-owner-binding.test.ts b/packages/spec/src/security/permission-set-owner-binding.test.ts new file mode 100644 index 00000000000..780b03fc5e8 --- /dev/null +++ b/packages/spec/src/security/permission-set-owner-binding.test.ts @@ -0,0 +1,78 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0086 D3] The owner-binding judge every door of the rule calls + * (`judgePermissionSetOwnerBinding`): its verdict table, and the words of the + * two doors it gained when it moved here from the metadata write doors. + * + * The words of the save, restore and publish doors are pinned where those + * doors are (`protocol.item-name-every-door.test.ts`, `@objectstack/metadata-protocol`), + * through the door itself; this file pins the verdict every door shares and + * the build and registration wording. + */ +import { describe, it, expect } from 'vitest'; +import { judgePermissionSetOwnerBinding, type PermissionSetOwnerBindingDoor } from './permission-set-owner-binding'; + +const DOORS: PermissionSetOwnerBindingDoor[] = ['save', 'restore', 'publish', 'build', 'register']; +const set = (packageId?: unknown) => ({ + name: 'probe_set', + objects: {}, + ...(packageId === undefined ? {} : { packageId }), +}); + +describe('[ADR-0086 D3] the verdict, at every door', () => { + it.each(DOORS)('%s: a body `packageId` that names another package is refused, naming both values', (door) => { + const refusal = judgePermissionSetOwnerBinding(set('com.acme.other'), 'probe_set', 'com.acme.crm', door); + expect(refusal).toMatchObject({ declared: 'com.acme.other', bound: 'com.acme.crm' }); + expect(refusal!.message).toContain("'com.acme.other'"); + expect(refusal!.message).toContain("'com.acme.crm'"); + expect(refusal!.issueMessage).toContain("'com.acme.crm'"); + }); + + it.each(DOORS)('%s: any body `packageId` on a set no package binds is refused, the empty string included', (door) => { + for (const binding of [undefined, null, '']) { + expect(judgePermissionSetOwnerBinding(set('com.acme.other'), 'probe_set', binding, door)).toMatchObject({ + declared: 'com.acme.other', + bound: null, + }); + expect(judgePermissionSetOwnerBinding(set(''), 'probe_set', binding, door)).toMatchObject({ declared: '', bound: null }); + } + }); + + it.each(DOORS)('CONTROL %s: a body `packageId` equal to the binding, and an absent one, are accepted', (door) => { + expect(judgePermissionSetOwnerBinding(set('com.acme.crm'), 'probe_set', 'com.acme.crm', door)).toBeUndefined(); + for (const binding of ['com.acme.crm', undefined, null, '']) { + expect(judgePermissionSetOwnerBinding(set(), 'probe_set', binding, door)).toBeUndefined(); + } + }); + + it('a non-string `packageId` and a non-object body are the schema\'s to refuse, never judged here', () => { + for (const item of [set(1), set(null), null, undefined, 'probe_set', [set('x')]]) { + expect(judgePermissionSetOwnerBinding(item, 'probe_set', 'com.acme.crm', 'build')).toBeUndefined(); + } + }); +}); + +describe('[ADR-0086 D3] the build and registration words prescribe first', () => { + it('build: drop the key, or ship the set from the stack the key names', () => { + const refusal = judgePermissionSetOwnerBinding(set('com.acme.other'), 'probe_set', 'com.acme.crm', 'build')!; + expect(refusal.message.indexOf("Drop `packageId` from permission set 'probe_set', or ship the set from package 'com.acme.other'")).toBe(0); + expect(refusal.message).toContain("this stack ships it from package 'com.acme.crm'"); + }); + + it('build: a stack with no `manifest` ships the set from no package, and the empty key offers only the drop', () => { + const refusal = judgePermissionSetOwnerBinding(set(''), 'probe_set', undefined, 'build')!; + expect(refusal.message.indexOf("Drop `packageId` from permission set 'probe_set'. ")).toBe(0); + expect(refusal.message).toContain('from no package (the stack declares no `manifest`)'); + expect(refusal.issueMessage).toContain('(none)'); + expect(refusal.issueMessage).not.toContain('ship the set from package'); + }); + + it('register: drop the key, or ship the set from the package the key names, then rebuild', () => { + const refusal = judgePermissionSetOwnerBinding(set('com.acme.other'), 'probe_set', 'com.acme.crm', 'register')!; + expect(refusal.message.indexOf("Drop `packageId` from permission set 'probe_set', or ship the set from package 'com.acme.other', then build and install the package again.")).toBe(0); + expect(refusal.message).toContain("it is registered under package 'com.acme.crm'"); + expect(refusal.issueMessage).toContain("('com.acme.crm')"); + expect(refusal.issueMessage).toContain("ship the set from package 'com.acme.other'"); + }); +}); diff --git a/packages/spec/src/stack-permission-set-owner-binding.test.ts b/packages/spec/src/stack-permission-set-owner-binding.test.ts new file mode 100644 index 00000000000..28285895112 --- /dev/null +++ b/packages/spec/src/stack-permission-set-owner-binding.test.ts @@ -0,0 +1,125 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [ADR-0086 D3] The BUILD door of the permission-set owner-binding rule: the + * stack schema refuses a set whose body `packageId` disagrees with the package + * that ships it (`applyPermissionSetOwnerBindingGate`, `stack.zod.ts`). + * + * Before this gate a stack whose set declared another package's id parsed, and + * the registry filed the set under the stack's own package with no word to the + * author. `defineStack`, `os validate`, `os build` and the artifact door all + * run this one parse, so it is pinned here on the parse and through + * `defineStack`, with the envelope each answers: the issue's `code` and path + * at the parse, `STACK_SCHEMA_INVALID` / 422 from `defineStack`. + */ +import { describe, it, expect } from 'vitest'; +import { composeStacks, defineStack, ObjectStackDefinitionSchema } from './stack.zod'; + +type Envelope = Error & { code?: string; status?: number; issues?: Array<{ code?: string; path?: PropertyKey[]; message?: string }> }; + +const CRM = 'com.acme.crm'; +const OTHER = 'com.acme.other'; + +const set = (name: string, packageId?: string) => ({ + name, + label: 'Probe', + objects: {}, + ...(packageId === undefined ? {} : { packageId }), +}); + +const manifest = (id: string, extra: Record = {}) => ({ + id, + name: id, + namespace: 'crm', + version: '1.0.0', + type: 'app' as const, + ...extra, +}); + +function refusal(config: unknown): Envelope | null { + try { + defineStack(config as never); + return null; + } catch (e) { + return e as Envelope; + } +} + +/** The issues the parse raises at a `packageId`, as `[path, message]`. */ +function ownerIssues(config: unknown): Array<[string, string]> { + const result = ObjectStackDefinitionSchema.safeParse(config); + if (result.success) return []; + return result.error.issues + .filter((i) => i.path[i.path.length - 1] === 'packageId') + .map((i) => [i.path.join('.'), i.message]); +} + +describe('[ADR-0086 D3] defineStack refuses a permission set whose body `packageId` is not the stack\'s package', () => { + it('a set naming another package: STACK_SCHEMA_INVALID / 422, the issue at its `packageId`, the prescription first', () => { + const error = refusal({ manifest: manifest(CRM), permissions: [set('crm_member', OTHER)] }); + expect(error, 'the stack must be refused').not.toBeNull(); + expect(error).toMatchObject({ code: 'STACK_SCHEMA_INVALID', status: 422 }); + const issue = error!.issues!.find((i) => i.path?.join('.') === 'permissions.0.packageId'); + expect(issue, 'located at the set\'s key').toMatchObject({ code: 'custom' }); + expect(issue!.message!.indexOf("Drop `packageId` from permission set 'crm_member'")).toBe(0); + expect(issue!.message).toContain(`from package '${CRM}'`); + }); + + it('a stack with no `manifest` ships its sets from no package: any key is refused, the empty string included', () => { + for (const declared of [OTHER, CRM, '']) { + const error = refusal({ permissions: [set('crm_member', declared)] }); + expect(error, `packageId '${declared}'`).toMatchObject({ code: 'STACK_SCHEMA_INVALID', status: 422 }); + expect(error!.issues!.map((i) => i.path?.join('.'))).toContain('permissions.0.packageId'); + } + }); + + it('a map-format collection is judged on the array it normalises to', () => { + const error = refusal({ manifest: manifest(CRM), permissions: { crm_member: { label: 'Probe', objects: {}, packageId: OTHER } } }); + expect(error).toMatchObject({ code: 'STACK_SCHEMA_INVALID', status: 422 }); + expect(error!.issues!.map((i) => i.path?.join('.'))).toContain('permissions.0.packageId'); + }); + + it('CONTROL: a key equal to `manifest.id`, and an absent key with or without a manifest, are accepted', () => { + expect(refusal({ manifest: manifest(CRM), permissions: [set('crm_member', CRM)] })).toBeNull(); + expect(refusal({ manifest: manifest(CRM), permissions: [set('crm_member')] })).toBeNull(); + expect(refusal({ permissions: [set('crm_member')] })).toBeNull(); + }); +}); + +describe('[ADR-0086 D3] a release artifact\'s sets are judged against the package that registers each one', () => { + /** Two packages composed as `os build` does (ADR-0130 D4), the core set's key as the variable. */ + // The artifact's own manifest is the LAST input's (`'preserve'` keeps the + // `'last'` pick), so it is the orders module's, never the package under test. + const twoPackages = (coreKey?: string) => composeStacks([ + defineStack({ manifest: manifest(CRM), permissions: [set('crm_member', coreKey)] } as never, { strict: false }), + defineStack({ manifest: manifest('com.acme.orders', { type: 'module' }), permissions: [set('orders_member')] } as never), + ], { manifest: 'preserve' }) as unknown as Record & { packages: Array<{ manifest: Record }> }; + + it('a body\'s set is judged against that body\'s own id', () => { + const artifact = twoPackages(OTHER); + const at = artifact.packages.findIndex((p) => p.manifest.id === CRM); + expect(ownerIssues(artifact).map(([path]) => path)).toEqual([`packages.${at}.manifest.permissions.0.packageId`]); + // CONTROL: the same artifact, the key naming the body that ships it. + expect(ownerIssues(twoPackages(CRM))).toEqual([]); + expect(ownerIssues(twoPackages())).toEqual([]); + }); + + it('a top-level set no body declares is the residual, judged against the artifact\'s `manifest.id`', () => { + const artifact = { ...twoPackages(), permissions: [set('stray_member', OTHER)] }; + const owner = String((artifact.manifest as { id: string }).id); + expect(ownerIssues(artifact)).toEqual([['permissions.0.packageId', expect.stringContaining(`from package '${owner}'`)]]); + // CONTROL: the residual declaring the artifact's own id. + expect(ownerIssues({ ...twoPackages(), permissions: [set('stray_member', owner)] })).toEqual([]); + }); + + it('CONTROL: a top-level COPY of a body\'s set is not judged against the artifact\'s id (the both-halves artifact shape)', () => { + // An artifact built before #14512 carried every body's collections a second + // time at the top level, under the artifact's own (last-picked) manifest. + // That copy is never registered from the top level, so its key — which + // names the body that ships it — is no disagreement. + const artifact = twoPackages(CRM); + const flattened = { ...artifact, permissions: [set('crm_member', CRM), set('orders_member')] }; + expect(String((flattened.manifest as { id: string }).id)).not.toBe(CRM); + expect(ownerIssues(flattened)).toEqual([]); + }); +}); From 6b048c0ed418cca2eb227ab492391f90a92e2269 Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 01:11:15 +0000 Subject: [PATCH 3/4] chore(changeset): the owner-binding rule at the build and registration doors Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- ...et-owner-binding-build-and-registration.md | 38 +++++++++++++++++++ 1 file changed, 38 insertions(+) create mode 100644 .changeset/23312-permission-set-owner-binding-build-and-registration.md diff --git a/.changeset/23312-permission-set-owner-binding-build-and-registration.md b/.changeset/23312-permission-set-owner-binding-build-and-registration.md new file mode 100644 index 00000000000..9a6d5620ffe --- /dev/null +++ b/.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) + + + +**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. From 9903d68d4e9235755f3d0b2c181facbca658e3be Mon Sep 17 00:00:00 2001 From: Claude Date: Wed, 14 Oct 2026 01:18:03 +0000 Subject: [PATCH 4/4] chore(spec): regenerate api-surface, export-origins and the permission reference for the owner-binding judge Claude-Session: https://claude.ai/code/session_01YRp5z4kK6vgYkrdH97ztsy Co-authored-by: Claude --- content/docs/references/security/permission.mdx | 2 +- packages/spec/api-surface/security.json | 3 +++ packages/spec/export-origins/security.json | 3 +++ .../src/stack-permission-set-owner-binding.test.ts | 10 +++++++--- 4 files changed, 14 insertions(+), 4 deletions(-) diff --git a/content/docs/references/security/permission.mdx b/content/docs/references/security/permission.mdx index 496374fe024..b68bfa9e3ef 100644 --- a/content/docs/references/security/permission.mdx +++ b/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` | ✅ | Entity permissions | diff --git a/packages/spec/api-surface/security.json b/packages/spec/api-surface/security.json index 69c58258d0a..b3c8a5a33bd 100644 --- a/packages/spec/api-surface/security.json +++ b/packages/spec/api-surface/security.json @@ -56,6 +56,8 @@ "PLATFORM_CAPABILITY_NAMES (const)", "PUBLIC_FORM_SERVER_MANAGED_FIELDS (const)", "PermissionSet (type)", + "PermissionSetOwnerBindingDoor (type)", + "PermissionSetOwnerBindingRefusal (interface)", "PermissionSetParsed (type)", "PermissionSetSchema (const)", "PlatformCapability (interface)", @@ -90,6 +92,7 @@ "defineSharingRule (function)", "describeAnchorForbiddenBits (function)", "describeHighPrivilegeBits (function)", + "judgePermissionSetOwnerBinding (function)", "normalizeTenancyPosture (function)", "objectPermissionGrants (function)", "permissionForm (const)", diff --git a/packages/spec/export-origins/security.json b/packages/spec/export-origins/security.json index 5bc04d050cc..4e97ed9a343 100644 --- a/packages/spec/export-origins/security.json +++ b/packages/spec/export-origins/security.json @@ -55,6 +55,8 @@ "PLATFORM_CAPABILITY_NAMES": "src/security/capabilities.ts#PLATFORM_CAPABILITY_NAMES (const)", "PUBLIC_FORM_SERVER_MANAGED_FIELDS": "src/security/public-form.ts#PUBLIC_FORM_SERVER_MANAGED_FIELDS (const)", "PermissionSet": "src/security/permission.zod.ts#PermissionSet (type)", + "PermissionSetOwnerBindingDoor": "src/security/permission-set-owner-binding.ts#PermissionSetOwnerBindingDoor (type)", + "PermissionSetOwnerBindingRefusal": "src/security/permission-set-owner-binding.ts#PermissionSetOwnerBindingRefusal (interface)", "PermissionSetParsed": "src/security/permission.zod.ts#PermissionSetParsed (type)", "PermissionSetSchema": "src/security/permission.zod.ts#PermissionSetSchema (const)", "PlatformCapability": "src/security/capabilities.ts#PlatformCapability (interface)", @@ -85,6 +87,7 @@ "defineSharingRule": "src/security/sharing.zod.ts#defineSharingRule (function)", "describeAnchorForbiddenBits": "src/security/high-privilege.ts#describeAnchorForbiddenBits (function)", "describeHighPrivilegeBits": "src/security/high-privilege.ts#describeHighPrivilegeBits (function)", + "judgePermissionSetOwnerBinding": "src/security/permission-set-owner-binding.ts#judgePermissionSetOwnerBinding (function)", "normalizeTenancyPosture": "src/security/tenancy-posture.ts#normalizeTenancyPosture (function)", "objectPermissionGrants": "src/security/permission.zod.ts#objectPermissionGrants (function)", "permissionForm": "src/security/permission.form.ts#permissionForm (const)", diff --git a/packages/spec/src/stack-permission-set-owner-binding.test.ts b/packages/spec/src/stack-permission-set-owner-binding.test.ts index 28285895112..ca4cc83f2ca 100644 --- a/packages/spec/src/stack-permission-set-owner-binding.test.ts +++ b/packages/spec/src/stack-permission-set-owner-binding.test.ts @@ -93,7 +93,11 @@ describe('[ADR-0086 D3] a release artifact\'s sets are judged against the packag const twoPackages = (coreKey?: string) => composeStacks([ defineStack({ manifest: manifest(CRM), permissions: [set('crm_member', coreKey)] } as never, { strict: false }), defineStack({ manifest: manifest('com.acme.orders', { type: 'module' }), permissions: [set('orders_member')] } as never), - ], { manifest: 'preserve' }) as unknown as Record & { packages: Array<{ manifest: Record }> }; + ], { manifest: 'preserve' }) as unknown as { + manifest: { id: string }; + packages: Array<{ manifest: Record }>; + [key: string]: unknown; + }; it('a body\'s set is judged against that body\'s own id', () => { const artifact = twoPackages(OTHER); @@ -106,7 +110,7 @@ describe('[ADR-0086 D3] a release artifact\'s sets are judged against the packag it('a top-level set no body declares is the residual, judged against the artifact\'s `manifest.id`', () => { const artifact = { ...twoPackages(), permissions: [set('stray_member', OTHER)] }; - const owner = String((artifact.manifest as { id: string }).id); + const owner = artifact.manifest.id; expect(ownerIssues(artifact)).toEqual([['permissions.0.packageId', expect.stringContaining(`from package '${owner}'`)]]); // CONTROL: the residual declaring the artifact's own id. expect(ownerIssues({ ...twoPackages(), permissions: [set('stray_member', owner)] })).toEqual([]); @@ -119,7 +123,7 @@ describe('[ADR-0086 D3] a release artifact\'s sets are judged against the packag // names the body that ships it — is no disagreement. const artifact = twoPackages(CRM); const flattened = { ...artifact, permissions: [set('crm_member', CRM), set('orders_member')] }; - expect(String((flattened.manifest as { id: string }).id)).not.toBe(CRM); + expect(flattened.manifest.id).not.toBe(CRM); expect(ownerIssues(flattened)).toEqual([]); }); });