From 5e66b541535f4818791123c71a57d2b7e6018398 Mon Sep 17 00:00:00 2001 From: Anso Date: Sun, 31 May 2026 16:36:07 -0400 Subject: [PATCH] fix(audit-log): neutralize CSV export injection, clamp pagination, bound anomaly history (#1259) * fix(audit-log): neutralize CSV export injection, clamp pagination, bound anomaly history Harden the Admiral audit log without changing its tier or hub-only gating. - CSV export now defuses formula injection: any field that a spreadsheet would evaluate as a formula (leading = + - @, or a trigger behind leading whitespace, or a leading tab/CR) is prefixed with a single quote before RFC 4180 quoting. Audit summaries embed user-controlled resource names, so this closes a path where a crafted name could execute on export open. - Clamp page and limit to positive bounds on the list endpoint so a negative limit can no longer reach SQLite as "unlimited" and dump the whole table. - Bound the anomaly and stats history reads to a capped slice of recent rows so the analysis paths stay within fixed memory and latency on large histories instead of scanning the full retention window per request. - Surface failed audit list and stats fetches through the standard error toast instead of leaving the view silently stale. - Add a developer-mode-gated diagnostic log to the stats endpoint for parity with the list and export handlers. Covered by new unit and HTTP-integration tests (CSV neutralization through the real export route, pagination clamps, bounded history, stats endpoint, anomaly annotation) and verified end to end in the browser. * fix(audit-log): compute signal-rail stats with exact SQL aggregates Address review feedback on the earlier history-cap change. The cap was correct for the anomaly baseline but made the stats tiles (events, actors, failure rate, hourly series) silently undercount on a hub with more than the cap's worth of rows in the window, since they were derived from the capped row slice. - Add DatabaseService.getAuditStatsInputs: exact counts via SQL COUNT / COUNT(DISTINCT) / GROUP BY hour, and new-ip detection over the small DISTINCT (user, ip) pair sets. No row cap, so the tiles stay exact at any window size while memory stays bounded. - Reduce computeAuditStats to a pure formatter over those aggregates. - Keep the bounded history read only for the list endpoint's anomaly annotation, where a recent-activity baseline is an acceptable heuristic. - Skip the redundant load-failure toast when a fetch fails with a handled 401, so an expired session does not stack toasts on top of logout. Adds exactness tests for the aggregate counts, distinct-actor handling, and new-ip detection, and strengthens the pagination-clamp tests. * fix(audit-log): exclude future-dated rows and make the new-ip sample deterministic Two small parity fixes on the stats aggregates: upper-bound every current window by `now` so a future-dated row (clock skew or a fixture) cannot inflate the live counts, and order the new-ip pair scan so the sample actor shown in the tile detail is stable. Adds a test asserting a future row is excluded. --- .../__tests__/audit-anomaly-service.test.ts | 88 +++--- backend/src/__tests__/audit-log.test.ts | 284 ++++++++++++++++++ backend/src/__tests__/csv.test.ts | 66 ++++ backend/src/routes/auditLog.ts | 19 +- backend/src/services/AuditAnomalyService.ts | 63 ++-- backend/src/services/DatabaseService.ts | 83 ++++- backend/src/utils/csv.ts | 27 +- docs/features/audit-log.mdx | 2 +- frontend/src/components/AuditLogView.tsx | 19 ++ 9 files changed, 553 insertions(+), 98 deletions(-) create mode 100644 backend/src/__tests__/csv.test.ts diff --git a/backend/src/__tests__/audit-anomaly-service.test.ts b/backend/src/__tests__/audit-anomaly-service.test.ts index e1db5a33..6f91e4fd 100644 --- a/backend/src/__tests__/audit-anomaly-service.test.ts +++ b/backend/src/__tests__/audit-anomaly-service.test.ts @@ -113,57 +113,65 @@ describe('AuditAnomalyService - annotateEntries', () => { }); describe('AuditAnomalyService - computeAuditStats', () => { - function buildEntries(now: number): { last24h: AuditLogEntry[]; last7d: AuditLogEntry[]; last30d: AuditLogEntry[] } { - const last24h: AuditLogEntry[] = []; - for (let i = 0; i < 20; i++) { - last24h.push(entry({ - id: i + 1, - timestamp: now - i * HOUR, - status_code: i < 3 ? 500 : 200, - ip_address: i === 5 ? '45.76.1.2' : '10.0.0.1', - })); - } - const older: AuditLogEntry[] = []; - for (let i = 0; i < 60; i++) { - older.push(entry({ - id: 100 + i, - timestamp: now - DAY - i * HOUR, - ip_address: '10.0.0.1', - })); - } - return { - last24h, - last7d: [...last24h, ...older.filter(e => e.timestamp >= now - 7 * DAY)], - last30d: [...last24h, ...older], - }; - } + const baseInput = (over: Partial[0]> = {}) => ({ + events24: 0, + events7d: 0, + actors24: 0, + failures24: 0, + activityByHour: Array.from({ length: 24 }, () => 0), + failuresByHour: Array.from({ length: 24 }, () => 0), + newIpCount: 0, + sampleNewIpActor: null as string | null, + ...over, + }); - it('summarizes events, actors, failures, and peak hour', () => { - const now = new Date('2026-04-18T12:00:00Z').getTime(); - const { last24h, last7d, last30d } = buildEntries(now); - const stats = computeAuditStats({ now, last24h, last7d, last30d }); + it('summarizes events, actors, and failure rate from exact aggregates', () => { + const activityByHour = Array.from({ length: 24 }, () => 0); + activityByHour[12] = 20; + const stats = computeAuditStats(baseInput({ + events24: 20, + events7d: 20, + actors24: 1, + failures24: 3, + activityByHour, + })); expect(stats.events_24h.value).toBe(20); expect(stats.actors_24h.value).toBe(1); expect(stats.failure_rate.value).toBe(15); + expect(stats.failure_rate.detail).toBe('3 of 20 requests'); expect(stats.activity_by_hour).toHaveLength(24); - expect(stats.activity_by_hour.reduce((a, b) => a + b, 0)).toBe(20); }); - it('flags the new_ip detail when an actor uses an ip not seen in prior 29 days', () => { - const now = new Date('2026-04-18T12:00:00Z').getTime(); - const { last24h, last7d, last30d } = buildEntries(now); - const stats = computeAuditStats({ now, last24h, last7d, last30d }); - expect(stats.actors_24h.detail).toMatch(/new ip/); + it('flags the new_ip detail when the aggregate reports a new pair', () => { + const stats = computeAuditStats(baseInput({ + events24: 5, + events7d: 5, + actors24: 1, + newIpCount: 2, + sampleNewIpActor: 'admin', + })); + expect(stats.actors_24h.detail).toMatch(/2 new ips/); + expect(stats.actors_24h.detail).toMatch(/admin/); + expect(stats.actors_24h.severity).toBe('warn'); }); it('surfaces peak hour when it falls outside working hours', () => { - const now = new Date(2026, 3, 18, 12, 0, 0).getTime(); - const nightBase = new Date(2026, 3, 18, 3, 15, 0).getTime(); - const nightEntries: AuditLogEntry[] = Array.from({ length: 10 }, (_, i) => - entry({ id: i + 1, timestamp: nightBase - i * 5000 }) - ); - const stats = computeAuditStats({ now, last24h: nightEntries, last7d: nightEntries, last30d: nightEntries }); + const activityByHour = Array.from({ length: 24 }, () => 0); + activityByHour[3] = 10; + const stats = computeAuditStats(baseInput({ + events24: 10, + events7d: 10, + actors24: 1, + activityByHour, + })); expect(stats.unusual_hour.severity).toBe('warn'); expect(stats.unusual_hour.value).toBe(3); }); + + it('keeps peak hour blank inside working hours', () => { + const activityByHour = Array.from({ length: 24 }, () => 0); + activityByHour[14] = 8; + const stats = computeAuditStats(baseInput({ events24: 8, events7d: 8, actors24: 1, activityByHour })); + expect(stats.unusual_hour.value).toBeNull(); + }); }); diff --git a/backend/src/__tests__/audit-log.test.ts b/backend/src/__tests__/audit-log.test.ts index 0d3de7e4..6f3cc29e 100644 --- a/backend/src/__tests__/audit-log.test.ts +++ b/backend/src/__tests__/audit-log.test.ts @@ -260,6 +260,142 @@ describe('DatabaseService audit methods', () => { const page2Ids = page2.entries.map(e => e.id); expect(page1Ids.some(id => page2Ids.includes(id))).toBe(false); }); + + it('getAuditLogsInRange caps to the most-recent rows and returns ascending order', () => { + const db = DatabaseService.getInstance(); + // Old, isolated window: keeps these rows out of the "most recent" DESC + // queries other tests rely on, while staying easy to range-query here. + const base = 1_000_000_000_000; // 2001, far from any now()-based entry + for (let i = 0; i < 10; i++) { + db.insertAuditLog({ + timestamp: base + i, + username: 'rangecapuser', + method: 'POST', + path: `/api/stacks/cap${i}`, + status_code: 200, + node_id: null, + ip_address: '127.0.0.1', + summary: `cap entry ${i}`, + }); + } + + const capped = db.getAuditLogsInRange(base, base + 100, 3); + expect(capped.length).toBe(3); + // Most-recent three (timestamps base+7, +8, +9), returned ascending. + expect(capped.map(e => e.timestamp)).toEqual([base + 7, base + 8, base + 9]); + + const uncapped = db.getAuditLogsInRange(base, base + 100); + expect(uncapped.length).toBe(10); + expect(uncapped[0].timestamp).toBe(base); + }); + + it('getAuditStatsInputs counts the current window exactly (no row cap)', () => { + const db = DatabaseService.getInstance(); + const now = Date.now(); + const ts = now - 60 * 60 * 1000; // 1 hour ago, inside the 24h window + const hour = new Date(ts).getHours(); + + const before = db.getAuditStatsInputs(now); + const K = 12; + const FAILURES = 4; + for (let i = 0; i < K; i++) { + db.insertAuditLog({ + timestamp: ts, + username: 'statsexactuser', + method: 'POST', + path: `/api/stacks/statsexact${i}`, + status_code: i < FAILURES ? 500 : 200, + node_id: null, + ip_address: '127.0.0.1', + summary: `stats exact ${i}`, + }); + } + const after = db.getAuditStatsInputs(now); + + expect(after.events24 - before.events24).toBe(K); + expect(after.events7d - before.events7d).toBe(K); + expect(after.failures24 - before.failures24).toBe(FAILURES); + expect(after.activityByHour[hour] - before.activityByHour[hour]).toBe(K); + expect(after.failuresByHour[hour] - before.failuresByHour[hour]).toBe(FAILURES); + }); + + it('getAuditStatsInputs excludes future-dated rows from the current window', () => { + const db = DatabaseService.getInstance(); + const now = Date.now(); + const before = db.getAuditStatsInputs(now); + // A row dated after `now` (clock skew / fixture) must not inflate the live counts. + db.insertAuditLog({ + timestamp: now + 60 * 60 * 1000, + username: 'futureuser', + method: 'POST', + path: '/api/stacks/future', + status_code: 500, + node_id: null, + ip_address: '127.0.0.1', + summary: 'future entry', + }); + const after = db.getAuditStatsInputs(now); + + expect(after.events24 - before.events24).toBe(0); + expect(after.events7d - before.events7d).toBe(0); + expect(after.failures24 - before.failures24).toBe(0); + }); + + it('getAuditStatsInputs flags an actor whose recent ip is new versus prior history', () => { + const db = DatabaseService.getInstance(); + const now = Date.now(); + // Prior IP for this actor, older than 24h but inside 30d. + db.insertAuditLog({ + timestamp: now - 5 * 24 * 60 * 60 * 1000, + username: 'newipscenariouser', + method: 'POST', + path: '/api/stacks/old', + status_code: 200, + node_id: null, + ip_address: '10.0.0.1', + summary: 'old', + }); + const before = db.getAuditStatsInputs(now); + // Recent action from a different IP. + db.insertAuditLog({ + timestamp: now - 60 * 1000, + username: 'newipscenariouser', + method: 'POST', + path: '/api/stacks/new', + status_code: 200, + node_id: null, + ip_address: '203.0.113.9', + summary: 'new', + }); + const after = db.getAuditStatsInputs(now); + + expect(after.newIpCount).toBeGreaterThan(before.newIpCount); + }); + + it('getAuditStatsInputs counts distinct non-empty actors, excluding blank usernames', () => { + const db = DatabaseService.getInstance(); + const now = Date.now(); + const ts = now - 30 * 60 * 1000; // inside 24h + const before = db.getAuditStatsInputs(now); + + const usernames = ['actorcountA', 'actorcountB', 'actorcountB', 'actorcountC', '']; + for (const username of usernames) { + db.insertAuditLog({ + timestamp: ts, + username, + method: 'POST', + path: '/api/stacks/actorcount', + status_code: 200, + node_id: null, + ip_address: '127.0.0.1', + summary: 'actor count', + }); + } + const after = db.getAuditStatsInputs(now); + + // Three distinct non-empty actors (A, B, C); the blank username is excluded. + expect(after.actors24 - before.actors24).toBe(3); + }); }); // ---- API endpoint tests ---- @@ -318,6 +454,131 @@ describe('GET /api/audit-log', () => { expect(res.status).toBe(200); expect(res.body.entries.length).toBeGreaterThanOrEqual(1); }); + + it('clamps a negative limit to a single row instead of returning the whole table', async () => { + // A negative LIMIT reaches SQLite as "unlimited" without the clamp. + const res = await request(app) + .get('/api/audit-log?limit=-1') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(200); + expect(res.body.entries.length).toBeLessThanOrEqual(1); + expect(res.body.total).toBeGreaterThan(1); + }); + + it('clamps an oversized limit to the 200 cap even when more rows match', async () => { + const db = DatabaseService.getInstance(); + for (let i = 0; i < 205; i++) { + db.insertAuditLog({ + timestamp: Date.now() - i, + username: 'limitcapuser205', + method: 'POST', + path: `/api/stacks/limitcap${i}`, + status_code: 200, + node_id: null, + ip_address: '127.0.0.1', + summary: `limit cap entry ${i}`, + }); + } + + const res = await request(app) + .get('/api/audit-log?limit=99999&search=limitcapuser205') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(200); + expect(res.body.total).toBe(205); + expect(res.body.entries.length).toBe(200); + }); + + it('clamps a non-positive page to page 1', async () => { + const negative = await request(app) + .get('/api/audit-log?page=-5&limit=5') + .set('Authorization', `Bearer ${adminToken()}`); + const first = await request(app) + .get('/api/audit-log?page=1&limit=5') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(negative.status).toBe(200); + expect(negative.body.entries.length).toBeGreaterThan(0); + // page=-5 must resolve to the same first page, not a negative offset. + expect(negative.body.entries[0].id).toBe(first.body.entries[0].id); + }); + + it('annotates entries with a flags array when with_anomalies=1', async () => { + const res = await request(app) + .get('/api/audit-log?with_anomalies=1&limit=5') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(200); + expect(res.body.entries.length).toBeGreaterThan(0); + for (const entry of res.body.entries) { + expect(Array.isArray(entry.flags)).toBe(true); + } + }); + + it('flags a never-before-seen actor as first_seen_actor', async () => { + const db = DatabaseService.getInstance(); + db.insertAuditLog({ + timestamp: Date.now(), + username: 'brandnewactor_unique', + method: 'POST', + path: '/api/stacks/firstseen', + status_code: 200, + node_id: null, + ip_address: '127.0.0.1', + summary: 'Deployed stack: firstseen', + }); + + const res = await request(app) + .get('/api/audit-log?with_anomalies=1&search=brandnewactor_unique') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(200); + const entry = res.body.entries.find( + (e: { username: string }) => e.username === 'brandnewactor_unique', + ); + expect(entry).toBeDefined(); + expect(entry.flags).toContain('first_seen_actor'); + }); + + it('treats limit=0 as the default page size, not zero rows', async () => { + // parseInt('0') is falsy, so the `|| 50` default applies before the clamp. + const res = await request(app) + .get('/api/audit-log?limit=0') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(200); + expect(res.body.entries.length).toBeGreaterThan(0); + expect(res.body.entries.length).toBeLessThanOrEqual(50); + }); +}); + +describe('GET /api/audit-log/stats', () => { + it('returns 403 without Admiral license', async () => { + const { LicenseService } = await import('../services/LicenseService'); + vi.spyOn(LicenseService.getInstance(), 'getTier').mockReturnValueOnce('community'); + vi.spyOn(LicenseService.getInstance(), 'getVariant').mockReturnValueOnce(null); + + const res = await request(app) + .get('/api/audit-log/stats') + .set('Authorization', `Bearer ${adminToken()}`); + expect(res.status).toBe(403); + }); + + it('returns the four-tile stat structure for admin', async () => { + const res = await request(app) + .get('/api/audit-log/stats') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(200); + expect(res.body).toHaveProperty('events_24h'); + expect(res.body).toHaveProperty('actors_24h'); + expect(res.body).toHaveProperty('failure_rate'); + expect(res.body).toHaveProperty('unusual_hour'); + expect(Array.isArray(res.body.activity_by_hour)).toBe(true); + expect(res.body.activity_by_hour.length).toBe(24); + expect(res.body.failures_by_hour.length).toBe(24); + }); }); describe('GET /api/audit-log/export', () => { @@ -368,6 +629,29 @@ describe('GET /api/audit-log/export', () => { expect(entry.method).toBe('DELETE'); } }); + + it('neutralizes a formula-injection payload in the CSV export', async () => { + const db = DatabaseService.getInstance(); + db.insertAuditLog({ + timestamp: Date.now(), + username: 'csvinjectuser', + method: 'POST', + path: '/api/stacks/csvinject', + status_code: 200, + node_id: null, + ip_address: '127.0.0.1', + summary: '=DANGER_FORMULA', + }); + + const res = await request(app) + .get('/api/audit-log/export?format=csv&search=csvinjectuser') + .set('Authorization', `Bearer ${adminToken()}`); + + expect(res.status).toBe(200); + // The leading '=' must be defused with a single-quote prefix. + expect(res.text).toContain("'=DANGER_FORMULA"); + expect(res.text).not.toMatch(/(^|,)=DANGER_FORMULA/); + }); }); // ---- Audit middleware integration ---- diff --git a/backend/src/__tests__/csv.test.ts b/backend/src/__tests__/csv.test.ts new file mode 100644 index 00000000..7927f5ee --- /dev/null +++ b/backend/src/__tests__/csv.test.ts @@ -0,0 +1,66 @@ +/** + * Tests for escapeCsvField: RFC 4180 quoting plus CSV formula-injection + * neutralization (CWE-1236). Audit-log export embeds user-controlled resource + * names, so any field can carry a leading formula trigger. + */ +import { describe, it, expect } from 'vitest'; +import { escapeCsvField } from '../utils/csv'; + +describe('escapeCsvField', () => { + it('returns empty string for null and undefined', () => { + expect(escapeCsvField(null)).toBe(''); + expect(escapeCsvField(undefined)).toBe(''); + }); + + it('passes benign values through unchanged', () => { + expect(escapeCsvField('Created stack: web')).toBe('Created stack: web'); + expect(escapeCsvField('admin')).toBe('admin'); + expect(escapeCsvField(201)).toBe('201'); + expect(escapeCsvField(0)).toBe('0'); + }); + + it('quotes and doubles embedded quotes / commas / newlines', () => { + expect(escapeCsvField('a,b')).toBe('"a,b"'); + expect(escapeCsvField('say "hi"')).toBe('"say ""hi"""'); + expect(escapeCsvField('line1\nline2')).toBe('"line1\nline2"'); + }); + + it('prefixes a single quote on each formula trigger', () => { + expect(escapeCsvField('=1+1')).toBe("'=1+1"); + expect(escapeCsvField('+1')).toBe("'+1"); + expect(escapeCsvField('-1')).toBe("'-1"); + expect(escapeCsvField('@SUM(A1)')).toBe("'@SUM(A1)"); + expect(escapeCsvField('\tTAB')).toBe("'\tTAB"); + // A leading CR is both defused and RFC 4180 quoted. + expect(escapeCsvField('\rCR')).toBe('"\'\rCR"'); + }); + + it('defuses a trigger hidden behind leading whitespace', () => { + expect(escapeCsvField(' =cmd')).toBe("' =cmd"); + expect(escapeCsvField(' +1')).toBe("' +1"); + expect(escapeCsvField('\t=cmd')).toBe("'\t=cmd"); + }); + + it('prefixes a negative number (conservative, by design)', () => { + expect(escapeCsvField(-5)).toBe("'-5"); + }); + + it('neutralizes a HYPERLINK formula payload in a resource name', () => { + // Embedded quotes + commas force RFC 4180 quoting, so the cell is wrapped; + // the defused content (leading single quote) sits just inside the wrapper. + const malicious = '=HYPERLINK("http://evil","click")'; + const out = escapeCsvField(malicious); + expect(out).toContain("'=HYPERLINK"); + expect(out.startsWith('=')).toBe(false); + }); + + it('combines formula prefix with RFC 4180 quoting when needed', () => { + // Leading trigger AND an embedded comma: prefix first, then quote. + expect(escapeCsvField('=cmd,inject')).toBe('"\'=cmd,inject"'); + }); + + it('does not alter a value where the trigger is not first', () => { + expect(escapeCsvField('stack=web')).toBe('stack=web'); + expect(escapeCsvField('a-b-c')).toBe('a-b-c'); + }); +}); diff --git a/backend/src/routes/auditLog.ts b/backend/src/routes/auditLog.ts index 8a588549..f6db80d5 100644 --- a/backend/src/routes/auditLog.ts +++ b/backend/src/routes/auditLog.ts @@ -1,5 +1,5 @@ import { Router, type Request, type Response } from 'express'; -import { DatabaseService } from '../services/DatabaseService'; +import { DatabaseService, AUDIT_ANOMALY_HISTORY_CAP } from '../services/DatabaseService'; import { annotateEntries, computeAuditStats, HISTORY_WINDOW_MS } from '../services/AuditAnomalyService'; import { requireAdmiral } from '../middleware/tierGates'; import { requirePermission } from '../middleware/permissions'; @@ -14,8 +14,8 @@ auditLogRouter.get('/', async (req: Request, res: Response): Promise => { if (!requirePermission(req, res, 'system:audit')) return; try { - const page = parseInt(req.query.page as string) || 1; - const limit = Math.min(parseInt(req.query.limit as string) || 50, 200); + const page = Math.max(1, parseInt(req.query.page as string) || 1); + const limit = Math.min(Math.max(1, parseInt(req.query.limit as string) || 50), 200); const username = req.query.username as string | undefined; const method = req.query.method as string | undefined; const search = req.query.search as string | undefined; @@ -36,7 +36,7 @@ auditLogRouter.get('/', async (req: Request, res: Response): Promise => { (min, e) => Math.min(min, e.timestamp), result.entries[0].timestamp, ); - const history = db.getAuditLogsInRange(historyFrom, oldestInPage); + const history = db.getAuditLogsInRange(historyFrom, oldestInPage, AUDIT_ANOMALY_HISTORY_CAP); res.json({ ...result, entries: annotateEntries(result.entries, history, now) }); return; } @@ -52,14 +52,11 @@ auditLogRouter.get('/stats', async (req: Request, res: Response): Promise if (!requirePermission(req, res, 'system:audit')) return; try { - const now = Date.now(); + if (isDebugEnabled()) { + console.log('[Audit:diag] Stats requested'); + } const db = DatabaseService.getInstance(); - const cutoff24h = now - 24 * 60 * 60 * 1000; - const cutoff7d = now - 7 * 24 * 60 * 60 * 1000; - const last30d = db.getAuditLogsInRange(now - HISTORY_WINDOW_MS, now); - const last7d = last30d.filter(e => e.timestamp >= cutoff7d); - const last24h = last7d.filter(e => e.timestamp >= cutoff24h); - res.json(computeAuditStats({ now, last24h, last7d, last30d })); + res.json(computeAuditStats(db.getAuditStatsInputs(Date.now()))); } catch (error) { console.error('[AuditLog] Failed to compute audit stats:', error); res.status(500).json({ error: 'Failed to compute audit stats' }); diff --git a/backend/src/services/AuditAnomalyService.ts b/backend/src/services/AuditAnomalyService.ts index 2a6dae73..987b48ad 100644 --- a/backend/src/services/AuditAnomalyService.ts +++ b/backend/src/services/AuditAnomalyService.ts @@ -68,49 +68,32 @@ export interface AuditStats { failures_by_hour: number[]; } -export function computeAuditStats(input: { - now: number; - last24h: AuditLogEntry[]; - last7d: AuditLogEntry[]; - last30d: AuditLogEntry[]; -}): AuditStats { - const { now, last24h, last7d, last30d } = input; - const events24 = last24h.length; - const prior7d = last7d.length - events24; +/** + * Format the signal-rail stats from exact aggregate inputs (see + * DatabaseService.getAuditStatsInputs). This is pure presentation: counts, + * hourly series, and new-ip detection are computed exactly upstream in SQL so + * the tiles never undercount on a large window. + */ +export interface AuditStatsInput { + events24: number; + events7d: number; + actors24: number; + failures24: number; + activityByHour: number[]; + failuresByHour: number[]; + newIpCount: number; + sampleNewIpActor: string | null; +} + +export function computeAuditStats(input: AuditStatsInput): AuditStats { + const { events24, events7d, actors24, failures24, activityByHour, failuresByHour, newIpCount, sampleNewIpActor } = input; + const prior7d = events7d - events24; const avg7dPerDay = Math.max(0, prior7d) / 6; const deltaPct = avg7dPerDay > 0 ? Math.round(((events24 - avg7dPerDay) / avg7dPerDay) * 100) : null; - const actors24 = new Set(last24h.map(e => e.username).filter(Boolean)); - const olderIpByActor = new Map>(); - for (const e of last30d) { - if (!e.username || !e.ip_address) continue; - if (e.timestamp >= now - 24 * 60 * 60 * 1000) continue; - let set = olderIpByActor.get(e.username); - if (!set) { set = new Set(); olderIpByActor.set(e.username, set); } - set.add(e.ip_address); - } - let newIpCount = 0; - let sampleNewIpActor: string | null = null; - for (const e of last24h) { - if (!e.username || !e.ip_address) continue; - const prior = olderIpByActor.get(e.username); - if (prior && prior.size > 0 && !prior.has(e.ip_address)) { - newIpCount++; - if (!sampleNewIpActor) sampleNewIpActor = e.username; - } - } - - const failureCount = last24h.filter(e => e.status_code >= 400).length; - const failureRate = events24 > 0 ? failureCount / events24 : 0; + const failureRate = events24 > 0 ? failures24 / events24 : 0; const failurePct = Math.round(failureRate * 100); - const activityByHour = Array.from({ length: 24 }, () => 0); - const failuresByHour = Array.from({ length: 24 }, () => 0); - for (const e of last24h) { - const hour = new Date(e.timestamp).getHours(); - activityByHour[hour]++; - if (e.status_code >= 400) failuresByHour[hour]++; - } const peakHour = activityByHour.reduce( (best, count, hour) => (count > best.count ? { count, hour } : best), { count: -1, hour: 0 } @@ -125,7 +108,7 @@ export function computeAuditStats(input: { severity: deltaPct !== null && deltaPct > 150 ? 'warn' : 'ok', }, actors_24h: { - value: actors24.size, + value: actors24, label: 'actors', detail: newIpCount > 0 ? `${newIpCount} new ip${newIpCount === 1 ? '' : 's'}${sampleNewIpActor ? ` ยท ${sampleNewIpActor}` : ''}` @@ -135,7 +118,7 @@ export function computeAuditStats(input: { failure_rate: { value: failurePct, label: 'failure rate', - detail: `${failureCount} of ${events24} request${events24 === 1 ? '' : 's'}`, + detail: `${failures24} of ${events24} request${events24 === 1 ? '' : 's'}`, severity: failurePct >= 20 ? 'alert' : failurePct >= 5 ? 'warn' : 'ok', }, unusual_hour: { diff --git a/backend/src/services/DatabaseService.ts b/backend/src/services/DatabaseService.ts index 2567769c..098414dc 100644 --- a/backend/src/services/DatabaseService.ts +++ b/backend/src/services/DatabaseService.ts @@ -3,6 +3,7 @@ import path from 'path'; import fs from 'fs'; import { CryptoService } from './CryptoService'; import { isSeverityAtLeast } from '../utils/severity'; +import type { AuditStatsInput } from './AuditAnomalyService'; function isPilotMode(): boolean { return process.env.SENCHO_MODE === 'pilot'; @@ -612,6 +613,14 @@ export interface ScanSummary { const AUDIT_LOG_FLUSH_INTERVAL_MS = 1_000; const AUDIT_LOG_FLUSH_THRESHOLD = 100; +// Upper bound on the rows the anomaly baseline / stats computations pull into +// memory for a single request. The audit table grows unbounded within the +// retention window, and the analysis paths run on every list page and every +// stats refresh, so without a cap a busy fleet would scan the whole window into +// a JS array each time. When the window holds more than this, the most-recent +// rows are used and baselines become an approximation over recent activity. +export const AUDIT_ANOMALY_HISTORY_CAP = 20_000; + export const PILOT_METRICS_COUNTERS_KEY = 'pilot_metrics_counters'; export class DatabaseService { @@ -2984,13 +2993,85 @@ export class DatabaseService { this.db.prepare('DELETE FROM audit_log WHERE timestamp < ?').run(cutoff); } - public getAuditLogsInRange(from: number, to: number): AuditLogEntry[] { + public getAuditLogsInRange(from: number, to: number, limit?: number): AuditLogEntry[] { this.flushAuditLogBuffer(); + if (limit !== undefined) { + // Cap to the most-recent `limit` rows in the window, then return + // them in ascending order to preserve this method's contract. + return this.db.prepare( + `SELECT * FROM ( + SELECT * FROM audit_log WHERE timestamp >= ? AND timestamp < ? + ORDER BY timestamp DESC LIMIT ? + ) ORDER BY timestamp ASC` + ).all(from, to, limit) as AuditLogEntry[]; + } return this.db.prepare( 'SELECT * FROM audit_log WHERE timestamp >= ? AND timestamp < ? ORDER BY timestamp ASC' ).all(from, to) as AuditLogEntry[]; } + /** + * Exact aggregate inputs for the audit signal-rail stats, computed with SQL + * COUNT / GROUP BY rather than materializing rows. The counts and hourly + * series stay exact regardless of window size (no row cap), while the + * new-ip detection works over the small DISTINCT (user, ip) pair sets. + */ + public getAuditStatsInputs(now: number): AuditStatsInput { + this.flushAuditLogBuffer(); + const cutoff24h = now - 24 * 60 * 60 * 1000; + const cutoff7d = now - 7 * 24 * 60 * 60 * 1000; + const cutoff30d = now - 30 * 24 * 60 * 60 * 1000; + // Every current-window query is upper-bounded by `now` so a future-dated + // row (clock skew, a test fixture) never inflates the live counts. + const countOf = (sql: string, ...params: number[]): number => + (this.db.prepare(sql).get(...params) as { c: number }).c; + + const events24 = countOf('SELECT COUNT(*) AS c FROM audit_log WHERE timestamp >= ? AND timestamp < ?', cutoff24h, now); + const events7d = countOf('SELECT COUNT(*) AS c FROM audit_log WHERE timestamp >= ? AND timestamp < ?', cutoff7d, now); + const actors24 = countOf("SELECT COUNT(DISTINCT username) AS c FROM audit_log WHERE timestamp >= ? AND timestamp < ? AND username != ''", cutoff24h, now); + const failures24 = countOf('SELECT COUNT(*) AS c FROM audit_log WHERE timestamp >= ? AND timestamp < ? AND status_code >= 400', cutoff24h, now); + + const activityByHour = Array.from({ length: 24 }, () => 0); + const failuresByHour = Array.from({ length: 24 }, () => 0); + const hourRows = this.db.prepare( + `SELECT CAST(strftime('%H', timestamp / 1000, 'unixepoch', 'localtime') AS INTEGER) AS hour, + COUNT(*) AS total, + SUM(CASE WHEN status_code >= 400 THEN 1 ELSE 0 END) AS failures + FROM audit_log WHERE timestamp >= ? AND timestamp < ? GROUP BY hour`, + ).all(cutoff24h, now) as { hour: number; total: number; failures: number }[]; + for (const r of hourRows) { + if (r.hour >= 0 && r.hour < 24) { + activityByHour[r.hour] = r.total; + failuresByHour[r.hour] = r.failures; + } + } + + // ORDER BY makes both the new-ip scan and the sample actor deterministic. + const recentPairs = this.db.prepare( + "SELECT DISTINCT username, ip_address FROM audit_log WHERE timestamp >= ? AND timestamp < ? AND username != '' AND ip_address != '' ORDER BY username, ip_address", + ).all(cutoff24h, now) as { username: string; ip_address: string }[]; + const priorPairs = this.db.prepare( + "SELECT DISTINCT username, ip_address FROM audit_log WHERE timestamp >= ? AND timestamp < ? AND username != '' AND ip_address != ''", + ).all(cutoff30d, cutoff24h) as { username: string; ip_address: string }[]; + const priorByActor = new Map>(); + for (const p of priorPairs) { + let set = priorByActor.get(p.username); + if (!set) { set = new Set(); priorByActor.set(p.username, set); } + set.add(p.ip_address); + } + let newIpCount = 0; + let sampleNewIpActor: string | null = null; + for (const p of recentPairs) { + const prior = priorByActor.get(p.username); + if (prior && prior.size > 0 && !prior.has(p.ip_address)) { + newIpCount++; + if (!sampleNewIpActor) sampleNewIpActor = p.username; + } + } + + return { events24, events7d, actors24, failures24, activityByHour, failuresByHour, newIpCount, sampleNewIpActor }; + } + // --- API Tokens --- public addApiToken(token: Omit): number { diff --git a/backend/src/utils/csv.ts b/backend/src/utils/csv.ts index f146d918..500722a3 100644 --- a/backend/src/utils/csv.ts +++ b/backend/src/utils/csv.ts @@ -1,12 +1,29 @@ +// Matches a value a spreadsheet would evaluate as a formula: a metacharacter +// (= + - @), optionally preceded by whitespace, since Excel / Sheets / +// LibreOffice ignore leading blanks when detecting formulas. A leading tab or +// carriage return is also treated as a trigger because some parsers strip it +// before evaluating what follows. Audit summaries embed user-controlled +// resource names, so any field can reach the export untrusted. +const FORMULA_LEAD_RE = /^\s*[=+\-@]/; + +function startsFormula(str: string): boolean { + return str[0] === '\t' || str[0] === '\r' || FORMULA_LEAD_RE.test(str); +} + /** - * Escape a single field for RFC 4180 CSV output. Wraps the value in quotes - * and doubles embedded quotes when the field contains a comma, quote, or - * newline. Null / undefined become the empty string. + * Escape a single field for RFC 4180 CSV output. Neutralizes formula injection + * by prefixing a single quote when the value would be read as a formula + * (CWE-1236), then wraps the value in quotes and doubles embedded quotes when + * it contains a comma, quote, or line break. Null / undefined become the empty + * string. */ export function escapeCsvField(val: string | number | null | undefined): string { if (val === null || val === undefined) return ''; - const str = String(val); - if (str.includes(',') || str.includes('"') || str.includes('\n')) { + let str = String(val); + if (str.length > 0 && startsFormula(str)) { + str = `'${str}`; + } + if (str.includes(',') || str.includes('"') || str.includes('\n') || str.includes('\r')) { return `"${str.replace(/"/g, '""')}"`; } return str; diff --git a/docs/features/audit-log.mdx b/docs/features/audit-log.mdx index 83fbbbb4..cf99f46a 100644 --- a/docs/features/audit-log.mdx +++ b/docs/features/audit-log.mdx @@ -113,7 +113,7 @@ In Stream view, Sencho annotates individual entries with lightweight anomaly fla | **new ip** | The IP address on this entry has not been seen for this actor in the last 30 days, and the actor already has at least one prior IP on record. | | **first seen** | The actor has no prior entries in the 30-day history window. Useful for spotting brand-new service accounts, CLI tools, or compromised sessions. | -Flags are computed at read time against your existing audit history. No new tables, no per-entry storage overhead, and the logic is cache-friendly, so enabling Stream view does not slow down the audit log endpoint. +Flags are computed at read time against your existing audit history. No new tables, no per-entry storage overhead, and the logic is cache-friendly, so enabling Stream view does not slow down the audit log endpoint. On instances with a very large audit history, baselines are computed from the most recent slice of activity so the view stays responsive; this keeps memory and latency bounded without changing how flags behave for typical histories. ## Filtering and search diff --git a/frontend/src/components/AuditLogView.tsx b/frontend/src/components/AuditLogView.tsx index 75a15171..148add76 100644 --- a/frontend/src/components/AuditLogView.tsx +++ b/frontend/src/components/AuditLogView.tsx @@ -101,6 +101,13 @@ function dayKey(ts: number): string { return `${d.getFullYear()}-${d.getMonth()}-${d.getDate()}`; } +// These views fetch localOnly, so a 401 is the user's own expired session, +// which apiFetch already turns into a global logout. Skip the redundant +// load-failure toast in that case so logout does not stack toasts. +function isExpiredSession(err: unknown): boolean { + return err instanceof Error && err.message === 'Unauthorized'; +} + export function AuditLogView() { const [entries, setEntries] = useState([]); const [total, setTotal] = useState(0); @@ -151,9 +158,15 @@ export function AuditLogView() { const data = await res.json(); setEntries(data.entries); setTotal(data.total); + } else { + const err = await res.json().catch(() => ({})); + toast.error(err?.error || err?.message || 'Failed to load audit log.'); } } catch (err) { console.error('[AuditLog] Failed to fetch:', err); + if (!isExpiredSession(err)) { + toast.error('Failed to load audit log.'); + } } finally { setLoading(false); } @@ -164,9 +177,15 @@ export function AuditLogView() { const res = await apiFetch('/audit-log/stats', { localOnly: true }); if (res.ok) { setStats(await res.json()); + } else { + const err = await res.json().catch(() => ({})); + toast.error(err?.error || err?.message || 'Failed to load audit stats.'); } } catch (err) { console.error('[AuditLog] Failed to fetch stats:', err); + if (!isExpiredSession(err)) { + toast.error('Failed to load audit stats.'); + } } }, []);