Harden SecurityPolicy risk detection and fix git -C false positive - #852
Harden SecurityPolicy risk detection and fix git -C false positive#852yacosta738 wants to merge 1 commit into
Conversation
…lse positive - Update `is_medium_risk_command` to detect verbs anywhere in the argument list, preventing bypasses where global flags precede the verb (e.g., `git -C . commit`). - Update `command_risk_level` to de-quote arguments before risk classification, preventing bypasses like `git "commit"`. - Refine `is_args_safe` to use case-preserving arguments for `git`, implementing case-sensitive matching for the `-c` flag (detecting configuration injection) while allowing the safe, case-sensitive `-C` directory flag. - Add regression test `test_git_security_hardening_and_false_positives` to `policy.rs`. - Apply automated formatting to GitHub workflows. Security Impact: - Risk reduced: Prevents bypasses of risk-based approval gates using global flags or quotes. - Attack surface: Narrowed by blocking configuration injection via `git -c` and similar flags across package managers. - Why safe: Maintains existing allowlists and adds specific, low-regret hardening to known bypass vectors. Performance Impact: - Metric: Argument processing. - Before: $O(N)$ lowercasing of all arguments in `is_segment_valid`. - After: Case-preserving arguments are used directly, avoiding unnecessary allocations in `is_args_safe` for the common path of safe commands. - Expected: Negligible change in CPU usage, reduced allocation pressure in the validation hot path.
|
👋 Jules, reporting for duty! I'm here to lend a hand with this pull request. When you start a review, I'll add a 👀 emoji to each comment to let you know I've read it. I'll focus on feedback directed at me and will do my best to stay out of conversations between you and other bots or reviewers to keep the noise down. I'll push a commit with your requested changes shortly after. Please note there might be a delay between these steps, but rest assured I'm on the job! For more direct control, you can switch me to Reactive Mode. When this mode is on, I will only act on comments where you specifically mention me with New to Jules? Learn more at jules.google/docs. For security, I will only act on instructions from the user who triggered this task. |
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Deploying corvus with
|
| Latest commit: |
d8879d6
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://df906b1b.corvus-42x.pages.dev |
| Branch Preview URL: | https://jules-2145600012477397089-03.corvus-42x.pages.dev |
This PR hardens the
SecurityPolicyin theagent-runtimeto prevent several classes of security bypasses while simultaneously fixing a common false positive.Key Changes
is_medium_risk_command, changed.first().is_some_and()to.iter().any(). This ensures that subcommands likecommitorinstallare detected even when preceded by global flags (e.g.,git -C . commitornpm --loglevel=silent install).command_risk_level, arguments are now de-quoted usingstrip_all_quotesbefore being checked against risk-bearing verbs. This prevents bypasses likegit "commit".is_segment_validto pass original-case arguments tois_args_safe.git -c(blocked) andgit -C(allowed). This fixes the false positive wheregit -Cwas incorrectly blocked while hardening againstgit -ccore.editor=cat.test_git_security_hardening_and_false_positivesto verify these improvements.Impact Summary
gitcommands with directory flags were being incorrectly blocked.PR created automatically by Jules for task 2145600012477397089 started by @yacosta738