From 5cceceaea9ac479ccd127ace15d4d9a75d828a50 Mon Sep 17 00:00:00 2001 From: natreex Date: Tue, 25 Nov 2025 11:50:11 -0500 Subject: [PATCH] Refactor `SecureStorage` for stricter typings, enhanced error handling, and cleanup - Add explicit type annotations for improved code clarity and type safety. - Refactor encryption/decryption logic with stricter validations and error handling. - Remove redundant `appReady` logic and unused comments. - Convert instance methods to `public` for consistent access modifiers. - Streamline `saveToDisk` to ensure robust encryption and error reporting. --- electron/storage/SecureStorage.ts | 137 +++++++++++++----------------- 1 file changed, 61 insertions(+), 76 deletions(-) diff --git a/electron/storage/SecureStorage.ts b/electron/storage/SecureStorage.ts index a262765..8f8d405 100644 --- a/electron/storage/SecureStorage.ts +++ b/electron/storage/SecureStorage.ts @@ -16,24 +16,18 @@ import * as path from 'path'; * - Does NOT protect against malware running under same user * - On Linux, check getStorageBackend() - if 'basic_text', encryption is weak */ + +type StorageValue = string; +type StoredData = Record; + class SecureStorage { - private storePath: string; - private cache: Map = new Map(); + private readonly storePath: string; + private readonly cache: Map = new Map(); private isLoaded: boolean = false; - private appReady: boolean = false; constructor() { - const userDataPath = app.getPath('userData'); + const userDataPath: string = app.getPath('userData'); this.storePath = path.join(userDataPath, 'secure-config.json'); - - // Wait for app to be ready before using safeStorage - if (app.isReady()) { - this.appReady = true; - } else { - app.whenReady().then(() => { - this.appReady = true; - }); - } } /** @@ -55,43 +49,47 @@ class SecureStorage { return; } - const fileData = fs.readFileSync(this.storePath, 'utf-8'); - const parsed = JSON.parse(fileData); + const fileData: string = fs.readFileSync(this.storePath, 'utf-8'); + const parsed: unknown = JSON.parse(fileData); - // Load all values and store in cache - for (const [key, storedValue] of Object.entries(parsed)) { - if (typeof storedValue !== 'string' || storedValue.length === 0) { + if (typeof parsed !== 'object' || parsed === null) { + console.error('[SecureStorage] Invalid data format in storage file'); + return; + } + + for (const [key, unknownValue] of Object.entries(parsed)) { + if (typeof unknownValue !== 'string' || unknownValue.length === 0) { continue; } + const storedValue: string = unknownValue; + try { if (storedValue.startsWith('encrypted:')) { - // Decrypt encrypted value - const encryptedBase64 = storedValue.substring('encrypted:'.length); - const buffer = Buffer.from(encryptedBase64, 'base64'); - const decrypted = safeStorage.decryptString(buffer); + const encryptedBase64: string = storedValue.substring('encrypted:'.length); + const buffer: Buffer = Buffer.from(encryptedBase64, 'base64'); + const decrypted: string = safeStorage.decryptString(buffer); this.cache.set(key, decrypted); } else if (storedValue.startsWith('plain:')) { - // Load plain value - const plainValue = storedValue.substring('plain:'.length); + const plainValue: string = storedValue.substring('plain:'.length); this.cache.set(key, plainValue); } else { - // Legacy format (try to decrypt) try { - const buffer = Buffer.from(storedValue, 'base64'); - const decrypted = safeStorage.decryptString(buffer); + const buffer: Buffer = Buffer.from(storedValue, 'base64'); + const decrypted: string = safeStorage.decryptString(buffer); this.cache.set(key, decrypted); - } catch { - // If decrypt fails, assume it's plain text + } catch (decryptError: unknown) { this.cache.set(key, storedValue); } } - } catch (error) { - console.error(`[SecureStorage] Failed to load key '${key}':`, error); + } catch (error: unknown) { + const errorMessage: string = error instanceof Error ? error.message : 'Unknown error'; + console.error(`[SecureStorage] Failed to load key '${key}': ${errorMessage}`); } } - } catch (error) { - console.error('[SecureStorage] Failed to load from disk:', error); + } catch (error: unknown) { + const errorMessage: string = error instanceof Error ? error.message : 'Unknown error'; + console.error(`[SecureStorage] Failed to load from disk: ${errorMessage}`); } } @@ -99,43 +97,35 @@ class SecureStorage { * Save encrypted data from memory cache to disk */ private saveToDisk(): void { - try { - const data: Record = {}; + if (!safeStorage.isEncryptionAvailable()) { + throw new Error('Encryption not available - cannot save securely'); + } - // Check if encryption is available - const canEncrypt = safeStorage.isEncryptionAvailable(); + const data: StoredData = {}; - for (const [key, value] of this.cache.entries()) { - if (canEncrypt && safeStorage.isEncryptionAvailable()) { - try { - if (value && typeof value === 'string') { - const buffer = safeStorage.encryptString(value); - if (buffer && buffer.length > 0) { - data[key] = `encrypted:${buffer.toString('base64')}`; - } else { - throw new Error(`Failed to encrypt key '${key}'`); - } - } else { - throw new Error(`Invalid value for key '${key}'`); - } - } catch (encryptError) { - console.error(`[SecureStorage] CRITICAL: Cannot encrypt key '${key}':`, encryptError); - throw encryptError; - } - } else { - throw new Error('Encryption not available - cannot save securely'); - } + for (const [key, value] of this.cache.entries()) { + if (!value) { + throw new Error(`Invalid value for key '${key}'`); } - // Ensure directory exists - const dir = path.dirname(this.storePath); + const buffer: Buffer = safeStorage.encryptString(value); + if (!buffer || buffer.length === 0) { + throw new Error(`Failed to encrypt key '${key}'`); + } + data[key] = `encrypted:${buffer.toString('base64')}`; + } + + try { + const dir: string = path.dirname(this.storePath); if (!fs.existsSync(dir)) { fs.mkdirSync(dir, { recursive: true }); } fs.writeFileSync(this.storePath, JSON.stringify(data, null, 2), 'utf-8'); - } catch (error) { - console.error('[SecureStorage] Failed to save to disk:', error); + } catch (error: unknown) { + const errorMessage: string = error instanceof Error ? error.message : 'Unknown error'; + console.error(`[SecureStorage] Failed to save to disk: ${errorMessage}`); + throw error; } } @@ -145,18 +135,17 @@ class SecureStorage { * @param defaultValue - Default value if key doesn't exist * @returns Stored value or default */ - get(key: string, defaultValue: T | null = null): T | null { + public get(key: string, defaultValue: T | null = null): T | null { this.ensureLoaded(); - const value = this.cache.get(key); + const value: StorageValue | undefined = this.cache.get(key); + if (value === undefined) { return defaultValue; } - // Try to parse as JSON for objects/arrays try { return JSON.parse(value) as T; } catch { - // Return as-is if not JSON return value as unknown as T; } } @@ -166,11 +155,9 @@ class SecureStorage { * @param key - Storage key * @param value - Value to store */ - set(key: string, value: unknown): void { + public set(key: string, value: unknown): void { this.ensureLoaded(); - // Convert to string (JSON if object/array) - const stringValue = typeof value === 'string' ? value : JSON.stringify(value); - + const stringValue: string = typeof value === 'string' ? value : JSON.stringify(value); this.cache.set(key, stringValue); } @@ -178,7 +165,7 @@ class SecureStorage { * Delete a value from secure storage (memory only) * @param key - Storage key */ - delete(key: string): void { + public delete(key: string): void { this.ensureLoaded(); this.cache.delete(key); } @@ -188,7 +175,7 @@ class SecureStorage { * @param key - Storage key * @returns True if key exists */ - has(key: string): boolean { + public has(key: string): boolean { this.ensureLoaded(); return this.cache.has(key); } @@ -196,7 +183,7 @@ class SecureStorage { /** * Clear all data from secure storage (memory only) */ - clear(): void { + public clear(): void { this.cache.clear(); } @@ -204,7 +191,7 @@ class SecureStorage { * Manually save to disk (encrypted with safeStorage) * Call this when you want to persist data */ - save(): void { + public save(): void { this.saveToDisk(); } @@ -212,12 +199,11 @@ class SecureStorage { * Check if encryption is available * @returns True if OS-level encryption is available */ - isEncryptionAvailable(): boolean { + public isEncryptionAvailable(): boolean { return safeStorage.isEncryptionAvailable(); } } -// Store singleton in global scope to avoid multiple instances with dynamic imports declare global { var __secureStorageInstance: SecureStorage | undefined; } @@ -230,7 +216,6 @@ export function getSecureStorage(): SecureStorage { if (!global.__secureStorageInstance) { global.__secureStorageInstance = new SecureStorage(); - // Log encryption availability if (!global.__secureStorageInstance.isEncryptionAvailable()) { console.warn( '[SecureStorage] WARNING: OS-level encryption is not available. ' +