fix(redact): scrub unknown binary parts

.changeset/redact-binary-allowlist.md

@@ -0,0 +1,5 @@
+---
+"@betteroffice/rust-crates": patch
+---
+
+Scrub unrecognized binary parts during redaction. A part is kept only when it is recognized media or XML; every other part is emptied, and it is removed outright — together with its owned relationship part, that part's exclusive targets and its content-type declaration — when no surviving relationship points at it. The XML rewriter and the scrubber now share one reading of relationship markup, so they cannot disagree about which targets leave the package, and a part is only removed when every surviving relationship resolves to a stored entry.

crates/ooxml-redact/src/lib.rs

@@ -1,11 +1,15 @@
 mod media;
+mod rels;
+mod scrub;
 mod xml;
 
+use std::collections::HashSet;
 use std::fmt;
 
 use thiserror::Error;
 
 use crate::media::replace_media;
+use crate::scrub::{normalize_part_name, prune_scrubbed_parts};
 use crate::xml::redact_xml;
 
 #[derive(Clone, Copy, Debug, Default, Eq, PartialEq)]
@@ -90,15 +94,25 @@ pub fn redact_with_report(
         format: detected,
         ..RedactionReport::default()
     };
+    let scrubbed: HashSet<String> = parts
+        .iter()
+        .map(|(path, _)| normalize_part_name(path))
+        .filter(|name| !media::is_replaceable_part(name) && !is_xml_part(name))
+        .collect();
+    report.binary_parts = scrubbed.len();
+    let blanked = if scrubbed.is_empty() {
+        HashSet::new()
+    } else {
+        prune_scrubbed_parts(&mut parts, &scrubbed)?
+    };
     for (path, data) in &mut parts {
-        let lower = path.to_ascii_lowercase();
-        if media::is_replaceable_part(&lower) {
-            *data = replace_media(path, data, &mut report)?;
-        } else if is_xml_part(&lower) {
-            *data = redact_xml(detected, path, data, &mut report)?;
-        } else if is_sensitive_binary(&lower) {
+        let canonical = normalize_part_name(path);
+        if blanked.contains(&canonical) {
             data.clear();
-            report.binary_parts += 1;
+        } else if media::is_replaceable_part(&canonical) {
+            *data = replace_media(&canonical, data, &mut report)?;
+        } else {
+            *data = redact_xml(detected, &canonical, data, &mut report)?;
         }
     }
 
@@ -109,7 +123,7 @@ pub fn redact_with_report(
 fn detect_parts(parts: &[(String, Vec<u8>)]) -> Result<Format, RedactError> {
     if let Some((_, content_types)) = parts
         .iter()
-        .find(|(path, _)| path.eq_ignore_ascii_case("[Content_Types].xml"))
+        .find(|(path, _)| normalize_part_name(path) == "[content_types].xml")
     {
         let text = String::from_utf8_lossy(content_types).to_ascii_lowercase();
         if text.contains("wordprocessingml.document.main+xml")
@@ -149,12 +163,5 @@ fn is_xml_part(path: &str) -> bool {
     path.ends_with(".xml") || path.ends_with(".rels") || path.ends_with(".vml")
 }
 
-fn is_sensitive_binary(path: &str) -> bool {
-    path.ends_with("vbaproject.bin")
-        || path.contains("/embeddings/")
-        || path.contains("/activex/") && path.ends_with(".bin")
-        || path.contains("/printersettings/")
-}
-
 #[cfg(test)]
 mod tests;

crates/ooxml-redact/src/rels.rs

@@ -0,0 +1,74 @@
+//! One reading of OPC relationship markup, shared by the XML rewriter and the
+//! scrubber so the two cannot disagree about which targets leave the package.
+
+/// Whether the attribute name carries no namespace prefix. Only unqualified
+/// OPC attributes take part in relationship decisions.
+pub(crate) fn is_unqualified(key: &str) -> bool {
+    !key.contains(':')
+}
+
+pub(crate) fn attribute_local(name: &str) -> &str {
+    name.rsplit_once(':').map_or(name, |(_, local)| local)
+}
+
+/// The value of an unqualified attribute with this name. OPC names are
+/// case-sensitive, so the exact spelling a consumer reads wins over a tolerated
+/// variant appearing earlier.
+pub(crate) fn unqualified_value<'a>(
+    attributes: &'a [(String, String)],
+    expected: &str,
+) -> Option<&'a str> {
+    let mut variant = None;
+    for (name, value) in attributes {
+        if !is_unqualified(name) {
+            continue;
+        }
+        if name == expected {
+            return Some(value.as_str());
+        }
+        if variant.is_none() && name.eq_ignore_ascii_case(expected) {
+            variant = Some(value.as_str());
+        }
+    }
+    variant
+}
+
+/// Whether a relationship's attributes mark it as pointing outside the
+/// package. `package_part` enables reading the target's shape, which only a
+/// `.rels` part's consumers resolve; a shape is read from the exact-case
+/// `Target` those consumers use.
+pub(crate) fn external_relationship(attributes: &[(String, String)], package_part: bool) -> bool {
+    attributes.iter().any(|(key, value)| {
+        if !is_unqualified(key) {
+            return false;
+        }
+        let local = attribute_local(key);
+        local.eq_ignore_ascii_case("TargetMode") && value.trim().eq_ignore_ascii_case("External")
+            || package_part && local == "Target" && external_target(value)
+    })
+}
+
+/// Whether a relationship target points outside the package. Query and
+/// fragment are dropped first: neither names a part, and either may carry a
+/// URI of its own.
+pub(crate) fn external_target(target: &str) -> bool {
+    let lower = target
+        .trim()
+        .split(['?', '#'])
+        .next()
+        .unwrap_or_default()
+        .to_ascii_lowercase();
+    lower.starts_with("//")
+        || lower.starts_with(r"\\")
+        || lower
+            .split_once(':')
+            .is_some_and(|(scheme, _)| is_uri_scheme(scheme))
+}
+
+fn is_uri_scheme(scheme: &str) -> bool {
+    let mut chars = scheme.chars();
+    if !matches!(chars.next(), Some(first) if first.is_ascii_alphabetic()) {
+        return false;
+    }
+    chars.all(|character| character.is_ascii_alphanumeric() || matches!(character, '+' | '-' | '.'))
+}