372 lines
13 KiB
JavaScript
372 lines
13 KiB
JavaScript
/**
|
|
* @file Tests for server/lib/codex-cli.js — the `codex mcp`/`codex plugin`
|
|
* wrapper that backs the Codex config panel's MCP-server and plugin
|
|
* management. Uses a fake `codex` script on PATH (the fixture style
|
|
* server/__tests__/cc-plugin-cli.test.js uses) rather than mocking
|
|
* child_process, and pins the exact argv each operation builds — that argv
|
|
* IS the security boundary.
|
|
* @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-codexcli-"));
|
|
const cli = require("../lib/codex-cli");
|
|
|
|
/**
|
|
* Put a fake `codex` on PATH. The default body echoes the argv, one element
|
|
* per line, so a test can assert the exact array that was spawned.
|
|
*/
|
|
function stubCodex(scriptBody = 'for a in "$@"; do echo "$a"; done') {
|
|
const bin = path.join(ROOT, `bin-${process.hrtime.bigint()}`);
|
|
fs.mkdirSync(bin, { recursive: true });
|
|
const file = path.join(bin, "codex");
|
|
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;
|
|
};
|
|
}
|
|
|
|
/** PATH with no `codex` on it at all. */
|
|
function stubMissingCodex() {
|
|
const empty = path.join(ROOT, `empty-${process.hrtime.bigint()}`);
|
|
fs.mkdirSync(empty, { recursive: true });
|
|
const originalPath = process.env.PATH;
|
|
process.env.PATH = empty;
|
|
return () => {
|
|
process.env.PATH = originalPath;
|
|
};
|
|
}
|
|
|
|
const argvOf = (result) => result.stdout.trim().split("\n");
|
|
|
|
after(() => fs.rmSync(ROOT, { recursive: true, force: true, maxRetries: 5 }));
|
|
|
|
describe("codex-cli argument validation", () => {
|
|
it("refuses a name that could be read as a flag or a path", async () => {
|
|
await assert.rejects(() => cli.mcpRemove("--force"), /name/i);
|
|
await assert.rejects(() => cli.mcpRemove("../../etc"), /name/i);
|
|
await assert.rejects(() => cli.pluginRemove("--force"), /spec/i);
|
|
await assert.rejects(() => cli.pluginRemove("../../etc"), /spec/i);
|
|
await assert.rejects(() => cli.marketplaceRemove("--force"), /name/i);
|
|
await assert.rejects(() => cli.mcpAdd({ name: "--force", command: "node" }), /name/i);
|
|
});
|
|
|
|
it("accepts a plain identifier as a name", () => {
|
|
assert.match("docs", cli.NAME_RE);
|
|
assert.doesNotMatch("--force", cli.NAME_RE);
|
|
assert.doesNotMatch("../../etc", cli.NAME_RE);
|
|
});
|
|
|
|
it("accepts <name> and <name>@<marketplace> plugin specs only", () => {
|
|
assert.equal(cli.assertPluginSpec("agenthub"), "agenthub");
|
|
assert.equal(cli.assertPluginSpec("agenthub@sg"), "agenthub@sg");
|
|
for (const bad of ["../agenthub", "agenthub agenthub", "@sg", "-agenthub", "a/b"]) {
|
|
assert.throws(() => cli.assertPluginSpec(bad), undefined, `accepted ${bad}`);
|
|
}
|
|
});
|
|
|
|
it("accepts only http(s) URLs (mcpAdd's --url form)", () => {
|
|
assert.equal(cli.assertUrl("https://example.com/mp.git", "url"), "https://example.com/mp.git");
|
|
for (const bad of ["ssh://example.com/mp.git", "not a url", "file:///tmp/x"]) {
|
|
assert.throws(() => cli.assertUrl(bad, "url"), /http\(s\)/, `accepted ${bad}`);
|
|
}
|
|
// rejected earlier, by the shared leading-dash guard, not the URL check
|
|
assert.throws(() => cli.assertUrl("--force", "url"), /may not start with/);
|
|
});
|
|
|
|
it("marketplaceAdd accepts the source forms `codex plugin marketplace add` actually takes", async () => {
|
|
// Not assertUrl: `codex plugin marketplace add <SOURCE>` also takes
|
|
// owner/repo and a local path, which a bare http(s)-only check rejected.
|
|
const restore = stubCodex();
|
|
try {
|
|
assert.deepEqual(await cli.marketplaceAdd("openai/codex").then(argvOf), [
|
|
"plugin",
|
|
"marketplace",
|
|
"add",
|
|
"openai/codex",
|
|
]);
|
|
assert.deepEqual(await cli.marketplaceAdd("/abs/local/path").then(argvOf), [
|
|
"plugin",
|
|
"marketplace",
|
|
"add",
|
|
"/abs/local/path",
|
|
]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
// ssh remotes are deliberately out of scope, same as cc-plugin-cli's
|
|
// marketplace source guard — no credential/host-key story for them yet.
|
|
await assert.rejects(() => cli.marketplaceAdd("git@github.com:owner/repo.git"), /source/);
|
|
await assert.rejects(() => cli.marketplaceAdd("../escape"), /source/);
|
|
});
|
|
|
|
it("rejects mcpAdd with neither or both of url/command", async () => {
|
|
await assert.rejects(() => cli.mcpAdd({ name: "docs" }), /requires either/);
|
|
await assert.rejects(
|
|
() => cli.mcpAdd({ name: "docs", url: "https://x", command: "node" }),
|
|
/either url or command, not both/
|
|
);
|
|
});
|
|
|
|
it("rejects env together with url — --env is invalid on the url form", async () => {
|
|
await assert.rejects(
|
|
() => cli.mcpAdd({ name: "docs", url: "https://x", env: { A: "1" } }),
|
|
/stdio-only/
|
|
);
|
|
});
|
|
|
|
it("rejects an env key that is not a valid identifier", async () => {
|
|
await assert.rejects(
|
|
() => cli.mcpAdd({ name: "docs", command: "node", env: { "1bad": "1" } }),
|
|
/identifier/
|
|
);
|
|
});
|
|
});
|
|
|
|
describe("codex-cli argv construction", () => {
|
|
it("passes the server name as one argv entry, never through a shell", async () => {
|
|
const restore = stubCodex();
|
|
try {
|
|
const result = await cli.mcpAdd({ name: "docs", command: "node", args: ["server.js"] });
|
|
assert.deepEqual(argvOf(result), ["mcp", "add", "docs", "--", "node", "server.js"]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("puts --env flags between NAME and -- for a stdio server", async () => {
|
|
const restore = stubCodex();
|
|
try {
|
|
const result = await cli.mcpAdd({
|
|
name: "docs",
|
|
command: "node",
|
|
args: ["server.js"],
|
|
env: { API_KEY: "secret", MODE: "prod" },
|
|
});
|
|
assert.deepEqual(argvOf(result), [
|
|
"mcp",
|
|
"add",
|
|
"docs",
|
|
"--env",
|
|
"API_KEY=secret",
|
|
"--env",
|
|
"MODE=prod",
|
|
"--",
|
|
"node",
|
|
"server.js",
|
|
]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("builds the --url form with an optional --bearer-token-env-var", async () => {
|
|
const restore = stubCodex();
|
|
try {
|
|
const result = await cli.mcpAdd({
|
|
name: "docs",
|
|
url: "https://example.com/mcp",
|
|
bearerTokenEnvVar: "DOCS_TOKEN",
|
|
});
|
|
assert.deepEqual(argvOf(result), [
|
|
"mcp",
|
|
"add",
|
|
"docs",
|
|
"--bearer-token-env-var",
|
|
"DOCS_TOKEN",
|
|
"--url",
|
|
"https://example.com/mcp",
|
|
]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("accepts an env value starting with '-' — it's the RHS of one KEY=VALUE argv element, never a flag", async () => {
|
|
const restore = stubCodex();
|
|
try {
|
|
const result = await cli.mcpAdd({
|
|
name: "docs",
|
|
command: "node",
|
|
env: { OFFSET: "-1" },
|
|
});
|
|
assert.deepEqual(argvOf(result), ["mcp", "add", "docs", "--env", "OFFSET=-1", "--", "node"]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("builds mcp list / remove argvs", async () => {
|
|
const restore = stubCodex();
|
|
try {
|
|
assert.deepEqual(argvOf(await cli.mcpList()), ["mcp", "list", "--json"]);
|
|
assert.deepEqual(argvOf(await cli.mcpRemove("docs")), ["mcp", "remove", "docs"]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("builds plugin list / add / remove argvs", async () => {
|
|
const restore = stubCodex();
|
|
try {
|
|
assert.deepEqual(argvOf(await cli.pluginList()), ["plugin", "list", "--json"]);
|
|
assert.deepEqual(argvOf(await cli.pluginAdd("demo@sg")), ["plugin", "add", "demo@sg"]);
|
|
assert.deepEqual(argvOf(await cli.pluginRemove("demo")), ["plugin", "remove", "demo"]);
|
|
// `plugin list` reports marketplace-scoped plugins as `<name>@<marketplace>`,
|
|
// and `plugin add` takes that form — so remove has to accept it back.
|
|
assert.deepEqual(argvOf(await cli.pluginRemove("demo@sg")), ["plugin", "remove", "demo@sg"]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("builds marketplace list / add / remove argvs, nested under plugin", async () => {
|
|
const restore = stubCodex();
|
|
try {
|
|
assert.deepEqual(argvOf(await cli.marketplaceList()), [
|
|
"plugin",
|
|
"marketplace",
|
|
"list",
|
|
"--json",
|
|
]);
|
|
assert.deepEqual(argvOf(await cli.marketplaceAdd("https://example.com/mp.git")), [
|
|
"plugin",
|
|
"marketplace",
|
|
"add",
|
|
"https://example.com/mp.git",
|
|
]);
|
|
assert.deepEqual(argvOf(await cli.marketplaceRemove("sg")), [
|
|
"plugin",
|
|
"marketplace",
|
|
"remove",
|
|
"sg",
|
|
]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("never lets a rejected name reach a shell", async () => {
|
|
// If the value were interpolated into a shell string this would create
|
|
// the file; the NAME_RE guard rejects it before any spawn happens.
|
|
const canary = path.join(ROOT, "pwned");
|
|
const restore = stubCodex();
|
|
try {
|
|
await assert.rejects(() => cli.mcpRemove("docs; touch " + canary), /name/i);
|
|
assert.equal(fs.existsSync(canary), false);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
});
|
|
|
|
describe("codex-cli run outcomes", () => {
|
|
it("surfaces a non-zero exit as ok:false with stderr kept", async () => {
|
|
const restore = stubCodex('echo "no such marketplace" >&2\nexit 2');
|
|
try {
|
|
const res = await cli.marketplaceRemove("nope");
|
|
assert.equal(res.ok, false);
|
|
assert.equal(res.code, 2);
|
|
assert.match(res.stderr, /no such marketplace/);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("throws a clear error when the codex CLI is not on PATH", async () => {
|
|
const restore = stubMissingCodex();
|
|
try {
|
|
await assert.rejects(
|
|
() => cli.mcpList(),
|
|
(err) => {
|
|
assert.equal(err.code, "ECLINOTFOUND");
|
|
assert.match(err.message, /not found on PATH/);
|
|
return true;
|
|
}
|
|
);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("parses JSON output into `data`", async () => {
|
|
const restore = stubCodex(`cat <<'JSON'\n{"servers":[{"name":"docs"}]}\nJSON`);
|
|
try {
|
|
const res = await cli.mcpList();
|
|
assert.equal(res.ok, true);
|
|
assert.deepEqual(res.data, { servers: [{ name: "docs" }] });
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("unwraps `codex plugin list --json`'s {installed, available} object into a bare array", async () => {
|
|
// Real codex-cli 0.149.0 shape — an object, unlike `mcp list`'s array.
|
|
const restore = stubCodex(
|
|
`cat <<'JSON'\n{"installed":[{"name":"demo"}],"available":[{"name":"other"}]}\nJSON`
|
|
);
|
|
try {
|
|
const res = await cli.pluginList();
|
|
assert.equal(res.ok, true);
|
|
assert.deepEqual(res.data, [{ name: "demo" }]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("treats a literal `null` on stdout as an empty list, not a crash", async () => {
|
|
// `null` is valid JSON, so it parses and slips past the unparseable-output
|
|
// guard. Reading `.installed` off it would surface as a 500 from a route
|
|
// whose whole contract is that CLI output never produces one.
|
|
const restore = stubCodex("echo null");
|
|
try {
|
|
assert.deepEqual((await cli.pluginList()).data, []);
|
|
assert.deepEqual((await cli.marketplaceList()).data, []);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("unwraps `codex plugin marketplace list --json`'s {marketplaces} object into a bare array", async () => {
|
|
const restore = stubCodex(`cat <<'JSON'\n{"marketplaces":[{"name":"sg"}]}\nJSON`);
|
|
try {
|
|
const res = await cli.marketplaceList();
|
|
assert.equal(res.ok, true);
|
|
assert.deepEqual(res.data, [{ name: "sg" }]);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("leaves `data` undefined when stdout is not JSON", async () => {
|
|
const restore = stubCodex('echo "removed docs"');
|
|
try {
|
|
const res = await cli.mcpRemove("docs");
|
|
assert.equal(res.ok, true);
|
|
assert.equal(res.data, undefined);
|
|
} finally {
|
|
restore();
|
|
}
|
|
});
|
|
|
|
it("serializes mutating calls so two clicks cannot interleave", async () => {
|
|
const log = path.join(ROOT, `order-${process.hrtime.bigint()}.txt`);
|
|
const restore = stubCodex(`echo "start $2" >> ${log}\nsleep 0.2\necho "end $2" >> ${log}`);
|
|
try {
|
|
await Promise.all([cli.mcpRemove("first"), cli.mcpRemove("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();
|
|
}
|
|
});
|
|
});
|