fix: encrypt files added to an encrypted archive (#253) - #266
Merged
sarensw merged 1 commit intoSep 21, 2026
Merged
Conversation
Adding a file to an encrypted zip, or to a 7z whose names are not encrypted, wrote the new file without a password. The archive kept its lock in the status bar, and the added file extracted without one. A Save updates the archive with options that carry no password, and 7-Zip copies what it keeps as it is: the old files stayed encrypted, everything added came out in plain. A Save As that keeps the format and sets no new password took the same path. A 7z with encrypted names was spared, because 7-Zip reuses the password it needed to read them. The writer now gives what an update writes anew into an encrypted archive the archive's own password, unless the save sets a new one: - The cipher follows the archive's. A ZipCrypto zip stays ZipCrypto, so the tools that read it before, unzip and Archive Utility among them, still read all of it. It is 7-Zip's own rule for a zip it updates. - The password is tried first on the entry with the least to decrypt. Nothing in a zip, or in a 7z with plain names, tells a wrong password before something is decrypted, and a typo when the archive was opened would lock the added files away under a password nobody knows. A wrong one is asked for again, the way Save As already asks. - Without one, because the prompt was dismissed when the archive was opened, the save asks. Cancelling writes nothing. - A zip password 7-Zip cannot write with, which Info-ZIP's zip allows, refuses the addition with the reason instead of storing it in plain. Removing a file from such a 7z failed for the same reason: 7-Zip packs the rest of its solid block again and had no password to read it with. A 7z removal now takes the password too. Closes #253
|
Understand this PR’s impact Explore downstream dependencies and potential security impact with Blast Radius. No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Advanced Run ID: 📒 Files selected for processing (3)
Included review availability: Your plan provides up to 4 included reviews per hour; 2 remain after this review. 📝 WalkthroughWalkthroughThe writer now reuses a source archive’s password and cipher when adding content without explicit options. It validates passwords before writing and preserves encryption during removals. Tests cover encrypted zip and 7z archives, password prompts, invalid passwords, and reader engines. ChangesEncrypted archive writes
Priority: ➖ Normal Estimated code review effort: 3 (Moderate) | ~25 minutes Change: Bug fix · Severity of issue fixed: Medium Sequence Diagram(s)sequenceDiagram
participant SaveAsState
participant SevenZipWriter
participant EncryptedSource
participant ArchiveOutput
SaveAsState->>SevenZipWriter: Save archive with pending content
SevenZipWriter->>EncryptedSource: Validate sourcePassword
EncryptedSource-->>SevenZipWriter: Return source cipher
SevenZipWriter->>ArchiveOutput: Write new and retained entries with encryption
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 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 |
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.
Adding a file to an encrypted zip, or to a 7z whose names are not encrypted, wrote the new file without a password. The archive kept its lock in the status bar, and the added file extracted without one.
What it was
A Save updates the archive with options that carry no password, and 7-Zip copies the entries it keeps as they are: the old files stayed encrypted, everything added came out in plain. A Save As that keeps the format and sets no new password took the same path. A 7z with encrypted names was spared, because 7-Zip reuses the password it needed to read them (#247).
Every save goes through
SevenZipArchive.writeArchive, whichever engine read the archive, so that is where the fix is.What changed
What an update writes anew into an encrypted archive takes the archive's own password, unless the save sets a new one:
zip -emakes — stays ZipCrypto, so the tools that read it before,unzipand Archive Utility among them, still read all of it. It is 7-Zip's own rule for a zip it updates: AES as soon as one entry uses it.ziptakes any, 7-Zip only plain ASCII — refuses the addition with the reason instead of storing it in plain.Removing a file from such a 7z failed for the same reason: 7-Zip packs the rest of the solid block again and had no password to read it with ("UpdateItems failed"). A 7z removal now takes the password too. Every 7z removal asks, although only one cutting into a solid block needs it; the
ponytail:note inSevenZipWriter.swiftsays how to narrow that.Tests
In
SaveOptionsStateTests, through the archive window:aFileAddedToALockedArchiveIsLockedToo— AES zip, ZipCrypto zip, 7z with plain names, 7z with encrypted names; Save and Save As; read by 7-Zip and by XAD. The added file is encrypted, in the archive's cipher, the password was asked for once, andunzipstill reads the ZipCrypto one.aSaveAsksForThePasswordItLacks— dismissed at opening, or wrong at opening: the save asks, a cancel changes nothing, the answer is used.aZipPasswordTooOddToWriteWithRefusesTheAddition— a Unicode ZipCrypto password.In
SaveOptionsWriteTests:EncryptedRemovalTests.aRemovalLeavesTheRestLocked— a 7z asks for the password (missing, wrong, right) and keeps the rest encrypted; a zip removes without one.Each fails without the fix. Removing the password check or the cipher line makes them fail too.
Not in here
Closes #253
Summary by CodeRabbit