From c8488e37d1578ad173b0e8115634d4889e07648b Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Wed, 25 Feb 2026 01:49:51 +0900 Subject: [PATCH 1/4] feat(cli): accept positional command arguments for `check` subcommand Replace `--command` named option with trailing_var_arg positional arguments, matching the `exec` subcommand interface. Co-Authored-By: Claude Opus 4.6 --- src/adapter/check_adapter.rs | 2 +- src/cli/mod.rs | 22 ++++++++-------- src/cli/route.rs | 49 ++++++++++++++++++------------------ src/main.rs | 10 ++++---- tests/e2e/check_generic.rs | 18 +++++++------ 5 files changed, 52 insertions(+), 49 deletions(-) diff --git a/src/adapter/check_adapter.rs b/src/adapter/check_adapter.rs index 75bf25c..f04b5bb 100644 --- a/src/adapter/check_adapter.rs +++ b/src/adapter/check_adapter.rs @@ -39,7 +39,7 @@ pub struct CheckAdapter { } impl CheckAdapter { - /// Build from the `--command` CLI argument. + /// Build from positional command arguments. pub fn from_command(command: String) -> Self { Self { command } } diff --git a/src/cli/mod.rs b/src/cli/mod.rs index 163d108..c2e8da9 100644 --- a/src/cli/mod.rs +++ b/src/cli/mod.rs @@ -46,10 +46,6 @@ pub struct ExecArgs { #[derive(clap::Args)] #[cfg_attr(test, derive(Debug, PartialEq))] pub struct CheckArgs { - /// Command string to check (skips stdin) - #[arg(long)] - pub command: Option, - /// Input format: "claude-code-hook" or omit for auto-detection #[arg(long)] pub format: Option, @@ -57,6 +53,10 @@ pub struct CheckArgs { /// Output detailed rule matching information to stderr #[arg(long)] pub verbose: bool, + + /// Command and arguments to check (skips stdin) + #[arg(trailing_var_arg = true, allow_hyphen_values = true)] + pub command: Vec, } #[cfg(test)] @@ -86,20 +86,20 @@ mod tests { Commands::Exec(ExecArgs { command: vec!["ls".into()], sandbox: None, dry_run: true, verbose: true }), )] #[case::check_with_command( - &["runok", "check", "--command", "git status"], - Commands::Check(CheckArgs { command: Some("git status".into()), format: None, verbose: false }), + &["runok", "check", "--", "git", "status"], + Commands::Check(CheckArgs { format: None, verbose: false, command: vec!["git".into(), "status".into()] }), )] #[case::check_with_format( &["runok", "check", "--format", "claude-code-hook"], - Commands::Check(CheckArgs { command: None, format: Some("claude-code-hook".into()), verbose: false }), + Commands::Check(CheckArgs { format: Some("claude-code-hook".into()), verbose: false, command: vec![] }), )] #[case::check_with_both( - &["runok", "check", "--command", "ls", "--format", "claude-code-hook"], - Commands::Check(CheckArgs { command: Some("ls".into()), format: Some("claude-code-hook".into()), verbose: false }), + &["runok", "check", "--format", "claude-code-hook", "--", "ls"], + Commands::Check(CheckArgs { format: Some("claude-code-hook".into()), verbose: false, command: vec!["ls".into()] }), )] #[case::check_with_verbose( - &["runok", "check", "--verbose", "--command", "git status"], - Commands::Check(CheckArgs { command: Some("git status".into()), format: None, verbose: true }), + &["runok", "check", "--verbose", "--", "git", "status"], + Commands::Check(CheckArgs { format: None, verbose: true, command: vec!["git".into(), "status".into()] }), )] fn cli_parsing(#[case] argv: &[&str], #[case] expected: Commands) { let cli = Cli::parse_from(argv); diff --git a/src/cli/route.rs b/src/cli/route.rs index 7e4d0bb..b0ed148 100644 --- a/src/cli/route.rs +++ b/src/cli/route.rs @@ -6,7 +6,7 @@ use super::CheckArgs; /// Result of routing `runok check`: either a single endpoint or multiple commands. pub enum CheckRoute { - /// Single endpoint (--command, JSON stdin, or single-line plaintext). + /// Single endpoint (positional command, JSON stdin, or single-line plaintext). Single(Box), /// Multiple commands from multi-line plaintext stdin. Multi(Vec), @@ -17,10 +17,11 @@ pub fn route_check( args: &CheckArgs, mut stdin: impl std::io::Read, ) -> Result { - // 1. --command CLI argument → always generic mode (no stdin) - if let Some(command) = &args.command { + // 1. Positional command arguments → always generic mode (no stdin) + if !args.command.is_empty() { + let command = args.command.join(" "); return Ok(CheckRoute::Single(Box::new(CheckAdapter::from_command( - command.clone(), + command, )))); } @@ -116,11 +117,11 @@ mod tests { use rstest::rstest; /// Helper: build CheckArgs for testing - fn check_args(command: Option<&str>, format: Option<&str>) -> CheckArgs { + fn check_args(command: Vec<&str>, format: Option<&str>) -> CheckArgs { CheckArgs { - command: command.map(String::from), format: format.map(String::from), verbose: false, + command: command.into_iter().map(String::from).collect(), } } @@ -143,17 +144,17 @@ mod tests { // === route_check: --command flag === #[rstest] - #[case::simple_command("git status")] - #[case::command_with_flags("ls -la /tmp")] - fn route_check_with_command_arg(#[case] cmd: &str) { - let args = check_args(Some(cmd), None); + #[case::simple_command(&["git", "status"], "git status")] + #[case::command_with_flags(&["ls", "-la", "/tmp"], "ls -la /tmp")] + fn route_check_with_command_arg(#[case] cmd: &[&str], #[case] expected: &str) { + let args = check_args(cmd.to_vec(), None); let route = route_check(&args, std::io::empty()); let endpoint = unwrap_single(route.unwrap_or_else(|e| panic!("unexpected error: {e}"))); assert_eq!( endpoint .extract_command() .unwrap_or_else(|e| panic!("unexpected error: {e}")), - Some(cmd.to_string()) + Some(expected.to_string()) ); } @@ -180,7 +181,7 @@ mod tests { #[case] stdin_json: &str, #[case] expected_command: Option<&str>, ) { - let args = check_args(None, None); + let args = check_args(vec![], None); let route = route_check(&args, stdin_json.as_bytes()); let endpoint = unwrap_single(route.unwrap_or_else(|e| panic!("unexpected error: {e}"))); assert_eq!( @@ -193,7 +194,7 @@ mod tests { #[rstest] fn route_check_stdin_unknown_json_format_returns_error() { - let args = check_args(None, None); + let args = check_args(vec![], None); let result = route_check(&args, r#"{"unknown_field": "value"}"#.as_bytes()); match result { Err(e) => assert!( @@ -208,7 +209,7 @@ mod tests { #[rstest] fn route_check_format_with_non_json_stdin_returns_error() { - let args = check_args(None, Some("claude-code-hook")); + let args = check_args(vec![], Some("claude-code-hook")); let result = route_check(&args, "not valid json".as_bytes()); match result { Err(e) => assert!( @@ -229,7 +230,7 @@ mod tests { #[case] input: &str, #[case] expected_command: &str, ) { - let args = check_args(None, None); + let args = check_args(vec![], None); let route = route_check(&args, input.as_bytes()); let endpoint = unwrap_single(route.unwrap_or_else(|e| panic!("unexpected error: {e}"))); assert_eq!( @@ -244,7 +245,7 @@ mod tests { #[rstest] fn route_check_plaintext_single_line() { - let args = check_args(None, None); + let args = check_args(vec![], None); let route = route_check(&args, "git status\n".as_bytes()); let endpoint = unwrap_single(route.unwrap_or_else(|e| panic!("unexpected error: {e}"))); assert_eq!( @@ -257,7 +258,7 @@ mod tests { #[rstest] fn route_check_plaintext_multi_line() { - let args = check_args(None, None); + let args = check_args(vec![], None); let input = indoc! {" git status ls -la @@ -277,7 +278,7 @@ mod tests { #[rstest] fn route_check_plaintext_skips_empty_lines() { - let args = check_args(None, None); + let args = check_args(vec![], None); let input = indoc! {" git status @@ -298,7 +299,7 @@ mod tests { #[rstest] fn route_check_plaintext_trims_whitespace() { - let args = check_args(None, None); + let args = check_args(vec![], None); let route = route_check(&args, " git status \n".as_bytes()); let endpoint = unwrap_single(route.unwrap_or_else(|e| panic!("unexpected error: {e}"))); assert_eq!( @@ -311,7 +312,7 @@ mod tests { #[rstest] fn route_check_empty_stdin_returns_error() { - let args = check_args(None, None); + let args = check_args(vec![], None); let result = route_check(&args, "".as_bytes()); match result { Err(e) => assert!( @@ -324,7 +325,7 @@ mod tests { #[rstest] fn route_check_only_empty_lines_returns_error() { - let args = check_args(None, None); + let args = check_args(vec![], None); let result = route_check(&args, "\n\n \n".as_bytes()); match result { Err(e) => assert!( @@ -339,7 +340,7 @@ mod tests { #[rstest] fn route_check_command_flag_takes_precedence_over_stdin() { - let args = check_args(Some("echo hello"), Some("claude-code-hook")); + let args = check_args(vec!["echo", "hello"], Some("claude-code-hook")); let route = route_check(&args, std::io::empty()); let endpoint = unwrap_single(route.unwrap_or_else(|e| panic!("unexpected error: {e}"))); assert_eq!( @@ -354,7 +355,7 @@ mod tests { #[rstest] fn route_check_explicit_format_claude_code_hook() { - let args = check_args(None, Some("claude-code-hook")); + let args = check_args(vec![], Some("claude-code-hook")); let stdin_json = indoc! {r#" { "tool_name": "Bash", @@ -379,7 +380,7 @@ mod tests { #[rstest] fn route_check_unknown_format_returns_error() { - let args = check_args(None, Some("invalid-format")); + let args = check_args(vec![], Some("invalid-format")); let result = route_check(&args, r#"{"command": "ls"}"#.as_bytes()); match result { Err(e) => assert!( diff --git a/src/main.rs b/src/main.rs index 26ad37e..5f36bcf 100644 --- a/src/main.rs +++ b/src/main.rs @@ -140,9 +140,9 @@ mod tests { #[rstest] fn run_command_check_with_command_returns_zero() { let cmd = Commands::Check(CheckArgs { - command: Some("echo hello".into()), format: None, verbose: false, + command: vec!["echo".into(), "hello".into()], }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let exit_code = run_command(cmd, &cwd, std::io::empty()); @@ -152,9 +152,9 @@ mod tests { #[rstest] fn run_command_check_with_empty_stdin_returns_two() { let cmd = Commands::Check(CheckArgs { - command: None, format: None, verbose: false, + command: vec![], }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let exit_code = run_command(cmd, &cwd, "".as_bytes()); @@ -164,9 +164,9 @@ mod tests { #[rstest] fn run_command_check_with_stdin_json_returns_zero() { let cmd = Commands::Check(CheckArgs { - command: None, format: None, verbose: false, + command: vec![], }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let exit_code = run_command(cmd, &cwd, r#"{"command": "ls"}"#.as_bytes()); @@ -176,9 +176,9 @@ mod tests { #[rstest] fn run_command_check_with_plaintext_stdin_returns_zero() { let cmd = Commands::Check(CheckArgs { - command: None, format: None, verbose: false, + command: vec![], }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let exit_code = run_command(cmd, &cwd, "echo hello\n".as_bytes()); @@ -208,9 +208,9 @@ mod tests { #[rstest] fn run_command_check_with_multiline_plaintext_stdin_returns_zero() { let cmd = Commands::Check(CheckArgs { - command: None, format: None, verbose: false, + command: vec![], }); let cwd = std::env::current_dir().unwrap_or_else(|_| PathBuf::from(".")); let input = indoc! {" diff --git a/tests/e2e/check_generic.rs b/tests/e2e/check_generic.rs index 216b16d..ad78af8 100644 --- a/tests/e2e/check_generic.rs +++ b/tests/e2e/check_generic.rs @@ -16,19 +16,21 @@ fn check_env() -> TestEnv { // --- CLI argument mode --- #[rstest] -#[case::deny_rm("rm -rf /", 0, "deny")] -#[case::allow_git_status("git status", 0, "allow")] -#[case::comment_before_command("# description\ngit status", 0, "allow")] -#[case::comment_only("# just a comment", 0, "ask")] +#[case::deny_rm(&["rm", "-rf", "/"], 0, "deny")] +#[case::allow_git_status(&["git", "status"], 0, "allow")] +#[case::comment_before_command(&["# description\ngit status"], 0, "allow")] +#[case::comment_only(&["# just a comment"], 0, "ask")] fn check_command_arg( check_env: TestEnv, - #[case] command: &str, + #[case] command: &[&str], #[case] expected_exit: i32, #[case] expected_decision: &str, ) { let assert = check_env .command() - .args(["check", "--command", command]) + .arg("check") + .arg("--") + .args(command) .assert(); let output = assert.code(expected_exit).get_output().stdout.clone(); let json: serde_json::Value = @@ -93,7 +95,7 @@ fn check_plaintext_stdin_single_line(check_env: TestEnv) { fn check_deny_includes_reason(check_env: TestEnv) { let assert = check_env .command() - .args(["check", "--command", "rm -rf /"]) + .args(["check", "--", "rm", "-rf", "/"]) .assert(); let output = assert.code(0).get_output().stdout.clone(); let json: serde_json::Value = @@ -120,7 +122,7 @@ fn check_allow_with_sandbox_info() { "}); let assert = env .command() - .args(["check", "--command", "python3 script.py"]) + .args(["check", "--", "python3", "script.py"]) .assert(); let output = assert.code(0).get_output().stdout.clone(); let json: serde_json::Value = From 5aedc635b1804180ec63e0e0df924e5e863119df Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Wed, 25 Feb 2026 02:07:20 +0900 Subject: [PATCH 2/4] fix(cli): use shell_quote_join for `check` positional args to preserve argument boundaries Co-Authored-By: Claude Opus 4.6 --- src/cli/route.rs | 4 +++- tests/e2e/check_generic.rs | 38 ++++++++++++++++++++++++++++++++++++-- 2 files changed, 39 insertions(+), 3 deletions(-) diff --git a/src/cli/route.rs b/src/cli/route.rs index b0ed148..4b3fe30 100644 --- a/src/cli/route.rs +++ b/src/cli/route.rs @@ -1,6 +1,7 @@ use crate::adapter::Endpoint; use crate::adapter::check_adapter::{CheckAdapter, CheckInput}; use crate::adapter::hook_adapter::{ClaudeCodeHookAdapter, HookInput}; +use runok::rules::command_parser::shell_quote_join; use super::CheckArgs; @@ -19,7 +20,7 @@ pub fn route_check( ) -> Result { // 1. Positional command arguments → always generic mode (no stdin) if !args.command.is_empty() { - let command = args.command.join(" "); + let command = shell_quote_join(&args.command); return Ok(CheckRoute::Single(Box::new(CheckAdapter::from_command( command, )))); @@ -146,6 +147,7 @@ mod tests { #[rstest] #[case::simple_command(&["git", "status"], "git status")] #[case::command_with_flags(&["ls", "-la", "/tmp"], "ls -la /tmp")] + #[case::arg_with_spaces(&["echo", "hello world"], "echo 'hello world'")] fn route_check_with_command_arg(#[case] cmd: &[&str], #[case] expected: &str) { let args = check_args(cmd.to_vec(), None); let route = route_check(&args, std::io::empty()); diff --git a/tests/e2e/check_generic.rs b/tests/e2e/check_generic.rs index ad78af8..4cfd295 100644 --- a/tests/e2e/check_generic.rs +++ b/tests/e2e/check_generic.rs @@ -18,8 +18,6 @@ fn check_env() -> TestEnv { #[rstest] #[case::deny_rm(&["rm", "-rf", "/"], 0, "deny")] #[case::allow_git_status(&["git", "status"], 0, "allow")] -#[case::comment_before_command(&["# description\ngit status"], 0, "allow")] -#[case::comment_only(&["# just a comment"], 0, "ask")] fn check_command_arg( check_env: TestEnv, #[case] command: &[&str], @@ -38,6 +36,42 @@ fn check_command_arg( assert_eq!(json["decision"], expected_decision); } +// --- Plaintext stdin with comments --- + +#[rstest] +fn check_stdin_comment_before_command(check_env: TestEnv) { + let assert = check_env + .command() + .arg("check") + .write_stdin(indoc! {" + # description + git status + "}) + .assert(); + let output = assert.code(0).get_output().stdout.clone(); + let stdout = String::from_utf8(output).unwrap(); + let jsons: Vec = stdout + .lines() + .map(|line| serde_json::from_str(line).unwrap()) + .collect(); + assert_eq!(jsons.len(), 2); + assert_eq!(jsons[0]["decision"], "ask"); + assert_eq!(jsons[1]["decision"], "allow"); +} + +#[rstest] +fn check_stdin_comment_only(check_env: TestEnv) { + let assert = check_env + .command() + .arg("check") + .write_stdin("# just a comment\n") + .assert(); + let output = assert.code(0).get_output().stdout.clone(); + let json: serde_json::Value = + serde_json::from_slice(&output).unwrap_or_else(|e| panic!("invalid JSON: {e}")); + assert_eq!(json["decision"], "ask"); +} + // --- stdin JSON mode --- #[rstest] From 0d1d63d3e4482ae58b7397ac80d3432c4e8630de Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Wed, 25 Feb 2026 02:19:54 +0900 Subject: [PATCH 3/4] fix(cli): skip shell quoting for single-element positional arg to preserve compound command detection Co-Authored-By: Claude Opus 4.6 --- src/cli/route.rs | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/src/cli/route.rs b/src/cli/route.rs index 4b3fe30..875e3d8 100644 --- a/src/cli/route.rs +++ b/src/cli/route.rs @@ -20,7 +20,13 @@ pub fn route_check( ) -> Result { // 1. Positional command arguments → always generic mode (no stdin) if !args.command.is_empty() { - let command = shell_quote_join(&args.command); + // Return single arguments unquoted so the rule engine can detect + // shell metacharacters (&&, ;, |) in compound commands. + let command = if args.command.len() == 1 { + args.command[0].clone() + } else { + shell_quote_join(&args.command) + }; return Ok(CheckRoute::Single(Box::new(CheckAdapter::from_command( command, )))); From 2c7bb42e2c62cd75c56f1faf50485c345364d75f Mon Sep 17 00:00:00 2001 From: Hayato Kawai Date: Wed, 25 Feb 2026 02:50:57 +0900 Subject: [PATCH 4/4] fix(test): update error_handling E2E tests to use positional args instead of removed `--command` flag Co-Authored-By: Claude Opus 4.6 --- tests/e2e/error_handling.rs | 14 ++++---------- 1 file changed, 4 insertions(+), 10 deletions(-) diff --git a/tests/e2e/error_handling.rs b/tests/e2e/error_handling.rs index 04ba68f..952bc81 100644 --- a/tests/e2e/error_handling.rs +++ b/tests/e2e/error_handling.rs @@ -6,7 +6,7 @@ use super::helpers::TestEnv; // --- Invalid config: syntax error --- #[rstest] -#[case::check(&["check", "--command", "git status"], 2)] +#[case::check(&["check", "--", "git", "status"], 2)] #[case::exec(&["exec", "--dry-run", "--", "echo", "hello"], 1)] fn invalid_config_exits_with_error(#[case] args: &[&str], #[case] expected_exit: i32) { let env = TestEnv::new("rules: [invalid yaml\n broken:"); @@ -19,7 +19,7 @@ fn invalid_config_exits_with_error(#[case] args: &[&str], #[case] expected_exit: // --- Invalid config: validation error (deny + sandbox) --- #[rstest] -#[case::check(&["check", "--command", "rm -rf /"], 2)] +#[case::check(&["check", "--", "rm", "-rf", "/"], 2)] #[case::exec(&["exec", "--dry-run", "--", "rm", "-rf", "/"], 1)] fn validation_error_deny_with_sandbox(#[case] args: &[&str], #[case] expected_exit: i32) { let env = TestEnv::new(indoc! {" @@ -66,7 +66,7 @@ fn exec_nonexistent_command() { "}, )] #[case::check_deny( - &["check", "--command", "rm -rf /"], + &["check", "--", "rm", "-rf", "/"], 0, indoc! {" rules: @@ -86,13 +86,7 @@ fn no_config_check_returns_default() { let env = TestEnv::new("{}"); let assert = env .command() - .args([ - "check", - "--output-format", - "json", - "--command", - "git status", - ]) + .args(["check", "--output-format", "json", "--", "git", "status"]) .assert(); let output = assert.code(0).get_output().stdout.clone(); let json: serde_json::Value =