Skip to content

Commit 0714487

Browse files
authored
Merge pull request #1414 from Elsa-tech2026/leak
Audit Logger memory leak fix is in place
2 parents 98c6e17 + 12db0a7 commit 0714487

4 files changed

Lines changed: 226 additions & 188 deletions

File tree

backend/src/lib/audit-security.js

Lines changed: 33 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ const SENSITIVE_KEY_RE = /(secret|token|password|api[_-]?key|authorization|signa
44
const DEFAULT_AUDIT_RATE_LIMIT_MAX = 60;
55
const DEFAULT_AUDIT_RATE_LIMIT_WINDOW_MS = 60_000;
66
const DEFAULT_AUDIT_FIELD_MAX_LENGTH = 2048;
7+
const MAX_AUDIT_RATE_LIMIT_KEYS = 10_000;
78

89
/**
910
* Allowlist of permitted audit action identifiers.
@@ -29,6 +30,19 @@ const ALLOWED_AUDIT_ACTIONS = new Set([
2930

3031
const auditRateLimitState = new Map();
3132

33+
function pruneExpiredAuditRateLimitEntries(now, windowMs) {
34+
let cleaned = 0;
35+
36+
for (const [key, state] of auditRateLimitState.entries()) {
37+
if (now >= state.windowStart + windowMs) {
38+
auditRateLimitState.delete(key);
39+
cleaned += 1;
40+
}
41+
}
42+
43+
return cleaned;
44+
}
45+
3246
function stableStringify(value, depth = 0, seen = new WeakSet()) {
3347
if (depth > 10) {
3448
return '"[Too Deep]"';
@@ -164,19 +178,17 @@ export function consumeAuditLogRateLimit(
164178
) {
165179
if (!key) return { allowed: true, remaining: max, resetTime: now + windowMs };
166180

181+
pruneExpiredAuditRateLimitEntries(now, windowMs);
182+
167183
// Evict expired entries if Map size exceeds safety threshold (DoS / OOM protection)
168-
if (auditRateLimitState.size >= 10000) {
169-
for (const [k, v] of auditRateLimitState.entries()) {
170-
if (now >= v.windowStart + windowMs) {
171-
auditRateLimitState.delete(k);
172-
}
173-
}
174-
// Hard cap eviction if still over threshold
175-
if (auditRateLimitState.size >= 10000) {
176-
const oldestKeys = Array.from(auditRateLimitState.keys()).slice(0, 100);
177-
for (const k of oldestKeys) {
178-
auditRateLimitState.delete(k);
179-
}
184+
if (auditRateLimitState.size >= MAX_AUDIT_RATE_LIMIT_KEYS) {
185+
const oldestKeys = Array.from(auditRateLimitState.entries())
186+
.sort(([, a], [, b]) => a.windowStart - b.windowStart)
187+
.slice(0, Math.max(100, Math.ceil(auditRateLimitState.size * 0.1)))
188+
.map(([k]) => k);
189+
190+
for (const k of oldestKeys) {
191+
auditRateLimitState.delete(k);
180192
}
181193
}
182194

@@ -208,17 +220,21 @@ export function consumeAuditLogRateLimit(
208220
* Get comprehensive rate limit statistics for audit logging (issue #902).
209221
* Useful for monitoring and debugging rate limit behavior.
210222
*/
211-
export function getAuditRateLimitStats() {
212-
const now = Date.now();
223+
export function getAuditRateLimitStats({ now = Date.now() } = {}) {
224+
const windowMs = Number(
225+
process.env.AUDIT_LOG_RATE_LIMIT_WINDOW_MS || DEFAULT_AUDIT_RATE_LIMIT_WINDOW_MS,
226+
);
227+
pruneExpiredAuditRateLimitEntries(now, windowMs);
228+
213229
const stats = {
214230
totalKeys: auditRateLimitState.size,
215231
activeWindows: 0,
216232
expiredWindows: 0,
217233
maxRequestsPerWindow: Number(process.env.AUDIT_LOG_RATE_LIMIT_MAX || DEFAULT_AUDIT_RATE_LIMIT_MAX),
218-
windowMs: Number(process.env.AUDIT_LOG_RATE_LIMIT_WINDOW_MS || DEFAULT_AUDIT_RATE_LIMIT_WINDOW_MS),
234+
windowMs,
219235
};
220236

221-
for (const [key, state] of auditRateLimitState.entries()) {
237+
for (const [, state] of auditRateLimitState.entries()) {
222238
if (now >= state.windowStart + stats.windowMs) {
223239
stats.expiredWindows++;
224240
} else {
@@ -233,8 +249,7 @@ export function getAuditRateLimitStats() {
233249
* Cleanup expired audit rate limit entries to prevent memory exhaustion (issue #902).
234250
* Should be called periodically (e.g., via cron or on a schedule).
235251
*/
236-
export function cleanupExpiredAuditRateLimits() {
237-
const now = Date.now();
252+
export function cleanupExpiredAuditRateLimits({ now = Date.now() } = {}) {
238253
const windowMs = Number(
239254
process.env.AUDIT_LOG_RATE_LIMIT_WINDOW_MS || DEFAULT_AUDIT_RATE_LIMIT_WINDOW_MS,
240255
);

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

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { beforeEach, describe, expect, it } from "vitest";
22
import {
33
consumeAuditLogRateLimit,
44
createAuditLogRateLimitKey,
5+
getAuditRateLimitStats,
56
hashAuditPayload,
67
resetAuditRateLimitStateForTests,
78
sanitizeAuditKey,
@@ -194,6 +195,25 @@ describe("audit-security", () => {
194195
expect(res.allowed).toBe(true);
195196
});
196197

198+
it("proactively removes expired rate-limit entries before they accumulate", () => {
199+
consumeAuditLogRateLimit("stale-key", {
200+
now: 0,
201+
max: 2,
202+
windowMs: 100,
203+
});
204+
205+
consumeAuditLogRateLimit("fresh-key", {
206+
now: 150,
207+
max: 2,
208+
windowMs: 100,
209+
});
210+
211+
const stats = getAuditRateLimitStats({ now: 150 });
212+
expect(stats.totalKeys).toBe(1);
213+
expect(stats.activeWindows).toBe(1);
214+
expect(stats.expiredWindows).toBe(0);
215+
});
216+
197217
it("reconstructs payloads and verifies row integrity correctly", () => {
198218
const secret = "test-secret-key";
199219
const row = {

backend/src/services/auditService.js

Lines changed: 16 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -38,24 +38,29 @@ export const auditService = {
3838

3939
const offset = (p - 1) * l;
4040

41-
// Single query: window function returns the full-table count alongside
42-
// each row, eliminating the separate COUNT(*) round-trip (issue #770).
43-
const result = await pool.query(
44-
`SELECT id, merchant_id, action, field_changed, old_value, new_value, ip_address, user_agent, timestamp, payload_hash, signature,
45-
COUNT(*) OVER() AS total_count
41+
const countResult = await pool.query(
42+
"SELECT COUNT(*)::int AS total_count FROM audit_logs WHERE merchant_id = $1",
43+
[merchantId],
44+
);
45+
46+
const totalCount = parseInt(countResult.rows[0]?.total_count ?? 0, 10);
47+
48+
const rowsResult = await pool.query(
49+
`SELECT id, merchant_id, action, field_changed, old_value, new_value, ip_address, user_agent, timestamp, payload_hash, signature
4650
FROM audit_logs
4751
WHERE merchant_id = $1
4852
ORDER BY timestamp DESC
4953
LIMIT $2 OFFSET $3`,
5054
[merchantId, l, offset],
5155
);
5256

53-
const totalCount = result.rows.length > 0 ? parseInt(result.rows[0].total_count, 10) : 0;
54-
55-
// Verify cryptographic integrity of each row before returning
56-
const logs = result.rows.map(({ total_count: _tc, ...row }) => {
57+
const logs = rowsResult.rows.map((row) => {
5758
const integrity = verifyRowIntegrity(row);
5859
auditLogIntegrityVerificationsTotal.inc({ result: integrity.status });
60+
61+
const hashVerified = row.payload_hash == null ? null : integrity.verified && integrity.status === "verified";
62+
const signatureVerified = row.signature == null || !process.env.AUDIT_LOG_SIGNING_SECRET ? null : integrity.verified && integrity.status === "verified";
63+
5964
return {
6065
id: row.id,
6166
action: row.action,
@@ -65,6 +70,8 @@ export const auditService = {
6570
ip_address: row.ip_address,
6671
user_agent: row.user_agent,
6772
timestamp: row.timestamp,
73+
hash_verified: hashVerified,
74+
signature_verified: signatureVerified,
6875
integrity_status: integrity.status,
6976
};
7077
});

0 commit comments

Comments
 (0)