Skip to content
Draft
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
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,8 @@ class AgentCustomizationScope extends Disposable {
}
scheduleUpdate();
}));
// Republish so a newly blocked server is withdrawn rather than lingering for the session.
this._register(this._allowedMcpServersService.onDidChangeAllowedMcpServers(() => scheduleUpdate()));
}

acquire(): IAgentCustomizationScope {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { CancellationToken } from '../../../../../../base/common/cancellation.js
import { ResourceSet } from '../../../../../../base/common/map.js';
import { basename, isEqualOrParent } from '../../../../../../base/common/resources.js';
import { URI } from '../../../../../../base/common/uri.js';
import { CustomizationEnablementKind, type AgentCustomization, CustomizationType, type URI as ProtocolURI } from '../../../../../../platform/agentHost/common/state/protocol/state.js';
import { CustomizationEnablementKind, type AgentCustomization, type CustomizationEnablement, CustomizationType, type URI as ProtocolURI } from '../../../../../../platform/agentHost/common/state/protocol/state.js';
import { customizationId, type ClientPluginCustomization } from '../../../../../../platform/agentHost/common/state/sessionState.js';
import { withCustomizationEnablement } from '../../../../../../platform/agentHost/common/customizationEnablement.js';
import { AICustomizationSource, AICustomizationSources } from '../../../common/aiCustomizationWorkspaceService.js';
Expand All @@ -16,12 +16,13 @@ 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';
import { isDefined } from '../../../../../../base/common/types.js';
import { PromptFileParser } from '../../../common/promptSyntax/promptFileParser.js';
import { AgentHostMcpServerDelivery, resolveMcpServersForAgentHostDelivery } from './agentHostMcpServerSupport.js';
import { AgentHostMcpServerDelivery, isMcpServerConfigurationBlockedByPolicy, resolveMcpServersForAgentHostDelivery } from './agentHostMcpServerSupport.js';

/**
* Prompt types that participate in auto-sync to an agent host harness.
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 @@ -273,6 +276,20 @@ export async function resolveCustomizationRefs(
if (nonce !== undefined) {
ref.nonce = nonce.toString(16);
}
// A plugin syncs as a directory, so a policy-blocked server it contributes can only be
// withheld by disabling it as a child; the host maps this to `disabledMcpServers`.
const blockedChildren: Record<string, CustomizationEnablement[]> = {};
for (const definition of plugin.mcpServerDefinitions.get()) {
if (isMcpServerConfigurationBlockedByPolicy(allowedMcpServersService, definition.name, definition.configuration)) {
blockedChildren[definition.name] = withCustomizationEnablement(undefined, CustomizationEnablementKind.Global, {
kind: CustomizationEnablementKind.Global,
enabled: false,
});
}
}
if (Object.keys(blockedChildren).length > 0) {
ref.childEnablement = { ...ref.childEnablement, ...blockedChildren };
}
return ref;
})();
pluginRefs.set(key, promise);
Expand Down Expand Up @@ -311,7 +328,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 { IMcpServerIdentity, 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 All @@ -230,6 +236,11 @@ async function resolveMcpServerForAgentHostDelivery(
const source = getMcpServerSource(server, collection, definition);
const applicability = getMcpServerApplicability(collection, source.kind, workingDirectories);

// Gate every handoff path, not just client forwarding, so a server blocked for the local agent is blocked identically once delegated.
if (isBlockedByMcpPolicy(allowedMcpServersService, identityFromLaunch(server.definition.label, definition?.launch))) {
return createResolution(server, definition, source, applicability, AgentHostMcpServerDelivery.NotDelivered, unsupported([AgentHostMcpSupportReason.BlockedByPolicy]));
}

if (isPluginCollection(server, collection)) {
return createResolution(server, definition, source, applicability, AgentHostMcpServerDelivery.AgentPlugin, supported());
}
Expand Down Expand Up @@ -297,6 +308,19 @@ async function resolveMcpServerForAgentHostDelivery(
? AgentHostMcpServerDelivery.ClientForwarded
: deliveryForInapplicable(applicability);

// Re-checked against the resolved configuration: variable resolution can reveal a URL or command the raw definition hid.
if (isBlockedByMcpPolicy(allowedMcpServersService, mcpServerIdentityFromConfiguration(server.definition.label, projectedConfiguration))) {
return {
server,
definition,
source,
applicability,
delivery: AgentHostMcpServerDelivery.NotDelivered,
compatibility: unsupported([AgentHostMcpSupportReason.BlockedByPolicy]),
projectedConfiguration: undefined,
};
}

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

/**
* The identity the policy matches on, derived from a launch the same way `McpServer` derives it so
* the local and delegated paths cannot disagree. A launch that is absent or carries no usable
* command yields a name-only identity, which still matches `serverName` rules.
*/
function identityFromLaunch(name: string, launch: McpServerLaunch | undefined): IMcpServerIdentity {
if (launch?.type === McpServerTransportType.HTTP) {
return { name, url: launch.uri.toString(true) };
}
if (launch?.type === McpServerTransportType.Stdio && typeof launch.command === 'string') {
return { name, command: [launch.command, ...(launch.args ?? []).filter(arg => typeof arg === 'string')] };
}
return { name };
}

/** Whether a URL or command the policy matches on still carries an unresolved `${...}` variable. */
function hasUnresolvedPolicyFields(identity: IMcpServerIdentity): boolean {
const marker = ConfigurationResolverExpression.VARIABLE_LHS;
return !!identity.url?.includes(marker) || !!identity.command?.some(arg => arg.includes(marker));
}

/**
* Whether the enterprise `chat.mcp.allowedServers` / `chat.mcp.deniedServers` policy blocks handing
* this server to the agent host.
*
* Handing the server over is the last point at which the client can refuse — unlike `McpServer`,
* which defers an unverifiable verdict and re-evaluates once the launch resolves. So when a URL or
* command still carries `${...}`, an allow verdict that relied on that text is not trustworthy: the
* server is re-checked with the unresolved field dropped and blocked unless it is still allowed on
* its name alone. An allowlist entry matching by `serverName` is therefore honoured (the URL never
* mattered), while a `serverUrl` wildcard that merely matched the literal `${...}` text is not.
*/
export function isBlockedByMcpPolicy(allowedMcpServersService: IAllowedMcpServersService, identity: IMcpServerIdentity): boolean {
if (allowedMcpServersService.isServerAllowed(identity) !== true) {
return true;
}
if (!hasUnresolvedPolicyFields(identity)) {
return false;
}
return allowedMcpServersService.isServerAllowed({ name: identity.name }) !== true;
}

/** Whether the policy blocks a declaratively-configured server, e.g. one contributed by a plugin. */
export function isMcpServerConfigurationBlockedByPolicy(allowedMcpServersService: IAllowedMcpServersService, name: string, configuration: IMcpServerConfiguration): boolean {
return isBlockedByMcpPolicy(allowedMcpServersService, mcpServerIdentityFromConfiguration(name, configuration));
}

function createResolution(
server: IMcpServer,
definition: McpServerDefinition | undefined,
Expand Down
Loading
Loading