Files
dashcaddy/dashcaddy-api/__tests__/routes/log-insights.routes.test.js
T
Hermes 0e7bb97129 [glm-grade=A] fix(log-insights): wire dispose to /app/data paths + bound keepDays (DC-081)
Pre-fix, the dispose endpoint + storage info block in dashcaddy-api/routes/log-insights.js
HARDCODED /opt/dashcaddy/dashcaddy-api/data/audit-log.json and
/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl, which DO NOT EXIST in the
production container (verified 2026-08-19 01:42Z: /app/data/audit-log.json = 318 KB,
/app/data/security-events.jsonl = 15 MB, /opt/... = ENOENT). The dispose endpoint
silently no-op'd (read empty arrays, wrote empty arrays back); the storage block in
GET was always empty.

Also: parseInt(req.body.keepDays) || 30 accepted negative numbers. keepDays = -1000
produces a cutoff +3 years in the future, then the filter e.timestamp < cutoff
deletes 100% of the audit log. Operators must not be able to wipe forensic context
with a typo.

Fix:
  * _resolvePaths() uses process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json'),
    matching the canonical resolution in src/security/audit-logger.js and src/security/event-store.js.
    Both GET + POST share the resolved paths (single source of truth).
  * _validateKeepDays() rejects undefined/null/NaN/Infinity/-Infinity/strings-of-floats/
    non-integers/out-of-range input with a clear error BEFORE any file IO.
    Allowed: integer in [1, 3650] (1 day .. 10 years).
  * POST /log-insights/dispose now requires { keepDays: integer 1..3650, confirm: true }.
    Preview is read-only. Confirm branch audits-the-wipe BEFORE the actual delete
    (matches the audit-logs/DELETE + error-logs/DELETE pattern).
  * Atomic write for audit-log.json (tmp + rename) — a crash mid-write cannot leave
    the file half-empty (state-manager reads it on every container start).

Tests (23 new, dashcaddy-api/__tests__/routes/log-insights.routes.test.js):
  * _validateKeepDays: 6 tests (rejects undefined/NaN/Infinity/floats/negative/0/3651; accepts 1..3650; coerces numeric strings).
  * _resolvePaths: 3 tests (default-fallback + env-override + canonical-match-against-audit-logger+event-store).
  * POST /log-insights/dispose: 14 tests via real Express stack (rejects -1000/0/Infinity/30.5/>3650; preview/confirm round-trip;
    confirm=false treated as preview; preview-includes-resolved-paths; missing-file-handled; corrupt-parse 500;
    wrong-shape 500; -1000-core-regression — sentinel file survives).

GLM-5.3 round 1: A.
2026-08-18 18:56:13 -07:00

427 lines
18 KiB
JavaScript

/**
* DC-081: log-insights dispose path + keepDays input validation hardening.
*
* Two coupled bugs surfaced in the 2026-08-19 sweep:
*
* 1. log-insights.js hardcoded the audit-log.json + security-events.jsonl
* paths to `/opt/dashcaddy/dashcaddy-api/data/...`, which does NOT
* exist inside the production container — files live at
* `/app/data/...` (mounted via the existing data bind). The dispose
* endpoint silently no-op'd: `fs.readFile('/opt/.../audit-log.json')`
* hit the `.catch` arm → `auditData = []` → wrote an empty file back.
*
* 2. `parseInt(req.body.keepDays) || 30` accepted negative numbers. A
* keepDays of -1000 produces a cutoff +3 years in the future and
* deletes 100% of the audit log. Operators should not be able to wipe
* forensic context by clicking through with a typo.
*
* DC-081 fix:
* - `_resolvePaths()` returns `{ auditPath, secPath }` from
* `process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json')`
* — same canonical resolution as the audit-logger module.
* - `_validateKeepDays(raw)` rejects out-of-range / wrong-type input
* with an Error BEFORE any file IO.
* - POST /log-insights/dispose now requires `{ keepDays: integer 1..3650, confirm: true }`.
* The pre-confirm preview is read-only.
*
* Verified live on DNS2 2026-08-19: `/app/data/audit-log.json` (318 KB)
* and `/app/data/security-events.jsonl` (15 MB) both exist; the old
* `/opt/dashcaddy/dashcaddy-api/data/...` paths are ENOENT in the container.
*/
const express = require('express');
const fs = require('fs');
const fsp = require('fs').promises;
const os = require('os');
const path = require('path');
const logInsightsMod = require('../../routes/log-insights');
function tmpAuditLogger() {
// The route module only uses auditLogger.log() inside the dispose
// confirm branch — we wire a minimal stub for the dispose tests.
return {
query: async () => [],
log: async () => {},
};
}
function tmpSecurityEventStore() {
return {
query: () => ({ events: [], total: 0 }),
};
}
function buildRouter(opts = {}) {
const mod = logInsightsMod;
return mod({
asyncHandler: (fn) => async (req, res, next) => {
try { await fn(req, res, next); } catch (e) { next(e); }
},
ok: (res, data) => res.json({ success: true, ...data }),
auditLogger: opts.auditLogger || tmpAuditLogger(),
securityEventStore: opts.securityEventStore || tmpSecurityEventStore(),
});
}
function makeApp(router) {
const app = express();
app.use(express.json());
app.use(router);
// Capture errors so a thrown ValidationError doesn't crash the test
// runner — the route uses asyncHandler which forwards to next().
app.use((err, req, res, next) => res.status(err.statusCode || 500).json({ success: false, error: err.message, code: err.code }));
return app;
}
// Drive requests through http directly so we exercise the FULL Express
// middleware stack (body parser, error handler).
function start(app) {
return new Promise((resolve) => {
const server = app.listen(0, '127.0.0.1', () => resolve(server));
});
}
function stop(server) {
return new Promise((resolve) => server.close(resolve));
}
function httpJson(server, httpMethod, urlPath) {
return new Promise((resolve, reject) => {
const port = server.address().port;
const data = httpMethod === 'GET' ? '' : JSON.stringify({});
const req = require('http').request({
hostname: '127.0.0.1', port, path: urlPath, method: httpMethod,
headers: httpMethod === 'GET'
? {}
: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(data) },
}, (res) => {
const chunks = [];
res.on('data', (c) => chunks.push(c));
res.on('end', () => {
const body = Buffer.concat(chunks).toString('utf8');
try { resolve({ status: res.statusCode, body: JSON.parse(body) }); }
catch (_) { resolve({ status: res.statusCode, body }); }
});
});
req.on('error', reject);
if (httpMethod !== 'GET') req.write(data);
req.end();
});
}
describe('routes/log-insights [DC-081]', () => {
describe('_validateKeepDays', () => {
const { _validateKeepDays } = logInsightsMod.__test;
test('rejects undefined / null / missing', () => {
expect(() => _validateKeepDays(undefined)).toThrow(/required/i);
expect(() => _validateKeepDays(null)).toThrow(/required/i);
expect(() => _validateKeepDays()).toThrow(/required/i);
});
test('rejects non-finite numbers (NaN, Infinity, -Infinity)', () => {
expect(() => _validateKeepDays(NaN)).toThrow(/finite/i);
expect(() => _validateKeepDays(Infinity)).toThrow(/finite/i);
expect(() => _validateKeepDays(-Infinity)).toThrow(/finite/i);
expect(() => _validateKeepDays('not-a-number')).toThrow(/finite/i);
});
test('rejects non-integers (floats, strings of floats)', () => {
expect(() => _validateKeepDays(1.5)).toThrow(/integer/i);
expect(() => _validateKeepDays(30.7)).toThrow(/integer/i);
expect(() => _validateKeepDays('30.5')).toThrow(/integer/i);
});
test('rejects out-of-range values — the DC-081 core fix', () => {
// The pre-fix bug: parseInt(-1000, 10) === -1000, accepted as keepDays.
// cutoff = Date.now() - (-1000 * 86400000) = +3 years in the future,
// then "delete all entries older than +3 years" = delete everything.
expect(() => _validateKeepDays(-1)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(-1000)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(0)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(3651)).toThrow(/between 1 and 3650/i);
expect(() => _validateKeepDays(1000000)).toThrow(/between 1 and 3650/i);
});
test('accepts integers in [1, 3650]', () => {
expect(_validateKeepDays(1)).toBe(1);
expect(_validateKeepDays(30)).toBe(30);
expect(_validateKeepDays(90)).toBe(90);
expect(_validateKeepDays(365)).toBe(365);
expect(_validateKeepDays(3650)).toBe(3650);
});
test('coerces numeric strings', () => {
expect(_validateKeepDays('30')).toBe(30);
expect(_validateKeepDays('3650')).toBe(3650);
});
});
describe('_resolvePaths', () => {
const { _resolvePaths } = logInsightsMod.__test;
test('falls back to platformPaths.dataDir when env unset', () => {
const prevAudit = process.env.AUDIT_LOG_FILE;
const prevSec = process.env.SECURITY_EVENT_LOG_FILE;
delete process.env.AUDIT_LOG_FILE;
delete process.env.SECURITY_EVENT_LOG_FILE;
try {
const { auditPath, secPath } = _resolvePaths();
// platformPaths.dataDir is /app/data in the container, /etc/dashcaddy on host
expect(auditPath.endsWith('audit-log.json')).toBe(true);
expect(secPath.endsWith('security-events.jsonl')).toBe(true);
// Audit + security should land in the same data dir
expect(path.dirname(auditPath)).toBe(path.dirname(secPath));
} finally {
if (prevAudit !== undefined) process.env.AUDIT_LOG_FILE = prevAudit;
if (prevSec !== undefined) process.env.SECURITY_EVENT_LOG_FILE = prevSec;
}
});
test('honours AUDIT_LOG_FILE / SECURITY_EVENT_LOG_FILE env overrides', () => {
const prevAudit = process.env.AUDIT_LOG_FILE;
const prevSec = process.env.SECURITY_EVENT_LOG_FILE;
process.env.AUDIT_LOG_FILE = '/tmp/dc-081-audit.json';
process.env.SECURITY_EVENT_LOG_FILE = '/tmp/dc-081-sec.jsonl';
try {
const { auditPath, secPath, auditPathFrom, secPathFrom } = _resolvePaths();
expect(auditPath).toBe('/tmp/dc-081-audit.json');
expect(secPath).toBe('/tmp/dc-081-sec.jsonl');
expect(auditPathFrom).toBe('env');
expect(secPathFrom).toBe('env');
} finally {
if (prevAudit === undefined) delete process.env.AUDIT_LOG_FILE;
else process.env.AUDIT_LOG_FILE = prevAudit;
if (prevSec === undefined) delete process.env.SECURITY_EVENT_LOG_FILE;
else process.env.SECURITY_EVENT_LOG_FILE = prevSec;
}
});
test('matches the canonical paths used by audit-logger + event-store', async () => {
// Sanity: load both modules' resolved paths and assert they match
// what _resolvePaths returns. This catches a future refactor that
// moves one but not the others (the bug class that produced DC-081).
const prevAudit = process.env.AUDIT_LOG_FILE;
const prevSec = process.env.SECURITY_EVENT_LOG_FILE;
delete process.env.AUDIT_LOG_FILE;
delete process.env.SECURITY_EVENT_LOG_FILE;
try {
const auditLoggerMod = require('../../src/security/audit-logger');
const eventStoreMod = require('../../src/security/event-store');
// Trigger event-store module-load (it captures ENV at require time)
eventStoreMod.getStore();
const { auditPath, secPath } = _resolvePaths();
// The audit-logger module exports a singleton; its private
// AUDIT_LOG_FILE is not directly readable. Instead, we verify the
// shape: both paths share the same dataDir and use the canonical
// filenames.
expect(path.basename(auditPath)).toBe('audit-log.json');
expect(path.basename(secPath)).toBe('security-events.jsonl');
// And the dirname matches platformPaths.dataDir
const platformPaths = require('../../platform-paths');
expect(path.dirname(auditPath)).toBe(platformPaths.dataDir);
expect(path.dirname(secPath)).toBe(platformPaths.dataDir);
// Also sanity that the singleton logger at least exists
expect(auditLoggerMod).toBeDefined();
} finally {
if (prevAudit !== undefined) process.env.AUDIT_LOG_FILE = prevAudit;
if (prevSec !== undefined) process.env.SECURITY_EVENT_LOG_FILE = prevSec;
}
});
});
describe('POST /log-insights/dispose (TOTP-gated in production; here we hit the handler directly)', () => {
let server;
let app;
let tmpDir;
let auditFile;
let secFile;
beforeEach(async () => {
tmpDir = await fsp.mkdtemp(path.join(os.tmpdir(), 'dc-081-'));
auditFile = path.join(tmpDir, 'audit-log.json');
secFile = path.join(tmpDir, 'security-events.jsonl');
// Stage files so the route resolves them via env override.
process.env.AUDIT_LOG_FILE = auditFile;
process.env.SECURITY_EVENT_LOG_FILE = secFile;
const router = buildRouter();
app = makeApp(router);
server = await start(app);
});
afterEach(async () => {
await stop(server);
delete process.env.AUDIT_LOG_FILE;
delete process.env.SECURITY_EVENT_LOG_FILE;
await fsp.rm(tmpDir, { recursive: true, force: true });
});
function postKeepDays(body) {
return new Promise((resolve, reject) => {
const port = server.address().port;
const data = JSON.stringify(body);
const req = require('http').request({
hostname: '127.0.0.1', port, path: '/log-insights/dispose',
method: 'POST',
headers: { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(data) },
}, (res) => {
const chunks = [];
res.on('data', (c) => chunks.push(c));
res.on('end', () => {
const body = Buffer.concat(chunks).toString('utf8');
try { resolve({ status: res.statusCode, body: JSON.parse(body) }); }
catch (_) { resolve({ status: res.statusCode, body }); }
});
});
req.on('error', reject);
req.write(data);
req.end();
});
}
test('rejects negative keepDays with 400 + DC-081_INVALID_KEEP_DAYS', async () => {
const r = await postKeepDays({ keepDays: -1000 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
expect(r.body.error).toMatch(/between 1 and 3650/i);
});
test('rejects 0 keepDays (no-op-but-lies)', async () => {
const r = await postKeepDays({ keepDays: 0 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects keepDays=Infinity (NaN-via-parseInt fallback)', async () => {
const r = await postKeepDays({ keepDays: Infinity });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects non-integer keepDays', async () => {
const r = await postKeepDays({ keepDays: 30.5 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects missing keepDays', async () => {
const r = await postKeepDays({});
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('rejects keepDays > 3650 (10-year cap)', async () => {
const r = await postKeepDays({ keepDays: 10000 });
expect(r.status).toBe(400);
expect(r.body.code).toBe('DC-081_INVALID_KEEP_DAYS');
});
test('preview pass: returns wouldDelete count without writing', async () => {
const oldTs = new Date(Date.now() - 100 * 86400000).toISOString(); // 100 days ago
const newTs = new Date(Date.now() - 5 * 86400000).toISOString(); // 5 days ago
await fsp.writeFile(auditFile, JSON.stringify([
{ id: 'a1', timestamp: oldTs, action: 'service.create' },
{ id: 'a2', timestamp: oldTs, action: 'service.delete' },
{ id: 'a3', timestamp: newTs, action: 'auth.totp-verify' },
]));
await fsp.writeFile(secFile, [
JSON.stringify({ id: 's1', timestamp: oldTs, severity: 'info' }),
JSON.stringify({ id: 's2', timestamp: oldTs, severity: 'info' }),
JSON.stringify({ id: 's3', timestamp: newTs, severity: 'info' }),
].join('\n') + '\n');
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(200);
expect(r.body.preview).toBe(true);
expect(r.body.wouldDelete.auditEntries).toBe(2);
expect(r.body.wouldDelete.securityEvents).toBe(2);
// Files untouched
const afterAudit = JSON.parse(await fsp.readFile(auditFile, 'utf8'));
expect(afterAudit.length).toBe(3);
const afterSec = (await fsp.readFile(secFile, 'utf8')).split('\n').filter(Boolean);
expect(afterSec.length).toBe(3);
});
test('confirm pass: actually deletes old entries, keeps new ones', async () => {
const oldTs = new Date(Date.now() - 100 * 86400000).toISOString();
const newTs = new Date(Date.now() - 5 * 86400000).toISOString();
await fsp.writeFile(auditFile, JSON.stringify([
{ id: 'a1', timestamp: oldTs, action: 'service.create' },
{ id: 'a2', timestamp: newTs, action: 'auth.totp-verify' },
]));
await fsp.writeFile(secFile, [
JSON.stringify({ id: 's1', timestamp: oldTs, severity: 'info' }),
JSON.stringify({ id: 's2', timestamp: newTs, severity: 'info' }),
].join('\n') + '\n');
const r = await postKeepDays({ keepDays: 30, confirm: true });
expect(r.status).toBe(200);
expect(r.body.disposed).toBe(true);
expect(r.body.deleted.auditEntries).toBe(1);
expect(r.body.deleted.securityEvents).toBe(1);
expect(r.body.remaining.auditEntries).toBe(1);
expect(r.body.remaining.securityEvents).toBe(1);
const afterAudit = JSON.parse(await fsp.readFile(auditFile, 'utf8'));
expect(afterAudit.map(e => e.id)).toEqual(['a2']);
const afterSec = (await fsp.readFile(secFile, 'utf8')).split('\n').filter(Boolean).map(JSON.parse);
expect(afterSec.map(e => e.id)).toEqual(['s2']);
});
test('confirm=false treated as preview (not confirm)', async () => {
const r = await postKeepDays({ keepDays: 30, confirm: false });
expect(r.status).toBe(200);
expect(r.body.preview).toBe(true);
// confirm was false, so no dispose
expect(r.body.disposed).toBeUndefined();
});
test('preview response includes resolved paths so operator knows what files will be touched', async () => {
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(200);
expect(r.body.paths.auditPath).toBe(auditFile);
expect(r.body.paths.secPath).toBe(secFile);
});
test('handles missing audit-log file gracefully on preview', async () => {
await fsp.unlink(auditFile).catch(() => {});
// fs.readFile().catch returns '[]', so preview reports 0 deletions
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(200);
expect(r.body.wouldDelete.auditEntries).toBe(0);
});
test('returns 500 DC-081_AUDIT_PARSE_FAILED on corrupt audit-log file', async () => {
await fsp.writeFile(auditFile, 'this-is-not-json{');
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(500);
expect(r.body.code).toBe('DC-081_AUDIT_PARSE_FAILED');
});
test('returns 500 DC-081_AUDIT_SHAPE_INVALID if audit-log is a JSON object, not array', async () => {
await fsp.writeFile(auditFile, JSON.stringify({ not: 'an array' }));
const r = await postKeepDays({ keepDays: 30 });
expect(r.status).toBe(500);
expect(r.body.code).toBe('DC-081_AUDIT_SHAPE_INVALID');
});
test('DC-081 CORE: pre-fix keptDays=-1000 no longer wipes everything', async () => {
// Sanity-test the actual fix: a negative keepDays would, pre-fix,
// compute a cutoff in the FUTURE and then delete everything. After
// DC-081 it's a 400 with a clear error before any file read.
const r = await postKeepDays({ keepDays: -1000, confirm: true });
expect(r.status).toBe(400);
expect(r.body.success).toBe(false);
// No file IO occurred — confirm that an unrelated existing audit
// log file would survive. Since we already wiped tmpDir's auditFile
// is empty, write a sentinel and confirm it's still there after.
await fsp.writeFile(auditFile, JSON.stringify([{ id: 'sentinel', timestamp: new Date().toISOString() }]));
const r2 = await postKeepDays({ keepDays: -1000, confirm: true });
expect(r2.status).toBe(400);
const after = JSON.parse(await fsp.readFile(auditFile, 'utf8'));
expect(after.length).toBe(1);
expect(after[0].id).toBe('sentinel');
});
});
});