fix(mcp): reject undeclared tool arguments

Previously, tools/list left top-level input objects open and cbm_mcp_handle_tool dispatched raw JSON without checking property names. A search_code call containing repo_path could therefore ignore the caller's intended project and return a plausible empty result. The canonical schemas also omitted the implemented index_repository.format and index_status.verbose inputs.

Add mcp_add_tool_input_schema to close every classic, streamlined pre-reveal, streamlined post-reveal, and _hidden_tools input schema. Add mcp_validate_tool_argument_keys before dispatch so direct, stdio, HTTP, and raw-JSON CLI calls return an isError:true result naming the unknown key and canonical supported keys. Keep the public schema registry as the property authority; output schemas remain open.

Preserve only verified, unambiguous raw-JSON compatibility: string-valued project_name/project_id/projectName map to project; query_graph.cypher maps to query per 32a3820c; search_code search_in=source remains from the 3d133a3c to 39539eff split. Other search_in values name search_graph, and ignored query_graph label input is rejected.

MCP Tool Execution Error behavior follows https://modelcontextprotocol.io/specification/2025-11-25/server/tools. Closed no-argument schemas follow https://github.com/modelcontextprotocol/modelcontextprotocol/issues/1613.

Verification: ASan/UBSan MCP 267/267; TSan MCP 267/267; CLI 232/232; tool consolidation 113/113; HTTP 44/44 with one platform skip; production smoke 35/35; four focused macOS leaks checks each 0 leaks/0 bytes; scripts/check-source-safety.sh; optimized real-store CLI query_graph and typo-rejection dogfood.
Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This commit is contained in:
Andrew Hundt
2026-07-19 21:46:24 -04:00
parent fced498a32
commit e9f3ffa88b
2 changed files with 291 additions and 5 deletions
+167 -5
View File
@@ -1063,6 +1063,9 @@ static const tool_def_t TOOLS[] = {
"\"persistence\":{\"type\":\"boolean\",\"default\":false,\"description\":"
"\"Write compressed artifact to .codebase-memory/graph.db.zst for team sharing. "
"Teammates can bootstrap from the artifact instead of full re-indexing.\"}"
",\"format\":{\"type\":\"string\",\"enum\":[\"toon\",\"json\"],\"default\":\"toon\","
"\"description\":\"Compact TOON by default; json returns legacy objects. Omit to use "
"default_response_format.\"}"
"},\"required\":[\"repo_path\"]}"},
{"search_graph", "Search graph",
@@ -1315,7 +1318,9 @@ static const tool_def_t TOOLS[] = {
"overlay compaction state. Git metadata labels HEAD as committed-only and reports whether "
"the current working tree is dirty.",
"{\"type\":\"object\",\"properties\":{\"project\":{\"type\":\"string\",\"description\":"
"\"Indexed project name to inspect.\"}},\"required\":["
"\"Indexed project name to inspect.\"},\"verbose\":{\"type\":\"boolean\",\"default\":false,"
"\"description\":\"Include repository path and Git identity/freshness "
"metadata.\"}},\"required\":["
"\"project\"]}"},
{"check_index_coverage", "Check index coverage",
@@ -1434,6 +1439,7 @@ static const int STREAMLINED_TOOL_COUNT = sizeof(STREAMLINED_TOOLS) / sizeof(STR
static const char MCP_TOOL_OUTPUT_SCHEMA[] =
"{\"type\":\"object\",\"additionalProperties\":true}";
static const char MCP_HIDDEN_TOOL_INPUT_SCHEMA[] = "{\"type\":\"object\",\"properties\":{}}";
typedef struct {
const char *name;
@@ -1492,6 +1498,30 @@ static void mcp_add_json_schema(yyjson_mut_doc *doc, yyjson_mut_val *obj, const
yyjson_doc_free(schema_doc);
}
/* MCP inputs are closed globally: an undeclared key is almost always a typo
* that would otherwise leave a real option at its default and return a
* plausible but wrong result. Keep the property/required definitions in the
* canonical registry and apply this policy during serialization rather than
* duplicating it in every schema literal. Runtime dispatch enforces the same
* policy for raw-JSON CLI and clients that do not validate tools/list. */
static void mcp_add_tool_input_schema(yyjson_mut_doc *doc, yyjson_mut_val *obj,
const char *schema_json) {
if (!schema_json) {
return;
}
yyjson_doc *schema_doc = yyjson_read(schema_json, strlen(schema_json), 0);
if (!schema_doc) {
return;
}
yyjson_mut_val *schema = yyjson_val_mut_copy(doc, yyjson_doc_get_root(schema_doc));
if (schema && yyjson_mut_is_obj(schema)) {
(void)yyjson_mut_obj_remove_key(schema, "additionalProperties");
yyjson_mut_obj_add_bool(doc, schema, "additionalProperties", false);
yyjson_mut_obj_add_val(doc, obj, "inputSchema", schema);
}
yyjson_doc_free(schema_doc);
}
/* Canonical tool serialization for classic, streamlined, and paged lists. */
static void emit_tool(yyjson_mut_doc *doc, yyjson_mut_val *tools, const tool_def_t *tool_def,
const char *description_override) {
@@ -1501,7 +1531,7 @@ static void emit_tool(yyjson_mut_doc *doc, yyjson_mut_val *tools, const tool_def
tool_def->title ? tool_def->title : tool_def->name);
yyjson_mut_obj_add_str(doc, tool, "description",
description_override ? description_override : tool_def->description);
mcp_add_json_schema(doc, tool, "inputSchema", tool_def->input_schema);
mcp_add_tool_input_schema(doc, tool, tool_def->input_schema);
mcp_add_json_schema(doc, tool, "outputSchema", MCP_TOOL_OUTPUT_SCHEMA);
const tool_annotation_def_t *def = mcp_tool_annotations(tool_def->name);
yyjson_mut_val *annotations = yyjson_mut_obj(doc);
@@ -1526,7 +1556,9 @@ static bool is_streamlined_default_tool(const char *name) {
strcmp(name, "trace_path") == 0);
}
/* Return the same schema advertised by tools/list. Static lifetime; do not free. */
/* Return the canonical property and required definitions used by tools/list,
* CLI flag typing, and runtime key validation. tools/list additionally applies
* the global closed-input policy. Static lifetime; do not free. */
const char *cbm_mcp_tool_input_schema(const char *tool_name) {
if (!tool_name) {
return NULL;
@@ -1536,6 +1568,9 @@ const char *cbm_mcp_tool_input_schema(const char *tool_name) {
if (strcmp(tool_name, "trace_call_path") == 0) {
tool_name = "trace_path";
}
if (strcmp(tool_name, "_hidden_tools") == 0) {
return MCP_HIDDEN_TOOL_INPUT_SCHEMA;
}
for (int i = 0; i < TOOL_COUNT; i++) {
if (strcmp(TOOLS[i].name, tool_name) == 0) {
return TOOLS[i].input_schema;
@@ -1550,8 +1585,7 @@ const char *cbm_mcp_tool_input_schema(const char *tool_name) {
}
static bool mcp_tool_name_is_known(const char *tool_name) {
return cbm_mcp_tool_input_schema(tool_name) != NULL ||
(tool_name && strcmp(tool_name, "_hidden_tools") == 0);
return cbm_mcp_tool_input_schema(tool_name) != NULL;
}
static bool mcp_tool_allowed(cbm_mcp_tool_profile_t profile, const char *name) {
@@ -3001,6 +3035,7 @@ static char *cbm_mcp_tools_list_range(cbm_mcp_server_t *srv, int offset, int lim
yyjson_mut_obj_add_str(doc, hint_schema, "type", "object");
yyjson_mut_val *hint_props = yyjson_mut_obj(doc);
yyjson_mut_obj_add_val(doc, hint_schema, "properties", hint_props);
yyjson_mut_obj_add_bool(doc, hint_schema, "additionalProperties", false);
yyjson_mut_obj_add_val(doc, hint_tool, "inputSchema", hint_schema);
mcp_add_json_schema(doc, hint_tool, "outputSchema", MCP_TOOL_OUTPUT_SCHEMA);
yyjson_mut_arr_add_val(tools, hint_tool);
@@ -14951,6 +14986,129 @@ static char *build_hidden_tools_payload(cbm_mcp_server_t *srv) {
return out ? out : heap_strdup("{\"error\":\"failed to serialize hidden tools payload\"}");
}
/* Compatibility spellings accepted by older raw-JSON callers but deliberately
* omitted from tools/list and CLI help. Keep this list narrow and semantic:
* each entry must map unambiguously to a current canonical input.
*
* History: 3d133a3c added search_in to the former combined search tool;
* 39539eff split graph and source search into search_graph and search_code.
* Existing callers/tests still pass search_in="source", which is an exact
* no-op spelling of search_code's contract. 32a3820c accepted `cypher` before
* the query_graph schema standardized on `query`; those strings are exact
* aliases. Other values/keys are ambiguous and must fail with the canonical
* spelling or tool. Remove an exception only in an announced compatibility
* break after usage evidence shows it is unused. */
static bool mcp_legacy_argument_is_valid(const char *tool_name, yyjson_val *properties,
yyjson_val *args, const char *key,
bool *out_invalid_search_in) {
if (out_invalid_search_in) {
*out_invalid_search_in = false;
}
if (!tool_name || !properties || !args || !key) {
return false;
}
yyjson_val *value = yyjson_obj_get(args, key);
if (yyjson_obj_get(properties, "project") && yyjson_is_str(value) &&
(strcmp(key, "project_name") == 0 || strcmp(key, "project_id") == 0 ||
strcmp(key, "projectName") == 0)) {
return true;
}
if (strcmp(tool_name, "search_code") == 0 && strcmp(key, "search_in") == 0) {
bool is_source = yyjson_is_str(value) && strcmp(yyjson_get_str(value), "source") == 0;
if (!is_source && out_invalid_search_in) {
*out_invalid_search_in = true;
}
return is_source;
}
if (strcmp(tool_name, "query_graph") == 0 && strcmp(key, "cypher") == 0) {
return yyjson_is_str(value);
}
return false;
}
static char *mcp_validate_tool_argument_keys(const char *tool_name, const char *args_json) {
const char *schema_json = cbm_mcp_tool_input_schema(tool_name);
if (!schema_json) {
return NULL; /* unknown-tool handling remains a protocol-level authority */
}
const char *effective_args = args_json ? args_json : "{}";
yyjson_doc *args_doc = yyjson_read(effective_args, strlen(effective_args), 0);
yyjson_val *args = args_doc ? yyjson_doc_get_root(args_doc) : NULL;
if (!args || !yyjson_is_obj(args)) {
yyjson_doc_free(args_doc);
return cbm_mcp_text_result(
"tool arguments must be a JSON object; use tools/list or 'cli <tool> --help' "
"for the supported arguments",
true);
}
yyjson_doc *schema_doc = yyjson_read(schema_json, strlen(schema_json), 0);
yyjson_val *schema = schema_doc ? yyjson_doc_get_root(schema_doc) : NULL;
yyjson_val *properties = schema ? yyjson_obj_get(schema, "properties") : NULL;
if (!properties || !yyjson_is_obj(properties)) {
yyjson_doc_free(schema_doc);
yyjson_doc_free(args_doc);
return cbm_mcp_text_result("tool input schema is unavailable; retry after reinstalling",
true);
}
const char *unknown = NULL;
bool invalid_search_in = false;
yyjson_obj_iter arg_iter;
yyjson_obj_iter_init(args, &arg_iter);
yyjson_val *arg_key;
while ((arg_key = yyjson_obj_iter_next(&arg_iter)) != NULL) {
const char *key = yyjson_get_str(arg_key);
bool accepted = key && yyjson_obj_get(properties, key);
if (!accepted) {
accepted =
mcp_legacy_argument_is_valid(tool_name, properties, args, key, &invalid_search_in);
}
if (!accepted) {
unknown = key ? key : "<invalid>";
break;
}
}
char *error_result = NULL;
if (invalid_search_in) {
error_result = cbm_mcp_text_result(
"search_code supports source text search directly: omit search_in or use "
"search_in='source' for legacy callers; use search_graph for graph search",
true);
} else if (unknown) {
char supported[CBM_SZ_1K] = "";
size_t used = 0;
yyjson_obj_iter prop_iter;
yyjson_obj_iter_init(properties, &prop_iter);
yyjson_val *prop_key;
while ((prop_key = yyjson_obj_iter_next(&prop_iter)) != NULL) {
const char *name = yyjson_get_str(prop_key);
if (!name) {
continue;
}
int written = snprintf(supported + used, sizeof(supported) - used, "%s%s",
used ? ", " : "", name);
if (written < 0 || (size_t)written >= sizeof(supported) - used) {
break;
}
used += (size_t)written;
}
char message[CBM_SZ_2K];
snprintf(message, sizeof(message),
"unknown argument '%.*s' for tool '%.*s'; supported arguments: %s. "
"Use tools/list or 'cli %.*s --help' for types and descriptions.",
CBM_SZ_256, unknown, CBM_SZ_256, tool_name, supported[0] ? supported : "(none)",
CBM_SZ_256, tool_name);
error_result = cbm_mcp_text_result(message, true);
}
yyjson_doc_free(schema_doc);
yyjson_doc_free(args_doc);
return error_result;
}
char *cbm_mcp_handle_tool(cbm_mcp_server_t *srv, const char *tool_name, const char *args_json) {
if (!tool_name) {
return cbm_mcp_text_result(
@@ -14965,6 +15123,10 @@ char *cbm_mcp_handle_tool(cbm_mcp_server_t *srv, const char *tool_name, const ch
tool_name, mcp_tool_profile_name(srv->tool_profile));
return cbm_mcp_text_result(message, true);
}
char *argument_error = mcp_validate_tool_argument_keys(tool_name, args_json);
if (argument_error) {
return argument_error;
}
/* Streamlined alias: get_code → get_code_snippet handler.
* (The 3 search tools dispatch by their real names below.) */
+124
View File
@@ -558,6 +558,58 @@ TEST(mcp_tools_list_latest_metadata) {
PASS();
}
TEST(mcp_tool_input_schemas_are_closed_in_classic_and_streamlined_modes) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
ASSERT_NOT_NULL(srv);
char *revealed = cbm_mcp_handle_tool(srv, "_hidden_tools", "{}");
ASSERT_NOT_NULL(revealed);
ASSERT_NULL(strstr(revealed, "\"isError\":true"));
free(revealed);
char *snapshots[] = {mcp_tools_list_classic_snapshot(), cbm_mcp_tools_list(NULL),
cbm_mcp_tools_list(srv)};
for (size_t si = 0; si < sizeof(snapshots) / sizeof(snapshots[0]); si++) {
ASSERT_NOT_NULL(snapshots[si]);
yyjson_doc *doc = yyjson_read(snapshots[si], strlen(snapshots[si]), 0);
ASSERT_NOT_NULL(doc);
yyjson_val *tools = yyjson_obj_get(yyjson_doc_get_root(doc), "tools");
ASSERT_TRUE(yyjson_is_arr(tools));
yyjson_arr_iter iter;
yyjson_arr_iter_init(tools, &iter);
yyjson_val *tool;
while ((tool = yyjson_arr_iter_next(&iter)) != NULL) {
yyjson_val *schema = yyjson_obj_get(tool, "inputSchema");
ASSERT_TRUE(yyjson_is_obj(schema));
yyjson_val *closed = yyjson_obj_get(schema, "additionalProperties");
ASSERT_TRUE(yyjson_is_bool(closed));
ASSERT_FALSE(yyjson_get_bool(closed));
}
yyjson_doc_free(doc);
free(snapshots[si]);
}
cbm_mcp_server_free(srv);
PASS();
}
TEST(mcp_canonical_input_schemas_cover_implemented_format_and_verbose_options) {
struct {
const char *tool;
const char *property;
} cases[] = {{"index_repository", "format"}, {"index_status", "verbose"}};
for (size_t i = 0; i < sizeof(cases) / sizeof(cases[0]); i++) {
const char *schema_json = cbm_mcp_tool_input_schema(cases[i].tool);
ASSERT_NOT_NULL(schema_json);
yyjson_doc *doc = yyjson_read(schema_json, strlen(schema_json), 0);
ASSERT_NOT_NULL(doc);
yyjson_val *properties = yyjson_obj_get(yyjson_doc_get_root(doc), "properties");
ASSERT_TRUE(yyjson_is_obj(properties));
ASSERT_NOT_NULL(yyjson_obj_get(properties, cases[i].property));
yyjson_doc_free(doc);
}
PASS();
}
TEST(mcp_tools_have_behavior_annotations) {
struct {
const char *name;
@@ -1791,6 +1843,73 @@ TEST(tool_unknown_tool) {
PASS();
}
TEST(tool_unknown_argument_is_actionable_execution_error) {
cbm_mcp_server_t *srv = setup_mcp_with_data();
char *direct = cbm_mcp_handle_tool(
srv, "search_code",
"{\"pattern\":\"HandleOrder\",\"repo_path\":\"/tmp/not-a-project-argument\"}");
ASSERT_NOT_NULL(direct);
ASSERT_NOT_NULL(strstr(direct, "\"isError\":true"));
ASSERT_NOT_NULL(strstr(direct, "repo_path"));
ASSERT_NOT_NULL(strstr(direct, "project"));
ASSERT_NOT_NULL(strstr(direct, "supported"));
free(direct);
char *framed = cbm_mcp_server_handle(
srv, "{\"jsonrpc\":\"2.0\",\"id\":1201,\"method\":\"tools/call\","
"\"params\":{\"name\":\"search_code\",\"arguments\":{"
"\"pattern\":\"HandleOrder\",\"repo_path\":\"/tmp/not-a-project-argument\"}}}");
ASSERT_NOT_NULL(framed);
/* MCP input validation is a Tool Execution Error so a model receives the
* actionable correction; malformed tools/call envelopes remain protocol errors. */
ASSERT_NOT_NULL(strstr(framed, "\"result\""));
ASSERT_NOT_NULL(strstr(framed, "\"isError\":true"));
ASSERT_NULL(strstr(framed, "\"code\":-32602"));
ASSERT_NOT_NULL(strstr(framed, "repo_path"));
ASSERT_NOT_NULL(strstr(framed, "project"));
free(framed);
cbm_mcp_server_free(srv);
PASS();
}
TEST(tool_search_code_legacy_search_in_is_bounded_and_actionable) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
ASSERT_NOT_NULL(srv);
char *response =
cbm_mcp_handle_tool(srv, "search_code", "{\"pattern\":\"needle\",\"search_in\":\"graph\"}");
ASSERT_NOT_NULL(response);
ASSERT_NOT_NULL(strstr(response, "\"isError\":true"));
ASSERT_NOT_NULL(strstr(response, "search_graph"));
ASSERT_NOT_NULL(strstr(response, "omit search_in"));
free(response);
cbm_mcp_server_free(srv);
PASS();
}
TEST(tool_query_graph_legacy_cypher_alias_remains_bounded) {
cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL);
ASSERT_NOT_NULL(srv);
char *response = cbm_mcp_handle_tool(
srv, "query_graph", "{\"cypher\":\"MATCH (n) RETURN n LIMIT 1\"}");
ASSERT_NOT_NULL(response);
ASSERT_NULL(strstr(response, "unknown argument 'cypher'"));
free(response);
response = cbm_mcp_handle_tool(srv, "query_graph", "{\"label\":\"Function\"}");
ASSERT_NOT_NULL(response);
ASSERT_NOT_NULL(strstr(response, "unknown argument 'label'"));
ASSERT_NOT_NULL(strstr(response, "query"));
free(response);
cbm_mcp_server_free(srv);
PASS();
}
TEST(tool_search_graph_basic) {
cbm_mcp_server_t *srv = setup_mcp_with_data();
@@ -12299,6 +12418,8 @@ SUITE(mcp) {
RUN_TEST(mcp_tools_list);
RUN_TEST(mcp_tools_list_classic_mode);
RUN_TEST(mcp_tools_list_latest_metadata);
RUN_TEST(mcp_tool_input_schemas_are_closed_in_classic_and_streamlined_modes);
RUN_TEST(mcp_canonical_input_schemas_cover_implemented_format_and_verbose_options);
RUN_TEST(mcp_tools_have_behavior_annotations);
RUN_TEST(mcp_index_repository_declares_name_override_issue571);
RUN_TEST(mcp_tools_array_schemas_have_items);
@@ -12367,6 +12488,9 @@ SUITE(mcp) {
RUN_TEST(tool_get_graph_schema_uses_ready_overlay_schema);
RUN_TEST(first_response_context_uses_ready_overlay_schema);
RUN_TEST(tool_unknown_tool);
RUN_TEST(tool_unknown_argument_is_actionable_execution_error);
RUN_TEST(tool_search_code_legacy_search_in_is_bounded_and_actionable);
RUN_TEST(tool_query_graph_legacy_cypher_alias_remains_bounded);
RUN_TEST(tool_search_graph_basic);
RUN_TEST(tool_trace_totals_respect_test_filter);
RUN_TEST(tool_get_architecture_cycles_detects_scc);