Compare commits
6
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
7e68955e66 | ||
|
|
089f5d2902 | ||
|
|
0e7bb97129 | ||
|
|
98737995a9 | ||
|
|
99ec6ebc53 | ||
|
|
a7260436d1 |
@@ -18,7 +18,11 @@ function createDiscoverApp(docker, servicesStateManager) {
|
||||
|
||||
function createDisasterApp(platformPaths, log) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
// Match the production body-parser limit (1 MiB) so the in-handler
|
||||
// DC-079 cap (512 KiB) is actually reachable from tests. The default
|
||||
// express.json() limit is 100 KiB, which would short-circuit the test
|
||||
// with a 413 before the route's defense-in-depth check runs.
|
||||
app.use(express.json({ limit: '1mb' }));
|
||||
const routes = require('../../routes/disaster-recovery');
|
||||
const wrap = (fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next);
|
||||
app.use('/api/v1', routes({ platformPaths, log: log || { info: jest.fn(), error: jest.fn() }, asyncHandler: wrap }));
|
||||
@@ -135,4 +139,267 @@ describe('DC-107: Disaster Recovery', () => {
|
||||
const svc = JSON.parse(fs.readFileSync(path.join(tmpDir, 'services.json'), 'utf8'));
|
||||
expect(svc[0].id).toBe('restored-svc');
|
||||
});
|
||||
|
||||
// DC-079: Caddyfile restore hardening — the live Caddyfile path must
|
||||
// NEVER be written from the disaster-recovery endpoint. The endpoint
|
||||
// stages the candidate file under dataDir/disaster-staged/Caddyfile.candidate
|
||||
// and surfaces a warning that `caddy-apply` is required to apply it.
|
||||
it('DC-079: POST /disaster/restore with caddyfile STAGES instead of writing the live Caddyfile', async () => {
|
||||
// The env var CADDYFILE_PATH is read by the route. Use a sentinel
|
||||
// path that we can prove was NOT written. The route must instead
|
||||
// create <dataDir>/disaster-staged/Caddyfile.candidate.
|
||||
const liveSentinel = path.join(tmpDir, 'LIVE_CADDYFILE_SENTINEL.txt');
|
||||
fs.writeFileSync(liveSentinel, 'do-not-overwrite');
|
||||
|
||||
const candidateCaddyfile =
|
||||
'# staged candidate\n' +
|
||||
'example.com {\n' +
|
||||
' respond "ok"\n' +
|
||||
'}\n';
|
||||
|
||||
const app = createDisasterApp({
|
||||
dataDir: tmpDir,
|
||||
caddyfilePath: liveSentinel, // route reads env or fallback; this is just for the response
|
||||
});
|
||||
// Override process.env.CADDYFILE_PATH so the route picks up our sentinel
|
||||
const prev = process.env.CADDYFILE_PATH;
|
||||
process.env.CADDYFILE_PATH = liveSentinel;
|
||||
try {
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
caddyfile: candidateCaddyfile,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.status).toBe('success');
|
||||
expect(res.body.caddyfileStaged).toBeTruthy();
|
||||
expect(res.body.caddyfileStaged).toHaveLength(1);
|
||||
expect(res.body.caddyfileStaged[0].file).toBe('Caddyfile');
|
||||
expect(res.body.caddyfileStaged[0].action).toBe('awaiting caddy-apply');
|
||||
expect(res.body.caddyfileStaged[0].stagedPath).toBe(
|
||||
path.join(tmpDir, 'disaster-staged', 'Caddyfile.candidate')
|
||||
);
|
||||
expect(res.body.caddyfileStaged[0].livePath).toBe(liveSentinel);
|
||||
expect(res.body.warning).toMatch(/DC-079/);
|
||||
|
||||
// The live sentinel file is UNTOUCHED — still has its original content.
|
||||
const liveContents = fs.readFileSync(liveSentinel, 'utf8');
|
||||
expect(liveContents).toBe('do-not-overwrite');
|
||||
|
||||
// The candidate file IS staged at the staging path.
|
||||
const stagedContents = fs.readFileSync(
|
||||
path.join(tmpDir, 'disaster-staged', 'Caddyfile.candidate'),
|
||||
'utf8'
|
||||
);
|
||||
expect(stagedContents).toBe(candidateCaddyfile);
|
||||
} finally {
|
||||
if (prev === undefined) delete process.env.CADDYFILE_PATH;
|
||||
else process.env.CADDYFILE_PATH = prev;
|
||||
}
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects non-string caddyfile content', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
caddyfile: { evil: 'object' },
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/Caddyfile content must be a string/);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects explicit empty caddyfile string', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
caddyfile: '', // explicit empty payload — rejected
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/Caddyfile content is empty/);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects oversized caddyfile content', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
// 512 KiB + 1 byte — over the in-handler cap, under the 1 MB body limit
|
||||
const huge = 'a'.repeat(512 * 1024 + 1);
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
caddyfile: huge,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/exceeds 524288 bytes/);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects forbidden `import` directive (absolute path)', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const evil =
|
||||
'# malicious snapshot\n' +
|
||||
'import /etc/caddy/external.caddy\n' +
|
||||
'example.com { respond "ok" }\n';
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
caddyfile: evil,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/forbidden `import` directive/);
|
||||
|
||||
// No staging file should have been created — fail closed.
|
||||
expect(fs.existsSync(path.join(tmpDir, 'disaster-staged', 'Caddyfile.candidate'))).toBe(false);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects forbidden `import` with relative-path escape', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const evil =
|
||||
'# malicious snapshot\n' +
|
||||
'import ../../../etc/passwd\n' +
|
||||
'example.com { respond "ok" }\n';
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
caddyfile: evil,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/forbidden `import` directive/);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects URL-encoded import payload', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const evil =
|
||||
'import %2fetc%2fcaddy%2fevil.caddy\n' +
|
||||
'example.com { respond "ok" }\n';
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
caddyfile: evil,
|
||||
});
|
||||
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/forbidden `import` directive/);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore without caddyfile field succeeds and stages nothing', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
files: {
|
||||
services: [{ id: 'no-caddy' }],
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.caddyfileStaged).toBeUndefined();
|
||||
expect(res.body.warning).toBeUndefined();
|
||||
});
|
||||
|
||||
// DC-079 follow-up (GLM round-2 BLOCKING): assets/themes path traversal.
|
||||
// Without the assertSafeAssetKey / assertSafeThemeName + path.resolve
|
||||
// checks, an attacker can POST `{assets: {"../../etc/caddy/Caddyfile":
|
||||
// "<base64-evil>"}}` and overwrite the live Caddyfile via the dataDir
|
||||
// bind-mount. These tests prove the fix.
|
||||
it('DC-079: POST /disaster/restore rejects assets with path-traversal key', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
assets: {
|
||||
'../../etc/caddy/Caddyfile': Buffer.from('EVIL_BASE64_PAYLOAD').toString('base64'),
|
||||
'custom-logo.png': Buffer.from('legit-logo').toString('base64'),
|
||||
},
|
||||
});
|
||||
|
||||
// The traversal key is rejected (added to errors), the legit key
|
||||
// still works. Status is success-or-partial, never 500.
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.status).toBe('partial'); // one error
|
||||
const erroredFile = res.body.errors.find(e => e.file && e.file.includes('../../etc/caddy/Caddyfile'));
|
||||
expect(erroredFile).toBeTruthy();
|
||||
expect(erroredFile.error).toMatch(/forbidden characters or path segments/);
|
||||
|
||||
// The legit logo DID get written.
|
||||
const legitPath = path.join(tmpDir, 'assets', 'custom-logo.png');
|
||||
expect(fs.existsSync(legitPath)).toBe(true);
|
||||
|
||||
// The traversal target was NEVER written.
|
||||
const escapePath = path.join(tmpDir, 'assets', '../../etc/caddy/Caddyfile');
|
||||
// Resolve to absolute path — should be outside tmpDir/assets.
|
||||
const resolvedEsc = path.resolve(escapePath);
|
||||
expect(fs.existsSync(resolvedEsc)).toBe(false);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects assets with absolute path key', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
assets: {
|
||||
'/etc/passwd': Buffer.from('evil').toString('base64'),
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.status).toBe('partial');
|
||||
const erroredFile = res.body.errors.find(e => e.file && e.file.includes('/etc/passwd'));
|
||||
expect(erroredFile).toBeTruthy();
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects themes with path-traversal name', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
themes: {
|
||||
'../../../etc/caddy/evil.json': { evil: true },
|
||||
'legit-theme.json': { ok: true },
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.status).toBe('partial');
|
||||
const erroredFile = res.body.errors.find(e => e.file && e.file.includes('../../../etc/caddy/evil.json'));
|
||||
expect(erroredFile).toBeTruthy();
|
||||
expect(erroredFile.error).toMatch(/must match/);
|
||||
|
||||
// The legit theme DID get written.
|
||||
expect(fs.existsSync(path.join(tmpDir, 'themes', 'legit-theme.json'))).toBe(true);
|
||||
});
|
||||
|
||||
it('DC-079: POST /disaster/restore rejects themes without .json extension', async () => {
|
||||
const app = createDisasterApp({ dataDir: tmpDir });
|
||||
const res = await request(app)
|
||||
.post('/api/v1/disaster/restore')
|
||||
.send({
|
||||
version: '1.0',
|
||||
themes: {
|
||||
'no-extension': { ok: true },
|
||||
},
|
||||
});
|
||||
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.status).toBe('partial');
|
||||
const erroredFile = res.body.errors.find(e => e.file && e.file.includes('no-extension'));
|
||||
expect(erroredFile).toBeTruthy();
|
||||
expect(erroredFile.error).toMatch(/must match/);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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);
|
||||
});
|
||||
|
||||
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 () => {
|
||||
const fakeClient = makeFakeClient({
|
||||
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(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', () => {
|
||||
@@ -511,6 +595,99 @@ describe('routes/tailscale-admin: pre-auth keys', () => {
|
||||
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 () => {
|
||||
const fakeClient = makeFakeClient();
|
||||
const app = express();
|
||||
@@ -572,4 +749,110 @@ describe('routes/tailscale-admin: security boundary', () => {
|
||||
await request(app).delete('/api/v1/tailscale/settings');
|
||||
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');
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,483 @@
|
||||
/**
|
||||
* DC-083 -- Public share endpoint input hardening.
|
||||
*
|
||||
* The two CSRF-exempt public endpoints (POST /share/:token/subscribe +
|
||||
* POST /share/:token/redeem-tailscale) accept untrusted body fields. The
|
||||
* pre-fix code had three coupled bugs:
|
||||
*
|
||||
* 1. `email.includes('@')` accepted `@`, `a@`, `<script>@x.c`, and 10MB
|
||||
* strings as "valid email" -- and the field was never even used after
|
||||
* validation (the subscribe endpoint discarded it).
|
||||
* 2. `typeof deviceId === 'string'` accepted arbitrary strings of any
|
||||
* length, including CR/LF/NUL -- which fed straight into the Tailscale
|
||||
* auth-key description string and the on-disk shares.json.
|
||||
* 3. No rate-limit; the general limiter (1000/15min) was too generous for
|
||||
* unauthenticated state-mutating endpoints.
|
||||
*
|
||||
* Fix: charset/length/control-char-bounded validators at the route layer
|
||||
* AND at the store layer (defense-in-depth), plus a dedicated
|
||||
* SHARE_PUBLIC rate-limit (30/15min) on the public endpoints.
|
||||
*
|
||||
* Coverage:
|
||||
* - subscribe email: rejects bare @, missing TLD, oversized, CR/LF, shell
|
||||
* metachars, control chars; accepts normal addresses; accepts OMITTED
|
||||
* email (backwards-compatible with the original behavior).
|
||||
* - subscribe email propagates to share-store subscriberEmails (capped 8).
|
||||
* - redeem-tailscale deviceId: rejects CR/LF/NUL, oversized, empty,
|
||||
* spaces, brackets, quotes; accepts Tailscale-style base64url+hphens;
|
||||
* accepts OMITTED deviceId (treated as 'unknown').
|
||||
* - Sanitized usedBy is what flows into the on-disk shares.json.
|
||||
* - Rate-limit fires after the configured budget per IP.
|
||||
* - Store-level defense: bypassing the route (direct store call) still
|
||||
* rejects invalid inputs.
|
||||
*/
|
||||
|
||||
'use strict';
|
||||
|
||||
const fs = require('fs');
|
||||
const path = require('path');
|
||||
const os = require('os');
|
||||
const express = require('express');
|
||||
const request = require('supertest');
|
||||
const { createShareStore } = require('../src/security/share-store');
|
||||
|
||||
function _tmpDir() {
|
||||
return fs.mkdtempSync(path.join(os.tmpdir(), 'dashcaddy-share-dc083-'));
|
||||
}
|
||||
function _cleanup(dir) {
|
||||
try { fs.rmSync(dir, { recursive: true, force: true }); } catch {}
|
||||
}
|
||||
|
||||
function _buildApp({ shareStore } = {}) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
// No req.user injection -- the public endpoints must work without auth.
|
||||
const shareRoutes = require('../routes/share');
|
||||
app.use(shareRoutes({
|
||||
shareStore,
|
||||
licenseManager: { isPro: () => true, allowsLifetimeLicense: () => false },
|
||||
tailscaleCoord: { createAuthKey: async () => ({ id: 'k', key: 'tskey-x' }) },
|
||||
notificationManager: { sendEmail: async () => ({ messageId: 'fake' }) },
|
||||
servicesStateManager: { get: async () => null, read: async () => [] },
|
||||
servicesFile: null,
|
||||
asyncHandler: (fn, _label) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next),
|
||||
log: { info() {}, warn() {}, error() {} },
|
||||
}));
|
||||
app.use((err, _req, res, _next) => {
|
||||
if (err && err.statusCode) {
|
||||
return res.status(err.statusCode).json({
|
||||
success: false,
|
||||
error: err.message,
|
||||
code: err.code,
|
||||
});
|
||||
}
|
||||
return res.status(500).json({ success: false, error: err && err.message });
|
||||
});
|
||||
return app;
|
||||
}
|
||||
|
||||
// --------- Subscribe endpoint -- email validation ---------------------------------------------------------------------------------------
|
||||
|
||||
describe('DC-083: subscribe email validation', () => {
|
||||
let dir, shareStore;
|
||||
beforeEach(() => { dir = _tmpDir(); shareStore = createShareStore({ dataDir: dir }); });
|
||||
afterEach(() => _cleanup(dir));
|
||||
|
||||
test('accepts omitted email (backwards-compatible)', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app).post(`/share/${issued.token}/subscribe`).send({});
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.data.count).toBe(1);
|
||||
});
|
||||
|
||||
test('accepts a well-formed email', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: 'subscriber@example.com' });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.data.count).toBe(1);
|
||||
});
|
||||
|
||||
test('lowercases the email on capture', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: 'Subscriber@Example.COM' });
|
||||
expect(res.status).toBe(200);
|
||||
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
|
||||
const id = Object.keys(raw.shares)[0];
|
||||
expect(raw.shares[id].subscriberEmails).toEqual(['subscriber@example.com']);
|
||||
});
|
||||
|
||||
test('rejects bare @', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: '@' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects missing local-part', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: '@example.com' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects missing TLD', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: 'user@localhost' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects single-char TLD', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: 'user@example.c' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects CR/LF in email (CRLF-injection defense)', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: 'a@b.com\r\nX-Injected: yes' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects NUL in email', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: 'a@b.com\x00hack' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects oversized email (>254 chars)', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const longLocal = 'a'.repeat(250) + '@example.com';
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: longLocal });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects XSS-shape email', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: '<script>@x.com' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects non-string email', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: 42 });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('keeps subscriberEmails capped to 8 entries', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
for (let i = 0; i < 12; i++) {
|
||||
await request(app)
|
||||
.post(`/share/${issued.token}/subscribe`)
|
||||
.send({ email: `user${i}@example.com` });
|
||||
}
|
||||
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
|
||||
const id = Object.keys(raw.shares)[0];
|
||||
expect(raw.shares[id].subscriberEmails).toHaveLength(8);
|
||||
// FIFO cap -- the first 4 got dropped, latest 8 remain.
|
||||
expect(raw.shares[id].subscriberEmails[0]).toBe('user4@example.com');
|
||||
expect(raw.shares[id].subscriberEmails[7]).toBe('user11@example.com');
|
||||
});
|
||||
|
||||
test('omitted email does not write subscriberEmails', async () => {
|
||||
const issued = await shareStore.issuePublic({ serviceId: 'plex' });
|
||||
const app = _buildApp({ shareStore });
|
||||
await request(app).post(`/share/${issued.token}/subscribe`).send({});
|
||||
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
|
||||
const id = Object.keys(raw.shares)[0];
|
||||
expect(raw.shares[id].subscriberEmails).toBeUndefined();
|
||||
});
|
||||
});
|
||||
|
||||
// --------- Redeem-tailscale endpoint -- deviceId validation ---------------------------------------------------------
|
||||
|
||||
describe('DC-083: redeem-tailscale deviceId validation', () => {
|
||||
let dir, shareStore;
|
||||
beforeEach(() => { dir = _tmpDir(); shareStore = createShareStore({ dataDir: dir }); });
|
||||
afterEach(() => _cleanup(dir));
|
||||
|
||||
test('accepts Tailscale-style base64url ID', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'nodekey-abc123-def456' });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.data.redeemed).toBe(true);
|
||||
});
|
||||
|
||||
test('accepts OMITTED deviceId (treated as "unknown")', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({});
|
||||
expect(res.status).toBe(200);
|
||||
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
|
||||
const id = Object.keys(raw.shares)[0];
|
||||
expect(raw.shares[id].usedBy).toBe('unknown');
|
||||
});
|
||||
|
||||
test('rejects CR/LF in deviceId (CRLF-injection defense)', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'nodekey\r\nX-Injected: yes' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects NUL in deviceId', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'nodekey\x00hack' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects oversized deviceId (>128 chars)', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const long = 'a'.repeat(200);
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: long });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects empty string deviceId', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: '' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects whitespace in deviceId', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'node key 1' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects shell metachars in deviceId', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'nodekey; rm -rf /' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects non-string deviceId', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: { evil: true } });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('sanitized usedBy flows into the on-disk shares.json', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'node-abc.def-123' });
|
||||
const raw = JSON.parse(fs.readFileSync(path.join(dir, 'shares.json'), 'utf8'));
|
||||
const id = Object.keys(raw.shares)[0];
|
||||
expect(raw.shares[id].usedBy).toBe('node-abc.def-123');
|
||||
});
|
||||
|
||||
test('rejection does NOT mark the share used', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore });
|
||||
const bad = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'node with spaces' });
|
||||
expect(bad.status).toBe(400);
|
||||
// A FOLLOW-UP valid redeem should still succeed.
|
||||
const ok = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'node-clean' });
|
||||
expect(ok.status).toBe(200);
|
||||
});
|
||||
});
|
||||
|
||||
// --------- Store-layer defense-in-depth (bypass the route, hit the store) ------------
|
||||
|
||||
describe('DC-083: store-layer defense-in-depth', () => {
|
||||
let dir, store;
|
||||
beforeEach(() => { dir = _tmpDir(); store = createShareStore({ dataDir: dir }); });
|
||||
afterEach(() => _cleanup(dir));
|
||||
|
||||
test('recordPublicSubscribe rejects CRLF in email', async () => {
|
||||
const issued = await store.issuePublic({ serviceId: 'svc' });
|
||||
const r = await store.recordPublicSubscribe(issued.token, { email: 'a@b.com\r\nX: 1' });
|
||||
expect(r.ok).toBe(false);
|
||||
expect(r.reason).toBe('invalid_email');
|
||||
});
|
||||
|
||||
test('recordPublicSubscribe rejects oversized email', async () => {
|
||||
const issued = await store.issuePublic({ serviceId: 'svc' });
|
||||
const r = await store.recordPublicSubscribe(issued.token, { email: 'a'.repeat(300) + '@x.com' });
|
||||
expect(r.ok).toBe(false);
|
||||
expect(r.reason).toBe('invalid_email');
|
||||
});
|
||||
|
||||
test('recordTailscaleUse rejects CRLF in deviceId', async () => {
|
||||
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
|
||||
const r = await store.recordTailscaleUse(issued.token, { deviceId: 'node\r\nhack' });
|
||||
expect(r.ok).toBe(false);
|
||||
expect(r.reason).toBe('invalid_device_id');
|
||||
});
|
||||
|
||||
test('recordTailscaleUse rejects oversized deviceId', async () => {
|
||||
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
|
||||
const r = await store.recordTailscaleUse(issued.token, { deviceId: 'a'.repeat(200) });
|
||||
expect(r.ok).toBe(false);
|
||||
expect(r.reason).toBe('invalid_device_id');
|
||||
});
|
||||
|
||||
test('recordTailscaleUse accepts null deviceId (defaults to "unknown")', async () => {
|
||||
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
|
||||
const r = await store.recordTailscaleUse(issued.token, { deviceId: null });
|
||||
expect(r.ok).toBe(true);
|
||||
expect(r.share.usedBy).toBe('unknown');
|
||||
});
|
||||
|
||||
test('recordTailscaleUse accepts omitted deviceId (defaults to "unknown")', async () => {
|
||||
const issued = await store.issueTailscale({ serviceId: 'svc', email: 'a@b.com' });
|
||||
const r = await store.recordTailscaleUse(issued.token, {});
|
||||
expect(r.ok).toBe(true);
|
||||
expect(r.share.usedBy).toBe('unknown');
|
||||
});
|
||||
|
||||
test('recordPublicSubscribe accepts omitted email (backwards-compatible)', async () => {
|
||||
const issued = await store.issuePublic({ serviceId: 'svc' });
|
||||
const r = await store.recordPublicSubscribe(issued.token);
|
||||
expect(r.ok).toBe(true);
|
||||
});
|
||||
|
||||
test('recordPublicSubscribe accepts null email (backwards-compatible)', async () => {
|
||||
const issued = await store.issuePublic({ serviceId: 'svc' });
|
||||
const r = await store.recordPublicSubscribe(issued.token, { email: null });
|
||||
expect(r.ok).toBe(true);
|
||||
});
|
||||
});
|
||||
|
||||
// --------- Rate-limit guard ------------------------------------------------------------------------------------------------------------------------------------------------------
|
||||
|
||||
describe('DC-083: SHARE_PUBLIC rate-limit', () => {
|
||||
// We can't easily trigger the rate-limit in a unit test because the
|
||||
// default 30/15min is high. Instead, verify the constant is wired and
|
||||
// that the limiter is mounted on the public endpoints (the test env
|
||||
// skips the limiter, so we just confirm the constants).
|
||||
test('RATE_LIMITS.SHARE_PUBLIC is bounded tighter than GENERAL', () => {
|
||||
const { RATE_LIMITS } = require('../src/utilities/constants');
|
||||
expect(RATE_LIMITS.SHARE_PUBLIC).toBeDefined();
|
||||
expect(RATE_LIMITS.SHARE_PUBLIC.max).toBeLessThan(RATE_LIMITS.GENERAL.max);
|
||||
expect(RATE_LIMITS.SHARE_PUBLIC.max).toBeLessThanOrEqual(30);
|
||||
expect(RATE_LIMITS.SHARE_PUBLIC.windowMs).toBe(15 * 60 * 1000);
|
||||
});
|
||||
|
||||
test('route module loads without throwing when express-rate-limit is wired', () => {
|
||||
// Smoke test: the route factory must succeed with the limiter attached.
|
||||
const dir = _tmpDir();
|
||||
try {
|
||||
const shareStore = createShareStore({ dataDir: dir });
|
||||
const app = _buildApp({ shareStore });
|
||||
// _buildApp would have thrown if the route factory threw.
|
||||
expect(typeof app).toBe('function');
|
||||
} finally {
|
||||
_cleanup(dir);
|
||||
}
|
||||
});
|
||||
|
||||
test('sharePublicLimiter is mounted on /preview (route stack contains limiter)', () => {
|
||||
// Verify the limiter middleware is actually wired into /preview's route
|
||||
// stack. The route uses express.Router().use(path, ...mw, handler) so we
|
||||
// can inspect the stack via the router's internal `stack` array.
|
||||
const dir = _tmpDir();
|
||||
try {
|
||||
const shareStore = createShareStore({ dataDir: dir });
|
||||
const router = require('../routes/share')({
|
||||
shareStore,
|
||||
licenseManager: { isPro: () => true, allowsLifetimeLicense: () => false },
|
||||
tailscaleCoord: { createAuthKey: async () => ({ id: 'k', key: 'tskey-x' }) },
|
||||
notificationManager: { sendEmail: async () => ({ messageId: 'fake' }) },
|
||||
servicesStateManager: { get: async () => null, read: async () => [] },
|
||||
servicesFile: null,
|
||||
asyncHandler: (fn, _label) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next),
|
||||
log: { info() {}, warn() {}, error() {} },
|
||||
});
|
||||
const previewStack = router.stack.find(
|
||||
(layer) => layer.route && layer.route.path === '/share/:token/preview'
|
||||
);
|
||||
expect(previewStack).toBeDefined();
|
||||
// The route handler should be preceded by at least one middleware
|
||||
// layer (the limiter). route.stack contains the per-route middleware.
|
||||
// In express, .route.stack has the route-local middleware + handler.
|
||||
// The limiter is mounted at the router level (router.use pattern), so
|
||||
// it's actually a separate layer in router.stack. Look for any layer
|
||||
// that has a regex/path matching /share/:token.
|
||||
const limiterLayer = router.stack.find(
|
||||
(layer) => layer.regexp && layer.regexp.test && layer.regexp.test('/share/abc/preview')
|
||||
);
|
||||
expect(limiterLayer).toBeDefined();
|
||||
} finally {
|
||||
_cleanup(dir);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe('DC-083: positive smoke tests (legitimate inputs)', () => {
|
||||
test('validates user+tag@sub.domain.io (RFC 5322 plus addressing)', () => {
|
||||
const { validatePublicEmail } = require('../src/security/share-store');
|
||||
const v = validatePublicEmail('user+tag@sub.domain.io');
|
||||
expect(v).toEqual({ ok: true, email: 'user+tag@sub.domain.io' });
|
||||
});
|
||||
|
||||
test('validates a typical Tailscale node ID as deviceId', () => {
|
||||
const { validatePublicDeviceId } = require('../src/security/share-store');
|
||||
// Tailscale node IDs look like "nodekey:abcdef0123456789" or just hex
|
||||
const v = validatePublicDeviceId('nodekey:abcdef0123456789');
|
||||
expect(v).toEqual({ ok: true, deviceId: 'nodekey:abcdef0123456789' });
|
||||
});
|
||||
});
|
||||
@@ -378,12 +378,35 @@ describe('share routes: POST /share/:token/redeem-tailscale (public)', () => {
|
||||
expect(r2.body.error).toMatch(/already_used/);
|
||||
});
|
||||
|
||||
test('rejects missing deviceId', async () => {
|
||||
test('rejects missing deviceId — DC-083 accepts omitted, treats as "unknown"', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore, noAdmin: true });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({});
|
||||
// DC-083: omitted deviceId is now accepted; the store defaults usedBy
|
||||
// to 'unknown'. The pre-fix route layer required deviceId be present;
|
||||
// the new behavior matches the store's defensive default and is
|
||||
// safer for partially-malformed forward_auth calls from Caddy.
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.data.redeemed).toBe(true);
|
||||
});
|
||||
|
||||
test('rejects invalid deviceId (control chars / oversized)', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore, noAdmin: true });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: 'node\r\nhack' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
|
||||
test('rejects empty deviceId', async () => {
|
||||
const issued = await shareStore.issueTailscale({ serviceId: 'plex', email: 'a@b.com' });
|
||||
const app = _buildApp({ shareStore, noAdmin: true });
|
||||
const res = await request(app)
|
||||
.post(`/share/${issued.token}/redeem-tailscale`)
|
||||
.send({ deviceId: '' });
|
||||
expect(res.status).toBe(400);
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,228 @@
|
||||
/**
|
||||
* DC-082: Update-manager image-name parsing for docker-compose prefixed names.
|
||||
*
|
||||
* The pre-fix code normalized `dashcaddy-dashcaddy-api:latest` to
|
||||
* `library/dashcaddy-dashcaddy-api:latest` before probing Docker Hub.
|
||||
* Compose-prefixed names (single hyphen, no slash, lowercase) need to
|
||||
* split on the FIRST hyphen to recover `<project>/<service>` — that's
|
||||
* the actual upstream namespace for a compose-prefixed image.
|
||||
*
|
||||
* The fix also adds a "no upstream registry image, skip cleanly" path
|
||||
* for when the authed GET 401s against a compose-prefixed name (the
|
||||
* compose-prefixed image is built locally and not published to Docker
|
||||
* Hub). That should log as info, not error.
|
||||
*/
|
||||
const updateManager = require('../src/managers/update-manager');
|
||||
|
||||
describe('DC-082 update-manager / compose-prefixed image names', () => {
|
||||
let um = updateManager; // module exports the singleton instance
|
||||
|
||||
describe('_composeProjectToRepo', () => {
|
||||
test('splits dashcaddy-dashcaddy-api on the first hyphen', () => {
|
||||
expect(um._composeProjectToRepo('dashcaddy-dashcaddy-api')).toBe('dashcaddy/dashcaddy-api');
|
||||
});
|
||||
|
||||
test('splits myproject-myservice on the first hyphen', () => {
|
||||
expect(um._composeProjectToRepo('myproject-myservice')).toBe('myproject/myservice');
|
||||
});
|
||||
|
||||
test('splits multi-hyphen names on the FIRST hyphen only', () => {
|
||||
// "myproj-grandchild-service" -> "myproj/grandchild-service"
|
||||
// (first hyphen is the project/service boundary; later hyphens are
|
||||
// part of the service name like docker-compose's `web-cache`).
|
||||
expect(um._composeProjectToRepo('myproj-grandchild-service')).toBe('myproj/grandchild-service');
|
||||
});
|
||||
|
||||
test('returns null for slash-namespaced names (handled by other path)', () => {
|
||||
expect(um._composeProjectToRepo('dashcaddy/dashcaddy-api')).toBe(null);
|
||||
expect(um._composeProjectToRepo('library/nginx')).toBe(null);
|
||||
expect(um._composeProjectToRepo('ghcr.io/x/y')).toBe(null);
|
||||
});
|
||||
|
||||
test('returns null for Docker Official Image names (no hyphen)', () => {
|
||||
expect(um._composeProjectToRepo('nginx')).toBe(null);
|
||||
expect(um._composeProjectToRepo('alpine')).toBe(null);
|
||||
expect(um._composeProjectToRepo('node')).toBe(null);
|
||||
});
|
||||
|
||||
test('returns null for empty / malformed input', () => {
|
||||
expect(um._composeProjectToRepo('')).toBe(null);
|
||||
expect(um._composeProjectToRepo(null)).toBe(null);
|
||||
expect(um._composeProjectToRepo(undefined)).toBe(null);
|
||||
expect(um._composeProjectToRepo(123)).toBe(null);
|
||||
expect(um._composeProjectToRepo('-foo')).toBe(null); // leading hyphen
|
||||
expect(um._composeProjectToRepo('foo-')).toBe(null); // trailing hyphen
|
||||
// The regex tolerates mixed-case via the /i flag for defensiveness
|
||||
// even though Docker Compose names are typically lowercase — the
|
||||
// important shape constraints are the letter/digit/underscore/hyphen
|
||||
// charset and the non-empty two-part split.
|
||||
});
|
||||
|
||||
test('accepts names with underscores and digits (compose allows)', () => {
|
||||
expect(um._composeProjectToRepo('proj-v2_service')).toBe('proj/v2_service');
|
||||
expect(um._composeProjectToRepo('dashcaddy-api-v2')).toBe('dashcaddy/api-v2');
|
||||
});
|
||||
|
||||
test('rejects names with chars compose never produces', () => {
|
||||
// dot/colon/slash should never pass — they're either already-namespaced
|
||||
// or invalid in a Docker Compose service name.
|
||||
expect(um._composeProjectToRepo('foo:bar')).toBe(null);
|
||||
expect(um._composeProjectToRepo('foo.bar')).toBe(null);
|
||||
expect(um._composeProjectToRepo('foo/bar')).toBe(null);
|
||||
});
|
||||
});
|
||||
|
||||
describe('_isNotPublishedError', () => {
|
||||
test('returns true for HTTP 401 + compose-prefixed remainder', () => {
|
||||
const err = new Error('Docker Hub registry returned HTTP 401 after auth');
|
||||
expect(um._isNotPublishedError(err, 'dashcaddy-dashcaddy-api')).toBe(true);
|
||||
});
|
||||
|
||||
test('returns false for HTTP 401 with non-compose-prefixed remainder', () => {
|
||||
const err = new Error('Docker Hub registry returned HTTP 401 after auth');
|
||||
expect(um._isNotPublishedError(err, 'nginx')).toBe(false);
|
||||
expect(um._isNotPublishedError(err, 'library/nginx')).toBe(false);
|
||||
expect(um._isNotPublishedError(err, 'dashcaddy/some-image')).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false for non-401 errors', () => {
|
||||
const err = new Error('network timeout after 10s');
|
||||
expect(um._isNotPublishedError(err, 'dashcaddy-dashcaddy-api')).toBe(false);
|
||||
});
|
||||
|
||||
test('returns false for malformed error or remainder', () => {
|
||||
expect(um._isNotPublishedError(null, 'dashcaddy-dashcaddy-api')).toBe(false);
|
||||
expect(um._isNotPublishedError({}, 'dashcaddy-dashcaddy-api')).toBe(false);
|
||||
expect(um._isNotPublishedError({ message: 'no string' }, 'dashcaddy-dashcaddy-api')).toBe(false);
|
||||
expect(um._isNotPublishedError(new Error('HTTP 401'), null)).toBe(false);
|
||||
expect(um._isNotPublishedError(new Error('HTTP 401'), '')).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('getLatestImageDigest (mocked fetchWithReliability)', () => {
|
||||
let originalFetch;
|
||||
let originalFetchAuth;
|
||||
let originalFetchRetry;
|
||||
beforeEach(() => {
|
||||
originalFetch = um.fetchWithReliability.bind(um);
|
||||
originalFetchAuth = um.fetchAuthToken.bind(um);
|
||||
});
|
||||
|
||||
test('compose-prefixed name (dashcaddy-dashcaddy-api) probes dashcaddy/dashcaddy-api (NOT library/dashcaddy-dashcaddy-api)', async () => {
|
||||
const calls = [];
|
||||
um.fetchWithReliability = async (opts) => {
|
||||
calls.push(opts);
|
||||
// Simulate the real Docker Hub: 401 with WWW-Auth, then 401 after token
|
||||
// (because dashcaddy/dashcaddy-api doesn't exist on Docker Hub).
|
||||
if (calls.length === 1) {
|
||||
return {
|
||||
statusCode: 401,
|
||||
headers: {
|
||||
'www-authenticate': 'Bearer realm="https://auth.docker.io/token",service="registry.docker.io",scope="repository:dashcaddy/dashcaddy-api:pull"',
|
||||
},
|
||||
body: '',
|
||||
};
|
||||
}
|
||||
return { statusCode: 401, headers: {}, body: '{"errors":[{"code":"UNAUTHORIZED","message":"authentication required"}]}' };
|
||||
};
|
||||
um.fetchAuthToken = async () => 'fake-token';
|
||||
const { log } = require('../src/utils/logging');
|
||||
const infoSpy = jest.spyOn(log, 'info').mockImplementation(() => {});
|
||||
const errorSpy = jest.spyOn(log, 'error').mockImplementation(() => {});
|
||||
|
||||
const result = await um.getLatestImageDigest('dashcaddy-dashcaddy-api:latest');
|
||||
expect(result).toBe(null);
|
||||
|
||||
// First probe must target /v2/dashcaddy/dashcaddy-api/manifests/latest
|
||||
// NOT /v2/library/dashcaddy-dashcaddy-api/manifests/latest
|
||||
const firstPath = calls[0].path;
|
||||
expect(firstPath).toBe('/v2/dashcaddy/dashcaddy-api/manifests/latest');
|
||||
expect(firstPath).not.toContain('library/dashcaddy-dashcaddy-api');
|
||||
|
||||
// The 401 after auth should produce an INFO log about "no upstream"
|
||||
// NOT an error log.
|
||||
const infoMsgs = infoSpy.mock.calls.map((c) => c[1]);
|
||||
expect(infoMsgs).toContain('No upstream registry image — skipping update check');
|
||||
const errorMsgs = errorSpy.mock.calls.map((c) => c[1]);
|
||||
expect(errorMsgs).not.toContain('Docker Hub registry returned HTTP 401 after auth');
|
||||
|
||||
infoSpy.mockRestore();
|
||||
errorSpy.mockRestore();
|
||||
});
|
||||
|
||||
test('official image (nginx) still probes library/nginx', async () => {
|
||||
const calls = [];
|
||||
um.fetchWithReliability = async (opts) => {
|
||||
calls.push(opts);
|
||||
if (calls.length === 1) {
|
||||
return { statusCode: 200, headers: { 'docker-content-digest': 'sha256:abc123' }, body: '' };
|
||||
}
|
||||
return { statusCode: 200, headers: {}, body: '' };
|
||||
};
|
||||
|
||||
const result = await um.getLatestImageDigest('nginx:latest');
|
||||
expect(result).toBe('sha256:abc123');
|
||||
expect(calls[0].path).toBe('/v2/library/nginx/manifests/latest');
|
||||
});
|
||||
|
||||
test('library/nginx (explicit) probes library/nginx', async () => {
|
||||
const calls = [];
|
||||
um.fetchWithReliability = async (opts) => {
|
||||
calls.push(opts);
|
||||
if (calls.length === 1) {
|
||||
return { statusCode: 200, headers: { 'docker-content-digest': 'sha256:abc' }, body: '' };
|
||||
}
|
||||
return { statusCode: 200, headers: {}, body: '' };
|
||||
};
|
||||
|
||||
const result = await um.getLatestImageDigest('library/nginx:latest');
|
||||
expect(result).toBe('sha256:abc');
|
||||
expect(calls[0].path).toBe('/v2/library/nginx/manifests/latest');
|
||||
});
|
||||
|
||||
test('ghcr.io/samiahmed7777/dashcaddy-api uses ghcr.io path (not docker hub)', async () => {
|
||||
const calls = [];
|
||||
um.fetchWithReliability = async (opts) => {
|
||||
calls.push(opts);
|
||||
return { statusCode: 200, headers: { 'docker-content-digest': 'sha256:ghcr' }, body: '' };
|
||||
};
|
||||
|
||||
const result = await um.getLatestImageDigest('ghcr.io/samiahmed7777/dashcaddy-api:latest');
|
||||
expect(result).toBe('sha256:ghcr');
|
||||
expect(calls[0].hostname).toBe('ghcr.io');
|
||||
expect(calls[0].path).toBe('/v2/samiahmed7777/dashcaddy-api/manifests/latest');
|
||||
});
|
||||
|
||||
test('returns null + skips cleanly when compose-prefixed image has no upstream', async () => {
|
||||
let callCount = 0;
|
||||
um.fetchWithReliability = async (opts) => {
|
||||
callCount += 1;
|
||||
if (callCount === 1) {
|
||||
return {
|
||||
statusCode: 401,
|
||||
headers: { 'www-authenticate': 'Bearer realm="https://auth.docker.io/token",service="registry.docker.io",scope="repository:myproj-myservice:pull"' },
|
||||
body: '',
|
||||
};
|
||||
}
|
||||
return { statusCode: 401, headers: {}, body: '{"errors":[{"code":"UNAUTHORIZED"}]}' };
|
||||
};
|
||||
um.fetchAuthToken = async () => 'fake-token';
|
||||
|
||||
const result = await um.getLatestImageDigest('myproj-myservice:latest');
|
||||
expect(result).toBe(null);
|
||||
// Probe targets the correct namespace (myproj/myservice), not library/.
|
||||
const firstCall = await (async () => {
|
||||
let p;
|
||||
um.fetchWithReliability = async (opts) => { p = opts; return { statusCode: 200, headers: {}, body: '' }; };
|
||||
await um.getLatestImageDigest('myproj-myservice:latest');
|
||||
return p;
|
||||
})();
|
||||
expect(firstCall.path).toBe('/v2/myproj/myservice/manifests/latest');
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
um.fetchWithReliability = originalFetch;
|
||||
um.fetchAuthToken = originalFetchAuth;
|
||||
});
|
||||
});
|
||||
});
|
||||
@@ -37,6 +37,81 @@ const BACKUP_FILES = [
|
||||
|
||||
const ASSET_FILES = ['custom-logo.png', 'custom-favicon.png', 'custom-logo.svg'];
|
||||
|
||||
// DC-079: Restrict restored assets to the hardcoded ASSET_FILES allowlist.
|
||||
// The asset KEYS in the snapshot are user-controlled JSON, so iterating
|
||||
// `Object.entries(snapshot.assets)` and writing each name verbatim into
|
||||
// `path.join(assetsDir, name)` lets an attacker POST `{assets: {"../../etc/caddy/Caddyfile":
|
||||
// "<base64-evil>"}}` and overwrite the live Caddyfile via the bind-mount
|
||||
// (path.join('/app/data/assets', '../../etc/caddy/Caddyfile') resolves
|
||||
// to /etc/caddy/Caddyfile). This bypasses the caddyfile-staging gate
|
||||
// above because the dataDir bind-mount can write to /etc/caddy on the host.
|
||||
const ASSET_KEY_RE = /^[a-zA-Z0-9._-]+$/;
|
||||
const ASSET_PATH_TRAVERSAL_RE = /(^|\/)\.\.($|\/)|^\//;
|
||||
|
||||
// DC-079: Caddyfile content safety limits for disaster-recovery restore.
|
||||
// The live Caddyfile on DNS2 is ~17 KB and grows linearly with vhost count.
|
||||
// Express's default JSON body parser limit (1 MB) is the outer gate; this
|
||||
// in-handler cap is defense-in-depth against either a future body-limit
|
||||
// raise or a custom body parser. Cap well below the body-parser ceiling.
|
||||
const MAX_CADDYFILE_BYTES = 512 * 1024; // 512 KiB — 30x the live file, far below 1 MB body limit
|
||||
|
||||
// DC-079: theme filenames must match this pattern. No slashes (no path
|
||||
// traversal), no `..`, must end in `.json`, and only filename-safe chars.
|
||||
// Themes are written to <dataDir>/themes/<name>; we also defense-in-depth
|
||||
// check the resolved path stays inside that dir.
|
||||
const THEME_NAME_RE = /^[a-zA-Z0-9._-]+\.json$/;
|
||||
|
||||
function assertSafeAssetKey(key) {
|
||||
if (typeof key !== 'string' || key.length === 0 || key.length > 128) {
|
||||
throw new Error(`asset key must be a non-empty string up to 128 chars`);
|
||||
}
|
||||
if (ASSET_PATH_TRAVERSAL_RE.test(key) || !ASSET_KEY_RE.test(key)) {
|
||||
throw new Error(`asset key contains forbidden characters or path segments`);
|
||||
}
|
||||
}
|
||||
|
||||
function assertSafeThemeName(name) {
|
||||
if (typeof name !== 'string' || name.length === 0 || name.length > 128) {
|
||||
throw new Error(`theme name must be a non-empty string up to 128 chars`);
|
||||
}
|
||||
if (!THEME_NAME_RE.test(name)) {
|
||||
throw new Error(`theme name must match ${THEME_NAME_RE} (alphanum / dot / dash / underscore, ending in .json)`);
|
||||
}
|
||||
}
|
||||
|
||||
// Reject Caddyfile content that smuggles in arbitrary `import` directives.
|
||||
// caddy-apply expects the single top-level Caddyfile; any `import` to an
|
||||
// absolute path means "load another file from disk at Caddy reload time" —
|
||||
// that's a classic injection vector (an attacker can craft a snapshot whose
|
||||
// `import /etc/caddy/external.caddy` reads any file Caddy can read).
|
||||
// We allow the relative-style `import <snippet>` form ONLY if the snippet
|
||||
// name matches a small allowlist of well-known Caddy snippet names (none
|
||||
// today; add explicit names if a future snippet module is needed).
|
||||
const FORBIDDEN_IMPORT_RE = /^\s*import\s+(["']|\/|\.\.|~\/|%[A-F0-9]{2})/im;
|
||||
|
||||
function validateCaddyfileContent(content) {
|
||||
if (typeof content !== 'string') {
|
||||
return { ok: false, error: 'Caddyfile content must be a string' };
|
||||
}
|
||||
if (content.length === 0) {
|
||||
return { ok: false, error: 'Caddyfile content is empty' };
|
||||
}
|
||||
if (Buffer.byteLength(content, 'utf8') > MAX_CADDYFILE_BYTES) {
|
||||
return { ok: false, error: `Caddyfile content exceeds ${MAX_CADDYFILE_BYTES} bytes` };
|
||||
}
|
||||
if (FORBIDDEN_IMPORT_RE.test(content)) {
|
||||
// Allow the canonical single-quoted snippet import form ONLY if the
|
||||
// snippet name is on the explicit allowlist (currently empty). This
|
||||
// catches absolute paths, ../, ~/, and URL-encoded payloads while
|
||||
// leaving room for future snippet additions without touching this gate.
|
||||
return {
|
||||
ok: false,
|
||||
error: 'Caddyfile contains forbidden `import` directive (absolute path, encoded, or non-allowlisted snippet)'
|
||||
};
|
||||
}
|
||||
return { ok: true };
|
||||
}
|
||||
|
||||
module.exports = function({ servicesStateManager, platformPaths, log, asyncHandler }) {
|
||||
const wrap = asyncHandler || ((fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next));
|
||||
const router = express.Router();
|
||||
@@ -44,6 +119,15 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
|
||||
let lastBackupStatus = { timestamp: null, status: null, size: null };
|
||||
let lastRestoreStatus = { timestamp: null, status: null };
|
||||
|
||||
// DC-079: Staging dir for the candidate Caddyfile. The disaster-recovery
|
||||
// restore endpoint stages here instead of writing directly to the live
|
||||
// Caddyfile path. The operator must run `caddy-apply` (or its equivalent)
|
||||
// to validate + reload + git-commit the staged file. This keeps the live
|
||||
// Caddyfile under the same atomic-commit guard as every other edit.
|
||||
function getStagedCaddyfileDir(dataDir) {
|
||||
return path.join(dataDir, 'disaster-staged');
|
||||
}
|
||||
|
||||
/**
|
||||
* POST /api/v1/disaster/backup
|
||||
* Creates a complete system snapshot as a downloadable JSON file.
|
||||
@@ -175,13 +259,64 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
|
||||
}
|
||||
}
|
||||
|
||||
// Restore Caddyfile
|
||||
if (snapshot.caddyfile) {
|
||||
// DC-079: Stage the Caddyfile to a staging path inside dataDir
|
||||
// instead of writing directly to caddyfilePath (which is the LIVE
|
||||
// /etc/caddy/Caddyfile bind-mounted into the container as /caddyfile).
|
||||
//
|
||||
// Threat model (defense-in-depth, mirrors DC-070 / DC-074 / DC-076):
|
||||
// the endpoint is TOTP-gated, but a compromised operator / phished
|
||||
// session / pivot path could POST a snapshot with `caddyfile: <evil>`
|
||||
// and the pre-fix code would call `fsp.writeFile(caddyfilePath, ...)`
|
||||
// which writes the attacker-controlled string straight to the live
|
||||
// Caddyfile. Caddy then reads that file on the next reload (which can
|
||||
// be triggered by ACME renewals, health probes, or any admin API
|
||||
// touch), executing whatever directives the attacker embedded:
|
||||
// - `admin off` + arbitrary config write
|
||||
// - `import /etc/caddy/<anything-caddy-can-read>` for content theft
|
||||
// - `reverse_proxy` to attacker-controlled upstreams
|
||||
// - `acme_ca` override to attacker CA
|
||||
// - `log` directives to attacker-writable paths
|
||||
//
|
||||
// The Caddyfile is managed by the `caddy-apply` wrapper (validates +
|
||||
// reloads + git-commits atomically — see CLAUDE.md hard rule). This
|
||||
// endpoint previously bypassed that wrapper. The fix stages the
|
||||
// candidate file under dataDir/disaster-staged/Caddyfile.candidate and
|
||||
// returns the path so the operator can apply it via the normal flow.
|
||||
const caddyfileStaged = [];
|
||||
// DC-079: handle three cases for the caddyfile field:
|
||||
// - absent/null/undefined: back-compat — no Caddyfile in snapshot
|
||||
// - empty string "": explicit empty payload is suspicious — reject
|
||||
// - non-string (object/array/number): type confusion attempt — reject
|
||||
// - valid string: stage to dataDir/disaster-staged/Caddyfile.candidate
|
||||
if (snapshot.caddyfile !== undefined && snapshot.caddyfile !== null) {
|
||||
const validation = validateCaddyfileContent(snapshot.caddyfile);
|
||||
if (!validation.ok) {
|
||||
return errorResponse(res, 400, `Invalid Caddyfile in snapshot: ${validation.error}`, {
|
||||
code: ErrorCodes.BACKUP.INVALID_CONFIG,
|
||||
});
|
||||
}
|
||||
|
||||
const stagedDir = getStagedCaddyfileDir(dataDir);
|
||||
try {
|
||||
await fsp.writeFile(caddyfilePath, snapshot.caddyfile);
|
||||
restored.push('Caddyfile');
|
||||
await fsp.mkdir(stagedDir, { recursive: true });
|
||||
const stagedPath = path.join(stagedDir, 'Caddyfile.candidate');
|
||||
// Atomic write: write to .candidate.tmp then rename. The live
|
||||
// Caddyfile is NEVER touched from this endpoint.
|
||||
const tmpPath = stagedPath + '.tmp';
|
||||
await fsp.writeFile(tmpPath, snapshot.caddyfile, { mode: 0o644 });
|
||||
await fsp.rename(tmpPath, stagedPath);
|
||||
caddyfileStaged.push({
|
||||
file: 'Caddyfile',
|
||||
stagedPath,
|
||||
action: 'awaiting caddy-apply',
|
||||
livePath: caddyfilePath,
|
||||
});
|
||||
if (log) log.info('disaster-recovery', 'Caddyfile staged (not applied)', {
|
||||
stagedPath,
|
||||
size: Buffer.byteLength(snapshot.caddyfile, 'utf8'),
|
||||
});
|
||||
} catch (err) {
|
||||
errors.push({ file: 'Caddyfile', error: err.message });
|
||||
errors.push({ file: 'Caddyfile (staging)', error: err.message });
|
||||
}
|
||||
}
|
||||
|
||||
@@ -189,8 +324,20 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
|
||||
const assetsDir = platformPaths?.resolveAssetsPath?.() || path.join(dataDir, 'assets');
|
||||
for (const [name, base64] of Object.entries(snapshot.assets || {})) {
|
||||
try {
|
||||
// DC-079: assets directory is the first attack surface that
|
||||
// bypasses the Caddyfile-staging gate. `name` is a user-supplied
|
||||
// JSON key; without validation, `path.join(assetsDir, name)` lets
|
||||
// an attacker escape to /etc/caddy via path traversal.
|
||||
assertSafeAssetKey(name);
|
||||
const resolved = path.resolve(assetsDir, name);
|
||||
// Defense-in-depth: even after charset checks, the resolved path
|
||||
// MUST stay inside assetsDir. If it doesn't, refuse the write.
|
||||
if (!resolved.startsWith(path.resolve(assetsDir) + path.sep) &&
|
||||
resolved !== path.resolve(assetsDir)) {
|
||||
throw new Error(`asset path resolves outside assets directory`);
|
||||
}
|
||||
await fsp.mkdir(assetsDir, { recursive: true });
|
||||
await fsp.writeFile(path.join(assetsDir, name), Buffer.from(base64, 'base64'));
|
||||
await fsp.writeFile(resolved, Buffer.from(base64, 'base64'));
|
||||
restored.push(`assets/${name}`);
|
||||
} catch (err) {
|
||||
errors.push({ file: `assets/${name}`, error: err.message });
|
||||
@@ -203,8 +350,21 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
|
||||
try {
|
||||
await fsp.mkdir(themesDir, { recursive: true });
|
||||
for (const [name, content] of Object.entries(snapshot.themes)) {
|
||||
await fsp.writeFile(path.join(themesDir, name), JSON.stringify(content, null, 2));
|
||||
restored.push(`themes/${name}`);
|
||||
// DC-079: same path-traversal vector as assets — keys are
|
||||
// user-controlled JSON. Validate the name AND confirm the
|
||||
// resolved path stays inside themesDir.
|
||||
try {
|
||||
assertSafeThemeName(name);
|
||||
const resolved = path.resolve(themesDir, name);
|
||||
if (!resolved.startsWith(path.resolve(themesDir) + path.sep) &&
|
||||
resolved !== path.resolve(themesDir)) {
|
||||
throw new Error(`theme path resolves outside themes directory`);
|
||||
}
|
||||
await fsp.writeFile(resolved, JSON.stringify(content, null, 2));
|
||||
restored.push(`themes/${name}`);
|
||||
} catch (err) {
|
||||
errors.push({ file: `themes/${name}`, error: err.message });
|
||||
}
|
||||
}
|
||||
} catch (err) {
|
||||
errors.push({ file: 'themes', error: err.message });
|
||||
@@ -215,19 +375,33 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
|
||||
timestamp: new Date().toISOString(),
|
||||
status: errors.length === 0 ? 'success' : 'partial',
|
||||
restored: restored.length,
|
||||
staged: caddyfileStaged.length,
|
||||
errors: errors.length,
|
||||
};
|
||||
|
||||
if (log) log.info('disaster-recovery', 'Restore completed', lastRestoreStatus);
|
||||
|
||||
ok(res, {
|
||||
// DC-079: Surface the staged-Caddyfile warning in the response body so
|
||||
// the UI / operator can see that the Caddyfile is NOT yet live. The
|
||||
// restore endpoint stages under dataDir/disaster-staged/Caddyfile.candidate
|
||||
// and the operator must run `caddy-apply` (or its equivalent) to
|
||||
// validate + reload + git-commit the staged file. The live Caddyfile
|
||||
// is owned by the caddy-apply wrapper per CLAUDE.md hard rule.
|
||||
const responseBody = {
|
||||
status: errors.length === 0 ? 'success' : 'partial',
|
||||
restored,
|
||||
errors,
|
||||
message: errors.length === 0
|
||||
? `Successfully restored ${restored.length} files. Restart DashCaddy to apply.`
|
||||
? `Successfully restored ${restored.length} files${caddyfileStaged.length > 0 ? ` (Caddyfile staged — ${caddyfileStaged[0].stagedPath}; run caddy-apply to apply)` : ''}. Restart DashCaddy to apply.`
|
||||
: `Restored ${restored.length} files with ${errors.length} errors. Check error details.`,
|
||||
});
|
||||
};
|
||||
|
||||
if (caddyfileStaged.length > 0) {
|
||||
responseBody.caddyfileStaged = caddyfileStaged;
|
||||
responseBody.warning = '[DC-079] Caddyfile is STAGED, not applied. Live /etc/caddy/Caddyfile was NOT modified by this restore. Run `caddy-apply <reason>` (or equivalent) to validate + reload + git-commit the staged candidate.';
|
||||
}
|
||||
|
||||
ok(res, responseBody);
|
||||
}));
|
||||
|
||||
/**
|
||||
|
||||
@@ -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 path = require('path');
|
||||
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 }) {
|
||||
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
|
||||
router.get('/log-insights', asyncHandler(async (req, res) => {
|
||||
@@ -74,16 +169,18 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
||||
}
|
||||
|
||||
// --- Storage info ---
|
||||
const auditPath = process.env.AUDIT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/audit-log.json';
|
||||
const secPath = process.env.SECURITY_EVENT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl';
|
||||
// DC-081: read from the canonical resolved paths (NOT the hardcoded
|
||||
// /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 = {};
|
||||
try {
|
||||
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 {}
|
||||
try {
|
||||
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 {}
|
||||
|
||||
ok(res, {
|
||||
@@ -108,16 +205,44 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
||||
}));
|
||||
|
||||
// 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) => {
|
||||
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 cutoff = new Date(Date.now() - keepDays * 86400000).toISOString();
|
||||
|
||||
const auditPath = process.env.AUDIT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/audit-log.json';
|
||||
const secPath = process.env.SECURITY_EVENT_LOG_FILE || '/opt/dashcaddy/dashcaddy-api/data/security-events.jsonl';
|
||||
|
||||
// Read both files via the canonical resolved paths (NOT the hardcoded
|
||||
// /opt/... paths from before — those don't exist in the container).
|
||||
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 secRaw = await fs.readFile(secPath, 'utf8').catch(function () { return ''; });
|
||||
@@ -127,16 +252,40 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
||||
if (!confirm) {
|
||||
ok(res, {
|
||||
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 },
|
||||
cutoffDate: cutoff
|
||||
cutoffDate: cutoff,
|
||||
paths: { auditPath, secPath },
|
||||
});
|
||||
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; });
|
||||
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; } });
|
||||
await fs.writeFile(secPath, keptSec.join('\n') + '\n');
|
||||
@@ -145,9 +294,16 @@ module.exports = function({ asyncHandler, ok, auditLogger, securityEventStore })
|
||||
disposed: true,
|
||||
deleted: { auditEntries: oldAudit.length, securityEvents: oldSec.length },
|
||||
remaining: { auditEntries: keptAudit.length, securityEvents: keptSec.length },
|
||||
cutoffDate: cutoff
|
||||
cutoffDate: cutoff,
|
||||
});
|
||||
}));
|
||||
|
||||
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,
|
||||
};
|
||||
@@ -37,6 +37,10 @@
|
||||
const { ValidationError, NotFoundError } = require('../src/utilities/errors');
|
||||
const { PaymentRequiredError } = require('../src/utilities/errors');
|
||||
const { ok, created, badRequest, notFound } = require('../src/utils/responses');
|
||||
// DC-083: route-layer validators for the public CSRF-exempt endpoints. These
|
||||
// are imported from share-store so the route and store stay in lockstep
|
||||
// (drift risk if one set is updated and the other is forgotten).
|
||||
const { validatePublicEmail, validatePublicDeviceId } = require('../src/security/share-store');
|
||||
|
||||
const PUBLIC_TTL_OPTIONS = new Set([
|
||||
60 * 60 * 1000,
|
||||
@@ -293,7 +297,35 @@ module.exports = function shareRoutesFactory({
|
||||
|
||||
// ─── Public endpoints (no auth, no Pro gate) ──────────────────────────────
|
||||
|
||||
router.get('/share/:token/preview', asyncHandler(async (req, res) => {
|
||||
// DC-083: rate-limit the two CSRF-exempt public endpoints. The general
|
||||
// limiter (1000/15min) is mounted globally in app.js and is too generous
|
||||
// for unauthenticated state-mutating endpoints. 30/15min per IP is
|
||||
// enough for a legitimate user clicking "subscribe" once or twice; anything
|
||||
// beyond is abuse. Skipped in test envs via the standard isTest guard.
|
||||
// Lazy-loaded so test environments without the dep installed don't blow up;
|
||||
// a missing-dep in production logs a warning and falls back to no-op (still
|
||||
// safe — the route+store validators are the primary defense).
|
||||
const { RATE_LIMITS } = require('../src/utilities/constants');
|
||||
const isTest = process.env.NODE_ENV === 'test';
|
||||
let _sharePublicLimiter = (req, _res, next) => next(); // no-op default
|
||||
try {
|
||||
const rateLimit = require('express-rate-limit'); // eslint-disable-line global-require
|
||||
_sharePublicLimiter = rateLimit({
|
||||
...RATE_LIMITS.SHARE_PUBLIC,
|
||||
standardHeaders: true,
|
||||
legacyHeaders: false,
|
||||
skip: () => isTest,
|
||||
message: { success: false, error: 'Too many share requests, please try again later' },
|
||||
});
|
||||
} catch (e) {
|
||||
// Don't crash on missing dep in a bare-bones env — but log so it's not
|
||||
// invisible if production misconfigured.
|
||||
if (log && typeof log.warn === 'function') {
|
||||
log.warn({ ctx: 'share-routes', err: e.message }, 'express-rate-limit unavailable; share public endpoints have NO rate limit');
|
||||
}
|
||||
}
|
||||
|
||||
router.get('/share/:token/preview', _sharePublicLimiter, asyncHandler(async (req, res) => {
|
||||
const meta = await shareStore.peek(req.params.token);
|
||||
if (!meta) {
|
||||
return res.status(404).json({ success: false, error: '[DC-553] share not found or expired' });
|
||||
@@ -310,12 +342,21 @@ module.exports = function shareRoutesFactory({
|
||||
});
|
||||
}, 'share-preview'));
|
||||
|
||||
router.post('/share/:token/subscribe', asyncHandler(async (req, res) => {
|
||||
router.post('/share/:token/subscribe', _sharePublicLimiter, asyncHandler(async (req, res) => {
|
||||
// DC-083: replace the primitive `email.includes('@')` check with a
|
||||
// charset/length/control-char-bounded validator. The pre-fix code
|
||||
// accepted `@`, `a@`, `<script>@x.c`, and 10MB strings as "valid email".
|
||||
// The subscribe body's `email` is now also captured to the share record
|
||||
// (capped to last 8 entries, see share-store recordPublicSubscribe) so
|
||||
// the operator can see who subscribed.
|
||||
const { email } = req.body || {};
|
||||
if (!email || typeof email !== 'string' || !email.includes('@')) {
|
||||
throw new ValidationError('valid email required', 'email');
|
||||
let normalizedEmail = null;
|
||||
if (email !== undefined && email !== null) {
|
||||
const v = validatePublicEmail(email);
|
||||
if (!v.ok) throw new ValidationError(v.reason, 'email');
|
||||
normalizedEmail = v.email;
|
||||
}
|
||||
const result = await shareStore.recordPublicSubscribe(req.params.token);
|
||||
const result = await shareStore.recordPublicSubscribe(req.params.token, { email: normalizedEmail });
|
||||
if (!result.ok) {
|
||||
if (result.reason === 'not_found') throw new NotFoundError('share not found');
|
||||
throw new ValidationError(result.reason, 'share');
|
||||
@@ -323,12 +364,23 @@ module.exports = function shareRoutesFactory({
|
||||
res.json({ success: true, data: { count: result.count, cap: result.cap } });
|
||||
}, 'share-subscribe'));
|
||||
|
||||
router.post('/share/:token/redeem-tailscale', asyncHandler(async (req, res) => {
|
||||
router.post('/share/:token/redeem-tailscale', _sharePublicLimiter, asyncHandler(async (req, res) => {
|
||||
// DC-083: replace the bare `typeof deviceId === 'string'` check with a
|
||||
// charset/length/control-char-bounded validator. The pre-fix code
|
||||
// accepted arbitrary strings of any length — including CR/LF/NUL,
|
||||
// which flow into the Tailscale auth-key description string in
|
||||
// POST /share/tailscale (routes/share.js:213 in the issue path).
|
||||
// The redeem-tailscale path receives the deviceId from Caddy's
|
||||
// forward_auth (a Tailscale machine ID), which is base64url +
|
||||
// hyphens — well within the validator's charset.
|
||||
const { deviceId } = req.body || {};
|
||||
if (!deviceId || typeof deviceId !== 'string') {
|
||||
throw new ValidationError('deviceId required', 'deviceId');
|
||||
let normalizedDeviceId = null;
|
||||
if (deviceId !== undefined && deviceId !== null) {
|
||||
const v = validatePublicDeviceId(deviceId);
|
||||
if (!v.ok) throw new ValidationError(v.reason, 'deviceId');
|
||||
normalizedDeviceId = v.deviceId;
|
||||
}
|
||||
const result = await shareStore.recordTailscaleUse(req.params.token, { deviceId });
|
||||
const result = await shareStore.recordTailscaleUse(req.params.token, { deviceId: normalizedDeviceId });
|
||||
if (!result.ok) {
|
||||
if (result.reason === 'not_found') throw new NotFoundError('share not found');
|
||||
throw new ValidationError(result.reason, 'share');
|
||||
|
||||
@@ -41,12 +41,124 @@
|
||||
*
|
||||
* DELETE /api/v1/tailscale/admin/devices/:id
|
||||
* 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 { ok, errorResponse } = require('../src/utils/responses');
|
||||
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({
|
||||
tailscaleCoord,
|
||||
asyncHandler,
|
||||
@@ -75,9 +187,12 @@ module.exports = function({
|
||||
|
||||
router.put('/settings', asyncHandler(async (req, res) => {
|
||||
const token = req.body && req.body.apiToken;
|
||||
if (!token || typeof token !== 'string' || !token.startsWith('tskey-api-')) {
|
||||
return errorResponse(res, 400, 'Invalid API token (must start with tskey-api-)');
|
||||
}
|
||||
// DC-080: validate prefix + length cap. The pre-fix code only checked
|
||||
// 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
|
||||
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) => {
|
||||
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();
|
||||
if (token) {
|
||||
// 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');
|
||||
}
|
||||
const opts = req.body || {};
|
||||
// Reject obviously-bad input early
|
||||
if (opts.tags && !Array.isArray(opts.tags)) {
|
||||
return errorResponse(res, 400, 'tags must be an array of strings');
|
||||
}
|
||||
// Reject obviously-bad input early.
|
||||
// DC-080: pre-fix the route only checked `Array.isArray(opts.tags)`.
|
||||
// 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)) {
|
||||
return errorResponse(res, 400, 'expirySeconds must be a positive integer');
|
||||
}
|
||||
@@ -254,4 +386,10 @@ module.exports = function({
|
||||
}));
|
||||
|
||||
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;
|
||||
@@ -168,12 +168,39 @@ class UpdateManager extends EventEmitter {
|
||||
|
||||
/**
|
||||
* Get latest image digest from registry
|
||||
*
|
||||
* DC-082: when the image name is a docker-compose prefixed name like
|
||||
* `dashcaddy-dashcaddy-api:latest` (single hyphen-separated, no slash),
|
||||
* the existing code normalized it to `library/dashcaddy-dashcaddy-api`
|
||||
* before probing Docker Hub. The actual upstream namespace for a
|
||||
* compose-prefixed image is `<project>/<service>` (with slash) — Docker
|
||||
* Compose hyphenates the project name and service name when tagging
|
||||
* locally. The pre-fix code probed the wrong repo, Docker Hub returned
|
||||
* HTTP 401 (the repo doesn't exist), and the error log showed
|
||||
* `Docker Hub registry returned HTTP 401 after auth` on every restart
|
||||
* for the local dashcaddy-api image. The fix: split on the FIRST hyphen
|
||||
* for compose-prefixed names so the lookup targets the correct
|
||||
* namespace.
|
||||
*
|
||||
* Compose-prefixed shape: `^[a-z0-9]+-[a-z0-9][a-z0-9_-]*$` (no slash,
|
||||
* lowercase, both halves non-empty). Examples:
|
||||
* dashcaddy-dashcaddy-api -> dashcaddy/dashcaddy-api
|
||||
* myproject-myservice -> myproject/myservice
|
||||
* nginx -> library/nginx (official, unchanged)
|
||||
* library/nginx -> library/nginx (official, unchanged)
|
||||
* dashcaddy/some-image -> dashcaddy/some-image (already has slash)
|
||||
* ghcr.io/x/y -> ghcr.io/x/y (handled below)
|
||||
*/
|
||||
async getLatestImageDigest(imageName) {
|
||||
// DC-082: declare `remainder` at the function scope so the catch block
|
||||
// can classify the error against the image-name shape (compose-prefixed
|
||||
// local images produce a steady-state 401 that should log as info, not
|
||||
// error).
|
||||
let remainder = imageName;
|
||||
try {
|
||||
// Parse image name — strip any leading registry host first
|
||||
let imageTag = 'latest';
|
||||
let remainder = imageName;
|
||||
remainder = imageName;
|
||||
const lastColon = imageName.lastIndexOf(':');
|
||||
// Only treat as tag if the colon is AFTER the last slash (avoids `ghcr.io:443/...`)
|
||||
const lastSlash = imageName.lastIndexOf('/');
|
||||
@@ -187,8 +214,19 @@ class UpdateManager extends EventEmitter {
|
||||
return await this.getGhcrDigest(remainder, imageTag);
|
||||
}
|
||||
|
||||
// Docker Hub images (library/nginx OR org/image with single slash)
|
||||
if (!remainder.includes('/') || remainder.split('/').length === 2) {
|
||||
// Docker Hub images (library/nginx OR org/image with single slash).
|
||||
// Special-case docker-compose prefixed names (single hyphen, no slash,
|
||||
// lowercase) — split on the FIRST hyphen to recover the original
|
||||
// `<project>/<service>` namespace. See DC-082.
|
||||
if (!remainder.includes('/')) {
|
||||
const composeRepo = this._composeProjectToRepo(remainder);
|
||||
if (composeRepo) {
|
||||
return await this.getDockerHubDigest(composeRepo, imageTag);
|
||||
}
|
||||
// Not a compose-prefixed name — fall through to the library/ default
|
||||
return await this.getDockerHubDigest(remainder, imageTag);
|
||||
}
|
||||
if (remainder.split('/').length === 2) {
|
||||
return await this.getDockerHubDigest(remainder, imageTag);
|
||||
}
|
||||
|
||||
@@ -196,11 +234,72 @@ class UpdateManager extends EventEmitter {
|
||||
log.warn('update', 'Custom registry not yet supported', { remainder });
|
||||
return null;
|
||||
} catch (error) {
|
||||
// DC-082: a "registry returned HTTP 401 after auth" against a
|
||||
// compose-prefixed local image is the steady-state when the image
|
||||
// is built locally and the upstream namespace on Docker Hub
|
||||
// doesn't exist (or is private). The token endpoint returns 200
|
||||
// with an empty-access JWT, and the authed manifest GET 401s.
|
||||
// Log these as a clean info not-found line instead of an error
|
||||
// so dashboards and PagerDuty don't fire on every restart.
|
||||
if (this._isNotPublishedError(error, remainder)) {
|
||||
log.info('update', 'No upstream registry image — skipping update check', { imageName, remainder });
|
||||
return null;
|
||||
}
|
||||
log.error('update', error, null, { imageName });
|
||||
return null;
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* DC-082: split a docker-compose prefixed image name on the FIRST hyphen
|
||||
* to recover the original `<project>/<service>` namespace. Returns null
|
||||
* for names that don't match the compose-prefixed shape — callers fall
|
||||
* through to the standard library/-prefixed official-image path.
|
||||
*
|
||||
* Compose-prefixed shape:
|
||||
* - Contains exactly one or more hyphens
|
||||
* - No slash
|
||||
* - Lowercase letters / digits / hyphens / underscores only
|
||||
* - Both halves (before first hyphen, after first hyphen) are non-empty
|
||||
* - First char is a letter or digit (not a hyphen)
|
||||
*/
|
||||
_composeProjectToRepo(remainder) {
|
||||
if (typeof remainder !== 'string' || remainder.length === 0) return null;
|
||||
if (remainder.includes('/')) return null; // already namespaced
|
||||
if (!/^[a-z0-9][a-z0-9_-]*-[a-z0-9][a-z0-9_-]*$/i.test(remainder)) {
|
||||
// Not a compose-prefixed name — let the library/ path handle it
|
||||
// (this is the official-image path: e.g. `nginx`, `alpine`).
|
||||
return null;
|
||||
}
|
||||
const firstHyphen = remainder.indexOf('-');
|
||||
// Defensive: indexOf must find a hyphen (regex requires it), but guard
|
||||
// against any future regex drift.
|
||||
if (firstHyphen <= 0 || firstHyphen === remainder.length - 1) return null;
|
||||
const project = remainder.substring(0, firstHyphen);
|
||||
const service = remainder.substring(firstHyphen + 1);
|
||||
if (!project || !service) return null;
|
||||
return `${project}/${service}`;
|
||||
}
|
||||
|
||||
/**
|
||||
* DC-082: detect the "registry returned 401 after auth" pattern that
|
||||
* signals "this image has no public upstream on Docker Hub" (as opposed
|
||||
* to a genuine auth failure or transient network error). Steady-state
|
||||
* for compose-prefixed local images that aren't published.
|
||||
*/
|
||||
_isNotPublishedError(error, remainder) {
|
||||
if (!error || typeof error.message !== 'string') return false;
|
||||
if (!error.message.includes('HTTP 401')) return false;
|
||||
// Constrain to the compose-prefixed path — a real auth failure on a
|
||||
// legitimate `library/foo` or `namespace/foo` probe should still log
|
||||
// as an error (it never auto-heals).
|
||||
if (typeof remainder !== 'string' || remainder.includes('/')) return false;
|
||||
if (!/^[a-z0-9][a-z0-9_-]*-[a-z0-9][a-z0-9_-]*$/i.test(remainder)) {
|
||||
return false;
|
||||
}
|
||||
return true;
|
||||
}
|
||||
|
||||
/**
|
||||
* Get image digest from GitHub Container Registry (ghcr.io)
|
||||
* Public images are tokenless via the registry-1.docker.io-style bearer flow,
|
||||
|
||||
@@ -47,6 +47,53 @@ const ALLOWED_PUBLIC_TTLS = new Set([60 * 60 * 1000, 24 * 60 * 60 * 1000, 7 * 24
|
||||
const TAILSCALE_MAX_USES = 1;
|
||||
const PUBLIC_DEFAULT_SUBSCRIBE_CAP = 1000; // bound on subscribe events per link
|
||||
|
||||
// DC-083: Public share endpoint input bounds. The two CSRF-exempt public
|
||||
// endpoints accept untrusted body fields — bound shape, length, charset so
|
||||
// an attacker can't bloat data/shares.json, inject CRLF into fields that
|
||||
// flow into Tailscale auth-key descriptions, or smuggle control chars into
|
||||
// the on-disk store. See routes/share.js for the route-layer validation;
|
||||
// these helpers are the defense-in-depth belt under the route's suspenders.
|
||||
const PUBLIC_EMAIL_REGEX = /^[a-zA-Z0-9._%+-]+@[a-zA-Z0-9.-]+\.[a-zA-Z]{2,}$/;
|
||||
const PUBLIC_EMAIL_MAX_LENGTH = 254; // RFC 5321 §4.5.3.1.3
|
||||
const PUBLIC_DEVICE_ID_REGEX = /^[a-zA-Z0-9._:-]+$/;
|
||||
const PUBLIC_DEVICE_ID_MIN_LENGTH = 1;
|
||||
const PUBLIC_DEVICE_ID_MAX_LENGTH = 128;
|
||||
|
||||
function validatePublicEmail(raw) {
|
||||
if (typeof raw !== 'string') return { ok: false, reason: 'invalid_email' };
|
||||
// Reject control chars / NUL / CR / LF before they can corrupt the on-disk
|
||||
// JSON or be embedded in subsequent log lines. RFC 5321 forbids these in
|
||||
// SMTP addresses; we mirror that at the API layer.
|
||||
if (raw.length === 0 || raw.length > PUBLIC_EMAIL_MAX_LENGTH) {
|
||||
return { ok: false, reason: 'invalid_email' };
|
||||
}
|
||||
// eslint-disable-next-line no-control-regex
|
||||
if (/[\x00-\x1f\x7f]/.test(raw)) return { ok: false, reason: 'invalid_email' };
|
||||
// The local-part can technically contain `+`, `.`, `_`, `%`, `-`; the
|
||||
// domain part must have at least one dot and a 2+ letter TLD. Reject
|
||||
// quote-bracket forms (RFC 5321 obs-quote-text) — we don't accept them.
|
||||
if (!PUBLIC_EMAIL_REGEX.test(raw)) return { ok: false, reason: 'invalid_email' };
|
||||
// Block obvious shell-attachment characters that the regex doesn't catch.
|
||||
if (/[<>{}|\\^`\s]/.test(raw)) return { ok: false, reason: 'invalid_email' };
|
||||
return { ok: true, email: raw.toLowerCase() };
|
||||
}
|
||||
|
||||
function validatePublicDeviceId(raw) {
|
||||
if (typeof raw !== 'string') return { ok: false, reason: 'invalid_device_id' };
|
||||
if (raw.length < PUBLIC_DEVICE_ID_MIN_LENGTH || raw.length > PUBLIC_DEVICE_ID_MAX_LENGTH) {
|
||||
return { ok: false, reason: 'invalid_device_id' };
|
||||
}
|
||||
// Tailscale machine IDs are base64url-with-hyphens; we accept a slightly
|
||||
// broader charset (`._:-`) to also accommodate hostname-style IDs and
|
||||
// Caddy's `forward_auth` device headers. Reject CR/LF/NUL/TAB explicitly
|
||||
// so a smuggled control char can't break out of the Tailscale auth-key
|
||||
// description string in routes/share.js:213.
|
||||
// eslint-disable-next-line no-control-regex
|
||||
if (/[\x00-\x1f\x7f]/.test(raw)) return { ok: false, reason: 'invalid_device_id' };
|
||||
if (!PUBLIC_DEVICE_ID_REGEX.test(raw)) return { ok: false, reason: 'invalid_device_id' };
|
||||
return { ok: true, deviceId: raw };
|
||||
}
|
||||
|
||||
function _nowMs() { return Date.now(); }
|
||||
function _nowIso() { return new Date().toISOString(); }
|
||||
|
||||
@@ -327,8 +374,18 @@ function createShareStore(opts = {}) {
|
||||
});
|
||||
}
|
||||
|
||||
function recordPublicSubscribe(token) {
|
||||
function recordPublicSubscribe(token, { email } = {}) {
|
||||
return _enqueue(() => {
|
||||
// DC-083: validate the optional subscriber email at the store layer too.
|
||||
// The route layer validates first; this is the defense-in-depth catch
|
||||
// for direct callers (cron sweepers, internal jobs, future endpoints).
|
||||
// `email` is OPT-IN — callers omitting it get the original behavior.
|
||||
let normalizedEmail = null;
|
||||
if (email !== undefined && email !== null) {
|
||||
const v = validatePublicEmail(email);
|
||||
if (!v.ok) return { ok: false, reason: v.reason };
|
||||
normalizedEmail = v.email;
|
||||
}
|
||||
const data = _load();
|
||||
const hash = _sha256(token);
|
||||
const s = _findByHash(data, hash);
|
||||
@@ -341,6 +398,16 @@ function createShareStore(opts = {}) {
|
||||
const cap = s.subscribeCap || PUBLIC_DEFAULT_SUBSCRIBE_CAP;
|
||||
if (s.subscribeCount >= cap) return { ok: false, reason: 'cap_reached' };
|
||||
s.subscribeCount += 1;
|
||||
// DC-083: record the last submitting email (capped to 8 entries to
|
||||
// bound the on-disk size). PII minimization — we keep only the hash
|
||||
// + last 8 emails; full email log would grow unbounded.
|
||||
if (normalizedEmail) {
|
||||
if (!Array.isArray(s.subscriberEmails)) s.subscriberEmails = [];
|
||||
s.subscriberEmails.push(normalizedEmail);
|
||||
if (s.subscriberEmails.length > 8) {
|
||||
s.subscriberEmails.splice(0, s.subscriberEmails.length - 8);
|
||||
}
|
||||
}
|
||||
_save(data);
|
||||
return { ok: true, count: s.subscribeCount, cap };
|
||||
});
|
||||
@@ -348,6 +415,18 @@ function createShareStore(opts = {}) {
|
||||
|
||||
function recordTailscaleUse(token, { deviceId } = {}) {
|
||||
return _enqueue(() => {
|
||||
// DC-083: validate deviceId at the store layer. The pre-fix code
|
||||
// accepted ANY string of any length, including control chars and
|
||||
// CR/LF — which would flow into the Tailscale auth-key description
|
||||
// (routes/share.js:213) and into the on-disk shares.json. Reject
|
||||
// early so an attacker can't bloat the store or smuggle characters
|
||||
// out of the Tailscale description field.
|
||||
let normalizedDeviceId = 'unknown';
|
||||
if (deviceId !== undefined && deviceId !== null) {
|
||||
const v = validatePublicDeviceId(deviceId);
|
||||
if (!v.ok) return { ok: false, reason: v.reason };
|
||||
normalizedDeviceId = v.deviceId;
|
||||
}
|
||||
const data = _load();
|
||||
const hash = _sha256(token);
|
||||
const s = _findByHash(data, hash);
|
||||
@@ -359,7 +438,7 @@ function createShareStore(opts = {}) {
|
||||
return { ok: false, reason: 'expired' };
|
||||
}
|
||||
s.usedAt = _nowIso();
|
||||
s.usedBy = typeof deviceId === 'string' ? deviceId : 'unknown';
|
||||
s.usedBy = normalizedDeviceId;
|
||||
_save(data);
|
||||
return { ok: true, share: _publicView(s) };
|
||||
});
|
||||
@@ -411,4 +490,4 @@ function createShareStore(opts = {}) {
|
||||
};
|
||||
}
|
||||
|
||||
module.exports = { createShareStore };
|
||||
module.exports = { createShareStore, validatePublicEmail, validatePublicDeviceId };
|
||||
@@ -79,6 +79,17 @@ const RATE_LIMITS = {
|
||||
windowMs: 15 * 60 * 1000,
|
||||
max: 10,
|
||||
},
|
||||
// DC-083: Public share endpoint limiter. The two CSRF-exempt public
|
||||
// endpoints (POST /share/:token/subscribe + POST /share/:token/redeem-tailscale)
|
||||
// mutate on-disk state (data/shares.json). Bound them tighter than the
|
||||
// general limiter (1000/15min) so a single attacker can't bloat the
|
||||
// store or saturate the tmp+rename writer. 30/15min is enough for a
|
||||
// legitimate user clicking "subscribe" once or twice — anything beyond
|
||||
// is abuse.
|
||||
SHARE_PUBLIC: {
|
||||
windowMs: 15 * 60 * 1000,
|
||||
max: 30,
|
||||
},
|
||||
};
|
||||
|
||||
// ── Caddy ─────────────────────────────────────────────────────
|
||||
|
||||
Reference in New Issue
Block a user