diff --git a/dashcaddy-api/__tests__/credential-manager.test.js b/dashcaddy-api/__tests__/credential-manager.test.js index de42924..b3faf8b 100644 --- a/dashcaddy-api/__tests__/credential-manager.test.js +++ b/dashcaddy-api/__tests__/credential-manager.test.js @@ -15,6 +15,8 @@ jest.mock('../src/security/crypto-utils', () => ({ isEncrypted: jest.fn(data => typeof data === 'string' && data.startsWith('enc:')), loadOrCreateKey: jest.fn(() => Buffer.alloc(32, 'k')), rotateKey: jest.fn(() => ({ oldKey: Buffer.alloc(32, 'k'), newKey: Buffer.alloc(32, 'n') })), + // DC-107 rollback support – restore old key in-process after a failed write + restoreKey: jest.fn(() => true), })); jest.mock('proper-lockfile', () => ({ @@ -357,6 +359,44 @@ describe('CredentialManager', () => { lockfile.lock.mockRejectedValue(new Error('nope')); const result = await credentialManager.rotateEncryptionKey(); expect(result).toBe(false); + // DC-107: failure before rotateKey() must NOT trigger a rollback + expect(cryptoUtils.restoreKey).not.toHaveBeenCalled(); + }); + + it('rolls back the encryption key when the rotated write fails (DC-107)', async () => { + const releaseFn = jest.fn().mockResolvedValue(); + lockfile.lock.mockResolvedValue(releaseFn); + fs.readFileSync.mockReturnValue(JSON.stringify({ + 'key1': { value: 'enc:tag:' + Buffer.from('secret1').toString('base64'), metadata: {} } + })); + // atomicWriteJSON fails at the rename step, AFTER rotateKey() already + // swapped the on-disk key and in-memory cache + fs.renameSync.mockImplementationOnce(() => { throw new Error('EIO: rename'); }); + + const result = await credentialManager.rotateEncryptionKey(); + + expect(result).toBe(false); + expect(cryptoUtils.rotateKey).toHaveBeenCalled(); + const expectedOldHex = Buffer.alloc(32, 'k').toString('hex'); + expect(cryptoUtils.restoreKey).toHaveBeenCalledTimes(1); + expect(cryptoUtils.restoreKey).toHaveBeenCalledWith(expectedOldHex); + expect(releaseFn).toHaveBeenCalled(); // lock still released + }); + + it('returns false without crashing when the rollback itself fails (DC-107)', async () => { + const releaseFn = jest.fn().mockResolvedValue(); + lockfile.lock.mockResolvedValue(releaseFn); + fs.readFileSync.mockReturnValue(JSON.stringify({ + 'key1': { value: 'enc:tag:' + Buffer.from('secret1').toString('base64'), metadata: {} } + })); + fs.renameSync.mockImplementationOnce(() => { throw new Error('EIO: rename'); }); + cryptoUtils.restoreKey.mockImplementationOnce(() => { throw new Error('rollback ENOSPC'); }); + + const result = await credentialManager.rotateEncryptionKey(); + + expect(result).toBe(false); + expect(cryptoUtils.restoreKey).toHaveBeenCalledTimes(1); + expect(releaseFn).toHaveBeenCalled(); // lock released even on double failure }); }); diff --git a/dashcaddy-api/src/managers/credential-manager.js b/dashcaddy-api/src/managers/credential-manager.js index d4f9e45..0a5719f 100644 --- a/dashcaddy-api/src/managers/credential-manager.js +++ b/dashcaddy-api/src/managers/credential-manager.js @@ -176,6 +176,7 @@ class CredentialManager { */ async rotateEncryptionKey() { let release; + let oldKey = null; // DC-107: hoisted so the catch can roll back try { log.info('cred', 'Starting encryption key rotation'); @@ -203,7 +204,7 @@ class CredentialManager { } // Generate new key (this replaces the cached key and saves to disk) - const { oldKey } = cryptoUtils.rotateKey(); + ({ oldKey } = cryptoUtils.rotateKey()); // Re-encrypt all credentials with the new key const rotated = {}; @@ -225,6 +226,24 @@ class CredentialManager { log.info('cred', 'Rotated credentials', { count: keys.length }); return true; } catch (error) { + // DC-107: in-process rollback — the write above failed after rotateKey() + // already swapped the on-disk key and in-memory cache. Restore the old + // key so this process keeps running with a key that can still read the + // on-disk credentials.json. (Hard-crash window between rotateKey() and + // the atomic write is covered separately by the startup .bak fallback + // in crypto-utils.loadOrCreateKey.) + if (oldKey) { + try { + cryptoUtils.restoreKey(oldKey.toString('hex')); + log.warn('cred', 'Rolled back encryption key after write failure'); + } catch (rollbackError) { + // If THIS write fails too (or we hard-crash mid-rollback), restart + // recovery is covered by the startup .bak fallback in + // crypto-utils.loadOrCreateKey (KEY_FILE+.bak still holds the old + // key from rotateKey's pre-swap save). + log.error('cred', rollbackError, { operation: 'rotate-rollback' }); + } + } log.error('cred', error, { operation: 'rotate' }); return false; } finally { diff --git a/dashcaddy-api/src/security/crypto-utils.js b/dashcaddy-api/src/security/crypto-utils.js index e27df30..a370a68 100644 --- a/dashcaddy-api/src/security/crypto-utils.js +++ b/dashcaddy-api/src/security/crypto-utils.js @@ -424,6 +424,27 @@ function clearCachedKey() { encryptionKey = null; } +/** + * Restore the encryption key to a previous value (in-process rollback). + * Writes `oldKeyHex` back to KEY_FILE atomically (DC-107: same canonical + * atomic-write path as every other state file — tmp + fsync + rename, mode + * 0600) and clears the cached key so the next operation reloads from disk. + * Used when a write (e.g. atomicWriteJSON of rotated credentials) fails + * after rotateKey() has already swapped the on-disk key and in-memory cache. + * @param {string} oldKeyHex - Previous key as hex string (32 bytes = 64 hex chars) + * @returns {string} the final path (KEY_FILE) + * @throws {Error} If oldKeyHex is not a 64-char hex string or the write fails + */ +function restoreKey(oldKeyHex) { + if (typeof oldKeyHex !== 'string' || !/^[0-9a-fA-F]{64}$/.test(oldKeyHex)) { + throw new Error('restoreKey: expected 64-char hex string (32-byte key)'); + } + const { atomicWriteFile } = require('../utils/atomic-write'); + atomicWriteFile(KEY_FILE, oldKeyHex, { mode: 0o600 }); + clearCachedKey(); + return KEY_FILE; +} + module.exports = { encrypt, decrypt, @@ -437,5 +458,6 @@ module.exports = { deriveKey, rotateKey, decryptWithKey, - clearCachedKey + clearCachedKey, + restoreKey };