Files
zed/crates/agent/src/pattern_extraction.rs
T
Richard FeldmanandZed Zippy 6ff5dec7b7 Fix shell injection vulnerability in terminal tool permissions (#47807)
<img width="1110" height="280" alt="Screenshot 2026-01-28 at 3 35 52 PM"
src="https://github.com/user-attachments/assets/4d467e2c-2e7b-4ec7-bc87-6f0df8e667f0"
/>

<img width="1094" height="411" alt="Screenshot 2026-01-28 at 3 40 54 PM"
src="https://github.com/user-attachments/assets/f559df93-e72e-4457-ba1b-f7d6239f3285"
/>


Previously, if a user configured `^ls` as an always-allow pattern, an
attacker could craft a command like `ls && rm -rf /` which would be
auto-approved because the regex only matched the beginning of the
command string.

Now the command is parsed into individual sub-commands (`ls`, `rm -rf
/`) and EACH sub-command must match an allow pattern for auto-approval.
This prevents shell injection attacks using operators like:
- `&&` and `||` (boolean operators)
- `;` and `&` (sequential/background execution)
- `|` (pipes)
- Newlines
- Command substitution (`$()` and backticks)
- Process substitution (`<()` and `>()`)

## Matching Logic

- **always_deny**: if ANY sub-command matches, deny the entire command
- **always_confirm**: if ANY sub-command matches, require confirmation
(unless always_deny matched, in which case deny)
- **always_allow**: ALL sub-commands must match for auto-approval
(unless always_confirm or always_deny matched, in which case defer to
those)
- If parsing fails, or if the shell is unsupported, then always_allow is
disabled for this command

As usual, `always_allow_tool_actions` supercedes all of these. If it is
`true`, then we always allow all tool calls, no questions asked.

## Shell Compatibility

The shell parser only supports POSIX-like command chaining syntax (`&&`,
`||`, `;`, `|`).

**Supported shells:** Posix (sh, bash, dash, zsh), Fish 3.0+, PowerShell
7+/Pwsh, Cmd, Xonsh, Csh, Tcsh

**Unsupported shells:** Nushell (uses `and`/`or` keywords), Elvish (uses
`and`/`or` keywords), Rc (Plan 9 shell - no `&&`/`||` operators)

For unsupported shells:
- The "Always allow" UI options are hidden for the terminal tool
- If the user has `always_allow` patterns configured in settings, they
will see a `Deny` with an explanatory error message

(No release notes because granular tool permissions are behind a feature
flag.)

Release Notes:

- N/A

---------

Co-authored-by: Zed Zippy <234243425+zed-zippy[bot]@users.noreply.github.com>
2026-01-28 20:51:39 +00:00

174 lines
5.6 KiB
Rust

use crate::shell_parser::extract_commands;
use url::Url;
/// Extracts the command name from a shell command using the shell parser.
///
/// This parses the command properly to extract just the command name (first word),
/// handling shell syntax correctly. Returns `None` if parsing fails or if the
/// command name contains path separators (for security reasons).
fn extract_command_name(command: &str) -> Option<String> {
let commands = extract_commands(command)?;
let first_command = commands.first()?;
let first_token = first_command.split_whitespace().next()?;
// Only allow alphanumeric commands with hyphens/underscores.
// Reject paths like "./script.sh" or "/usr/bin/python" to prevent
// users from accidentally allowing arbitrary script execution.
if first_token
.chars()
.all(|c| c.is_alphanumeric() || c == '-' || c == '_')
{
Some(first_token.to_string())
} else {
None
}
}
/// Extracts a regex pattern from a terminal command based on the first token (command name).
///
/// Returns `None` for commands starting with `./`, `/`, or other path-like prefixes.
/// This is a deliberate security decision: we only allow pattern-based "always allow"
/// rules for well-known command names (like `cargo`, `npm`, `git`), not for arbitrary
/// scripts or absolute paths which could be manipulated by an attacker.
pub fn extract_terminal_pattern(command: &str) -> Option<String> {
let command_name = extract_command_name(command)?;
Some(format!("^{}\\b", regex::escape(&command_name)))
}
pub fn extract_terminal_pattern_display(command: &str) -> Option<String> {
extract_command_name(command)
}
pub fn extract_path_pattern(path: &str) -> Option<String> {
let parent = std::path::Path::new(path).parent()?;
let parent_str = parent.to_str()?;
if parent_str.is_empty() || parent_str == "/" {
return None;
}
Some(format!("^{}/", regex::escape(parent_str)))
}
pub fn extract_path_pattern_display(path: &str) -> Option<String> {
let parent = std::path::Path::new(path).parent()?;
let parent_str = parent.to_str()?;
if parent_str.is_empty() || parent_str == "/" {
return None;
}
Some(format!("{}/", parent_str))
}
pub fn extract_url_pattern(url: &str) -> Option<String> {
let parsed = Url::parse(url).ok()?;
let domain = parsed.host_str()?;
Some(format!("^https?://{}", regex::escape(domain)))
}
pub fn extract_url_pattern_display(url: &str) -> Option<String> {
let parsed = Url::parse(url).ok()?;
let domain = parsed.host_str()?;
Some(domain.to_string())
}
#[cfg(test)]
mod tests {
use super::*;
#[test]
fn test_extract_terminal_pattern() {
assert_eq!(
extract_terminal_pattern("cargo build --release"),
Some("^cargo\\b".to_string())
);
assert_eq!(
extract_terminal_pattern("npm install"),
Some("^npm\\b".to_string())
);
assert_eq!(
extract_terminal_pattern("git-lfs pull"),
Some("^git\\-lfs\\b".to_string())
);
assert_eq!(
extract_terminal_pattern("my_script arg"),
Some("^my_script\\b".to_string())
);
assert_eq!(extract_terminal_pattern("./script.sh arg"), None);
assert_eq!(extract_terminal_pattern("/usr/bin/python arg"), None);
}
#[test]
fn test_extract_terminal_pattern_display() {
assert_eq!(
extract_terminal_pattern_display("cargo build --release"),
Some("cargo".to_string())
);
assert_eq!(
extract_terminal_pattern_display("npm install"),
Some("npm".to_string())
);
}
#[test]
fn test_extract_path_pattern() {
assert_eq!(
extract_path_pattern("/Users/alice/project/src/main.rs"),
Some("^/Users/alice/project/src/".to_string())
);
assert_eq!(
extract_path_pattern("src/lib.rs"),
Some("^src/".to_string())
);
assert_eq!(extract_path_pattern("file.txt"), None);
assert_eq!(extract_path_pattern("/file.txt"), None);
}
#[test]
fn test_extract_path_pattern_display() {
assert_eq!(
extract_path_pattern_display("/Users/alice/project/src/main.rs"),
Some("/Users/alice/project/src/".to_string())
);
assert_eq!(
extract_path_pattern_display("src/lib.rs"),
Some("src/".to_string())
);
}
#[test]
fn test_extract_url_pattern() {
assert_eq!(
extract_url_pattern("https://github.com/user/repo"),
Some("^https?://github\\.com".to_string())
);
assert_eq!(
extract_url_pattern("http://example.com/path?query=1"),
Some("^https?://example\\.com".to_string())
);
assert_eq!(extract_url_pattern("not a url"), None);
}
#[test]
fn test_extract_url_pattern_display() {
assert_eq!(
extract_url_pattern_display("https://github.com/user/repo"),
Some("github.com".to_string())
);
assert_eq!(
extract_url_pattern_display("http://api.example.com/v1/users"),
Some("api.example.com".to_string())
);
}
#[test]
fn test_special_chars_are_escaped() {
assert_eq!(
extract_path_pattern("/path/with (parens)/file.txt"),
Some("^/path/with \\(parens\\)/".to_string())
);
assert_eq!(
extract_url_pattern("https://test.example.com/path"),
Some("^https?://test\\.example\\.com".to_string())
);
}
}