Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@ export class SecretsManagerEncryptedStorage extends AbstractSecretsManagerStorag
}

async getAll(): Promise<SecretProviderConfig[]> {
const allData = this.encryptedStore.getAll<SecretProviderConfig>();
const allData =
this.encryptedStore.getAll<Record<string, SecretProviderConfig>>();
return Object.values(allData);
}

Expand Down
54 changes: 54 additions & 0 deletions src/lib/secretsManager/errors.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,54 @@
import { SecretReference } from "./types";

export enum SecretsErrorCode {
SAFE_STORAGE_ENCRYPTION_NOT_AVAILABLE = "safe_storage_encryption_not_available",

PROVIDER_NOT_FOUND = "provider_not_found",

AUTH_FAILED = "auth_failed",
PERMISSION_DENIED = "permission_denied",

SECRET_NOT_FOUND = "secret_not_found",
SECRET_FETCH_FAILED = "secret_fetch_failed",

STORAGE_READ_FAILED = "storage_read_failed",
STORAGE_WRITE_FAILED = "storage_write_failed",

UNKNOWN = "unknown",
}

export interface SecretsError {
code: SecretsErrorCode;
message: string;
providerId?: string;
secretRef?: SecretReference;
cause?: Error; // Original error
}

export type SecretsManagerError = {
type: "error";
error: SecretsError;
};

export type SecretsSuccess<T> = T extends void
? { type: "success" }
: { type: "success"; data: T };

export type SecretsResult<T> = SecretsSuccess<T> | SecretsManagerError;

export type SecretsResultPromise<T> = Promise<SecretsResult<T>>;

export function createSecretsError(
code: SecretsErrorCode,
message: string,
context?: Omit<SecretsError, "code" | "message">
): SecretsManagerError {
return {
type: "error",
error: {
code,
message,
...context,
},
};
}
112 changes: 112 additions & 0 deletions src/lib/secretsManager/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,112 @@
import { SecretsManagerEncryptedStorage } from "./encryptedStorage/SecretsManagerEncryptedStorage";
import { FileBasedProviderRegistry } from "./providerRegistry/FileBasedProviderRegistry";
import { ProviderChangeCallback } from "./providerRegistry/AbstractProviderRegistry";
import { SecretsManager } from "./secretsManager";
import {
SecretProviderConfig,
SecretProviderMetadata,
SecretReference,
SecretValue,
} from "./types";
import {
createSecretsError,
SecretsErrorCode,
SecretsResultPromise,
} from "./errors";

const getSecretsManager = (): SecretsManager => {
if (!SecretsManager.isInitialized()) {
return null as any;
}
return SecretsManager.getInstance();
};
Comment on lines +17 to +22
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🔴 Critical

Returning null as any causes silent runtime crashes.

When SecretsManager.isInitialized() returns false, this function returns null cast to SecretsManager. All exported functions then call methods on this null value (e.g., getSecretsManager().onProvidersChange(callback)), causing NPEs at runtime with confusing error messages.

Consider throwing an explicit error to fail fast with a clear message:

🐛 Proposed fix
 const getSecretsManager = (): SecretsManager => {
   if (!SecretsManager.isInitialized()) {
-    return null as any;
+    throw new Error("SecretsManager is not initialized. Call initSecretsManager() first.");
   }
   return SecretsManager.getInstance();
 };
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
const getSecretsManager = (): SecretsManager => {
if (!SecretsManager.isInitialized()) {
return null as any;
}
return SecretsManager.getInstance();
};
const getSecretsManager = (): SecretsManager => {
if (!SecretsManager.isInitialized()) {
throw new Error("SecretsManager is not initialized. Call initSecretsManager() first.");
}
return SecretsManager.getInstance();
};
🤖 Prompt for AI Agents
In `@src/lib/secretsManager/index.ts` around lines 7 - 12, The current
getSecretsManager function returns null when SecretsManager.isInitialized() is
false, causing NPEs elsewhere; change getSecretsManager to throw a clear,
descriptive Error (e.g., "SecretsManager not initialized - call init first")
instead of returning null so callers like
getSecretsManager().onProvidersChange(...) fail fast; update any docs/comments
if needed and keep the checks around SecretsManager.isInitialized(),
SecretsManager.getInstance(), and calls to onProvidersChange/getSecret etc. to
reflect this contract.


const PROVIDERS_DIRECTORY = "providers";

export const initSecretsManager = async (): SecretsResultPromise<void> => {
try {
const secretsStorage = new SecretsManagerEncryptedStorage(
PROVIDERS_DIRECTORY
);
const registry = new FileBasedProviderRegistry(secretsStorage);

await SecretsManager.initialize(registry);

return {
type: "success",
};
} catch (error) {
if ((error as Error).name === "SafeStorageEncryptionNotAvailable") {
return createSecretsError(
SecretsErrorCode.SAFE_STORAGE_ENCRYPTION_NOT_AVAILABLE,
"Safe storage encryption is not available.", // UI to show OS specific message here
{
cause: error as Error,
}
);
}

return createSecretsError(
SecretsErrorCode.UNKNOWN,
"Failed to initialize SecretsManager.",
{
cause: error as Error,
}
);
}
};

export const subscribeToProvidersChange = (
callback: ProviderChangeCallback
): (() => void) => {
return getSecretsManager().onProvidersChange(callback);
};

export const setSecretProviderConfig = async (
config: SecretProviderConfig
): SecretsResultPromise<void> => {
return getSecretsManager().setProviderConfig(config);
};

export const removeSecretProviderConfig = async (
providerId: string
): SecretsResultPromise<void> => {
return getSecretsManager().removeProviderConfig(providerId);
};

export const getSecretProviderConfig = async (
providerId: string
): SecretsResultPromise<SecretProviderConfig | null> => {
return getSecretsManager().getProviderConfig(providerId);
};

export const testSecretProviderConnection = async (
providerId: string
): SecretsResultPromise<boolean> => {
return getSecretsManager().testProviderConnection(providerId);
};

export const getSecretValue = async (
providerId: string,
ref: SecretReference
): SecretsResultPromise<SecretValue | null> => {
return getSecretsManager().getSecret(providerId, ref);
};

export const getSecretValues = async (
secrets: Array<{ providerId: string; ref: SecretReference }>
): SecretsResultPromise<SecretValue[]> => {
return getSecretsManager().getSecrets(secrets);
};

export const refreshSecrets = async (
providerId: string
): SecretsResultPromise<(SecretValue | null)[]> => {
return getSecretsManager().refreshSecrets(providerId);
};

export const listSecretProviders = async (): SecretsResultPromise<
SecretProviderMetadata[]
> => {
return getSecretsManager().listProviders();
};
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
import { SecretProviderConfig, SecretProviderType } from "../types";
import {
SecretProviderConfig,
SecretProviderMetadata,
SecretProviderType,
} from "../types";
import { AbstractSecretsManagerStorage } from "../encryptedStorage/AbstractSecretsManagerStorage";
import { AbstractSecretProvider } from "../providerService/AbstractSecretProvider";

export type ProviderChangeCallback = (
configs: Omit<SecretProviderConfig, "config">[]
configs: SecretProviderMetadata[]
) => void;

export abstract class AbstractProviderRegistry {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,16 +17,7 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry {
private async initProvidersFromStorage(): Promise<void> {
const configs = await this.getAllProviderConfigs();
configs.forEach((config) => {
try {
this.providers.set(config.id, createProviderInstance(config));
} catch (error) {
// TODO error to be propagated
console.log(
"!!!debug",
`Failed to initialize provider for config id: ${config.id}`,
error
);
}
this.providers.set(config.id, createProviderInstance(config)); // TODO: check if this needs error handling
});
}

Expand All @@ -36,12 +27,7 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry {
}

async getProviderConfig(id: string): Promise<SecretProviderConfig | null> {
try {
return await this.store.get(id);
} catch (error) {
console.error(`Failed to load provider config for id: ${id}`, error);
return null;
}
return this.store.get(id);
}

async setProviderConfig(config: SecretProviderConfig): Promise<void> {
Expand All @@ -55,7 +41,9 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry {
this.providers.delete(id);
}

getProvider(providerId: string): AbstractSecretProvider<SecretProviderType> | null {
getProvider(
providerId: string
): AbstractSecretProvider<SecretProviderType> | null {
return this.providers.get(providerId) ?? null;
}

Expand Down Expand Up @@ -88,16 +76,8 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry {
}

for (const [id, config] of Object.entries(data)) {
try {
// recreate provider instance
this.providers.set(id, createProviderInstance(config));
} catch (error) {
console.log(
"!!!debug",
`Failed to sync provider for config id: ${id}`,
error
);
}
// recreate provider instance
this.providers.set(id, createProviderInstance(config));
}
}

Expand All @@ -106,7 +86,7 @@ export class FileBasedProviderRegistry extends AbstractProviderRegistry {
): void {
this.changeCallbacks.forEach((callback) => {
const configsMetadata = Object.values(data).map((config) => {
const { config: _, ...metadata } = config;
const { credentials: _, ...metadata } = config;
return metadata;
});
callback(configsMetadata);
Expand Down
38 changes: 12 additions & 26 deletions src/lib/secretsManager/providerService/awsSecretManagerProvider.ts
Original file line number Diff line number Diff line change
Expand Up @@ -67,24 +67,16 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider<SecretProv
return false;
}

try {
const listSecretsCommand = new ListSecretsCommand({ MaxResults: 1 });
const res = await this.client.send(listSecretsCommand);
console.log("!!!debug", "aws result", res);

if (res.$metadata.httpStatusCode !== 200) {
return false;
}

return true;
} catch (err) {
console.error(
"!!!debug",
"aws secrets manager test connection error",
err
);

const listSecretsCommand = new ListSecretsCommand({ MaxResults: 1 });
const res = await this.client.send(listSecretsCommand);
console.log("!!!debug", "aws result", res);

if (res.$metadata.httpStatusCode !== 200) {
return false;
}

return true;
}

async getSecret(ref: AwsSecretReference): Promise<AwsSecretValue | null> {
Expand All @@ -93,7 +85,9 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider<SecretProv
}

const cacheKey = this.getCacheKey(ref);
const cachedSecret = this.getCachedSecret(cacheKey) as AwsSecretValue | null;
const cachedSecret = this.getCachedSecret(
cacheKey
) as AwsSecretValue | null;

if (cachedSecret) {
console.log("!!!debug", "returning from cache", cachedSecret);
Expand All @@ -108,13 +102,7 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider<SecretProv
const secretResponse = await this.client.send(getSecretCommand);

if (secretResponse.$metadata.httpStatusCode !== 200) {
console.error("!!!debug", "Failed to fetch secret", secretResponse);
return null;
}

if (!secretResponse.SecretString) {
console.error("!!!debug", "SecretString is empty", secretResponse);
return null;
throw new Error("Failed to fetch secret from AWS Secrets Manager.");
}

const awsSecret: AwsSecretValue = {
Expand All @@ -128,8 +116,6 @@ export class AWSSecretsManagerProvider extends AbstractSecretProvider<SecretProv
versionId: secretResponse.VersionId,
};

console.log("!!!debug", "returning after fetching", awsSecret);

this.setCacheEntry(cacheKey, awsSecret);

return awsSecret;
Expand Down
Loading