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.
This commit is contained in:
@@ -16,24 +16,18 @@ import * as path from 'path';
|
|||||||
* - Does NOT protect against malware running under same user
|
* - Does NOT protect against malware running under same user
|
||||||
* - On Linux, check getStorageBackend() - if 'basic_text', encryption is weak
|
* - On Linux, check getStorageBackend() - if 'basic_text', encryption is weak
|
||||||
*/
|
*/
|
||||||
|
|
||||||
|
type StorageValue = string;
|
||||||
|
type StoredData = Record<string, string>;
|
||||||
|
|
||||||
class SecureStorage {
|
class SecureStorage {
|
||||||
private storePath: string;
|
private readonly storePath: string;
|
||||||
private cache: Map<string, string> = new Map();
|
private readonly cache: Map<string, StorageValue> = new Map();
|
||||||
private isLoaded: boolean = false;
|
private isLoaded: boolean = false;
|
||||||
private appReady: boolean = false;
|
|
||||||
|
|
||||||
constructor() {
|
constructor() {
|
||||||
const userDataPath = app.getPath('userData');
|
const userDataPath: string = app.getPath('userData');
|
||||||
this.storePath = path.join(userDataPath, 'secure-config.json');
|
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;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
const fileData = fs.readFileSync(this.storePath, 'utf-8');
|
const fileData: string = fs.readFileSync(this.storePath, 'utf-8');
|
||||||
const parsed = JSON.parse(fileData);
|
const parsed: unknown = JSON.parse(fileData);
|
||||||
|
|
||||||
// Load all values and store in cache
|
if (typeof parsed !== 'object' || parsed === null) {
|
||||||
for (const [key, storedValue] of Object.entries(parsed)) {
|
console.error('[SecureStorage] Invalid data format in storage file');
|
||||||
if (typeof storedValue !== 'string' || storedValue.length === 0) {
|
return;
|
||||||
|
}
|
||||||
|
|
||||||
|
for (const [key, unknownValue] of Object.entries(parsed)) {
|
||||||
|
if (typeof unknownValue !== 'string' || unknownValue.length === 0) {
|
||||||
continue;
|
continue;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
const storedValue: string = unknownValue;
|
||||||
|
|
||||||
try {
|
try {
|
||||||
if (storedValue.startsWith('encrypted:')) {
|
if (storedValue.startsWith('encrypted:')) {
|
||||||
// Decrypt encrypted value
|
const encryptedBase64: string = storedValue.substring('encrypted:'.length);
|
||||||
const encryptedBase64 = storedValue.substring('encrypted:'.length);
|
const buffer: Buffer = Buffer.from(encryptedBase64, 'base64');
|
||||||
const buffer = Buffer.from(encryptedBase64, 'base64');
|
const decrypted: string = safeStorage.decryptString(buffer);
|
||||||
const decrypted = safeStorage.decryptString(buffer);
|
|
||||||
this.cache.set(key, decrypted);
|
this.cache.set(key, decrypted);
|
||||||
} else if (storedValue.startsWith('plain:')) {
|
} else if (storedValue.startsWith('plain:')) {
|
||||||
// Load plain value
|
const plainValue: string = storedValue.substring('plain:'.length);
|
||||||
const plainValue = storedValue.substring('plain:'.length);
|
|
||||||
this.cache.set(key, plainValue);
|
this.cache.set(key, plainValue);
|
||||||
} else {
|
} else {
|
||||||
// Legacy format (try to decrypt)
|
|
||||||
try {
|
try {
|
||||||
const buffer = Buffer.from(storedValue, 'base64');
|
const buffer: Buffer = Buffer.from(storedValue, 'base64');
|
||||||
const decrypted = safeStorage.decryptString(buffer);
|
const decrypted: string = safeStorage.decryptString(buffer);
|
||||||
this.cache.set(key, decrypted);
|
this.cache.set(key, decrypted);
|
||||||
} catch {
|
} catch (decryptError: unknown) {
|
||||||
// If decrypt fails, assume it's plain text
|
|
||||||
this.cache.set(key, storedValue);
|
this.cache.set(key, storedValue);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error: unknown) {
|
||||||
console.error(`[SecureStorage] Failed to load key '${key}':`, error);
|
const errorMessage: string = error instanceof Error ? error.message : 'Unknown error';
|
||||||
|
console.error(`[SecureStorage] Failed to load key '${key}': ${errorMessage}`);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
} catch (error) {
|
} catch (error: unknown) {
|
||||||
console.error('[SecureStorage] Failed to load from disk:', error);
|
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
|
* Save encrypted data from memory cache to disk
|
||||||
*/
|
*/
|
||||||
private saveToDisk(): void {
|
private saveToDisk(): void {
|
||||||
try {
|
if (!safeStorage.isEncryptionAvailable()) {
|
||||||
const data: Record<string, string> = {};
|
throw new Error('Encryption not available - cannot save securely');
|
||||||
|
}
|
||||||
|
|
||||||
// Check if encryption is available
|
const data: StoredData = {};
|
||||||
const canEncrypt = safeStorage.isEncryptionAvailable();
|
|
||||||
|
|
||||||
for (const [key, value] of this.cache.entries()) {
|
for (const [key, value] of this.cache.entries()) {
|
||||||
if (canEncrypt && safeStorage.isEncryptionAvailable()) {
|
if (!value) {
|
||||||
try {
|
throw new Error(`Invalid value for key '${key}'`);
|
||||||
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');
|
|
||||||
}
|
|
||||||
}
|
}
|
||||||
|
|
||||||
// Ensure directory exists
|
const buffer: Buffer = safeStorage.encryptString(value);
|
||||||
const dir = path.dirname(this.storePath);
|
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)) {
|
if (!fs.existsSync(dir)) {
|
||||||
fs.mkdirSync(dir, { recursive: true });
|
fs.mkdirSync(dir, { recursive: true });
|
||||||
}
|
}
|
||||||
|
|
||||||
fs.writeFileSync(this.storePath, JSON.stringify(data, null, 2), 'utf-8');
|
fs.writeFileSync(this.storePath, JSON.stringify(data, null, 2), 'utf-8');
|
||||||
} catch (error) {
|
} catch (error: unknown) {
|
||||||
console.error('[SecureStorage] Failed to save to disk:', error);
|
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
|
* @param defaultValue - Default value if key doesn't exist
|
||||||
* @returns Stored value or default
|
* @returns Stored value or default
|
||||||
*/
|
*/
|
||||||
get<T = string>(key: string, defaultValue: T | null = null): T | null {
|
public get<T = string>(key: string, defaultValue: T | null = null): T | null {
|
||||||
this.ensureLoaded();
|
this.ensureLoaded();
|
||||||
const value = this.cache.get(key);
|
const value: StorageValue | undefined = this.cache.get(key);
|
||||||
|
|
||||||
if (value === undefined) {
|
if (value === undefined) {
|
||||||
return defaultValue;
|
return defaultValue;
|
||||||
}
|
}
|
||||||
|
|
||||||
// Try to parse as JSON for objects/arrays
|
|
||||||
try {
|
try {
|
||||||
return JSON.parse(value) as T;
|
return JSON.parse(value) as T;
|
||||||
} catch {
|
} catch {
|
||||||
// Return as-is if not JSON
|
|
||||||
return value as unknown as T;
|
return value as unknown as T;
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
@@ -166,11 +155,9 @@ class SecureStorage {
|
|||||||
* @param key - Storage key
|
* @param key - Storage key
|
||||||
* @param value - Value to store
|
* @param value - Value to store
|
||||||
*/
|
*/
|
||||||
set(key: string, value: unknown): void {
|
public set(key: string, value: unknown): void {
|
||||||
this.ensureLoaded();
|
this.ensureLoaded();
|
||||||
// Convert to string (JSON if object/array)
|
const stringValue: string = typeof value === 'string' ? value : JSON.stringify(value);
|
||||||
const stringValue = typeof value === 'string' ? value : JSON.stringify(value);
|
|
||||||
|
|
||||||
this.cache.set(key, stringValue);
|
this.cache.set(key, stringValue);
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -178,7 +165,7 @@ class SecureStorage {
|
|||||||
* Delete a value from secure storage (memory only)
|
* Delete a value from secure storage (memory only)
|
||||||
* @param key - Storage key
|
* @param key - Storage key
|
||||||
*/
|
*/
|
||||||
delete(key: string): void {
|
public delete(key: string): void {
|
||||||
this.ensureLoaded();
|
this.ensureLoaded();
|
||||||
this.cache.delete(key);
|
this.cache.delete(key);
|
||||||
}
|
}
|
||||||
@@ -188,7 +175,7 @@ class SecureStorage {
|
|||||||
* @param key - Storage key
|
* @param key - Storage key
|
||||||
* @returns True if key exists
|
* @returns True if key exists
|
||||||
*/
|
*/
|
||||||
has(key: string): boolean {
|
public has(key: string): boolean {
|
||||||
this.ensureLoaded();
|
this.ensureLoaded();
|
||||||
return this.cache.has(key);
|
return this.cache.has(key);
|
||||||
}
|
}
|
||||||
@@ -196,7 +183,7 @@ class SecureStorage {
|
|||||||
/**
|
/**
|
||||||
* Clear all data from secure storage (memory only)
|
* Clear all data from secure storage (memory only)
|
||||||
*/
|
*/
|
||||||
clear(): void {
|
public clear(): void {
|
||||||
this.cache.clear();
|
this.cache.clear();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -204,7 +191,7 @@ class SecureStorage {
|
|||||||
* Manually save to disk (encrypted with safeStorage)
|
* Manually save to disk (encrypted with safeStorage)
|
||||||
* Call this when you want to persist data
|
* Call this when you want to persist data
|
||||||
*/
|
*/
|
||||||
save(): void {
|
public save(): void {
|
||||||
this.saveToDisk();
|
this.saveToDisk();
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -212,12 +199,11 @@ class SecureStorage {
|
|||||||
* Check if encryption is available
|
* Check if encryption is available
|
||||||
* @returns True if OS-level encryption is available
|
* @returns True if OS-level encryption is available
|
||||||
*/
|
*/
|
||||||
isEncryptionAvailable(): boolean {
|
public isEncryptionAvailable(): boolean {
|
||||||
return safeStorage.isEncryptionAvailable();
|
return safeStorage.isEncryptionAvailable();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
// Store singleton in global scope to avoid multiple instances with dynamic imports
|
|
||||||
declare global {
|
declare global {
|
||||||
var __secureStorageInstance: SecureStorage | undefined;
|
var __secureStorageInstance: SecureStorage | undefined;
|
||||||
}
|
}
|
||||||
@@ -230,7 +216,6 @@ export function getSecureStorage(): SecureStorage {
|
|||||||
if (!global.__secureStorageInstance) {
|
if (!global.__secureStorageInstance) {
|
||||||
global.__secureStorageInstance = new SecureStorage();
|
global.__secureStorageInstance = new SecureStorage();
|
||||||
|
|
||||||
// Log encryption availability
|
|
||||||
if (!global.__secureStorageInstance.isEncryptionAvailable()) {
|
if (!global.__secureStorageInstance.isEncryptionAvailable()) {
|
||||||
console.warn(
|
console.warn(
|
||||||
'[SecureStorage] WARNING: OS-level encryption is not available. ' +
|
'[SecureStorage] WARNING: OS-level encryption is not available. ' +
|
||||||
|
|||||||
Reference in New Issue
Block a user