From f0c877ca2379779aa21dbd587e27f9c5bf069e18 Mon Sep 17 00:00:00 2001 From: ejbarraza <72894266+ejbarraza@users.noreply.github.com> Date: Wed, 14 Oct 2026 23:27:44 -0700 Subject: [PATCH] audit: harden finding classes with golden tests and docs (#1892) - Pin FindingKind stable IDs (covered-text, unapplied-mark) with a test and document the no-rename contract on id() - Golden tests pinning the full AuditFinding struct for both finding classes (page, kind, exact rect, covered text) - Depth-limit boundary tests: 32 nested form XObjects audit clean, 33 fail closed with TooDeep - Fix stale crate docs: m/l axis-aligned rectangles count as covers (#1510) - Document AuditFinding.rect normalization and covered_text joining Tests + docs only, no behavior change. Co-authored-by: Erick Barraza --- crates/audit/src/lib.rs | 24 +++++++---- crates/audit/src/tests.rs | 91 ++++++++++++++++++++++++++++++++++++++- 2 files changed, 106 insertions(+), 9 deletions(-) diff --git a/crates/audit/src/lib.rs b/crates/audit/src/lib.rs index ad26c2559..23965f6c2 100644 --- a/crates/audit/src/lib.rs +++ b/crates/audit/src/lib.rs @@ -9,12 +9,13 @@ //! unreadable (WCAG contrast below 3.0). Page backgrounds, table shading and highlight //! marks are therefore not flagged. //! -//! Detection limits (documented, not hidden): only `re` rectangles closed by a fill operator -//! are considered — arbitrary filled paths, shadings and patterns are not; a rectangle drawn -//! with a translucent ExtGState (`/ca < 1`) is not flagged; annotation rectangles are compared -//! in unrotated user space, so pages with `/Rotate` may misalign; covers inside form XObjects -//! are found, but only text drawn directly in the page content is checked under them, so text -//! inside a form XObject is not audited. +//! Detection limits (documented, not hidden): `re` rectangles and single closed `m`/`l` +//! subpaths whose four corners form an axis-aligned rectangle in user space (#1510) are +//! considered when closed by a fill operator — other filled paths, shadings and patterns are +//! not; a rectangle drawn with a translucent ExtGState (`/ca < 1`) is not flagged; annotation +//! rectangles are compared in unrotated user space, so pages with `/Rotate` may misalign; +//! covers inside form XObjects are found, but only text drawn directly in the page content +//! is checked under them, so text inside a form XObject is not audited. //! //! The audit fails closed: any content that cannot be read (an undecodable content stream, //! malformed annotations, form XObjects nested deeper than 32 or in a cycle) is an error, never @@ -40,6 +41,11 @@ pub enum FindingKind { } impl FindingKind { + /// The stable machine-readable ID for this finding class. + /// + /// These strings are the crate's public vocabulary: they appear in CLI output and are + /// the IDs a future machine-readable format would serialize. Do not rename them; + /// add a new variant instead. pub fn id(self) -> &'static str { match self { FindingKind::CoveredText => "covered-text", @@ -54,9 +60,11 @@ pub struct AuditFinding { /// 0-based page index. pub page: usize, pub kind: FindingKind, - /// The covering rectangle (or the mark's rectangle), in page user space. + /// The covering rectangle (or the mark's rectangle), in page user space, normalized so + /// `rect[0] <= rect[2]` and `rect[1] <= rect[3]`. pub rect: [f64; 4], - /// The extractable text the cover hides (possibly empty when nothing decodable remains). + /// The extractable text the cover hides, lines joined with single spaces and trimmed. + /// Possibly empty when nothing decodable remains under the cover. pub covered_text: String, } diff --git a/crates/audit/src/tests.rs b/crates/audit/src/tests.rs index a9397145a..ff20f5e84 100644 --- a/crates/audit/src/tests.rs +++ b/crates/audit/src/tests.rs @@ -5,7 +5,7 @@ use std::sync::Arc; use pdfcraft_cos::Document; -use crate::{FindingKind, audit_redactions}; +use crate::{AuditFinding, FindingKind, audit_redactions}; /// Minimal PDF assembler: objects are numbered 1..=n in order. struct Builder { @@ -281,3 +281,92 @@ fn non_rectangular_path_is_not_a_cover() { assert!(findings.is_empty(), "{:?}: {findings:?}", String::from_utf8_lossy(cover)); } } + +#[test] +fn finding_kind_ids_are_stable() { + // The string IDs are the crate's public vocabulary (CLI output, future machine-readable + // formats). They must not change; add a variant instead of renaming. + assert_eq!(FindingKind::CoveredText.id(), "covered-text"); + assert_eq!(FindingKind::UnappliedMark.id(), "unapplied-mark"); +} + +#[test] +fn covered_text_finding_is_fully_determined() { + // Golden test: the whole finding, not just its presence, is pinned. + let content = b"BT /F1 24 Tf 100 700 Td (SECRET DATA 123) Tj ET\n0 g\n100 688 220 32 re f\n"; + let findings = audit_redactions(&open(one_page(content, ""))).expect("audit must run"); + assert_eq!( + findings, + vec![AuditFinding { + page: 0, + kind: FindingKind::CoveredText, + rect: [100.0, 688.0, 320.0, 720.0], + covered_text: "SECRET DATA 123".to_string(), + }] + ); +} + +#[test] +fn unapplied_mark_finding_is_fully_determined() { + // Golden test for the second finding class, with its exact mark rectangle. + let content = b"BT /F1 24 Tf 100 700 Td (DO NOT SHARE) Tj ET\n"; + let page_extra = "/Annots [6 0 R]"; + let mut b = Builder::new(); + b.add("<< /Type /Catalog /Pages 2 0 R >>"); + b.add("<< /Type /Pages /Kids [3 0 R] /Count 1 >>"); + b.add(&format!("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R /Resources << /Font << /F1 5 0 R >> >> {page_extra} >>")); + b.add_stream(content); + b.add(FONT); + b.add("<< /Type /Annot /Subtype /Redact /Rect [95 685 305 725] /C [1 0 0] >>"); + let findings = audit_redactions(&open(b.build())).expect("audit must run"); + assert_eq!( + findings, + vec![AuditFinding { page: 0, kind: FindingKind::UnappliedMark, rect: [95.0, 685.0, 305.0, 725.0], covered_text: "DO NOT SHARE".to_string() }] + ); +} + +/// A page whose content draws a chain of `n` nested form XObjects. The innermost form draws +/// nothing, so a successful audit reports no findings. +fn nested_forms(n: usize) -> Vec { + let mut b = Builder::new(); + b.add("<< /Type /Catalog /Pages 2 0 R >>"); + b.add("<< /Type /Pages /Kids [3 0 R] /Count 1 >>"); + // Object 3: page; object 4: content; objects 5..5+n: the form chain. + b.add("<< /Type /Page /Parent 2 0 R /MediaBox [0 0 612 792] /Contents 4 0 R /Resources << /XObject << /F1 5 0 R >> >> >>"); + b.add_stream(b"/F1 Do"); + for i in 1..=n { + let obj = 4 + i; + if i < n { + let next = obj + 1; + let name = format!("/F{}", i + 1); + b.add_stream_with( + format!("/{name} Do", name = &name[1..]).as_bytes(), + &format!( + "/Type /XObject /Subtype /Form /BBox [0 0 612 792] /Resources << /XObject << /F{i} {next} 0 R >> >>", + i = i + 1, + next = next + ), + ); + } else { + b.add_stream_with(b"q Q", "/Type /XObject /Subtype /Form /BBox [0 0 612 792]"); + } + debug_assert_eq!(b.objects.len(), obj); + } + b.build() +} + +#[test] +fn forms_nested_at_depth_limit_audit_clean() { + // 32 nested forms is the deepest walk the audit follows: it must succeed and report + // the page clean rather than fail. + let findings = audit_redactions(&open(nested_forms(32))).expect("audit must run"); + assert!(findings.is_empty(), "{findings:?}"); +} + +#[test] +fn forms_nested_past_depth_limit_fail_closed() { + // The 33rd nesting level cannot be followed: the audit fails rather than report the + // page clean with a possible cover left unseen. + let result = audit_redactions(&open(nested_forms(33))); + assert!(matches!(result, Err(crate::AuditError::TooDeep)), "{result:?}"); +}