The publication audit reads each reachable blob through its encoding, and reports one that does not decode as unread

docs/REFERENCE.md

@@ -2634,6 +2634,17 @@ partial clone — is reported as a surface this run could not read, and so is ex
 `2`. It is not skipped: an object the audit could not open is not an object the
 audit found clean.
 
+Each blob is decoded the way the guards decode it: a byte-order mark is
+honoured, so a UTF-16 file is searched as the text it holds rather than as
+replacement characters. A blob that is neither text nor binary — Latin-1, or a
+UTF-16 mark over bytes that do not decode as UTF-16 — is reported as a surface
+this run could not read, is left out of the count of surfaces read, and so is
+exit `2`. It is still searched for what its bytes spell in ASCII, so a name
+written in it is reported too, and that finding makes the exit `1`: a violation
+outranks an unread surface, and both are printed. A binary blob is still
+searched, where the guards skip it, because a name written into one as plain
+bytes is published by the flip all the same.
+
 The edit history is a **standing caveat**, not an unreadable surface. It is true
 of every run, on every repository, and nothing about this run could change it —
 so it is stated in the body of every report and is *not* counted as something

src/audit.rs

@@ -47,6 +47,7 @@ use std::path::Path;
 use crate::config::{CheckKind, Policy, Rule};
 use crate::error::{Exit, Fatal, Result, verdict};
 use crate::git;
+use crate::guard::scope::{self, Decoded};
 use crate::guard::{Refusal, names};
 
 /// True of every run of this subcommand, on every repository, forever.
@@ -67,6 +68,11 @@ const STANDING_CAVEATS: &[&str] = &[
 struct Surface {
     label: String,
     text: String,
+    /// Whether the text is what the surface says. False for a blob whose bytes
+    /// would not decode: it is still searched, for whatever its bytes spell in
+    /// ASCII, but it is listed as unread and not counted among the surfaces
+    /// read, because nobody read the rest of it.
+    read: bool,
 }
 
 /// The rule to judge with, forced to the visibility being flipped TO.
@@ -178,6 +184,7 @@ fn commit_surfaces(listed: &str, what: &str) -> Vec<Surface> {
         .map(|(sha, body)| Surface {
             label: format!("{what} {}", &sha[..sha.len().min(8)]),
             text: body.to_owned(),
+            read: true,
         })
         .collect()
 }
@@ -371,6 +378,7 @@ fn read_conversation(
         Ok(text) => surfaces.push(Surface {
             label: format!("{kind} #{number} title, body and comments"),
             text,
+            read: true,
         }),
         Err(reason) => unreadable.push(format!("{kind} #{number} could not be read: {reason}")),
     }
@@ -392,6 +400,7 @@ fn read_conversation(
         Ok(text) => surfaces.push(Surface {
             label: format!("pr #{number} review bodies"),
             text,
+            read: true,
         }),
         Err(reason) => unreadable.push(format!(
             "pr #{number} review bodies could not be read: {reason}"
@@ -406,6 +415,7 @@ fn read_conversation(
         Ok(text) => surfaces.push(Surface {
             label: format!("pr #{number} review-thread comments"),
             text,
+            read: true,
         }),
         Err(reason) => unreadable.push(format!(
             "pr #{number} review-thread comments could not be read: {reason}"
@@ -529,13 +539,42 @@ fn reachable_blobs(root: &Path) -> Result<(Vec<Surface>, Vec<String>)> {
     let absent = git::each_blob(root, &shas, |sha, bytes| {
         read += 1;
         progress(read, total);
+        let path = paths.get(sha).map_or("?", String::as_str);
+        // Through the one decoder the guards read the same blobs with. This was
+        // `String::from_utf8_lossy`, which turned a UTF-16 file into replacement
+        // characters with NULs between them: `github.com/acme/secret` inside one
+        // matched nothing, the blob was counted among the surfaces read, and a
+        // run with a reachable forge ended on "every one of them is clean", exit
+        // 0, over a name the same text in UTF-8 is refused for -- immediately
+        // before the flip that cannot be taken back.
+        let (text, decoded) = match scope::decode(bytes) {
+            Decoded::Text(text) => (text, true),
+            // Still read, and read the way this audit always read it. The
+            // guards skip a binary blob because it has no lines to point at;
+            // this audit has no lines to point at either, and a name written
+            // into a binary as plain bytes is served by the flip all the same.
+            // Narrowing to the guards' reading would turn a finding this made
+            // into silence.
+            Decoded::Binary => (String::from_utf8_lossy(bytes).into_owned(), true),
+            // Two facts, and both are reported. The blob is not a surface read
+            // clean, so it goes to the list of what this run failed to open and
+            // is not counted as read. And it is still searched as it always
+            // was, because a lossy reading keeps every ASCII byte: a Latin-1
+            // file with `github.com/acme/secret` in it was a finding before
+            // this decoder existed, and recording the file as unread alone
+            // would turn that finding into a line saying nothing about it.
+            Decoded::Unreadable(why) => {
+                unreadable.push(format!(
+                    "{path} (blob {sha}) is reachable and could not be read as text ({why}); \
+                     only what its bytes spell in ASCII was searched"
+                ));
+                (String::from_utf8_lossy(bytes).into_owned(), false)
+            }
+        };
         surfaces.push(Surface {
-            label: format!(
-                "{} (blob {})",
-                paths.get(sha).map_or("?", String::as_str),
-                &sha[..8.min(sha.len())]
-            ),
-            text: String::from_utf8_lossy(bytes).into_owned(),
+            label: format!("{path} (blob {})", &sha[..8.min(sha.len())]),
+            text,
+            read: decoded,
         });
     })?;
     // An object the audit could not open is not an object the audit found clean.
@@ -695,7 +734,18 @@ pub(crate) fn for_publication(root: &Path, policy: &Policy) -> Result<Exit> {
         }
     }
 
-    println!("{} surface(s) read", surfaces.len());
+    // Counted by what was read, not by what was searched: a blob that would not
+    // decode was searched for what it spells in ASCII and is listed as unread
+    // below, and a total that included it would claim a reading nobody did.
+    let read = surfaces.iter().filter(|surface| surface.read).count();
+    println!("{read} surface(s) read");
+    let partly = surfaces.len().saturating_sub(read);
+    if partly > 0 {
+        println!(
+            "{partly} more searched only for what their bytes spell in ASCII, and listed as \
+             unread below"
+        );
+    }
     for refusal in &refusals {
         eprintln!("would be republished: {}", refusal.report.trim_end());
         eprintln!();

tests/audit_publication_cli.rs

@@ -730,3 +730,139 @@ fn a_commit_only_on_a_retained_pull_ref_is_read() {
     assert!(report.contains("retained pull ref"), "{report}");
     let _ = std::fs::remove_dir_all(&origin);
 }
+
+/// UTF-16 bytes as a file on disk holds them, byte-order mark first.
+fn utf16le(text: &str) -> Vec<u8> {
+    let mut bytes = vec![0xFF, 0xFE];
+    for unit in text.encode_utf16() {
+        bytes.extend_from_slice(&unit.to_le_bytes());
+    }
+    bytes
+}
+
+/// A private name in a UTF-16 file is found.
+///
+/// The blobs were read with `String::from_utf8_lossy`, which makes a UTF-16
+/// file replacement characters with NULs between them. The name inside matched
+/// nothing and the blob was still counted as read, so with a forge that
+/// answered, this fixture ended on "every one of them is clean" and exit 0 --
+/// the report the flip is taken on.
+#[test]
+fn a_name_in_a_utf16_file_is_found() {
+    let root = repository();
+    let origin = origin_for(&root, "publication-utf16-origin");
+    std::fs::write(
+        root.join("NOTES.txt"),
+        utf16le("we hit this in PrivateOrg first\n"),
+    )
+    .unwrap();
+    support::git(&root, &["add", "-A"]);
+    support::git(&root, &["commit", "-qm", "notes", "--no-verify"]);
+    support::git(&root, &["push", "-q", "origin", "main"]);
+
+    // A stub `gh`, for the reason `audit_with_gh` gives, and so that the forge
+    // half reads clean: what is left to decide the exit is the blob.
+    let output = audit_with_gh(&root, "exit 0\n");
+    let report = text(&output);
+    assert_eq!(
+        code(&output),
+        1,
+        "the name is in the file, written in UTF-16:\n{report}"
+    );
+    assert!(report.contains("NOTES.txt (blob "), "{report}");
+    let _ = std::fs::remove_dir_all(&origin);
+}
+
+/// A blob that is neither text nor binary is a surface this run did not read.
+///
+/// Latin-1 is the ordinary case: one byte that is not UTF-8 and no NUL to call
+/// the file an image. Read lossily it was counted among the surfaces read and
+/// the run could exit 0, a coverage claim over text nobody decoded.
+#[test]
+fn a_blob_that_does_not_decode_is_reported_unread() {
+    let root = repository();
+    let origin = origin_for(&root, "publication-undecodable-origin");
+    std::fs::write(root.join("latin1.txt"), b"caf\xe9 au lait\n").unwrap();
+    support::git(&root, &["add", "-A"]);
+    support::git(&root, &["commit", "-qm", "one", "--no-verify"]);
+    support::git(&root, &["push", "-q", "origin", "main"]);
+
+    let output = audit_with_gh(&root, "exit 0\n");
+    let report = text(&output);
+    assert_eq!(
+        code(&output),
+        2,
+        "a blob nobody could decode is not a blob found clean:\n{report}"
+    );
+    let (_, measured) = report.split_once("could NOT be read:").unwrap();
+    assert!(measured.contains("latin1.txt (blob "), "{report}");
+    assert!(
+        !report.contains("every surface a flip would republish"),
+        "{report}"
+    );
+    // Searched, but not counted as read: the total names only what was.
+    assert!(
+        report.contains("1 more searched only for what their bytes spell in ASCII"),
+        "{report}"
+    );
+    let _ = std::fs::remove_dir_all(&origin);
+}
+
+/// A name in ASCII inside a blob that does not decode is still found.
+///
+/// Recording the blob as unread is half the answer. A lossy reading keeps every
+/// ASCII byte, so the name in this Latin-1 file was a finding before the audit
+/// learned to decode blobs, and reporting only "could not be read" would turn
+/// that finding into a line that says nothing about it. Both are reported; the
+/// finding decides the exit, as a violation outranks an unread surface.
+#[test]
+fn a_name_in_a_blob_that_does_not_decode_is_still_found() {
+    let root = repository();
+    let origin = origin_for(&root, "publication-undecodable-name-origin");
+    std::fs::write(
+        root.join("latin1.txt"),
+        b"caf\xe9 au lait, as PrivateOrg serves it\n",
+    )
+    .unwrap();
+    support::git(&root, &["add", "-A"]);
+    support::git(&root, &["commit", "-qm", "one", "--no-verify"]);
+    support::git(&root, &["push", "-q", "origin", "main"]);
+
+    let output = audit_with_gh(&root, "exit 0\n");
+    let report = text(&output);
+    assert_eq!(
+        code(&output),
+        1,
+        "the name is plain ASCII in a file that is not UTF-8:\n{report}"
+    );
+    // The finding, on stderr where every finding is printed.
+    let findings = String::from_utf8_lossy(&output.stderr);
+    assert!(findings.contains("would be republished"), "{report}");
+    assert!(findings.contains("latin1.txt (blob "), "{report}");
+    // And the same blob listed as unread, on stdout with the rest of the report.
+    let stdout = String::from_utf8_lossy(&output.stdout);
+    let (_, measured) = stdout.split_once("could NOT be read:").unwrap();
+    assert!(measured.contains("latin1.txt (blob "), "{report}");
+    let _ = std::fs::remove_dir_all(&origin);
+}
+
+/// A name written into a binary blob as plain bytes is still found.
+///
+/// The guards skip a binary blob, for want of lines to point at. This audit
+/// read every blob's bytes before it learned to decode them, and the decoder
+/// is not a reason to stop: the flip serves the binary with the name in it.
+#[test]
+fn a_name_in_a_binary_blob_is_still_found() {
+    let root = repository();
+    let origin = origin_for(&root, "publication-binary-origin");
+    std::fs::write(root.join("blob.bin"), b"\x00\x01\xffPrivateOrg\x00\n").unwrap();
+    support::git(&root, &["add", "-A"]);
+    support::git(&root, &["commit", "-qm", "one", "--no-verify"]);
+    support::git(&root, &["push", "-q", "origin", "main"]);
+
+    let output = audit_with_gh(&root, "exit 0\n");
+    let report = text(&output);
+    assert_eq!(code(&output), 1, "{report}");
+    assert!(report.contains("blob.bin (blob "), "{report}");
+    let _ = std::fs::remove_dir_all(&origin);
+}