fix(rules-rm): allow temp targets when cwd is home directory#42
Conversation
Consolidate home-cwd classification into analyzeRm by introducing a dedicated 'home_cwd_target' kind. Temp targets are now checked before the home-cwd guard so rm -rf /tmp/... is allowed even when cwd is HOME. Remove the duplicate pre-checks from analyzeSegment and explainSegment.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (3)
📒 Files selected for processing (5)
📝 WalkthroughWalkthroughThe PR refactors Changes
Possibly Related PRs
Estimated Code Review Effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 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 |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #42 +/- ##
==========================================
- Coverage 99.51% 99.51% -0.01%
==========================================
Files 50 50
Lines 5379 5362 -17
==========================================
- Hits 5353 5336 -17
Misses 26 26 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Greptile SummaryThis PR fixes a bug where
Confidence Score: 5/5
Important Files Changed
Flowchart%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["classifyTarget(target, ctx)"] --> B{"isDangerousRootOrHomeTarget?\n(~, $HOME, /, /*)"}
B -- yes --> C["root_or_home_target\n→ BLOCKED (extremely dangerous)"]
B -- no --> D{"isTempTarget?\n(/tmp/…, /var/tmp/…, $TMPDIR/…)"}
D -- yes --> E["temp_target\n→ ALLOWED ✓"]
D -- no --> F{"anchoredCwd present?"}
F -- no --> K["outside_anchored_cwd\n→ BLOCKED"]
F -- yes --> G{"isCwdHomeForRmPolicy?\n(cwd == HOME)"}
G -- yes --> H["home_cwd_target ★ NEW\n→ BLOCKED (change directory first)"]
G -- no --> I{"isCwdSelfTarget?\n(target resolves to cwd)"}
I -- yes --> J["cwd_self_target\n→ BLOCKED"]
I -- no --> L{"isTargetWithinCwd?"}
L -- yes --> M{"paranoid mode?"}
M -- yes --> N["within_anchored_cwd\n→ BLOCKED (paranoid)"]
M -- no --> O["within_anchored_cwd\n→ ALLOWED ✓"]
L -- no --> K
style E fill:#c8f7c5,stroke:#2ecc71
style O fill:#c8f7c5,stroke:#2ecc71
style H fill:#ffeaa7,stroke:#fdcb6e
style C fill:#fab1a0,stroke:#e17055
style J fill:#fab1a0,stroke:#e17055
style K fill:#fab1a0,stroke:#e17055
style N fill:#fab1a0,stroke:#e17055
Reviews (1): Last reviewed commit: "fix(rules-rm): allow temp targets when c..." | Re-trigger Greptile |
Summary
rm -rfclassification when cwd is the home directory to correctly allow temp targets (e.g./tmp/...,/var/tmp/...,$TMPDIR/...)analyzeSegment/explainSegmentintoclassifyTargetinrules-rm.ts, so the decision respects target classification order (temp targets checked first)Changes
src/core/rules-rm.ts: Addedhome_cwd_targetvariant toTargetClassificationunion. RefactoredclassifyTargetto checkisTempTargetbeforeisCwdHomeForRmPolicy, so temp targets are allowed even when cwd is$HOME. AddedREASON_RM_HOME_CWDconstant and correspondingreasonForClassificationcase. ExportedisHomeDirectoryfor testing.src/core/analyze/segment.ts: Removed the inlineisHomeDirectorypre-check andREASON_RM_HOME_CWDconstant; the logic is now handled insideanalyzeRmviaclassifyTarget.src/bin/explain/segment.ts: Removed the separateisHomeDirectoryexplain step; the explain flow now delegates toanalyzeRmwhich covers the home-cwd case internally.tests/core/rules-rm.test.ts: Added tests for temp targets (/tmp,/var/tmp,$TMPDIR) being allowed when cwd is home, mixed temp + relative targets being blocked, and updated therm -rf cwd itselftest to reflect that a temp-path cwd-self target is now allowed. UpdatedanalyzeRmunit test expectation from'extremely dangerous'to'home directory'.tests/bin/explain/command.test.ts: Updated explain trace assertions to expectanalyzeRmrule step instead of the removedisHomeDirectorystep. Added test for temp-targetrmin home-directory cwd being allowed.Testing
Related Issue
#41
PR Checklist
bun run checkpasses (lint, types, dead code, rules, tests)package.jsonSummary by CodeRabbit
rmcommand safety analysis to use a unified processing flow with refined context variables.