Skip to content

Commit f2d43c2

Browse files
fix(cli,mcp): harden MCP probe and observability
1 parent 8256aa4 commit f2d43c2

9 files changed

Lines changed: 374 additions & 56 deletions

File tree

packages/cli/src/commands/mcp.mjs

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -444,7 +444,7 @@ export function resolveProbeTimeoutMs(timeout) {
444444
return Math.max(1000, usableSeconds * 1000);
445445
}
446446

447-
export function mcpSpawnOptions(env = {}, os = platform()) {
447+
export function mcpSpawnOptions(env = {}, _os = platform()) {
448448
return {
449449
env: { ...process.env, ...(env || {}) },
450450
stdio: ["pipe", "pipe", "pipe"],
@@ -470,11 +470,37 @@ function isWindowsExecutable(command) {
470470

471471
function wrapWindowsCommandLine(parts) {
472472
const commandLine = parts
473-
.map((part) => shellQuoteForPlatform(part, "win32"))
473+
.map((part) => quoteWindowsCommandArgument(part))
474474
.join(" ");
475475
return `"${commandLine}"`;
476476
}
477477

478+
function quoteWindowsCommandArgument(value) {
479+
const text = String(value).replaceAll("%", "%%");
480+
let quoted = "\"";
481+
let backslashes = 0;
482+
483+
for (const char of text) {
484+
if (char === "\\") {
485+
backslashes += 1;
486+
continue;
487+
}
488+
if (char === "\"") {
489+
quoted += "\\".repeat(backslashes * 2 + 1);
490+
quoted += char;
491+
backslashes = 0;
492+
continue;
493+
}
494+
quoted += "\\".repeat(backslashes);
495+
backslashes = 0;
496+
quoted += char;
497+
}
498+
499+
quoted += "\\".repeat(backslashes * 2);
500+
quoted += "\"";
501+
return quoted;
502+
}
503+
478504
function extractServer(target, config) {
479505
if (target === "cursor" || target === "claude-desktop") return config?.mcpServers?.qveris;
480506
if (target === "opencode") return config?.mcp?.qveris;

packages/cli/test/mcp.test.mjs

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,29 @@ test("mcp live probe wraps Windows command shims through cmd.exe without shell o
100100
]);
101101
});
102102

103+
test("mcp live probe safely quotes Windows command shim arguments", () => {
104+
const spec = mcpSpawnCommand(
105+
{
106+
command: "C:\\Tools\\npm shim\\npx.cmd",
107+
args: [
108+
"--name=foo\"bar",
109+
"C:\\temp\\tail\\",
110+
"100% ready",
111+
"safe&literal",
112+
],
113+
},
114+
"win32",
115+
);
116+
117+
assert.equal(spec.command.endsWith("cmd.exe"), true);
118+
assert.deepEqual(spec.args, [
119+
"/d",
120+
"/s",
121+
"/c",
122+
"\"\"C:\\Tools\\npm shim\\npx.cmd\" \"--name=foo\\\"bar\" \"C:\\temp\\tail\\\\\" \"100%% ready\" \"safe&literal\"\"",
123+
]);
124+
});
125+
103126
test("mcp command quoting follows the target shell platform", () => {
104127
assert.equal(shellQuoteForPlatform("L'Ondon", "darwin"), `'L'\\''Ondon'`);
105128
assert.equal(shellQuoteForPlatform('say "hi"', "win32"), `"say ""hi"""`);

packages/mcp/src/api/client.test.ts

Lines changed: 51 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -99,19 +99,19 @@ describe('QverisClient', () => {
9999
});
100100

101101
it('should throw ApiError on non-OK response', async () => {
102-
fetchMock.mockResolvedValueOnce({
103-
ok: false,
104-
status: 401,
105-
statusText: 'Unauthorized',
106-
json: async () => ({ message: 'Invalid API key' }),
107-
});
108-
109-
await expect(client.searchTools({ query: 'test' })).rejects.toEqual({
110-
status: 401,
111-
message: 'Invalid API key',
112-
details: { message: 'Invalid API key' },
113-
});
114-
});
102+
fetchMock.mockResolvedValueOnce({
103+
ok: false,
104+
status: 401,
105+
statusText: 'Unauthorized',
106+
json: async () => ({ message: 'Invalid API key' }),
107+
});
108+
109+
await expect(client.searchTools({ query: 'test' })).rejects.toMatchObject({
110+
status: 401,
111+
message: 'Invalid API key',
112+
details: { message: 'Invalid API key' },
113+
});
114+
});
115115

116116
it('should handle non-JSON error response', async () => {
117117
fetchMock.mockResolvedValueOnce({
@@ -123,12 +123,29 @@ describe('QverisClient', () => {
123123
},
124124
});
125125

126-
await expect(client.searchTools({ query: 'test' })).rejects.toEqual({
127-
status: 500,
128-
message: 'Internal Server Error',
129-
details: undefined,
130-
});
131-
});
126+
await expect(client.searchTools({ query: 'test' })).rejects.toMatchObject({
127+
status: 500,
128+
message: 'Internal Server Error',
129+
});
130+
});
131+
132+
it('should convert fetch network failures to observable ApiError', async () => {
133+
fetchMock.mockRejectedValueOnce(
134+
new Error('fetch failed', { cause: new Error('ECONNRESET') })
135+
);
136+
137+
await expect(client.searchTools({ query: 'test' })).rejects.toMatchObject({
138+
status: 0,
139+
message: 'fetch failed',
140+
cause: 'ECONNRESET',
141+
observability: {
142+
operation: 'discover',
143+
endpoint: '/search',
144+
http_status: 0,
145+
error_type: 'network_error',
146+
},
147+
});
148+
});
132149
});
133150

134151
describe('getToolsByIds', () => {
@@ -242,14 +259,14 @@ describe('QverisClient', () => {
242259
json: async () => ({ message: 'Invalid tool_ids' }),
243260
});
244261

245-
await expect(
246-
client.getToolsByIds({ tool_ids: [] })
247-
).rejects.toEqual({
248-
status: 400,
249-
message: 'Invalid tool_ids',
250-
details: { message: 'Invalid tool_ids' },
251-
});
252-
});
262+
await expect(
263+
client.getToolsByIds({ tool_ids: [] })
264+
).rejects.toMatchObject({
265+
status: 400,
266+
message: 'Invalid tool_ids',
267+
details: { message: 'Invalid tool_ids' },
268+
});
269+
});
253270

254271
it('should handle non-JSON error response', async () => {
255272
fetchMock.mockResolvedValueOnce({
@@ -261,14 +278,13 @@ describe('QverisClient', () => {
261278
},
262279
});
263280

264-
await expect(
265-
client.getToolsByIds({ tool_ids: ['tool-1'] })
266-
).rejects.toEqual({
267-
status: 500,
268-
message: 'Internal Server Error',
269-
details: undefined,
270-
});
271-
});
281+
await expect(
282+
client.getToolsByIds({ tool_ids: ['tool-1'] })
283+
).rejects.toMatchObject({
284+
status: 500,
285+
message: 'Internal Server Error',
286+
});
287+
});
272288
});
273289

274290
describe('executeTool', () => {

0 commit comments

Comments
 (0)