fix(caddycode): validate + escape generation config — block CRLF / " / brace injection in Caddyfile interpolation (DC-070) [glm-grade=A]

dashcaddy-api/routes/caddycode.js

@@ -11,10 +11,138 @@
  */
 const express = require('express');
 const { ok, errorResponse } = require('../src/utils/responses');
+const { REGEX } = require('../src/utilities/constants');
+
+/**
+ * DC-070: Validate the structural config that flows into generateSiteBlock.
+ *
+ * Threat model: `generateSiteBlock` interpolates user-controlled fields
+ * (domain, tls, authService, headers.*, stripPrefix, upstream) DIRECTLY into
+ * a Caddyfile text block that is later fed to `caddy.modify()` and the
+ * Caddy admin /load endpoint. The /caddycode/generate endpoint is
+ * authenticated (forward_auth gated), but the bug class is "compromised
+ * middleware / pivot" — a JSON-only payload can be smuggled past any
+ * UI-side input checks.
+ *
+ * Pre-fix, every field was trusted: `lines.push(`${domain} {`)` accepted any
+ * string (including newlines that close the block and inject a new site),
+ * `headers[key] = "${value}"` accepted arbitrary quotes (which would break
+ * the surrounding `"..."` Caddy quoted-string context and inject directives),
+ * and `tls`, `authService`, `stripPrefix`, `upstream` had no charset
+ * restrictions at all (spaces, braces, semicolons would land verbatim).
+ *
+ * Post-fix: every field is constrained to a known-safe character class
+ * BEFORE interpolation, and CRLF is rejected outright. Quoted-string
+ * injection in header values is closed by escaping `\` and `"` per the
+ * Caddy quoted-string spec (backslash escapes the next character).
+ */
+function validateGenerationConfig(config) {
+  const errors = [];
+  const {
+    domain,
+    upstream,
+    upstreamProtocol = 'http',
+    tls = 'auto',
+    auth = false,
+    authService = null,
+    headers = {},
+    stripPrefix = null,
+  } = config;
+
+  // 1. domain — RFC 1123 hostname. Reject anything with whitespace, brace,
+  //    semicolon, newline, or non-printable. REGEX.DOMAIN is
+  //    /^[a-z0-9]([a-z0-9.-]{0,251}[a-z0-9])?$/i in constants.js.
+  if (typeof domain !== 'string' || !REGEX.DOMAIN.test(domain)) {
+    errors.push('domain must be a valid hostname (letters, digits, dots, hyphens)');
+  }
+
+  // 2. upstream — `host:port` form (the only shape Caddy's reverse_proxy
+  //    directive takes for non-URL upstreams). Reject `://`, whitespace,
+  //    braces. Allow optional IPv6 bracket form `[::1]:5000`. Must
+  //    include an explicit :port segment — a bare `localhost` would
+  //    produce a Caddyfile that fails to reload (port required for
+  //    reverse_proxy upstreams). Two regex branches: (a) bare host with
+  //    required :port, (b) bracketed IPv6 literal with required :port.
+  if (typeof upstream !== 'string'
+      || !/^[a-z0-9.\-]+:\d{1,5}$/i.test(upstream)
+      && !/^\[[a-z0-9.\-:.]+\]:\d{1,5}$/i.test(upstream)
+  ) {
+    errors.push('upstream must be host:port (host letters/digits/dots/hyphens, port 1-65535, optional IPv6 brackets)');
+  }
+
+  // 3. tls — either the literal strings 'auto' / 'internal' (handled
+  //    specially below) OR a CA name like 'letsencrypt' / 'internal' that
+  //    must match /^[a-z0-9._-]+$/i. Reject whitespace + braces + quotes.
+  if (typeof tls !== 'string' || !/^[a-z0-9._-]+$/i.test(tls)) {
+    errors.push('tls must be one of: auto, internal, or a CA name (letters, digits, dots, underscores, hyphens)');
+  }
+
+  // 4. authService — only meaningful when auth=true; otherwise ignore. Must
+  //    match the existing SSO service-id charset (REGEX.SUBDOMAIN).
+  if (auth) {
+    if (typeof authService !== 'string' || !REGEX.SUBDOMAIN.test(authService)) {
+      errors.push('authService must be a valid subdomain (lowercase, alphanumeric, hyphens)');
+    }
+  }
+
+  // 5. upstreamProtocol — only 'http' or 'https'. Anything else gets coerced
+  //    to 'http' but only after we explicitly accept it; reject obvious
+  //    injection vectors here.
+  if (upstreamProtocol !== 'http' && upstreamProtocol !== 'https') {
+    errors.push('upstreamProtocol must be "http" or "https"');
+  }
+
+  // 6. headers — each key must be a valid HTTP header name ([A-Za-z0-9-]+),
+  //    each value must be a string with no CR/LF and no unescaped quotes.
+  if (headers && typeof headers === 'object') {
+    for (const [key, value] of Object.entries(headers)) {
+      if (typeof key !== 'string' || !/^[A-Za-z0-9-]+$/.test(key)) {
+        errors.push(`header key "${String(key)}" must be HTTP-token chars only ([A-Za-z0-9-])`);
+      }
+      if (typeof value !== 'string') {
+        errors.push(`header "${key}" value must be a string`);
+        continue;
+      }
+      if (/[\r\n]/.test(value)) {
+        errors.push(`header "${key}" value must not contain CR or LF`);
+      }
+    }
+  }
+
+  // 7. stripPrefix — must be a leading-slash path with safe chars. Reject
+  //    braces, quotes, whitespace, and { } which would let the attacker
+  //    open a new Caddyfile block.
+  if (stripPrefix != null) {
+    if (typeof stripPrefix !== 'string' || !/^\/[A-Za-z0-9._\-/]*$/.test(stripPrefix)) {
+      errors.push('stripPrefix must be an absolute path (letters, digits, dots, hyphens, slashes)');
+    }
+  }
+
+  return { valid: errors.length === 0, errors };
+}
+
+/**
+ * Escape a string for safe interpolation inside a Caddyfile quoted-string
+ * context. Caddy uses the same backslash-escape semantics as JSON-ish
+ * contexts — `\` and `"` MUST be escaped, otherwise the attacker breaks out
+ * of the quoted string and injects arbitrary directives.
+ *
+ * @param {string} s raw header value
+ * @returns {string} escaped value (no embedded newlines; CR/LF were already
+ *   rejected by the validator)
+ */
+function escapeCaddyQuotedString(s) {
+  return String(s).replace(/\\/g, '\\\\').replace(/"/g, '\\"');
+}
 
 /**
  * Generate a Caddyfile site block from a structured config.
- * @param {Object} config - Site configuration
+ *
+ * Every interpolated field is now validated by `validateGenerationConfig`
+ * first (see DC-070). Quoted-string values are escaped via
+ * `escapeCaddyQuotedString` so a `"` in a header value cannot break out.
+ *
+ * @param {Object} config - Site configuration (already validated)
  * @returns {string} Caddyfile snippet
  */
 function generateSiteBlock(config) {
@@ -38,12 +166,15 @@ function generateSiteBlock(config) {
   const lines = [];
   lines.push(`${domain} {`);
 
-  // TLS
+  // TLS — only emit a tls directive when explicitly 'internal' or a CA
+  // name; 'auto' means Caddy's default behaviour (no directive needed).
   if (tls === 'internal') {
     lines.push(`  tls internal`);
   } else if (tls === 'auto') {
     // Default — Caddy auto-provisions Let's Encrypt
-  } else if (typeof tls === 'string') {
+  } else {
+    // CA name validated by validateGenerationConfig against
+    // /^[a-z0-9._-]+$/i — safe to interpolate verbatim.
     lines.push(`  tls ${tls}`);
   }
 
@@ -52,7 +183,8 @@ function generateSiteBlock(config) {
     lines.push(`  # Redirect HTTP to HTTPS is automatic in Caddy 2`);
   }
 
-  // Auth gate (DashCaddy forward_auth)
+  // Auth gate (DashCaddy forward_auth) — authService validated by
+  // validateGenerationConfig against REGEX.SUBDOMAIN — safe to interpolate.
   if (auth && authService) {
     lines.push(`  import dashcaddy_auth ${authService}`);
   }
@@ -66,16 +198,17 @@ function generateSiteBlock(config) {
     lines.push(`  }`);
   }
 
-  // Custom headers
-  if (Object.keys(headers).length > 0) {
+  // Custom headers — keys validated against /^[A-Za-z0-9-]+$/, values
+  // escaped via escapeCaddyQuotedString before being placed inside "..."
+  if (headers && typeof headers === 'object' && Object.keys(headers).length > 0) {
     lines.push(`  header {`);
     for (const [key, value] of Object.entries(headers)) {
-      lines.push(`    ${key} "${value}"`);
+      lines.push(`    ${key} "${escapeCaddyQuotedString(value)}"`);
     }
     lines.push(`  }`);
   }
 
-  // Strip prefix
+  // Strip prefix — validated to /^\/[A-Za-z0-9._\-/]*$/ — safe.
   if (stripPrefix) {
     lines.push(`  uri strip_prefix ${stripPrefix}`);
   }
@@ -118,6 +251,19 @@ module.exports = function({ asyncHandler }) {
       return errorResponse(res, 400, 'upstream is required (e.g. localhost:8080)');
     }
 
+    // DC-070: structural validation BEFORE interpolation. Every field that
+    // flows into the Caddyfile text must satisfy a known-safe charset rule,
+    // and CRLF is rejected outright. Run this BEFORE generateSiteBlock so
+    // the bad input is rejected with a clean 400 + enumerable error list,
+    // not a generated-Caddyfile + 500.
+    const validation = validateGenerationConfig(config);
+    if (!validation.valid) {
+      return errorResponse(res, 400, 'Invalid configuration', {
+        code: 'DC-CCD-700',
+        errors: validation.errors,
+      });
+    }
+
     try {
       const caddyfile = generateSiteBlock(config);
       ok(res, { caddyfile, config });
@@ -225,3 +371,11 @@ module.exports = function({ asyncHandler }) {
 
   return router;
 };
+
+// DC-070: export helpers for unit-testing the sanitization surface
+// independently of the route handler.
+module.exports.__test = {
+  validateGenerationConfig,
+  escapeCaddyQuotedString,
+  generateSiteBlock,
+};