Skip to content
Draft
Show file tree
Hide file tree
Changes from 1 commit
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
15 changes: 15 additions & 0 deletions src/vs/platform/mcp/common/allowedMcpServers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { equals } from '../../../base/common/arrays.js';
import { escapeRegExpCharacters } from '../../../base/common/strings.js';
import { isObject, isString } from '../../../base/common/types.js';
import { IMcpServerConfiguration, McpServerType } from './mcpPlatformTypes.js';

/**
* A single entry in the `chat.mcp.allowedServers` allowlist. Identifies an MCP server by exactly
Expand Down Expand Up @@ -70,6 +71,20 @@ function isValidMatcher(entry: unknown): entry is IMcpServerMatcher {
return (hasName ? 1 : 0) + (hasUrl ? 1 : 0) + (hasCommand ? 1 : 0) === 1;
}

/**
* Reduces a declarative MCP server configuration to the identity used for allow/deny matching:
* `url` for remote servers, the full `[command, ...args]` invocation for local stdio servers.
*
* Shared by every enforcement path so that a server blocked when it is installed or started
* locally is blocked identically when it is forwarded to an agent host.
*/
export function mcpServerIdentityFromConfiguration(name: string, configuration: IMcpServerConfiguration): IMcpServerIdentity {
if (configuration.type === McpServerType.REMOTE) {
return { name, url: configuration.url };
}
return { name, command: [configuration.command, ...(configuration.args ?? [])] };
}

/**
* Whether the server identity matches at least one of the given matchers. A `undefined` or empty
* matcher list matches nothing.
Expand Down
9 changes: 2 additions & 7 deletions src/vs/platform/mcp/common/allowedMcpServersService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,8 @@ import { createCommandUri, IMarkdownString, MarkdownString } from '../../../base
import { IConfigurationService } from '../../configuration/common/configuration.js';
import { Emitter } from '../../../base/common/event.js';
import { hasKey } from '../../../base/common/types.js';
import { checkMcpServerAllowed, getMcpServerMatchers, IMcpServerIdentity, IMcpServerMatcher, McpServerAllowResult } from './allowedMcpServers.js';
import { checkMcpServerAllowed, getMcpServerMatchers, IMcpServerIdentity, IMcpServerMatcher, mcpServerIdentityFromConfiguration, McpServerAllowResult } from './allowedMcpServers.js';
import { IAllowedMcpServersService, IGalleryMcpServer, IInstallableMcpServer, ILocalMcpServer, mcpAccessConfig, mcpAllowedServersConfig, mcpDeniedServersConfig, McpAccessValue } from './mcpManagement.js';
import { McpServerType } from './mcpPlatformTypes.js';
import { COPILOT_ALLOW_MANAGED_MCP_SERVERS_ONLY_CONFIG } from '../../policy/common/copilotManagedSettings.js';

export class AllowedMcpServersService extends Disposable implements IAllowedMcpServersService {
Expand Down Expand Up @@ -75,11 +74,7 @@ export class AllowedMcpServersService extends Disposable implements IAllowedMcpS

private toIdentity(mcpServer: IGalleryMcpServer | ILocalMcpServer | IInstallableMcpServer): IMcpServerIdentity {
if (hasKey(mcpServer, { config: true })) {
const config = mcpServer.config;
if (config.type === McpServerType.REMOTE) {
return { name: mcpServer.name, url: config.url };
}
return { name: mcpServer.name, command: [config.command, ...(config.args ?? [])] };
return mcpServerIdentityFromConfiguration(mcpServer.name, mcpServer.config);
}

// Gallery server: match by name or a remote URL; the local command invocation is only
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ import { IAgentPluginService } from '../../../common/plugins/agentPluginService.
import { IPromptsService } from '../../../common/promptSyntax/service/promptsService.js';
import { ILanguageModelToolsService, IToolData, IToolSet } from '../../../common/tools/languageModelToolsService.js';
import { IMcpService } from '../../../../mcp/common/mcpTypes.js';
import { IAllowedMcpServersService } from '../../../../../../platform/mcp/common/mcpManagement.js';
import { IConfigurationResolverService } from '../../../../../services/configurationResolver/common/configurationResolver.js';
import { AgentCustomizationSyncProvider } from './agentCustomizationSyncProvider.js';
import { type ILocalCustomizationSyncOptions, resolveCustomizationRefs, resolveLocalCustomAgents } from './agentHostLocalCustomizations.js';
Expand Down Expand Up @@ -119,6 +120,7 @@ class AgentCustomizationScope extends Disposable {
@IInstantiationService instantiationService: IInstantiationService,
@IMcpService private readonly _mcpService: IMcpService,
@IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService,
@IAllowedMcpServersService private readonly _allowedMcpServersService: IAllowedMcpServersService,
) {
super();
this._bundler = this._register(instantiationService.createInstance(SyncedCustomizationBundler, createScopeAuthority(_sessionType, scopeKey)));
Expand All @@ -136,6 +138,7 @@ class AgentCustomizationScope extends Disposable {
this._agentPluginService,
this._mcpService,
this._configurationResolverService,
this._allowedMcpServersService,
this._bundler,
this._sessionType,
this._options,
Expand Down Expand Up @@ -198,6 +201,9 @@ class AgentCustomizationScope extends Disposable {
}
scheduleUpdate();
}));
// Republish when the enterprise MCP allow/deny policy changes so a newly blocked server is
// withdrawn from the forwarded bundle rather than lingering for the life of the session.
Comment thread
joshspicer marked this conversation as resolved.
Outdated
this._register(this._allowedMcpServersService.onDidChangeAllowedMcpServers(() => scheduleUpdate()));
}

acquire(): IAgentCustomizationScope {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { IPromptsService, isUserToggleableCustomization, matchesSessionType, Pro
import { type ICustomizationSyncProvider } from '../../../common/customizationHarnessService.js';
import { IAgentPlugin, IAgentPluginService } from '../../../common/plugins/agentPluginService.js';
import { IMcpService } from '../../../../mcp/common/mcpTypes.js';
import { IAllowedMcpServersService } from '../../../../../../platform/mcp/common/mcpManagement.js';
import { IConfigurationResolverService } from '../../../../../services/configurationResolver/common/configurationResolver.js';
import type { ISyncableFile, ISyncableMcpServer, SyncedCustomizationBundler } from './syncedCustomizationBundler.js';
import { IFileService } from '../../../../../../platform/files/common/files.js';
Expand Down Expand Up @@ -200,11 +201,12 @@ export async function resolveLocalCustomAgents(
* exception is `.vscode/mcp.json`, which the agent host does not discover
* (despite what the SDK's `enableConfigDiscovery` docs imply) — those are
* synced, but only when their config can be resolved without requiring user
* interaction. For agent-host providers with their own GitHub MCP server, the
* interaction. Servers blocked by the enterprise MCP allow/deny policy are never
* forwarded. For agent-host providers with their own GitHub MCP server, the
* Copilot Chat extension's duplicate provider is excluded.
*/
export async function collectNonPluginMcpServers(mcpService: IMcpService, configurationResolverService: IConfigurationResolverService, sessionType: string, workingDirectories: readonly URI[]): Promise<ISyncableMcpServer[]> {
const resolved = await resolveMcpServersForAgentHostDelivery(mcpService.servers.get(), configurationResolverService, sessionType, workingDirectories);
export async function collectNonPluginMcpServers(mcpService: IMcpService, configurationResolverService: IConfigurationResolverService, allowedMcpServersService: IAllowedMcpServersService, sessionType: string, workingDirectories: readonly URI[]): Promise<ISyncableMcpServer[]> {
const resolved = await resolveMcpServersForAgentHostDelivery(mcpService.servers.get(), configurationResolverService, allowedMcpServersService, sessionType, workingDirectories);
return resolved.flatMap(({ server, definition, delivery, projectedConfiguration }) => {
if (delivery !== AgentHostMcpServerDelivery.ClientForwarded || !definition || !projectedConfiguration) {
return [];
Expand Down Expand Up @@ -237,6 +239,7 @@ export async function resolveCustomizationRefs(
agentPluginService: IAgentPluginService,
mcpService: IMcpService,
configurationResolverService: IConfigurationResolverService,
allowedMcpServersService: IAllowedMcpServersService,
bundler: SyncedCustomizationBundler,
sessionType: string,
options: ILocalCustomizationSyncOptions | undefined,
Expand Down Expand Up @@ -311,7 +314,7 @@ export async function resolveCustomizationRefs(
}

const refs: Promise<ClientPluginCustomization | undefined>[] = [...pluginRefs.values()];
const mcpServers = await collectNonPluginMcpServers(mcpService, configurationResolverService, sessionType, workingDirectories);
const mcpServers = await collectNonPluginMcpServers(mcpService, configurationResolverService, allowedMcpServersService, sessionType, workingDirectories);
if (looseFiles.length > 0 || mcpServers.length > 0) {
refs.push(bundler.bundle(looseFiles, mcpServers).then(r => r?.ref));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ import { URI } from '../../../../../../base/common/uri.js';
import { Location } from '../../../../../../editor/common/languages.js';
import { ConfigurationTarget } from '../../../../../../platform/configuration/common/configuration.js';
import { ExtensionIdentifier } from '../../../../../../platform/extensions/common/extensions.js';
import { mcpServerIdentityFromConfiguration } from '../../../../../../platform/mcp/common/allowedMcpServers.js';
import { IAllowedMcpServersService } from '../../../../../../platform/mcp/common/mcpManagement.js';
import { IMcpSandboxConfiguration, IMcpServerConfiguration, McpServerType } from '../../../../../../platform/mcp/common/mcpPlatformTypes.js';
import { IWorkspaceFolderData } from '../../../../../../platform/workspace/common/workspace.js';
import { AICustomizationSource, AICustomizationSources } from '../../../common/aiCustomizationWorkspaceService.js';
Expand Down Expand Up @@ -76,6 +78,7 @@ export const enum AgentHostMcpSupportReason {
OAuthClientConfigurationIgnored = 'oauthClientConfigurationIgnored',
DefinitionNotLoaded = 'definitionNotLoaded',
SourceUnknown = 'sourceUnknown',
BlockedByPolicy = 'blockedByPolicy',
}

export type AgentHostMcpServerCompatibility =
Expand Down Expand Up @@ -156,6 +159,7 @@ export function agentHostProviderHasBuiltInGitHubMcpServer(provider: string): bo
export async function assessMcpServersForCopilotAgentHost(
servers: readonly IMcpServer[],
configurationResolverService: IConfigurationResolverService,
allowedMcpServersService: IAllowedMcpServersService,
sessionType: string,
workingDirectories: readonly URI[] | undefined,
lazyCollectionState: LazyCollectionState,
Expand All @@ -164,7 +168,7 @@ export async function assessMcpServersForCopilotAgentHost(
return undefined;
}

const resolved = await resolveMcpServersForAgentHostDelivery(servers, configurationResolverService, sessionType, workingDirectories);
const resolved = await resolveMcpServersForAgentHostDelivery(servers, configurationResolverService, allowedMcpServersService, sessionType, workingDirectories);
return {
servers: resolved.map(({ server, source, applicability, delivery, compatibility }) => ({
id: server.definition.id,
Expand Down Expand Up @@ -212,15 +216,17 @@ export async function mergeInstalledMcpServersIntoAgentHostSupportAssessment(
export function resolveMcpServersForAgentHostDelivery(
servers: readonly IMcpServer[],
configurationResolverService: IConfigurationResolverService,
allowedMcpServersService: IAllowedMcpServersService,
sessionType: string,
workingDirectories: readonly URI[] | undefined,
): Promise<readonly IAgentHostMcpServerDeliveryResolution[]> {
return Promise.all(servers.map(server => resolveMcpServerForAgentHostDelivery(server, configurationResolverService, sessionType, workingDirectories)));
return Promise.all(servers.map(server => resolveMcpServerForAgentHostDelivery(server, configurationResolverService, allowedMcpServersService, sessionType, workingDirectories)));
}

async function resolveMcpServerForAgentHostDelivery(
server: IMcpServer,
configurationResolverService: IConfigurationResolverService,
allowedMcpServersService: IAllowedMcpServersService,
sessionType: string,
workingDirectories: readonly URI[] | undefined,
): Promise<IAgentHostMcpServerDeliveryResolution> {
Expand Down Expand Up @@ -297,6 +303,22 @@ async function resolveMcpServerForAgentHostDelivery(
? AgentHostMcpServerDelivery.ClientForwarded
: deliveryForInapplicable(applicability);

// Enterprise allow/deny policy is enforced here, on the *resolved* configuration that would be
// forwarded, so that a server blocked for the local agent (in `McpServer`) is blocked identically
// for a delegated agent-host session. Forwarding hands the server to a separate process that
// launches it itself, so it is the only point at which the client can still refuse.
Comment thread
joshspicer marked this conversation as resolved.
Outdated
if (isBlockedByMcpPolicy(allowedMcpServersService, server.definition.label, projectedConfiguration)) {
Comment thread
joshspicer marked this conversation as resolved.
Outdated
return {
server,
definition,
source,
applicability,
delivery: AgentHostMcpServerDelivery.NotDelivered,
compatibility: unsupported([AgentHostMcpSupportReason.BlockedByPolicy]),
projectedConfiguration: undefined,
};
}

return {
server,
definition,
Expand All @@ -310,6 +332,17 @@ async function resolveMcpServerForAgentHostDelivery(
};
}

/**
* Whether the enterprise `chat.mcp.allowedServers` / `chat.mcp.deniedServers` policy blocks
* forwarding this server. The check runs against the configuration as it would be forwarded, so a
* configuration whose URL or command still carries unresolved `${...}` variables cannot match an
* allow entry and is therefore blocked whenever an allowlist is configured — failing closed rather
* than handing an unverifiable server to the agent host.
*/
function isBlockedByMcpPolicy(allowedMcpServersService: IAllowedMcpServersService, name: string, configuration: IMcpServerConfiguration): boolean {
return allowedMcpServersService.isServerAllowed(mcpServerIdentityFromConfiguration(name, configuration)) !== true;
Comment thread
joshspicer marked this conversation as resolved.
Outdated
}

function createResolution(
server: IMcpServer,
definition: McpServerDefinition | undefined,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import { Disposable, IDisposable } from '../../../../../../base/common/lifecycle
import { autorun, IObservable, observableValue, transaction } from '../../../../../../base/common/observable.js';
import { URI } from '../../../../../../base/common/uri.js';
import { IConfigurationService } from '../../../../../../platform/configuration/common/configuration.js';
import { mcpAccessConfig, McpAccessValue } from '../../../../../../platform/mcp/common/mcpManagement.js';
import { IAllowedMcpServersService, mcpAccessConfig, McpAccessValue } from '../../../../../../platform/mcp/common/mcpManagement.js';
import { COPILOT_STRICT_PLUGIN_ONLY_CUSTOMIZATION_CONFIG } from '../../../../../../platform/policy/common/copilotManagedSettings.js';
import { isStrictPluginOnlyCustomizationEnabled, StrictPluginOnlyCustomization } from '../../../common/customizationLockdown.js';
import { IMcpService, IMcpWorkbenchService } from '../../../../mcp/common/mcpTypes.js';
Expand Down Expand Up @@ -53,6 +53,7 @@ export class AgentHostMcpServerSupportScope extends Disposable {
@IMcpWorkbenchService private readonly _mcpWorkbenchService: IMcpWorkbenchService,
@IConfigurationResolverService private readonly _configurationResolverService: IConfigurationResolverService,
@IConfigurationService private readonly _configurationService: IConfigurationService,
@IAllowedMcpServersService private readonly _allowedMcpServersService: IAllowedMcpServersService,
) {
super();
this._updateDelayer = this._register(new Delayer<void>(MCP_SUPPORT_UPDATE_DEBOUNCE_DELAY));
Expand All @@ -64,6 +65,7 @@ export class AgentHostMcpServerSupportScope extends Disposable {
const initialAssessment = await assessMcpServersForCopilotAgentHost(
this._mcpService.servers.get(),
this._configurationResolverService,
this._allowedMcpServersService,
this._sessionType,
this._roots,
lazyState.state,
Expand Down Expand Up @@ -129,6 +131,7 @@ export class AgentHostMcpServerSupportScope extends Disposable {
scheduleUpdate();
}));
this._register(this._mcpWorkbenchService.onChange(scheduleUpdate));
this._register(this._allowedMcpServersService.onDidChangeAllowedMcpServers(scheduleUpdate));
this._register(this._configurationService.onDidChangeConfiguration(event => {
if (event.affectsConfiguration(mcpAccessConfig) || event.affectsConfiguration(COPILOT_STRICT_PLUGIN_ONLY_CUSTOMIZATION_CONFIG)) {
scheduleUpdate();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ import { MockLabelService } from '../../../../../services/label/test/common/mock
import { IAgentHostFileSystemService } from '../../../../../services/agentHost/common/agentHostFileSystemService.js';
import { IAgentHostImportConversationStore } from '../../../browser/agentSessions/agentHost/agentHostImportConversationStore.js';
import { IStorageService, InMemoryStorageService } from '../../../../../../platform/storage/common/storage.js';
import { mcpAccessConfig, McpAccessValue } from '../../../../../../platform/mcp/common/mcpManagement.js';
import { IAllowedMcpServersService, mcpAccessConfig, McpAccessValue } from '../../../../../../platform/mcp/common/mcpManagement.js';
import { IWorkbenchAssignmentService } from '../../../../../services/assignment/common/assignmentService.js';
import { NullWorkbenchAssignmentService } from '../../../../../services/assignment/test/common/nullAssignmentService.js';
import { IAgentSubscription } from '../../../../../../platform/agentHost/common/state/agentSubscription.js';
Expand Down Expand Up @@ -128,6 +128,10 @@ suite('AgentHostClientTools', () => {
servers: observableValue('mcpServers', []),
lazyCollectionState: observableValue('mcpLazyCollectionState', { state: LazyCollectionState.AllKnown, collections: [] }),
});
instantiationService.stub(IAllowedMcpServersService, {
onDidChangeAllowedMcpServers: Event.None,
isServerAllowed: () => true,
} as Partial<IAllowedMcpServersService> as IAllowedMcpServersService);
instantiationService.stub(IMcpWorkbenchService, {
local: [],
onChange: Event.None,
Expand Down Expand Up @@ -840,6 +844,10 @@ suite('AgentHostClientTools', () => {
instantiationService.stub(IMcpService, {
servers: observableValue('mcpServers', []),
});
instantiationService.stub(IAllowedMcpServersService, {
onDidChangeAllowedMcpServers: Event.None,
isServerAllowed: () => true,
} as Partial<IAllowedMcpServersService> as IAllowedMcpServersService);
instantiationService.stub(IConfigurationResolverService, {} as Partial<IConfigurationResolverService>);
instantiationService.stub(IPromptsService, new class extends mock<IPromptsService>() {
override readonly onDidChangeCustomAgents = Event.None;
Expand Down
Loading
Loading