Merge pull request #243275 from microsoft/connor4312/mcp-secrets

mcp: securely store mcp inputs
This commit is contained in:
Connor Peet
2025-03-12 10:37:57 -07:00
committed by GitHub
8 changed files with 439 additions and 92 deletions
+2 -1
View File
@@ -81,7 +81,8 @@ const CORE_TYPES = [
'ImportMeta',
// webcrypto has been available since Node.js 19, but still live in dom.d.ts
'Crypto',
'SubtleCrypto'
'SubtleCrypto',
'JsonWebKey',
];
// Types that are defined in a common layer but are known to be only
// available in native environments should not be allowed in browser
+2 -1
View File
@@ -80,7 +80,8 @@ const CORE_TYPES = [
// webcrypto has been available since Node.js 19, but still live in dom.d.ts
'Crypto',
'SubtleCrypto'
'SubtleCrypto',
'JsonWebKey',
];
// Types that are defined in a common layer but are known to be only
@@ -0,0 +1,36 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Emitter } from '../../../../base/common/event.js';
import { ISecretStorageService } from '../../common/secrets.js';
export class TestSecretStorageService implements ISecretStorageService {
declare readonly _serviceBrand: undefined;
private readonly _storage = new Map<string, string>();
private readonly _onDidChangeSecretEmitter = new Emitter<string>();
readonly onDidChangeSecret = this._onDidChangeSecretEmitter.event;
type = 'in-memory' as const;
async get(key: string): Promise<string | undefined> {
return this._storage.get(key);
}
async set(key: string, value: string): Promise<void> {
this._storage.set(key, value);
this._onDidChangeSecretEmitter.fire(key);
}
async delete(key: string): Promise<void> {
this._storage.delete(key);
this._onDidChangeSecretEmitter.fire(key);
}
// Helper method for tests to clear all secrets
clear(): void {
this._storage.clear();
}
}
@@ -3,15 +3,16 @@
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Lazy } from '../../../../base/common/lazy.js';
import { Disposable, IDisposable } from '../../../../base/common/lifecycle.js';
import { IObservable, observableValue } from '../../../../base/common/observable.js';
import { isEmptyObject } from '../../../../base/common/types.js';
import { IInstantiationService } from '../../../../platform/instantiation/common/instantiation.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js';
import { StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js';
import { IConfigurationResolverService } from '../../../services/configurationResolver/common/configurationResolver.js';
import { McpRegistryInputStorage } from './mcpRegistryInputStorage.js';
import { IMcpHostDelegate, IMcpRegistry } from './mcpRegistryTypes.js';
import { McpServerConnection } from './mcpServerConnection.js';
import { McpCollectionDefinition, IMcpServerConnection, McpServerDefinition } from './mcpTypes.js';
import { IMcpServerConnection, McpCollectionDefinition, McpServerDefinition } from './mcpTypes.js';
export class McpRegistry extends Disposable implements IMcpRegistry {
declare public readonly _serviceBrand: undefined;
@@ -21,6 +22,9 @@ export class McpRegistry extends Disposable implements IMcpRegistry {
public readonly collections: IObservable<readonly McpCollectionDefinition[]> = this._collections;
private readonly _workspaceStorage = new Lazy(() => this._register(this._instantiationService.createInstance(McpRegistryInputStorage, StorageScope.WORKSPACE, StorageTarget.USER)));
private readonly _profileStorage = new Lazy(() => this._register(this._instantiationService.createInstance(McpRegistryInputStorage, StorageScope.PROFILE, StorageTarget.USER)));
public get delegates(): readonly IMcpHostDelegate[] {
return this._delegates;
}
@@ -28,7 +32,6 @@ export class McpRegistry extends Disposable implements IMcpRegistry {
constructor(
@IInstantiationService private readonly _instantiationService: IInstantiationService,
@IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService,
@IStorageService private readonly _storageService: IStorageService,
) {
super();
}
@@ -57,16 +60,9 @@ export class McpRegistry extends Disposable implements IMcpRegistry {
};
}
public hasSavedInputs(collection: McpCollectionDefinition, definition: McpServerDefinition): boolean {
const stored = this.getInputStorageData(collection, definition);
return !!stored && !!stored.map && !isEmptyObject(stored.map);
}
public clearSavedInputs(collection: McpCollectionDefinition, definition: McpServerDefinition) {
const stored = this.getInputStorageData(collection, definition);
if (stored) {
this._storageService.remove(stored.key, stored.scope);
}
public clearSavedInputs() {
this._profileStorage.value.clearAll();
this._workspaceStorage.value.clearAll();
}
public async resolveConnection(
@@ -80,17 +76,21 @@ export class McpRegistry extends Disposable implements IMcpRegistry {
let launch = definition.launch;
const storage = this.getInputStorageData(collection, definition);
if (definition.variableReplacement && storage) {
if (definition.variableReplacement) {
const inputStorage = definition.variableReplacement.folder ? this._workspaceStorage.value : this._profileStorage.value;
const previouslyStored = await inputStorage.getMap();
const { folder, section, target } = definition.variableReplacement;
// based on _configurationResolverService.resolveWithInteractionReplace
launch = await this._configurationResolverService.resolveAnyAsync(folder, launch);
const newVariables = await this._configurationResolverService.resolveWithInteraction(folder, launch, section, storage.map, target);
const newVariables = await this._configurationResolverService.resolveWithInteraction(folder, launch, section, previouslyStored, target);
if (newVariables?.size) {
launch = await this._configurationResolverService.resolveAnyAsync(folder, launch, Object.fromEntries(newVariables));
this._storageService.store(storage.key, JSON.stringify(Object.fromEntries(newVariables)), storage.scope, StorageTarget.MACHINE);
const completeVariables = { ...previouslyStored, ...Object.fromEntries(newVariables) };
launch = await this._configurationResolverService.resolveAnyAsync(folder, launch, completeVariables);
await inputStorage.setSecrets(completeVariables);
}
}
@@ -102,23 +102,5 @@ export class McpRegistry extends Disposable implements IMcpRegistry {
launch,
);
}
private getInputStorageData(collection: McpCollectionDefinition, definition: McpServerDefinition) {
if (!definition.variableReplacement) {
return undefined;
}
const key = `mcpConfig.${collection.id}.${definition.id}`;
const scope = definition.variableReplacement.folder ? StorageScope.WORKSPACE : StorageScope.APPLICATION;
let map: Record<string, string> | undefined;
try {
map = this._storageService.getObject(key, scope);
} catch {
// ignord
}
return { key, scope, map };
}
}
@@ -0,0 +1,192 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import { Sequencer } from '../../../../base/common/async.js';
import { decodeBase64, encodeBase64, VSBuffer } from '../../../../base/common/buffer.js';
import { Lazy } from '../../../../base/common/lazy.js';
import { Disposable } from '../../../../base/common/lifecycle.js';
import { isEmptyObject } from '../../../../base/common/types.js';
import { ILogService } from '../../../../platform/log/common/log.js';
import { ISecretStorageService } from '../../../../platform/secrets/common/secrets.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../platform/storage/common/storage.js';
const MCP_ENCRYPTION_KEY_NAME = 'mcpEncryptionKey';
const MCP_ENCRYPTION_KEY_ALGORITHM = 'AES-GCM';
const MCP_ENCRYPTION_KEY_LEN = 256;
const MCP_ENCRYPTION_IV_LENGTH = 12; // 96 bits
const MCP_DATA_STORED_VERSION = 1;
const MCP_DATA_STORED_KEY = 'mcpInputs';
interface IStoredData {
version: number;
values: Record<string, string>;
secrets?: { value: string; iv: string }; // base64, encrypted
}
interface IHydratedData extends IStoredData {
unsealedSecrets?: Record<string, string>;
}
export class McpRegistryInputStorage extends Disposable {
private static secretSequencer = new Sequencer();
private readonly _secretsSealerSequencer = new Sequencer();
private readonly _getEncryptionKey = new Lazy(() => {
return McpRegistryInputStorage.secretSequencer.queue(async () => {
const existing = await this._secretStorageService.get(MCP_ENCRYPTION_KEY_NAME);
if (existing) {
try {
const parsed: JsonWebKey = JSON.parse(existing);
return await crypto.subtle.importKey('jwk', parsed, MCP_ENCRYPTION_KEY_ALGORITHM, false, ['encrypt', 'decrypt']);
} catch {
// fall through
}
}
const key = await crypto.subtle.generateKey(
{ name: MCP_ENCRYPTION_KEY_ALGORITHM, length: MCP_ENCRYPTION_KEY_LEN },
true,
['encrypt', 'decrypt'],
);
const exported = await crypto.subtle.exportKey('jwk', key);
await this._secretStorageService.set(MCP_ENCRYPTION_KEY_NAME, JSON.stringify(exported));
return key;
});
});
private _didChange = false;
private _record = new Lazy<IHydratedData>(() => {
const stored = this._storageService.getObject<IStoredData>(MCP_DATA_STORED_KEY, this._scope);
return stored?.version === MCP_DATA_STORED_VERSION ? { ...stored } : { version: MCP_DATA_STORED_VERSION, values: {} };
});
constructor(
private readonly _scope: StorageScope,
_target: StorageTarget,
@IStorageService private readonly _storageService: IStorageService,
@ISecretStorageService private readonly _secretStorageService: ISecretStorageService,
@ILogService private readonly _logService: ILogService,
) {
super();
this._register(_storageService.onWillSaveState(() => {
if (this._didChange) {
this._storageService.store(MCP_DATA_STORED_KEY, {
version: MCP_DATA_STORED_VERSION,
values: this._record.value.values,
secrets: this._record.value.secrets,
} satisfies IStoredData, this._scope, _target);
this._didChange = false;
}
}));
}
/** Deletes all collection data from storage. */
public clearAll() {
this._record.value.values = {};
this._record.value.secrets = undefined;
this._record.value.unsealedSecrets = undefined;
this._didChange = true;
}
/** Delete a single collection data from the storage. */
public async clear(inputKey: string) {
const secrets = await this._unsealSecrets();
delete this._record.value.values[inputKey];
this._didChange = true;
if (secrets.hasOwnProperty(inputKey)) {
delete secrets[inputKey];
await this._sealSecrets();
}
}
/** Gets a mapping of saved input data. */
public async getMap() {
const secrets = await this._unsealSecrets();
return { ...this._record.value.values, ...secrets };
}
/** Updates the input data mapping. */
public async setPlainText(values: Record<string, string>) {
Object.assign(this._record.value.values, values);
this._didChange = true;
}
/** Updates the input secrets mapping. */
public async setSecrets(values: Record<string, string>) {
const unsealed = await this._unsealSecrets();
Object.assign(unsealed, values);
await this._sealSecrets();
}
private async _sealSecrets() {
return this._secretsSealerSequencer.queue(async () => {
if (!this._record.value.unsealedSecrets || isEmptyObject(this._record.value.unsealedSecrets)) {
this._record.value.secrets = undefined;
return;
}
if (!this._record.value.secrets) {
const iv = crypto.getRandomValues(new Uint8Array(MCP_ENCRYPTION_IV_LENGTH));
this._record.value.secrets = {
value: '',
iv: encodeBase64(VSBuffer.wrap(iv)),
};
}
const toSeal = JSON.stringify(this._record.value.unsealedSecrets);
const iv = decodeBase64(this._record.value.secrets.iv);
const key = await this._getEncryptionKey.value;
const encrypted = await crypto.subtle.encrypt(
{ name: MCP_ENCRYPTION_KEY_ALGORITHM, iv: iv.buffer },
key,
new TextEncoder().encode(toSeal).buffer,
);
const enc = encodeBase64(VSBuffer.wrap(new Uint8Array(encrypted)));
if (this._record.value.secrets.value === enc) {
return;
}
this._record.value.secrets.value = enc;
this._didChange = true;
});
}
private async _unsealSecrets(): Promise<Record<string, string>> {
if (!this._record.value.secrets) {
return this._record.value.unsealedSecrets ??= {};
}
if (this._record.value.unsealedSecrets) {
return this._record.value.unsealedSecrets;
}
try {
const key = await this._getEncryptionKey.value;
const iv = decodeBase64(this._record.value.secrets.iv);
const encrypted = decodeBase64(this._record.value.secrets.value);
const decrypted = await crypto.subtle.decrypt(
{ name: MCP_ENCRYPTION_KEY_ALGORITHM, iv: iv.buffer },
key,
encrypted.buffer,
);
const unsealedSecrets = JSON.parse(new TextDecoder().decode(decrypted));
this._record.value.unsealedSecrets = unsealedSecrets;
return unsealedSecrets;
} catch (e) {
this._logService.warn('Error unsealing MCP secrets', e);
this._record.value.secrets = undefined;
}
return {};
}
}
@@ -35,8 +35,6 @@ export interface IMcpRegistry {
registerDelegate(delegate: IMcpHostDelegate): IDisposable;
registerCollection(collection: McpCollectionDefinition): IDisposable;
/** Gets whether there are saved inputs used to resolve the connection */
hasSavedInputs(collection: McpCollectionDefinition, definition: McpServerDefinition): boolean;
/** Resets any saved inputs for the connection. */
clearSavedInputs(collection: McpCollectionDefinition, definition: McpServerDefinition): void;
/** Createse a connection for the collection and definition. */
@@ -13,7 +13,9 @@ import { ConfigurationTarget } from '../../../../../platform/configuration/commo
import { ServiceCollection } from '../../../../../platform/instantiation/common/serviceCollection.js';
import { TestInstantiationService } from '../../../../../platform/instantiation/test/common/instantiationServiceMock.js';
import { ILoggerService } from '../../../../../platform/log/common/log.js';
import { IStorageService, StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
import { ISecretStorageService } from '../../../../../platform/secrets/common/secrets.js';
import { TestSecretStorageService } from '../../../../../platform/secrets/test/common/testSecretStorageService.js';
import { IStorageService, StorageScope } from '../../../../../platform/storage/common/storage.js';
import { IConfigurationResolverService } from '../../../../services/configurationResolver/common/configurationResolver.js';
import { IOutputService } from '../../../../services/output/common/output.js';
import { TestLoggerService, TestStorageService } from '../../../../test/common/workbenchTestServices.js';
@@ -131,6 +133,7 @@ suite('Workbench - MCP - Registry', () => {
const services = new ServiceCollection(
[IConfigurationResolverService, testConfigResolverService],
[IStorageService, testStorageService],
[ISecretStorageService, new TestSecretStorageService()],
[ILoggerService, store.add(new TestLoggerService())],
[IOutputService, upcast({ showChannel: () => { } })],
);
@@ -185,28 +188,7 @@ suite('Workbench - MCP - Registry', () => {
assert.strictEqual(registry.delegates.length, 0);
});
test('hasSavedInputs returns false when no inputs are saved', () => {
assert.strictEqual(registry.hasSavedInputs(testCollection, baseDefinition), false);
});
test('clearSavedInputs removes stored inputs', () => {
const definition: McpServerDefinition = {
...baseDefinition,
variableReplacement: {
section: 'mcp'
}
};
// Save some mock inputs
const key = `mcpConfig.${testCollection.id}.${definition.id}`;
testStorageService.store(key, JSON.stringify({ 'input:foo': 'bar' }), StorageScope.APPLICATION, StorageTarget.MACHINE);
assert.strictEqual(registry.hasSavedInputs(testCollection, definition), true);
registry.clearSavedInputs(testCollection, definition);
assert.strictEqual(registry.hasSavedInputs(testCollection, definition), false);
});
test('resolveConnection creates connection with resolved variables and memorizes them', async () => {
test('resolveConnection creates connection with resolved variables and memorizes them until cleared', async () => {
const definition: McpServerDefinition = {
...baseDefinition,
launch: {
@@ -241,36 +223,13 @@ suite('Workbench - MCP - Registry', () => {
assert.ok(connection2);
assert.strictEqual((connection2.launchDefinition as any).env.PATH, 'interactiveValue0');
connection2.dispose();
});
test('resolveConnection with stored variables resolves them', async () => {
const definition: McpServerDefinition = {
...baseDefinition,
launch: {
type: McpServerTransportType.Stdio,
command: '${storedVar}',
args: [],
env: {},
cwd: URI.parse('file:///test')
},
variableReplacement: {
section: 'mcp'
}
};
registry.clearSavedInputs();
// Save some mock inputs
const key = `mcpConfig.${testCollection.id}.${definition.id}`;
testStorageService.store(key, { 'storedVar': 'resolved-value' }, StorageScope.APPLICATION, StorageTarget.MACHINE);
const connection3 = await registry.resolveConnection(testCollection, definition) as McpServerConnection;
// Register a delegate that can handle the connection
const delegate = new TestMcpHostDelegate();
const disposable = registry.registerDelegate(delegate);
store.add(disposable);
const connection = await registry.resolveConnection(testCollection, definition) as McpServerConnection;
assert.ok(connection);
assert.strictEqual((connection.launchDefinition as any).command, 'resolved-value');
connection.dispose();
assert.ok(connection3);
assert.strictEqual((connection3.launchDefinition as any).env.PATH, 'interactiveValue4');
connection3.dispose();
});
});
@@ -0,0 +1,178 @@
/*---------------------------------------------------------------------------------------------
* Copyright (c) Microsoft Corporation. All rights reserved.
* Licensed under the MIT License. See License.txt in the project root for license information.
*--------------------------------------------------------------------------------------------*/
import * as assert from 'assert';
import { ensureNoDisposablesAreLeakedInTestSuite } from '../../../../../base/test/common/utils.js';
import { ILogService, NullLogService } from '../../../../../platform/log/common/log.js';
import { TestSecretStorageService } from '../../../../../platform/secrets/test/common/testSecretStorageService.js';
import { StorageScope, StorageTarget } from '../../../../../platform/storage/common/storage.js';
import { TestStorageService } from '../../../../test/common/workbenchTestServices.js';
import { McpRegistryInputStorage } from '../../common/mcpRegistryInputStorage.js';
suite('Workbench - MCP - RegistryInputStorage', () => {
const store = ensureNoDisposablesAreLeakedInTestSuite();
let testStorageService: TestStorageService;
let testSecretStorageService: TestSecretStorageService;
let testLogService: ILogService;
let mcpInputStorage: McpRegistryInputStorage;
setup(() => {
testStorageService = store.add(new TestStorageService());
testSecretStorageService = new TestSecretStorageService();
testLogService = store.add(new NullLogService());
// Create the input storage with APPLICATION scope
mcpInputStorage = store.add(new McpRegistryInputStorage(
StorageScope.APPLICATION,
StorageTarget.MACHINE,
testStorageService,
testSecretStorageService,
testLogService
));
});
test('setPlainText stores values that can be retrieved with getMap', async () => {
const values = {
'key1': 'value1',
'key2': 'value2'
};
await mcpInputStorage.setPlainText(values);
const result = await mcpInputStorage.getMap();
assert.strictEqual(result.key1, 'value1');
assert.strictEqual(result.key2, 'value2');
});
test('setSecrets stores encrypted values that can be retrieved with getMap', async () => {
const secrets = {
'secretKey1': 'secretValue1',
'secretKey2': 'secretValue2'
};
await mcpInputStorage.setSecrets(secrets);
const result = await mcpInputStorage.getMap();
assert.strictEqual(result.secretKey1, 'secretValue1');
assert.strictEqual(result.secretKey2, 'secretValue2');
});
test('getMap returns combined plain text and secret values', async () => {
await mcpInputStorage.setPlainText({
'plainKey': 'plainValue'
});
await mcpInputStorage.setSecrets({
'secretKey': 'secretValue'
});
const result = await mcpInputStorage.getMap();
assert.strictEqual(result.plainKey, 'plainValue');
assert.strictEqual(result.secretKey, 'secretValue');
});
test('clear removes specific values', async () => {
await mcpInputStorage.setPlainText({
'key1': 'value1',
'key2': 'value2'
});
await mcpInputStorage.setSecrets({
'secretKey1': 'secretValue1',
'secretKey2': 'secretValue2'
});
// Clear one plain and one secret value
await mcpInputStorage.clear('key1');
await mcpInputStorage.clear('secretKey1');
const result = await mcpInputStorage.getMap();
assert.strictEqual(result.key1, undefined);
assert.strictEqual(result.key2, 'value2');
assert.strictEqual(result.secretKey1, undefined);
assert.strictEqual(result.secretKey2, 'secretValue2');
});
test('clearAll removes all values', async () => {
await mcpInputStorage.setPlainText({
'key1': 'value1'
});
await mcpInputStorage.setSecrets({
'secretKey1': 'secretValue1'
});
mcpInputStorage.clearAll();
const result = await mcpInputStorage.getMap();
assert.deepStrictEqual(result, {});
});
test('updates to plain text values overwrite existing values', async () => {
await mcpInputStorage.setPlainText({
'key1': 'value1',
'key2': 'value2'
});
await mcpInputStorage.setPlainText({
'key1': 'updatedValue1'
});
const result = await mcpInputStorage.getMap();
assert.strictEqual(result.key1, 'updatedValue1');
assert.strictEqual(result.key2, 'value2');
});
test('updates to secret values overwrite existing values', async () => {
await mcpInputStorage.setSecrets({
'secretKey1': 'secretValue1',
'secretKey2': 'secretValue2'
});
await mcpInputStorage.setSecrets({
'secretKey1': 'updatedSecretValue1'
});
const result = await mcpInputStorage.getMap();
assert.strictEqual(result.secretKey1, 'updatedSecretValue1');
assert.strictEqual(result.secretKey2, 'secretValue2');
});
test('storage persists values across instances', async () => {
// Set values on first instance
await mcpInputStorage.setPlainText({
'key1': 'value1'
});
await mcpInputStorage.setSecrets({
'secretKey1': 'secretValue1'
});
await testStorageService.flush();
// Create a second instance that should have access to the same storage
const secondInstance = store.add(new McpRegistryInputStorage(
StorageScope.APPLICATION,
StorageTarget.MACHINE,
testStorageService,
testSecretStorageService,
testLogService
));
const result = await secondInstance.getMap();
assert.strictEqual(result.key1, 'value1');
assert.strictEqual(result.secretKey1, 'secretValue1');
assert.ok(!testStorageService.get('mcpInputs', StorageScope.APPLICATION)?.includes('secretValue1'));
});
});