Uh oh!
There was an error while loading. Please reload this page.
fix(security): enforce active on sys_permission_set and sys_position (#8613) - #8713
Conversation
#8613) Both objects ship a Deactivate action promising, in four locales, that access stops. Nothing read the column: a position seeded `active: false` still granted its permission sets, and a permission set seeded `active: false` still returned `posture: PLATFORM_ADMIN`. Enforced at the single derivation seam every transport resolves through — `resolveAuthzContext` / `resolveUserAuthzGrants` in @objectstack/core. Both tables were already read there, so this costs zero new hot-path queries: - step 6a drops a deactivated `sys_position` and its bound permission sets, and removes the name from `positions` so the name-reuse path cannot resolve the same grant one layer down; - step 6b drops a deactivated `sys_permission_set` BEFORE any derivation, so `hasPlatformAdminGrant` cannot be read off a set that no longer grants; - 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. `isRowActive` (new, @objectstack/core) is the one predicate all three readers share: explicitly deactivated, never "explicitly active" — an absent column is ACTIVE, so rows predating the field are not mass-revoked, and the 0/1 storage shape is judged as well as a literal `false`. Grant-resolution reads ONLY. The write gates and blast-radius reads in plugin-security (`assertAudienceAnchorBindingGate`, `setsBoundToPosition`, the delegated-admin surfaces) stay unfiltered: dropping a deactivated row there makes a refused binding permitted, narrows a delegate's boundary, and makes a deactivated position unmanageable. Break-glass, closed in the same change: `active` joins PERMISSION_SET_STANDING_KEYS, the enumeration projects and judges the column, and an environment whose `admin_full_access` row is already deactivated reads as emptied rather than as a bootstrap window. Re-activation stays permitted. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH
The latest updates on your projects. Learn more about Vercel for GitHub. 1 Skipped Deployment
|
📓 Docs Drift CheckThis PR changes 3 package(s): 30 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:
⛔ 5 release-owned page(s) also reference the affected code. These are read-only:
|
…ndows (#8613) Screened the 30 hand-written docs the docs-drift mapper flags for this change (the 5 release-owned pages are read-only and untouched). No existing sentence is made wrong: nothing documented what `sys_permission_set.active` or `sys_position.active` meant, in either direction. The three files carrying the deactivation vocabulary all describe `sys_sharing_rule.active` — a different object whose deactivation already worked — and remain correct as written. The real gap is the omission. `authorization.mdx` documents ADR-0091 validity windows as THE resolution-time grant-lifecycle filter, down to "an expired unscoped `admin_full_access` grant no longer derives `platform_admin`" — the exact consequence this change adds a second path to. A reader would conclude there is only one such filter. Adds a sibling section stating the enforced semantics, that assignments are untouched and re-activation restores them, that absent means ACTIVE (so rows predating the column keep granting), that deactivating the break-glass set is refused under ADR-0024 D5.2 while re-activation never is, and what deactivation deliberately does NOT touch (admin surfaces, write gates, blast-radius reads — filtering those is access-widening). Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MX1qcBzfwZb5wkRrJTNbhH
qq9340100
commented
Aug 14, 2026
Sequencing note — in-flight sibling PR #8709 also edits |
qq9340100
commented
Aug 14, 2026
Veto window closed — maintainer approved the |
Fixes#8613
Both RBAC grant catalogues ship a Deactivate action whose confirmation dialog promises, in all four locales, that access stops. Nothing read the column. Measured on the real resolver before this change: a position seeded
active: falsestill granted its permission sets, and a permission set seededactive: falsestill returnedposture: PLATFORM_ADMINwith its system permissions.Implements the maintainer ruling of 2026-08-14 (comment
5293338173, and the scope widening in5294608710): enforce for both objects, at every true grant-resolution source, never partial.Where it is enforced, and why there
packages/core/src/security/resolve-authz-context.ts— the single seam every transport (REST server, runtime dispatcher, MCP,runAs:'user'automation) resolves authorization through. Both tables were already read there, so the predicates cost zero new hot-path queries.sys_positionsys_position_permission_setgrants, and its name leavespositionssys_permission_setsystem_permissions, notab_permissions, and nohasPlatformAdminGrantplugin-securityDB loaderwhere: { name: { $in: names } }sys_position.activeis only enforceable here. Downstream inplugin-securitythe position to permission-set linkage is already collapsed into a flatpermissionslist, so a set held via a deactivated position is indistinguishable there from one granted directly, and filtering there would over-revoke a set the user also holds in their own right (pinned: "a set held via BOTH a deactivated position and a direct grant still resolves").Dropping the position NAME is not cosmetic:
resolvePermissionSetsForContextrequestscontext.positionsas permission-set names too (position names are commonly reused as set names), so a name left standing would resolve the same grant one layer down.The
plugin-securityloader is not dead codeMeasured, and pinned by
permission-set-active.test.ts: an active position whose name matches a deactivatedsys_permission_setrow arrives at that loader with the name still standing. Core filtered the position catalogue and the sets reached by id; neither judged that row. Assumption A2 in the dispatch is therefore confirmed live, not defence-in-depth-that-nothing-reaches.One predicate, three readers
New:
isRowActivein@objectstack/core(security/row-active.ts), shared by the resolver, the loader, and the break-glass guard that SIMULATES a write to the flag — a guard modelling "deactivated" differently from the resolver would permit exactly the write it exists to refuse.It is "explicitly deactivated", never "explicitly active", and both halves are measured:
activecarriesdefaultValue: true, but a row predating the column, arriving through a migration, or projected with afieldslist that omits it carries no value. Requiringtruewould revoke every such row on upgrade — a silent mass revocation nobody asked for.isGrantActivereads absent bounds as unbounded for the same reason (ADR-0091 D2). Every pre-existing fixture inresolve-authz-context.test.tscarries noactivekey and is the pin for this direction.row.active === falsealone is an enforcement hole. SQLite stores booleans as 0/1; the memory driver round-trips real booleans. The false-set isfalse | 0 | '0' | 'false', and the guard's cases run on a real better-sqlite3:memory:database so the 0 shape is exercised for real, not asserted about.Filtering in memory rather than as an
active: truewhere: the predicate form would ALSO drop NULL rows, and booleanwherecoercion differs per driver. The rows are already fetched, so the answer is identical on every driver at no query cost.Grant-resolution reads ONLY — the correction the escalation flagged is binding
Taken literally, "filter the named
sys_positionpositional lookups" is access-WIDENING. These four seams stay unfiltered, each with a comment naming why:security-plugin.tsassertAudienceAnchorBindingGate— a write gate. Filtering out a deactivatedeveryonerow makes the name resolve empty, the loopcontinues and the gate SKIPS: a tenant-wide anchor binding refused today would be permitted.delegated-admin-gate.tssetsBoundToPosition— a blast-radius read. Dropping rows NARROWS a delegated admin's boundary and lets a delegate act past their scope.delegated-admin-gate.ts:338/:586— administration surfaces. Filtering makes a deactivated position unmanageable, the opposite of the incident-response posture this card is for.A fifth seam was found and classified rather than filtered — see "Findings filed" below.
Break-glass: the prerequisite, closed here
Enforcing the flag makes
plugin-auth'sPERMISSION_SET_STANDING_KEYS = ['name']false, and its #6084 comment asserted in writing that only a payload touchingnamecan move the administrator enumeration. Deactivatingadmin_full_accessun-makes every platform admin at once, by a payload touching neithernamenor any identity table, through a row action with no visibility or condition guard — and re-activating needs the permission just lost (the seeders deliberately never reconcileactive, so no restart restores it).activejoins the standing keys and the stale comment is rewritten.active(fieldsis a projection — a column left out would read as absent, i.e. ACTIVE, and the guard would model an environment where nothing is ever deactivated) and judges it with the same shared predicate.Two things beyond the card's letter, both stated so they can be pushed back on:
admin_full_access那条sys_permission_set行,一次废掉所有 platform admin #6084's zero-administrator exemption reads "nobody to protect, proceed". A deactivated break-glass set produces that emptiness and leaves no dangling grant, so the break-glass 不变量的第四条路径无守卫:删/改名admin_full_access那条sys_permission_set行,一次废掉所有 platform admin #6084 predicate cannot see it — one deactivation would empty the population and wave every later ban, delete and downgrade through. Anadmin_full_accessrow that exists, is correctly named, is switched off, and still has unscoped in-window grants pointing at it is now read as emptied rather than fresh, with its own remedy sentence.sys_positionneeds no analogous guard, and the reason is measured: platform-admin standing is read from UNSCOPEDsys_user_permission_setgrants only (a position-boundadmin_full_accessnever conferred it, in the resolver or in the guard), and org-administrator standing fromsys_member.role. Deactivating a position cannot empty either. Stated in the code so the next author does not have to re-derive it.Behaviour change on deployed data — stated, not discovered
Any
sys_permission_setorsys_positionrow currently carryingactive: falsestops granting the moment this lands, 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. The changeset isminorand carries a**BREAKING**marker, an ADR-0087 disposition, and the pre-upgrade query for listing deactivated rows. A row whoseactiveis absent or NULL is unaffected.No dialog copy changes — the dialogs become true rather than needing rewording — so the serial-after-#8599/#8601 locale-bundle constraint is not re-armed.
pnpm check:i18nconfirms all 9 packages' bundles in sync.packages/specis untouched:activeis row state, not authored metadata (PermissionSetSchemais astrictObjectandpermission-set-projection.ts'sROW_STATE_COLUMNSsays so in as many words). No spec key, export, or stored shape moves.Docs: all 30 flagged pages screened, one gap filled
The
docs-drift-checkset was reproduced locally (node scripts/docs-audit/affected-docs.mjs --json origin/main— 35 entries, of which 5 are release-owned and were not opened or touched) and every one of the 30 hand-written pages was screened fordeactivat|reactivat|停用and foractiveco-occurring with permission-set / position / capability / grant vocabulary.No existing sentence is made wrong by this change. Only four pages carry the vocabulary at all:
permissions/sharing-rules.mdx:189-190sys_sharing_rule.active— a different object whose deactivation already worked. Correct as written, untouched.permissions/permissions-matrix.mdx:164kernel/runtime-services/sharing-service.mdx:62permissions/authorization.mdx:211The other 26 flagged pages: screened, 0 hits — the mapper's breadth here is a three-package dependency edge on
@objectstack/core, not 30 candidates.The PM's four highest-risk candidates, each opened and read:
permissions-matrix.mdx(verdict above),access-recipes.mdx(0 hits — it describes the capability/assignment/requirement split and never the lifecycle of either row),explain.mdx(0 hits, but see #8714 below),authorization.mdx— which is where the one real gap was.The gap was an omission, not an error.
authorization.mdxdocuments ADR-0091 validity windows as the resolution-time grant-lifecycle filter, down to "an expired unscopedadmin_full_accessgrant no longer derivesplatform_admin" — the exact consequence this change adds a second path to. A reader would reasonably conclude there is only one such filter. A sibling section now states the enforced semantics, that assignments are untouched and re-activation restores them, that absent means ACTIVE, that deactivating the break-glass set is refused while re-activation never is, and what deactivation deliberately does not touch (admin surfaces, write gates, blast-radius reads).check:docs-audit-scopeandcheck:role-wordgreen.Verification — all at
6fb96f8d1, the head of this PRRe-run in full after the docs commit, not carried over from the code commit.
Gates re-derived from the actual changed paths (
scripts/pm/dispatch-gates.mjs) and run after the final commit —check:authz-resolver,check:nul-bytes,check:cross-package-test-inputs,check:kernel-hook-pairs,check:test-source-alias,check:type-source-resolution,check:changeset-gate-self-tests,check:objectui-changeset,check:query-options-erasure,check:type-check-coverage,check:docs-audit-scope,check:role-word,check:i18n,check:type-check-debt --re-measure(33 ledger entries,none above its recorded number), pluscheck-adr-0087-registration,check-changeset-no-major,check-empty-changeset: all OK.Reverse verification, direction predicted before running: RED. Reverting only
resolve-authz-context.tstoorigin/main(fix committed first, restored withgit restore --source=HEAD) turns 9 of the new cases red with the deactivated grants resolving —expected [ 'admin_full_access' ] to not include 'admin_full_access'— whilerow-active.test.ts, a pure predicate with no dependency on that seam, stays green. The ACTIVE and ABSENT cases pass in both directions, so the block is not vacuous in either.The break-glass half carries its own reverse verification in-file: unguarded, the deactivation succeeds, every row survives untouched, and the ban of the last administrator then succeeds — the amplification, on the engine as it behaved before this change.
Findings filed (not fixed here)
sys_positionstill receive sharing-rule shares?expandPositionUsersnever reads the catalogue row, so today it does #8710 — a sharing rule with apositionrecipient resolves holders throughexpandPositionUsers, which never reads thesys_positioncatalogue row, so a deactivated position keeps receiving shares. The fifth seam: classified as a genuine ruling question (grant vs directory selector), in a package outside this card's authorized surface, and the only one that would cost a new query.activeflag, and its completeness check cannot notice #8711 — the ADR-0056 D10 authz conformance matrix has no row for this flag, and its completeness ratchet matches HTTP entry points, so it structurally cannot notice a primitive enforced inside an existing resolver.explain.mdx. Reporting, not enforcement, so out of this card's ruling.Generated by Claude Code