From c490e6e86724cbc380ee5e4a3c4fe909403cbc5c Mon Sep 17 00:00:00 2001 From: Ismar Iljazovic Date: Sat, 20 Dec 2025 10:13:23 +0100 Subject: [PATCH 1/6] Implement `check-hook-updates` builtin hook (#1243) Add a new builtin hook that checks if configured hooks have newer versions available. Supports: - `--cooldown-days`: minimum release age for eligible updates - `--fail-on-updates`: fail instead of just warning Closes #1243 --- crates/prek/src/cli/auto_update.rs | 10 +- crates/prek/src/cli/mod.rs | 2 +- crates/prek/src/hooks/builtin_hooks/mod.rs | 21 +- .../pre_commit_hooks/check_hook_updates.rs | 182 ++++++++++++++++++ crates/prek/src/hooks/pre_commit_hooks/mod.rs | 2 + crates/prek/tests/builtin_hooks.rs | 52 +++++ docs/builtin.md | 7 +- 7 files changed, 268 insertions(+), 8 deletions(-) create mode 100644 crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs diff --git a/crates/prek/src/cli/auto_update.rs b/crates/prek/src/cli/auto_update.rs index 2689beaae..e4b026317 100644 --- a/crates/prek/src/cli/auto_update.rs +++ b/crates/prek/src/cli/auto_update.rs @@ -223,7 +223,7 @@ async fn update_repo( Ok(Revision { rev, frozen }) } -async fn setup_and_fetch_repo(repo_url: &str, repo_path: &Path) -> Result<()> { +pub(crate) async fn setup_and_fetch_repo(repo_url: &str, repo_path: &Path) -> Result<()> { git::init_repo(repo_url, repo_path).await?; git::git_cmd("git config")? .arg("config") @@ -288,7 +288,7 @@ async fn resolve_bleeding_edge(repo_path: &Path) -> Result> { } /// Returns all tags and their Unix timestamps (newest first). -async fn get_tag_timestamps(repo: &Path) -> Result> { +pub(crate) async fn get_tag_timestamps(repo: &Path) -> Result> { let output = git::git_cmd("git for-each-ref")? .arg("for-each-ref") .arg("--sort=-creatordate") @@ -427,7 +427,11 @@ async fn checkout_and_validate_manifest( /// Multiple tags can exist on an SHA. Sometimes a moving tag is attached /// to a version tag. Try to pick the tag that looks like a version and most similar /// to the current revision. -async fn get_best_candidate_tag(repo: &Path, rev: &str, current_rev: &str) -> Result { +pub(crate) async fn get_best_candidate_tag( + repo: &Path, + rev: &str, + current_rev: &str, +) -> Result { let stdout = git::git_cmd("git tag")? .arg("tag") .arg("--points-at") diff --git a/crates/prek/src/cli/mod.rs b/crates/prek/src/cli/mod.rs index ecea98170..eb701bfca 100644 --- a/crates/prek/src/cli/mod.rs +++ b/crates/prek/src/cli/mod.rs @@ -13,7 +13,7 @@ use prek_consts::env_vars::EnvVars; use crate::config::{HookType, Language, Stage}; -mod auto_update; +pub(crate) mod auto_update; mod cache_clean; mod cache_gc; mod cache_size; diff --git a/crates/prek/src/hooks/builtin_hooks/mod.rs b/crates/prek/src/hooks/builtin_hooks/mod.rs index ea3998bc8..e31dd384c 100644 --- a/crates/prek/src/hooks/builtin_hooks/mod.rs +++ b/crates/prek/src/hooks/builtin_hooks/mod.rs @@ -1,7 +1,8 @@ -use anyhow::Result; use std::path::Path; use std::str::FromStr; +use anyhow::Result; + use crate::config::{BuiltinHook, HookOptions, Stage}; use crate::hook::Hook; use crate::hooks::pre_commit_hooks; @@ -29,6 +30,7 @@ pub(crate) enum BuiltinHooks { MixedLineEnding, NoCommitToBranch, TrailingWhitespace, + CheckHookUpdates, } impl FromStr for BuiltinHooks { @@ -52,6 +54,7 @@ impl FromStr for BuiltinHooks { "mixed-line-ending" => Ok(Self::MixedLineEnding), "no-commit-to-branch" => Ok(Self::NoCommitToBranch), "trailing-whitespace" => Ok(Self::TrailingWhitespace), + "check-hook-updates" => Ok(Self::CheckHookUpdates), _ => Err(()), } } @@ -91,6 +94,7 @@ impl BuiltinHooks { Self::TrailingWhitespace => { pre_commit_hooks::fix_trailing_whitespace(hook, filenames).await } + Self::CheckHookUpdates => pre_commit_hooks::check_hook_updates(hook, filenames).await, } } } @@ -289,6 +293,21 @@ impl BuiltinHook { ..Default::default() }, }, + BuiltinHooks::CheckHookUpdates => BuiltinHook { + id: "check-hook-updates".to_string(), + name: "check for hook updates".to_string(), + entry: "check-hook-updates".to_string(), + priority: None, + options: HookOptions { + description: Some( + "checks if configured hooks have newer versions available.".to_string(), + ), + pass_filenames: Some(false), + always_run: Some(true), + stages: Some(vec![Stage::PreCommit, Stage::PrePush, Stage::Manual]), + ..Default::default() + }, + }, }) } } diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs b/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs new file mode 100644 index 000000000..bc1c3d177 --- /dev/null +++ b/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs @@ -0,0 +1,182 @@ +use std::io::Write; +use std::path::Path; +use std::time::{SystemTime, UNIX_EPOCH}; + +use anyhow::Result; +use clap::Parser; + +use crate::cli::auto_update::{get_best_candidate_tag, get_tag_timestamps, setup_and_fetch_repo}; +use crate::config; +use crate::git::git_cmd; +use crate::hook::Hook; + +#[derive(Parser)] +#[command(disable_help_subcommand = true)] +#[command(disable_version_flag = true)] +#[command(disable_help_flag = true)] +struct Args { + /// Minimum release age (in days) required for a version to be eligible. + /// A value of `0` disables this check. + #[arg(long, value_name = "DAYS", default_value_t = 0)] + cooldown_days: u8, + + /// Fail the hook if updates are available (default: warn only). + #[arg(long, default_value_t = false)] + fail_on_updates: bool, +} + +/// Check if configured hooks have newer versions available. +pub(crate) async fn check_hook_updates( + hook: &Hook, + _filenames: &[&Path], +) -> Result<(i32, Vec)> { + let args = Args::try_parse_from(hook.entry.resolve(None)?.iter().chain(&hook.args))?; + + let project_config = hook.project().config(); + + let mut code = 0; + let mut output = Vec::new(); + + for repo in &project_config.repos { + if let config::Repo::Remote(remote_repo) = repo { + match check_repo_for_updates(remote_repo, args.cooldown_days).await { + Ok(Some(update_info)) => { + writeln!( + &mut output, + "{}: {} -> {} available", + remote_repo.repo, remote_repo.rev, update_info.new_rev + )?; + if args.fail_on_updates { + code = 1; + } + } + Ok(None) => { + // No update available or already up to date + } + Err(e) => { + writeln!( + &mut output, + "{}: failed to check for updates: {}", + remote_repo.repo, e + )?; + // Don't fail on network errors, just warn + } + } + } + } + + Ok((code, output)) +} + +struct UpdateInfo { + new_rev: String, +} + +async fn check_repo_for_updates( + repo: &config::RemoteRepo, + cooldown_days: u8, +) -> Result> { + let tmp_dir = tempfile::tempdir()?; + let repo_path = tmp_dir.path(); + + // Initialize and fetch the repo (lightweight fetch, tags only) + setup_and_fetch_repo(&repo.repo, repo_path).await?; + + // Get the latest eligible revision + let latest_rev = resolve_latest_revision(repo_path, &repo.rev, cooldown_days).await?; + + let Some(latest_rev) = latest_rev else { + return Ok(None); + }; + + // Check if the latest revision is different from the current one + if is_same_revision(repo_path, &repo.rev, &latest_rev).await? { + return Ok(None); + } + + Ok(Some(UpdateInfo { + new_rev: latest_rev, + })) +} + +async fn resolve_latest_revision( + repo_path: &Path, + current_rev: &str, + cooldown_days: u8, +) -> Result> { + let tags_with_ts = get_tag_timestamps(repo_path).await?; + + if tags_with_ts.is_empty() { + // No tags, try to get the latest commit from HEAD + return resolve_head_revision(repo_path).await; + } + + let cutoff_secs = u64::from(cooldown_days) * 86400; + let now = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); + let cutoff = now.saturating_sub(cutoff_secs); + + // tags_with_ts is sorted newest -> oldest; find the first bucket where ts <= cutoff. + let left = match tags_with_ts.binary_search_by(|(_, ts)| ts.cmp(&cutoff).reverse()) { + Ok(i) | Err(i) => i, + }; + + let Some((target_tag, _)) = tags_with_ts.get(left) else { + // All tags are too new + return Ok(None); + }; + + // Try to find the best candidate tag (prefer version-like tags) + let best = get_best_candidate_tag(repo_path, target_tag, current_rev) + .await + .unwrap_or_else(|_| target_tag.clone()); + + Ok(Some(best)) +} + +async fn resolve_head_revision(repo_path: &Path) -> Result> { + let output = git_cmd("git rev-parse")? + .arg("rev-parse") + .arg("FETCH_HEAD") + .check(false) + .current_dir(repo_path) + .output() + .await?; + + if output.status.success() { + Ok(Some( + String::from_utf8_lossy(&output.stdout).trim().to_string(), + )) + } else { + Ok(None) + } +} + +/// Check if two revisions point to the same commit. +async fn is_same_revision(repo_path: &Path, rev1: &str, rev2: &str) -> Result { + let hash1 = resolve_to_hash(repo_path, rev1).await?; + let hash2 = resolve_to_hash(repo_path, rev2).await?; + + match (hash1, hash2) { + (Some(h1), Some(h2)) => Ok(h1 == h2), + // If we can't resolve one of them, assume they're different + _ => Ok(false), + } +} + +async fn resolve_to_hash(repo_path: &Path, rev: &str) -> Result> { + let output = git_cmd("git rev-parse")? + .arg("rev-parse") + .arg(format!("{rev}^{{}}")) + .check(false) + .current_dir(repo_path) + .output() + .await?; + + if output.status.success() { + Ok(Some( + String::from_utf8_lossy(&output.stdout).trim().to_string(), + )) + } else { + Ok(None) + } +} diff --git a/crates/prek/src/hooks/pre_commit_hooks/mod.rs b/crates/prek/src/hooks/pre_commit_hooks/mod.rs index 63e71809b..1647481c7 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/mod.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/mod.rs @@ -9,6 +9,7 @@ use crate::hook::Hook; mod check_added_large_files; mod check_case_conflict; mod check_executables_have_shebangs; +mod check_hook_updates; pub(crate) mod check_json; mod check_merge_conflict; mod check_symlinks; @@ -25,6 +26,7 @@ mod no_commit_to_branch; pub(crate) use check_added_large_files::check_added_large_files; pub(crate) use check_case_conflict::check_case_conflict; pub(crate) use check_executables_have_shebangs::check_executables_have_shebangs; +pub(crate) use check_hook_updates::check_hook_updates; pub(crate) use check_json::check_json; pub(crate) use check_merge_conflict::check_merge_conflict; pub(crate) use check_symlinks::check_symlinks; diff --git a/crates/prek/tests/builtin_hooks.rs b/crates/prek/tests/builtin_hooks.rs index 39d660f43..6c5f76100 100644 --- a/crates/prek/tests/builtin_hooks.rs +++ b/crates/prek/tests/builtin_hooks.rs @@ -2212,6 +2212,7 @@ fn check_json5() -> Result<()> { Ok(()) } +<<<<<<< HEAD /// Test that builtin hooks work correctly even when a system-wide binary with the /// same name exists on PATH (regression test for ). /// @@ -2275,3 +2276,54 @@ fn builtin_hooks_ignore_system_path_binaries() -> Result<()> { Ok(()) } + +/// Tests that `check-hook-updates` hook is recognized as a valid builtin hook. +#[test] +fn check_hook_updates_hook_recognized() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: builtin + hooks: + - id: check-hook-updates + "}); + context.git_add("."); + + // The hook should be recognized and run (it will pass since there are no remote repos to check) + cmd_snapshot!(context.filters(), context.run(), @r" + success: true + exit_code: 0 + ----- stdout ----- + check for hook updates...................................................Passed + + ----- stderr ----- + "); +} + +/// Tests that `check-hook-updates` hook with `--fail-on-updates` argument is recognized. +#[test] +fn check_hook_updates_hook_with_args() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: builtin + hooks: + - id: check-hook-updates + args: ['--cooldown-days=7', '--fail-on-updates'] + "}); + context.git_add("."); + + // The hook should be recognized and run with the arguments + cmd_snapshot!(context.filters(), context.run(), @r" + success: true + exit_code: 0 + ----- stdout ----- + check for hook updates...................................................Passed + + ----- stderr ----- + "); +} diff --git a/docs/builtin.md b/docs/builtin.md index 0d3d2adca..01d3d9bfd 100644 --- a/docs/builtin.md +++ b/docs/builtin.md @@ -105,6 +105,7 @@ For `repo: builtin`, the following hooks are supported: - [`detect-private-key`](#detect-private-key) (Detect private keys) - [`no-commit-to-branch`](#no-commit-to-branch) (Prevent committing to protected branches) - [`check-executables-have-shebangs`](#check-executables-have-shebangs) (Ensures that (non-binary) executables have a shebang) +- [`check-hook-updates`](#check-hook-updates) (Check if configured hooks have newer versions available) ### Hook Reference @@ -140,7 +141,7 @@ Trims trailing whitespace from each line. - Preserves Markdown hard line breaks (two trailing spaces) for files with the given extension(s). - Use `--markdown-linebreak-ext=*` to treat **all** files as Markdown. - `--chars=` - - Trim only the specified set of characters instead of “all trailing whitespace”. + - Trim only the specified set of characters instead of "all trailing whitespace". - Example: `args: [--chars, " \t"]` (space + tab). **Caveats** @@ -283,7 +284,7 @@ Attempts to load all XML files to verify syntax. **Caveats** - Empty files are treated as invalid XML. -- Fails if there is “junk after the document element” (multiple top-level roots). +- Fails if there is "junk after the document element" (multiple top-level roots). --- @@ -380,4 +381,4 @@ Checks that non-binary executables have a proper shebang. **Caveats** - The check is intentionally lightweight: it only verifies that the file starts with `#!`. -- On systems where the executable bit is not tracked by the filesystem, `prek` consults git’s staged mode bits. +- On systems where the executable bit is not tracked by the filesystem, `prek` consults git's staged mode bits. From 83db4416ba09bf2cfde0616b94ca85a2541160de Mon Sep 17 00:00:00 2001 From: Ismar Iljazovic Date: Tue, 23 Dec 2025 12:18:31 +0100 Subject: [PATCH 2/6] refactor: extract shared tag-selection and rev-parse logic to eliminate duplication Extract shared functionality from auto_update.rs that check_hook_updates can reuse: - `find_eligible_tag`: cooldown-based tag selection with best candidate logic - `resolve_rev_to_commit_hash`: dereference a revision to its commit hash This eliminates duplicated code for: - Cooldown calculation and binary search for eligible tags - Git rev-parse logic for resolving revisions to commit hashes --- crates/prek/src/cli/auto_update.rs | 77 ++++++++++++++----- .../pre_commit_hooks/check_hook_updates.rs | 65 ++-------------- 2 files changed, 65 insertions(+), 77 deletions(-) diff --git a/crates/prek/src/cli/auto_update.rs b/crates/prek/src/cli/auto_update.rs index e4b026317..3eedd53f2 100644 --- a/crates/prek/src/cli/auto_update.rs +++ b/crates/prek/src/cli/auto_update.rs @@ -314,18 +314,19 @@ pub(crate) async fn get_tag_timestamps(repo: &Path) -> Result .collect()) } -async fn resolve_revision( +/// Find the best tag that meets the cooldown requirement. +/// +/// Given a list of tags sorted newest-to-oldest with timestamps, finds the first tag +/// that is at least `cooldown_days` old, then picks the best version-like tag pointing +/// to the same commit. +/// +/// Returns `None` if no tags meet the cooldown requirement. +pub(crate) async fn find_eligible_tag( repo_path: &Path, + tags_with_ts: &[(String, u64)], current_rev: &str, - bleeding_edge: bool, cooldown_days: u8, ) -> Result> { - if bleeding_edge { - return resolve_bleeding_edge(repo_path).await; - } - - let tags_with_ts = get_tag_timestamps(repo_path).await?; - let cutoff_secs = u64::from(cooldown_days) * 86400; let now = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); let cutoff = now.saturating_sub(cutoff_secs); @@ -335,35 +336,73 @@ async fn resolve_revision( Ok(i) | Err(i) => i, }; - let Some((target_tag, target_ts)) = tags_with_ts.get(left) else { - trace!("No tags meet cooldown cutoff {cutoff_secs}s"); + let Some((target_tag, _)) = tags_with_ts.get(left) else { return Ok(None); }; - debug!("Using tag `{target_tag}` cutoff timestamp {target_ts}"); - let best = get_best_candidate_tag(repo_path, target_tag, current_rev) .await .unwrap_or_else(|_| target_tag.clone()); - debug!("Using best candidate tag `{best}` for revision `{target_tag}`"); Ok(Some(best)) } -async fn freeze_revision(repo_path: &Path, rev: &str) -> Result> { - let exact = git::git_cmd("git rev-parse")? +async fn resolve_revision( + repo_path: &Path, + current_rev: &str, + bleeding_edge: bool, + cooldown_days: u8, +) -> Result> { + if bleeding_edge { + return resolve_bleeding_edge(repo_path).await; + } + + let tags_with_ts = get_tag_timestamps(repo_path).await?; + + let best = find_eligible_tag(repo_path, &tags_with_ts, current_rev, cooldown_days).await?; + + if let Some(ref tag) = best { + debug!("Using best candidate tag `{tag}`"); + } else { + trace!("No tags meet cooldown cutoff"); + } + + Ok(best) +} + +/// Resolve a revision (tag, branch, etc.) to its dereferenced commit hash. +/// +/// Returns `None` if the revision cannot be resolved. +pub(crate) async fn resolve_rev_to_commit_hash( + repo_path: &Path, + rev: &str, +) -> Result> { + let output = git::git_cmd("git rev-parse")? .arg("rev-parse") .arg(format!("{rev}^{{}}")) + .check(false) .current_dir(repo_path) .remove_git_envs() .output() - .await? - .stdout; - let exact = str::from_utf8(&exact)?.trim(); + .await?; + + if output.status.success() { + Ok(Some( + String::from_utf8_lossy(&output.stdout).trim().to_string(), + )) + } else { + Ok(None) + } +} + +async fn freeze_revision(repo_path: &Path, rev: &str) -> Result> { + let Some(exact) = resolve_rev_to_commit_hash(repo_path, rev).await? else { + return Ok(None); + }; if rev == exact { Ok(None) } else { - Ok(Some(exact.to_string())) + Ok(Some(exact)) } } diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs b/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs index bc1c3d177..580394423 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs @@ -1,13 +1,13 @@ use std::io::Write; use std::path::Path; -use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::Result; use clap::Parser; -use crate::cli::auto_update::{get_best_candidate_tag, get_tag_timestamps, setup_and_fetch_repo}; +use crate::cli::auto_update::{ + find_eligible_tag, get_tag_timestamps, resolve_rev_to_commit_hash, setup_and_fetch_repo, +}; use crate::config; -use crate::git::git_cmd; use crate::hook::Hook; #[derive(Parser)] @@ -111,50 +111,17 @@ async fn resolve_latest_revision( return resolve_head_revision(repo_path).await; } - let cutoff_secs = u64::from(cooldown_days) * 86400; - let now = SystemTime::now().duration_since(UNIX_EPOCH)?.as_secs(); - let cutoff = now.saturating_sub(cutoff_secs); - - // tags_with_ts is sorted newest -> oldest; find the first bucket where ts <= cutoff. - let left = match tags_with_ts.binary_search_by(|(_, ts)| ts.cmp(&cutoff).reverse()) { - Ok(i) | Err(i) => i, - }; - - let Some((target_tag, _)) = tags_with_ts.get(left) else { - // All tags are too new - return Ok(None); - }; - - // Try to find the best candidate tag (prefer version-like tags) - let best = get_best_candidate_tag(repo_path, target_tag, current_rev) - .await - .unwrap_or_else(|_| target_tag.clone()); - - Ok(Some(best)) + find_eligible_tag(repo_path, &tags_with_ts, current_rev, cooldown_days).await } async fn resolve_head_revision(repo_path: &Path) -> Result> { - let output = git_cmd("git rev-parse")? - .arg("rev-parse") - .arg("FETCH_HEAD") - .check(false) - .current_dir(repo_path) - .output() - .await?; - - if output.status.success() { - Ok(Some( - String::from_utf8_lossy(&output.stdout).trim().to_string(), - )) - } else { - Ok(None) - } + resolve_rev_to_commit_hash(repo_path, "FETCH_HEAD").await } /// Check if two revisions point to the same commit. async fn is_same_revision(repo_path: &Path, rev1: &str, rev2: &str) -> Result { - let hash1 = resolve_to_hash(repo_path, rev1).await?; - let hash2 = resolve_to_hash(repo_path, rev2).await?; + let hash1 = resolve_rev_to_commit_hash(repo_path, rev1).await?; + let hash2 = resolve_rev_to_commit_hash(repo_path, rev2).await?; match (hash1, hash2) { (Some(h1), Some(h2)) => Ok(h1 == h2), @@ -162,21 +129,3 @@ async fn is_same_revision(repo_path: &Path, rev1: &str, rev2: &str) -> Result Ok(false), } } - -async fn resolve_to_hash(repo_path: &Path, rev: &str) -> Result> { - let output = git_cmd("git rev-parse")? - .arg("rev-parse") - .arg(format!("{rev}^{{}}")) - .check(false) - .current_dir(repo_path) - .output() - .await?; - - if output.status.success() { - Ok(Some( - String::from_utf8_lossy(&output.stdout).trim().to_string(), - )) - } else { - Ok(None) - } -} From 86852b4b5c50e236ee6e54a4d07e96be0a66f4a3 Mon Sep 17 00:00:00 2001 From: Ismar Iljazovic Date: Tue, 23 Dec 2025 13:06:30 +0100 Subject: [PATCH 3/6] test: add tests for check-hook-updates with remote repos Add tests that actually exercise the update-checking logic by using a real remote repo (pre-commit/pre-commit-hooks) with an outdated version (v4.0.0). - check_hook_updates_detects_outdated_repo: verifies hook runs with remote repos - check_hook_updates_fails_on_updates: verifies --fail-on-updates works --- crates/prek/tests/builtin_hooks.rs | 68 ++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) diff --git a/crates/prek/tests/builtin_hooks.rs b/crates/prek/tests/builtin_hooks.rs index 6c5f76100..4c2596615 100644 --- a/crates/prek/tests/builtin_hooks.rs +++ b/crates/prek/tests/builtin_hooks.rs @@ -2327,3 +2327,71 @@ fn check_hook_updates_hook_with_args() { ----- stderr ----- "); } + +/// Tests that `check-hook-updates` hook detects available updates for remote repos. +/// Without `--fail-on-updates`, the hook passes but outputs available updates. +#[test] +fn check_hook_updates_detects_outdated_repo() { + let context = TestContext::new(); + context.init_project(); + + // Use an old version of pre-commit-hooks that has newer versions available + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: builtin + hooks: + - id: check-hook-updates + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.0.0 + hooks: + - id: trailing-whitespace + "}); + context.git_add("."); + + // The hook should detect that v4.0.0 is outdated and report available updates (but still pass) + cmd_snapshot!(context.filters(), context.run(), @r" + success: true + exit_code: 0 + ----- stdout ----- + check for hook updates...................................................Passed + Trim Trailing Whitespace.................................................Passed + + ----- stderr ----- + "); +} + +/// Tests that `check-hook-updates` hook fails when `--fail-on-updates` is set and updates are available. +#[test] +fn check_hook_updates_fails_on_updates() { + let context = TestContext::new(); + context.init_project(); + + // Use an old version with --fail-on-updates + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: builtin + hooks: + - id: check-hook-updates + args: ['--fail-on-updates'] + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v4.0.0 + hooks: + - id: trailing-whitespace + "}); + context.git_add("."); + + // The hook should fail because updates are available + cmd_snapshot!(context.filters(), context.run(), @r" + success: false + exit_code: 1 + ----- stdout ----- + check for hook updates...................................................Failed + - hook id: check-hook-updates + - exit code: 1 + + https://github.com/pre-commit/pre-commit-hooks: v4.0.0 -> v6.0.0 available + Trim Trailing Whitespace.................................................Passed + + ----- stderr ----- + "); +} From ff56f40a16484e84470cfc2fe6219ddbc071c45c Mon Sep 17 00:00:00 2001 From: Ismar Iljazovic Date: Sun, 18 Jan 2026 21:48:10 +0100 Subject: [PATCH 4/6] feat: add check-interval-hours to throttle hook update checks - Default to checking at most once every 24 hours to avoid slowing commits - Store last check timestamp in prek cache directory - Change cooldown-days default from 0 to 7 for supply chain security - Add --check-interval-hours=0 to force check every time (useful for CI) - Update docs with new arguments and security rationale - Add tests for check interval behavior Addresses review feedback about hook running on every commit. --- .../pre_commit_hooks/check_hook_updates.rs | 70 ++++++++++++++++++- crates/prek/tests/builtin_hooks.rs | 42 ++++++++++- docs/builtin.md | 21 ++++++ 3 files changed, 130 insertions(+), 3 deletions(-) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs b/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs index 580394423..f0568bb25 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs @@ -1,14 +1,17 @@ use std::io::Write; use std::path::Path; +use std::time::{SystemTime, UNIX_EPOCH}; use anyhow::Result; use clap::Parser; +use tracing::debug; use crate::cli::auto_update::{ find_eligible_tag, get_tag_timestamps, resolve_rev_to_commit_hash, setup_and_fetch_repo, }; use crate::config; use crate::hook::Hook; +use crate::store::{CacheBucket, Store}; #[derive(Parser)] #[command(disable_help_subcommand = true)] @@ -17,14 +20,20 @@ use crate::hook::Hook; struct Args { /// Minimum release age (in days) required for a version to be eligible. /// A value of `0` disables this check. - #[arg(long, value_name = "DAYS", default_value_t = 0)] + #[arg(long, value_name = "DAYS", default_value_t = 7)] cooldown_days: u8, /// Fail the hook if updates are available (default: warn only). #[arg(long, default_value_t = false)] fail_on_updates: bool, + + /// Minimum hours between checks (default: 24). Set to 0 to check every time. + #[arg(long, value_name = "HOURS", default_value_t = 24)] + check_interval_hours: u64, } +const LAST_CHECK_FILE: &str = "hook-updates-last-check"; + /// Check if configured hooks have newer versions available. pub(crate) async fn check_hook_updates( hook: &Hook, @@ -32,6 +41,19 @@ pub(crate) async fn check_hook_updates( ) -> Result<(i32, Vec)> { let args = Args::try_parse_from(hook.entry.resolve(None)?.iter().chain(&hook.args))?; + // Check if we should skip based on check interval + if args.check_interval_hours > 0 { + if let Ok(store) = Store::from_settings() { + if should_skip_check(&store, args.check_interval_hours) { + debug!( + "Skipping hook update check (last check was within {} hours)", + args.check_interval_hours + ); + return Ok((0, Vec::new())); + } + } + } + let project_config = hook.project().config(); let mut code = 0; @@ -65,9 +87,55 @@ pub(crate) async fn check_hook_updates( } } + // Mark check as complete (only if we actually ran the check) + if args.check_interval_hours > 0 { + if let Ok(store) = Store::from_settings() { + mark_check_complete(&store); + } + } + Ok((code, output)) } +/// Check if we should skip the update check based on the last check time. +fn should_skip_check(store: &Store, interval_hours: u64) -> bool { + let cache_file = store.cache_path(CacheBucket::Prek).join(LAST_CHECK_FILE); + + let Ok(metadata) = std::fs::metadata(&cache_file) else { + return false; + }; + + let Ok(modified) = metadata.modified() else { + return false; + }; + + let Ok(age) = SystemTime::now().duration_since(modified) else { + return false; + }; + + let interval_secs = interval_hours * 3600; + age.as_secs() < interval_secs +} + +/// Mark the check as complete by touching the cache file. +fn mark_check_complete(store: &Store) { + let cache_dir = store.cache_path(CacheBucket::Prek); + if std::fs::create_dir_all(&cache_dir).is_err() { + return; + } + + let cache_file = cache_dir.join(LAST_CHECK_FILE); + // Write current timestamp to the file + let now = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + + if let Err(e) = std::fs::write(&cache_file, now.to_string()) { + debug!("Failed to write last check timestamp: {}", e); + } +} + struct UpdateInfo { new_rev: String, } diff --git a/crates/prek/tests/builtin_hooks.rs b/crates/prek/tests/builtin_hooks.rs index 4c2596615..eda016c45 100644 --- a/crates/prek/tests/builtin_hooks.rs +++ b/crates/prek/tests/builtin_hooks.rs @@ -2212,7 +2212,6 @@ fn check_json5() -> Result<()> { Ok(()) } -<<<<<<< HEAD /// Test that builtin hooks work correctly even when a system-wide binary with the /// same name exists on PATH (regression test for ). /// @@ -2328,6 +2327,42 @@ fn check_hook_updates_hook_with_args() { "); } +/// Tests that `check-hook-updates` hook with `--check-interval-hours=0` runs every time. +#[test] +fn check_hook_updates_check_interval_zero_runs_every_time() { + let context = TestContext::new(); + context.init_project(); + + context.write_pre_commit_config(indoc::indoc! {r" + repos: + - repo: builtin + hooks: + - id: check-hook-updates + args: ['--check-interval-hours=0'] + "}); + context.git_add("."); + + // First run + cmd_snapshot!(context.filters(), context.run(), @r" + success: true + exit_code: 0 + ----- stdout ----- + check for hook updates...................................................Passed + + ----- stderr ----- + "); + + // Second run should also execute (not skip) + cmd_snapshot!(context.filters(), context.run(), @r" + success: true + exit_code: 0 + ----- stdout ----- + check for hook updates...................................................Passed + + ----- stderr ----- + "); +} + /// Tests that `check-hook-updates` hook detects available updates for remote repos. /// Without `--fail-on-updates`, the hook passes but outputs available updates. #[test] @@ -2336,11 +2371,13 @@ fn check_hook_updates_detects_outdated_repo() { context.init_project(); // Use an old version of pre-commit-hooks that has newer versions available + // Use --check-interval-hours=0 to ensure the check runs context.write_pre_commit_config(indoc::indoc! {r" repos: - repo: builtin hooks: - id: check-hook-updates + args: ['--check-interval-hours=0'] - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.0.0 hooks: @@ -2367,12 +2404,13 @@ fn check_hook_updates_fails_on_updates() { context.init_project(); // Use an old version with --fail-on-updates + // Use --check-interval-hours=0 to ensure the check runs context.write_pre_commit_config(indoc::indoc! {r" repos: - repo: builtin hooks: - id: check-hook-updates - args: ['--fail-on-updates'] + args: ['--fail-on-updates', '--check-interval-hours=0'] - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.0.0 hooks: diff --git a/docs/builtin.md b/docs/builtin.md index 01d3d9bfd..1458cc1a5 100644 --- a/docs/builtin.md +++ b/docs/builtin.md @@ -382,3 +382,24 @@ Checks that non-binary executables have a proper shebang. - The check is intentionally lightweight: it only verifies that the file starts with `#!`. - On systems where the executable bit is not tracked by the filesystem, `prek` consults git's staged mode bits. + +--- + +#### `check-hook-updates` + +Checks if configured hooks have newer versions available. + +**Supported arguments**: + +- `--cooldown-days=` (default: `7`) + - Minimum release age (in days) required for a version to be eligible. Helps prevent supply chain attacks by not immediately suggesting brand-new releases. A value of `0` disables this check. +- `--fail-on-updates` + - Fail the hook if updates are available (default: warn only). +- `--check-interval-hours=` (default: `24`) + - Minimum hours between checks. Set to `0` to check every time. + +**Behavior / caveats** + +- By default, the hook only runs once every 24 hours to avoid slowing down commits. The last check time is stored in the prek cache directory. +- Network errors are reported as warnings but do not fail the hook. +- This hook is configured as `always_run: true` by default, and does not take filenames. From 98efe3dc16b13d267f0bc5072b7b99933da233f8 Mon Sep 17 00:00:00 2001 From: Ismar Iljazovic Date: Sun, 25 Jan 2026 02:28:44 +0100 Subject: [PATCH 5/6] chore: regenerate schema with check-hook-updates builtin --- prek.schema.json | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/prek.schema.json b/prek.schema.json index 06d268101..d2586d26f 100644 --- a/prek.schema.json +++ b/prek.schema.json @@ -383,7 +383,8 @@ "fix-byte-order-marker", "mixed-line-ending", "no-commit-to-branch", - "trailing-whitespace" + "trailing-whitespace", + "check-hook-updates" ] }, "BuiltinRepo": { From 6b3673c4b923e898c6b8faa0442ce186246156c0 Mon Sep 17 00:00:00 2001 From: Jo <10510431+j178@users.noreply.github.com> Date: Tue, 27 Jan 2026 14:58:54 +0800 Subject: [PATCH 6/6] Move `check_hook_updates` to builtin_hooks --- .../check_hook_updates.rs | 0 crates/prek/src/hooks/builtin_hooks/mod.rs | 7 ++++--- crates/prek/src/hooks/pre_commit_hooks/mod.rs | 2 -- prek.schema.json | 4 ++-- 4 files changed, 6 insertions(+), 7 deletions(-) rename crates/prek/src/hooks/{pre_commit_hooks => builtin_hooks}/check_hook_updates.rs (100%) diff --git a/crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs b/crates/prek/src/hooks/builtin_hooks/check_hook_updates.rs similarity index 100% rename from crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs rename to crates/prek/src/hooks/builtin_hooks/check_hook_updates.rs diff --git a/crates/prek/src/hooks/builtin_hooks/mod.rs b/crates/prek/src/hooks/builtin_hooks/mod.rs index e31dd384c..73734b0ce 100644 --- a/crates/prek/src/hooks/builtin_hooks/mod.rs +++ b/crates/prek/src/hooks/builtin_hooks/mod.rs @@ -8,6 +8,7 @@ use crate::hook::Hook; use crate::hooks::pre_commit_hooks; use crate::store::Store; +mod check_hook_updates; mod check_json5; #[derive(Debug, Copy, Clone, PartialEq, Eq)] @@ -17,6 +18,7 @@ pub(crate) enum BuiltinHooks { CheckAddedLargeFiles, CheckCaseConflict, CheckExecutablesHaveShebangs, + CheckHookUpdates, CheckJson, CheckJson5, CheckMergeConflict, @@ -30,7 +32,6 @@ pub(crate) enum BuiltinHooks { MixedLineEnding, NoCommitToBranch, TrailingWhitespace, - CheckHookUpdates, } impl FromStr for BuiltinHooks { @@ -41,6 +42,7 @@ impl FromStr for BuiltinHooks { "check-added-large-files" => Ok(Self::CheckAddedLargeFiles), "check-case-conflict" => Ok(Self::CheckCaseConflict), "check-executables-have-shebangs" => Ok(Self::CheckExecutablesHaveShebangs), + "check-hook-updates" => Ok(Self::CheckHookUpdates), "check-json" => Ok(Self::CheckJson), "check-json5" => Ok(Self::CheckJson5), "check-merge-conflict" => Ok(Self::CheckMergeConflict), @@ -54,7 +56,6 @@ impl FromStr for BuiltinHooks { "mixed-line-ending" => Ok(Self::MixedLineEnding), "no-commit-to-branch" => Ok(Self::NoCommitToBranch), "trailing-whitespace" => Ok(Self::TrailingWhitespace), - "check-hook-updates" => Ok(Self::CheckHookUpdates), _ => Err(()), } } @@ -75,6 +76,7 @@ impl BuiltinHooks { Self::CheckExecutablesHaveShebangs => { pre_commit_hooks::check_executables_have_shebangs(hook, filenames).await } + Self::CheckHookUpdates => check_hook_updates::check_hook_updates(hook, filenames).await, Self::CheckJson => pre_commit_hooks::check_json(hook, filenames).await, Self::CheckJson5 => check_json5::check_json5(hook, filenames).await, Self::CheckMergeConflict => { @@ -94,7 +96,6 @@ impl BuiltinHooks { Self::TrailingWhitespace => { pre_commit_hooks::fix_trailing_whitespace(hook, filenames).await } - Self::CheckHookUpdates => pre_commit_hooks::check_hook_updates(hook, filenames).await, } } } diff --git a/crates/prek/src/hooks/pre_commit_hooks/mod.rs b/crates/prek/src/hooks/pre_commit_hooks/mod.rs index 1647481c7..63e71809b 100644 --- a/crates/prek/src/hooks/pre_commit_hooks/mod.rs +++ b/crates/prek/src/hooks/pre_commit_hooks/mod.rs @@ -9,7 +9,6 @@ use crate::hook::Hook; mod check_added_large_files; mod check_case_conflict; mod check_executables_have_shebangs; -mod check_hook_updates; pub(crate) mod check_json; mod check_merge_conflict; mod check_symlinks; @@ -26,7 +25,6 @@ mod no_commit_to_branch; pub(crate) use check_added_large_files::check_added_large_files; pub(crate) use check_case_conflict::check_case_conflict; pub(crate) use check_executables_have_shebangs::check_executables_have_shebangs; -pub(crate) use check_hook_updates::check_hook_updates; pub(crate) use check_json::check_json; pub(crate) use check_merge_conflict::check_merge_conflict; pub(crate) use check_symlinks::check_symlinks; diff --git a/prek.schema.json b/prek.schema.json index d2586d26f..e59c7fb98 100644 --- a/prek.schema.json +++ b/prek.schema.json @@ -371,6 +371,7 @@ "check-added-large-files", "check-case-conflict", "check-executables-have-shebangs", + "check-hook-updates", "check-json", "check-json5", "check-merge-conflict", @@ -383,8 +384,7 @@ "fix-byte-order-marker", "mixed-line-ending", "no-commit-to-branch", - "trailing-whitespace", - "check-hook-updates" + "trailing-whitespace" ] }, "BuiltinRepo": {