fix(config): monitoring.public gate was dead 4 ways — live gate + schema + dedupe (DC-096) [glm-grade=A]
The documented hardening option for exposed deploys (monitoring: {public: false}
in config.json / MONITORING_PUBLIC env) never worked:
1. applyConfigFields dropped the monitoring key entirely
2. monitoring missing from config-schema KNOWN_KEYS (Unknown-key warnings)
3. MONITORING_PUBLIC frozen at mount + re-required singleton instead of injected dep
4. PUBLIC_ROUTES had unconditional duplicate entries defeating the gated spread
- site.js: copy monitoring through; drop dead write-only siteConfig.caName
- config-schema: +monitoring key, validateMonitoring (public must be boolean);
remove never-written typo-footgun keys setupCompleted/setupMode (git -S: zero writers ever)
- middleware: live isMonitoringPublic() (env > config > default public), per-request
gate via monitoring:true flag, remove duplicate unconditional route entries
- default unchanged (endpoints stay public — System Overview widget)
Tests: +11 (__tests__/monitoring-public-gate-dc096.test.js); suite 118/2735 green.
Judge: GLM-5.3 cold-read A (deleg_98aba845), URN urn:ump:zgvtskqljurasdagc632atnybb2p6gakk4cxcmjd3i4ivy7rvwta
This commit is contained in:
@@ -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?'
|
||||
]);
|
||||
});
|
||||
});
|
||||
@@ -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);
|
||||
|
||||
@@ -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 };
|
||||
|
||||
@@ -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
|
||||
|
||||
Reference in New Issue
Block a user