diff --git a/.changeset/last-admin-standing-keys-gate.md b/.changeset/last-admin-standing-keys-gate.md new file mode 100644 index 0000000000..06108bb469 --- /dev/null +++ b/.changeset/last-admin-standing-keys-gate.md @@ -0,0 +1,59 @@ +--- +"@objectstack/core": minor +"@objectstack/plugin-auth": minor +--- + +feat(security): bind the break-glass standing-key lists to what the authz resolver actually reads — the correspondence stops being prose (#8734) + +`plugin-auth`'s last-administrator guard (ADR-0024 D5.2) decides whether a +pending write can empty the administrator population by testing the payload +against three standing-key lists (`MEMBER_STANDING_KEYS`, +`GRANT_STANDING_KEYS`, `PERMISSION_SET_STANDING_KEYS`). A payload touching none +of them is skipped without any reads — so a column `resolveAuthzContext` starts +reading that a list omits is a write class the guard **silently stops judging**, +on the one path whose failure mode is an installation-wide administrator lockout +with no in-product recovery. + +Nothing bound the two together. The correspondence lived in a comment, and it +had already gone false once: #6084 wrote — naming `active` explicitly — that +everything a permission-set write touches other than `name` is invisible to "who +is an administrator". That was true when written; #8613 made `active` a +resolution-time predicate and the sentence became false. Nothing mechanical +would have caught it, because the guard's own tests stay green precisely when +the guard is never consulted. + +**The mechanism is two links, and the first one is a measurement.** + +- `@objectstack/core` now exports `ADMIN_STANDING_SURFACE` — declared beside the + resolver, listing every table the administrator-derivation path reads, each + classified `derives` or `reads-only` with its reason, and for the deriving + tables every column read. It is asserted **equal** to what the real + `resolveAuthzContext` reads, observed at runtime through a recording engine + that records every property access and every `where` key per table. Observation + rather than source extraction because the reads that matter have moved into + helpers: `active` is read by `isRowActive(row)` and the ADR-0091 window bounds + by `isGrantActive(row, now)`, neither named at the resolver's own call site — + the exact shape #8613 had. + +- `@objectstack/plugin-auth` now exports its standing-key lists plus + `STANDING_KEYS_BY_TABLE` and `STANDING_KEY_EXCLUSIONS`, and a gate requires + every column of that measured surface to have an answer: it is standing-bearing + (in a list) or it is excluded with the reason it cannot empty the administrator + population. There is no third state — the third state is what `active` was + between #6084 and #8613. + +So a resolver change that starts reading a new column fails at the first link +until the declaration is updated, and at the second until the guard has an +explicit answer for it. Landing #8613 green would have required writing down that +deactivating `admin_full_access` cannot empty the administrator population — +which is false, and which is what the old comment asserted by accident. + +**No guard behaviour changes.** Every list keeps exactly the values it had; the +gate is one-directional by construction (it can only ever demand that the guard +judges *more*), because the other direction would put pressure on a break-glass +guard to fire less often. + +The table-level half is covered too: a resolver that started deriving +administrator standing from a **new** table is invisible to any column-set +comparison, since the table is absent from both sides — so the surface enumerates +every table the path reads, and an unclassified one fails. diff --git a/packages/core/src/security/admin-standing-surface.test.ts b/packages/core/src/security/admin-standing-surface.test.ts new file mode 100644 index 0000000000..547e338c77 --- /dev/null +++ b/packages/core/src/security/admin-standing-surface.test.ts @@ -0,0 +1,324 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8734] The FIRST of the two links that bind `plugin-auth`'s break-glass + * standing-key lists to what this resolver actually reads. + * + * This half answers one question mechanically: **which columns does + * `resolveAuthzContext` read on the tables administrator standing is derived + * from?** It answers it by OBSERVATION — the real resolver is driven over a + * recording engine that returns `Proxy`-wrapped rows and records every property + * access, plus every `where` key, per table — and asserts the answer equals + * `ADMIN_STANDING_SURFACE`. + * + * The second link lives in `plugin-auth` + * (`last-admin-standing-keys.test.ts`): it requires every column declared here + * to be either in a standing-key list or explicitly excluded with a reason. So + * a resolver change that starts reading a new column fails HERE until the + * declaration is updated, and fails THERE until the guard has an answer for it. + * + * ## Why observation rather than a static extractor + * + * A source-parsing gate would have to follow `psRowsAll` into `psRows` into the + * `for (const ps of psRows)` loop to learn that `ps.name` is a read of + * `sys_permission_set.name` — real dataflow analysis, brittle in exactly the + * places that matter. Worse, it would have to inline the helpers: `active` is + * never named on the resolver's own call site (`isRowActive(r)` reads it), and + * neither is `valid_from` (`isGrantActive(row, now)` reads it). #8613's whole + * defect was a read that moved into a predicate; a gate that reads the caller + * and not the callee would have missed it for the same reason the comment did. + * + * ## Why the fixtures come in variants + * + * A conditional read is invisible in a fixture that never takes the branch. The + * resolver's tolerated-spelling chains are the sharp case: `r.organization_id ?? + * r.organizationId` never touches the camelCase spelling while the snake_case + * one is non-nullish. So the observation is the UNION over fixtures chosen to + * take both sides of every such chain — snake-only rows, camel-only rows, and a + * pass where the flags and windows are set the other way. `assertVariantsStay- + * Distinct` keeps that honest: if two variants ever observe the same set, one of + * them has stopped contributing and the union has silently narrowed. + */ + +import { describe, it, expect } from 'vitest'; + +import { ADMIN_STANDING_SURFACE, adminStandingTables } from './admin-standing-surface.js'; +import { resolveAuthzContext } from './resolve-authz-context.js'; + +/** table -> every column name the resolver touched on it. */ +type Observation = Map>; + +const camelOf = (key: string): string => key.replace(/_([a-z])/g, (_m, c: string) => c.toUpperCase()); + +/** + * An ObjectQL stand-in that records what the resolver READS. + * + * Two rules keep the recording faithful: + * + * - the `where` match runs against the RAW row, never the proxy, so the fake + * driver's own reads are not mistaken for the resolver's; + * - `where` keys ARE recorded, because filtering on a column is reading it — + * a resolver that started passing `where: { active: true }` would be + * consuming `active` just as surely as `isRowActive` does. + * + * The matcher resolves a column through either spelling so a camelCase-only + * fixture still matches a snake_case `where`; that leniency is the harness's, + * never the resolver's, and it exists so the camel variant can reach the same + * code path rather than returning nothing. + */ +function makeRecordingQl(tables: Record>>, seen: Observation) { + const note = (table: string, column: string): void => { + let cols = seen.get(table); + if (!cols) { + cols = new Set(); + seen.set(table, cols); + } + cols.add(column); + }; + const raw = (row: Record, key: string): unknown => + key in row ? row[key] : row[camelOf(key)]; + + return { + async find(object: string, opts: { where?: Record } = {}) { + if (!seen.has(object)) seen.set(object, new Set()); + const where = opts?.where ?? {}; + for (const key of Object.keys(where)) { + if (!key.startsWith('$')) note(object, key); + } + const rows = (tables[object] ?? []).filter((row) => + Object.entries(where).every(([key, cond]) => { + if (cond && typeof cond === 'object') { + const c = cond as Record; + if ('$in' in c) return (c.$in as unknown[]).includes(raw(row, key)); + if ('$nin' in c) return !(c.$nin as unknown[]).includes(raw(row, key)); + if ('$ne' in c) return raw(row, key) !== c.$ne; + } + return raw(row, key) === cond; + }), + ); + return rows.map( + (row) => + new Proxy(row, { + get(target, prop, receiver) { + // `then` would make the row look thenable to an `await`; symbols + // are never column names. + if (typeof prop === 'string' && prop !== 'then') note(object, prop); + return Reflect.get(target, prop, receiver); + }, + }), + ); + }, + }; +} + +const headers = () => new Headers(); +const sessionFor = (userId: string, org?: string) => async () => ({ + user: { id: userId, email: 'ada@example.com' }, + session: { activeOrganizationId: org ?? null }, +}); + +const HOUR = 3_600_000; +const NOW = Date.parse('2026-08-15T00:00:00.000Z'); + +/** + * Fixture variants, each reaching the platform-admin derivation and each + * deliberately taking a different side of the resolver's conditional reads. + */ +const VARIANTS: Record>>; org?: string }> = { + // Snake_case rows, unscoped in-window grant, active set: the happy platform-admin path. + 'snake-case rows, standing intact': { + org: 'org_1', + tables: { + sys_user: [{ id: 'usr_1', email: 'ada@example.com', ai_access: 1 }], + sys_member: [ + { id: 'mem_1', user_id: 'usr_1', organization_id: 'org_1', role: 'owner', valid_from: null, valid_until: null }, + ], + sys_user_position: [ + { id: 'upo_1', user_id: 'usr_1', position: 'contributor', organization_id: null, valid_from: null, valid_until: null }, + ], + sys_position: [{ id: 'pos_1', name: 'contributor', active: true }], + sys_position_permission_set: [{ position_id: 'pos_1', permission_set_id: 'pst_2' }], + sys_user_permission_set: [ + { + id: 'ups_1', + user_id: 'usr_1', + permission_set_id: 'pst_1', + organization_id: null, + valid_from: null, + valid_until: null, + }, + ], + sys_permission_set: [ + { + id: 'pst_1', + name: 'admin_full_access', + active: true, + system_permissions: ['view_all_records'], + tab_permissions: { setup: 'visible' }, + }, + { id: 'pst_2', name: 'contributor_set', active: true }, + ], + }, + }, + + // camelCase-only rows: every `snake ?? camel` chain must fall through to its + // second limb, which is the only way the camelCase spellings are observed. + 'camelCase-only rows': { + org: 'org_1', + tables: { + sys_user: [{ id: 'usr_1', email: 'ada@example.com', ai_access: true }], + sys_member: [{ id: 'mem_1', userId: 'usr_1', organizationId: 'org_1', role: 'admin' }], + sys_user_position: [{ id: 'upo_1', userId: 'usr_1', position: 'contributor' }], + sys_position: [{ id: 'pos_1', name: 'contributor', active: true }], + sys_position_permission_set: [{ positionId: 'pos_1', permissionSetId: 'pst_2' }], + sys_user_permission_set: [{ id: 'ups_1', userId: 'usr_1', permissionSetId: 'pst_1' }], + sys_permission_set: [ + { + id: 'pst_1', + name: 'admin_full_access', + active: true, + systemPermissions: ['view_all_records'], + tabPermissions: { setup: 'visible' }, + }, + { id: 'pst_2', name: 'contributor_set', active: true }, + ], + }, + }, + + // Standing taken away every way the resolver knows: the set switched off + // (ADR-0049), the grant scoped to an organization, the window closed + // (ADR-0091), the position deactivated, and the JSON blobs stored as strings. + 'standing revoked every way': { + org: 'org_1', + tables: { + sys_user: [{ id: 'usr_1', email: 'ada@example.com', ai_access: 0 }], + sys_member: [ + { + id: 'mem_1', + user_id: 'usr_1', + organization_id: 'org_1', + role: 'member', + valid_from: new Date(NOW - HOUR).toISOString(), + valid_until: new Date(NOW + HOUR).toISOString(), + }, + ], + sys_user_position: [ + { + id: 'upo_1', + user_id: 'usr_1', + position: 'contributor', + organization_id: 'org_1', + valid_from: new Date(NOW - HOUR).toISOString(), + valid_until: new Date(NOW - 1).toISOString(), + }, + ], + sys_position: [{ id: 'pos_1', name: 'contributor', active: false }], + sys_position_permission_set: [{ position_id: 'pos_1', permission_set_id: 'pst_2' }], + sys_user_permission_set: [ + { + id: 'ups_1', + user_id: 'usr_1', + permission_set_id: 'pst_1', + organization_id: 'org_1', + valid_from: new Date(NOW - HOUR).toISOString(), + valid_until: new Date(NOW + HOUR).toISOString(), + }, + ], + sys_permission_set: [ + { + id: 'pst_1', + name: 'admin_full_access', + active: false, + system_permissions: JSON.stringify(['view_all_records']), + tab_permissions: JSON.stringify({ setup: 'visible' }), + }, + { id: 'pst_2', name: 'contributor_set', active: true }, + ], + }, + }, +}; + +async function observe(variant: keyof typeof VARIANTS): Promise { + const seen: Observation = new Map(); + const { tables, org } = VARIANTS[variant]; + await resolveAuthzContext({ + ql: makeRecordingQl(tables, seen), + headers: headers(), + getSession: sessionFor('usr_1', org), + nowMs: NOW, + }); + return seen; +} + +async function observeAll(): Promise { + const union: Observation = new Map(); + for (const name of Object.keys(VARIANTS)) { + const seen = await observe(name); + for (const [table, cols] of seen) { + const into = union.get(table) ?? new Set(); + for (const col of cols) into.add(col); + union.set(table, into); + } + } + return union; +} + +const sorted = (s: Iterable): string[] => [...s].sort(); + +describe('[#8734] ADMIN_STANDING_SURFACE is what resolveAuthzContext actually reads', () => { + it('declares every table the resolution path reads — a new one must be classified', async () => { + const union = await observeAll(); + expect(sorted(union.keys())).toEqual(sorted(Object.keys(ADMIN_STANDING_SURFACE))); + }); + + it.each(adminStandingTables())( + 'declares exactly the columns read on %s', + async (table) => { + const union = await observeAll(); + const observed = sorted(union.get(table) ?? []); + const declared = sorted(ADMIN_STANDING_SURFACE[table]!.columns ?? []); + // Equality, not containment, in BOTH directions on purpose. An undeclared + // read is the #8613 defect. A declared column nothing reads is the stale + // comment this file replaced, and left alone it would go on demanding a + // guard entry for a column that stopped mattering. + expect(observed).toEqual(declared); + }, + ); + + it('reaches the platform-admin derivation — otherwise the observation proves nothing', async () => { + const ctx = await resolveAuthzContext({ + ql: makeRecordingQl(VARIANTS['snake-case rows, standing intact']!.tables, new Map()), + headers: headers(), + getSession: sessionFor('usr_1', 'org_1'), + nowMs: NOW, + }); + // A positive control on the fixture itself: if the happy variant ever stops + // resolving a platform admin, every column below it goes unobserved and the + // equality above starts passing over a path nothing walked. + expect(ctx.positions).toContain('platform_admin'); + expect(ctx.posture).toBe('PLATFORM_ADMIN'); + }); + + it('keeps the variants distinct — a variant that stops contributing narrows the union silently', async () => { + const perVariant = new Map(); + for (const name of Object.keys(VARIANTS)) { + const seen = await observe(name); + const signature = sorted(seen.keys()) + .map((t) => `${t}:${sorted(seen.get(t)!).join(',')}`) + .join('|'); + perVariant.set(name, signature); + } + expect(new Set(perVariant.values()).size).toBe(perVariant.size); + }); + + it('every declared table carries a reason, and only deriving tables carry columns', () => { + for (const [table, entry] of Object.entries(ADMIN_STANDING_SURFACE)) { + expect(entry.reason.length, `${table} needs a reason`).toBeGreaterThan(40); + if (entry.role === 'derives') { + expect(entry.columns, `${table} derives standing and must declare its columns`).toBeDefined(); + } else { + expect(entry.columns, `${table} reads only and must not declare columns`).toBeUndefined(); + } + } + }); +}); diff --git a/packages/core/src/security/admin-standing-surface.ts b/packages/core/src/security/admin-standing-surface.ts new file mode 100644 index 0000000000..c4c6a7d5ff --- /dev/null +++ b/packages/core/src/security/admin-standing-surface.ts @@ -0,0 +1,198 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * ADMIN_STANDING_SURFACE — what `resolveAuthzContext` READS when it decides + * who is an administrator, declared beside the resolver that reads it. + * + * ## Why this file exists (#8734) + * + * `plugin-auth`'s break-glass guard (`last-admin-guard.ts`, ADR-0024 D5.2) + * decides whether a pending write can empty the administrator population by + * testing the payload against three standing-key lists — `MEMBER_STANDING_KEYS`, + * `GRANT_STANDING_KEYS`, `PERMISSION_SET_STANDING_KEYS`. Those lists are not an + * independent design artifact: they are a CACHE of the columns this resolver + * consumes. A payload touching none of them is skipped without any reads, so a + * column this resolver starts reading and the guard's list omits is a write + * class the guard silently stops judging — the one write class that can lock an + * installation out of its own administration, with no in-product recovery. + * + * Nothing bound the two together. The correspondence was carried by a comment, + * and it had already gone false once: #6084 wrote, beside the list, that + * everything a permission-set write touches other than `name` — naming `active` + * explicitly — is invisible to "who is an administrator". That was true when + * written. #8613 made `active` a resolution-time predicate (a DEACTIVATED + * `admin_full_access` set confers nothing, §6b below), and the sentence became + * false. It was caught by one agent reading the comment closely enough to + * notice it contradicted the code being written. Nothing mechanical would have + * caught it: the guard's own tests stay green, because the guard is simply never + * consulted for that write. + * + * ## What this file is, and what it is NOT + * + * It is a MEASUREMENT, not a wish. Its column lists are asserted equal to what + * the resolver actually reads at runtime, by + * `admin-standing-surface.test.ts`, which drives the real + * `resolveAuthzContext` over a recording engine and collects every property + * access and every `where` key per table. That is deliberate: a hand-written + * list of "columns the derivation reads" is the same artifact as the comment + * that went stale, one indirection along. Observation is also the only reading + * that survives the derivation moving INTO a helper — `active` is read by + * `isRowActive(ps)` and the window bounds by `isGrantActive(row, now)`, neither + * of which names a column at the resolver's own call site. + * + * It is NOT a projection the resolver consumes. `ql.find` here returns whole + * rows and the reads are ordinary property accesses on untyped rows, so nothing + * in this file can FORCE the resolver to read only what it declares. The force + * comes from the observation test: add a read, and this declaration is red + * until it is updated; update this declaration, and `plugin-auth`'s + * correspondence test is red until every new column is either in a standing-key + * list or explicitly excluded with a reason. + * + * ## Reading the entries + * + * Every table this resolution path reads is listed — including the ones that + * CANNOT confer administrator standing, each with the reason it cannot. That is + * the table-level half of the same guarantee: a resolver that starts deriving + * administrator standing from a new table would otherwise be invisible to a + * column-set comparison, because the new table appears in neither side's list. + */ + +/** How a table this resolver reads relates to "who is an administrator". */ +export interface AdminStandingTable { + /** + * `derives` — a write to this table can change the administrator population, + * so `last-admin-guard.ts` must carry a standing-key list for it. + * `reads-only` — this resolver reads the table for something else entirely. + */ + readonly role: 'derives' | 'reads-only'; + /** Why the row above is the right classification. Prose, but pinned to a measured table. */ + readonly reason: string; + /** + * Every column this resolver reads on the table — property accesses and + * `where` keys alike, in every spelling it actually touches. Declared for + * `derives` tables only; asserted equal to the observed set. + */ + readonly columns?: readonly string[]; +} + +/** + * The measured read surface of the administrator derivation. + * + * Scope, stated so the gate cannot be read as claiming more than it measures: + * this is the SESSION/user-id resolution path — `resolveAuthzContext` with a + * principal, and therefore all of `resolveUserAuthzGrants`. The API-key + * ADMISSION path (`resolveApiKeyAdmission`) is outside it on purpose: it + * authenticates a principal and seeds `permissions` with the key's scopes, and + * confers no administrator standing of its own — `hasPlatformAdminGrant` (§6b) + * is set only from a `sys_permission_set` row reached through an UNSCOPED + * `sys_user_permission_set` grant, never from a scope string. + */ +export const ADMIN_STANDING_SURFACE: Readonly> = { + sys_permission_set: { + role: 'derives', + reason: + 'The row `platform_admin` is resolved BY NAME from (§6b). Renaming it, deleting it or ' + + 'switching it off (ADR-0049 `active`, read here since #8613) un-makes every platform ' + + 'admin at once, with no identity table touched.', + columns: [ + 'id', + 'name', + 'active', + 'system_permissions', + 'systemPermissions', + 'tab_permissions', + 'tabPermissions', + ], + }, + + sys_user_permission_set: { + role: 'derives', + reason: + 'The grant that makes a user a platform admin: an UNSCOPED, in-window (ADR-0091) grant of ' + + '`admin_full_access` (§6). Re-pointing it, scoping it to an organization or moving it out ' + + 'of its window revokes the standing while leaving the row in place.', + columns: [ + 'user_id', + 'permission_set_id', + 'permissionSetId', + 'organization_id', + 'organizationId', + 'valid_from', + 'validFrom', + 'valid_until', + 'validUntil', + ], + }, + + sys_member: { + role: 'derives', + reason: + 'Organization owner/admin standing (§3). The graded `role` is projected into `positions` ' + + 'here and separately drives the `organization_admin` capability grant, which is what the ' + + 'posture ladder reads; the break-glass guard counts the same rows one step earlier, by ' + + 'grade (ADR-0108). Either way a downgrade of the last graded membership is a write that ' + + 'can empty the administrator population.', + columns: [ + 'user_id', + 'userId', + 'organization_id', + 'organizationId', + 'role', + 'valid_from', + 'validFrom', + 'valid_until', + 'validUntil', + ], + }, + + sys_user: { + role: 'reads-only', + reason: + 'Read for the `current_user.email` RLS fallback and the ADR-0024 `ai_seat` synthesis (§7). ' + + 'Neither confers administrator standing. The guard does watch this table, but for the ' + + 'ban/delete WRITE SHAPES — `banned` is never read here, so it is not a derivation column ' + + 'and carries no standing-key list.', + }, + + sys_user_position: { + role: 'reads-only', + reason: + 'ADR-0057 D4 platform-RBAC position assignments (§4). A position can carry permission sets ' + + '(see `sys_position_permission_set`) but never platform-admin standing — §6b requires the ' + + 'set to be reached through an unscoped USER grant (`unscopedUserPsIds`), so a ' + + 'position-bound `admin_full_access` resolves the set name into `permissions` and leaves ' + + '`hasPlatformAdminGrant` false.', + }, + + sys_position: { + role: 'reads-only', + reason: + 'Read to drop DEACTIVATED positions (ADR-0049, §6a). Same reason as `sys_user_position`: ' + + 'the position path cannot reach `hasPlatformAdminGrant`.', + }, + + sys_position_permission_set: { + role: 'reads-only', + reason: + 'Position-bound permission sets (§6a). Contributes ids to `psIds` — and therefore names to ' + + '`permissions` — but not to `unscopedUserPsIds`, which is the set §6b tests for ' + + 'platform-admin standing.', + }, +}; + +/** The tables a write to which can change who is an administrator. */ +export function adminStandingTables(): string[] { + return Object.entries(ADMIN_STANDING_SURFACE) + .filter(([, t]) => t.role === 'derives') + .map(([name]) => name) + .sort(); +} + +/** + * The columns this resolver reads on `table`, or `undefined` when the table is + * not part of the administrator derivation. + */ +export function adminStandingColumns(table: string): readonly string[] | undefined { + const entry = ADMIN_STANDING_SURFACE[table]; + return entry?.role === 'derives' ? entry.columns : undefined; +} diff --git a/packages/core/src/security/index.ts b/packages/core/src/security/index.ts index 59f148f28a..5444c00b9d 100644 --- a/packages/core/src/security/index.ts +++ b/packages/core/src/security/index.ts @@ -147,6 +147,15 @@ export { isGrantActive, isGrantExpired, type GrantValidityWindow } from './grant // enforces it and the break-glass guard that simulates a write to it. export { isRowActive, type ActivatableRow } from './row-active.js'; +// [#8734] The measured read surface of the administrator derivation — the +// single source `plugin-auth`'s break-glass standing-key lists correspond to. +export { + ADMIN_STANDING_SURFACE, + adminStandingTables, + adminStandingColumns, + type AdminStandingTable, +} from './admin-standing-surface.js'; + // [#7678] ADR-0090 D5/D9 — the audience-binding suggestion `?status=` vocabulary, // shared by the runtime dispatcher's `/security` domain and the live REST route. export { diff --git a/packages/plugins/plugin-auth/src/last-admin-guard.ts b/packages/plugins/plugin-auth/src/last-admin-guard.ts index 28a1635ffc..3de6a20ecc 100644 --- a/packages/plugins/plugin-auth/src/last-admin-guard.ts +++ b/packages/plugins/plugin-auth/src/last-admin-guard.ts @@ -546,7 +546,7 @@ function applyPending( * scoped to the ENVIRONMENT, so which org a membership sits in never changes * who administers this deployment.) */ -const MEMBER_STANDING_KEYS = ['role', 'user_id', 'userId'] as const; +export const MEMBER_STANDING_KEYS = ['role', 'user_id', 'userId'] as const; /** * Same, for `sys_user_permission_set`: which permission set the grant points @@ -554,7 +554,7 @@ const MEMBER_STANDING_KEYS = ['role', 'user_id', 'userId'] as const; * — every column the grant half of the enumeration consumes, in both the * snake_case and camelCase spellings the readers already tolerate. */ -const GRANT_STANDING_KEYS = [ +export const GRANT_STANDING_KEYS = [ 'permission_set_id', 'permissionSetId', 'user_id', @@ -608,7 +608,90 @@ const GRANT_STANDING_KEYS = [ * never conferred it, in the resolver or here), and org-administrator standing * is read from `sys_member.role`. Deactivating a position cannot empty either. */ -const PERMISSION_SET_STANDING_KEYS = ['name', 'active'] as const; +export const PERMISSION_SET_STANDING_KEYS = ['name', 'active'] as const; + +/** + * [#8734] The three lists above, keyed by the table each one judges — the shape + * the correspondence gate consumes. + * + * The gate (`last-admin-standing-keys.test.ts`) reads + * `ADMIN_STANDING_SURFACE` from `@objectstack/core`, which is the MEASURED set + * of columns `resolveAuthzContext` reads per table, and requires every column + * of every standing-bearing table to have an answer here: either the guard + * treats it as standing-bearing (it is in the list) or it is excluded below + * with the reason it cannot move the administrator population. + * + * Keying by table is not cosmetic. It is what makes a resolver that starts + * deriving administrator standing from a NEW table fail: core reclassifies the + * table as `derives`, and the gate then demands a list here that does not + * exist. A column-set comparison alone cannot see that, because a new table is + * absent from both sides. + */ +export const STANDING_KEYS_BY_TABLE: Readonly> = { + [SystemObjectName.MEMBER]: MEMBER_STANDING_KEYS, + [USER_PERMISSION_SET]: GRANT_STANDING_KEYS, + [SystemObjectName.PERMISSION_SET]: PERMISSION_SET_STANDING_KEYS, +}; + +/** + * [#8734] Columns the resolver reads that this guard deliberately does NOT + * treat as standing-bearing, each with the reason it cannot empty the + * administrator population. + * + * Every exclusion here was already argued in the prose above; what changes is + * that the argument is now attached to a column the gate has measured, and a + * column that acquires a reader gets no disposition until someone writes one. + * The reason strings can still go out of date in their CONTENT — that is true + * of any prose — but they can no longer go out of date in their SUBJECT, which + * is how #6084's comment about `active` survived #8613. + * + * ⚠️ An entry here is a decision that a write to that column is safe, on a path + * whose failure mode is an installation-wide administrator lockout with no + * in-product recovery. Adding one to make a red gate green is the wrong move in + * exactly the way this card exists to prevent; the right move is almost always + * to add the column to the list above. + */ +export const STANDING_KEY_EXCLUSIONS: Readonly>>> = { + [SystemObjectName.MEMBER]: { + organization_id: + 'The invariant is scoped to the ENVIRONMENT, not to each organization ("Scope" above), so ' + + 'which org a membership sits in never changes who administers this deployment. The ' + + 'resolver reads it to scope positions to the ACTIVE org and to build accessible_org_ids; ' + + 'neither is a count of administrators.', + organizationId: 'Camel-case spelling of `organization_id` — same reason.', + valid_from: + 'ADR-0091 windows are not columns on `sys_member` today. The resolver calls isGrantActive ' + + 'on membership rows only when building `accessible_org_ids` (the group posture read ' + + 'reach); the org-administration role projection it feeds `positions` from is NOT window ' + + 'filtered, and this guard counts org administrators by GRADE alone (isOrgAdminGrade). So ' + + 'no reader of administrator standing consults these bounds, and writing one cannot revoke ' + + 'a grade. If the columns ever land on `sys_member`, the resolver is where the two halves ' + + 'have to be reconciled first — this list follows it, it does not lead.', + validFrom: 'Camel-case spelling of `valid_from` — same reason.', + valid_until: 'Upper bound of the same absent window as `valid_from` — same reason.', + validUntil: 'Camel-case spelling of `valid_until` — same reason.', + }, + + [USER_PERMISSION_SET]: {}, + + [SystemObjectName.PERMISSION_SET]: { + id: + 'On this engine `data.id` on an update ADDRESSES the row rather than proposing a new ' + + 'primary key (see the note above `PERMISSION_SET_STANDING_KEYS`), so a key rewrite is not ' + + 'expressible through this write path at all.', + system_permissions: + 'What the set CONTAINS does not un-make a platform admin: `hasPlatformAdminGrant` is set ' + + "from `ps.name === 'admin_full_access'` on an ACTIVE set, and the posture rung and " + + 'superuser bypass ride on that boolean. Emptying the blob costs the holder setup/studio ' + + 'access — recoverable from inside the product, an ADR-0086 capability question, not a ' + + 'break-glass one.', + systemPermissions: 'Camel-case spelling of `system_permissions` — same reason.', + tab_permissions: + 'Tab visibility per app. Same reason as `system_permissions`: it is content of the set, ' + + 'never the name-and-active pair the derivation reads.', + tabPermissions: 'Camel-case spelling of `tab_permissions` — same reason.', + }, +}; function touchesAny(data: Record, keys: readonly string[]): boolean { return keys.some((k) => k in data); diff --git a/packages/plugins/plugin-auth/src/last-admin-standing-keys.test.ts b/packages/plugins/plugin-auth/src/last-admin-standing-keys.test.ts new file mode 100644 index 0000000000..6083e653a4 --- /dev/null +++ b/packages/plugins/plugin-auth/src/last-admin-standing-keys.test.ts @@ -0,0 +1,132 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8734] The SECOND of the two links that bind this guard's standing-key lists + * to what the authorization resolver actually reads. + * + * Link 1 lives in `@objectstack/core` + * (`security/admin-standing-surface.test.ts`): it drives the real + * `resolveAuthzContext` over a recording engine and asserts + * `ADMIN_STANDING_SURFACE` equals the columns observed. So that declaration is + * a measurement of the resolver, not a copy of it, and a resolver change that + * starts reading a new column cannot land while it is stale. + * + * This link consumes that measurement and requires the guard to have an ANSWER + * for every column of it: the column is standing-bearing (it is in a list, so a + * payload touching it pays for a full enumeration), or it is excluded with the + * reason it cannot empty the administrator population. There is no third state + * — which is the whole point, because the third state is what + * `PERMISSION_SET_STANDING_KEYS` was in between #6084 and #8613: a column the + * resolver had started reading, that the guard did not judge, and that a + * confident, recently-dated comment said was invisible to "who is an + * administrator". + * + * ## Would this have caught #8613? + * + * Yes, and at the earlier of the two links. #8613 made `active` a + * resolution-time predicate by adding `isRowActive(r)` to the + * `sys_permission_set` read. That read makes `active` observable, so link 1 + * goes red on the PR that adds it, naming the column. Declaring it there turns + * link 2 red, because `active` would then be in neither + * `PERMISSION_SET_STANDING_KEYS` nor the exclusion ledger. Landing #8613 green + * would have required writing down, explicitly, that deactivating + * `admin_full_access` cannot empty the administrator population — which is + * false, and which is the sentence the old comment asserted by accident. + * + * ## What this gate deliberately does NOT assert + * + * That every standing KEY is a column the resolver reads. That direction reads + * as the natural other half and it is not: `GRANT_STANDING_KEYS` carries + * `userId` although the resolver reaches `sys_user_permission_set` rows through + * a `where` on `user_id` and never touches the camelCase spelling on the row. + * Enforcing it would put pressure on the guard to DROP entries — to become + * cheaper, to fire less often — and the guard's list is allowed to be a + * superset of what any one reader touches. The gate is one-directional on + * purpose: it can only ever demand that the guard judges MORE. + */ + +import { describe, it, expect } from 'vitest'; +import { ADMIN_STANDING_SURFACE, adminStandingTables } from '@objectstack/core'; + +import { + GRANT_STANDING_KEYS, + MEMBER_STANDING_KEYS, + PERMISSION_SET_STANDING_KEYS, + STANDING_KEYS_BY_TABLE, + STANDING_KEY_EXCLUSIONS, +} from './last-admin-guard.js'; + +const derivingTables = adminStandingTables(); + +describe('[#8734] standing-key lists correspond to what resolveAuthzContext reads', () => { + it('the resolver derives administrator standing from at least one table', () => { + // A positive control on the imported declaration itself. Every assertion + // below is a `for` over this list; an empty one would make all of them pass + // while checking nothing — the shape that makes a guard green forever. + expect(derivingTables.length).toBeGreaterThan(0); + }); + + it.each(derivingTables)('the guard carries a standing-key list for %s', (table) => { + // The table-level link: a resolver that starts deriving administrator + // standing from a NEW table is reclassified `derives` in core, and this + // fails until the guard grows a list — and, being a list, a hook. + expect(Object.keys(STANDING_KEYS_BY_TABLE)).toContain(table); + expect(STANDING_KEYS_BY_TABLE[table]!.length).toBeGreaterThan(0); + }); + + it.each(derivingTables)('every column read on %s is judged or excluded with a reason', (table) => { + const declared = ADMIN_STANDING_SURFACE[table]!.columns ?? []; + const keys = new Set(STANDING_KEYS_BY_TABLE[table] ?? []); + const excluded = STANDING_KEY_EXCLUSIONS[table] ?? {}; + + const undecided = declared.filter((c) => !keys.has(c) && !(c in excluded)); + expect( + undecided, + `Columns resolveAuthzContext reads on '${table}' that this guard neither judges nor ` + + 'excludes. Add each to the standing-key list (the usual answer — a payload touching it ' + + 'must pay for an administrator enumeration), or to STANDING_KEY_EXCLUSIONS with the ' + + 'reason it cannot empty the administrator population.', + ).toEqual([]); + }); + + it.each(derivingTables)('no column on %s is both judged and excluded', (table) => { + const keys = new Set(STANDING_KEYS_BY_TABLE[table] ?? []); + const both = Object.keys(STANDING_KEY_EXCLUSIONS[table] ?? {}).filter((c) => keys.has(c)); + expect(both, `contradictory disposition on '${table}'`).toEqual([]); + }); + + it.each(derivingTables)('no exclusion on %s names a column nothing reads', (table) => { + const declared = new Set(ADMIN_STANDING_SURFACE[table]!.columns ?? []); + const stale = Object.keys(STANDING_KEY_EXCLUSIONS[table] ?? {}).filter((c) => !declared.has(c)); + // A ledger that keeps entries for columns no reader consults is the stale + // comment again, in a machine-readable coat: it reads as a considered + // decision about live code long after the code moved. + expect(stale, `stale exclusion(s) on '${table}' — the resolver no longer reads them`).toEqual([]); + }); + + it.each(derivingTables)('every exclusion on %s carries a real reason', (table) => { + for (const [column, reason] of Object.entries(STANDING_KEY_EXCLUSIONS[table] ?? {})) { + expect(reason.trim().length, `'${table}.${column}' needs a reason, not a placeholder`) + .toBeGreaterThan(40); + } + }); + + it('the guard has no standing-key list for a table the resolver does not derive from', () => { + // The reverse of the table-level link. A list for a table core classifies + // `reads-only` is either a guard judging a write class nothing derives from, + // or — far more likely — core's classification having quietly gone wrong. + for (const table of Object.keys(STANDING_KEYS_BY_TABLE)) { + expect(derivingTables, `'${table}' has a standing-key list but core does not derive from it`) + .toContain(table); + } + }); + + it('the exported lists are the ones the guard actually tests payloads against', () => { + // STANDING_KEYS_BY_TABLE is what this gate reads; the three consts are what + // `touchesAny` is called with. Identity, not equality, so the map cannot + // drift into a second copy that is checked while the guard uses another. + expect(STANDING_KEYS_BY_TABLE.sys_member).toBe(MEMBER_STANDING_KEYS); + expect(STANDING_KEYS_BY_TABLE.sys_user_permission_set).toBe(GRANT_STANDING_KEYS); + expect(STANDING_KEYS_BY_TABLE.sys_permission_set).toBe(PERMISSION_SET_STANDING_KEYS); + }); +});