t304: Fix rm -rf on potentially empty variable in setup.sh#1187
t304: Fix rm -rf on potentially empty variable in setup.sh#1187marcusquinn merged 4 commits intomainfrom
Conversation
|
Warning Rate limit exceeded
⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ✨ 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 |
|
Note Gemini is unable to generate a summary for this pull request due to the file types involved not being currently supported. |
- Add validate_plugin_namespace() function to reject: - Empty namespaces - Absolute paths (starting with /) - Path traversal attempts (..) - Paths with slashes (must be single directory) - Non-alphanumeric characters (except - _ .) - Validate namespaces when collecting plugin_namespaces array - Validate disabled_ns before rm -rf in cleanup loop - Validate pns before using in clone_dir path - Addresses CodeRabbit security finding from PR#762
- All ShellCheck validations pass - Manual testing confirms validation function correctly: - Accepts: alphanumeric, hyphens, underscores, dots - Rejects: empty, absolute paths, path traversal, slashes, special chars - 13/13 test cases pass
Protect against potential empty variable expansion in rm -rf commands:
- Use ${var:?} expansion for old_backup_dir, legacy_state_dir, tmp_preserve
- Add [[ -n "$var" ]] check for old_backup in while loop
- Add agents_dir validation in cleanup_deprecated_paths
Addresses CodeRabbit security finding from PR#762.
Add validation for remaining rm -rf instances: - Check path is non-empty in deprecated_paths cleanup loop - Validate venv_dir before cleanup in error handler Ensures comprehensive protection against empty variable expansion.
marcusquinn
force-pushed
the
feature/t304
branch
from
b2c9e9a to
a33295e
Compare
🔍 Code Quality Report�[0;35m[MONITOR]�[0m Code Review Monitoring Report �[0;34m[INFO]�[0m Latest Quality Status: �[0;34m[INFO]�[0m Recent monitoring activity: 📈 Current Quality Metrics
Generated on: Thu Feb 12 02:40:35 UTC 2026 Generated by AI DevOps Framework Code Review Monitoring |
|
Add escalating conflict resolution to rebase_sibling_pr(): 1. Plain rebase (no conflicts) 2. Rebase with -Xtheirs (feature branch wins on conflicts) 3. AI CLI resolution for complex cases (Strategy 3 fallback) Also adds: - resolve_rebase_conflicts() for AI-assisted per-file resolution - Stale worktree cleanup (abort stuck rebases, fix detached HEAD) - Temp worktree creation when no worktree exists (avoids dirty tree) - CONFLICTING added to t298 handler (was only BEHIND/DIRTY) - Phase 7b: periodic retry of merge-conflict-blocked tasks (30min) Tested: resolved 4 blocked PRs (#1171, #1187, #1188, #1191) that were stuck with merge conflicts — all resolved with -Xtheirs.
…1203) Add escalating conflict resolution to rebase_sibling_pr(): 1. Plain rebase (no conflicts) 2. Rebase with -Xtheirs (feature branch wins on conflicts) 3. AI CLI resolution for complex cases (Strategy 3 fallback) Also adds: - resolve_rebase_conflicts() for AI-assisted per-file resolution - Stale worktree cleanup (abort stuck rebases, fix detached HEAD) - Temp worktree creation when no worktree exists (avoids dirty tree) - CONFLICTING added to t298 handler (was only BEHIND/DIRTY) - Phase 7b: periodic retry of merge-conflict-blocked tasks (30min) Tested: resolved 4 blocked PRs (#1171, #1187, #1188, #1191) that were stuck with merge conflicts — all resolved with -Xtheirs.
1 participant
Summary
Addresses CodeRabbit security finding from PR#762 regarding
rm -rfcommands on potentially empty variables.Changes
${var:?}parameter expansion for critical variables (old_backup_dir, legacy_state_dir, tmp_preserve)[[ -n "$var" ]]validation checks for loop variables (old_backup, path, venv_dir)Safety Improvements
All
rm -rfcommands with variables in setup.sh now have protection against empty variable expansion::?expansion causes immediate exit if variable is unset/emptyTesting
Protected Instances
old_backup- protected with[[ -n "$old_backup" ]]path- protected with[[ -e "$path" && -n "$path" ]]old_backup_dir- protected with${old_backup_dir:?}legacy_state_dir- protected with${legacy_state_dir:?}tmp_preserve- protected with${tmp_preserve:?}venv_dir- protected with[[ -n "$venv_dir" ]]