fix(api): rehydrate process.env from disk-settings.json on boot (DC-048) [glm-grade=A]

- New src/config/disk-settings-loader.js runs once at boot (require'd into src/app.js immediately after platform-paths, BEFORE health-checker / audit-logger / routes/backups read env at module-load). - Routes the persisted values from <dataDir>/disk-settings.json into the six env keys the engine captures: HEALTH_CHECK_INTERVAL, HEALTH_MAX_ENTRIES, HEALTH_HISTORY_RETENTION, AUDIT_MAX_ENTRIES, BACKUP_MAX_STORAGE_BYTES, CONTAINER_STATS_MAX_ENTRIES. - Explicit process.env values WIN over persisted file (operator override). - Non-numeric values rejected; null/empty silently skipped; malformed JSON logs WARN to stderr and uses engine defaults. - Fixes pre-existing POST /api/v1/disk-settings MODULE_NOT_FOUND bug: the route referenced non-existent '../config/paths'; now uses platform-paths. - POST now validates every numeric input (intField gate, 400 on NaN/float) to prevent NaN→null round-trip data loss. - Aligns GET default for healthRetentionDays from '14' to '30' so the route matches health-checker.js:34 (engine) and the modal's ||30 fallback. - 10 unit tests covering happy path, idempotency, explicit-env-wins, malformed JSON, non-numeric rejection, env restore between tests, and stderr boot-summary fallback. GLM-5.3 round 1: B (route MODULE_NOT_FOUND + MEDIUM POST NaN→null + boot log LOW). GLM-5.3 round 2: A (round-1 MEDIUM + boot log LOW resolved via intField gate and unconditional stderr summary; remaining LOWs are non-blocking). Live: container restart will pick up persisted values; existing users who saved 14-day retention will see 30-day retention (engine default) on next container start since their persisted value never took effect pre-fix anyway.
2026-08-17 16:04:18 -07:00
parent 4555d829ac
commit 6d875e4631
5 changed files with 447 additions and 14 deletions
@@ -19,6 +19,11 @@ const { asyncHandler } = require('./utils/async-handler');
 // Managers and utilities
 const StateManager = require('./managers/state-manager');
 const platformPaths = require('../platform-paths');
+// DC-048 — rehydrate process.env from disk-settings.json BEFORE any engine
+// module reads env at module-load time. Must run before health-checker,
+// audit-logger, and the backups route module (backups.js reads
+// BACKUP_MAX_STORAGE_BYTES at module load too).
+require('./config/disk-settings-loader')();
 const { LicenseManager } = require('./managers/license-manager');
 const credentialManager = require('./managers/credential-manager');
 const authManager = require('./managers/auth-manager');
@@ -0,0 +1,193 @@
+/**
+ * Disk Settings Bootstrap Loader (DC-048)
+ *
+ * Reads /app/data/disk-settings.json (resolved via platform-paths.dataDir)
+ * at boot time and rehydrates process.env values for engine settings that
+ * were previously captured only via in-memory process.env writes on the
+ * POST /api/v1/disk-settings route.
+ *
+ * Why this exists:
+ *   health-checker.js, audit-logger.js, and backups.js all read
+ *   `process.env.HEALTH_*` / `process.env.AUDIT_MAX_ENTRIES` /
+ *   `process.env.BACKUP_MAX_STORAGE_BYTES` at MODULE LOAD. The previous
+ *   POST handler only wrote those values to process.env at runtime, so
+ *   any value persisted to disk-settings.json was silently discarded on
+ *   every container restart. Users who saved "Health Retention = 7 days"
+ *   would see 30 days come back at the next boot.
+ *
+ * Behavior:
+ *   - Only sets a key if process.env[key] is already UNDEFINED. Explicit
+ *     container / compose env still wins on cold boot (so operators can
+ *     override via the env without editing disk-settings.json).
+ *   - Logs a single INFO line at boot summarizing what was rehydrated.
+ *   - Never throws. A missing or malformed disk-settings.json is logged
+ *     and ignored — the engine falls back to its compiled-in defaults.
+ *
+ * Order of operations in src/app.js:
+ *   require('./config/disk-settings-loader')();   // ← MUST be before any
+ *   const healthChecker = require('./monitoring/health-checker');         //  engine module
+ *   const auditLogger   = require('./security/audit-logger');            //  that reads env
+ *
+ * Mapping table (mirrors the POST handler in routes/disk-settings.js):
+ *   disk-settings.json field  →  process.env key
+ *   healthCheckInterval       →  HEALTH_CHECK_INTERVAL      (ms)
+ *   healthMaxEntries          →  HEALTH_MAX_ENTRIES         (entries)
+ *   healthRetentionDays       →  HEALTH_HISTORY_RETENTION   (days)
+ *   statsMaxEntries           →  CONTAINER_STATS_MAX_ENTRIES(entries; reserved, no engine consumer yet)
+ *   auditMaxEntries           →  AUDIT_MAX_ENTRIES          (entries)
+ *   backupMaxStorageBytes     →  BACKUP_MAX_STORAGE_BYTES   (bytes)
+ *
+ * Returns an object describing what was applied — useful for tests + boot logs.
+ */
+
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+
+const ENV_MAP = Object.freeze({
+  healthCheckInterval:   'HEALTH_CHECK_INTERVAL',
+  healthMaxEntries:      'HEALTH_MAX_ENTRIES',
+  healthRetentionDays:   'HEALTH_HISTORY_RETENTION',
+  statsMaxEntries:       'CONTAINER_STATS_MAX_ENTRIES',
+  auditMaxEntries:       'AUDIT_MAX_ENTRIES',
+  backupMaxStorageBytes: 'BACKUP_MAX_STORAGE_BYTES',
+});
+
+// Numeric fields MUST be coerced to integers; a stray string in disk-settings.json
+// would otherwise land in process.env as a string and the next
+// parseInt(process.env.X || 'N') in the engine would silently fall back to N
+// when the value is unparseable. Defensive coercion here keeps the engine
+// consistent with the values the user just saved.
+const NUMERIC_FIELDS = Object.freeze([
+  'healthCheckInterval',
+  'healthMaxEntries',
+  'healthRetentionDays',
+  'statsMaxEntries',
+  'auditMaxEntries',
+  'backupMaxStorageBytes',
+]);
+
+function loadPersistedSettings(dataDir) {
+  if (!dataDir) return null;
+  const settingsFile = path.join(dataDir, 'disk-settings.json');
+  if (!fs.existsSync(settingsFile)) return null;
+  try {
+    const raw = fs.readFileSync(settingsFile, 'utf8');
+    const parsed = JSON.parse(raw);
+    if (parsed && typeof parsed === 'object' && !Array.isArray(parsed)) {
+      return parsed;
+    }
+    return null;
+  } catch (err) {
+    // Log + swallow. The engine's compiled-in defaults are the safe fallback.
+    // Do NOT re-throw — a malformed settings file must not stop the API from booting.
+    process.stderr.write(
+      `[disk-settings-loader] WARN: failed to parse ${settingsFile}: ${err.message}; using engine defaults\n`,
+    );
+    return null;
+  }
+}
+
+/**
+ * Resolve dataDir WITHOUT importing platform-paths at the top level — the loader
+ * is required very early in app.js, before platform-paths has been fully loaded
+ * by sibling modules. A local require is safe (it's idempotent and side-effect
+ * free — platform-paths is pure constants).
+ */
+function resolveDataDir() {
+  try {
+    // eslint-disable-next-line global-require
+    const platformPaths = require('../../platform-paths');
+    return platformPaths.dataDir;
+  } catch {
+    return process.env.DATA_DIR || '/etc/dashcaddy';
+  }
+}
+
+function applyToEnv(persisted, { logger } = {}) {
+  const applied = [];
+  const skipped = [];
+  if (!persisted) return { applied, skipped };
+
+  for (const [field, envKey] of Object.entries(ENV_MAP)) {
+    if (!Object.prototype.hasOwnProperty.call(persisted, field)) continue;
+    let value = persisted[field];
+    if (value === null || value === undefined || value === '') continue;
+
+    if (NUMERIC_FIELDS.includes(field)) {
+      const n = Number(value);
+      if (!Number.isFinite(n)) {
+        skipped.push({ field, envKey, reason: 'non-numeric' });
+        continue;
+      }
+      value = String(Math.trunc(n));
+    } else {
+      value = String(value);
+    }
+
+    if (process.env[envKey] !== undefined && process.env[envKey] !== '') {
+      // Explicit env wins over persisted file. This is the only way operators
+      // can override a saved value without first deleting the file.
+      skipped.push({ field, envKey, reason: 'env-already-set' });
+      continue;
+    }
+
+    process.env[envKey] = value;
+    applied.push({ field, envKey, value });
+  }
+
+  return { applied, skipped };
+}
+
+let hasRun = false;
+
+/**
+ * Run the loader once. Idempotent — second invocation is a no-op so test
+ * suites that `jest.resetModules()` between cases don't re-apply values
+ * from a stale persisted file across tests.
+ */
+function applyDiskSettings(options = {}) {
+  if (hasRun) return { applied: [], skipped: [], alreadyRun: true };
+  hasRun = true;
+
+  const dataDir = options.dataDir || resolveDataDir();
+  const persisted = loadPersistedSettings(dataDir);
+  const { applied, skipped } = applyToEnv(persisted, options);
+
+  const summary = {
+    applied,
+    skipped,
+    source: persisted ? path.join(dataDir, 'disk-settings.json') : null,
+    alreadyRun: false,
+  };
+
+  if (applied.length > 0) {
+    const msg = `[disk-settings-loader] rehydrated ${applied.length} setting(s) from ${summary.source}: `
+      + applied.map((a) => `${a.field}=${a.value}`).join(', ');
+    // Always emit to stderr at boot — operators need to see rehydration
+    // regardless of whether the app logger is wired yet (the loader runs
+    // at module-load time, before app.js createApp() builds the logger).
+    if (options.logger) options.logger.info(msg);
+    else process.stderr.write(msg + '\n');
+  } else if (skipped.length === 0 && !persisted) {
+    // No persisted file: silent. (No boot noise when nothing to do.)
+  } else if (skipped.length > 0) {
+    const msg = `[disk-settings-loader] skipped ${skipped.length} setting(s) (env-already-set or non-numeric): `
+      + skipped.map((s) => `${s.envKey}(${s.reason})`).join(', ');
+    if (options.logger) options.logger.info(msg);
+    else process.stderr.write(msg + '\n');
+  }
+
+  return summary;
+}
+
+// Exposed for tests that need to reset the once-guard between cases.
+function _resetForTesting() {
+  hasRun = false;
+}
+
+module.exports = applyDiskSettings;
+module.exports.applyDiskSettings = applyDiskSettings;
+module.exports._resetForTesting = _resetForTesting;
+module.exports.ENV_MAP = ENV_MAP;