feat(backend): Audit Logger — signature verification, SQL optimization, circuit-breaker error recovery, security hardening

backend/src/lib/audit-security.js

@@ -5,6 +5,28 @@ const DEFAULT_AUDIT_RATE_LIMIT_MAX = 60;
 const DEFAULT_AUDIT_RATE_LIMIT_WINDOW_MS = 60_000;
 const DEFAULT_AUDIT_FIELD_MAX_LENGTH = 2048;
 
+/**
+ * Allowlist of permitted audit action identifiers.
+ * Restricting actions to a known set prevents log-injection attacks where
+ * a crafted action string embeds control characters or arbitrary content
+ * into the audit trail (issue #772).
+ */
+const ALLOWED_AUDIT_ACTIONS = new Set([
+  "login",
+  "logout",
+  "update",
+  "create",
+  "delete",
+  "export",
+  "import",
+  "password_change",
+  "api_key_rotate",
+  "webhook_trigger",
+  "payment_initiated",
+  "payment_confirmed",
+  "payment_failed",
+]);
+
 const auditRateLimitState = new Map();
 
 function stableStringify(value) {
@@ -66,6 +88,45 @@ export function signAuditPayload(payload, secret = process.env.AUDIT_LOG_SIGNING
     .digest("hex");
 }
 
+/**
+ * Verify a stored HMAC signature against the original payload using a
+ * constant-time comparison to prevent timing-oracle attacks (issue #769).
+ *
+ * @param {object} payload   - The original audit payload object
+ * @param {string} signature - The hex-encoded HMAC stored in the database
+ * @param {string} [secret]  - Signing secret (defaults to AUDIT_LOG_SIGNING_SECRET env var)
+ * @returns {boolean} true if the signature is valid and untampered
+ */
+export function verifyAuditSignature(payload, signature, secret = process.env.AUDIT_LOG_SIGNING_SECRET) {
+  if (!secret || !signature || typeof signature !== "string") return false;
+
+  const expected = signAuditPayload(payload, secret);
+  if (!expected) return false;
+
+  try {
+    // timingSafeEqual requires same-length Buffers; mismatched lengths → false
+    const expectedBuf = Buffer.from(expected, "hex");
+    const actualBuf = Buffer.from(signature, "hex");
+    if (expectedBuf.length !== actualBuf.length) return false;
+    return crypto.timingSafeEqual(expectedBuf, actualBuf);
+  } catch {
+    return false;
+  }
+}
+
+/**
+ * Validate that an action string is within the known-safe allowlist before
+ * storing it in the audit trail.  Prevents log-injection through crafted
+ * action values (issue #772).
+ *
+ * @param {string} action
+ * @returns {boolean}
+ */
+export function validateAuditAction(action) {
+  if (!action || typeof action !== "string") return false;
+  return ALLOWED_AUDIT_ACTIONS.has(action.toLowerCase().trim());
+}
+
 export function createAuditLogRateLimitKey({ merchantId, action, ipAddress }) {
   return [merchantId || "anonymous", action || "unknown-action", ipAddress || "unknown-ip"].join(":");
 }

backend/src/lib/audit-security.test.js

@@ -7,6 +7,8 @@ import {
   sanitizeAuditKey,
   sanitizeAuditValue,
   signAuditPayload,
+  validateAuditAction,
+  verifyAuditSignature,
 } from "./audit-security.js";
 
 describe("audit-security", () => {
@@ -37,6 +39,78 @@ describe("audit-security", () => {
     expect(signature).toMatch(/^[a-f0-9]{64}$/);
   });
 
+  it("returns null signature when no secret is provided", () => {
+    const payload = { merchant_id: "m1", action: "login" };
+    const sig = signAuditPayload(payload, undefined);
+    expect(sig).toBeNull();
+  });
+
+  // ── verifyAuditSignature (issue #769) ──────────────────────────────────────
+
+  it("verifies a valid HMAC signature", () => {
+    const secret = "test-secret";
+    const payload = { merchant_id: "m1", action: "login", status: "success" };
+    const signature = signAuditPayload(payload, secret);
+    expect(verifyAuditSignature(payload, signature, secret)).toBe(true);
+  });
+
+  it("rejects a tampered payload signature", () => {
+    const secret = "test-secret";
+    const payload = { merchant_id: "m1", action: "login", status: "success" };
+    const signature = signAuditPayload(payload, secret);
+    const tampered = { ...payload, status: "failure" };
+    expect(verifyAuditSignature(tampered, signature, secret)).toBe(false);
+  });
+
+  it("rejects a tampered signature string", () => {
+    const secret = "test-secret";
+    const payload = { merchant_id: "m1", action: "login" };
+    const signature = signAuditPayload(payload, secret);
+    const bad = signature.replace(/.$/, signature.endsWith("a") ? "b" : "a");
+    expect(verifyAuditSignature(payload, bad, secret)).toBe(false);
+  });
+
+  it("returns false when signature is null", () => {
+    const payload = { merchant_id: "m1", action: "login" };
+    expect(verifyAuditSignature(payload, null, "secret")).toBe(false);
+  });
+
+  it("returns false when secret is not provided", () => {
+    const payload = { merchant_id: "m1", action: "login" };
+    expect(verifyAuditSignature(payload, "a".repeat(64))).toBe(false);
+  });
+
+  it("is resistant to length-extension by using timingSafeEqual", () => {
+    // Signatures of different length must not throw; they return false
+    const secret = "test-secret";
+    const payload = { merchant_id: "m1", action: "login" };
+    expect(verifyAuditSignature(payload, "short", secret)).toBe(false);
+  });
+
+  // ── validateAuditAction (issue #772) ──────────────────────────────────────
+
+  it("accepts known allowed action values", () => {
+    expect(validateAuditAction("login")).toBe(true);
+    expect(validateAuditAction("update")).toBe(true);
+    expect(validateAuditAction("payment_initiated")).toBe(true);
+  });
+
+  it("rejects unknown action values", () => {
+    expect(validateAuditAction("DROP TABLE audit_logs")).toBe(false);
+    expect(validateAuditAction("arbitrary_action")).toBe(false);
+    expect(validateAuditAction("")).toBe(false);
+  });
+
+  it("is case-insensitive for action validation", () => {
+    expect(validateAuditAction("LOGIN")).toBe(true);
+    expect(validateAuditAction("Update")).toBe(true);
+  });
+
+  it("rejects null and undefined actions", () => {
+    expect(validateAuditAction(null)).toBe(false);
+    expect(validateAuditAction(undefined)).toBe(false);
+  });
+
   it("enforces per-key rate limiting in a fixed window", () => {
     const key = createAuditLogRateLimitKey({
       merchantId: "m1",

backend/src/lib/audit.js

@@ -17,6 +17,7 @@ import {
   hashAuditPayload,
   sanitizeAuditValue,
   signAuditPayload,
+  validateAuditAction,
 } from "./audit-security.js";
 import fs from "node:fs";
 import path from "node:path";
@@ -27,6 +28,57 @@ const AUDIT_FALLBACK_LOG_PATH = process.env.AUDIT_FALLBACK_LOG_PATH || path.join
 const AUDIT_DB_RETRY_ATTEMPTS = Number.parseInt(process.env.AUDIT_DB_RETRY_ATTEMPTS || "2", 10);
 const AUDIT_DB_RETRY_DELAY_MS = Number.parseInt(process.env.AUDIT_DB_RETRY_DELAY_MS || "100", 10);
 
+/**
+ * Circuit-breaker state for the audit DB path (issue #771).
+ * After CIRCUIT_FAILURE_THRESHOLD consecutive DB failures the circuit opens
+ * and all writes are routed directly to the fallback log for
+ * CIRCUIT_RESET_MS milliseconds before a single probe attempt is made.
+ */
+const CIRCUIT_FAILURE_THRESHOLD = Number.parseInt(process.env.AUDIT_CIRCUIT_FAILURE_THRESHOLD || "5", 10);
+const CIRCUIT_RESET_MS = Number.parseInt(process.env.AUDIT_CIRCUIT_RESET_MS || "60000", 10);
+
+const _auditCircuit = {
+  open: false,
+  failures: 0,
+  openedAt: 0,
+};
+
+export function getAuditCircuitState() {
+  return { ..._auditCircuit };
+}
+
+export function _resetAuditCircuitForTests() {
+  _auditCircuit.open = false;
+  _auditCircuit.failures = 0;
+  _auditCircuit.openedAt = 0;
+}
+
+function isCircuitOpen(now = Date.now()) {
+  if (!_auditCircuit.open) return false;
+  if (now - _auditCircuit.openedAt >= CIRCUIT_RESET_MS) {
+    // Half-open: allow a single probe through
+    _auditCircuit.open = false;
+    return false;
+  }
+  return true;
+}
+
+function recordCircuitSuccess() {
+  _auditCircuit.failures = 0;
+  _auditCircuit.open = false;
+}
+
+function recordCircuitFailure(now = Date.now()) {
+  _auditCircuit.failures += 1;
+  if (_auditCircuit.failures >= CIRCUIT_FAILURE_THRESHOLD) {
+    _auditCircuit.open = true;
+    _auditCircuit.openedAt = now;
+    console.warn(
+      `[audit] Circuit breaker opened after ${_auditCircuit.failures} consecutive failures. DB writes suspended for ${CIRCUIT_RESET_MS}ms.`,
+    );
+  }
+}
+
 function sleep(ms) {
   return new Promise((resolve) => setTimeout(resolve, ms));
 }
@@ -46,6 +98,10 @@ function writeFallbackLog(payload, error) {
 }
 
 async function insertAuditLog({ payload, payloadHash, signature }) {
+  if (isCircuitOpen()) {
+    return { success: false, error: new Error("Circuit breaker open: DB writes suspended"), circuitOpen: true };
+  }
+
   for (let attempt = 0; attempt <= AUDIT_DB_RETRY_ATTEMPTS; attempt += 1) {
     try {
       await pool.query(
@@ -61,10 +117,12 @@ async function insertAuditLog({ payload, payloadHash, signature }) {
           signature,
         ],
       );
+      recordCircuitSuccess();
       return { success: true };
     } catch (err) {
       const isRetryable = attempt < AUDIT_DB_RETRY_ATTEMPTS && isRetryablePoolError(err);
       if (!isRetryable) {
+        recordCircuitFailure();
         return { success: false, error: err };
       }
       const delayMs = AUDIT_DB_RETRY_DELAY_MS * (attempt + 1);
@@ -74,6 +132,7 @@ async function insertAuditLog({ payload, payloadHash, signature }) {
       await sleep(delayMs);
     }
   }
+  recordCircuitFailure();
   return { success: false, error: new Error("Max retry attempts exceeded") };
 }
 
@@ -89,6 +148,12 @@ async function insertAuditLog({ payload, payloadHash, signature }) {
  */
 export async function logLoginAttempt({ merchantId, ipAddress, userAgent, status }) {
   const action = "login";
+
+  // Guard against unexpected action values reaching the DB (issue #772)
+  if (!validateAuditAction(action)) {
+    console.error(`[audit] Rejected disallowed action: ${action}`);
+    return;
+  }
   const rateLimitKey = createAuditLogRateLimitKey({
     merchantId,
     action,

backend/src/lib/audit.test.js

@@ -11,13 +11,14 @@
 import { beforeEach, describe, expect, it, vi } from "vitest";
 import fs from "node:fs";
 
-const { mockQuery, mockIsRetryablePoolError, mockConsumeRateLimit, mockHashPayload, mockSignPayload, mockSanitizeAuditValue } = vi.hoisted(() => ({
+const { mockQuery, mockIsRetryablePoolError, mockConsumeRateLimit, mockHashPayload, mockSignPayload, mockSanitizeAuditValue, mockValidateAuditAction } = vi.hoisted(() => ({
   mockQuery: vi.fn(),
   mockIsRetryablePoolError: vi.fn(),
   mockConsumeRateLimit: vi.fn(),
   mockHashPayload: vi.fn(),
   mockSignPayload: vi.fn(),
   mockSanitizeAuditValue: vi.fn((v) => v),
+  mockValidateAuditAction: vi.fn(() => true),
 }));
 
 vi.mock("./db.js", () => ({
@@ -31,9 +32,10 @@ vi.mock("./audit-security.js", () => ({
   hashAuditPayload: mockHashPayload,
   sanitizeAuditValue: mockSanitizeAuditValue,
   signAuditPayload: mockSignPayload,
+  validateAuditAction: mockValidateAuditAction,
 }));
 
-import { logLoginAttempt } from "./audit.js";
+import { logLoginAttempt, _resetAuditCircuitForTests } from "./audit.js";
 
 describe("logLoginAttempt", () => {
   beforeEach(() => {
@@ -43,6 +45,9 @@ describe("logLoginAttempt", () => {
     mockHashPayload.mockReset();
     mockSignPayload.mockReset();
     mockSanitizeAuditValue.mockReset();
+    mockValidateAuditAction.mockReset();
+    mockValidateAuditAction.mockReturnValue(true);
+    _resetAuditCircuitForTests();
   });
 
   it("inserts a login success row with correct parameters", async () => {
@@ -205,4 +210,41 @@ describe("logLoginAttempt", () => {
     expect(appendFileSyncSpy).toHaveBeenCalled();
     appendFileSyncSpy.mockRestore();
   });
+
+  // ── Circuit breaker (issue #771) ──────────────────────────────────────────
+
+  it("routes to fallback after circuit breaker opens from repeated DB failures", async () => {
+    const permError = new Error("DB down");
+    mockQuery.mockRejectedValue(permError);
+    mockIsRetryablePoolError.mockReturnValue(false);
+    mockConsumeRateLimit.mockReturnValue({ allowed: true });
+    mockHashPayload.mockReturnValue("a".repeat(64));
+    mockSignPayload.mockReturnValue("b".repeat(64));
+
+    const appendFileSyncSpy = vi.spyOn(fs, "appendFileSync").mockImplementation(() => {});
+
+    // Exceed the default threshold (5) to open the circuit
+    for (let i = 0; i < 6; i += 1) {
+      await logLoginAttempt({ merchantId: `m-${i}`, ipAddress: "1.2.3.4", userAgent: "ua", status: "failure" });
+    }
+
+    // After circuit opens, subsequent calls bypass the DB entirely
+    const callCountBeforeCircuitOpen = mockQuery.mock.calls.length;
+    await logLoginAttempt({ merchantId: "m-circuit", ipAddress: "1.2.3.4", userAgent: "ua", status: "failure" });
+    expect(mockQuery.mock.calls.length).toBe(callCountBeforeCircuitOpen);
+    expect(appendFileSyncSpy).toHaveBeenCalled();
+
+    appendFileSyncSpy.mockRestore();
+  });
+
+  it("resets circuit after success", async () => {
+    mockQuery.mockResolvedValue({ rows: [] });
+    mockIsRetryablePoolError.mockReturnValue(false);
+    mockConsumeRateLimit.mockReturnValue({ allowed: true });
+    mockHashPayload.mockReturnValue("a".repeat(64));
+    mockSignPayload.mockReturnValue("b".repeat(64));
+
+    await logLoginAttempt({ merchantId: "m-ok", ipAddress: "1.2.3.4", userAgent: "ua", status: "success" });
+    expect(mockQuery).toHaveBeenCalledOnce();
+  });
 });