diff --git a/dashcaddy-api/__tests__/routes/discover-disaster.routes.test.js b/dashcaddy-api/__tests__/routes/discover-disaster.routes.test.js index 0d2037c..200f6b2 100644 --- a/dashcaddy-api/__tests__/routes/discover-disaster.routes.test.js +++ b/dashcaddy-api/__tests__/routes/discover-disaster.routes.test.js @@ -18,7 +18,11 @@ function createDiscoverApp(docker, servicesStateManager) { function createDisasterApp(platformPaths, log) { const app = express(); - app.use(express.json()); + // Match the production body-parser limit (1 MiB) so the in-handler + // DC-079 cap (512 KiB) is actually reachable from tests. The default + // express.json() limit is 100 KiB, which would short-circuit the test + // with a 413 before the route's defense-in-depth check runs. + app.use(express.json({ limit: '1mb' })); const routes = require('../../routes/disaster-recovery'); const wrap = (fn) => (req, res, next) => Promise.resolve(fn(req, res, next)).catch(next); app.use('/api/v1', routes({ platformPaths, log: log || { info: jest.fn(), error: jest.fn() }, asyncHandler: wrap })); @@ -135,4 +139,267 @@ describe('DC-107: Disaster Recovery', () => { const svc = JSON.parse(fs.readFileSync(path.join(tmpDir, 'services.json'), 'utf8')); expect(svc[0].id).toBe('restored-svc'); }); + + // DC-079: Caddyfile restore hardening — the live Caddyfile path must + // NEVER be written from the disaster-recovery endpoint. The endpoint + // stages the candidate file under dataDir/disaster-staged/Caddyfile.candidate + // and surfaces a warning that `caddy-apply` is required to apply it. + it('DC-079: POST /disaster/restore with caddyfile STAGES instead of writing the live Caddyfile', async () => { + // The env var CADDYFILE_PATH is read by the route. Use a sentinel + // path that we can prove was NOT written. The route must instead + // create /disaster-staged/Caddyfile.candidate. + const liveSentinel = path.join(tmpDir, 'LIVE_CADDYFILE_SENTINEL.txt'); + fs.writeFileSync(liveSentinel, 'do-not-overwrite'); + + const candidateCaddyfile = + '# staged candidate\n' + + 'example.com {\n' + + ' respond "ok"\n' + + '}\n'; + + const app = createDisasterApp({ + dataDir: tmpDir, + caddyfilePath: liveSentinel, // route reads env or fallback; this is just for the response + }); + // Override process.env.CADDYFILE_PATH so the route picks up our sentinel + const prev = process.env.CADDYFILE_PATH; + process.env.CADDYFILE_PATH = liveSentinel; + try { + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + caddyfile: candidateCaddyfile, + }); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('success'); + expect(res.body.caddyfileStaged).toBeTruthy(); + expect(res.body.caddyfileStaged).toHaveLength(1); + expect(res.body.caddyfileStaged[0].file).toBe('Caddyfile'); + expect(res.body.caddyfileStaged[0].action).toBe('awaiting caddy-apply'); + expect(res.body.caddyfileStaged[0].stagedPath).toBe( + path.join(tmpDir, 'disaster-staged', 'Caddyfile.candidate') + ); + expect(res.body.caddyfileStaged[0].livePath).toBe(liveSentinel); + expect(res.body.warning).toMatch(/DC-079/); + + // The live sentinel file is UNTOUCHED — still has its original content. + const liveContents = fs.readFileSync(liveSentinel, 'utf8'); + expect(liveContents).toBe('do-not-overwrite'); + + // The candidate file IS staged at the staging path. + const stagedContents = fs.readFileSync( + path.join(tmpDir, 'disaster-staged', 'Caddyfile.candidate'), + 'utf8' + ); + expect(stagedContents).toBe(candidateCaddyfile); + } finally { + if (prev === undefined) delete process.env.CADDYFILE_PATH; + else process.env.CADDYFILE_PATH = prev; + } + }); + + it('DC-079: POST /disaster/restore rejects non-string caddyfile content', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + caddyfile: { evil: 'object' }, + }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/Caddyfile content must be a string/); + }); + + it('DC-079: POST /disaster/restore rejects explicit empty caddyfile string', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + caddyfile: '', // explicit empty payload — rejected + }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/Caddyfile content is empty/); + }); + + it('DC-079: POST /disaster/restore rejects oversized caddyfile content', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + // 512 KiB + 1 byte — over the in-handler cap, under the 1 MB body limit + const huge = 'a'.repeat(512 * 1024 + 1); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + caddyfile: huge, + }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/exceeds 524288 bytes/); + }); + + it('DC-079: POST /disaster/restore rejects forbidden `import` directive (absolute path)', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const evil = + '# malicious snapshot\n' + + 'import /etc/caddy/external.caddy\n' + + 'example.com { respond "ok" }\n'; + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + caddyfile: evil, + }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/forbidden `import` directive/); + + // No staging file should have been created — fail closed. + expect(fs.existsSync(path.join(tmpDir, 'disaster-staged', 'Caddyfile.candidate'))).toBe(false); + }); + + it('DC-079: POST /disaster/restore rejects forbidden `import` with relative-path escape', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const evil = + '# malicious snapshot\n' + + 'import ../../../etc/passwd\n' + + 'example.com { respond "ok" }\n'; + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + caddyfile: evil, + }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/forbidden `import` directive/); + }); + + it('DC-079: POST /disaster/restore rejects URL-encoded import payload', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const evil = + 'import %2fetc%2fcaddy%2fevil.caddy\n' + + 'example.com { respond "ok" }\n'; + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + caddyfile: evil, + }); + + expect(res.status).toBe(400); + expect(res.body.error).toMatch(/forbidden `import` directive/); + }); + + it('DC-079: POST /disaster/restore without caddyfile field succeeds and stages nothing', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + files: { + services: [{ id: 'no-caddy' }], + }, + }); + + expect(res.status).toBe(200); + expect(res.body.caddyfileStaged).toBeUndefined(); + expect(res.body.warning).toBeUndefined(); + }); + + // DC-079 follow-up (GLM round-2 BLOCKING): assets/themes path traversal. + // Without the assertSafeAssetKey / assertSafeThemeName + path.resolve + // checks, an attacker can POST `{assets: {"../../etc/caddy/Caddyfile": + // ""}}` and overwrite the live Caddyfile via the dataDir + // bind-mount. These tests prove the fix. + it('DC-079: POST /disaster/restore rejects assets with path-traversal key', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + assets: { + '../../etc/caddy/Caddyfile': Buffer.from('EVIL_BASE64_PAYLOAD').toString('base64'), + 'custom-logo.png': Buffer.from('legit-logo').toString('base64'), + }, + }); + + // The traversal key is rejected (added to errors), the legit key + // still works. Status is success-or-partial, never 500. + expect(res.status).toBe(200); + expect(res.body.status).toBe('partial'); // one error + const erroredFile = res.body.errors.find(e => e.file && e.file.includes('../../etc/caddy/Caddyfile')); + expect(erroredFile).toBeTruthy(); + expect(erroredFile.error).toMatch(/forbidden characters or path segments/); + + // The legit logo DID get written. + const legitPath = path.join(tmpDir, 'assets', 'custom-logo.png'); + expect(fs.existsSync(legitPath)).toBe(true); + + // The traversal target was NEVER written. + const escapePath = path.join(tmpDir, 'assets', '../../etc/caddy/Caddyfile'); + // Resolve to absolute path — should be outside tmpDir/assets. + const resolvedEsc = path.resolve(escapePath); + expect(fs.existsSync(resolvedEsc)).toBe(false); + }); + + it('DC-079: POST /disaster/restore rejects assets with absolute path key', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + assets: { + '/etc/passwd': Buffer.from('evil').toString('base64'), + }, + }); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('partial'); + const erroredFile = res.body.errors.find(e => e.file && e.file.includes('/etc/passwd')); + expect(erroredFile).toBeTruthy(); + }); + + it('DC-079: POST /disaster/restore rejects themes with path-traversal name', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + themes: { + '../../../etc/caddy/evil.json': { evil: true }, + 'legit-theme.json': { ok: true }, + }, + }); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('partial'); + const erroredFile = res.body.errors.find(e => e.file && e.file.includes('../../../etc/caddy/evil.json')); + expect(erroredFile).toBeTruthy(); + expect(erroredFile.error).toMatch(/must match/); + + // The legit theme DID get written. + expect(fs.existsSync(path.join(tmpDir, 'themes', 'legit-theme.json'))).toBe(true); + }); + + it('DC-079: POST /disaster/restore rejects themes without .json extension', async () => { + const app = createDisasterApp({ dataDir: tmpDir }); + const res = await request(app) + .post('/api/v1/disaster/restore') + .send({ + version: '1.0', + themes: { + 'no-extension': { ok: true }, + }, + }); + + expect(res.status).toBe(200); + expect(res.body.status).toBe('partial'); + const erroredFile = res.body.errors.find(e => e.file && e.file.includes('no-extension')); + expect(erroredFile).toBeTruthy(); + expect(erroredFile.error).toMatch(/must match/); + }); }); diff --git a/dashcaddy-api/routes/disaster-recovery.js b/dashcaddy-api/routes/disaster-recovery.js index f6f40c2..3aedd94 100644 --- a/dashcaddy-api/routes/disaster-recovery.js +++ b/dashcaddy-api/routes/disaster-recovery.js @@ -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": +// ""}}` 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 /themes/; 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 ` 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: ` + // 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/` 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 ` (or equivalent) to validate + reload + git-commit the staged candidate.'; + } + + ok(res, responseBody); })); /**