fix(mcp-bridge): harden argument handling for routed tools

.github/workflows/e2e.yml

@@ -210,16 +210,16 @@ jobs:
       matrix:
         include:
           - runtime: node
-            unit: node --test mcp-bridge/tests/dispatch_test.js mcp-bridge/tests/http_transport_test.js mcp-bridge/tests/path_claims_test.js
+            unit: node --test mcp-bridge/tests/dispatch_test.js mcp-bridge/tests/http_transport_test.js mcp-bridge/tests/path_claims_test.js mcp-bridge/tests/routing_args_test.js
             boot: node mcp-bridge/main.js
           - runtime: deno
-            unit: deno test --allow-read --allow-env --allow-run --allow-net mcp-bridge/tests/dispatch_test.js mcp-bridge/tests/http_transport_test.js mcp-bridge/tests/path_claims_test.js
+            unit: deno test --allow-read --allow-env --allow-run --allow-net mcp-bridge/tests/dispatch_test.js mcp-bridge/tests/http_transport_test.js mcp-bridge/tests/path_claims_test.js mcp-bridge/tests/routing_args_test.js
             # Scoped perms matching main.js's shebang, NOT -A: the boot smoke
             # must exercise the exact permission set a real install uses, or it
             # would mask a missing-grant bug that scoped users would hit.
             boot: deno run --allow-net --allow-env --allow-read mcp-bridge/main.js
           - runtime: bun
-            unit: bun test mcp-bridge/tests/dispatch_test.js mcp-bridge/tests/http_transport_test.js mcp-bridge/tests/path_claims_test.js
+            unit: bun test mcp-bridge/tests/dispatch_test.js mcp-bridge/tests/http_transport_test.js mcp-bridge/tests/path_claims_test.js mcp-bridge/tests/routing_args_test.js
             boot: bun mcp-bridge/main.js
 
     steps:

mcp-bridge/lib/dispatcher.js

@@ -53,6 +53,64 @@ function rpcError(id, code, message) {
   return { jsonrpc: "2.0", id, error: { code, message: sanitizeErrorMessage(message) } };
 }
 
+// Tools that share one cartridge and are told apart by a routing key
+// derived from the tool name. The bridge sets the key, never the caller:
+// dispatch writes it last, and the gate refuses any argument the tool's
+// inputSchema does not declare.
+const ROUTED_TOOLS = new Map([
+  ...["verpex", "cloudflare", "vercel"].map((p) => [`boj_cloud_${p}`, { cartridge: "cloud-mcp", key: "provider", value: p }]),
+  ...["gmail", "calendar"].map((p) => [`boj_comms_${p}`, { cartridge: "comms-mcp", key: "provider", value: p }]),
+  ["boj_ml_huggingface", { cartridge: "ml-mcp", key: "provider", value: "huggingface" }],
+  ...["navigate", "click", "type", "read_page", "screenshot", "tabs", "execute_js"].map((a) => [`boj_browser_${a}`, { cartridge: "browser-mcp", key: "action", value: a }]),
+]);
+
+let declaredArgsCache = null;
+
+/**
+ * Return the argument names a tool's inputSchema declares, or null when
+ * the tool is not in the full tool list.
+ *
+ * @param {string} toolName
+ * @returns {Set<string>|null}
+ */
+function declaredArgs(toolName) {
+  if (!declaredArgsCache) {
+    declaredArgsCache = new Map(
+      buildToolList("full").map((t) => [t.name, new Set(Object.keys(t.inputSchema?.properties ?? {}))]),
+    );
+  }
+  return declaredArgsCache.get(toolName) ?? null;
+}
+
+/**
+ * Check a routed tool's arguments against its inputSchema. Other tools
+ * are not checked here.
+ *
+ * @param {string} toolName
+ * @param {Record<string, unknown>} args
+ * @returns {string|null} error message, or null when every argument is declared
+ */
+function validateRoutedArgs(toolName, args) {
+  const route = ROUTED_TOOLS.get(toolName);
+  if (!route) return null;
+  const allowed = declaredArgs(toolName);
+  if (!allowed) return "Unknown tool";
+  for (const name of Object.keys(args)) {
+    if (name === route.key || !allowed.has(name)) {
+      return `Unexpected argument '${name}' for ${toolName}`;
+    }
+  }
+  return null;
+}
+
+/**
+ * Pre-dispatch checks for a tools/call: rate limit, tool-name shape,
+ * argument size, injection scan, and per-tool argument validation.
+ *
+ * @param {string} toolName
+ * @param {Record<string, unknown>} args
+ * @returns {{code: number, message: string}|null} a JSON-RPC error, or null to proceed
+ */
 function hardeningGate(toolName, args) {
   if (!rateLimitAllow()) {
     return { code: -32000, message: "Rate limit exceeded. Max " + RATE_LIMIT + " tool calls per minute." };
@@ -72,6 +130,14 @@ function hardeningGate(toolName, args) {
     warn("Injection warning", { tool: toolName, confidence: injectionLevel });
   }
 
+  if (args === null || typeof args !== "object" || Array.isArray(args)) {
+    return { code: -32602, message: "Tool arguments must be an object" };
+  }
+  const routedError = validateRoutedArgs(toolName, args);
+  if (routedError) {
+    return { code: -32602, message: routedError };
+  }
+
   let validationError = null;
   if (toolName === "boj_cartridge_info" || toolName === "boj_cartridge_invoke") {
     validationError = validateRequiredStrings(args, ["name"]);
@@ -103,7 +169,22 @@ function hardeningGate(toolName, args) {
   return null;
 }
 
+/**
+ * Dispatch a gated tools/call to its handler.
+ *
+ * Routed tools (see ROUTED_TOOLS) go to their shared cartridge with the
+ * routing key written after the caller's arguments, so the tool name
+ * alone decides the route. Returns null for an unknown tool.
+ *
+ * @param {string} toolName
+ * @param {Record<string, unknown>} args
+ * @returns {Promise<object|null>}
+ */
 async function dispatchTool(toolName, args) {
+  const route = ROUTED_TOOLS.get(toolName);
+  if (route) {
+    return invokeCartridge(route.cartridge, { ...args, [route.key]: route.value });
+  }
   switch (toolName) {
     case "boj_health":
       return fetchHealth();
@@ -116,26 +197,6 @@ async function dispatchTool(toolName, args) {
     case "boj_cartridge_invoke":
       return invokeCartridge(args.name, args.params);
 
-    case "boj_cloud_verpex":
-    case "boj_cloud_cloudflare":
-    case "boj_cloud_vercel":
-      return invokeCartridge("cloud-mcp", { provider: toolName.replace("boj_cloud_", ""), ...args });
-
-    case "boj_comms_gmail":
-    case "boj_comms_calendar":
-      return invokeCartridge("comms-mcp", { provider: toolName.replace("boj_comms_", ""), ...args });
-
-    case "boj_ml_huggingface":
-      return invokeCartridge("ml-mcp", { provider: "huggingface", ...args });
-
-    case "boj_browser_navigate":
-    case "boj_browser_click":
-    case "boj_browser_type":
-    case "boj_browser_read_page":
-    case "boj_browser_screenshot":
-    case "boj_browser_tabs":
-    case "boj_browser_execute_js":
-      return invokeCartridge("browser-mcp", { action: toolName.replace("boj_browser_", ""), ...args });
 
     case "boj_github_list_repos":
     case "boj_github_get_repo":

mcp-bridge/tests/routing_args_test.js

@@ -0,0 +1,85 @@
+// SPDX-License-Identifier: MPL-2.0
+// Copyright (c) Jonathan D.A. Jewell <j.d.a.jewell@open.ac.uk>
+//
+// BoJ Server — routed-tool argument tests
+//
+// The browser / cloud / comms / ml tools are routed to a shared
+// cartridge with a routing key (`action` or `provider`) derived from
+// the tool name. These tests pin two properties:
+//   1. arguments outside a routed tool's inputSchema are refused, and
+//   2. the routing key the cartridge receives always matches the tool
+//      that was called.
+// `fetch` is stubbed so no backend is needed.
+//
+// Run: node --test mcp-bridge/tests/routing_args_test.js
+
+import { after, before, test } from "node:test";
+import assert from "node:assert/strict";
+
+import { dispatchMcpMessage } from "../lib/dispatcher.js";
+
+// Some runners (bun) share one process across test files, so the stub is
+// installed for this file only and the real fetch is put back afterwards.
+const posted = [];
+const realFetch = globalThis.fetch;
+before(() => {
+  globalThis.fetch = async (url, init) => {
+    posted.push({ url: String(url), body: init?.body ? JSON.parse(init.body) : null });
+    return { ok: true, status: 200, json: async () => ({ ok: true }) };
+  };
+});
+after(() => {
+  globalThis.fetch = realFetch;
+});
+
+let nextId = 1;
+/** Issue one tools/call and return the JSON-RPC response plus what was posted. */
+async function call(name, args) {
+  posted.length = 0;
+  const res = await dispatchMcpMessage({
+    jsonrpc: "2.0",
+    id: nextId++,
+    method: "tools/call",
+    params: { name, arguments: args },
+  });
+  return { res, sent: [...posted] };
+}
+
+const ROUTED = [
+  ["boj_browser_read_page", {}, "action", "read_page"],
+  ["boj_browser_screenshot", {}, "action", "screenshot"],
+  ["boj_browser_navigate", { url: "https://example.org/" }, "action", "navigate"],
+  ["boj_cloud_verpex", { operation: "list" }, "provider", "verpex"],
+  ["boj_cloud_vercel", { operation: "list" }, "provider", "vercel"],
+  ["boj_comms_calendar", { operation: "list" }, "provider", "calendar"],
+  ["boj_ml_huggingface", { operation: "list" }, "provider", "huggingface"],
+];
+
+for (const [tool, args, key, expected] of ROUTED) {
+  test(`${tool}: a well-formed call reaches the cartridge with ${key}=${expected}`, async () => {
+    const { res, sent } = await call(tool, args);
+    assert.equal(res.error, undefined, JSON.stringify(res.error));
+    assert.equal(sent.length, 1);
+    assert.equal(sent[0].body[key], expected);
+  });
+
+  test(`${tool}: an argument named '${key}' cannot change the routing`, async () => {
+    const { res, sent } = await call(tool, { ...args, [key]: "something-else" });
+    for (const s of sent) assert.equal(s.body[key], expected, "routing key was overridden");
+    assert.ok(res.error, "expected the call to be refused");
+    assert.equal(res.error.code, -32602);
+  });
+}
+
+test("routed tools refuse arguments outside their inputSchema", async () => {
+  const { res, sent } = await call("boj_browser_screenshot", { not_in_schema: 1 });
+  assert.ok(res.error, "expected the call to be refused");
+  assert.equal(res.error.code, -32602);
+  assert.equal(sent.length, 0);
+});
+
+test("non-routed tools keep their existing argument handling", async () => {
+  // coord_send reads `sender_role`, which no inputSchema declares.
+  const { res } = await call("coord_send", { message: "hi", target: "peer", sender_role: "worker" });
+  assert.equal(res.error, undefined, JSON.stringify(res.error));
+});

package.json

@@ -20,7 +20,7 @@
   },
   "scripts": {
     "start": "deno run -A mcp-bridge/main.js",
-    "test": "node --test mcp-bridge/tests/dispatch_test.js"
+    "test": "node --test mcp-bridge/tests/dispatch_test.js mcp-bridge/tests/routing_args_test.js"
   },
   "engines": {
     "node": ">=18.0.0"