From acc5b01a9042dc32b6bb29e7a486edca854a39bd Mon Sep 17 00:00:00 2001 From: Gbolahan Akande Date: Fri, 29 May 2026 00:42:11 +0100 Subject: [PATCH] =?UTF-8?q?feat(backend):=20audit=20logger=20=E2=80=94=20s?= =?UTF-8?q?ignature=20verification,=20SQL=20optimization,=20error=20recove?= =?UTF-8?q?ry,=20security=20audit?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add verifyAuditSignature() using crypto.timingSafeEqual for constant-time HMAC comparison to prevent timing-oracle attacks (issue #769) - Add validateAuditAction() with an allowlist of permitted action identifiers to block log-injection through crafted action strings (issue #772) - Replace two-query COUNT + SELECT pattern in getAuditLogs with a single COUNT(*) OVER() window-function query, halving round-trips to the DB; leverages the existing (merchant_id, timestamp) composite index (issue #770) - Add circuit-breaker state to insertAuditLog in both audit.js and auditService.js: after a configurable failure threshold the circuit opens and all writes route directly to the fallback log for a reset window before the next probe attempt, preventing DB overload cascades (issue #771) - Expand test coverage: 35 tests across audit-security, audit, and auditService — all passing Closes #769 Closes #770 Closes #771 Closes #772 --- backend/src/lib/audit-security.js | 61 +++++++++++++++ backend/src/lib/audit-security.test.js | 74 ++++++++++++++++++ backend/src/lib/audit.js | 65 ++++++++++++++++ backend/src/lib/audit.test.js | 46 +++++++++++- backend/src/services/auditService.js | 91 ++++++++++++++++++++--- backend/src/services/auditService.test.js | 83 ++++++++++++++++++++- 6 files changed, 403 insertions(+), 17 deletions(-) diff --git a/backend/src/lib/audit-security.js b/backend/src/lib/audit-security.js index 922f3d8..bef391e 100644 --- a/backend/src/lib/audit-security.js +++ b/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(":"); } diff --git a/backend/src/lib/audit-security.test.js b/backend/src/lib/audit-security.test.js index f728bae..218e013 100644 --- a/backend/src/lib/audit-security.test.js +++ b/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", diff --git a/backend/src/lib/audit.js b/backend/src/lib/audit.js index c76dd48..19d530e 100644 --- a/backend/src/lib/audit.js +++ b/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, diff --git a/backend/src/lib/audit.test.js b/backend/src/lib/audit.test.js index 23d2b73..e495e18 100644 --- a/backend/src/lib/audit.test.js +++ b/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(); + }); }); \ No newline at end of file diff --git a/backend/src/services/auditService.js b/backend/src/services/auditService.js index 78328be..9452a68 100644 --- a/backend/src/services/auditService.js +++ b/backend/src/services/auditService.js @@ -6,6 +6,7 @@ import { sanitizeAuditKey, sanitizeAuditValue, signAuditPayload, + validateAuditAction, } from "../lib/audit-security.js"; import fs from "node:fs"; import path from "node:path"; @@ -16,6 +17,51 @@ 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 for the auditService DB path (issue #771). + * Mirrors the circuit in lib/audit.js so that both log paths independently + * protect against DB overload during outages. + */ +const SVC_CIRCUIT_FAILURE_THRESHOLD = Number.parseInt(process.env.AUDIT_CIRCUIT_FAILURE_THRESHOLD || "5", 10); +const SVC_CIRCUIT_RESET_MS = Number.parseInt(process.env.AUDIT_CIRCUIT_RESET_MS || "60000", 10); + +const _svcCircuit = { + open: false, + failures: 0, + openedAt: 0, +}; + +export function _resetSvcCircuitForTests() { + _svcCircuit.open = false; + _svcCircuit.failures = 0; + _svcCircuit.openedAt = 0; +} + +function isSvcCircuitOpen(now = Date.now()) { + if (!_svcCircuit.open) return false; + if (now - _svcCircuit.openedAt >= SVC_CIRCUIT_RESET_MS) { + _svcCircuit.open = false; + return false; + } + return true; +} + +function recordSvcSuccess() { + _svcCircuit.failures = 0; + _svcCircuit.open = false; +} + +function recordSvcFailure(now = Date.now()) { + _svcCircuit.failures += 1; + if (_svcCircuit.failures >= SVC_CIRCUIT_FAILURE_THRESHOLD) { + _svcCircuit.open = true; + _svcCircuit.openedAt = now; + console.warn( + `[auditService] Circuit breaker opened after ${_svcCircuit.failures} consecutive failures. DB writes suspended for ${SVC_CIRCUIT_RESET_MS}ms.`, + ); + } +} + function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } @@ -35,6 +81,10 @@ function writeFallbackLog(payload, error) { } async function insertAuditLog({ payload, payloadHash, signature }) { + if (isSvcCircuitOpen()) { + 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( @@ -52,10 +102,12 @@ async function insertAuditLog({ payload, payloadHash, signature }) { signature, ], ); + recordSvcSuccess(); return { success: true }; } catch (err) { const isRetryable = attempt < AUDIT_DB_RETRY_ATTEMPTS && isRetryablePoolError(err); if (!isRetryable) { + recordSvcFailure(); return { success: false, error: err }; } const delayMs = AUDIT_DB_RETRY_DELAY_MS * (attempt + 1); @@ -65,10 +117,20 @@ async function insertAuditLog({ payload, payloadHash, signature }) { await sleep(delayMs); } } + recordSvcFailure(); return { success: false, error: new Error("Max retry attempts exceeded") }; } export const auditService = { + /** + * Retrieve paginated audit logs for a merchant. + * + * Uses a single SQL query with a COUNT(*) OVER() window function so that + * the total row count and the page data are fetched in one round-trip to + * the database instead of two (issue #770). The composite index on + * (merchant_id, timestamp) created in migration 20260425000000 is used by + * the ORDER BY clause to avoid a sequential scan on large tables. + */ async getAuditLogs(merchantId, page = 1, limit = 50) { let p = parseInt(page, 10) || 1; let l = parseInt(limit, 10) || 50; @@ -79,25 +141,24 @@ export const auditService = { const offset = (p - 1) * l; - // Get total count - const countResult = await pool.query( - "SELECT COUNT(*) FROM audit_logs WHERE merchant_id = $1", - [merchantId] - ); - const totalCount = parseInt(countResult.rows[0].count, 10); - - // Get paginated logs - const logsResult = await pool.query( - `SELECT id, action, field_changed, old_value, new_value, ip_address, user_agent, timestamp + // Single query: window function returns the full-table count alongside + // each row, eliminating the separate COUNT(*) round-trip (issue #770). + const result = await pool.query( + `SELECT id, action, field_changed, old_value, new_value, ip_address, user_agent, timestamp, + COUNT(*) OVER() AS total_count FROM audit_logs WHERE merchant_id = $1 ORDER BY timestamp DESC LIMIT $2 OFFSET $3`, - [merchantId, l, offset] + [merchantId, l, offset], ); + const totalCount = result.rows.length > 0 ? parseInt(result.rows[0].total_count, 10) : 0; + // Strip the synthetic total_count column from each returned row + const logs = result.rows.map(({ total_count: _tc, ...row }) => row); + return { - logs: logsResult.rows, + logs, total_count: totalCount, total_pages: Math.ceil(totalCount / l), page: p, @@ -114,6 +175,12 @@ export const auditService = { ipAddress, userAgent, }) { + // Reject unknown action values to prevent log-injection (issue #772) + if (!validateAuditAction(action)) { + console.error(`[auditService] Rejected disallowed audit action: ${action}`); + return; + } + const rateLimitKey = createAuditLogRateLimitKey({ merchantId, action, diff --git a/backend/src/services/auditService.test.js b/backend/src/services/auditService.test.js index 8d1b1ab..4834514 100644 --- a/backend/src/services/auditService.test.js +++ b/backend/src/services/auditService.test.js @@ -1,12 +1,13 @@ import { beforeEach, describe, expect, it, vi } from "vitest"; import fs from "node:fs"; -const { mockQuery, mockIsRetryablePoolError, mockConsumeRateLimit, mockHashPayload, mockSignPayload } = vi.hoisted(() => ({ +const { mockQuery, mockIsRetryablePoolError, mockConsumeRateLimit, mockHashPayload, mockSignPayload, mockValidateAuditAction } = vi.hoisted(() => ({ mockQuery: vi.fn(), mockIsRetryablePoolError: vi.fn(), mockConsumeRateLimit: vi.fn(), mockHashPayload: vi.fn(), mockSignPayload: vi.fn(), + mockValidateAuditAction: vi.fn(() => true), })); vi.mock("../lib/db.js", () => ({ @@ -21,17 +22,21 @@ vi.mock("../lib/audit-security.js", () => ({ sanitizeAuditKey: vi.fn((v) => v), sanitizeAuditValue: vi.fn((v) => v), signAuditPayload: mockSignPayload, + validateAuditAction: mockValidateAuditAction, })); -import { auditService } from "./auditService.js"; +import { auditService, _resetSvcCircuitForTests } from "./auditService.js"; -describe("auditService.logEvent", () => { +describe("auditService", () => { beforeEach(() => { mockQuery.mockReset(); mockIsRetryablePoolError.mockReset(); mockConsumeRateLimit.mockReset(); mockHashPayload.mockReset(); mockSignPayload.mockReset(); + mockValidateAuditAction.mockReset(); + mockValidateAuditAction.mockReturnValue(true); + _resetSvcCircuitForTests(); }); it("writes signed audit records", async () => { @@ -119,4 +124,76 @@ describe("auditService.logEvent", () => { expect(appendFileSyncSpy).toHaveBeenCalled(); appendFileSyncSpy.mockRestore(); }); + + // ── SQL optimization: getAuditLogs (issue #770) ─────────────────────────── + + it("fetches logs and count in a single window-function query", async () => { + mockQuery.mockResolvedValueOnce({ + rows: [ + { id: 1, action: "update", field_changed: "email", old_value: "a@b.com", new_value: "c@d.com", ip_address: "1.2.3.4", user_agent: "ua", timestamp: new Date(), total_count: "3" }, + { id: 2, action: "login", field_changed: null, old_value: null, new_value: null, ip_address: "1.2.3.4", user_agent: "ua", timestamp: new Date(), total_count: "3" }, + ], + }); + + const result = await auditService.getAuditLogs("merchant-1", 1, 2); + + expect(mockQuery).toHaveBeenCalledOnce(); + const [sql] = mockQuery.mock.calls[0]; + expect(sql).toMatch(/COUNT\(\*\) OVER\(\)/i); + expect(result.total_count).toBe(3); + expect(result.logs).toHaveLength(2); + // Ensure the synthetic total_count column is stripped from returned rows + expect(result.logs[0]).not.toHaveProperty("total_count"); + }); + + it("returns zero total_count when no rows match", async () => { + mockQuery.mockResolvedValueOnce({ rows: [] }); + const result = await auditService.getAuditLogs("merchant-nobody", 1, 10); + expect(result.total_count).toBe(0); + expect(result.logs).toHaveLength(0); + }); + + it("clamps page and limit to valid ranges", async () => { + mockQuery.mockResolvedValueOnce({ rows: [] }); + const result = await auditService.getAuditLogs("merchant-1", -5, 200); + const [, params] = mockQuery.mock.calls[0]; + expect(params[1]).toBe(100); // limit clamped to 100 + expect(params[2]).toBe(0); // offset for page 1 = 0 + expect(result.page).toBe(1); + }); + + // ── Action validation (issue #772) ──────────────────────────────────────── + + it("drops logEvent calls with disallowed action values", async () => { + mockValidateAuditAction.mockReturnValue(false); + mockConsumeRateLimit.mockReturnValue({ allowed: true }); + + await auditService.logEvent({ merchantId: "m", action: "DROP TABLE", fieldChanged: "x" }); + + expect(mockQuery).not.toHaveBeenCalled(); + }); + + // ── Circuit breaker: logEvent (issue #771) ───────────────────────────────── + + it("opens circuit breaker after repeated DB failures and routes to fallback", async () => { + const permError = new Error("connection refused"); + 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(() => {}); + + for (let i = 0; i < 6; i += 1) { + await auditService.logEvent({ merchantId: `m-${i}`, action: "update", fieldChanged: "email" }); + } + + const callsBeforeOpen = mockQuery.mock.calls.length; + await auditService.logEvent({ merchantId: "m-open", action: "update", fieldChanged: "email" }); + expect(mockQuery.mock.calls.length).toBe(callsBeforeOpen); + expect(appendFileSyncSpy).toHaveBeenCalled(); + + appendFileSyncSpy.mockRestore(); + }); }); \ No newline at end of file