Skip to content

fix(rules): match FlagWithValue patterns against =-joined tokens #180

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): match FlagWithValue patterns against =-joined tokens #180

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
a8181a4
fix(rules): match `FlagWithValue` patterns against `=`-joined tokens
fohte Mar 10, 2026
c62af86
fix(rules): match `=`-joined tokens in `extract_placeholder_all`
fohte Mar 10, 2026
6604b75
docs: document `=`-joined flag-value matching
fohte Mar 10, 2026
15e18ae
test(rules): parameterize `extract_placeholder_with_flag_with_value` …
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
2 changes: 1 addition & 1 deletion 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 @@ -109,7 +109,7 @@ A step counter (`MAX_MATCH_STEPS = 10,000`) prevents exponential blowup on patho
| `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 |
| `FlagWithValue` | Scans the entire token list for the flag, then checks the next token matches the value. Also matches `=`-joined forms (e.g. `--flag=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 |
Expand Down
14 changes: 14 additions & 0 deletions 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 @@ -68,6 +68,20 @@ Flags (tokens starting with `-`) in patterns are matched **regardless of their p

This applies to standalone flags ([alternation](/pattern-syntax/alternation/)), [flag-value pairs](/pattern-syntax/matching-behavior/#flag-schema-inference), and flag-only [negations](/pattern-syntax/alternation/#negation). The matcher scans the entire command token list to find a matching flag, removes it, and continues matching the remaining tokens.

### `=`-joined Flag Values

Flag-value patterns also match `=`-joined forms. A pattern like `-X POST` matches both the space-separated `curl -X POST` and the `=`-joined `curl -X=POST`:

```yaml
- deny: 'curl -X|--request POST *'
```

| Command | Result |
| ----------------------------------------- | ------- |
| `curl -X POST https://example.com` | Matches |
| `curl -X=POST https://example.com` | Matches |
| `curl --request=POST https://example.com` | Matches |

### 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. 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):
Expand Down
2 changes: 2 additions & 0 deletions docs/src/content/docs/pattern-syntax/optional-groups.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,8 @@ When an optional group is skipped, runok verifies that any flags in the group ar
| `curl https://example.com` | Matches | Optional group is absent |
| `curl -X POST https://example.com` | Matches | Optional group matches |
| `curl -X DELETE https://example.com` | Does not match | `-X` is present but with wrong value |
| `curl -X=POST https://example.com` | Matches | `=`-joined form is also supported |
| `curl -X=DELETE https://example.com` | Does not match | `-X` is present but with wrong value |

## Literal `[` and `]`

Expand Down
107 changes: 96 additions & 11 deletions src/rules/pattern_matcher.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,7 @@ fn match_tokens_core<'a>(
// Search for the flag anywhere in the remaining command tokens
// (order-independent matching)
for i in 0..cmd_tokens.len() {
// Case 1: space-separated flag and value (e.g. `--sort value`)
if aliases.iter().any(|a| a.as_str() == cmd_tokens[i])
&& i + 1 < cmd_tokens.len()
&& match_single_token(value, cmd_tokens[i + 1], definitions)
Expand All @@ -217,6 +218,31 @@ fn match_tokens_core<'a>(
return true;
}
}

// Case 2: `=`-joined flag and value (e.g. `--sort=value`)
if let Some(eq_pos) = cmd_tokens[i].find('=') {
let flag_part = &cmd_tokens[i][..eq_pos];
let value_part = &cmd_tokens[i][eq_pos + 1..];
if flag_part.starts_with('-')
&& aliases.iter().any(|a| a.as_str() == flag_part)
&& match_single_token(value, value_part, definitions)
{
let remaining = remove_indices(cmd_tokens, &[i]);
if let Some(ref mut caps) = captures {
let saved_len = caps.len();
if matches!(value.as_ref(), PatternToken::Wildcard) {
caps.push(value_part);
}
if match_tokens_core(rest, &remaining, definitions, steps, Some(*caps))
{
return true;
}
caps.truncate(saved_len);
} else if match_tokens_core(rest, &remaining, definitions, steps, None) {
return true;
}
}
}
Comment on lines +222 to +245
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.

security-high high

The =-joined flag matching for FlagWithValue patterns is incomplete in extract_placeholder_all, leading to a security bypass. This allows wrapper patterns to be circumvented if flags are provided in =-joined form, potentially bypassing security rules. This violates the principle that pattern matching for security rules must check all occurrences of flags in the raw input, and that rules must match tokens literally, especially for flags with attached values. The same =-joined matching logic applied elsewhere should also be implemented in extract_placeholder_all for PatternToken::FlagWithValue. Additionally, consider refactoring the duplicated logic for space-separated and =-joined FlagWithValue matching in match_tokens_core to improve maintainability.

References
  1. For command parsing, pattern matching for security rules should be performed on the raw, tokenized input (raw_tokens) to ensure all occurrences of flags are checked. Incomplete matching for =-joined flags can lead to security bypasses, violating this rule.
  2. The command parser should follow a 'What You See Is How It Parses' principle. Rules must match tokens literally, especially for flags with attached values (e.g., -XPOST or --flag=value), and incomplete matching for these forms violates this principle.

}
false
}
Expand Down Expand Up @@ -470,6 +496,7 @@ fn extract_placeholder_all<'a>(

PatternToken::FlagWithValue { aliases, value } => {
for i in 0..cmd_tokens.len() {
// Case 1: space-separated flag and value
if aliases.iter().any(|a| a.as_str() == cmd_tokens[i])
&& i + 1 < cmd_tokens.len()
&& match_single_token(value, cmd_tokens[i + 1], definitions)
Expand All @@ -484,6 +511,26 @@ fn extract_placeholder_all<'a>(
all_candidates,
)?;
}

// Case 2: `=`-joined flag and value
if let Some(eq_pos) = cmd_tokens[i].find('=') {
let flag_part = &cmd_tokens[i][..eq_pos];
let value_part = &cmd_tokens[i][eq_pos + 1..];
if flag_part.starts_with('-')
&& aliases.iter().any(|a| a.as_str() == flag_part)
&& match_single_token(value, value_part, definitions)
{
let remaining = remove_indices(cmd_tokens, &[i]);
extract_placeholder_all(
rest,
&remaining,
definitions,
steps,
captured,
all_candidates,
)?;
}
}
}
Ok(())
}
Expand Down Expand Up @@ -624,10 +671,22 @@ fn optional_flags_absent(optional_tokens: &[PatternToken], cmd_tokens: &[&str])
for token in optional_tokens {
match token {
PatternToken::FlagWithValue { aliases, .. } => {
if cmd_tokens
.iter()
.any(|t| aliases.iter().any(|a| a.as_str() == *t))
{
if cmd_tokens.iter().any(|t| {
// Exact match (space-separated form)
if aliases.iter().any(|a| a.as_str() == *t) {
return true;
}
// `=`-joined form: check the flag portion before `=`
if let Some(eq_pos) = t.find('=') {
let flag_part = &t[..eq_pos];
if flag_part.starts_with('-')
&& aliases.iter().any(|a| a.as_str() == flag_part)
{
return true;
}
}
false
}) {
return false;
}
}
Expand Down Expand Up @@ -1164,6 +1223,32 @@ mod tests {
"curl -X POST https://example.com -s",
false
)]
// `=`-joined flag with value
#[case::optional_flag_with_value_equals_joined(
"git branch [--sort *]",
"git branch --sort=-committerdate",
true
)]
#[case::optional_flag_with_value_equals_joined_absent(
"git branch [--sort *]",
"git branch",
true
)]
#[case::optional_flag_with_value_equals_joined_with_other_flags(
"git branch [-a] [--sort *]",
"git branch -a --sort=-committerdate",
true
)]
#[case::optional_flag_with_value_equals_joined_specific_value(
"curl [-X|--request GET] *",
"curl -X=GET https://example.com",
true
)]
#[case::optional_flag_with_value_equals_joined_wrong_value(
"curl [-X|--request GET] *",
"curl -X=POST https://example.com",
false
)]
fn optional_matching(
#[case] pattern_str: &str,
#[case] command_str: &str,
Expand Down Expand Up @@ -1567,13 +1652,13 @@ mod tests {
}

#[rstest]
fn extract_placeholder_with_flag_with_value(empty_defs: Definitions) {
// FlagWithValue token before <cmd>
let result = check_extract(
"run -m|--mode debug <cmd>",
"run -m debug echo hello",
&empty_defs,
);
#[case::space_separated("run -m debug echo hello")]
#[case::equals_joined("run -m=debug echo hello")]
fn extract_placeholder_with_flag_with_value(
#[case] command_str: &str,
empty_defs: Definitions,
) {
let result = check_extract("run -m|--mode debug <cmd>", command_str, &empty_defs);
assert_eq!(result, vec![vec!["echo".to_string(), "hello".to_string()]]);
}

Expand Down
48 changes: 48 additions & 0 deletions tests/integration/optional_notation_and_path_ref.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ use runok::rules::rule_engine::{EvalContext, evaluate_command};
#[case::with_short_flag("curl -X GET https://example.com", assert_allow as ActionAssertion)]
#[case::with_long_flag("curl --request GET https://example.com", assert_allow as ActionAssertion)]
#[case::with_wrong_value("curl -X POST https://example.com", assert_ask as ActionAssertion)]
#[case::equals_joined_short("curl -X=GET https://example.com", assert_allow as ActionAssertion)]
#[case::equals_joined_long("curl --request=GET https://example.com", assert_allow as ActionAssertion)]
#[case::equals_joined_wrong_value("curl -X=POST https://example.com", assert_ask as ActionAssertion)]
fn optional_flag_with_value(
#[case] command: &str,
#[case] expected: ActionAssertion,
Expand All @@ -37,6 +40,7 @@ fn optional_flag_with_value(
#[case::without_c("git status", assert_allow as ActionAssertion)]
#[case::with_c("git -C /some/path status", assert_allow as ActionAssertion)]
#[case::with_c_different_path("git -C /another/path status", assert_allow as ActionAssertion)]
#[case::equals_joined("git -C=/tmp status", assert_allow as ActionAssertion)]
fn optional_flag_with_wildcard_value(
#[case] command: &str,
#[case] expected: ActionAssertion,
Expand All @@ -52,6 +56,42 @@ fn optional_flag_with_wildcard_value(
expected(&result.action);
}

// ========================================
// Optional notation with `=`-joined flag value (e.g. --sort=-committerdate)
// ========================================

#[rstest]
#[case::space_separated(
"git branch -a --sort -committerdate",
assert_allow as ActionAssertion,
)]
#[case::equals_joined(
"git branch -a --sort=-committerdate",
assert_allow as ActionAssertion,
)]
#[case::without_sort(
"git branch -a",
assert_allow as ActionAssertion,
)]
#[case::sort_only(
"git branch --sort=-committerdate",
assert_allow as ActionAssertion,
)]
fn optional_flag_with_value_equals_joined(
#[case] command: &str,
#[case] expected: ActionAssertion,
empty_context: EvalContext,
) {
let config = parse_config(indoc! {"
rules:
- allow: 'git branch [-a|--all] [-r|--remotes] [-v|--verbose] [--sort *] [-l|--list *]'
"})
.unwrap();

let result = evaluate_command(&config, command, &empty_context).unwrap();
expected(&result.action);
}

// ========================================
// Optional notation: rm [-f] *
// ========================================
Expand Down Expand Up @@ -308,6 +348,14 @@ fn path_normalization_resolves_traversal(
"curl -H 'Content-Type: application/json' -X POST https://example.com",
assert_deny as ActionAssertion,
)]
#[case::equals_joined_short(
"curl -X=POST https://example.com",
assert_deny as ActionAssertion,
)]
#[case::equals_joined_long(
"curl --request=POST https://example.com",
assert_deny as ActionAssertion,
)]
fn flag_with_value_position_independence(
#[case] command: &str,
#[case] expected: ActionAssertion,
Expand Down
Loading