fix(document): refuse to save a package we only hold decrypted - #813
Merged
Conversation
andiwand
force-pushed
the
fix/no-plaintext-save-of-a-decrypted-package
branch
from
September 3, 2026 19:16
a718908 to
e8c1abc
Compare
`is_savable(false)` was true for a document decrypted from a password-protected package, and odf's `save` stripped every `manifest:encryption-data` on the way out. Opening a protected file with its password and saving it therefore produced a plaintext package, silently: the content left the protection its author asked for, and nothing said so. `.docx` had the same shape, rebuilding the zip from the decrypted filesystem. A document now carries the `EncryptionState` its file was decoded at, and the two savable engines refuse one that was decrypted. Every public `save` overload consults `is_savable` before it writes — the stream and memory ones did not — so the refusal reaches all of them. The manifest rewrite goes with it: nothing else ever had encryption data to strip. Re-encrypting on save is #64 and unaffected; this only stops the hole in the meantime. Towards #64. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01UDi21mwuKRGvGiFcGwETtS
andiwand
force-pushed
the
fix/no-plaintext-save-of-a-decrypted-package
branch
from
September 3, 2026 19:17
e8c1abc to
7fbed17
Compare
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
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.
🤖 Generated with Claude Code
Towards #64.
The hole
odf::Document::is_savablereturned!encrypted, which is true foris_savable(false)whatever the document was decoded from — a decrypted package included. Andsavewalked the decrypted filesystem, stripping everymanifest:encryption-datanode on the way out (odf_document.cpp, under a bare// TODO).So: open a password-protected ODF with its password, save it, and you get a plaintext package. No error, no warning, no flag on the resulting file. The content leaves the protection its author asked for, and the only trace is that it opens without a password afterwards.
.docxhad the same shape —ooxml/text'ssaverebuilds the zip from the decrypted filesystem.test/src/document_test.cpp::edit_ods_diffwas quietly relying on exactly this: it decryptedpages.ods, edited it, saved, and reopened the result without a password.The change
internal::Documentcarries theEncryptionStateits file was decoded at — the two file classes already track it — and exposesis_decrypted(). The two engines that can save consult it:saveguards on it as well, not justis_savable.Document::save(path)checksis_savableat the public boundary, butsave(std::ostream &)andsave_to_memory()do not, so the guard has to sit in the engine to close all three. Every other engine inherits the base'sis_savable == falseand is unaffected.The manifest rewrite goes away with it. It existed only to make this save "work"; a package that was never encrypted has no
manifest:encryption-datato strip, so the branch was dead the moment the decrypted case became an error — and the manifest is now copied through like any other part instead of being re-serialised.This does not implement encrypted save (#64). It stops the silent downgrade until that lands.
Breaking
A caller that opened a protected document and saved it now gets
UnsupportedOperationwhere it used to get a plaintext file. That is the point, and the changelog says so. No signature changes.Verified
edit_ods_diffnow applies its diff in memory and asserts there; the round trip through a file is already covered by the odt and docx cases. NewDocument.a_decrypted_package_is_not_savablepins all three save overloads plusis_savable, and that no file is left behind.Full suite: 1407 passed, 6 pre-existing skips. No rendering code is touched, so the reference output is unchanged and no pin moves. Changed TUs re-checked under
-Werror.