diff --git a/.changeset/audit-login-logout-writers.md b/.changeset/audit-login-logout-writers.md new file mode 100644 index 0000000000..a380236243 --- /dev/null +++ b/.changeset/audit-login-logout-writers.md @@ -0,0 +1,48 @@ +--- +"@objectstack/plugin-audit": minor +"@objectstack/plugin-auth": minor +--- + +feat(audit): sign-in and sign-out are recorded in `sys_audit_log`, with the actor and the tenant (#8144) + +`sys_audit_log.action` declares `login` and `logout`, the shipped `auth_events` +list view filters on them, and two System Overview dashboard widgets chart them — +but **nothing in the platform ever wrote either row**. The audit writers subscribe +to the ObjectQL CRUD lifecycle, so `create`/`update`/`delete`/`restore` were the +only actions that could ever materialize. On a fresh boot, signing in and then +querying `GET /api/v1/data/sys_audit_log?$filter={"action":"login"}` returned +**total 0**, and the `auth_events` view was empty by construction. + +The whole trace a sign-in left behind was one **unattributed** `update sys_user` +row (`user_id` null) diffing `last_login_at` — a compliance ledger recording that +somebody, unknown, had signed in. + +Both halves are fixed: + +- **`login` on every session creation.** The writer is wired to better-auth's + `session.create` database hook rather than to the `/sign-in/email` endpoint, so + it covers every way a session is minted — email sign-in, sign-up auto-sign-in, + SSO, OAuth callback, magic link, email OTP, passkey. The row carries the actor + (`user_id`), the tenant (`tenant_id` + the RLS `organization_id`), the session + it is about, and the client fingerprint better-auth recorded (IP, user agent). + An impersonation session keeps the subject on `user_id` and names the + impersonating admin on `actor`, so it cannot be misread as a self-service login. +- **`logout` on sign-out.** Scoped to `POST /sign-out` deliberately: a session row + is also deleted by admin revokes, `/revoke-session`, bans, user erasure and + better-auth's own collection of expired rows, and recording any of those as + `logout` would name an action the user never took. Those revocations already + carry their own cause on the ADR-0069 D4 session tombstone. +- **The `last_login_at` write is now attributed.** It goes out through the + platform's existing attribution channel (`ExecutionContext.attributedUserId`), + so the row names the person who signed in. It is attribution only — the write + still authorizes as the system, and nothing about who may touch `sys_user` + changes. The row is kept rather than suppressed: a login from a new address is + exactly what a compliance ledger is read for. + +The audit plugin now registers the `audit` service, the ledger's write ingress +for events that are not CRUD. `@objectstack/plugin-auth` resolves it lazily and +takes no dependency on the audit package — a deployment without the audit plugin +installed writes no auth rows, exactly as before. + +No API, schema or enum changes: `login`/`logout` were already declared members of +the `action` enum, and `sys_audit_log` remains `get`/`list`-only over HTTP. diff --git a/packages/plugins/plugin-audit/src/audit-plugin.ts b/packages/plugins/plugin-audit/src/audit-plugin.ts index 4b031bde06..3ed9d7a702 100644 --- a/packages/plugins/plugin-audit/src/audit-plugin.ts +++ b/packages/plugins/plugin-audit/src/audit-plugin.ts @@ -14,6 +14,7 @@ import { SysAuditLog, SysActivity, SysComment } from './objects/index.js'; // @objectstack/service-storage for the same ownership reason (ADR-0052 §3: a // file↔record link belongs with storage, not the compliance ledger). import { installAuditWriters, type AuditI18nSurface, type MessagingEmitSurface } from './audit-writers.js'; +import { createAuthEventAuditSink } from './auth-event-audit.js'; import { installCommentAccessHooks, installCommentReadVisibility } from './comment-access-hooks.js'; /** @@ -30,6 +31,13 @@ export class AuditPlugin implements Plugin { type = 'standard'; version = '1.0.0'; dependencies = ['com.objectstack.engine.objectql']; + /** + * [#8144] The `audit` slot — the ledger's WRITE ingress for events that are + * not CRUD (`login`/`logout` today). Declared here because `init()` registers + * it unconditionally, which is what ADR-0116 / `plugin-order.ts` reads this + * field to mean. + */ + providesServices = ['audit']; async init(ctx: PluginContext): Promise { // Register audit system objects via the manifest service. @@ -56,6 +64,31 @@ export class AuditPlugin implements Plugin { ], }); + // [#8144] The non-CRUD write ingress. Registered in init() — plugin-auth + // resolves it lazily and calls it from better-auth's session lifecycle + // hooks, i.e. at request time, so the engine is resolved per call rather + // than captured: the service exists from init() while `objectql` only + // resolves at kernel:ready, and every caller arrives long after both. + ctx.registerService( + 'audit', + createAuthEventAuditSink({ + getEngine: () => { + try { + return ctx.getService('objectql'); + } catch { + // Same fallback alias `start()` uses below — some kernels register + // the engine as `data`. + try { + return ctx.getService('data'); + } catch { + return undefined; + } + } + }, + logger: ctx.logger, + }), + ); + // ADR-0029 D8 — contribute this plugin's object translations to the i18n // service on kernel:ready (the i18n plugin may register after this one). if (typeof (ctx as any).hook === 'function') { diff --git a/packages/plugins/plugin-audit/src/audit-writers.ts b/packages/plugins/plugin-audit/src/audit-writers.ts index d287ef270b..9f8a2bdc58 100644 --- a/packages/plugins/plugin-audit/src/audit-writers.ts +++ b/packages/plugins/plugin-audit/src/audit-writers.ts @@ -193,6 +193,54 @@ const NOISE_FIELDS = new Set([ 'created_by', ]); +/** + * "Does this object's REGISTERED schema declare this field?", memoized per + * object. + * + * Extracted to module scope (#8144) so the CRUD writer below and the auth-event + * writer (`auth-event-audit.ts`) ask the question ONE way. Both stamp the same + * two conditional columns on the same table, and a second hand-rolled probe + * would answer differently on the day one of them is fixed. + * + * Why the probe exists at all: the SchemaRegistry auto-injects + * `organization_id` only in multi-tenant mode (`applySystemFields({ + * multiTenant })`), so on single-tenant stacks the `sys_audit_log` / + * `sys_activity` tables have no such column. Unconditionally stamping it there + * made every audit INSERT fail with "table sys_audit_log has no column named + * organization_id" — and the error was swallowed, so audit logging was silently + * non-functional. Resolve the field set lazily from the engine schema and cache + * it; object schemas are static after registration. + * + * Best-effort in both directions: an engine with no `getSchema` (an in-memory + * test double) reports every field absent, which skips the stamp rather than + * failing the write. + */ +export function createFieldPresenceProbe( + engine: unknown, +): (objectName: string, field: string) => boolean { + const fieldSetCache = new Map | null>(); + return (objectName: string, field: string): boolean => { + let set = fieldSetCache.get(objectName); + if (set === undefined) { + set = null; + try { + const schema: any = + typeof (engine as any)?.getSchema === 'function' ? (engine as any).getSchema(objectName) : null; + const fields = schema?.fields; + if (fields && typeof fields === 'object' && !Array.isArray(fields)) { + set = new Set(Object.keys(fields)); + } else if (Array.isArray(fields)) { + set = new Set(fields.map((f: any) => f?.name).filter(Boolean)); + } + } catch { + /* ignore — best-effort; absence just means we skip the stamp */ + } + fieldSetCache.set(objectName, set); + } + return set != null && set.has(field); + }; +} + /** Action name produced from a HookContext.event string. */ function actionFor(event: string): 'create' | 'update' | 'delete' | null { if (event === 'afterInsert') return 'create'; @@ -777,36 +825,10 @@ export function installAuditWriters( engine.unregisterHooksByPackage(packageId); } - // Whether a given object's *registered* schema declares a field. The - // SchemaRegistry auto-injects `organization_id` only in multi-tenant mode - // (`applySystemFields({ multiTenant })`), so on single-tenant stacks the - // `sys_audit_log` / `sys_activity` tables have no `organization_id` column. - // Unconditionally stamping it there made every audit INSERT fail with - // "table sys_audit_log has no column named organization_id" (the error was - // swallowed, so audit logging was silently non-functional). Resolve the - // field set lazily from the engine schema and cache it — object schemas are - // static after registration. - const fieldSetCache = new Map | null>(); - const objectHasField = (objectName: string, field: string): boolean => { - let set = fieldSetCache.get(objectName); - if (set === undefined) { - set = null; - try { - const schema: any = - typeof (engine as any).getSchema === 'function' ? (engine as any).getSchema(objectName) : null; - const fields = schema?.fields; - if (fields && typeof fields === 'object' && !Array.isArray(fields)) { - set = new Set(Object.keys(fields)); - } else if (Array.isArray(fields)) { - set = new Set(fields.map((f: any) => f?.name).filter(Boolean)); - } - } catch { - /* ignore — best-effort; absence just means we skip the stamp */ - } - fieldSetCache.set(objectName, set); - } - return set != null && set.has(field); - }; + // Whether a given object's *registered* schema declares a field — see + // `createFieldPresenceProbe` for why the conditional stamp exists. Shared + // with the auth-event writer so both stamp on the same answer. + const objectHasField = createFieldPresenceProbe(engine); // Cached full field-definition map per object (for ADR-0052 §5b trackHistory // rendering — needs labels/options, not just field names). diff --git a/packages/plugins/plugin-audit/src/auth-event-audit.test.ts b/packages/plugins/plugin-audit/src/auth-event-audit.test.ts new file mode 100644 index 0000000000..5e8cf476cc --- /dev/null +++ b/packages/plugins/plugin-audit/src/auth-event-audit.test.ts @@ -0,0 +1,322 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8144] The auth-event sink writes a REAL, READ-BACK ledger row. + * + * ## Why every assertion here reads the row back + * + * `sys_audit_log` declares every field `readonly`, and `validateRecord` skips + * readonly/system fields on both branches (#8203) — so the `action` enum is a + * vocabulary nothing validates in either direction. A write carrying an action + * the enum has never heard of is accepted silently. That makes "the call did + * not throw" worth exactly nothing on this object: it is equally true when the + * row landed correctly, when it landed wrong, and (before this change) when it + * was never attempted at all. So each case below inserts through a REAL + * ObjectQL engine and reads the stored row, asserting `action`, the actor + * (`user_id` / `actor`) and the tenant. + * + * The end-to-end claim — that a real sign-in through better-auth produces this + * row, visible through the real data API — is `packages/qa/dogfood/test/ + * auth-session-audit-trail.dogfood.test.ts`. This file covers what that one + * cannot reach cheaply: the single-tenant table shape (no `organization_id` + * column), the impersonation actor split, and the durability report on a + * failing write. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { ObjectQL } from '@objectstack/objectql'; +import { createAuthEventAuditSink } from './auth-event-audit.js'; + +const OWNER_PACKAGE = 'com.objectstack.test.auth-event-audit'; + +/** The ledger, in the shape a MULTI-tenant stack registers it. */ +const sysAuditLogMultiTenant = { + name: 'sys_audit_log', + label: 'Audit Log', + fields: { + id: { name: 'id', label: 'ID', type: 'text' as const, primaryKey: true }, + action: { name: 'action', label: 'Action', type: 'text' as const }, + user_id: { name: 'user_id', label: 'User', type: 'text' as const }, + actor: { name: 'actor', label: 'Actor', type: 'text' as const }, + object_name: { name: 'object_name', label: 'Object', type: 'text' as const }, + record_id: { name: 'record_id', label: 'Record', type: 'text' as const }, + old_value: { name: 'old_value', label: 'Old', type: 'textarea' as const }, + new_value: { name: 'new_value', label: 'New', type: 'textarea' as const }, + ip_address: { name: 'ip_address', label: 'IP', type: 'text' as const }, + user_agent: { name: 'user_agent', label: 'UA', type: 'textarea' as const }, + tenant_id: { name: 'tenant_id', label: 'Tenant', type: 'text' as const }, + metadata: { name: 'metadata', label: 'Metadata', type: 'textarea' as const }, + organization_id: { name: 'organization_id', label: 'Org', type: 'text' as const }, + }, +}; + +/** + * An OLDER ledger table: no `actor` column, and none declared for + * `organization_id` either. + * + * Both columns are stamped conditionally by the writer, for the same reason and + * with different fates today — worth stating because the fixture looks + * redundant otherwise: + * + * - `organization_id` comes BACK. `applySystemFields` provisions the tenant + * column unconditionally now (only `systemFields: false` / `managedBy: + * 'better-auth'` / `tenancy.enabled: false` opt out) — precisely so "sudo + * writers (audit / messaging / inbox / outbox …)" stop failing with "no + * column named organization_id" on single-tenant stacks. So this fixture + * measures that the probe reads the REGISTERED schema (post-injection), + * not the document an author wrote. + * - `actor` does NOT. It is a plain declared field that older audit tables + * predate, so it is the live half of the conditional stamp: writing it into + * a table that lacks it fails the INSERT, and the writer swallows — audit + * logging would just stop, with nothing in the response to show it. + */ +const sysAuditLogNoActor = { + ...sysAuditLogMultiTenant, + fields: Object.fromEntries( + Object.entries(sysAuditLogMultiTenant.fields).filter( + ([k]) => k !== 'organization_id' && k !== 'actor', + ), + ), +}; + +/** Minimal in-memory driver — the `audit-hook-object-scope.test.ts` shape. */ +function makeMemoryDriver() { + const stores = new Map>>(); + const storeFor = (obj: string) => { + let s = stores.get(obj); + if (!s) { + s = new Map(); + stores.set(obj, s); + } + return s; + }; + let nextId = 0; + const matchesWhere = (row: Record, where: any): boolean => { + if (!where || typeof where !== 'object') return true; + for (const [k, v] of Object.entries(where)) { + if (k === '$and' && Array.isArray(v)) { + if (!v.every((sub) => matchesWhere(row, sub))) return false; + continue; + } + if (k.startsWith('$')) continue; + const expected = v && typeof v === 'object' && '$eq' in (v as any) ? (v as any).$eq : v; + if ((row[k] ?? null) !== (expected ?? null)) return false; + } + return true; + }; + const driver: any = { + name: 'memory', + version: '0.0.0', + supports: {} as any, + async connect() {}, + async disconnect() {}, + async checkHealth() { + return true; + }, + async execute() { + return null; + }, + async find(object: string, ast: any) { + return Array.from(storeFor(object).values()).filter((r) => matchesWhere(r, ast?.where)); + }, + async findOne(object: string, ast: any) { + for (const r of storeFor(object).values()) if (matchesWhere(r, ast?.where)) return r; + return null; + }, + async create(object: string, data: Record) { + nextId += 1; + const id = (data.id as string) ?? `r_${nextId}`; + const row: Record = { ...data, id }; + storeFor(object).set(id, row); + return row; + }, + async update(object: string, id: string, data: Record) { + const s = storeFor(object); + const cur = s.get(id); + if (!cur) return null; + const updated = { ...cur, ...data, id }; + s.set(id, updated); + return updated; + }, + async upsert(object: string, data: Record) { + const id = data.id as string | undefined; + if (id && storeFor(object).has(id)) return this.update(object, id, data); + return this.create(object, data); + }, + async delete(object: string, id: string) { + return storeFor(object).delete(id); + }, + async count(object: string, ast: any) { + return (await this.find(object, ast)).length; + }, + async bulkCreate(object: string, rows: Record[]) { + return Promise.all(rows.map((r) => this.create(object, r))); + }, + async bulkUpdate() { + return []; + }, + async bulkDelete() {}, + async updateMany() { + return 0; + }, + async beginTransaction() { + return { commit: async () => {}, rollback: async () => {} }; + }, + async commit() {}, + async rollback() {}, + }; + return { driver, storeFor }; +} + +async function boot(ledger: unknown = sysAuditLogMultiTenant) { + const engine = new ObjectQL(); + const mem = makeMemoryDriver(); + engine.registerDriver(mem.driver, true); + await engine.init(); + engine.registry.registerObject(ledger as any, OWNER_PACKAGE); + return { engine, storeFor: mem.storeFor }; +} + +/** Read the ledger back THROUGH THE ENGINE, not out of the driver's map. */ +async function ledgerRows(engine: any, where: Record = {}) { + return (await engine.find('sys_audit_log', { where }, { context: { isSystem: true } })) as any[]; +} + +describe('[#8144] createAuthEventAuditSink writes a login row that names its actor', () => { + it('records action/user/tenant/session, read back from the ledger', async () => { + const { engine } = await boot(); + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + + await sink.recordAuthEvent({ + action: 'login', + userId: 'usr_1', + sessionId: 'ses_1', + organizationId: 'org_1', + ipAddress: '203.0.113.7', + userAgent: 'Mozilla/5.0 (probe)', + context: { endpoint: '/sign-in/email' }, + }); + + // Filter on the action the acceptance criterion filters on, so the query is + // the one the `auth_events` list view issues rather than a friendlier one. + const rows = await ledgerRows(engine, { action: 'login' }); + expect(rows).toHaveLength(1); + const [row] = rows; + // The three things #7675 said were missing: the event, the actor, the tenant. + expect(row.action).toBe('login'); + expect(row.user_id).toBe('usr_1'); + expect(row.tenant_id).toBe('org_1'); + expect(row.organization_id).toBe('org_1'); + // ADR-0014 D2's principal label — the subject acted for themselves here. + expect(row.actor).toBe('usr_1'); + // Navigable back to the session it is about. + expect(row.object_name).toBe('sys_session'); + expect(row.record_id).toBe('ses_1'); + expect(row.ip_address).toBe('203.0.113.7'); + expect(row.user_agent).toBe('Mozilla/5.0 (probe)'); + expect(JSON.parse(String(row.metadata))).toEqual({ endpoint: '/sign-in/email' }); + // An auth event is not a field diff — recording `{}` would be a claim about + // a record that never changed. + expect(row.old_value ?? null).toBeNull(); + expect(row.new_value ?? null).toBeNull(); + }); + + it('logout is a distinct row with its own action', async () => { + const { engine } = await boot(); + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + + await sink.recordAuthEvent({ action: 'login', userId: 'usr_1', sessionId: 'ses_1' }); + await sink.recordAuthEvent({ + action: 'logout', + userId: 'usr_1', + sessionId: 'ses_1', + context: { endpoint: '/sign-out' }, + }); + + expect(await ledgerRows(engine, { action: 'login' })).toHaveLength(1); + const outs = await ledgerRows(engine, { action: 'logout' }); + expect(outs).toHaveLength(1); + expect(outs[0].user_id).toBe('usr_1'); + // …and the two are different rows, not one row overwritten. + expect(await ledgerRows(engine)).toHaveLength(2); + }); + + it('an impersonation session credits the ADMIN as actor and keeps the subject on user_id', async () => { + // `user_id` is a strict `sys_user` lookup and the session really is the + // subject's; `actor` is "the principal that performed the action". Writing + // the impersonated user as the sole principal would be a WRONG record — the + // reader could not tell it from a self-service sign-in. + const { engine } = await boot(); + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + + await sink.recordAuthEvent({ + action: 'login', + userId: 'usr_subject', + actor: 'usr_admin', + context: { endpoint: '/admin/impersonate-user', impersonated_by: 'usr_admin' }, + }); + + const [row] = await ledgerRows(engine, { action: 'login' }); + expect(row.user_id).toBe('usr_subject'); + expect(row.actor).toBe('usr_admin'); + expect(JSON.parse(String(row.metadata)).impersonated_by).toBe('usr_admin'); + }); + + it('a ledger table without an `actor` column still gets its row, unstamped', async () => { + // The failure this guards is silent by construction: stamping a column the + // table does not have makes the INSERT fail, and the writer swallows — so + // audit logging would simply stop, with nothing in the response to show it. + const { engine } = await boot(sysAuditLogNoActor); + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + + await sink.recordAuthEvent({ action: 'login', userId: 'usr_1', organizationId: 'org_1' }); + + const rows = await ledgerRows(engine, { action: 'login' }); + expect(rows).toHaveLength(1); + expect(rows[0].user_id).toBe('usr_1'); + // The declared lookup carries the tenant either way… + expect(rows[0].tenant_id).toBe('org_1'); + // …and the probe reads the REGISTERED schema, so the unconditionally + // provisioned tenant column is stamped even though the fixture omits it. + expect(rows[0].organization_id).toBe('org_1'); + // The one column that really is absent is left alone. + expect(rows[0].actor).toBeUndefined(); + }); + + it('no engine yet, or no subject: nothing is written and nothing throws', async () => { + const { engine } = await boot(); + const noEngine = createAuthEventAuditSink({ getEngine: () => undefined }); + await expect(noEngine.recordAuthEvent({ action: 'login', userId: 'usr_1' })).resolves.toBeUndefined(); + + const sink = createAuthEventAuditSink({ getEngine: () => engine as any }); + await sink.recordAuthEvent({ action: 'login', userId: '' as string }); + // An unattributed auth row is the defect this card removes; not writing one + // is the correct answer, not a silent loss. + expect(await ledgerRows(engine)).toHaveLength(0); + }); + + it('a failed ledger write is reported at ERROR, once, and never breaks the caller', async () => { + // #5226's discipline on the auth seam: the sign-in returned 200 and the + // session is on disk, so the only sign the ledger lost a row is this line. + const broken: any = { + getSchema: () => null, + insert: async () => { + throw new Error('no such table: sys_audit_log'); + }, + }; + const logger = { error: vi.fn(), debug: vi.fn() }; + const sink = createAuthEventAuditSink({ getEngine: () => broken, logger }); + + await expect(sink.recordAuthEvent({ action: 'login', userId: 'usr_1' })).resolves.toBeUndefined(); + await sink.recordAuthEvent({ action: 'logout', userId: 'usr_1' }); + + expect(logger.error).toHaveBeenCalledTimes(1); + const [msg] = logger.error.mock.calls[0]; + // The two things a durability `error` owes, in its first line. + expect(String(msg)).toContain('INCOMPLETE'); + expect(String(msg)).toContain('Fix:'); + // Said once — an auth event fires on every sign-in, so a per-write `error` + // on a systemic cause trains everyone to skim the channel. + expect(logger.debug).toHaveBeenCalledTimes(1); + }); +}); diff --git a/packages/plugins/plugin-audit/src/auth-event-audit.ts b/packages/plugins/plugin-audit/src/auth-event-audit.ts new file mode 100644 index 0000000000..bea3113f3a --- /dev/null +++ b/packages/plugins/plugin-audit/src/auth-event-audit.ts @@ -0,0 +1,249 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8144, sub-issue A of #7675] `login` / `logout` rows in the compliance + * ledger — the writer half. + * + * ## The gap this closes + * + * `sys_audit_log.action` declares ten values. `audit-writers.ts` subscribes to + * the ObjectQL wildcard `before*`/`after*` CRUD lifecycle, so it can emit + * `create`/`update`/`delete`/`restore` and nothing else — four of the declared + * values had no writer anywhere in the repo. Two of them are auth session + * events, and the whole trace a sign-in left behind was an **unattributed** + * `update sys_user` row diffing `last_login_at` (`user_id` null). The shipped + * `auth_events` list view and two `system_overview` dashboard widgets were + * therefore permanently empty, by construction. + * + * ## Why the row is built HERE and not at the auth seam + * + * plugin-audit owns the `sys_audit_log` row shape — `packages/spec/src/system/ + * index.ts` records that explicitly when it retired `audit.zod`: "the LIVE + * audit path (plugin-audit) captures unconditionally via engine hooks and + * defines its own sys_audit_log row shape". A second package hand-assembling + * ledger rows would be a second de-facto definition of that shape, drifting on + * the day either side is fixed — the argument `audit-writers.ts` makes for + * importing `SECRET_MASK` and `resolveDisplayField` rather than re-typing them. + * + * That matters more than usual on THIS object. Every `sys_audit_log` field is + * `readonly`, and `validateRecord` skips readonly/system fields on both + * branches (#8203), so **the `action` enum is a vocabulary nothing validates in + * either direction**: a row with a misspelled action is accepted silently and + * no test that merely watches for a throw can see it. The only structural + * protection available is at authoring time, which is why + * {@link AuthSessionAuditAction} is a closed literal union rather than + * `string`, and why the caller hands over an EVENT (what happened) instead of a + * row (what to store). + * + * ## Who calls it + * + * `AuditPlugin` registers this sink under the `audit` service slot; plugin-auth + * resolves it lazily through a locally-declared structural surface and calls it + * from better-auth's session lifecycle hooks. Neither package depends on the + * other — the same shape `audit-writers.ts` uses to reach messaging/i18n + * without depending on those services. + */ + +import type { IDataEngine } from '@objectstack/spec/contracts'; +import { createFieldPresenceProbe } from './audit-writers.js'; + +/** The two auth session events the ledger records (`sys_audit_log.action`). */ +export type AuthSessionAuditAction = 'login' | 'logout'; + +/** + * One auth session event, in the vocabulary of what HAPPENED rather than of + * what to store. Everything but `action` and `userId` is optional: an event + * missing its client fingerprint is still worth recording, and a partial row + * beats no row on a compliance ledger. + */ +export interface AuthSessionAuditEvent { + /** Which event. Closed union — see the module note on the unvalidated enum. */ + action: AuthSessionAuditAction; + /** The session's subject — a real `sys_user` id (the `user_id` lookup). */ + userId: string; + /** The `sys_session` row this event is about, when known. */ + sessionId?: string; + /** Tenant context — the session's active organization, when it has one. */ + organizationId?: string; + /** Client fingerprint, as better-auth recorded it on the session row. */ + ipAddress?: string; + userAgent?: string; + /** + * The principal that CAUSED the event, when it is not the subject — an + * admin's user id on an impersonation session. Lands on `actor`, which the + * schema defines as "Principal that performed the action", independent of the + * `user_id` lookup. Absent → the subject acted for themselves. + */ + actor?: string; + /** + * Free-form context recorded in `metadata`, e.g. the better-auth endpoint + * that produced the event (`/sign-in/email`, `/sign-out`). + */ + context?: Record; +} + +/** + * Minimal logger surface — structurally the kernel `ctx.logger` (`ILogger`), + * declared locally so this module needs no import for two optional methods. + */ +export interface AuthEventAuditLogger { + error?(msg: string, err?: Error, meta?: Record): void; + debug?(msg: string, meta?: Record): void; +} + +/** What {@link createAuthEventAuditSink} registers under the `audit` slot. */ +export interface AuthEventAuditSink { + /** + * Record one auth session event in `sys_audit_log`. Never throws — an audit + * write must never turn a valid sign-in into an error — and reports a lost + * row loudly (see {@link createAuthEventAuditSink}). + */ + recordAuthEvent(event: AuthSessionAuditEvent): Promise; +} + +export interface AuthEventAuditSinkOptions { + /** + * Resolve the data engine at CALL time. Lazy because the sink is registered + * during `init()` (that is what `providesServices` means) while the engine + * only resolves at `kernel:ready`; every call happens at request time, long + * after both. + */ + getEngine(): IDataEngine | undefined; + logger?: AuthEventAuditLogger; +} + +/** The object the auth-event rows name as their target. */ +const SESSION_OBJECT = 'sys_session'; + +/** JSON that cannot throw on a cyclic/odd value — same rule as the CRUD writer. */ +function safeStringify(v: unknown): string { + try { + return JSON.stringify(v); + } catch { + return String(v); + } +} + +/** + * Create the auth-event sink. + * + * The write goes through the engine under a system context: `sys_audit_log` + * exposes only `get`/`list` on the API (creation happens via internal system + * hooks only) and every field is `readonly`, so a user-context write would be + * refused. This is the same posture `audit-writers.ts` takes with + * `api.sudo()`. + */ +export function createAuthEventAuditSink(opts: AuthEventAuditSinkOptions): AuthEventAuditSink { + const { getEngine, logger } = opts; + let probe: ((objectName: string, field: string) => boolean) | undefined; + let probedEngine: IDataEngine | undefined; + + /** + * Report a lost auth-event row — once per process, not once per failure. + * + * Same discipline, and the same reason, as `reportAuditWriteFailure` in + * `audit-writers.ts`: a systemic cause (the table is unreachable from this + * connection) would otherwise emit one `error` per sign-in and train everyone + * to skim the channel. + */ + let failureReported = false; + const reportAuthEventWriteFailure = (action: string, err: unknown): void => { + const detail = String((err as any)?.message ?? err); + try { + if (failureReported) { + logger?.debug?.('Auth-event audit write failed (already reported)', { action, err: detail }); + return; + } + failureReported = true; + logger?.error?.( + 'Auth-event audit write FAILED — the compliance trail is now INCOMPLETE. The sign-in/sign-out itself ' + + 'SUCCEEDED and the user holds a valid session, so the API returned 200 and nothing downstream looks ' + + `broken; only the \`sys_audit_log\` row recording the ${action} never landed, and nothing retries it. ` + + 'Every subsequent auth event is likely losing its row the same way (this is reported ONCE — raise the ' + + 'log level to `debug` to see the rest). The shipped `auth_events` list view and the system-overview ' + + 'widgets read exactly these rows, so they will keep showing an empty, healthy-looking screen. ' + + 'Fix: confirm `sys_audit_log` is reachable from the connection this write ran on — its ADR-0057 §3.6 ' + + 'lifecycle class routes it to the dedicated `telemetry` datasource whenever one is registered (`os dev` ' + + 'provisions one by default as a SIBLING SQLite file), so a "no such table" here usually means the write ' + + 'executed against a DIFFERENT datasource than the one the table was created in. Set `OS_TELEMETRY_DB=0` ' + + 'to keep every lifecycle-classed object on the primary datasource.', + err instanceof Error ? err : new Error(detail), + { action }, + ); + } catch { + /* logging must never break the auth response */ + } + }; + + /** + * Write the ledger row. + * + * Extracted as a NAMED callee so `pnpm check:durability-log-level` can anchor + * on it — it is declared in that gate's `DURABILITY_CRITICAL_CALLEES` in the + * same PR, so a future edit cannot quietly walk the failure report back down + * to `warn`. A bare `.insert()` is far too generic a name to put in a + * repo-wide vocabulary. + */ + const persistAuthEventAuditRow = async ( + engine: IDataEngine, + row: Record, + ): Promise => { + await engine.insert('sys_audit_log', row, { context: { isSystem: true } } as any); + }; + + return { + async recordAuthEvent(event: AuthSessionAuditEvent): Promise { + const engine = getEngine(); + if (!engine || !event?.userId) return; + if (probe === undefined || probedEngine !== engine) { + probe = createFieldPresenceProbe(engine); + probedEngine = engine; + } + const objectHasField = probe; + + const tenantId = event.organizationId ?? null; + const row: Record = { + action: event.action, + // The whole point of the card: the event names its actor. `user_id` is + // a strict `sys_user` lookup, and a session always has a real subject. + user_id: event.userId, + // The session this event is about, so the row is navigable from the + // ledger to the session row (and back) rather than being a bare verb. + object_name: SESSION_OBJECT, + record_id: event.sessionId ?? null, + // No before/after state: an auth event is not a field diff. Recording + // `{}` here would be a claim about a record that never changed. + old_value: null, + new_value: null, + ip_address: event.ipAddress ?? null, + user_agent: event.userAgent ?? null, + tenant_id: tenantId, + metadata: + event.context && Object.keys(event.context).length > 0 ? safeStringify(event.context) : null, + }; + // Both columns are conditionally present — see `createFieldPresenceProbe`. + // `organization_id` is what the SecurityPlugin's RLS predicate gates on, + // so an unstamped row is a row non-admin members can never see. + if (objectHasField('sys_audit_log', 'organization_id')) { + row.organization_id = tenantId; + } + if (objectHasField('sys_audit_log', 'actor')) { + // ADR-0014 D2's principal label. For an ordinary sign-in the subject IS + // the principal; for an impersonation session the admin who started it + // is, and recording the impersonated user as the sole principal there + // would be a WRONG record rather than a vague one. + row.actor = event.actor ?? event.userId; + } + + try { + await persistAuthEventAuditRow(engine, row); + } catch (err) { + // #5226's class, on the auth seam: a DURABILITY degradation, not a + // functional one — the sign-in returned 200 and the session is on disk, + // so the system looks completely normal from the outside while the + // ledger entry that records WHO signed in is simply absent. + reportAuthEventWriteFailure(event.action, err); + } + }, + }; +} diff --git a/packages/plugins/plugin-audit/src/index.ts b/packages/plugins/plugin-audit/src/index.ts index 83286f394d..760ffd3faa 100644 --- a/packages/plugins/plugin-audit/src/index.ts +++ b/packages/plugins/plugin-audit/src/index.ts @@ -8,7 +8,15 @@ */ export { AuditPlugin } from './audit-plugin.js'; -export { installAuditWriters } from './audit-writers.js'; +export { createFieldPresenceProbe, installAuditWriters } from './audit-writers.js'; +export { createAuthEventAuditSink } from './auth-event-audit.js'; +export type { + AuthEventAuditLogger, + AuthEventAuditSink, + AuthEventAuditSinkOptions, + AuthSessionAuditAction, + AuthSessionAuditEvent, +} from './auth-event-audit.js'; export { installCommentAccessHooks, installCommentReadVisibility, diff --git a/packages/plugins/plugin-auth/src/auth-manager.ts b/packages/plugins/plugin-auth/src/auth-manager.ts index ecfda92766..ca2a2d7a93 100644 --- a/packages/plugins/plugin-auth/src/auth-manager.ts +++ b/packages/plugins/plugin-auth/src/auth-manager.ts @@ -22,6 +22,12 @@ import { postureEnforcesWall, type TenancyPosture } from '@objectstack/spec/secu import { MCP_OAUTH_SCOPES } from '@objectstack/spec/ai'; import { createObjectQLAdapterFactory, withSystemReadContext } from './objectql-adapter.js'; import { runWithAuthActorScope, setAuthActorResolver } from './auth-actor-attribution.js'; +import { + emitAuthSessionAuditEvent, + loginEventFor, + logoutEventFor, + type AuthEventAuditSurface, +} from './auth-session-audit.js'; import { SESSION_ERASURE_PATHS } from './session-tombstone.js'; import { invitationRoleCapFailure, @@ -571,6 +577,19 @@ export interface AuthManagerOptions extends Partial { */ getTenancy?: () => TenancyService | undefined; + /** + * [#8144] Accessor for the `audit` service — the compliance ledger's ingress + * for events that are not CRUD. Consulted by the session lifecycle hooks to + * record `login` / `logout` in `sys_audit_log`. + * + * Lazy for the same reason `getTenancy` is: the service is registered on the + * kernel after the AuthManager is constructed, and every call happens at + * request time. Omitted, or resolving to `undefined` (the audit plugin is not + * installed — it is an OPTIONAL pair in the CLI) → no auth rows are written, + * which is exactly today's behaviour. + */ + getAuditSink?: () => AuthEventAuditSurface | undefined; + /** * Optional structured logger (the kernel `ctx.logger`) for best-effort * bookkeeping surfaces such as the ADR-0093 membership reconciler. Omitted → @@ -4193,6 +4212,35 @@ export class AuthManager { } : hostSessionBefore; + // [#8144] The `login` / `logout` audit writers (#7675 sub-issue A). See + // `auth-session-audit.ts` for why these two hooks and no others, and why + // logout reads the endpoint path from the hook's own `ctx` argument rather + // than from the ambient auth context. + // + // Host hooks chain FIRST and keep their result, exactly as `sessionBefore` + // above does: the audit row is an observation, never a participant in what + // better-auth decides. Both emitters are awaited but cannot throw + // (`emitAuthSessionAuditEvent` swallows), so neither can turn a valid + // sign-in or sign-out into an error. + const hostSessionCreateAfter = (host as any)?.session?.create?.after; + const sessionCreateAfter = async (session: any, ctx: any) => { + const result = hostSessionCreateAfter ? await hostSessionCreateAfter(session, ctx) : undefined; + await emitAuthSessionAuditEvent( + this.config.getAuditSink?.(), + loginEventFor(session, typeof ctx?.path === 'string' ? ctx.path : undefined), + ); + return result; + }; + const hostSessionDeleteAfter = (host as any)?.session?.delete?.after; + const sessionDeleteAfter = async (session: any, ctx: any) => { + const result = hostSessionDeleteAfter ? await hostSessionDeleteAfter(session, ctx) : undefined; + await emitAuthSessionAuditEvent( + this.config.getAuditSink?.(), + logoutEventFor(session, typeof ctx?.path === 'string' ? ctx.path : undefined), + ); + return result; + }; + // ADR-0093 D2 — the single owner of the membership invariant. Composed into // `user.create.after`, the one seam EVERY creation path flows through // (email signup, admin create-user, bulk import, SSO JIT). Host hook (e.g. @@ -4244,17 +4292,21 @@ export class AuthManager { after: userAfter, }, }, - ...(sessionBefore - ? { - session: { - ...((host as any)?.session ?? {}), - create: { - ...((host as any)?.session?.create ?? {}), - before: sessionBefore, - }, - }, - } - : {}), + session: { + ...((host as any)?.session ?? {}), + create: { + ...((host as any)?.session?.create ?? {}), + // `sessionBefore` is undefined only when `autoActiveOrganization` is + // off AND the host supplied none — spread it in conditionally so the + // key is absent rather than explicitly `undefined`. + ...(sessionBefore ? { before: sessionBefore } : {}), + after: sessionCreateAfter, + }, + delete: { + ...((host as any)?.session?.delete ?? {}), + after: sessionDeleteAfter, + }, + }, } as BetterAuthOptions['databaseHooks']; } @@ -4852,12 +4904,34 @@ export class AuthManager { * successful sign-in. Best-effort and always fire-and-forget safe: a login * audit write must never turn a valid login into an error, and it runs * unconditionally (unlike lockout accounting, which is gated on a threshold). + * + * [#8144] The write is ATTRIBUTED to the user who just signed in. Before this + * it went out as a bare `{ isSystem: true }`, so the audit writer recorded it + * with `user_id: null` — and since nothing else wrote a `login` row, that + * unattributed `update sys_user` row was the *only* trace a sign-in left + * behind (#7675's reproduction). `attributedUserId` is the platform's one + * channel for "the human CREDITED for a write the system authorized" (#4586, + * `auth-actor-attribution.ts`); it travels as provenance, no security + * middleware reads it, and it never becomes the subject the write authorizes + * as — `isSystem: true` stays exactly where it is. + * + * Passed EXPLICITLY rather than through `authSystemWriteContext()`: the + * ambient actor scope is filled by better-auth's global before-hook from the + * request's session, and on `/sign-in/email` there is no session yet when + * that hook runs. The one path that knows who this is, is this one. + * + * Attributed rather than EXCLUDED (the ruling allowed either). Suppressing it + * would mean adding `last_login_at`/`last_login_ip` to the CRUD writer's + * repo-wide `NOISE_FIELDS`, which deletes the `last_login_ip` change trail — + * a login from a new address is exactly the kind of thing a compliance ledger + * is read for — for every object and every deployment. The defect the ruling + * named is the missing actor, and this is that actor. */ private async stampLastLogin(userId: string, ip: string | undefined): Promise { const engine = this.getDataEngine(); if (!engine || !userId) return; try { - const SYSTEM_CTX = { isSystem: true, positions: [], permissions: [] }; + const SYSTEM_CTX = { isSystem: true, attributedUserId: userId, positions: [], permissions: [] }; const patch: Record = { id: userId, last_login_at: new Date() }; // Cap to the column width (IPv6 textual max 45) — a malformed/oversized // forwarded header must not blow up the write. diff --git a/packages/plugins/plugin-auth/src/auth-plugin.ts b/packages/plugins/plugin-auth/src/auth-plugin.ts index 26e0838679..4e49d73339 100644 --- a/packages/plugins/plugin-auth/src/auth-plugin.ts +++ b/packages/plugins/plugin-auth/src/auth-plugin.ts @@ -28,6 +28,7 @@ import { } from './auth-manager.js'; import { ensureDefaultOrganization } from './ensure-default-organization.js'; import { runAttributedToUser } from './auth-actor-attribution.js'; +import type { AuthEventAuditSurface } from './auth-session-audit.js'; import type { ResolvedSocialProvider } from './backfill-account-issuer.js'; import { createTenancyService, type TenancyService } from './tenancy-service.js'; import { @@ -348,6 +349,20 @@ export class AuthPlugin implements Plugin { return undefined; } }, + // [#8144] The compliance ledger's non-CRUD ingress, resolved LAZILY at + // session-hook fire time (i.e. per request, long after boot). AuditPlugin + // is an optional pair in the CLI, so a miss is the ordinary case, not a + // degradation to report: no audit plugin means no `login`/`logout` rows, + // which is exactly the behaviour before this card. Deliberately NOT an + // `optionalDependencies` entry — nothing in `init()` consumes it, so + // there is no boot ordering to constrain. + getAuditSink: () => { + try { + return ctx.getService('audit'); + } catch { + return undefined; + } + }, }; // ADR-0069 D2 — cross-node rate-limit counters, backed by the kernel diff --git a/packages/plugins/plugin-auth/src/auth-session-audit.test.ts b/packages/plugins/plugin-auth/src/auth-session-audit.test.ts new file mode 100644 index 0000000000..6b0235b65e --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-session-audit.test.ts @@ -0,0 +1,290 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8144] The `login` / `logout` emitters, at the composition seam. + * + * These cases drive `composeDatabaseHooks` — the real composition better-auth + * is handed — rather than the mapping functions in isolation, because the whole + * question this card can get wrong is whether the hooks are WIRED, and a + * mapping function called directly proves nothing about that. (The end-to-end + * claim — a real sign-in through better-auth producing a row readable through + * the data API — is `packages/qa/dogfood/test/auth-session-audit-trail.dogfood. + * test.ts`.) + * + * The negative cases carry as much weight as the positive ones: a session row + * is deleted by revokes, bans, user erasure and better-auth's own collection of + * expired rows, and recording any of those as `logout` would be a WRONG audit + * record — it names an action the subject never took. + */ + +import { describe, it, expect, vi } from 'vitest'; +import { AuthManager } from './auth-manager'; +import { + loginEventFor, + logoutEventFor, + SIGN_OUT_PATH, + type AuthSessionAuditEventInput, +} from './auth-session-audit'; + +const SECRET = 'test-secret-at-least-32-chars-long'; + +/** A better-auth session row as the databaseHooks see it (camelCase). */ +const SESSION = { + id: 'ses_1', + userId: 'usr_1', + activeOrganizationId: 'org_1', + ipAddress: '203.0.113.7', + userAgent: 'Mozilla/5.0 (probe)', +}; + +/** + * The structural minimum this file needs from the sink spy: a call log whose + * entries are the one-argument tuple `recordAuthEvent` is declared with. + * + * Declared rather than inferred from `vi.fn()`'s return type, for two reasons. + * A `vi.fn(async () => …)` whose implementation takes NO parameter types its + * `mock.calls` as `[][]` — a zero-length tuple — so every `calls[0][0]` in this + * file was reaching past the end of a tuple the type system believed was empty + * (TS2493), and reading the argument back is the entire point of these cases. + * And pinning the element type to `AuthSessionAuditEventInput` is what makes + * the assertions below type-check against the REAL event shape rather than + * against `any`: a field renamed on the event surface fails here at compile + * time instead of quietly comparing `undefined` to `undefined`. + */ +type RecordAuthEventSpy = { mock: { calls: Array<[AuthSessionAuditEventInput]> } }; + +/** Every event the sink was handed, in call order. */ +function recordedEvents(spy: RecordAuthEventSpy): AuthSessionAuditEventInput[] { + return spy.mock.calls.map(([event]) => event); +} + +/** + * The first event the sink was handed. + * + * The "never called" case is named here rather than left to blow up on a + * property access: a writer that was never wired is precisely the failure these + * cases exist to catch, and it should read as that sentence, not as a + * `TypeError` about `undefined`. + */ +function firstEvent(spy: RecordAuthEventSpy): AuthSessionAuditEventInput { + const [event] = recordedEvents(spy); + if (!event) throw new Error('the audit sink was never handed an event'); + return event; +} + +function hooksWithSink(config: Record = {}) { + const recordAuthEvent = vi.fn(async (_event: AuthSessionAuditEventInput) => undefined); + const manager = new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + getAuditSink: () => ({ recordAuthEvent }), + ...config, + } as any); + const hooks = (manager as any).composeDatabaseHooks((config as any).databaseHooks) as any; + return { hooks, recordAuthEvent }; +} + +describe('[#8144] session.create.after records a login', () => { + it('emits the event with subject, tenant, session and client fingerprint', async () => { + const { hooks, recordAuthEvent } = hooksWithSink(); + + await hooks.session.create.after(SESSION, { path: '/sign-in/email' }); + + expect(recordAuthEvent).toHaveBeenCalledTimes(1); + expect(firstEvent(recordAuthEvent)).toEqual({ + action: 'login', + userId: 'usr_1', + sessionId: 'ses_1', + organizationId: 'org_1', + ipAddress: '203.0.113.7', + userAgent: 'Mozilla/5.0 (probe)', + context: { endpoint: '/sign-in/email' }, + }); + }); + + it('fires for EVERY session-minting endpoint, not just /sign-in/email', async () => { + // The reason the seam is the session hook and not the `/sign-in/email` + // after-middleware where `stampLastLogin` lives: on a real deployment most + // sign-ins are federated, and a writer wired to one endpoint would audit + // the minority and silently miss the rest. + const { hooks, recordAuthEvent } = hooksWithSink(); + + for (const path of ['/sign-up/email', '/callback/google', '/sso/callback/acme', '/magic-link/verify']) { + await hooks.session.create.after({ ...SESSION, id: `ses_${path}` }, { path }); + } + + expect(recordAuthEvent).toHaveBeenCalledTimes(4); + expect(recordedEvents(recordAuthEvent).map((event) => event.action)).toEqual([ + 'login', + 'login', + 'login', + 'login', + ]); + }); + + it('an impersonation session credits the admin as actor, subject stays on userId', async () => { + const { hooks, recordAuthEvent } = hooksWithSink(); + + await hooks.session.create.after( + { ...SESSION, impersonatedBy: 'usr_admin' }, + { path: '/admin/impersonate-user' }, + ); + + const event = firstEvent(recordAuthEvent); + expect(event.userId).toBe('usr_1'); + expect(event.actor).toBe('usr_admin'); + expect(event.context).toEqual({ + endpoint: '/admin/impersonate-user', + impersonated_by: 'usr_admin', + }); + }); + + it('a session with no subject writes nothing — an unattributed auth row is the defect', async () => { + const { hooks, recordAuthEvent } = hooksWithSink(); + await hooks.session.create.after({ id: 'ses_x' }, { path: '/sign-in/email' }); + expect(recordAuthEvent).not.toHaveBeenCalled(); + }); + + it('the HOST session.create.after still runs, and its result is preserved', async () => { + const hostAfter = vi.fn(async () => 'host-result'); + const { hooks, recordAuthEvent } = hooksWithSink({ + databaseHooks: { session: { create: { after: hostAfter } } }, + }); + + const result = await hooks.session.create.after(SESSION, { path: '/sign-in/email' }); + + expect(hostAfter).toHaveBeenCalledTimes(1); + expect(result).toBe('host-result'); + expect(recordAuthEvent).toHaveBeenCalledTimes(1); + }); + + it('is wired even when autoActiveOrganization is off (which removes create.before)', async () => { + const { hooks, recordAuthEvent } = hooksWithSink({ autoActiveOrganization: false }); + // The pre-#8144 shape omitted the whole `session` block when there was no + // `before` hook to install — the audit writer must not inherit that gate. + expect(hooks.session?.create?.before).toBeUndefined(); + await hooks.session.create.after(SESSION, { path: '/sign-in/email' }); + expect(recordAuthEvent).toHaveBeenCalledTimes(1); + }); + + it('no audit plugin installed: the hook is a no-op, not an error', async () => { + const manager = new AuthManager({ secret: SECRET, baseUrl: 'http://localhost:3000' } as any); + const hooks = (manager as any).composeDatabaseHooks(undefined) as any; + await expect( + hooks.session.create.after(SESSION, { path: '/sign-in/email' }), + ).resolves.toBeUndefined(); + }); + + it('a throwing sink cannot break the sign-in', async () => { + const boom = vi.fn(async () => { + throw new Error('ledger unreachable'); + }); + const manager = new AuthManager({ + secret: SECRET, + baseUrl: 'http://localhost:3000', + getAuditSink: () => ({ recordAuthEvent: boom }), + } as any); + const hooks = (manager as any).composeDatabaseHooks(undefined) as any; + + await expect( + hooks.session.create.after(SESSION, { path: '/sign-in/email' }), + ).resolves.toBeUndefined(); + expect(boom).toHaveBeenCalledTimes(1); + }); +}); + +describe('[#8144] session.delete.after records a logout — and ONLY for /sign-out', () => { + it('emits logout under /sign-out', async () => { + const { hooks, recordAuthEvent } = hooksWithSink(); + + await hooks.session.delete.after(SESSION, { path: SIGN_OUT_PATH }); + + expect(recordAuthEvent).toHaveBeenCalledTimes(1); + expect(firstEvent(recordAuthEvent)).toEqual({ + action: 'logout', + userId: 'usr_1', + sessionId: 'ses_1', + organizationId: 'org_1', + ipAddress: '203.0.113.7', + userAgent: 'Mozilla/5.0 (probe)', + context: { endpoint: '/sign-out' }, + }); + }); + + it.each([ + ['/revoke-session'], + ['/revoke-sessions'], + ['/revoke-other-sessions'], + ['/admin/revoke-user-session'], + ['/admin/revoke-user-sessions'], + ['/admin/remove-user'], + ['/delete-user'], + ['/get-session'], + ])('writes NOTHING when the delete came from %s', async (path) => { + // Each of these deletes a session row without the subject signing out — + // a revoke, an erasure, or better-auth's own collection of an expired row + // inside `GET /get-session`. `logout` there would name an action the + // subject never took, which is worse for an auditor than no row: the + // revoke families already carry their cause on the ADR-0069 D4 tombstone. + const { hooks, recordAuthEvent } = hooksWithSink(); + await hooks.session.delete.after(SESSION, { path }); + expect(recordAuthEvent).not.toHaveBeenCalled(); + }); + + it('writes nothing when the endpoint context is unknown (null ctx)', async () => { + // `delete.after` runs inside `queueAfterTransactionHook`, so the ambient + // auth context may be gone; the captured argument is the only reliable + // answer, and no answer means no row. + const { hooks, recordAuthEvent } = hooksWithSink(); + await hooks.session.delete.after(SESSION, null); + expect(recordAuthEvent).not.toHaveBeenCalled(); + }); + + it('the HOST session.delete.after still runs (back-channel logout must not be lost)', async () => { + const hostAfter = vi.fn(async () => 'host-result'); + const { hooks, recordAuthEvent } = hooksWithSink({ + databaseHooks: { session: { delete: { after: hostAfter } } }, + }); + + const result = await hooks.session.delete.after(SESSION, { path: SIGN_OUT_PATH }); + + expect(hostAfter).toHaveBeenCalledTimes(1); + expect(result).toBe('host-result'); + expect(recordAuthEvent).toHaveBeenCalledTimes(1); + }); + + it('a host session.delete.before survives composition untouched', async () => { + // @better-auth/oauth-provider registers `session.delete.before`/`after` to + // prepare and dispatch OIDC back-channel logout. Dropping either would + // trade an audit row for a security hole (`session-tombstone.ts`). + const before = vi.fn(async () => undefined); + const { hooks } = hooksWithSink({ + databaseHooks: { session: { delete: { before } } }, + }); + expect(hooks.session.delete.before).toBe(before); + }); +}); + +describe('[#8144] the mapping refuses to invent an actor', () => { + it('loginEventFor: no subject → null', () => { + expect(loginEventFor({ id: 'ses_1' })).toBeNull(); + expect(loginEventFor(null)).toBeNull(); + expect(loginEventFor({ userId: '' })).toBeNull(); + }); + + it('logoutEventFor: right path, no subject → null', () => { + expect(logoutEventFor({ id: 'ses_1' }, SIGN_OUT_PATH)).toBeNull(); + }); + + it('logoutEventFor: right subject, wrong path → null', () => { + expect(logoutEventFor(SESSION, '/revoke-session')).toBeNull(); + expect(logoutEventFor(SESSION, undefined)).toBeNull(); + }); + + it('omits absent optional fields rather than writing empty strings', () => { + // ADR-0118 D1: absence records as absence. An `ipAddress: ''` would read as + // a client fingerprint that was captured and was blank. + const event = loginEventFor({ userId: 'usr_1', ipAddress: '', userAgent: null as any }); + expect(event).toEqual({ action: 'login', userId: 'usr_1' }); + }); +}); diff --git a/packages/plugins/plugin-auth/src/auth-session-audit.ts b/packages/plugins/plugin-auth/src/auth-session-audit.ts new file mode 100644 index 0000000000..c843c4a6a2 --- /dev/null +++ b/packages/plugins/plugin-auth/src/auth-session-audit.ts @@ -0,0 +1,190 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8144, sub-issue A of #7675] `login` / `logout` in the compliance ledger — + * the AUTH half. + * + * ## What was missing + * + * `sys_audit_log.action` declares `login` and `logout`; nothing in the repo + * ever wrote either. The only trace a sign-in left was an unattributed `update + * sys_user` row (`user_id` null) diffing `last_login_at`, so the shipped + * `auth_events` list view was empty by construction. The maintainer ruling of + * 2026-08-12 on #7675 named the fix and the seam: + * + * > **补 writer(3 个)**:`login` / `logout`(auth 事件已有钩点,顺带解决那条 + * > `user_id` null 的未归因 `last_login_at` diff 行)… + * + * ## The hook points, and why these two + * + * Both are better-auth `databaseHooks` on the session model, composed in + * `AuthManager.composeDatabaseHooks`: + * + * - **`session.create.after` ⇒ `login`.** A session row IS a sign-in, whatever + * minted it — `/sign-in/email`, sign-up auto-sign-in, SSO, OAuth callback, + * magic link, email OTP, passkey. Branching on `/sign-in/email` in the + * endpoint middleware instead (where `stampLastLogin` lives) would have + * audited exactly one of those and silently missed every federated login, + * which on a real deployment is most of them. + * - **`session.delete.after` under `/sign-out` ⇒ `logout`.** Ending a session + * is the only thing a sign-out does, and the deleted row is handed to the + * hook, so the actor is known even though the request no longer has a + * session. + * + * ## Why logout is scoped to `/sign-out` + * + * A session row is deleted for many reasons that are not a logout: an admin + * revoke, `/revoke-session` on another device, ban, user erasure, and + * better-auth's own collection of an expired row inside `GET /get-session`. + * Recording those as `logout` would not be a vague audit record, it would be a + * **wrong** one — it names an action the subject did not take. The revoke + * families already have their own trail (ADR-0069 D4 tombstones, + * `session-tombstone.ts`), which is why that module left this question open: + * "whether `logout` earns an audit record at all is #7675's question, not this + * one." + * + * ## Why the endpoint path comes from the hook's `context` argument + * + * `deleteWithHooks` resolves `getCurrentAuthContext()` once, at entry, and + * hands the result to every `delete.after` hook — but it runs those hooks + * inside `queueAfterTransactionHook`, i.e. deferred. Calling + * `currentAuthEndpointPath()` from inside the deferred callback asks the + * AsyncLocalStorage a question it may no longer be able to answer, and on + * WebContainer (whose `node:async_hooks` does not propagate across `await`) + * it never could. The captured argument is the same value, taken at a moment it + * is guaranteed present. No context at all ⇒ the cause is unknown ⇒ no row: + * losing the record is the safe direction, inventing one is not. + */ + +/** + * The audit sink's shape, declared LOCALLY so plugin-auth takes no runtime + * dependency on plugin-audit — the mirror of `MessagingEmitSurface` / + * `AuditI18nSurface` in `plugin-audit/src/audit-writers.ts`, which declare + * messaging and i18n the same way for the same reason. The real implementation + * is `createAuthEventAuditSink` in that package, registered under the `audit` + * service slot; absent (audit plugin not installed) ⇒ no rows, no error. + * + * `action` is a closed literal union on purpose. Every `sys_audit_log` field is + * `readonly` and `validateRecord` skips readonly/system fields on both + * branches (#8203), so the declared enum validates nothing in either + * direction — a misspelled action would be accepted silently, at both ends. + * This union is the only thing standing between an author and that row. + */ +export interface AuthEventAuditSurface { + recordAuthEvent(event: { + action: 'login' | 'logout'; + userId: string; + sessionId?: string; + organizationId?: string; + ipAddress?: string; + userAgent?: string; + actor?: string; + context?: Record; + }): Promise; +} + +/** The event this module hands the sink, before it becomes a ledger row. */ +export type AuthSessionAuditEventInput = Parameters< + AuthEventAuditSurface['recordAuthEvent'] +>[0]; + +/** + * better-auth's session row, as the `databaseHooks` see it: camelCase field + * names, mapped back from the `sys_session` columns by the ObjectQL adapter + * (`auth-schema-config.ts` owns that mapping). + */ +interface BetterAuthSessionRow { + id?: unknown; + userId?: unknown; + activeOrganizationId?: unknown; + ipAddress?: unknown; + userAgent?: unknown; + /** Set by better-auth's admin plugin on an impersonation session. */ + impersonatedBy?: unknown; +} + +/** The endpoint whose whole job is ending the caller's own session. */ +export const SIGN_OUT_PATH = '/sign-out'; + +/** A non-empty string, or undefined — never `''`, never `String(undefined)`. */ +function str(v: unknown): string | undefined { + return typeof v === 'string' && v.length > 0 ? v : undefined; +} + +/** + * Map a created session row to the `login` event, or `null` when the row names + * no subject (nothing to attribute — and an unattributed auth row is the defect + * this card exists to remove, so it is better not written). + * + * `path` is the better-auth endpoint that minted the session; it is recorded as + * context, never used to gate — every session creation is a login. + */ +export function loginEventFor( + session: BetterAuthSessionRow | null | undefined, + path?: string, +): AuthSessionAuditEventInput | null { + const userId = str(session?.userId); + if (!userId) return null; + // An impersonation session belongs to its subject but was STARTED by an + // admin. `user_id` keeps the subject (the session really is theirs, and the + // lookup must still join); `actor` names the principal that acted, which is + // exactly what that field is for. Dropping the row instead would hide the + // single most sensitive session creation in the system. + const impersonatedBy = str(session?.impersonatedBy); + const context: Record = {}; + if (path) context.endpoint = path; + if (impersonatedBy) context.impersonated_by = impersonatedBy; + return { + action: 'login', + userId, + sessionId: str(session?.id), + organizationId: str(session?.activeOrganizationId), + ipAddress: str(session?.ipAddress), + userAgent: str(session?.userAgent), + ...(impersonatedBy ? { actor: impersonatedBy } : {}), + ...(Object.keys(context).length > 0 ? { context } : {}), + }; +} + +/** + * Map a deleted session row to the `logout` event, or `null` when this delete + * is not a sign-out (see the module note: a revoke, a ban, an erasure and the + * expired-row collector all reach the same hook, and none of them is a logout). + */ +export function logoutEventFor( + session: BetterAuthSessionRow | null | undefined, + path?: string, +): AuthSessionAuditEventInput | null { + if (path !== SIGN_OUT_PATH) return null; + const userId = str(session?.userId); + if (!userId) return null; + return { + action: 'logout', + userId, + sessionId: str(session?.id), + organizationId: str(session?.activeOrganizationId), + ipAddress: str(session?.ipAddress), + userAgent: str(session?.userAgent), + context: { endpoint: path }, + }; +} + +/** + * Hand one event to the sink, swallowing everything. + * + * Auth must never fail on bookkeeping: the sign-in has already happened, the + * session is on disk, and the user is holding a valid token. The sink itself + * reports a lost row at `error` (once per process) — that is where the + * durability report belongs, and duplicating it here would double every line. + */ +export async function emitAuthSessionAuditEvent( + sink: AuthEventAuditSurface | undefined, + event: AuthSessionAuditEventInput | null, +): Promise { + if (!sink || !event || typeof sink.recordAuthEvent !== 'function') return; + try { + await sink.recordAuthEvent(event); + } catch { + /* never break the auth response — the sink already reported it */ + } +} diff --git a/packages/plugins/plugin-auth/src/index.ts b/packages/plugins/plugin-auth/src/index.ts index fe7e55e8d5..abccb1abb3 100644 --- a/packages/plugins/plugin-auth/src/index.ts +++ b/packages/plugins/plugin-auth/src/index.ts @@ -51,6 +51,11 @@ export * from './session-tombstone.js'; // `isSystem` for authorization, `attributedUserId` for attribution — rather // than inventing a second way to say "the system did this, for that person". export * from './auth-actor-attribution.js'; +// [#8144] The `login`/`logout` ledger seam. Exported because a host that +// composes its own `databaseHooks.session.*` (the cloud does) needs the same +// mapping — and because `AuthEventAuditSurface` is the shape anything +// registering the `audit` service must satisfy. +export * from './auth-session-audit.js'; export * from './auth-schema-config.js'; // ADR-0093 — membership reconciler + tenancy service (public host API: hosts // compose the reconciler into their own hooks; embeddings query tenancy mode). diff --git a/packages/qa/dogfood/test/auth-session-audit-trail.dogfood.test.ts b/packages/qa/dogfood/test/auth-session-audit-trail.dogfood.test.ts new file mode 100644 index 0000000000..f32c07961e --- /dev/null +++ b/packages/qa/dogfood/test/auth-session-audit-trail.dogfood.test.ts @@ -0,0 +1,259 @@ +// Copyright (c) 2026 ObjectStack. Licensed under the Apache-2.0 license. + +/** + * [#8144, sub-issue A of #7675] A real sign-in leaves a `login` row that names + * its actor and its tenant — measured end to end, exactly as #7675 measured its + * absence. + * + * ## The reproduction, inverted + * + * #7675, twice: fresh boot → sign up + sign in a member → as admin + * `GET /api/v1/data/sys_audit_log?$filter={"action":"login"}` → **total 0**; + * the only trace was an unattributed `update sys_user` row (`user_id` null) + * diffing `last_login_at`. This file runs that same script and asserts the + * opposite, through the same route. + * + * ## Why every assertion reads the row back + * + * Every `sys_audit_log` field is `readonly`, and `validateRecord` skips + * readonly/system fields on both branches (#8203), so the `action` enum + * declares a vocabulary **nothing validates in either direction** — a row + * carrying an action the enum never heard of is accepted silently, and a writer + * that never ran throws nothing either. On this object "no error was raised" is + * not evidence of anything. So the claims here are made against rows read back + * through the platform's own read paths: the data API for the acceptance + * criterion (which also proves an admin can actually SEE the rows through RLS — + * a row written into a tenant nobody can read would satisfy a `ql.find` under a + * system context and still leave the `auth_events` view empty), and the engine + * for the field-level detail. + * + * Harness notes: + * - `bootStack` installs no audit plugin; `AuditPlugin` is added here, which is + * both what registers the `audit` service the auth hooks call and what turns + * ordinary writes into ledger rows. + * - `orgContext: true` binds the harness admin to an organization, so their + * session carries an `activeOrganizationId` — without it the "and tenant" + * half of the acceptance criterion could not be measured at all. + * - Audit rows land ASYNCHRONOUSLY: better-auth settles `session.create.after` + * /`session.delete.after` through `queueAfterTransactionHook`, after the + * response. Every read polls. + */ + +import { describe, it, expect, beforeAll, afterAll } from 'vitest'; +import showcaseStack from '@objectstack/example-showcase'; +import { bootStack, type VerifyStack } from '@objectstack/verify'; +import { AuditPlugin } from '@objectstack/plugin-audit'; + +const SYSTEM_CTX = { isSystem: true }; +const MEMBER_EMAIL = 'member.8144@example.com'; +const MEMBER_PASSWORD = 'Member!Pass8144'; + +async function findRows(ql: any, object: string, where: any, limit = 200): Promise { + const rows = await ql.find(object, { where, limit }, { context: SYSTEM_CTX }); + return Array.isArray(rows) ? rows : (rows?.records ?? []); +} + +/** Rows out of a data-API list response, whichever envelope it uses. */ +async function rowsOf(res: Response): Promise { + const body = (await res.json()) as any; + return body?.records ?? body?.data ?? (Array.isArray(body) ? body : []); +} + +/** Poll until `predicate` holds, then return the rows that satisfied it. */ +async function waitForRows( + load: () => Promise, + predicate: (rows: any[]) => boolean, + what: string, +): Promise { + let rows: any[] = []; + for (let i = 0; i < 40; i++) { + rows = await load(); + if (predicate(rows)) return rows; + await new Promise((r) => setTimeout(r, 250)); + } + throw new Error( + `${what} — last saw ${rows.length} row(s): ` + + JSON.stringify(rows.map((r) => ({ action: r.action, user_id: r.user_id, tenant: r.tenant_id }))), + ); +} + +describe('#8144: auth session events reach sys_audit_log', () => { + let stack: VerifyStack; + let ql: any; + let adminToken: string; + let memberToken: string; + let memberUserId: string; + let memberOrgId: string; + + beforeAll(async () => { + stack = await bootStack(showcaseStack, { extraPlugins: [new AuditPlugin()], orgContext: true }); + ql = await stack.kernel.getServiceAsync('objectql'); + adminToken = await stack.signIn(); + + // #7675's script: a member signs up and signs in. + await stack.signUp(MEMBER_EMAIL, MEMBER_PASSWORD, 'Member 8144'); + const [memberUser] = await findRows(ql, 'sys_user', { email: MEMBER_EMAIL }, 1); + memberUserId = String(memberUser.id); + + // Settle the membership BEFORE the measured sign-in, and say why rather + // than sleeping: `session.create.before` derives `activeOrganizationId` + // from the caller's `sys_member` row, and ADR-0093's reconciler runs on + // `user.create.after`, which better-auth defers past the signup + // transaction. So a user's very FIRST session — the one sign-up mints — + // legitimately predates their membership and carries no active org; its + // ledger row therefore has no tenant, and no seam downstream can invent + // one. That is a property of the sign-up ordering, not of this writer, and + // it is why the tenant half is measured on an ordinary sign-in. + const [membership] = await waitForRows( + () => findRows(ql, 'sys_member', { user_id: memberUserId }, 5), + (rows) => rows.length > 0, + 'ADR-0093 reconciler never bound the new member to an organization', + ); + memberOrgId = String(membership.organization_id); + memberToken = await stack.signIn(MEMBER_EMAIL, MEMBER_PASSWORD); + }, 180_000); + + afterAll(async () => { + await stack?.stop?.(); + }); + + // ── The acceptance criterion, through the reported route ──────────────── + + it('the issue\'s own query returns the login event, with actor and tenant', async () => { + const query = `/data/sys_audit_log?$filter=${encodeURIComponent(JSON.stringify({ action: 'login' }))}`; + const rows = await waitForRows( + async () => { + const res = await stack.apiAs(adminToken, 'GET', query); + expect(res.status, await res.clone().text()).toBe(200); + return rowsOf(res); + }, + (found) => found.some((r: any) => r.user_id === memberUserId && r.tenant_id), + 'GET /data/sys_audit_log?$filter={"action":"login"} never returned the member\'s login (this was `total 0` before #8144)', + ); + + const mine = rows.filter((r: any) => r.user_id === memberUserId); + expect(mine.length).toBeGreaterThan(0); + for (const row of mine) { + // WHAT happened… + expect(row.action).toBe('login'); + // …and WHO — the half #7675 called out as missing (`user_id` null). + expect(row.user_id).toBe(memberUserId); + } + + // …and WHERE, so the row survives the RLS predicate a non-admin reads + // through: a tenant-less row is invisible to every member of every org. + // Checked against the membership the platform actually wrote, NOT against + // anything this writer passed in — an expectation and a reality drawn from + // the same source could not disagree. + const tenanted = mine.filter((r: any) => r.tenant_id); + expect(tenanted.length, 'no login row carries a tenant').toBeGreaterThan(0); + for (const row of tenanted) expect(row.tenant_id).toBe(memberOrgId); + + // The actor is a REAL sys_user row, not a sentinel (ADR-0118 D1). + expect(await findRows(ql, 'sys_user', { id: mine[0].user_id }, 1)).toHaveLength(1); + }, 120_000); + + it('the shipped `auth_events` list view stops being empty', async () => { + // The view shipped against rows nothing wrote, so it was permanently empty + // by construction — that is the user-visible half of #7675. + // + // Its filter is READ FROM THE RUNNING REGISTRY, never copied here. The + // vocabulary moves: #8200 retired `permission_change` and `export` from the + // action enum and narrowed this very view in the same PR, and a hard-coded + // copy would have gone on querying a value nothing can hold while still + // reporting success (the view has exactly the shape that makes that + // invisible — it would just find the login rows and pass). + const schema: any = (ql as any).getSchema('sys_audit_log'); + const actions: string[] = schema?.listViews?.auth_events?.filter?.[0]?.value ?? []; + expect(actions.length, 'auth_events view declares no action filter').toBeGreaterThan(0); + expect(actions).toContain('login'); + + const query = `/data/sys_audit_log?$filter=${encodeURIComponent( + JSON.stringify({ action: { $in: actions } }), + )}`; + const res = await stack.apiAs(adminToken, 'GET', query); + expect(res.status, await res.clone().text()).toBe(200); + const rows = await rowsOf(res); + expect(rows.length, 'the auth_events view is still empty').toBeGreaterThan(0); + expect(rows.every((r: any) => actions.includes(r.action))).toBe(true); + }, 60_000); + + // ── The other half of the ruling: logout ──────────────────────────────── + + it('signing out writes a `logout` row for the same actor', async () => { + const before = (await findRows(ql, 'sys_audit_log', { action: 'logout', user_id: memberUserId })).length; + + const res = await stack.apiAs(memberToken, 'POST', '/auth/sign-out', {}); + expect(res.status, await res.clone().text()).toBe(200); + + const rows = await waitForRows( + () => findRows(ql, 'sys_audit_log', { action: 'logout', user_id: memberUserId }), + (found) => found.length > before, + 'POST /auth/sign-out wrote no `logout` row', + ); + const row = rows[rows.length - 1]; + expect(row.action).toBe('logout'); + expect(row.user_id).toBe(memberUserId); + // Navigable back to the session that ended. + expect(row.object_name).toBe('sys_session'); + expect(String(row.metadata)).toContain('/sign-out'); + }, 120_000); + + it('an admin revoke is NOT recorded as the member logging out', async () => { + // A session row is deleted by revokes, bans, erasure and better-auth's own + // collection of expired rows. Recording those as `logout` would name an + // action the subject never took — worse for an auditor than no row, and the + // revoke already carries its own cause on the ADR-0069 D4 tombstone. + const freshToken = await stack.signIn(MEMBER_EMAIL, MEMBER_PASSWORD); + await waitForRows( + () => findRows(ql, 'sys_session', { user_id: memberUserId }), + (rows) => rows.some((r: any) => !r.revoked_at), + 'no live session for the member after signing back in', + ); + const logoutsBefore = ( + await findRows(ql, 'sys_audit_log', { action: 'logout', user_id: memberUserId }) + ).length; + + const res = await stack.apiAs(freshToken, 'POST', '/auth/revoke-sessions', {}); + expect(res.status, await res.clone().text()).toBe(200); + + // The revoke really happened — otherwise the assertion below is vacuous. + await waitForRows( + () => findRows(ql, 'sys_session', { user_id: memberUserId }), + (rows) => rows.length > 0 && rows.every((r: any) => r.revoked_at), + 'revoke-sessions left a live session behind', + ); + // Give any late-settling hook the same window a positive assertion gets. + await new Promise((r) => setTimeout(r, 1_000)); + expect( + (await findRows(ql, 'sys_audit_log', { action: 'logout', user_id: memberUserId })).length, + ).toBe(logoutsBefore); + }, 120_000); + + // ── The incidental defect the ruling named ────────────────────────────── + + it('the `last_login_at` system write is attributed to the user who signed in', async () => { + // #7675: "the only trace is an **unattributed** `update sys_user` row + // (`user_id` null) diffing `last_login_at`". The row still exists — a login + // from a new address is worth keeping — but it now names its actor, through + // the platform's one attribution channel (`attributedUserId`, #4586). + await stack.signIn(MEMBER_EMAIL, MEMBER_PASSWORD); + + const rows = await waitForRows( + () => + findRows(ql, 'sys_audit_log', { + object_name: 'sys_user', + record_id: memberUserId, + action: 'update', + }), + (found) => found.some((r: any) => String(r.new_value ?? '').includes('last_login_at')), + 'no `update sys_user` row diffing last_login_at appeared', + ); + + const lastLoginRows = rows.filter((r: any) => String(r.new_value ?? '').includes('last_login_at')); + expect(lastLoginRows.length).toBeGreaterThan(0); + for (const row of lastLoginRows) { + expect(row.user_id, '`last_login_at` diff row is still unattributed').toBe(memberUserId); + } + }, 120_000); +}); diff --git a/scripts/check-durability-degradation-log-level.mjs b/scripts/check-durability-degradation-log-level.mjs index 33a5dbe136..89c97de118 100644 --- a/scripts/check-durability-degradation-log-level.mjs +++ b/scripts/check-durability-degradation-log-level.mjs @@ -187,6 +187,10 @@ const DURABILITY_CRITICAL_CALLEES = new Map([ 'persistAuditTrailRow', 'The compliance audit row was never written — the audited write itself succeeded and returned 200, so the API, the data and every counter read clean, while the `sys_audit_log` entry that records WHO did it is simply absent and nothing retries it. The gap surfaces, if ever, to an auditor who cannot connect it back to the write (#5226, the #4420 shape on the compliance ledger).', ], + [ + 'persistAuthEventAuditRow', + 'The compliance audit row for a sign-in / sign-out was never written — the auth request itself succeeded and the user holds a valid session, so the API, the cookie and every counter read clean, while the `sys_audit_log` row recording WHO signed in is simply absent and nothing retries it. The shipped `auth_events` list view and the system-overview widgets read exactly those rows, so the screen an operator checks stays empty and healthy-looking (#8144, the #5226 shape on the auth seam).', + ], [ 'deleteMetaItemFromLoader', 'The metadata definition was never deleted from the authoritative store — `unregister()` still resolves and still announces `deleted`, the in-memory registry entry is gone, and the surviving row is read straight back out of storage by the very next `list()`/`get()`, so the "deleted" item reappears and survives every restart. Nothing retries it (#5259).',