Skip to content

feat(rules): add <flag:name> placeholder to inspect every value of repeated and aliased flags in when clauses#278

Merged
fohte merged 5 commits intomainfrom
fohte/flag-groups
Apr 9, 2026
Merged

feat(rules): add <flag:name> placeholder to inspect every value of repeated and aliased flags in when clauses#278
fohte merged 5 commits intomainfrom
fohte/flag-groups

Conversation

@fohte
Copy link
Copy Markdown
Owner

@fohte fohte commented Apr 8, 2026

Why

  • when clauses cannot inspect every value of a repeated or aliased flag
    • gh api graphql -f query=mutation{...} -f variables={} should expose all -f values to detect mutations
    • curl -d @/etc/passwd -d "innocent=data" should fire when even one value sends a sensitive file
    • docker run -v /etc/shadow:/leak:ro -v ./app:/app should expose every --volume mount, not just the last one
  • The existing flags map is last-wins: when the same flag appears multiple times, only one value survives
  • Aliased flags such as -f and --field land under separate keys (flags.f vs flags["field"]), so a when clause cannot check them as a single set

What

  • Add definitions.flag_groups to declare named groups of aliased flags
  • Add a <flag:name> placeholder that matches any flag in a named group
  • Expose every captured value through a new flag_groups[name] CEL variable, always as a list
definitions:
  flag_groups:
    field-flag: ['-f', '-F', '--field', '--raw-field']

rules:
  # Ask whenever any -f / -F / --field / --raw-field value is a mutation
  - allow: 'gh api graphql <flag:field-flag> *'
    when: '!flag_groups["field-flag"].exists(v, v.startsWith("query=mutation"))'
  - ask: 'gh api graphql <flag:field-flag> *'
  • flag_groups[name] is always a list, even when only one value matches, so CEL macros (exists, all, size) work uniformly
  • Every group declared in definitions.flag_groups is seeded as an empty list [] even when the matched rule did not capture any value, so flag_groups["name"] never raises an undeclared-reference error
  • Referencing an undefined group through <flag:undefined> is rejected at config load time as a validation error
  • The existing flags map and <var:name> / <path:name> placeholders are unchanged (additive, non-breaking)

Open with Devin

intent(rules): expose every value of repeated and aliased flags to `when`
  clauses so security policies can inspect all of them at once
  (e.g. `gh api graphql -f query=mutation...`, `curl -d @/etc/passwd`,
  `docker run -v /etc/shadow:...`).
decision(rules): introduce a new `definitions.flag_groups` block plus a
  `<flag:name>` pattern placeholder rather than overloading the existing
  `flags` map. The placeholder collects every occurrence of any aliased
  flag and surfaces them through a new `flag_groups[name]` CEL variable
  whose values are always lists, matching how clap/cobra/argparse expose
  multi-value flags.
rejected(rules): extend the existing `flags` map with mixed string/list
  values — type-inconsistent variables are bug-prone in CEL.
rejected(rules): convert `flags` itself to lists — breaking change that
  invalidates every existing `when: flags.X == ...` rule.
constraint(rules): CEL `has()` macro does not support bracket notation, so
  `flag_groups[name]` is always seeded with empty lists for every group
  declared in `definitions.flag_groups`. This guarantees that
  `flag_groups["name"]` never raises an undeclared-reference error.
learned(rules): the matching engine's `should_consume_as_value` heuristic
  refuses to consume a trailing wildcard as a flag value (because that
  would leave nothing for the wildcard to match). For `<flag:name>` we
  bypass this heuristic — the placeholder is unambiguously a
  flag-with-value construct, so the next lex token is always its value.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 8, 2026

Codecov Report

❌ Patch coverage is 91.93548% with 25 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.55%. Comparing base (fd61be6) to head (20adad4).
⚠️ Report is 10 commits behind head on main.

Files with missing lines Patch % Lines
src/rules/pattern_matcher/mod.rs 89.59% 18 Missing ⚠️
src/rules/pattern_parser.rs 91.22% 5 Missing ⚠️
src/config/model.rs 98.00% 1 Missing ⚠️
src/rules/rule_engine.rs 96.15% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main     #278      +/-   ##
==========================================
+ Coverage   89.50%   89.55%   +0.04%     
==========================================
  Files          52       52              
  Lines       10731    11010     +279     
==========================================
+ Hits         9605     9860     +255     
- Misses       1126     1150      +24     
Flag Coverage Δ
Linux 89.35% <91.93%> (+0.08%) ⬆️
macOS 90.86% <91.93%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

gemini-code-assist[bot]

This comment was marked as resolved.

intent(docs): unblock the docs site CI build that was failing with
  "invalid hash" link validation errors on the new flag_groups section.
learned(docs): Starlight slugifies `### \`flag_groups\` -- Captured ...`
  to `flag_groups--captured-...` (collapsing the surrounding spaces into
  a single dash on each side, leaving exactly two dashes around `--`).
  My initial cross-references included four dashes which the link
  validator rejected.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

intent(test): consolidate the three validation-error tests in
  `flag_group_evaluation.rs` and the four `flag_groups` CEL access tests
  in `expr_evaluator.rs` into single `#[rstest]` parameterized functions,
  matching CLAUDE.md's testing convention. Devin Review flagged both as
  duplicated test bodies that differ only in input/expected values.
intent(docs): document that the `<flag:name>` placeholder also recognizes
  `=`-joined short flags (e.g. `-f=value`), not just `--field=value`.
  Gemini Code Assist pointed out the omission was confusing.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…flag gap

intent(rules): close two follow-up gaps in the new `<flag:name>` feature
  before merging — one was a silent miscompile risk, the other a missing
  reminder for future contributors.
decision(rules/parser): error out when `<flag:name>` appears inside an
  Optional group `[...]`. The matcher's `optional_flags_absent` only
  knows about `FlagWithValue` aliases, so it cannot tell that grouped
  flags are still present in the command tokens — taking the "absent"
  path would silently drop captured values and produce an incorrect
  match. Failing fast at parse time is safer than supporting it
  half-heartedly.
decision(rules/matcher, config/model): document the bool-flag limitation
  with TODO comments next to the matcher arm and the
  `definitions.flag_groups` field. The original spec deferred boolean
  flag support, and committing the constraint inline keeps the gap
  visible to future contributors instead of relying on the PR description.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
devin-ai-integration[bot]

This comment was marked as resolved.

intent(test): teach the matcher-level test helper `build_schema_from_pattern`
  about `FlagGroupRef` so unit tests using `<flag:name>` build the same
  value-flag schema the production `rule_engine::build_flag_schema` does.
  Without it, `parse_command` would mis-tokenize `-f query=hello` as a
  boolean flag plus a positional `query=hello`, silently breaking any
  matcher-level test exercising the new placeholder.
intent(docs): add the missing PR link to the `flag_groups` release-notes
  heading per CLAUDE.md's release-notes format requirement.

learned(test): the matcher submodule keeps its own copy of the schema-
  building helpers, so production refactors that touch
  `rule_engine::collect_value_flags` need to be mirrored here too. Added a
  `flag_group_ref_matching` rstest case that would have caught this gap.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@fohte fohte merged commit 2fe0c76 into main Apr 9, 2026
11 checks passed
@fohte fohte deleted the fohte/flag-groups branch April 9, 2026 00:57
@fohte-bot fohte-bot Bot mentioned this pull request Apr 9, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant