diff --git a/.changeset/17189-app-capability-not-high-privilege.md b/.changeset/17189-app-capability-not-high-privilege.md new file mode 100644 index 0000000000..3b84fef065 --- /dev/null +++ b/.changeset/17189-app-capability-not-high-privilege.md @@ -0,0 +1,81 @@ +--- +'@objectstack/spec': minor +--- + +feat(spec): an app-declared capability token is not a platform system permission at the `everyone` anchor + +`describeHighPrivilegeBits` counted **any** non-empty `systemPermissions` as a +high-privilege bit, so a permission set carrying the capability token its own +app declared could not be bound to the `everyone` audience anchor: + +``` +FROM describeHighPrivilegeBits({ systemPermissions: ['clm_requester.access'] }) + -> 'system permissions' // the app's own navigation gate, refused +TO describeHighPrivilegeBits({ systemPermissions: ['clm_requester.access'] }, + { declaredCapabilities: ['clm_requester.access'] }) + -> null +``` + +One list carries two unlike things: the platform's own powers (`manage_users` +and friends) and a capability a package **declared for itself** (ADR-0066 D1, +entering `sys_capability` with `managed_by: 'package'` + `package_id` +provenance). An app whose navigation gates on its own token therefore could not +ship the set every employee holds — the set's own gate made it unbindable — and +authors were pushed toward declaring no gates at all, the opposite of what +ADR-0066 D1 exists to encourage. + +**The discriminator is provenance, not spelling.** Both predicates +(`describeHighPrivilegeBits`, `describeAnchorForbiddenBits`) take a new optional +`AnchorBindingContext` naming the capability names *this stack declared*; a +token on that list is the app's own gate and is not counted. ⛔ A naming-syntax +rule (dotted ⇒ app token) was considered and rejected: it misjudges in silence +the first dotted platform permission — `setup.access` is one today — and the +first undotted app token. + +**What is still refused**, each pinned in `high-privilege.test.ts`: + +- a platform capability name, **however it is declared** — a package declaring + `manage_users` cannot launder it past the gate (the platform floor); +- any token absent from the declared list, and every token when no list is + passed — omission gets the pre-change verdict, so the narrowing fails closed; +- a mixed set: one unexcused token still refuses the whole set; +- the `guest` tier (ADR-0090 D9), which does not honour the excusal at all — + D5 speaks for authenticated members, and anonymous visitors are not that. + +**No shipped behaviour moves in this release.** Every current caller invokes the +predicates with the old arity, and with no context the code path is identical — +so this release widens the API, not any live anchor binding. The +`@objectstack/plugin-security` boot refusal and the `@objectstack/lint` +`security-anchor-high-privilege` rule pass the declared list in a follow-up, in +the ruled order (protocol first). + +ADR-0090 D5's offending-bit list is revised to match in its own governed PR +(objectstack#17814), per the ruling's 「ADR-0090 修订单独受管 PR」: the offending +bit is a `systemPermissions` entry naming a **platform** system permission. +Both halves are phase ①; ⛔ neither lands without the other following. + +**This is shipped, which is why it carries a changeset rather than +`skip-changeset`.** `@objectstack/spec`'s published `files[]` ships `dist`, and +the new code reaches it. + +Counts below are taken on a **clean full build of this head** — an empty `dist`, +then `pnpm --filter @objectstack/spec build` with both passes (JS and DTS): exit +0, `check-dts-emitted` reporting 34/34 declaration files, and +`dist/.build-input-hash` and `.build-input-hash-dts` both matching `src`. The +build state is named because it changes the answer: on a JS-only `dist` — one +still mid-DTS, or built under `OS_SKIP_DTS` — every declaration file is missing +and each count below that reaches one is halved. + +| identifier | built files | where | +|---|---|---| +| `declaredCapabilities` | **4** | `security/index.js`, `index.mjs`, `index.d.ts`, `index.d.mts` | +| `AnchorBindingContext` | **2** | `index.d.ts`, `index.d.mts` — a type, so the declarations are its whole published reach | +| `appDeclaredCapabilityNames` | **2** | `index.js`, `index.mjs` — module-private, so it has no declaration presence at all | +| `describeHighPrivilegeBits` | **4** | the positive control: a symbol already known to ship | + +Negative control: a sentence occurring **only** in the ADR revision — `As first +written, the bullet above made` — occurs in **0** built files, and `docs/adr/**` +is in no package's `files[]`. ⚠️ The control has to be a sentence the source +does not also carry: `The platform floor is absolute` reads 2, not 0, because +that sentence is in this predicate's JSDoc as well as in the ADR, and an emitted +JSDoc reaches `index.d.ts` / `index.d.mts` like any other declaration text. diff --git a/packages/spec/api-surface/security.json b/packages/spec/api-surface/security.json index 7c636172c0..33d72aa1fd 100644 --- a/packages/spec/api-surface/security.json +++ b/packages/spec/api-surface/security.json @@ -10,6 +10,7 @@ "AdminScope (type)", "AdminScopeParsed (type)", "AdminScopeSchema (const)", + "AnchorBindingContext (interface)", "AuthzPosture (type)", "AuthzPostureSchema (const)", "CapabilityDeclaration (type)", diff --git a/packages/spec/export-origins/security.json b/packages/spec/export-origins/security.json index 1a465a67b0..0e039a5554 100644 --- a/packages/spec/export-origins/security.json +++ b/packages/spec/export-origins/security.json @@ -10,6 +10,7 @@ "AdminScope": "src/security/permission.zod.ts#AdminScope (type)", "AdminScopeParsed": "src/security/permission.zod.ts#AdminScopeParsed (type)", "AdminScopeSchema": "src/security/permission.zod.ts#AdminScopeSchema (const)", + "AnchorBindingContext": "src/security/high-privilege.ts#AnchorBindingContext (interface)", "AuthzPosture": "src/security/explain.zod.ts#AuthzPosture (type)", "AuthzPostureSchema": "src/security/explain.zod.ts#AuthzPostureSchema (const)", "CapabilityDeclaration": "src/security/capabilities.ts#CapabilityDeclaration (type)", diff --git a/packages/spec/src/security/high-privilege.test.ts b/packages/spec/src/security/high-privilege.test.ts new file mode 100644 index 0000000000..a2f3c796eb --- /dev/null +++ b/packages/spec/src/security/high-privilege.test.ts @@ -0,0 +1,164 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#17189] `systemPermissions` carries two unlike kinds of token, and only one + * of them may block an `everyone` anchor binding (ADR-0090 D5). + * + * Every acceptance below is paired with the SAME definition judged without the + * declaration list, which refuses — so no pin here can pass because the + * predicate went blind. The platform floor is pinned against + * `PLATFORM_CAPABILITY_NAMES` itself rather than a transcribed name, so a + * renamed platform capability cannot leave the floor testing nothing. + */ + +import { describe, it, expect } from 'vitest'; + +import { PLATFORM_CAPABILITY_NAMES } from './capabilities'; +import { describeHighPrivilegeBits, describeAnchorForbiddenBits } from './high-privilege'; + +/** The app token from the filing consumer: declared by the app, gates its own nav. */ +const APP_TOKEN = 'clm_requester.access'; +/** A platform system permission — the other kind. */ +const PLATFORM_TOKEN = 'manage_users'; +/** + * A platform capability that is spelled like an app token (dotted). It is the + * reason the discriminator may not be a spelling rule: a name-shape heuristic + * would hand this one to `everyone`. + */ +const DOTTED_PLATFORM_TOKEN = 'setup.access'; + +describe('describeHighPrivilegeBits — app-declared capability vs platform system permission (#17189)', () => { + it('pins the platform names this suite reasons about (else the floor tests nothing)', () => { + expect(PLATFORM_CAPABILITY_NAMES.has(PLATFORM_TOKEN)).toBe(true); + expect(PLATFORM_CAPABILITY_NAMES.has(DOTTED_PLATFORM_TOKEN)).toBe(true); + // The app token must NOT be a platform capability, or "newly accepted" + // below would be measuring the floor instead of the excusal. + expect(PLATFORM_CAPABILITY_NAMES.has(APP_TOKEN)).toBe(false); + }); + + // ---- STILL REFUSED: the pre-#17189 verdict is the default ---- + + it('still refuses an app token when no declaration list is supplied', () => { + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN] })).toMatch(/system permissions/); + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN] }, {})).toMatch(/system permissions/); + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN] }, { declaredCapabilities: [] })) + .toMatch(/system permissions/); + }); + + it('still refuses a token this stack did not declare', () => { + expect( + describeHighPrivilegeBits({ systemPermissions: ['other_app.access'] }, { declaredCapabilities: [APP_TOKEN] }), + ).toMatch(/system permissions/); + }); + + // ---- NEWLY ACCEPTED: a token the stack declared is the app's own gate ---- + + it('accepts a set whose only system permission is a capability this stack declared', () => { + expect( + describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN] }, { declaredCapabilities: [APP_TOKEN] }), + ).toBeNull(); + }); + + it('accepts declaration/registry rows as well as bare names', () => { + expect( + describeHighPrivilegeBits( + { systemPermissions: [APP_TOKEN] }, + { declaredCapabilities: [{ name: APP_TOKEN, scope: 'org' }] }, + ), + ).toBeNull(); + // An entry carrying no usable name excuses nothing. + expect( + describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN] }, { declaredCapabilities: [{}, { name: 42 }] }), + ).toMatch(/system permissions/); + }); + + it('accepts the `sys_permission_set` JSON-string column shape too', () => { + const row = { system_permissions: JSON.stringify([APP_TOKEN]) }; + expect(describeHighPrivilegeBits(row)).toMatch(/system permissions/); + expect(describeHighPrivilegeBits(row, { declaredCapabilities: [APP_TOKEN] })).toBeNull(); + }); + + it('accepts the filing consumer’s real shape: read grants plus its own nav token', () => { + const clmRequester = { + isDefault: true, + objects: { contract: { allowRead: true, allowCreate: true, allowEdit: true } }, + systemPermissions: [APP_TOKEN], + }; + expect(describeHighPrivilegeBits(clmRequester)).toMatch(/system permissions/); + expect(describeHighPrivilegeBits(clmRequester, { declaredCapabilities: [APP_TOKEN] })).toBeNull(); + }); + + // ---- THE PLATFORM FLOOR: declaring a platform name laundered nothing ---- + + it('still refuses a platform system permission even when a package declares that name', () => { + expect( + describeHighPrivilegeBits({ systemPermissions: [PLATFORM_TOKEN] }, { declaredCapabilities: [PLATFORM_TOKEN] }), + ).toMatch(/system permissions/); + }); + + it('is provenance, not spelling: a DOTTED platform capability is still refused when declared', () => { + expect( + describeHighPrivilegeBits( + { systemPermissions: [DOTTED_PLATFORM_TOKEN] }, + { declaredCapabilities: [DOTTED_PLATFORM_TOKEN] }, + ), + ).toMatch(/system permissions/); + }); + + it('still refuses a mixed set — one unexcused token is enough', () => { + expect( + describeHighPrivilegeBits( + { systemPermissions: [APP_TOKEN, PLATFORM_TOKEN] }, + { declaredCapabilities: [APP_TOKEN, PLATFORM_TOKEN] }, + ), + ).toMatch(/system permissions/); + }); + + it('never excuses a non-string entry, however the list is declared', () => { + expect( + describeHighPrivilegeBits( + { systemPermissions: [{ name: APP_TOKEN }] }, + { declaredCapabilities: [APP_TOKEN, { name: APP_TOKEN }] }, + ), + ).toMatch(/system permissions/); + }); + + // ---- THE REST OF THE D5 LIST IS UNTOUCHED ---- + + it('leaves every other D5 bit refusing, declaration list or not', () => { + const ctx = { declaredCapabilities: [APP_TOKEN] }; + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN], objects: { a: { viewAllRecords: true } } }, ctx)) + .toMatch(/View\/Modify All/); + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN], objects: { a: { modifyAllRecords: true } } }, ctx)) + .toMatch(/View\/Modify All/); + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN], objects: { a: { allowDelete: true } } }, ctx)) + .toMatch(/delete\/purge\/transfer/); + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN], objects: { a: { allowTransfer: true } } }, ctx)) + .toMatch(/delete\/purge\/transfer/); + expect(describeHighPrivilegeBits({ systemPermissions: [APP_TOKEN], objects: { a: { allowExport: true } } }, ctx)) + .toMatch(/bulk export/); + }); +}); + +describe('describeAnchorForbiddenBits — the excusal is the `everyone` tier’s alone (ADR-0090 D9)', () => { + const declaredOnly = { systemPermissions: [APP_TOKEN] }; + const ctx = { declaredCapabilities: [APP_TOKEN] }; + + it('lets a declared app token bind to `everyone`', () => { + expect(describeAnchorForbiddenBits(declaredOnly, 'everyone')).toMatch(/system permissions/); + expect(describeAnchorForbiddenBits(declaredOnly, 'everyone', ctx)).toBeNull(); + }); + + it('still refuses it for `guest` — the strictest tier does not honour the D5 excusal', () => { + expect(describeAnchorForbiddenBits(declaredOnly, 'guest', ctx)).toMatch(/system permissions/); + // Lit control: the guest tier CAN return null, so the line above is a + // verdict about the token and not about the tier refusing everything. + expect(describeAnchorForbiddenBits({ objects: { a: { allowRead: true } } }, 'guest')).toBeNull(); + }); + + it('leaves the guest tier’s own rules exactly as they were', () => { + expect(describeAnchorForbiddenBits({ objects: { '*': { allowRead: true } } }, 'guest', ctx)).toMatch(/wildcard/); + expect(describeAnchorForbiddenBits({ objects: { a: { allowEdit: true } } }, 'guest', ctx)).toMatch(/read-only/); + expect(describeAnchorForbiddenBits({ objects: { a: { allowEdit: true } } }, 'everyone', ctx)).toBeNull(); + }); +}); diff --git a/packages/spec/src/security/high-privilege.ts b/packages/spec/src/security/high-privilege.ts index a9ecf2eab1..cf047ce9a1 100644 --- a/packages/spec/src/security/high-privilege.ts +++ b/packages/spec/src/security/high-privilege.ts @@ -14,6 +14,8 @@ * `system_permissions` JSON-string columns) — callers pass whatever they have. */ +import { PLATFORM_CAPABILITY_NAMES } from './capabilities'; + /** Tolerant JSON access: value may be the parsed object or a JSON string column. */ function coerceRecord(v: unknown): Record | undefined { if (typeof v === 'string') { @@ -22,14 +24,67 @@ function coerceRecord(v: unknown): Record | undefined { return v && typeof v === 'object' && !Array.isArray(v) ? (v as Record) : undefined; } +/** + * [#17189, ADR-0066 D1] What the anchor predicates need to know about the + * stack they are judging a set FOR. + * + * The predicates are pure and synchronous — they read one permission-set + * definition and nothing else — so the one fact they cannot discover for + * themselves is which capability names this stack DECLARED. The caller holds + * it: at boot from the `sys_capability` rows carrying `managed_by: 'package'` + * provenance, at authoring time from the stack's own `capabilities` array. + * + * ⛔ Never synthesize this from the set under test. The point of the input is + * that a set cannot vouch for its own tokens; a "declared" list derived from + * `systemPermissions` would excuse every token by construction and turn the + * gate off. + */ +export interface AnchorBindingContext { + /** + * Capability names declared by the packages installed in this stack — plain + * names, or declarations/`sys_capability` rows carrying a `name` (every other + * field on such a row is ignored, so a caller hands over what it already has + * and never transcribes). Omit it (or pass an empty list) and the predicates + * refuse exactly as they did before the input existed. + */ + declaredCapabilities?: Iterable; +} + +/** + * The app-declared names that may be excused, with the platform floor applied. + * + * Returns `undefined` when nothing is excusable, so the caller keeps the + * pre-#17189 code path verbatim rather than filtering against an empty set. + */ +function appDeclaredCapabilityNames( + context: AnchorBindingContext | undefined, +): ReadonlySet | undefined { + const declared = context?.declaredCapabilities; + if (!declared) return undefined; + const names = new Set(); + for (const entry of declared) { + const name = typeof entry === 'string' + ? entry + : (entry && typeof entry === 'object' ? (entry as { name?: unknown }).name : undefined); + if (typeof name !== 'string' || name.length === 0) continue; + // THE PLATFORM FLOOR. A platform capability stays high-privilege no matter + // who declares a capability of that name — otherwise declaring + // `manage_users` would launder it past the anchor gate, and the widening + // would be a bypass rather than a distinction. + if (PLATFORM_CAPABILITY_NAMES.has(name)) continue; + names.add(name); + } + return names.size > 0 ? names : undefined; +} + /** * Does a permission-set definition carry bits too dangerous for an audience * anchor (`everyone` / `guest`)? Returns a human-readable description of the * first offending bit, or `null` when the set is anchor-safe. * - * Offending bits — the ADR-0090 D5 list: any `systemPermissions`, - * View/Modify All Data (VAMA), or delete/purge/transfer on any object, plus - * bulk `export` (#3544). + * Offending bits — the ADR-0090 D5 list: a `systemPermissions` entry naming a + * PLATFORM system permission, View/Modify All Data (VAMA), or + * delete/purge/transfer on any object, plus bulk `export` (#3544). * A plain `'*'` wildcard grant is NOT high-privilege by itself (D5 permits a * read — or read/create/edit-own — baseline to cover all objects; the * platform's own `viewer_readonly` is exactly that shape, and `member_default` @@ -49,14 +104,53 @@ function coerceRecord(v: unknown): Record | undefined { * not a baseline right, and never something an anchor should confer wholesale. * (`member_default` deliberately carries no `allowExport`, so the platform's * own baseline stays anchor-bindable.) + * + * ## [#17189] `systemPermissions` carries TWO unlike kinds of token + * + * One list, two things: the platform's own powers (`manage_users` and friends + * — the curated `PLATFORM_CAPABILITIES`), and a capability an APP declared for + * itself (ADR-0066 D1 `defineCapability`, entering the `sys_capability` + * registry at boot with `managed_by: 'package'` + `package_id` provenance). + * Until `context` existed the predicate saw only a list of NAMES and judged + * both alike, so an app could not ship the "every employee holds this" set its + * own navigation gates on: the set's own token made it unbindable to + * `everyone`, and the app was pushed toward not declaring gates at all. + * + * {@link AnchorBindingContext.declaredCapabilities} supplies the missing half — + * the capability names THIS stack declared. A name on that list is the app's + * own gate and is not counted as a system permission. The discriminator is + * **provenance**, not spelling: ⛔ never infer "app token" from the shape of a + * name (a dotted segment, a prefix). A spelling rule misjudges in silence the + * first platform permission that is dotted or the first app token that is not, + * and it is the CALLER — which can read what this stack declared — that holds + * the fact, never the string. + * + * Two properties keep the widening honest, both fail-CLOSED: + * + * - **The platform floor is absolute.** A name in + * {@link PLATFORM_CAPABILITY_NAMES} is high-privilege however it is + * declared, so an app cannot launder `manage_users` past the anchor gate by + * declaring a capability of that name. + * - **Omission refuses.** With no `context` — or with a token absent from it — + * the verdict is exactly the pre-parameter one: a non-empty + * `systemPermissions` offends. A caller that cannot enumerate the stack's + * declarations errs toward REFUSING a binding, never toward granting one. */ -export function describeHighPrivilegeBits(def: any): string | null { +export function describeHighPrivilegeBits(def: any, context?: AnchorBindingContext): string | null { if (!def || typeof def !== 'object') return null; const sysRaw = def.systemPermissions ?? def.system_permissions; const sys = typeof sysRaw === 'string' ? (() => { try { return JSON.parse(sysRaw); } catch { return undefined; } })() : sysRaw; - if (Array.isArray(sys) && sys.length > 0) return 'system permissions'; + if (Array.isArray(sys) && sys.length > 0) { + const declared = appDeclaredCapabilityNames(context); + // A non-string entry is never excused: only a name can be matched against a + // declaration, so anything else stays on the offending side. + const unexcused = declared + ? sys.filter((token: unknown) => typeof token !== 'string' || !declared.has(token)) + : sys; + if (unexcused.length > 0) return 'system permissions'; + } const objects = coerceRecord(def.objects ?? def.object_permissions); if (objects) { for (const [objName, rawPerm] of Object.entries(objects)) { @@ -90,8 +184,15 @@ export function describeHighPrivilegeBits(def: any): string | null { export function describeAnchorForbiddenBits( def: any, anchor: 'everyone' | 'guest', + context?: AnchorBindingContext, ): string | null { - const high = describeHighPrivilegeBits(def); + // [#17189] The D5 app-capability excusal is the `everyone` tier's alone. D9 + // gives `guest` the STRICTEST tier, and an app token handed to `guest` is + // handed to anonymous visitors — a different act from handing it to the + // authenticated members D5 speaks for, and one this ruling did not decide. + // So the guest tier asks the question with NO context and keeps refusing any + // non-empty `systemPermissions`. + const high = describeHighPrivilegeBits(def, anchor === 'guest' ? undefined : context); if (high) return high; if (anchor !== 'guest') return null; const objects = coerceRecord(def?.objects ?? def?.object_permissions);