fix(security): shredFile must not report a locked/unwritable plaintext copy as shredded#262
Merged
Merged
Conversation
…t copy as shredded Codex stop-time review: the shredFile rewrite always returned true except for ENOENT, so a file that EXISTS but where both the truncate (openSync r+ throws EACCES/EBUSY, or ftruncate fails) AND the unlink fail — a locked, read-only or otherwise unwritable plaintext copy, or a directory — was reported as shredded even though it is still fully recoverable on disk. The migration then lists it in `shredded[]`, so the user wrongly believes a plaintext key copy is destroyed. Fix: track whether the plaintext was ACTUALLY made unrecoverable and return that — true only when the bytes were truncated OR the file was removed; false when the target stays intact (both arms failed). "Existed" is not "shredded". Tests: the directory case (deterministic cross-platform "both arms fail": EISDIR on open + EISDIR/EPERM on unlink — same code path as a locked file) now asserts false; added a positive test that a real file is truncated+removed -> true. keystore 32/32, full renderer vitest 100% coverage, tsc + biome clean. Claude-Session: https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL
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.
Codex stop-time review found that the v1.4.1 keystore shredFile always returned true (except ENOENT), so a locked/read-only/unwritable plaintext copy still fully intact on disk was reported as shredded and listed in the migration's \shredded[]\ - the user wrongly believes the plaintext is destroyed. Fix returns true only when the bytes were truncated OR the file removed; false when the target stays intact. Lands before v1.4.1 is built/published. keystore 32/32, vitest 100%, tsc+biome clean.
https://claude.ai/code/session_01CaUSwiidebWvpgMs2wQgqL