Add support for sensitive keychain-stored per-extension settings (#11953)

packages/core/src/mcp/token-storage/keychain-token-storage.test.ts

@@ -386,5 +386,54 @@ describe('KeychainTokenStorage', () => {
         );
       });
     });
+
+    describe('Secrets', () => {
+      it('should set and get a secret', async () => {
+        mockKeytar.setPassword.mockResolvedValue(undefined);
+        mockKeytar.getPassword.mockResolvedValue('secret-value');
+
+        await storage.setSecret('secret-key', 'secret-value');
+        const value = await storage.getSecret('secret-key');
+
+        expect(mockKeytar.setPassword).toHaveBeenCalledWith(
+          mockServiceName,
+          '__secret__secret-key',
+          'secret-value',
+        );
+        expect(mockKeytar.getPassword).toHaveBeenCalledWith(
+          mockServiceName,
+          '__secret__secret-key',
+        );
+        expect(value).toBe('secret-value');
+      });
+
+      it('should delete a secret', async () => {
+        mockKeytar.deletePassword.mockResolvedValue(true);
+        await storage.deleteSecret('secret-key');
+        expect(mockKeytar.deletePassword).toHaveBeenCalledWith(
+          mockServiceName,
+          '__secret__secret-key',
+        );
+      });
+
+      it('should list secrets', async () => {
+        mockKeytar.findCredentials.mockResolvedValue([
+          { account: '__secret__secret1', password: '' },
+          { account: '__secret__secret2', password: '' },
+          { account: 'server1', password: '' },
+        ]);
+        const secrets = await storage.listSecrets();
+        expect(secrets).toEqual(['secret1', 'secret2']);
+      });
+
+      it('should not list secrets in listServers', async () => {
+        mockKeytar.findCredentials.mockResolvedValue([
+          { account: '__secret__secret1', password: '' },
+          { account: 'server1', password: '' },
+        ]);
+        const servers = await storage.listServers();
+        expect(servers).toEqual(['server1']);
+      });
+    });
   });
 });

packages/core/src/mcp/token-storage/keychain-token-storage.ts

@@ -6,7 +6,7 @@
 
 import * as crypto from 'node:crypto';
 import { BaseTokenStorage } from './base-token-storage.js';
-import type { OAuthCredentials } from './types.js';
+import type { OAuthCredentials, SecretStorage } from './types.js';
 import { coreEvents } from '../../utils/events.js';
 
 interface Keytar {
@@ -23,8 +23,12 @@ interface Keytar {
 }
 
 const KEYCHAIN_TEST_PREFIX = '__keychain_test__';
+const SECRET_PREFIX = '__secret__';
 
-export class KeychainTokenStorage extends BaseTokenStorage {
+export class KeychainTokenStorage
+  extends BaseTokenStorage
+  implements SecretStorage
+{
   private keychainAvailable: boolean | null = null;
   private keytarModule: Keytar | null = null;
   private keytarLoadAttempted = false;
@@ -137,7 +141,11 @@ export class KeychainTokenStorage extends BaseTokenStorage {
     try {
       const credentials = await keytar.findCredentials(this.serviceName);
       return credentials
-        .filter((cred) => !cred.account.startsWith(KEYCHAIN_TEST_PREFIX))
+        .filter(
+          (cred) =>
+            !cred.account.startsWith(KEYCHAIN_TEST_PREFIX) &&
+            !cred.account.startsWith(SECRET_PREFIX),
+        )
         .map((cred: { account: string }) => cred.account);
     } catch (error) {
       coreEvents.emitFeedback(
@@ -163,7 +171,11 @@ export class KeychainTokenStorage extends BaseTokenStorage {
     try {
       const credentials = (
         await keytar.findCredentials(this.serviceName)
-      ).filter((c) => !c.account.startsWith(KEYCHAIN_TEST_PREFIX));
+      ).filter(
+        (c) =>
+          !c.account.startsWith(KEYCHAIN_TEST_PREFIX) &&
+          !c.account.startsWith(SECRET_PREFIX),
+      );
 
       for (const cred of credentials) {
         try {
@@ -258,4 +270,62 @@ export class KeychainTokenStorage extends BaseTokenStorage {
   async isAvailable(): Promise<boolean> {
     return this.checkKeychainAvailability();
   }
+
+  async setSecret(key: string, value: string): Promise<void> {
+    if (!(await this.checkKeychainAvailability())) {
+      throw new Error('Keychain is not available');
+    }
+    const keytar = await this.getKeytar();
+    if (!keytar) {
+      throw new Error('Keytar module not available');
+    }
+    await keytar.setPassword(this.serviceName, `${SECRET_PREFIX}${key}`, value);
+  }
+
+  async getSecret(key: string): Promise<string | null> {
+    if (!(await this.checkKeychainAvailability())) {
+      throw new Error('Keychain is not available');
+    }
+    const keytar = await this.getKeytar();
+    if (!keytar) {
+      throw new Error('Keytar module not available');
+    }
+    return keytar.getPassword(this.serviceName, `${SECRET_PREFIX}${key}`);
+  }
+
+  async deleteSecret(key: string): Promise<void> {
+    if (!(await this.checkKeychainAvailability())) {
+      throw new Error('Keychain is not available');
+    }
+    const keytar = await this.getKeytar();
+    if (!keytar) {
+      throw new Error('Keytar module not available');
+    }
+    const deleted = await keytar.deletePassword(
+      this.serviceName,
+      `${SECRET_PREFIX}${key}`,
+    );
+    if (!deleted) {
+      throw new Error(`No secret found for key: ${key}`);
+    }
+  }
+
+  async listSecrets(): Promise<string[]> {
+    if (!(await this.checkKeychainAvailability())) {
+      throw new Error('Keychain is not available');
+    }
+    const keytar = await this.getKeytar();
+    if (!keytar) {
+      throw new Error('Keytar module not available');
+    }
+    try {
+      const credentials = await keytar.findCredentials(this.serviceName);
+      return credentials
+        .filter((cred) => cred.account.startsWith(SECRET_PREFIX))
+        .map((cred) => cred.account.substring(SECRET_PREFIX.length));
+    } catch (error) {
+      console.error('Failed to list secrets from keychain:', error);
+      return [];
+    }
+  }
 }

packages/core/src/mcp/token-storage/types.ts

@@ -36,6 +36,13 @@ export interface TokenStorage {
   clearAll(): Promise<void>;
 }
 
+export interface SecretStorage {
+  setSecret(key: string, value: string): Promise<void>;
+  getSecret(key: string): Promise<string | null>;
+  deleteSecret(key: string): Promise<void>;
+  listSecrets(): Promise<string[]>;
+}
+
 export enum TokenStorageType {
   KEYCHAIN = 'keychain',
   ENCRYPTED_FILE = 'encrypted_file',