Compare commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
0e7bb97129 | ||
|
|
98737995a9 | ||
|
|
99ec6ebc53 |
@@ -0,0 +1,427 @@
|
|||||||
|
/**
|
||||||
|
* 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');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
@@ -131,6 +131,20 @@ describe('routes/tailscale-admin: PUT /settings', () => {
|
|||||||
expect(res.status).toBe(400);
|
expect(res.status).toBe(400);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('400 on apiToken exceeding 256-char length cap (DC-080)', async () => {
|
||||||
|
const { app } = createApp();
|
||||||
|
const oversized = 'tskey-api-' + 'x'.repeat(300); // > 256 chars
|
||||||
|
const res = await request(app).put('/api/v1/tailscale/settings').send({ apiToken: oversized });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(res.body.error || res.body.message).toMatch(/exceeds maximum length/i);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('400 on non-string apiToken (DC-080)', async () => {
|
||||||
|
const { app } = createApp();
|
||||||
|
const res = await request(app).put('/api/v1/tailscale/settings').send({ apiToken: 12345 });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
});
|
||||||
|
|
||||||
test('200 + saves token + writes metadata on valid token', async () => {
|
test('200 + saves token + writes metadata on valid token', async () => {
|
||||||
const fakeClient = makeFakeClient({
|
const fakeClient = makeFakeClient({
|
||||||
ping: jest.fn(async () => ({ domain: 'real.ts.net' })),
|
ping: jest.fn(async () => ({ domain: 'real.ts.net' })),
|
||||||
@@ -293,6 +307,76 @@ describe('routes/tailscale-admin: POST /settings/test', () => {
|
|||||||
expect(res.body.valid).toBe(true);
|
expect(res.body.valid).toBe(true);
|
||||||
expect(fakeClient.setApiToken).toHaveBeenCalledWith('tskey-api-test-only');
|
expect(fakeClient.setApiToken).toHaveBeenCalledWith('tskey-api-test-only');
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('400 on body.apiToken not starting with tskey-api- (DC-080)', async () => {
|
||||||
|
const fakeClient = makeFakeClient();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
const routes = require('../../routes/tailscale-admin');
|
||||||
|
const tailscaleCoord = {
|
||||||
|
loadMetadata: () => ({ configured: false }),
|
||||||
|
saveMetadata: jest.fn(),
|
||||||
|
setApiToken: jest.fn(),
|
||||||
|
getClient: jest.fn(async () => fakeClient),
|
||||||
|
hasApiToken: jest.fn(),
|
||||||
|
};
|
||||||
|
app.use('/api/v1/tailscale', routes({
|
||||||
|
tailscaleCoord, asyncHandler,
|
||||||
|
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
|
||||||
|
}));
|
||||||
|
const res = await request(app)
|
||||||
|
.post('/api/v1/tailscale/settings/test')
|
||||||
|
.send({ apiToken: 'arbitrary-junk' });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(fakeClient.setApiToken).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('400 on body.apiToken exceeding length cap (DC-080)', async () => {
|
||||||
|
const fakeClient = makeFakeClient();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
const routes = require('../../routes/tailscale-admin');
|
||||||
|
const tailscaleCoord = {
|
||||||
|
loadMetadata: () => ({ configured: false }),
|
||||||
|
saveMetadata: jest.fn(),
|
||||||
|
setApiToken: jest.fn(),
|
||||||
|
getClient: jest.fn(async () => fakeClient),
|
||||||
|
hasApiToken: jest.fn(),
|
||||||
|
};
|
||||||
|
app.use('/api/v1/tailscale', routes({
|
||||||
|
tailscaleCoord, asyncHandler,
|
||||||
|
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
|
||||||
|
}));
|
||||||
|
const oversized = 'tskey-api-' + 'x'.repeat(300);
|
||||||
|
const res = await request(app)
|
||||||
|
.post('/api/v1/tailscale/settings/test')
|
||||||
|
.send({ apiToken: oversized });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(fakeClient.setApiToken).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('omitting apiToken is allowed (uses stored token path) (DC-080)', async () => {
|
||||||
|
const fakeClient = makeFakeClient({ ping: jest.fn(async () => ({ domain: 'stored.ts.net' })) });
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
const routes = require('../../routes/tailscale-admin');
|
||||||
|
const tailscaleCoord = {
|
||||||
|
loadMetadata: () => ({ configured: true }),
|
||||||
|
saveMetadata: jest.fn(),
|
||||||
|
setApiToken: jest.fn(),
|
||||||
|
getClient: jest.fn(async () => fakeClient),
|
||||||
|
hasApiToken: jest.fn(),
|
||||||
|
};
|
||||||
|
app.use('/api/v1/tailscale', routes({
|
||||||
|
tailscaleCoord, asyncHandler,
|
||||||
|
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
|
||||||
|
}));
|
||||||
|
const res = await request(app)
|
||||||
|
.post('/api/v1/tailscale/settings/test')
|
||||||
|
.send({}); // no apiToken in body
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(res.body.valid).toBe(true);
|
||||||
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
describe('routes/tailscale-admin: GET /admin/devices', () => {
|
describe('routes/tailscale-admin: GET /admin/devices', () => {
|
||||||
@@ -511,6 +595,99 @@ describe('routes/tailscale-admin: pre-auth keys', () => {
|
|||||||
expect(res.status).toBe(400);
|
expect(res.status).toBe(400);
|
||||||
});
|
});
|
||||||
|
|
||||||
|
test('POST /admin/keys rejects null/123/object tags entries (DC-080)', async () => {
|
||||||
|
const fakeClient = makeFakeClient();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
const routes = require('../../routes/tailscale-admin');
|
||||||
|
const tailscaleCoord = {
|
||||||
|
loadMetadata: () => ({ configured: true }),
|
||||||
|
saveMetadata: jest.fn(),
|
||||||
|
setApiToken: jest.fn(),
|
||||||
|
getClient: jest.fn(async () => fakeClient),
|
||||||
|
hasApiToken: jest.fn(),
|
||||||
|
};
|
||||||
|
app.use('/api/v1/tailscale', routes({
|
||||||
|
tailscaleCoord, asyncHandler,
|
||||||
|
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
|
||||||
|
}));
|
||||||
|
// Mixed: null, number, object — all must be rejected
|
||||||
|
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({ tags: ['tag:guest', null, 123, { x: 1 }] });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(fakeClient.createAuthKey).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('POST /admin/keys rejects uppercase / whitespace / CRLF in tags (DC-080)', async () => {
|
||||||
|
const fakeClient = makeFakeClient();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
const routes = require('../../routes/tailscale-admin');
|
||||||
|
const tailscaleCoord = {
|
||||||
|
loadMetadata: () => ({ configured: true }),
|
||||||
|
saveMetadata: jest.fn(),
|
||||||
|
setApiToken: jest.fn(),
|
||||||
|
getClient: jest.fn(async () => fakeClient),
|
||||||
|
hasApiToken: jest.fn(),
|
||||||
|
};
|
||||||
|
app.use('/api/v1/tailscale', routes({
|
||||||
|
tailscaleCoord, asyncHandler,
|
||||||
|
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
|
||||||
|
}));
|
||||||
|
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({ tags: ['TAG:guest', 'tag:foo bar', 'tag:x\r\ninjection'] });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(fakeClient.createAuthKey).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('POST /admin/keys rejects description exceeding 120 chars (DC-080)', async () => {
|
||||||
|
const fakeClient = makeFakeClient();
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
const routes = require('../../routes/tailscale-admin');
|
||||||
|
const tailscaleCoord = {
|
||||||
|
loadMetadata: () => ({ configured: true }),
|
||||||
|
saveMetadata: jest.fn(),
|
||||||
|
setApiToken: jest.fn(),
|
||||||
|
getClient: jest.fn(async () => fakeClient),
|
||||||
|
hasApiToken: jest.fn(),
|
||||||
|
};
|
||||||
|
app.use('/api/v1/tailscale', routes({
|
||||||
|
tailscaleCoord, asyncHandler,
|
||||||
|
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
|
||||||
|
}));
|
||||||
|
const longDesc = 'a'.repeat(200); // > 120 chars
|
||||||
|
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({ description: longDesc });
|
||||||
|
expect(res.status).toBe(400);
|
||||||
|
expect(fakeClient.createAuthKey).not.toHaveBeenCalled();
|
||||||
|
});
|
||||||
|
|
||||||
|
test('POST /admin/keys accepts canonical lowercase tag: form (DC-080)', async () => {
|
||||||
|
const fakeClient = makeFakeClient({
|
||||||
|
createAuthKey: jest.fn(async (opts) => ({ id: 'k2', key: 'tskey-secret-2', ...opts })),
|
||||||
|
});
|
||||||
|
const app = express();
|
||||||
|
app.use(express.json());
|
||||||
|
const routes = require('../../routes/tailscale-admin');
|
||||||
|
const tailscaleCoord = {
|
||||||
|
loadMetadata: () => ({ configured: true }),
|
||||||
|
saveMetadata: jest.fn(),
|
||||||
|
setApiToken: jest.fn(),
|
||||||
|
getClient: jest.fn(async () => fakeClient),
|
||||||
|
hasApiToken: jest.fn(),
|
||||||
|
};
|
||||||
|
app.use('/api/v1/tailscale', routes({
|
||||||
|
tailscaleCoord, asyncHandler,
|
||||||
|
log: { info: jest.fn(), error: jest.fn(), warn: jest.fn() },
|
||||||
|
}));
|
||||||
|
const res = await request(app).post('/api/v1/tailscale/admin/keys').send({
|
||||||
|
tags: ['tag:guest-plex', 'tag:server'],
|
||||||
|
expirySeconds: 86400,
|
||||||
|
});
|
||||||
|
expect(res.status).toBe(200);
|
||||||
|
expect(fakeClient.createAuthKey).toHaveBeenCalledWith(expect.objectContaining({
|
||||||
|
tags: ['tag:guest-plex', 'tag:server'],
|
||||||
|
}));
|
||||||
|
});
|
||||||
|
|
||||||
test('POST /admin/keys rejects negative expirySeconds', async () => {
|
test('POST /admin/keys rejects negative expirySeconds', async () => {
|
||||||
const fakeClient = makeFakeClient();
|
const fakeClient = makeFakeClient();
|
||||||
const app = express();
|
const app = express();
|
||||||
@@ -572,4 +749,110 @@ describe('routes/tailscale-admin: security boundary', () => {
|
|||||||
await request(app).delete('/api/v1/tailscale/settings');
|
await request(app).delete('/api/v1/tailscale/settings');
|
||||||
expect(stored.token).toBeNull();
|
expect(stored.token).toBeNull();
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
// DC-080 direct validator unit tests (no supertest, no Express)
|
||||||
|
describe('routes/tailscale-admin: DC-080 validators (direct)', () => {
|
||||||
|
const { _validators } = require('../../routes/tailscale-admin');
|
||||||
|
const {
|
||||||
|
validateApiToken,
|
||||||
|
validateTags,
|
||||||
|
validateDescription,
|
||||||
|
TAILSCALE_TOKEN_PREFIX,
|
||||||
|
TAILSCALE_TOKEN_MAX_LEN,
|
||||||
|
DESCRIPTION_MAX_LEN,
|
||||||
|
} = _validators;
|
||||||
|
|
||||||
|
describe('validateApiToken', () => {
|
||||||
|
test('accepts canonical tskey-api-...', () => {
|
||||||
|
expect(validateApiToken('tskey-api-abc123')).toBeNull();
|
||||||
|
});
|
||||||
|
test('rejects empty', () => {
|
||||||
|
expect(validateApiToken('')).toMatch(/required/);
|
||||||
|
});
|
||||||
|
test('rejects undefined / null', () => {
|
||||||
|
expect(validateApiToken(undefined)).toMatch(/required/);
|
||||||
|
expect(validateApiToken(null)).toMatch(/required/);
|
||||||
|
});
|
||||||
|
test('rejects non-string (number, object, array)', () => {
|
||||||
|
expect(validateApiToken(123)).toMatch(/must be a string/);
|
||||||
|
expect(validateApiToken({})).toMatch(/must be a string/);
|
||||||
|
expect(validateApiToken(['x'])).toMatch(/must be a string/);
|
||||||
|
});
|
||||||
|
test('rejects wrong prefix', () => {
|
||||||
|
expect(validateApiToken('not-a-token')).toMatch(/must start with/);
|
||||||
|
});
|
||||||
|
test('accepts exactly at length cap', () => {
|
||||||
|
const token = 'tskey-api-' + 'x'.repeat(TAILSCALE_TOKEN_MAX_LEN - 'tskey-api-'.length);
|
||||||
|
expect(validateApiToken(token)).toBeNull();
|
||||||
|
});
|
||||||
|
test('rejects 1 over length cap', () => {
|
||||||
|
const token = 'tskey-api-' + 'x'.repeat(TAILSCALE_TOKEN_MAX_LEN - 'tskey-api-'.length + 1);
|
||||||
|
expect(validateApiToken(token)).toMatch(/exceeds maximum length/);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('validateTags', () => {
|
||||||
|
test('accepts undefined / null (optional)', () => {
|
||||||
|
expect(validateTags(undefined)).toBeNull();
|
||||||
|
expect(validateTags(null)).toBeNull();
|
||||||
|
});
|
||||||
|
test('rejects non-array', () => {
|
||||||
|
expect(validateTags('tag:foo')).toMatch(/must be an array/);
|
||||||
|
expect(validateTags({})).toMatch(/must be an array/);
|
||||||
|
});
|
||||||
|
test('rejects entries that are not strings', () => {
|
||||||
|
expect(validateTags(['tag:a', null])).toMatch(/tags\[1\]/);
|
||||||
|
expect(validateTags(['tag:a', 123])).toMatch(/tags\[1\]/);
|
||||||
|
expect(validateTags(['tag:a', {}])).toMatch(/tags\[1\]/);
|
||||||
|
});
|
||||||
|
test('rejects uppercase / whitespace / CRLF', () => {
|
||||||
|
expect(validateTags(['TAG:foo'])).toMatch(/tags\[0\]/);
|
||||||
|
expect(validateTags(['tag:foo bar'])).toMatch(/tags\[0\]/);
|
||||||
|
expect(validateTags(['tag:foo\r\nbar'])).toMatch(/tags\[0\]/);
|
||||||
|
});
|
||||||
|
test('rejects entries starting with non-alnum (no leading colon)', () => {
|
||||||
|
expect(validateTags([':foo'])).toMatch(/tags\[0\]/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects bare "tag:" with empty name (Tailscale spec violation) (DC-080 round-2)', () => {
|
||||||
|
expect(validateTags(['tag:'])).toMatch(/tags\[0\]/);
|
||||||
|
});
|
||||||
|
|
||||||
|
test('rejects colon-only chars after tag: prefix (DC-080 round-2)', () => {
|
||||||
|
expect(validateTags(['tag:::'])).toMatch(/tags\[0\]/);
|
||||||
|
expect(validateTags(['tag:---'])).toMatch(/tags\[0\]/);
|
||||||
|
});
|
||||||
|
test('accepts canonical tag:server form', () => {
|
||||||
|
expect(validateTags(['tag:server'])).toBeNull();
|
||||||
|
expect(validateTags(['tag:guest-plex', 'tag:server'])).toBeNull();
|
||||||
|
});
|
||||||
|
test('rejects empty array entry', () => {
|
||||||
|
expect(validateTags(['tag:a', ''])).toMatch(/tags\[1\]/);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('validateDescription', () => {
|
||||||
|
test('accepts undefined / null', () => {
|
||||||
|
expect(validateDescription(undefined)).toBeNull();
|
||||||
|
expect(validateDescription(null)).toBeNull();
|
||||||
|
});
|
||||||
|
test('rejects non-string', () => {
|
||||||
|
expect(validateDescription(123)).toMatch(/must be a string/);
|
||||||
|
});
|
||||||
|
test('rejects over 120 chars', () => {
|
||||||
|
const long = 'a'.repeat(DESCRIPTION_MAX_LEN + 1);
|
||||||
|
expect(validateDescription(long)).toMatch(/exceeds maximum length/);
|
||||||
|
});
|
||||||
|
test('accepts at the cap', () => {
|
||||||
|
const exact = 'a'.repeat(DESCRIPTION_MAX_LEN);
|
||||||
|
expect(validateDescription(exact)).toBeNull();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
test('exports surface stays in sync with constants used inside validators', () => {
|
||||||
|
// Guard against drift: if a future refactor renames a constant, this fails
|
||||||
|
expect(TAILSCALE_TOKEN_PREFIX).toBe('tskey-api-');
|
||||||
|
expect(typeof TAILSCALE_TOKEN_MAX_LEN).toBe('number');
|
||||||
|
expect(typeof DESCRIPTION_MAX_LEN).toBe('number');
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
@@ -1,8 +1,103 @@
|
|||||||
|
/**
|
||||||
|
* DC-081: Plain-English log insights + dispose endpoint
|
||||||
|
*
|
||||||
|
* GET /api/v1/log-insights — Plain English summary of who's doing what
|
||||||
|
* POST /api/v1/log-insights/dispose — Preview then confirm cleanup
|
||||||
|
*
|
||||||
|
* DC-081 hardening (paired with the deploy path fix):
|
||||||
|
* - AUDIT_LOG_FILE / SECURITY_EVENT_LOG_FILE were HARDCODED to
|
||||||
|
* `/opt/dashcaddy/dashcaddy-api/data/...` which DOES NOT EXIST in the
|
||||||
|
* production container — files live at `/app/data/...`. The dispose
|
||||||
|
* endpoint silently no-op'd (read empty arrays, wrote empty arrays
|
||||||
|
* back) and the GET endpoint dropped the storage-size block. Both
|
||||||
|
* paths now use the same canonical resolution as the audit-logger
|
||||||
|
* itself: `process.env.AUDIT_LOG_FILE || path.join(platformPaths.dataDir, 'audit-log.json')`.
|
||||||
|
* - keepDays was unbounded — `parseInt(req.body.keepDays) || 30` accepted
|
||||||
|
* negative numbers (e.g. -1000 → cutoff = +3 years in the future,
|
||||||
|
* deleting 100% of forensic context) and non-integers (Infinity,
|
||||||
|
* floats). Now validated to an integer in [1, 3650] (1 day .. 10 years)
|
||||||
|
* before any file read.
|
||||||
|
* - confirm gate added: must send { confirm: true, keepDays: N } — the
|
||||||
|
* preview pass is read-only, the confirm pass writes. Matches the
|
||||||
|
* audit-logs/DELETE confirm=CLEAR pattern.
|
||||||
|
* - The dispose handler now uses a single shared `_resolvePaths()` helper
|
||||||
|
* to keep GET and POST in lockstep (and so a future path-config change
|
||||||
|
* touches one site, not four).
|
||||||
|
*
|
||||||
|
* Pre-DC-081 verification: from inside the running container, both
|
||||||
|
* `/app/data/audit-log.json` (318 KB) and `/app/data/security-events.jsonl`
|
||||||
|
* (15 MB) exist, but the old hardcoded `/opt/dashcaddy/dashcaddy-api/data/...`
|
||||||
|
* paths resolve to ENOENT. The dispose endpoint therefore did nothing;
|
||||||
|
* this fix wires it back to the actual files.
|
||||||
|
*/
|
||||||
|
|
||||||
const express = require('express');
|
const express = require('express');
|
||||||
|
const path = require('path');
|
||||||
const fs = require('fs').promises;
|
const fs = require('fs').promises;
|
||||||
|
const platformPaths = require('../platform-paths');
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Resolve the canonical paths for the audit log + security event log.
|
||||||
|
*
|
||||||
|
* Both store the file path in their own module-level constants, so any
|
||||||
|
* environment override (e.g. AUDIT_LOG_FILE=...) is honoured here too —
|
||||||
|
* exactly the same behaviour as src/security/audit-logger.js and
|
||||||
|
* src/security/event-store.js. Without this, a container with
|
||||||
|
* AUDIT_LOG_FILE set would see the dispose handler read from one file
|
||||||
|
* and the audit-logger write to a different one.
|
||||||
|
*
|
||||||
|
* @returns {{auditPath: string, secPath: string, auditPathFrom: string, secPathFrom: string}}
|
||||||
|
* paths + the source ("env" or "default") so tests can verify.
|
||||||
|
*/
|
||||||
|
function _resolvePaths() {
|
||||||
|
const auditPath = process.env.AUDIT_LOG_FILE
|
||||||
|
|| path.join(platformPaths.dataDir, 'audit-log.json');
|
||||||
|
const secPath = process.env.SECURITY_EVENT_LOG_FILE
|
||||||
|
|| path.join(platformPaths.dataDir, 'security-events.jsonl');
|
||||||
|
return {
|
||||||
|
auditPath,
|
||||||
|
secPath,
|
||||||
|
auditPathFrom: process.env.AUDIT_LOG_FILE ? 'env' : 'default',
|
||||||
|
secPathFrom: process.env.SECURITY_EVENT_LOG_FILE ? 'env' : 'default',
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validate the keepDays input. Coerces + bounds-checks BEFORE any file
|
||||||
|
* read so a malicious or mistyped client can't:
|
||||||
|
* - pass a negative number (cutoff = far future → wipe 100%)
|
||||||
|
* - pass Infinity (parseInt(Infinity, 10) === NaN, currently falls
|
||||||
|
* through `|| 30` — fixed to fail-fast instead)
|
||||||
|
* - pass a non-integer (e.g. 1.5 → cutoff mid-day, off-by-half-day)
|
||||||
|
* - pass 0 (no-op-but-lies) or 10000 (way past retention policy)
|
||||||
|
*
|
||||||
|
* @param {unknown} raw - value from req.body.keepDays
|
||||||
|
* @returns {number} validated integer in [1, 3650]
|
||||||
|
* @throws {Error} when out of range / wrong type
|
||||||
|
*/
|
||||||
|
function _validateKeepDays(raw) {
|
||||||
|
if (raw === undefined || raw === null) {
|
||||||
|
throw new Error('keepDays is required (integer in [1, 3650])');
|
||||||
|
}
|
||||||
|
const n = Number(raw);
|
||||||
|
if (!Number.isFinite(n)) {
|
||||||
|
throw new Error(`keepDays must be a finite number (received ${JSON.stringify(raw)})`);
|
||||||
|
}
|
||||||
|
if (!Number.isInteger(n)) {
|
||||||
|
throw new Error(`keepDays must be an integer (received ${raw})`);
|
||||||
|
}
|
||||||
|
if (n < 1 || n > 3650) {
|
||||||
|
throw new Error(`keepDays must be between 1 and 3650 (received ${n})`);
|
||||||
|
}
|
||||||
|
return n;
|
||||||
|
}
|
||||||
|
|
||||||
module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore }) {
|
module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore }) {
|
||||||
const router = express.Router();
|
const router = express.Router();
|
||||||
|
// Resolve once at module init so GET + POST both use the same files.
|
||||||
|
// If the env vars change at runtime (rare — start.sh wires them at
|
||||||
|
// container start), operators re-deploy rather than mutate env mid-flight.
|
||||||
|
const { auditPath, secPath } = _resolvePaths();
|
||||||
|
|
||||||
// GET /api/v1/log-insights — Plain English summary of who's doing what
|
// GET /api/v1/log-insights — Plain English summary of who's doing what
|
||||||
router.get('/log-insights', asyncHandler(async (req, res) => {
|
router.get('/log-insights', asyncHandler(async (req, res) => {
|
||||||
@@ -74,16 +169,18 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
|||||||
}
|
}
|
||||||
|
|
||||||
// --- Storage info ---
|
// --- Storage info ---
|
||||||
const auditPath = process.env.AUDIT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/audit-log.json';
|
// DC-081: read from the canonical resolved paths (NOT the hardcoded
|
||||||
const secPath = process.env.SECURITY_EVENT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl';
|
// /opt/... paths that don't exist in the container). Empty-object
|
||||||
|
// fallback on ENOENT — the file may legitimately be absent on a
|
||||||
|
// fresh install where the audit-logger hasn't written yet.
|
||||||
let storage = {};
|
let storage = {};
|
||||||
try {
|
try {
|
||||||
const a = await fs.stat(auditPath);
|
const a = await fs.stat(auditPath);
|
||||||
storage.auditLog = { sizeMB: +(a.size / 1048576).toFixed(2), entries: auditEntries.length };
|
storage.auditLog = { sizeMB: +(a.size / 1048576).toFixed(2), entries: auditEntries.length, path: auditPath };
|
||||||
} catch {}
|
} catch {}
|
||||||
try {
|
try {
|
||||||
const s = await fs.stat(secPath);
|
const s = await fs.stat(secPath);
|
||||||
storage.securityEvents = { sizeMB: +(s.size / 1048576).toFixed(2), entries: securityEvents.length };
|
storage.securityEvents = { sizeMB: +(s.size / 1048576).toFixed(2), entries: securityEvents.length, path: secPath };
|
||||||
} catch {}
|
} catch {}
|
||||||
|
|
||||||
ok(res, {
|
ok(res, {
|
||||||
@@ -108,16 +205,44 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
// POST /api/v1/log-insights/dispose — Preview then confirm cleanup
|
// POST /api/v1/log-insights/dispose — Preview then confirm cleanup
|
||||||
|
//
|
||||||
|
// Two-call pattern:
|
||||||
|
// 1. { keepDays: 30 } → preview, no writes
|
||||||
|
// 2. { keepDays: 30, confirm: true } → actually delete
|
||||||
|
//
|
||||||
|
// DC-081 hardening:
|
||||||
|
// - keepDays is validated to integer [1, 3650] BEFORE any file read.
|
||||||
|
// A negative keepDays (e.g. -1000) would previously compute a
|
||||||
|
// cutoff +3 years in the future, then delete every entry older
|
||||||
|
// than that — i.e. 100% of the audit log. Now rejected at the gate.
|
||||||
|
// - auditPath / secPath come from the canonical _resolvePaths() helper
|
||||||
|
// so the container's actual /app/data files are read (the pre-fix
|
||||||
|
// hardcoded /opt/dashcaddy/dashcaddy-api/data/... paths resolved to
|
||||||
|
// ENOENT inside the container, so the endpoint silently did nothing).
|
||||||
router.post('/log-insights/dispose', asyncHandler(async (req, res) => {
|
router.post('/log-insights/dispose', asyncHandler(async (req, res) => {
|
||||||
const keepDays = parseInt(req.body.keepDays) || 30;
|
// Validate keepDays first — fail-fast before any file IO so a bad
|
||||||
|
// client never touches disk.
|
||||||
|
let keepDays;
|
||||||
|
try {
|
||||||
|
keepDays = _validateKeepDays(req.body?.keepDays);
|
||||||
|
} catch (e) {
|
||||||
|
return res.status(400).json({ success: false, error: e.message, code: 'DC-081_INVALID_KEEP_DAYS' });
|
||||||
|
}
|
||||||
const confirm = req.body.confirm === true;
|
const confirm = req.body.confirm === true;
|
||||||
const cutoff = new Date(Date.now() - keepDays * 86400000).toISOString();
|
const cutoff = new Date(Date.now() - keepDays * 86400000).toISOString();
|
||||||
|
|
||||||
const auditPath = process.env.AUDIT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/audit-log.json';
|
// Read both files via the canonical resolved paths (NOT the hardcoded
|
||||||
const secPath = process.env.SECURITY_EVENT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl';
|
// /opt/... paths from before — those don't exist in the container).
|
||||||
|
|
||||||
const auditRaw = await fs.readFile(auditPath, 'utf8').catch(function () { return '[]'; });
|
const auditRaw = await fs.readFile(auditPath, 'utf8').catch(function () { return '[]'; });
|
||||||
const auditData = JSON.parse(auditRaw);
|
let auditData;
|
||||||
|
try {
|
||||||
|
auditData = JSON.parse(auditRaw);
|
||||||
|
} catch (e) {
|
||||||
|
return res.status(500).json({ success: false, error: `audit-log file is corrupt (${auditPath}): ${e.message}`, code: 'DC-081_AUDIT_PARSE_FAILED' });
|
||||||
|
}
|
||||||
|
if (!Array.isArray(auditData)) {
|
||||||
|
return res.status(500).json({ success: false, error: `audit-log file is not an array (${auditPath})`, code: 'DC-081_AUDIT_SHAPE_INVALID' });
|
||||||
|
}
|
||||||
const oldAudit = auditData.filter(function (e) { return e.timestamp < cutoff; });
|
const oldAudit = auditData.filter(function (e) { return e.timestamp < cutoff; });
|
||||||
|
|
||||||
const secRaw = await fs.readFile(secPath, 'utf8').catch(function () { return ''; });
|
const secRaw = await fs.readFile(secPath, 'utf8').catch(function () { return ''; });
|
||||||
@@ -127,16 +252,40 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
|||||||
if (!confirm) {
|
if (!confirm) {
|
||||||
ok(res, {
|
ok(res, {
|
||||||
preview: true,
|
preview: true,
|
||||||
message: 'This will delete ' + oldAudit.length + ' audit entries and ' + oldSec.length + ' security events older than ' + keepDays + ' days. Send {confirm: true} to proceed.',
|
message: 'This will delete ' + oldAudit.length + ' audit entries and ' + oldSec.length + ' security events older than ' + keepDays + ' days. Send {confirm: true, keepDays: ' + keepDays + '} to proceed.',
|
||||||
wouldDelete: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
|
wouldDelete: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
|
||||||
cutoffDate: cutoff
|
cutoffDate: cutoff,
|
||||||
|
paths: { auditPath, secPath },
|
||||||
});
|
});
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Execute cleanup
|
// Execute cleanup. Audit the wipe FIRST via the audit-logger so the
|
||||||
|
// fact that a delete happened is itself preserved (matches the
|
||||||
|
// audit-logs/DELETE + error-logs/DELETE pattern).
|
||||||
|
try {
|
||||||
|
if (auditLogger && typeof auditLogger.log === 'function') {
|
||||||
|
await auditLogger.log({
|
||||||
|
action: 'log-insights.dispose',
|
||||||
|
resource: 'audit-log,security-events',
|
||||||
|
outcome: 'success',
|
||||||
|
details: {
|
||||||
|
keepDays,
|
||||||
|
cutoff,
|
||||||
|
deleted: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
|
||||||
|
},
|
||||||
|
});
|
||||||
|
}
|
||||||
|
} catch { /* don't fail the dispose on audit-side errors */ }
|
||||||
|
|
||||||
|
// Rewrite audit-log.json atomically — write to tmp + rename so a
|
||||||
|
// crash mid-write can't leave the file half-empty (the file is read
|
||||||
|
// by state-manager on every container start; a corrupt file would
|
||||||
|
// block the whole API).
|
||||||
const keptAudit = auditData.filter(function (e) { return e.timestamp >= cutoff; });
|
const keptAudit = auditData.filter(function (e) { return e.timestamp >= cutoff; });
|
||||||
await fs.writeFile(auditPath, JSON.stringify(keptAudit, null, 2));
|
const tmpAudit = auditPath + '.tmp';
|
||||||
|
await fs.writeFile(tmpAudit, JSON.stringify(keptAudit, null, 2));
|
||||||
|
await fs.rename(tmpAudit, auditPath);
|
||||||
|
|
||||||
const keptSec = secLines.filter(function (l) { try { return JSON.parse(l).timestamp >= cutoff; } catch (e) { return false; } });
|
const keptSec = secLines.filter(function (l) { try { return JSON.parse(l).timestamp >= cutoff; } catch (e) { return false; } });
|
||||||
await fs.writeFile(secPath, keptSec.join('\n') + '\n');
|
await fs.writeFile(secPath, keptSec.join('\n') + '\n');
|
||||||
@@ -145,9 +294,16 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
|||||||
disposed: true,
|
disposed: true,
|
||||||
deleted: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
|
deleted: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
|
||||||
remaining: { auditEntries: keptAudit.length, securityEvents: keptSec.length },
|
remaining: { auditEntries: keptAudit.length, securityEvents: keptSec.length },
|
||||||
cutoffDate: cutoff
|
cutoffDate: cutoff,
|
||||||
});
|
});
|
||||||
}));
|
}));
|
||||||
|
|
||||||
return router;
|
return router;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// DC-081: export helpers for direct unit testing (the route handlers are
|
||||||
|
// otherwise unreachable from outside the factory closure).
|
||||||
|
module.exports.__test = {
|
||||||
|
_resolvePaths,
|
||||||
|
_validateKeepDays,
|
||||||
|
};
|
||||||
@@ -41,12 +41,124 @@
|
|||||||
*
|
*
|
||||||
* DELETE /api/v1/tailscale/admin/devices/:id
|
* DELETE /api/v1/tailscale/admin/devices/:id
|
||||||
* Revokes a device from the tailnet.
|
* Revokes a device from the tailnet.
|
||||||
|
*
|
||||||
|
* # DC-080 input validation
|
||||||
|
*
|
||||||
|
* Three coupled gaps in the route layer pre-fix:
|
||||||
|
*
|
||||||
|
* (a) PUT /settings validated `apiToken.startsWith('tskey-api-')` but had
|
||||||
|
* no length cap — body-parser limit was the only ceiling. A 1 MB
|
||||||
|
* string starting with `tskey-api-` would be `.trim()`-ed, sent to
|
||||||
|
* Tailscale's /devices endpoint, and waste server-side CPU on a
|
||||||
|
* request that will always 401.
|
||||||
|
* (b) POST /settings/test accepted `apiToken` from the body with NO
|
||||||
|
* validation at all. The PUT route's prefix check is bypassed on
|
||||||
|
* the test path — an operator could submit any string and have the
|
||||||
|
* container ping Tailscale's API with it (low impact, but inconsistent
|
||||||
|
* with PUT and surfaces fingerprinting via the 401 timing).
|
||||||
|
* (c) POST /admin/keys validated `tags` as Array but NOT per-element
|
||||||
|
* type — `tags: ['tag:guest', null, 123, {injection: true}]` would be
|
||||||
|
* forwarded to Tailscale verbatim. Tailscale's API is JSON-strict
|
||||||
|
* and would 400 the request, but the bad shape reached the wire.
|
||||||
|
* Similarly `description` had no length cap (Tailscale caps at 120
|
||||||
|
* chars per their docs).
|
||||||
|
*
|
||||||
|
* All three are gated by TOTP — this is a logged-in-operator / phished-
|
||||||
|
* session threat surface, not anonymous-unauth. The fix is defense-in-
|
||||||
|
* depth: a bug in the auth path (TOTP bypass, session theft, future
|
||||||
|
* route handler trust-boundary drift) should not turn these endpoints
|
||||||
|
* into a "submit anything and forward to Tailscale" relay.
|
||||||
*/
|
*/
|
||||||
|
|
||||||
const express = require('express');
|
const express = require('express');
|
||||||
const { ok, errorResponse } = require('../src/utils/responses');
|
const { ok, errorResponse } = require('../src/utils/responses');
|
||||||
const { TailscaleCoordError } = require('../src/managers/tailscale-coord');
|
const { TailscaleCoordError } = require('../src/managers/tailscale-coord');
|
||||||
|
|
||||||
|
// DC-080: shared validation helpers for the Tailscale admin surface.
|
||||||
|
// Tailscale API tokens follow the form `tskey-<kind>-<opaque>` where
|
||||||
|
// `<kind>` is one of a small set of values (`api`, `auth`, `partner`,
|
||||||
|
// `cli`). Real tokens observed in the wild are 40..80 chars; we cap at
|
||||||
|
// 256 to leave headroom for future Tailscale key formats without giving
|
||||||
|
// an unbounded buffer to validate+forward.
|
||||||
|
const TAILSCALE_TOKEN_PREFIX = 'tskey-api-';
|
||||||
|
const TAILSCALE_TOKEN_MAX_LEN = 256;
|
||||||
|
const TAG_KEY_MAX_LEN = 64;
|
||||||
|
const TAGS_MAX_LEN = 32;
|
||||||
|
const DESCRIPTION_MAX_LEN = 120;
|
||||||
|
|
||||||
|
// Tailscale tags are lowercased identifiers with optional colons
|
||||||
|
// (e.g. `tag:server`, `tag:guest-plex`). Reject whitespace, CR/LF,
|
||||||
|
// control chars, JSON metacharacters, and any character that could
|
||||||
|
// enable header-injection through the Tailscale coord client.
|
||||||
|
//
|
||||||
|
// DC-080 round-2 polish: Tailscale's tag spec requires `tag:` followed by
|
||||||
|
// ≥1 identifier char — bare `tag:` (empty name) is rejected by their API.
|
||||||
|
// We split the pattern in two so the error message names which form failed
|
||||||
|
// instead of dumping a generic regex.
|
||||||
|
const TAG_KEY_RE = /^tag:[a-z0-9][a-z0-9_-]{0,62}$/;
|
||||||
|
|
||||||
|
function _validateApiToken(token, fieldName = 'apiToken') {
|
||||||
|
if (typeof token !== 'string' || !token) {
|
||||||
|
return `${fieldName} is required and must be a string`;
|
||||||
|
}
|
||||||
|
if (!token.startsWith(TAILSCALE_TOKEN_PREFIX)) {
|
||||||
|
return `${fieldName} must start with ${TAILSCALE_TOKEN_PREFIX}`;
|
||||||
|
}
|
||||||
|
if (token.length > TAILSCALE_TOKEN_MAX_LEN) {
|
||||||
|
return `${fieldName} exceeds maximum length of ${TAILSCALE_TOKEN_MAX_LEN} characters`;
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function _validateTags(tags) {
|
||||||
|
if (tags === undefined || tags === null) return null;
|
||||||
|
if (!Array.isArray(tags)) {
|
||||||
|
return 'tags must be an array of strings';
|
||||||
|
}
|
||||||
|
if (tags.length > TAGS_MAX_LEN) {
|
||||||
|
return `tags exceeds maximum length of ${TAGS_MAX_LEN} entries`;
|
||||||
|
}
|
||||||
|
for (let i = 0; i < tags.length; i += 1) {
|
||||||
|
const t = tags[i];
|
||||||
|
if (typeof t !== 'string' || !t) {
|
||||||
|
return `tags[${i}] must be a non-empty string`;
|
||||||
|
}
|
||||||
|
if (t.length > TAG_KEY_MAX_LEN) {
|
||||||
|
return `tags[${i}] exceeds maximum length of ${TAG_KEY_MAX_LEN} characters`;
|
||||||
|
}
|
||||||
|
if (!TAG_KEY_RE.test(t)) {
|
||||||
|
return `tags[${i}] must match ${TAG_KEY_RE} (lowercase alnum + :_-)`;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
function _validateDescription(description) {
|
||||||
|
if (description === undefined || description === null) return null;
|
||||||
|
if (typeof description !== 'string') {
|
||||||
|
return 'description must be a string';
|
||||||
|
}
|
||||||
|
if (description.length > DESCRIPTION_MAX_LEN) {
|
||||||
|
return `description exceeds maximum length of ${DESCRIPTION_MAX_LEN} characters`;
|
||||||
|
}
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Exported for direct unit testing in __tests__/routes/tailscale-admin.test.js
|
||||||
|
// (the validator functions are otherwise unreachable from outside the factory
|
||||||
|
// closure; direct tests assert edge cases without supertest overhead).
|
||||||
|
const _validators = {
|
||||||
|
validateApiToken: _validateApiToken,
|
||||||
|
validateTags: _validateTags,
|
||||||
|
validateDescription: _validateDescription,
|
||||||
|
TAILSCALE_TOKEN_PREFIX,
|
||||||
|
TAILSCALE_TOKEN_MAX_LEN,
|
||||||
|
TAG_KEY_MAX_LEN,
|
||||||
|
TAGS_MAX_LEN,
|
||||||
|
DESCRIPTION_MAX_LEN,
|
||||||
|
TAG_KEY_RE,
|
||||||
|
};
|
||||||
|
|
||||||
module.exports = function({
|
module.exports = function({
|
||||||
tailscaleCoord,
|
tailscaleCoord,
|
||||||
asyncHandler,
|
asyncHandler,
|
||||||
@@ -75,9 +187,12 @@ module.exports = function({
|
|||||||
|
|
||||||
router.put('/settings', asyncHandler(async (req, res) => {
|
router.put('/settings', asyncHandler(async (req, res) => {
|
||||||
const token = req.body && req.body.apiToken;
|
const token = req.body && req.body.apiToken;
|
||||||
if (!token || typeof token !== 'string' || !token.startsWith('tskey-api-')) {
|
// DC-080: validate prefix + length cap. The pre-fix code only checked
|
||||||
return errorResponse(res, 400, 'Invalid API token (must start with tskey-api-)');
|
// the prefix — a 1 MB string starting with `tskey-api-` would have been
|
||||||
}
|
// sent to Tailscale's /devices endpoint and wasted server-side CPU
|
||||||
|
// before the inevitable 401.
|
||||||
|
const tokenErr = _validateApiToken(token);
|
||||||
|
if (tokenErr) return errorResponse(res, 400, tokenErr);
|
||||||
|
|
||||||
// Validate before storing
|
// Validate before storing
|
||||||
const client = new (require('../src/managers/tailscale-coord').TailscaleCoordClient)({ apiToken: token });
|
const client = new (require('../src/managers/tailscale-coord').TailscaleCoordClient)({ apiToken: token });
|
||||||
@@ -130,6 +245,17 @@ module.exports = function({
|
|||||||
|
|
||||||
router.post('/settings/test', asyncHandler(async (req, res) => {
|
router.post('/settings/test', asyncHandler(async (req, res) => {
|
||||||
const token = (req.body && req.body.apiToken) || null;
|
const token = (req.body && req.body.apiToken) || null;
|
||||||
|
// DC-080: validate any caller-provided token before it reaches the
|
||||||
|
// Tailscale API. Pre-fix the test endpoint accepted any string — the
|
||||||
|
// PUT route's prefix check did NOT extend to this path. An operator
|
||||||
|
// could submit arbitrary junk and the container would still call
|
||||||
|
// /devices on the Tailscale API with it (DoS-reflection + fingerprint
|
||||||
|
// timing for a future attacker probing whether this API token format
|
||||||
|
// is accepted at all).
|
||||||
|
if (token !== null && token !== undefined) {
|
||||||
|
const tokenErr = _validateApiToken(token);
|
||||||
|
if (tokenErr) return errorResponse(res, 400, tokenErr);
|
||||||
|
}
|
||||||
const client = await tailscaleCoord.getClient();
|
const client = await tailscaleCoord.getClient();
|
||||||
if (token) {
|
if (token) {
|
||||||
// Caller provided a fresh token to test — don't save it
|
// Caller provided a fresh token to test — don't save it
|
||||||
@@ -214,10 +340,16 @@ module.exports = function({
|
|||||||
return errorResponse(res, 503, 'Tailscale API token not configured');
|
return errorResponse(res, 503, 'Tailscale API token not configured');
|
||||||
}
|
}
|
||||||
const opts = req.body || {};
|
const opts = req.body || {};
|
||||||
// Reject obviously-bad input early
|
// Reject obviously-bad input early.
|
||||||
if (opts.tags && !Array.isArray(opts.tags)) {
|
// DC-080: pre-fix the route only checked `Array.isArray(opts.tags)`.
|
||||||
return errorResponse(res, 400, 'tags must be an array of strings');
|
// A `tags: ['tag:guest', null, 123, {injection: true}]` payload would
|
||||||
}
|
// be forwarded to Tailscale verbatim — Tailscale's API is JSON-strict
|
||||||
|
// and would 400 the request, but the bad shape reached the wire and
|
||||||
|
// would silently pass through the dashboard's JSON.stringify() flow.
|
||||||
|
const tagsErr = _validateTags(opts.tags);
|
||||||
|
if (tagsErr) return errorResponse(res, 400, tagsErr);
|
||||||
|
const descErr = _validateDescription(opts.description);
|
||||||
|
if (descErr) return errorResponse(res, 400, descErr);
|
||||||
if (opts.expirySeconds !== undefined && (!Number.isInteger(opts.expirySeconds) || opts.expirySeconds <= 0)) {
|
if (opts.expirySeconds !== undefined && (!Number.isInteger(opts.expirySeconds) || opts.expirySeconds <= 0)) {
|
||||||
return errorResponse(res, 400, 'expirySeconds must be a positive integer');
|
return errorResponse(res, 400, 'expirySeconds must be a positive integer');
|
||||||
}
|
}
|
||||||
@@ -254,4 +386,10 @@ module.exports = function({
|
|||||||
}));
|
}));
|
||||||
|
|
||||||
return router;
|
return router;
|
||||||
};
|
};
|
||||||
|
|
||||||
|
// DC-080: validators exported for direct unit testing in
|
||||||
|
// __tests__/routes/tailscale-admin.test.js — the route factory closes
|
||||||
|
// over the same functions, so the validators are exercised end-to-end via
|
||||||
|
// supertest AND in isolation here.
|
||||||
|
module.exports._validators = _validators;
|
||||||
Reference in New Issue
Block a user