From 3dd24719b0c6cd09158e1329b06382922282293a Mon Sep 17 00:00:00 2001 From: Martin Vogel Date: Thu, 2 Jul 2026 01:24:29 +0200 Subject: [PATCH] fix(search): scoped search must handle repo paths containing spaces search_code's scoped grep writes an "/" filelist and pipes it to xargs. With a newline-separated filelist + plain `xargs`, a space anywhere in the absolute path splits one file into several bogus arguments, so grep finds nothing and scoped search returns 0 matches for any repo indexed under a path containing a space (#687). Fix: write NUL-separated records and consume them with `xargs -0`, so each path stays a single argument regardless of spaces. On Windows the scoped path uses PowerShell `Get-Content | Select-String -LiteralPath`, which reads line-by-line and already handles spaces, so the filelist keeps newline separators there. write_scoped_filelist writes each record piece-by-piece with fwrite/fputc (no fixed-size stack buffer), so arbitrarily long absolute paths cannot overflow. Reproduce-first: search_code_scoped_path_with_spaces_issue687 indexes a repo rooted at "/my project" and asserts total_grep_matches > 0 (RED on the old newline + plain-xargs code -> 0 matches; GREEN after). Full suite green. Distilled from #693 (thanks @lg320531124). Keeps that PR's correct NUL/xargs-0 idea and its Windows newline branch, but replaces its char buf[2048] + unbounded memcpy (a stack buffer overflow reachable via long indexed paths, PATH_MAX 4096) with the direct piece-by-piece write, and adds the reproduce-first guard it lacked. Closes #687 Signed-off-by: Martin Vogel Co-Authored-By: lg320531124 --- src/mcp/mcp.c | 27 +++++++++++++---- tests/test_mcp.c | 76 ++++++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 97 insertions(+), 6 deletions(-) diff --git a/src/mcp/mcp.c b/src/mcp/mcp.c index 0fbe4f3e8..034e2e599 100644 --- a/src/mcp/mcp.c +++ b/src/mcp/mcp.c @@ -3857,10 +3857,13 @@ static void build_grep_cmd(char *cmd, size_t cmd_sz, bool use_regex, bool scoped const char *flag = use_regex ? "-E" : "-F"; if (scoped) { if (file_pattern) { - snprintf(cmd, cmd_sz, "xargs grep -Hn %s --include='%s' -f '%s' < '%s' 2>/dev/null", + /* -0: read NUL-separated paths from the filelist so paths containing + * spaces stay one argument (issue #687). Pairs with the NUL separator + * written by write_scoped_filelist. */ + snprintf(cmd, cmd_sz, "xargs -0 grep -Hn %s --include='%s' -f '%s' < '%s' 2>/dev/null", flag, file_pattern, tmpfile, filelist); } else { - snprintf(cmd, cmd_sz, "xargs grep -Hn %s -f '%s' < '%s' 2>/dev/null", flag, tmpfile, + snprintf(cmd, cmd_sz, "xargs -0 grep -Hn %s -f '%s' < '%s' 2>/dev/null", flag, tmpfile, filelist); } } else { @@ -4276,10 +4279,22 @@ static bool write_scoped_filelist(cbm_mcp_server_t *srv, const char *project, co bool ok = false; if (fl) { for (int fi = 0; fi < indexed_count; fi++) { - /* Use forward slashes so xargs doesn't interpret Windows - * backslashes as escape sequences (e.g. \n becomes newline). - * Binary mode to prevent CRLF (xargs would see trailing \r). */ - (void)fprintf(fl, "%s/%s\n", root_path, indexed_files[fi]); + /* Write "/" piece-by-piece (no fixed-size buffer, so an + * arbitrarily long absolute path cannot overflow). Forward slash join + * so xargs doesn't treat Windows backslashes as escapes; binary mode + * (wb) prevents CRLF translation. Record separator differs by platform: + * - Unix: NUL, consumed by `xargs -0` — handles spaces in paths (a + * newline separator would split plain xargs on the space). + * - Windows: newline, consumed by PowerShell `Get-Content | + * Select-String -LiteralPath` (NUL bytes break Get-Content). */ + (void)fwrite(root_path, 1, strlen(root_path), fl); + (void)fputc('/', fl); + (void)fwrite(indexed_files[fi], 1, strlen(indexed_files[fi]), fl); +#ifdef _WIN32 + (void)fputc('\n', fl); +#else + (void)fputc('\0', fl); +#endif } (void)fclose(fl); ok = true; diff --git a/tests/test_mcp.c b/tests/test_mcp.c index 4acb0bae4..d3bfbe65a 100644 --- a/tests/test_mcp.c +++ b/tests/test_mcp.c @@ -1304,6 +1304,81 @@ TEST(search_code_multi_word) { PASS(); } +/* Reproduce-first (#687): scoped content search over a repo whose ROOT PATH + * contains a space. write_scoped_filelist emits "/" records that the + * Unix pipeline pipes to grep via xargs. With plain `xargs` (newline-split) the + * space splits one path into several bogus args -> grep finds nothing -> + * total_grep_matches == 0 (RED on the unfixed code). The fix writes NUL-separated + * records + uses `xargs -0`, so the path stays a single argument -> match found + * (GREEN). On Windows the scoped path uses PowerShell Get-Content -LiteralPath, + * which already handles spaces, so this asserts correct behavior there too. */ +TEST(search_code_scoped_path_with_spaces_issue687) { + char tmp[512]; + snprintf(tmp, sizeof(tmp), "/tmp/cbm_srch_space_XXXXXX"); + if (!cbm_mkdtemp(tmp)) { + FAIL("cbm_mkdtemp failed"); + } + + /* Project root deliberately contains a space. */ + char proj_dir[640]; + snprintf(proj_dir, sizeof(proj_dir), "%s/my project", tmp); + cbm_mkdir(proj_dir); + + char src_path[768]; + snprintf(src_path, sizeof(src_path), "%s/main.go", proj_dir); + FILE *fp = fopen(src_path, "w"); + if (!fp) { + rmdir(proj_dir); + rmdir(tmp); + FAIL("cannot write source file under spaced path"); + } + fprintf(fp, "package main\n\nfunc HandleRequest() error {\n\treturn nil\n}\n"); + fclose(fp); + + cbm_mcp_server_t *srv = cbm_mcp_server_new(NULL); + ASSERT_NOT_NULL(srv); + cbm_store_t *st = cbm_mcp_server_store(srv); + ASSERT_NOT_NULL(st); + const char *proj = "space-search"; + cbm_mcp_server_set_project(srv, proj); + cbm_store_upsert_project(st, proj, proj_dir); + + /* A node so the file is "indexed" (cbm_store_list_files -> scoped grep path) + * and the grep hit classifies to a result. */ + cbm_node_t n = {.project = proj, + .label = "Function", + .name = "HandleRequest", + .qualified_name = "space-search.main.HandleRequest", + .file_path = "main.go", + .start_line = 3, + .end_line = 5}; + ASSERT_GT(cbm_store_upsert_node(st, &n), 0); + + char *resp = cbm_mcp_server_handle( + srv, "{\"jsonrpc\":\"2.0\",\"id\":94,\"method\":\"tools/call\"," + "\"params\":{\"name\":\"search_code\"," + "\"arguments\":{\"pattern\":\"HandleRequest\",\"project\":\"space-search\"}}}"); + ASSERT_NOT_NULL(resp); + char *inner = extract_text_content(resp); + ASSERT_NOT_NULL(inner); + + /* grep must have found the match despite the space in the root path. */ + int grep_matches = -1; + const char *g = strstr(inner, "\"total_grep_matches\":"); + if (g) { + sscanf(g, "\"total_grep_matches\":%d", &grep_matches); + } + ASSERT_TRUE(grep_matches > 0); + + free(inner); + free(resp); + cbm_mcp_server_free(srv); + unlink(src_path); + rmdir(proj_dir); + rmdir(tmp); + PASS(); +} + /* issue #283: search_code with regex=true and a syntactically invalid pattern * must return an explicit error, not an empty result indistinguishable from a * legitimate no-match. */ @@ -3248,6 +3323,7 @@ SUITE(mcp) { RUN_TEST(tool_search_code_missing_pattern); RUN_TEST(tool_search_code_no_project); RUN_TEST(search_code_multi_word); + RUN_TEST(search_code_scoped_path_with_spaces_issue687); RUN_TEST(search_code_invalid_regex_errors_issue283); RUN_TEST(search_code_literal_pipe_warns_issue282); RUN_TEST(search_code_ampersand_accepted_issue272);