fix(tools): block search_files pattern flag injection in rg/grep

[Vesna-9]

What does this PR do?

The search_files content search built the ripgrep/grep command line by
shell-escaping the model-supplied pattern and appending it as a positional
argument with no end-of-options separator. Single-quoting via
_escape_shell_arg blocks shell injection, but the escaped token is still
handed to rg/grep as one argv element, which the tool happily parses as a
flag when it starts with a dash.

Before: a single search_files call with pattern="--pre=/path/to/script"
reached _search_with_rg as rg ... '--pre=/path/to/script' <path>. ripgrep
treats --pre=CMD as a preprocessor and runs CMD on every searched file, so
the pattern argument alone executes arbitrary commands on the host with the
agent's privileges. I confirmed this live: rg --line-number --no-heading --with-filename '--pre=/tmp/pre.sh' needle <tree> ran the script. Because
search_files is in the default-enabled file toolset and is the agent's
primary code-search tool, a prompt-injected instruction in a page or file the
agent is asked to search can drive it. The grep fallback was injectable too
(e.g. -f<file> reads the pattern list from an arbitrary file).

After: a literal -- end-of-options marker is inserted immediately before the
pattern in both _search_with_rg and _search_with_grep, so --pre=...,
-f..., and any other dash-prefixed token are treated as ordinary search
terms. A --pre=value pattern now matches that literal text and never starts
a preprocessor.

Related Issue

N/A

Type of Change

• 🔒 Security fix

Changes Made

• tools/file_operations.py: insert a -- end-of-options token before the
  escaped pattern/path in _search_with_rg (line ~2196) and
  _search_with_grep (line ~2326); document why escaping alone is
  insufficient.
• tests/tools/test_search_error_guard.py: add TestSearchPatternFlagInjection,
  parametrized over both backends — one test drops a real executable
  preprocessor and asserts a --pre=<script> pattern never runs it, another
  asserts a dash-prefixed token is searched literally.

How to Test

1. scripts/run_tests.sh tests/tools/test_search_error_guard.py — all pass.
2. Revert just the two cmd_parts.append("--") lines and rerun: the new
   TestSearchPatternFlagInjection cases fail (rg reports
   preprocessor command could not start, proving the flag was parsed).
3. Manual: `rg --line-number --no-heading --with-filename -- '--pre=/bin/sh' ` treats `--pre=/bin/sh` as a literal pattern instead of executing it.

Checklist

Code

• I've read the Contributing Guide
• My commit messages follow Conventional Commits (fix(scope):, etc.)
• I searched for existing PRs to make sure this isn't a duplicate
• My PR contains only changes related to this fix (no unrelated commits)
• I've run pytest tests/ -q and all tests pass
• I've added tests for my changes (required for bug fixes)
• I've tested on my platform: macOS 15 (Darwin 25.5)

Documentation & Housekeeping

• I've updated relevant documentation (README, docs/, docstrings) — or N/A
• I've updated cli-config.yaml.example if I added/changed config keys — or N/A
• I've updated CONTRIBUTING.md or AGENTS.md if I changed architecture or workflows — or N/A
• I've considered cross-platform impact (Windows, macOS) per the compatibility guide — or N/A
• I've updated tool descriptions/schemas if I changed tool behavior — or N/A

[alt-glitch]

Duplicate of #44908 — same fix at the same site. Both insert a literal -- end-of-options marker before the pattern in _search_with_rg and _search_with_grep (tools/file_operations.py) so a dash-prefixed pattern (--pre=..., -f...) is treated as a search term rather than an rg/grep flag. #44908 frames it as a robustness bug and this PR frames it as the --pre=CMD RCE vector, but the one-line mechanism is identical; #44908 is the earlier open version.

[teknium1]

Thanks for the focused security fix. The premise remains verified on current main: tools/file_operations.py:2282 and tools/file_operations.py:2412 append the escaped user pattern directly after rg/grep options, while tools/file_operations.py:2247-2252 dispatches content search to exactly those backends. Adding -- before the positional pattern is the appropriate narrow fix, and the new tests exercise both available real backends.

The submitted diff applies cleanly to current main (gh pr diff 50357 | git apply --check), with the two production hunks matching at their moved locations.

Automated hermes-sweeper review.

[GottZ]

This was generated by AI during triage.

Summary

Three PRs address related search_files hardening at the rg/grep backend boundary, but their diffs target distinct causes: #39858 repairs masked search errors and diagnostic parsing, #40707 fixes the resulting files_only whitespace regression, and #50357 blocks option injection through dash-prefixed patterns.

Related pull requests

• #39858 [merged] related — (+266/-18) — merged reference implementation: made rg/grep failures reachable with pipefail, separated diagnostics from usable payload, and preserved partial matches instead of parsing errors as results or discarding valid output.
• #40707 [closed] related — (+84/-3) — distinct regression fix, still relevant although closed: makes diagnostic splitting mode-aware so files_only preserves paths containing spaces while retaining hard-error handling. The diff addresses the keep_open review on #40707 by adding the requested parameterized end-to-end coverage for both production backends.
• #50357 related — (+73/-4) — merge: inserts an end-of-options marker before the pattern in both rg and grep commands, preventing --pre/-f argument injection while preserving literal dash-prefixed searches; this matches the keep_open review on #50357 and includes real-backend regression tests.

Duplicates

None among #39858, #40707, and #50357: they modify the same search paths but fix different root causes. Contributor discussion identifies #44908 as the same-site, same-mechanism duplicate of #50357.

Suggested consolidation

Merge #50357 as the focused option-injection fix; its production hunks and backend tests directly cover the reported cause. Keep #39858 as the merged reference implementation and treat #40707 as a separate regression fix rather than a duplicate; no listed PR should be closed as a duplicate, while #44908 can be closed in favor of #50357 if its current diff remains mechanically identical.

Cross-PR triage: Reviewed 3 pull requests and 0 issues in this complex. Each diff was read against this issue; Assessment working set: 30 kB of PR diffs, 12 kB of issue/PR text, 4 kB of discussion (6 comments), 0 verify verdicts. verdicts reflect diff content, not PR titles. Part of an automated triage batch.