security: fail generic cleanup closed without object-bound recycle - #174
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughGeneric cleanup IPC 경로를 fail-closed 구현으로 전환했습니다. 모든 경로는 파일시스템 변경 없이 동일한 오류를 반환합니다. Tauri 명령 연결과 기존 개발 artifact 정리 경로를 검증하는 테스트를 추가했습니다. ChangesGeneric cleanup fail-closed 경로
Estimated code review effort: 3 (보통) | ~20분 Merge Risk: ⚪ Minimal · up to The localized fail-closed cleanup change has no actionable merge-blocking risk in the supplied evidence. An outdated build-version statement should be removed, but it does not affect runtime behavior. 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning Some tools did not complete. Review the errors below. 🔧 Clippy (1.97.1)Clippy execution failed 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 |
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current headd29f3fc37fe3ebcf11f90dd2e66ec929e87a847c. -
Head SHA:
d29f3fc37fe3ebcf11f90dd2e66ec929e87a847c -
Workflow run: 31545716676
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
OpenCode Review Overview
Pull request overviewOpenCode cannot approve yet because required coverage evidence did not pass. Review outcome1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
Coverage evidenceCoverage Decision
Changed-File Evidence Mapflowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
|
There was a problem hiding this comment.
Pull request overview
OpenCode cannot approve yet because required coverage evidence did not pass.
Review outcome
1. HIGH .github/workflows/opencode-review.yml:1 - Coverage evidence did not prove required test/docstring evidence
-
Problem: The required coverage-evidence job result was
failure, so OpenCode cannot establish approval sufficiency for this head. -
Root cause: Automated approval is only valid when the same-head coverage-evidence job proves supported repository test suites passed and configured docstring gates passed or were advisory, or reports not applicable because no supported source files or package manifests exist. Missing, failed, skipped, unavailable, or unsupported-tooling test evidence is a blocker.
-
Fix: Install or configure the repository test/docstring evidence tooling when source files or package manifests exist, rerun the current-head coverage-evidence job, and approve only after it reports
successwith required evidence or explicit no-source not-applicable evidence. -
Regression test: Keep the approval branch checking
needs.coverage-evidence.result == successbefore posting APPROVE, and publish REQUEST_CHANGES when coverage-evidence blocker states such as cancelled, skipped, failed, unsupported-tooling, or below-100 evidence are present. -
Result: REQUEST_CHANGES
-
Reason: coverage-evidence result was
failure, so required test/docstring evidence was not proven for current head8d88051b55845418e3d5c67288766903f770610c. -
Head SHA:
8d88051b55845418e3d5c67288766903f770610c -
Workflow run: 31876345461
-
Workflow attempt: 1
Coverage evidence
Coverage Decision
- Result: FAIL
- Test evidence: not proven passing
- Docstring evidence: not proven passing when configured
- Failure count: 1
Changed-File Evidence Map
flowchart LR
PR["PR changed files"] --> Evidence["OpenCode bounded evidence"]
Evidence --> S1["Changed file (3 files)"]
S1 --> I1["repository behavior"]
I1 --> R1["Review risk: Changed file (3 files)"]
R1 --> V1["required checks"]
Dismissing: this REQUEST_CHANGES referenced workflow run 31876345461 where coverage-evidence failed. At the same head SHA, coverage-evidence has since been re-run and now reports pass (verified via gh pr checks). No unresolved review threads remain. Re-request review to re-validate.
Purpose
Bounded security repair for #170. Generic desktop cleanup previously registered
commands::clean_paths, whose core ultimately passed a pathname tosafety::trash_delete. Path revalidation alone cannot preserve filesystem-object identity through a pathname-consuming OS recycle primitive.Exact current state
8d88051b55845418e3d5c67288766903f770610c.main:be418bac9bfbdc8aa41b1a698713833e3dbccfdc.main -> headcomparison isahead,ahead_by = 16,behind_by = 0, with merge base exactly equal to live protected main.src-tauri/src/generic_cleanup.rs,src-tauri/src/lib.rs, andsrc-tauri/tests/generic_cleanup_mutation_authority.rs.31585043488, Release31585043524, Security Scan31585043494, and SAST Semgrep31585043691all completed successfully.CHANGES_REQUESTEDreview anchored to predecessor headd29f3fc37fe3ebcf11f90dd2e66ec929e87a847c; its stated blocker was failed coverage evidence rather than a source defect on this head, and predecessor evidence does not transfer.31584223427and dynamic Code Quality run31584222543succeeded on current main, proving the latest integrated Homebrew-cleanup base is operationally green before this delta.Test-first repair
79c6e4e04198e76cf7e69c78163d4b46b9282d67required the registered Tauriclean_pathsroute to leave the pathname-consuming legacy command and move behind a dedicated fail-closed authority.src-tauri/src/generic_cleanup.rsreturns stablegeneric-cleanup-identity-bound-recycle-unavailablerefusals and performs no filesystem mutation.clean_paths; the Rust handler is nowfail_closed_clean_pathswhile the external IPC name remainsclean_pathsthrough#[tauri::command(rename = "clean_paths")].clean_dev_artifactsIPC route. The narrow fix restored onlycommands::clean_dev_artifacts; its production path continues to use object-identity-bound staging/recycle semantics while genericclean_pathsstays fail closed.src-tauri/src/lib.rstherefore registersgeneric_cleanup::fail_closed_clean_pathsandcommands::clean_dev_artifacts, and never registerscommands::clean_paths.Scope / non-duplication
Cache cleanup and Maven apply-mode already fail closed on protected main. Protected-main cloud-source eviction also fails closed before its historical pathname-consuming path. #171 is protected-main history for developer-artifact object-identity staging and Git-worktree mutation. #156 separately owns repository-wide exact-production-coverage evidence and is not imported here. #176 and #185 are protected-main history and are deliberately preserved by the current-base convergence.
Required before merge
Keep Draft until the unchanged exact head satisfies every live required organization workflow, repository-wide exact 100% owned-production coverage without exclusions or threshold weakening, zero valid current-head findings, the qualifying independent approval/latest-push requirement actually imposed by live governance, and a fresh live-base/ancestry check. Pending, queued, skipped-required, stale-head, predecessor, synthetic, diagnostic-only, status-only, model-only, author-only, or rate-limited evidence is not passing.
Summary by CodeRabbit
버그 수정
테스트