From 0e1b45399ea67bb41f02962f0c93e9e38b44f49f Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 31 Jul 2026 01:11:34 +0200 Subject: [PATCH 1/2] fix(cli): derive --help tool list from the MCP registry The top-level --help listed 14 hand-maintained tool names and omitted check_index_coverage; MCP tools/list correctly advertised all 15. The help string was a separate copy of the registry, so it drifted silently when the tool was added. Render the Tools block from the TOOLS registry itself (new cbm_mcp_tools_help_list, plus cbm_mcp_tool_count/cbm_mcp_tool_name accessors) so the two lists share one source of truth and cannot diverge again. A unit test pins the render against the registry: every advertised tool appears, cardinality matches, and lines stay 80-column safe. Fixes #1361 Signed-off-by: Martin Vogel --- src/main.c | 11 +++++++---- src/mcp/mcp.c | 42 ++++++++++++++++++++++++++++++++++++++++++ src/mcp/mcp.h | 9 +++++++++ tests/test_mcp.c | 35 +++++++++++++++++++++++++++++++++++ 4 files changed, 93 insertions(+), 4 deletions(-) diff --git a/src/main.c b/src/main.c index 7070a895b..5f982d0f1 100644 --- a/src/main.c +++ b/src/main.c @@ -888,10 +888,13 @@ static void print_help(void) { printf(" Manual/UI MCP boundaries: Qodo, Warp, JetBrains AI/ACP, Replit,\n"); printf(" Plandex, SWE-agent, BLACKBOX, GitHub cloud agents, Jules,\n"); printf(" CodeRabbit.\n"); - printf("\nTools: index_repository, search_graph, query_graph, trace_path,\n"); - printf(" get_code_snippet, get_graph_schema, get_architecture, search_code,\n"); - printf(" list_projects, delete_project, index_status, detect_changes,\n"); - printf(" manage_adr, ingest_traces\n"); + /* Rendered from the MCP tool registry: a hand-maintained copy here + * omitted check_index_coverage (#1361) and could silently drift again. */ + char *tools_help = cbm_mcp_tools_help_list(); + if (tools_help) { + printf("\n%s", tools_help); + free(tools_help); + } } /* ── Main ───────────────────────────────────────────────────────── */ diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 78706924c..3f79441c2 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -34,6 +34,7 @@ enum { MCP_CONTENT_PREFIX = 15, /* strlen("Content-Length:") */ MCP_RETURN_2 = 2, MCP_TOOLS_PAGE_SIZE = 8, + MCP_HELP_TOOLS_WRAP_COL = 74, /* --help tool list stays readable on 80-col terminals */ MCP_MAX_CROSS_REPO_TARGETS = 4096, }; #define MCP_MS_TO_US 1000LL @@ -890,6 +891,47 @@ const char *cbm_mcp_tool_input_schema(const char *tool_name) { return NULL; } +int cbm_mcp_tool_count(void) { + return TOOL_COUNT; +} + +const char *cbm_mcp_tool_name(int index) { + if (index < 0 || index >= TOOL_COUNT) { + return NULL; + } + return TOOLS[index].name; +} + +/* Render the top-level --help "Tools:" block from the registry tools/list + * serves. The list used to be hand-maintained in the help text and drifted + * when check_index_coverage was added (#1361); deriving it here makes that + * divergence impossible. Heap-allocated; caller frees. */ +char *cbm_mcp_tools_help_list(void) { + size_t cap = SLEN("Tools:") + 2; /* trailing newline + NUL */ + for (int i = 0; i < TOOL_COUNT; i++) { + cap += strlen(TOOLS[i].name) + SLEN(" ,\n "); /* per-tool worst case incl. a wrap */ + } + char *out = malloc(cap); + if (!out) { + return NULL; + } + size_t len = (size_t)snprintf(out, cap, "Tools:"); + size_t col = len; + for (int i = 0; i < TOOL_COUNT; i++) { + const char *sep = (i + 1 < TOOL_COUNT) ? "," : ""; + size_t item = SLEN(" ") + strlen(TOOLS[i].name) + strlen(sep); + if (i > 0 && col + item > MCP_HELP_TOOLS_WRAP_COL) { + len += (size_t)snprintf(out + len, cap - len, "\n "); + col = 1; + } + size_t wrote = (size_t)snprintf(out + len, cap - len, " %s%s", TOOLS[i].name, sep); + len += wrote; + col += wrote; + } + snprintf(out + len, cap - len, "\n"); + return out; +} + static int mcp_tools_cursor_offset(const char *params_json, bool *has_cursor_out) { if (has_cursor_out) { *has_cursor_out = false; diff --git a/src/mcp/mcp.h b/src/mcp/mcp.h index 58b01a32a..bc906770b 100644 --- a/src/mcp/mcp.h +++ b/src/mcp/mcp.h @@ -67,6 +67,15 @@ char *cbm_mcp_tools_list(void); * NULL if the tool is unknown. Backs the CLI flag parser + per-tool --help. */ const char *cbm_mcp_tool_input_schema(const char *tool_name); +/* Registry accessors: the number of tools tools/list advertises, and the name + * of tool `index` (static, do not free; NULL when out of range). */ +int cbm_mcp_tool_count(void); +const char *cbm_mcp_tool_name(int index); + +/* Render the top-level --help "Tools:" block from the registry so the help + * text cannot drift from tools/list (#1361). Heap-allocated; caller frees. */ +char *cbm_mcp_tools_help_list(void); + /* Format the initialize response. params_json is the raw initialize params * (used for protocol version negotiation). Returns heap-allocated JSON. */ char *cbm_mcp_initialize_response(const char *params_json); diff --git a/tests/test_mcp.c b/tests/test_mcp.c index a56aebdf5..ff057b0fc 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -762,6 +762,40 @@ TEST(mcp_tools_list) { PASS(); } +/* #1361: --help omitted check_index_coverage because its tool list was a + * hand-maintained copy. The list is now rendered from the registry; this pins + * the render so a formatter bug cannot reintroduce a silent omission. */ +TEST(mcp_tools_help_list_matches_registry) { + char *help = cbm_mcp_tools_help_list(); + ASSERT_NOT_NULL(help); + int count = cbm_mcp_tool_count(); + ASSERT_GT(count, 0); + for (int i = 0; i < count; i++) { + const char *name = cbm_mcp_tool_name(i); + ASSERT_NOT_NULL(name); + ASSERT_NOT_NULL(strstr(help, name)); + } + /* Exactly one comma between consecutive tools: the rendered cardinality + * equals the registry's, so truncation or duplication fails here. */ + int commas = 0; + for (const char *p = help; *p; p++) { + if (*p == ',') { + commas++; + } + } + ASSERT_EQ(commas, count - 1); + /* Wrapped for an 80-column terminal. */ + const char *line = help; + while (line && *line) { + const char *nl = strchr(line, '\n'); + size_t line_len = nl ? (size_t)(nl - line) : strlen(line); + ASSERT_LT((int)line_len, 80); + line = nl ? nl + 1 : NULL; + } + free(help); + PASS(); +} + TEST(mcp_tools_list_latest_metadata) { char *json = cbm_mcp_tools_list(); ASSERT_NOT_NULL(json); @@ -9517,6 +9551,7 @@ SUITE(mcp) { /* MCP protocol helpers */ RUN_TEST(mcp_initialize_response); RUN_TEST(mcp_tools_list); + RUN_TEST(mcp_tools_help_list_matches_registry); RUN_TEST(mcp_tools_list_latest_metadata); RUN_TEST(mcp_tools_have_behavior_annotations); RUN_TEST(mcp_index_repository_declares_name_override_issue571); From 644c8fd5e88e39161ddfd373c23681bebe99d2d7 Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Fri, 31 Jul 2026 02:15:48 +0200 Subject: [PATCH 2/2] fix(mcp): bound the help-list appends instead of trusting snprintf's return CodeQL flagged two high-severity alerts in the previous commit: the help-list builder accumulated snprintf's return value, which is the length it WOULD have written rather than what it did. Once len passes cap, the next `cap - len` underflows to a huge size_t and the following write lands outside the buffer. That is the same class the Cypher UNWIND list builder was just fixed for in #1173, where blen advanced past a 2KB buffer the same way. The capacity computed here is in fact sufficient for the current registry, so nothing overflows today -- which makes this the more dangerous shape, not the safer one. The code was correct only by an argument made ten lines away in the cap loop, so renaming a tool or changing the wrap rule would have turned it into an out-of-bounds write with nothing in the function to notice. help_append() returns the bytes ACTUALLY written, clamped to the space left, so `len <= cap - 1` becomes a local invariant that holds no matter what the registry or the wrap column later become. Also adds the the variadic helper needs. Verified: builds clean under -Werror; --help still renders all 15 tools in registry order with the same wrapping; mcp + cli suites 437 passed, 0 failed; make -f Makefile.cbm lint-ci clean. Signed-off-by: Martin Vogel --- src/mcp/mcp.c | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 3f79441c2..171f0dab2 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -89,6 +89,7 @@ enum { #include #include #include +#include // va_list, for the bounded help-list appender #include // int64_t #include #include @@ -906,6 +907,32 @@ const char *cbm_mcp_tool_name(int index) { * serves. The list used to be hand-maintained in the help text and drifted * when check_index_coverage was added (#1361); deriving it here makes that * divergence impossible. Heap-allocated; caller frees. */ +/* Append at out[len] and return the bytes ACTUALLY written. + * + * snprintf returns the length it WOULD have written, so accumulating that value + * lets len run past cap; the next `cap - len` then underflows to a huge size_t + * and the following write lands outside the buffer. CodeQL flagged exactly that + * shape here, and it is the same class #1173 just fixed in the Cypher list + * builder. The capacity computed below does happen to be sufficient today — + * which makes this the more dangerous version, not the safer one: the code is + * correct only by an argument made ten lines away, so renaming a tool or + * changing the wrap rule would turn it into an overflow with nothing to notice. + * Clamping makes `len <= cap - 1` a local invariant no later edit can void. */ +static size_t help_append(char *out, size_t cap, size_t len, const char *fmt, ...) { + if (len + 1 >= cap) { + return 0; + } + va_list args; + va_start(args, fmt); + int written = vsnprintf(out + len, cap - len, fmt, args); + va_end(args); + if (written <= 0) { + return 0; + } + size_t room = cap - len - 1; + return (size_t)written > room ? room : (size_t)written; +} + char *cbm_mcp_tools_help_list(void) { size_t cap = SLEN("Tools:") + 2; /* trailing newline + NUL */ for (int i = 0; i < TOOL_COUNT; i++) { @@ -915,20 +942,21 @@ char *cbm_mcp_tools_help_list(void) { if (!out) { return NULL; } - size_t len = (size_t)snprintf(out, cap, "Tools:"); + size_t len = help_append(out, cap, 0, "Tools:"); size_t col = len; for (int i = 0; i < TOOL_COUNT; i++) { const char *sep = (i + 1 < TOOL_COUNT) ? "," : ""; size_t item = SLEN(" ") + strlen(TOOLS[i].name) + strlen(sep); if (i > 0 && col + item > MCP_HELP_TOOLS_WRAP_COL) { - len += (size_t)snprintf(out + len, cap - len, "\n "); + len += help_append(out, cap, len, "\n "); col = 1; } - size_t wrote = (size_t)snprintf(out + len, cap - len, " %s%s", TOOLS[i].name, sep); + size_t wrote = help_append(out, cap, len, " %s%s", TOOLS[i].name, sep); len += wrote; col += wrote; } - snprintf(out + len, cap - len, "\n"); + len += help_append(out, cap, len, "\n"); + (void)len; /* final length is not needed; the buffer is NUL-terminated */ return out; }