fix(search): scoped search must handle repo paths containing spaces#751
Merged
Conversation
search_code's scoped grep writes an "<root>/<file>" 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 "<tmp>/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 <martin.vogel.tech@gmail.com> Co-Authored-By: lg320531124 <lg320531124@users.noreply.github.com>
This was referenced Jul 3, 2026
Open
dpersek
pushed a commit
to dpersek/codebase-memory-mcp
that referenced
this pull request
Jul 3, 2026
Apply path_filter while writing the scoped filelist instead of only after grep, so large indexed projects do not scan files whose hits the post-grep filter would discard anyway. The prefilter runs the SAME compiled regex via the same cbm_regexec call against the same root-relative path (Windows separators normalized first) as the post-grep filter in collect_grep_matches, which is kept as belt-and-suspenders — results-preserving by construction. Guard the edge the prefilter introduces: a path_filter matching zero indexed files now short-circuits with the normal empty result instead of invoking xargs on an empty filelist (GNU execs grep once with no operands, BSD skips). Tests: positive guard (filter matching the hit's file still returns it, out-of-filter file excluded) and zero-match guard (clean empty response, grep subprocess skipped). Both green pre-change too — perf-only change. Distilled from DeusData#756 rebased onto the post-DeusData#751 filelist writer. Co-authored-by: yutianyu111602-glitch <yutianyu111602@gmail.com> Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Distilled from #693 (thanks @lg320531124 for the NUL +
xargs -0idea).Bug (#687)
search_code's scoped grep writes each indexed file as<root>/<file>into a filelist and pipes it to grep viaxargs. With a newline-separated filelist and plainxargs, a space anywhere in the absolute path splits one file into several bogus arguments → grep finds nothing → scoped search returns 0 matches for any repo indexed under a path containing a space.Fix
xargs -0, so each path is one argument regardless of spaces. Windows keeps newline separators (its PowerShellGet-Content | Select-String -LiteralPathpath reads line-by-line and already handles spaces).fwrite/fputc(no fixed-size stack buffer), so long absolute paths can't overflow.Reproduce-first
search_code_scoped_path_with_spaces_issue687— indexes a repo rooted at<tmp>/my project(space in the path) and assertstotal_grep_matches > 0. RED on the old newline + plain-xargscode (0 matches), GREEN after. Full suite green. (On Windows the assertion holds either way — the space bug is Unix-xargs-specific — so it's a Unix reproduce guard plus a cross-platform invariant.)Why a distillation, not a merge of #693
#693 has the right concept (NUL +
xargs -0, plus a correct Windows-newline branch), but implements the filelist write aschar buf[CBM_SZ_2K](2048) + two unboundedmemcpys of<root>+/+<file>+sep — a stack buffer overflow (CWE-121) reachable via long indexed paths (PATH_MAX4096 > 2048), and ships without a test. This PR keeps the concept, writes the records safely, and adds the reproduce-first guard.Closes #687
Co-authored with @lg320531124.