CI: add MCP tool + e2e integration test coverage (Zod, Express, rate-limit)

package.json

@@ -20,8 +20,9 @@
     "zod": "^3.24.1"
   },
   "devDependencies": {
-    "vitest": "^4.1.10",
+    "@eslint/js": "^9.17.0",
     "eslint": "^9.17.0",
-    "@eslint/js": "^9.17.0"
+    "supertest": "^7.2.2",
+    "vitest": "^4.1.10"
   }
 }

server.js

@@ -153,15 +153,22 @@ app.post("/mcp", mcpLimiter, requireMcpKey, requireAllowedIp, handleMcp);
 app.post("/mcp/:key", mcpLimiter, requireMcpKey, requireAllowedIp, handleMcp);
 
 const PORT = process.env.PORT || 8080;
-app.listen(PORT, () => {
-  console.log(`madmcp-server v2.1.0 listening on port ${PORT}`);
-  if (!GITHUB_TOKEN)   console.warn("WARNING: GITHUB_TOKEN is not set.");
-  if (!NOTION_TOKEN)   console.warn("WARNING: NOTION_TOKEN is not set. Notion tools will fail.");
-  if (!MEM0_API_KEY)   console.warn("WARNING: MEM0_API_KEY is not set. Mem0 tools will fail.");
-  if (!CLOUDFLARE_API_TOKEN || !CLOUDFLARE_ACCOUNT_ID) console.warn("WARNING: CLOUDFLARE_API_TOKEN/CLOUDFLARE_ACCOUNT_ID not set. Cloudflare tools will fail.");
-  if (!CONTEXT7_API_KEY) console.warn("NOTE: CONTEXT7_API_KEY is not set. Context7 tools will work but at lower, unauthenticated rate limits.");
-  if (!GEMINI_API_KEY) console.warn("WARNING: GEMINI_API_KEY is not set. Gemini tools (delegate_research) will fail.");
-  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.");
-  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.");
-  console.log(`IP allowlist: ${IP_ALLOWLIST_ENABLED ? `ENABLED (${ALLOWED_IP_RANGES.join(", ")})` : "DISABLED"}`);
-});
+// Gated so importing this module (e.g. from tests via supertest, or the MCP
+// integration test's InMemoryTransport) never binds a real port. Tests set
+// NODE_ENV=test before importing server.js.
+if (process.env.NODE_ENV !== "test") {
+  app.listen(PORT, () => {
+    console.log(`madmcp-server v2.1.0 listening on port ${PORT}`);
+    if (!GITHUB_TOKEN)   console.warn("WARNING: GITHUB_TOKEN is not set.");
+    if (!NOTION_TOKEN)   console.warn("WARNING: NOTION_TOKEN is not set. Notion tools will fail.");
+    if (!MEM0_API_KEY)   console.warn("WARNING: MEM0_API_KEY is not set. Mem0 tools will fail.");
+    if (!CLOUDFLARE_API_TOKEN || !CLOUDFLARE_ACCOUNT_ID) console.warn("WARNING: CLOUDFLARE_API_TOKEN/CLOUDFLARE_ACCOUNT_ID not set. Cloudflare tools will fail.");
+    if (!CONTEXT7_API_KEY) console.warn("NOTE: CONTEXT7_API_KEY is not set. Context7 tools will work but at lower, unauthenticated rate limits.");
+    if (!GEMINI_API_KEY) console.warn("WARNING: GEMINI_API_KEY is not set. Gemini tools (delegate_research) will fail.");
+    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.");
+    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.");
+    console.log(`IP allowlist: ${IP_ALLOWLIST_ENABLED ? `ENABLED (${ALLOWED_IP_RANGES.join(", ")})` : "DISABLED"}`);
+  });
+}
+
+export { app, mcpServer };

test/mcp-integration.test.js

@@ -0,0 +1,63 @@
+// ---------------------------------------------------------------------------
+// test/mcp-integration.test.js
+// Exercises a real tool (get_repo) through the actual mcpServer instance and
+// real Zod schema validation, over an InMemoryTransport pair -- not a mock
+// of server.tool() or a hand-rolled call to the handler function directly.
+// This is the thing that would actually catch a zod 3->4 regression: a
+// breaking change in how zod parses/coerces args would surface here as
+// either a validation error on VALID input, or a non-validation error on
+// INVALID input, not as a normal Vitest assertion mismatch elsewhere.
+// ---------------------------------------------------------------------------
+
+process.env.NODE_ENV = "test";
+
+import { describe, it, expect, beforeAll } from "vitest";
+import { InMemoryTransport } from "@modelcontextprotocol/sdk/inMemory.js";
+import { Client } from "@modelcontextprotocol/sdk/client/index.js";
+import { mcpServer } from "../server.js";
+
+describe("MCP tool call — real Zod validation path (get_repo)", () => {
+  let client;
+
+  beforeAll(async () => {
+    const [serverTransport, clientTransport] = InMemoryTransport.createLinkedPair();
+    client = new Client({ name: "mcp-integration-test", version: "1.0.0" });
+    await mcpServer.connect(serverTransport);
+    await client.connect(clientTransport);
+  });
+
+  it("VALID args pass Zod and reach the handler (fails downstream on missing GITHUB_TOKEN, not on validation)", async () => {
+    // No GITHUB_TOKEN is set in this test run (it's a CI secret, not assumed
+    // available here), so the handler itself throws once it tries to call
+    // out. That's the point: reaching that error at all proves { owner,
+    // repo } passed Zod parsing/coercion and were handed to the handler.
+    const result = await client.callTool({
+      name: "get_repo",
+      arguments: { owner: "allocsys", repo: "madmcp" },
+    });
+
+    expect(result.isError).toBe(true);
+    const text = result.content[0].text;
+    expect(text).toMatch(/GITHUB_TOKEN/);
+    // Must NOT look like a schema/validation rejection.
+    expect(text).not.toMatch(/Invalid arguments/i);
+    expect(text).not.toMatch(/-32602/);
+  });
+
+  it("INVALID args (missing required `repo`) are rejected at the validation layer, never reaching the handler", async () => {
+    const result = await client.callTool({
+      name: "get_repo",
+      arguments: { owner: "allocsys" }, // `repo` omitted -- required by the schema
+    });
+
+    expect(result.isError).toBe(true);
+    const text = result.content[0].text;
+    // Must look like a schema/validation rejection...
+    expect(text).toMatch(/Invalid arguments/i);
+    expect(text).toMatch(/-32602/);
+    // ...and must NOT be the downstream GITHUB_TOKEN error -- if it were,
+    // that would mean bad input reached the handler instead of being
+    // stopped by Zod.
+    expect(text).not.toMatch(/GITHUB_TOKEN/);
+  });
+});

test/server-e2e.test.js

@@ -0,0 +1,92 @@
+// ---------------------------------------------------------------------------
+// test/server-e2e.test.js
+// Drives the real Express `app` (exported from server.js) through supertest:
+// actual route + middleware chain (mcpLimiter -> requireMcpKey ->
+// requireAllowedIp -> handler), not a mock of any of it.
+//
+// config.js reads its env vars at import time, so the relevant env vars are
+// set here BEFORE server.js (and therefore config.js) is imported, via a
+// dynamic import.
+// ---------------------------------------------------------------------------
+
+import { describe, it, expect, beforeAll, vi } from "vitest";
+
+process.env.NODE_ENV = "test";
+process.env.MCP_SHARED_KEY = "test-shared-key-for-e2e";
+process.env.IP_ALLOWLIST_ENABLED = "true";
+process.env.ALLOWED_IP_RANGES = "203.0.113.0/24";
+process.env.TRUST_PROXY_HOPS = "1";
+
+const ALLOWED_IP    = "203.0.113.42"; // inside 203.0.113.0/24
+const DISALLOWED_IP = "198.51.100.7"; // outside the allowed CIDR
+const VALID_KEY     = process.env.MCP_SHARED_KEY;
+
+let app;
+let request;
+
+beforeAll(async () => {
+  ({ app } = await import("../server.js"));
+  ({ default: request } = await import("supertest"));
+});
+
+describe("GET /health", () => {
+  it("returns 200 { status: 'ok' } with no auth required", async () => {
+    const res = await request(app).get("/health");
+    expect(res.status).toBe(200);
+    expect(res.body).toEqual({ status: "ok" });
+  });
+});
+
+describe("POST /mcp — auth + IP allowlist ordering", () => {
+  it("returns 401 when no key is provided, even from an allowlisted IP", async () => {
+    // requireMcpKey runs before requireAllowedIp, so a missing key always
+    // short-circuits first regardless of IP.
+    const res = await request(app)
+      .post("/mcp")
+      .set("X-Forwarded-For", ALLOWED_IP)
+      .send({ jsonrpc: "2.0", method: "initialize", id: 1 });
+
+    expect(res.status).toBe(401);
+  });
+
+  it("returns 403 when a valid key is provided from an IP outside the allowed CIDR", async () => {
+    const res = await request(app)
+      .post("/mcp")
+      .set("x-manufact-key", VALID_KEY)
+      .set("X-Forwarded-For", DISALLOWED_IP)
+      .send({ jsonrpc: "2.0", method: "initialize", id: 1 });
+
+    expect(res.status).toBe(403);
+  });
+});
+
+describe("POST /mcp — rate limiting", () => {
+  let freshApp;
+
+  beforeAll(async () => {
+    // The earlier describe blocks already sent a couple of requests through
+    // the shared `app` singleton's mcpLimiter, so re-importing it here would
+    // start this test partway into that quota. vi.resetModules() forces a
+    // brand-new module graph (and therefore a brand-new express-rate-limit
+    // instance with its own untouched counter) isolated from those tests.
+    vi.resetModules();
+    ({ app: freshApp } = await import("../server.js"));
+  });
+
+  it("allows 30 unauthenticated requests then returns 429 on the 31st", async () => {
+    // mcpLimiter is the first middleware in the chain, so it still counts
+    // requests that go on to fail auth. Sending them with no key keeps each
+    // one cheap (short-circuits at the 401 stage) instead of invoking the
+    // real MCP handler 30 times.
+    const statuses = [];
+    for (let i = 0; i < 31; i++) {
+      const res = await request(freshApp)
+        .post("/mcp")
+        .send({ jsonrpc: "2.0", method: "initialize", id: i });
+      statuses.push(res.status);
+    }
+
+    expect(statuses.slice(0, 30)).toEqual(Array(30).fill(401));
+    expect(statuses[30]).toBe(429);
+  }, 20000);
+});