diff --git a/docs/src/content/docs/architecture/pattern-matching.md b/docs/src/content/docs/architecture/pattern-matching.md index 18e1649..d43ffb8 100644 --- a/docs/src/content/docs/architecture/pattern-matching.md +++ b/docs/src/content/docs/architecture/pattern-matching.md @@ -104,16 +104,16 @@ A step counter (`MAX_MATCH_STEPS = 10,000`) prevents exponential blowup on patho ### Matching rules by token type -| Pattern token | Matching behavior | -| --------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------ | -| `Literal` | Exact string match at current position | -| `Wildcard` | Matches zero or more remaining tokens (greedy with backtracking) | -| `Alternation` | Matches if the command token equals any alternative | -| `FlagWithValue` | Scans the entire token list for the flag, then checks the next token matches the value. **Order-independent**: the flag can appear anywhere in the command | -| `Negation` | Matches if the command token does **not** equal the value. Flag-only negations (all alternatives start with `-`) are **order-independent**: scans all tokens | -| `Optional` | Tries matching with the optional tokens included, falls back to without | -| `Placeholder` | Captures remaining tokens for wrapper command re-evaluation | -| `PathRef` | Expands `definitions.paths` entries and matches against them | +| Pattern token | Matching behavior | +| --------------- | ----------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | +| `Literal` | Exact string match at current position | +| `Wildcard` | Matches zero or more remaining tokens (greedy with backtracking) | +| `Alternation` | Matches if the command token equals any alternative | +| `FlagWithValue` | Scans the entire token list for the flag, then checks the next token matches the value. **Order-independent**: the flag can appear anywhere in the command | +| `Negation` | **Positional negation**: matches if the current command token does **not** equal the value (consumes one token). **Flag-only negation** (all alternatives start with `-`): scans the entire token list for the forbidden flag without consuming a positional token. Passes when the flag is absent, including when no tokens remain | +| `Optional` | Tries matching with the optional tokens included, falls back to without | +| `Placeholder` | Captures remaining tokens for wrapper command re-evaluation | +| `PathRef` | Expands `definitions.paths` entries and matches against them | ### Order-independent flag matching diff --git a/docs/src/content/docs/pattern-syntax/alternation.md b/docs/src/content/docs/pattern-syntax/alternation.md index de029a8..3b39a5d 100644 --- a/docs/src/content/docs/pattern-syntax/alternation.md +++ b/docs/src/content/docs/pattern-syntax/alternation.md @@ -79,7 +79,7 @@ Combine negation with alternation to exclude multiple values. The `!` prefix app ``` :::note -Flag-only negation patterns (all alternatives starting with `-`) use [order-independent matching](/pattern-syntax/matching-behavior/#flag-only-negation) — the negated flag is checked against all command tokens, not just the one at the pattern's position. +Flag-only negation patterns (all alternatives starting with `-`) use [order-independent matching](/pattern-syntax/matching-behavior/#flag-only-negation) — the negated flag is checked against all command tokens, not just the one at the pattern's position. Flag-only negation does not consume a positional token and also passes when no tokens remain (the flag is trivially absent). ::: ### Negation with Glob diff --git a/docs/src/content/docs/pattern-syntax/matching-behavior.md b/docs/src/content/docs/pattern-syntax/matching-behavior.md index ad394fe..dc4f00b 100644 --- a/docs/src/content/docs/pattern-syntax/matching-behavior.md +++ b/docs/src/content/docs/pattern-syntax/matching-behavior.md @@ -70,7 +70,7 @@ This applies to standalone flags ([alternation](/pattern-syntax/alternation/)), ### Flag-only Negation -Negation patterns where all alternatives start with `-` also use order-independent matching. The matcher scans the entire command for any token matching the negated pattern and rejects the match if found: +Negation patterns where all alternatives start with `-` also use order-independent matching. The matcher scans the entire command for any token matching the negated pattern and rejects the match if found. Unlike positional negation, flag-only negation does **not** consume a positional token — it only asserts that the forbidden flag is absent. This means it also passes when there are no command tokens (the flag is trivially absent): ```yaml - allow: 'find !-delete|-fprint|-fls *' @@ -79,6 +79,7 @@ Negation patterns where all alternatives start with `-` also use order-independe | Command | Result | | -------------------------- | -------------- | | `find . -name foo -type f` | Matches | +| `find` | Matches | | `find . -delete` | Does not match | | `find -fprint output .` | Does not match | diff --git a/src/rules/pattern_matcher.rs b/src/rules/pattern_matcher.rs index f7a77e3..0abd8d7 100644 --- a/src/rules/pattern_matcher.rs +++ b/src/rules/pattern_matcher.rs @@ -222,23 +222,29 @@ fn match_tokens_core<'a>( } PatternToken::Negation(inner) => { - if cmd_tokens.is_empty() { - return false; - } - // Flag-only negations (all inner elements start with `-`, excluding - // bare `--`) use order-independent matching: scan the entire command - // token list. Other negations check only the positional token. - let negation_passed = if is_flag_only_negation(inner) { - !cmd_tokens + if is_flag_only_negation(inner) { + // Flag-only negations scan the entire command token list for + // the forbidden flag (order-independent). When no tokens remain + // the flag is trivially absent, so the negation passes. + let negation_passed = !cmd_tokens .iter() - .any(|t| match_flag_token_with_equals(inner, t, definitions)) - } else { - !match_single_token(inner, cmd_tokens[0], definitions) - }; - if negation_passed { - match_tokens_core(rest, &cmd_tokens[1..], definitions, steps, captures) + .any(|t| match_flag_token_with_equals(inner, t, definitions)); + if negation_passed { + // Flag-only negations do not consume a positional token. + match_tokens_core(rest, cmd_tokens, definitions, steps, captures) + } else { + false + } } else { - false + if cmd_tokens.is_empty() { + return false; + } + let negation_passed = !match_single_token(inner, cmd_tokens[0], definitions); + if negation_passed { + match_tokens_core(rest, &cmd_tokens[1..], definitions, steps, captures) + } else { + false + } } } @@ -497,25 +503,35 @@ fn extract_placeholder_all<'a>( } PatternToken::Negation(inner) => { - if cmd_tokens.is_empty() { - return Ok(()); - } - let negation_passed = if is_flag_only_negation(inner) { - !cmd_tokens + if is_flag_only_negation(inner) { + let negation_passed = !cmd_tokens .iter() - .any(|t| match_flag_token_with_equals(inner, t, definitions)) + .any(|t| match_flag_token_with_equals(inner, t, definitions)); + if negation_passed { + extract_placeholder_all( + rest, + cmd_tokens, + definitions, + steps, + captured, + all_candidates, + )?; + } } else { - !match_single_token(inner, cmd_tokens[0], definitions) - }; - if negation_passed { - extract_placeholder_all( - rest, - &cmd_tokens[1..], - definitions, - steps, - captured, - all_candidates, - )?; + if cmd_tokens.is_empty() { + return Ok(()); + } + let negation_passed = !match_single_token(inner, cmd_tokens[0], definitions); + if negation_passed { + extract_placeholder_all( + rest, + &cmd_tokens[1..], + definitions, + steps, + captured, + all_candidates, + )?; + } } Ok(()) } @@ -1028,6 +1044,12 @@ mod tests { "sort --reverse=true file.txt", true )] + // Flag-only negation with empty command tokens (no arguments after command) + #[case::flag_negation_empty_tokens_single("sort !-o *", "sort", true)] + #[case::flag_negation_empty_tokens_alt("sort !-o|--output|--compress-program *", "sort", true)] + #[case::flag_negation_empty_tokens_find("find !-delete *", "find", true)] + // Positional negation with empty tokens should still be false + #[case::positional_negation_empty_tokens("kubectl !describe *", "kubectl", false)] fn negation_matching( #[case] pattern_str: &str, #[case] command_str: &str, @@ -1496,8 +1518,33 @@ mod tests { #[case::negation_before_cmd( "run !--dry-run ", "run --verbose echo hello", + vec![vec!["--verbose", "echo", "hello"]], + )] + #[case::positional_negation_before_cmd( + "run !exec ", + "run start echo hello", vec![vec!["echo", "hello"]], )] + #[case::positional_negation_empty_tokens( + "run !exec ", + "run", + Vec::>::new(), + )] + #[case::flag_negation_empty_tokens_before_cmd( + "run !--dry-run ", + "run", + Vec::>::new(), + )] + #[case::flag_negation_rejected_before_cmd( + "run !--dry-run ", + "run --dry-run echo hello", + Vec::>::new(), + )] + #[case::positional_negation_rejected_before_cmd( + "run !exec ", + "run exec echo hello", + Vec::>::new(), + )] fn extract_placeholder_cases( #[case] pattern_str: &str, #[case] command_str: &str, diff --git a/tests/integration/compound_command_evaluation.rs b/tests/integration/compound_command_evaluation.rs index 562e8e2..4e57943 100644 --- a/tests/integration/compound_command_evaluation.rs +++ b/tests/integration/compound_command_evaluation.rs @@ -1149,3 +1149,33 @@ fn command_substitution_must_not_bypass_rules( let result = evaluate_compound(&config, command, &empty_context).unwrap(); expected(&result.action); } + +// ======================================== +// Flag-only negation with empty tokens in compound commands +// ======================================== + +#[rstest] +#[case::sort_no_args_in_pipeline( + "jq .type file | sort | uniq -c", + assert_allow as ActionAssertion, +)] +#[case::sort_with_banned_flag_in_pipeline( + "jq .type file | sort -o result.txt | uniq -c", + assert_ask as ActionAssertion, +)] +fn flag_negation_empty_tokens_in_compound( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {" + rules: + - allow: 'jq *' + - allow: 'sort !-o|--output|--compress-program *' + - allow: 'uniq *' + "}) + .unwrap(); + + let result = evaluate_compound(&config, command, &empty_context).unwrap(); + expected(&result.action); +} diff --git a/tests/integration/config_to_rule_evaluation.rs b/tests/integration/config_to_rule_evaluation.rs index e08654f..fd42a62 100644 --- a/tests/integration/config_to_rule_evaluation.rs +++ b/tests/integration/config_to_rule_evaluation.rs @@ -998,3 +998,48 @@ fn flag_negation_equals_form( let result = evaluate_command(&config, command, &empty_context).unwrap(); expected(&result.action); } + +// ======================================== +// Flag-only negation with empty tokens (no arguments after command) +// ======================================== + +#[rstest] +#[case::sort_no_args_allowed( + "sort", + assert_allow as ActionAssertion, +)] +#[case::sort_safe_flag_allowed( + "sort -r", + assert_allow as ActionAssertion, +)] +#[case::sort_banned_flag_rejected( + "sort -o result.txt", + assert_ask as ActionAssertion, +)] +#[case::find_no_args_allowed( + "find", + assert_allow as ActionAssertion, +)] +#[case::find_safe_args_allowed( + "find . -name foo", + assert_allow as ActionAssertion, +)] +#[case::find_banned_flag_rejected( + "find . -delete", + assert_ask as ActionAssertion, +)] +fn flag_negation_empty_tokens( + #[case] command: &str, + #[case] expected: ActionAssertion, + empty_context: EvalContext, +) { + let config = parse_config(indoc! {" + rules: + - allow: 'sort !-o|--output|--compress-program *' + - allow: 'find !-delete *' + "}) + .unwrap(); + + let result = evaluate_command(&config, command, &empty_context).unwrap(); + expected(&result.action); +}