Conversation
Replace the boolean push_touches_trust_root/touches_trust_root plumbing with a TrustRootTouch enum (Clean/Touched(Vec<String>)/Unknown) so the shim's deny message names exactly which trust-root path(s) matched instead of listing every known pattern. An unreadable diff now fails closed to Unknown (still denies) with a distinct message, rather than silently defaulting to Clean. Agentflare-Agent: claude-code_2-1-218_agent Agentflare-Branch: task/320 Agentflare-Item: 320
📝 WalkthroughWalkthrough
ChangesTrust-root classification
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/flare-git-core/src/classify.rs`:
- Around line 363-369: Replace the single-branch use of pushed_branch() in the
classification flow with parsing of every pushed source/destination refspec
pair. Use destinations for targets_default_branch protection checks and inspect
all sources for trust-root changes, including --all and --mirror; if any push
mode or refspec cannot be resolved, fail closed. Add end-to-end coverage for
feature:master, master:feature, and multi-ref pushes.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d900c56e-7b96-45e6-9f78-c05245a87d12
📒 Files selected for processing (1)
crates/flare-git-core/src/classify.rs
| let trust_root_touch = pushed | ||
| .as_deref() | ||
| .is_some_and(|b| push_touches_trust_root(repo_root, b, &default_branch)); | ||
| .map(|b| resolve_trust_root_touch(repo_root, b, &default_branch)) | ||
| .unwrap_or(TrustRootTouch::Clean); | ||
| let targets_default_branch = pushed | ||
| .as_deref() | ||
| .is_some_and(|b| is_protected_branch(b, Some(&default_branch))); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift
Parse push sources and destinations separately.
pushed_branch() returns the refspec source, so git push origin feature/x:master sets targets_default_branch to false and bypasses the default-branch denial; master:feature is wrongly denied. --all/--mirror also inspect only one branch, so trust-root changes on other pushed refs can evade classification. Model every pushed source/destination pair, check protection on destinations and trust-root changes on all sources; fail closed when a push mode cannot be resolved. Add end-to-end regressions for feature:master, master:feature, and multi-ref pushes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@crates/flare-git-core/src/classify.rs` around lines 363 - 369, Replace the
single-branch use of pushed_branch() in the classification flow with parsing of
every pushed source/destination refspec pair. Use destinations for
targets_default_branch protection checks and inspect all sources for trust-root
changes, including --all and --mirror; if any push mode or refspec cannot be
resolved, fail closed. Add end-to-end coverage for feature:master,
master:feature, and multi-ref pushes.
Auto-opened on
item donefor I6vs4hnmCa4pD0oVTiNq1.Summary by CodeRabbit