Skip to content

Commit 2a35786

Browse files
authored
Merge pull request #49 from allocsys/test/mcp-integration-coverage
CI: add MCP tool + e2e integration test coverage (Zod, Express, rate-limit)
2 parents 30e26f1 + 2c29173 commit 2a35786

4 files changed

Lines changed: 177 additions & 14 deletions

File tree

package.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,8 +20,9 @@
2020
"zod": "^3.24.1"
2121
},
2222
"devDependencies": {
23-
"vitest": "^4.1.10",
23+
"@eslint/js": "^9.17.0",
2424
"eslint": "^9.17.0",
25-
"@eslint/js": "^9.17.0"
25+
"supertest": "^7.2.2",
26+
"vitest": "^4.1.10"
2627
}
2728
}

server.js

Lines changed: 19 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -153,15 +153,22 @@ app.post("/mcp", mcpLimiter, requireMcpKey, requireAllowedIp, handleMcp);
153153
app.post("/mcp/:key", mcpLimiter, requireMcpKey, requireAllowedIp, handleMcp);
154154

155155
const PORT = process.env.PORT || 8080;
156-
app.listen(PORT, () => {
157-
console.log(`madmcp-server v2.1.0 listening on port ${PORT}`);
158-
if (!GITHUB_TOKEN) console.warn("WARNING: GITHUB_TOKEN is not set.");
159-
if (!NOTION_TOKEN) console.warn("WARNING: NOTION_TOKEN is not set. Notion tools will fail.");
160-
if (!MEM0_API_KEY) console.warn("WARNING: MEM0_API_KEY is not set. Mem0 tools will fail.");
161-
if (!CLOUDFLARE_API_TOKEN || !CLOUDFLARE_ACCOUNT_ID) console.warn("WARNING: CLOUDFLARE_API_TOKEN/CLOUDFLARE_ACCOUNT_ID not set. Cloudflare tools will fail.");
162-
if (!CONTEXT7_API_KEY) console.warn("NOTE: CONTEXT7_API_KEY is not set. Context7 tools will work but at lower, unauthenticated rate limits.");
163-
if (!GEMINI_API_KEY) console.warn("WARNING: GEMINI_API_KEY is not set. Gemini tools (delegate_research) will fail.");
164-
if (!MCP_SHARED_KEY) console.warn("WARNING: MCP_SHARED_KEY is not set. The /mcp, /mcp/:key, and / endpoints are OPEN to anyone who has the URL.");
165-
if (!GITHUB_APP_ID || !GITHUB_APP_INSTALLATION_ID || !GITHUB_APP_PRIVATE_KEY) console.warn("NOTE: GITHUB_APP_ID/GITHUB_APP_INSTALLATION_ID/GITHUB_APP_PRIVATE_KEY not fully set. get_repo_clone_token (private-repo sandbox clone) will fail until the GitHub App is configured.");
166-
console.log(`IP allowlist: ${IP_ALLOWLIST_ENABLED ? `ENABLED (${ALLOWED_IP_RANGES.join(", ")})` : "DISABLED"}`);
167-
});
156+
// Gated so importing this module (e.g. from tests via supertest, or the MCP
157+
// integration test's InMemoryTransport) never binds a real port. Tests set
158+
// NODE_ENV=test before importing server.js.
159+
if (process.env.NODE_ENV !== "test") {
160+
app.listen(PORT, () => {
161+
console.log(`madmcp-server v2.1.0 listening on port ${PORT}`);
162+
if (!GITHUB_TOKEN) console.warn("WARNING: GITHUB_TOKEN is not set.");
163+
if (!NOTION_TOKEN) console.warn("WARNING: NOTION_TOKEN is not set. Notion tools will fail.");
164+
if (!MEM0_API_KEY) console.warn("WARNING: MEM0_API_KEY is not set. Mem0 tools will fail.");
165+
if (!CLOUDFLARE_API_TOKEN || !CLOUDFLARE_ACCOUNT_ID) console.warn("WARNING: CLOUDFLARE_API_TOKEN/CLOUDFLARE_ACCOUNT_ID not set. Cloudflare tools will fail.");
166+
if (!CONTEXT7_API_KEY) console.warn("NOTE: CONTEXT7_API_KEY is not set. Context7 tools will work but at lower, unauthenticated rate limits.");
167+
if (!GEMINI_API_KEY) console.warn("WARNING: GEMINI_API_KEY is not set. Gemini tools (delegate_research) will fail.");
168+
if (!MCP_SHARED_KEY) console.warn("WARNING: MCP_SHARED_KEY is not set. The /mcp, /mcp/:key, and / endpoints are OPEN to anyone who has the URL.");
169+
if (!GITHUB_APP_ID || !GITHUB_APP_INSTALLATION_ID || !GITHUB_APP_PRIVATE_KEY) console.warn("NOTE: GITHUB_APP_ID/GITHUB_APP_INSTALLATION_ID/GITHUB_APP_PRIVATE_KEY not fully set. get_repo_clone_token (private-repo sandbox clone) will fail until the GitHub App is configured.");
170+
console.log(`IP allowlist: ${IP_ALLOWLIST_ENABLED ? `ENABLED (${ALLOWED_IP_RANGES.join(", ")})` : "DISABLED"}`);
171+
});
172+
}
173+
174+
export { app, mcpServer };

test/mcp-integration.test.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// ---------------------------------------------------------------------------
2+
// test/mcp-integration.test.js
3+
// Exercises a real tool (get_repo) through the actual mcpServer instance and
4+
// real Zod schema validation, over an InMemoryTransport pair -- not a mock
5+
// of server.tool() or a hand-rolled call to the handler function directly.
6+
// This is the thing that would actually catch a zod 3->4 regression: a
7+
// breaking change in how zod parses/coerces args would surface here as
8+
// either a validation error on VALID input, or a non-validation error on
9+
// INVALID input, not as a normal Vitest assertion mismatch elsewhere.
10+
// ---------------------------------------------------------------------------
11+
12+
process.env.NODE_ENV = "test";
13+
14+
import { describe, it, expect, beforeAll } from "vitest";
15+
import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
16+
import { Client } from "@modelcontextprotocol/sdk/client/index.js";
17+
import { mcpServer } from "../server.js";
18+
19+
describe("MCP tool call — real Zod validation path (get_repo)", () => {
20+
let client;
21+
22+
beforeAll(async () => {
23+
const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair();
24+
client = new Client({ name: "mcp-integration-test", version: "1.0.0" });
25+
await mcpServer.connect(serverTransport);
26+
await client.connect(clientTransport);
27+
});
28+
29+
it("VALID args pass Zod and reach the handler (fails downstream on missing GITHUB_TOKEN, not on validation)", async () => {
30+
// No GITHUB_TOKEN is set in this test run (it's a CI secret, not assumed
31+
// available here), so the handler itself throws once it tries to call
32+
// out. That's the point: reaching that error at all proves { owner,
33+
// repo } passed Zod parsing/coercion and were handed to the handler.
34+
const result = await client.callTool({
35+
name: "get_repo",
36+
arguments: { owner: "allocsys", repo: "madmcp" },
37+
});
38+
39+
expect(result.isError).toBe(true);
40+
const text = result.content[0].text;
41+
expect(text).toMatch(/GITHUB_TOKEN/);
42+
// Must NOT look like a schema/validation rejection.
43+
expect(text).not.toMatch(/Invalid arguments/i);
44+
expect(text).not.toMatch(/-32602/);
45+
});
46+
47+
it("INVALID args (missing required `repo`) are rejected at the validation layer, never reaching the handler", async () => {
48+
const result = await client.callTool({
49+
name: "get_repo",
50+
arguments: { owner: "allocsys" }, // `repo` omitted -- required by the schema
51+
});
52+
53+
expect(result.isError).toBe(true);
54+
const text = result.content[0].text;
55+
// Must look like a schema/validation rejection...
56+
expect(text).toMatch(/Invalid arguments/i);
57+
expect(text).toMatch(/-32602/);
58+
// ...and must NOT be the downstream GITHUB_TOKEN error -- if it were,
59+
// that would mean bad input reached the handler instead of being
60+
// stopped by Zod.
61+
expect(text).not.toMatch(/GITHUB_TOKEN/);
62+
});
63+
});

test/server-e2e.test.js

Lines changed: 92 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,92 @@
1+
// ---------------------------------------------------------------------------
2+
// test/server-e2e.test.js
3+
// Drives the real Express `app` (exported from server.js) through supertest:
4+
// actual route + middleware chain (mcpLimiter -> requireMcpKey ->
5+
// requireAllowedIp -> handler), not a mock of any of it.
6+
//
7+
// config.js reads its env vars at import time, so the relevant env vars are
8+
// set here BEFORE server.js (and therefore config.js) is imported, via a
9+
// dynamic import.
10+
// ---------------------------------------------------------------------------
11+
12+
import { describe, it, expect, beforeAll, vi } from "vitest";
13+
14+
process.env.NODE_ENV = "test";
15+
process.env.MCP_SHARED_KEY = "test-shared-key-for-e2e";
16+
process.env.IP_ALLOWLIST_ENABLED = "true";
17+
process.env.ALLOWED_IP_RANGES = "203.0.113.0/24";
18+
process.env.TRUST_PROXY_HOPS = "1";
19+
20+
const ALLOWED_IP = "203.0.113.42"; // inside 203.0.113.0/24
21+
const DISALLOWED_IP = "198.51.100.7"; // outside the allowed CIDR
22+
const VALID_KEY = process.env.MCP_SHARED_KEY;
23+
24+
let app;
25+
let request;
26+
27+
beforeAll(async () => {
28+
({ app } = await import("../server.js"));
29+
({ default: request } = await import("supertest"));
30+
});
31+
32+
describe("GET /health", () => {
33+
it("returns 200 { status: 'ok' } with no auth required", async () => {
34+
const res = await request(app).get("/health");
35+
expect(res.status).toBe(200);
36+
expect(res.body).toEqual({ status: "ok" });
37+
});
38+
});
39+
40+
describe("POST /mcp — auth + IP allowlist ordering", () => {
41+
it("returns 401 when no key is provided, even from an allowlisted IP", async () => {
42+
// requireMcpKey runs before requireAllowedIp, so a missing key always
43+
// short-circuits first regardless of IP.
44+
const res = await request(app)
45+
.post("/mcp")
46+
.set("X-Forwarded-For", ALLOWED_IP)
47+
.send({ jsonrpc: "2.0", method: "initialize", id: 1 });
48+
49+
expect(res.status).toBe(401);
50+
});
51+
52+
it("returns 403 when a valid key is provided from an IP outside the allowed CIDR", async () => {
53+
const res = await request(app)
54+
.post("/mcp")
55+
.set("x-manufact-key", VALID_KEY)
56+
.set("X-Forwarded-For", DISALLOWED_IP)
57+
.send({ jsonrpc: "2.0", method: "initialize", id: 1 });
58+
59+
expect(res.status).toBe(403);
60+
});
61+
});
62+
63+
describe("POST /mcp — rate limiting", () => {
64+
let freshApp;
65+
66+
beforeAll(async () => {
67+
// The earlier describe blocks already sent a couple of requests through
68+
// the shared `app` singleton's mcpLimiter, so re-importing it here would
69+
// start this test partway into that quota. vi.resetModules() forces a
70+
// brand-new module graph (and therefore a brand-new express-rate-limit
71+
// instance with its own untouched counter) isolated from those tests.
72+
vi.resetModules();
73+
({ app: freshApp } = await import("../server.js"));
74+
});
75+
76+
it("allows 30 unauthenticated requests then returns 429 on the 31st", async () => {
77+
// mcpLimiter is the first middleware in the chain, so it still counts
78+
// requests that go on to fail auth. Sending them with no key keeps each
79+
// one cheap (short-circuits at the 401 stage) instead of invoking the
80+
// real MCP handler 30 times.
81+
const statuses = [];
82+
for (let i = 0; i < 31; i++) {
83+
const res = await request(freshApp)
84+
.post("/mcp")
85+
.send({ jsonrpc: "2.0", method: "initialize", id: i });
86+
statuses.push(res.status);
87+
}
88+
89+
expect(statuses.slice(0, 30)).toEqual(Array(30).fill(401));
90+
expect(statuses[30]).toBe(429);
91+
}, 20000);
92+
});

0 commit comments

Comments
 (0)