Skip to content

fix(plugin-auth): refuse a /revoke-session that identifies no record instead of answering success - #10070

Merged
os-warren merged 3 commits into
mainfrom
claude/issue-9714-revoke-session-zero-match-answer
Aug 19, 2026
Merged

fix(plugin-auth): refuse a /revoke-session that identifies no record instead of answering success#10070
os-warren merged 3 commits into
mainfrom
claude/issue-9714-revoke-session-zero-match-answer

Conversation

@os-warren

Copy link
Copy Markdown
Collaborator

Fixes#9714

Premise re-verified first (binding condition from the triage grading)

The card's { status: true } reading was inherited from #8018's rc-era measurement; the unlock scan required re-measuring on the stable vendor line before any fix. Measured on this branch:

  • Installed vendor: better-auth 1.7.1 (read from the installed package, node_modules/.pnpm/better-auth@1.7.1_*/…/package.json — not from any card; the card said 1.7.0-rc.2, a sibling comment said 1.7.0, both stale).
  • Dist reading (dist/api/routes/session.mjs): the revoke-session handler's success line is unconditional — the delete runs only when findSession(token)?.session.userId === ctx.context.session.user.id, and return ctx.json({ status: true }) runs regardless. 1.7.1 added an ownership guard the rc line did not have, which widened the silent no-op: a foreign token now also skips the delete and still answers success.
  • Behavioural measurement, network-free, through the real pipeline (AuthManager.handleRequest → better-auth 1.7.1 → ObjectQL adapter), before the fix existed:
    • token matching zero rows → 200 {"status":true}
    • another user's live token → 200 {"status":true}, target row untouched

Premise holds on the stable line. premise_still_valid: true.

The fix

A hooks.before admission gate (the established seam — same as the /organization/remove-member and /sso/register gates, and before-hook for the same reason: an after-hook can replace a body but not a status). On /revoke-session, the guard asks the vendor's own admission predicate through the vendor's own internalAdapter.findSession call; when the token identifies no session belonging to the caller, it refuses with 404, code RESOURCE_NOT_FOUND (ADR-0112: code AND status). When it admits, the vendor's own condition holds on the same data and the success answer is true. Full reading in packages/plugins/plugin-auth/src/revoke-session-match-guard.ts, including:

Scope note: /revoke-sessions and /revoke-other-sessions match by the caller's user id and cannot mis-identify a record, so they are deliberately untouched. The server-resolved revoke mechanism stays out per the card's own note. The admin plugin's /admin/revoke-user-session has the same defect class upstream and is filed separately as #10069 — it is not addressed here and #9714's scope does not cover it.

Verification (all at head 940a9606b unless noted)

  • New pins (revoke-session-match-guard.test.ts, real better-auth pipeline, 8 tests): zero-match → 404 + code; foreign token → byte-identical answer + row survives untombstoned; matching revoke still succeeds end to end (tombstone written, cookie dead — preservation pin); double revoke → 404; unauthenticated → vendor 401, never this 404; empty-string token → 404; 2 unit tests on the reproduced predicate.
  • Ablation (fix committed first; mutation = early return neutering only the guard): predicted signature stated first — the four negative pins fail "expected 404, received 200", matching/401/unit pins keep passing. Observed exactly that: 4 failed | 4 passed, every failure AssertionError: expected 200 to be 404. Restored byte-identically, proven by git hash-object (25737f9d09fc87155a94e47868303869d868148e pre and post). No rebuild was required for the mutation to be observable: the suite imports ./auth-manager as an in-package relative source import, resolved by vitest from src/, never through dist.
  • Package suite: pnpm --filter @objectstack/plugin-auth testTest Files 59 passed (59) · Tests 1320 passed (1320) (includes the auth-route-ledger conformance suite — the route table is unchanged; a before-hook adds no route).
  • Typecheck/debt ratchet at 940a9606b: check-type-check-coverage --re-measure: OK — 33 ledger entr(ies) re-measured …, none above its recorded number. plugin-auth stays at exactly 109; this PR contributes 0 errors.
  • Gate union re-derived from the actual diff (node scripts/pm/dispatch-gates.mjs, no hand-fed paths) and run to completion at 940a9606b — all green, verdict lines quoted: check-engine-double-contract: OK — 325 pinned, 133 in the DEBT ledger, 2 exempt. (the ledger learned the new pinned double in its own commit); where-matcher conformance holds: 263 matcher(s) … (158 refuse); no lowercase error codes in 4288 scanned file(s) (ADR-0112); Route-envelope conformance — 10 route module(s) audited: 7 conformant, 2 ratcheted, 1 exempt; check-nul-bytes: OK; changeset gates (no major, no empty, ADR-0087 adds no declared-breaking changeset) all green; check:cross-package-test-inputs OK: 12 package(s) read outside themselves, all declared; plus slot-lookup, test-source-alias, type-source-resolution, query-options-erasure, docs-audit — all OK.
  • Repo-wide pin-sweep for assertions of the old success shape on this route: no { status: true } pins exist outside plugin-auth (checked packages/, apps/, examples/, including packages/rest and packages/client — the SDK's sessions.revoke returns res.json() without asserting the shape; also swept the objectui checkout: nothing).

Changeset

.changeset/revoke-session-zero-match-refusal.md — patch, states the new answer plainly for a release-notes reader, mechanism only.


Generated by Claude Code

…instead of answering success
better-auth 1.7.1's revoke-session handler skips the delete when the
supplied token matches zero rows (or another user's row) and still
answers 200 { status: true } — its success line is unconditional. A
before-hook admission gate now refuses those requests with 404
RESOURCE_NOT_FOUND (ADR-0112: code and status together), asking the
vendor's own predicate through the vendor's own adapter call. Zero-match
and foreign-token refusals are byte-identical (no existence oracle);
unauthenticated callers, non-string tokens and adapter read failures
fall through to the vendor's own answers.
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 1 package(s): @objectstack/plugin-auth, touching 6 documentable anchor(s).

3 hand-written doc(s) NAME something this change touched and may need an implementation-accuracy re-verification:

  • content/docs/kernel/contracts/auth-service.mdx(via AuthManager (symbol))
  • content/docs/kernel/services-checklist.mdx(via AuthManager (symbol))
  • content/docs/permissions/authentication.mdx(via AuthManager (symbol))
What this run could not see
  • 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 — 11 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 19031388db12560343600e7347ddda12f375d45epackageMentionDocs.

Which tree this was computed on

This run read content/docs from 5609711d197da76d7e6d20e5572421a523014a31 — the merge of head 940a9606b12eab9c13f5f9d9f42f9f8d3b3916f6 into base 19031388db12560343600e7347ddda12f375d45e, 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 5609711d197da76d7e6d20e5572421a523014a31 && git checkout 5609711d197da76d7e6d20e5572421a523014a31
# afterwards, rebuild it from the two parents, which stay fetchable
git fetch origin 19031388db12560343600e7347ddda12f375d45e 940a9606b12eab9c13f5f9d9f42f9f8d3b3916f6 && git checkout -B drift-repro 19031388db12560343600e7347ddda12f375d45e && git merge --no-ff 940a9606b12eab9c13f5f9d9f42f9f8d3b3916f6
node scripts/docs-audit/affected-docs.mjs --json 19031388db12560343600e7347ddda12f375d45e

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

@github-actionsgithub-actionsBot added documentation Improvements or additions to documentation tests tooling labels Aug 19, 2026
@os-warren
os-warren marked this pull request as ready for review August 19, 2026 20:58
@os-warren
os-warren added this pull request to the merge queueAug 19, 2026
@github-merge-queue
github-merge-queueBot removed this pull request from the merge queue due to failed status checks Aug 19, 2026
@os-warren
os-warren added this pull request to the merge queueAug 19, 2026
Merged via the queue into main with commit 03fa4c9Aug 19, 2026
26 checks passed
@os-warren
os-warren deleted the claude/issue-9714-revoke-session-zero-match-answer branch August 19, 2026 21:24
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

documentationImprovements or additions to documentationsize/lteststooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

better-auth revoke-session answers { status: true } when it matches and deletes nothing — the server-side half of #8018, now tracked nowhere

2 participants

@os-warren@claude