Skip to content

Commit ff56f40

Browse files
author
Ismar Iljazovic
committed
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.
1 parent 86852b4 commit ff56f40

3 files changed

Lines changed: 130 additions & 3 deletions

File tree

crates/prek/src/hooks/pre_commit_hooks/check_hook_updates.rs

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
11
use std::io::Write;
22
use std::path::Path;
3+
use std::time::{SystemTime, UNIX_EPOCH};
34

45
use anyhow::Result;
56
use clap::Parser;
7+
use tracing::debug;
68

79
use crate::cli::auto_update::{
810
find_eligible_tag, get_tag_timestamps, resolve_rev_to_commit_hash, setup_and_fetch_repo,
911
};
1012
use crate::config;
1113
use crate::hook::Hook;
14+
use crate::store::{CacheBucket, Store};
1215

1316
#[derive(Parser)]
1417
#[command(disable_help_subcommand = true)]
@@ -17,21 +20,40 @@ use crate::hook::Hook;
1720
struct Args {
1821
/// Minimum release age (in days) required for a version to be eligible.
1922
/// A value of `0` disables this check.
20-
#[arg(long, value_name = "DAYS", default_value_t = 0)]
23+
#[arg(long, value_name = "DAYS", default_value_t = 7)]
2124
cooldown_days: u8,
2225

2326
/// Fail the hook if updates are available (default: warn only).
2427
#[arg(long, default_value_t = false)]
2528
fail_on_updates: bool,
29+
30+
/// Minimum hours between checks (default: 24). Set to 0 to check every time.
31+
#[arg(long, value_name = "HOURS", default_value_t = 24)]
32+
check_interval_hours: u64,
2633
}
2734

35+
const LAST_CHECK_FILE: &str = "hook-updates-last-check";
36+
2837
/// Check if configured hooks have newer versions available.
2938
pub(crate) async fn check_hook_updates(
3039
hook: &Hook,
3140
_filenames: &[&Path],
3241
) -> Result<(i32, Vec<u8>)> {
3342
let args = Args::try_parse_from(hook.entry.resolve(None)?.iter().chain(&hook.args))?;
3443

44+
// Check if we should skip based on check interval
45+
if args.check_interval_hours > 0 {
46+
if let Ok(store) = Store::from_settings() {
47+
if should_skip_check(&store, args.check_interval_hours) {
48+
debug!(
49+
"Skipping hook update check (last check was within {} hours)",
50+
args.check_interval_hours
51+
);
52+
return Ok((0, Vec::new()));
53+
}
54+
}
55+
}
56+
3557
let project_config = hook.project().config();
3658

3759
let mut code = 0;
@@ -65,9 +87,55 @@ pub(crate) async fn check_hook_updates(
6587
}
6688
}
6789

90+
// Mark check as complete (only if we actually ran the check)
91+
if args.check_interval_hours > 0 {
92+
if let Ok(store) = Store::from_settings() {
93+
mark_check_complete(&store);
94+
}
95+
}
96+
6897
Ok((code, output))
6998
}
7099

100+
/// Check if we should skip the update check based on the last check time.
101+
fn should_skip_check(store: &Store, interval_hours: u64) -> bool {
102+
let cache_file = store.cache_path(CacheBucket::Prek).join(LAST_CHECK_FILE);
103+
104+
let Ok(metadata) = std::fs::metadata(&cache_file) else {
105+
return false;
106+
};
107+
108+
let Ok(modified) = metadata.modified() else {
109+
return false;
110+
};
111+
112+
let Ok(age) = SystemTime::now().duration_since(modified) else {
113+
return false;
114+
};
115+
116+
let interval_secs = interval_hours * 3600;
117+
age.as_secs() < interval_secs
118+
}
119+
120+
/// Mark the check as complete by touching the cache file.
121+
fn mark_check_complete(store: &Store) {
122+
let cache_dir = store.cache_path(CacheBucket::Prek);
123+
if std::fs::create_dir_all(&cache_dir).is_err() {
124+
return;
125+
}
126+
127+
let cache_file = cache_dir.join(LAST_CHECK_FILE);
128+
// Write current timestamp to the file
129+
let now = SystemTime::now()
130+
.duration_since(UNIX_EPOCH)
131+
.map(|d| d.as_secs())
132+
.unwrap_or(0);
133+
134+
if let Err(e) = std::fs::write(&cache_file, now.to_string()) {
135+
debug!("Failed to write last check timestamp: {}", e);
136+
}
137+
}
138+
71139
struct UpdateInfo {
72140
new_rev: String,
73141
}

crates/prek/tests/builtin_hooks.rs

Lines changed: 40 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2212,7 +2212,6 @@ fn check_json5() -> Result<()> {
22122212
Ok(())
22132213
}
22142214

2215-
<<<<<<< HEAD
22162215
/// Test that builtin hooks work correctly even when a system-wide binary with the
22172216
/// same name exists on PATH (regression test for <https://github.com/j178/prek/issues/1412>).
22182217
///
@@ -2328,6 +2327,42 @@ fn check_hook_updates_hook_with_args() {
23282327
");
23292328
}
23302329

2330+
/// Tests that `check-hook-updates` hook with `--check-interval-hours=0` runs every time.
2331+
#[test]
2332+
fn check_hook_updates_check_interval_zero_runs_every_time() {
2333+
let context = TestContext::new();
2334+
context.init_project();
2335+
2336+
context.write_pre_commit_config(indoc::indoc! {r"
2337+
repos:
2338+
- repo: builtin
2339+
hooks:
2340+
- id: check-hook-updates
2341+
args: ['--check-interval-hours=0']
2342+
"});
2343+
context.git_add(".");
2344+
2345+
// First run
2346+
cmd_snapshot!(context.filters(), context.run(), @r"
2347+
success: true
2348+
exit_code: 0
2349+
----- stdout -----
2350+
check for hook updates...................................................Passed
2351+
2352+
----- stderr -----
2353+
");
2354+
2355+
// Second run should also execute (not skip)
2356+
cmd_snapshot!(context.filters(), context.run(), @r"
2357+
success: true
2358+
exit_code: 0
2359+
----- stdout -----
2360+
check for hook updates...................................................Passed
2361+
2362+
----- stderr -----
2363+
");
2364+
}
2365+
23312366
/// Tests that `check-hook-updates` hook detects available updates for remote repos.
23322367
/// Without `--fail-on-updates`, the hook passes but outputs available updates.
23332368
#[test]
@@ -2336,11 +2371,13 @@ fn check_hook_updates_detects_outdated_repo() {
23362371
context.init_project();
23372372

23382373
// Use an old version of pre-commit-hooks that has newer versions available
2374+
// Use --check-interval-hours=0 to ensure the check runs
23392375
context.write_pre_commit_config(indoc::indoc! {r"
23402376
repos:
23412377
- repo: builtin
23422378
hooks:
23432379
- id: check-hook-updates
2380+
args: ['--check-interval-hours=0']
23442381
- repo: https://github.com/pre-commit/pre-commit-hooks
23452382
rev: v4.0.0
23462383
hooks:
@@ -2367,12 +2404,13 @@ fn check_hook_updates_fails_on_updates() {
23672404
context.init_project();
23682405

23692406
// Use an old version with --fail-on-updates
2407+
// Use --check-interval-hours=0 to ensure the check runs
23702408
context.write_pre_commit_config(indoc::indoc! {r"
23712409
repos:
23722410
- repo: builtin
23732411
hooks:
23742412
- id: check-hook-updates
2375-
args: ['--fail-on-updates']
2413+
args: ['--fail-on-updates', '--check-interval-hours=0']
23762414
- repo: https://github.com/pre-commit/pre-commit-hooks
23772415
rev: v4.0.0
23782416
hooks:

docs/builtin.md

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -382,3 +382,24 @@ Checks that non-binary executables have a proper shebang.
382382

383383
- The check is intentionally lightweight: it only verifies that the file starts with `#!`.
384384
- On systems where the executable bit is not tracked by the filesystem, `prek` consults git's staged mode bits.
385+
386+
---
387+
388+
#### `check-hook-updates`
389+
390+
Checks if configured hooks have newer versions available.
391+
392+
**Supported arguments**:
393+
394+
- `--cooldown-days=<N>` (default: `7`)
395+
- 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.
396+
- `--fail-on-updates`
397+
- Fail the hook if updates are available (default: warn only).
398+
- `--check-interval-hours=<N>` (default: `24`)
399+
- Minimum hours between checks. Set to `0` to check every time.
400+
401+
**Behavior / caveats**
402+
403+
- 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.
404+
- Network errors are reported as warnings but do not fail the hook.
405+
- This hook is configured as `always_run: true` by default, and does not take filenames.

0 commit comments

Comments
 (0)