fix(redact): clear redacted text from /Info and XMP metadata on apply (#2448)

[jonathanwxh-cell]

Title: fix(redact): clear redacted text from /Info and XMP metadata on apply (#2448)

Applying redactions removed the page content but left the redacted string in the document
metadata: a title or subject holding the secret stayed readable in /Info and in the XMP
/Metadata stream (#2448).

apply now records the Unicode text of every glyph run it removes and, after the residue
check, clears metadata that repeats it. A /Info string entry goes when its value contains a
removed run — or when its value is contained in one, so the author of a wholly redacted
sentence goes too. Clearing goes through pdfcraft_organize::set_info, which also removes the
XMP property mirroring the entry and nothing else in the packet. A catalog or page /Metadata
stream is removed whole when removed text survives in it where per-entry editing cannot reach
(custom schemas, entity-escaped values), or when the stream cannot be decoded and read as
UTF-8 within 16 MiB. A trailer /Info that is a direct dictionary is removed whole when it
still matches; the per-entry step cannot edit that shape. All of it runs before
require_full_save, so the next save is the full rewrite redaction already forces and the old
/Info and /Metadata objects are dropped from the saved file. Fixes #2448.

What this changes

• The interpreter's show() (crates/redact/src/interp.rs:637) records maximal removed runs
  as fragments: joined across TJ kerning numbers within one show() call, split at kept
  glyphs, at codes with no Unicode meaning (each side then matches on its own), and at operator
  ends. Collection is gated to redaction apply mode — sanitize and verify are untouched.
• clear_marked_metadata (crates/redact/src/lib.rs:642) keeps fragments of two characters
  or more: one removed glyph is no evidence a secret was removed, and would clear unrelated
  entries containing that character. The same two-character floor applies to the value in the
  contained-in-a-run direction.
• Report gains info_entries_cleared and xmp_removed (crates/redact/src/lib.rs:134) —
  library-level only; the engine discards the Report and redact_apply's JSON is unchanged.
• redact (L4) now uses organize (L4): ("redact", "organize") was added to the xtask
  SIDEWAYS allow-list.
• Limits (stated once; the parity entry and the reviewer note below refer back to this):
  matching errs towards clearing — a fully redacted common word takes unrelated entries
  containing it with it, and the wholesale fallback drops a whole stream for one custom-schema
  hit — and it cannot prove surviving metadata clean: text inside images never becomes a
  fragment, fragments do not join across operators, and component-level /Metadata (images,
  fonts, form XObjects) is not scanned, the same scope as sanitize. doc_remove_hidden with
  the metadata category remains the certain path.

How you verified it

Tests written first and run against unmodified main — behaviour-only assertions; the
Report-field asserts came with the fix, since the fields do not exist pre-fix.

$ cargo test -p pdfcraft-redact -j 2
failures:

---- tests::a_direct_info_dictionary_holding_redacted_text_is_removed_whole stdout ----

thread 'tests::a_direct_info_dictionary_holding_redacted_text_is_removed_whole' (50464) panicked at crates\redact\src\tests.rs:757:5:
the whole dictionary went
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace

---- tests::an_xmp_stream_redacted_text_survives_in_is_removed_whole stdout ----

thread 'tests::an_xmp_stream_redacted_text_survives_in_is_removed_whole' (37896) panicked at crates\redact\src\tests.rs:736:5:
the escaped secret took the catalog stream with it

---- tests::redacted_text_is_cleared_from_info_and_xmp_on_apply stdout ----

thread 'tests::redacted_text_is_cleared_from_info_and_xmp_on_apply' (64716) panicked at crates\redact\src\tests.rs:702:5:
assertion `left == right` failed: the entry repeating removed text is gone
  left: Some("TopSecret plan")
 right: None

---- tests::utf16_info_entries_holding_redacted_text_are_cleared stdout ----

thread 'tests::utf16_info_entries_holding_redacted_text_are_cleared' (57972) panicked at crates\redact\src\tests.rs:793:5:
assertion `left == right` failed: the UTF-16BE title repeating removed text is gone
  left: Some("Classified SECRET€ memo")
 right: None


failures:
    tests::a_direct_info_dictionary_holding_redacted_text_is_removed_whole
    tests::an_xmp_stream_redacted_text_survives_in_is_removed_whole
    tests::redacted_text_is_cleared_from_info_and_xmp_on_apply
    tests::utf16_info_entries_holding_redacted_text_are_cleared

test result: FAILED. 50 passed; 4 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.06s

error: test failed, to rerun pass `-p pdfcraft-redact --lib`

(The second test above has since been renamed an_xmp_stream_holding_redacted_text_is_removed_whole;
the failure output predates the rename. The fifth unit test,
applying_with_no_marks_leaves_metadata_alone, passes on main by design: with no marks
anywhere, apply fails with NothingToApply before changing anything, and the test asserts
/Info and the packet are still in place — it guards the insertion point. It was in the 50
passed above.)

$ cargo test -p pdfcraft-automation -j 2 redacting_clears_matching_document_info_through_tools
running 1 test
test redacting_clears_matching_document_info_through_tools ... FAILED

failures:

---- redacting_clears_matching_document_info_through_tools stdout ----

thread 'redacting_clears_matching_document_info_through_tools' (37628) panicked at crates\automation\tests\automation.rs:3303:5:
assertion `left == right` failed: the title repeating redacted text is gone
  left: Some("TOPSECRETX clearance memo")
 right: None
note: run with `RUST_BACKTRACE=1` environment variable to display a backtrace


failures:
    redacting_clears_matching_document_info_through_tools

test result: FAILED. 0 passed; 1 failed; 0 ignored; 0 measured; 175 filtered out; finished in 0.02s

error: test failed, to rerun pass `-p pdfcraft-automation --test automation`

A sixth unit test, a_value_inside_a_longer_removed_run_is_cleared_too, was also written
red-first, against the first version of the fix: with the whole sentence "JAMES BOND IS A SPY"
removed (19 glyphs), it failed with info_entries_cleared: 0, xmp_removed: 0, because the
author "James Bond" is shorter than the removed run and the matcher only looked one direction.
That failure is what made the matcher bidirectional.

Post-fix, on the final tree:

$ cargo test -p pdfcraft-redact -j 2
running 56 tests
test result: ok. 55 passed; 0 failed; 1 ignored; 0 measured; 0 filtered out; finished in 0.05s

$ cargo test -p pdfcraft-automation -j 2 redacting_clears_matching_document_info_through_tools
test redacting_clears_matching_document_info_through_tools ... ok
test result: ok. 1 passed; 0 failed; 0 ignored; 0 measured; 175 filtered out; finished in 0.01s

The green runs assert the safety property directly, not just the clears: Author "Ada" stays
while Title goes (crates/redact/src/tests.rs:700), the packet's pdf:Producer and a clean
page-level /Metadata stream stay (:703, :705), Subject "Spy stories" stays and survives
the saved file (:727, :733), and the end-to-end test reads Author back out of the saved
file (crates/automation/tests/automation.rs:3314). The "nothing else in the packet" property
is pdfcraft_organize's own, pinned by its test xmp::tests::element_values_are_replaced_in_place
(crates/organize/src/xmp.rs:194, "Everything else is untouched"); on this tree
cargo test -p pdfcraft-organize -j 2 gives 126 passed; 0 failed.

• cargo fmt --check — cargo fmt --all -- --check, clean.
• clippy with -D warnings on the crates I changed — cargo clippy -p pdfcraft-redact -j 2 --all-targets -- -D warnings and cargo clippy -p pdfcraft-automation -j 2 --all-targets -- -D warnings, both finished with no warnings.
• tests pass on the crates I changed — above.
• cargo xtask layers — OK: 37 crates, no layering violations. (the redact row now lists organize).
• cargo xtask assets — not run; no assets are touched. CI runs it on every PR.

Tests. Four red-first unit tests in crates/redact/src/tests.rs — per-entry clearing with
case-insensitivity; UTF-16BE /Info through a ToUnicode-mapped fixture font; escaped
custom-schema and page-level wholesale removal; direct-dictionary wholesale removal — plus the
passes-on-main guard, the bidirectional-matcher test above, and one end-to-end test in
crates/automation/tests/automation.rs driving
doc_create/doc_set_info/redact_mark/redact_apply/doc_info/doc_save and asserting
through a re-open of the saved file. A full write may pack /Info into a compressed object
stream, so file-level survival is asserted by re-opening the saved bytes, not by scanning them.

Not run: the full automation suite (two known-stale failures on main, #2417), and the CI-only
gates (full workspace test, wasm, no-default-features check).

Never-crash

• No new unwrap/expect/panic!/unreachable!/todo! outside tests — none in the new code; crates/redact/src/lib.rs:18 denies the family crate-wide and clippy passed with -D warnings.
• No indexing or slicing with input-derived positions — the new string splits in xml_unescape/xml_chunks (crates/redact/src/lib.rs:543, :595, with packet_hit at :588) sit at char boundaries by construction (positions from find of ASCII separators and len_utf8 advances); metadata streams decode through decoded_within(MAX_METADATA) (:538, 16 MiB), and a stream that fails to decode or read as UTF-8 is removed whole rather than failing apply.
• No unsafe.
• Any crash this fixes has a small synthetic regression test — it fixes a leak, not a crash; nothing applies.

Provenance

• No asset from an Adobe product, and no Adobe icon recreated, traced or imitated — no assets touched.
• No font files committed.
• No GPL/AGPL code copied in — the entity unescape and chunk scanner are written here from the XML 1.0 rules.
• Behaviour comes from the specification or black-box observation — XMP escaping and the /Info/XMP shapes per spec, driven by synthetic fixtures.
• Every new asset has an ATTRIBUTION.toml entry — no new assets, so there is nothing to attribute; cargo xtask assets was not run.

Security

• --root confinement still holds — no path handling in the change; redact operates on an already-open Document.
• MCP and the UI control channel stay opt-in, loopback-only and token-authenticated — untouched.
• No new outbound network access — none.

Parity and reach

• parity/acrobat-features.toml updated in this PR — protect.apply-redactions gains the new evidence tests and notes saying what is still missing (the Limits bullet above); protect.remove-hidden-information is noted as the certain metadata follow-up.
• Reachable headlessly through an automation tool — the end-to-end test above drives it through redact_apply.
• Docs updated — crates/redact/README.md (the "Metadata redaction (Redaction leaves redacted strings in the Info dictionary and XMP metadata #2448)" section) and a ROADMAP progress-log entry.

Anything a reviewer should look at first

The SIDEWAYS entry in xtask/src/layers.rs: redact (L4) now depends on organize (L4) — an
architecture-rule change, and the one judgement call in the PR. After that, the matcher's
direction: a value is also cleared when it is merely part of a removed run — deliberate, and
part of the errs-towards-clearing trade-off recorded in the Limits bullet above.

[mtlcl410]

Hi @jonathanwxh-cell — #2539 (opened about two hours before this one) also fixes #2448, so the two overlap. A short comparison, in case it helps the maintainers choose:

• Collecting the removed text. Here fragments are split at operator ends, and single characters fall under the two-character floor. Text shown one glyph per operator, which many generators write, therefore produces no fragment at all. Checked on this branch (34e7919): a page drawing (T) Tj (O) Tj (P) Tj … (T) Tj with /Title (TOPSECRET), redacted over the whole line, removes all 9 glyphs but keeps /Title (info_entries_cleared: 0). fix(redact): redaction removes Info entries and XMP that repeat the redacted text (#2448) #2539 joins removed glyphs into runs by position across operators, so the word is found (removed_text_is_collected_in_runs).
• The reverse check (an /Info value that is itself part of a removed run, e.g. the author of a redacted sentence) is a good idea fix(redact): redaction removes Info entries and XMP that repeat the redacted text (#2448) #2539 lacked. I've added it there, with a three-character floor, crediting this PR (an_info_value_inside_the_redacted_text_goes).
• XMP. This PR edits the mirrored property through organize::set_info, with a whole-stream fallback; that needs the new redact → organize sideways allowance. fix(redact): redaction removes Info entries and XMP that repeat the redacted text (#2448) #2539 removes a matching catalog or page stream whole, with no new dependency.

I'm happy to fold anything else useful from here into #2539, or to close it if the maintainers prefer this one.

[jonathanwxh-cell]

Thanks @mtlcl410 for the concrete example and for crediting the reverse-match case. I checked the current head, bb57f20: show() still flushes at each operator, and clear_marked_metadata drops the resulting one-character fragments. Your TOPSECRET example therefore identifies a remaining gap. I haven’t independently rerun the fixture. #2539’s position-based collection addresses the cross-operator case.
The main trade-off here is preserving unrelated properties in editable XMP through organize::set_info, with whole-stream removal as a fallback and the extra redact → organize dependency. One regression worth retaining whichever approach is chosen is a metadata value shorter than the removed sentence, including custom-schema XMP elements and attributes as well as /Info. The test here is a_value_inside_a_longer_removed_run_is_cleared_too

[mtlcl410]

Thanks for checking. Agreed on that regression — I've added it to #2539 as a_shorter_value_in_custom_xmp_goes: "James Bond" from a redacted "JAMES BOND IS A SPY", in a custom-schema element and in a custom-schema attribute, takes each packet with it, as it goes from /Info, while a clean packet and an unrelated entry stay. It already passed (the run's words are terms too); it fails if the XMP check is disabled.

One difference worth noting for the trade-off: #2539 also matches each word of three or more letters on its own, so it errs further towards clearing — with "JAMES BOND IS A SPY" redacted, a /Subject (Spy stories) would go there, while your test keeps it.

[jonathanwxh-cell]

@mtlcl410 Good catch — the cross-operator case you demonstrated, where (T) Tj (O) Tj … slips past the floor while /Title (TOPSECRET) survives, is fixed on 9248b8a. Removed glyphs now join into runs by position across operators before the floor applies, so the split-glyph form no longer dodges redaction. Your TOPSECRET example is now a regression test — thanks for the clean reproduction. Surgical matching is unchanged: /Subject (Spy stories) still survives.

Verified: test tests::a_value_inside_a_longer_removed_run_is_cleared_too ... ok and test tests::redacted_text_is_cleared_from_info_and_xmp_on_apply ... ok — full suite 61 passed, 0 failed, 1 ignored (including the new cross-operator and TJ-kern tests); clippy -D warnings clean, cargo fmt --all -- --check clean.

[Losminek]

Thanks for taking #2448, and for the fix in 9248b8a. I reported the issue, and I went through this head (530c761) and #2539 by reading the code, without building. I'm not choosing between the two PRs. These are the cases where text can still survive here:

1. TJ kerning joins words. Any number in a TJ array sets kerned, and removed_glyph then appends the next glyph with no space, whatever the size. pdfTeX writes word spaces as kerns ([(James)-333(Bond)]TJ), so the run becomes jamesbond. It matches /Author (James Bond) in neither direction. Applying the same 0.15-height gap rule to the kern (a large negative number gets a space) would fix it.
2. Whole-fragment matching only. clear_marked_metadata lowercases and trims, then checks substring in both directions. Inner whitespace isn't normalized and there are no word terms. So redacting "JAMES BOND IS A SPY" leaves /Title (Case: James Bond), because neither string contains the other. The same happens with "Smith, John" against a redacted "John Smith". fix(redact): redaction removes Info entries and XMP that repeat the redacted text (#2448) #2539's word terms (three or more letters) cover these cases.
3. The raw packet is searched, tag names included. packet_hit calls hit(text) on the whole XML, and the reverse direction applies to any chunk of two or more characters. A short fragment like "dc" or "id", or a value like en or 1.7 that happens to sit inside a removed fragment, drops the whole packet. This is the safe direction, but it will clear XMP more often than the per-entry /Info editing suggests.
4. No limits on fragments. Neither the number nor the length of removed_fragments is capped, and matching costs fragments × packet size, with packets up to 16 MiB. A redaction over a long document could stall here. fix(redact): redaction removes Info entries and XMP that repeat the redacted text (#2448) #2539 has caps, but they need to fail closed (I left the same note there).

Neither PR covers XMP attached to images, fonts or form XObjects, or Unicode normalization (NFD accents, ligatures like "fi"). Those may be fine as documented limits next to Sanitize's metadata category.

Tests that would help: a pdfTeX-style TJ with word kerns, a metadata value that contains only part of the redacted text, and an XMP packet whose tag names contain a short redacted fragment.

[Losminek]

Thanks for the quick update. I went through 192c745 against my earlier points, again by reading only.

• Point 1 (TJ kerning): fixed. A kern above 0.15 height is now an optional gap, and the joined form still matches.
• Point 3 (tag names): fixed. Parsing with roxmltree keeps names and namespace declarations out of the search, and the tests cover it.
• Point 2 (partial names) is now a stated non-goal. metadata_whitespace_is_normalized_without_broadening_name_matching keeps Case: James Bond and Smith, John. I understand the trade-off. For a redaction tool, though, it means a redacted name stays in /Title or /Author whenever the value has any other text around it. If that's the intended scope, a line in the redaction docs or report would help, for example: "metadata is cleared only when it contains a whole redacted run or is part of one; use Sanitize → metadata to remove the rest".
• Point 4 (limits): still open, and there is a new path:
  • Nothing caps the number of fragments or the total work.
  • matches restarts prefix at every occurrence of the first character. Kerned runs also add a second variant to compare. So one long, repetitive run against a long value costs roughly length × length.
  • Capping it and clearing the metadata when a cap is hit would keep it fail-closed.

One new gap: packet_hit checks text, comments and attribute values, but not processing instructions. <?audit James Bond?> in an otherwise clean packet now survives, where the earlier whole-packet search caught it. Adding node.is_pi() (its value) to the check would close it.

The new metadata tests check the reopened document, which is the right way. A few older ones in tests.rs still search raw bytes after a default save, and those pass either way because of the compressed object streams.

[Losminek]

Checked 0b7184b, by reading only. Collection and matching now share a work budget that fails closed, processing instructions are searched, and the shared test helper reads decoded streams. Thanks, that covers my earlier points, apart from partial names, which the README now states as out of scope.

Two small things left, neither blocking:

• node.pi() also reaches the packet wrapper <?xpacket begin="…" id="…"?> / <?xpacket end="w"?>. Redacting a word like "end" or "begin" would remove a clean packet, pdfaid included. Skipping PIs whose target is exactly xpacket would avoid it.
• organize::set_info reads and rewrites the XMP mirror (up to 16 MiB) before the budgeted packet check. That work isn't counted against the budget.