Skip to content

feat(plugin-auth): a membership that ends takes the session's claim on that organization with it — re-pointed if the user belongs elsewhere, revoked if not - #15879

Merged
hotlong merged 5 commits into
mainfrom
claude/issue-15784-revoke-session-on-membership-end
Sep 5, 2026
Merged

feat(plugin-auth): a membership that ends takes the session's claim on that organization with it — re-pointed if the user belongs elsewhere, revoked if not#15879
hotlong merged 5 commits into
mainfrom
claude/issue-15784-revoke-session-on-membership-end

Conversation

@hotlong

@hotlong hotlong commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

Fixes #15784

Removing a member deleted the sys_member row and left the session alive, for up to seven
days. The security half was already shut per request by the ruling family's boundary card
(#15409): a session whose activeOrganizationId is not backed by a membership resolves
with no active organization. This is the courtesy half — when an admin clicks
"Remove member", that person is actually signed out.

⛔ What this is NOT

It is not the enforcement, and nothing here may be read as making the per-request check
optional. packages/core/src/security/resolve-authz-context.ts is untouched — verifiable
in one line: git diff 59953d5a3 -- packages/core/src/security/resolve-authz-context.ts is
empty. Scope item 3 stands verbatim, and the new module's header says why in the place a
future reader will be standing: a trigger can be missed, an evaluation cannot.

The census below measured a path this trigger does not cover, and it is named in the
module header rather than left to be discovered.

The census came first (scope item 1)

Posted on the card before anything was wired:
#15784 (comment)

Three readings, each with its own control, over objectstack@59953d5a3 and
cloud@3856fbf7c. Two findings shaped the implementation:

  1. The dominant removal path never names the table. better-auth's ObjectQL adapter
    deletes through a variable object name (dataEngine.delete(objectName, …)), so a
    detector scanning for sys_member finds zero delete call sites in production —
    a true answer to a misleading question. The second reading enumerates the generic doors
    instead and classifies each by the abstraction it holds.
  2. A raw driver delete bypasses every engine hook, and there is a real production
    caller: cloud's package-uninstall sample-data purge takes its object name from the
    installed package's manifest, ungated. Filed as cloud#2003; not fixed here.

The controls did work rather than decorate: the generic-door detector failed its own
control by over-firing
on membershipCache.delete(cacheKey) and was tightened before its
numbers were used, and the runtime probe's cascade path first reported row gone=false; NOTHING because a required lookup restricted the user delete — the cascade had never run,
so it had never been measured.

Ruled shape: act on the organization's claim, never on the user

Maintainer ruling, decision batch #49 item 4, option B
(#15784 (comment)):

the user, after the membership ends what happens to the session
still holds another membership the claim is re-pointed to it; ⛔ never signed out
holds one with no organization id (single-org) the claim is cleared; still signed in
holds none at all revoked — next request unauthenticated, Console redirects to login

⛔ Option A (revoke every session of the user) and ⛔ option C (do nothing) are both
rejected on the card. The second row is the director seat's application where the option
text is silent, flagged there as flippable in one line; the measurement turned up no reason
it should be otherwise, so it is implemented as ruled.

Why the seam is an engine hook, not the endpoint

packages/plugins/plugin-auth/src/membership-ended-session.ts registers afterDelete and
afterUpdate on sys_member. The census measured which writers reach that seam:

PATH 0  engine.delete (direct)                  -> beforeDelete,afterDelete   (firing control)
PATH 1  better-auth /organization/remove-member  -> HTTP 200; row gone; beforeDelete,afterDelete
PATH 2  engine.delete multi:true                 -> beforeDelete,afterDelete
PATH 3  cascade via sys_user delete              -> row gone; beforeDelete,afterDelete
PATH 4  driver.delete (raw)                      -> row gone; NOTHING
PATH 5  engine.update re-point organization_id   -> beforeUpdate,afterUpdate

A hook on /organization/remove-member would have covered exactly one row of that table.
This is also the precedent already in the package: last-admin-guard.ts enforces its
invariant with beforeUpdate / beforeDelete on sys_member for the same stated reason.
Both new hooks are after hooks on purpose — a removal last-admin-guard refuses at
beforeDelete (priority 20) must not have its sessions touched.

The revocation reuses the existing mechanism unchanged: expires_at a second into the past
plus revoked_at / revoke_reason, the same shape enforceSessionControls and
enforceConcurrentCap already write. session-tombstone.ts already hides a revoked row
from better-auth's session reads, so findSession answers null and the Console's existing
401 to login redirect handles it. No client change.

Clause-② — the accept-set widening, re-judgeable from the diff

Yes, this widens an accept set, and the reviewer needs two facts to re-judge it:

  • Where the vocabulary lives. There is no carrier in packages/spec — measured:
    grep -rn 'revoke_reason\|RevokeReason' packages/spec/src/ returns nothing. The column is
    Field.text({ maxLength: 64 }) with no Zod enum behind it, so the field description on
    the published system object IS the vocabulary
    , and that is the surface this PR widens
    (packages/platform-objects/src/identity/sys-session.object.ts, plus the four generated
    i18n bundles that mirror it).
  • What the new value is, and why that spelling. organization_membership_ended — the
    first authorization-event reason on a column whose every prior value is a timer
    (idle_timeout, absolute_max, concurrent_cap) or an interactive revoke
    (user_revoked, admin). It is deliberately the same string the API-key arm of this
    same ruling family already mints for this event (authRefusal.reason in
    resolve-authz-context.ts, from [decision · p0] an ex-member API key reads AND writes another organization's rows on the single-kernel wiring under isolated — the wall compares against the caller's own unvetted claim #15256 decision 1A), so one grep finds every place the
    platform acts on a membership ending, across both credential kinds. 29 characters, inside
    the column's 64.

Changeset — derived from the diff, and it diverges from the ruling's literal split

The ruling said @objectstack/spec minor plus the implementing package patch,
"derive the exact set from your diff". The derivation lands elsewhere, and the divergence is
declared rather than quietly taken:

  • @objectstack/spec: nothing. The diff does not touch it, because the reason
    vocabulary has no carrier there (measured above). The ruling's minor was for "the new
    reason value"; that value lives on @objectstack/platform-objects, which therefore takes
    the minor.
  • @objectstack/plugin-auth: minor, not patch. It gains three public exports —
    MEMBERSHIP_ENDED_REVOKE_REASON, endSessionClaimsForEndedMembership,
    registerMembershipEndedSessionTrigger. A new export is a minor by semver; patch would
    have been right had the value lived in spec and the implementing package added no surface.

If either call should be otherwise, it is a one-line change to the changeset.

Ablation — the trigger can go red, proven on disk in both directions

Mutation: guard the registration behind a constant in auth-plugin.ts.

ABLATION_15784_DISABLED occurrences: 2   (expect 2)
unguarded registration occurrences : 0   (expect 0)
HEAD blob 2a247ce41b5a4d6b6b4ff5b7c6eea16afbcb77e1
on disk   2eb579f6e71e331543da117bac13ede29f9b4ccf
MUTATION CONFIRMED ON DISK

Rebuilt, and proven to have reached the artifact the dogfood suite actually consumes:

ablation-dist-preflight: ✓ dist/: marker present in 2 built files -- the ablation is live
                           in the artifact the suite consumes.

Ablated run — all three go red, the first on exactly the right assertion
(revoked.revoked_at, expected null to be truthy: the session survived the removal):

Test Files  1 failed (1)
     Tests  3 failed (3)

Restore leg, proven the same way rather than trusted:

whole-tree status (must be empty):   [empty]
git diff HEAD (must be empty):       [empty]
ABLATION_15784_DISABLED occurrences: 0   (expect 0)
unguarded registration occurrences : 1   (expect 1)
HEAD blob 2a247ce41b5a4d6b6b4ff5b7c6eea16afbcb77e1
on disk   2a247ce41b5a4d6b6b4ff5b7c6eea16afbcb77e1
RESTORE CONFIRMED ON DISK
ablation-dist-preflight --absent: ✓ marker absent from all 12 built files
                                  ✓ tree: working tree clean against HEAD

Acceptance and its three controls

packages/qa/dogfood/test/membership-ended-session-revoke.dogfood.test.ts — real booted
stack, real better-auth, driven through the endpoint the card names. All three controls run
in the same test as the case they control, not in a different run.

  • Acceptance: removed member with no remaining membership → row tombstoned (not
    deleted), revoke_reason == organization_membership_ended, expires_at in the past, and
    GET /auth/get-session on the same token comes back unauthenticated.
  • Control 1: a member of a different organization — session live, claim unchanged.
  • Control 2: an intact member of the same organization — session live, claim unchanged.
  • Control 3: the boundary card's pins still hold —
    packages/core/src/security/resolve-authz-context.test.ts 86 passed,
    packages/rest/src/single-kernel-isolated-session-org-claim-matrix.test.ts 13 passed,
    and the per-request check's own file is byte-identical to base.

Plus packages/plugins/plugin-auth/src/membership-ended-session.test.ts (15 tests) for the
decision table, the shapes that must be no-ops (a different organization, an already-revoked
row, an expired row, a role change), the bounded scan, and the two degradation paths.

Tests, at 61bc592f3

@objectstack/plugin-auth        Test Files  97 passed (97)   Tests  2050 passed (2050)
@objectstack/platform-objects   Test Files  33 passed (33)   Tests   518 passed (518)
@objectstack/dogfood (this file) Test Files  1 passed (1)    Tests     3 passed (3)
typecheck: plugin-auth + platform-objects — Done

Gates — 113 derived, 104 green, 9 NOT MEASURED with reasons

node scripts/pm/dispatch-gates.mjs --repo objectstack-ai/objectstack derived its own
change set (17 paths, three-dot against merge base 59953d5a3) at 61bc592f3. Re-derived
after the docs edits, which pulled in 21 further gates the first derivation did not name,
and again after the patch round below.

Four were genuinely red and are fixed here:

gate what it caught fix
check:doc-authoring six tracker ids in runtime log strings moved to comments; the strings name the mechanism instead
check:objectql-double-limit the new test double ignored the caller's limit applies it, by presence, after the filter
check:where-matcher the same double read a combinator as a field name refuses what it does not implement, and the matcher is now module-scope and liftable
check:system-context-census / check:tenant-audit-census an anchor rotted on a line shift; the write-call-site census moved 219 → 221 re-anchored; census regenerated and its 23 prose figures brought to the measurement

The nine non-zero results, every one NOT MEASURED rather than red:

  • check:partof-closing-keyword, check:single-claim-paths — exit 2, NOT WIRED: both
    need PR_BODY / PR_NUMBER. Their own words: "a wiring or usage failure, NOT a verdict".
    CI supplies the context.
  • check:test-completeness, check:dual-build-cjs-loads, check:published-readme-exports,
    check:skill-examples — exit 3 / PREREQUISITE NOT MET: they read built output the
    whole-repo pnpm build produces. Each says so itself and says it is not a pass.
  • check:react-declaration-parity — needs an objectui manifest and a browser dump.
  • pr-labels.mjs — a usage error; the runner invoked it with no mode flag.
  • check:merge-driverpre-existing and environmental, not this change set: two
    self-test cases about "a gate whose RUNNER is not installed" fail on this host, and
    git diff 59953d5a3 -- scripts/ .gitattributes is empty, so the verdict cannot be a
    function of this diff.

Declared narrowing — verification ran UNLOCKED. scripts/pm/os-verify-lock.sh could not
take the shared verify lock on this host: no usable flock. The shared verify lock is
declared Linux-only (flock is util-linux, and a stock macOS does not ship it), so every
command above was run directly, without the lock — a declared narrowing, not a silent one.
No serialization guarantee held for these runs.

Patch round — check:i18n was red, and the miss was in my gate extraction

CI failed Type Check · consumer gates at the step running pnpm check:i18n. Reproduced
locally before acting rather than fixed on a hypothesis. The first local run answered exit
3 — PREREQUISITE NOT MET
("the workspace CLI is not built … Nothing was checked"), which
is a refusal and not a verdict; with the closure the gate names built, the real verdict was:

  platform-objects               DRIFTED (3)
check-i18n-bundles: 1 bundle problem(s)
  • platform-objects: 3 bundle(s) drifted from the schema

Cause, confirmed by measurement: the three *.source-hashes.generated.ts companions
were left stale by the field-description change — the hashes are keyed on the source
strings. pnpm i18n:extract regenerates both families in one pass and changed exactly those
three files and nothing else; the gate then reported OK (9 package(s) — all bundles in sync). Nothing was hand-edited to reach green.

The three consequence checks --fill=default demands:

gate verdict
check:i18n OK (9 package(s) — all bundles in sync, no undeclared authoring keys)
check:i18n-coverage OK (13 config(s), 621 baselined untranslated string(s), none new)no growth, so no rebaseline
check:i18n-stale-fill OK (10 bundle set(s) — no new stale fills, 0 baselined)

Why this escaped the first round, stated plainly: the tool named the gate and my
extraction dropped it.
dispatch-gates printed check:i18n under Convention-triggered
gates
("edits a file in a package that owns an i18n-extract.config.ts"), a section indented
four spaces; the regex I scraped its output with anchored on a two-space bullet and silently
lost that whole section. Five gates were dropped that way — check:i18n,
check:dispatcher-error-vocabulary, check:query-options-erasure,
check:type-check-coverage, check:type-check-debt. All five have now been run and all
five are green
(check:type-check-debt needed the whole workspace built first:
12 ledger entr(ies) re-measured, 140 raw tsc error(s) total, none above its recorded number). That is the corrected 113-gate total above.

Re-verified at the patch head 61bc592f3, not at the previous one: plugin-auth 2050 tests,
platform-objects 518 tests, the dogfood acceptance file 3 tests, both typechecks Done, and
the full 113-gate union. Pushed append-only — 24536a537 is still an ancestor of the branch
head; no rebase, no force-push, no squash.

Known open, by measurement rather than omission

A raw driver delete bypasses this trigger entirely (census PATH 4), and cloud's
package-uninstall sample-data purge is one such caller — cloud#2003. The per-request
membership check covers it; the courtesy does not. That asymmetry is the whole reason this
card was filed separately from the boundary card, and it is why nothing here may be traded
against that check.


Generated by Claude Code

hotlong and others added 4 commits September 5, 2026 19:44
…efore review)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
#15784)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…regenerate i18n bundles (#15784)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… test double judgeable, re-derive the two censuses (#15784)

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@github-actions

github-actions Bot commented Sep 5, 2026

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/platform-objects, @objectstack/plugin-auth, touching 47 documentable anchor(s). ⚠️ 1 changed file(s) yielded no anchor (packages/plugins/plugin-auth/src/index.ts), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

60 hand-written doc(s) name something this change touched — list omitted above 15 rows. Re-derive on the tree named below: node scripts/docs-audit/affected-docs.mjs --json af29b662b06cc41d5a4f4c1d2cfaf27cb2ef123d.

6 release-owned page(s) also affected — read-only, see AGENTS.md Documentation Guardrails.

What this run could not see
  • 1 changed file(s) yielded no anchor (packages/plugins/plugin-auth/src/index.ts) — pages documenting those are invisible to this run
  • 1 cross-cutting symbol(s) contributed no route anchor: organizationId (7 routes)
  • 1 anchor(s) matched too much of the corpus to be a work list: created_at (literal, 33 pages)
  • 6 name(s) were too generic to anchor anything (single lowercase words)
  • the SDK route bridge reached 61 of 219 client-bound route-ledger rows — the other 158 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 158: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 56 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 102 are undecided (no in-repo declaration, on a ledger that has other in-repo registrars — absence and an unreadable spelling are not distinguishable here). The rows themselves: node scripts/docs-audit/affected-docs.mjs --bridge-coverage
  • a page that states a rule by its inputs shares no identifier with the emitter that implements the rule, so an emitter-only diff cannot list it — not on this run and not on any run. Measured on fix(driver-sql): emit varchar(maxLength) for a text field a declared index keys on #11430: content/docs/protocol/objectql/types.mdx documents the text-family column mapping by the ObjectQL type names it maps FROM (text / textarea / html) while the diff changed createColumn; it went unlisted, and it was the page that diff falsified, in four places. No shared token exists to detect this on, so a rule your change carries has to be re-read by hand in the pages that restate it.

Coarse fallback — 12 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 af29b662b06cc41d5a4f4c1d2cfaf27cb2ef123dpackageMentionDocs.

Which tree this was computed on

This run read content/docs from a8a0cc2927c780bc95c84962ed45c01ecbf67903 — the merge of head 61bc592f3d091ce71b6e01f30d0fd46cbe959450 into base af29b662b06cc41d5a4f4c1d2cfaf27cb2ef123d, 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 a8a0cc2927c780bc95c84962ed45c01ecbf67903 && git checkout a8a0cc2927c780bc95c84962ed45c01ecbf67903
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin af29b662b06cc41d5a4f4c1d2cfaf27cb2ef123d 61bc592f3d091ce71b6e01f30d0fd46cbe959450 && git checkout -B drift-repro af29b662b06cc41d5a4f4c1d2cfaf27cb2ef123d && git merge --no-ff 61bc592f3d091ce71b6e01f30d0fd46cbe959450

node scripts/docs-audit/affected-docs.mjs --json af29b662b06cc41d5a4f4c1d2cfaf27cb2ef123d

⚠️ 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 af29b662b06cc41d5a4f4c1d2cfaf27cb2ef123d → pass the list as
args.docs, on the commit named under Which tree this was computed on.

… the widened revoke_reason description (#15784)

The three translated locales' *.source-hashes.generated.ts files were left stale
by the field-description change: the hashes are keyed on the source strings, so
check:i18n read all three bundle sets as DRIFTED. Regenerated with the whole
pass (pnpm i18n:extract), which emits both families together.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hotlong
hotlong added this pull request to the merge queue Sep 5, 2026
Merged via the queue into main with commit 4ca358d Sep 5, 2026
38 checks passed
@hotlong
hotlong deleted the claude/issue-15784-revoke-session-on-membership-end branch September 5, 2026 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentation Improvements or additions to documentation size/xl tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

revoke a session whose membership ended, so "Remove member" actually signs the person out — the courtesy half of #15409, ⛔ never the enforcement

1 participant