Skip to content

fix(security): --yes no longer bypasses bash permissions#30

Merged
kienbui1995 merged 1 commit intomainfrom
fix/security-yes-flag
Apr 11, 2026
Merged

fix(security): --yes no longer bypasses bash permissions#30
kienbui1995 merged 1 commit intomainfrom
fix/security-yes-flag

Conversation

@kienbui1995
Copy link
Copy Markdown
Owner

@kienbui1995 kienbui1995 commented Apr 11, 2026

Security Fix

--yes previously allowed ALL tools including bash. LLM could run arbitrary commands in CI/CD.

Now:

  • --yes — approves file tools, bash still prompts
  • --yes --dangerously-allow-bash — approves everything (explicit opt-in)

180 tests pass.

Summary by CodeRabbit

  • New Features

    • Added --dangerously-allow-bash CLI flag: keeps bash tools in prompt mode by default even with --yes; set this flag to enable bash execution alongside --yes.
  • Tests

    • Internal test files reorganized and reformatted (no behavioral changes).

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Apr 11, 2026

Caution

Review failed

Pull request was closed or merged during review

📝 Walkthrough

Walkthrough

Added a new CLI boolean flag --dangerously-allow-bash and adjusted --yes behavior so full permission policy is applied globally except the bash tool remains in Prompt mode unless the dangerous flag is also set. Many files had test/function indentation and formatting fixes with no behavioral changes.

Changes

Cohort / File(s) Summary
CLI Permission Policy
mc/crates/mc-cli/src/main.rs
Added dangerously_allow_bash: bool to Cli and updated main() to apply PermissionPolicy::Allow globally when --yes is set but keep bash at Prompt unless --dangerously-allow-bash is also provided. Minor output formatting tweaks.
Core tests — reindent / scope fixes
mc/crates/mc-core/src/compact.rs, .../context_resolver.rs, .../cost.rs, .../cron.rs, .../memory.rs, .../model_registry.rs, .../session.rs, .../skills.rs, .../usage.rs
Moved or de-indented many #[test] functions into correct test-module scope and adjusted formatting; no test logic or assertions changed. Some changes may affect test compilation scope (ensure imports/use paths remain valid).
Core formatting / refactor-only
mc/crates/mc-core/src/parallel_tools.rs, .../runtime.rs, .../subagent.rs, .../token_budget.rs
Non-functional reformatting of match/macro/long expressions and minor refactor of multi-line matches! usage; behavior unchanged.
Provider / TUI message variants formatting
mc/crates/mc-provider/src/types.rs, mc/crates/mc-tui/src/app.rs, mc/crates/mc-tui/src/commands.rs
Reformatted enum/variant and iterator formatting into multi-line layouts; no signature or behavior changes.
Tools plugin formatting
mc/crates/mc-tools/src/plugin.rs
Reformatted test fixture creation and execute_plugin call across multiple lines; no functional change.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Possibly related PRs

Poem

🐰 I nibbled flags beneath the moonlight,
I hopped through tests all tidy and neat,
A cautious bash now waits for a bite,
Two flags together make the choice complete.

🚥 Pre-merge checks | ✅ 3
✅ Passed checks (3 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title directly and specifically describes the main security fix: preventing --yes from bypassing bash permissions, which is the primary objective of this PR.
Docstring Coverage ✅ Passed Docstring coverage is 93.33% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/security-yes-flag

Comment @coderabbitai help to get the list of available commands and usage tips.

--yes now allows file tools but keeps bash gated.
Use --dangerously-allow-bash to explicitly opt in.
Prevents LLM from running arbitrary commands in CI/CD.
@kienbui1995 kienbui1995 force-pushed the fix/security-yes-flag branch from 355816f to c192221 Compare April 11, 2026 19:46
@kienbui1995 kienbui1995 merged commit 0cf6ae1 into main Apr 11, 2026
4 of 5 checks passed
@kienbui1995 kienbui1995 deleted the fix/security-yes-flag branch April 11, 2026 19:47
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