From b21538c3f53ffbfc33bc27fde0ba35d68e753637 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 22 Aug 2026 15:45:37 +0000 Subject: [PATCH] fix(core): a lapsed sys_member row confers no org role either (#10982) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `resolveUserAuthzGrants` reads `sys_member` once and derives two facts from it — `accessible_org_ids` (ADR-0105 D2) and the org-administration role projection into `positions` (ADR-0095 D3). Only the first applied the ADR-0091 validity window, so a lapsed membership granted no org access while still conferring its better-auth role: two answers from one row. The role projection now drops out-of-window rows BEFORE the derivation — the shape step 6 already gives `sys_user_permission_set`, so a lapsed membership can no more yield `org_owner` than an expired `admin_full_access` can yield `platform_admin`. Fail-closed per ADR-0091 D2. Maintainer ruling, 2026-08-22 live session (item 2): a lapsed membership is no membership, not merely no org access. `sys_member` declares neither bound today and `isGrantActive` reads an absent bound as unbounded, so no shipped row changes answer — asserted directly. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx --- .../member-role-projection-validity-window.md | 43 +++++ .../security/resolve-authz-context.test.ts | 163 ++++++++++++++++++ .../src/security/resolve-authz-context.ts | 27 ++- .../dogfood/test/authz-conformance.matrix.ts | 4 +- 4 files changed, 228 insertions(+), 9 deletions(-) create mode 100644 .changeset/member-role-projection-validity-window.md diff --git a/.changeset/member-role-projection-validity-window.md b/.changeset/member-role-projection-validity-window.md new file mode 100644 index 0000000000..5d3a543c5e --- /dev/null +++ b/.changeset/member-role-projection-validity-window.md @@ -0,0 +1,43 @@ +--- +"@objectstack/core": patch +--- + +A lapsed `sys_member` row now confers no org role either — one row, one answer (#10982) + +`resolveUserAuthzGrants` reads `sys_member` once and derives two facts from it: +`accessible_org_ids` (the `group` posture's read reach, ADR-0105 D2) and the +org-administration role projection into `positions` (ADR-0095 D3). Only the +first applied the ADR-0091 validity window. A membership outside +`[valid_from, valid_until)` was therefore excluded from org access while still +projecting its better-auth role — two answers from one read, and with +`role: 'owner'` the role reaches the `organization_admin` capability that +`derivePosture` reads for `TENANT_ADMIN`. + +The role projection now drops out-of-window rows **before** the derivation, the +same shape `sys_user_permission_set` already had, so an expired membership can +no more yield `org_owner` than an expired `admin_full_access` can yield +`platform_admin`. Fail-closed per ADR-0091 D2. Maintainer ruling, 2026-08-22 +live session (item 2): a lapsed membership is *no membership*, not merely *no +org access*. + +**Why `patch` and not a breaking bump, argued in the open.** This is a real +change of authorization semantics — a membership that used to confer a role +stops conferring it — so the direction is a tightening, and tightenings are the +kind of change that normally earns a major. It is nevertheless `patch` because +the population it can affect is provably empty: `sys_member` declares neither +`valid_from` nor `valid_until` (see `sys-member.object.ts`), and `isGrantActive` +reads an absent bound as unbounded, so **no row any deployment can currently +store is lapsed** and every existing membership resolves exactly as before. That +is asserted directly rather than reasoned about, in +`resolve-authz-context.test.ts` ("a membership with NO bounds is unbounded — +every shipped row is unaffected"), alongside the load-bearing leg that an +in-window membership still projects its role. Landing it now is the cheap +moment: once the columns exist, the same change becomes a migration carrying +live semantics. + +**Not in scope, and deliberately so.** This does not add the validity columns to +`sys_member`, and it does not reach into `sys_user_permission_set` rows that +plugin-security's `reconcileOrgAdminGrant` provisioned from a membership role. +Such a grant is standing authority in its own right with its own ADR-0091 +window; the role is only its provisioning source (ADR-0095 D3). The boundary is +pinned as a measured fact rather than left as an assumption. diff --git a/packages/core/src/security/resolve-authz-context.test.ts b/packages/core/src/security/resolve-authz-context.test.ts index 5b9ce2a512..54ab651d69 100644 --- a/packages/core/src/security/resolve-authz-context.test.ts +++ b/packages/core/src/security/resolve-authz-context.test.ts @@ -355,6 +355,169 @@ describe('grant validity windows (ADR-0091 D1/D2)', () => { const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1'), nowMs: NOW }); expect(ctx.positions).not.toContain('approver'); }); + + // ── #10982 — the sys_member half: ONE row, ONE answer ─────────────────── + // + // `resolveUserAuthzGrants` reads `sys_member` ONCE and derives two facts from + // it: `accessible_org_ids` (the `group` posture's read reach) and the + // org-administration role projection into `positions`. Only the first applied + // the ADR-0091 window, so a lapsed membership granted no org ACCESS while + // still conferring its org-admin ROLE — two answers from one row. + // + // Maintainer ruling, 2026-08-22 live session (item 2): **A — the role + // projection honours the window. A lapsed membership is NO MEMBERSHIP, not + // merely no org access.** Fail-closed per ADR-0091 D2. The fix is one + // `isGrantActive` call in the `activeMembers` filter, placed BEFORE the + // derivation — the shape §6 already gives `sys_user_permission_set`. + // + // ⚠️ `sys_member` declares NEITHER bound today, and `isGrantActive` reads an + // absent bound as unbounded, so no shipped row can be lapsed and none changes + // answer. The unbounded leg below is what says that out loud; without it this + // block would be satisfied by an implementation that filtered everything and + // silently un-admined every real org member. + describe('#10982 — a lapsed sys_member row confers no role either', () => { + it('a lapsed membership projects NO org role, and no org access — one row, one answer', async () => { + const ql = makeQl({ + sys_user: [{ id: 'u1' }], + sys_member: [{ user_id: 'u1', role: 'member', organization_id: 'o1', valid_until: PAST }], + sys_user_position: [], + sys_user_permission_set: [], + }); + const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1', { org: 'o1' }), nowMs: NOW }); + // The half that was already correct. + expect(ctx.accessible_org_ids).toEqual([]); + // The half this card fixes — it used to carry `org_member`. + expect(ctx.positions).not.toContain('org_member'); + // `everyone` is the ADR-0090 D5 audience anchor, held by every + // AUTHENTICATED principal and NOT membership-derived: asserted present so + // the line above cannot pass by the resolver having returned nothing. + expect(ctx.positions).toContain('everyone'); + }); + + it('a not-yet-active membership (future valid_from) projects no role either', async () => { + const ql = makeQl({ + sys_user: [{ id: 'u1' }], + sys_member: [{ user_id: 'u1', role: 'admin', organization_id: 'o1', valid_from: FUTURE }], + sys_user_position: [], + sys_user_permission_set: [], + }); + const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1', { org: 'o1' }), nowMs: NOW }); + expect(ctx.positions).not.toContain('org_admin'); + expect(ctx.accessible_org_ids).toEqual([]); + }); + + // ⛔ LOAD-BEARING. An implementation that dropped every membership row would + // satisfy every lapsed assertion above while un-admining the entire + // installed base. Both rows sit in the SAME organization on purpose: the + // active-org filter cannot explain either verdict, so the ONLY thing + // separating them is the validity window — a blanket filter fails the first + // assertion, and no filter at all fails the second. + it('an ACTIVE membership still projects its role — the lapsed legs cannot pass vacuously', async () => { + const tables = { + sys_user: [{ id: 'u1' }], + sys_member: [ + { user_id: 'u1', role: 'owner', organization_id: 'o2', valid_until: PAST }, + { user_id: 'u1', role: 'member', organization_id: 'o2', valid_from: PAST, valid_until: FUTURE }, + ], + sys_user_position: [], + sys_user_permission_set: [], + }; + const ctx = await resolveAuthzContext({ + ql: makeQl(tables), headers: H(), getSession: session('u1', { org: 'o2' }), nowMs: NOW, + }); + expect(ctx.positions).toContain('org_member'); // the in-window row still grants + expect(ctx.positions).not.toContain('org_owner'); // the lapsed one, same org, does not + expect(ctx.accessible_org_ids).toEqual(['o2']); // and the two halves agree + }); + + // ⛔ LOAD-BEARING, the safe-to-land-now leg. `sys_member` has no + // `valid_from`/`valid_until` columns, so EVERY shipped row looks like this. + // If this reddens, the change is not a tightening — it is a regression on + // every existing deployment. + it('a membership with NO bounds is unbounded — every shipped row is unaffected', async () => { + const ql = makeQl({ + sys_user: [{ id: 'u1' }], + sys_member: [{ user_id: 'u1', role: 'owner,member', organization_id: 'o1' }], + sys_user_position: [], + sys_user_permission_set: [], + }); + const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('u1', { org: 'o1' }), nowMs: NOW }); + expect(ctx.positions).toContain('org_owner'); + expect(ctx.positions).toContain('org_member'); + expect(ctx.accessible_org_ids).toEqual(['o1']); + }); + + // ── The escalation the card is filed on ────────────────────────────── + // + // Make the lapsed row's role `owner` and the question stops being tidiness: + // `org_owner` is a capability-bearing name. A suite that only checked + // `org_member` would not catch a regression here, so the rung is pinned in + // BOTH directions on the same fixture shape. + // + // ⚠️ Which escalation path this closes, stated precisely, because the two + // differ: `derivePosture` reads HELD CAPABILITY GRANTS, never the + // better-auth role (ADR-0095 D2/D3). The role reaches TENANT_ADMIN by two + // routes — (P) `org_owner` resolves a `sys_position` row whose bound set is + // an org-admin grant, entirely inside this resolver; and (D) plugin-security's + // `reconcileOrgAdminGrant` provisions a direct `sys_user_permission_set` + // row from the role. This fix closes (P). It deliberately does NOT reach + // into (D): that row is standing authority in its own right, carrying its + // OWN ADR-0091 window, and revoking someone else's grant row from a read + // path is not this resolver's job. Both are pinned below so the boundary is + // a measured fact rather than an assumption. + const orgOwnerPositionTables = (memberRow: Record) => ({ + sys_user: [{ id: 'esc' }], + sys_member: [{ user_id: 'esc', role: 'owner', organization_id: 'o1', ...memberRow }], + sys_user_position: [], + sys_user_permission_set: [], + sys_position: [{ id: 'pos_owner', name: 'org_owner' }], + sys_position_permission_set: [{ position_id: 'pos_owner', permission_set_id: 'psO' }], + sys_permission_set: [{ id: 'psO', name: 'organization_admin' }], + }); + + it('escalation, BEFORE: an ACTIVE owner membership still resolves TENANT_ADMIN', async () => { + const ctx = await resolveAuthzContext({ + ql: makeQl(orgOwnerPositionTables({})), headers: H(), + getSession: session('esc', { org: 'o1' }), nowMs: NOW, + }); + expect(ctx.positions).toContain('org_owner'); + expect(ctx.permissions).toContain('organization_admin'); + expect(ctx.posture).toBe('TENANT_ADMIN'); + }); + + it('escalation, AFTER: a LAPSED owner membership resolves MEMBER, not TENANT_ADMIN', async () => { + const ctx = await resolveAuthzContext({ + ql: makeQl(orgOwnerPositionTables({ valid_until: PAST })), headers: H(), + getSession: session('esc', { org: 'o1' }), nowMs: NOW, + }); + expect(ctx.positions).not.toContain('org_owner'); + // The rung falls with the name: `org_owner` never resolves its + // `sys_position` row, so the bound org-admin set is never collected. + expect(ctx.permissions).not.toContain('organization_admin'); + expect(ctx.posture).toBe('MEMBER'); + expect(ctx.accessible_org_ids).toEqual([]); + }); + + it('the boundary: a DIRECT org-admin grant is standing authority and survives the lapse — by design', async () => { + // Route (D). The role is only the PROVISIONING source (ADR-0095 D3); the + // grant row it caused is separate authority with its own window. So the + // role projection goes quiet while the capability keeps resolving. This + // is the correct layering, not a residual hole — but it IS the reason the + // rung can outlive the membership, so it is pinned rather than assumed. + const ql = makeQl({ + sys_user: [{ id: 'esc2' }], + sys_member: [{ user_id: 'esc2', role: 'owner', organization_id: 'o1', valid_until: PAST }], + sys_user_position: [], + sys_user_permission_set: [{ user_id: 'esc2', permission_set_id: 'psO', organization_id: 'o1' }], + sys_permission_set: [{ id: 'psO', name: 'organization_admin' }], + }); + const ctx = await resolveAuthzContext({ ql, headers: H(), getSession: session('esc2', { org: 'o1' }), nowMs: NOW }); + expect(ctx.positions).not.toContain('org_owner'); // the role projection is closed + expect(ctx.permissions).toContain('organization_admin'); // the grant row is not + expect(ctx.posture).toBe('TENANT_ADMIN'); + expect(ctx.accessible_org_ids).toEqual([]); // and access is still withheld + }); + }); }); describe('audience anchors in the resolver (ADR-0090 D5)', () => { diff --git a/packages/core/src/security/resolve-authz-context.ts b/packages/core/src/security/resolve-authz-context.ts index cd376a0f38..991975da6d 100644 --- a/packages/core/src/security/resolve-authz-context.ts +++ b/packages/core/src/security/resolve-authz-context.ts @@ -373,13 +373,26 @@ export async function resolveUserAuthzGrants( } grants.accessible_org_ids = Array.from(accessibleOrgIds); - // Positions come from the ACTIVE org's membership only (unchanged): a role - // held in one organization must not grant its capabilities while the caller - // operates in another. With no active org, every membership contributes — - // exactly the pre-D2 behavior of the org-less read. - const activeMembers = tenantId - ? members.filter((m) => (m.organization_id ?? m.organizationId) === tenantId) - : members; + // Positions come from the ACTIVE org's membership only: a role held in one + // organization must not grant its capabilities while the caller operates in + // another. With no active org, every membership contributes — exactly the + // pre-D2 behavior of the org-less read. + // + // [ADR-0091 D2] Rows outside their validity window are dropped BEFORE the + // role derivation — the same discipline §6 gives `sys_user_permission_set`, + // so a lapsed membership can no more yield `org_owner` than an expired + // `admin_full_access` can yield `platform_admin`. Maintainer ruling + // 2026-08-22 (live session, item 2): a lapsed membership is NO MEMBERSHIP, + // not merely no org access — so this half now answers the same question as + // `accessible_org_ids` above, off the same rows, and (a)'s "correct the + // moment they do" promise covers BOTH derivations rather than one. Fail + // closed (D2). `sys_member` declares neither bound today and `isGrantActive` + // reads an absent bound as unbounded, so no shipped row changes answer. + const activeMembers = members.filter( + (m) => + isGrantActive(m, nowMs) + && (!tenantId || (m.organization_id ?? m.organizationId) === tenantId), + ); for (const m of activeMembers) { if (m.role && typeof m.role === 'string') { for (const raw of m.role.split(',').map((s: string) => s.trim()).filter(Boolean)) { diff --git a/packages/qa/dogfood/test/authz-conformance.matrix.ts b/packages/qa/dogfood/test/authz-conformance.matrix.ts index 48360d6627..7ebb49500a 100644 --- a/packages/qa/dogfood/test/authz-conformance.matrix.ts +++ b/packages/qa/dogfood/test/authz-conformance.matrix.ts @@ -302,9 +302,9 @@ export const AUTHZ_CONFORMANCE: AuthzPrimitive[] = [ // unlike API-key `isExpired`, because a grant row is standing authority // rather than a single credential. { id: 'grant-validity-window', summary: '`valid_from` / `valid_until` on a grant row — a grant outside its half-open window resolves to nothing, at resolution time and with no cleanup job (ADR-0091 D1/D2)', state: 'enforced', - enforcement: 'core/security/grant-validity.ts isGrantActive — ONE half-open [valid_from, valid_until) predicate (UTC; absent bound = unbounded; a PRESENT but unparseable bound fails CLOSED), applied by every reader that turns a window-carrying row into authority: core/security/resolve-authz-context.ts step 4 (sys_user_position) and step 6 (sys_user_permission_set, dropped BEFORE any derivation so an expired admin_full_access cannot yield platform_admin either); plugin-security/explain-engine.ts buildContextForUser (plus isGrantExpired for the dedicated "expired" contributor state); plugin-sharing/position-graph.ts PositionGraphService.expandPositionUsers; plugin-security/delegated-admin-gate.ts held-scope resolution; plugin-auth/last-admin-guard.ts administrator-standing reads; plugin-approvals/approval-service.ts lookupActiveDelegation (sys_approval_delegation); runtime/domains/keys.ts membership check', + enforcement: 'core/security/grant-validity.ts isGrantActive — ONE half-open [valid_from, valid_until) predicate (UTC; absent bound = unbounded; a PRESENT but unparseable bound fails CLOSED), applied by every reader that turns a window-carrying row into authority: core/security/resolve-authz-context.ts step 3 (sys_member — BOTH derivations off the one read: accessible_org_ids and, since #10982, the org-administration role projection), step 4 (sys_user_position) and step 6 (sys_user_permission_set, dropped BEFORE any derivation so an expired admin_full_access cannot yield platform_admin either); plugin-security/explain-engine.ts buildContextForUser (plus isGrantExpired for the dedicated "expired" contributor state); plugin-sharing/position-graph.ts PositionGraphService.expandPositionUsers; plugin-security/delegated-admin-gate.ts held-scope resolution; plugin-auth/last-admin-guard.ts administrator-standing reads; plugin-approvals/approval-service.ts lookupActiveDelegation (sys_approval_delegation); runtime/domains/keys.ts membership check', proof: 'delegation-of-duty.dogfood.test.ts', - note: '[#8811] NO `covers`, and — exactly like the two `active` rows above — that is a statement about the RATCHET rather than an omission: `discover()` enumerates HTTP entry points from a curated per-file probe table, and a predicate inside an existing resolver adds no entry point, so this primitive could never have surfaced as UNCLASSIFIED or STALE during any period it was inert. Per the #8711 ruling (2026-08-15) the invariant\'s advertised SCOPE is narrowed to what the ratchet can check; this row is owed under the hand-maintained half the file header still states. WHAT IS AND IS NOT WINDOW-FILTERED — measured at the call sites, not taken from the filing: the columns are declared on exactly THREE objects (sys_user_position, sys_user_permission_set, sys_approval_delegation), and on all three EVERY authorization-resolution seam applies the predicate, which is what makes this row `enforced` rather than partial. Two reads deliberately do NOT filter and neither is a live hole: (i) `sys_member` carries no window columns at all, so the resolver\'s membership call (which builds accessible_org_ids) is FORWARD-LOOKING, and the org-administration role projection beside it is NOT window-filtered (#8802) — both are no-ops until the columns exist, and last-admin-guard.ts\'s MEMBER standing-key notes already record that the resolver is where the two halves must be reconciled FIRST if they ever land; (ii) plugin-approvals\' own expandPositionUsers projects user_id alone by the #8710 ruling, because approval ROUTING is not grant resolution — that note explicitly forbids "fixing" it with a filter. ⛔ #8802 is a defect IN the enforcement, not in this ledger: it does not change this row\'s state, and closing it leaves the row exactly as owed. Predicate unit-pinned in core/security/grant-validity.test.ts (half-open boundaries, seconds-vs-ms epoch, fail-closed on garbage, camelCase spellings); the resolver halves in core/security/resolve-authz-context.test.ts "grant validity windows (ADR-0091 D1/D2)". Deliberately NOT in HIGH_RISK for the same reason as the `active` rows — that list marks primitives guarding object DATA through a sibling HTTP entry point, and this one guards grant DERIVATION. Unlike those rows an end-to-end proof EXISTS and is now cited (#9377): delegation-of-duty.dogfood.test.ts boots a real stack and asserts the delegate STOPS resolving the delegated sys_user_position at valid_until through the real resolveAuthzContext, and its header now claims this row back — the #7976 mutual-attribution contract is satisfied.' }, + note: '[#8811] NO `covers`, and — exactly like the two `active` rows above — that is a statement about the RATCHET rather than an omission: `discover()` enumerates HTTP entry points from a curated per-file probe table, and a predicate inside an existing resolver adds no entry point, so this primitive could never have surfaced as UNCLASSIFIED or STALE during any period it was inert. Per the #8711 ruling (2026-08-15) the invariant\'s advertised SCOPE is narrowed to what the ratchet can check; this row is owed under the hand-maintained half the file header still states. WHAT IS AND IS NOT WINDOW-FILTERED — measured at the call sites, not taken from the filing: the columns are declared on exactly THREE objects (sys_user_position, sys_user_permission_set, sys_approval_delegation), and on all three EVERY authorization-resolution seam applies the predicate, which is what makes this row `enforced` rather than partial. Two reads deliberately do NOT filter and neither is a live hole: (i) the resolver\'s FELLOW-ORG read (step 5, `sys_member {organization_id}`) projects peer user_ids into `org_user_ids` for identity-table RLS — that is a collaborator roster, not a grant, and it is a different read from the principal\'s own memberships; (ii) plugin-approvals\' own expandPositionUsers projects user_id alone by the #8710 ruling, because approval ROUTING is not grant resolution — that note explicitly forbids "fixing" it with a filter. [#10982] `sys_member` still carries no window columns, so every read of it stays FORWARD-LOOKING — but BOTH derivations off the principal\'s own membership read (step 3) now apply the predicate. Maintainer ruling, 2026-08-22 live session (item 2): a lapsed membership is NO MEMBERSHIP, not merely no org access, so the org-administration role projection is window-filtered BEFORE the derivation exactly as step 6 is. That closed a real defect IN the enforcement rather than in this ledger: the row\'s state was and remains `enforced`, and what changed is that the principal\'s own membership read is no longer one of the unfiltered ones listed above. ⚠️ last-admin-guard.ts\'s MEMBER standing-key notes (`valid_from`/`valid_until` in STANDING_KEY_EXCLUSIONS) still describe the role projection as unfiltered and now UNDER-state the enforcement; the exclusion decision itself is unaffected (that guard counts administrators by GRADE alone), so the prose is stale rather than wrong in outcome. Predicate unit-pinned in core/security/grant-validity.test.ts (half-open boundaries, seconds-vs-ms epoch, fail-closed on garbage, camelCase spellings); the resolver halves in core/security/resolve-authz-context.test.ts "grant validity windows (ADR-0091 D1/D2)". Deliberately NOT in HIGH_RISK for the same reason as the `active` rows — that list marks primitives guarding object DATA through a sibling HTTP entry point, and this one guards grant DERIVATION. Unlike those rows an end-to-end proof EXISTS and is now cited (#9377): delegation-of-duty.dogfood.test.ts boots a real stack and asserts the delegate STOPS resolving the delegated sys_user_position at valid_until through the real resolveAuthzContext, and its header now claims this row back — the #7976 mutual-attribution contract is satisfied.' }, // ── Experimental — declared, NOT enforced (ADR-0049/0056 D8) ─────────── { id: 'field-encryption', summary: 'at-rest field encryption', state: 'experimental',