feat(cc-config): add and remove MCP servers through the claude CLI
Mutation goes through `claude mcp add|remove`, never a direct edit of ~/.claude.json — the running CLI rewrites that file concurrently, so a dashboard write would race it. Reads stay on readMcpServers. The plan specified a --url flag; the real CLI (v2.1.266) has none — commandOrUrl is positional, and --header must trail it or the variadic flag swallows the url. Built against the verified shape.
This commit is contained in:
@@ -1112,4 +1112,98 @@ describe("/api/cc-config", () => {
|
||||
process.env.PATH = originalPath;
|
||||
}
|
||||
});
|
||||
|
||||
// ── MCP add/remove routes (fake `claude` on PATH) ───────────────────────
|
||||
//
|
||||
// `printf`, not `echo` — bash's builtin `echo "-e"` swallows a lone `-e`
|
||||
// argument as its own flag, and `-e KEY=VALUE` is exactly what mcpAdd
|
||||
// emits for a stdio server's env vars.
|
||||
const PRINTF_ARGV = 'for a in "$@"; do printf \'%s\\n\' "$a"; done';
|
||||
|
||||
it("POST /mcp/add builds an http server with a positional url", async () => {
|
||||
const restore = stubClaude(PRINTF_ARGV);
|
||||
try {
|
||||
const { status, body } = await fetchJson("/api/cc-config/mcp/add", {
|
||||
method: "POST",
|
||||
body: { name: "vercel", scope: "user", transport: "http", url: "https://mcp.vercel.com" },
|
||||
});
|
||||
assert.equal(status, 200);
|
||||
assert.deepEqual(body.stdout.trim().split("\n"), [
|
||||
"mcp",
|
||||
"add",
|
||||
"--scope",
|
||||
"user",
|
||||
"--transport",
|
||||
"http",
|
||||
"vercel",
|
||||
"https://mcp.vercel.com",
|
||||
]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("POST /mcp/add builds a stdio server with args after --", async () => {
|
||||
const restore = stubClaude(PRINTF_ARGV);
|
||||
try {
|
||||
const { status, body } = await fetchJson("/api/cc-config/mcp/add", {
|
||||
method: "POST",
|
||||
body: { name: "docs", scope: "user", command: "node", args: ["s.js", "--port", "3000"] },
|
||||
});
|
||||
assert.equal(status, 200);
|
||||
assert.deepEqual(body.stdout.trim().split("\n").slice(-5), [
|
||||
"--",
|
||||
"node",
|
||||
"s.js",
|
||||
"--port",
|
||||
"3000",
|
||||
]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("POST /mcp/add rejects a name that reads as a flag with 400", async () => {
|
||||
const { status, body } = await fetchJson("/api/cc-config/mcp/add", {
|
||||
method: "POST",
|
||||
body: { name: "--all", scope: "user", command: "node", args: [] },
|
||||
});
|
||||
assert.equal(status, 400);
|
||||
assert.equal(body.error.code, "EBADARG");
|
||||
});
|
||||
|
||||
it("POST /mcp/remove surfaces a failed CLI run as 422", async () => {
|
||||
const restore = stubClaude('echo "No MCP server named \\"nope\\"" >&2\nexit 1');
|
||||
try {
|
||||
const { status, body } = await fetchJson("/api/cc-config/mcp/remove", {
|
||||
method: "POST",
|
||||
body: { name: "nope", scope: "user" },
|
||||
});
|
||||
assert.equal(status, 422);
|
||||
assert.equal(body.error.code, "ECLIEXIT");
|
||||
assert.match(body.error.message, /No MCP server named/);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("POST /mcp/remove runs the CLI with the given scope", async () => {
|
||||
const restore = stubClaude(PRINTF_ARGV);
|
||||
try {
|
||||
const { status, body } = await fetchJson("/api/cc-config/mcp/remove", {
|
||||
method: "POST",
|
||||
body: { name: "docs", scope: "project" },
|
||||
});
|
||||
assert.equal(status, 200);
|
||||
assert.deepEqual(body.stdout.trim().split("\n"), [
|
||||
"mcp",
|
||||
"remove",
|
||||
"docs",
|
||||
"--scope",
|
||||
"project",
|
||||
]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
@@ -0,0 +1,267 @@
|
||||
/**
|
||||
* @file Tests for server/lib/cc-mcp-cli.js — the `claude mcp add`/`claude mcp
|
||||
* remove` wrapper. Uses a real fake `claude` script on PATH (the fixture
|
||||
* style server/__tests__/cc-plugin-cli.test.js already uses) rather than
|
||||
* mocking child_process, and pins the exact argv each operation builds —
|
||||
* that argv IS the security boundary.
|
||||
*
|
||||
* The argv shapes asserted here were verified against the real
|
||||
* `claude mcp add/remove --help` (v2.1.266), not assumed: there is no
|
||||
* `--url` flag (the url is positional), and `--header` must trail the url.
|
||||
* See cc-mcp-cli.js's file header for the full list of surprises.
|
||||
*
|
||||
* @author Nguyễn Ngọc Trí Vĩ <vinnt@smartgift.vn>
|
||||
*/
|
||||
const { describe, it, after } = require("node:test");
|
||||
const assert = require("node:assert/strict");
|
||||
const fs = require("node:fs");
|
||||
const os = require("node:os");
|
||||
const path = require("node:path");
|
||||
|
||||
const ROOT = fs.mkdtempSync(path.join(os.tmpdir(), "agenthub-ccmcp-"));
|
||||
const cli = require("../lib/cc-mcp-cli");
|
||||
|
||||
/**
|
||||
* Put a fake `claude` on PATH; it prints argv, one element per line.
|
||||
* `printf`, not `echo` — bash's builtin `echo "-e"` treats a lone `-e`
|
||||
* argument as its own flag and swallows it, which is exactly one of the
|
||||
* argv elements this wrapper emits (`-e KEY=VALUE`).
|
||||
*/
|
||||
function stubClaude(scriptBody = 'for a in "$@"; do printf \'%s\\n\' "$a"; done') {
|
||||
const bin = path.join(ROOT, `bin-${process.hrtime.bigint()}`);
|
||||
fs.mkdirSync(bin, { recursive: true });
|
||||
const file = path.join(bin, "claude");
|
||||
fs.writeFileSync(file, `#!/usr/bin/env bash\n${scriptBody}\n`);
|
||||
fs.chmodSync(file, 0o755);
|
||||
const originalPath = process.env.PATH;
|
||||
process.env.PATH = `${bin}${path.delimiter}${originalPath}`;
|
||||
return () => {
|
||||
process.env.PATH = originalPath;
|
||||
};
|
||||
}
|
||||
|
||||
const argvOf = (result) => result.stdout.trim().split("\n");
|
||||
|
||||
after(() => fs.rmSync(ROOT, { recursive: true, force: true, maxRetries: 5 }));
|
||||
|
||||
describe("cc-mcp-cli argument validation", () => {
|
||||
it("accepts letters, numbers, hyphens and underscores as a name", () => {
|
||||
assert.equal(cli.assertName("my-server_1"), "my-server_1");
|
||||
});
|
||||
|
||||
it("rejects a name with characters the CLI itself rejects", () => {
|
||||
for (const bad of ["a/b", "a b", "a.b", ""]) {
|
||||
assert.throws(() => cli.assertName(bad), undefined, `accepted ${bad}`);
|
||||
}
|
||||
});
|
||||
|
||||
it("refuses a server name that reads as a flag", async () => {
|
||||
await assert.rejects(() => cli.mcpRemove({ name: "--all" }), /name/i);
|
||||
});
|
||||
|
||||
it("requires an explicit scope on add but not on remove", () => {
|
||||
assert.throws(() => cli.assertMcpScope(undefined, { required: true }), /scope must be one of/);
|
||||
assert.equal(cli.assertMcpScope(undefined), undefined);
|
||||
assert.equal(cli.assertMcpScope("user", { required: true }), "user");
|
||||
});
|
||||
|
||||
it("rejects the settings-only project-local scope and any other unknown scope", () => {
|
||||
for (const bad of ["project-local", "local", "bogus"]) {
|
||||
assert.throws(() => cli.assertMcpScope(bad), /scope must be one of/, `accepted ${bad}`);
|
||||
}
|
||||
});
|
||||
});
|
||||
|
||||
describe("cc-mcp-cli argv construction", () => {
|
||||
it("builds an http server with a positional url, not a --url flag", async () => {
|
||||
const restore = stubClaude();
|
||||
try {
|
||||
const result = await cli.mcpAdd({
|
||||
name: "vercel",
|
||||
scope: "user",
|
||||
transport: "http",
|
||||
url: "https://mcp.vercel.com",
|
||||
});
|
||||
assert.deepEqual(argvOf(result), [
|
||||
"mcp",
|
||||
"add",
|
||||
"--scope",
|
||||
"user",
|
||||
"--transport",
|
||||
"http",
|
||||
"vercel",
|
||||
"https://mcp.vercel.com",
|
||||
]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("puts stdio command args after the -- separator", async () => {
|
||||
const restore = stubClaude();
|
||||
try {
|
||||
const result = await cli.mcpAdd({
|
||||
name: "docs",
|
||||
scope: "user",
|
||||
command: "node",
|
||||
args: ["s.js", "--port", "3000"],
|
||||
});
|
||||
assert.deepEqual(argvOf(result).slice(-5), ["--", "node", "s.js", "--port", "3000"]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("puts --header flags after the positional url, not before", async () => {
|
||||
// Verified against the real CLI: --header is variadic, so placed before
|
||||
// the url it would swallow the url as one of its own header values.
|
||||
const restore = stubClaude();
|
||||
try {
|
||||
const result = await cli.mcpAdd({
|
||||
name: "corridor",
|
||||
scope: "project",
|
||||
transport: "http",
|
||||
url: "https://app.corridor.dev/api/mcp",
|
||||
headers: { Authorization: "Bearer xyz" },
|
||||
});
|
||||
assert.deepEqual(argvOf(result), [
|
||||
"mcp",
|
||||
"add",
|
||||
"--scope",
|
||||
"project",
|
||||
"--transport",
|
||||
"http",
|
||||
"corridor",
|
||||
"https://app.corridor.dev/api/mcp",
|
||||
"--header",
|
||||
"Authorization: Bearer xyz",
|
||||
]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("passes env vars as -e KEY=VALUE for stdio servers, value may start with -", async () => {
|
||||
const restore = stubClaude();
|
||||
try {
|
||||
const result = await cli.mcpAdd({
|
||||
name: "envy",
|
||||
scope: "user",
|
||||
command: "node",
|
||||
args: ["s.js"],
|
||||
env: { API_KEY: "-not-a-flag" },
|
||||
});
|
||||
assert.deepEqual(argvOf(result), [
|
||||
"mcp",
|
||||
"add",
|
||||
"--scope",
|
||||
"user",
|
||||
"envy",
|
||||
"-e",
|
||||
"API_KEY=-not-a-flag",
|
||||
"--",
|
||||
"node",
|
||||
"s.js",
|
||||
]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("builds the remove argv, scope flag omitted when not given", async () => {
|
||||
const restore = stubClaude();
|
||||
try {
|
||||
assert.deepEqual(argvOf(await cli.mcpRemove({ name: "docs" })), ["mcp", "remove", "docs"]);
|
||||
assert.deepEqual(argvOf(await cli.mcpRemove({ name: "docs", scope: "project" })), [
|
||||
"mcp",
|
||||
"remove",
|
||||
"docs",
|
||||
"--scope",
|
||||
"project",
|
||||
]);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("rejects url and command together, and neither", async () => {
|
||||
await assert.rejects(
|
||||
() => cli.mcpAdd({ name: "x", scope: "user", url: "https://a", command: "b" }),
|
||||
/either url or command, not both/
|
||||
);
|
||||
await assert.rejects(
|
||||
() => cli.mcpAdd({ name: "x", scope: "user" }),
|
||||
/requires either url or command/
|
||||
);
|
||||
});
|
||||
|
||||
it("rejects env together with url, and headers together with command", async () => {
|
||||
await assert.rejects(
|
||||
() => cli.mcpAdd({ name: "x", scope: "user", url: "https://a", env: { K: "v" } }),
|
||||
/env is not valid together with url/
|
||||
);
|
||||
await assert.rejects(
|
||||
() =>
|
||||
cli.mcpAdd({
|
||||
name: "x",
|
||||
scope: "user",
|
||||
command: "node",
|
||||
headers: { K: "v" },
|
||||
}),
|
||||
/headers is not valid together with command/
|
||||
);
|
||||
});
|
||||
|
||||
it("never lets an args element reach a shell", async () => {
|
||||
// A shell-metacharacter payload is a legal argv element (many real flags
|
||||
// contain punctuation) — the guarantee is execFile's argv array, not a
|
||||
// character blocklist. If this were interpolated into a shell string the
|
||||
// `touch` would run; through execFile it stays one inert literal.
|
||||
const canary = path.join(ROOT, "pwned");
|
||||
const restore = stubClaude();
|
||||
try {
|
||||
const result = await cli.mcpAdd({
|
||||
name: "x",
|
||||
scope: "user",
|
||||
command: "node",
|
||||
args: [`s.js; touch ${canary}`],
|
||||
});
|
||||
assert.equal(argvOf(result).at(-1), `s.js; touch ${canary}`);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
assert.equal(fs.existsSync(canary), false);
|
||||
});
|
||||
});
|
||||
|
||||
describe("cc-mcp-cli run outcomes", () => {
|
||||
it("reports a non-zero exit as ok:false instead of throwing", async () => {
|
||||
const restore = stubClaude('echo "✘ Server name already exists" >&2\nexit 1');
|
||||
try {
|
||||
const result = await cli.mcpAdd({
|
||||
name: "dup",
|
||||
scope: "user",
|
||||
command: "node",
|
||||
args: ["s.js"],
|
||||
});
|
||||
assert.equal(result.ok, false);
|
||||
assert.equal(result.code, 1);
|
||||
assert.match(cli.cliMessage(result), /already exists/);
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
|
||||
it("serializes mutating calls so two clicks cannot interleave", async () => {
|
||||
const log = path.join(ROOT, `order-${process.hrtime.bigint()}.txt`);
|
||||
const restore = stubClaude(`echo "start $3" >> ${log}\nsleep 0.2\necho "end $3" >> ${log}`);
|
||||
try {
|
||||
await Promise.all([cli.mcpRemove({ name: "first" }), cli.mcpRemove({ name: "second" })]);
|
||||
const lines = fs.readFileSync(log, "utf8").trim().split("\n");
|
||||
assert.equal(lines[0].replace("start ", ""), lines[1].replace("end ", ""));
|
||||
assert.equal(lines[2].replace("start ", ""), lines[3].replace("end ", ""));
|
||||
} finally {
|
||||
restore();
|
||||
}
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user