security: block plain shell variable expansion and forbidden path args

src/security/policy.rs

@@ -404,6 +404,79 @@ fn contains_unquoted_char(command: &str, target: char) -> bool {
     false
 }
 
+/// Detect unquoted shell variable expansions like `$HOME`, `$1`, `$?`.
+///
+/// Escaped dollars (`\$`) are ignored. Variables inside single quotes are
+/// treated as literals and therefore ignored.
+fn contains_unquoted_shell_variable_expansion(command: &str) -> bool {
+    let mut quote = QuoteState::None;
+    let mut escaped = false;
+    let chars: Vec<char> = command.chars().collect();
+
+    for i in 0..chars.len() {
+        let ch = chars[i];
+
+        match quote {
+            QuoteState::Single => {
+                if ch == '\'' {
+                    quote = QuoteState::None;
+                }
+                continue;
+            }
+            QuoteState::Double => {
+                if escaped {
+                    escaped = false;
+                    continue;
+                }
+                if ch == '\\' {
+                    escaped = true;
+                    continue;
+                }
+                if ch == '"' {
+                    quote = QuoteState::None;
+                    continue;
+                }
+            }
+            QuoteState::None => {
+                if escaped {
+                    escaped = false;
+                    continue;
+                }
+                if ch == '\\' {
+                    escaped = true;
+                    continue;
+                }
+                if ch == '\'' {
+                    quote = QuoteState::Single;
+                    continue;
+                }
+                if ch == '"' {
+                    quote = QuoteState::Double;
+                    continue;
+                }
+            }
+        }
+
+        if ch != '$' {
+            continue;
+        }
+
+        let Some(next) = chars.get(i + 1).copied() else {
+            continue;
+        };
+        if next.is_ascii_alphanumeric()
+            || matches!(
+                next,
+                '_' | '{' | '(' | '#' | '?' | '!' | '$' | '*' | '@' | '-'
+            )
+        {
+            return true;
+        }
+    }
+
+    false
+}
+
 impl SecurityPolicy {
     // ── Risk Classification ──────────────────────────────────────────────
     // Risk is assessed per-segment (split on shell operators), and the
@@ -583,10 +656,12 @@ impl SecurityPolicy {
         }
 
         // Block subshell/expansion operators — these allow hiding arbitrary
-        // commands inside an allowed command (e.g. `echo $(rm -rf /)`)
+        // commands inside an allowed command (e.g. `echo $(rm -rf /)`) and
+        // bypassing path checks through variable indirection.
         if command.contains('`')
             || command.contains("$(")
             || command.contains("${")
+            || contains_unquoted_shell_variable_expansion(command)
             || command.contains("<(")
             || command.contains(">(")
         {
@@ -1431,6 +1506,13 @@ mod tests {
         assert!(!p.is_command_allowed("echo ${IFS}cat${IFS}/etc/passwd"));
     }
 
+    #[test]
+    fn command_injection_plain_dollar_var_blocked() {
+        let p = default_policy();
+        assert!(!p.is_command_allowed("cat $HOME/.ssh/id_rsa"));
+        assert!(!p.is_command_allowed("cat $SECRET_FILE"));
+    }
+
     #[test]
     fn command_injection_tee_blocked() {
         let p = default_policy();

src/tools/shell.rs

@@ -23,6 +23,64 @@ pub struct ShellTool {
     runtime: Arc<dyn RuntimeAdapter>,
 }
 
+fn is_env_assignment(word: &str) -> bool {
+    word.contains('=')
+        && word
+            .chars()
+            .next()
+            .is_some_and(|c| c.is_ascii_alphabetic() || c == '_')
+}
+
+fn strip_wrapping_quotes(token: &str) -> &str {
+    token.trim_matches(|c| c == '"' || c == '\'')
+}
+
+fn forbidden_path_argument(security: &SecurityPolicy, command: &str) -> Option<String> {
+    let mut normalized = command.to_string();
+    for sep in ["&&", "||"] {
+        normalized = normalized.replace(sep, "\x00");
+    }
+    for sep in ['\n', ';', '|'] {
+        normalized = normalized.replace(sep, "\x00");
+    }
+
+    for segment in normalized.split('\x00') {
+        let tokens: Vec<&str> = segment.split_whitespace().collect();
+        if tokens.is_empty() {
+            continue;
+        }
+
+        // Skip leading env assignments and executable token.
+        let mut idx = 0;
+        while idx < tokens.len() && is_env_assignment(tokens[idx]) {
+            idx += 1;
+        }
+        if idx >= tokens.len() {
+            continue;
+        }
+        idx += 1;
+
+        for token in &tokens[idx..] {
+            let candidate = strip_wrapping_quotes(token);
+            if candidate.is_empty() || candidate.starts_with('-') || candidate.contains("://") {
+                continue;
+            }
+
+            let looks_like_path = candidate.starts_with('/')
+                || candidate.starts_with("./")
+                || candidate.starts_with("../")
+                || candidate.starts_with("~/")
+                || candidate.contains('/');
+
+            if looks_like_path && !security.is_path_allowed(candidate) {
+                return Some(candidate.to_string());
+            }
+        }
+    }
+
+    None
+}
+
 impl ShellTool {
     pub fn new(security: Arc<SecurityPolicy>, runtime: Arc<dyn RuntimeAdapter>) -> Self {
         Self { security, runtime }
@@ -114,6 +172,14 @@ impl Tool for ShellTool {
             }
         }
 
+        if let Some(path) = forbidden_path_argument(&self.security, command) {
+            return Ok(ToolResult {
+                success: false,
+                output: String::new(),
+                error: Some(format!("Path blocked by security policy: {path}")),
+            });
+        }
+
         if !self.security.record_action() {
             return Ok(ToolResult {
                 success: false,
@@ -296,6 +362,21 @@ mod tests {
         assert!(!result.success);
     }
 
+    #[tokio::test]
+    async fn shell_blocks_absolute_path_argument() {
+        let tool = ShellTool::new(test_security(AutonomyLevel::Supervised), test_runtime());
+        let result = tool
+            .execute(json!({"command": "cat /etc/passwd"}))
+            .await
+            .expect("absolute path argument should be blocked");
+        assert!(!result.success);
+        assert!(result
+            .error
+            .as_deref()
+            .unwrap_or("")
+            .contains("Path blocked"));
+    }
+
     fn test_security_with_env_cmd() -> Arc<SecurityPolicy> {
         Arc::new(SecurityPolicy {
             autonomy: AutonomyLevel::Supervised,
@@ -361,28 +442,37 @@ mod tests {
     }
 
     #[tokio::test]
-    async fn shell_preserves_path_and_home() {
+    async fn shell_preserves_path_and_home_for_env_command() {
         let tool = ShellTool::new(test_security_with_env_cmd(), test_runtime());
 
+        let result = tool
+            .execute(json!({"command": "env"}))
+            .await
+            .expect("env command should succeed");
+        assert!(result.success);
+        assert!(
+            result.output.contains("HOME="),
+            "HOME should be available in shell environment"
+        );
+        assert!(
+            result.output.contains("PATH="),
+            "PATH should be available in shell environment"
+        );
+    }
+
+    #[tokio::test]
+    async fn shell_blocks_plain_variable_expansion() {
+        let tool = ShellTool::new(test_security_with_env_cmd(), test_runtime());
         let result = tool
             .execute(json!({"command": "echo $HOME"}))
             .await
-            .expect("echo HOME command should succeed");
-        assert!(result.success);
-        assert!(
-            !result.output.trim().is_empty(),
-            "HOME should be available in shell"
-        );
-
-        let result = tool
-            .execute(json!({"command": "echo $PATH"}))
-            .await
-            .expect("echo PATH command should succeed");
-        assert!(result.success);
-        assert!(
-            !result.output.trim().is_empty(),
-            "PATH should be available in shell"
-        );
+            .expect("plain variable expansion should be blocked");
+        assert!(!result.success);
+        assert!(result
+            .error
+            .as_deref()
+            .unwrap_or("")
+            .contains("not allowed"));
     }
 
     #[tokio::test(flavor = "current_thread")]