Skip to content
Merged
1 change: 1 addition & 0 deletions OpenClaw-Query-Submit
Submodule OpenClaw-Query-Submit added at be6657
17 changes: 12 additions & 5 deletions packages/cli/src/acp-integration/acpAgent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,8 @@ import {
unregisterGoalHook,
ToolNames,
FORK_SUBAGENT_TYPE,
matchesAnyServerPattern,
matchesServerPattern,
} from '@qwen-code/qwen-code-core';
Comment thread
DennisYu07 marked this conversation as resolved.
Resolved
import { randomUUID } from 'node:crypto';
import type {
Expand Down Expand Up @@ -5599,17 +5601,22 @@ class QwenAgent implements Agent {
for (const scope of [SettingScope.User, SettingScope.Workspace]) {
const scopeSettings = settings.forScope(scope).settings;
const currentExcluded = scopeSettings.mcp?.excluded || [];
if (currentExcluded.includes(serverName)) {
if (matchesAnyServerPattern(serverName, currentExcluded)) {
settings.setValue(
scope,
Comment thread
DennisYu07 marked this conversation as resolved.
Resolved
'mcp.excluded',
currentExcluded.filter((name: string) => name !== serverName),
currentExcluded.filter(
(pattern: string) =>
!matchesServerPattern(serverName, pattern),
),
);
}
}
const currentExcluded = this.config.getExcludedMcpServers() || [];
this.config.setExcludedMcpServers(
currentExcluded.filter((name: string) => name !== serverName),
currentExcluded.filter(
(pattern: string) => !matchesServerPattern(serverName, pattern),
),
);
await toolRegistry.discoverToolsForServer(serverName);
return { serverName, action, ok: true, changed: true };
Expand All @@ -5635,14 +5642,14 @@ class QwenAgent implements Agent {
}
const scopeSettings = settings.forScope(targetScope).settings;
const currentExcluded = scopeSettings.mcp?.excluded || [];
if (!currentExcluded.includes(serverName)) {
if (!matchesAnyServerPattern(serverName, currentExcluded)) {
Comment thread
DennisYu07 marked this conversation as resolved.
Resolved
settings.setValue(targetScope, 'mcp.excluded', [
...currentExcluded,
serverName,
]);
}
const runtimeExcluded = this.config.getExcludedMcpServers() || [];
if (!runtimeExcluded.includes(serverName)) {
if (!matchesAnyServerPattern(serverName, runtimeExcluded)) {
this.config.setExcludedMcpServers([...runtimeExcluded, serverName]);
}
await toolRegistry.disableMcpServer(serverName);
Expand Down
6 changes: 4 additions & 2 deletions packages/cli/src/config/settingsSchema.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2416,7 +2416,8 @@ const SETTINGS_SCHEMA = {
// restart-required.
requiresRestart: false,
default: undefined as string[] | undefined,
description: 'A list of MCP servers to allow.',
description:
'A list of MCP servers to allow. Supports glob patterns (e.g. "*puppeteer*").',
showInDialog: false,
mergeStrategy: MergeStrategy.CONCAT,
},
Expand All @@ -2429,7 +2430,8 @@ const SETTINGS_SCHEMA = {
// restart-required.
requiresRestart: false,
default: undefined as string[] | undefined,
description: 'A list of MCP servers to exclude.',
description:
'A list of MCP servers to exclude. Supports glob patterns (e.g. "*puppeteer*"). Takes precedence over mcp.allowed.',
showInDialog: false,
mergeStrategy: MergeStrategy.CONCAT,
},
Expand Down
137 changes: 137 additions & 0 deletions packages/core/src/config/config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@ import {
APPROVAL_MODE_INFO,
MCPServerConfig,
TrustGateError,
matchesServerPattern,
matchesAnyServerPattern,
} from './config.js';
import { Storage } from './storage.js';
import * as fs from 'node:fs';
Expand Down Expand Up @@ -344,6 +346,67 @@ vi.mock('../core/toolHookTriggers.js', () => ({
fireNotificationHook: vi.fn().mockResolvedValue({}),
}));

describe('matchesServerPattern', () => {
it('exact match when no glob characters', () => {
expect(matchesServerPattern('puppeteer', 'puppeteer')).toBe(true);
expect(matchesServerPattern('puppeteer', 'playwright')).toBe(false);
});

it('* matches any sequence including empty', () => {
expect(matchesServerPattern('puppeteer', '*puppeteer*')).toBe(true);
expect(matchesServerPattern('my-puppeteer-server', '*puppeteer*')).toBe(
true,
);
expect(matchesServerPattern('playwright', '*puppeteer*')).toBe(false);
expect(matchesServerPattern('anything', '*')).toBe(true);
expect(matchesServerPattern('prefix-suffix', 'prefix*')).toBe(true);
expect(matchesServerPattern('prefix-suffix', '*suffix')).toBe(true);
});

it('? matches exactly one character', () => {
expect(matchesServerPattern('abc', 'a?c')).toBe(true);
expect(matchesServerPattern('ac', 'a?c')).toBe(false);
expect(matchesServerPattern('axc', 'a?c')).toBe(true);
});

it('escapes regex special characters', () => {
expect(matchesServerPattern('my.server', 'my.server')).toBe(true);
expect(matchesServerPattern('myXserver', 'my.server')).toBe(false);
expect(matchesServerPattern('a+b', 'a+b')).toBe(true);
expect(matchesServerPattern('a^b', 'a^b')).toBe(true);
});

it('combines glob with exact segments', () => {
expect(matchesServerPattern('foo-bar-baz', 'foo-*-baz')).toBe(true);
expect(matchesServerPattern('foo-bar-qux', 'foo-*-baz')).toBe(false);
});
});

describe('matchesAnyServerPattern', () => {
it('returns false for undefined or empty list', () => {
expect(matchesAnyServerPattern('puppeteer', undefined)).toBe(false);
expect(matchesAnyServerPattern('puppeteer', [])).toBe(false);
});

it('matches if any pattern matches', () => {
expect(
matchesAnyServerPattern('puppeteer', ['playwright', '*puppeteer*']),
).toBe(true);
expect(
matchesAnyServerPattern('chrome', ['playwright', '*puppeteer*']),
).toBe(false);
});

it('works with mixed exact and glob patterns', () => {
expect(
matchesAnyServerPattern('playwright', ['playwright', '*puppeteer*']),
).toBe(true);
expect(
matchesAnyServerPattern('my-puppeteer', ['playwright', '*puppeteer*']),
).toBe(true);
Comment thread
DennisYu07 marked this conversation as resolved.
Resolved
});
});

describe('Server Config (config.ts)', () => {
const MODEL = 'qwen3-coder-plus';

Expand Down Expand Up @@ -1060,6 +1123,80 @@ describe('Server Config (config.ts)', () => {
});
expect(config.getAllowedMcpServers()).toEqual(['y']);
});

it('getMcpServers filters by glob pattern in allowedMcpServers', async () => {
const config = new Config({
...baseParams,
mcpServers: {
puppeteer: srvA,
'my-puppeteer-server': srvB,
playwright: srvA,
},
});
config.setAllowedMcpServers(['*puppeteer*']);
const result = config.getMcpServers();
expect(Object.keys(result!)).toEqual([
'puppeteer',
'my-puppeteer-server',
]);
});

it('isMcpServerDisabled supports glob patterns in excludedMcpServers', () => {
const config = new Config({ ...baseParams });
config.setExcludedMcpServers(['*puppeteer*']);
expect(config.isMcpServerDisabled('puppeteer')).toBe(true);
expect(config.isMcpServerDisabled('my-puppeteer')).toBe(true);
expect(config.isMcpServerDisabled('playwright')).toBe(false);
});

it('getMcpServerUnavailableReason classifies by glob match', async () => {
const config = new Config({
...baseParams,
mcpServers: {
puppeteer: srvA,
Comment thread
DennisYu07 marked this conversation as resolved.
Resolved
playwright: srvB,
chrome: srvA,
},
});
await config.reinitializeMcpServers({
puppeteer: srvA,
playwright: srvB,
chrome: srvA,
});

config.setAllowedMcpServers(['play*']);
expect(config.getMcpServerUnavailableReason('puppeteer')).toBe(
'not_allowed',
);
expect(
config.getMcpServerUnavailableReason('playwright'),
).toBeUndefined();

// Clear allow-list so the excluded check is reached.
config.setAllowedMcpServers(undefined);
config.setExcludedMcpServers(['*chrome*']);
expect(config.getMcpServerUnavailableReason('chrome')).toBe('excluded');
});

it('exclude takes precedence over allow with glob patterns', async () => {
const config = new Config({
...baseParams,
mcpServers: { puppeteer: srvA, playwright: srvB },
});
await config.reinitializeMcpServers({
puppeteer: srvA,
playwright: srvB,
});

config.setAllowedMcpServers(['*']);
config.setExcludedMcpServers(['puppeteer']);
expect(config.getMcpServerUnavailableReason('puppeteer')).toBe(
'excluded',
);
expect(
config.getMcpServerUnavailableReason('playwright'),
).toBeUndefined();
});
});

describe('MemoryPressureMonitor isolation', () => {
Expand Down
61 changes: 56 additions & 5 deletions packages/core/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -631,6 +631,55 @@ export function isGatedMcpScope(scope: McpServerScope | undefined): boolean {
return scope === 'project' || scope === 'workspace';
}

/**
* Test whether a server name matches a single pattern. Patterns use simple
* glob semantics: `*` matches any sequence of characters (including empty),
* `?` matches exactly one character. A pattern without glob characters is
* compared as an exact string (no behavior change for existing configs).
* Uses an iterative two-pointer algorithm — O(n×m) worst case, no regex,
* no backtracking vulnerability.
*/
export function matchesServerPattern(name: string, pattern: string): boolean {
if (!pattern.includes('*') && !pattern.includes('?')) {
return name === pattern;
}
let ni = 0;
let pi = 0;
let starNi = -1;
let starPi = -1;
while (ni < name.length) {
if (
pi < pattern.length &&
(pattern[pi] === '?' || pattern[pi] === name[ni])
) {
ni++;
pi++;
} else if (pi < pattern.length && pattern[pi] === '*') {
starPi = pi++;
starNi = ni;
} else if (starPi !== -1) {
pi = starPi + 1;
ni = ++starNi;
} else {
return false;
}
}
while (pi < pattern.length && pattern[pi] === '*') pi++;
return pi === pattern.length;
}

/**
* Test whether a server name matches any pattern in the given list.
* Returns false for an empty or undefined list.
*/
export function matchesAnyServerPattern(
name: string,
patterns: string[] | undefined,
): boolean {
if (!patterns || patterns.length === 0) return false;
return patterns.some((p) => matchesServerPattern(name, p));
}

export class MCPServerConfig {
constructor(
// For stdio transport
Expand Down Expand Up @@ -3755,7 +3804,7 @@ export class Config {
if (this.allowedMcpServers) {
mcpServers = Object.fromEntries(
Object.entries(mcpServers).filter(([key]) =>
this.allowedMcpServers?.includes(key),
matchesAnyServerPattern(key, this.allowedMcpServers),
),
);
}
Expand All @@ -3776,7 +3825,8 @@ export class Config {
}

isMcpServerDisabled(serverName: string): boolean {
if (this.excludedMcpServers?.includes(serverName)) return true;
if (matchesAnyServerPattern(serverName, this.excludedMcpServers))
return true;
// Extension-bundled servers can be disabled individually via extension
// preferences. Only the extension that actually contributed the server is
// consulted, so a same-named server from another source (e.g. a shadowing
Expand Down Expand Up @@ -3928,11 +3978,12 @@ export class Config {
if (!(serverName in this.getMergedMcpServers())) return undefined;
if (
this.allowedMcpServers &&
!this.allowedMcpServers.includes(serverName)
!matchesAnyServerPattern(serverName, this.allowedMcpServers)
) {
return 'not_allowed';
}
if (this.excludedMcpServers?.includes(serverName)) return 'excluded';
if (matchesAnyServerPattern(serverName, this.excludedMcpServers))
return 'excluded';
if (this.isMcpServerPendingApproval(serverName)) return 'pending_approval';
return undefined;
}
Expand Down Expand Up @@ -5092,7 +5143,7 @@ export class Config {

if (this.allowedMcpServers) {
Object.entries(mcpServers).forEach(([key, server]) => {
const isAllowed = this.allowedMcpServers?.includes(key);
const isAllowed = matchesAnyServerPattern(key, this.allowedMcpServers);
if (!isAllowed) {
blockedMcpServers.push({
name: key,
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/tools/tool-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type {
ToolInvocation,
} from './tools.js';
import { Kind, BaseDeclarativeTool, BaseToolInvocation } from './tools.js';
import type { Config } from '../config/config.js';
import { type Config, matchesAnyServerPattern } from '../config/config.js';
Comment thread
DennisYu07 marked this conversation as resolved.
Resolved
import { spawn } from 'node:child_process';
import { StringDecoder } from 'node:string_decoder';
import type { SendSdkMcpMessage } from './mcp-client.js';
Expand Down Expand Up @@ -449,7 +449,7 @@ export class ToolRegistry {
// while isMcpServerDisabled still returns false, mis-reporting
// an intentional disable as a connectivity failure.
const currentExcluded = this.config.getExcludedMcpServers() || [];
if (!currentExcluded.includes(serverName)) {
if (!matchesAnyServerPattern(serverName, currentExcluded)) {
this.config.setExcludedMcpServers([...currentExcluded, serverName]);

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[Suggestion] Auto-disable writes raw serverName literals into the excluded list, which is now glob-interpreted. If a server name contains * or ? (legal — the schema imposes no restriction), the appended entry is re-interpreted as a glob on subsequent reads. Example: server test?v2 fails → auto-excluded as ['test?v2'] → matchesServerPattern('testXv2', 'test?v2') returns true, disabling an unrelated server.

Escape glob metacharacters when appending literals, or document the constraint that server names must not contain */?.

— qwen3.7-max via Qwen Code /review

}
} finally {
Expand Down
4 changes: 2 additions & 2 deletions packages/vscode-ide-companion/schemas/settings.schema.json
Original file line number Diff line number Diff line change
Expand Up @@ -1119,14 +1119,14 @@
"type": "string"
},
"allowed": {
"description": "A list of MCP servers to allow.",
"description": "A list of MCP servers to allow. Supports glob patterns (e.g. \"*puppeteer*\").",
"type": "array",
"items": {
"type": "string"
}
},
"excluded": {
"description": "A list of MCP servers to exclude.",
"description": "A list of MCP servers to exclude. Supports glob patterns (e.g. \"*puppeteer*\"). Takes precedence over mcp.allowed.",
"type": "array",
"items": {
"type": "string"
Expand Down
Loading