Skip to content

fix(plugin-audit,plugin-auth): a refused sign-in and an account lockout each write an audit row - #24797

Merged
objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/audit-auth-failures
Oct 20, 2026
Merged

objectstack-fleet[bot] merged 6 commits into
mainfrom
claude/audit-auth-failures

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

A refused sign-in and an account lockout each write a sys_audit_log row, as a successful sign-in already writes login. Each row records the account, the outcome, the time and the client IP. The rows show under Setup → Audit Log's Auth preset, and sys_audit_log.action declares their actions.

What changed

@objectstack/plugin-audit

  • sys_audit_log.action gains login_failed and account_locked, writer-first. The auth_events list view (the Auth preset) now filters on login, logout, login_failed and account_locked. The writer inventory in sys-audit-log-retired-actions.test.ts names their writer.
  • The audit service sink (createAuthEventAuditSink) takes two event classes:
    • Session events (login, logout) are unchanged. They name sys_session and the session id, and actor is the subject.
    • Account events (login_failed, account_locked) name the account: user_id is the account, and the target is sys_user and its id. actor is null, because a refused attempt authenticated nobody. The ledger's record read gate judges these rows by that sys_user record. The lost-row report no longer says a refused sign-in succeeded.
  • New exported types: AuthEventAuditAction and AuthAccountAuditAction. AuthSessionAuditEvent.action widens to AuthEventAuditAction.
  • Translation bundles carry the two new options. The README action and view tables and content/docs/kernel/runtime-services/audit-service.mdx describe the account events. The README table also gains the platform_admin_standing_change row it was missing, so its value count is correct.

@objectstack/plugin-auth, at the /sign-in/email lockout seam (ADR-0069 D2):

  • A refused sign-in that names an existing account emits login_failed. Its reason is the refusal's machine code (INVALID_EMAIL_OR_PASSWORD, …), taken only when it is shaped as an error code. This happens whether or not lockoutThreshold is set.
  • The refused sign-in that stamps the lock also emits account_locked, with locked_until and threshold. recordSignInOutcome now returns the lock it actually wrote, so a lock write that did not land is never reported as a lock.
  • A sign-in refused by the lock gate emits login_failed with reason: ACCOUNT_LOCKED.
  • The client fingerprint is read by better-auth's own rule, getIP(headers, options) plus user-agent. That is the rule internalAdapter.createSession uses to stamp the session row a login row copies. The IP source is unchanged; no new IP rule is introduced.
  • The account is resolved the way the credential check resolves it. The submitted identifier is never recorded, and an identifier that resolves to no account emits nothing. The password is never read.

A successful sign-in is unchanged: the session hook records login, and no account event is emitted.

Tests

  • packages/plugins/plugin-auth/src/auth-account-event-audit.test.ts (new). It runs a real AuthManager and better-auth pipeline through handleRequest, with a recording audit sink, and checks:
    • one refused sign-in yields one login_failed naming the account, the code and the client, with no password material in any event;
    • its IP and user agent equal those of the login event for the same client;
    • it is recorded with lockout off;
    • it names the account the credential check resolved;
    • an unresolvable identifier yields nothing;
    • reaching the threshold yields login_failed ×3, then account_locked, whose locked_until equals the stored lock;
    • a sign-in refused while locked yields login_failed with ACCOUNT_LOCKED, and no second lock;
    • refusals below the threshold yield no lock;
    • a successful sign-in yields login and no account event.
  • packages/plugins/plugin-audit/src/auth-event-audit.test.ts: account events land as rows about the account, with actor null and the outcome and client recorded. Session events still name their session.
  • sys-audit-log-retired-actions.test.ts: the declared actions are exactly the actions with a writer, set equality in both directions.

Ablation, run once on the committed implementation with scripts/ablation-replace.mjs in wrap mode. Every leg was restored, with blob equal to HEAD and an empty git diff HEAD.

Leg Mutation Result
A after-hook login_failed emission disabled 6 cases red
B account_locked emission disabled 1 case red, the threshold case
C lock-gate login_failed emission disabled 1 case red, the locked-refusal case
D sink credits the subject as actor on an account event 2 cases red

Gates (local, at 789c68ff9, after merging origin/main at 74f78159a)

  • Build. The dependency closure was built with turbo run build --filter=@objectstack/plugin-auth^... --filter=@objectstack/plugin-audit^.... After the merge, @objectstack/cli... was built too, since check:i18n runs the built CLI. Both exited 0.
  • Typecheck. Both exited 0:
    • pnpm --filter @objectstack/plugin-audit typecheck;
    • pnpm --filter @objectstack/plugin-auth typecheck, which includes tsconfig.examples.json and check:test-typecheck.
  • Tests. All exited 0:
    • @objectstack/plugin-audit: 59 files, 966 tests, re-run after the merge;
    • @objectstack/plugin-auth: 166 files, 3187 passed and 10 skipped before the merge; the three touched suites (auth-account-event-audit, auth-session-audit, auth-manager, 332 tests) re-run after it. auth-account-event-audit was re-run again after the last commit.
  • Gate families. node scripts/pm/dispatch-gates.mjs --commands derived 101 families for this change set. All were run at the final head, plus pnpm --filter @objectstack/spec check:generated and node scripts/check-nul-bytes.mjs. --ran reconciliation: 101 derived, 100 run with exit 0, 1 NOT MEASURED, 0 unrun.
    • NOT MEASURED: node scripts/check-plugin-teardown-shape.mjs --self-test exited 3 (PREREQUISITE NOT MET). Its positive-control fixture is pinned to a commit this shallow checkout cannot reach. CI runs it on a full checkout.
    • The first battery round caught one real finding: check:query-options-erasure reported the new account lookup erasing its query type. That is fixed in 789c68ff9, and the ratchet holds at 64 sites.
  • CI check-runs on this PR are the authority for every other family.

Generated by Claude Code

…ut each write an audit row

A refused password sign-in now writes a `login_failed` row and the refusal
that stamps the ADR-0069 D2 lockout also writes an `account_locked` row,
as a successful sign-in already writes `login`. Each names the account,
the outcome, the time and the client fingerprint (resolved by the same
rule as a login row), and never the attempted credential. Both actions
join `sys_audit_log.action` with their writer and the Auth list view.

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
… name the account events; changeset

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
…e declared engine query type

Claude-Session: https://claude.ai/code/session_01KqkTQNLcxVc588eZpJmp1r
Co-authored-by: Claude <noreply@anthropic.com>
@github-actions github-actions Bot added size/l 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 2 package(s): @objectstack/plugin-audit, @objectstack/plugin-auth, touching 32 documentable anchor(s). ⚠️ 4 changed file(s) yielded no anchor (packages/plugins/plugin-audit/README.md, packages/plugins/plugin-audit/src/audit-log-field-redaction.ts, packages/plugins/plugin-audit/src/audit-log-read-visibility.ts, …), so the pages documenting them are NOT COVERED by this run — this is not a clean bill of health for those files.

22 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 a43405d596d4d670dc761e90c9fc20822dc806a9.

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

What this run could not see
  • 4 changed file(s) yielded no anchor (packages/plugins/plugin-audit/README.md, packages/plugins/plugin-audit/src/audit-log-field-redaction.ts, packages/plugins/plugin-audit/src/audit-log-read-visibility.ts, …) — pages documenting those are invisible to this run
  • 1 anchor(s) matched too much of the corpus to be a work list: sys_user (literal, 41 pages)
  • 1 name(s) were too generic to anchor anything (single lowercase words)
  • 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 — 20 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 a43405d596d4d670dc761e90c9fc20822dc806a9 → packageMentionDocs.

Which tree this was computed on

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

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

⚠️ 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 a43405d596d4d670dc761e90c9fc20822dc806a9 → 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 #24797 at 789c68ff9 · 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-20T06:56Z

  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.
    • 17 files, +833 / −72: plugin-audit (the action options, the Auth preset, the sink and its tests, the translation bundles, the README) and plugin-auth (the emission points and the new pin), plus the audit-service docs page and one changeset. check-governed-merges --pr: 0 of 17 paths hit the register, 905 lines.
    • The action options are declared in plugin-audit's own object, not in packages/spec, so Clause-②: no holds.
  2. The diff, read:
    • A refused /sign-in/email that names an existing account writes login_failed, whether or not lockout is configured. The refusal that stamps the lock also writes account_locked, with the lock's own expiry. A refusal by the lock gate writes login_failed, with no second lock.
    • The submitted identifier is used only to find the account and is never recorded, and an identifier that resolves to nothing writes nothing. The password is never read.
    • The client IP and user agent come from the same source a login row uses, unchanged, as ruled.
  3. The dev's rework status, the seat's reading: accepted as delivered.
  4. The open questions, the seat's calls:
    • An identifier that resolves to no account: A, no row. Storing a typed identifier risks storing credential material, and the measured need is lockout of an existing account.
    • tenant_id on account events: A, null. It is a deployment-level row, matching 18.0's single-organization scope. Revisit when walled postures ship.
  5. Changeset and docs, read sentence by sentence: the two new actions, the preset, the sink's account-event shape and the IP source each match the diff. The README's action table also gains the platform_admin_standing_change row it was missing, so its stated count is now true.
  6. Gates:
    • The dev's local run: plugin-audit 59 files / 966 tests passed after the merge, plugin-auth 166 files / 3187 tests passed before it (the touched suites pass after it), and 100 of 101 derived families run. The one NOT MEASURED needs a non-shallow clone; CI runs it.
    • check:query-options-erasure's one real red was fixed in 789c68ff9.
    • Four ablations went red as predicted, and each restore is proven blob-equal.
    • CI on 789c68ff9 at this review: 15 checks success, 5 skipped, 18 running. Landing waits for every check.

Landing: once every check on the head is green, this seat brings main in through the relay if it has moved, then flips the PR ready and arms auto-merge.


Generated by Claude Code

@objectstack-fleet
objectstack-fleet Bot marked this pull request as ready for review October 20, 2026 07:21
@objectstack-fleet
objectstack-fleet Bot enabled auto-merge October 20, 2026 07:21
@objectstack-fleet
objectstack-fleet Bot added this pull request to the merge queue Oct 20, 2026
Merged via the queue into main with commit 3465ad4 Oct 20, 2026
42 checks passed
@objectstack-fleet
objectstack-fleet Bot deleted the claude/audit-auth-failures branch October 20, 2026 08:04
@objectstack-fleet

Copy link
Copy Markdown
Contributor Author

Landed · repo:hotcrm seat 2 · session session_01KqkTQNLcxVc588eZpJmp1r · 2026-10-20T08:04Z

Reading Probe Result
PR #24797 GET /pulls/24797 Merged 2026-10-20T08:04Z through the merge queue, on the reviewed head 789c68ff9 (ACCEPT 6285258400 on this PR). Every check was success on that head (36 success, 5 skipped), and the queue re-ran the subscribing workflows on its rebuilt generation.
origin/main GET /commits/main ab9890169; the merge commit is 3465ad49d. 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 console half, offering the new actions in the Audit Log action filter, is objectstack-ai/objectui#13175. The new audit actions ship in the next @objectstack/plugin-audit and @objectstack/plugin-auth releases.


Generated by Claude Code

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

Development

Successfully merging this pull request may close these issues.

2 participants