Skip to content

fix(plugin-auth): tombstone interactive session revocations instead of deleting the row - #7825

Merged
huangyiirene merged 4 commits into
mainfrom
claude/issue-7732-interactive-revoke-tombstone
Aug 11, 2026
Merged

fix(plugin-auth): tombstone interactive session revocations instead of deleting the row#7825
huangyiirene merged 4 commits into
mainfrom
claude/issue-7732-interactive-revoke-tombstone

Conversation

@os-help

@os-help os-help commented Aug 11, 2026

Copy link
Copy Markdown
Collaborator

Fixes #7732

The defect

sys_session.revoked_at / revoke_reason are declared readonly and documented
"System-managed", and revoked_at's description names all four causes they capture:
idle / absolute-max / concurrent-cap / admin (ADR-0069 D4).

Three of them honour it. auth-manager.ts expires the row in place — expires_at into
the past plus both columns — from enforceSessionControls (idle / absolute-max) and
enforceConcurrentCap.

The fourth could not. An admin or user-initiated revoke reaches better-auth's
internalAdapter.deleteSession / deleteUserSessions, which delete the row, and a
deleted row carries no revoke_reason. The admin cause was unrecordable by
construction, so D4's audit trail was inert exactly where an audit most wants it.

Direction of record from triage (2026-08-11): restore-invariant, not a new decision —
the interactive revoke stamps the columns in place, matching the automatic path.
sys-session.object.ts's field declarations are unchanged.

What this does

Two rules, both at the better-auth → ObjectQL adapter, both in
packages/plugins/plugin-auth/src/session-tombstone.ts (whose module header carries the
full argument and the measurements below).

1. A delete under an interactive-revoke endpoint is a revocation, not a deletion.
Five routes, ledgered by better-auth's own endpoint path:

route revoke_reason
POST /revoke-session user_revoked
POST /revoke-sessions user_revoked
POST /revoke-other-sessions user_revoked
POST /admin/revoke-user-session admin
POST /admin/revoke-user-sessions admin

Why two values. revoke_reason is free text (maxLength: 64) whose description gives
an open-ended list — idle_timeout, absolute_max, concurrent_cap, … — so there is no
closed enum to violate, and the field is the only thing in the row that says who ended
the session. Recording admin for a user signing out their own other device would not be
a vague audit record, it would be a wrong one: it names an actor class that took no
action. admin therefore means exactly the two /admin/* routes.

2. A revoked session is not a session — a tombstoned row is invisible to better-auth's
own session reads. That is what makes rule 1 worth anything; see "GC, measured" below.

Everything else is byte-for-byte as before: sign-out still deletes, natural expiry is still
collected, ban / password-reset / two-factor session drops are untouched, and any write
with no endpoint context in scope degrades to a plain delete.

Why the seam is the adapter

Verified against better-auth 1.7.0-rc.2, the version this package pins.

The cause is read from getCurrentAuthContext().path — better-auth dispatches every
endpoint inside runWithEndpointContext(internalContext, …) where internalContext.path
is the endpoint's own declared path, so it is request-scoped, authoritative, and equally
present for a programmatic auth.api.* call.

GC, measured

better-auth 1.7.0-rc.2 has no scheduled sweeper of session rows. Its one expiry-driven
collection in the entire library is inside GET /get-session: on finding a row whose
expiresAt has passed it calls internalAdapter.deleteSession(token) "to clean up the
session". That single line is what makes the automatic path's stamps best-effort today,
and it would have eaten an interactive tombstone the moment the revoked client polled
once — seconds, for a browser session. Stamping alone would have satisfied the letter of
D4 and left the trail as inert as it was.

The collector only fires on a row findSession returned. Hiding the tombstone therefore
ends the session harder than expiring it — findSession answers null, the request is
unauthenticated, deleteSession is never called, and the record survives. It also removes
two problems a "refuse the delete" rule would have created: the delete hooks would
otherwise re-fire on every stale-cookie poll (re-dispatching back-channel logout), and a
later revoke-sessions sweep would re-date an older tombstone.

Cost, stated plainly: revoked rows are retained indefinitely. There is no retention
window, TTL or sweeper for sys_session — here or in better-auth. That is not a new
class of growth (a session abandoned without signing out is already immortal for the
same reason), but a retention policy is genuinely unowned. Called out on the issue rather
than invented here.

Erasure is not collection. User-deletion routes (/delete-user,
/delete-user/callback, /admin/remove-user) see tombstones again and physically remove
them: keeping an audit row about a user the deployment has erased is the wrong trade.

sys_session views

Retaining rows means the Sessions grids would otherwise list a revoked session as if it
were live. My Sessions and All now filter revoked_at is_nullrevoke_session
still makes the row leave the grid, exactly as it did when the row was deleted — and a new
Revoked view exposes revoked_at / revoke_reason, which the issue notes appear in no
listView at all. i18n bundles regenerated, the new label translated in all four locales so
the coverage ratchet does not move.

Tests

session-tombstone.test.ts — 18 cases, real better-auth pipeline through
AuthManager.handleRequest, following session-of-record.test.ts. Every case pins both
directions, because either alone is a defect dressed as a fix: the row survives with its
cause on it, and the cookie stops authenticating.

Ablation, direction predicted in advance, both matched exactly:

  • Stamp removed (reconcileSessionDelete always proceeds) → 6 red, all in the stamping
    half: the three revoke routes, the re-date guard, the poll-survival case, the ledger
    mapping. session-of-record.test.ts stayed fully green — the automatic paths are
    untouched.
  • Hiding removed → 5 red, all in the retention half: the two poll-survival cases, the
    sign-out-after-tombstone case, the hide/erasure unit case, and /revoke-session's
    trailing re-read. /revoke-other-sessions, /revoke-sessions and the re-date guard
    stayed green — correctly, since those are protected by the reconcile guard rather than by
    hiding.

Green locally: plugin-auth 1015/1015, platform-objects 311/311, the full downstream
consumer sweep (--filter '...@objectstack/plugin-auth' --filter '...@objectstack/platform-objects', prefix = consumers) including qa/dogfood 582 and
qa/http-conformance 72, plus check:i18n, check:i18n-coverage,
check:engine-double-contract, check:nul-bytes, ESLint and typecheck.

Not built here

No sys_audit_log writer. #7675 owns whether non-CRUD actions get audit-log writers
(logout is one of its writer-less enum values); this change stays inside sys_session
and plugin-auth's revoke paths so the two cards cannot build the same event twice.
MANAGED_EXTENSION_FIELDS is untouched — the tombstone write is the identity authority's
own isSystem write through the same engine the automatic path uses, so it needs no
ADR-0092 D2 whitelist entry.


Generated by Claude Code

claude added 4 commits August 11, 2026 15:57
…f deleting

better-auth's revoke-session / revoke-sessions / revoke-other-sessions and the
admin plugin's revoke-user-session(s) all end a session by DELETING the
sys_session row, so the `admin` cause ADR-0069 D4 declares
`revoked_at`/`revoke_reason` capture was unrecordable by construction (#7732).

Reconcile the physical write at the better-auth -> ObjectQL adapter, the same
seam #7725 used: under an interactive-revoke endpoint the delete becomes an
in-place stamp in the shape auth-manager.ts already writes (expires_at into the
past + both columns). Hook lifecycle is untouched, so OIDC back-channel logout
still fires.

A tombstone is also hidden from better-auth's session reads, which is what
makes the stamp worth keeping: the only expiry-driven collector in the library
is inside GET /get-session, and it only runs on a row findSession returned. A
hidden row therefore de-authenticates harder AND survives. User-erasure routes
see tombstones again so they are physically removed.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd
Every case asserts both halves of the same claim: the row survives with its
cause on it, AND the cookie stops authenticating. Real better-auth pipeline
through AuthManager.handleRequest, following session-of-record.test.ts.

Covers the three self-service revoke routes end to end, the measured GC (a
revoked client polling /get-session no longer collects its own record), the
non-revoke paths that must stay byte-for-byte (sign-out, natural expiry), the
path ledger for all five revoke routes plus the erasure exemption, and a
conformance check that every ledgered path is one better-auth still mounts.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd
…stones a view (#7732)

Retaining revoked rows means the two sys_session grids would otherwise list a
revoked session as if it were live. Both now filter `revoked_at is_null`, so
revoke_session still makes the row leave the grid exactly as it did when the row
was deleted, and a new Revoked view exposes revoked_at / revoke_reason — the
columns the issue notes appear in no listView at all. Field declarations are
untouched.

Changeset + regenerated i18n bundles for the new view label (translated in all
four locales, so the coverage ratchet does not move).

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd
…spatch (#7732)

check:engine-double-contract flagged the new double: its update() did not route
through assertEngineUpdateDispatch, and the whole fix IS an update, so a fake
looser than ObjectQLEngine.update could green a write the engine refuses. The
sibling session-of-record.test.ts copy still carries the #5480 DEBT entry; a new
double does not get to inherit it.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BVc1ekPpi6yaWywAUhfzfd
@vercel

vercel Bot commented Aug 11, 2026

Copy link
Copy Markdown

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment
Project Deployment Actions Updated (UTC)
objectstack Ignored Ignored Aug 11, 2026 5:05pm

Request Review

@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 2 package(s): @objectstack/platform-objects, @objectstack/plugin-auth.

9 hand-written doc(s) reference the affected code and may need an implementation-accuracy re-verification:

  • content/docs/deployment/cli.mdx (via @objectstack/plugin-auth)
  • content/docs/deployment/production-readiness.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/contracts/cache-service.mdx (via @objectstack/plugin-auth)
  • content/docs/kernel/services-checklist.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/authentication.mdx (via @objectstack/plugin-auth)
  • content/docs/permissions/sso.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/index.mdx (via @objectstack/plugin-auth)
  • content/docs/plugins/packages.mdx (via @objectstack/platform-objects, @objectstack/plugin-auth)
  • content/docs/ui/setup-app.mdx (via @objectstack/platform-objects)

2 release-owned page(s) also reference the affected code. These are read-only:

  • content/docs/releases/implementation-status.mdx (via @objectstack/plugin-auth)
  • content/docs/releases/v9.mdx (via @objectstack/plugin-auth)

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.

Advisory only. To re-verify, run the docs-accuracy-audit workflow scoped to these files:
node scripts/docs-audit/affected-docs.mjs origin/main → pass the list as args.docs.

@github-actions github-actions Bot added size/l documentation Improvements or additions to documentation tests tooling labels Aug 11, 2026
@huangyiirene
huangyiirene marked this pull request as ready for review August 11, 2026 19:54
@huangyiirene
huangyiirene added this pull request to the merge queue Aug 11, 2026
Merged via the queue into main with commit 3f296bf Aug 11, 2026
26 checks passed
@huangyiirene
huangyiirene deleted the claude/issue-7732-interactive-revoke-tombstone branch August 11, 2026 20:22
huangyiirene pushed a commit that referenced this pull request Aug 11, 2026
…ept-invitation-adopt-membership

Resolved packages/plugins/plugin-auth/src/objectql-adapter.ts: the only
conflict was the import block, where #7825's session-tombstone helpers and
this branch's adoptExistingMembership landed on the same line. Both imports
are kept — the two features hook disjoint adapter methods (adoption on
`create`, tombstoning on `findOne`/`findMany`/`delete`/`deleteMany`), so
neither side may be taken wholesale.
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/l tests tooling

Projects

None yet

3 participants