Compare commits
3
Commits
23922923a5
...
dc/DC-058
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
8105bed3fb | ||
|
|
2f76b83565 | ||
|
|
0714bf2334 |
@@ -322,6 +322,89 @@ describe('CSRF Protection', () => {
|
||||
|
||||
process.env.NODE_ENV = origEnv;
|
||||
});
|
||||
|
||||
// DC-058: differentiate "browser auto-retry" from "real probe" by the
|
||||
// presence of the X-CSRF-Token header. The 403 response is identical in
|
||||
// both branches; only the stderr log tag changes.
|
||||
describe('DC-058: browser-auto-retry vs real-probe log tagging', () => {
|
||||
let stderrSpy;
|
||||
let origEnv;
|
||||
|
||||
beforeEach(() => {
|
||||
origEnv = process.env.NODE_ENV;
|
||||
process.env.NODE_ENV = 'production';
|
||||
stderrSpy = jest.spyOn(process.stderr, 'write').mockImplementation(() => true);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
process.env.NODE_ENV = origEnv;
|
||||
stderrSpy.mockRestore();
|
||||
});
|
||||
|
||||
it('tags missing-cookie with [CSRF-debug] when X-CSRF-Token header also present (browser auto-retry)', () => {
|
||||
const { req, res, next } = createMockReqRes({
|
||||
method: 'POST', path: '/api/v1/backups/schedule',
|
||||
headers: { cookie: '', 'x-csrf-token': 'some-signature-attempt' }
|
||||
});
|
||||
csrfValidationMiddleware(req, res, next);
|
||||
|
||||
// 403 response unchanged
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(res.json).toHaveBeenCalledWith(
|
||||
expect.objectContaining({ error: expect.stringContaining('DC-100') })
|
||||
);
|
||||
// Log tag is [CSRF-debug]
|
||||
expect(stderrSpy).toHaveBeenCalled();
|
||||
const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0];
|
||||
expect(lastWrite).toContain('[CSRF-debug]');
|
||||
expect(lastWrite).toContain('browser auto-retry');
|
||||
expect(lastWrite).not.toMatch(/^\[CSRF\][^-]/); // not bare [CSRF]
|
||||
});
|
||||
|
||||
it('tags missing-cookie with [CSRF] when no X-CSRF-Token header present (real probe)', () => {
|
||||
const { req, res, next } = createMockReqRes({
|
||||
method: 'POST', path: '/api/v1/backups/schedule',
|
||||
headers: { cookie: '' }
|
||||
});
|
||||
csrfValidationMiddleware(req, res, next);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(stderrSpy).toHaveBeenCalled();
|
||||
const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0];
|
||||
expect(lastWrite).toContain('[CSRF]');
|
||||
expect(lastWrite).not.toContain('[CSRF-debug]');
|
||||
expect(lastWrite).not.toContain('browser auto-retry');
|
||||
});
|
||||
|
||||
it('keeps [CSRF] tag when cookie present but header missing (curl probe with manual cookie)', () => {
|
||||
const nonce = generateToken();
|
||||
const { req, res, next } = createMockReqRes({
|
||||
method: 'POST', path: '/api/v1/backups/schedule',
|
||||
headers: { cookie: `${CSRF_COOKIE_NAME}=${nonce}` }
|
||||
});
|
||||
csrfValidationMiddleware(req, res, next);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
expect(stderrSpy).toHaveBeenCalled();
|
||||
const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0];
|
||||
expect(lastWrite).toContain('[CSRF]');
|
||||
expect(lastWrite).not.toContain('[CSRF-debug]');
|
||||
});
|
||||
|
||||
it('headerToken is read from x-csrf-token (lowercased by Express) — lowercase header triggers [CSRF-debug]', () => {
|
||||
// Express/Node lowercases all incoming header keys, so production code
|
||||
// only ever sees lowercase. We test the exact code path here.
|
||||
const { req, res, next } = createMockReqRes({
|
||||
method: 'POST', path: '/api/v1/backups/schedule',
|
||||
headers: { cookie: '', 'x-csrf-token': 'some-signature-attempt' }
|
||||
});
|
||||
csrfValidationMiddleware(req, res, next);
|
||||
|
||||
expect(res.status).toHaveBeenCalledWith(403);
|
||||
const lastWrite = stderrSpy.mock.calls[stderrSpy.mock.calls.length - 1][0];
|
||||
expect(lastWrite).toContain('[CSRF-debug]');
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
describe('renewCSRFToken', () => {
|
||||
|
||||
@@ -0,0 +1,218 @@
|
||||
/**
|
||||
* DC-057: dead-shadow /backups/schedule handler removed.
|
||||
*
|
||||
* The duplicate `router.post('/backups/schedule', ...)` previously registered
|
||||
* far below the canonical one was unreachable (Express matches the first
|
||||
* registered handler per METHOD+PATH). It bypassed `premiumGating` and
|
||||
* `validateBody` and used a `name`-keyed schema that would have corrupted the
|
||||
* backup config if it ever ran. The canonical handler uses the error code
|
||||
* `backups-schedule-update`; the dead handler used `backups-schedule-legacy`.
|
||||
* This test proves:
|
||||
*
|
||||
* 1. The router registers exactly ONE POST /backups/schedule handler
|
||||
* (the canonical, appId-keyed one).
|
||||
* 2. No handler references the legacy "backups-schedule-legacy" error code.
|
||||
* 3. The legacy "name"-keyed schema now produces a 400 ValidationError
|
||||
* from the canonical Joi schema (dead handler is gone).
|
||||
* 4. The canonical appId-keyed schema still succeeds (200).
|
||||
* 5. premiumGating is enforced on the canonical POST.
|
||||
*
|
||||
* Mirrors the audit-log.routes.test.js pattern.
|
||||
*/
|
||||
|
||||
const express = require('express');
|
||||
|
||||
function buildFakeBackupManager() {
|
||||
const config = { backups: {}, defaultRetention: { keep: 7 } };
|
||||
return {
|
||||
getConfig: jest.fn(() => config),
|
||||
updateConfig: jest.fn((next) => {
|
||||
config.backups = next.backups || {};
|
||||
}),
|
||||
getHistory: jest.fn(() => []),
|
||||
restoreBackup: jest.fn(async (id) => {
|
||||
// Suppress require-await — keep async shape for parity with the
|
||||
// real backupManager.restoreBackup contract.
|
||||
return Promise.resolve({ id, status: 'restored' });
|
||||
}),
|
||||
};
|
||||
}
|
||||
|
||||
function buildFakeLicenseManager() {
|
||||
const requirePremium = jest.fn(() => (_req, _res, next) => next());
|
||||
return {
|
||||
requirePremium,
|
||||
isPremium: jest.fn(() => true),
|
||||
};
|
||||
}
|
||||
|
||||
function buildRouter(licenseManager, backupManager) {
|
||||
// Reset module cache so each test starts fresh
|
||||
jest.resetModules();
|
||||
const mod = require('../../routes/backups');
|
||||
return mod({
|
||||
backupManager,
|
||||
licenseManager,
|
||||
asyncHandler: (fn) => async (req, res, next) => { // eslint-disable-line require-await
|
||||
try { await fn(req, res, next); } catch (e) { next(e); }
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
function buildApp(router) {
|
||||
// Catch-all error handler so ValidationError / NotFoundError become JSON
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, res, next) => {
|
||||
// intentionally strip auth — the test does not exercise it
|
||||
next();
|
||||
});
|
||||
app.use('/', router);
|
||||
// eslint-disable-next-line no-unused-vars
|
||||
app.use((err, req, res, next) => {
|
||||
const status = err.statusCode || err.status || 500;
|
||||
res.status(status).json({
|
||||
error: err.message,
|
||||
code: err.code || 'ERR',
|
||||
});
|
||||
});
|
||||
return app;
|
||||
}
|
||||
|
||||
function supertestFetch(app) {
|
||||
// Tiny in-process fetch helper (no need to add supertest dep)
|
||||
const http = require('http');
|
||||
return function (method, path, body) {
|
||||
return new Promise((resolve, reject) => {
|
||||
const server = app.listen(0, () => {
|
||||
const { port } = server.address();
|
||||
const data = body ? JSON.stringify(body) : null;
|
||||
const req = http.request({
|
||||
method,
|
||||
hostname: '127.0.0.1',
|
||||
port,
|
||||
path,
|
||||
headers: data ? { 'Content-Type': 'application/json', 'Content-Length': Buffer.byteLength(data) } : {},
|
||||
}, (res) => {
|
||||
let chunks = '';
|
||||
res.on('data', (c) => { chunks += c; });
|
||||
res.on('end', () => {
|
||||
server.close();
|
||||
let parsed;
|
||||
try { parsed = JSON.parse(chunks); } catch { parsed = chunks; }
|
||||
resolve({ status: res.statusCode, body: parsed });
|
||||
});
|
||||
});
|
||||
req.on('error', (e) => { server.close(); reject(e); });
|
||||
if (data) req.write(data);
|
||||
req.end();
|
||||
});
|
||||
});
|
||||
};
|
||||
}
|
||||
|
||||
describe('routes/backups POST /backups/schedule (DC-057)', () => {
|
||||
let backupManager, licenseManager, app, fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
backupManager = buildFakeBackupManager();
|
||||
licenseManager = buildFakeLicenseManager();
|
||||
const router = buildRouter(licenseManager, backupManager);
|
||||
app = buildApp(router);
|
||||
fetch = supertestFetch(app);
|
||||
});
|
||||
|
||||
test('registers exactly ONE POST /backups/schedule handler (canonical)', () => {
|
||||
// Inspect the registered router layers and confirm only one POST /backups/schedule
|
||||
// route exists (no shadowed / unreachable duplicate).
|
||||
const router = buildRouter(licenseManager, backupManager);
|
||||
const seen = [];
|
||||
router.stack.forEach((layer) => {
|
||||
if (layer.route && layer.route.path === '/backups/schedule' && layer.route.methods.post) {
|
||||
seen.push(layer.route);
|
||||
}
|
||||
});
|
||||
expect(seen).toHaveLength(1);
|
||||
});
|
||||
|
||||
test('no handler references the legacy "backups-schedule-legacy" error code', () => {
|
||||
// The canonical handler uses error code 'backups-schedule-update'.
|
||||
// Walk the router stack and assert no route uses the legacy error code.
|
||||
const router = buildRouter(licenseManager, backupManager);
|
||||
const handlerStrings = [];
|
||||
function walk(node) {
|
||||
if (!node) return;
|
||||
if (node.stack) node.stack.forEach(walk);
|
||||
if (node.handle) {
|
||||
const code = node.handle.toString();
|
||||
handlerStrings.push(code);
|
||||
}
|
||||
}
|
||||
walk(router);
|
||||
const all = handlerStrings.join('\n');
|
||||
expect(all).not.toContain('backups-schedule-legacy');
|
||||
});
|
||||
|
||||
test('legacy name-keyed schema is REJECTED with 400 (dead route truly gone)', async () => {
|
||||
// The dead handler accepted { name, schedule, maxStorageBytes, ...backupConfig }.
|
||||
// After removal, the canonical Joi schema (backupScheduleCreate) rejects this
|
||||
// shape because it requires `appId`. So we expect a 400.
|
||||
const res = await fetch('POST', '/backups/schedule', {
|
||||
name: 'mybackup',
|
||||
schedule: 'daily',
|
||||
maxStorageBytes: 1024,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/appId.*required|appId is required/i);
|
||||
});
|
||||
|
||||
test('canonical appId-keyed schema SUCCEEDS (200) and writes backup config', async () => {
|
||||
const res = await fetch('POST', '/backups/schedule', {
|
||||
appId: 'plex',
|
||||
schedule: 'daily',
|
||||
retention: { keep: 7 },
|
||||
destination: 'local',
|
||||
destinationPath: '/var/backups/plex',
|
||||
maxStorageBytes: 1024,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.success).toBe(true);
|
||||
expect(backupManager.updateConfig).toHaveBeenCalledTimes(1);
|
||||
const written = backupManager.updateConfig.mock.calls[0][0];
|
||||
expect(written.backups).toHaveProperty('plex');
|
||||
expect(written.backups.plex.schedule).toBe('daily');
|
||||
expect(written.backups.plex.enabled).toBe(true);
|
||||
expect(written.backups.plex.maxStorageBytes).toBe(1024);
|
||||
});
|
||||
|
||||
test('premium gating is enforced on POST /backups/schedule', async () => {
|
||||
// Replace the premium gate with one that 403s, then verify it runs.
|
||||
licenseManager.requirePremium.mockReturnValueOnce(
|
||||
(_req, res) => res.status(403).json({ error: 'premium required' }),
|
||||
);
|
||||
const router = buildRouter(licenseManager, backupManager);
|
||||
app = buildApp(router);
|
||||
fetch = supertestFetch(app);
|
||||
const res = await fetch('POST', '/backups/schedule', {
|
||||
appId: 'plex',
|
||||
schedule: 'daily',
|
||||
});
|
||||
expect(res.status).toBe(403);
|
||||
expect(backupManager.updateConfig).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('GET /backups/schedule still works (no collateral damage)', async () => {
|
||||
const res = await fetch('GET', '/backups/schedule');
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.success).toBe(true);
|
||||
expect(res.body).toHaveProperty('schedules');
|
||||
});
|
||||
|
||||
test('DELETE /backups/schedule/:appId still works', async () => {
|
||||
// Seed the config so the delete has something to remove
|
||||
backupManager.getConfig().backups.plex = { schedule: 'daily' };
|
||||
const res = await fetch('DELETE', '/backups/schedule/plex');
|
||||
expect(res.status).toBe(200);
|
||||
expect(backupManager.updateConfig).toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
@@ -22,6 +22,13 @@ module.exports = function({ backupManager, licenseManager, asyncHandler }) {
|
||||
const router = express.Router();
|
||||
|
||||
// ==================== SCHEDULE ENDPOINTS (PREMIUM) ====================
|
||||
// NOTE: POST /backups/schedule has a single canonical registration below
|
||||
// (the appId-keyed handler at the top of this section). Earlier versions
|
||||
// registered a duplicate "name"-keyed handler later in the file — Express
|
||||
// only matches the first registered handler per METHOD+PATH, so the
|
||||
// duplicate was unreachable dead code. Do not re-add it; if you need a
|
||||
// different schema, change the canonical Joi schema in
|
||||
// src/utilities/validate.js (backupScheduleCreate) instead.
|
||||
|
||||
// Apply premium gating to schedule-related routes
|
||||
const premiumGating = licenseManager.requirePremium('auto-backup');
|
||||
@@ -511,38 +518,6 @@ module.exports = function({ backupManager, licenseManager, asyncHandler }) {
|
||||
success(res, storageInfo);
|
||||
}, 'backups-storage-info'));
|
||||
|
||||
// Schedule a backup
|
||||
// LEGACY: this is a duplicate registration of POST /backups/schedule (see also line 60,
|
||||
// which uses the appId-keyed schema and is the route the frontend actually calls).
|
||||
// Express only matches the first registered handler per METHOD+PATH, so this handler
|
||||
// is unreachable. It is preserved for now to avoid removing a route any unknown
|
||||
// integration might still POST to. TODO: audit + remove in a dedicated cleanup PR.
|
||||
router.post('/backups/schedule', asyncHandler(async (req, res) => {
|
||||
const { name, schedule, maxStorageBytes, ...backupConfig } = req.body;
|
||||
|
||||
if (!name || !schedule) {
|
||||
return res.status(400).json({ error: 'name and schedule are required' });
|
||||
}
|
||||
|
||||
const config = backupManager.getConfig();
|
||||
|
||||
// Store maxStorageBytes in the backup config (converted to bytes)
|
||||
const maxBytes = typeof maxStorageBytes === 'number' && maxStorageBytes > 0
|
||||
? maxStorageBytes
|
||||
: (typeof maxStorageBytes === 'string' ? parseStorageSize(maxStorageBytes) : 0);
|
||||
|
||||
config.backups[name] = {
|
||||
...backupConfig,
|
||||
enabled: true,
|
||||
schedule,
|
||||
maxStorageBytes: maxBytes,
|
||||
destinations: backupConfig.destinations || [{ type: 'local' }]
|
||||
};
|
||||
|
||||
backupManager.updateConfig(config);
|
||||
success(res, { message: `Backup '${name}' scheduled`, maxStorageBytes: maxBytes });
|
||||
}, 'backups-schedule-legacy'));
|
||||
|
||||
// Restore from backup
|
||||
router.post('/backups/restore/:backupId', validateBody(schemas.backupRestore), asyncHandler(async (req, res) => {
|
||||
const result = await backupManager.restoreBackup(req.params.backupId, req.body);
|
||||
|
||||
@@ -214,14 +214,30 @@ function csrfValidationMiddleware(req, res, next) {
|
||||
return next();
|
||||
}
|
||||
|
||||
// Validate both values exist
|
||||
// DC-058: differentiate "browser auto-retry" from "real probe" using the
|
||||
// X-CSRF-Token header as a signal. The dashboard JS in status/js/globals.js
|
||||
// secureFetch() pre-fetches /api/v1/csrf-token (which sets the CSRF cookie
|
||||
// via csrfCookieMiddleware) before posting; if the GET raced with container
|
||||
// restart OR the user cleared cookies mid-session, the POST can arrive with
|
||||
// a header but no cookie. secureFetch catches the 403 and auto-retries
|
||||
// with a fresh token (lines 225-238 of globals.js). For these "has header
|
||||
// but no cookie" misses, tag the log line [CSRF-debug] — operators can
|
||||
// grep them out as expected noise. A request with NEITHER cookie NOR
|
||||
// header (curl probe, exploit scanner, broken client) keeps the louder
|
||||
// [CSRF] tag.
|
||||
if (!cookieNonce) {
|
||||
process.stderr.write(`[CSRF] Missing CSRF cookie: ${method} ${req.path} from ${req.ip}\n`);
|
||||
const isLikelyBrowserAutoRetry = !!headerToken;
|
||||
const tag = isLikelyBrowserAutoRetry ? '[CSRF-debug]' : '[CSRF]';
|
||||
process.stderr.write(`${tag} Missing CSRF cookie: ${method} ${req.path} from ${req.ip}` +
|
||||
(isLikelyBrowserAutoRetry ? ' (browser auto-retry — header present, expect self-heal)' : '') + '\n');
|
||||
return errorResponse(res, 403, '[DC-100] CSRF token missing', {
|
||||
message: 'CSRF cookie not found. Please refresh the page (Ctrl+Shift+R) and try again.'
|
||||
});
|
||||
}
|
||||
|
||||
// Cookie present but no header — a real browser POST always sends both, so
|
||||
// header-less is suspicious (curl probe with manual cookie, misconfigured
|
||||
// client). Keep WARN level.
|
||||
if (!headerToken) {
|
||||
process.stderr.write(`[CSRF] Missing CSRF header: ${method} ${req.path} from ${req.ip}\n`);
|
||||
return errorResponse(res, 403, '[DC-100] CSRF token missing', {
|
||||
|
||||
Reference in New Issue
Block a user