fix(rules): resolve unmatched sub-commands to concrete actions in compound evaluation

[fohte]

Why

• Command substitutions ($()) containing an allowed command could bypass rule controls for the entire compound command
  • e.g. gh pr edit 10 --body "$(echo test)" was incorrectly allowed when only echo * had an allow rule, because the Allow from echo test propagated to the entire compound despite gh pr edit having no matching rule
  • Action::Default had the lowest priority (0), so it always lost to Allow (1) during merge

What

• Remove Action::Default and resolve unmatched commands to concrete actions at evaluation time using defaults.action (falling back to Ask)
• Prefer rule-matched candidates over unmatched ones in wrapper wildcard candidate selection
• Remove the Dispatch enum and resolve_no_match from the adapter layer to simplify dispatch logic

---

[gemini-code-assist]

Summary of Changes

Hello, I'm Gemini Code Assist¹! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!

This pull request significantly enhances the security and predictability of rule evaluation, particularly for compound commands and command substitutions. By removing the ambiguous Action::Default and explicitly resolving unmatched commands to concrete actions, it prevents scenarios where allowed sub-commands could inadvertently grant permission to a larger, unapproved command. The changes also refine how wrapper patterns are evaluated, prioritizing explicit rule matches to ensure consistent and secure behavior.

Highlights

• Security Fix for Command Substitution Bypass: Resolved a critical issue where command substitutions containing allowed commands could bypass rule controls for the entire compound command, leading to unintended 'Allow' actions.
• Elimination of Action::Default: The Action::Default enum variant has been removed. Unmatched commands now explicitly resolve to a concrete action (Allow, Deny, or Ask) at evaluation time, using the configured defaults.action or falling back to Ask.
• Refined Wrapper Wildcard Selection: The logic for selecting candidates in wrapper wildcard patterns (e.g., xargs * <cmd>) has been improved to prefer rule-matched candidates over unmatched ones, ensuring more restrictive actions take precedence.
• Simplified Adapter Layer: The Dispatch enum and resolve_no_match function have been removed from the adapter layer, streamlining the action dispatch logic.

Changelog

• src/adapter/check_adapter.rs
  • Removed Action::Default case from build_check_output function.
• src/adapter/exec_adapter.rs
  • Removed Action::Default handling from handle_action and handle_dry_run methods.
  • Removed associated test cases for Action::Default.
• src/adapter/hook_adapter.rs
  • Removed Action::Default case from ClaudeCodeHookAdapter implementation.
• src/adapter/mod.rs
  • Imported default_action from rule_engine module.
  • Removed Dispatch enum and resolve_no_match function.
  • Added verbose logging for sub-command details in run_with_options.
  • Updated handling of empty commands to use default_action(config) instead of Action::Default.
  • Simplified the main execution flow in run_with_options by removing Dispatch logic and directly handling action_result.
• src/rules/rule_engine.rs
  • Added SubCommandDetail struct and included it in CompoundEvalResult for verbose logging.
  • Removed Action::Default variant from the Action enum.
  • Made Action and DenyResponse enums Clone.
  • Modified evaluate_compound to store sub-command details and use default_action for initial action resolution.
  • Removed Action::Default from escalate_to_ask logic.
  • Updated evaluate_command_inner and evaluate_simple_command to use default_action(config) for unmatched commands.
  • Improved try_unwrap_wrapper to prioritize rule-matched candidates over unmatched ones during wrapper evaluation.
  • Adjusted action_priority values to reflect the removal of Action::Default (Allow is now 0, Ask 1, Deny 2).
  • Refactored resolve_default_action into a new public function default_action which always returns a concrete action (Allow/Deny/Ask) and defaults to Ask if not configured.
• tests/integration/compound_command_evaluation.rs
  • Removed assert_default helper function.
  • Updated test cases to assert Action::Ask(None) instead of Action::Default for unmatched commands.
  • Added new regression tests to ensure command substitutions do not bypass rule controls.
• tests/integration/config_to_rule_evaluation.rs
  • Removed assert_default helper function.
  • Updated test cases to assert Action::Ask(None) instead of Action::Default for unmatched commands.
• tests/integration/extends_resolution.rs
  • Removed assert_default helper function.
  • Updated test cases to assert Action::Ask(None) instead of Action::Default for unmatched commands.
• tests/integration/main.rs
  • Removed assert_default helper function.
• tests/integration/optional_notation_and_path_ref.rs
  • Removed assert_default helper function.
  • Updated test cases to assert Action::Ask(None) instead of Action::Default for unmatched commands.
• tests/integration/remote_preset_with_path.rs
  • Removed assert_default helper function.
  • Updated test cases to assert Action::Ask(None) instead of Action::Default for unmatched commands.
• tests/integration/when_clause_rules.rs
  • Removed assert_default helper function.
  • Updated test cases to assert Action::Ask(None) instead of Action::Default for unmatched commands.
• tests/integration/wrapper_recursive_evaluation.rs
  • Removed assert_default helper function.
  • Updated test cases to assert Action::Ask(None) instead of Action::Default for unmatched commands.

Activity

• The pull request author, fohte, created this pull request to address a security vulnerability and improve rule evaluation logic.
• The changes involve significant refactoring across the rule engine and adapter layers to remove the Action::Default state and ensure explicit action resolution.
• New test cases have been added and existing ones updated to validate the corrected behavior, especially for compound commands and wrapper evaluations.

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature	Command	Description
Code Review	/gemini review	Performs a code review for the current pull request in its current state.
Pull Request Summary	/gemini summary	Provides a summary of the current pull request in its current state.
Comment	@gemini-code-assist	Responds in comments when explicitly tagged, both in pull request comments and review comments.
Help	/gemini help	Displays a list of available commands.

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.

Footnotes

1. Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩

[codecov]

Codecov Report

❌ Patch coverage is 83.33333% with 8 lines in your changes missing coverage. Please review.
✅ Project coverage is 89.86%. Comparing base (7cf8eaa) to head (1e06a9a).
⚠️ Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
src/adapter/mod.rs	36.36%	7 Missing ⚠️
src/adapter/hook_adapter.rs	66.66%	1 Missing ⚠️

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #178      +/-   ##
==========================================
- Coverage   89.94%   89.86%   -0.08%     
==========================================
  Files          38       38              
  Lines        7248     7212      -36     
==========================================
- Hits         6519     6481      -38     
- Misses        729      731       +2     

Flag	Coverage Δ
Linux	89.72% <83.33%> (-0.08%)	⬇️
macOS	91.59% <83.33%> (-0.08%)	⬇️

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.

[devin-ai-integration]

Devin Review found 2 new potential issues.

View 7 additional findings in Devin Review.