fix(agent): shell-escape paths interpolated by grep/file_glob/is_file_path helpers (#11132) by david-engelmann · Pull Request #11391 · warpdotdev/warp

david-engelmann · 2026-05-20T05:58:46Z

Description

The internal helpers behind the AI agent's grep and file_glob tools build shell commands by interpolating an agent-supplied path inside double quotes and handing the result to Session::execute_command. Inside POSIX "..." the shell still expands $VAR, backticks, and $(...); inside PowerShell "..." it still expands $x (including $env:USERPROFILE) and treats backticks specially. So a path containing any of those metacharacters is parsed — and potentially executed — by the user's shell before the underlying tool sees it.

is_file_path and is_git_repository are particularly exposed because they run as side-effects of the grep and file_glob tools (run_grep → is_file_path + is_git_repository, run_file_glob → is_git_repository) before the user-facing command is shown, so even path values the user never typed flow into the shell verbatim.

Fixes #11132.

Root cause

Eight unquoted call sites across three files:

File	Function	Line(s) on master
`app/src/ai/blocklist/action_model/execute.rs`	`is_file_path` (POSIX)	1299
`app/src/ai/blocklist/action_model/execute.rs`	`is_file_path` (PowerShell)	1297
`app/src/ai/blocklist/action_model/execute.rs`	`is_git_repository`	1310
`app/src/ai/blocklist/action_model/execute/grep.rs`	`run_git_grep_command`	488
`app/src/ai/blocklist/action_model/execute/grep.rs`	`run_grep_command`	547
`app/src/ai/blocklist/action_model/execute/grep.rs`	`run_select_string_command`	599
`app/src/ai/blocklist/action_model/execute/file_glob.rs`	`run_find_command`	295
`app/src/ai/blocklist/action_model/execute/file_glob.rs`	`run_powershell_get_childitem_command`	339

Each one used format!("… \"{path}\" …") — a literal double-quoted interpolation. The sister fixes for cd "{path}" (open_repo_folder, conversation-restore) and git checkout (#10444 / #10639) already use the right primitive: warp_util::path::ShellFamily::shell_escape.

Fix

Each helper grew a pure build_*_command(...) extraction that takes the inputs and a ShellFamily / ShellType and returns the rendered command. The async wrappers are now thin and call the pure builder. The pure builders are unit-tested without a live Session.

// Before
async fn is_file_path(path: &str, session: &Session) -> bool {
    let command = if session.shell().shell_type() == ShellType::PowerShell {
        format!("if (Test-Path -PathType Leaf \"{path}\") {{ exit 0 }} else {{ exit 1 }}")
    } else {
        format!("test -f \"{path}\"")
    };
    …
}

// After
fn build_is_file_path_command(path: &str, family: ShellFamily) -> String {
    let escaped_path = family.shell_escape(path);
    match family {
        ShellFamily::PowerShell =>
            format!("if (Test-Path -PathType Leaf {escaped_path}) {{ exit 0 }} else {{ exit 1 }}"),
        ShellFamily::Posix => format!("test -f {escaped_path}"),
    }
}

async fn is_file_path(path: &str, session: &Session) -> bool {
    let command = build_is_file_path_command(path, session.shell_family());
    …
}

The same shape is applied to all eight sites.

Query escaping (escape_double_quotes / powershell_escape_double_quotes) is intentionally left alone — queries are regex literals with their own escaping concerns, separate from path-as-shell-word handling.

Shell-level before/after

$ bash -c 'test -f "/tmp/innocent$(touch /tmp/PROBE_RAN)"'
$ ls /tmp/PROBE_RAN
/tmp/PROBE_RAN   ← touch ran via command substitution (BUG)

$ bash -c 'test -f /tmp/innocent\$\(touch\ /tmp/PROBE_RAN\)'
$ ls /tmp/PROBE_RAN
ls: /tmp/PROBE_RAN: No such file or directory   ← shell-escaped: no side effect (FIX)

This is the security boundary the new build_is_file_path_command (and its siblings) enforces.

Testing

21 new unit tests across three sibling *_tests.rs files:

POSIX backslash-escaping: spaces, $(...), backticks, ~ mid-path, empty-string paths
PowerShell backtick-escaping: spaces, $env:USERPROFILE, drive-letter paths
A regression test per builder that pins the security boundary — no bare $(...) or backticks may survive into the rendered command

$ cargo nextest run -p warp -E 'test(path_quoting::) or test(file_glob::tests::)'
21 tests run: 21 passed; 0 failed

$ cargo clippy -p warp --tests -- -D warnings
Finished (no warnings)

$ cargo fmt -p warp -- --check
clean

Server API dependencies

None — this is client-only.

Agent Mode

Unchecked (external contribution).

Changelog

CHANGELOG-BUG-FIX: Paths containing shell metacharacters (spaces, $, backticks, $(...), etc.) are now correctly passed through the AI agent's grep / file_glob / file-existence helpers without being expanded by the user's shell.

Fixes #11132

…_path helpers (warpdotdev#11132) The internal helpers behind the AI agent's `grep` and `file_glob` tools built shell commands by interpolating an agent-supplied path inside double quotes and handing the result to `Session::execute_command`. POSIX double quotes still expand `$VAR`, backticks, and `$(...)`; PowerShell double quotes still expand `$x` and `$env:USERPROFILE`. A path containing any of those metacharacters was therefore parsed (and potentially executed) by the user's shell before the underlying tool received it — e.g. `is_file_path("/tmp/innocent$(touch ~/PROBE_RAN)")` would run `touch` as a side effect of the existence probe. Fixes warpdotdev#11132. Eight call sites updated: * `execute.rs` — `is_file_path` (POSIX + PowerShell), `is_git_repository` * `execute/grep.rs` — `run_git_grep_command`, `run_grep_command`, `run_select_string_command` * `execute/file_glob.rs` — `run_find_command`, `run_powershell_get_childitem_command` Each path now flows through `ShellFamily::shell_escape` (the same primitive used by the recently-fixed cd / open-folder / cli-install paths), so a single literal word reaches the underlying `test -f` / `Test-Path` / `git -C` / `grep` / `find` / `Get-ChildItem` / `Select-String` regardless of any metacharacters in the path. The query escaping (`escape_double_quotes` / `powershell_escape_double_quotes`) is intentionally left alone — queries are regex literals with their own escaping concerns, separate from path-as-shell-word handling. To keep the change reviewable, each helper grew a pure `build_*_command(...)` extraction that takes the inputs and a `ShellFamily` / `ShellType` and returns the rendered command. The async wrappers are thin; the builders are unit-tested without a live `Session`. Tests: 21 new unit tests across three sibling `*_tests.rs` files cover POSIX backslash-escaping (spaces, `$(...)`, backticks, `~` mid-path), PowerShell backtick-escaping (spaces, `$env:USERPROFILE`), empty-path handling, and pin the security boundary that `$(...)` and backtick command substitution can never survive into the rendered command. ``` $ cargo nextest run -p warp -E 'test(path_quoting::) or test(file_glob::tests::)' 21 passed; 0 failed $ cargo clippy -p warp --tests -- -D warnings clean $ cargo fmt -p warp -- --check clean ``` Shell-level before/after (the security boundary this fixes): ``` $ bash -c 'test -f "/tmp/innocent$(touch /tmp/PROBE_RAN)"' $ ls /tmp/PROBE_RAN /tmp/PROBE_RAN ← touch ran via command substitution (BUG) $ bash -c 'test -f /tmp/innocent\$$touch\ /tmp/PROBE_RAN$' $ ls /tmp/PROBE_RAN ls: /tmp/PROBE_RAN: No such file or directory ← shell-escaped: no side effect (FIX) ``` Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

oz-for-oss · 2026-05-20T05:59:04Z

@david-engelmann

I'm starting a first review of this pull request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR switches several grep/file_glob/is_file_path shell command builders from double-quoted path interpolation to ShellFamily::shell_escape and adds unit coverage for the extracted builders. No approved spec context was available for comparison.

Concerns

🚨 [CRITICAL] [SECURITY] file_glob still routes git-repository searches through run_git_ls_files_command, which builds git ls-files ... -- '{target_path/...}' from the raw target_path. Because run_file_glob chooses this branch before the newly escaped find/Get-ChildItem builders whenever the target is in a git repo, a directory path containing a single quote plus shell substitution can still break out of the quoted pathspec and execute in the user's shell. Please apply the same argument-safe escaping to the git-backed file_glob path/pattern arguments and add a regression test for a git-repo path containing quotes/command substitution.

Security

Critical path command injection remains in the git-backed file_glob implementation.

Verdict

Found: 1 critical, 0 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

…eview) `run_git_ls_files_command` wrapped each joined `<target_path>/<pattern>` in literal single quotes (`'...'`). A target_path containing a single quote closed the wrapper and let any following metacharacters parse as shell input — a real command-injection vector once the agent's path contained `'$(...)'` or backticks. Oz flagged this on PR warpdotdev#11391: > [CRITICAL] [SECURITY] file_glob still routes git-repository searches > through run_git_ls_files_command, which builds > `git ls-files ... -- '{target_path/...}'` from the raw target_path. Extract the command into `build_git_ls_files_command` (matching the pure-builder pattern of the other helpers in this PR) and run each pattern arg through `ShellFamily::from(shell_type).shell_escape` instead of single-quoting. The shell consumes the escapes and hands `git ls-files` the literal pathspec, so glob characters in the pattern (e.g. `*.rs`) still survive for git's own matching. This also fixes the previously-broken PowerShell variant, which was shipping POSIX-only single-quoting on Windows. Adds 6 regression tests covering: spaces, embedded single quote + command substitution (the exact injection shape), command substitution alone, backticks, PowerShell-shaped paths, and the top-level + subdir doubled pattern arg. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

david-engelmann · 2026-05-20T15:36:52Z

Thanks for catching that — you're right, the git-backed branch was wrapping each <target_path>/<pattern> in literal single quotes, so a single quote inside the path closed the wrapper and let the rest parse as shell input. Real injection vector.

Addressed in e5698c5a:

Extracted build_git_ls_files_command(patterns, target_path, shell_type, shell_launch_data) matching the pure-builder pattern of the other helpers in this PR.
Each pattern arg now runs through ShellFamily::from(shell_type).shell_escape instead of format!(\"'{...}'\", ...). The shell consumes the escapes and hands git ls-files the literal pathspec, so *.rs (or any glob) still survives for git's own pathspec matching.
As a side effect this also fixes the previously-broken PowerShell variant — the old single-quote wrapping was POSIX-only and was shipping bare on Windows.

Added 6 regression tests in file_glob_tests.rs::git_ls_files_*:

git_ls_files_posix_escapes_target_path_spaces
git_ls_files_posix_escapes_single_quote_in_target_path — the exact injection shape from your review
git_ls_files_posix_escapes_command_substitution
git_ls_files_posix_escapes_backticks
git_ls_files_powershell_escapes_target_path
git_ls_files_emits_both_top_level_and_subdir_pattern_args

13/13 in file_glob pass; clippy + fmt clean.

/oz-review

oz-for-oss · 2026-05-20T15:37:13Z

@david-engelmann

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I completed the review and no human review was requested for this pull request.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss

Overview

This PR extracts command builders and applies ShellFamily::shell_escape to path arguments used by the agent grep, file_glob, is_file_path, and is_git_repository helpers. The path escaping direction matches the reported issue, but the same shell-command construction still interpolates other agent-supplied fields without shell-safe quoting.

Concerns

Grep queries are still inserted into shell commands inside double quotes after only escaping \"; POSIX and PowerShell both expand command substitutions / variables in that context.
Non-git file_glob patterns are still inserted into shell commands inside single quotes, so an embedded single quote can break out and execute additional shell syntax.

Security

Agent-supplied grep queries and file_glob patterns remain shell-injection vectors despite the new path escaping. These inputs should be shell-escaped/quoted for the active shell, or passed without going through shell string interpolation.

Verdict

Found: 2 critical, 0 important, 0 suggestions

Request changes

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

oz-for-oss · 2026-05-20T15:42:27Z

+fn build_grep_command(queries: &[String], target_path: &str) -> String {
+    let mut grep_command = "grep --color=never -nrIHE --devices=skip".to_string();
+    for query in queries {
+        grep_command.push_str(format!(" -e \"{}\"", escape_double_quotes(query)).as_str());


🚨 [CRITICAL] [SECURITY] query is agent-supplied and still goes inside double quotes after only escaping \"; POSIX shells expand $(), $VAR, and backticks inside double quotes, so a grep query can execute before grep runs. Render each -e value with shell-family escaping/quoting, or avoid shell string interpolation for query argv construction.

oz-for-oss · 2026-05-20T15:42:27Z

+fn build_find_command(patterns: &[String], target_path: &str) -> String {
+    let pattern_args = patterns
+        .iter()
+        .map(|pattern| format!(" -name '{pattern}'"))


🚨 [CRITICAL] [SECURITY] pattern is agent-supplied and embedding it in single quotes lets an embedded ' close the string (for example, x'; touch /tmp/pwn; '), so non-git file_glob can still execute shell input even though the path is escaped. Shell-escape/quote each pattern for the active shell before interpolating it.

…arpdotdev#11132 review) The first round of this PR escaped agent-supplied paths but left other agent-controlled inputs going through unsafe quoting. Oz flagged two remaining injection vectors: > Grep queries are still inserted into shell commands inside double quotes > after only escaping `\"`; POSIX and PowerShell both expand command > substitutions / variables in that context. > Non-git file_glob patterns are still inserted into shell commands inside > single quotes, so an embedded single quote can break out and execute > additional shell syntax. Replace both unsafe quoting strategies with `ShellFamily::shell_escape`: - `build_git_grep_command`, `build_grep_command`, `build_select_string_command`: agent queries now go through `family.shell_escape(query)` instead of `"..."` + `\"`-only escape. The shell consumes the escapes and hands grep/Select-String the literal query for regex matching. - `build_find_command`, `build_get_childitem_command`: agent glob patterns now go through `ShellFamily::shell_escape(pattern)` instead of `'...'` wrapping. The shell consumes the escapes and `find`/`Get-ChildItem` see the literal pattern for their own glob matching. - Dropped the now-unused `escape_double_quotes` and `powershell_escape_double_quotes` helpers in grep.rs. Adds 8 new injection-vector regression tests covering: - `$(...)` in grep queries (POSIX, both git and non-git paths) - backticks in grep queries - `$env:VAR` in grep queries and Select-String patterns (PowerShell) - single quote + `$(...)` in find patterns - `$env:VAR` in Get-ChildItem patterns 97/97 in `ai::blocklist::action_model::execute` pass. clippy + fmt clean. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>

david-engelmann · 2026-05-20T18:15:58Z

Both flagged injection vectors closed in `a2f5cfd9`.

Grep queries (POSIX + PowerShell, both git grep and plain grep and Select-String):

Dropped the "..." + escape_double_quotes-only / powershell_escape_double_quotes-only strategy. Each query now runs through ShellFamily::from(shell_type).shell_escape(query). The shell consumes the escapes and the grep tool receives the literal query string for its own regex matching.
Removed the now-unused escape_double_quotes / powershell_escape_double_quotes helpers.

Non-git file_glob patterns (POSIX find + PowerShell Get-ChildItem):

Dropped the '...' wrapping in both build_find_command and build_get_childitem_command. Patterns now go through ShellFamily::shell_escape(pattern). The shell consumes the escapes and find/Get-ChildItem see the literal pattern (with * etc. preserved) for their own glob matching.

Added 8 new injection-vector regression tests:

POSIX: $(...) and backticks in grep queries (both git-backed and plain branches)
POSIX: single-quote + $(...) in find patterns
PowerShell: $env:VAR in git grep queries
PowerShell: $env:VAR in Select-String patterns
PowerShell: $env:VAR in Get-ChildItem patterns

Existing path tests updated where assertions previously baked in the old '...' / "..." wrapping shape.

97/97 in ai::blocklist::action_model::execute pass; clippy + fmt clean.

/oz-review

oz-for-oss · 2026-05-20T18:16:20Z

@david-engelmann

I'm re-reviewing this pull request in response to a review request.

You can view the conversation on Warp.

I reviewed this pull request and requested human review from: @moirahuang.

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

Oz no longer requests changes for this pull request after the latest automated review.

oz-for-oss

Overview

This PR replaces double-quoted interpolation in the agent grep, file_glob, file-existence, and git-repository shell helpers with the shared shell-family escaping helper, and adds unit coverage for POSIX and PowerShell metacharacter handling.

Concerns

No blocking correctness, security, or spec-alignment concerns found in the annotated diff. The attached spec context does not define additional implementation commitments.

Verdict

Found: 0 critical, 0 important, 0 suggestions

Approve

Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).

Powered by Oz

cla-bot Bot added the cla-signed label May 20, 2026

github-actions Bot added the external-contributor Indicates that a PR has been opened by someone outside the Warp team. label May 20, 2026

oz-for-oss Bot previously requested changes May 20, 2026

View reviewed changes

david-engelmann mentioned this pull request May 20, 2026

fix(tab-configs): shell-escape worktree path prefix so spaces in repo names don't break git worktree add / cd (#11144) #11392

Open

oz-for-oss Bot previously requested changes May 20, 2026

View reviewed changes

oz-for-oss Bot reviewed May 20, 2026

View reviewed changes

oz-for-oss Bot requested a review from moirahuang May 20, 2026 18:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(agent): shell-escape paths interpolated by grep/file_glob/is_file_path helpers (#11132)#11391

fix(agent): shell-escape paths interpolated by grep/file_glob/is_file_path helpers (#11132)#11391
david-engelmann wants to merge 3 commits into
warpdotdev:masterfrom
david-engelmann:david/11132-agent-path-quoting

david-engelmann commented May 20, 2026

Uh oh!

oz-for-oss Bot commented May 20, 2026 •

edited

Loading

Uh oh!

oz-for-oss Bot left a comment

Uh oh!

david-engelmann commented May 20, 2026

Uh oh!

oz-for-oss Bot commented May 20, 2026 •

edited

Loading

Uh oh!

oz-for-oss Bot left a comment

Uh oh!

oz-for-oss Bot May 20, 2026

Uh oh!

oz-for-oss Bot May 20, 2026

Uh oh!

david-engelmann commented May 20, 2026

Uh oh!

oz-for-oss Bot commented May 20, 2026 •

edited

Loading

Uh oh!

oz-for-oss Bot left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

david-engelmann commented May 20, 2026

Description

Root cause

Fix

Shell-level before/after

Testing

Server API dependencies

Agent Mode

Changelog

Uh oh!

oz-for-oss Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Security

Verdict

Uh oh!

david-engelmann commented May 20, 2026

Uh oh!

oz-for-oss Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Security

Verdict

Uh oh!

oz-for-oss Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

oz-for-oss Bot May 20, 2026

Choose a reason for hiding this comment

Uh oh!

david-engelmann commented May 20, 2026

Uh oh!

oz-for-oss Bot commented May 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

oz-for-oss Bot left a comment

Choose a reason for hiding this comment

Overview

Concerns

Verdict

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

oz-for-oss Bot commented May 20, 2026 •

edited

Loading

oz-for-oss Bot commented May 20, 2026 •

edited

Loading

oz-for-oss Bot commented May 20, 2026 •

edited

Loading