Skip to content

fix(mcp): dispatch enforces none of the published tool schemas — every enum/min/max is decorative at runtime #8942

Description

@JSONbored

Context

validateToolArguments (src/mcp-server.ts, the single dispatch call site in dispatchTool) is the
only argument validation the MCP server performs. It checks exactly two things: that args is a
non-array object, and that it carries no unknown keys when the tool's inputSchema sets
additionalProperties: false.

function validateToolArguments(tool: Row, args: Row) {
  if (args === undefined || args === null) return {};
  if (
    typeof args !== "object" ||
    Array.isArray(args) ||
    (tool.inputSchema?.additionalProperties === false &&
      Object.keys(args).some(
        (key) => !Object.hasOwn(tool.inputSchema?.properties ?? {}, key),
      ))
  ) {
    throw toolError("invalid_params", `Invalid arguments for tool ${tool.name}.`);
  }
  return args;
}

No type check, no enum check, no required check, no numeric bounds. Every z.enum(...),
z.int().min(1), .max(200), and required across all 200+ entries in MCP_TOOLS is therefore
decorative at runtime — honoured only by clients that validate against the published schema, and
by whatever ad-hoc guard the individual handler happens to have written by hand.

#8804 fixed one instance of the consequence (network reached networkArtifactPath unvalidated and
silently served the testnet registry) by wiring that one argument to the existing optionalEnum
guard. That fix is correct and complete for network, but it is per-handler: the next argument that
skips its guard fails the same way. #8804's Requirement 5 asked for an explicit decision on
dispatch-level enforcement and it was answered "no, not in that PR", with the audit below named
as the reason. This issue is that audit.

Why this is maintainer-only

Turning enforcement on is a behaviour change to a public agent-facing surface, across 206 tools at
once. Handlers are deliberately lenient in places — clampLimit accepts and clamps a limit the
schema forbids; optionalEnum accepts "" where a published enum does not — so schema-driven
validation converts an unknown number of currently-succeeding agent calls into hard invalid_params.
The size of that set is the whole question, and it has to be measured before anything is enforced.

Requirements

  1. Audit first, enforce second — and the audit is the deliverable that gates the rest. Produce a
    per-tool table of every argument whose runtime behaviour would change under schema enforcement.
    Mechanical way to get it: for each tool, drive its handler with arguments that the published
    inputSchema rejects but the handler currently accepts (out-of-enum strings, out-of-range
    integers, wrong types, omitted required keys) and record which ones return a result today.
  2. Then choose the enforcement scope, with the audit as evidence:
    • (A) Full schema enforcement — validate against the source Zod schema at dispatch. Cheaper
      than re-walking the derived JSON Schema, but MCP_TOOLS carries only the
      z.toJSONSchema-derived object, so the Zod schemas must be threaded through all 206 entries.
    • (B) Enum-only enforcement — reject a provided string argument that is not a member of
      inputSchema.properties[key].enum, continuing to accept absent/"" exactly as optionalEnum
      does. Needs no Zod threading (the JSON Schema is already at dispatch) and closes the whole
      fix(mcp): network arg is never validated at runtime — MCP silently serves the testnet registry as mainnet #8804 bug class, but still changes behaviour for any handler that today tolerates an
      out-of-enum value by falling back to a default — which is what the audit must enumerate.
    • (C) Keep per-handler guards, and add a CI gate instead — a test asserting that every tool
      argument whose schema declares an enum is resolved through optionalEnum (or an equivalent
      guard) in its handler. Prevents the next fix(mcp): network arg is never validated at runtime — MCP silently serves the testnet registry as mainnet #8804 without changing any runtime behaviour.
  3. Whichever is chosen, the leniency that survives must be deliberate and named. clampLimit's
    clamping and optionalEnum's "" acceptance are load-bearing for real agent callers; if they
    stay, say so in a comment at the enforcement site rather than leaving them as accidents.
  4. No silent narrowing. A tool that starts rejecting a value it accepted yesterday must appear in
    the PR body's list, with the value and the previous behaviour.

Deliverables

  • The per-tool behaviour-change audit (Requirement 1), committed as a test or a doc, not just
    posted in a PR comment — it is the artifact the next person needs.
  • The chosen option implemented, with every behaviour change from the audit listed in the PR body.
  • A regression test that a schema-invalid argument is rejected for at least one tool of each
    constraint kind the chosen option covers (enum, bound, type, required).
  • Patch coverage: every changed line and branch in src/** / workers/** is covered —
    codecov/patch enforces target: 99%, threshold: 0%. Measure unsharded with
    npm run test:coverage.

Expected Outcome

Either the MCP dispatch enforces the schemas it publishes — with every tool whose behaviour changed
named explicitly — or it deliberately does not, and a CI gate makes the next unguarded enum argument
fail in CI rather than in production. In both cases the gap between "what the tool listing advertises"
and "what the runtime accepts" stops being invisible.

Links & Resources

Metadata

Metadata

Assignees

Labels

backendmaintainer-onlyOwner-only work — yields no Gittensor points.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions