Merge dc/DC-058: CSRF log tag differentiation for browser auto-retry vs real probe

dashcaddy-api/__tests__/routes/disk-space.routes.test.js

@@ -1,277 +0,0 @@
-/**
- * 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
-  });
-});

dashcaddy-api/routes/disk-space.js

@@ -1,6 +1,5 @@
 const express = require('express');
 const { success, error: errorResponse } = require('../src/utils/responses');
-const { ValidationError } = require('../src/utilities/errors');
 
 /**
  * Disk space management routes
@@ -11,76 +10,6 @@ const { ValidationError } = require('../src/utilities/errors');
  * 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();
 
@@ -107,16 +36,9 @@ module.exports = function({ diskSpaceMonitor, asyncHandler, log }) {
 
     const updates = {};
     if (typeof diskBudgetGB === 'number' && diskBudgetGB > 0) updates.diskBudgetGB = Math.min(diskBudgetGB, 1000);
-    // DC-059: threshold percentages must satisfy a strict monotonic order
+    if (typeof warningThresholdPct === 'number') updates.warningThresholdPct = Math.min(Math.max(warningThresholdPct, 50), 99);
-    // (warning < critical < aggressive) so _getBudgetStatus() reaches the
+    if (typeof criticalThresholdPct === 'number') updates.criticalThresholdPct = Math.min(Math.max(criticalThresholdPct, 60), 99);
-    // correct branch. mergeAndCheckOrdering() validates against the live
+    if (typeof cleanupAggressivePct === 'number') updates.cleanupAggressivePct = Math.min(Math.max(cleanupAggressivePct, 70), 99);
-    // 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;