Repository navigation
Conversation
OcspResponse::check skipped the delegated-responder checks (issued by the issuer, signed with its key, id-kp-OCSPSigning) whenever the responder's subject equalled the issuer's. A certificate carried inside the response, selected by its own key hash, is vouched for by nothing but those checks, so one copying the issuer's subject under the attacker's key signed a revocation verdict, good or revoked, that the signature panel believed. Now only the issuer certificate the caller hands in may skip the checks; a certificate taken from the response is always a delegate. The parser also reads the response's certificates as RFC 6960 encodes them, `[0] EXPLICIT SEQUENCE OF Certificate`, as well as directly under the tag. Test: a_responder_copying_the_issuer_name_under_another_key_is_not_the_issuer (red before the fix: the forged response answered Good). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A timestamp token counted as trusted time whenever its signing certificate chained to the trust store and was valid when it stamped. The store is one list, so any correspondent whose signing certificate the user trusts, or any subscriber of a trusted CA, could mint tokens and document timestamps that showed Valid with a time of their choosing. Trusted time now also needs the certificate to be issued for timestamping (id-kp-timeStamping, RFC 3161 2.3), as a delegated OCSP responder already needs its purpose. Without it, an embedded token is reported as unverified time and the signer's claimed time is used, and a standalone document timestamp stays Unknown, through the existing "not in your list of trusted certificates" wording (no new sentence, so no new translations). Certificate::self_signed_with_purposes creates a self-signed certificate with given extended key usages; the tests' in-process timestamp authority now uses the RSA test identity under such a certificate. Test: a_trusted_certificate_without_the_timestamping_purpose_is_not_a_time_authority (red before the fix: "its authority is trusted; trusted time is ..."). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…nce is not free to define Two more ways a verdict described something other than the signature: - A signature dictionary's /ByteRange was taken at face value: a second field, appended after signing, whose value copied the genuine dictionary (range and contents included) under a reason, date and name of its author's verified over the genuine bytes and was listed Valid with the genuine signer's name. The gap must now be the /Contents token of the object that carries the dictionary (its /V object, or the field when /V is direct): that object's header lies before the gap in the file at the place its cross-reference entry gives, nothing ends it before the gap, and the last token before the gap is /Contents. Anything else is "does not exclude exactly its contents". The same binding applies to document timestamps and the legacy x509 format. - A new object was skipped as "just a new object" even when the signed revision already referenced its number: a page whose resources named an undefined XObject, font or pattern drew it once a later revision defined it, while every signed object stayed byte-for-byte the same. The signed revision is walked once for the numbers it names but does not define; defining one later is a page content change, at every level. Tests: a_signature_field_borrowing_another_fields_byte_range_is_invalid (red before: the borrowed field was Valid), defining_an_object_the_signed_page_already_named_is_a_change (red before: Valid at every level). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…rmits only a timestamp The change classifier labelled any new /Subtype /Widget "form fill" and any new /FT /Sig widget "signing", without comparing a changed widget or field with its signed version. After a P=2 certification an attacker could add fields and widgets with appearances over the page, put actions on existing fields, move or hide them; after a P=1 certification an unsigned signature field with a page-covering appearance was "signing", which the permission table allowed at every level. Now a new field or widget is a new annotation (permitted only where comments are), unless it is a signature field that is signed; an existing field or widget is form fill only when nothing but its value and what a filler regenerates from it (/V, /AS, /AP, /M, /I, /RV) differs, otherwise it is a change no permission covers; the same holds for field dictionaries without a widget and for fields registered only in /Fields. Under P=1 the one signature change permitted is a document timestamp (the timestamp dictionary, a field whose value is one, or a page, form or catalog that only gained such fields). Revisions are compared by what their strings say, not how a writer spelt them: an encrypted update writes its strings as hex, which made the same field look changed. Tests: changes_to_fields_after_signing_are_judged_by_what_changed (red before: a new text field was Allowed(["form fill"]) at P=2), an_unsigned_signature_field_added_after_certifying_is_not_a_signature (red before: Allowed(["signature"]) at P=1). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A signature's FieldMDP references and a signature field's /Lock were never read: a change to a field the signer locked validated as "Signature is valid" with the change reported as permitted form fill. Validation now reads the lock from the signed revision (every FieldMDP reference the signature carries, combined; else the signed field's own /Lock) and the change classifier treats a fill of a locked field, named by its fully qualified name under its parents, as a change no permission covers. Fills of unlocked fields and later signatures are unaffected. Signing an existing field that has a /Lock now writes the matching FieldMDP reference into the signature (ISO 32000-2 12.8.2.4), for readers that only read references; PdfCraft used to sign such fields without one. Still missing: the /Lock fallback is exercised by inspection only (every PdfCraft-signed file now carries the reference); a change to the lock itself after signing is caught as a field change, not named as such. Test: a_field_the_signer_locked_may_not_be_filled_in_afterwards (red before: no FieldMDP reference was written, and the locked fill was Allowed(["form fill"])). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…the signature's own bytes Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
A script is refused before it runs when it is nested deeper than any form needs, because boa parses nesting on the thread's stack. A script could get the same source past that check by building it at run time and handing it to eval() or Function(): the parser took it on unscreened, and a few hundred kilobytes of brackets held the script thread for over a minute, or overflowed it. Every script context now has a host whose HostEnsureCanCompileStrings applies the same refusal to the body and parameters of eval() and Function(), so such a call fails with a SyntaxError inside the script instead. Test: source_built_at_run_time_is_screened_like_the_script (red before the fix: eval() of 20,000 nested parentheses ran for 70 s and succeeded). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Search & Redact ▸ Patterns took the longest run of digits and separators from each start and dropped it when its digit count or Luhn checksum was wrong, with no shorter end tried. Page text joins words and lines with one space, so a number at the end of a line followed by a street number, an expiry date, a CVV or another number became one over-long run and was never marked, while the search still reported a plausible count and Apply Redactions left it in the saved file. Invariant now: when the longest run from a start is not a valid number, the same start is retried with the run cut back at each space, longest first (trailing separators trimmed), and the first candidate that passes the existing digit-count and checksum tests is marked. Every former match still matches; the change only adds matches. One pinned non-match changed with it: a grouped `+` number with more than 15 digits now has its longest valid prefix marked instead of nothing, because it cannot be told apart from a number that runs into another number, and marking more is the safer error. Still not covered: the first candidate is taken longest first, so a flexible format (E.164, trunk) followed by another number can swallow that number's first group; the SSN and date patterns were not changed. Tests: patterns::tests::numbers_followed_by_another_number_still_match (red before the fix: `found(Phone, "Tel 555-123-4567 12 Elm Street")` returned `[]` instead of `["555-123-4567"]`); phone_numbers updated for the over-long case. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The cleared copy of a partly covered image was built from a clone of the original dictionary, so its /SMask and /Mask entries still pointed at the original mask streams, which no code cleared and the forced full-rewrite save kept because the copy referenced them. For an image whose shape lives in the mask (a transparent signature, handwriting, alpha-rendered text) that is the redacted content itself. Invariant now: every stream that carries samples of the covered area is cleared over the same regions or the image is removed. The soft mask and a stencil /Mask stream are decoded and cleared through the same unit-square transform as the colour samples (so a mask of another resolution is covered by the same regions), written as new objects, and the copy points at them; a mask that cannot be decoded or cleared fails the whole image, which the caller then removes, as it already does for unsupported codecs. /Alternates and /OPI, which hold the same pixels again, are dropped from the copy. A colour-key /Mask array holds no samples and stays. Still not covered: a mask so coarse that no mask pixel centre falls under the regions is kept as it is (the same centre rule the colour samples use); /SMask inside the image data (/SMaskInData, JPX) is a codec that is removed whole anyway. Test: tests::a_partly_redacted_images_soft_mask_and_stencil_mask_are_cleared_too (red before the fix: "SMask: the covered mask row survived the save"). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Apply Redactions remembered the fields owning a widget under a mark by their fully qualified name, once per name, and deleted each through pdfcraft_forms::delete_field, which takes the first field of that name. Field enumeration lists every terminal field dictionary separately, so a document with two same-named fields (merged or repeated forms) kept the marked field's dictionary and /V in /AcroForm /Fields, which the full-rewrite save wrote, while an unmarked field of the same name was deleted in its place; with both marked only one went. redact_apply reported success in every case. Invariant now: every field owning a widget under an applied mark is removed with its value, identified by its object reference, and no other field is touched. pdfcraft-forms gains delete_field_object(doc, ObjRef); delete_field keeps its name-based contract (the first of the name) and both share one deletion routine. Still not covered: the verify pass still checks page content only, so a surviving field is not detected after the fact; a widget written as a direct dictionary in /Annots is not a Field in the forms model (see the direct annotation finding). Tests: tests::redacting_one_of_two_same_named_fields_deletes_that_field_by_object (red before the fix: "the marked field's value survived the save"); pdfcraft-forms tests::deleting_by_object_removes_only_that_one_of_two_same_named_fields pins the new helper. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ertain The redactor worked out a partly covered image's sample layout (components per pixel, row length) on its own, from /ColorSpace and an ICC profile's /N, while the renderer resolves the colour space by other rules: it reads the short /CS key before /ColorSpace, uses the profile's /Alternate when the profile does not load, resolves a bare name only for the Device spaces, and draws anything it cannot resolve as DeviceGray. An image the page shows as one-component gray could be treated as three or five components, so with the data padded to the longer layout the redactor zeroed bytes in padding the renderer never draws and wrote the displayed samples back untouched, under a black box, while reporting the mark applied. Invariant now: a partly covered image is cleared only when its component count is established by rules under which the renderer cannot arrive at another count; otherwise it is removed whole, as images in codecs that cannot be re-encoded already are. In detail: a dictionary carrying any inline-image abbreviation the renderer prefers over the full key (/CS, /W, /H, /BPC, /IM, /D, /F, /DP) is refused; a bare name counts only for DeviceGray, DeviceRGB and DeviceCMYK; CalRGB and Lab arrays only with their dictionary; ICCBased only with /N 1, 3 or 4, an /Alternate (if any) of the same count, and for 3 or 4 a profile stream that decodes; CalGray, Indexed and Separation are one component either way. Size arithmetic is checked. Still not covered: whether the renderer's own decoder accepts an ICC profile cos decodes was not cross-checked; DeviceN images were and are removed whole rather than cleared; the verify pass still skips images. Test: tests::an_image_whose_sample_layout_is_uncertain_is_removed_whole (red before the fix: "icc N 3 with a gray alternate and a profile that does not load: the marked samples survived the save"). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The content parser accepted the end of an inline image only at white space + `EI`. When the data ran straight into `EI` (ASCIIHex data ending `>EI`), the parser took the rest of the stream as image data and emitted one `BI` operator, while the renderer ends the image there and draws what follows, text layer included. Apply Redactions never saw that content, its verify pass parsed the stream the same way and found no residue, and the saved file kept the marked text under the box. Invariant now: the parser ends inline image data at the first filter's end-of-data marker (`>` for ASCIIHexDecode, `~>` for ASCII85Decode) followed by `EI` as well, and reports in `Parsed::unterminated_inline_image` when no `EI` ended an image at all. The redaction interpreter carries that flag out of a page's streams and of every nested form XObject (`Output::unreadable`), and `apply` returns `RedactError::Unreadable` for a page with marks in both the apply and the verify pass, so nothing is changed. The parser's `skipped` count is still not consulted. Still not covered: unfiltered binary data that runs into `EI` with no marker, or a stream that ends inside an inline image, is reported unterminated and refused rather than measured from the image's size and colour space; the sanitize paths share the interpreter but do not read the flag. Tests: pdfcraft-redact tests::content_after_an_inline_image_without_white_space_before_ei_is_redacted_or_refused (red before the fix: `r.glyphs` was 0, not 4, the text after `00>EI` was not seen, and the binary case returned Ok instead of Unreadable); pdfcraft-content tests::an_inline_image_ends_at_its_filter_marker_or_is_reported_unterminated. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Two findings with one root cause: the redactor acted on a cos decode that
could differ from the viewer's, with no signal.
- Stream::filters read only a direct /Filter name or array and a direct
/DecodeParms, so an indirect /Filter (legal, ISO 32000-2 §7.3.8.2) gave an
empty chain and the stored, still-compressed bytes passed as a successful
decode. Apply Redactions found no text in them, left the stream as it was,
drew the box, and its verify pass found no residue; an image with an
indirect /DecodeParms predictor was cleared at offsets in the still-tagged
bytes and written back without the predictor.
- The tolerant decode turned a Corrupt failure with no output (a /Predictor
outside 1, 2, 10–15, /Colors outside 1..255, a zlib header with the
preset-dictionary bit) into an empty page, and /DP on an image was ignored
while the viewer applies it; apply reported the marks applied with the
content untouched in the saved file.
Invariant now: everything redaction decodes (page content, form XObjects,
images, ICC profiles) goes through the new Stream::decoded_strict_in(doc):
the chain is resolved through the document (/Filter, /DecodeParms, their
array elements and the parameter values may be references), any entry that
does not resolve to a name or dictionary is an error, a dictionary carrying
/F (its data lives in another file, Table 5) or /DP (an inline-image key the
viewer reads first) is an error, and any corruption is an error. page_streams
then returns RedactError::Unreadable, a form is removed, an image is removed
whole. Stream::filters and decoded keep their tolerant contract for the other
consumers; cos re-exports Filter and Params. The strict decode also refuses
truncated streams a viewer would draw partially: redaction of a damaged page
now fails rather than guessing.
Still not covered: sanitize, the audit and text extraction still use the
tolerant, unresolved decode; whether the viewer's own decoder accepts every
stream cos accepts was not cross-checked; parameter values that are not
integers fall back to the defaults as before.
Tests: tests::indirect_filter_and_decode_parms_are_resolved_before_redacting
(red before the fix: apply returned Ok with glyphs 0 and the image cleared
in the wrong layout); tests::streams_cos_cannot_decode_cleanly_fail_closed
(red before the fix: "predictor 16" returned Ok(Report { glyphs: 0, .. })
instead of Err(Unreadable(1))); cos
tests::filters_in_resolves_references_and_refuses_what_it_cannot.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The fail-closed check for vertical-writing text under a redaction mark
decided from the /Encoding name ending in -V, or from a /WMode entry in an
embedded CMap's stream dictionary. The renderer decides from the parsed
CMap, so an embedded CMap whose program says `/WMode 1 def` with no /WMode
in its dictionary, and the predefined CMap named plainly `V`, were drawn as a
column but laid out horizontally by the redaction interpreter: the mark over
the column matched no glyph, apply reported success and the text stayed
extractable from the saved file.
Invariant now: a Type0 font is vertical when its predefined CMap name is `V`
or ends in -V (ISO 32000-2 Table 118), or its embedded CMap has /WMode 1 in
the stream dictionary or `/WMode 1 def` in its program (§9.7.5.4, read by
the new pdfcraft_fonts::pdf::cmap_is_vertical); an embedded CMap that cannot
be decoded counts as vertical. Apply then refuses the page as before.
Still not covered: a CMap that inherits a writing mode through usecmap (the
renderer does not follow it either, so it draws horizontally); a Type0
/Encoding that does not resolve is still treated as horizontal rather than
refused.
Tests: tests::vertical_writing_from_the_cmap_program_or_the_cmap_named_v_fails_closed
(red before the fix: "WMode 1 in the CMap program" returned Ok(Report {
glyphs: 2, .. }) instead of Err(VerticalText(1))); pdfcraft-fonts
tests::wmode_is_read_from_the_cmap_program.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The redaction interpreter learned the current font and size only from a Tf
operator in the scope it was processing: the gs operator fell through the
catch-all arm, so a font selected through an ExtGState's /Font entry was
unknown to it, and every form XObject started from a fresh text state, so a
form showing text with the font and size its caller selected was unknown
too. Both kept the fallback font at size 0, which places every glyph of the
line as a point at the line's start; a mark over a word further along the
line matched none of them, nothing was removed, the verify pass used the same
model, and the word stayed in the saved file under the box while the
renderer, which honours both, drew it where the user marked it.
Invariant now: the interpreter tracks font and size the way a viewer does.
The gs arm reads /Font [font size] from the scope's ExtGState resources and
sets the font, size and vertical flag as Tf does (ISO 32000-2 §8.4.5, Table
57); a form XObject is processed from its caller's graphics state, text
state included, under its own matrix (§8.10.1) instead of the initial state.
The font cache keys an ExtGState's font apart from the /Font names.
Still not covered: text shown on a marked page with no font set at all
(fallback metrics at size 0) is still laid out at the line origin rather
than refused; Type 3 glyph procedures and patterns are not interpreted.
Test: tests::text_set_through_an_extgstate_font_or_inherited_by_a_form_is_redacted
(red before the fix: "ExtGState font: Report { glyphs: 0, .. }", 4 expected).
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The annotation step of Apply Redactions considered only /Annots entries that are indirect references: a direct dictionary was skipped before the Subtype and /Rect tests and always kept, so a link's target or a comment's text under a mark stayed in the saved file and the annotation stayed active under the box. Invariant now: every annotation whose rectangle overlaps a mark is removed, whether /Annots holds it by reference or directly. Entries are resolved through the document and removed by their position in the array; pop-ups whose /Parent is a removed reference go with it as before. A direct dictionary cannot be a redaction mark (marks are listed by reference) and cannot be the parent of a pop-up. Still not covered: a widget written directly in /Annots is not a field in the forms model, so it leaves the page with its value but no field deletion is reported for it; other referrers of a removed annotation (a structure tree's OBJR, a surviving reply's /IRT) are a separate finding. Test: tests::annotations_written_directly_in_annots_are_removed_under_a_mark (red before the fix: "LEAKDIRECTLINK survived the save"). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…n layers Remove Hidden Information > Hidden layers (and Sanitize) deleted every operator between an off layer's `/OC /name BDC` and its `EMC`, but never recorded which XObjects the dropped `Do` operators named. The page's /Resources /XObject (and a rewritten form's) kept naming them, so the full rewrite on save wrote the layer's image bytes and the form's text into the sanitized file while the feature reported the layer as removed. The layer's property-list entry kept its OCG dictionary (and name) reachable too. Now `strip_hidden_layers` returns the `Do` names it dropped, `process` adds them to `Output.replaced` and `Output.doomed`, so `content_pass`, the nested form branch and `unlink_doomed` prune them (a name a surviving `Do` still draws is kept, as before), and the off layers' /Properties entries go once no surviving BDC names them, on the page and in rewritten forms. Still not covered: a /Properties dictionary inherited from a /Pages node keeps the OCG dictionary reachable (its name only; the content is gone). Tests (crates/redact/src/tests.rs): - off_layer_blocks_take_the_images_and_forms_they_draw_with_them; red before the fix: "LAYERBLOCKIMAGEB survived the save (sanitize: false)". - off_layer_blocks_inside_a_form_take_their_images_with_them; red before the fix: "the image the off layer's block drew inside the form survived the save". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Apply Redactions, Remove Hidden Information and Sanitize took a removed annotation out of the page's /Annots (and a field out of the field tree) and relied on the forced full rewrite to drop the object. The rewrite keeps everything still reachable from the catalog, so in a tagged document the structure tree's OBJR kids kept the widget (with /T and /V), the comment (with /Contents), the link (with its URI) and the file attachment (with the embedded file); for Apply Redactions a surviving reply's /IRT kept the comment and /AcroForm /CO kept the deleted field. The edit reported success. Now removal is deletion: every removed annotation, pop-up, widget and field object is overwritten with null (`retire`), which the full save skips, so no other referrer can carry its content into the saved file. A surviving reply that pointed at a removed comment loses its /IRT and /RT; a deleted field leaves /AcroForm /CO. Under Form fields, a widget that flattening took off the page is deleted, and a field is deleted once none of its widgets is on a page (its /V would otherwise stay reachable). Still not covered: the structure elements whose OBJR kid pointed at a deleted object keep that kid (it now resolves to null); nothing reads it back, but the empty element is not pruned. Tests (crates/redact/src/tests.rs): - redacted_annotations_and_fields_do_not_survive_through_the_structure_tree_irt_or_co; red before the fix: "FIELDVALUESECRET survived the save". - removed_hidden_annotations_and_fields_do_not_survive_through_the_structure_tree; red before the fix: "ANNOTBODYSECRET survived Sanitize". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Form fields category flattened the widgets and then removed /AcroForm, relying on flattening to take every widget off the pages. Flattening deliberately leaves annotations whose /F has Hidden or NoView set alone (neither drawn nor removed), so those widgets stayed in the page's /Annots with /T, /V and /AP, the full rewrite wrote them, and the saved file had live fields with their values again on reopen. The tool reported success. Now, after flattening, every widget still on a page is taken off it with nothing drawn, and (with the previous commit) deleted with its field, so neither the widget nor a parent field holding /V stays reachable once /AcroForm is gone. Ordinary Flatten keeps its "hidden annotations are left alone" behaviour. Test (crates/redact/src/tests.rs): - hidden_and_noview_form_fields_are_removed_with_the_rest; red before the fix: "MARK_HIDDEN_VALUE survived (sanitize: false)". Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…hable object Sanitize and Remove Hidden Information counted and removed XMP, /PieceInfo and /AF from a fixed list of dictionaries: the trailer's /Info, the catalog and the page dictionaries (and, for attachments, the EmbeddedFiles tree and FileAttachment annotations). Nothing below a page dictionary was visited, so XMP on image and form XObjects, /PieceInfo on form XObjects and associated files on pages, XObjects, annotations and structure elements stayed referenced and the full rewrite wrote them. /AF was not counted anywhere, so a file whose only attachments were associated files was reported as having none, and asking for that category alone returned NothingToApply with the catalog /AF still in the saved file. Now each of the three categories is a walk over every object reachable from the catalog (each object once, a seen-set per object number, nesting bounded): Metadata removes /Metadata from every dictionary and stream dictionary, Private data removes /PieceInfo (and the /LastModified that goes with it) everywhere, and File attachments removes /AF everywhere, so the file specifications only /AF reached drop out of the full rewrite. The scan counts the same locations (/AF entries included), so the dialog offers the category and a chosen category with data is never reported as nothing to apply. Still not covered: an object reachable only from the trailer other than /Info (none carries these keys in practice). Test (crates/redact/src/tests.rs): - metadata_private_data_and_associated_files_below_the_page_are_removed; red before the fix: "MARK_IMG_XMP survived Metadata" (and the scan reported 3 metadata items instead of 5). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Sanitize and Remove Hidden Information worked from a fixed list of containers: annotation actions were removed only from /AA, or from /A on widgets; only comment, FileAttachment and Link annotations were dropped; the catalog pass removed only /Names /EmbeddedFiles and /Names /JavaScript. A JavaScript or Rendition action on a Screen annotation, the script a 3D stream names in /OnInstantiate, embedded media and assets of Screen, RichMedia and 3D annotations, a Movie's external path, /Names /Renditions, /Names /AlternatePresentations and page /Thumb all stayed in the saved file, the scan counted none of them, and the operation reported success. Now, under Links, actions and scripts: /A and /AA go from every annotation, /Names /AlternatePresentations goes, and a 3D stream loses /OnInstantiate. Under File attachments: Screen, Movie, RichMedia and 3D annotations go (their media, assets, models and paths with them) and so does /Names /Renditions. A page's stored /Thumb goes whenever a chosen category changes what the page shows. The scan counts each of these, so the dialog and doc_hidden_info show them. Sound annotations were already comments. Still not covered: a /Thumb is not counted as an item of its own, so a document whose only hidden information is a thumbnail reports nothing to remove (Sanitize still drops it, since every category is chosen). Test (crates/redact/src/tests.rs): - multimedia_scripts_embedded_files_paths_and_thumbnails_are_removed; red before the fix: "SCRIPTMARKER_RENDITION_ACTION survived Sanitize" (and the scan counted 1 attachment instead of 7). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… and tag ancestors Apply Redactions removed the glyphs under a mark but kept the text that stood for them: the property list of the `BDC` wrapping the glyphs (inline, or a named /Properties entry) kept /ActualText, /Alt and /E whether the whole span went or part of it, and structure elements above the element directly owning the changed marked content kept /ActualText, /Alt, /E and /T. The saved content stream, the page's /Properties and the structure elements spelled the redacted words after a full rewrite; for a partly redacted span the text layer showed the whole original replacement text. Now the interpreter keeps the open marked-content sequences (across content stream pieces) and, when a glyph, inline image, path, shading or XObject inside one is removed or cut, strips /ActualText, /Alt and /E from every enclosing `BDC`: an inline list is edited; a named list is replaced by a cleaned copy, inline, or under a fresh /Properties name when it holds indirect references (which a content stream cannot carry); the shared original is dropped from /Properties once no surviving `BDC` names it. The same happens inside rewritten form XObjects, under hidden-text removal, and around a removed hidden-layer block. Structure cleanup now carries sensitivity upward: the owning element and every element above it (found by the walk, not by /P) lose /ActualText, /Alt, /E and /T; /T goes too because a title may repeat the removed words (fail closed). Untouched siblings keep theirs. The tag pass resolves both the original and the added list names. Still not covered: a property list held only by a /Resources another, unredacted page still inherits keeps its replacement text (that page may still use it); the crate README now says what is and is not cleaned. Tests (crates/redact/src/tests.rs): - marked_content_property_lists_lose_the_replacement_text_of_redacted_glyphs; red before the fix: "QINLINEACTUAL survived the save". - structure_elements_above_the_owner_lose_their_replacement_text; red before the fix: report.tags was 1 (the owner only; QOWNERTITLE, QPARENTACTUAL and QGRANDACTUAL survived). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ening Flattening form fields prunes the AcroForm field tree afterwards: a walk that drops the flattened widgets and the fields left without any. The walk revisited a node for every /Kids entry naming it, and a producer fault that is tolerated on open (one kid listed twice) can be written at every level, so 24 levels of pairs were sixteen million visits, each cloning the node and its kids: a small file made Flatten take tens of seconds, growing with the depth, on a worker thread the UI cannot cancel. The depth cap of 64 bounded nothing, since the width doubled under it. Now each node is judged once (a map from reference to its verdict), however many entries name it; while it is being judged it counts as kept, so a cycle through it keeps it as well, as the depth cap did. What survives is the same: a repeated kid is pruned the first time and found pruned the next. Test: pdfcraft-edit flattening_prunes_a_field_tree_with_shared_kids_in_linear_time (red before the fix: "pruning the form took 24.07s" against a 5 s bound; a few milliseconds after). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…e line The audit decided that a filled rectangle (or an unapplied /Redact mark) hid a text line by dividing their overlap by the area of the whole line box, and needed 40 %. text_lines unions every run on a baseline into one line, so a black box over a name inside a sentence (the commonest redaction shape) overlapped far under 40 % and the page was reported clean, with exit status 0, while the text was still extractable. Four adjacent boxes that blacked out a whole line passed the same way, each being tested alone. The invariant now: a cover hides a line when it spans at least half the line's height over at least a glyph's width (a quarter of the line's height, or all of a shorter line). A box over one word counts, each of several adjacent boxes counts, an underline, a strikethrough or a table rule does not. The module docs and README state the rule. Still not covered: covered_text is the whole text of every line the cover touches, since text_lines does not expose glyph boxes; a cover shorter than half the line's height (one over the x-height only) is not reported. Tests (crates/audit/src/tests.rs), red before the fix: 0 findings instead of 1 for cover_over_one_word_of_a_long_line_is_found, 0 instead of 4 for adjacent_covers_over_a_line_are_each_found, 0 instead of 1 for redact_mark_over_part_of_a_line_is_found; rules_painted_after_text_are_not_covers guards the other direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…vers The faux-redaction audit collected covers only from a page's content streams and the form XObjects they draw. Annotations were read only to find /Redact marks, so a black rectangle placed over text as a Square or Highlight annotation, whose appearance stream is the same re + f fill the audit flags in page content, was reported clean (count 0, exit status 0) while the text was still extractable and the renderer painted the region black. The invariant now: every annotation a viewer paints (not Hidden or NoView, not a popup, not a /Redact mark, which is reported on its own) has its normal appearance stream (/AP /N, the /AS state or the only state) walked as a form placed by /Rect, /BBox and /Matrix (ISO 32000-2 §12.5.5, algorithm 8.1), with a paint position after every page operator, clipped to /Rect, through the same cover test as page content. An appearance that cannot be decoded fails the audit like a content stream. The annotation list is parsed once per page and feeds both the cover walk and the unapplied-mark check. Still not covered: text drawn inside an appearance is not audited under covers (as for form XObjects); an /AP /N with several states and no /AS contributes nothing; image, inline-image and stroked-path covers are still out of scope. The crate docs and README state these. Tests (crates/audit/src/tests.rs), red before the fix: 0 findings instead of 1 for annotation_appearance_over_text_is_found (Square and Highlight), annotation_appearance_is_placed_by_rect_bbox_and_matrix and annotation_appearance_state_selects_the_stream, and the audit returned Ok instead of an error for undecodable_annotation_appearance_fails_the_audit; hidden_or_noview_annotation_appearance_is_not_a_cover guards the other direction. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
…ever its colour The faux-redaction audit counted a filled rectangle as a cover only when the WCAG contrast between its fill colour and the text's fill colour was below 3. That measured how alike the two colours were, not whether the text was still visible: grey, red or coloured text under an opaque black box, and black text under an opaque white, light-grey, mid-grey or yellow box, all audited clean with exit status 0 while the text was still extractable and the renderer showed one flat colour. The gate existed to keep highlights quiet. The invariant now: an opaque fill painted after the text and hiding glyphs of it is a cover, whatever the two colours. What keeps highlights quiet is what actually leaves text visible: the walker drops fills with /ca below 1 (as before) and now also fills drawn with a blend mode other than Normal or Compatible (/BM as a name or the first of an array), as a Multiply highlight is. An ExtGState sets only the entries it has (ISO 32000-2 §8.4.5): a `gs` whose dictionary has no /ca no longer resets the alpha to opaque. The fill colour is no longer tracked or reported (FilledRect loses its private rgb field; the public finding is unchanged). The module docs, README and the doc_audit_redactions tool description state the rule; the parity note is the parent's to update. Expect findings on documents that paint opaque light rectangles over text for other reasons (shading added after the text): they do hide the text, and the docs say so. Tests (crates/audit/src/tests.rs), red before the fix: 0 findings instead of 1 for 0.4-grey text under a black box in opaque_cover_hides_text_whatever_the_colours (also red for the red-text, white-out, 0.9-grey, 0.5-grey, yellow and CMYK-white cases), blended_highlight_is_not_a_cover reported 0 instead of 1 for an opaque yellow box in /BM /Normal, and ext_gstate_without_an_entry_leaves_it_unchanged reported 1 finding for a fill left translucent by the first `gs`. The Multiply and Darken cases of blended_highlight_is_not_a_cover were red with the colour gate removed and /BM not yet read. graphics_state_carries_across_streams now carries the alpha instead of the colour; light_shading_after_text_is_not_flagged asserted the old behaviour and is replaced. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
PageText::find_opts_limited (Find, and Find text and redact: phrase and word
list) compared the page's glyph code points with the query after case folding
and white-space collapsing only, and PageText::find_with (pattern search:
SSN, phone, card, date) handed the raw code points to the matcher without
collapsing white space. Text whose glyphs map to a ligature (U+FB01 "fi",
U+FB03 "ffi", also produced from glyph names when a font has no ToUnicode), a
soft hyphen (U+00AD), a zero width space (U+200B), a decomposed accent
(e + U+0301) or a typographic dash (U+2011, U+2013) was not found by what the
user typed, and so not marked for redaction, while the plain occurrences were
marked and a non-zero count reported.
The invariant now: the query and the page text go through the same
normalisation before they are compared (search_chars): compatibility
ligatures U+FB00..FB06 are spelled out, soft hyphens and zero width
characters (U+00AD, U+200B..200D, U+2060, U+FEFF) are dropped, dash variants
(U+2010..2015, U+2212, U+FE58, U+FE63, U+FF0D) read as "-", white space is a
space and a run of spaces is one space on both paths, and the precomposed
letters of Latin-1 Supplement and Latin Extended-A are decomposed into base
letter plus combining mark (their canonical decomposition, from
UnicodeData.txt, written by hand: no new dependency) so composed and
decomposed spellings compare equal. Case folding follows the normalisation;
U+0130 is still folded by fold_case. Every produced character keeps its glyph
owner, so a match covers exactly the glyphs it came from, and a glyph that
produces nothing (a soft hyphen) inside a match stays inside it. A combining
mark continues a word for whole-word search. find_with keeps the original
case as before.
Still not covered: precomposed letters outside Latin-1 Supplement and Latin
Extended-A (Vietnamese, Latin Extended Additional) are not decomposed;
quotation-mark variants are not folded; the GUI message still gives one total
and does not name words that matched nowhere.
Tests (crates/render/src/text.rs), red before the fix:
search_matches_text_the_way_it_reads reported [] instead of [0..11] for
find("Confidential") over a line set with an fi-ligature glyph (the soft
hyphen, zero width space, decomposed accent and dash cases fail the same way);
pattern_search_sees_normalised_text reported [] instead of [4..15] for an SSN
set with U+2011 separators. decomposition_table_is_sorted_and_canonical pins
the table's order and contents.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The Action Wizard batch runner (engine::actions::apply_step) turned every
error from Session::apply into a log line and went on, so that steps with
nothing to do were not failures. But Sanitize, Remove hidden information and
Flatten are refused outright on a file with a signature field that has a
value (a full rewrite would break the signatures), on one whose /P forbids
changes, and on one with a page whose content cannot be read, and that
refusal was logged the same way: run_on saved the unsanitized file and
returned Ok, the desktop Action Wizard wrote it into the results folder and
said "N files done", and MCP action_run listed it with an output path and no
error.
The invariant now: only RedactError::NothingToApply (the step found nothing
to remove) is a skip, logged as "<step>: nothing to do"; every other error
fails the file: run_on returns Err("<step>: <reason>") and nothing is
written. The desktop Action Wizard already lists an Err under "failed" with
the reason and action_run already returns an error entry for it, so neither
needs a change. The other step kinds return Ok when they have nothing to do
(Flatten with nothing to flatten returns Ok(0)), so no other benign error
exists to let through.
Still not covered: a file that fails is not retried with the remaining
steps. Seen while writing the test, not fixed here (redact crate): Sanitize's
verify pass skips a page whose content cannot be read when no other page has
hidden text to remove (content_pass continues past it unless it is writing),
so such a file sanitizes "clean" without that page ever being checked; the
test therefore puts invisible text on page 1 so the write pass runs and page
2's unreadable content is refused.
Test (crates/engine/src/tests.rs)
actions_fail_the_file_when_a_sanitizing_step_is_refused, red before the fix:
"Sanitize Documents produced the file as done, log ["Sanitize document:
Sanitize document isn't possible in a signed document: it would rewrite the
file and invalidate the signatures"]" for a file with a bare /V signature
field; the /P-restricted and unreadable-page cases and "Prepare for
Distribution" on the signed file fail the same way, and the controls (a plain
file sanitized; a step with nothing to do skipped) pass before and after.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
The multi-file sinks (write_files_then behind Split, Extract pages as
separate files, the Accessibility report and Export certificate, and the
Export sink behind Export to Text, Image and All Images) joined each output
name onto the chosen folder with dir.join(name) and no check. The names come
from the document: an embedded PDF opened from the Attachments panel is named
by the outer file's /UF or /F string as its author wrote it, and a signer's
certificate export is named by its CN. A name with "../" segments or an
absolute path then resolved beside or outside the chosen folder, and
write_atomically replaced whatever file was there by rename.
The invariant now: every name reaching a sink is made exactly one safe file
name first (safe_file_name: the stem cleaned the way file_stem_from_user
cleans a typed name, the short alphanumeric extension kept, "document" when
nothing usable is left), and the sink itself refuses any name that is not a
single normal path component whose join has the chosen folder as its parent
(inside), reporting it instead of writing. A PDF attachment opened in its own
tab is now named by attachment_file_name, the cleaned name already used for
its Save suggestion (the function is compiled on every target now), so Save
As and the file sinks start from a plain name; the Attachments panel still
lists the raw name.
Still not covered: the single-file Save As and Save Attachment dialogs are
native pickers the user confirms, and are unchanged; Windows drive-relative
forms are refused by the sink but were not exercised through the GUI.
Tests (crates/ui-egui/src/files.rs, sink_tests), red before the fix:
written_files_never_leave_the_chosen_folder found the user's file beside the
chosen folder replaced ("1" instead of "USER DATA") by a file named
"../escaped.txt"; exported_text_never_leaves_the_chosen_folder found
"notes.txt" written beside the chosen folder for a document named
"../notes.pdf". safe_file_names_are_one_component_with_the_extension_kept
pins the cleaning and the sink's refusal.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
Export all images, Save image as and the images embedded in Export to Word/HTML/RTF resolve an image's /ColorSpace before decoding it. For an /Indexed space the base was resolved by calling space() on it again, and the "nested Indexed" check ran only after that call returned, so a base that is a reference back to the colour-space array itself (or a cycle of two Indexed arrays) recursed until the thread's stack overflowed. A stack overflow aborts the process, which catch_unwind cannot intercept: a ~600-byte PDF took the whole app, or the MCP / run --script session and every other open document, down. The base is now resolved with a depth counter: an /Indexed base is never itself Indexed (ISO 32000-2 §8.6.6.3), so one level is followed and a second is refused with the existing "nested indexed colour spaces aren't valid" error. The image is reported as skipped, as other unsupported colour spaces are. Nothing else about colour-space resolution changes. Not covered: the GUI export worker still runs on a default-size thread stack; no other recursion in extract.rs depends on document structure (collect() already has a depth cap). Test: pdfcraft-create a_self_referential_indexed_colour_space_is_refused_not_followed (red before the fix: the test thread overflowed its stack and the test process aborted with STATUS_STACK_OVERFLOW). Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
… decoders Stream::filters and Stream::filters_in now hand their names and parameter dictionaries to one private filter_chain, which holds the /Crypt skip and the five parameter defaults once; the two differ only in how an integer parameter is read (as written, or resolved through the document). decoded_strict, decoded_strict_in and decoded_first_filter_strict share one decode_strict (empty chain: the stored bytes under the limit; otherwise the strict decode). No behaviour change: the crate's tests pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
inline_image returned (Dict, Vec<u8>, usize, bool), with a bare true or false at
its three return sites. It now returns a private struct InlineImage { params,
data, end, terminated }, so each site says which outcome it is and the parser
reads image.end and image.terminated by name.
No behaviour change: the crate's tests pass unchanged.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
find_opts_limited and find_with each walked the glyphs, inserted word and line separators and collapsed runs of spaces. Both now take the text and each character's glyph (None for an inserted separator) from one private search_text, and map a character range back to glyphs with one glyph_range. The collapse test, written three times, is is_redundant_space, used for the query as well. fold_char is a plain match followed by to_lowercase. No behaviour change: the crate's tests pass unchanged. The one deliberate difference: a hand-built PageText whose line_of or space_before is shorter than its glyphs no longer panics in find_opts_limited; it reads them with get, as find_with already did. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A pure move out of the 2,900-line text.rs: search_chars, fold_char, is_combining and the hand-written DECOMPOSE table, with the test decomposition_table_is_sorted_and_canonical (same name; the parity checklist does not cite it), go to the private sibling module search_fold.rs. The three functions text.rs calls are pub(crate); the table stays private. Two doc links to fold_case now name its path. Nothing else changes. No behaviour change: the crate's tests pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
walk_annotation did four jobs in one body. Which annotations are walked (Hidden, NoView, popups and, moved in from filled_rects, /Redact marks) is is_painted; the Algorithm 8.1 matrix is the pure placement; cutting what the appearance added to /Rect is Walker::clip_since. Rectangle arrays are read by one cover::rect (numbers::<4>, then min/max), used for /Rect and /BBox and by redact_marks in lib.rs, which accepted exactly the same arrays by hand. No behaviour change: the crate's tests pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
write_files_then and the export sink each placed a cleaned name with inside, wrote it with write_atomically and carried their own copy of "the name is not a plain file name". Both now call files::write_inside(dir, name, bytes), which places and writes and returns the reason as the error; each keeps its own message around it, and inside is private to files.rs and its tests. No behaviour change: the crate's tests pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MAX_CONTENT_BYTES was declared in text.rs and again in objects.rs with the same value, kept equal by a comment. It is now one pub(crate) const in text.rs, which objects.rs imports. No behaviour change: the crate's tests pass unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
a_page_repeated_under_a_large_inherited_resources_is_bounded hand-rolled the writer that doc_with holds a few lines above. The writer is now a test helper open(objs), used by both; it writes the same bytes as before. Test code only: no assertion changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…pers space_audit_of_a_deep_form_chain_does_not_overflow_the_stack built each form's /Resources << /XObject << /F … >> >> by hand; it now uses the resources and xobjects helpers, which build the same dictionaries. The form dictionaries stay hand-built: the form helper differs (no /Type, a 1×1 /BBox, Flate content), so using it would change the fixture. Test code only: no assertion changed. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Refactor for readability, on top of the green head (7f2b437), plus one regression fix. The fix comes first: 4614119 makes an applied redaction mark take its pop-up with it again. Removing annotations by position (so that ones written directly in Everything after it is behaviour-preserving: each commit is one step, and after every source commit the crate's existing tests pass unchanged (tests are only touched in separate, test-only commits).
Checked locally on the branch head: the suites of cos (74), content (9), render (272), audit (24), edit (134), model (3), optimize (14), sign (119) and redact (61, including the new tests); clippy |
Contributor
|
Compared ddf05cb with our five open redaction PRs. I carried ten of our regression cases into a disposable checkout: seven pass, three still retain secret data after a full save. I adapted the thumbnail report assertion, mask counters and empty
The same form case also retains the original |
This was referenced Oct 14, 2026
13 tasks done
magdale76
added a commit
to magdale76/pdfcraft
that referenced
this pull request
Oct 14, 2026
…ge goes whole Review of storytold#1772: the cleared copy cloned the original dictionary, so /Alternates and /Metadata (an XMP packet can hold a thumbnail) still pointed at versions of the picture with the covered pixels, and a /SMask or stencil /Mask kept its own samples of the same area. The copy now drops /Alternates and /Metadata, for JPEG and lossless images alike, and an image with a soft or stencil mask is removed whole (fail-closed), as in storytold#1151; a colour-key /Mask array holds no samples and is still cleared. storytold#1949's mask clearing can replace the fallback once it lands. Both regressions check the bytes of the full rewrite redaction forces, and fail with the fix reverted (the metadata check also on its own). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
magdale76
added a commit
to magdale76/pdfcraft
that referenced
this pull request
Oct 15, 2026
…ge goes whole Review of storytold#1772: the cleared copy cloned the original dictionary, so /Alternates and /Metadata (an XMP packet can hold a thumbnail) still pointed at versions of the picture with the covered pixels, and a /SMask or stencil /Mask kept its own samples of the same area. The copy now drops /Alternates and /Metadata, for JPEG and lossless images alike, and an image with a soft or stencil mask is removed whole (fail-closed), as in storytold#1151; a colour-key /Mask array holds no samples and is still cleared. storytold#1949's mask clearing can replace the fallback once it lands. Both regressions check the bytes of the full rewrite redaction forces, and fail with the fix reverted (the metadata check also on its own). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
11 of 12 tasks
9 tasks done
Contributor
|
Integration update: #1151 has now merged. #1772 also fixed the issue raised in our overlap discussion: its shared copy_dict drops /Alternates and /Metadata on both JPEG and lossless copies, with saved-byte regressions for an XMP thumbnail. This head (ddf05cb) removes alternates but still clones the image's /Metadata in clear_samples, so please carry that additional cleanup and regression forward. Separately, #1346 was closed unmerged; current main still lacks selected-page /Thumb removal during plain apply, so the earlier thumbnail gap is not resolved by that closure. |
Closed
Brings the branch up to main 924c4da. Where main now fixes the same issue, main's code is taken and ours is dropped. Where main changed the code under one of our fixes, the fix is ported onto main's version. Taken from main, ours dropped: - crates/model: main's PageTree iterator (storytold#1926) bounds page-tree memory for repeated kids. Our a_page_repeated_under_a_large_inherited_ resources_is_bounded passes on it. Ours marked a kid seen when queued, which would break main's depth-first inheritance test. - crates/optimize: main's MAX_FORM_DEPTH (storytold#2118) bounds the space audit's form walk. Our space_audit_of_a_deep_form_chain_does_not_ overflow_the_stack passes on it. Both files equal main's. - crates/audit covers(): main's word-based measure (storytold#2662) finds a box over one word of a long line. Our line-height rule and our two tests that duplicate main's are dropped. The four-adjacent-boxes test and the rules-are-not-covers test stay. - crates/redact image masks: main removes an image with a separate /SMask or /Mask whole, which is stricter than clearing the mask into a copy. Our mask clearing and its test are dropped. - crates/ui-egui/tests/create.rs: main pinned the clock the same way. Ported onto main's code: - crates/render search: main moved literal search into text/search.rs as a KMP matcher (storytold#1725). It now runs over our normalised needle and page text (ligatures, soft hyphens, accents, dashes) and maps hits through glyph_range. search_text takes main's tolerant line-break test. Main's reference oracle in tests/search.rs learns the one rule our normalisation needs: a combining mark continues a word, as the composed letter does. Our search_matches_text_the_way_it_reads fails on main's search module. - crates/redact patterns: main's storytold#1884 rejects the head or tail of a longer number as a phone number. Our cut-back at a space now applies to a phone number only when the part kept is punctuated like one (- . ( )), since spaces alone also group one longer number. Main's phone tests pass unchanged. "555-123-4567 12 Elm Street" is still found, and card numbers keep their cut-backs. - crates/redact image.rs: main's checked sizing and mask policy, with our strict decode, layout checks and /OPI removal. /OPI gets a test, an_opi_original_does_not_keep_redacted_pixels, which fails without the removal. - crates/content: main's unfiltered-length end comes first, then our marker-aware end_of_image. The unterminated flag is kept. - crates/create: our depth-bounded space_at, then main's hival check. - crates/sign: main's revision(doc, ..) and storytold#1787 resource rule, with our field lock and dangling-reference arm first. The dangling test now names the undefined object from an annotation's /AP, which main's storytold#1787 walk does not reach. Its old form passed on main through storytold#1787. It fails with the arm reverted. - crates/edit: main moved a content read in rewrite_styled_block. It takes the `?` our per-page budget needs. - crates/redact: main made the test stream helper generic and grouped marks per page, so the tests pass their data by value and apply passes the page's marks without another borrow (clippy). Docs, README and parity text follow the code that survived. light_shading_after_text_is_not_flagged leaves the audit's evidence: an opaque light box over text does hide it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Review of the merge with main: - parity (protect.redact-patterns) and ROADMAP: the phone cut-back keeps main's storytold#1884 rule ahead of it. So no number at all is found in a run of numbers joined by single spaces longer than 28 characters (a column of three numbers, or two written as (555) 123-4567), the first included. The parity note lists that as missing, and the ROADMAP line no longer claims more than the card and punctuated-phone cases. For the same reason the merge dropped this branch's assertion that "555-123-4567 555-987-6543" finds both numbers. - crates/render/tests/search.rs: the reference oracle is independent only for text search normalisation leaves alone. Its comment now says the alphabet must avoid ligatures, soft hyphens, zero-width characters, dash variants and precomposed letters. - crates/sign/tests/pdf.rs: the dangling-appearance fixture writes its content stream on one line with \n escapes, like the rest of the file. - ROADMAP.md and docs/roadmap.md: this branch's status lines carry today's date. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings the branch up to main bc7a12b (six commits). Main's 08b44e8 keeps output file names inside the chosen folder: write_files refuses a name that is not plain (is_plain_file_name), the attachment tab is named with attachment_file_name, and Export certificate names its file the same way. That covers what this branch's 9d041ee did in write_files_then, so main's code is taken there: a name with a separator is refused, not rewritten. Main's write_files_refuses_names_that_leave_the_folder replaces this branch's written_files_never_leave_the_chosen_folder. Main left Export's own sink (export_ui.rs) joining a name made from the document's name onto the folder. That part of 9d041ee stays: safe_file_name, then write_inside, which now checks with main's is_plain_file_name instead of a second rule of its own (inside() is gone). exported_text_never_leaves_the_chosen_folder fails with main's sink ("notes.txt" lands beside the chosen folder) and passes with this one. The lib.rs conflict was the comment on the same attachment_file_name call; main's is kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Contributor
Author
|
Merged Where main now fixes the same issue, I took main's code and dropped ours:
Where main changed the code under a fix, I ported the fix:
I re-checked every remaining fix on the merged tree by undoing it alone and watching its test fail. The description is updated, and the merge commit messages list each resolution. 🤖 Generated with Claude Code |
Brings the branch up to main be0254d: plain XML form values keep all their text (storytold#2686), and cargo-deny acknowledges the new zbus unmaintained advisories (storytold#3091, which main's own licence check needed). Only the roadmap status lines and a revision-history row conflicted. Main's storytold#2686 replaced a status line in each file, so this branch's line stays first, followed by main's. The xfdf change merged cleanly beside this branch's FDF walk bound, and the xfdf suite passes. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings the branch up to main 645016a (Release 0.9.0). Main's storytold#3081 joins a page's content streams with a space, as renderers do, and records which stream boundaries a comment crosses, so that a redaction rewrite keeps a comment that hid the next stream's first line hidden. Text reading now reads the streams joined, in one pass. Both meet this branch's content and redaction changes. - crates/content: one internal parse_pieces(data, ends, max_ops) now sits behind parse, parse_capped, Pieces::parse and Pieces::parse_capped. Parsed carries both this branch's unterminated_inline_image and main's comment_edges. Pieces::parse_capped records comment edges too; it used to parse the joined bytes without the piece boundaries. Both sides' tests kept. - crates/edit reading_lines: main's single joined read, with the same per-page operator cap as text_lines (refused past MAX_OPS, not read in part), in place of this branch's per-stream budget. - crates/redact process: this branch still collects each piece (`Built`), since stripping replacement text can change an earlier piece; main's comment-edge wrapping moves to the loop that serializes them. apply compares tags on main's space-joined streams with this branch's tag resources. Both sides' tests kept. - crates/audit tests: main's new comment tests use the two_streams helper, which this branch had removed with the light-shading test (6410252). It is back, as main has it. - ROADMAP.md: both sides' progress-log entries kept. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Brings the branch up to main 12fcc64. Main's storytold#2812 makes files_and_quit_from_the_operating_system wait for the tab before it deletes the file the open worker reads, the same fix this branch carried (7f2b437). Main's version is taken, so this branch no longer changes crates/ui-egui/tests/ui.rs. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
12 of 13 tasks
11 tasks done
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What this changes
The medium-severity findings of a defensive security review of the codebase (synthetic fixtures only, no personal data), one commit per finding, each with the red-before-fix evidence in its message. The high-severity findings are in #1916; this branch is based on
mainindependently of it, so whichever merges second needs a rebase (both touchcrates/sign/src/pdf.rsclassify_changes, the sign README and the parity notes).Signatures (M9). An OCSP responder carried in a response is a delegate by how it was found, whatever subject name it copies. A certificate trusted for signing is not thereby a time authority: a timestamp token is trusted time only from a certificate with id-kp-timeStamping. The byte range must exclude exactly the
/Contentsof the object carrying the signature dictionary, so a second field borrowing a genuine signature's range and value is invalid. Defining an object the signed revision referenced but never defined is page content. Form changes after signing are judged by what changed, not by where: a field is form fill only when nothing but its value and what is regenerated from it changed, a new field or widget is a comment-level change unless it is a signed signature field, and under P=1 the one signature change permitted is a document timestamp. A field the signer locked (FieldMDP references, else the signed field's/Lock, which signing now writes as a reference) may not be filled in afterwards.Scripts (M6). Source a script builds at run time for
eval()orFunction()is screened as the script itself was (boaHostHooks::ensure_can_compile_strings).Apply Redactions (M1/M8), fail-closed where it guessed before. Streams are decoded strictly and as the document declares them (an indirect
/Filteror/DecodeParmsis resolved; truncated or corrupt data, an out-of-range predictor,/For/DPon the dictionary are errors —Stream::filters_inandStream::decoded_strict_in, additive). An inline image noEIends is reported by the content parser (Parsed::unterminated_inline_image, and>EI/~>EIend ASCIIHex/ASCII85 data) and refused. Vertical writing set in the CMap program or by the predefined CMap V fails closed. Text set through an ExtGState/Fontor inherited by a form from its caller is removed. A partly covered image is cleared only when its sample layout is certain to be what the page is drawn with (otherwise removed whole); a cleared copy drops/OPIas well as/Alternates. Fields are deleted by object, not by name; annotations written directly in/Annotsare removed; removed annotations, widgets and fields are nulled so the structure tree, a reply's/IRTand/AcroForm /COcannot carry them into the save. Replacement text around removed glyphs (/ActualText,/Alt,/Ein BDC property lists, and on the owning structure element and its ancestors,/Tincluded) is stripped. A card number that runs into another number is still found, and so is a phone number punctuated like one (main's #1884 rule that the head or tail of a longer number is not a phone number stands).Remove Hidden Information and Sanitize (M8). Metadata, private data and associated files are removed from every object reachable from the catalog, not a fixed list; Hidden and NoView widgets are removed with the other fields; the images and forms an off layer's block alone drew go with it; multimedia (Screen, Movie, RichMedia, 3D annotations and their media,
/Names /Renditionsand/AlternatePresentations, 3D/OnInstantiate),/Aand/AAon every annotation, and page thumbnails are covered and counted.Faux-redaction audit (M8) and Find (M2). An opaque fill in a Normal blend mode painted after the text is a cover whatever its colour; annotation appearance streams are walked like page content. Find, Find text and redact and the pattern search compare text the way it reads (ligatures, soft hyphens, zero-width characters, dash variants, decomposed accents of Latin-1 Supplement and Latin Extended-A).
Bounded work (M2/M4/M6/M7). The Indexed colour-space base is bounded when exporting images; page content read for text is budgeted per page (
content::parse_capped, additive) and the text interpreter's save stack is capped; the comment summary writes each reply once whatever/NMrepeats; XFA layout charges its measurement budget per visited subform; FDF import bounds its walk over aliased/Kids; flattening judges each field-tree node once; an OCR result is refused when the document changed since recognition started.Action Wizard and file sinks (M3/M4). A refused sanitizing step fails the file instead of writing the unsanitized file as done; Export's files land directly inside the chosen folder, whatever the document is called (main's 08b44e8 now refuses a name that is not plain in every other folder write).
Behaviour users will notice: Apply Redactions and Remove Hidden Information now refuse a page whose stream does not decode strictly (a damaged page a viewer draws partially) rather than guess; partly covered images with an uncertain layout are removed whole; the audit reports light boxes over text; Flatten keeps leaving hidden annotations alone. The parity notes say what each feature still lacks.
Design-level follow-ups, not attempted here: cutting the geometry of paths clipped by a redaction; a memory budget for the script engine; predefined Type0 CMaps and tiling patterns in redaction; one page list shared by the model, annotation and organize crates; showing a reconstructed document's own table; trust-store roles (signer, issuer, TSA).
How you verified it
Per crate, scoped to what each commit touched, never the workspace. Each commit was gated on its crate's suite (
cargo test -p <crate> -j 2), clippy-D warningsand fmt; every regression test was run red before its fix (the output is quoted in each commit message).Merged with main 924c4da (4224099), then bc7a12b (bb37a1e). Where main now fixes the same issue, main's code is taken and ours dropped: page-tree memory (#1926), the space audit's form depth (#2118), a cover over one word of a long line (#2662), images with a separate mask (removed whole, #1151 — stricter than clearing the mask), folder writes through
write_files(08b44e8, which refuses a name that is not plain where this branch rewrote it). Where main changed the code under a fix, the fix is ported: search normalisation runs on main's KMP matcher (#1725), the phone cut-back keeps #1884's rule, the inline-image end keeps main's sample-length end first (#1562), the Indexed bound sits before main's hival check (#1581), and the change classifier keeps #1787's resource rule and #1808's signed-revision key. The merge commit messages list each resolution. Each fix that remains was re-checked on the merged tree by undoing it alone and watching its test fail.On the merged tree:
cargo clippy --all-targets -- -D warningsover the 16 touched crates;cargo fmt --all --check; the full suites of audit, content, cos, create, edit, fonts, forms, js, redact, render, sign, xfa, xfdf and engine; automation filtered by name (-- audit redact sign flatten import search sanitiz hidden, 28 tests);pdfcraft-cli --test audit; ui-egui--test create --test ui;cargo xtask parity,layersandassets— all green.After the second merge (main's six commits touch ui-egui, engine links and the build): clippy
-D warningson ui-egui and engine, the engine suite, the ui-egui file-name tests (--lib -- sink_tests stem_tests an_exported_certificate export),--test ui --test pickers,cargo xtask parityandassets— green.exported_text_never_leaves_the_chosen_folderfails with main's export sink.cargo fmt --check-D warningson the crates I changedcargo xtask layersandcargo xtask assetsNever-crash
unwrap/expect/panic!/unreachable!/todo!outside testsunsafeProvenance
Security
--rootconfinement unchangedParity and reach
parity/acrobat-features.tomlupdated (protect.redact-*, protect.apply-redactions, protect.redaction-verification, protect.remove-hidden-information, protect.sanitize, protect.audit-faux-redactions, sign.field-mdp, sign.lock-after-signing, sign.certify-permissions, sign.changes-after-signing, sign.ocsp-crl, sign.pades-bt, sign.document-timestamp, form.js-security, view.find-options), each saying what is still missingAnything a reviewer should look at first
Unreadableon any page stream that does not decode strictly and completely, where before it proceeded on the partial decode. That is the deliberate restraint: a redaction must not be reported applied on bytes a viewer reads differently.classify_changesincrates/sign/src/pdf.rsnow takes the field lock and judges form fill by the entries that changed; Security: bind signature verdicts and redaction to what the page can show #1916 edits the same function, so the second of the two to merge needs a rebase.🤖 Generated with Claude Code