From e7718ec0e30d70afeb12b2e1e61d153eb44be675 Mon Sep 17 00:00:00 2001 From: nafees87n Date: Wed, 4 Mar 2026 20:43:34 +0530 Subject: [PATCH 1/3] [DB-48] added persistence to secret values --- .../AbstractSecretsManagerStorage.ts | 36 ++++++-- .../SecretsManagerEncryptedStorage.ts | 86 ++++++++++++++++--- src/lib/secretsManager/index.ts | 61 +++++++++++-- .../FileBasedProviderRegistry.ts | 22 +++-- .../providerService/AbstractSecretProvider.ts | 83 ++++++++---------- .../awsSecretManagerProvider.ts | 73 +++++++++------- .../providerService/hashicorpVaultProvider.ts | 13 +-- .../providerService/providerFactory.ts | 7 +- src/lib/secretsManager/secretsManager.ts | 79 +++++++++++++++-- src/lib/storage/EncryptedElectronStore.ts | 8 ++ src/main/events.js | 17 +++- 11 files changed, 348 insertions(+), 137 deletions(-) diff --git a/src/lib/secretsManager/encryptedStorage/AbstractSecretsManagerStorage.ts b/src/lib/secretsManager/encryptedStorage/AbstractSecretsManagerStorage.ts index 67ba72ba..efeccef8 100644 --- a/src/lib/secretsManager/encryptedStorage/AbstractSecretsManagerStorage.ts +++ b/src/lib/secretsManager/encryptedStorage/AbstractSecretsManagerStorage.ts @@ -1,17 +1,35 @@ -import { SecretProviderConfig } from "../types"; +import { SecretProviderConfig, SecretValue } from "../types"; -export type StorageChangeCallback = ( - data: Record +export type ProviderStorageChangeCallback = ( + providers: Record +) => void; +export type SecretStorageChangeCallback = ( + secrets: Record ) => void; export abstract class AbstractSecretsManagerStorage { - abstract set(_key: string, _data: SecretProviderConfig): Promise; - - abstract get(_key: string): Promise; + abstract setProviderConfig( + _providerId: string, + _data: SecretProviderConfig + ): Promise; + abstract setSecretValue(_secretId: string, _data: SecretValue): Promise; + abstract setSecretValues( + _entries: Record + ): Promise; - abstract getAll(): Promise; + abstract getProviderConfig( + _providerId: string + ): Promise; + abstract getSecretValue(_secretId: string): Promise; - abstract delete(_key: string): Promise; + abstract getAllProviderConfigs(): Promise; + abstract getAllSecretValues(): Promise; + abstract deleteProviderConfig(_providerId: string): Promise; + abstract deleteSecretValue(_secretId: string): Promise; + abstract deleteSecretValues(_keys: string[]): Promise; - abstract onStorageChange(callback: StorageChangeCallback): () => void; + abstract onProvidersChange( + callback: ProviderStorageChangeCallback + ): () => void; + abstract onSecretsChange(callback: SecretStorageChangeCallback): () => void; } diff --git a/src/lib/secretsManager/encryptedStorage/SecretsManagerEncryptedStorage.ts b/src/lib/secretsManager/encryptedStorage/SecretsManagerEncryptedStorage.ts index 314c36fc..8102498b 100644 --- a/src/lib/secretsManager/encryptedStorage/SecretsManagerEncryptedStorage.ts +++ b/src/lib/secretsManager/encryptedStorage/SecretsManagerEncryptedStorage.ts @@ -1,9 +1,10 @@ import { AbstractSecretsManagerStorage, - StorageChangeCallback, + ProviderStorageChangeCallback, + SecretStorageChangeCallback, } from "./AbstractSecretsManagerStorage"; import { EncryptedElectronStore } from "../../storage/EncryptedElectronStore"; -import { SecretProviderConfig } from "../types"; +import { SecretProviderConfig, SecretReference, SecretValue } from "../types"; export class SecretsManagerEncryptedStorage extends AbstractSecretsManagerStorage { private encryptedStore: EncryptedElectronStore; @@ -13,25 +14,82 @@ export class SecretsManagerEncryptedStorage extends AbstractSecretsManagerStorag this.encryptedStore = new EncryptedElectronStore(storeName); } - async set(key: string, data: SecretProviderConfig): Promise { - return this.encryptedStore.set(key, data); + async setProviderConfig( + providerId: string, + data: SecretProviderConfig + ): Promise { + return this.encryptedStore.set( + `providers.${providerId}`, + data + ); } - async get(key: string): Promise { - return this.encryptedStore.get(key); + async setSecretValue(secretId: string, data: SecretValue): Promise { + return this.encryptedStore.set(`secrets.${secretId}`, data); } - async getAll(): Promise { - const allData = - this.encryptedStore.getAll>(); - return Object.values(allData); + async setSecretValues(entries: Record): Promise { + const current = + this.encryptedStore.get>("secrets") ?? {}; + this.encryptedStore.set>("secrets", { + ...current, + ...entries, + }); } - async delete(key: string): Promise { - return this.encryptedStore.delete(key); + async getProviderConfig( + providerId: string + ): Promise { + return this.encryptedStore.get( + `providers.${providerId}` + ); } - onStorageChange(callback: StorageChangeCallback): () => void { - return this.encryptedStore.onChange(callback); + async getSecretValue(secretId: string): Promise { + return this.encryptedStore.get(`secrets.${secretId}`); + } + + async getAllProviderConfigs(): Promise { + const allProviders = + this.encryptedStore.get>( + `providers` + ); + return Object.values(allProviders ?? {}); + } + + async getAllSecretValues(): Promise { + const allSecrets = + this.encryptedStore.get>(`secrets`); + return Object.values(allSecrets ?? {}); + } + + async deleteProviderConfig(providerId: string): Promise { + return this.encryptedStore.delete(`providers.${providerId}`); + } + + async deleteSecretValue(secretId: string): Promise { + return this.encryptedStore.delete(`secrets.${secretId}`); + } + + async deleteSecretValues(keys: string[]): Promise { + const current = + this.encryptedStore.get>("secrets") ?? {}; + for (const key of keys) { + delete current[key]; + } + this.encryptedStore.set>("secrets", current); + } + + onProvidersChange(callback: ProviderStorageChangeCallback): () => void { + return this.encryptedStore.onKeyChange< + Record + >("providers", callback); + } + + onSecretsChange(callback: SecretStorageChangeCallback): () => void { + return this.encryptedStore.onKeyChange>( + "secrets", + callback + ); } } diff --git a/src/lib/secretsManager/index.ts b/src/lib/secretsManager/index.ts index 48a32b8a..e5c2ad81 100644 --- a/src/lib/secretsManager/index.ts +++ b/src/lib/secretsManager/index.ts @@ -1,4 +1,9 @@ import { SecretsManagerEncryptedStorage } from "./encryptedStorage/SecretsManagerEncryptedStorage"; +import { + AbstractSecretsManagerStorage, + ProviderStorageChangeCallback, + SecretStorageChangeCallback, +} from "./encryptedStorage/AbstractSecretsManagerStorage"; import { FileBasedProviderRegistry } from "./providerRegistry/FileBasedProviderRegistry"; import { ProviderChangeCallback } from "./providerRegistry/AbstractProviderRegistry"; import { SecretsManager } from "./secretsManager"; @@ -15,6 +20,33 @@ import { } from "./errors"; import { createProviderInstance } from "./providerService/providerFactory"; +export class NoopSecretsManagerStorage extends AbstractSecretsManagerStorage { + async setProviderConfig(): Promise {} + async setSecretValue(): Promise {} + async setSecretValues(): Promise {} + async deleteSecretValues(): Promise {} + async getProviderConfig(): Promise { + return null; + } + async getSecretValue(): Promise { + return null; + } + async getAllProviderConfigs(): Promise<[]> { + return []; + } + async getAllSecretValues(): Promise<[]> { + return []; + } + async deleteProviderConfig(): Promise {} + async deleteSecretValue(): Promise {} + onProvidersChange(_callback: ProviderStorageChangeCallback): () => void { + return () => {}; + } + onSecretsChange(_callback: SecretStorageChangeCallback): () => void { + return () => {}; + } +} + const getSecretsManager = (): SecretsManager => { if (!SecretsManager.isInitialized()) { return null as any; @@ -22,13 +54,12 @@ const getSecretsManager = (): SecretsManager => { return SecretsManager.getInstance(); }; -const PROVIDERS_DIRECTORY = "providers"; - -export const initSecretsManager = async (): SecretsResultPromise => { +export const initSecretsManager = async ( + userId: string +): SecretsResultPromise => { try { - const secretsStorage = new SecretsManagerEncryptedStorage( - PROVIDERS_DIRECTORY - ); + const storeName = `sm-${userId}`; + const secretsStorage = new SecretsManagerEncryptedStorage(storeName); const registry = new FileBasedProviderRegistry(secretsStorage); await SecretsManager.initialize(registry); @@ -112,11 +143,27 @@ export const listSecretProviders = async (): SecretsResultPromise< return getSecretsManager().listProviders(); }; +export const removeSecretValue = async ( + providerId: string, + ref: SecretReference +): SecretsResultPromise => { + return getSecretsManager().removeSecret(providerId, ref); +}; + +export const removeSecretValues = async ( + secrets: Array<{ providerId: string; ref: SecretReference }> +): SecretsResultPromise => { + return getSecretsManager().removeSecrets(secrets); +}; + export const testSecretProviderConnectionWithConfig = async ( config: SecretProviderConfig ): SecretsResultPromise => { try { - const provider = createProviderInstance(config); + const provider = createProviderInstance( + config, + new NoopSecretsManagerStorage() + ); const isConnected = await provider.testConnection(); return { type: "success", data: isConnected ?? false }; } catch (error) { diff --git a/src/lib/secretsManager/providerRegistry/FileBasedProviderRegistry.ts b/src/lib/secretsManager/providerRegistry/FileBasedProviderRegistry.ts index d330bff5..72f3f6eb 100644 --- a/src/lib/secretsManager/providerRegistry/FileBasedProviderRegistry.ts +++ b/src/lib/secretsManager/providerRegistry/FileBasedProviderRegistry.ts @@ -17,27 +17,26 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry { private async initProvidersFromStorage(): Promise { const configs = await this.getAllProviderConfigs(); configs.forEach((config) => { - this.providers.set(config.id, createProviderInstance(config)); // TODO: check if this needs error handling + this.providers.set(config.id, createProviderInstance(config, this.store)); }); } async getAllProviderConfigs(): Promise { - const allConfigs = this.store.getAll(); - return allConfigs; + return this.store.getAllProviderConfigs(); } async getProviderConfig(id: string): Promise { - return this.store.get(id); + return this.store.getProviderConfig(id); } async setProviderConfig(config: SecretProviderConfig): Promise { - const provider = createProviderInstance(config); - await this.store.set(config.id, config); + const provider = createProviderInstance(config, this.store); + await this.store.setProviderConfig(config.id, config); this.providers.set(config.id, provider); } async deleteProviderConfig(id: string): Promise { - await this.store.delete(id); + await this.store.deleteProviderConfig(id); this.providers.delete(id); } @@ -56,9 +55,9 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry { } private setupStorageListener(): void { - this.store.onStorageChange((data) => { - this.syncProvidersFromStorageData(data); - this.notifyChangeCallbacks(data); + this.store.onProvidersChange((providers) => { + this.syncProvidersFromStorageData(providers); + this.notifyChangeCallbacks(providers); }); } @@ -76,8 +75,7 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry { } for (const [id, config] of Object.entries(data)) { - // recreate provider instance - this.providers.set(id, createProviderInstance(config)); + this.providers.set(id, createProviderInstance(config, this.store)); } } diff --git a/src/lib/secretsManager/providerService/AbstractSecretProvider.ts b/src/lib/secretsManager/providerService/AbstractSecretProvider.ts index ae5149ff..ffe62072 100644 --- a/src/lib/secretsManager/providerService/AbstractSecretProvider.ts +++ b/src/lib/secretsManager/providerService/AbstractSecretProvider.ts @@ -2,11 +2,10 @@ import { SecretProviderType } from "../baseTypes"; import { CredentialsForProvider, ReferenceForProvider, + SecretValue, ValueForProvider, } from "../types"; - -const DEFAULT_CACHE_TTL_MS = 60 * 60 * 1000; // 1 hour -const DEFAULT_MAX_CACHE_SIZE = 100; +import { AbstractSecretsManagerStorage } from "../encryptedStorage/AbstractSecretsManagerStorage"; /** * Generic abstract base class for secret providers. @@ -14,11 +13,7 @@ const DEFAULT_MAX_CACHE_SIZE = 100; * @template T - The provider type */ export abstract class AbstractSecretProvider { - protected cache: Map> = new Map(); - - protected cacheTtlMs: number = DEFAULT_CACHE_TTL_MS; - - protected maxCacheSize: number = DEFAULT_MAX_CACHE_SIZE; + protected store: AbstractSecretsManagerStorage; abstract readonly type: T; @@ -26,7 +21,15 @@ export abstract class AbstractSecretProvider { protected abstract config: CredentialsForProvider; - protected abstract getCacheKey(_ref: ReferenceForProvider): string; + /** + * Returns the key used to persist/retrieve this secret in the store. + * Must be unique across all secrets for this provider. + */ + protected abstract getStorageKey(_ref: ReferenceForProvider): string; + + constructor(store: AbstractSecretsManagerStorage) { + this.store = store; + } abstract testConnection(): Promise; @@ -38,15 +41,11 @@ export abstract class AbstractSecretProvider { _refs: ReferenceForProvider[] ): Promise<(ValueForProvider | null)[]>; - abstract setSecret( - _ref: ReferenceForProvider, - _value: string | Record - ): Promise; + abstract setSecret(_value: ValueForProvider): Promise; abstract setSecrets( _entries: Array<{ - ref: ReferenceForProvider; - value: string | Record; + value: ValueForProvider; }> ): Promise; @@ -66,46 +65,32 @@ export abstract class AbstractSecretProvider { return false; } - protected invalidateCache(): void { - this.cache.clear(); + protected async getPersistedSecret( + key: string + ): Promise | null> { + return this.store.getSecretValue( + key + ) as Promise | null>; } - protected getCachedSecret(key: string): ValueForProvider | null { - const cached = this.cache.get(key); - if (cached && cached.fetchedAt + this.cacheTtlMs > Date.now()) { - return cached; - } - return null; + protected async persistSecret( + key: string, + value: ValueForProvider + ): Promise { + return this.store.setSecretValue(key, value); } - protected setCacheEntry(key: string, value: ValueForProvider): void { - if (this.maxCacheSize <= 0) { - return; - } - - this.evictExpiredEntries(); - - while (this.cache.size >= this.maxCacheSize) { - const oldestKey = this.cache.keys().next().value; - if (!oldestKey) { - break; - } - this.cache.delete(oldestKey); - } - - this.cache.set(key, value); + protected async persistSecrets( + entries: Record> + ): Promise { + return this.store.setSecretValues(entries as Record); } - protected evictExpiredEntries(): void { - const now = Date.now(); - const keysToDelete: string[] = []; - - this.cache.forEach((value, key) => { - if (value.fetchedAt + this.cacheTtlMs <= now) { - keysToDelete.push(key); - } - }); + protected async deletePersistedSecret(key: string): Promise { + return this.store.deleteSecretValue(key); + } - keysToDelete.forEach((key) => this.cache.delete(key)); + protected async deletePersistedSecrets(keys: string[]): Promise { + return this.store.deleteSecretValues(keys); } } diff --git a/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts b/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts index 55e3d651..461df634 100644 --- a/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts +++ b/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts @@ -1,5 +1,10 @@ -import { SecretProviderType, ProviderConfig, SecretReference } from "../baseTypes"; +import { + SecretProviderType, + ProviderConfig, + SecretReference, +} from "../baseTypes"; import { AbstractSecretProvider } from "./AbstractSecretProvider"; +import { AbstractSecretsManagerStorage } from "../encryptedStorage/AbstractSecretsManagerStorage"; import { GetSecretValueCommand, GetSecretValueCommandOutput, @@ -19,7 +24,8 @@ export type AWSSecretProviderConfig = ProviderConfig< AWSSecretsManagerCredentials >; -export interface AwsSecretReference extends SecretReference { +export interface AwsSecretReference + extends SecretReference { identifier: string; version?: string; } @@ -44,8 +50,11 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider { @@ -67,10 +78,8 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider this.getSecret(ref))); } - async setSecret(): Promise { - throw new Error("Method not implemented."); + async setSecret(value: AwsSecretValue): Promise { + await this.persistSecret(this.getStorageKey(value.secretReference), value); } - async setSecrets(): Promise { - throw new Error("Method not implemented."); + async setSecrets(entries: Array<{ value: AwsSecretValue }>): Promise { + const toSet: Record = {}; + for (const entry of entries) { + toSet[this.getStorageKey(entry.value.secretReference)] = entry.value; + } + await this.persistSecrets(toSet); } - async removeSecret(): Promise { - throw new Error("Method not implemented."); + async removeSecret(ref: AwsSecretReference): Promise { + await this.deletePersistedSecret(this.getStorageKey(ref)); } - async removeSecrets(): Promise { - throw new Error("Method not implemented."); + async removeSecrets(refs: AwsSecretReference[]): Promise { + await this.deletePersistedSecrets( + refs.map((ref) => this.getStorageKey(ref)) + ); } async refreshSecrets(): Promise<(AwsSecretValue | null)[]> { - const allSecretRefs = Array.from(this.cache.values()).map( - (secret) => secret.secretReference - ); + const allSecrets = await this.store.getAllSecretValues(); + const providerSecrets = allSecrets.filter( + (s) => s.providerId === this.id + ) as AwsSecretValue[]; - this.invalidateCache(); + await this.deletePersistedSecrets( + providerSecrets.map((s) => this.getStorageKey(s.secretReference)) + ); - return this.getSecrets(allSecretRefs); + return this.getSecrets(providerSecrets.map((s) => s.secretReference)); } static validateConfig(config: AWSSecretsManagerCredentials): boolean { diff --git a/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts b/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts index fdc6a1de..aa897828 100644 --- a/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts +++ b/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts @@ -1,3 +1,4 @@ +import { NoopSecretsManagerStorage } from ".."; import { SecretProviderType, ProviderConfig, SecretReference } from "../baseTypes"; import { AbstractSecretProvider } from "./AbstractSecretProvider"; @@ -41,13 +42,14 @@ export class HashicorpVaultProvider extends AbstractSecretProvider { @@ -63,14 +65,13 @@ export class HashicorpVaultProvider extends AbstractSecretProvider + _value: VaultSecretValue ): Promise { throw new Error("Method not implemented."); } async setSecrets( - _entries: Array<{ ref: VaultSecretReference; value: string | Record }> + _entries: Array<{ value: string | Record }> ): Promise { throw new Error("Method not implemented."); } diff --git a/src/lib/secretsManager/providerService/providerFactory.ts b/src/lib/secretsManager/providerService/providerFactory.ts index 794c44fb..fc472eba 100644 --- a/src/lib/secretsManager/providerService/providerFactory.ts +++ b/src/lib/secretsManager/providerService/providerFactory.ts @@ -1,14 +1,15 @@ import { SecretProviderConfig, SecretProviderType } from "../types"; +import { AbstractSecretsManagerStorage } from "../encryptedStorage/AbstractSecretsManagerStorage"; import { AWSSecretsManagerProvider } from "./awsSecretManagerProvider"; import { AbstractSecretProvider } from "./AbstractSecretProvider"; export function createProviderInstance( - config: SecretProviderConfig + config: SecretProviderConfig, + store: AbstractSecretsManagerStorage ): AbstractSecretProvider { switch (config.type) { case SecretProviderType.AWS_SECRETS_MANAGER: { - // TypeScript knows config is AWSSecretProviderConfig here - return new AWSSecretsManagerProvider(config); + return new AWSSecretsManagerProvider(config, store); } default: { diff --git a/src/lib/secretsManager/secretsManager.ts b/src/lib/secretsManager/secretsManager.ts index c866f3f9..4f030b4f 100644 --- a/src/lib/secretsManager/secretsManager.ts +++ b/src/lib/secretsManager/secretsManager.ts @@ -1,4 +1,9 @@ -import { SecretProviderConfig, SecretProviderMetadata, SecretReference, SecretValue } from "./types"; +import { + SecretProviderConfig, + SecretProviderMetadata, + SecretReference, + SecretValue, +} from "./types"; import { AbstractProviderRegistry, ProviderChangeCallback, @@ -214,6 +219,69 @@ export class SecretsManager { return { type: "success", data: results }; } + async removeSecret( + providerId: string, + ref: SecretReference + ): SecretsResultPromise { + try { + const provider = this.registry.getProvider(providerId); + if (!provider) { + return createSecretsError( + SecretsErrorCode.PROVIDER_NOT_FOUND, + `Provider with id ${providerId} not found`, + { providerId } + ); + } + await provider.removeSecret(ref); + return { type: "success" }; + } catch (error) { + return createSecretsError( + SecretsErrorCode.STORAGE_WRITE_FAILED, + error instanceof Error + ? error.message + : `Failed to remove secret from provider ${providerId}`, + { providerId, secretRef: ref, cause: error as Error } + ); + } + } + + async removeSecrets( + secrets: Array<{ providerId: string; ref: SecretReference }> + ): SecretsResultPromise { + const providerMap: Map = new Map(); + + for (const s of secrets) { + if (!providerMap.has(s.providerId)) { + providerMap.set(s.providerId, []); + } + providerMap.get(s.providerId)?.push(s.ref); + } + + for (const [providerId, refs] of providerMap.entries()) { + try { + const provider = this.registry.getProvider(providerId); + if (!provider) { + return createSecretsError( + SecretsErrorCode.PROVIDER_NOT_FOUND, + `Provider with id ${providerId} not found`, + { providerId } + ); + } + await provider.removeSecrets(refs); + } catch (error) { + return createSecretsError( + SecretsErrorCode.STORAGE_WRITE_FAILED, + error instanceof Error + ? error.message + : `Failed to remove secrets from provider ${providerId}`, + { providerId, cause: error as Error } + ); + } + } + + return { type: "success" }; + } + async refreshSecrets( providerId: string ): SecretsResultPromise<(SecretValue | null)[]> { @@ -242,13 +310,12 @@ export class SecretsManager { } } - async listProviders(): SecretsResultPromise< - SecretProviderMetadata[] - > { + async listProviders(): SecretsResultPromise { try { const configs = await this.registry.getAllProviderConfigs(); - const configMetadata: SecretProviderMetadata[] = - configs.map(({ credentials: _, ...rest }) => rest); + const configMetadata: SecretProviderMetadata[] = configs.map( + ({ credentials: _, ...rest }) => rest + ); return { type: "success", data: configMetadata }; } catch (error) { return createSecretsError( diff --git a/src/lib/storage/EncryptedElectronStore.ts b/src/lib/storage/EncryptedElectronStore.ts index 2c6aff87..6610c6a1 100644 --- a/src/lib/storage/EncryptedElectronStore.ts +++ b/src/lib/storage/EncryptedElectronStore.ts @@ -99,6 +99,14 @@ export class EncryptedElectronStore { }); } + onKeyChange(subKey: string, callback: (_data: T) => void): () => void { + return this.store.onDidChange(`data.${subKey}` as keyof EncryptedStoreSchema, (newValue) => { + if (newValue !== undefined) { + callback(newValue as T); + } + }); + } + getStore(): Store { return this.store; } diff --git a/src/main/events.js b/src/main/events.js index e0868451..fb124ebb 100644 --- a/src/main/events.js +++ b/src/main/events.js @@ -31,6 +31,8 @@ import { listSecretProviders, refreshSecrets, removeSecretProviderConfig, + removeSecretValue, + removeSecretValues, setSecretProviderConfig, subscribeToProvidersChange, testSecretProviderConnection, @@ -301,8 +303,8 @@ export const registerMainProcessEventsForWebAppWindow = (webAppWindow) => { webAppWindow?.send("helper-server-hit"); }); - ipcMain.handle("secretsManager:init", () => { - return initSecretsManager(); + ipcMain.handle("secretsManager:init", (event, { userId }) => { + return initSecretsManager(userId); }); ipcMain.handle("secretsManager:subscribeToProvidersChange", () => { @@ -367,6 +369,17 @@ export const registerMainProcessEventsForWebAppWindow = (webAppWindow) => { ipcMain.handle("secretsManager:listSecretProviders", () => { return listSecretProviders(); }); + + ipcMain.handle( + "secretsManager:removeSecretValue", + (event, { providerId, secretReference }) => { + return removeSecretValue(providerId, secretReference); + } + ); + + ipcMain.handle("secretsManager:removeSecretValues", (event, { secrets }) => { + return removeSecretValues(secrets); + }); }; export const registerMainProcessCommonEvents = () => { From 39c784bdafecd3d4eb6d065d2b71d4683a58c3e0 Mon Sep 17 00:00:00 2001 From: nafees87n Date: Thu, 5 Mar 2026 21:27:00 +0530 Subject: [PATCH 2/3] handled editing variables using secret IDs --- src/lib/secretsManager/baseTypes.ts | 1 + src/lib/secretsManager/index.ts | 30 ++++++++----- .../providerService/AbstractSecretProvider.ts | 35 ++------------- .../awsSecretManagerProvider.ts | 45 +++++++++---------- .../providerService/hashicorpVaultProvider.ts | 9 ++-- src/lib/secretsManager/secretsManager.ts | 37 ++++++++++++--- src/main/events.js | 11 +++-- 7 files changed, 87 insertions(+), 81 deletions(-) diff --git a/src/lib/secretsManager/baseTypes.ts b/src/lib/secretsManager/baseTypes.ts index 7b087e57..68be3622 100644 --- a/src/lib/secretsManager/baseTypes.ts +++ b/src/lib/secretsManager/baseTypes.ts @@ -25,6 +25,7 @@ export interface ProviderConfig { * @template T - The provider type */ export interface SecretReference { + id: string; type: T; alias: string; } diff --git a/src/lib/secretsManager/index.ts b/src/lib/secretsManager/index.ts index e5c2ad81..733b614b 100644 --- a/src/lib/secretsManager/index.ts +++ b/src/lib/secretsManager/index.ts @@ -21,10 +21,10 @@ import { import { createProviderInstance } from "./providerService/providerFactory"; export class NoopSecretsManagerStorage extends AbstractSecretsManagerStorage { - async setProviderConfig(): Promise {} - async setSecretValue(): Promise {} - async setSecretValues(): Promise {} - async deleteSecretValues(): Promise {} + async setProviderConfig(): Promise { } + async setSecretValue(): Promise { } + async setSecretValues(): Promise { } + async deleteSecretValues(): Promise { } async getProviderConfig(): Promise { return null; } @@ -37,13 +37,13 @@ export class NoopSecretsManagerStorage extends AbstractSecretsManagerStorage { async getAllSecretValues(): Promise<[]> { return []; } - async deleteProviderConfig(): Promise {} - async deleteSecretValue(): Promise {} + async deleteProviderConfig(): Promise { } + async deleteSecretValue(): Promise { } onProvidersChange(_callback: ProviderStorageChangeCallback): () => void { - return () => {}; + return () => { }; } onSecretsChange(_callback: SecretStorageChangeCallback): () => void { - return () => {}; + return () => { }; } } @@ -131,10 +131,11 @@ export const getSecretValues = async ( return getSecretsManager().getSecrets(secrets); }; -export const refreshSecrets = async ( - providerId: string +export const fetchAndSaveSecrets = async ( + providerId: string, + secretRefs: SecretReference[] ): SecretsResultPromise<(SecretValue | null)[]> => { - return getSecretsManager().refreshSecrets(providerId); + return getSecretsManager().fetchAndSaveSecrets(providerId, secretRefs); }; export const listSecretProviders = async (): SecretsResultPromise< @@ -174,3 +175,10 @@ export const testSecretProviderConnectionWithConfig = async ( ); } }; + + +export const listSecrets = async ( + providerId: string +): SecretsResultPromise => { + return getSecretsManager().listSecrets(providerId); +}; \ No newline at end of file diff --git a/src/lib/secretsManager/providerService/AbstractSecretProvider.ts b/src/lib/secretsManager/providerService/AbstractSecretProvider.ts index ffe62072..cea4df06 100644 --- a/src/lib/secretsManager/providerService/AbstractSecretProvider.ts +++ b/src/lib/secretsManager/providerService/AbstractSecretProvider.ts @@ -33,11 +33,11 @@ export abstract class AbstractSecretProvider { abstract testConnection(): Promise; - abstract getSecret( + abstract getSecretValue( _ref: ReferenceForProvider ): Promise | null>; - abstract getSecrets( + abstract getSecretValues( _refs: ReferenceForProvider[] ): Promise<(ValueForProvider | null)[]>; @@ -53,7 +53,7 @@ export abstract class AbstractSecretProvider { abstract removeSecrets(_refs: ReferenceForProvider[]): Promise; - abstract refreshSecrets(): Promise<(ValueForProvider | null)[]>; + abstract listAllSecrets(): Promise<(ValueForProvider)[]>; static validateConfig(config: any): boolean { // Base implementation rejects all configs as a fail-safe. @@ -64,33 +64,4 @@ export abstract class AbstractSecretProvider { return false; } - - protected async getPersistedSecret( - key: string - ): Promise | null> { - return this.store.getSecretValue( - key - ) as Promise | null>; - } - - protected async persistSecret( - key: string, - value: ValueForProvider - ): Promise { - return this.store.setSecretValue(key, value); - } - - protected async persistSecrets( - entries: Record> - ): Promise { - return this.store.setSecretValues(entries as Record); - } - - protected async deletePersistedSecret(key: string): Promise { - return this.store.deleteSecretValue(key); - } - - protected async deletePersistedSecrets(keys: string[]): Promise { - return this.store.deleteSecretValues(keys); - } } diff --git a/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts b/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts index 461df634..46e27c49 100644 --- a/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts +++ b/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts @@ -68,9 +68,7 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider { @@ -88,18 +86,11 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider { + async getSecretValue(ref: AwsSecretReference): Promise { if (!this.client) { throw new Error("AWS Secrets Manager client is not initialized."); } - const storageKey = this.getStorageKey(ref); - const persisted = await this.getPersistedSecret(storageKey); - - if (persisted) { - return persisted as AwsSecretValue; - } - const getSecretCommand = new GetSecretValueCommand({ SecretId: ref.identifier, VersionId: ref.version, @@ -122,12 +113,10 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider { if (!this.client) { @@ -135,42 +124,48 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider this.getSecret(ref))); + // TODO@nafees: check for null values and verify if error thrown - handle partial failures appropriately + const secretValues = await Promise.all(refs.map((ref) => this.getSecretValue(ref))); + return secretValues; } + // upserts a secret in the store async setSecret(value: AwsSecretValue): Promise { - await this.persistSecret(this.getStorageKey(value.secretReference), value); + await this.store.setSecretValue(this.getStorageKey(value.secretReference), value); } + // removes and sets all the secrets for the provider async setSecrets(entries: Array<{ value: AwsSecretValue }>): Promise { + const allSecrets = await this.listAllSecrets(); + + const toRemove = allSecrets.map((s) => this.getStorageKey(s.secretReference)); + + await this.store.deleteSecretValues(toRemove); + const toSet: Record = {}; for (const entry of entries) { toSet[this.getStorageKey(entry.value.secretReference)] = entry.value; } - await this.persistSecrets(toSet); + await this.store.setSecretValues(toSet); } async removeSecret(ref: AwsSecretReference): Promise { - await this.deletePersistedSecret(this.getStorageKey(ref)); + await this.store.deleteSecretValue(this.getStorageKey(ref)); } async removeSecrets(refs: AwsSecretReference[]): Promise { - await this.deletePersistedSecrets( + await this.store.deleteSecretValues( refs.map((ref) => this.getStorageKey(ref)) ); } - async refreshSecrets(): Promise<(AwsSecretValue | null)[]> { + async listAllSecrets(): Promise<(AwsSecretValue)[]> { const allSecrets = await this.store.getAllSecretValues(); const providerSecrets = allSecrets.filter( (s) => s.providerId === this.id ) as AwsSecretValue[]; - await this.deletePersistedSecrets( - providerSecrets.map((s) => this.getStorageKey(s.secretReference)) - ); - - return this.getSecrets(providerSecrets.map((s) => s.secretReference)); + return providerSecrets; } static validateConfig(config: AWSSecretsManagerCredentials): boolean { diff --git a/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts b/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts index aa897828..310d6b32 100644 --- a/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts +++ b/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts @@ -48,19 +48,18 @@ export class HashicorpVaultProvider extends AbstractSecretProvider { throw new Error("Method not implemented."); } - async getSecret(_ref: VaultSecretReference): Promise { + async getSecretValue(_ref: VaultSecretReference): Promise { throw new Error("Method not implemented."); } - async getSecrets(_refs: VaultSecretReference[]): Promise<(VaultSecretValue | null)[]> { + async getSecretValues(_refs: VaultSecretReference[]): Promise<(VaultSecretValue | null)[]> { throw new Error("Method not implemented."); } @@ -84,7 +83,7 @@ export class HashicorpVaultProvider extends AbstractSecretProvider { + async listAllSecrets(): Promise<(VaultSecretValue)[]> { throw new Error("Method not implemented."); } diff --git a/src/lib/secretsManager/secretsManager.ts b/src/lib/secretsManager/secretsManager.ts index 4f030b4f..78ae6f5a 100644 --- a/src/lib/secretsManager/secretsManager.ts +++ b/src/lib/secretsManager/secretsManager.ts @@ -1,4 +1,5 @@ import { + AwsSecretValue, SecretProviderConfig, SecretProviderMetadata, SecretReference, @@ -162,7 +163,7 @@ export class SecretsManager { { providerId } ); } - const secretValue = await provider.getSecret(ref); + const secretValue = await provider.getSecretValue(ref); return { type: "success", data: secretValue }; } catch (error) { return createSecretsError( @@ -201,7 +202,7 @@ export class SecretsManager { ); } - const secretValues = await provider.getSecrets(refs); + const secretValues = await provider.getSecretValues(refs); results.push( ...secretValues.filter((sv): sv is SecretValue => sv !== null) ); @@ -282,8 +283,9 @@ export class SecretsManager { return { type: "success" }; } - async refreshSecrets( - providerId: string + async fetchAndSaveSecrets( + providerId: string, + secretRefs: SecretReference[] ): SecretsResultPromise<(SecretValue | null)[]> { try { const provider = this.registry.getProvider(providerId); @@ -296,7 +298,11 @@ export class SecretsManager { ); } - const secrets = await provider.refreshSecrets(); + const secrets = await provider.getSecretValues(secretRefs) + + // TODO@nafees: Handle null values + // @ts-ignore + await provider.setSecrets(secrets.map((s) => ({ value: s }))); return { type: "success", data: secrets }; } catch (error) { @@ -329,4 +335,25 @@ export class SecretsManager { onProvidersChange(callback: ProviderChangeCallback): () => void { return this.registry.onProvidersChange(callback); } + + async listSecrets(providerId: string): SecretsResultPromise { + try { + const provider = this.registry.getProvider(providerId); + if (!provider) { + return createSecretsError( + SecretsErrorCode.PROVIDER_NOT_FOUND, + `Provider with id ${providerId} not found`, + { providerId } + ); + } + const secrets = await provider.listAllSecrets(); + return { type: "success", data: secrets }; + } catch (error) { + return createSecretsError( + SecretsErrorCode.STORAGE_READ_FAILED, + error instanceof Error ? error.message : "Failed to list secrets", + { providerId, cause: error as Error } + ); + } + } } diff --git a/src/main/events.js b/src/main/events.js index fb124ebb..98c7f147 100644 --- a/src/main/events.js +++ b/src/main/events.js @@ -24,12 +24,13 @@ import { createOrUpdateAxiosInstance } from "./actions/getProxiedAxios"; // eslint-disable-next-line import/no-cycle import createTrayMenu, { loadWebAppUrl } from "./main"; import { + fetchAndSaveSecrets, getSecretProviderConfig, getSecretValue, getSecretValues, initSecretsManager, listSecretProviders, - refreshSecrets, + listSecrets, removeSecretProviderConfig, removeSecretValue, removeSecretValues, @@ -362,8 +363,8 @@ export const registerMainProcessEventsForWebAppWindow = (webAppWindow) => { return getSecretValues(secrets); }); - ipcMain.handle("secretsManager:refreshSecrets", (event, { providerId }) => { - return refreshSecrets(providerId); + ipcMain.handle("secretsManager:fetchAndSaveSecrets", (event, { providerId, secretRefs }) => { + return fetchAndSaveSecrets(providerId, secretRefs); }); ipcMain.handle("secretsManager:listSecretProviders", () => { @@ -380,6 +381,10 @@ export const registerMainProcessEventsForWebAppWindow = (webAppWindow) => { ipcMain.handle("secretsManager:removeSecretValues", (event, { secrets }) => { return removeSecretValues(secrets); }); + + ipcMain.handle("secretsManager:listSecrets", (event, { providerId }) => { + return listSecrets(providerId); + }); }; export const registerMainProcessCommonEvents = () => { From 31a1353a800be8401c614b70eef376ba96b67836 Mon Sep 17 00:00:00 2001 From: nafees87n Date: Fri, 6 Mar 2026 00:54:16 +0530 Subject: [PATCH 3/3] handled errors on secrets fetch --- src/lib/secretsManager/errors.ts | 12 ++++++- src/lib/secretsManager/index.ts | 3 +- .../providerService/AbstractSecretProvider.ts | 7 +++- .../awsSecretManagerProvider.ts | 24 +++++++++++--- .../providerService/hashicorpVaultProvider.ts | 2 +- src/lib/secretsManager/secretsManager.ts | 32 +++++++++++++++---- 6 files changed, 64 insertions(+), 16 deletions(-) diff --git a/src/lib/secretsManager/errors.ts b/src/lib/secretsManager/errors.ts index 62dec904..d0b0a6d2 100644 --- a/src/lib/secretsManager/errors.ts +++ b/src/lib/secretsManager/errors.ts @@ -1,4 +1,4 @@ -import { SecretReference } from "./types"; +import { SecretReference, SecretValue } from "./types"; export enum SecretsErrorCode { SAFE_STORAGE_ENCRYPTION_NOT_AVAILABLE = "safe_storage_encryption_not_available", @@ -38,6 +38,16 @@ export type SecretsResult = SecretsSuccess | SecretsManagerError; export type SecretsResultPromise = Promise>; +export interface SecretFetchError { + secretRefId: string; + message: string; +} + +export interface FetchSecretsResultData { + secrets: SecretValue[]; + errors: SecretFetchError[]; +} + export function createSecretsError( code: SecretsErrorCode, message: string, diff --git a/src/lib/secretsManager/index.ts b/src/lib/secretsManager/index.ts index 733b614b..08eb67a9 100644 --- a/src/lib/secretsManager/index.ts +++ b/src/lib/secretsManager/index.ts @@ -17,6 +17,7 @@ import { createSecretsError, SecretsErrorCode, SecretsResultPromise, + FetchSecretsResultData, } from "./errors"; import { createProviderInstance } from "./providerService/providerFactory"; @@ -134,7 +135,7 @@ export const getSecretValues = async ( export const fetchAndSaveSecrets = async ( providerId: string, secretRefs: SecretReference[] -): SecretsResultPromise<(SecretValue | null)[]> => { +): SecretsResultPromise => { return getSecretsManager().fetchAndSaveSecrets(providerId, secretRefs); }; diff --git a/src/lib/secretsManager/providerService/AbstractSecretProvider.ts b/src/lib/secretsManager/providerService/AbstractSecretProvider.ts index cea4df06..ed3ef47a 100644 --- a/src/lib/secretsManager/providerService/AbstractSecretProvider.ts +++ b/src/lib/secretsManager/providerService/AbstractSecretProvider.ts @@ -7,6 +7,11 @@ import { } from "../types"; import { AbstractSecretsManagerStorage } from "../encryptedStorage/AbstractSecretsManagerStorage"; +export interface GetSecretValuesResult { + results: (ValueForProvider | null)[]; + errors: Array<{ ref: ReferenceForProvider; message: string }>; +} + /** * Generic abstract base class for secret providers. * @@ -39,7 +44,7 @@ export abstract class AbstractSecretProvider { abstract getSecretValues( _refs: ReferenceForProvider[] - ): Promise<(ValueForProvider | null)[]>; + ): Promise>; abstract setSecret(_value: ValueForProvider): Promise; diff --git a/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts b/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts index 46e27c49..a723eba3 100644 --- a/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts +++ b/src/lib/secretsManager/providerService/awsSecretManagerProvider.ts @@ -118,15 +118,29 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider { + ): Promise<{ results: (AwsSecretValue | null)[]; errors: Array<{ ref: AwsSecretReference; message: string }> }> { if (!this.client) { throw new Error("AWS Secrets Manager client is not initialized."); } - // Not using BatchGetSecretValueCommand as it would require additional permissions - // TODO@nafees: check for null values and verify if error thrown - handle partial failures appropriately - const secretValues = await Promise.all(refs.map((ref) => this.getSecretValue(ref))); - return secretValues; + const settled = await Promise.allSettled(refs.map((ref) => this.getSecretValue(ref))); + const results: (AwsSecretValue | null)[] = []; + const errors: Array<{ ref: AwsSecretReference; message: string }> = []; + + for (let i = 0; i < settled.length; i++) { + const r = settled[i]; + if (r.status === "fulfilled") { + results.push(r.value); + } else { + results.push(null); + errors.push({ + ref: refs[i], + message: r.reason instanceof Error ? r.reason.message : String(r.reason), + }); + } + } + + return { results, errors }; } // upserts a secret in the store diff --git a/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts b/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts index 310d6b32..ad5a8112 100644 --- a/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts +++ b/src/lib/secretsManager/providerService/hashicorpVaultProvider.ts @@ -59,7 +59,7 @@ export class HashicorpVaultProvider extends AbstractSecretProvider { + async getSecretValues(_refs: VaultSecretReference[]): Promise<{ results: (VaultSecretValue | null)[]; errors: Array<{ ref: VaultSecretReference; message: string }> }> { throw new Error("Method not implemented."); } diff --git a/src/lib/secretsManager/secretsManager.ts b/src/lib/secretsManager/secretsManager.ts index 78ae6f5a..69673c9e 100644 --- a/src/lib/secretsManager/secretsManager.ts +++ b/src/lib/secretsManager/secretsManager.ts @@ -2,6 +2,7 @@ import { AwsSecretValue, SecretProviderConfig, SecretProviderMetadata, + SecretProviderType, SecretReference, SecretValue, } from "./types"; @@ -13,6 +14,8 @@ import { SecretsResultPromise, createSecretsError, SecretsErrorCode, + SecretFetchError, + FetchSecretsResultData, } from "./errors"; export class SecretsManager { @@ -202,7 +205,7 @@ export class SecretsManager { ); } - const secretValues = await provider.getSecretValues(refs); + const { results: secretValues } = await provider.getSecretValues(refs); results.push( ...secretValues.filter((sv): sv is SecretValue => sv !== null) ); @@ -286,7 +289,7 @@ export class SecretsManager { async fetchAndSaveSecrets( providerId: string, secretRefs: SecretReference[] - ): SecretsResultPromise<(SecretValue | null)[]> { + ): SecretsResultPromise { try { const provider = this.registry.getProvider(providerId); @@ -298,13 +301,28 @@ export class SecretsManager { ); } - const secrets = await provider.getSecretValues(secretRefs) + const { results, errors: fetchErrors } = await provider.getSecretValues(secretRefs); + + const perSecretErrors: SecretFetchError[] = fetchErrors.map((e) => ({ + secretRefId: e.ref.id, + message: e.message, + })); + + const successSecrets: SecretValue[] = []; + for (const s of results) { + if (!s) continue; + const hasEmptyValue = + (s.type === SecretProviderType.AWS_SECRETS_MANAGER && !s.value); + if (hasEmptyValue) { + perSecretErrors.push({ secretRefId: s.secretReference.id, message: "Secret value is empty" }); + continue; + } + successSecrets.push(s); + } - // TODO@nafees: Handle null values - // @ts-ignore - await provider.setSecrets(secrets.map((s) => ({ value: s }))); + await provider.setSecrets(successSecrets.map((s) => ({ value: s }))); - return { type: "success", data: secrets }; + return { type: "success", data: { secrets: successSecrets, errors: perSecretErrors } }; } catch (error) { return createSecretsError( SecretsErrorCode.SECRET_FETCH_FAILED,