Commit graph

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.
2026-08-18 07:27:45 +00:00
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.
2026-08-17 09:31:41 +00:00
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.
2026-08-17 09:11:03 +00:00