Skip to content

Commit c804a26

Browse files
gmethvingmethvin
committed
use enum for storage method
1 parent c303fae commit c804a26

File tree

1 file changed

+143
-122
lines changed

1 file changed

+143
-122
lines changed

‎src/key-manager.ts‎

Lines changed: 143 additions & 122 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,18 @@ async function execSecurityDefault(args: string[]): Promise<string> {
5252
// Type for the security executor function (for dependency injection in tests)
5353
export type SecurityExecutor = (args: string[]) => Promise<string>;
5454

55+
/**
56+
* Storage method for API keys
57+
*/
58+
enum StorageMethod {
59+
/** macOS Keychain - keys stored and encrypted by OS keychain service */
60+
KEYCHAIN = "keychain",
61+
/** Windows DPAPI - keys encrypted by OS (user-scoped keys), stored in JSON file */
62+
DPAPI = "dpapi",
63+
/** File storage - plaintext keys in JSON file with restrictive permissions (Linux/other) */
64+
FILE = "file",
65+
}
66+
5567
export interface ApiKeyMetadata {
5668
/** Unique identifier for this key */
5769
id: string;
@@ -85,8 +97,7 @@ export class KeyManager {
8597
private store: KeyStore | null = null;
8698
private saveLock: Promise<void> | null = null;
8799
private readonly execSecurity: SecurityExecutor;
88-
private readonly useKeychain: boolean;
89-
private readonly useDpapi: boolean;
100+
private readonly storageMethod: StorageMethod;
90101

91102
/**
92103
* Create a new KeyManager instance
@@ -102,22 +113,18 @@ export class KeyManager {
102113

103114
// Determine storage method based on platform
104115
// Can be overridden via ITERABLE_MCP_FORCE_FILE_STORAGE=true
105-
const forceFileStorage =
106-
process.env.ITERABLE_MCP_FORCE_FILE_STORAGE === "true";
107-
108-
// When execSecurity is provided (tests), always use Keychain mode
109-
// This allows Keychain tests to run on any platform
110-
if (execSecurity && !forceFileStorage) {
111-
this.useKeychain = true;
112-
this.useDpapi = false;
113-
} else if (!forceFileStorage) {
114-
// Production mode: use platform-specific secure storage
115-
this.useKeychain = process.platform === "darwin";
116-
this.useDpapi = process.platform === "win32";
117-
} else {
116+
if (process.env.ITERABLE_MCP_FORCE_FILE_STORAGE === "true") {
118117
// Force file storage mode (for testing or debugging)
119-
this.useKeychain = false;
120-
this.useDpapi = false;
118+
this.storageMethod = StorageMethod.FILE;
119+
} else if (execSecurity || process.platform === "darwin") {
120+
// Use Keychain on macOS or when execSecurity is provided (for tests)
121+
this.storageMethod = StorageMethod.KEYCHAIN;
122+
} else if (process.platform === "win32") {
123+
// Use DPAPI on Windows
124+
this.storageMethod = StorageMethod.DPAPI;
125+
} else {
126+
// Use file storage on other platforms
127+
this.storageMethod = StorageMethod.FILE;
121128
}
122129
}
123130

@@ -132,7 +139,11 @@ export class KeyManager {
132139
*/
133140
private async validateAndCleanup(): Promise<string[]> {
134141
// Only validate Keychain on macOS
135-
if (!this.useKeychain || !this.store || this.store.keys.length === 0) {
142+
if (
143+
this.storageMethod !== StorageMethod.KEYCHAIN ||
144+
!this.store ||
145+
this.store.keys.length === 0
146+
) {
136147
return [];
137148
}
138149

@@ -623,38 +634,42 @@ export class KeyManager {
623634
};
624635

625636
// Store API key
626-
if (this.useKeychain) {
627-
// macOS: Store in Keychain
628-
try {
629-
await this.execSecurity([
630-
"add-generic-password",
631-
"-a",
632-
id,
633-
"-s",
634-
SERVICE_NAME,
635-
"-w",
636-
apiKey,
637-
"-U",
638-
]);
639-
} catch (error) {
640-
logger.error("Failed to store key in keychain", { error, id });
641-
throw new Error(
642-
`Failed to store key in macOS Keychain: ${error instanceof Error ? error.message : String(error)}`
643-
);
644-
}
645-
} else if (this.useDpapi) {
646-
// Windows: Store encrypted in JSON
647-
try {
648-
metadata.encryptedApiKey = await this.encryptWindows(apiKey);
649-
} catch (error) {
650-
logger.error("Failed to encrypt key with DPAPI", { error, id });
651-
throw new Error(
652-
`Failed to encrypt key with Windows DPAPI: ${error instanceof Error ? error.message : String(error)}`
653-
);
654-
}
655-
} else {
656-
// Linux/Other: Store in JSON
657-
metadata.apiKey = apiKey;
637+
switch (this.storageMethod) {
638+
case StorageMethod.KEYCHAIN:
639+
try {
640+
await this.execSecurity([
641+
"add-generic-password",
642+
"-a",
643+
id,
644+
"-s",
645+
SERVICE_NAME,
646+
"-w",
647+
apiKey,
648+
"-U",
649+
]);
650+
} catch (error) {
651+
logger.error("Failed to store key in keychain", { error, id });
652+
throw new Error(
653+
`Failed to store key in macOS Keychain: ${error instanceof Error ? error.message : String(error)}`
654+
);
655+
}
656+
break;
657+
658+
case StorageMethod.DPAPI:
659+
try {
660+
metadata.encryptedApiKey = await this.encryptWindows(apiKey);
661+
} catch (error) {
662+
logger.error("Failed to encrypt key with DPAPI", { error, id });
663+
throw new Error(
664+
`Failed to encrypt key with Windows DPAPI: ${error instanceof Error ? error.message : String(error)}`
665+
);
666+
}
667+
break;
668+
669+
default:
670+
// Linux/Other: Store in JSON
671+
metadata.apiKey = apiKey;
672+
break;
658673
}
659674

660675
// Store metadata
@@ -736,63 +751,66 @@ export class KeyManager {
736751
}
737752

738753
// Get API key
739-
if (this.useKeychain) {
740-
// macOS: Get from Keychain
741-
try {
742-
const apiKey = await this.execSecurity([
743-
"find-generic-password",
744-
"-a",
745-
keyMeta.id,
746-
"-s",
747-
SERVICE_NAME,
748-
"-w",
749-
]);
754+
switch (this.storageMethod) {
755+
case StorageMethod.KEYCHAIN:
756+
// macOS: Get from Keychain
757+
try {
758+
const apiKey = await this.execSecurity([
759+
"find-generic-password",
760+
"-a",
761+
keyMeta.id,
762+
"-s",
763+
SERVICE_NAME,
764+
"-w",
765+
]);
750766

751-
if (!apiKey) {
752-
logger.error("Key not found in keychain", { id: keyMeta.id });
753-
throw new Error(
754-
`Key not found in macOS Keychain for ID ${keyMeta.id}`
755-
);
756-
}
767+
if (!apiKey) {
768+
logger.error("Key not found in keychain", { id: keyMeta.id });
769+
throw new Error(
770+
`Key not found in macOS Keychain for ID ${keyMeta.id}`
771+
);
772+
}
757773

758-
return apiKey;
759-
} catch (error) {
760-
logger.error("Failed to retrieve key from keychain", {
761-
error,
762-
id: keyMeta.id,
763-
});
764-
throw new Error(
765-
`Failed to retrieve key from macOS Keychain: ${error instanceof Error ? error.message : String(error)}`
766-
);
767-
}
768-
} else if (this.useDpapi) {
769-
// Windows: Decrypt from JSON
770-
if (keyMeta.encryptedApiKey) {
771-
try {
772-
return await this.decryptWindows(keyMeta.encryptedApiKey);
774+
return apiKey;
773775
} catch (error) {
774-
logger.error("Failed to decrypt key with DPAPI", {
776+
logger.error("Failed to retrieve key from keychain", {
775777
error,
776778
id: keyMeta.id,
777779
});
778780
throw new Error(
779-
`Failed to decrypt key with Windows DPAPI: ${error instanceof Error ? error.message : String(error)}`
781+
`Failed to retrieve key from macOS Keychain: ${error instanceof Error ? error.message : String(error)}`
780782
);
781783
}
782-
} else if (keyMeta.apiKey) {
783-
// Fallback for legacy keys stored in plaintext on Windows
784+
785+
case StorageMethod.DPAPI:
786+
// Windows: Decrypt from JSON
787+
if (keyMeta.encryptedApiKey) {
788+
try {
789+
return await this.decryptWindows(keyMeta.encryptedApiKey);
790+
} catch (error) {
791+
logger.error("Failed to decrypt key with DPAPI", {
792+
error,
793+
id: keyMeta.id,
794+
});
795+
throw new Error(
796+
`Failed to decrypt key with Windows DPAPI: ${error instanceof Error ? error.message : String(error)}`
797+
);
798+
}
799+
} else if (keyMeta.apiKey) {
800+
// Fallback for legacy keys stored in plaintext on Windows
801+
return keyMeta.apiKey;
802+
} else {
803+
logger.error("Key not found in metadata", { id: keyMeta.id });
804+
throw new Error(`Key not found in storage for ID ${keyMeta.id}`);
805+
}
806+
807+
default:
808+
// Linux/Other: Get from JSON
809+
if (!keyMeta.apiKey) {
810+
logger.error("Key not found in metadata", { id: keyMeta.id });
811+
throw new Error(`Key not found in storage for ID ${keyMeta.id}`);
812+
}
784813
return keyMeta.apiKey;
785-
} else {
786-
logger.error("Key not found in metadata", { id: keyMeta.id });
787-
throw new Error(`Key not found in storage for ID ${keyMeta.id}`);
788-
}
789-
} else {
790-
// Linux/Other: Get from JSON
791-
if (!keyMeta.apiKey) {
792-
logger.error("Key not found in metadata", { id: keyMeta.id });
793-
throw new Error(`Key not found in storage for ID ${keyMeta.id}`);
794-
}
795-
return keyMeta.apiKey;
796814
}
797815
}
798816

@@ -920,7 +938,7 @@ export class KeyManager {
920938
}
921939

922940
// Delete from storage
923-
if (this.useKeychain) {
941+
if (this.storageMethod === StorageMethod.KEYCHAIN) {
924942
// macOS: Delete from Keychain
925943
let keychainDeleted = false;
926944
try {
@@ -1013,31 +1031,34 @@ export class KeyManager {
10131031
try {
10141032
let storedKey: string;
10151033

1016-
if (this.useKeychain) {
1017-
// macOS: Check Keychain
1018-
storedKey = await this.execSecurity([
1019-
"find-generic-password",
1020-
"-a",
1021-
keyMeta.id,
1022-
"-s",
1023-
SERVICE_NAME,
1024-
"-w",
1025-
]);
1026-
} else if (this.useDpapi) {
1027-
// Windows: Check DPAPI
1028-
if (keyMeta.encryptedApiKey) {
1029-
storedKey = await this.decryptWindows(keyMeta.encryptedApiKey);
1030-
} else if (keyMeta.apiKey) {
1034+
switch (this.storageMethod) {
1035+
case StorageMethod.KEYCHAIN:
1036+
storedKey = await this.execSecurity([
1037+
"find-generic-password",
1038+
"-a",
1039+
keyMeta.id,
1040+
"-s",
1041+
SERVICE_NAME,
1042+
"-w",
1043+
]);
1044+
break;
1045+
1046+
case StorageMethod.DPAPI:
1047+
if (keyMeta.encryptedApiKey) {
1048+
storedKey = await this.decryptWindows(keyMeta.encryptedApiKey);
1049+
} else if (keyMeta.apiKey) {
1050+
storedKey = keyMeta.apiKey;
1051+
} else {
1052+
continue;
1053+
}
1054+
break;
1055+
1056+
default:
1057+
if (!keyMeta.apiKey) {
1058+
continue;
1059+
}
10311060
storedKey = keyMeta.apiKey;
1032-
} else {
1033-
continue;
1034-
}
1035-
} else {
1036-
// Linux/Other: Check JSON
1037-
if (!keyMeta.apiKey) {
1038-
continue;
1039-
}
1040-
storedKey = keyMeta.apiKey;
1061+
break;
10411062
}
10421063

10431064
if (storedKey.trim() === apiKeyValue) {

0 commit comments

Comments
 (0)