fix(security): stop an audit write failure disabling account lockout

.changeset/quiet-bats-guard.md

@@ -0,0 +1,33 @@
+---
+'seamless-auth-api': minor
+---
+
+Stop an audit write failure from silently disabling account lockout.
+
+Failed attempts were counted by querying `auth_events`, whose writes swallow every
+error. Any condition that degraded audit writes while leaving the service running
+stopped failures being counted, so lockout silently stopped enforcing on every
+account while authentication carried on. Disk exhaustion, a table lock, a failed
+migration or connection pool exhaustion would all do it, and the absent records
+are the same absent records that would have shown it happening. The practical
+difference was a bounded versus an unbounded guessing attack against a numeric
+OTP.
+
+Failed attempts now go to their own `auth_failures` table, written separately from
+the audit event and read only by the lockout policy, so losing the trail no longer
+loses the control.
+
+`getUserLockoutStatus` refuses rather than guessing when it cannot read the
+counter: an authentication the server cannot vouch for gets the same `423` a
+locked account gets.
+
+Audit write failures are reported where monitoring already looks.
+`GET /health/status` answers `200 { "message": "System up, audit degraded",
+"degraded": { "audit": { … } } }` for five minutes after one. The healthy body is
+unchanged, so anything already parsing it is unaffected, and the status stays
+`200` because the service is still serving. That is the defined action NIST
+800-53 AU-5 asks for; a log line nobody reads is not.
+
+Audit writes themselves still do not throw. 137 call sites await them, many from
+inside error handlers, so failing there would turn a bookkeeping failure into a
+failed request.

docs/security-posture.md

@@ -252,3 +252,56 @@ continues, because failing an authentication over a housekeeping step is worse
 than briefly exceeding the cap.
 
 This is NIST 800-53 AC-10.
+
+## Audit write failure
+
+**Posture: the trail may be lost, no security control may be.**
+
+`AuthEventService` still swallows a failed write to `auth_events`. 137 call sites
+await it, many from inside error handlers, so throwing there would turn a
+bookkeeping failure into a failed request and would take down the very paths that
+report problems.
+
+What changed is that a failure is no longer silent, and no longer takes anything
+with it.
+
+### Nothing security-relevant is derived from the audit trail
+
+Account lockout used to count failed attempts by querying `auth_events`. The
+control and its telemetry were one data path, and that path fails open: a
+condition that degraded audit writes while leaving the service running stopped
+failures being counted, so lockout silently stopped enforcing on every account
+while authentication carried on. Disk exhaustion, a table lock, a failed
+migration or pool exhaustion would all do it, and the missing records are the same
+missing records that would have shown it happening.
+
+Failed attempts are now recorded in `auth_failures`, written separately from the
+audit event and read only by the lockout policy. Losing the trail no longer loses
+the control.
+
+### An unknown count means locked
+
+`getUserLockoutStatus` refuses rather than guessing when it cannot read the
+counter. An authentication the server cannot vouch for is refused with the same
+`423` a locked account gets, rather than admitted because the count came back
+empty.
+
+### Failures are reported where monitoring already looks
+
+`GET /health/status` answers `200 { "message": "System up, audit degraded",
+"degraded": { "audit": { … } } }` for five minutes after a failed audit write. The
+healthy body is unchanged, so anything already parsing it is unaffected, and the
+status stays `200` because the service is still serving and should not be pulled
+from a load balancer for this. This is the defined action NIST 800-53 AU-5 asks
+for on audit logging failure; a line in the application log is not one, because
+nothing reads it.
+
+### What is still accepted
+
+An audit write that fails is still a lost record. Recording is best effort by
+design, and the tradeoff is stated here rather than made by default. A deployment
+that needs authentication to fail closed on audit failure does not have that
+option today.
+
+`auth_failures` rows are not pruned, which matches `auth_events`. Retention is
+[issue #173](https://github.com/fells-code/seamless-auth-api/issues/173).

openapi.json

@@ -3194,10 +3194,29 @@
             "description": "HTTP 200",
             "content": {
               "application/json": {
-                "example": { "message": "string" },
+                "example": {
+                  "message": "string",
+                  "degraded": { "audit": { "failureCount": 0, "lastFailureAt": "string" } }
+                },
                 "schema": {
                   "type": "object",
-                  "properties": { "message": { "type": "string" } },
+                  "properties": {
+                    "message": { "type": "string" },
+                    "degraded": {
+                      "type": "object",
+                      "properties": {
+                        "audit": {
+                          "type": "object",
+                          "properties": {
+                            "failureCount": { "type": "number" },
+                            "lastFailureAt": { "type": "string", "nullable": true }
+                          },
+                          "required": ["failureCount", "lastFailureAt"]
+                        }
+                      },
+                      "required": ["audit"]
+                    }
+                  },
                   "required": ["message"]
                 }
               }

src/controllers/health.ts

@@ -7,11 +7,30 @@
 import { Request, Response } from 'express';
 
 import { getPackageVersion } from '../openapi/document.js';
+import { getAuditHealth } from '../services/auditHealth.js';
 import getLogger from '../utils/logger.js';
 
 const logger = getLogger('health');
 
 export const healthCheck = (req: Request, res: Response) => {
+  const audit = getAuditHealth();
+
+  // Still 200: the service is serving requests, and a load balancer should not
+  // pull it out for this. What changes is that an instance which has stopped
+  // being able to record what it is doing says so, which is the defined action
+  // NIST 800-53 AU-5 asks for. Nothing is added to the body while healthy.
+  if (audit.degraded) {
+    return res.status(200).json({
+      message: 'System up, audit degraded',
+      degraded: {
+        audit: {
+          failureCount: audit.failureCount,
+          lastFailureAt: audit.lastFailureAt,
+        },
+      },
+    });
+  }
+
   return res.status(200).json({ message: 'System up' });
 };
 

src/generated/api.ts

@@ -3026,11 +3026,23 @@ export interface paths {
           content: {
             /**
              * @example {
-             *       "message": "string"
+             *       "message": "string",
+             *       "degraded": {
+             *         "audit": {
+             *           "failureCount": 0,
+             *           "lastFailureAt": "string"
+             *         }
+             *       }
              *     }
              */
             'application/json': {
               message: string;
+              degraded?: {
+                audit: {
+                  failureCount: number;
+                  lastFailureAt: string | null;
+                };
+              };
             };
           };
         };

src/migrations/20260831010000-create-auth-failures.cjs

@@ -0,0 +1,64 @@
+'use strict';
+
+/**
+ * A dedicated store for the failed authentication attempts that drive account
+ * lockout.
+ *
+ * The count used to be derived from `auth_events`, whose writes swallow every
+ * error, so any condition that degraded audit writes stopped failures being
+ * counted and silently disabled lockout on every account while authentication
+ * carried on. The control and its telemetry were the same data path, failing
+ * open at both ends.
+ *
+ * This table exists so losing the audit trail does not lose the control. It is
+ * written separately from the audit event and carries only what the lockout
+ * decision needs.
+ *
+ * @type {import('sequelize-cli').Migration}
+ */
+module.exports = {
+  async up(queryInterface, Sequelize) {
+    await queryInterface.createTable('auth_failures', {
+      id: {
+        type: Sequelize.UUID,
+        defaultValue: Sequelize.literal('gen_random_uuid()'),
+        primaryKey: true,
+      },
+      user_id: {
+        type: Sequelize.UUID,
+        allowNull: false,
+        references: { model: 'users', key: 'id' },
+        onDelete: 'CASCADE',
+      },
+      type: {
+        type: Sequelize.STRING,
+        allowNull: false,
+      },
+      occurred_at: {
+        type: Sequelize.DATE,
+        allowNull: false,
+        defaultValue: Sequelize.literal('CURRENT_TIMESTAMP'),
+      },
+      created_at: {
+        type: Sequelize.DATE,
+        allowNull: false,
+        defaultValue: Sequelize.literal('CURRENT_TIMESTAMP'),
+      },
+      updated_at: {
+        type: Sequelize.DATE,
+        allowNull: false,
+        defaultValue: Sequelize.literal('CURRENT_TIMESTAMP'),
+      },
+    });
+
+    // The only query this table serves: failures for one user inside a window.
+    await queryInterface.addIndex('auth_failures', ['user_id', 'occurred_at'], {
+      name: 'auth_failures_user_id_occurred_at_idx',
+    });
+  },
+
+  async down(queryInterface) {
+    await queryInterface.removeIndex('auth_failures', 'auth_failures_user_id_occurred_at_idx');
+    await queryInterface.dropTable('auth_failures');
+  },
+};

src/models/authFailures.ts

@@ -0,0 +1,69 @@
+/*
+ * Copyright © 2026 Fells Code, LLC
+ * Licensed under the GNU Affero General Public License v3.0
+ * See LICENSE file in the project root for full license information
+ */
+
+import { DataTypes, Model, Sequelize } from 'sequelize';
+
+export interface AuthFailureAttributes {
+  id?: string;
+  userId: string;
+  type: string;
+  occurredAt?: Date;
+  createdAt?: Date;
+  updatedAt?: Date;
+}
+
+/**
+ * A failed authentication attempt, counted by the lockout policy.
+ *
+ * Deliberately separate from `AuthEvent`. The audit trail swallows write errors
+ * so that recording an event can never fail the operation it describes, which is
+ * the right posture for a record and the wrong one for a security control that
+ * is derived from it.
+ */
+export class AuthFailure extends Model<AuthFailureAttributes> implements AuthFailureAttributes {
+  declare id: string;
+  declare userId: string;
+  declare type: string;
+  declare occurredAt: Date;
+  declare readonly createdAt: Date;
+  declare readonly updatedAt: Date;
+}
+
+const initializeAuthFailureModel = (sequelize: Sequelize) => {
+  AuthFailure.init(
+    {
+      id: {
+        type: DataTypes.UUID,
+        primaryKey: true,
+        defaultValue: DataTypes.UUIDV4,
+        allowNull: false,
+      },
+      userId: {
+        type: DataTypes.UUID,
+        allowNull: false,
+      },
+      type: {
+        type: DataTypes.STRING,
+        allowNull: false,
+      },
+      occurredAt: {
+        type: DataTypes.DATE,
+        allowNull: false,
+        defaultValue: DataTypes.NOW,
+      },
+    },
+    {
+      sequelize,
+      modelName: 'AuthFailure',
+      tableName: 'auth_failures',
+      underscored: true,
+    },
+  );
+
+  return AuthFailure;
+};
+
+export default initializeAuthFailureModel;

src/schemas/health.responses.ts

@@ -8,6 +8,18 @@ import { z } from 'zod';
 
 export const HealthStatusResponseSchema = z.object({
   message: z.string(),
+  /**
+   * Present only when something is wrong, so the healthy body is unchanged for
+   * anything already parsing it.
+   */
+  degraded: z
+    .object({
+      audit: z.object({
+        failureCount: z.number(),
+        lastFailureAt: z.string().nullable(),
+      }),
+    })
+    .optional(),
 });
 
 export const VersionResponseSchema = z.object({