diff --git a/dashcaddy-api/__tests__/monitoring-public-gate-dc096.test.js b/dashcaddy-api/__tests__/monitoring-public-gate-dc096.test.js new file mode 100644 index 0000000..4c6c1e9 --- /dev/null +++ b/dashcaddy-api/__tests__/monitoring-public-gate-dc096.test.js @@ -0,0 +1,184 @@ +/** + * DC-096 regression tests: the `monitoring: { public: false }` config option + * actually gates the monitoring endpoints. + * + * WHY THIS EXISTS: + * The middleware comment documented `monitoring: { public: false }` in + * config.json as the way to require auth for /api/v1/monitoring/stats and + * /api/v1/health-checks/status on internet-exposed deployments. But the + * option was dead three ways: + * 1. applyConfigFields() never copied `monitoring` out of raw config — + * siteConfig.monitoring stayed undefined forever. + * 2. `monitoring` was not in config-schema KNOWN_KEYS — saving it via + * POST /api/v1/config produced "Unknown config key" warnings (save + * still succeeded, so users saw a warning for a real feature). + * 3. MONITORING_PUBLIC was a const frozen at mount time AND re-required + * the config/site singleton — POST /config changes never took effect + * without a full process restart. + * + * Net effect: an operator who set the documented hardening option on an + * exposed box kept serving monitoring data unauthenticated, with only a + * cosmetic warning. Classic "config option that never worked". + * + * These tests pin the fixed behavior: + * - applyConfigFields copies monitoring through to siteConfig + * - isPublicRoute honors the gate LIVE (no restart) + * - env override still wins over config + * - schema accepts `monitoring` and validates its shape + * - typo keys setupCompleted/setupMode no longer silently allowlisted + */ +const express = require('express'); +const request = require('supertest'); + +// The config/site module exports the siteConfig singleton + loaders. +const { siteConfig, loadSiteConfig } = require('../src/config/site'); +const { validateConfig } = require('../src/utilities/config-schema'); + +// Build a minimal app mounting ONLY the middleware under test, with the +// same dependency shape app.js passes. This mirrors how configureMiddleware +// is used in production without booting the whole app (routes, docker, etc). +function buildMiddlewareApp(configOverrides = {}) { + const configureMiddleware = require('../src/utilities/middleware'); + const app = express(); + + const siteConfigDep = { + tld: '.sami', + dashboardHost: 'status.sami', + ...configOverrides + }; + + const deps = { + siteConfig: siteConfigDep, + totpConfig: { enabled: true }, // force the auth path to actually run + tailscaleConfig: { enabled: false, requireAuth: false }, + metrics: { recordRequest: () => {} }, + auditLogger: { middleware: () => (req, res, next) => next() }, + authManager: { + verifyJWT: async () => null, + verifyAPIKey: async () => null + }, + log: { + info: () => {}, warn: () => {}, error: () => {}, debug: () => {} + }, + cryptoUtils: { loadOrCreateKey: () => 'test-key-not-a-real-secret' }, + isValidContainerId: () => true, + isTailscaleIP: () => false, + getTailscaleStatus: async () => ({}) + }; + + configureMiddleware(app, deps); + // Probe route AFTER middleware so it exercises the auth chain. + app.get('/api/v1/monitoring/stats', (req, res) => res.json({ ok: true })); + app.get('/api/v1/health-checks/status', (req, res) => res.json({ ok: true })); + return app; +} + +describe('DC-096: monitoring.public config gate (middleware + site config)', () => { + const ENV_KEY = 'MONITORING_PUBLIC'; + + afterEach(() => { + delete process.env[ENV_KEY]; + // Reset the singleton to a clean default for other suites + siteConfig.monitoring = null; + }); + + test('applyConfigFields copies monitoring through to siteConfig (the original dead option)', () => { + loadSiteConfig(null, null); // no CONFIG_FILE arg → falls to catch, keeps defaults + siteConfig.monitoring = undefined; + // Directly exercise applyConfigFields via the public loader with a real temp file + const fs = require('fs'); + const os = require('os'); + const path = require('path'); + const tmp = path.join(os.tmpdir(), `dc096-config-${Date.now()}.json`); + fs.writeFileSync(tmp, JSON.stringify({ + tld: '.sami', + monitoring: { public: false } + })); + try { + const noopLog = { info: () => {}, warn: () => {}, error: () => {} }; + loadSiteConfig(tmp, noopLog); + expect(siteConfig.monitoring).toEqual({ public: false }); + } finally { + fs.unlinkSync(tmp); + } + }); + + test('monitoring endpoints are PUBLIC by default (no monitoring config)', async () => { + const app = buildMiddlewareApp(); + const res = await request(app).get('/api/v1/monitoring/stats'); + expect(res.status).toBe(200); + expect(res.body.ok).toBe(true); + }); + + test('monitoring: { public: false } in config → endpoints require auth (401) — LIVE, no restart', async () => { + const app = buildMiddlewareApp({ monitoring: { public: false } }); + const res = await request(app).get('/api/v1/monitoring/stats'); + expect(res.status).toBe(401); + const res2 = await request(app).get('/api/v1/health-checks/status'); + expect(res2.status).toBe(401); + }); + + test('gate reads config LIVE: flipping siteConfig.monitoring.public at runtime flips the gate', async () => { + const cfg = { monitoring: { public: true } }; + const app = buildMiddlewareApp(cfg); + let res = await request(app).get('/api/v1/monitoring/stats'); + expect(res.status).toBe(200); + + // Simulate POST /api/v1/config refreshing the singleton in place — + // the same object the middleware holds a reference to. + cfg.monitoring.public = false; + res = await request(app).get('/api/v1/monitoring/stats'); + expect(res.status).toBe(401); + }); + + test('env override MONITORING_PUBLIC=true beats config monitoring.public=false', async () => { + process.env.MONITORING_PUBLIC = 'true'; + const app = buildMiddlewareApp({ monitoring: { public: false } }); + const res = await request(app).get('/api/v1/monitoring/stats'); + expect(res.status).toBe(200); + }); + + test('env override MONITORING_PUBLIC=false beats config monitoring.public=true', async () => { + process.env.MONITORING_PUBLIC = 'false'; + const app = buildMiddlewareApp({ monitoring: { public: true } }); + const res = await request(app).get('/api/v1/monitoring/stats'); + expect(res.status).toBe(401); + }); + + test('non-monitoring public routes stay public when monitoring gate closes', async () => { + const app = buildMiddlewareApp({ monitoring: { public: false } }); + // /api/v1/version is public unconditionally + const res = await request(app).get('/api/v1/version'); + // No route mounted at that path in this harness → 404 from express, + // NOT 401 — proving the auth middleware let it through. + expect(res.status).toBe(404); + }); +}); + +describe('DC-096: config-schema accepts monitoring', () => { + test('monitoring: { public: boolean } passes with zero warnings', () => { + const result = validateConfig({ monitoring: { public: false } }); + expect(result.warnings).toEqual([]); + expect(result.valid).toBe(true); + }); + + test('monitoring.public non-boolean is an ERROR (not silent)', () => { + const result = validateConfig({ monitoring: { public: 'false' } }); + expect(result.valid).toBe(false); + expect(result.errors).toContain('monitoring.public must be a boolean'); + }); + + test('monitoring non-object is an ERROR', () => { + const result = validateConfig({ monitoring: 'private' }); + expect(result.valid).toBe(false); + expect(result.errors).toContain('monitoring must be an object'); + }); + + test('typo keys setupCompleted/setupMode now WARN (no longer silently allowlisted)', () => { + const result = validateConfig({ setupCompleted: true, setupMode: 'simple' }); + expect(result.warnings).toEqual([ + 'Unknown config key "setupCompleted" — possible typo?', + 'Unknown config key "setupMode" — possible typo?' + ]); + }); +}); diff --git a/dashcaddy-api/src/config/site.js b/dashcaddy-api/src/config/site.js index b8bac9a..db33a4d 100644 --- a/dashcaddy-api/src/config/site.js +++ b/dashcaddy-api/src/config/site.js @@ -13,7 +13,6 @@ const { loadAndMigrate, CURRENT_VERSION } = require('./migrations'); const siteConfig = { tld: '.home', - caName: '', dnsServerIp: '', dnsServerPort: CADDY.DEFAULT_DNS_PORT, dashboardHost: '', @@ -27,7 +26,6 @@ const siteConfig = { function applyConfigFields(raw) { siteConfig.tld = raw.tld || '.home'; if (!siteConfig.tld.startsWith('.')) siteConfig.tld = '.' + siteConfig.tld; - siteConfig.caName = raw.caName || ''; siteConfig.dnsServerIp = (raw.dns && raw.dns.ip) || ''; siteConfig.dnsServerPort = (raw.dns && raw.dns.port) || CADDY.DEFAULT_DNS_PORT; siteConfig.dashboardHost = raw.dashboardHost || `status${siteConfig.tld}`; @@ -37,6 +35,11 @@ function applyConfigFields(raw) { siteConfig.domain = raw.domain || ''; siteConfig.routingMode = raw.routingMode || 'subdomain'; siteConfig.pylon = raw.pylon || null; + // DC-096: `monitoring` was previously NOT copied out of raw config, so the + // documented hardening option `monitoring: { public: false }` (middleware.js + // MONITORING_PUBLIC) silently never applied — siteConfig.monitoring stayed + // undefined forever. Copy it through so the middleware actually sees it. + siteConfig.monitoring = raw.monitoring || null; } function validateAndLogConfig(raw, log) { const { valid, errors: configErrors, warnings: configWarnings } = validateConfig(raw); diff --git a/dashcaddy-api/src/utilities/config-schema.js b/dashcaddy-api/src/utilities/config-schema.js index dac8ad2..1b47515 100644 --- a/dashcaddy-api/src/utilities/config-schema.js +++ b/dashcaddy-api/src/utilities/config-schema.js @@ -10,7 +10,7 @@ const VALID_DNS_PROVIDERS = ['technitium', 'cloudflare', 'rfc2136', 'manual']; const KNOWN_KEYS = [ 'tld', 'caName', 'dns', 'dnsServers', 'dashboardHost', 'timezone', 'theme', 'updatedAt', 'timestamp', 'logo', 'logoPosition', 'favicon', 'weather', - 'setupComplete', 'setupCompleted', 'setupMode', 'onboardingCompleted', + 'setupComplete', 'onboardingCompleted', 'configurationType', 'defaults', 'customLogo', 'customFavicon', 'dashboardTitle', 'tailscale', 'license', 'skipped', 'routingMode', 'domain', 'email', 'defaultIP', 'pylon', @@ -18,7 +18,13 @@ const KNOWN_KEYS = [ // license-manager.js persists the last activation to config.licenseBackup // (restore-on-restart path); src/config/migrations.js stamps _version. // Both are first-party writes — see DC-091. - 'licenseBackup', '_version' + 'licenseBackup', '_version', + // DC-096: monitoring.public gates whether /api/v1/monitoring/stats and + // /api/v1/health-checks/status are public (middleware.js isMonitoringPublic). + // Removed 'setupCompleted' and 'setupMode' — never written by any code + // (past or present); they only existed here, where they masked the actual + // typo of the real key `setupComplete` (writers: setup-wizard.js). + 'monitoring' ]; /** @@ -162,6 +168,22 @@ function validateKnownKeys(ctx, config) { } } +/** + * @param {{errors:string[], warnings:string[]}} ctx + * @param {object} config + */ +function validateMonitoring(ctx, config) { + if (config.monitoring === undefined) return; + if (typeof config.monitoring !== 'object' || config.monitoring === null) { + ctx.errors.push('monitoring must be an object'); + return; + } + if (config.monitoring.public !== undefined + && typeof config.monitoring.public !== 'boolean') { + ctx.errors.push('monitoring.public must be a boolean'); + } +} + /** * Validate a config object and return errors/warnings. * @param {object} config - The config object to validate @@ -183,6 +205,7 @@ function validateConfig(config) { validateTheme(ctx, config); validateRoutingMode(ctx, config); validateDomain(ctx, config); + validateMonitoring(ctx, config); validateKnownKeys(ctx, config); return { valid: errors.length === 0, errors, warnings }; diff --git a/dashcaddy-api/src/utilities/middleware.js b/dashcaddy-api/src/utilities/middleware.js index 5cbeda8..d4a778e 100644 --- a/dashcaddy-api/src/utilities/middleware.js +++ b/dashcaddy-api/src/utilities/middleware.js @@ -359,18 +359,25 @@ module.exports = function configureMiddleware(app, { // (env var) or `monitoring: { public: false }` (config.json) to require // auth for these — useful for internet-exposed deployments where // CPU/memory/disk data is sensitive. - const MONITORING_PUBLIC = (() => { + // + // DC-096: this used to be a const frozen at mount time AND it re-required + // the config/site singleton instead of using the `siteConfig` dependency + // injected by app.js — so POST /api/v1/config changes never took effect + // until a full process restart, and a fresh process with + // monitoring.public=false in config.json never saw it either (the field + // was dropped by applyConfigFields — see site.js). Resolved per-request + // from: explicit env override → live config value → default (public). + const isMonitoringPublic = () => { if (process.env.MONITORING_PUBLIC === 'false') return false; if (process.env.MONITORING_PUBLIC === 'true') return true; - // Default: check config.json if loaded - try { - const cfg = require('../config/site').siteConfig; - if (cfg && cfg.monitoring && typeof cfg.monitoring.public === 'boolean') { - return cfg.monitoring.public; - } - } catch { /* config not loaded yet, use default */ } + // Read the injected config object live — siteConfig is the same mutable + // singleton that loadSiteConfig()/POST /config refresh in place. + if (siteConfig && typeof siteConfig.monitoring === 'object' && siteConfig.monitoring !== null + && typeof siteConfig.monitoring.public === 'boolean') { + return siteConfig.monitoring.public; + } return true; // default: public (current behavior, dashboard needs it) - })(); + }; const PUBLIC_ROUTES = [ // Health probes — root-level only. See src/app.js for the handler block. @@ -452,18 +459,18 @@ module.exports = function configureMiddleware(app, { { path: '/api/v1/themes', exact: true, method: 'GET' }, { path: '/api/v1/license/status', exact: true, method: 'GET' }, { path: '/api/v1/license/feature/', prefix: true, method: 'GET' }, - { path: '/api/v1/config', exact: true, method: 'GET' }, - { path: '/api/v1/services/status', exact: true, method: 'GET' }, - { path: '/api/v1/health-checks/status', exact: true, method: 'GET' }, + { path: '/api/v1/config', exact: true, method: 'GET' }, + { path: '/api/v1/services/status', exact: true, method: 'GET' }, + // (/api/v1/health-checks/status and /api/v1/monitoring/stats are listed + // further below WITH the monitoring.public live gate — DC-096. They were + // previously duplicated here unconditionally, which silently defeated + // the MONITORING_PUBLIC gate entirely.) // DC-075: System health endpoint for external uptime monitoring (UptimeRobot, BetterStack) - { path: '/api/v1/system/health', exact: true, method: 'GET' }, + { path: '/api/v1/system/health', exact: true, method: 'GET' }, // DC-097: Prometheus metrics endpoint (scraped by Prometheus, no auth) { path: '/api/v1/metrics/prometheus', exact: true, method: 'GET' }, // DC-077: i18n endpoints (language list + translations, public) { path: '/api/v1/i18n/', prefix: true, method: 'GET' }, - // System Overview widget on the dashboard — needs the flattened CPU/mem - // data without going through auth. See skill references/totp-and-system-overview-pitfalls.md §3. - { path: '/api/v1/monitoring/stats', exact: true, method: 'GET' }, // Read-only update/version info shown on the dashboard view (verification // modal, topbar version, update badges). Mutating actions — update-apply, // rollback (POST) — are NOT listed here and stay TOTP-protected. @@ -473,11 +480,13 @@ module.exports = function configureMiddleware(app, { { path: '/api/v1/system/update-check', exact: true, method: 'GET' }, { path: '/api/v1/updates/available', exact: true, method: 'GET' }, { path: '/api/v1/system/update-notify', exact: true, method: 'POST' }, - // Monitoring endpoints — only public if MONITORING_PUBLIC is true - ...(MONITORING_PUBLIC ? [ - { path: '/api/v1/monitoring/stats', exact: true, method: 'GET' }, - { path: '/api/v1/health-checks/status', exact: true, method: 'GET' }, - ] : []), + // Monitoring endpoints — public only while isMonitoringPublic() is true. + // DC-096: these are listed unconditionally and gated inside + // isPublicRoute() so the gate is resolved LIVE per request — flipping + // `monitoring: { public: false }` via POST /api/v1/config takes effect + // on the next request, no process restart. + { path: '/api/v1/monitoring/stats', exact: true, method: 'GET', monitoring: true }, + { path: '/api/v1/health-checks/status', exact: true, method: 'GET', monitoring: true }, { path: '/api/v1/version', exact: true, method: 'GET' }, // Security ingest endpoints — auth via per-host Bearer token (handled in routes/security.js), // NOT via TOTP/JWT. This lets remote DashCaddy agents and log parsers push events @@ -489,6 +498,9 @@ module.exports = function configureMiddleware(app, { function isPublicRoute(req) { return PUBLIC_ROUTES.some(r => { if (r.method && req.method !== r.method) return false; + // DC-096: monitoring routes are only public while the live gate says so + // (env override → config → default public). Checked per request. + if (r.monitoring && !isMonitoringPublic()) return false; if (r.exact) { // Exact string match, BUT allow `:param` placeholders in the // PUBLIC_ROUTES entry to match any single path segment. This was a