fix(security): enforce active on sys_permission_set and sys_position (#8613)

.changeset/enforce-active-on-grant-catalogues.md

@@ -0,0 +1,79 @@
+---
+"@objectstack/core": minor
+"@objectstack/plugin-security": minor
+"@objectstack/plugin-auth": minor
+---
+
+fix(security): `sys_permission_set.active` and `sys_position.active` now actually stop granting access (#8613)
+
+<!-- adr-0087: not-required (no-migration-prescription) Nothing authorable is
+added, renamed or retired. `active` is a ROW property of a `sys_permission_set`
+/ `sys_position` record, not a key on `PermissionSetSchema` (which is a strict
+object in `packages/spec` and deliberately declares no such key — see
+`permission-set-projection.ts`'s ROW_STATE_COLUMNS). No spec schema, export or
+stored metadata shape changes, so there is no conversion to register and no
+tombstone to write. The change is a runtime predicate at the authorization
+resolution seam; the remedy for an affected deployment is operational
+(re-activate rows that were switched off), not a metadata migration. -->
+
+**BREAKING for deployments that already switched a permission set or position
+off.** Both objects ship a Deactivate action whose confirmation dialog promises,
+in all four locales, that access stops:
+
+> Deactivate this permission set? Existing assignments stay in place but stop
+> granting access until re-activated.
+> Deactivate this position? Users keep their assignment but the position stops
+> granting permissions until re-activated.
+
+Nothing read the column. Measured on the real resolver: a position seeded
+`active: false` still granted its permission sets, and a permission set seeded
+`active: false` still returned `posture: PLATFORM_ADMIN` with its system
+permissions. Deactivation moved a badge in Setup and nothing else — while the
+admin who had just revoked a compromised or over-broad grant was told the
+opposite, and whose likely next step was therefore *not* the action that would
+have worked (delete the set, or remove the assignments).
+
+**What changes at runtime.** `resolveAuthzContext` / `resolveUserAuthzGrants`
+(`@objectstack/core`) — the single seam every transport resolves authorization
+through — now drop a deactivated row **before** any derivation:
+
+- a deactivated `sys_position` no longer contributes its
+ `sys_position_permission_set` grants, and its name leaves `positions` (so the
+ name-reuse path cannot resolve the same grant one layer down);
+- a deactivated `sys_permission_set` contributes no name, no
+ `system_permissions`, no `tab_permissions`, **and no `PLATFORM_ADMIN`
+ posture** — the flag is applied before the posture is derived, not after;
+- the `plugin-security` DB loader applies the same predicate, which is what
+ judges a set reached by NAME through an active position of the same name.
+
+Both tables were already read at that seam, so this costs **zero new hot-path
+queries**.
+
+**⚠️ Read this before upgrading.** Any `sys_permission_set` or `sys_position`
+row currently carrying `active: false` **stops granting the moment this
+lands** — on live data, with no migration step to notice. That is the correct
+direction (it is what the dialog said when someone clicked Deactivate), but on
+an installation that used the switch believing it was inert it is a real
+revocation. Before upgrading, list the deactivated rows and re-activate any that
+are still meant to grant:
+
+```
+GET /api/v1/data/sys_permission_set?filters=[["active","=",false]]
+GET /api/v1/data/sys_position?filters=[["active","=",false]]
+```
+
+A row whose `active` column is **absent or NULL** is unaffected: the predicate
+is "explicitly deactivated", never "explicitly active", so rows that predate the
+column keep granting exactly as before.
+
+**Break-glass, closed in the same change** (`@objectstack/plugin-auth`).
+Enforcing the flag opened a one-click, installation-wide lockout: deactivating
+`admin_full_access` un-makes every platform admin at once, through a payload
+that touches neither `name` nor any identity table, and re-activating requires
+the permission the click just took away (the seeders deliberately never
+reconcile `active`, so no restart restores it). The last-administrator guard now
+judges that write like the delete and rename spellings it already refused, and
+an environment whose break-glass set is *already* off is read as emptied rather
+than as a bootstrap window — so it does not silently disarm the guard for every
+other identity write. Re-activation itself stays permitted, or the refusal would
+have no way out from inside the product.

content/docs/permissions/authorization.mdx

@@ -330,6 +330,48 @@ product — see ADR-0091 D4–D7 for the open-core line; their community *shapes
 (a time-boxed direct grant with a reason; certification stamps) are the L1
 substrate above.
 
+## Grant lifecycle: the `active` switch (ADR-0049)
+
+Validity windows above date a **user's grant row**. The second lifecycle
+control dates the **catalogue row itself**: `sys_permission_set.active` and
+`sys_position.active`, the switch behind the Deactivate action on both objects.
+It answers a different question — "switch this grant off for everyone, without
+deleting it or unwinding the assignments" — and it is enforced in the same
+place, by the same discipline: **resolution-time filtering, fail-closed**, in
+`resolveAuthzContext`, with no cleanup job involved.
+
+- A deactivated **permission set** contributes nothing: not its name, not its
+ `system_permissions`, not its `tab_permissions`. As with an expired grant, a
+ deactivated unscoped `admin_full_access` no longer derives `platform_admin`
+ — the flag is applied *before* the posture is derived, not after.
+- A deactivated **position** stops carrying its permission sets, and its name
+ stops appearing in `positions`, so a permission set that merely shares the
+ position's name cannot resolve through it either.
+- Assignments are untouched. `sys_user_position` and
+ `sys_user_permission_set` rows stay exactly as they were, and re-activating
+ the catalogue row restores every grant it carried, at the next resolution.
+
+**Absent is ACTIVE.** Only a stored value that really reads false takes a grant
+away, so a row that predates the column keeps granting. The same predicate
+(`isRowActive`) is used by every reader, including the last-administrator
+guard's simulation — a guard that modelled "deactivated" differently from the
+resolver would permit exactly the write it exists to refuse.
+
+**Deactivating the break-glass set is refused.** `admin_full_access` is what
+makes the environment's platform admins, so switching it off would un-make all
+of them in one write — and re-activating it needs the permission just lost.
+That write is judged like deleting or renaming the row (ADR-0024 D5.2): it is
+refused while it would leave the environment with no administrator who can
+sign in. Re-activation is never refused.
+
+Deactivation is an incident-response control, so what it does **not** touch is
+deliberate. Administration surfaces keep listing and editing a deactivated
+position — an admin must still be able to unbind and clean up what they just
+switched off — and the write gates that judge audience-anchor bindings and a
+delegated administrator's blast radius keep reading every row, deactivated
+included: dropping rows there would make a refused binding *permitted* and a
+delegate's boundary *narrower*, which is the opposite of switching access off.
+
 ## Governance: how "declared = enforced" is kept true
 
 Five mechanisms — four CI-time, one runtime — make the security posture a

packages/core/src/security/index.ts

@@ -137,6 +137,11 @@ export {
 // ADR-0091 D1/D2 — grant validity windows, the shared resolution-time predicate.
 export { isGrantActive, isGrantExpired, type GrantValidityWindow } from './grant-validity.js';
 
+// ADR-0049 enforce-or-remove — the `active` flag on the RBAC grant catalogues
+// (`sys_permission_set` / `sys_position`). One predicate for the resolver that
+// enforces it and the break-glass guard that simulates a write to it.
+export { isRowActive, type ActivatableRow } from './row-active.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 {

packages/core/src/security/resolve-authz-context.test.ts

@@ -466,3 +466,228 @@ describe('resolveUserAuthzGrants — userId-driven authz for non-HTTP surfaces (
  });
 });
 
+
+/**
+ * [#8613 / ADR-0049] `sys_permission_set.active` and `sys_position.active` —
+ * enforce-or-remove, enforced.
+ *
+ * Both objects ship a Deactivate action whose dialog promises, in four locales,
+ * that access stops. Nothing read the column, so the promise was false: the
+ * assignments kept granting and the admin who trusted the dialog did not take
+ * the action that would actually have worked.
+ *
+ * This is the ONLY seam where either flag is enforceable. Downstream in
+ * plugin-security the position → permission-set linkage is already collapsed
+ * into a flat `permissions` list, so a set held via a deactivated position is
+ * indistinguishable there from one granted directly — filtering there would
+ * over-revoke a set the user also holds in their own right.
+ *
+ * The predicate is "explicitly deactivated", not "explicitly active": absent
+ * means ACTIVE, so a row that predates the column keeps working. Every fixture
+ * above this block carries no `active` key at all and is the pin for that
+ * direction — requiring `true` would have turned this file red wholesale, which
+ * is what it would do to deployed data.
+ */
+describe('[#8613] the `active` flag on the grant catalogues (ADR-0049)', () => {
+ const withActive = (v: unknown) => ({
+ sys_user: [{ id: 'u1' }],
+ sys_member: [],
+ sys_user_position: [{ user_id: 'u1', position: 'contributor', organization_id: null }],
+ sys_user_permission_set: [],
+ sys_position: [{ id: 'r1', name: 'contributor', active: v }],
+ sys_position_permission_set: [{ position_id: 'r1', permission_set_id: 'ps1' }],
+ sys_permission_set: [{ id: 'ps1', name: 'contributor_ps', system_permissions: ['cap_x'] }],
+ });
+
+ // ── sys_position ──────────────────────────────────────────────────────────
+
+ it('a DEACTIVATED position stops granting its permission sets', async () => {
+ const ctx = await resolveAuthzContext({
+ ql: makeQl(withActive(false)),
+ headers: H(),
+ getSession: session('u1'),
+ });
+ expect(ctx.permissions).not.toContain('contributor_ps');
+ expect(ctx.systemPermissions).not.toContain('cap_x');
+ });
+
+ it('…and its NAME leaves `positions` too, or the name alone would resolve the set', async () => {
+ // `resolvePermissionSetsForContext` requests `context.positions` as
+ // permission-set names (position names are commonly reused as set names),
+ // so a name left standing would resolve the same grant one layer down.
+ const ctx = await resolveAuthzContext({
+ ql: makeQl(withActive(false)),
+ headers: H(),
+ getSession: session('u1'),
+ });
+ expect(ctx.positions).not.toContain('contributor');
+ // The audience anchor is untouched — it is not the deactivated row.
+ expect(ctx.positions).toContain('everyone');
+ });
+
+ it('an ACTIVE position still grants (the flag is not a blanket revocation)', async () => {
+ const ctx = await resolveAuthzContext({
+ ql: makeQl(withActive(true)),
+ headers: H(),
+ getSession: session('u1'),
+ });
+ expect(ctx.positions).toContain('contributor');
+ expect(ctx.permissions).toContain('contributor_ps');
+ expect(ctx.systemPermissions).toContain('cap_x');
+ });
+
+ it('an ABSENT `active` column grants — deployed rows are not mass-revoked', async () => {
+ const ctx = await resolveAuthzContext({
+ ql: makeQl(withActive(undefined)),
+ headers: H(),
+ getSession: session('u1'),
+ });
+ expect(ctx.positions).toContain('contributor');
+ expect(ctx.permissions).toContain('contributor_ps');
+ });
+
+ it('the 0/1 storage shape deactivates too — what the primary driver returns', async () => {
+ const off = await resolveAuthzContext({
+ ql: makeQl(withActive(0)),
+ headers: H(),
+ getSession: session('u1'),
+ });
+ expect(off.permissions).not.toContain('contributor_ps');
+ const on = await resolveAuthzContext({
+ ql: makeQl(withActive(1)),
+ headers: H(),
+ getSession: session('u1'),
+ });
+ expect(on.permissions).toContain('contributor_ps');
+ });
+
+ it('a position name with NO `sys_position` row is untouched (org roles, memberships)', async () => {
+ const ql = makeQl({
+ sys_user: [{ id: 'u1' }],
+ sys_member: [{ user_id: 'u1', role: 'owner', organization_id: 'o1' }],
+ sys_user_position: [],
+ sys_user_permission_set: [],
+ // `org_owner` is projected from the membership and has no catalogue row —
+ // there is no flag to read, so nothing may be inferred from its absence.
+ sys_position: [{ id: 'r9', name: 'something_else', active: false }],
+ });
+ const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1', { org: 'o1' }) });
+ expect(ctx.positions).toContain('org_owner');
+ });
+
+ it('deactivating ONE position leaves the others granting', async () => {
+ const ql = makeQl({
+ sys_user: [{ id: 'u1' }],
+ sys_member: [],
+ sys_user_position: [
+ { user_id: 'u1', position: 'contributor', organization_id: null },
+ { user_id: 'u1', position: 'reviewer', organization_id: null },
+ ],
+ sys_user_permission_set: [],
+ sys_position: [
+ { id: 'r1', name: 'contributor', active: false },
+ { id: 'r2', name: 'reviewer', active: true },
+ ],
+ sys_position_permission_set: [
+ { position_id: 'r1', permission_set_id: 'ps1' },
+ { position_id: 'r2', permission_set_id: 'ps2' },
+ ],
+ sys_permission_set: [
+ { id: 'ps1', name: 'contributor_ps' },
+ { id: 'ps2', name: 'reviewer_ps' },
+ ],
+ });
+ const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1') });
+ expect(ctx.permissions).not.toContain('contributor_ps');
+ expect(ctx.permissions).toContain('reviewer_ps');
+ expect(ctx.positions).not.toContain('contributor');
+ expect(ctx.positions).toContain('reviewer');
+ });
+
+ // ── sys_permission_set ────────────────────────────────────────────────────
+
+ it('a DEACTIVATED permission set grants nothing — name, capabilities and tabs', async () => {
+ const ql = makeQl({
+ sys_user: [{ id: 'u1' }],
+ sys_member: [],
+ sys_user_position: [],
+ sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'ps1', organization_id: null }],
+ sys_permission_set: [{
+ id: 'ps1',
+ name: 'crm_full',
+ active: false,
+ system_permissions: ['cap_x'],
+ tab_permissions: { crm: 'visible' },
+ }],
+ });
+ const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1') });
+ expect(ctx.permissions).not.toContain('crm_full');
+ expect(ctx.systemPermissions).not.toContain('cap_x');
+ expect(ctx.tabPermissions?.crm).toBeUndefined();
+ });
+
+ it('THE HIGH-BLAST-RADIUS CASE: a deactivated admin_full_access confers no PLATFORM_ADMIN', async () => {
+ const ql = makeQl({
+ sys_user: [{ id: 'u1' }],
+ sys_member: [],
+ sys_user_position: [],
+ sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'psA', organization_id: null }],
+ sys_permission_set: [{
+ id: 'psA',
+ name: 'admin_full_access',
+ active: false,
+ system_permissions: ['manage_users'],
+ }],
+ });
+ const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1') });
+ // Dropped BEFORE the derivation, so the posture cannot be read off a set
+ // that no longer grants — the whole point of filtering at §6b rather than
+ // after it.
+ expect(ctx.permissions).not.toContain('admin_full_access');
+ expect(ctx.posture).not.toBe('PLATFORM_ADMIN');
+ expect(ctx.positions).not.toContain('platform_admin');
+ expect(ctx.systemPermissions).not.toContain('manage_users');
+ });
+
+ it('deactivating ONE set leaves the others granting', async () => {
+ const ql = makeQl({
+ sys_user: [{ id: 'u1' }],
+ sys_member: [],
+ sys_user_position: [],
+ sys_user_permission_set: [
+ { user_id: 'u1', permission_set_id: 'ps1', organization_id: null },
+ { user_id: 'u1', permission_set_id: 'ps2', organization_id: null },
+ ],
+ sys_permission_set: [
+ { id: 'ps1', name: 'crm_full', active: false },
+ { id: 'ps2', name: 'crm_read', active: true },
+ ],
+ });
+ const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1') });
+ expect(ctx.permissions).not.toContain('crm_full');
+ expect(ctx.permissions).toContain('crm_read');
+ });
+
+ it('a set held via BOTH a deactivated position and a direct grant still resolves', async () => {
+ // The over-revocation this seam is chosen to avoid: the direct grant is a
+ // separate authority and the position's deactivation may not touch it.
+ const ql = makeQl({
+ sys_user: [{ id: 'u1' }],
+ sys_member: [],
+ sys_user_position: [{ user_id: 'u1', position: 'contributor', organization_id: null }],
+ sys_user_permission_set: [{ user_id: 'u1', permission_set_id: 'ps1', organization_id: null }],
+ sys_position: [{ id: 'r1', name: 'contributor', active: false }],
+ sys_position_permission_set: [{ position_id: 'r1', permission_set_id: 'ps1' }],
+ sys_permission_set: [{ id: 'ps1', name: 'crm_full' }],
+ });
+ const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1') });
+ expect(ctx.permissions).toContain('crm_full');
+ expect(ctx.positions).not.toContain('contributor');
+ });
+
+ it('resolveUserAuthzGrants enforces it too — the non-HTTP surfaces share the seam', async () => {
+ const grants = await resolveUserAuthzGrants(makeQl(withActive(false)), 'u1');
+ expect(grants.permissions).not.toContain('contributor_ps');
+ expect(grants.positions).not.toContain('contributor');
+ });
+});