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 <ejbarraza1994@gmail.com>

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,
 }
 

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<u8> {
+    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:?}");
+}