From 6979302fb7ff88a9092bffcdace20c7cd90cc11d Mon Sep 17 00:00:00 2001 From: Sami Date: Fri, 6 Mar 2026 23:08:30 -0800 Subject: [PATCH] Fix 7 critical security bugs and 1 high-severity data loss bug - CSRF: HMAC-signed double-submit cookie (server-bound, not raw compare) - Keychain: execFileSync with arg arrays to prevent command injection - Caddy config: always use structured generation, never accept raw config - Templates: replace {{GENERATED_SECRET}} with crypto.randomBytes - Caddyfile removal: move regex inside ctx.caddy.modify() to fix TOCTOU race - Credentials: proper-lockfile for all file operations, fix key rotation to decrypt with old key before generating new key - Service removal: filter by ID only, not AND with appTemplate Co-Authored-By: Claude Opus 4.6 --- dashcaddy-api/app-templates.js | 2 +- dashcaddy-api/credential-manager.js | 185 +++++++++++++++++---------- dashcaddy-api/crypto-utils.js | 49 ++++++- dashcaddy-api/csrf-protection.js | 77 ++++++----- dashcaddy-api/keychain-manager.js | 58 +++------ dashcaddy-api/routes/apps/helpers.js | 4 +- dashcaddy-api/routes/apps/removal.js | 18 +-- dashcaddy-api/routes/sites.js | 8 +- 8 files changed, 242 insertions(+), 159 deletions(-) diff --git a/dashcaddy-api/app-templates.js b/dashcaddy-api/app-templates.js index 54f0561..610ca29 100644 --- a/dashcaddy-api/app-templates.js +++ b/dashcaddy-api/app-templates.js @@ -1323,7 +1323,7 @@ const APP_TEMPLATES = { "USER_GID": "1000" } }, - subdomain: "git", + subdomain: "gitea", defaultPort: 3005, healthCheck: "/", subpathSupport: 'native', diff --git a/dashcaddy-api/credential-manager.js b/dashcaddy-api/credential-manager.js index d3a9e43..d89f685 100644 --- a/dashcaddy-api/credential-manager.js +++ b/dashcaddy-api/credential-manager.js @@ -6,6 +6,7 @@ const keychainManager = require('./keychain-manager'); const cryptoUtils = require('./crypto-utils'); +const lockfile = require('proper-lockfile'); const fs = require('fs'); const path = require('path'); @@ -15,7 +16,11 @@ class CredentialManager { constructor() { this.useKeychain = keychainManager.available; this.cache = new Map(); // In-memory cache for performance - + this.lockOptions = { + retries: { retries: 10, minTimeout: 100, maxTimeout: 300 }, + stale: 30000 + }; + console.log(`[CredentialManager] Initialized with ${this.useKeychain ? 'OS keychain' : 'encrypted file'} storage`); } @@ -152,47 +157,61 @@ class CredentialManager { * @returns {Promise} Success status */ async rotateEncryptionKey() { + let release; try { console.log('[CredentialManager] Starting encryption key rotation...'); - - // Load all credentials with old key - const credentials = await this.loadCredentialsFile(); + + // Ensure file exists before locking + this._ensureFileExists(); + release = await lockfile.lock(CREDENTIALS_FILE, this.lockOptions); + + const data = fs.readFileSync(CREDENTIALS_FILE, 'utf8'); + const credentials = JSON.parse(data); const keys = Object.keys(credentials); - + if (keys.length === 0) { console.log('[CredentialManager] No credentials to rotate'); return true; } - // Generate new encryption key - cryptoUtils.loadOrCreateKey(); // This will generate a new key - - // Re-encrypt all credentials - const rotated = {}; + // Decrypt all values with the CURRENT key first + const decryptedEntries = {}; for (const key of keys) { const value = credentials[key].value; - const metadata = credentials[key].metadata; - - // Decrypt with old key, encrypt with new key - const decrypted = cryptoUtils.isEncrypted(value) ? cryptoUtils.decrypt(value) : value; + decryptedEntries[key] = { + plaintext: cryptoUtils.isEncrypted(value) ? cryptoUtils.decrypt(value) : value, + metadata: credentials[key].metadata + }; + } + + // Generate new key (this replaces the cached key and saves to disk) + const { oldKey } = cryptoUtils.rotateKey(); + + // Re-encrypt all credentials with the new key + const rotated = {}; + for (const key of keys) { rotated[key] = { - value: cryptoUtils.encrypt(decrypted), - metadata, + value: cryptoUtils.encrypt(decryptedEntries[key].plaintext), + metadata: decryptedEntries[key].metadata, rotatedAt: new Date().toISOString() }; } // Save with new encryption - await this.saveCredentialsFile(rotated); - + fs.writeFileSync(CREDENTIALS_FILE, JSON.stringify(rotated, null, 2), { mode: 0o600 }); + // Clear cache to force reload this.cache.clear(); - + console.log(`[CredentialManager] Successfully rotated ${keys.length} credentials`); return true; } catch (error) { console.error('[CredentialManager] Key rotation failed:', error.message); return false; + } finally { + if (release) { + try { await release(); } catch (e) { /* lock will expire via stale timeout */ } + } } } @@ -202,22 +221,23 @@ class CredentialManager { */ async migrateToEncrypted() { try { - const credentials = await this.loadCredentialsFile(); let migrated = 0; let skipped = 0; - for (const [key, data] of Object.entries(credentials)) { - if (!cryptoUtils.isEncrypted(data.value)) { - credentials[key].value = cryptoUtils.encrypt(data.value); - credentials[key].migratedAt = new Date().toISOString(); - migrated++; - } else { - skipped++; + await this._lockedUpdate(credentials => { + for (const [key, data] of Object.entries(credentials)) { + if (!cryptoUtils.isEncrypted(data.value)) { + credentials[key].value = cryptoUtils.encrypt(data.value); + credentials[key].migratedAt = new Date().toISOString(); + migrated++; + } else { + skipped++; + } } - } + return credentials; + }); if (migrated > 0) { - await this.saveCredentialsFile(credentials); this.cache.clear(); console.log(`[CredentialManager] Migrated ${migrated} plaintext credentials to encrypted format`); } @@ -231,14 +251,57 @@ class CredentialManager { // Private methods + /** + * Ensure credentials file exists (needed before locking) + * @private + */ + _ensureFileExists() { + if (!fs.existsSync(CREDENTIALS_FILE)) { + const dir = path.dirname(CREDENTIALS_FILE); + if (!fs.existsSync(dir)) { + fs.mkdirSync(dir, { recursive: true }); + } + fs.writeFileSync(CREDENTIALS_FILE, '{}', { mode: 0o600 }); + } + } + + /** + * Atomic read-modify-write with file locking + * @param {Function} updateFn - Receives current credentials object, returns updated object + * @returns {Promise} Updated credentials + * @private + */ + async _lockedUpdate(updateFn) { + this._ensureFileExists(); + let release; + try { + release = await lockfile.lock(CREDENTIALS_FILE, this.lockOptions); + const data = fs.readFileSync(CREDENTIALS_FILE, 'utf8'); + const credentials = JSON.parse(data); + const updated = await updateFn(credentials); + fs.writeFileSync(CREDENTIALS_FILE, JSON.stringify(updated, null, 2), { mode: 0o600 }); + return updated; + } catch (error) { + if (error.code === 'ELOCKED') { + throw new Error('Credentials file is locked by another process. Try again.'); + } + throw error; + } finally { + if (release) { + try { await release(); } catch (e) { /* lock will expire via stale timeout */ } + } + } + } + async storeInFile(key, value, metadata) { - const credentials = await this.loadCredentialsFile(); - credentials[key] = { - value: cryptoUtils.encrypt(value), - metadata, - updatedAt: new Date().toISOString() - }; - await this.saveCredentialsFile(credentials); + await this._lockedUpdate(credentials => { + credentials[key] = { + value: cryptoUtils.encrypt(value), + metadata, + updatedAt: new Date().toISOString() + }; + return credentials; + }); } async retrieveFromFile(key) { @@ -246,26 +309,28 @@ class CredentialManager { const data = credentials[key]; if (!data) return null; - return cryptoUtils.isEncrypted(data.value) - ? cryptoUtils.decrypt(data.value) + return cryptoUtils.isEncrypted(data.value) + ? cryptoUtils.decrypt(data.value) : data.value; } async deleteFromFile(key) { - const credentials = await this.loadCredentialsFile(); - delete credentials[key]; - await this.saveCredentialsFile(credentials); + await this._lockedUpdate(credentials => { + delete credentials[key]; + return credentials; + }); } async storeMetadata(key, metadata) { - const credentials = await this.loadCredentialsFile(); - if (!credentials[key]) { - credentials[key] = { metadata }; - } else { - credentials[key].metadata = metadata; - } - credentials[key].updatedAt = new Date().toISOString(); - await this.saveCredentialsFile(credentials); + await this._lockedUpdate(credentials => { + if (!credentials[key]) { + credentials[key] = { metadata }; + } else { + credentials[key].metadata = metadata; + } + credentials[key].updatedAt = new Date().toISOString(); + return credentials; + }); } async loadCredentialsFile() { @@ -281,22 +346,6 @@ class CredentialManager { } } - async saveCredentialsFile(credentials) { - try { - // Ensure directory exists - const dir = path.dirname(CREDENTIALS_FILE); - if (!fs.existsSync(dir)) { - fs.mkdirSync(dir, { recursive: true }); - } - - // Write with restrictive permissions - fs.writeFileSync(CREDENTIALS_FILE, JSON.stringify(credentials, null, 2), { mode: 0o600 }); - } catch (error) { - console.error('[CredentialManager] Failed to save credentials file:', error.message); - throw error; - } - } - /** * Export credentials for backup (encrypted) * @returns {Promise} Encrypted backup data @@ -320,14 +369,14 @@ class CredentialManager { try { const decrypted = cryptoUtils.decrypt(encryptedBackup); const backup = JSON.parse(decrypted); - + if (backup.version !== '1.0') { throw new Error('Unsupported backup version'); } - await this.saveCredentialsFile(backup.credentials); + await this._lockedUpdate(() => backup.credentials); this.cache.clear(); - + console.log('[CredentialManager] Successfully imported backup'); return true; } catch (error) { diff --git a/dashcaddy-api/crypto-utils.js b/dashcaddy-api/crypto-utils.js index fbc7e9a..3ff6030 100644 --- a/dashcaddy-api/crypto-utils.js +++ b/dashcaddy-api/crypto-utils.js @@ -267,6 +267,51 @@ function writeEncryptedFile(filePath, credentials, sensitiveFields = ['password' console.log(`[Crypto] Saved encrypted credentials to ${filePath}`); } +/** + * Rotate the encryption key — generates a new key and returns both old and new + * @returns {{ oldKey: Buffer, newKey: Buffer }} Old and new key pair + * @throws {Error} If new key cannot be saved to disk + */ +function rotateKey() { + const oldKey = loadOrCreateKey(); // Ensure we have the current key loaded + const newKey = generateKey(); + + try { + fs.writeFileSync(KEY_FILE, newKey.toString('hex'), { mode: 0o600 }); + } catch (error) { + throw new Error(`Failed to save new encryption key: ${error.message}`); + } + + // Only update the cached key after file write succeeds + encryptionKey = newKey; + return { oldKey, newKey }; +} + +/** + * Decrypt data using a specific key (for key rotation) + * @param {string} encryptedData - Encrypted string in format iv:authTag:ciphertext + * @param {Buffer} key - The key to decrypt with + * @returns {string} Decrypted plaintext + */ +function decryptWithKey(encryptedData, key) { + const parts = encryptedData.split(':'); + if (parts.length !== 3) { + throw new Error('Invalid encrypted data format'); + } + + const iv = Buffer.from(parts[0], 'base64'); + const authTag = Buffer.from(parts[1], 'base64'); + const ciphertext = parts[2]; + + const decipher = crypto.createDecipheriv(ALGORITHM, key, iv); + decipher.setAuthTag(authTag); + + let decrypted = decipher.update(ciphertext, 'base64', 'utf8'); + decrypted += decipher.final('utf8'); + + return decrypted; +} + // Initialize key on module load loadOrCreateKey(); @@ -280,5 +325,7 @@ module.exports = { readEncryptedFile, writeEncryptedFile, loadOrCreateKey, - deriveKey + deriveKey, + rotateKey, + decryptWithKey }; diff --git a/dashcaddy-api/csrf-protection.js b/dashcaddy-api/csrf-protection.js index ac95749..9462467 100644 --- a/dashcaddy-api/csrf-protection.js +++ b/dashcaddy-api/csrf-protection.js @@ -1,22 +1,36 @@ /** * CSRF Protection Module - * Implements double-submit cookie pattern for stateless CSRF protection + * Implements HMAC-signed double-submit cookie pattern for stateless CSRF protection. + * The cookie contains a random nonce; the header must carry the HMAC signature + * of that nonce computed with a server-side secret. An attacker who can inject + * a cookie still cannot forge the matching header without the secret. */ const crypto = require('crypto'); +const cryptoUtils = require('./crypto-utils'); const CSRF_TOKEN_LENGTH = 32; const CSRF_COOKIE_NAME = 'dashcaddy_csrf'; const CSRF_HEADER_NAME = 'x-csrf-token'; /** - * Generate a cryptographically secure CSRF token - * @returns {string} Base64URL-encoded random token + * Generate a cryptographically secure CSRF nonce + * @returns {string} Base64URL-encoded random nonce */ function generateToken() { return crypto.randomBytes(CSRF_TOKEN_LENGTH).toString('base64url'); } +/** + * Compute HMAC signature for a CSRF nonce using the server-side encryption key + * @param {string} nonce - The random nonce to sign + * @returns {string} Base64URL-encoded HMAC signature + */ +function signToken(nonce) { + const key = cryptoUtils.loadOrCreateKey(); + return crypto.createHmac('sha256', key).update(nonce).digest('base64url'); +} + /** * Parse cookie header string into object * @param {string} cookieHeader - Cookie header value @@ -35,25 +49,23 @@ function parseCookie(cookieHeader) { } /** - * Middleware to set CSRF cookie on all requests - * Generates and sets a new token if none exists + * Middleware to set CSRF cookie on all requests. + * Always generates a fresh nonce server-side (never trusts client-supplied values). + * The cookie holds the nonce; JavaScript must read it and send the HMAC signature + * in the x-csrf-token header. The /api/csrf-token endpoint provides the signature. */ function csrfCookieMiddleware(req, res, next) { - const cookies = parseCookie(req.headers.cookie); - let csrfToken = cookies[CSRF_COOKIE_NAME]; + // Always generate a fresh server-side nonce + const csrfNonce = generateToken(); - // Generate new token if none exists - if (!csrfToken) { - csrfToken = generateToken(); - } + // Store nonce + signature on request so endpoints can access them + req.csrfToken = signToken(csrfNonce); + req.csrfNonce = csrfNonce; - // Store token on request so endpoints can access it - req.csrfToken = csrfToken; - - // Set cookie (SameSite=Strict for additional protection) - res.cookie(CSRF_COOKIE_NAME, csrfToken, { - httpOnly: false, // Must be readable by JavaScript for sending in headers - secure: false, // Set to true in production with HTTPS + // Set cookie with the nonce (SameSite=Strict for additional protection) + res.cookie(CSRF_COOKIE_NAME, csrfNonce, { + httpOnly: false, // Must be readable by JavaScript for signing + secure: true, sameSite: 'strict', path: '/', maxAge: 24 * 60 * 60 * 1000 // 24 hours @@ -95,16 +107,21 @@ function csrfValidationMiddleware(req, res, next) { return next(); } - // Get token from cookie + // Get nonce from cookie const cookies = parseCookie(req.headers.cookie); - const cookieToken = cookies[CSRF_COOKIE_NAME]; + const cookieNonce = cookies[CSRF_COOKIE_NAME]; - // Get token from header (case-insensitive) + // Get signed token from header (case-insensitive) const headerToken = req.headers[CSRF_HEADER_NAME] || req.headers[CSRF_HEADER_NAME.toLowerCase()]; - // Validate both tokens exist - if (!cookieToken) { + // Skip CSRF for API key-authenticated requests (API keys are not sent automatically by browsers) + if (req.headers['x-api-key'] || (req.headers.authorization && req.headers.authorization.startsWith('Bearer '))) { + return next(); + } + + // Validate both values exist + if (!cookieNonce) { console.warn(`[CSRF] Missing CSRF cookie: ${method} ${req.path} from ${req.ip}`); return res.status(403).json({ success: false, @@ -122,22 +139,21 @@ function csrfValidationMiddleware(req, res, next) { }); } - // Validate tokens match using constant-time comparison + // Validate that the header token is the correct HMAC signature of the cookie nonce try { - const cookieBuffer = Buffer.from(cookieToken, 'base64url'); + const expectedSig = signToken(cookieNonce); + const expectedBuffer = Buffer.from(expectedSig, 'base64url'); const headerBuffer = Buffer.from(headerToken, 'base64url'); - // Ensure buffers are same length - if (cookieBuffer.length !== headerBuffer.length) { + if (expectedBuffer.length !== headerBuffer.length) { throw new Error('Token length mismatch'); } - // Constant-time comparison - if (!crypto.timingSafeEqual(cookieBuffer, headerBuffer)) { + if (!crypto.timingSafeEqual(expectedBuffer, headerBuffer)) { throw new Error('Token mismatch'); } - // Tokens match - request is valid + // Signature valid — request is authentic next(); } catch (err) { @@ -155,6 +171,7 @@ module.exports = { CSRF_COOKIE_NAME, CSRF_HEADER_NAME, generateToken, + signToken, parseCookie, csrfCookieMiddleware, csrfValidationMiddleware diff --git a/dashcaddy-api/keychain-manager.js b/dashcaddy-api/keychain-manager.js index 2ba5962..66f5908 100644 --- a/dashcaddy-api/keychain-manager.js +++ b/dashcaddy-api/keychain-manager.js @@ -4,7 +4,7 @@ * Falls back to encrypted file storage if keychain is unavailable */ -const { execSync } = require('child_process'); +const { execSync, execFileSync } = require('child_process'); const os = require('os'); const crypto = require('crypto'); @@ -131,53 +131,41 @@ class KeychainManager { } } - // Windows Credential Manager implementation + // Windows Credential Manager implementation (uses execFileSync to prevent injection) async storeWindows(account, value) { - const escapedValue = value.replace(/"/g, '""'); - const script = ` - $password = ConvertTo-SecureString -String "${escapedValue}" -AsPlainText -Force - $credential = New-Object System.Management.Automation.PSCredential("${account}", $password) - cmdkey /generic:"${SERVICE_NAME}:${account}" /user:"${account}" /pass:"${escapedValue}" - `; - execSync(`powershell -Command "${script.replace(/\n/g, ' ')}"`, { stdio: 'ignore' }); + execFileSync('cmdkey', [`/generic:${SERVICE_NAME}:${account}`, `/user:${account}`, `/pass:${value}`], { stdio: 'ignore' }); return true; } async retrieveWindows(account) { try { - const script = ` - $cred = cmdkey /list:"${SERVICE_NAME}:${account}" - if ($cred -match "Password: (.+)") { $matches[1] } - `; - const result = execSync(`powershell -Command "${script.replace(/\n/g, ' ')}"`, { encoding: 'utf8' }); - return result.trim() || null; + const result = execFileSync('cmdkey', [`/list:${SERVICE_NAME}:${account}`], { encoding: 'utf8' }); + const match = result.match(/Password:\s*(.+)/); + return match ? match[1].trim() : null; } catch { return null; } } async deleteWindows(account) { - execSync(`cmdkey /delete:"${SERVICE_NAME}:${account}"`, { stdio: 'ignore' }); + execFileSync('cmdkey', [`/delete:${SERVICE_NAME}:${account}`], { stdio: 'ignore' }); return true; } - // macOS Keychain implementation + // macOS Keychain implementation (uses execFileSync to prevent injection) async storeMacOS(account, value) { - // Delete existing entry first try { - execSync(`security delete-generic-password -s "${SERVICE_NAME}" -a "${account}"`, { stdio: 'ignore' }); + execFileSync('security', ['delete-generic-password', '-s', SERVICE_NAME, '-a', account], { stdio: 'ignore' }); } catch { // Ignore if doesn't exist } - - // Add new entry - execSync(`security add-generic-password -s "${SERVICE_NAME}" -a "${account}" -w "${value}"`, { stdio: 'ignore' }); + execFileSync('security', ['add-generic-password', '-s', SERVICE_NAME, '-a', account, '-w', value], { stdio: 'ignore' }); return true; } async retrieveMacOS(account) { try { - const result = execSync(`security find-generic-password -s "${SERVICE_NAME}" -a "${account}" -w`, { encoding: 'utf8' }); + const result = execFileSync('security', ['find-generic-password', '-s', SERVICE_NAME, '-a', account, '-w'], { encoding: 'utf8' }); return result.trim() || null; } catch { return null; @@ -185,38 +173,26 @@ class KeychainManager { } async deleteMacOS(account) { - execSync(`security delete-generic-password -s "${SERVICE_NAME}" -a "${account}"`, { stdio: 'ignore' }); + execFileSync('security', ['delete-generic-password', '-s', SERVICE_NAME, '-a', account], { stdio: 'ignore' }); return true; } - // Linux Secret Service implementation + // Linux Secret Service implementation (uses execFileSync + stdin to prevent injection) async storeLinux(account, value) { try { - // Try secret-tool first (libsecret) - execSync(`secret-tool store --label="${SERVICE_NAME}:${account}" service "${SERVICE_NAME}" account "${account}"`, { + execFileSync('secret-tool', ['store', `--label=${SERVICE_NAME}:${account}`, 'service', SERVICE_NAME, 'account', account], { input: value, stdio: ['pipe', 'ignore', 'ignore'] }); return true; } catch { - // Fallback to gnome-keyring if available - try { - const script = ` - echo "${value}" | gnome-keyring-daemon --unlock - echo "${value}" | gnome-keyring --set-password "${SERVICE_NAME}:${account}" - `; - execSync(script, { stdio: 'ignore' }); - return true; - } catch { - return false; - } + return false; } } async retrieveLinux(account) { try { - // Try secret-tool first - const result = execSync(`secret-tool lookup service "${SERVICE_NAME}" account "${account}"`, { encoding: 'utf8' }); + const result = execFileSync('secret-tool', ['lookup', 'service', SERVICE_NAME, 'account', account], { encoding: 'utf8' }); return result.trim() || null; } catch { return null; @@ -225,7 +201,7 @@ class KeychainManager { async deleteLinux(account) { try { - execSync(`secret-tool clear service "${SERVICE_NAME}" account "${account}"`, { stdio: 'ignore' }); + execFileSync('secret-tool', ['clear', 'service', SERVICE_NAME, 'account', account], { stdio: 'ignore' }); return true; } catch { return false; diff --git a/dashcaddy-api/routes/apps/helpers.js b/dashcaddy-api/routes/apps/helpers.js index b136f24..17097a7 100644 --- a/dashcaddy-api/routes/apps/helpers.js +++ b/dashcaddy-api/routes/apps/helpers.js @@ -1,6 +1,7 @@ const fs = require('fs'); const fsp = require('fs').promises; const path = require('path'); +const crypto = require('crypto'); const { REGEX, DOCKER } = require('../../constants'); const { exists } = require('../../fs-helpers'); const platformPaths = require('../../platform-paths'); @@ -70,7 +71,8 @@ module.exports = function(ctx) { '{{SUBDOMAIN}}': config.subdomain, '{{PORT}}': config.port || template.defaultPort, '{{MEDIA_PATH}}': mediaPaths[0] || '/media', - '{{TIMEZONE}}': ctx.siteConfig.timezone || 'UTC' + '{{TIMEZONE}}': ctx.siteConfig.timezone || 'UTC', + '{{GENERATED_SECRET}}': crypto.randomBytes(32).toString('hex') }; function replaceInObject(obj) { diff --git a/dashcaddy-api/routes/apps/removal.js b/dashcaddy-api/routes/apps/removal.js index 54ee2ee..92e65e2 100644 --- a/dashcaddy-api/routes/apps/removal.js +++ b/dashcaddy-api/routes/apps/removal.js @@ -68,18 +68,14 @@ module.exports = function(ctx, helpers) { } else { // Subdomain mode: remove standalone domain block const domain = ctx.buildDomain(subdomain); - let content = await ctx.caddy.read(); const escapedDomain = domain.replace(/[.*+?^${}()|[\]\\]/g, '\\$&'); const siteBlockRegex = new RegExp(`\\n?${escapedDomain}\\s*\\{[^{}]*(?:\\{[^{}]*(?:\\{[^{}]*\\}[^{}]*)*\\}[^{}]*)*\\}\\s*`, 'g'); - const originalLength = content.length; - content = content.replace(siteBlockRegex, '\n'); - if (content.length !== originalLength) { - content = content.replace(/\n{3,}/g, '\n\n'); - const caddyResult = await ctx.caddy.modify(() => content); - results.caddy = caddyResult.success ? 'removed' : 'removed (reload failed)'; - } else { - results.caddy = 'not found'; - } + const caddyResult = await ctx.caddy.modify(currentContent => { + const replaced = currentContent.replace(siteBlockRegex, '\n'); + if (replaced.length === currentContent.length) return null; + return replaced.replace(/\n{3,}/g, '\n\n'); + }); + results.caddy = caddyResult.success ? 'removed' : (caddyResult.rolledBack ? 'removed (reload failed)' : 'not found'); } ctx.log.info('caddy', 'Caddy config removal', { result: results.caddy }); } catch (error) { @@ -94,7 +90,7 @@ module.exports = function(ctx, helpers) { let removed = false; await ctx.servicesStateManager.update(services => { const initialLength = services.length; - const filtered = services.filter(s => s.id !== subdomain && s.appTemplate !== appId); + const filtered = services.filter(s => s.id !== subdomain); removed = filtered.length !== initialLength; return filtered; }); diff --git a/dashcaddy-api/routes/sites.js b/dashcaddy-api/routes/sites.js index f8a4fb1..54dbc87 100644 --- a/dashcaddy-api/routes/sites.js +++ b/dashcaddy-api/routes/sites.js @@ -155,12 +155,8 @@ module.exports = function(ctx) { return ctx.errorResponse(res, 409, `[DC-302] Site block for "${domain}" already exists in Caddyfile`); } - let newSiteBlock; - if (config) { - newSiteBlock = `\n${config}\n`; - } else { - newSiteBlock = `\n${domain} {\n reverse_proxy ${upstream}\n tls internal\n}\n`; - } + // Always generate structured config — never allow raw Caddy config injection + const newSiteBlock = `\n${domain} {\n reverse_proxy ${upstream}\n tls internal\n}\n`; const result = await ctx.caddy.modify(c => c + newSiteBlock); if (!result.success) {