andodeki
ad3fe19b90
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
Ten PDF feature commits landed without an ADR, covering three whole phases.
Every feature gets one; these are the four that were owed. Written against
the code as it stands and re-verified by running it, not transcribed from
the commit messages.
0020 CCITT, JBIG2 and JPEG 2000 — the codecs ADR 0015 refused by name
0021 Phase 4 completion — stamping, reconciliation, CFF, cmap, and the
audit that corrected a false "complete" in ADR 0019
0022 Editing — content_edit, page_ops, flatten, catalog_edit
0023 Redaction and compaction, and the three reader defects they found
Verified rather than assumed, on the tree at f75c1cc:
ccitt 30, jbig2 29, jpx 44 unit tests; jpx_roundtrip's 7 compare against
OpenJPEG-generated pixels, exactly, because a JPEG 2000 bug produces a
plausible image rather than an error.
Redaction re-checked with a test written from outside the module. The
middle line is the one worth keeping:
after redact, secret present = true <- earlier revision
after compact, secret present = false
public text retained = true
Redaction alone leaves the secret in the bytes. That is why the two
shipped together, and why the report carries
earlier_revisions_retain_content.
Determinism re-checked by generating the same document from four separate
processes: byte-identical. The HashMap key-order defect really is fixed.
This ADR contains a correction to its own first draft. 0022 initially
listed text-run rewriting and annotation sub-types as deferred, on the
strength of a grep for "rewrite" finding nothing. Both are implemented —
the rewriter is ContentEditor::set_text, which preserves the operator kind
so a ' keeps its line advance and a TJ keeps its kerning; and "callout" and
"comment" are dart-pdf's names for a FreeText with a /CL and a Text
annotation, not distinct /Subtype values. Checking the plan's claim against
the code was right; concluding from a failed grep was not.
One criterion across the four is left unticked, and it is real: the ui.rs
interaction tests, blocked on the Makepad headless backend since Phase 1.
0021 also records a process note. ADR 0019's merge criteria were derived
from what had been built, so all of them ticked while three unbuilt items
stayed invisible. Criteria should come from the plan text first, then be
marked done or explicitly deferred — a deferral stated is fine, a deferral
unstated is a false claim.
pdf: 1187 passed. No code changed.
181 lines
8.1 KiB
Markdown
181 lines
8.1 KiB
Markdown
# ADR 0023: redaction and compaction — removing content, and removing the revision that still holds it
|
|
|
|
- **Status:** Accepted
|
|
- **Date:** 2026-08-17
|
|
- **Review item:** `NIGIG_PDF_FEATURE_PARITY_PLAN.md` §1 Phase 5,
|
|
"Redaction: content analysis, redaction annotations, content removal" and
|
|
"Object compaction for incremental save"
|
|
- **Supersedes:** nothing
|
|
- **Related:** ADR 0022 (the rest of Phase 5), ADR 0008 (incremental save —
|
|
what compaction is the counterpart to)
|
|
- **Written retrospectively.** The work landed in `41c43df` without an ADR.
|
|
|
|
## Context
|
|
|
|
Redaction is the feature where being *nearly* right is a data breach. The
|
|
canonical failure is drawing a black rectangle over text and shipping it:
|
|
the rectangle is decoration, the text is still in the content stream, and
|
|
`pdftotext` prints it. This has leaked court filings, medical records and
|
|
intelligence documents, repeatedly.
|
|
|
|
The second-order failure is subtler and just as bad. Remove the text
|
|
properly, save incrementally, and the original text is **still in the file**
|
|
— in the previous revision, which an incremental save preserves by design.
|
|
The document looks redacted in every viewer and the secret is one `strings`
|
|
away.
|
|
|
|
That is why these two shipped together: redaction removes the content, and
|
|
compaction removes the revision that still holds it. Either alone is a
|
|
false promise.
|
|
|
|
## Decision
|
|
|
|
### Redaction removes operators; it draws nothing (`redact.rs`, 761 lines, 18 tests)
|
|
|
|
`redact_page(doc, source, page_index, rects)` removes the text-showing
|
|
operators whose position falls inside a rectangle. It paints no black
|
|
boxes. The test that matters is not that something was drawn — it is that
|
|
the text can no longer be extracted.
|
|
|
|
Positioning requires the text matrix, so the module tracks `Tm`, `Td`, `TD`
|
|
and `T*` along with the CTM through `q`/`Q`/`cm`. It cannot reach the
|
|
graphics layer — the crate boundary again — so it treats a showing
|
|
operator's origin as its position and removes the whole run.
|
|
|
|
That is coarse, and coarse **in the safe direction**: removing more than
|
|
asked loses content the user can see is missing; removing less leaves the
|
|
secret in the file. When a trade-off is between a visible error and an
|
|
invisible one, take the visible one.
|
|
|
|
What it refuses to claim matters as much as what it does:
|
|
|
|
- images are removed **entirely** rather than cropped;
|
|
- metadata and attachments are **untouched**;
|
|
- an incremental redaction leaves the original text in the earlier revision,
|
|
and the report says so through `earlier_revisions_retain_content` rather
|
|
than implying the job is finished.
|
|
|
|
That last flag is the honest version of the second-order failure above. The
|
|
API tells the caller the file is not yet safe.
|
|
|
|
### Compaction finishes it (`compact.rs`, 427 lines, 9 tests)
|
|
|
|
`compact(doc, source, options)` rebuilds the file from the object graph
|
|
reachable from `/Root`. Dead objects, superseded revisions and the bytes
|
|
behind a redaction are not deleted — they are **never written**. Rebuilding
|
|
from reachability is what makes the guarantee total; a delete-based approach
|
|
has to enumerate everything that might hold a copy, and enumeration is where
|
|
this class of bug lives.
|
|
|
|
A signed document is refused unless `allow_signed` is set, because
|
|
compaction destroys the revision a signature covers and would otherwise
|
|
leave every signature unverifiable with no warning.
|
|
|
|
**The end-to-end test is the point:** redact, compact, then search the
|
|
output bytes for the secret.
|
|
|
|
## Three reader defects, found by writing these tests
|
|
|
|
None of these are in the new code. All three were pre-existing, and each was
|
|
exposed by a test written for redaction or compaction.
|
|
|
|
**1. Every generated PDF differed run to run.** `PdfWriter` wrote dictionary
|
|
keys in `HashMap` order, and Rust seeds its hasher per process. Found by
|
|
compaction's idempotence test: compacting an already-compact file produced
|
|
the same objects at the same offsets with their keys shuffled. Fixed by
|
|
sorting keys (`writer.rs:114`), verified by generating from four separate
|
|
processes and getting a byte-identical file.
|
|
|
|
This affected **every file this codebase has ever written**, and no test had
|
|
ever compared two outputs to each other.
|
|
|
|
**2. A short `/Length` silently truncated a stream.** The reader guarded a
|
|
`/Length` running *past* the buffer but trusted one that was too *small*,
|
|
cutting the stream early and losing the rest with no error. Short lengths
|
|
are common in hand-edited files. `endstream` is now the authority when the
|
|
two disagree — but only when it is further on, so binary data containing the
|
|
bytes `endstream` is still bounded by its declared length.
|
|
|
|
**3. Two stream readers disagreed by one byte.** `read_object_at` did not
|
|
trim the EOL before `endstream` while `find_endstream` did, so a
|
|
write-read-write cycle grew every stream by a newline each time. **A test
|
|
fixture had encoded the bug**: it declared `/Length 9` for eight bytes and
|
|
asserted the newline came back as data. Both were corrected — the newline is
|
|
syntax (§7.3.8.1), not content.
|
|
|
|
That fixture is the third time in this project a fixture has enshrined the
|
|
defect it was meant to catch. A fixture written from observed output tests
|
|
that the code still does what it did.
|
|
|
|
## Verification
|
|
|
|
Independently re-verified against the tree at `f75c1cc`, with a test written
|
|
fresh rather than reusing the module's own:
|
|
|
|
```
|
|
removed_anything = true
|
|
after redact, secret present = true <- still there: earlier revision
|
|
FINAL secret present = false <- gone after compaction
|
|
FINAL public retained = true <- did not over-remove
|
|
```
|
|
|
|
The middle line is the finding worth keeping. Redaction alone leaves the
|
|
secret in the bytes; only compaction removes it. The API says so, and now a
|
|
test outside the module says so too.
|
|
|
|
Mutation testing, ten mutations, all killed:
|
|
|
|
| Mutation | Tests failed |
|
|
|---|---|
|
|
| redaction covers instead of removes | 16 |
|
|
| CTM ignored | 1 |
|
|
| `Q` does not restore the CTM | 1 |
|
|
| operands kept when operator removed | 12 |
|
|
| revision warning always false | 1 |
|
|
| signature guard removed | 1 |
|
|
| reachability keeps everything | 2 |
|
|
| dropped reference left dangling | 1 |
|
|
| unresolvable object kept as reachable | 1 |
|
|
| writer dictionary order unsorted | 1 |
|
|
|
|
## Merge criteria
|
|
|
|
- [x] Redaction removes showing operators, never paints over them
|
|
- [x] Extracted text no longer contains redacted content
|
|
- [x] Text matrix and CTM tracked through `q`/`Q`/`cm`
|
|
- [x] Over-removal preferred to under-removal, and documented
|
|
- [x] Images removed entirely rather than cropped
|
|
- [x] Untouched metadata and attachments stated, not implied
|
|
- [x] `earlier_revisions_retain_content` warns that redaction alone is not
|
|
enough
|
|
- [x] Compaction rebuilds from `/Root` reachability
|
|
- [x] Signed documents refused unless explicitly allowed
|
|
- [x] End-to-end: redact → compact → secret absent from the bytes
|
|
- [x] Output is byte-identical across processes
|
|
- [x] Short `/Length` no longer truncates
|
|
- [x] The one-byte `endstream` disagreement resolved, and the fixture that
|
|
encoded it corrected
|
|
- [x] Ten mutations, all killed
|
|
|
|
## Consequences
|
|
|
|
**Positive.** Redaction that actually redacts, with the residual-revision
|
|
problem surfaced rather than hidden. Generated files are now reproducible,
|
|
which makes them diffable in review and cacheable in CI.
|
|
|
|
**Negative.** Compaction is destructive: it discards revision history and
|
|
therefore any signature over it. The `allow_signed` guard makes that a
|
|
deliberate act, but a caller who sets the flag without understanding it gets
|
|
a document whose signatures no longer verify.
|
|
|
|
**Risk, stated plainly.** Redaction operates at operator granularity, not
|
|
glyph granularity. A single `Tj` containing both public and secret text is
|
|
removed whole — safe — but a caller expecting character-level precision will
|
|
not get it. Text drawn as *outlines* rather than text operators is not text
|
|
to this module and is not removed. Neither limitation is detectable from the
|
|
API's return value, and both should be in user-facing documentation before
|
|
this is offered as a security feature.
|
|
|
|
**Not done, deliberately:** redaction annotations (`/Redact` as an
|
|
annotation subtype a user places interactively), content analysis to
|
|
*suggest* redactions, and glyph-level removal within a text run.
|