uphold 1.1.1: one answer about a file that is not text

Cargo.toml

@@ -1,6 +1,6 @@
 [package]
 name = "uphold"
-version = "1.1.0"
+version = "1.1.1"
 edition = "2021"
 # 1.88, not 1.85: the ripgrep stack this crate embeds -- globset 0.4.20 and
 # ignore 0.4.33 in Cargo.lock -- refuses anything older. The 1.85 that stood here

README.md

@@ -25,7 +25,7 @@ bootstraps).
 default_install_hook_types: [pre-commit, commit-msg, pre-merge-commit, pre-push]
 repos:
   - repo: https://github.com/HackingGate/uphold
-    rev: v1.1.0
+    rev: v1.1.1
     hooks:
       - id: uphold-check            # the claims still hold
       - id: uphold-scan             # the content policy
@@ -48,13 +48,13 @@ the binary must be on PATH.
 # lefthook.yml
 remotes:
   - git_url: https://github.com/HackingGate/uphold
-    ref: v1.1.0
+    ref: v1.1.1
     configs:
       - hooks/lefthook.yml
 ```
 
 ```sh
-cargo install --git https://github.com/HackingGate/uphold --tag v1.1.0
+cargo install --git https://github.com/HackingGate/uphold --tag v1.1.1
 ```
 
 ## Declare what enforces what

hooks/lefthook.yml

@@ -10,7 +10,7 @@
 #   # lefthook.yml, in the consuming repository
 #   remotes:
 #     - git_url: https://github.com/HackingGate/uphold
-#       ref: v1.1.0
+#       ref: v1.1.1
 #       configs:
 #         - hooks/lefthook.yml
 #
@@ -21,7 +21,7 @@
 # their manifest contract; lefthook runs commands and has no such contract, so
 # `uphold` must be on PATH:
 #
-#   cargo install --git https://github.com/HackingGate/uphold --tag v1.1.0
+#   cargo install --git https://github.com/HackingGate/uphold --tag v1.1.1
 #
 # The names below are prefixed `uphold-` on purpose. A remote config is
 # merged into the consumer's own, and two commands sharing a name under one hook

src/guard/unicode.rs

@@ -35,6 +35,7 @@ use globset::{Glob, GlobMatcher};
 use super::scope;
 use super::{Refusal, Request};
 use crate::error::{Fatal, Result};
+use crate::selection::{normalize_rel, not_text_paths};
 
 /// A codepoint admitted, optionally only under one path glob.
 struct Allowance {
@@ -273,6 +274,24 @@ pub(crate) fn in_files(request: &Request<'_>) -> Result<Option<Refusal>> {
         request.push_source,
         request.remote_name,
     )?;
+    // The same declaration `uphold scan` reads, from the same place, because a
+    // file the repository declares is not text is one file with one answer and
+    // not two. It said so at the tree seam and refused at this one: a captured
+    // page kept byte-for-byte in the encoding its venue served -- the use
+    // `not_text_paths` names -- was skipped by the scan and made every commit
+    // touching it exit 2 here, and `.gitattributes` is one of the three cures
+    // the reference names for exactly that.
+    //
+    // The NUL test below keeps its job, which is a different one: it is the
+    // guess about bytes NOBODY declared. A declaration is not a guess, and
+    // where there is one it answers first. A `.gitattributes` that could not be
+    // read leaves the list empty and the reason set, and then the refusal
+    // stands with that reason attached -- an unanswered question is not a
+    // declaration that a file is fine to skip.
+    let (not_text, unmeasured) = not_text_paths(request.root);
+    let declared_not_text: BTreeSet<&str> =
+        not_text.iter().map(|path| normalize_rel(path)).collect();
+    let mut skipped: Vec<&str> = Vec::new();
     let mut findings: Vec<String> = Vec::new();
     let mut looked = 0usize;
 
@@ -294,6 +313,12 @@ pub(crate) fn in_files(request: &Request<'_>) -> Result<Option<Refusal>> {
         if !blob.has_content() {
             continue;
         }
+        // After the name and before the bytes. The name is committed text
+        // whatever the content is declared to be.
+        if let Some(path) = declared_not_text.get(normalize_rel(&blob.path)) {
+            skipped.push(path);
+            continue;
+        }
         let bytes = scope::read(request.root, blob)?;
         match decode_for_scan(&bytes) {
             Decoded::Text(text) => {
@@ -307,15 +332,30 @@ pub(crate) fn in_files(request: &Request<'_>) -> Result<Option<Refusal>> {
             // reported as a file with nothing in it -- `explicit-unknown` by
             // name, in the guard that reports it about everyone else.
             Decoded::Unreadable(why) => {
+                let unknown = unmeasured.as_deref().unwrap_or(
+                    "Declare it not text in .gitattributes, declare its charset with an \
+                     `encoding` rule, or exclude it from this rule.",
+                );
                 return Err(Fatal::new(format!(
                     "{}: cannot be read as text ({why}); refusing to report it clean \
-                     over content that was never examined",
+                     over content that was never examined. {unknown}",
                     blob.path
                 )));
             }
         }
     }
 
+    // Said on the way past, refusal or not, for the reason `not_text_paths`
+    // gives about its own two answers: "we did not check these" and "these were
+    // clean" must never look the same on the way out.
+    if !skipped.is_empty() {
+        eprintln!(
+            "{}: {} path(s) skipped, declared not text in .gitattributes:\n{}",
+            request.rule.id,
+            skipped.len(),
+            skipped.join("\n")
+        );
+    }
     if findings.is_empty() {
         return Ok(None);
     }

tests/guard_cli.rs

@@ -26,11 +26,19 @@ fn repository(policy: &str) -> PathBuf {
     ));
     let _ = std::fs::remove_dir_all(&root);
     std::fs::create_dir_all(root.join("policy")).unwrap();
-    std::fs::write(root.join("policy/principles.toml"), policy).unwrap();
 
+    // Git first, policy second, and the order is load-bearing on any machine
+    // where this binary is installed in front of `git`. Several fixtures here
+    // carry a policy that is MEANT to be refused at load, and the shim reads
+    // the policy of the directory the command was typed in before it knows
+    // whether anything stands in front of that command -- so writing the file
+    // first made `git init` exit 2 inside the fixture, and the failure looked
+    // like the test's own subject rather than like its setup.
     git(&root, &["init", "-q", "-b", "main"]);
     git(&root, &["config", "user.name", "Test"]);
     git(&root, &["config", "user.email", "test@example.test"]);
+
+    std::fs::write(root.join("policy/principles.toml"), policy).unwrap();
     root
 }
 
@@ -227,6 +235,49 @@ fn an_allowance_scoped_to_a_path_admits_the_character_only_there() {
     );
 }
 
+/// One file, one answer, at both seams.
+///
+/// A captured page kept byte-for-byte in the encoding its venue served is the
+/// use `.gitattributes` `-text` exists for, and `uphold scan` skips one and says
+/// so. This guard read the same file, failed to decode it, and refused -- so a
+/// repository following the documented cure got a clean tree from one seam and
+/// exit 2 from the other on every commit that touched the file.
+///
+/// The NUL test keeps its own job below: it is the guess about bytes nobody
+/// declared, and a declaration is not a guess.
+#[test]
+fn a_path_declared_not_text_is_skipped_by_the_guard_that_cannot_decode_it() {
+    let root =
+        repository("[rule.prevent-unusual-unicode-in-files]\nbuiltin = \"prevent-unusual-unicode-in-files\"\n\n[rule.prevent-unusual-unicode-in-files.git]\nhooks = [\"pre-commit\"]\n");
+    // Shift-JIS bytes: valid text in their own encoding, not UTF-8, and no NUL
+    // to make the binary guess fire.
+    std::fs::write(root.join("captured.html"), [0x93, 0xFA, 0x96, 0x7B, 0x0A]).unwrap();
+    git(&root, &["add", "-A"]);
+
+    let output = guard(&root, &["--stage", "pre-commit"]);
+    assert_eq!(
+        code(&output),
+        2,
+        "undeclared bytes that will not decode are still a surface nobody read:\n{}",
+        stderr(&output)
+    );
+
+    write(&root, ".gitattributes", "captured.html -text\n");
+    git(&root, &["add", "-A"]);
+    let output = guard(&root, &["--stage", "pre-commit"]);
+    assert_eq!(code(&output), 0, "{}", stderr(&output));
+    assert!(
+        stderr(&output).contains("skipped, declared not text"),
+        "a skipped file and a clean file must not look alike:\n{}",
+        stderr(&output)
+    );
+    assert!(
+        stderr(&output).contains("captured.html"),
+        "{}",
+        stderr(&output)
+    );
+}
+
 #[test]
 fn a_symlinks_blob_is_its_target_path() {
     // Git stores the TARGET PATH as the blob, so a link whose target carries a