security: fail closed cloud eviction without identity-bound recycle - #172
Conversation
|
Warning Review limit reached
Next review available in: 51 minutes You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthrough인간 승인 기반 클라우드 소스 퇴출이 확인용 영수증 ID를 먼저 검증하도록 변경되었습니다. identity-bound recycling 기능이 없으면 지정 오류로 거부합니다. 관련 테스트는 원본 파일, 퇴출 디렉터리, 저널이 변경되지 않음을 검증합니다. Changes클라우드 소스 퇴출 fail-closed
Estimated code review effort: 3 (Moderate) | ~20 minutes Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
📝 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 |
|
@coderabbitai review |
✅ Action performedReview finished.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
src-tauri/tests/cloud_eviction_fail_closed.rs (1)
141-144: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winstaging directory 불변 조건을 두 테스트에서 검증하십시오.
현재 assertion은 eviction record와 journal만 검사합니다.
src-tauri/src/cloud_eviction.rs는 staging directory도 생성할 수 있으므로, gate 위치가 나중에 변경되면 source를 복원해도 이 테스트는 통과할 수 있습니다.
src-tauri/tests/cloud_eviction_fail_closed.rs#L141-L144:source.parent()와receipt.receipt_id에서 staging directory 경로를 구성하고, 해당 경로가 존재하지 않는지 검사하십시오.src-tauri/tests/cloud_eviction_fail_closed.rs#L177-L180: 잘못된 confirmation ID 경로에도 같은 staging directory 부재 assertion을 추가하십시오.🤖 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 `@src-tauri/tests/cloud_eviction_fail_closed.rs` around lines 141 - 144, 두 테스트에서 source.parent()와 receipt.receipt_id로 staging directory 경로를 구성하고, 해당 경로가 존재하지 않는지 검증하십시오. 앵커 위치 src-tauri/tests/cloud_eviction_fail_closed.rs#L141-L144와 sibling 위치 src-tauri/tests/cloud_eviction_fail_closed.rs#L177-L180 모두에 동일한 staging directory 부재 assertion을 추가하고, 기존 eviction record·journal 검증은 유지하십시오.
🤖 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 `@src-tauri/tests/cloud_eviction_fail_closed.rs`:
- Around line 9-11: Update prepare_cloud_copy and the cloud eviction test
integration so cloud_eviction_fail_closed.rs no longer depends on a
library-unit-test-only symbol guarded by #[cfg(all(test, not(coverage)))].
Either expose a supported public API for integration tests or change the test to
construct valid receipt and permit values using existing public APIs, while
preserving the test’s fail-closed behavior.
---
Nitpick comments:
In `@src-tauri/tests/cloud_eviction_fail_closed.rs`:
- Around line 141-144: 두 테스트에서 source.parent()와 receipt.receipt_id로 staging
directory 경로를 구성하고, 해당 경로가 존재하지 않는지 검증하십시오. 앵커 위치
src-tauri/tests/cloud_eviction_fail_closed.rs#L141-L144와 sibling 위치
src-tauri/tests/cloud_eviction_fail_closed.rs#L177-L180 모두에 동일한 staging
directory 부재 assertion을 추가하고, 기존 eviction record·journal 검증은 유지하십시오.
🪄 Autofix
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: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 867ec87f-d0d2-4442-8a27-6085990e0881
📒 Files selected for processing (2)
src-tauri/src/cloud_eviction.rssrc-tauri/tests/cloud_eviction_fail_closed.rs
|
@coderabbitai review |
|
1 participant
Security boundary
Closes the
cloud_evictionproduction manifestation of #170: the current adapter stages and revalidates the authorized source identity, then hands a pathname to the OS-trash helper. A same-user actor can still replace that pathname between the final validation and the path-consuming recycle operation. Private0700staging excludes other users but not the same UID.Test-first state
This draft intentionally starts RED with a production-path regression that requires
evict_source_with_human_approvalto fail closed before any eviction record, journal, staging, or recycle mutation when no object-identity-bound reversible recycle primitive is available. The authorized source must remain byte-identical and present.The smallest safe remedy is fail-closed production behavior until a reversible recycle primitive can bind the actual authorized filesystem object identity at the mutation boundary. Another metadata/path recheck cannot close the race; descriptor-relative permanent deletion would violate DiskSage's reversible-delete contract.
This PR is intentionally Draft until the RED boundary is observed, the narrow production fix turns it GREEN, and exact-head CI/security/review evidence is revalidated.
Summary by CodeRabbit