Skip to content

fix(plugin-auth): the account lockout resolves the account the sign-in credential check resolves - #24843

Merged
objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/lockout-account-resolution
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 4 commits into
mainfrom
claude/lockout-account-resolution

Conversation

@objectstack-fleet

Copy link
Copy Markdown
Contributor

Security fix (maintainer-ordered; no public card; detail withheld under the disclosure ruling).

Clause-②: no

The invariant this restores

ADR-0069 D2's account lockout counts every refused password sign-in against the account the credential check resolved, and enforces an existing lock on every sign-in attempt that resolves to that account, whatever spelling of the identifier the attempt used. The lock gate, the failed sign-in counter, the credential check and the login_failed / account_locked account events name one account.

What changed

@objectstack/plugin-auth, auth-manager.ts, at the /sign-in/email lockout seam:

  • A new private resolver, findSignInAccount(engine, email, fields), is the one answer to "which account does this password sign-in name". It asks sys_user exactly what better-auth 1.7.3's /sign-in/email credential check asks: internalAdapter.findUserByEmail(email.toLowerCase()), an equality match on the lower-cased address, which is the spelling internalAdapter.createUser stores.
  • The lock gate (assertAccountNotLocked), the counter (recordSignInOutcome) and the account events (resolveSignInAccountId) all read the account through it. Each keeps its own failure posture: the gate fails open, and the counter and the ledger report a lost write at error, as before.
  • The lockout state keeps its shape. It stays on the account's own sys_user row (failed_login_count, locked_until), and every write to it is by the row's id, so one account holds one counter. There is nothing to migrate.
  • The gate's and the counter's query options lose their as any casts, so scripts/query-options-erasure-baseline.json records auth-manager.ts at 12, down from 14 (pnpm check:query-options-erasure --update).

Docs. content/docs/permissions/authentication.mdx states that the password-stage counter and lock belong to the account the sign-in's credential check resolves the address to.

Changeset. .changeset/auth-lockout-resolved-account.md, a @objectstack/plugin-auth patch.

Tests

packages/plugins/plugin-auth/src/account-lockout-resolved-account.test.ts (new) drives a real AuthManager and better-auth pipeline through handleRequest, over the in-memory engine this package's better-auth suites share (its equality is exact, as a SQL column's is by default). The threshold is 3, and the lock is read back from the sys_user row. It pins:

  • the spellings used are ones the credential check resolves to the account: a correct password under each signs in to that account's id;
  • refusals under different spellings of one address accumulate toward its one lock: three login_failed and then one account_locked, all naming the account, and the event's locked_until equals the stored lock;
  • a locked account refuses the correct password under every spelling, with 403 ACCOUNT_LOCKED; each refusal records login_failed with reason ACCOUNT_LOCKED, no session is minted, and the stored lock does not move;
  • a lock reached under one spelling refuses the correct password under another;
  • a successful sign-in under any spelling clears the counter;
  • an identifier that resolves to no account changes no sys_user row and writes no account event, and the identifier never reaches the audit sink.

The existing lockout unit cases in auth-manager.test.ts, the account-event cases in auth-account-event-audit.test.ts and the durability cases in durability-swallow-repair.test.ts pass unchanged.

Ablation, run once on the committed fix with scripts/ablation-replace.mjs in wrap mode. One mutation of findSignInAccount's query turned 5 cases red, across the new file and auth-account-event-audit.test.ts. The leg was restored, with the blob equal to HEAD and an empty git diff HEAD. The mutation's detail is withheld under the disclosure ruling.

Gates (local, at 94ea60bd5, after merging origin/main at c3a8e07b6)

  • Build. The dependency closure and the package itself were built with pnpm --filter '@objectstack/plugin-auth...' build, which exited 0. pnpm --filter @objectstack/spec check:generated reported all 14 generated artifacts up to date.
  • Typecheck. pnpm --filter @objectstack/plugin-auth typecheck exited 0. It includes tsconfig.examples.json and check:test-typecheck, whose test layer compiles the new file with no new debt.
  • Tests. pnpm --filter @objectstack/plugin-auth test exited 0: 169 files, 3240 passed and 10 skipped.
  • Gate families. node scripts/pm/dispatch-gates.mjs --commands derived 109 families for this change set, and all were run at the final head. The --ran reconciliation, with every exit code recorded, reads: 109 derived, 108 run with exit 0, 1 NOT MEASURED, 0 unrun.
    • NOT MEASURED: node scripts/check-plugin-teardown-shape.mjs --self-test exited 3. Its positive control is pinned to a commit that this shallow checkout cannot reach. CI runs it.
  • The root test:repo task (the vitest repo project) was not run locally: the diff touches no test of that project, and CI's Test Core runs it.

Generated by Claude Code

…tial check resolves

ADR-0069 D2's lock gate, its counter and the account events now share one
resolver, findSignInAccount, which asks sys_user exactly what better-auth's
/sign-in/email credential check asks. Every refused sign-in that resolves to
an account counts toward that account's one lock, an existing lock refuses
every sign-in that resolves to the account, and a success under any spelling
clears the counter.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
…check resolves

The authentication guide states which account the password-stage counter and
lock are held on, and the changeset records the fix for plugin-auth.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
…resolver removed

The lock gate and the counter no longer cast their sys_user query to any, so
the query-options-erasure ratchet for auth-manager.ts moves down from 14 to 12.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/m documentation Improvements or additions to documentation tests tooling labels Oct 20, 2026
@github-actions

Copy link
Copy Markdown
Contributor

📓 Docs Drift Check

This PR changes 1 package(s): @objectstack/plugin-auth, touching 7 documentable anchor(s).

4 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, a top-level class))
  • content/docs/kernel/runtime-services/audit-service.mdx (via locked_until (literal, a string literal in assertAccountNotLocked; a string literal in recordSignInOutcome))
  • content/docs/kernel/services-checklist.mdx (via AuthManager (symbol, a top-level class))
  • content/docs/permissions/authentication.mdx (via AuthManager (symbol, a top-level class))
What this run could not see
  • 1 anchor(s) matched too much of the corpus to be a work list: sys_user (literal, 41 pages)
  • the SDK route bridge reached 54 of 206 client-bound route-ledger rows — the other 152 have no registrar path: tail to select them, so pages documenting THEIR client methods cannot appear above, on this or any run. Of those 152: 0 are remediable by widening that discovery convention (an in-repo file declares the path; the convention did not scan it); 55 are structural — on a ledger where NOT ONE row is declared in-repo, so no discovery change reaches them at any price; 97 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.
  • a key NAME is not a key, so the hand re-read the line above prescribes can land on the wrong schema. The same spelling is authorable on one governed type and a [REMOVED] tombstone on another for each of active, aria, joins, objects, template, tools and version (censused on [finding] tools is a key on BOTH AgentSchema (tombstoned, dead) and SkillSchema (live, cloud-attested), so a name-based search attributes skill examples to the agent key — it produced a false stop-the-line alarm on PR #19059 #19093 over the liveness ledger's governed types, top-level keys); nothing in a search result distinguishes the two, so a grep hit on a LIVE example reads as evidence about the DEAD key. Measured on fix(spec): the agent.tools liveness row says dead — it claimed live on a key the schema tombstoned #19059: content/docs/ai/agents.mdx was reported as contradicting the agent.tools tombstone over its tools: example at :161, which is inside the defineSkill({ block opened at :155 — the page was already correct. Settle ownership by PARSING the value against both schemas, never by the name: that literal PASSES SkillSchema, and as an AgentSchema it FAILS at tools with the tombstone prescription. ⛔ These names are not the whole class — a key retired through a .strict() guidance map leaves no tombstone in the walked shape and none of them here (tool.category, live as AIToolDefinition.category).

Coarse fallback — 16 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 99e4eaa227e0375b59c7bbdf6e36ad7f026bb22d → packageMentionDocs.

Which tree this was computed on

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

node scripts/docs-audit/affected-docs.mjs --json 99e4eaa227e0375b59c7bbdf6e36ad7f026bb22d

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

@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

席位核对

ACCEPT · PR #24843 at 94ea60bd5 · repo:hotcrm seat 2 (the maintainer's direct dispatch; no public card, by the maintainer's ruling) · os-support-ai · session session_01KqkTQNLcxVc588eZpJmp1r · 2026-10-20T09:46Z

  1. Shape:
    • Draft, base main. It opens with the mandated security first line and Clause-②: no, and the body carries no closing keyword and names no issue. Assignee os-support-ai.
    • 5 files, +306 / −27:
      • auth-manager.ts;
      • one new pin file;
      • one docs paragraph;
      • one changeset;
      • the query-options-erasure baseline, which goes down by 2, is declared in the body and is the gate's own --update.
    • check-governed-merges --pr: 0 of 5 paths hit the register, under the human-merge threshold.
    • The 4 commits carry the model-free trailer pair, with 0 model-identifier lines.
  2. The diff, read:
    • One private resolver, findSignInAccount, now answers "which account does this password sign-in name". The lock gate, the counter and the account-event resolver all read through it, and none keeps a lookup of its own.
    • Its query is the one the account-event resolver from PR fix(plugin-audit,plugin-auth): a refused sign-in and an account lockout each write an audit row #24797 already used, so that path's behaviour is unchanged, and the gate and counter now agree with it.
    • Each caller keeps its failure posture: the gate fails open on a lookup error, and the counter and the ledger report a lost write at error.
    • The typeof id === 'string' checks moved into the resolver, so the callers' narrower reads are equivalent.
    • The lockout state's shape and storage are unchanged, and every write is by row id. There is nothing to migrate, as the changeset says.
  3. The open question, the seat's call: A, as landed. One resolver issuing the vendor's own query follows the file's existing pattern and keeps one answer in one place. The real-pipeline pins go red if the vendor's lookup changes. B adds a read on the hot path and couples the private helpers to the hook context.
  4. Changeset and docs, read sentence by sentence: each bullet matches the diff. The docs paragraph states the guarantee (the counter and lock belong to the account the credential check resolves) and nothing beyond it.
  5. Gates:
    • The dev's local run on 94ea60bd5, with origin/main merged in:
      • the closure build and check:generated: exit 0;
      • the package typecheck: exit 0, with no new test-typecheck debt;
      • the package tests: 169 files, 3240 passed, 10 skipped;
      • the derived gate families: 109, of which 108 ran with exit 0. One is NOT MEASURED locally (check-plugin-teardown-shape --self-test, which needs a non-shallow clone); CI runs it.
    • Red before the fix: 4 of the 6 new cases. One ablation of the resolver's query went red on 5 cases, and the restore is proven blob-equal.
    • CI on 94ea60bd5 at this review: 14 checks success, 6 skipped, 19 running. Landing waits for every check.

Landing: once every check on the head is green, this seat flips the PR ready and arms auto-merge. This repo's merge queue rebuilds on the current main.

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 10:17
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 10:17
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit c849af2 Oct 20, 2026
43 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/lockout-account-resolution branch October 20, 2026 10:48
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Landed · repo:hotcrm seat 2 · session session_01KqkTQNLcxVc588eZpJmp1r · 2026-10-20T10:49Z

Reading Probe Result
PR #24843 GET /pulls/24843 Merged 2026-10-20T10:48Z through the merge queue, on the reviewed head 94ea60bd5 (ACCEPT 6288163164 on this PR). Every check was success on that head (36 success, 6 skipped), and the queue re-ran the subscribing workflows on its rebuilt generation.
origin/main GET /commits/main b3fc122f0; the merge commit is c849af28e. Its message carries 0 model-identifier lines.

State: there is no card to close. This was the maintainer's direct dispatch, with no public card by their ruling. The fix ships in the next @objectstack/plugin-auth release.

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/m tests tooling

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants