From 936d8b0e692fcf99bc8eff1831faef50791b0946 Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Fri, 13 Mar 2026 22:51:45 +0900 Subject: [PATCH 1/6] rules: add `redirects` and `pipe` variables to `when` clause The `when` clause could only reference env, flags, args, and paths, making it impossible to write rules based on shell redirect operators or pipeline position. This prevented use cases like requiring output redirects for noisy commands or blocking `curl | sh` patterns. Add two new context variables available in CEL `when` expressions: - `redirects`: list of redirect operators (type, operator, target, descriptor) attached to the command - `pipe`: object with stdin/stdout bools indicating pipeline position Extract redirect/pipe metadata from the tree-sitter AST during command parsing via `extract_commands_with_metadata`, thread it through evaluate_compound -> evaluate_command_inner -> build_expr_context, and register as CEL variables. Co-Authored-By: Claude Opus 4.6 --- .../docs/rule-evaluation/when-clause.md | 90 +++- src/rules/command_parser.rs | 385 +++++++++++++++++- src/rules/expr_evaluator.rs | 128 +++++- src/rules/rule_engine.rs | 75 +++- tests/integration/when_clause_rules.rs | 125 +++++- 5 files changed, 761 insertions(+), 42 deletions(-) diff --git a/docs/src/content/docs/rule-evaluation/when-clause.md b/docs/src/content/docs/rule-evaluation/when-clause.md index 1f546f1..2884f34 100644 --- a/docs/src/content/docs/rule-evaluation/when-clause.md +++ b/docs/src/content/docs/rule-evaluation/when-clause.md @@ -23,7 +23,7 @@ CEL expressions must evaluate to a **boolean** (`true` or `false`). If the expre ## Context variables -Four context variables are available inside `when` expressions: +Six context variables are available inside `when` expressions: ### `env` — Environment variables @@ -94,6 +94,58 @@ rules: The `paths` variable is most useful for checking properties of the defined path list itself (e.g., its size), since the `` pattern already handles matching individual files against the list. +### `redirects` — Redirect operators + +A list of redirect operators attached to the command. Each element is an object with the following fields: + +| Field | Type | Description | Example | +| ------------ | --------------- | ------------------------------------ | ------------------------------------------------------------------- | --- | +| `type` | `string` | `"input"`, `"output"`, or `"dup"` | `"output"` | +| `operator` | `string` | The redirect operator | `">"`, `">>"`, `"<"`, `"<<<"`, `">&"`, `"<&"`, `"&>"`, `"&>>"`, `"> | "` | +| `target` | `string` | The redirect destination | `"/tmp/log.txt"`, `"/dev/null"` | +| `descriptor` | `int` or `null` | File descriptor number, if specified | `2` (for `2>`) | + +Type classification: + +- `"output"`: `>`, `>>`, `>|`, `&>`, `&>>` +- `"input"`: `<`, `<<<`, `<<`, `<<-` +- `"dup"`: `>&`, `<&` + +```yaml +# Require output redirect for renovate-dryrun +- deny: 'renovate-dryrun' + when: '!redirects.exists(r, r.type == "output")' + message: 'Please redirect output to a log file' + fix_suggestion: 'renovate-dryrun > /tmp/renovate-dryrun.log 2>&1' + +# Only allow output redirect to /tmp/ +- allow: 'renovate-dryrun' + when: 'redirects.exists(r, r.type == "output" && r.target.startsWith("/tmp/"))' +``` + +:::note +The `redirects` variable is only populated when the command is evaluated through `evaluate_compound` (e.g., compound commands with pipes, `&&`, or redirects). For simple commands without redirects or pipes, the list is empty. +::: + +### `pipe` — Pipeline position + +An object indicating whether the command receives piped input or sends piped output: + +| Field | Type | Description | +| -------- | ------ | ---------------------------------------------------------- | +| `stdin` | `bool` | `true` if the command receives input from a preceding pipe | +| `stdout` | `bool` | `true` if the command's output feeds into a following pipe | + +Both fields are `false` when the command is not part of a pipeline. + +```yaml +# Block piped execution of sh/bash (e.g., curl | sh) +- deny: 'sh' + when: 'pipe.stdin' +- deny: 'bash' + when: 'pipe.stdin' +``` + ## Operators CEL supports standard operators for building conditions: @@ -125,10 +177,12 @@ CEL supports standard operators for building conditions: ### Collection -| Expression | Description | -| --------------- | ------------------------------- | -| `value in list` | Check if value exists in a list | -| `size(list)` | Get the length of a list or map | +| Expression | Description | +| -------------------- | ------------------------------------------------ | +| `value in list` | Check if value exists in a list | +| `size(list)` | Get the length of a list or map | +| `x.exists(e, p)` | Check if any element satisfies predicate | +| `x.exists_one(e, p)` | Check if exactly one element satisfies predicate | ## Evaluation order @@ -183,6 +237,32 @@ rules: when: "flags.request == 'POST' && args[0].startsWith('https://prod.')" ``` +### Redirect-based gating + +```yaml +rules: + # Require output redirect for commands that produce large output + - deny: 'renovate-dryrun' + when: '!redirects.exists(r, r.type == "output")' + message: 'Please redirect output to a log file' + fix_suggestion: 'renovate-dryrun > /tmp/renovate-dryrun.log 2>&1' + + # Allow with output redirect to /tmp/ + - allow: 'renovate-dryrun' + when: 'redirects.exists(r, r.type == "output" && r.target.startsWith("/tmp/"))' +``` + +### Pipe safety + +```yaml +rules: + # Block curl-pipe-sh attacks + - deny: 'sh' + when: 'pipe.stdin' + - deny: 'bash' + when: 'pipe.stdin' +``` + ## Related - [Configuration Schema: `when`](/configuration/schema/#when) -- Configuration reference for the `when` field. diff --git a/src/rules/command_parser.rs b/src/rules/command_parser.rs index 82952b3..8f08c9e 100644 --- a/src/rules/command_parser.rs +++ b/src/rules/command_parser.rs @@ -32,6 +32,39 @@ pub struct ParsedCommand { pub raw_tokens: Vec, } +/// Information about a single redirect operator attached to a command. +#[derive(Debug, Clone, PartialEq)] +pub struct RedirectInfo { + /// Redirect category: "input", "output", or "dup". + pub redirect_type: String, + /// The redirect operator (e.g., ">", ">>", "<", "<<<", ">&", "<&", "&>", "&>>", ">|"). + pub operator: String, + /// The redirect target (e.g., "/dev/null", "&1", "file.txt"). + pub target: String, + /// File descriptor number, if explicitly specified (e.g., `2` in `2>`). + pub descriptor: Option, +} + +/// Information about a command's position in a pipeline. +#[derive(Debug, Clone, PartialEq, Default)] +pub struct PipeInfo { + /// Whether stdin comes from a preceding pipe. + pub stdin: bool, + /// Whether stdout feeds into a following pipe. + pub stdout: bool, +} + +/// A command extracted from a compound shell expression, with metadata. +#[derive(Debug, Clone, PartialEq)] +pub struct ExtractedCommand { + /// The command string (redirects stripped, as before). + pub command: String, + /// Redirect operators that were attached to this command. + pub redirects: Vec, + /// Pipeline position information. + pub pipe: PipeInfo, +} + /// Parse a command string into a structured `ParsedCommand`. /// /// Uses `FlagSchema` to determine whether a flag consumes the next token @@ -191,6 +224,19 @@ pub fn tokenize(input: &str) -> Result, CommandParseError> { /// Uses tree-sitter-bash to correctly handle quoting and nesting. /// Returns `SyntaxError` if the input contains parse errors. pub fn extract_commands(input: &str) -> Result, CommandParseError> { + Ok(extract_commands_with_metadata(input)? + .into_iter() + .map(|ec| ec.command) + .collect()) +} + +/// Extract individual commands with redirect and pipe metadata. +/// +/// Like `extract_commands`, but each command includes information about +/// attached redirects and pipeline position. +pub fn extract_commands_with_metadata( + input: &str, +) -> Result, CommandParseError> { let trimmed = input.trim(); if trimmed.is_empty() { return Err(CommandParseError::EmptyCommand); @@ -212,7 +258,13 @@ pub fn extract_commands(input: &str) -> Result, CommandParseError> { } let mut commands = Vec::new(); - collect_commands(root, trimmed.as_bytes(), &mut commands); + collect_commands( + root, + trimmed.as_bytes(), + &mut commands, + &PipeInfo::default(), + &[], + ); Ok(commands) } @@ -222,13 +274,21 @@ pub fn extract_commands(input: &str) -> Result, CommandParseError> { /// Compound constructs (pipeline, list, subshell, control structures) are split /// into their constituent commands. Conditions and value lists are also recursed /// into so that commands within them (including command substitutions) are extracted. -fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec) { +/// +/// `pipe_info` carries the current pipeline position context. +/// `redirects` carries redirect info inherited from a parent `redirected_statement`. +fn collect_commands( + node: tree_sitter::Node, + source: &[u8], + commands: &mut Vec, + pipe_info: &PipeInfo, + redirects: &[RedirectInfo], +) { match node.kind() { - // Transparent containers: recurse into all named children. - // Skips anonymous tokens like `;`, `&&`, `||`, `|`, `(`, `)`, + // Transparent containers (except pipeline): recurse into all named children. + // Skips anonymous tokens like `;`, `&&`, `||`, `(`, `)`, // `do`, `done`, `then`, `fi`, `esac`, keywords, etc. "program" - | "pipeline" | "list" | "subshell" | "do_group" @@ -241,7 +301,22 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec { let mut cursor = node.walk(); for child in node.named_children(&mut cursor) { - collect_commands(child, source, commands); + collect_commands(child, source, commands, pipe_info, redirects); + } + } + // pipeline: compute pipe position for each child command. + "pipeline" => { + let children: Vec<_> = { + let mut cursor = node.walk(); + node.named_children(&mut cursor).collect() + }; + let len = children.len(); + for (i, child) in children.iter().enumerate() { + let child_pipe = PipeInfo { + stdin: i > 0, + stdout: i < len - 1, + }; + collect_commands(*child, source, commands, &child_pipe, redirects); } } // for_statement: recurse into body (do_group) and any command_substitution @@ -251,7 +326,7 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec { - collect_commands(child, source, commands); + collect_commands(child, source, commands, pipe_info, redirects); } // Recurse into value list items (e.g. string nodes) // to find nested command substitutions like @@ -271,10 +346,10 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec { - collect_commands(child, source, commands); + collect_commands(child, source, commands, pipe_info, redirects); } "command_substitution" => { - collect_commands(child, source, commands); + collect_commands(child, source, commands, pipe_info, redirects); } _ => { collect_substitutions_recursive(child, source, commands); @@ -295,19 +370,30 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec, >>, <, 2>&1, etc.). + // redirected_statement: extract redirect info, then recurse into the body. // Redirect target paths are left to the OS-level sandbox to enforce. // Also recurse into redirect children to extract nested commands // (e.g. process substitutions: `cmd > >(nested_cmd)`). "redirected_statement" => { + let mut all_redirects = redirects.to_vec(); + // First pass: extract redirect metadata only + for i in 0..node.child_count() { + if node.field_name_for_child(i as u32) == Some("redirect") + && let Some(child) = node.child(i as u32) + && let Some(info) = extract_redirect_info(child, source) + { + all_redirects.push(info); + } + } + // Process body first (preserves original command ordering) if let Some(body) = node.child_by_field_name("body") { - collect_commands(body, source, commands); + collect_commands(body, source, commands, pipe_info, &all_redirects); } + // Second pass: recurse into redirect children for nested substitutions for i in 0..node.child_count() { if node.field_name_for_child(i as u32) == Some("redirect") && let Some(child) = node.child(i as u32) @@ -328,7 +414,7 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec { if let Some(body) = node.child_by_field_name("body") { - collect_commands(body, source, commands); + collect_commands(body, source, commands, pipe_info, redirects); } } // command node: strip leading variable_assignment children @@ -337,6 +423,7 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec { + let mut cmd_redirects = redirects.to_vec(); for i in 0..node.child_count() { let Some(child) = node.child(i as u32) else { continue; @@ -345,13 +432,17 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec { - collect_commands(child, source, commands); + collect_commands(child, source, commands, &PipeInfo::default(), &[]); } "variable_assignment" => { collect_substitutions_recursive(child, source, commands); @@ -387,7 +478,11 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec &'static str { + match operator { + ">" | ">>" | ">|" | "&>" | "&>>" => "output", + "<" | "<<<" | "<<" | "<<-" => "input", + ">&" | "<&" => "dup", + _ => "output", + } +} + +/// Extract redirect information from a tree-sitter redirect node. +/// +/// Handles `file_redirect`, `heredoc_redirect`, and `herestring_redirect` nodes. +fn extract_redirect_info(node: tree_sitter::Node, source: &[u8]) -> Option { + match node.kind() { + "file_redirect" => { + // Extract the operator from anonymous children + let mut operator = String::new(); + let mut descriptor: Option = None; + let mut target = String::new(); + + for i in 0..node.child_count() { + let child = node.child(i as u32)?; + if child.kind() == "file_descriptor" { + let text = + std::str::from_utf8(&source[child.start_byte()..child.end_byte()]).ok()?; + descriptor = text.parse::().ok(); + } else if !child.is_named() { + // Anonymous node = operator token (>, >>, <, >&, <&, &>, &>>, >|) + let text = + std::str::from_utf8(&source[child.start_byte()..child.end_byte()]).ok()?; + operator = text.to_string(); + } else if node.field_name_for_child(i as u32) == Some("destination") { + let text = + std::str::from_utf8(&source[child.start_byte()..child.end_byte()]).ok()?; + target = text.to_string(); + } + } + + if operator.is_empty() { + return None; + } + + Some(RedirectInfo { + redirect_type: classify_redirect(&operator).to_string(), + operator, + target, + descriptor, + }) + } + "herestring_redirect" => { + // <<< 'content' + let mut target = String::new(); + for i in 0..node.child_count() { + let child = node.child(i as u32)?; + if child.is_named() { + let text = + std::str::from_utf8(&source[child.start_byte()..child.end_byte()]).ok()?; + target = text.to_string(); + break; + } + } + Some(RedirectInfo { + redirect_type: "input".to_string(), + operator: "<<<".to_string(), + target, + descriptor: None, + }) + } + "heredoc_redirect" => { + // << or <<- + let mut operator = "<<".to_string(); + for i in 0..node.child_count() { + let child = node.child(i as u32)?; + if !child.is_named() { + let text = + std::str::from_utf8(&source[child.start_byte()..child.end_byte()]).ok()?; + if text == "<<-" || text == "<<" { + operator = text.to_string(); + } + } } + Some(RedirectInfo { + redirect_type: "input".to_string(), + operator, + target: String::new(), + descriptor: None, + }) } + _ => None, } } @@ -410,13 +602,13 @@ fn collect_commands(node: tree_sitter::Node, source: &[u8], commands: &mut Vec, + commands: &mut Vec, ) { let mut cursor = node.walk(); for child in node.named_children(&mut cursor) { match child.kind() { "command_substitution" | "process_substitution" => { - collect_commands(child, source, commands); + collect_commands(child, source, commands, &PipeInfo::default(), &[]); } _ => { collect_substitutions_recursive(child, source, commands); @@ -1062,4 +1254,159 @@ mod tests { let owned: Vec = tokens.iter().map(|s| s.to_string()).collect(); assert_eq!(shell_quote_join(&owned).unwrap(), expected); } + + // ======================================== + // extract_commands_with_metadata: redirects + // ======================================== + + #[rstest] + #[case::output_redirect( + "echo hello > /tmp/log.txt", + vec![RedirectInfo { + redirect_type: "output".to_string(), + operator: ">".to_string(), + target: "/tmp/log.txt".to_string(), + descriptor: None, + }], + PipeInfo { stdin: false, stdout: false }, + )] + #[case::append_redirect( + "echo hello >> /tmp/log.txt", + vec![RedirectInfo { + redirect_type: "output".to_string(), + operator: ">>".to_string(), + target: "/tmp/log.txt".to_string(), + descriptor: None, + }], + PipeInfo { stdin: false, stdout: false }, + )] + #[case::dup_redirect_2_to_1( + "echo hello 2>&1", + vec![RedirectInfo { + redirect_type: "dup".to_string(), + operator: ">&".to_string(), + target: "1".to_string(), + descriptor: Some(2), + }], + PipeInfo { stdin: false, stdout: false }, + )] + #[case::input_redirect( + "cat < input.txt", + vec![RedirectInfo { + redirect_type: "input".to_string(), + operator: "<".to_string(), + target: "input.txt".to_string(), + descriptor: None, + }], + PipeInfo { stdin: false, stdout: false }, + )] + #[case::herestring_redirect( + "cat <<< 'hello'", + vec![RedirectInfo { + redirect_type: "input".to_string(), + operator: "<<<".to_string(), + target: "'hello'".to_string(), + descriptor: None, + }], + PipeInfo { stdin: false, stdout: false }, + )] + #[case::ampersand_redirect( + "echo hello &>/dev/null", + vec![RedirectInfo { + redirect_type: "output".to_string(), + operator: "&>".to_string(), + target: "/dev/null".to_string(), + descriptor: None, + }], + PipeInfo { stdin: false, stdout: false }, + )] + #[case::clobber_redirect( + "echo hello >| /tmp/log.txt", + vec![RedirectInfo { + redirect_type: "output".to_string(), + operator: ">|".to_string(), + target: "/tmp/log.txt".to_string(), + descriptor: None, + }], + PipeInfo { stdin: false, stdout: false }, + )] + fn extract_commands_metadata_single( + #[case] input: &str, + #[case] expected_redirects: Vec, + #[case] expected_pipe: PipeInfo, + ) { + let commands = extract_commands_with_metadata(input).unwrap(); + assert_eq!(commands.len(), 1, "expected 1 command for input: {}", input); + assert_eq!(commands[0].redirects, expected_redirects); + assert_eq!(commands[0].pipe, expected_pipe); + } + + // ======================================== + // extract_commands_with_metadata: simple command (no pipe/redirect) + // ======================================== + + #[test] + fn extract_commands_metadata_simple_command() { + let commands = extract_commands_with_metadata("echo hello").unwrap(); + assert_eq!(commands.len(), 1); + assert_eq!( + commands[0].pipe, + PipeInfo { + stdin: false, + stdout: false + } + ); + assert!(commands[0].redirects.is_empty()); + } + + // ======================================== + // extract_commands_with_metadata: pipelines + // ======================================== + + #[test] + fn extract_commands_metadata_two_stage_pipeline() { + let commands = extract_commands_with_metadata("echo hello | grep foo").unwrap(); + assert_eq!(commands.len(), 2); + assert_eq!( + commands[0].pipe, + PipeInfo { + stdin: false, + stdout: true + } + ); + assert_eq!( + commands[1].pipe, + PipeInfo { + stdin: true, + stdout: false + } + ); + } + + #[test] + fn extract_commands_metadata_three_stage_pipeline() { + let commands = extract_commands_with_metadata("echo hello | grep foo | wc -l").unwrap(); + assert_eq!(commands.len(), 3); + assert_eq!( + commands[0].pipe, + PipeInfo { + stdin: false, + stdout: true + } + ); + assert_eq!( + commands[1].pipe, + PipeInfo { + stdin: true, + stdout: true + } + ); + assert_eq!( + commands[2].pipe, + PipeInfo { + stdin: true, + stdout: false + } + ); + } } diff --git a/src/rules/expr_evaluator.rs b/src/rules/expr_evaluator.rs index 216822b..dfd3a40 100644 --- a/src/rules/expr_evaluator.rs +++ b/src/rules/expr_evaluator.rs @@ -1,14 +1,18 @@ use std::collections::HashMap; use super::ExprError; +use super::command_parser::{PipeInfo, RedirectInfo}; /// Context for CEL expression evaluation, providing access to -/// environment variables, parsed flags, positional arguments, and path lists. +/// environment variables, parsed flags, positional arguments, path lists, +/// redirect operators, and pipeline position. pub struct ExprContext { pub env: HashMap, pub flags: HashMap>, pub args: Vec, pub paths: HashMap>, + pub redirects: Vec, + pub pipe: PipeInfo, } /// Evaluates a CEL expression against a given context, returning a boolean result. @@ -41,6 +45,49 @@ pub fn evaluate(expr: &str, context: &ExprContext) -> Result { .add_variable("paths", &context.paths) .map_err(|e| ExprError::Eval(e.to_string()))?; + // Register redirects: list of maps with type, operator, target, descriptor + let redirects_value: Vec> = context + .redirects + .iter() + .map(|r| { + let mut m = HashMap::new(); + m.insert( + "type".to_string(), + cel_interpreter::Value::String(r.redirect_type.clone().into()), + ); + m.insert( + "operator".to_string(), + cel_interpreter::Value::String(r.operator.clone().into()), + ); + m.insert( + "target".to_string(), + cel_interpreter::Value::String(r.target.clone().into()), + ); + m.insert( + "descriptor".to_string(), + match r.descriptor { + Some(fd) => cel_interpreter::Value::Int(fd), + None => cel_interpreter::Value::Null, + }, + ); + m + }) + .collect(); + cel_context.add_variable_from_value("redirects", redirects_value); + + // Register pipe: map with stdin and stdout booleans + let pipe_value: HashMap = HashMap::from([ + ( + "stdin".to_string(), + cel_interpreter::Value::Bool(context.pipe.stdin), + ), + ( + "stdout".to_string(), + cel_interpreter::Value::Bool(context.pipe.stdout), + ), + ]); + cel_context.add_variable_from_value("pipe", pipe_value); + let result = program .execute(&cel_context) .map_err(|e| ExprError::Eval(e.to_string()))?; @@ -62,6 +109,8 @@ mod tests { flags: HashMap::new(), args: Vec::new(), paths: HashMap::new(), + redirects: Vec::new(), + pipe: PipeInfo::default(), } } @@ -203,7 +252,7 @@ mod tests { env: HashMap::from([("AWS_PROFILE".to_string(), "prod".to_string())]), flags: HashMap::from([("method".to_string(), Some("POST".to_string()))]), args: vec!["https://prod.example.com/api".to_string()], - paths: HashMap::new(), + ..empty_context() }; assert!( evaluate( @@ -256,4 +305,79 @@ mod tests { other => panic!("expected ExprError::TypeError, got {:?}", other), } } + + // === Redirect variable access === + + #[rstest] + #[case::exists_output_redirect_present( + "redirects.exists(r, r.type == \"output\")", + vec![RedirectInfo { + redirect_type: "output".to_string(), + operator: ">".to_string(), + target: "/tmp/log.txt".to_string(), + descriptor: None, + }], + true + )] + #[case::exists_output_redirect_absent( + "redirects.exists(r, r.type == \"output\")", + vec![], + false + )] + #[case::size_redirects_with_entry( + "size(redirects) > 0", + vec![RedirectInfo { + redirect_type: "output".to_string(), + operator: ">".to_string(), + target: "/tmp/log.txt".to_string(), + descriptor: None, + }], + true + )] + #[case::redirect_target_starts_with( + "redirects[0].target.startsWith(\"/tmp/\")", + vec![RedirectInfo { + redirect_type: "output".to_string(), + operator: ">".to_string(), + target: "/tmp/log.txt".to_string(), + descriptor: None, + }], + true + )] + #[case::redirect_descriptor_equals_2( + "redirects[0].descriptor == 2", + vec![RedirectInfo { + redirect_type: "dup".to_string(), + operator: ">&".to_string(), + target: "&1".to_string(), + descriptor: Some(2), + }], + true + )] + fn redirects_variable_access( + #[case] expr: &str, + #[case] redirects: Vec, + #[case] expected: bool, + ) { + let context = ExprContext { + redirects, + ..empty_context() + }; + assert_eq!(evaluate(expr, &context).unwrap(), expected); + } + + // === Pipe variable access === + + #[rstest] + #[case::pipe_stdin_true("pipe.stdin == true", PipeInfo { stdin: true, stdout: false }, true)] + #[case::pipe_stdin_false("pipe.stdin == true", PipeInfo { stdin: false, stdout: false }, false)] + #[case::pipe_stdout_true("pipe.stdout == true", PipeInfo { stdin: false, stdout: true }, true)] + #[case::pipe_stdout_false("pipe.stdout == true", PipeInfo { stdin: false, stdout: false }, false)] + fn pipe_variable_access(#[case] expr: &str, #[case] pipe: PipeInfo, #[case] expected: bool) { + let context = ExprContext { + pipe, + ..empty_context() + }; + assert_eq!(evaluate(expr, &context).unwrap(), expected); + } } diff --git a/src/rules/rule_engine.rs b/src/rules/rule_engine.rs index 3129607..59ad55c 100644 --- a/src/rules/rule_engine.rs +++ b/src/rules/rule_engine.rs @@ -6,7 +6,8 @@ use crate::config::{ }; use crate::rules::RuleError; use crate::rules::command_parser::{ - FlagSchema, ParsedCommand, extract_commands, parse_command, shell_quote_join, + ExtractedCommand, FlagSchema, ParsedCommand, PipeInfo, RedirectInfo, extract_commands, + extract_commands_with_metadata, parse_command, shell_quote_join, }; use crate::rules::expr_evaluator::{ExprContext, evaluate}; use crate::rules::pattern_matcher::{extract_placeholder, matches_with_captures}; @@ -99,7 +100,7 @@ pub fn evaluate_command( command: &str, context: &EvalContext, ) -> Result { - evaluate_command_inner(config, command, context, 0) + evaluate_command_inner(config, command, context, 0, &[], &PipeInfo::default()) } /// Evaluate a potentially compound command (containing `|`, `&&`, `||`, `;`) @@ -118,7 +119,13 @@ pub fn evaluate_compound( command: &str, context: &EvalContext, ) -> Result { - let commands = extract_commands(command).unwrap_or_else(|_| vec![command.to_string()]); + let extracted = extract_commands_with_metadata(command).unwrap_or_else(|_| { + vec![ExtractedCommand { + command: command.to_string(), + redirects: vec![], + pipe: PipeInfo::default(), + }] + }); let default_definitions = Definitions::default(); let definitions = config.definitions.as_ref().unwrap_or(&default_definitions); @@ -129,8 +136,15 @@ pub fn evaluate_compound( let mut sub_results: Vec = Vec::new(); let mut sub_command_details: Vec = Vec::new(); - for cmd in &commands { - let result = evaluate_command(config, cmd, context)?; + for ext_cmd in &extracted { + let result = evaluate_command_inner( + config, + &ext_cmd.command, + context, + 0, + &ext_cmd.redirects, + &ext_cmd.pipe, + )?; // Collect sandbox preset names if let Some(ref name) = result.sandbox_preset { @@ -138,7 +152,7 @@ pub fn evaluate_compound( } sub_command_details.push(SubCommandDetail { - command: cmd.clone(), + command: ext_cmd.command.clone(), action: result.action.clone(), matched_rules: result.matched_rules.clone(), }); @@ -249,6 +263,8 @@ fn evaluate_command_inner( command: &str, context: &EvalContext, depth: usize, + redirects: &[RedirectInfo], + pipe: &PipeInfo, ) -> Result { if depth > MAX_WRAPPER_DEPTH { return Err(RuleError::RecursionDepthExceeded(MAX_WRAPPER_DEPTH)); @@ -270,13 +286,13 @@ fn evaluate_command_inner( // command falls through to simple-command rule evaluation below. Both // results are merged so that rules matching the outer command (e.g. // `rm *` for `rm $(echo hello)`) are not skipped. - if let Ok(sub_commands) = extract_commands(command) + if let Ok(sub_commands) = extract_commands_with_metadata(command) && sub_commands.len() > 1 { let normalized_command = normalize_whitespace(command); - let nested_subs: Vec<&String> = sub_commands + let nested_subs: Vec<&ExtractedCommand> = sub_commands .iter() - .filter(|sub| normalize_whitespace(sub) != normalized_command) + .filter(|sub| normalize_whitespace(&sub.command) != normalized_command) .collect(); let had_self_reference = nested_subs.len() < sub_commands.len(); @@ -284,7 +300,14 @@ fn evaluate_command_inner( // Pure compound (no self-reference): evaluate all sub-commands and return. let mut merged: Option = None; for sub in &nested_subs { - let result = evaluate_command_inner(config, sub, context, depth + 1)?; + let result = evaluate_command_inner( + config, + &sub.command, + context, + depth + 1, + &sub.redirects, + &sub.pipe, + )?; merged = Some(match merged { Some(prev) => merge_results(prev, result), None => result, @@ -302,7 +325,14 @@ fn evaluate_command_inner( if !nested_subs.is_empty() { let mut nested_merged: Option = None; for sub in &nested_subs { - let result = evaluate_command_inner(config, sub, context, depth + 1)?; + let result = evaluate_command_inner( + config, + &sub.command, + context, + depth + 1, + &sub.redirects, + &sub.pipe, + )?; nested_merged = Some(match nested_merged { Some(prev) => merge_results(prev, result), None => result, @@ -315,7 +345,8 @@ fn evaluate_command_inner( if let Some(nested_result) = nested_merged { // Evaluate the original command as a simple command (skip the // compound guard by calling the remaining logic directly). - let simple_result = evaluate_simple_command(config, command, context, depth)?; + let simple_result = + evaluate_simple_command(config, command, context, depth, redirects, pipe)?; return Ok(merge_results(nested_result, simple_result)); } } @@ -323,7 +354,7 @@ fn evaluate_command_inner( // simple-command evaluation. } - evaluate_simple_command(config, command, context, depth) + evaluate_simple_command(config, command, context, depth, redirects, pipe) } /// Evaluate a single (non-compound) command against rules and wrappers. @@ -338,6 +369,8 @@ fn evaluate_simple_command( command: &str, context: &EvalContext, depth: usize, + redirects: &[RedirectInfo], + pipe: &PipeInfo, ) -> Result { let rules = match &config.rules { Some(rules) => rules, @@ -378,7 +411,8 @@ fn evaluate_simple_command( // Evaluate when clause if present if let Some(when_expr) = &rule.when { - let expr_context = build_expr_context(&parsed_command, context, definitions); + let expr_context = + build_expr_context(&parsed_command, context, definitions, redirects, pipe); match evaluate(when_expr, &expr_context) { Ok(true) => {} Ok(false) => continue, @@ -513,7 +547,14 @@ fn try_unwrap_wrapper( let mut result: Option = None; for cmd in &sub_commands { - let sub_result = evaluate_command_inner(config, cmd, context, depth + 1)?; + let sub_result = evaluate_command_inner( + config, + cmd, + context, + depth + 1, + &[], + &PipeInfo::default(), + )?; result = Some(match result { Some(prev) => merge_results(prev, sub_result), None => sub_result, @@ -641,6 +682,8 @@ fn build_expr_context( parsed_command: &ParsedCommand, eval_context: &EvalContext, definitions: &Definitions, + redirects: &[RedirectInfo], + pipe: &PipeInfo, ) -> ExprContext { let flags: HashMap> = parsed_command .flags @@ -659,6 +702,8 @@ fn build_expr_context( flags, args: parsed_command.args.clone(), paths, + redirects: redirects.to_vec(), + pipe: pipe.clone(), } } diff --git a/tests/integration/when_clause_rules.rs b/tests/integration/when_clause_rules.rs index 35d947e..dae2866 100644 --- a/tests/integration/when_clause_rules.rs +++ b/tests/integration/when_clause_rules.rs @@ -6,7 +6,7 @@ use std::path::PathBuf; use indoc::indoc; use rstest::rstest; use runok::config::parse_config; -use runok::rules::rule_engine::{Action, EvalContext, evaluate_command}; +use runok::rules::rule_engine::{Action, EvalContext, evaluate_command, evaluate_compound}; // ======================================== // Environment variable conditions @@ -470,3 +470,126 @@ fn when_clause_with_path_ref( let result = evaluate_command(&config, command, &context).unwrap(); expected(&result.action); } + +// ======================================== +// Redirect conditions in when clause +// ======================================== + +#[rstest] +#[case::output_redirect_denied("renovate-dryrun > /tmp/log.txt", assert_deny as ActionAssertion)] +#[case::no_redirect_default("renovate-dryrun", assert_ask as ActionAssertion)] +fn redirect_output_exists( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {r#" + rules: + - deny: 'renovate-dryrun' + when: 'redirects.exists(r, r.type == "output")' + "#}) + .unwrap(); + + let result = evaluate_compound(&config, command, &empty_context).unwrap(); + expected(&result.action); +} + +// ======================================== +// Redirect target check in when clause +// ======================================== + +#[rstest] +#[case::redirect_to_tmp_allowed("renovate-dryrun > /tmp/log.txt", assert_allow as ActionAssertion)] +#[case::redirect_to_var_default("renovate-dryrun > /var/log.txt", assert_ask as ActionAssertion)] +fn redirect_target_starts_with( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {r#" + rules: + - allow: 'renovate-dryrun' + when: 'redirects.exists(r, r.type == "output" && r.target.startsWith("/tmp/"))' + "#}) + .unwrap(); + + let result = evaluate_compound(&config, command, &empty_context).unwrap(); + expected(&result.action); +} + +// ======================================== +// Pipe stdin detection (curl | sh prevention) +// ======================================== + +#[rstest] +#[case::curl_pipe_sh_denied("curl http://example.com | sh", assert_deny as ActionAssertion)] +#[case::sh_standalone_allowed("sh", assert_allow as ActionAssertion)] +#[case::echo_pipe_bash_denied("echo hello | bash", assert_deny as ActionAssertion)] +fn pipe_stdin_prevents_shell_execution( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {" + rules: + - deny: 'sh' + when: 'pipe.stdin' + - deny: 'bash' + when: 'pipe.stdin' + - allow: 'curl *' + - allow: 'echo *' + - allow: 'sh' + - allow: 'bash' + "}) + .unwrap(); + + let result = evaluate_compound(&config, command, &empty_context).unwrap(); + expected(&result.action); +} + +// ======================================== +// Redirect descriptor check in when clause +// ======================================== + +#[rstest] +#[case::stderr_redirect_denied("cmd 2>/dev/null", assert_deny as ActionAssertion)] +#[case::stdout_redirect_default("cmd > /dev/null", assert_ask as ActionAssertion)] +fn redirect_descriptor_check( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {" + rules: + - deny: 'cmd' + when: 'redirects.exists(r, r.descriptor == 2)' + "}) + .unwrap(); + + let result = evaluate_compound(&config, command, &empty_context).unwrap(); + expected(&result.action); +} + +// ======================================== +// Combined redirect count check in when clause +// ======================================== + +#[rstest] +#[case::with_redirect_allowed("cmd > /dev/null", assert_allow as ActionAssertion)] +#[case::no_redirect_denied("cmd", assert_deny as ActionAssertion)] +fn redirect_count_check( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {" + rules: + - deny: 'cmd' + when: 'size(redirects) == 0' + - allow: 'cmd' + "}) + .unwrap(); + + let result = evaluate_compound(&config, command, &empty_context).unwrap(); + expected(&result.action); +} From 6a3a0f41c2dfb37a0b5b8036bf346bc3941e0318 Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Sat, 14 Mar 2026 01:30:01 +0900 Subject: [PATCH 2/6] rules, docs: address review feedback The `>|` operator in the docs markdown table contained an unescaped pipe character, breaking the table rendering. Pipeline unit tests were separate functions instead of parameterized cases, violating the repository's rstest style guide. Escape `|` in the operator table and consolidate pipeline tests into a single `#[rstest]` parameterized test. Co-Authored-By: Claude Opus 4.6 --- .../docs/rule-evaluation/when-clause.md | 12 +-- src/rules/command_parser.rs | 98 +++++++------------ 2 files changed, 42 insertions(+), 68 deletions(-) diff --git a/docs/src/content/docs/rule-evaluation/when-clause.md b/docs/src/content/docs/rule-evaluation/when-clause.md index 2884f34..3b70571 100644 --- a/docs/src/content/docs/rule-evaluation/when-clause.md +++ b/docs/src/content/docs/rule-evaluation/when-clause.md @@ -98,12 +98,12 @@ The `paths` variable is most useful for checking properties of the defined path A list of redirect operators attached to the command. Each element is an object with the following fields: -| Field | Type | Description | Example | -| ------------ | --------------- | ------------------------------------ | ------------------------------------------------------------------- | --- | -| `type` | `string` | `"input"`, `"output"`, or `"dup"` | `"output"` | -| `operator` | `string` | The redirect operator | `">"`, `">>"`, `"<"`, `"<<<"`, `">&"`, `"<&"`, `"&>"`, `"&>>"`, `"> | "` | -| `target` | `string` | The redirect destination | `"/tmp/log.txt"`, `"/dev/null"` | -| `descriptor` | `int` or `null` | File descriptor number, if specified | `2` (for `2>`) | +| Field | Type | Description | Example | +| ------------ | --------------- | ------------------------------------ | ----------------------------------------------------------------------- | +| `type` | `string` | `"input"`, `"output"`, or `"dup"` | `"output"` | +| `operator` | `string` | The redirect operator | `">"`, `">>"`, `"<"`, `"<<<"`, `">&"`, `"<&"`, `"&>"`, `"&>>"`, `">\|"` | +| `target` | `string` | The redirect destination | `"/tmp/log.txt"`, `"/dev/null"` | +| `descriptor` | `int` or `null` | File descriptor number, if specified | `2` (for `2>`) | Type classification: diff --git a/src/rules/command_parser.rs b/src/rules/command_parser.rs index 8f08c9e..df20d76 100644 --- a/src/rules/command_parser.rs +++ b/src/rules/command_parser.rs @@ -1342,71 +1342,45 @@ mod tests { } // ======================================== - // extract_commands_with_metadata: simple command (no pipe/redirect) + // extract_commands_with_metadata: pipeline position // ======================================== - #[test] - fn extract_commands_metadata_simple_command() { - let commands = extract_commands_with_metadata("echo hello").unwrap(); - assert_eq!(commands.len(), 1); - assert_eq!( - commands[0].pipe, - PipeInfo { - stdin: false, - stdout: false - } - ); - assert!(commands[0].redirects.is_empty()); - } - - // ======================================== - // extract_commands_with_metadata: pipelines - // ======================================== - - #[test] - fn extract_commands_metadata_two_stage_pipeline() { - let commands = extract_commands_with_metadata("echo hello | grep foo").unwrap(); - assert_eq!(commands.len(), 2); - assert_eq!( - commands[0].pipe, - PipeInfo { - stdin: false, - stdout: true - } - ); - assert_eq!( - commands[1].pipe, - PipeInfo { - stdin: true, - stdout: false - } - ); - } - - #[test] - fn extract_commands_metadata_three_stage_pipeline() { - let commands = extract_commands_with_metadata("echo hello | grep foo | wc -l").unwrap(); - assert_eq!(commands.len(), 3); - assert_eq!( - commands[0].pipe, - PipeInfo { - stdin: false, - stdout: true - } - ); - assert_eq!( - commands[1].pipe, - PipeInfo { - stdin: true, - stdout: true - } - ); + #[rstest] + #[case::simple_command("echo hello", vec![ + PipeInfo { stdin: false, stdout: false }, + ])] + #[case::two_stage_pipeline("echo hello | grep foo", vec![ + PipeInfo { stdin: false, stdout: true }, + PipeInfo { stdin: true, stdout: false }, + ])] + #[case::three_stage_pipeline("echo hello | grep foo | wc -l", vec![ + PipeInfo { stdin: false, stdout: true }, + PipeInfo { stdin: true, stdout: true }, + PipeInfo { stdin: true, stdout: false }, + ])] + fn extract_commands_metadata_pipelines( + #[case] input: &str, + #[case] expected_pipes: Vec, + ) { + let commands = extract_commands_with_metadata(input).unwrap(); assert_eq!( - commands[2].pipe, - PipeInfo { - stdin: true, - stdout: false - } + commands.len(), + expected_pipes.len(), + "command count mismatch for: {}", + input ); + for (i, expected_pipe) in expected_pipes.iter().enumerate() { + assert_eq!( + commands[i].pipe, *expected_pipe, + "PipeInfo mismatch for command #{} in: {}", + i, input + ); + assert!( + commands[i].redirects.is_empty(), + "redirects should be empty for command #{} in: {}", + i, + input + ); + } } } From fa321a9ed296669935b64c58dc58c509261664c5 Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Sat, 14 Mar 2026 01:44:25 +0900 Subject: [PATCH 3/6] rules, adapter: fix evaluate_command not propagating redirect/pipe metadata evaluate_command() always passed empty redirects and default PipeInfo to evaluate_command_inner(), so `when` clauses referencing `redirects` or `pipe` always saw empty values for single commands. This meant rules like `when: 'redirects.exists(r, r.type == "output")'` never matched when evaluated through the evaluate_command path. Fix evaluate_command() to extract metadata via extract_commands_with_metadata() for single commands, and add evaluate_command_with_metadata() for callers (like the adapter) that already have pre-extracted metadata. Update the adapter to use extract_commands_with_metadata() and pass the metadata through. --- src/adapter/mod.rs | 22 ++++++++++++++---- src/rules/rule_engine.rs | 32 ++++++++++++++++++++++++++ tests/integration/when_clause_rules.rs | 26 ++++++++++++++++++++- 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/src/adapter/mod.rs b/src/adapter/mod.rs index d665772..3b018ce 100644 --- a/src/adapter/mod.rs +++ b/src/adapter/mod.rs @@ -7,9 +7,10 @@ use crate::audit::{ SubEvaluation, }; use crate::config::{Config, Defaults, MergedSandboxPolicy}; -use crate::rules::command_parser::extract_commands; +use crate::rules::command_parser::extract_commands_with_metadata; use crate::rules::rule_engine::{ - Action, EvalContext, RuleMatchInfo, default_action, evaluate_command, evaluate_compound, + Action, EvalContext, RuleMatchInfo, default_action, evaluate_command_with_metadata, + evaluate_compound, }; /// Unified evaluation result for the adapter layer. @@ -229,7 +230,11 @@ pub fn run_with_options(endpoint: &dyn Endpoint, config: &Config, options: &RunO let context = EvalContext::from_env(); // Determine if the command is compound (contains pipes, &&, ||, ;) - let commands = extract_commands(&command).unwrap_or_else(|_| vec![command.clone()]); + let extracted_commands = extract_commands_with_metadata(&command).unwrap_or_else(|_| vec![]); + let commands: Vec = extracted_commands + .iter() + .map(|ec| ec.command.clone()) + .collect(); if options.verbose && commands.len() > 1 { eprintln!( @@ -302,7 +307,16 @@ pub fn run_with_options(endpoint: &dyn Endpoint, config: &Config, options: &RunO sub_evaluations: None, } } else { - match evaluate_command(config, effective_command, &context) { + // Pass redirect/pipe metadata from the first extracted command so that + // `when` clauses referencing `redirects` or `pipe` work correctly + // even for single commands with redirects (e.g., `cmd > /tmp/log.txt`). + let first_extracted = extracted_commands.first(); + let empty_redirects = vec![]; + let default_pipe = crate::rules::command_parser::PipeInfo::default(); + let (redirects, pipe) = first_extracted + .map(|ec| (ec.redirects.as_slice(), &ec.pipe)) + .unwrap_or((empty_redirects.as_slice(), &default_pipe)); + match evaluate_command_with_metadata(config, effective_command, &context, redirects, pipe) { Ok(result) => { if options.verbose { log_matched_rules(&result.matched_rules); diff --git a/src/rules/rule_engine.rs b/src/rules/rule_engine.rs index 59ad55c..7af77d6 100644 --- a/src/rules/rule_engine.rs +++ b/src/rules/rule_engine.rs @@ -100,9 +100,41 @@ pub fn evaluate_command( command: &str, context: &EvalContext, ) -> Result { + // For single commands, extract redirect/pipe metadata so that `when` + // clauses referencing `redirects` or `pipe` work correctly. + // Compound commands (multiple extracted commands) are left to the + // compound guard inside evaluate_command_inner. + if let Ok(extracted) = extract_commands_with_metadata(command) + && extracted.len() == 1 + { + let first = &extracted[0]; + return evaluate_command_inner( + config, + &first.command, + context, + 0, + &first.redirects, + &first.pipe, + ); + } evaluate_command_inner(config, command, context, 0, &[], &PipeInfo::default()) } +/// Like `evaluate_command`, but with pre-extracted redirect and pipe metadata. +/// +/// Use this when the caller has already parsed the original command for +/// redirect/pipe information (e.g., the adapter extracts metadata from the +/// original input before stripping redirects for pattern matching). +pub fn evaluate_command_with_metadata( + config: &Config, + command: &str, + context: &EvalContext, + redirects: &[RedirectInfo], + pipe: &PipeInfo, +) -> Result { + evaluate_command_inner(config, command, context, 0, redirects, pipe) +} + /// Evaluate a potentially compound command (containing `|`, `&&`, `||`, `;`) /// by splitting it into individual commands, evaluating each, and aggregating /// the results. diff --git a/tests/integration/when_clause_rules.rs b/tests/integration/when_clause_rules.rs index dae2866..01022c2 100644 --- a/tests/integration/when_clause_rules.rs +++ b/tests/integration/when_clause_rules.rs @@ -472,7 +472,31 @@ fn when_clause_with_path_ref( } // ======================================== -// Redirect conditions in when clause +// Redirect conditions in when clause (via evaluate_command) +// ======================================== + +#[rstest] +#[case::output_redirect_denied("renovate-dryrun > /tmp/log.txt", assert_deny as ActionAssertion)] +#[case::no_redirect_default("renovate-dryrun", assert_ask as ActionAssertion)] +fn redirect_output_exists_via_evaluate_command( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {r#" + rules: + - deny: 'renovate-dryrun' + when: 'redirects.exists(r, r.type == "output")' + "#}) + .unwrap(); + + // evaluate_command (not evaluate_compound) must also propagate redirect metadata + let result = evaluate_command(&config, command, &empty_context).unwrap(); + expected(&result.action); +} + +// ======================================== +// Redirect conditions in when clause (via evaluate_compound) // ======================================== #[rstest] From 7eca3eda080080bd3777b387bd40fabc988a4a39 Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Sat, 14 Mar 2026 01:51:02 +0900 Subject: [PATCH 4/6] adapter: fix parse-failure fallback bypassing rule evaluation When extract_commands_with_metadata() failed (e.g., SyntaxError from malformed input), the fallback was changed from vec![command.clone()] to vec![], which hit the commands.is_empty() branch and returned default_action() without evaluating any rules. This could silently allow commands that should be denied. Restore the original behavior: on parse failure, fall back to an ExtractedCommand containing the raw command with empty metadata. --- src/adapter/mod.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/adapter/mod.rs b/src/adapter/mod.rs index 3b018ce..5cbbd6b 100644 --- a/src/adapter/mod.rs +++ b/src/adapter/mod.rs @@ -7,7 +7,7 @@ use crate::audit::{ SubEvaluation, }; use crate::config::{Config, Defaults, MergedSandboxPolicy}; -use crate::rules::command_parser::extract_commands_with_metadata; +use crate::rules::command_parser::{ExtractedCommand, PipeInfo, extract_commands_with_metadata}; use crate::rules::rule_engine::{ Action, EvalContext, RuleMatchInfo, default_action, evaluate_command_with_metadata, evaluate_compound, @@ -230,7 +230,13 @@ pub fn run_with_options(endpoint: &dyn Endpoint, config: &Config, options: &RunO let context = EvalContext::from_env(); // Determine if the command is compound (contains pipes, &&, ||, ;) - let extracted_commands = extract_commands_with_metadata(&command).unwrap_or_else(|_| vec![]); + let extracted_commands = extract_commands_with_metadata(&command).unwrap_or_else(|_| { + vec![ExtractedCommand { + command: command.clone(), + redirects: vec![], + pipe: PipeInfo::default(), + }] + }); let commands: Vec = extracted_commands .iter() .map(|ec| ec.command.clone()) @@ -312,7 +318,7 @@ pub fn run_with_options(endpoint: &dyn Endpoint, config: &Config, options: &RunO // even for single commands with redirects (e.g., `cmd > /tmp/log.txt`). let first_extracted = extracted_commands.first(); let empty_redirects = vec![]; - let default_pipe = crate::rules::command_parser::PipeInfo::default(); + let default_pipe = PipeInfo::default(); let (redirects, pipe) = first_extracted .map(|ec| (ec.redirects.as_slice(), &ec.pipe)) .unwrap_or((empty_redirects.as_slice(), &default_pipe)); From 3ad244f41b5abb0dbd2cdfbdbdb346cc4575f213 Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Sat, 14 Mar 2026 01:59:35 +0900 Subject: [PATCH 5/6] docs: update redirects note to reflect evaluate_command fix The note stated that redirects are only populated through evaluate_compound, but evaluate_command now also extracts and passes redirect/pipe metadata for single commands. --- docs/src/content/docs/rule-evaluation/when-clause.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/src/content/docs/rule-evaluation/when-clause.md b/docs/src/content/docs/rule-evaluation/when-clause.md index 3b70571..988261e 100644 --- a/docs/src/content/docs/rule-evaluation/when-clause.md +++ b/docs/src/content/docs/rule-evaluation/when-clause.md @@ -124,7 +124,7 @@ Type classification: ``` :::note -The `redirects` variable is only populated when the command is evaluated through `evaluate_compound` (e.g., compound commands with pipes, `&&`, or redirects). For simple commands without redirects or pipes, the list is empty. +The `redirects` list is empty when the command has no redirects attached. Both single commands (e.g., `renovate-dryrun > /tmp/log.txt`) and compound commands (e.g., `cmd > file && cmd2`) populate redirect metadata correctly. ::: ### `pipe` — Pipeline position From 6bb4220d5a4a6159ec3d31e8724ce74e4b0af185 Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Sat, 14 Mar 2026 02:22:29 +0900 Subject: [PATCH 6/6] rules: fix nested pipeline ignoring parent pipe context The pipeline handler computed PipeInfo based solely on the child's index position, ignoring the parent pipe context. For nested pipelines like `cmd1 | (cmd2 | cmd3)`, cmd2 would get stdin=false instead of stdin=true, causing when clauses like `pipe.stdin` to incorrectly evaluate to false. Compose parent and local pipe context with logical OR so that nested pipeline commands inherit the outer pipeline's position. --- src/rules/command_parser.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/rules/command_parser.rs b/src/rules/command_parser.rs index df20d76..c1e8005 100644 --- a/src/rules/command_parser.rs +++ b/src/rules/command_parser.rs @@ -313,8 +313,8 @@ fn collect_commands( let len = children.len(); for (i, child) in children.iter().enumerate() { let child_pipe = PipeInfo { - stdin: i > 0, - stdout: i < len - 1, + stdin: pipe_info.stdin || i > 0, + stdout: pipe_info.stdout || i < len - 1, }; collect_commands(*child, source, commands, &child_pipe, redirects); } @@ -1358,6 +1358,11 @@ mod tests { PipeInfo { stdin: true, stdout: true }, PipeInfo { stdin: true, stdout: false }, ])] + #[case::nested_pipeline_in_subshell("cmd1 | (cmd2 | cmd3)", vec![ + PipeInfo { stdin: false, stdout: true }, + PipeInfo { stdin: true, stdout: true }, + PipeInfo { stdin: true, stdout: false }, + ])] fn extract_commands_metadata_pipelines( #[case] input: &str, #[case] expected_pipes: Vec,