Skip to content

fix(core): a lapsed sys_member row confers no org role either (#10982) - #11088

Merged
os-warren merged 1 commit into
mainfrom
claude/issue-10982-membership-role-validity-window
Aug 22, 2026
Merged

fix(core): a lapsed sys_member row confers no org role either (#10982)#11088
os-warren merged 1 commit into
mainfrom
claude/issue-10982-membership-role-validity-window

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#10982

resolveUserAuthzGrants reads sys_memberonce and derives two facts from that one read. Only one of them applied the ADR-0091 validity window.

derivationwindow beforewindow after
(a) accessible_org_ids — the group posture's read reach (ADR-0105 D2)honouredhonoured
(b) org-administration role projection into positions (ADR-0095 D3)ignoredhonoured

So a membership outside its window granted no org access while still conferring its better-auth role — one row, two answers.

The ruling

Maintainer, 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 change

One isGrantActive call in the activeMembers filter, placed before the derivation — the shape §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. The org-scoping half is unchanged; with no active org every membership still contributes.

⛔ This does not add valid_from / valid_until to sys_member, and does not touch packages/spec.

Premise re-derived on the current tree — one delta, reported not worked around

The card measured at 38bc74ed1. Re-derived at merge base 93304c260: the defect is exactly as filed — both derivations still read the same const members = await tryFind(ql, 'sys_member', …) (one read, line 367), (a) windows at line 370, (b) did not.

⚠️Delta: the dispatch asked for the lapsed-own-membership-among-active-peers golden in resolve-authz-context.batch-equivalence.test.ts to be flipped. That file does not exist on main. PR #10981, which added it, was closed unmerged (superseded by an earlier claim on #10825); PR #10980 was closed too, so neither the batching nor the fixture landed, and #10825 is still open and queued. git log -S across all refs finds the fixture name nowhere. There was no golden to flip and nothing was deleted. The four directions are pinned instead in the file that does exist, resolve-authz-context.test.ts, inside its existing grant validity windows (ADR-0091 D1/D2) block — beside the §4 and §6 cases, which is where a reader looks for this. See "Hand-off" below.

What is pinned — four directions, seven tests

All in resolve-authz-context.test.ts#10982 — a lapsed sys_member row confers no role either.

  1. A lapsed membership projects no role.positions no longer carries org_member; accessible_org_ids is still []. everyone is asserted present so the negative cannot pass by the resolver having returned nothing.
  2. A not-yet-active membership (future valid_from) is equally inert — the window is half-open at both ends, not just the upper one.
  3. Load-bearing: an active membership still projects its role. Both rows sit in the same organization on purpose, so the active-org filter cannot explain either verdict and the window is the only separator: a blanket filter fails the first assertion, no filter at all fails the second.
  4. Load-bearing: absent bounds still mean unbounded.sys_member declares neither column, so every shipped row looks like this one and must be unaffected. This is the leg that says the change is safe to land now.

The escalation case, before and after — measured, not inferred

Fixture: the lapsed row's role is owner, with a sys_position row named org_owner bound to the organization_admin set.

  • before (pre-fix shape, measured during the ablation below): positions: ["org_owner","everyone"], permissions: ["organization_admin"], posture: "TENANT_ADMIN", accessible_org_ids: [] — the card's exact defect, as an envelope.
  • after: org_owner absent, organization_admin never resolved, posture: "MEMBER", accessible_org_ids: [].

⚠️Which escalation path this closes, stated precisely.derivePosture reads held capability grants, never the role (ADR-0095 D2/D3). The role reaches TENANT_ADMIN two ways: (P)org_owner resolves a sys_position row whose bound set is an org-admin grant — entirely inside this resolver, and this fix closes it; (D) plugin-security's reconcileOrgAdminGrant provisions a direct sys_user_permission_set row from the role. This fix 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. A seventh test pins (D) explicitly so the boundary is a measured fact rather than an assumption.

No ADR-0112 envelope is involved anywhere in this change — nothing here throws or returns a refusal; the resolver is fail-closed by omission. So there is no code/status pair to assert, and none is invented.

Ablation — signature predicted in writing first

Predicted before mutating (isGrantActive(m, nowMs) && removed from the filter, restoring main's shape, with an on-disk marker): 5 failed / 55 passed of 60, failures being tests 1, 2, 3, and the two escalation legs; tests 4 (unbounded) and 5 (active owner) predicted to stay green in both directions.

Observed: 5 failed | 55 passed (60) — the exact five predicted, with tests 4 and 5 green. First failure: expected [ 'org_owner', 'everyone' ] to not include 'org_owner'.

On-disk proof of the mutation: marker present in src (1), the fixed-form conjunct gone (0). Restore proved byte-identical — git hash-object on the file returned 991975da6dbcda6d9096cc3a38abcd096265945e, equal to what git rev-parse reports for that same path at HEAD (spelled HEAD, then a colon, then the file path — written out because the angle-bracket placeholder form is stripped from GitHub bodies) — with git diff --exit-code clean and empty porcelain, and then re-run to a real verdict rather than trusted: 60 passed (60).

src vs dist, in the falsifiable form

packages/core/dist did not exist in the worktree at all (the closure build builds core's dependencies), yet the suite ran — the test imports ./resolve-authz-context.js, a relative specifier inside packages/core/src/security/, which never consults the package exports map (that map points exclusively into dist/). The package's vitest config declares no aliases.

Made falsifiable rather than argued: packages/core was then built from the fixed source, so dist/index.js compiled the fixed filter, and only src was mutated. Across all three points — before, during, after — the 12-file dist md5 manifest was identical and the marker string appeared in distzero times. Had the suite resolved through dist it would have stayed 60/60 green throughout; it went 5-red and back. The verdict follows src.

Zero-hit counter-check — positive control first

Positive control, run first: the instrument finds the four known windowed call sites in the resolver (lines 370, 393, 412, 433). It works on this corpus.

Then the zero-hit search, over the closed population rather than a grep count: the raw members array is referenced in exactly three places — the read itself (367), the accessible_org_ids loop (369, windowed at 370), and the activeMembers filter (391, windowed at 393). No derivation off the principal's own membership read bypasses the window any more.

Declared narrowing: this counter-check is scoped to resolveUserAuthzGrants, not to every reader of sys_member.role repo-wide. A repo-wide .role sweep returns ~38 candidate files and a hit count there would be a number, not a fact.

Bounded in-place fix, declared

packages/qa/dogfood/test/authz-conformance.matrix.ts is a ledger of what is enforced, and its grant-validity-window row said in prose that "the org-administration role projection beside it is NOT window-filtered" — which this PR makes false. Leaving it is the stale-explanation-beside-changed-behaviour shape (#11046). Updated: the enforcement field now names step 3, and the note records the ruling. Two by-products: it also cited #8802, which 404s in this repo — a dangling reference, replaced with #10982; and its remaining unfiltered-read list now names the resolver's fellow-org peer read explicitly (see Open question).

⚠️What this exemption cost, stated rather than glossed: it pulled in a gate family the card would not otherwise have — spec-liveness-check.yml triggers on packages/qa/dogfood/**. All four ran and are green below. The file's last commits are #10600 and #9794, neither in flight today.

Breaking vs not — argued in the open

Changeset is patch, and the direction genuinely is a tightening: a membership that used to confer a role stops conferring it, which is normally major-shaped. It is patch because the affected population is provably emptysys_member declares neither valid_from nor valid_until, and isGrantActive reads an absent bound as unbounded, so no row any deployment can currently store is lapsed. That is asserted directly (direction 4), not reasoned about. Landing now is the cheap moment: once the columns exist, the same change becomes a migration carrying live semantics.

Hand-off — please read before #10825 is re-dispatched

The deliberate pin the ruling relied on is gone with PR #10981. Whoever re-lands the batching will capture goldens from a main that already honours the window, so lapsed-own-membership-among-active-peers will record the corrected answer as its baseline and the "flip is an act, not drift" property evaporates silently. The seven tests here are the durable replacement. Noted on #10825.

Verification

Gate union derived on the final commit b21538c3f, clean tree, node scripts/pm/dispatch-gates.mjs with no path arguments (it read the change set from the merge base itself: 4 committed paths, 0 working-tree, 0 untracked). Exit codes captured before any pipe; each row quotes the gate's own verdict line, never a bare exit status.

gateexitits own verdict line
check:authz-resolver0single shared authorization resolver intact; both entry points delegate
check:nul-bytes0self-test: 75 assertions over a temp git repo
check:cross-package-test-inputs (+ script)013 package(s) read outside themselves, all declared
check:kernel-hook-pairs04 dispatched kernel hook(s), each pinned in both files
check:slot-lookup0ratchet holds: 107 unswept site(s), none new
check:test-source-alias0OK — 72 packages with tests scanned
check:type-source-resolution0OK — 77 packages with a tsconfig.json scanned
check:changeset-gate-self-tests0116 assertions
check:objectui-changeset0objectui-range self-test: all checks passed
check:query-options-erasure0ratchet holds: 67 unswept non-test site(s), none new
check:engine-double-contract0OK — 377 pinned, 133 in DEBT, 2 exempt
check:where-matcher0277 matcher(s) discovered, 277 answer correctly
check:type-check-coverage0OK — 65/78 packages type-checked, 13 in DEBT, 1 exempt
check:type-check-debt --re-measure033 ledger entr(ies) re-measured in 246.5s, 1908 raw error(s), none above its recorded number
spec check:empty-state · check:liveness · check:strictness-ledger · check:variant-docs0all four current (pulled in by the dogfood path)
check-adr-0087-registration · check-changeset-no-major · check-ci-filter-parity · check-empty-changeset · check-plugin-teardown-shape · docs-audit/check-affected-docs0each printed its own OK line

No gate printed a refusal — PREREQUISITE NOT MET / cannot run / Nothing was checked scored 0 occurrences, so none of these is an unmeasured green. The two ratchets ran against a fully built workspace closure (turbo run build --filter=./packages/* --filter=./packages/*/*, 70/70 successful) exactly as lint.yml does. check:optional-error-sink is absent by construction — it computes its own population and scores silent for every card.

⚠️check:type-check-debt reported a pre-existing surplus of 12 in @objectstack/plugin-auth's TEST_DEBT entry. Not this diff — this PR touches no plugin-auth file — and PR #11038 is open on that package, so lowering it here would be claiming another lane's work.

Tests, on b21538c3f:

suiteresult
@objectstack/core (full package suite)37 files, 894 passed
resolve-authz-context.test.ts alone, verbose60 passed, including the 7 new
@objectstack/dogfoodtypecheck (tsc --noEmit)0 errors — script echoed, so this is a real run, not the no-match-exits-0 trap

Everything heavy ran through scripts/pm/os-verify-lock.sh; the wider farm is CI's run.

Open question I am not deciding here

The resolver's fellow-org read (step 5, sys_member {organization_id}org_user_ids, the collaborator roster identity-table RLS scopes to) is not window-filtered. I have recorded it in the ledger as a roster rather than a grant — the same reasoning the #8710 ruling applied to approval routing — but whether a lapsed peer should stay visible to their former colleagues is an authorization-semantics call of exactly the kind this card's ruling settled for the role projection, and it is a different read from the one the ruling scoped. Flagged for the maintainer rather than decided by me, and no code here touches it.

Out-of-scope finding

packages/plugins/plugin-auth/src/last-admin-guard.ts — its STANDING_KEY_EXCLUSIONS note for sys_member.valid_from states that "the org-administration role projection it feeds positions from is NOT window filtered", which this PR makes stale. The exclusion decision is unaffected (that guard counts administrators by grade alone), so this is prose accuracy, not a live defect. Not fixed here: plugin-auth is out of this lane and PR #11038 is open on it. Filed separately and linked below.


Generated by Claude Code

`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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01PnJHU45vPJj5UQrxe946Bx
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/core, @objectstack/dogfood, touching 1 documentable anchor(s).

2 release-owned page(s) name something this change touched. These are read-only:

  • content/docs/releases/v16.mdx(via resolveUserAuthzGrants (symbol))
  • content/docs/releases/v17.mdx(via resolveUserAuthzGrants (symbol))

content/docs/releases/ is RELEASE-OWNED (AGENTS.md "Documentation Guardrails"): release
notes are written centrally at release time, and a code PR that edits them is the exact PR
that guardrail exists to stop. They are still audited — read-only. If one of them is actually
wrong, file an issue or open a dedicated docs-only PR; do not edit it here.

What this run could not see
  • 2 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 45 of 221 client-bound route-ledger rows — the other 176 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run: node scripts/docs-audit/affected-docs.mjs --bridge-coverage

Coarse fallback — 24 page(s) merely mention a changed package (the pre-#9192 predicate, kept for the deliberately-wide backstop): node scripts/docs-audit/affected-docs.mjs --json ab47f6974a275586355f06abd83a0975b8637d12packageMentionDocs.

Which tree this was computed on

This run read content/docs from 4134c5b1229602b4561bbe33c0e66e9ffebfbc2a — the merge of head b21538c3f53ffbfc33bc27fde0ba35d68e753637 into base ab47f6974a275586355f06abd83a0975b8637d12, which is what actions/checkout gives a pull_request run. Not the PR head.

A worktree cut from an older main holds a different content/docs, so re-deriving there can legitimately return a different list — that is a different tree, not a wrong row. To answer on the same tree:

# while this PR is open — GitHub drops the merge commit once it closes
git fetch origin 4134c5b1229602b4561bbe33c0e66e9ffebfbc2a && git checkout 4134c5b1229602b4561bbe33c0e66e9ffebfbc2a
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin ab47f6974a275586355f06abd83a0975b8637d12 b21538c3f53ffbfc33bc27fde0ba35d68e753637 && git checkout -B drift-repro ab47f6974a275586355f06abd83a0975b8637d12 && git merge --no-ff b21538c3f53ffbfc33bc27fde0ba35d68e753637
node scripts/docs-audit/affected-docs.mjs --json ab47f6974a275586355f06abd83a0975b8637d12

⚠️ That checkout carried uncommitted changes, so the commit above does not fully identify what was read.

Advisory only, and a precision-first one (#9192): a page is listed because it names a
symbol, wire route or SDK method this diff touched — not because it mentions a changed
package. Each row says which anchor put it there, so a wrong row is reportable rather than
merely annoying. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs ab47f6974a275586355f06abd83a0975b8637d12 → pass the list as
args.docs, on the commit named under Which tree this was computed on.

@os-warrenClaude

Copy link
Copy Markdown
CollaboratorAuthor

CI is green on b21538c3f, verified by job name. All check runs terminal, zero failures; the only non-success conclusions are Console Pin Gate and Build Docs, both skipped. The Spec property liveness job — the gate family the declared bounded in-place fix pulled in via packages/qa/dogfood/** — is green in CI, not merely locally.

Verified by enumerating jobs rather than reading a check-suite conclusion: two check_suite.completed: success events arrived on this head while three jobs were still running, so the suite conclusion was not the answer.

This PR stays draft on purpose and is NOT waiting on me. The card carries needs:contract-review; draft is the compensating control for developing a Clause-② card below CONTRACT_REVIEW_TIER. It is now waiting on a non-dispatching seat's contract review — nothing else blocks it.

As the PM seat that dispatched this card, I will not mark it ready, will not arm auto-merge, and will not clear the label. Earlier today I did exactly that on PR #11064 — flipped a PR in this same posture to ready and armed auto-merge, and it merged with zero reviews recorded, leaving the label in place so the control still looked intact. Recording that here so this PR's posture is read as deliberate rather than stalled.

Note for the reviewer: the PR body flags one open question the seat deliberately did not decide — whether the resolver's fellow-org peer read (step 5) should also honour the window. No code here touches it.


Generated by Claude Code

@os-warren
os-warren marked this pull request as ready for review August 22, 2026 20:25
@os-warren
os-warren added this pull request to the merge queueAug 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

⛔ merge queue 构建失败 — 先分诊,再决定要不要重排

队列构建 32596638074 红了。队列跑的是全量套件(PR 侧 CI 只跑 affected 子集),
所以失败的测试可能在本 PR 没碰过的包里 —— 那不是重排能修的。每次盲目重排都会让排在后面的所有 PR 重建一轮。

失败的 job(日志抽取,best effort):

  • Console Pin Gate — 失败步骤: Build the Console SPA at the pinned objectui SHA

    ✗ Build failed in 6.02s
    

↳ 失败原因 是判读的关键:超时Test timed out in … / Hook timed out in …)多半是负载/时序,不是本 PR 的回归;
断言AssertionError: …)才指向真实的行为改变。两者的 FAIL 行长得一模一样,只有这一行能区分。

跨 PR 相同签名(24h,按失败测试文件聚合):

  • ⚠️本次没有可用的聚合签名(日志里没有能解析出测试文件名的 FAIL 行)—— 这不是「没有同签名的其他 PR」,是这一轮没测到。跨 PR 聚合本次不可用,请手工比对其他 PR 的同类评论。
  • ⚠️ 24h 评论账本没读完(超过 5 页仍未读到窗口尽头),所以上面的「不同 PR 数」是下界,不是全量。

历史信号:

  • 本 PR 过去 24h 无队列失败记录(首次)。
  • 过去 24h 队列共有 98 个失败构建(不含本次)。

分诊清单:

  1. 失败测试在本 PR 改动的包里 → 真回归,修 PR。
  2. 失败测试与本 PR 无关 → 看上面的「跨 PR 相同签名」;已有汇总 issue ⇒ flaky/环境问题实锤,去那张 issue 上谈,修好前重排只会再烧一轮全队列。
  3. 两者都不是 → 可能与同组 PR 语义冲突;等前面的 PR 落地或失败出队后再重排一次即可,不要连续重排。

Generated by Claude Code · merge-queue-triage workflow (#4859)

Merged via the queue into main with commit 795ea05Aug 22, 2026
33 checks passed
@os-warren
os-warren deleted the claude/issue-10982-membership-role-validity-window branch August 22, 2026 20:39
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/mteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

sys_member's role projection ignores the ADR-0091 validity window while accessible_org_ids honours it — one row, two answers

2 participants

@os-warren@claude