nigig-org/REVIEWS/adr/0023-pdf-redaction-and-compaction.md
andodeki ad3fe19b90
Some checks failed
repo hygiene / hygiene (push) Has been cancelled
docs(pdf): the four missing ADRs — codecs, Phase 4 completion, editing, redaction
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.
2026-08-18 05:53:36 +00:00

8.1 KiB

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

  • Redaction removes showing operators, never paints over them
  • Extracted text no longer contains redacted content
  • Text matrix and CTM tracked through q/Q/cm
  • Over-removal preferred to under-removal, and documented
  • Images removed entirely rather than cropped
  • Untouched metadata and attachments stated, not implied
  • earlier_revisions_retain_content warns that redaction alone is not enough
  • Compaction rebuilds from /Root reachability
  • Signed documents refused unless explicitly allowed
  • End-to-end: redact → compact → secret absent from the bytes
  • Output is byte-identical across processes
  • Short /Length no longer truncates
  • The one-byte endstream disagreement resolved, and the fixture that encoded it corrected
  • 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.