fix: catch UnauthorizedAccessException when deleting a duplicate [patch] - #117
Merged
Merged
Conversation
File.Delete throws UnauthorizedAccessException -- not IOException -- when the target is read-only or the process lacks permission to unlink it. Nothing in the chain up to Program.Main caught it, so one protected duplicate ended a destructive run partway through: files already deleted stayed deleted, the remaining groups were never processed, and the user saw neither the summary nor any report of what had happened. The refusal is now recorded against that one file and the run carries on, the same way an IOException already did. Fixes #113 Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011LU3aHGPGmAeEEVUGDiTK7
Folding the two identical catch bodies into a shared helper rewrote the IOException block, which pulled its never-covered lines into SonarCloud's new-code measure and failed the quality gate at 75%. That branch has no test and predates this change, so the tidy-up is not worth dragging it in. The two bodies now repeat, the way the two catch blocks in StillMatchesGroup already do, and the production diff is purely the new UnauthorizedAccessException block -- every line of which the new test executes. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011LU3aHGPGmAeEEVUGDiTK7
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #113
What was wrong
File.DeletethrowsUnauthorizedAccessException— notIOException— when the target is read-only or the process lacks permission to unlink it.Deduplicator.DeleteDuplicatesonly caughtIOException, and nothing further up the chain (Deduplicate.Run,BaseVerb<T>.Run,Program.Main) caught it either.So one protected duplicate ended a destructive run partway through: files already deleted stayed deleted, every group after it went unprocessed, and the user saw neither the
Deleted {n} file(s).summary nor any report of what had happened — a silent, inconsistent partial deletion.The change
DeleteDuplicatesnow records the refusal against that one file and carries on, exactly as it already did forIOException. The production diff is purely additive: one newcatchblock. Its body repeats theIOExceptionbody verbatim, the way the twocatchblocks inStillMatchesGroupalready do — an earlier revision folded both into a shared helper, but that rewrote the untested, pre-existingIOExceptionbody and pulled its lines into SonarCloud's new-code coverage measure, failing the quality gate at 75% for a branch this PR did not introduce.Test
ACopyThatCannotBeDeletedIsReportedAndTheRunCarriesOnstages two duplicate groups, one holding a copy that cannot be removed, and asserts the refusal lands inDeduplicationResult.Errorswhile the other group is still deduplicated.Making a file undeletable is platform-specific, so
DeletionBlockstages it either way and restores the permissions on disposal:FileAttributes.ReadOnly.unlinkis governed by write permission on the containing directory — so the directory is what gets write-protected.A process running as root deletes through a write-protected directory regardless, so
DeletionBlockspends a throwaway probe file to measure whether the block actually holds rather than guessing from platform and uid. If it does not, the test reports inconclusive instead of passing vacuously. CI runners are unprivileged, so it runs for real there — SonarCloud's line data on the first push confirms the newcatchwas executed in CI.Verified by reverting the
catchand re-running as an unprivileged user: the test fails with theUnauthorizedAccessExceptionescapingDeleteDuplicatesatDeduplicator.cs:74. With the fix, the full suite is 27/27 green (0 skipped) unprivileged, and Release builds with 0 warnings.Not in this change
The triage on #113 suggested also adding a top-level handler in
Program.Main, and noted #112 and #114 as the other two defects in the same confirm-and-delete flow. Those are left out to keep this PR to the one defect and its regression test.🤖 Generated with Claude Code
https://claude.ai/code/session_011LU3aHGPGmAeEEVUGDiTK7