AgentHub/server/__tests__/cc-mcp-cli.test.js

/**
 * @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 CLI's own `project` scope — readMcpServers never reads .mcp.json", () => {
    // The Critical this pins: `--scope project` writes .mcp.json, which the
    // dashboard's read path never parses, so a successful add would report
    // an "added" toast for a server that never appears in the list.
    for (const bad of ["project", "project-local", "bogus"]) {
      assert.throws(() => cli.assertMcpScope(bad), /scope must be one of/, `accepted ${bad}`);
    }
  });

  it("accepts local — the CLI scope that actually backs the panel's projectScoped bucket", () => {
    assert.equal(cli.assertMcpScope("local", { required: true }), "local");
  });
});

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: "local",
        transport: "http",
        url: "https://app.corridor.dev/api/mcp",
        headers: { Authorization: "Bearer xyz" },
      });
      assert.deepEqual(argvOf(result), [
        "mcp",
        "add",
        "--scope",
        "local",
        "--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: "user" })), [
        "mcp",
        "remove",
        "docs",
        "--scope",
        "user",
      ]);
    } finally {
      restore();
    }
  });

  it("builds a local-scoped remove — the argv the CLI actually accepts for the panel's projectScoped rows", async () => {
    // The other Critical this pins: the panel used to send scope: "project"
    // for these rows, which the real CLI never matches (it stores them under
    // "local"), so Remove failed every time. `argvOf` is the exact argv
    // `execFile` spawns — there is no shell in between to paper over a wrong
    // scope string.
    const restore = stubClaude();
    try {
      assert.deepEqual(argvOf(await cli.mcpRemove({ name: "docs", scope: "local" })), [
        "mcp",
        "remove",
        "docs",
        "--scope",
        "local",
      ]);
    } finally {
      restore();
    }
  });

  it("rejects a remove scoped to the CLI's project (.mcp.json is not addressable here)", async () => {
    await assert.rejects(
      () => cli.mcpRemove({ name: "docs", scope: "project" }),
      /scope must be one of/
    );
  });

  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("allows an empty stdio arg — only type/length/control-char bounds are enforced", async () => {
    // A trailing empty argv element is invisible to `.trim().split("\n")`
    // (it collapses with the final newline), so assert with a marker arg
    // after it rather than checking the last line directly.
    const restore = stubClaude();
    try {
      const result = await cli.mcpAdd({
        name: "x",
        scope: "user",
        command: "node",
        args: ["", "marker"],
      });
      assert.deepEqual(argvOf(result).slice(-3), ["node", "", "marker"]);
    } finally {
      restore();
    }
  });

  it("rejects an object env/header value as a 400 instead of writing [object Object]", async () => {
    await assert.rejects(
      () =>
        cli.mcpAdd({
          name: "x",
          scope: "user",
          command: "node",
          env: { KEY: { nope: true } },
        }),
      /KEY is required/
    );
    await assert.rejects(
      () =>
        cli.mcpAdd({
          name: "x",
          scope: "user",
          transport: "http",
          url: "https://a",
          headers: { Authorization: { nope: true } },
        }),
      /Authorization is required/
    );
  });

  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();
    }
  });
});