Compare commits
8
Commits
| Author | SHA1 | Date | |
|---|---|---|---|
|
|
71d20ceef3 | ||
|
|
87f76aef66 | ||
|
|
8105bed3fb | ||
|
|
2f76b83565 | ||
|
|
0714bf2334 | ||
|
|
23922923a5 | ||
|
|
3137d4c16d | ||
|
|
ab87c10355 |
@@ -336,32 +336,77 @@ describe('AutoRestartManager', () => {
|
||||
});
|
||||
|
||||
describe('_resolveContainerId', () => {
|
||||
test('returns containerId from status.details when present', () => {
|
||||
test('returns containerId from status.details when present', async () => {
|
||||
const { manager } = makeManager();
|
||||
const cid = manager._resolveContainerId('svc-1', { details: { containerId: 'cid-details' } });
|
||||
const cid = await manager._resolveContainerId('svc-1', { details: { containerId: 'cid-details' } });
|
||||
expect(cid).toBe('cid-details');
|
||||
});
|
||||
|
||||
test('falls back to healthChecker.config.services[serviceId].containerId', () => {
|
||||
test('falls back to healthChecker.config.services[serviceId].containerId', async () => {
|
||||
const { manager, healthChecker } = makeManager();
|
||||
healthChecker.config = { services: { 'svc-1': { containerId: 'cid-hc' } } };
|
||||
const cid = manager._resolveContainerId('svc-1', { details: {} });
|
||||
const cid = await manager._resolveContainerId('svc-1', { details: {} });
|
||||
expect(cid).toBe('cid-hc');
|
||||
});
|
||||
|
||||
test('falls back to servicesStateManager.read when sync list is returned', () => {
|
||||
test('DC-060: awaits async servicesStateManager.read() and resolves containerId', async () => {
|
||||
// Regression test for the auto-restart silently no-op bug:
|
||||
// _resolveContainerId used to fire servicesStateManager.read() via
|
||||
// .then(...) and discard the result. Callers gated on the return
|
||||
// value, so a healthy→unhealthy transition whose only containerId
|
||||
// source was the async state manager never triggered handleContainerDown.
|
||||
const { manager, servicesStateManager } = makeManager();
|
||||
servicesStateManager.read.mockReturnValue([
|
||||
servicesStateManager.read.mockResolvedValue([
|
||||
{ id: 'svc-1', containerId: 'cid-state' },
|
||||
]);
|
||||
const cid = manager._resolveContainerId('svc-1', { details: {} });
|
||||
const cid = await manager._resolveContainerId('svc-1', { details: {} });
|
||||
expect(cid).toBe('cid-state');
|
||||
});
|
||||
|
||||
test('returns null when no source has a containerId', () => {
|
||||
test('returns null when no source has a containerId', async () => {
|
||||
const { manager } = makeManager();
|
||||
const cid = manager._resolveContainerId('svc-unknown', { details: {} });
|
||||
const cid = await manager._resolveContainerId('svc-unknown', { details: {} });
|
||||
expect(cid).toBeNull();
|
||||
});
|
||||
|
||||
test('swallows servicesStateManager.read() rejection', async () => {
|
||||
const { manager, servicesStateManager } = makeManager();
|
||||
servicesStateManager.read.mockRejectedValue(new Error('disk gone'));
|
||||
const cid = await manager._resolveContainerId('svc-1', { details: {} });
|
||||
expect(cid).toBeNull();
|
||||
});
|
||||
});
|
||||
|
||||
describe('DC-060: healthy→unhealthy transitions trigger restart via async lookup', () => {
|
||||
test('handleContainerDown is invoked with containerId from async state-manager lookup', async () => {
|
||||
// End-to-end: containerId comes ONLY from servicesStateManager.read()
|
||||
// (the production path for services.json-backed deployments).
|
||||
const { manager, docker, servicesStateManager } = makeManager();
|
||||
docker.client.getContainer.mockReturnValue({
|
||||
start: jest.fn().mockResolvedValue(undefined),
|
||||
});
|
||||
await manager.setPolicy('svc-1', { maxRetries: 3, retryIntervalMs: 0 });
|
||||
manager._previousHealth.set('svc-1', 'up');
|
||||
servicesStateManager.read.mockResolvedValue([
|
||||
{ id: 'svc-1', containerId: 'cid-from-state' },
|
||||
]);
|
||||
|
||||
const handleDownSpy = jest.spyOn(manager, 'handleContainerDown');
|
||||
await manager._handleStatusCheck({ serviceId: 'svc-1', status: 'down' });
|
||||
expect(handleDownSpy).toHaveBeenCalledWith('svc-1', 'cid-from-state');
|
||||
});
|
||||
|
||||
test('handleContainerDown is NOT invoked when async lookup returns no containerId', async () => {
|
||||
const { manager, servicesStateManager } = makeManager();
|
||||
await manager.setPolicy('svc-1', { maxRetries: 3, retryIntervalMs: 0 });
|
||||
manager._previousHealth.set('svc-1', 'up');
|
||||
servicesStateManager.read.mockResolvedValue([
|
||||
{ id: 'svc-1' /* no containerId */ },
|
||||
]);
|
||||
|
||||
const handleDownSpy = jest.spyOn(manager, 'handleContainerDown');
|
||||
await manager._handleStatusCheck({ serviceId: 'svc-1', status: 'down' });
|
||||
expect(handleDownSpy).not.toHaveBeenCalled();
|
||||
});
|
||||
});
|
||||
});
|
||||
|
||||
@@ -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();
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,277 @@
|
||||
/**
|
||||
* DC-059: disk-space POST /config threshold-ordering invariant.
|
||||
*
|
||||
* DiskSpaceMonitor._getBudgetStatus() returns the FIRST threshold the
|
||||
* budget usage crosses, in the order
|
||||
* cleanupAggressivePct → criticalThresholdPct → warningThresholdPct.
|
||||
* If a caller writes the three thresholds out of order
|
||||
* (e.g. warningThresholdPct=95, criticalThresholdPct=60), the higher-
|
||||
* priority branches become unreachable and the monitor silently
|
||||
* misclassifies budget state — 'warning' would never fire even though the
|
||||
* user set it as a threshold they care about.
|
||||
*
|
||||
* The fix lives in `routes/disk-space.js`: a `mergeAndCheckOrdering()`
|
||||
* helper validates the *effective* (merged with live baseline) config
|
||||
* against the invariant `warningThresholdPct < criticalThresholdPct <
|
||||
* cleanupAggressivePct` BEFORE the route mutates diskSpaceMonitor.diskConfig.
|
||||
*
|
||||
* Tests cover:
|
||||
* 1. Monotonic ascending order is accepted (happy path).
|
||||
* 2. warningThresholdPct >= criticalThresholdPct is rejected with 400.
|
||||
* 3. criticalThresholdPct >= cleanupAggressivePct is rejected with 400.
|
||||
* 4. Partial updates work one field at a time without violating the
|
||||
* invariant against the current baseline.
|
||||
* 5. Out-of-bounds numeric values are clamped to the same bounds the
|
||||
* original inline Math.min/Math.max chains enforced (50/60/70 → 99).
|
||||
* 6. DiskSpaceMonitor.configure is NEVER called when the request is
|
||||
* rejected (no partial mutation).
|
||||
* 7. The merged config returned to the client is the post-clamp value,
|
||||
* not the raw request body.
|
||||
*/
|
||||
|
||||
const express = require('express');
|
||||
const http = require('http');
|
||||
|
||||
const DEFAULT_CONFIG = {
|
||||
enabled: true,
|
||||
diskBudgetGB: 10,
|
||||
warningThresholdPct: 80,
|
||||
criticalThresholdPct: 90,
|
||||
autoCleanup: true,
|
||||
cleanupAggressivePct: 95,
|
||||
};
|
||||
|
||||
function buildFakeDiskSpaceMonitor(initial = { ...DEFAULT_CONFIG }) {
|
||||
const state = { ...initial };
|
||||
return {
|
||||
configure: jest.fn((updates) => {
|
||||
Object.assign(state, updates);
|
||||
return { ...state };
|
||||
}),
|
||||
getConfig: jest.fn(() => ({ ...state })),
|
||||
getSnapshot: jest.fn(async () => ({})),
|
||||
getDetailedBreakdown: jest.fn(async () => ({})),
|
||||
performCleanup: jest.fn(async () => ({})),
|
||||
// Test-only: peek at the internal state to confirm no mutation on rejection
|
||||
_state: state,
|
||||
};
|
||||
}
|
||||
|
||||
function buildRouter(monitor) {
|
||||
// Reset module cache so each test starts fresh
|
||||
jest.resetModules();
|
||||
const mod = require('../../routes/disk-space');
|
||||
return mod({
|
||||
diskSpaceMonitor: monitor,
|
||||
asyncHandler: (fn) => async (req, res, next) => { // eslint-disable-line require-await
|
||||
try { await fn(req, res, next); } catch (e) { next(e); }
|
||||
},
|
||||
log: { info: jest.fn(), warn: jest.fn(), error: jest.fn() },
|
||||
});
|
||||
}
|
||||
|
||||
function buildApp(router) {
|
||||
const app = express();
|
||||
app.use(express.json());
|
||||
app.use((req, _res, next) => { next(); }); // strip auth
|
||||
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',
|
||||
field: err.field || null,
|
||||
});
|
||||
});
|
||||
return app;
|
||||
}
|
||||
|
||||
function supertestFetch(app) {
|
||||
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/disk-space POST /config (DC-059 threshold ordering)', () => {
|
||||
let monitor, app, fetch;
|
||||
|
||||
beforeEach(() => {
|
||||
monitor = buildFakeDiskSpaceMonitor();
|
||||
const router = buildRouter(monitor);
|
||||
app = buildApp(router);
|
||||
fetch = supertestFetch(app);
|
||||
});
|
||||
|
||||
test('happy path — strict monotonic ascending order is accepted', async () => {
|
||||
const res = await fetch('POST', '/config', {
|
||||
warningThresholdPct: 75,
|
||||
criticalThresholdPct: 88,
|
||||
cleanupAggressivePct: 95,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.config).toEqual(expect.objectContaining({
|
||||
warningThresholdPct: 75,
|
||||
criticalThresholdPct: 88,
|
||||
cleanupAggressivePct: 95,
|
||||
}));
|
||||
expect(monitor.configure).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('warningThresholdPct >= criticalThresholdPct is rejected with 400', async () => {
|
||||
const res = await fetch('POST', '/config', {
|
||||
warningThresholdPct: 95,
|
||||
criticalThresholdPct: 80,
|
||||
cleanupAggressivePct: 99,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/warningThresholdPct.*strictly less than.*criticalThresholdPct/);
|
||||
expect(res.body.field).toBe('warningThresholdPct');
|
||||
// Critical invariant: monitor.configure was NEVER called.
|
||||
expect(monitor.configure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('criticalThresholdPct >= cleanupAggressivePct is rejected with 400', async () => {
|
||||
const res = await fetch('POST', '/config', {
|
||||
warningThresholdPct: 60,
|
||||
criticalThresholdPct: 95,
|
||||
cleanupAggressivePct: 80,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/criticalThresholdPct.*strictly less than.*cleanupAggressivePct/);
|
||||
expect(res.body.field).toBe('criticalThresholdPct');
|
||||
expect(monitor.configure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('equal thresholds are rejected (strict <, not <=)', async () => {
|
||||
const res = await fetch('POST', '/config', {
|
||||
warningThresholdPct: 80,
|
||||
criticalThresholdPct: 80,
|
||||
cleanupAggressivePct: 90,
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(monitor.configure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('partial update — single field accepted against existing baseline', async () => {
|
||||
// Defaults: warning=80, critical=90, aggressive=95. Raise warning to 85.
|
||||
const res = await fetch('POST', '/config', { warningThresholdPct: 85 });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.config.warningThresholdPct).toBe(85);
|
||||
expect(res.body.config.criticalThresholdPct).toBe(90);
|
||||
expect(res.body.config.cleanupAggressivePct).toBe(95);
|
||||
});
|
||||
|
||||
test('partial update — would violate invariant against baseline, rejected', async () => {
|
||||
// Defaults: warning=80, critical=90, aggressive=95. Setting warning=95
|
||||
// would collide with the existing critical=90 (warning >= critical).
|
||||
const res = await fetch('POST', '/config', { warningThresholdPct: 95 });
|
||||
expect(res.status).toBe(400);
|
||||
expect(res.body.error).toMatch(/warningThresholdPct.*strictly less than.*criticalThresholdPct/);
|
||||
expect(monitor.configure).not.toHaveBeenCalled();
|
||||
});
|
||||
|
||||
test('partial update — succeeds after baseline was updated in a prior request', async () => {
|
||||
// First request: bump warning from 80 → 85 (within current critical=90).
|
||||
let res = await fetch('POST', '/config', { warningThresholdPct: 85 });
|
||||
expect(res.status).toBe(200);
|
||||
// Second request: now bump warning from 85 → 89. Still under critical=90.
|
||||
res = await fetch('POST', '/config', { warningThresholdPct: 89 });
|
||||
expect(res.status).toBe(200);
|
||||
expect(monitor.configure).toHaveBeenCalledTimes(2);
|
||||
});
|
||||
|
||||
test('partial update — would violate against the NEW baseline, rejected', async () => {
|
||||
// Step 1: raise warning to 85.
|
||||
let res = await fetch('POST', '/config', { warningThresholdPct: 85 });
|
||||
expect(res.status).toBe(200);
|
||||
// Step 2: try to raise warning to 95 — would collide with critical=90.
|
||||
res = await fetch('POST', '/config', { warningThresholdPct: 95 });
|
||||
expect(res.status).toBe(400);
|
||||
// monitor.configure should have run exactly once (the accepted request).
|
||||
expect(monitor.configure).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
test('out-of-bounds values are clamped to documented ranges', async () => {
|
||||
// Note: the three values must produce a valid monotonic ordering AFTER
|
||||
// clamping. Setting warning=20 (→ 50), critical=200 (→ 99), aggressive=70
|
||||
// would produce critical=99 > aggressive=70 which is rejected by the
|
||||
// ordering check. Use values that clamp into a valid range.
|
||||
const res = await fetch('POST', '/config', {
|
||||
warningThresholdPct: 20, // below warning min 50 → clamped to 50
|
||||
criticalThresholdPct: 85, // valid
|
||||
cleanupAggressivePct: 200, // above aggressive max 99 → clamped to 99
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.config).toEqual(expect.objectContaining({
|
||||
warningThresholdPct: 50,
|
||||
criticalThresholdPct: 85,
|
||||
cleanupAggressivePct: 99,
|
||||
}));
|
||||
});
|
||||
|
||||
test('non-numeric threshold values are silently dropped (legacy behaviour preserved)', async () => {
|
||||
// Strings are not numbers → unchanged from baseline. Confirms the
|
||||
// ordering check doesn\'t reject legitimate "I didn\'t change this" requests.
|
||||
const res = await fetch('POST', '/config', {
|
||||
warningThresholdPct: '80',
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.config.warningThresholdPct).toBe(80); // baseline unchanged
|
||||
expect(monitor.configure).toHaveBeenCalledWith({}); // empty updates
|
||||
});
|
||||
|
||||
test('diskBudgetGB and autoCleanup updates still work alongside threshold validation', async () => {
|
||||
const res = await fetch('POST', '/config', {
|
||||
diskBudgetGB: 50,
|
||||
autoCleanup: false,
|
||||
warningThresholdPct: 81,
|
||||
});
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.config.diskBudgetGB).toBe(50);
|
||||
expect(res.body.config.autoCleanup).toBe(false);
|
||||
expect(res.body.config.warningThresholdPct).toBe(81);
|
||||
});
|
||||
|
||||
test('rejected request does NOT mutate the live diskConfig', async () => {
|
||||
const before = { ...monitor._state };
|
||||
const res = await fetch('POST', '/config', {
|
||||
warningThresholdPct: 95, // collides with critical=90
|
||||
});
|
||||
expect(res.status).toBe(400);
|
||||
expect(monitor._state).toEqual(before);
|
||||
});
|
||||
|
||||
test('POST /config with no thresholds in body is a no-op against baseline', async () => {
|
||||
const res = await fetch('POST', '/config', { diskBudgetGB: 25 });
|
||||
expect(res.status).toBe(200);
|
||||
expect(res.body.config.diskBudgetGB).toBe(25);
|
||||
expect(res.body.config.warningThresholdPct).toBe(80); // unchanged
|
||||
expect(res.body.config.criticalThresholdPct).toBe(90); // unchanged
|
||||
expect(res.body.config.cleanupAggressivePct).toBe(95); // unchanged
|
||||
});
|
||||
});
|
||||
@@ -0,0 +1,209 @@
|
||||
/**
|
||||
* Tests for AggregateError / .cause-chain diagnostic surfacing in
|
||||
* src/utils/logging.js writeErrorLog().
|
||||
*
|
||||
* Bug fixed: writeErrorLog previously emitted `error.message` alone.
|
||||
* AggregateError's `.message` is "" by spec, so a real aggregate (e.g.
|
||||
* `await Promise.any([fetch(...), fetch(...)])` or a multi-A DNS lookup
|
||||
* that times out) ended up in error.log as a single empty line:
|
||||
*
|
||||
* [2026-08-18T06:49:03.345Z] [ERR] update:
|
||||
* context: {"imageName":"ipfs/kubo:latest"}
|
||||
*
|
||||
* Operators couldn't tell why the check failed. This file asserts the
|
||||
* fixed behavior:
|
||||
*
|
||||
* - AggregateError → emits a diagnostic block listing each sub-error's
|
||||
* .code/.message.
|
||||
* - Regular Error → no spurious diagnostic block.
|
||||
* - Plain Error with `.code` (e.g. EPIPE) → head now shows
|
||||
* `Error [EPIPE]: write EPIPE` (regression: `code` used to be dropped).
|
||||
* - Error wrapping another Error via `.cause` → lists the cause.
|
||||
* - AggregateError with mixed sub-errors (some Aggregate, some plain) →
|
||||
* recurses correctly without losing any message.
|
||||
* - Empty error.message is replaced with the error name so a bare
|
||||
* AggregateError still renders something readable.
|
||||
*
|
||||
* log.error signature on this codebase: error(ctx, err, req?, extra?)
|
||||
* where extra is the JSON tail (and req is the Express req if any).
|
||||
*/
|
||||
const path = require('path');
|
||||
const fs = require('fs').promises;
|
||||
const os = require('os');
|
||||
|
||||
// Important: set LOG_DIR / ERROR_LOG_FILE BEFORE requiring logging.js so
|
||||
// the per-test temp file is used as the log target.
|
||||
const tmpDir = fs.realpathSync ? require('fs').realpathSync(os.tmpdir()) : os.tmpdir();
|
||||
const TMP_LOG = path.join(tmpDir, `dashcaddy-error-test-${process.pid}-${Date.now()}-${Math.random().toString(36).slice(2)}.log`);
|
||||
|
||||
process.env.LOG_DIR = tmpDir;
|
||||
process.env.ERROR_LOG_FILE = TMP_LOG;
|
||||
process.env.AUDIT_LOG_FILE = path.join(tmpDir, 'unused-audit.json');
|
||||
|
||||
const { log } = require('../src/utils/logging');
|
||||
|
||||
async function readTail(n = 1) {
|
||||
const raw = await fs.readFile(TMP_LOG, 'utf8').catch(() => '');
|
||||
const sep = '\u2500'.repeat(72);
|
||||
const entries = raw.split(sep).map(s => s.replace(/^\s+|\s+$/g, '')).filter(Boolean);
|
||||
return entries.slice(-n);
|
||||
}
|
||||
|
||||
describe('writeErrorLog() — AggregateError + .cause diagnostics', () => {
|
||||
afterAll(async () => {
|
||||
try { await fs.unlink(TMP_LOG); } catch (_) {}
|
||||
});
|
||||
|
||||
beforeEach(async () => {
|
||||
try { await fs.unlink(TMP_LOG); } catch (_) {}
|
||||
});
|
||||
|
||||
test('plain Error: head contains name + message + stack', async () => {
|
||||
await log.error('plain', new Error('boom'), null, { requestId: 'r1' });
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/\[ERR\] plain: Error: boom/);
|
||||
expect(entry).not.toMatch(/diagnostic:/); // no spurious diagnostic block
|
||||
expect(entry).toMatch(/\n {4}at /); // stack preserved (lowercase `at` from V8)
|
||||
expect(entry).toMatch(/context: \{.*requestId.*"r1".*\}/);
|
||||
});
|
||||
|
||||
test('plain Error with .code renders the code in the head (regression fix)', async () => {
|
||||
const e = Object.assign(new Error('write EPIPE'), { code: 'EPIPE' });
|
||||
await log.error('stream', e);
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/\[ERR\] stream: Error \[EPIPE\]: write EPIPE/);
|
||||
expect(entry).not.toMatch(/diagnostic:/);
|
||||
});
|
||||
|
||||
test('custom Error subclass name is preserved in the head', async () => {
|
||||
class WidgetError extends Error {
|
||||
constructor(msg) { super(msg); this.name = 'WidgetError'; }
|
||||
}
|
||||
await log.error('sub', new WidgetError('blew up'));
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/\[ERR\] sub: WidgetError: blew up/);
|
||||
});
|
||||
|
||||
test('empty error.message falls back to the bare error.name (defensive)', async () => {
|
||||
const empty = new Error('');
|
||||
await log.error('empty', empty);
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/\[ERR\] empty: Error$/m);
|
||||
});
|
||||
|
||||
test('AggregateError with sub-errors emits a diagnostic block listing each cause', async () => {
|
||||
// Realistic shape: registry-1.docker.io multi-A lookup timeout returning
|
||||
// an AggregateError of ECONNREFUSED / Timeout / EAI_AGAIN sub-errors.
|
||||
const agg = new AggregateError(
|
||||
[
|
||||
Object.assign(new Error('connect ECONNREFUSED 157.240.20.50:443'), { code: 'ECONNREFUSED' }),
|
||||
Object.assign(new Error('connect ETIMEDOUT 157.240.21.50:443'), { code: 'ETIMEDOUT' }),
|
||||
Object.assign(new Error('getaddrinfo EAI_AGAIN registry-1.docker.io'), { code: 'EAI_AGAIN' }),
|
||||
],
|
||||
''
|
||||
);
|
||||
await log.error('update', agg, null, { imageName: 'ipfs/kubo:latest' });
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/\[ERR\] update: AggregateError/);
|
||||
expect(entry).toMatch(/diagnostic:/);
|
||||
expect(entry).toMatch(/cause #1:/);
|
||||
expect(entry).toMatch(/cause #2:/);
|
||||
expect(entry).toMatch(/cause #3:/);
|
||||
expect(entry).toMatch(/Error \[ECONNREFUSED\]: connect ECONNREFUSED 157\.240\.20\.50:443/);
|
||||
expect(entry).toMatch(/Error \[ETIMEDOUT\]: connect ETIMEDOUT 157\.240\.21\.50:443/);
|
||||
expect(entry).toMatch(/Error \[EAI_AGAIN\]: getaddrinfo EAI_AGAIN registry-1\.docker\.io/);
|
||||
expect(entry).toMatch(/context: \{.*imageName.*"ipfs\/kubo:latest".*\}/);
|
||||
// No double header for AggregateError (we suppress the empty head line).
|
||||
expect(entry).not.toMatch(/diagnostic: AggregateError/);
|
||||
});
|
||||
|
||||
test('Error with .cause emits a nested diagnostic block', async () => {
|
||||
const inner = new Error('TLS handshake failed');
|
||||
const outer = new Error('fetch failed', { cause: inner });
|
||||
await log.error('net', outer);
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/\[ERR\] net: Error: fetch failed/);
|
||||
expect(entry).toMatch(/cause:/);
|
||||
expect(entry).toMatch(/Error: TLS handshake failed/);
|
||||
});
|
||||
|
||||
test('nested AggregateError (sub-error is itself an Aggregate) recurses', async () => {
|
||||
const inner = new AggregateError([new Error('inner-A'), new Error('inner-B')], '');
|
||||
const outer = new AggregateError([new Error('outer-X'), inner], '');
|
||||
await log.error('rec', outer);
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/\[ERR\] rec: AggregateError/);
|
||||
expect(entry).toMatch(/cause #1:[\s\S]*Error: outer-X/);
|
||||
// inner is itself an Aggregate, so its child errors surface as "cause #N":
|
||||
expect(entry).toMatch(/inner-A/);
|
||||
expect(entry).toMatch(/inner-B/);
|
||||
});
|
||||
|
||||
test('separator is appended after each entry (file-format invariant)', async () => {
|
||||
await log.error('sep', new Error('one'));
|
||||
await log.error('sep', new Error('two'));
|
||||
const raw = await fs.readFile(TMP_LOG, 'utf8');
|
||||
const sep = '\u2500'.repeat(72);
|
||||
// Count separator occurrences without reserved regex chars tripping us up.
|
||||
const re = new RegExp(sep.split('').map(c => '\\u' + c.charCodeAt(0).toString(16).padStart(4, '0')).join(''), 'g');
|
||||
const occurrences = (raw.match(re) || []).length;
|
||||
expect(occurrences).toBeGreaterThanOrEqual(2);
|
||||
});
|
||||
|
||||
test('req field is still emitted when the calling site passes a request', async () => {
|
||||
const req = { method: 'POST', path: '/api/v1/widgets', ip: '10.0.0.5', get: () => 'curl/8', id: 'r-42' };
|
||||
await log.error('withreq', new Error('widget blew up'), req);
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/request: POST \/api\/v1\/widgets \| ip: 10\.0\.0\.5 \| ua: curl\/8 \| id: r-42/);
|
||||
});
|
||||
|
||||
test('extra context JSON is still emitted after stack (regression)', async () => {
|
||||
await log.error('ctx', new Error('payload'), null, { operation: 'rotate', tenantId: 7 });
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/context: \{"operation":"rotate","tenantId":7\}/);
|
||||
});
|
||||
|
||||
// Polish-grade hardening (per GLM round-1 B+ findings): cycle guard + depth cap.
|
||||
|
||||
test('circular .cause references do not infinite-loop (cycle guard)', async () => {
|
||||
const a = new Error('top');
|
||||
const b = new Error('middle');
|
||||
const c = new Error('bottom');
|
||||
// c.cause = b would be normal; force a CYCLE by linking back to a.
|
||||
b.cause = a;
|
||||
a.cause = c;
|
||||
c.cause = a; // cycle: a <-> a
|
||||
await expect(log.error('cycle', a, null)).resolves.not.toThrow();
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/top/);
|
||||
expect(entry).toMatch(/cycle: same Error instance seen earlier/);
|
||||
});
|
||||
|
||||
test('excessively deep .cause chains are truncated, not crashed (depth cap)', async () => {
|
||||
// Build a chain 50 deep ending in 'level-50' at the deepest; each layer
|
||||
// wraps the previous via .cause. log.error is called with the deepest
|
||||
// (outer) Error.
|
||||
let cur = new Error('level-1');
|
||||
for (let i = 2; i <= 50; i++) {
|
||||
const parent = new Error(`level-${i}`);
|
||||
parent.cause = cur;
|
||||
cur = parent;
|
||||
}
|
||||
await expect(log.error('deep', cur)).resolves.not.toThrow();
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/chain truncated at depth 16/);
|
||||
expect(entry).toMatch(/level-50/); // the deepest/head shown in headline
|
||||
expect(entry).not.toMatch(/level-1/); // the leaf is too deep to render
|
||||
});
|
||||
|
||||
test('circular `.errors` array (sub-error is itself in the parent) is bounded', async () => {
|
||||
const sub = new Error('shared sub-error');
|
||||
const agg = new AggregateError([sub, new Error('other')], '');
|
||||
// pathological: sub-Aggregate references the parent
|
||||
sub.errors = [agg];
|
||||
await expect(log.error('aggcycle', agg)).resolves.not.toThrow();
|
||||
const [entry] = await readTail();
|
||||
expect(entry).toMatch(/shared sub-error/);
|
||||
expect(entry).toMatch(/cycle: same Error instance seen earlier/);
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
@@ -1,5 +1,6 @@
|
||||
const express = require('express');
|
||||
const { success, error: errorResponse } = require('../src/utils/responses');
|
||||
const { ValidationError } = require('../src/utilities/errors');
|
||||
|
||||
/**
|
||||
* Disk space management routes
|
||||
@@ -10,6 +11,76 @@ const { success, error: errorResponse } = require('../src/utils/responses');
|
||||
* POST /disk/config — update disk budget settings
|
||||
* POST /disk/cleanup — trigger manual cleanup (standard|aggressive|logs-only)
|
||||
*/
|
||||
|
||||
// DC-059: monotonic-ordering invariant for the three threshold percentages.
|
||||
// DiskSpaceMonitor._getBudgetStatus() walks them in order
|
||||
// (cleanupAggressivePct → criticalThresholdPct → warningThresholdPct) and
|
||||
// returns at the FIRST threshold the usage crosses. If a caller writes
|
||||
// them out of order (e.g. warningThresholdPct=95, criticalThresholdPct=60),
|
||||
// the higher-priority branches become unreachable and the monitor silently
|
||||
// misclassifies budget state. Validate against the *effective* config
|
||||
// (current value + incoming update for each field) so partial updates can
|
||||
// be applied one field at a time without violating the invariant.
|
||||
//
|
||||
// Clamp values to the same ranges the previous inline Math.min/Math.max
|
||||
// chains enforced (warning 50..99, critical 60..99, aggressive 70..99)
|
||||
// so we don't loosen the original bounds while adding the new check.
|
||||
const THRESHOLD_BOUNDS = Object.freeze({
|
||||
warning: { min: 50, max: 99 },
|
||||
critical: { min: 60, max: 99 },
|
||||
aggressive: { min: 70, max: 99 },
|
||||
});
|
||||
|
||||
function clampThreshold(name, value) {
|
||||
const { min, max } = THRESHOLD_BOUNDS[name];
|
||||
return Math.min(Math.max(value, min), max);
|
||||
}
|
||||
|
||||
/**
|
||||
* Apply a candidate update to a baseline config, then verify the three
|
||||
* threshold percentages still satisfy
|
||||
* warningThresholdPct < criticalThresholdPct < cleanupAggressivePct.
|
||||
* The POST /config endpoint accepts partial updates (single field at a
|
||||
* time), so we merge into the live diskSpaceMonitor config first, then test
|
||||
* the merged value. Returns the merged candidate on success; throws
|
||||
* ValidationError if the ordering invariant would be violated.
|
||||
*
|
||||
* @param {Object} baseline - current effective config from diskSpaceMonitor
|
||||
* @param {Object} candidate - the partial update being applied this request
|
||||
* @returns {Object} merged candidate with thresholds clamped to bounds
|
||||
*/
|
||||
function mergeAndCheckOrdering(baseline, candidate) {
|
||||
const next = { ...baseline };
|
||||
if (typeof candidate.warningThresholdPct === 'number') {
|
||||
next.warningThresholdPct = clampThreshold('warning', candidate.warningThresholdPct);
|
||||
}
|
||||
if (typeof candidate.criticalThresholdPct === 'number') {
|
||||
next.criticalThresholdPct = clampThreshold('critical', candidate.criticalThresholdPct);
|
||||
}
|
||||
if (typeof candidate.cleanupAggressivePct === 'number') {
|
||||
next.cleanupAggressivePct = clampThreshold('aggressive', candidate.cleanupAggressivePct);
|
||||
}
|
||||
if (!(next.warningThresholdPct < next.criticalThresholdPct)) {
|
||||
throw new ValidationError(
|
||||
`warningThresholdPct (${next.warningThresholdPct}) must be strictly less than criticalThresholdPct (${next.criticalThresholdPct})`,
|
||||
'warningThresholdPct'
|
||||
);
|
||||
}
|
||||
if (!(next.criticalThresholdPct < next.cleanupAggressivePct)) {
|
||||
throw new ValidationError(
|
||||
`criticalThresholdPct (${next.criticalThresholdPct}) must be strictly less than cleanupAggressivePct (${next.cleanupAggressivePct})`,
|
||||
'criticalThresholdPct'
|
||||
);
|
||||
}
|
||||
// Return only the fields the caller asked to change (preserves partial-
|
||||
// update semantics; diskSpaceMonitor.configure does its own merge).
|
||||
const out = {};
|
||||
if (typeof candidate.warningThresholdPct === 'number') out.warningThresholdPct = next.warningThresholdPct;
|
||||
if (typeof candidate.criticalThresholdPct === 'number') out.criticalThresholdPct = next.criticalThresholdPct;
|
||||
if (typeof candidate.cleanupAggressivePct === 'number') out.cleanupAggressivePct = next.cleanupAggressivePct;
|
||||
return out;
|
||||
}
|
||||
|
||||
module.exports = function({ diskSpaceMonitor, asyncHandler, log }) {
|
||||
const router = express.Router();
|
||||
|
||||
@@ -36,9 +107,16 @@ module.exports = function({ diskSpaceMonitor, asyncHandler, log }) {
|
||||
|
||||
const updates = {};
|
||||
if (typeof diskBudgetGB === 'number' && diskBudgetGB > 0) updates.diskBudgetGB = Math.min(diskBudgetGB, 1000);
|
||||
if (typeof warningThresholdPct === 'number') updates.warningThresholdPct = Math.min(Math.max(warningThresholdPct, 50), 99);
|
||||
if (typeof criticalThresholdPct === 'number') updates.criticalThresholdPct = Math.min(Math.max(criticalThresholdPct, 60), 99);
|
||||
if (typeof cleanupAggressivePct === 'number') updates.cleanupAggressivePct = Math.min(Math.max(cleanupAggressivePct, 70), 99);
|
||||
// DC-059: threshold percentages must satisfy a strict monotonic order
|
||||
// (warning < critical < aggressive) so _getBudgetStatus() reaches the
|
||||
// correct branch. mergeAndCheckOrdering() validates against the live
|
||||
// baseline, so partial updates that violate the invariant are rejected
|
||||
// BEFORE we mutate diskSpaceMonitor.diskConfig.
|
||||
const thresholdUpdates = mergeAndCheckOrdering(
|
||||
diskSpaceMonitor.getConfig(),
|
||||
{ warningThresholdPct, criticalThresholdPct, cleanupAggressivePct }
|
||||
);
|
||||
Object.assign(updates, thresholdUpdates);
|
||||
if (typeof autoCleanup === 'boolean') updates.autoCleanup = autoCleanup;
|
||||
if (typeof enabled === 'boolean') updates.enabled = enabled;
|
||||
|
||||
|
||||
@@ -406,7 +406,7 @@ class AutoRestartManager extends EventEmitter {
|
||||
// Transition: healthy → unhealthy
|
||||
if (previousStatus === 'up' && currentStatus === 'down') {
|
||||
// Find the containerId from the health checker config or status details
|
||||
const containerId = this._resolveContainerId(serviceId, status);
|
||||
const containerId = await this._resolveContainerId(serviceId, status);
|
||||
if (containerId) {
|
||||
try {
|
||||
await this.handleContainerDown(serviceId, containerId);
|
||||
@@ -429,12 +429,19 @@ class AutoRestartManager extends EventEmitter {
|
||||
/**
|
||||
* Attempt to find the containerId for a service from various sources.
|
||||
*
|
||||
* DC-060: the previous implementation fired the async lookup via `.then(...)`
|
||||
* but discarded the returned containerId, returning `undefined` from the
|
||||
* function. Callers (`_handleStatusCheck`) gate on the return value, so
|
||||
* every auto-restart whose containerId came from servicesStateManager
|
||||
* silently no-op'd. Now awaits the read() promise so the containerId
|
||||
* actually propagates.
|
||||
*
|
||||
* @param {string} serviceId
|
||||
* @param {Object} status - The status-check event data
|
||||
* @returns {string|null}
|
||||
* @returns {Promise<string|null>}
|
||||
* @private
|
||||
*/
|
||||
_resolveContainerId(serviceId, status) {
|
||||
async _resolveContainerId(serviceId, status) {
|
||||
// Check if it's in the status details (some health checks embed it)
|
||||
if (status.details?.containerId) return status.details.containerId;
|
||||
|
||||
@@ -442,23 +449,20 @@ class AutoRestartManager extends EventEmitter {
|
||||
const hcService = this.healthChecker?.config?.services?.[serviceId];
|
||||
if (hcService?.containerId) return hcService.containerId;
|
||||
|
||||
// Try to look it up from the services state manager
|
||||
// Try to look it up from the services state manager. StateManager.read()
|
||||
// is async (returns a Promise) — must await, not fire-and-forget.
|
||||
try {
|
||||
const servicesStateManager = this.ctx.servicesStateManager;
|
||||
if (servicesStateManager) {
|
||||
const readResult = servicesStateManager.read();
|
||||
if (readResult && typeof readResult.then === 'function') {
|
||||
// It returns a promise — fire-and-forget lookup
|
||||
readResult.then(list => {
|
||||
if (!servicesStateManager) return null;
|
||||
const list = await servicesStateManager.read();
|
||||
const found = (list || []).find(s => s.id === serviceId);
|
||||
return found?.containerId || null;
|
||||
}).catch(() => null);
|
||||
} else {
|
||||
const found = (readResult || []).find(s => s.id === serviceId);
|
||||
if (found?.containerId) return found.containerId;
|
||||
} catch (err) {
|
||||
// Best-effort: a state-manager read failure must not break the bridge.
|
||||
// Surface at debug level so an operator hunting "why didn't auto-restart
|
||||
// fire?" can find it without polluting the info-level event stream.
|
||||
this.log?.debug?.('auto-restart', 'containerId resolve failed', { serviceId, error: err?.message });
|
||||
}
|
||||
}
|
||||
} catch (_) { /* best effort */ }
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
@@ -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', {
|
||||
|
||||
@@ -112,12 +112,78 @@ async function appendErrorLog(line) {
|
||||
}
|
||||
}
|
||||
|
||||
// Flatten an error chain into readable lines so error.log records why a
|
||||
// request failed, not just that it did. Handle AggregateError (`.errors[]`,
|
||||
// common from lookups/DNS-fetch timeouts) and the modern `.cause` chain —
|
||||
// both common in Node 18+ networking. Always returns at least one line
|
||||
// (a head line with `name [code]: message`), and appends cause lines for
|
||||
// any `.errors` / `.cause` chains present.
|
||||
//
|
||||
// Defensive against:
|
||||
// - Circular `.cause` references (a pathological error payload pointing
|
||||
// `err.cause = err` would otherwise infinite-recurse and crash the
|
||||
// error-path). Visited set carries forward via parameter.
|
||||
// - Excessively deep chains (> MAX_CHAIN_DEPTH): truncated with a marker
|
||||
// so the operator can see something IS coming from underneath.
|
||||
const MAX_CHAIN_DEPTH = 16;
|
||||
function describeErrorChain(err, depth = 0, seen = new WeakSet()) {
|
||||
const out = [];
|
||||
if (depth > MAX_CHAIN_DEPTH) {
|
||||
out.push(`${' '.repeat(depth)} ... (chain truncated at depth ${MAX_CHAIN_DEPTH})`);
|
||||
return out;
|
||||
}
|
||||
if (!(err instanceof Error)) {
|
||||
out.push(`${' '.repeat(depth)}${String(err)}`);
|
||||
return out;
|
||||
}
|
||||
// Cycle guard — same Error instance already on the chain.
|
||||
if (seen.has(err)) {
|
||||
out.push(`${' '.repeat(depth)} ... (cycle: same Error instance seen earlier)`);
|
||||
return out;
|
||||
}
|
||||
seen.add(err);
|
||||
const indent = ' '.repeat(depth);
|
||||
const code = err.code ? ` [${err.code}]` : '';
|
||||
const msg = err.message ? `: ${err.message}` : '';
|
||||
// For every error (including AggregateError), render the head line; an
|
||||
// empty `.message` simply produces `Name [code]:` which is still useful.
|
||||
out.push(`${indent}${err.name || 'Error'}${code}${msg}`);
|
||||
if (Array.isArray(err.errors) && err.errors.length) {
|
||||
err.errors.forEach((sub, i) => {
|
||||
out.push(`${indent} cause #${i + 1}:`);
|
||||
out.push(...describeErrorChain(sub, depth + 2, seen));
|
||||
});
|
||||
}
|
||||
if (err.cause instanceof Error) {
|
||||
out.push(`${indent} cause:`);
|
||||
out.push(...describeErrorChain(err.cause, depth + 2, seen));
|
||||
}
|
||||
return out;
|
||||
}
|
||||
|
||||
async function writeErrorLog(ctx, error, req, extra) {
|
||||
const ts = new Date().toISOString();
|
||||
const errMsg = error instanceof Error ? error.message : String(error);
|
||||
const errStack = error instanceof Error ? error.stack : '';
|
||||
const parts = [`[${ts}] [ERR] ${ctx}: ${errMsg}`];
|
||||
// Build the head line AND a tail diagnostic from the same describeErrorChain,
|
||||
// so plain errors with .code get `[CODE]` formatted into the head (regression)
|
||||
// and AggregateError with empty `.message` gets a diagnostic block listing
|
||||
// every cause (the actual bug fix).
|
||||
let headLine;
|
||||
let diagLines = [];
|
||||
if (error instanceof Error) {
|
||||
const chain = describeErrorChain(error);
|
||||
// The chain head is always the error itself (now including AggregateError),
|
||||
// so chain[0] is what we want in the headline and chain[1..] is the rest.
|
||||
headLine = chain[0] || `${error.name || 'Error'}`;
|
||||
diagLines = chain.slice(1);
|
||||
} else {
|
||||
headLine = String(error);
|
||||
}
|
||||
// Preserve the historical `ctx: <head>` shape so log scrapers don't break.
|
||||
// The head now carries `name [code]: message` instead of bare `.message`.
|
||||
const parts = [`[${ts}] [ERR] ${ctx}: ${headLine.replace(/^\s+/, '')}`];
|
||||
if (errStack) parts.push(errStack);
|
||||
if (diagLines.length) parts.push(' diagnostic: ' + diagLines.join('\n diagnostic: '));
|
||||
if (req) {
|
||||
const ip = req.ip || req.socket?.remoteAddress || '';
|
||||
const ua = req.get ? req.get('user-agent') : '';
|
||||
|
||||
Reference in New Issue
Block a user