Fix 16 HIGH/MEDIUM security bugs across API

HIGH fixes: - TOTP disable now requires valid code verification - TOTP secret removed from plaintext file storage - Container ID validated before update/check-update/logs operations - DNS server parameter restricted to configured servers (SSRF prevention) - Backup export no longer includes encryption key - Backup restore of sensitive files requires TOTP re-authentication MEDIUM fixes: - Session cookie Secure flag added - Caddy reload errors no longer leaked to client - saveConfig uses atomic locked updates via configStateManager - Log file path traversal prevented via symlink resolution - Credential cache entries now expire after 5 minutes - _httpFetch enforces 10MB response size limit - External URL path injection into Caddyfile blocked - Custom volume host paths validated against allowed roots - Error logs endpoint no longer returns stack traces - Logo delete path traversal prevented via path.basename() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
2026-03-07 00:15:28 -08:00
parent 6979302fb7
commit 59b6d7d360
12 changed files with 172 additions and 69 deletions
--- a/dashcaddy-api/credential-manager.js
+++ b/dashcaddy-api/credential-manager.js
@@ -15,7 +15,8 @@ const CREDENTIALS_FILE = process.env.CREDENTIALS_FILE || path.join(__dirname, 'c
 class CredentialManager {
  constructor() {
    this.useKeychain = keychainManager.available;
-    this.cache = new Map(); // In-memory cache for performance
+    this.cache = new Map(); // In-memory cache with TTL
+    this.CACHE_TTL_MS = 5 * 60 * 1000; // 5 minutes
    this.lockOptions = {
      retries: { retries: 10, minTimeout: 100, maxTimeout: 300 },
      stale: 30000
@@ -47,7 +48,7 @@ class CredentialManager {
        if (success) {
          // Store metadata separately in file
          await this.storeMetadata(key, metadata);
-          this.cache.set(key, value);
+          this.cache.set(key, { value, exp: Date.now() + this.CACHE_TTL_MS });
          console.log(`[CredentialManager] Stored '${key}' in OS keychain`);
          return true;
        }
@@ -56,7 +57,7 @@ class CredentialManager {

      // Fallback to encrypted file storage
      await this.storeInFile(key, value, metadata);
-      this.cache.set(key, value);
+      this.cache.set(key, { value, exp: Date.now() + this.CACHE_TTL_MS });
      console.log(`[CredentialManager] Stored '${key}' in encrypted file`);
      return true;
    } catch (error) {
@@ -72,16 +73,20 @@ class CredentialManager {
   */
  async retrieve(key) {
    try {
-      // Check cache first
+      // Check cache first (with TTL expiration)
      if (this.cache.has(key)) {
-        return this.cache.get(key);
+        const cached = this.cache.get(key);
+        if (Date.now() < cached.exp) {
+          return cached.value;
+        }
+        this.cache.delete(key);
      }

      // Try OS keychain first
      if (this.useKeychain) {
        const value = await keychainManager.retrieve(key);
        if (value) {
-          this.cache.set(key, value);
+          this.cache.set(key, { value, exp: Date.now() + this.CACHE_TTL_MS });
          return value;
        }
      }
@@ -89,7 +94,7 @@ class CredentialManager {
      // Fallback to encrypted file storage
      const value = await this.retrieveFromFile(key);
      if (value) {
-        this.cache.set(key, value);
+        this.cache.set(key, { value, exp: Date.now() + this.CACHE_TTL_MS });
      }
      return value;
    } catch (error) {
--- a/dashcaddy-api/middleware.js
+++ b/dashcaddy-api/middleware.js
@@ -222,7 +222,7 @@ module.exports = function configureMiddleware(app, {
    const key = cryptoUtils.loadOrCreateKey();
    const sig = crypto.createHmac('sha256', key).update(payloadB64).digest('base64url');
    res.setHeader('Set-Cookie',
-      `${SESSION_COOKIE_NAME}=${payloadB64}.${sig}; Max-Age=${maxAge}; Path=/; HttpOnly; SameSite=Lax`
+      `${SESSION_COOKIE_NAME}=${payloadB64}.${sig}; Max-Age=${maxAge}; Path=/; HttpOnly; Secure; SameSite=Lax`
    );
  }

--- a/dashcaddy-api/routes/apps/helpers.js
+++ b/dashcaddy-api/routes/apps/helpers.js
@@ -129,7 +129,22 @@ module.exports = function(ctx) {
        const parts = vol.split(':');
        const containerPath = parts.slice(1).join(':');
        const override = config.customVolumes.find(cv => cv.containerPath === containerPath);
-        if (override && override.hostPath) return `${toDockerDesktopPath(override.hostPath)}:${containerPath}`;
+        if (override && override.hostPath) {
+          // Validate host path is under allowed roots (docker data dir or media paths)
+          const normalizedHost = path.resolve(override.hostPath);
+          const allowedRoots = [path.resolve(platformPaths.dockerData)];
+          if (config.mediaPath) {
+            config.mediaPath.split(',').map(p => p.trim()).filter(Boolean).forEach(p => allowedRoots.push(path.resolve(p)));
+          }
+          const isAllowed = allowedRoots.some(root =>
+            normalizedHost === root || normalizedHost.startsWith(root + path.sep)
+          );
+          if (!isAllowed) {
+            ctx.log.warn('deploy', 'Custom volume host path rejected', { hostPath: override.hostPath, allowed: allowedRoots });
+            return vol; // Keep original volume, don't apply unsafe override
+          }
+          return `${toDockerDesktopPath(override.hostPath)}:${containerPath}`;
+        }
        return vol;
      });
    }
--- a/dashcaddy-api/routes/auth/totp.js
+++ b/dashcaddy-api/routes/auth/totp.js
@@ -66,7 +66,6 @@ module.exports = function(ctx) {

    ctx.totpConfig.isSetUp = true;
    ctx.totpConfig.enabled = true;
-    ctx.totpConfig.secret = pendingSecret; // Persist to file for auto-restore
    if (ctx.totpConfig.sessionDuration === 'never') {
      ctx.totpConfig.sessionDuration = '24h';
    }
@@ -132,7 +131,11 @@ module.exports = function(ctx) {
  router.post('/totp/disable', ctx.asyncHandler(async (req, res) => {
    const { code } = req.body;

-    if (ctx.totpConfig.enabled && ctx.totpConfig.isSetUp && code) {
+    // Always require a valid TOTP code when TOTP is active
+    if (ctx.totpConfig.enabled && ctx.totpConfig.isSetUp) {
+      if (!code || !/^\d{6}$/.test(code)) {
+        return ctx.errorResponse(res, 400, 'A valid TOTP code is required to disable TOTP');
+      }
      const { authenticator } = require('otplib');
      const secret = await ctx.credentialManager.retrieve('totp.secret');
      if (secret) {
--- a/dashcaddy-api/routes/config/assets.js
+++ b/dashcaddy-api/routes/config/assets.js
@@ -166,10 +166,10 @@ module.exports = function(ctx) {
    const logoPaths = [config.customLogo, config.customLogoDark, config.customLogoLight].filter(Boolean);
    const seen = new Set();
    for (const logoPath of logoPaths) {
-      const filename = logoPath.replace('/assets/', '');
-      if (seen.has(filename)) continue;
+      const filename = path.basename(logoPath);
+      if (!filename || seen.has(filename)) continue;
      seen.add(filename);
-      const filePath = `${assetsPath}/${filename}`;
+      const filePath = path.join(assetsPath, filename);
      if (await exists(filePath)) {
        await fsp.unlink(filePath);
      }
--- a/dashcaddy-api/routes/config/backup.js
+++ b/dashcaddy-api/routes/config/backup.js
@@ -28,7 +28,7 @@ module.exports = function(ctx) {
      { key: 'config', path: ctx.CONFIG_FILE, required: false },
      { key: 'dnsCredentials', path: ctx.dns.credentialsFile, required: false },
      { key: 'credentials', path: process.env.CREDENTIALS_FILE || path.join(__dirname, '..', '..', 'credentials.json'), required: false },
-      { key: 'encryptionKey', path: ENCRYPTION_KEY_FILE, required: false },
+      // NOTE: encryptionKey deliberately excluded — bundling it with encrypted data defeats the encryption
      { key: 'totpConfig', path: ctx.TOTP_CONFIG_FILE, required: false },
      { key: 'tailscaleConfig', path: ctx.TAILSCALE_CONFIG_FILE, required: false },
      { key: 'notifications', path: ctx.NOTIFICATIONS_FILE, required: false }
@@ -70,7 +70,7 @@ module.exports = function(ctx) {
            width: 256, margin: 2,
            color: { dark: '#000000', light: '#ffffff' }
          });
-          backup.totp = { qrCode: qrDataUrl, manualKey: secret, issuer: 'DashCaddy' };
+          backup.totp = { qrCode: qrDataUrl, issuer: 'DashCaddy' };
        }
      } catch (e) {
        ctx.log.warn('backup', 'Could not include TOTP QR in backup', { error: e.message });
@@ -161,27 +161,44 @@ module.exports = function(ctx) {

  // Restore configuration from backup
  router.post('/backup/restore', ctx.asyncHandler(async (req, res) => {
-    const { backup, options = {} } = req.body;
+    const { backup, options = {}, totpCode } = req.body;

    if (!backup || !backup.version || !backup.files) {
      return ctx.errorResponse(res, 400, 'Invalid backup file format');
    }

+    // Require TOTP verification for restores that include security-sensitive files
+    const sensitiveKeys = ['credentials', 'totpConfig', 'encryptionKey'];
+    const restoresSensitive = sensitiveKeys.some(key =>
+      backup.files[key] && backup.files[key].type !== 'missing' && !(options.skip || []).includes(key)
+    );
+    if (restoresSensitive && ctx.totpConfig.enabled && ctx.totpConfig.isSetUp) {
+      if (!totpCode || !/^\d{6}$/.test(totpCode)) {
+        return ctx.errorResponse(res, 400, 'TOTP code required for restoring security-sensitive files');
+      }
+      const { authenticator } = require('otplib');
+      const secret = await ctx.credentialManager.retrieve('totp.secret');
+      if (secret) {
+        authenticator.options = { window: 1 };
+        if (!authenticator.verify({ token: totpCode, secret })) {
+          return ctx.errorResponse(res, 401, '[DC-111] Invalid TOTP code');
+        }
+      }
+    }
+
    const results = {
      restored: [],
      skipped: [],
      errors: []
    };

-    // File mapping
-    const ENCRYPTION_KEY_FILE = process.env.ENCRYPTION_KEY_FILE || path.join(path.dirname(ctx.SERVICES_FILE), '.encryption-key');
+    // File mapping (encryptionKey excluded — must never be overwritten from backup)
    const fileMapping = {
      services: ctx.SERVICES_FILE,
      caddyfile: ctx.caddy.filePath,
      config: ctx.CONFIG_FILE,
      dnsCredentials: ctx.dns.credentialsFile,
      credentials: process.env.CREDENTIALS_FILE || path.join(__dirname, '..', '..', 'credentials.json'),
-      encryptionKey: ENCRYPTION_KEY_FILE,
      totpConfig: ctx.TOTP_CONFIG_FILE,
      tailscaleConfig: ctx.TAILSCALE_CONFIG_FILE,
      notifications: ctx.NOTIFICATIONS_FILE
--- a/dashcaddy-api/routes/containers.js
+++ b/dashcaddy-api/routes/containers.js
@@ -44,7 +44,7 @@ module.exports = function(ctx) {
  // Update container to latest image version
  router.post('/:id/update', ctx.asyncHandler(async (req, res) => {
    const containerId = req.params.id;
-    const container = ctx.docker.client.getContainer(containerId);
+    const container = await getVerifiedContainer(containerId);

      // Get container info
      const containerInfo = await container.inspect();
@@ -124,7 +124,7 @@ module.exports = function(ctx) {
  // Check for available updates (compares local and remote image digests)
  router.get('/:id/check-update', ctx.asyncHandler(async (req, res) => {
    const containerId = req.params.id;
-    const container = ctx.docker.client.getContainer(containerId);
+    const container = await getVerifiedContainer(containerId);
    const containerInfo = await container.inspect();
    const imageName = containerInfo.Config.Image;

--- a/dashcaddy-api/routes/dns.js
+++ b/dashcaddy-api/routes/dns.js
@@ -8,6 +8,17 @@ const { exists } = require('../fs-helpers');
 module.exports = function(ctx) {
  const router = express.Router();

+  /** Validate that a server IP is in the configured DNS servers list */
+  function validateDnsServer(server) {
+    const serverIp = server.includes(':') ? server.split(':')[0] : server;
+    if (!validatorLib.isIP(serverIp)) return null;
+    const configuredIps = Object.values(ctx.siteConfig.dnsServers || {}).map(s => s.ip).filter(Boolean);
+    // Also allow the default dnsServerIp
+    if (ctx.siteConfig.dnsServerIp) configuredIps.push(ctx.siteConfig.dnsServerIp);
+    if (!configuredIps.includes(serverIp)) return null;
+    return serverIp;
+  }
+
  // DELETE /record — Delete a DNS record from Technitium
  router.delete('/record', ctx.asyncHandler(async (req, res) => {
    const { domain, type, token, server, ipAddress } = req.query;
@@ -33,9 +44,9 @@ module.exports = function(ctx) {
      return ctx.errorResponse(res, 400, '[DC-210] Invalid IP address');
    }

-    // Validate server if provided
-    if (server && !validatorLib.isIP(server)) {
-      return ctx.errorResponse(res, 400, 'Invalid DNS server address');
+    // Validate server against configured DNS servers
+    if (server && !validateDnsServer(server)) {
+      return ctx.errorResponse(res, 400, 'Server must be a configured DNS server');
    }

    // Default to dns1 LAN IP, allow override
@@ -85,9 +96,9 @@ module.exports = function(ctx) {
      }
    }

-    // Validate server IP if provided
-    if (server && !validatorLib.isIP(server)) {
-      return ctx.errorResponse(res, 400, 'Invalid DNS server address');
+    // Validate server against configured DNS servers
+    if (server && !validateDnsServer(server)) {
+      return ctx.errorResponse(res, 400, 'Server must be a configured DNS server');
    }

    // Default to dns1 LAN IP since Docker container can't access Tailscale network
@@ -131,9 +142,9 @@ module.exports = function(ctx) {
      return ctx.errorResponse(res, 400, '[DC-301] Invalid domain format');
    }

-    // Validate server if provided
-    if (server && !validatorLib.isIP(server)) {
-      return ctx.errorResponse(res, 400, 'Invalid DNS server address');
+    // Validate server against configured DNS servers
+    if (server && !validateDnsServer(server)) {
+      return ctx.errorResponse(res, 400, 'Server must be a configured DNS server');
    }

    const dnsServer = server || ctx.siteConfig.dnsServerIp;
@@ -169,17 +180,17 @@ module.exports = function(ctx) {
      return ctx.errorResponse(res, 400, 'server is required');
    }

-    // Validate server is an IP address or hostname to prevent SSRF
-    const serverClean = server.includes(':') ? server.split(':')[0] : server;
-    if (!validatorLib.isIP(serverClean) && !validatorLib.isFQDN(serverClean)) {
-      return ctx.errorResponse(res, 400, 'Invalid DNS server address');
+    // Validate server against configured DNS servers
+    const serverIp = validateDnsServer(server);
+    if (!serverIp) {
+      return ctx.errorResponse(res, 400, 'Server must be a configured DNS server');
    }

    const logLimit = Math.min(parseInt(limit) || 25, 1000);
+    const dnsPort = ctx.siteConfig.dnsServerPort || '5380';

    try {
      // Auto-authenticate using stored read-only credentials for log access
-      const serverIp = server.includes(':') ? server.split(':')[0] : server;
      const authResult = await ctx.dns.getTokenForServer(serverIp, 'readonly');
      if (!authResult.success) {
        return ctx.errorResponse(res, 401, 'DNS auto-authentication failed. Ensure credentials are configured via the DNS panel.');
@@ -187,7 +198,7 @@ module.exports = function(ctx) {
      const effectiveToken = authResult.token;

      // Try to get available log files first
-      const listUrl = `http://${server}/api/logs/list?token=${encodeURIComponent(effectiveToken)}`;
+      const listUrl = `http://${serverIp}:${dnsPort}/api/logs/list?token=${encodeURIComponent(effectiveToken)}`;
      const listResponse = await ctx.fetchT(listUrl, { method: 'GET', headers: { 'Accept': 'application/json' } });

      let logFileName = new Date().toISOString().split('T')[0]; // Default to today
@@ -201,7 +212,7 @@ module.exports = function(ctx) {
      }

      // Technitium logs/download endpoint - returns plain text logs
-      const technitiumUrl = `http://${server}/api/logs/download?token=${encodeURIComponent(effectiveToken)}&fileName=${logFileName}`;
+      const technitiumUrl = `http://${serverIp}:${dnsPort}/api/logs/download?token=${encodeURIComponent(effectiveToken)}&fileName=${logFileName}`;
      ctx.log.info('dns', 'Fetching DNS logs', { server, logFileName });

      const response = await ctx.fetchT(technitiumUrl, {
@@ -400,8 +411,8 @@ module.exports = function(ctx) {
      return ctx.errorResponse(res, 400, 'Username contains invalid characters');
    }

-    if (server && !validatorLib.isIP(server)) {
-      return ctx.errorResponse(res, 400, 'Invalid DNS server address');
+    if (server && !validateDnsServer(server)) {
+      return ctx.errorResponse(res, 400, 'Server must be a configured DNS server');
    }

    const testResult = await ctx.dns.refresh(username, password, server || ctx.siteConfig.dnsServerIp);
@@ -499,13 +510,19 @@ module.exports = function(ctx) {
        return ctx.errorResponse(res, 400, 'Server IP required');
      }

+      const serverIp = validateDnsServer(server);
+      if (!serverIp) {
+        return ctx.errorResponse(res, 400, 'Server must be a configured DNS server');
+      }
+
      // Authenticate with admin credentials for update check
-      const tokenResult = await ctx.dns.getTokenForServer(server, 'admin');
+      const tokenResult = await ctx.dns.getTokenForServer(serverIp, 'admin');
      if (!tokenResult.success) {
        return ctx.errorResponse(res, 401, 'DNS authentication failed. Ensure credentials are configured.');
      }

-      const url = `http://${server}:5380/api/user/checkForUpdate?token=${encodeURIComponent(tokenResult.token)}`;
+      const dnsPort = ctx.siteConfig.dnsServerPort || '5380';
+      const url = `http://${serverIp}:${dnsPort}/api/user/checkForUpdate?token=${encodeURIComponent(tokenResult.token)}`;
      ctx.log.info('dns', 'Checking DNS update', { server });

      const response = await ctx.fetchT(url, {
@@ -554,15 +571,21 @@ module.exports = function(ctx) {
        return ctx.errorResponse(res, 400, 'Server IP required');
      }

+      const serverIp = validateDnsServer(server);
+      if (!serverIp) {
+        return ctx.errorResponse(res, 400, 'Server must be a configured DNS server');
+      }
+
      // Authenticate with admin credentials for update operations
-      const tokenResult = await ctx.dns.getTokenForServer(server, 'admin');
+      const tokenResult = await ctx.dns.getTokenForServer(serverIp, 'admin');
      if (!tokenResult.success) {
        return ctx.errorResponse(res, 401, 'DNS authentication failed. Ensure credentials are configured.');
      }

+      const dnsPort = ctx.siteConfig.dnsServerPort || '5380';
      // Check if update is available
      const checkResponse = await ctx.fetchT(
-        `http://${server}:5380/api/user/checkForUpdate?token=${encodeURIComponent(tokenResult.token)}`,
+        `http://${serverIp}:${dnsPort}/api/user/checkForUpdate?token=${encodeURIComponent(tokenResult.token)}`,
        { method: 'GET', headers: { 'Accept': 'application/json' } }
      );

--- a/dashcaddy-api/routes/errorlogs.js
+++ b/dashcaddy-api/routes/errorlogs.js
@@ -25,8 +25,7 @@ module.exports = function(ctx) {
          return {
            timestamp: match[1],
            context: match[2],
-            error: match[3],
-            details: lines.slice(1).join('\n').trim()
+            error: match[3]
          };
        }
        return null;
--- a/dashcaddy-api/routes/logs.js
+++ b/dashcaddy-api/routes/logs.js
@@ -32,7 +32,16 @@ module.exports = function(ctx) {
    const timestamps = req.query.timestamps !== 'false';

    const container = ctx.docker.client.getContainer(containerId);
-    const info = await container.inspect();
+    let info;
+    try {
+      info = await container.inspect();
+    } catch (err) {
+      if (err.statusCode === 404 || (err.message && err.message.includes('no such container'))) {
+        const { NotFoundError } = require('../errors');
+        throw new NotFoundError(`Container ${containerId}`);
+      }
+      throw err;
+    }
    const containerName = info.Name.replace(/^\//, '');

    const logs = await container.logs({
@@ -74,6 +83,15 @@ module.exports = function(ctx) {
  router.get('/logs/stream/:id', ctx.asyncHandler(async (req, res) => {
    const containerId = req.params.id;
    const container = ctx.docker.client.getContainer(containerId);
+    try {
+      await container.inspect();
+    } catch (err) {
+      if (err.statusCode === 404 || (err.message && err.message.includes('no such container'))) {
+        const { NotFoundError } = require('../errors');
+        throw new NotFoundError(`Container ${containerId}`);
+      }
+      throw err;
+    }

    res.setHeader('Content-Type', 'text/event-stream');
    res.setHeader('Cache-Control', 'no-cache');
@@ -132,20 +150,32 @@ module.exports = function(ctx) {
    const allowedPaths = platformPaths.allowedLogPaths;

    const normalizedPath = path.normalize(logPath);
-    const isAllowed = allowedPaths.some(allowed =>
-      normalizedPath.startsWith(path.normalize(allowed))
-    );
+
+    // Resolve symlinks to prevent symlink-based traversal
+    let resolvedPath;
+    try {
+      resolvedPath = await fsp.realpath(normalizedPath);
+    } catch {
+      const { NotFoundError } = require('../errors');
+      throw new NotFoundError('Log file');
+    }
+
+    // Check path against allowed roots with separator boundary
+    const isAllowed = allowedPaths.some(allowed => {
+      const normalizedAllowed = path.normalize(allowed);
+      return resolvedPath === normalizedAllowed || resolvedPath.startsWith(normalizedAllowed + path.sep);
+    });

    if (!isAllowed) {
      return ctx.errorResponse(res, 403, 'Access to this log path is not allowed');
    }

-    if (!await exists(normalizedPath)) {
+    if (!await exists(resolvedPath)) {
      const { NotFoundError } = require('../errors');
      throw new NotFoundError('Log file');
    }

-    const fileContent = await fsp.readFile(normalizedPath, 'utf8');
+    const fileContent = await fsp.readFile(resolvedPath, 'utf8');
    const lines = fileContent.split('\n').filter(line => line.trim());
    const tailLines = lines.slice(-tail);

--- a/dashcaddy-api/routes/sites.js
+++ b/dashcaddy-api/routes/sites.js
@@ -30,7 +30,8 @@ module.exports = function(ctx) {

    if (!response.ok) {
      const errorText = await response.text();
-      return ctx.errorResponse(res, 500, `[DC-303] Caddy reload failed: ${errorText}`);
+      ctx.log.error('caddy', 'Caddy reload failed', { error: errorText });
+      return ctx.errorResponse(res, 500, '[DC-303] Caddy reload failed. Check server logs for details.');
    }

    res.json({ success: true, message: 'Caddy configuration reloaded successfully' });
@@ -202,6 +203,13 @@ module.exports = function(ctx) {
      const hostHeader = preserveHost ? `\n        header_up Host {upstream_hostport}` : '';

      const urlObj = new URL(externalUrl);
+
+      // Validate URL components are safe for Caddyfile syntax
+      const unsafeCaddyChars = /[{}\n\r]/;
+      if (unsafeCaddyChars.test(urlObj.host) || unsafeCaddyChars.test(urlObj.pathname)) {
+        return ctx.errorResponse(res, 400, 'External URL contains characters not safe for Caddy configuration');
+      }
+
      const baseUrl = `${urlObj.protocol}//${urlObj.host}`;
      const urlPath = urlObj.pathname.replace(/\/$/, '');

--- a/dashcaddy-api/server.js
+++ b/dashcaddy-api/server.js
@@ -230,9 +230,19 @@ function _httpFetch(url, opts = {}, timeoutMs = TIMEOUTS.HTTP_DEFAULT) {
    if (opts.body) {
      options.headers['Content-Length'] = Buffer.byteLength(opts.body);
    }
+    const MAX_RESPONSE_SIZE = 10 * 1024 * 1024; // 10MB
    const req = http.request(options, (res) => {
      let data = '';
-      res.on('data', chunk => data += chunk);
+      let size = 0;
+      res.on('data', chunk => {
+        size += chunk.length;
+        if (size > MAX_RESPONSE_SIZE) {
+          res.destroy();
+          reject(new Error(`Response from ${url} exceeded ${MAX_RESPONSE_SIZE} bytes`));
+          return;
+        }
+        data += chunk;
+      });
      res.on('end', () => {
        resolve({
          ok: res.statusCode >= 200 && res.statusCode < 300,
@@ -317,12 +327,11 @@ async function readConfig() {
  return readJsonFile(CONFIG_FILE, {});
 }

-/** Save config.json (merges with existing) */
+/** Save config.json (merges with existing, atomic with locking) */
 async function saveConfig(updates) {
-  const config = await readConfig();
-  Object.assign(config, updates);
-  await writeJsonFile(CONFIG_FILE, config);
-  return config;
+  return await configStateManager.update(config => {
+    return Object.assign(config, updates);
+  });
 }

 /**
@@ -676,17 +685,11 @@ async function loadTotpConfig() {
  try {
    if (await exists(TOTP_CONFIG_FILE)) {
      const data = await fsp.readFile(TOTP_CONFIG_FILE, 'utf8');
-      Object.assign(totpConfig, JSON.parse(data));
+      const loaded = JSON.parse(data);
+      // Never load secret from file — it belongs only in credential-manager
+      delete loaded.secret;
+      Object.assign(totpConfig, loaded);
      log.info('config', 'TOTP config loaded', { enabled: totpConfig.enabled });
-
-      // Auto-restore: if config has a backup secret but credential manager lost it, re-store it
-      if (totpConfig.secret && totpConfig.isSetUp) {
-        const existing = await credentialManager.retrieve('totp.secret');
-        if (!existing) {
-          await credentialManager.store('totp.secret', totpConfig.secret);
-          log.info('config', 'TOTP secret auto-restored from config backup');
-        }
-      }
    }
  } catch (e) {
    await logError('loadTotpConfig', e);