3 commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
| d4e3e9a443 |
feat(pdf): encryption on save — AES-128 and AES-256 (Phase 6, part one)
Some checks failed
nigig-build (CAD) / supply-chain (push) Has been cancelled
nigig-build (CAD) / cad-module (push) Has been cancelled
nigig-build (CAD) / full-crate-check (push) Has been cancelled
nigig-build (CAD) / cad-engine-coverage (push) Has been cancelled
nigig-build (CAD) / doc-workspace-coverage (push) Has been cancelled
repo hygiene / hygiene (push) Has been cancelled
doc-engine / engine (push) Successful in 21s
doc-engine / coverage (push) Successful in 31s
doc-engine / consumer (push) Failing after 16m57s
email / gates (push) Has been cancelled
email / email-domain (push) Has been cancelled
email / nigig-email (push) Has been cancelled
email / supply-chain (push) Has been cancelled
nigig-map / test (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
sms / gates (push) Has been cancelled
sms / robius-sms (push) Has been cancelled
sms / android (push) Has been cancelled
sms / nigig-sms (push) Has been cancelled
sms / supply-chain (push) Has been cancelled
ADR 0024. This reverses ADR 0005's "never write encryption", and the reason it is safe to reverse is that the facts changed underneath it. A crate that only reads cannot produce weak ciphertext, so refusing to write any was free. Now that Phase 4 creates documents and Phase 5 edits them, the refusal does something worse than protect nobody: open a password-protected file, change one annotation, save, and the output is plaintext. No error, no warning — the protection is silently dropped. That is this project's recurring failure mode in the one place where the consequence is a breach. The principle survives in a narrower form: no hand-rolled crypto, and no weak cipher offered as an option. RC4 stays readable because files use it and is not writable — EncryptionAlgorithm has no RC4 variant, so the refusal is a type, not a runtime check someone can route around. The encryptor is the literal inverse of the decryptor and imports its primitives rather than restating them; two implementations of one algorithm drift, and here they drift towards "decrypts to garbage". Every unit test round-trips through the existing Decryptor. Encryption sits at one choke point: PdfWriter holds the Encryptor and write_object_at encrypts everything passing through. Not per call site — there are twenty-two of those in PdfDocBuilder, and one stream written in the clear inside an encrypted document is not a partial failure, it is a leak that no reader will report because the file is otherwise valid. The /Encrypt dictionary is the single deliberate exemption: it holds the salts a reader needs before it has a key, so encrypting it bricks the file. Verified against implementations we share no code with, now gated in CI: ok qpdf opens it with the password ok it really is AES-256 ok the wrong password is refused ok poppler decrypts the content ok no plaintext in the encrypted file Four mutations, all killed — two only after the tests were strengthened, and both misses are the interesting part: A fixed IV survived two_saves_of_one_document_are_not_byte_identical, because the AES-256 file key is fresh per save and that alone makes the output differ. The property actually needed is narrower: one encryptor, identical plaintext, different bytes. In CBC a repeated IV under one key leaks that two plaintexts are equal. A wrong /Length survived because our own reader recovers by scanning for endstream — a robustness fix from ADR 0023. An independent reader that trusts /Length reads a truncated stream and decrypts garbage. A lenient reader hides a broken writer, which is why the external gate exists. The /Length test itself had a bug first: it searched a from_utf8_lossy view and reported a stream declaring 80 bytes holding 156. Ciphertext is not UTF-8; the replacement characters shifted every offset. Unencrypted output stays byte-reproducible; encrypted output cannot be, and a test asserts that loss rather than leaving it implicit. pdf: 1220 passed (was 1187). pdf-ui: green. Coverage 88.21%, encrypt_write.rs at 96.5%. Signing is NOT started. It needs the trust-anchor decision ADR 0010 deferred: VerificationStatus::Valid is unreachable by construction, and making sign -> verify pass is a policy change, not an implementation detail. The plan's Phase 6 status now says so. |
|||
| 1220f89fc6 |
feat(pdf): close the last three Phase 4 items — reconciliation, CFF, cmap
Some checks failed
email.yml / feat(pdf): close the last three Phase 4 items — reconciliation, CFF, cmap (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
The three items the previous commit's audit found unimplemented while the status line said "complete". All three are done and externally verified. **1. Field-value reconciliation** (`reconcile.rs`). A field carries its value in /V and its rendered look in /AP, and nothing in the format keeps them in step. Files arrive with them disagreeing all the time: a producer writes /V and leaves appearances to the viewer, or something edits /V without touching /AP. Until now this crate simply believed /V and regenerated appearances only for fields it had itself edited — right for a field we changed, wrong for a field that arrived inconsistent. The module deliberately does **not** pick a winner. PDF 32000-1 §12.7.3.3 settles exactly one case — /NeedAppearances true means /V is authoritative — and is silent on the other, where a conforming viewer renders /AP and never looks at /V. So it classifies the disagreement and resolves it against a caller-declared `Intent`, because the right answer genuinely differs: a viewer must show /AP to match other viewers, an extractor must read /V, an editor must regenerate so the saved file agrees with itself. Silently choosing one would be ADR 0017's failure in a new place — every answer plausible, none checkable, the caller unaware a decision was made for it. Two cases are not judgement calls and are handled outright. A missing or dangling appearance renders *blank*, and blank is never what the producer meant, so even Display regenerates. An unselected radio member showing /Off while the group's /V names another member is correct, not a conflict — reporting it would flag every well-built radio group there is. **2. Type1/CFF embedding** (`embed_opentype_whole`). The spec says "if feasible". Subsetting CFF is not — it means rebuilding the CFF INDEX, charset and charstrings, a second font format inside the first — and `subset_truetype` rightly keeps refusing it by name. Embedding the program *whole* is feasible, and that is what this does: /FontFile3 with /Subtype /OpenType under a CIDFontType0 descendant, per Table 126. Each of those keys matters and none is guessable from the others. A CFF program in /FontFile2, or under a CIDFontType2 descendant, still produces a file qpdf accepts and a font that loads as the wrong type or not at all. /CIDToGIDMap is omitted because it is defined for CIDFontType2 only. The trade is made visible rather than buried: `EmbeddedFont::is_subsetted` is false here, so a caller with a size budget — or a licence that forbids shipping a whole face — can refuse instead of discovering it from the output size. **3. `repair-cmap`** (`glyph_index`). A symbol font declares no Unicode subtable: it maps glyphs into the private-use area at 0xF000 + the low byte under platform 3, encoding 0. Asking it for 'A' found nothing and the character silently vanished from the output — the font "missing" a glyph it plainly has. Now the (3,0) subtable is kept as a fallback and retried at 0xF000 + low byte, after the proper lookup fails so a font with both subtables is still read through the Unicode one. Format 0 is read too; omitting it left legacy and symbol fonts mapping nothing while appearing to have a usable cmap. The repair must not manufacture glyphs, which is its own test: a character the font genuinely lacks still returns None, because turning a missing character into a wrong one is worse. **Fixtures.** No CFF or symbol font ships on the CI image, and neither can be tested honestly against a hand-built stub — the point is that the bytes are a font program a third-party reader accepts. Both are generated from DejaVu by checked-in fontTools scripts: `cff_sample.otf` (1.6 KB, real OTTO/CFF outlines) and `symbol_sample.ttf` (664 B, a single (3,0) subtable so the repair path is the only route to its glyphs). Both generators pin `head.created`/`head.modified` to zero. fontTools stamps the current time, so the output differed on every run and CI's "fixtures match their generator" check failed against a file nobody had edited. Caught by running that check rather than assuming it passed. A fixture that cannot be regenerated byte-for-byte is not reviewable: you cannot tell a deliberate change from a rebuild. **Verified by mutation**, seven injected defects, each confirmed red: NeedAppearances ignored 1 fail dangling /AS not detected 1 fail blank rendering shown faithfully 1 fail CFF written to /FontFile2 1 fail CFF given a CIDFontType2 descendant 1 fail whole font claims to be subset 1 fail cmap 0xF000 retry removed 3 fail **Verified externally.** The sample now carries a third page set in the whole-embedded CFF font, and `check-pdf-external-readers.sh` gained `pdffonts` — the only check that inspects a font *program* rather than the file structure, which is exactly where a wrong /FontFile key shows up. poppler reports both fonts embedded and distinguishes them correctly: ETXLDI+DejaVuSans CID TrueType Identity-H emb yes sub yes NigigTestCFF CID Type 0C (OT) Identity-H emb yes sub no and extracts "Hello CFF 123", which only works if the CFF program loaded, /Identity-H addressed its glyphs and /ToUnicode mapped them back. That check also caught its own page-count assertion going stale when the third page landed — a gate that notices its own fixture changing is working. Engine suite 953 -> 985. Coverage 87.27%, all floors met. Phase 4 is complete but for the ui.rs interaction tests, which are written and blocked on the Makepad fork's missing headless backend. |
|||
| 89ca5186c6 |
docs(pdf): Phase 4 is not 100% — audit it, and verify the half that is
Some checks failed
email.yml / docs(pdf): Phase 4 is not 100% — audit it, and verify the half that is (push) Failing after 0s
repo hygiene / hygiene (push) Has been cancelled
PDF engine / engine (push) Has been cancelled
PDF engine / makepad-integration (push) Has been cancelled
PDF engine / fuzz (push) Has been cancelled
Asked whether Phase 4 was complete, I checked the tree instead of my own
commit message, and the commit message was wrong.
Three items named in the Phase 4 spec are **not** implemented, and the
status line said "complete" over them:
- **Field-value reconciliation** (`form_reconcile_test.dart`). Setting a
value writes /V, marks the field dirty and regenerates /AP — that all
works. What is missing is the reconciliation case: a file opened with
/V and /AP *already disagreeing*, where the right answer depends on
/NeedAppearances. Nothing decides that today.
- **Type1/CFF embedding.** The spec hedges with "if feasible", so this is
a legitimate deferral rather than an oversight — but "complete" did not
say so. `sfnt.rs` detects CFF outlines and `font.rs` reads an existing
/FontFile3; nothing writes one. Creation is TrueType-only.
- **`repair-cmap`.** No equivalent exists.
`text_box_appearance_test.dart` *is* covered, by appearance.rs:235 — it
just does not carry that filename, which is why a grep for the dart test
names is a starting point and not an answer.
The other half of the exit criterion — "generated PDFs open cleanly in
external viewers" — had never been checked at all. The sample generator's
own doc comment admits no test in this repository can assert it. So I
ran it through implementations we share no code with, and **it passes**:
qpdf --check no syntax or stream encoding errors
pdfinfo title, author, subject, keywords, 2 pages,
Form: AcroForm
pdftotext all text, including the embedded DejaVu subset
and its em-dash
qpdf --list-attachments readme.txt, extracted by name with description
catalogue /Outlines /Names /EmbeddedFiles /PageLabels
/Dests /PageMode /ViewerPreferences /AcroForm
`tools/check-pdf-external-readers.sh` makes that repeatable, and pdf.yml
runs it. It treats a qpdf *warning* as failure, not just an error: qpdf
warns where it had to reconstruct, and reconstructing is exactly what a
stricter viewer will refuse to do. Negative-tested twice — removing the
attachment fails 3 checks, and corrupting the startxref offset makes
qpdf report "file is damaged".
Two defects that audit found:
- **The sample never exercised XMP**, so the Phase 4 feature most likely
to be silently missing was also the one nothing looked at. Probed
separately: `set_xmp_metadata` works, pdfinfo reports
`Metadata Stream: yes`.
- **A `Banner` naming an unregistered font produces a structurally valid
PDF that renders no text.** qpdf --check passes; poppler says
`Unknown font tag 'F1'` and draws nothing. `stamp.rs` cannot register
the font itself — fonts belong to the document, and a banner does not
know which document it will be drawn into — so this is now documented
on `Banner` with a worked example, and pinned by
`a_banner_font_must_be_registered_or_the_page_lacks_the_resource`,
which asserts on the page's /Font resources because that is the thing
actually missing and the thing a caller can check.
The plan now records that it was wrong once, rather than quietly
correcting itself. A status line that has been overstated should show its
working.
Engine suite 952 -> 953. Phase 4's engine half is verified end to end
against third-party readers; the ui.rs interaction half is written and
still blocked on the Makepad headless backend.
|