diff --git a/Cargo.lock b/Cargo.lock index 39b2b7a..3d2bb06 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -501,7 +501,7 @@ checksum = "673aac59facbab8a9007c7f6108d11f63b603f7cabff99fabf650fea5c32b861" [[package]] name = "uphold" -version = "1.1.0" +version = "1.1.1" dependencies = [ "encoding_rs", "globset", diff --git a/Cargo.toml b/Cargo.toml index 4bd4614..7777a43 100644 --- a/Cargo.toml +++ b/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 diff --git a/README.md b/README.md index 4e9ddf3..489bafa 100644 --- a/README.md +++ b/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 diff --git a/hooks/lefthook.yml b/hooks/lefthook.yml index 726dfec..514e16d 100644 --- a/hooks/lefthook.yml +++ b/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 diff --git a/src/guard/unicode.rs b/src/guard/unicode.rs index b3a8cef..378498a 100644 --- a/src/guard/unicode.rs +++ b/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> { 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 = Vec::new(); let mut looked = 0usize; @@ -294,6 +313,12 @@ pub(crate) fn in_files(request: &Request<'_>) -> Result> { 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> { // 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); } diff --git a/tests/guard_cli.rs b/tests/guard_cli.rs index 53d2cac..1e94304 100644 --- a/tests/guard_cli.rs +++ b/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