fix(command-risk): resolve relative targets after cd, stop reading redirects as operands

crates/jcode-command-risk/src/assess_tests.rs

@@ -723,3 +723,88 @@ fn issue_1753_inert_text_stays_safe() {
         assert!(level(command).runs_immediately(), "{command:?}");
     }
 }
+
+#[test]
+fn issue_932_a_leading_cd_moves_where_relative_targets_resolve() {
+    // A relative delete after `cd` resolves against the directory the shell is
+    // actually in, not the tool call's working directory. Without the tracking
+    // these looked like ordinary deletes inside the workspace.
+    assert_eq!(level("cd /etc && rm -rf passwd"), RiskLevel::Catastrophic);
+    assert_eq!(level("cd /etc; rm -rf passwd"), RiskLevel::Catastrophic);
+    assert_eq!(
+        level("cd / && cd etc && rm -rf passwd"),
+        RiskLevel::Catastrophic
+    );
+    assert_eq!(level("cd ~/.ssh && rm -rf *"), RiskLevel::Catastrophic);
+    // `cd` with no operand goes home.
+    assert_eq!(level("cd && rm -rf Documents"), RiskLevel::Catastrophic);
+
+    // A temp-directory cleanup must not be attributed to the workspace.
+    let assessment = assess("cd /tmp && rm -rf build-xyz", &ctx());
+    assert!(
+        assessment
+            .findings
+            .iter()
+            .all(|finding| finding.target.as_deref() != Some("/home/u/proj/build-xyz")),
+        "{:?}",
+        assessment.findings
+    );
+    assert!(
+        assessment.level.runs_immediately(),
+        "{:?}",
+        assessment.findings
+    );
+
+    // A destination that cannot be resolved statically is not assumed to be
+    // the workspace either.
+    let assessment = assess("cd $UNKNOWN_DIR && rm -rf build-xyz", &ctx());
+    assert!(
+        assessment
+            .findings
+            .iter()
+            .all(|finding| finding.target.as_deref() != Some("/home/u/proj/build-xyz")),
+        "{:?}",
+        assessment.findings
+    );
+}
+
+#[test]
+fn issue_932_redirect_syntax_is_not_a_delete_target() {
+    let bare = assess("rm -rf build-xyz", &ctx());
+    for command in [
+        "rm -rf build-xyz 2>/dev/null",
+        "rm -rf build-xyz 2> /dev/null",
+        "rm -rf build-xyz >/dev/null 2>&1",
+        "rm -rf build-xyz 2>>/dev/null",
+        "rm -rf build-xyz &>/dev/null",
+        "rm -rf build-xyz < /dev/null",
+    ] {
+        let assessment = assess(command, &ctx());
+        assert_eq!(
+            assessment.level, bare.level,
+            "{command:?}: {:?}",
+            assessment.findings
+        );
+        // Neither the descriptor number nor the destination is an operand.
+        assert!(
+            assessment
+                .findings
+                .iter()
+                .all(|finding| finding.target.as_deref() != Some("/home/u/proj/2")),
+            "{command:?}: {:?}",
+            assessment.findings
+        );
+        assert!(
+            assessment.findings.len() <= bare.findings.len(),
+            "{command:?}: {:?}",
+            assessment.findings
+        );
+    }
+
+    // A space between the number and the operator makes it a real operand,
+    // which the shell would pass to the command.
+    assert_eq!(level("rm -rf build-xyz 2 >/dev/null"), RiskLevel::Low);
+
+    // Truncating a protected path through a redirect is still catastrophic.
+    assert_eq!(level("echo pwned > /etc/passwd"), RiskLevel::Catastrophic);
+}

crates/jcode-command-risk/src/lib.rs

@@ -214,8 +214,28 @@ pub fn assess(command: &str, ctx: &RiskContext) -> RiskAssessment {
         });
     }
 
+    // A `cd` earlier in the same command line decides where later relative
+    // paths resolve (#932). Without that, `cd /etc && rm -rf passwd` was judged
+    // as a delete inside the workspace while the shell would remove
+    // `/etc/passwd`. Subshell parentheses are flattened away by the tokenizer,
+    // so a `cd` inside `( ... )` over-approximates the rest of the line; that
+    // direction only ever costs a confirmation.
+    let mut cwd = ctx.working_dir.clone();
     for segment in tokenize::split_segments(command) {
-        assess_segment(&segment, ctx, &mut findings);
+        let segment_ctx = RiskContext {
+            working_dir: cwd.clone(),
+            ..ctx.clone()
+        };
+        match directory_change(&segment, &segment_ctx) {
+            DirectoryChange::None => {}
+            DirectoryChange::Known(dir) => cwd = Some(dir),
+            DirectoryChange::Unknown => cwd = None,
+        }
+        let segment_ctx = RiskContext {
+            working_dir: cwd.clone(),
+            ..ctx.clone()
+        };
+        assess_segment(&segment, &segment_ctx, &mut findings);
     }
 
     // `"$(...)"`, backticks and unquoted heredoc bodies run commands the
@@ -235,6 +255,67 @@ pub fn assess(command: &str, ctx: &RiskContext) -> RiskAssessment {
     RiskAssessment::from_findings(findings)
 }
 
+/// What a segment does to the shell's working directory.
+enum DirectoryChange {
+    /// Not a `cd`.
+    None,
+    /// A `cd` whose destination can be resolved statically.
+    Known(std::path::PathBuf),
+    /// A `cd` whose destination depends on runtime state (`cd -`, `cd $VAR`).
+    Unknown,
+}
+
+/// Track `cd` so later relative operands resolve where the shell will be.
+fn directory_change(tokens: &[Token], ctx: &RiskContext) -> DirectoryChange {
+    let mut rest = tokens;
+    while rest.len() > 1 && SHELL_CONTROL_PREFIXES.contains(&rest[0].text.as_str()) {
+        rest = &rest[1..];
+    }
+    // Leading `VAR=value` assignments prefix a command without changing it.
+    while rest.len() > 1 && rest[0].text.contains('=') && !rest[0].is_flag() {
+        rest = &rest[1..];
+    }
+    if rest.first().map(Token::basename).as_deref() != Some("cd") {
+        return DirectoryChange::None;
+    }
+
+    let mut destination: Option<&str> = None;
+    for token in rest.iter().skip(1) {
+        if token.is_operator || token.is_redirect_target {
+            continue;
+        }
+        // `cd -P /x`, `cd -- /x`: options precede the destination.
+        if destination.is_none() && (token.is_flag() || token.text == "--") {
+            continue;
+        }
+        if destination.is_some() {
+            // `cd a b` is an error in every shell, so the directory did not
+            // change in a way worth guessing at.
+            return DirectoryChange::Unknown;
+        }
+        destination = Some(token.text.as_str());
+    }
+
+    let Some(destination) = destination else {
+        // A bare `cd` goes home.
+        return match &ctx.home_dir {
+            Some(home) => DirectoryChange::Known(home.clone()),
+            None => DirectoryChange::Unknown,
+        };
+    };
+    // `cd -` uses OLDPWD, and substitutions or globs need the shell's own
+    // expansion before the directory is known.
+    if destination == "-" || destination.contains(['$', '`', '*', '?']) {
+        return DirectoryChange::Unknown;
+    }
+    let resolved = paths::expand(destination, ctx);
+    if resolved.is_absolute() {
+        DirectoryChange::Known(resolved)
+    } else {
+        DirectoryChange::Unknown
+    }
+}
+
 fn assess_segment(tokens: &[Token], ctx: &RiskContext, findings: &mut Vec<RiskFinding>) {
     // Shell redirects happen even when a wrapper only prints information.
     for target in tokens.iter().filter(|t| t.is_truncating_redirect_target) {
@@ -389,7 +470,7 @@ fn assess_segment(tokens: &[Token], ctx: &RiskContext, findings: &mut Vec<RiskFi
     let targets: Vec<&Token> = tokens
         .iter()
         .skip(1)
-        .filter(|t| !t.is_flag() && !t.is_operator && !t.is_truncating_redirect_target)
+        .filter(|t| !t.is_flag() && !t.is_operator && !t.is_redirect_target)
         .collect();
 
     // A destructive command fed by a pipe takes its operands from the previous
@@ -449,7 +530,7 @@ fn assess_find(tokens: &[Token], ctx: &RiskContext, findings: &mut Vec<RiskFindi
         .skip(1)
         .skip_while(|t| matches!(t.text.as_str(), "-H" | "-L" | "-P" | "--"))
         .take_while(|t| !t.is_flag() && !t.is_operator && !matches!(t.text.as_str(), "!" | "("))
-        .filter(|t| !t.is_truncating_redirect_target)
+        .filter(|t| !t.is_redirect_target)
         .collect();
     let classify_roots = |findings: &mut Vec<RiskFinding>| {
         for raw in roots

crates/jcode-command-risk/src/tokenize.rs

@@ -14,6 +14,11 @@ pub struct Token {
     pub receives_pipe: bool,
     /// True for `>` / `>|` redirect destinations, which are truncated on open.
     pub is_truncating_redirect_target: bool,
+    /// True for every redirect destination (`>`, `>>`, `<`, `&>`). These are
+    /// never operands of the command, so they are excluded from target
+    /// collection; only the truncating subset is also classified as a write
+    /// (#932: `rm -rf x 2>/dev/null` reported `/dev/null` as a delete target).
+    pub is_redirect_target: bool,
     /// True for control operators like `&&`, which are never path targets.
     pub is_operator: bool,
 }
@@ -24,6 +29,7 @@ impl Token {
             text: text.into(),
             receives_pipe: false,
             is_truncating_redirect_target: false,
+            is_redirect_target: false,
             is_operator: false,
         }
     }
@@ -99,22 +105,40 @@ pub fn tokenize(command: &str) -> Vec<Token> {
     let mut has_content = false;
     let mut chars = command.chars().peekable();
     let mut pending_redirect = false;
+    let mut pending_truncating = false;
 
     macro_rules! flush {
-        () => {
+        () => {{
             if has_content {
                 let mut token = Token::word(std::mem::take(&mut current));
+                if pending_redirect {
+                    token.is_redirect_target = true;
+                    token.is_truncating_redirect_target = pending_truncating;
+                }
                 #[allow(unused_assignments)]
                 {
-                    if pending_redirect {
-                        token.is_truncating_redirect_target = true;
-                        pending_redirect = false;
-                    }
                     has_content = false;
                 }
                 tokens.push(token);
+                #[allow(unused_assignments)]
+                {
+                    pending_redirect = false;
+                    pending_truncating = false;
+                }
+            }
+        }};
+    }
+
+    // A redirect operator with no destination word leaves nothing to mark, and
+    // the flag must not carry over across a command boundary to a later word.
+    macro_rules! clear_redirect {
+        () => {{
+            #[allow(unused_assignments)]
+            {
+                pending_redirect = false;
+                pending_truncating = false;
             }
-        };
+        }};
     }
 
     while let Some(c) = chars.next() {
@@ -154,12 +178,14 @@ pub fn tokenize(command: &str) -> Vec<Token> {
             ' ' | '\t' => flush!(),
             '\n' | ';' => {
                 flush!();
+                clear_redirect!();
                 let mut op = Token::word(c.to_string());
                 op.is_operator = true;
                 tokens.push(op);
             }
             '&' | '|' => {
                 flush!();
+                clear_redirect!();
                 let mut text = c.to_string();
                 if chars.peek() == Some(&c) {
                     text.push(c);
@@ -171,16 +197,19 @@ pub fn tokenize(command: &str) -> Vec<Token> {
             }
             '(' | ')' => {
                 flush!();
+                clear_redirect!();
                 let mut op = Token::word(c.to_string());
                 op.is_operator = true;
                 tokens.push(op);
             }
             '>' => {
+                drop_fd_prefix(&mut current, &mut has_content);
                 flush!();
                 // `>>` appends and does not truncate, so it is far less
                 // destructive; only a single `>` clobbers.
                 if chars.peek() == Some(&'>') {
                     chars.next();
+                    pending_redirect = true;
                 } else if chars.peek() == Some(&'&') {
                     // `>&N` / `>&-` duplicate or close a file descriptor and
                     // never name a file to truncate. Consume the FD word.
@@ -197,15 +226,21 @@ pub fn tokenize(command: &str) -> Vec<Token> {
                     if fd.is_empty() {
                         // `>& file` (bash: redirect stdout+stderr) truncates.
                         pending_redirect = true;
+                        pending_truncating = true;
                     }
                 } else {
                     if chars.peek() == Some(&'|') {
                         chars.next();
                     }
                     pending_redirect = true;
+                    pending_truncating = true;
                 }
             }
-            '<' => flush!(),
+            '<' => {
+                drop_fd_prefix(&mut current, &mut has_content);
+                flush!();
+                pending_redirect = true;
+            }
             _ => {
                 has_content = true;
                 current.push(c);
@@ -217,6 +252,21 @@ pub fn tokenize(command: &str) -> Vec<Token> {
     tokens
 }
 
+/// Drop a bare file-descriptor number that prefixes a redirect (`2>/dev/null`).
+///
+/// The number is redirect syntax, not an operand, but it was emitted as an
+/// ordinary word and then collected as a delete target: `rm -rf x 2>/dev/null`
+/// reported a recursive delete of `.../2` (#932). A descriptor prefix has to be
+/// adjacent to the operator, and a space flushes the word first, so only the
+/// adjacent spelling reaches here. `rm -rf x 2 >/dev/null` really does name a
+/// file called `2` and is left alone.
+fn drop_fd_prefix(current: &mut String, has_content: &mut bool) {
+    if *has_content && current.chars().all(|c| c.is_ascii_digit()) {
+        current.clear();
+        *has_content = false;
+    }
+}
+
 /// Remove heredoc payloads before tokenizing shell source.
 ///
 /// A heredoc body is input data for the command on the declaration line, not

crates/jcode-command-risk/src/tokenize_tests.rs

@@ -179,3 +179,42 @@ fn parentheses_split_subshell_and_command_substitution_bodies() {
         vec![vec!["x=$"], vec!["rm", "-rf", "~"], vec!["echo", "ok"]]
     );
 }
+
+#[test]
+fn redirect_destinations_are_marked_and_fd_prefixes_are_not_words() {
+    // Every destination is a redirect target, so it is never an operand.
+    for (command, destination) in [
+        ("rm -rf x 2>/dev/null", "/dev/null"),
+        ("rm -rf x 2> /dev/null", "/dev/null"),
+        ("rm -rf x >> log.txt", "log.txt"),
+        ("rm -rf x < /dev/null", "/dev/null"),
+        ("rm -rf x &>/dev/null", "/dev/null"),
+    ] {
+        let tokens = tokenize(command);
+        assert!(
+            tokens
+                .iter()
+                .any(|t| t.is_redirect_target && t.text == destination),
+            "{command}: {tokens:?}"
+        );
+        assert!(
+            !tokens.iter().any(|t| t.text == "2"),
+            "{command} keeps the fd prefix as a word: {tokens:?}"
+        );
+    }
+
+    // An input redirect is a destination but never truncates.
+    let input = tokenize("rm -rf x < /dev/null");
+    assert!(
+        !input
+            .iter()
+            .any(|t| t.is_truncating_redirect_target && t.text == "/dev/null")
+    );
+
+    // A space before the operator makes the number a real argument.
+    assert!(
+        tokenize("rm -rf x 2 >/dev/null")
+            .iter()
+            .any(|t| t.text == "2" && !t.is_redirect_target)
+    );
+}