From 61f734e6337885836d4e3f0b40fb795f11469e1d Mon Sep 17 00:00:00 2001 From: Tong Date: Thu, 13 Aug 2026 00:49:31 +0900 Subject: [PATCH] uphold 1.1.1: one answer about a file that is not text `uphold scan` reads `.gitattributes`, skips a path declared `-text` and says it skipped it -- the use its own doc comment names, a captured page kept byte-for-byte in the encoding its venue served. The guard over the same files did not read the declaration: it opened the bytes, failed to decode them, and refused. So a repository that took one of the three cures the reference names 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, and the comment beside it is still right about what it is for: it is the guess about bytes NOBODY declared. A declaration is not a guess, so where there is one it answers first, and where the declaration itself could not be read the refusal stands with that reason attached. The fixture builder here now writes its policy after `git init` rather than before, because several fixtures carry a policy meant to be refused at load -- and on a machine where this binary is installed in front of `git`, the shim reads that policy before it knows whether anything stands in front of the command, so the setup failed in the shape of the subject. --- Cargo.lock | 2 +- Cargo.toml | 2 +- README.md | 6 ++--- hooks/lefthook.yml | 4 ++-- src/guard/unicode.rs | 42 ++++++++++++++++++++++++++++++++++- tests/guard_cli.rs | 53 +++++++++++++++++++++++++++++++++++++++++++- 6 files changed, 100 insertions(+), 9 deletions(-) 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