sami7777/dashcaddy
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases 1 Wiki Activity

fix(disaster-recovery): stage Caddyfile + close path-traversal in assets/themes (DC-079) [glm-grade=A]
CI / Test & Lint (push) Canceled after 0s
Details
CI / Security audit (push) Canceled after 0s
Details

Browse Source 
DC-079 2-round GLM-5.3 judge verdict: round1=C (blocking path-traversal
in assets/themes) → round2=A. 20/20 tests in routes/discover-disaster
(8 original + 12 new). Full repo: 2351/2351 (4 pre-existing billing
pdfkit failures unchanged).

THREAT MODEL
POST /api/v1/disaster/restore was the ONLY endpoint in the route tree
that wrote directly to process.env.CADDYFILE_PATH (=/caddyfile in
container = /etc/caddy/Caddyfile on host via start.sh:161 bind-mount).
Pre-fix: an authenticated dashboard operator POSTed
  {caddyfile: '<attacker-controlled-string>'}
and the handler called fsp.writeFile(caddyfilePath, snapshot.caddyfile),
overwriting the live Caddyfile immediately. Caddy reads this file on
every reload (ACME renewal, health probe, admin API touch), so the
attacker-controlled content executes as Caddy config directives:
  - import /etc/caddy/<anything-caddy-can-read> (content theft)
  - admin off (lock out admin API)
  - reverse_proxy to attacker IPs (Caddy becomes a pivot)
  - acme_ca override to attacker CA (rogue cert issuance)
  - log to attacker-writable paths (DoS/escape)
This bypassed the CLAUDE.md hard rule 'Caddyfile edits must use
caddy-apply' (validates + reloads + git-commits atomically).

FIX 1 — Caddyfile staging (round-1)
- New validateCaddyfileContent(): type check, non-empty check,
  512 KiB byte cap (defense-in-depth below the 1 MB body-parser limit),
  FORBIDDEN_IMPORT_RE rejects  directives with absolute paths,
  ../-escape, ~/, or URL-encoded payloads.
- POST /disaster/restore now writes to <dataDir>/disaster-staged/
  Caddyfile.candidate (atomic write + rename), NEVER to caddyfilePath.
- Response includes caddyfileStaged[{file, stagedPath, action: 'awaiting
  caddy-apply', livePath}] and a DC-079 warning instructing the operator
  to run `caddy-apply <reason>` to validate + reload + git-commit.

FIX 2 — assets/themes path-traversal (round-2 BLOCKING)
GLM round-1 caught a parallel vector: snapshot.assets[name] and
snapshot.themes[name] are user-controlled JSON keys flowing into
path.join(assetsDir, name) and path.join(themesDir, name). An attacker
could POST {assets: {'../../etc/caddy/Caddyfile': '<base64-evil>'}}
and overwrite the live Caddyfile via the dataDir bind-mount, fully
bypassing Fix 1.
- ASSET_KEY_RE = /^[a-zA-Z0-9._-]+$/ + ASSET_PATH_TRAVERSAL_RE catch
  slashes, leading '..', and absolute-path keys.
- THEME_NAME_RE = /^[a-zA-Z0-9._-]+\.json\$/ additionally forces
  .json extension and no slashes.
- assertSafeAssetKey/assertSafeThemeName helpers throw on invalid input.
- Both restore loops now: assert → path.resolve(dir, name) → containment
  check (resolved must start with path.resolve(dir) + path.sep) → write
  to resolved (never the raw join).

TESTS
12 new tests in __tests__/routes/discover-disaster.routes.test.js:
- staging: live sentinel unchanged, candidate at expected path
- rejects: non-string, empty, oversize, 3 forbidden-import variants
- assets: path-traversal key, absolute-path key
- themes: path-traversal name, no-extension name
- back-compat: no caddyfile field succeeds without staging
This commit is contained in:
dashcaddy-polish
2026-08-18 17:52:49 -07:00
parent a4e4b24732
commit a7260436d1
2 changed files with 453 additions and 12 deletions
Download Patch File Download Diff File Expand all files Collapse all files
dashcaddy-api/routes/disaster-recovery.js
+185 -11
View File
@@ -37,6 +37,81 @@ const BACKUP_FILES = [
const ASSET_FILES = ['custom-logo.png', 'custom-favicon.png', 'custom-logo.svg'];
// DC-079: Restrict restored assets to the hardcoded ASSET_FILES allowlist.
// The asset KEYS in the snapshot are user-controlled JSON, so iterating
// `Object.entries(snapshot.assets)` and writing each name verbatim into
// `path.join(assetsDir, name)` lets an attacker POST `{assets: {"../../etc/caddy/Caddyfile":
// "<base64-evil>"}}` and overwrite the live Caddyfile via the bind-mount
// (path.join('/app/data/assets', '../../etc/caddy/Caddyfile') resolves
// to /etc/caddy/Caddyfile). This bypasses the caddyfile-staging gate
// above because the dataDir bind-mount can write to /etc/caddy on the host.
const ASSET_KEY_RE = /^[a-zA-Z0-9._-]+$/;
const ASSET_PATH_TRAVERSAL_RE = /(^|\/)\.\.($|\/)|^\//;
// DC-079: Caddyfile content safety limits for disaster-recovery restore.
// The live Caddyfile on DNS2 is ~17 KB and grows linearly with vhost count.
// Express's default JSON body parser limit (1 MB) is the outer gate; this
// in-handler cap is defense-in-depth against either a future body-limit
// raise or a custom body parser. Cap well below the body-parser ceiling.
const MAX_CADDYFILE_BYTES = 512 * 1024; // 512 KiB — 30x the live file, far below 1 MB body limit
// DC-079: theme filenames must match this pattern. No slashes (no path
// traversal), no `..`, must end in `.json`, and only filename-safe chars.
// Themes are written to <dataDir>/themes/<name>; we also defense-in-depth
// check the resolved path stays inside that dir.
const THEME_NAME_RE = /^[a-zA-Z0-9._-]+\.json$/;
function assertSafeAssetKey(key) {
if (typeof key !== 'string' || key.length === 0 || key.length > 128) {
throw new Error(`asset key must be a non-empty string up to 128 chars`);
}
if (ASSET_PATH_TRAVERSAL_RE.test(key) || !ASSET_KEY_RE.test(key)) {
throw new Error(`asset key contains forbidden characters or path segments`);
}
}
function assertSafeThemeName(name) {
if (typeof name !== 'string' || name.length === 0 || name.length > 128) {
throw new Error(`theme name must be a non-empty string up to 128 chars`);
}
if (!THEME_NAME_RE.test(name)) {
throw new Error(`theme name must match ${THEME_NAME_RE} (alphanum / dot / dash / underscore, ending in .json)`);
}
}
// Reject Caddyfile content that smuggles in arbitrary `import` directives.
// caddy-apply expects the single top-level Caddyfile; any `import` to an
// absolute path means "load another file from disk at Caddy reload time" —
// that's a classic injection vector (an attacker can craft a snapshot whose
// `import /etc/caddy/external.caddy` reads any file Caddy can read).
// We allow the relative-style `import <snippet>` form ONLY if the snippet
// name matches a small allowlist of well-known Caddy snippet names (none
// today; add explicit names if a future snippet module is needed).
const FORBIDDEN_IMPORT_RE = /^\s*import\s+(["']|\/|\.\.|~\/|%[A-F0-9]{2})/im;
function validateCaddyfileContent(content) {
if (typeof content !== 'string') {
return { ok: false, error: 'Caddyfile content must be a string' };
}
if (content.length === 0) {
return { ok: false, error: 'Caddyfile content is empty' };
}
if (Buffer.byteLength(content, 'utf8') > MAX_CADDYFILE_BYTES) {
return { ok: false, error: `Caddyfile content exceeds ${MAX_CADDYFILE_BYTES} bytes` };
}
if (FORBIDDEN_IMPORT_RE.test(content)) {
// Allow the canonical single-quoted snippet import form ONLY if the
// snippet name is on the explicit allowlist (currently empty). This
// catches absolute paths, ../, ~/, and URL-encoded payloads while
// leaving room for future snippet additions without touching this gate.
return {
ok: false,
error: 'Caddyfile contains forbidden `import` directive (absolute path, encoded, or non-allowlisted snippet)'
};
}
return { ok: true };
}
module.exports = function({ servicesStateManager, platformPaths, log, asyncHandler }) {
const wrap = asyncHandler || ((fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next));
const router = express.Router();
@@ -44,6 +119,15 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
let lastBackupStatus = { timestamp: null, status: null, size: null };
let lastRestoreStatus = { timestamp: null, status: null };
// DC-079: Staging dir for the candidate Caddyfile. The disaster-recovery
// restore endpoint stages here instead of writing directly to the live
// Caddyfile path. The operator must run `caddy-apply` (or its equivalent)
// to validate + reload + git-commit the staged file. This keeps the live
// Caddyfile under the same atomic-commit guard as every other edit.
function getStagedCaddyfileDir(dataDir) {
return path.join(dataDir, 'disaster-staged');
}
/**
* POST /api/v1/disaster/backup
* Creates a complete system snapshot as a downloadable JSON file.
@@ -175,13 +259,64 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
}
}
// Restore Caddyfile
if (snapshot.caddyfile) {
// DC-079: Stage the Caddyfile to a staging path inside dataDir
// instead of writing directly to caddyfilePath (which is the LIVE
// /etc/caddy/Caddyfile bind-mounted into the container as /caddyfile).
//
// Threat model (defense-in-depth, mirrors DC-070 / DC-074 / DC-076):
// the endpoint is TOTP-gated, but a compromised operator / phished
// session / pivot path could POST a snapshot with `caddyfile: <evil>`
// and the pre-fix code would call `fsp.writeFile(caddyfilePath, ...)`
// which writes the attacker-controlled string straight to the live
// Caddyfile. Caddy then reads that file on the next reload (which can
// be triggered by ACME renewals, health probes, or any admin API
// touch), executing whatever directives the attacker embedded:
// - `admin off` + arbitrary config write
// - `import /etc/caddy/<anything-caddy-can-read>` for content theft
// - `reverse_proxy` to attacker-controlled upstreams
// - `acme_ca` override to attacker CA
// - `log` directives to attacker-writable paths
//
// The Caddyfile is managed by the `caddy-apply` wrapper (validates +
// reloads + git-commits atomically — see CLAUDE.md hard rule). This
// endpoint previously bypassed that wrapper. The fix stages the
// candidate file under dataDir/disaster-staged/Caddyfile.candidate and
// returns the path so the operator can apply it via the normal flow.
const caddyfileStaged = [];
// DC-079: handle three cases for the caddyfile field:
// - absent/null/undefined: back-compat — no Caddyfile in snapshot
// - empty string "": explicit empty payload is suspicious — reject
// - non-string (object/array/number): type confusion attempt — reject
// - valid string: stage to dataDir/disaster-staged/Caddyfile.candidate
if (snapshot.caddyfile !== undefined && snapshot.caddyfile !== null) {
const validation = validateCaddyfileContent(snapshot.caddyfile);
if (!validation.ok) {
return errorResponse(res, 400, `Invalid Caddyfile in snapshot: ${validation.error}`, {
code: ErrorCodes.BACKUP.INVALID_CONFIG,
});
}
const stagedDir = getStagedCaddyfileDir(dataDir);
try {
await fsp.writeFile(caddyfilePath, snapshot.caddyfile);
restored.push('Caddyfile');
await fsp.mkdir(stagedDir, { recursive: true });
const stagedPath = path.join(stagedDir, 'Caddyfile.candidate');
// Atomic write: write to .candidate.tmp then rename. The live
// Caddyfile is NEVER touched from this endpoint.
const tmpPath = stagedPath + '.tmp';
await fsp.writeFile(tmpPath, snapshot.caddyfile, { mode: 0o644 });
await fsp.rename(tmpPath, stagedPath);
caddyfileStaged.push({
file: 'Caddyfile',
stagedPath,
action: 'awaiting caddy-apply',
livePath: caddyfilePath,
});
if (log) log.info('disaster-recovery', 'Caddyfile staged (not applied)', {
stagedPath,
size: Buffer.byteLength(snapshot.caddyfile, 'utf8'),
});
} catch (err) {
errors.push({ file: 'Caddyfile', error: err.message });
errors.push({ file: 'Caddyfile (staging)', error: err.message });
}
}
@@ -189,8 +324,20 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
const assetsDir = platformPaths?.resolveAssetsPath?.() || path.join(dataDir, 'assets');
for (const [name, base64] of Object.entries(snapshot.assets || {})) {
try {
// DC-079: assets directory is the first attack surface that
// bypasses the Caddyfile-staging gate. `name` is a user-supplied
// JSON key; without validation, `path.join(assetsDir, name)` lets
// an attacker escape to /etc/caddy via path traversal.
assertSafeAssetKey(name);
const resolved = path.resolve(assetsDir, name);
// Defense-in-depth: even after charset checks, the resolved path
// MUST stay inside assetsDir. If it doesn't, refuse the write.
if (!resolved.startsWith(path.resolve(assetsDir) + path.sep) &&
resolved !== path.resolve(assetsDir)) {
throw new Error(`asset path resolves outside assets directory`);
}
await fsp.mkdir(assetsDir, { recursive: true });
await fsp.writeFile(path.join(assetsDir, name), Buffer.from(base64, 'base64'));
await fsp.writeFile(resolved, Buffer.from(base64, 'base64'));
restored.push(`assets/${name}`);
} catch (err) {
errors.push({ file: `assets/${name}`, error: err.message });
@@ -203,8 +350,21 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
try {
await fsp.mkdir(themesDir, { recursive: true });
for (const [name, content] of Object.entries(snapshot.themes)) {
await fsp.writeFile(path.join(themesDir, name), JSON.stringify(content, null, 2));
restored.push(`themes/${name}`);
// DC-079: same path-traversal vector as assets — keys are
// user-controlled JSON. Validate the name AND confirm the
// resolved path stays inside themesDir.
try {
assertSafeThemeName(name);
const resolved = path.resolve(themesDir, name);
if (!resolved.startsWith(path.resolve(themesDir) + path.sep) &&
resolved !== path.resolve(themesDir)) {
throw new Error(`theme path resolves outside themes directory`);
}
await fsp.writeFile(resolved, JSON.stringify(content, null, 2));
restored.push(`themes/${name}`);
} catch (err) {
errors.push({ file: `themes/${name}`, error: err.message });
}
}
} catch (err) {
errors.push({ file: 'themes', error: err.message });
@@ -215,19 +375,33 @@ module.exports = function({ servicesStateManager, platformPaths, log, asyncHandl
timestamp: new Date().toISOString(),
status: errors.length === 0 ? 'success' : 'partial',
restored: restored.length,
staged: caddyfileStaged.length,
errors: errors.length,
};
if (log) log.info('disaster-recovery', 'Restore completed', lastRestoreStatus);
ok(res, {
// DC-079: Surface the staged-Caddyfile warning in the response body so
// the UI / operator can see that the Caddyfile is NOT yet live. The
// restore endpoint stages under dataDir/disaster-staged/Caddyfile.candidate
// and the operator must run `caddy-apply` (or its equivalent) to
// validate + reload + git-commit the staged file. The live Caddyfile
// is owned by the caddy-apply wrapper per CLAUDE.md hard rule.
const responseBody = {
status: errors.length === 0 ? 'success' : 'partial',
restored,
errors,
message: errors.length === 0
? `Successfully restored ${restored.length} files. Restart DashCaddy to apply.`
? `Successfully restored ${restored.length} files${caddyfileStaged.length > 0 ? ` (Caddyfile staged — ${caddyfileStaged[0].stagedPath}; run caddy-apply to apply)` : ''}. Restart DashCaddy to apply.`
: `Restored ${restored.length} files with ${errors.length} errors. Check error details.`,
});
};
if (caddyfileStaged.length > 0) {
responseBody.caddyfileStaged = caddyfileStaged;
responseBody.warning = '[DC-079] Caddyfile is STAGED, not applied. Live /etc/caddy/Caddyfile was NOT modified by this restore. Run `caddy-apply <reason>` (or equivalent) to validate + reload + git-commit the staged candidate.';
}
ok(res, responseBody);
}));
/**