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 <noreply@anthropic.com>
This commit is contained in:
@@ -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<boolean>} 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<Object>} 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<string>} 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) {
|
||||
|
||||
Reference in New Issue
Block a user