Skip to content

fix(rules): allow flag-only negation to pass with empty command tokens #179

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
fohte merged 4 commits into from
Mar 10, 2026
Merged

fix(rules): allow flag-only negation to pass with empty command tokens #179

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e31144c
fix(rules): allow flag-only negation to pass when no command tokens r…
fohte Mar 10, 2026
8c1d4b3
test(rules): add extract_placeholder_all coverage for negation branches
fohte Mar 10, 2026
6202fa0
test(rules): add negation-rejection cases to extract_placeholder_all …
fohte Mar 10, 2026
c29ca3b
docs(pattern-syntax): document flag-only negation non-consuming behavior
fohte Mar 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 10 additions & 10 deletions docs/src/content/docs/architecture/pattern-matching.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
2 changes: 1 addition & 1 deletion docs/src/content/docs/pattern-syntax/alternation.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 2 additions & 1 deletion docs/src/content/docs/pattern-syntax/matching-behavior.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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 *'
Expand All @@ -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 |

Expand Down
111 changes: 79 additions & 32 deletions src/rules/pattern_matcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
}
}
Comment on lines 224 to 249
Copy link

@gemini-code-assist gemini-code-assist bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The logic for handling flag-only and positional negations has some duplication. You can refactor this to reduce redundancy and make the distinction between the two cases (the number of tokens consumed) more explicit. This will improve maintainability.

        PatternToken::Negation(inner) => {
            let (negation_passed, consumed) = if is_flag_only_negation(inner) {
                // Flag-only negations scan the entire command token list.
                // If no tokens remain, the flag is trivially absent.
                // They do not consume a positional token.
                let passed = !cmd_tokens
                    .iter()
                    .any(|t| match_flag_token_with_equals(inner, t, definitions));
                (passed, 0)
            } else {
                // Positional negations check only the next token.
                if cmd_tokens.is_empty() {
                    return false;
                }
                let passed = !match_single_token(inner, cmd_tokens[0], definitions);
                (passed, 1)
            };

            if negation_passed {
                match_tokens_core(rest, &cmd_tokens[consumed..], definitions, steps, captures)
            } else {
                false
            }
        }


Expand Down Expand Up @@ -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(())
}
Comment on lines 505 to 537
Copy link

@gemini-code-assist gemini-code-assist bot Mar 10, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to match_tokens_core, the logic for PatternToken::Negation here has some duplicated code. Refactoring this would improve clarity and maintainability by making the difference in token consumption explicit.

        PatternToken::Negation(inner) => {
            let (negation_passed, consumed) = if is_flag_only_negation(inner) {
                let passed = !cmd_tokens
                    .iter()
                    .any(|t| match_flag_token_with_equals(inner, t, definitions));
                (passed, 0)
            } else {
                if cmd_tokens.is_empty() {
                    return Ok(());
                }
                let passed = !match_single_token(inner, cmd_tokens[0], definitions);
                (passed, 1)
            };

            if negation_passed {
                extract_placeholder_all(
                    rest,
                    &cmd_tokens[consumed..],
                    definitions,
                    steps,
                    captured,
                    all_candidates,
                )?;
            }
            Ok(())
        }

Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -1496,8 +1518,33 @@ mod tests {
#[case::negation_before_cmd(
"run !--dry-run <cmd>",
"run --verbose echo hello",
vec![vec!["--verbose", "echo", "hello"]],
)]
#[case::positional_negation_before_cmd(
"run !exec <cmd>",
"run start echo hello",
vec![vec!["echo", "hello"]],
)]
#[case::positional_negation_empty_tokens(
"run !exec <cmd>",
"run",
Vec::<Vec<&str>>::new(),
)]
#[case::flag_negation_empty_tokens_before_cmd(
"run !--dry-run <cmd>",
"run",
Vec::<Vec<&str>>::new(),
)]
#[case::flag_negation_rejected_before_cmd(
"run !--dry-run <cmd>",
"run --dry-run echo hello",
Vec::<Vec<&str>>::new(),
)]
#[case::positional_negation_rejected_before_cmd(
"run !exec <cmd>",
"run exec echo hello",
Vec::<Vec<&str>>::new(),
)]
fn extract_placeholder_cases(
#[case] pattern_str: &str,
#[case] command_str: &str,
Expand Down
30 changes: 30 additions & 0 deletions tests/integration/compound_command_evaluation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
45 changes: 45 additions & 0 deletions tests/integration/config_to_rule_evaluation.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
Loading