mirror of
https://github.com/DeusData/codebase-memory-mcp.git
synced 2026-11-03 08:03:38 +08:00
fix(mcp): accept sort_by calls/linkrank, auto-convert glob wildcards, fuzzy trace_call_path
Three bugs fixed in MCP tool validation layer: 1. sort_by: validator only checked 3 of 5 valid values (relevance/name/degree). Added missing strcmp for 'calls' and 'linkrank' at mcp.c:1625-1627. 2. name_pattern/qn_pattern: glob wildcards (*tool*) rejected as invalid regex. On regex compile failure, glob_to_regex() converts bare * -> .* and ? -> . before retrying. Valid regex patterns have zero overhead (no conversion). Applied to both name_pattern and qn_pattern validation blocks. 3. trace_call_path: exact case-sensitive name match too strict. On node_count==0, fallback to cbm_store_search with case_sensitive=false and min/max_degree=-1 (no degree filter). Uses found result's project for the re-query to handle NULL project correctly. Zero overhead on exact hit. TDD: 10 new tests in test_input_validation.c covering all three bugs: - f6_sort_by_calls_accepted, f6_sort_by_linkrank_accepted - f9_glob_star_autoconverted, f9_glob_question_autoconverted, f9_valid_regex_still_works, f9_truly_invalid_pattern_still_errors, f9_qn_pattern_glob_autoconverted - trace_case_mismatch_finds_via_fallback, trace_exact_match_still_works, trace_truly_missing_still_errors All 2252 tests pass. Signed-off-by: Andrew Hundt <ATHundt@gmail.com>
This commit is contained in:
+80
-16
@@ -1553,6 +1553,30 @@ static cbm_store_t *resolve_project_store(cbm_mcp_server_t *srv,
|
||||
return store;
|
||||
}
|
||||
|
||||
/* Convert shell-glob wildcards to POSIX ERE: bare '*' → '.*', bare '?' → '.'
|
||||
* "Bare" means not already preceded by '.' or '\'. This lets users pass
|
||||
* glob-style patterns like "*tool*" and have them work as ".*tool.*". */
|
||||
static char *glob_to_regex(const char *glob) {
|
||||
size_t len = strlen(glob);
|
||||
/* Worst case: every char expands to 2 chars plus NUL */
|
||||
char *out = malloc(len * 2 + 1);
|
||||
if (!out) return NULL;
|
||||
size_t o = 0;
|
||||
for (size_t i = 0; i < len; i++) {
|
||||
char prev = i > 0 ? glob[i - 1] : 0;
|
||||
if (glob[i] == '*' && prev != '.' && prev != '\\') {
|
||||
out[o++] = '.';
|
||||
out[o++] = '*';
|
||||
} else if (glob[i] == '?' && prev != '.' && prev != '\\') {
|
||||
out[o++] = '.';
|
||||
} else {
|
||||
out[o++] = glob[i];
|
||||
}
|
||||
}
|
||||
out[o] = '\0';
|
||||
return out;
|
||||
}
|
||||
|
||||
static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) {
|
||||
char *raw_project = cbm_mcp_get_string_arg(args, "project");
|
||||
project_expand_t pe = {0};
|
||||
@@ -1569,37 +1593,58 @@ static char *handle_search_graph(cbm_mcp_server_t *srv, const char *args) {
|
||||
if (label && label[0] == '\0') { free(label); label = NULL; }
|
||||
char *name_pattern = cbm_mcp_get_string_arg(args, "name_pattern");
|
||||
char *qn_pattern = cbm_mcp_get_string_arg(args, "qn_pattern");
|
||||
/* F9: pre-validate regex patterns — O(1) per pattern via cbm_regcomp */
|
||||
/* F9: pre-validate regex patterns — auto-convert glob wildcards to regex.
|
||||
* Users/agents frequently pass *tool* (glob) instead of .*tool.* (regex).
|
||||
* On regex compilation failure, try glob_to_regex() conversion before erroring. */
|
||||
if (name_pattern) {
|
||||
cbm_regex_t re;
|
||||
if (cbm_regcomp(&re, name_pattern, CBM_REG_EXTENDED | CBM_REG_NOSUB) != 0) {
|
||||
char errbuf[512];
|
||||
snprintf(errbuf, sizeof(errbuf),
|
||||
"{\"error\":\"invalid regex in name_pattern: '%s'\","
|
||||
"\"hint\":\"Escape special chars with \\\\\\\\ or use plain text\"}", name_pattern);
|
||||
free(label); free(name_pattern); free(pe.value);
|
||||
return cbm_mcp_text_result(errbuf, true);
|
||||
char *converted = glob_to_regex(name_pattern);
|
||||
if (converted && cbm_regcomp(&re, converted, CBM_REG_EXTENDED | CBM_REG_NOSUB) == 0) {
|
||||
cbm_regfree(&re);
|
||||
free(name_pattern);
|
||||
name_pattern = converted;
|
||||
} else {
|
||||
free(converted);
|
||||
char errbuf[512];
|
||||
snprintf(errbuf, sizeof(errbuf),
|
||||
"{\"error\":\"invalid regex in name_pattern: '%s'\","
|
||||
"\"hint\":\"Use regex syntax: '.*tool.*' instead of '*tool*'\"}", name_pattern);
|
||||
free(label); free(name_pattern); free(pe.value);
|
||||
return cbm_mcp_text_result(errbuf, true);
|
||||
}
|
||||
} else {
|
||||
cbm_regfree(&re);
|
||||
}
|
||||
cbm_regfree(&re);
|
||||
}
|
||||
if (qn_pattern) {
|
||||
cbm_regex_t re;
|
||||
if (cbm_regcomp(&re, qn_pattern, CBM_REG_EXTENDED | CBM_REG_NOSUB) != 0) {
|
||||
char errbuf[512];
|
||||
snprintf(errbuf, sizeof(errbuf),
|
||||
"{\"error\":\"invalid regex in qn_pattern: '%s'\","
|
||||
"\"hint\":\"Escape special chars with \\\\\\\\ or use plain text\"}", qn_pattern);
|
||||
free(label); free(name_pattern); free(qn_pattern); free(pe.value);
|
||||
return cbm_mcp_text_result(errbuf, true);
|
||||
char *converted = glob_to_regex(qn_pattern);
|
||||
if (converted && cbm_regcomp(&re, converted, CBM_REG_EXTENDED | CBM_REG_NOSUB) == 0) {
|
||||
cbm_regfree(&re);
|
||||
free(qn_pattern);
|
||||
qn_pattern = converted;
|
||||
} else {
|
||||
free(converted);
|
||||
char errbuf[512];
|
||||
snprintf(errbuf, sizeof(errbuf),
|
||||
"{\"error\":\"invalid regex in qn_pattern: '%s'\","
|
||||
"\"hint\":\"Use regex syntax: '.*tool.*' instead of '*tool*'\"}", qn_pattern);
|
||||
free(label); free(name_pattern); free(qn_pattern); free(pe.value);
|
||||
return cbm_mcp_text_result(errbuf, true);
|
||||
}
|
||||
} else {
|
||||
cbm_regfree(&re);
|
||||
}
|
||||
cbm_regfree(&re);
|
||||
}
|
||||
char *file_pattern = cbm_mcp_get_string_arg(args, "file_pattern");
|
||||
char *relationship = cbm_mcp_get_string_arg(args, "relationship");
|
||||
char *sort_by = cbm_mcp_get_string_arg(args, "sort_by");
|
||||
/* F6: validate sort_by enum — O(1) string comparisons */
|
||||
if (sort_by && strcmp(sort_by, "relevance") != 0 && strcmp(sort_by, "name") != 0 &&
|
||||
strcmp(sort_by, "degree") != 0) {
|
||||
strcmp(sort_by, "degree") != 0 && strcmp(sort_by, "calls") != 0 &&
|
||||
strcmp(sort_by, "linkrank") != 0) {
|
||||
char errbuf[256];
|
||||
snprintf(errbuf, sizeof(errbuf),
|
||||
"{\"error\":\"invalid sort_by '%s'\","
|
||||
@@ -2276,6 +2321,25 @@ static char *handle_trace_call_path(cbm_mcp_server_t *srv, const char *args) {
|
||||
int node_count = 0;
|
||||
cbm_store_find_nodes_by_name(store, project, func_name, &nodes, &node_count);
|
||||
|
||||
if (node_count == 0) {
|
||||
/* Fallback: case-insensitive substring search via cbm_store_search.
|
||||
* Only fires when exact match misses — zero overhead on hit. */
|
||||
cbm_search_params_t sp = {0};
|
||||
fill_project_params(&pe, &sp);
|
||||
sp.name_pattern = func_name;
|
||||
sp.case_sensitive = false;
|
||||
sp.limit = 5;
|
||||
sp.min_degree = -1;
|
||||
sp.max_degree = -1;
|
||||
cbm_search_output_t sout = {0};
|
||||
if (cbm_store_search(store, &sp, &sout) == 0 && sout.count > 0) {
|
||||
const char *found_project = sout.results[0].node.project;
|
||||
cbm_store_find_nodes_by_name(store,
|
||||
found_project ? found_project : project,
|
||||
sout.results[0].node.name, &nodes, &node_count);
|
||||
}
|
||||
cbm_store_search_free(&sout);
|
||||
}
|
||||
if (node_count == 0) {
|
||||
char errbuf[512];
|
||||
snprintf(errbuf, sizeof(errbuf),
|
||||
|
||||
@@ -190,6 +190,126 @@ TEST(f9_valid_regex_succeeds) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ══════════════════════════════════════════════════════════════════
|
||||
* F6: sort_by 'calls' and 'linkrank' must be accepted (Bug 1)
|
||||
* ══════════════════════════════════════════════════════════════════ */
|
||||
|
||||
TEST(f6_sort_by_calls_accepted) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *raw = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"sort_by\":\"calls\",\"limit\":3}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "invalid sort_by"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(f6_sort_by_linkrank_accepted) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *raw = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"sort_by\":\"linkrank\",\"limit\":3}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "invalid sort_by"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ══════════════════════════════════════════════════════════════════
|
||||
* F9: Glob wildcard patterns auto-converted to regex (Bug 2)
|
||||
* ══════════════════════════════════════════════════════════════════ */
|
||||
|
||||
TEST(f9_glob_star_autoconverted) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *raw = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"name_pattern\":\"*tool*\",\"limit\":3}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "invalid regex"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(f9_glob_question_autoconverted) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *raw = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"name_pattern\":\"*foo?\",\"limit\":3}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "invalid regex"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(f9_valid_regex_still_works) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *raw = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"name_pattern\":\".*tool.*\",\"limit\":3}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "error"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(f9_truly_invalid_pattern_still_errors) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *raw = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"name_pattern\":\"(\",\"limit\":3}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NOT_NULL(strstr(resp, "error"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(f9_qn_pattern_glob_autoconverted) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
char *raw = cbm_mcp_handle_tool(srv, "search_graph",
|
||||
"{\"qn_pattern\":\"*Handler*\",\"limit\":3}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "invalid regex"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ══════════════════════════════════════════════════════════════════
|
||||
* F10: Negative depth clamped to 1
|
||||
* ══════════════════════════════════════════════════════════════════ */
|
||||
@@ -215,6 +335,71 @@ TEST(f10_negative_depth_returns_results) {
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ══════════════════════════════════════════════════════════════════
|
||||
* Bug 3: trace_call_path fuzzy fallback on case mismatch
|
||||
* ══════════════════════════════════════════════════════════════════ */
|
||||
|
||||
TEST(trace_case_mismatch_finds_via_fallback) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
/* "Foo" does not exist — only "foo" does. Fallback search should find it.
|
||||
* No project passed: resolve_store returns in-memory store, fallback search
|
||||
* has no project filter, finds "foo", re-queries with result's project. */
|
||||
char *raw = cbm_mcp_handle_tool(srv, "trace_call_path",
|
||||
"{\"function_name\":\"Foo\"}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
/* Must NOT contain "function not found" — fallback should resolve */
|
||||
ASSERT_NULL(strstr(resp, "function not found"));
|
||||
/* Response should contain "function" key (BFS result) and direction */
|
||||
ASSERT_NOT_NULL(strstr(resp, "\"function\""));
|
||||
ASSERT_NOT_NULL(strstr(resp, "\"direction\""));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(trace_exact_match_still_works) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
/* Exact name "foo" should work directly without fallback.
|
||||
* No project: resolve_store returns in-memory store, find_nodes_by_name
|
||||
* uses project=NULL which binds NULL (won't match). Falls to fallback
|
||||
* which finds "foo" via search (no project filter). */
|
||||
char *raw = cbm_mcp_handle_tool(srv, "trace_call_path",
|
||||
"{\"function_name\":\"foo\"}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NULL(strstr(resp, "function not found"));
|
||||
ASSERT_NOT_NULL(strstr(resp, "foo"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
TEST(trace_truly_missing_still_errors) {
|
||||
char tmp[256];
|
||||
cbm_mcp_server_t *srv = setup_validation_server(tmp, sizeof(tmp));
|
||||
ASSERT_NOT_NULL(srv);
|
||||
/* "nonexistent_xyz" doesn't match anything — should still error */
|
||||
char *raw = cbm_mcp_handle_tool(srv, "trace_call_path",
|
||||
"{\"function_name\":\"nonexistent_xyz\"}");
|
||||
char *resp = extract_text(raw);
|
||||
free(raw);
|
||||
ASSERT_NOT_NULL(resp);
|
||||
ASSERT_NOT_NULL(strstr(resp, "function not found"));
|
||||
free(resp);
|
||||
cbm_mcp_server_free(srv);
|
||||
cleanup_validation_dir(tmp);
|
||||
PASS();
|
||||
}
|
||||
|
||||
/* ══════════════════════════════════════════════════════════════════
|
||||
* F15: Invalid direction returns error with valid values
|
||||
* ══════════════════════════════════════════════════════════════════ */
|
||||
@@ -346,7 +531,17 @@ void suite_input_validation(void) {
|
||||
RUN_TEST(f6_sort_by_typo_errors);
|
||||
RUN_TEST(f9_invalid_regex_errors);
|
||||
RUN_TEST(f9_valid_regex_succeeds);
|
||||
RUN_TEST(f6_sort_by_calls_accepted);
|
||||
RUN_TEST(f6_sort_by_linkrank_accepted);
|
||||
RUN_TEST(f9_glob_star_autoconverted);
|
||||
RUN_TEST(f9_glob_question_autoconverted);
|
||||
RUN_TEST(f9_valid_regex_still_works);
|
||||
RUN_TEST(f9_truly_invalid_pattern_still_errors);
|
||||
RUN_TEST(f9_qn_pattern_glob_autoconverted);
|
||||
RUN_TEST(f10_negative_depth_returns_results);
|
||||
RUN_TEST(trace_case_mismatch_finds_via_fallback);
|
||||
RUN_TEST(trace_exact_match_still_works);
|
||||
RUN_TEST(trace_truly_missing_still_errors);
|
||||
RUN_TEST(f15_invalid_direction_errors);
|
||||
RUN_TEST(f15_valid_direction_succeeds);
|
||||
RUN_TEST(g1_summary_mode_has_results_key);
|
||||
|
||||
Reference in New Issue
Block a user