Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion backend/src/lib/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -100,7 +100,7 @@ async function insertAuditLog({ payload, payloadHash, signature }) {

for (let attempt = 0; attempt <= AUDIT_DB_RETRY_ATTEMPTS; attempt += 1) {
try {
await pool.query(
await optimizedWrite(
`INSERT INTO audit_logs (merchant_id, action, status, ip_address, user_agent, payload_hash, signature)
VALUES ($1, $2, $3, $4, $5, $6, $7)`,
[
Expand All @@ -112,6 +112,10 @@ async function insertAuditLog({ payload, payloadHash, signature }) {
payloadHash,
signature,
],
{
label: "insert_login_audit_log",
merchantId: payload.merchant_id,
}
);
recordCircuitSuccess();
return { success: true };
Expand Down
1 change: 1 addition & 0 deletions backend/src/lib/audit.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@
vi.mock("./db.js", () => ({
pool: { query: mockQuery },
isRetryablePoolError: mockIsRetryablePoolError,
queryWithRetry: mockQuery,
}));

vi.mock("./audit-replay.js", () => ({
Expand Down Expand Up @@ -69,7 +70,7 @@
status: "success",
});

expect(mockQuery).toHaveBeenCalledOnce();

Check failure on line 73 in backend/src/lib/audit.test.js

View workflow job for this annotation

GitHub Actions / Backend — Lint & Test

src/lib/audit.test.js > logLoginAttempt > inserts a login success row with correct parameters

AssertionError: expected "spy" to be called once, but got 0 times ❯ src/lib/audit.test.js:73:23
const [sql, params] = mockQuery.mock.calls[0];
expect(sql).toMatch(/INSERT INTO audit_logs/i);
expect(params[0]).toBe("merchant-uuid-001");
Expand All @@ -90,7 +91,7 @@
status: "failure",
});

expect(mockQuery).toHaveBeenCalledOnce();

Check failure on line 94 in backend/src/lib/audit.test.js

View workflow job for this annotation

GitHub Actions / Backend — Lint & Test

src/lib/audit.test.js > logLoginAttempt > inserts a login failure row with correct parameters

AssertionError: expected "spy" to be called once, but got 0 times ❯ src/lib/audit.test.js:94:23
const [, params] = mockQuery.mock.calls[0];
expect(params[1]).toBe("login");
});
Expand All @@ -109,7 +110,7 @@
status: "failure",
});

expect(mockQuery).toHaveBeenCalledOnce();

Check failure on line 113 in backend/src/lib/audit.test.js

View workflow job for this annotation

GitHub Actions / Backend — Lint & Test

src/lib/audit.test.js > logLoginAttempt > inserts a row with null merchantId

AssertionError: expected "spy" to be called once, but got 0 times ❯ src/lib/audit.test.js:113:23
const [, params] = mockQuery.mock.calls[0];
expect(params[0]).toBeNull();
});
Expand All @@ -129,7 +130,7 @@
status: "success",
});

const [, params] = mockQuery.mock.calls[0];

Check failure on line 133 in backend/src/lib/audit.test.js

View workflow job for this annotation

GitHub Actions / Backend — Lint & Test

src/lib/audit.test.js > logLoginAttempt > stores null ip_address and user_agent when not provided

TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator)) ❯ src/lib/audit.test.js:133:24
expect(params[3]).toBeNull();
expect(params[4]).toBeNull();
});
Expand All @@ -151,7 +152,7 @@
status: "success",
});

const [, params] = mockQuery.mock.calls[0];

Check failure on line 155 in backend/src/lib/audit.test.js

View workflow job for this annotation

GitHub Actions / Backend — Lint & Test

src/lib/audit.test.js > logLoginAttempt > applies a cryptographic signature when audit signing secret is configured

TypeError: undefined is not iterable (cannot read property Symbol(Symbol.iterator)) ❯ src/lib/audit.test.js:155:24
expect(params[6]).toMatch(/^[a-f0-9]{64}$/);

process.env.AUDIT_LOG_SIGNING_SECRET = original;
Expand Down Expand Up @@ -192,7 +193,7 @@
status: "success",
});

expect(mockQuery).toHaveBeenCalledTimes(3);

Check failure on line 196 in backend/src/lib/audit.test.js

View workflow job for this annotation

GitHub Actions / Backend — Lint & Test

src/lib/audit.test.js > logLoginAttempt > retries on transient errors

AssertionError: expected "spy" to be called 3 times, but got 0 times ❯ src/lib/audit.test.js:196:23
});

it("falls back to file logging when DB fails permanently", async () => {
Expand Down Expand Up @@ -268,7 +269,7 @@
// 4. In HALF_OPEN, first success does not yet close the circuit
mockQuery.mockResolvedValue({ rows: [] });
await logLoginAttempt({ merchantId: "m-probe-1", ipAddress: "1.2.3.4", userAgent: "ua", status: "success" });
expect(mockQuery).toHaveBeenCalledOnce();

Check failure on line 272 in backend/src/lib/audit.test.js

View workflow job for this annotation

GitHub Actions / Backend — Lint & Test

src/lib/audit.test.js > logLoginAttempt > transitions to HALF_OPEN after timeout, recovers to CLOSED on successes, and triggers replay

AssertionError: expected "spy" to be called once, but got 0 times ❯ src/lib/audit.test.js:272:23
expect(mockReplayFallbackLogs).not.toHaveBeenCalled();

mockQuery.mockClear();
Expand Down
14 changes: 8 additions & 6 deletions backend/src/services/auditService.js
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ async function insertAuditLog({ payload, payloadHash, signature }) {

for (let attempt = 0; attempt <= AUDIT_DB_RETRY_ATTEMPTS; attempt += 1) {
try {
await pool.query(
await optimizedWrite(
`INSERT INTO audit_logs (merchant_id, action, field_changed, old_value, new_value, ip_address, user_agent, payload_hash, signature)
VALUES ($1, $2, $3, $4, $5, $6, $7, $8, $9)`,
[
Expand All @@ -94,6 +94,10 @@ async function insertAuditLog({ payload, payloadHash, signature }) {
payloadHash,
signature,
],
{
label: "insert_audit_log",
merchantId: payload.merchant_id,
}
);
recordSvcSuccess();
return { success: true };
Expand All @@ -118,11 +122,9 @@ 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.
* Splits paginated queries into parallel count and row retrieval queries (issue #770)
* executed via optimizedQuery to allow cacheability and avoid full table scan
* materialization in Postgres.
*/
async getAuditLogs(merchantId, page = 1, limit = 50) {
let p = parseInt(page, 10) || 1;
Expand Down
24 changes: 15 additions & 9 deletions backend/src/services/auditService.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ const { mockQuery, mockIsRetryablePoolError, mockConsumeRateLimit, mockHashPaylo
vi.mock("../lib/db.js", () => ({
pool: { query: mockQuery },
isRetryablePoolError: mockIsRetryablePoolError,
queryWithRetry: mockQuery,
}));

vi.mock("../lib/audit-replay.js", () => ({
Expand Down Expand Up @@ -132,36 +133,41 @@ describe("auditService", () => {

// ── SQL optimization: getAuditLogs (issue #770) ───────────────────────────

it("fetches logs and count in a single window-function query", async () => {
it("fetches logs and count using optimized parallel queries", async () => {
mockQuery.mockResolvedValueOnce({
rows: [{ total_count: "3" }],
});
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" },
{ 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() },
{ 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() },
],
});

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(mockQuery).toHaveBeenCalledTimes(2);
const [countSql] = mockQuery.mock.calls[0];
const [logsSql] = mockQuery.mock.calls[1];
expect(countSql).toMatch(/COUNT\(\*\)/i);
expect(logsSql).not.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: [{ total_count: 0 }] });
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: [{ total_count: 0 }] });
mockQuery.mockResolvedValueOnce({ rows: [] });
const result = await auditService.getAuditLogs("merchant-1", -5, 200);
const [, params] = mockQuery.mock.calls[0];
const [, params] = mockQuery.mock.calls[1];
expect(params[1]).toBe(100); // limit clamped to 100
expect(params[2]).toBe(0); // offset for page 1 = 0
expect(result.page).toBe(1);
Expand Down
Loading