Add check-signed-commit builtin hook - #2725
Open
ebuildy wants to merge 4 commits into
Open
ebuildy wants to merge 4 commits into
ebuildy wants to merge 4 commits into
Conversation
Copy link
Copy Markdown
Sorry, something went wrong.
Copy link
Copy Markdown
Codecov Report❌ Patch coverage is 95.09804% with 10 lines in your changes missing coverage. Please review.
@@ Coverage Diff @@
## master #2725 +/- ##
========================================
Coverage 94.19% 94.19%
========================================
Files 142 143 +1
Lines 30292 30496 +204
========================================
+ Hits 28534 28727 +193
- Misses 1758 1769 +11 ☔ View full report in Codecov by Harness.
|
Sorry, something went wrong.
ebuildy
marked this pull request as draft
September 15, 2026 04:41
Copy link
Copy Markdown
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: bc8f15d75f
ℹ️ About Codex in GitHubCodex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
Sorry, something went wrong.
Comment thread
crates/prek/src/hooks/builtin_hooks/check_signed_commit.rs
| .arg("--no-merges") | ||
| .arg("-z") | ||
| .arg("--pretty=format:%h\u{1f}%G?\u{1f}%s") | ||
| .arg(range) |
Copy link
Copy Markdown
There was a problem hiding this comment.
Check every ref in a multi-ref push
When one git push updates multiple refs, Git supplies one pre-push input line per ref, but parse_pre_push_info in crates/prek/src/cli/hook_impl.rs returns after selecting the first applicable line and exposes only that single range through these environment variables. Consequently, this git log checks only the first selected ref, so an unsigned commit reachable exclusively from a later branch or tag is pushed without inspection. The pre-push plumbing needs to retain all pushed ranges, and this hook must inspect each of them.
Useful? React with 👍 / 👎.
Sorry, something went wrong.
Copy link
Copy Markdown
Author
There was a problem hiding this comment.
Ho good point! but it's an infrastructure gap, not something scoped to this PR's file, I could do another PR if you want
Sorry, something went wrong.
ebuildy
force-pushed
the
check-signed-commit
branch
from
bc8f15d to
099b4e7
Compare
September 15, 2026 04:46
Copy link
Copy Markdown
📦 Cargo Bloat Comparison.text size change: +3.25% (12.3 MiB → 12.7 MiB) Expand for cargo-bloat outputHead Branch ResultsFile .text Size Crate Name
0.1% 2.6% 332.0KiB aws_lc_sys aws_lc_0_45_0_aes_gcm_encrypt_avx512
0.1% 2.6% 332.0KiB aws_lc_sys aws_lc_0_45_0_aes_gcm_decrypt_avx512
0.0% 0.7% 96.5KiB prek <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
0.0% 0.4% 50.1KiB prek <<prek::config::hook::HookWire as serde_core::de::Deserialize>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map::<&mut <serde_saphyr::de::deserializer::YamlDeserializer as serde_core::de::Deserializer>::deserialize_map::MA>
0.0% 0.4% 48.7KiB annotate_snippets annotate_snippets::renderer::render::render
0.0% 0.3% 44.7KiB prek prek::run::{closure#0}
0.0% 0.3% 43.9KiB prek <<prek::config::Config as serde_core::de::Deserialize>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map::<&mut <serde_saphyr::de::deserializer::YamlDeserializer as serde_core::de::Deserializer>::deserialize_map::MA>
0.0% 0.3% 40.3KiB prek <prek::cli::RunOptions as clap_builder::derive::Args>::augment_args
0.0% 0.3% 35.5KiB prek prek::cli::run::run::run::{closure#0}
0.0% 0.2% 28.8KiB serde_saphyr <granit_parser::scanner::Scanner<granit_parser::input::str::StrInput>>::fetch_more_tokens
0.0% 0.2% 28.5KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 28.4KiB serde_saphyr <granit_parser::scanner::Scanner<granit_parser::input::str::StrInput>>::fetch_more_tokens
0.0% 0.2% 28.0KiB aws_lc_sys aws_lc_0_45_0_edwards25519_scalarmuldouble_alt
0.0% 0.2% 27.5KiB aws_lc_sys aws_lc_0_45_0_edwards25519_scalarmuldouble
0.0% 0.2% 27.0KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 27.0KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 27.0KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 27.0KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 26.9KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 24.5KiB prek <prek::workspace::Project>::init_hooks::{closure#0}
4.2% 86.9% 11.0MiB And 20339 smaller methods. Use -n N to show more.
4.8% 100.0% 12.7MiB .text section size, the file size is 264.6MiB
Base Branch ResultsFile .text Size Crate Name
0.1% 2.6% 332.0KiB aws_lc_sys aws_lc_0_45_0_aes_gcm_encrypt_avx512
0.1% 2.6% 332.0KiB aws_lc_sys aws_lc_0_45_0_aes_gcm_decrypt_avx512
0.0% 0.8% 96.5KiB prek <prek::cli::Command as clap_builder::derive::Subcommand>::augment_subcommands
0.0% 0.4% 50.1KiB prek <<prek::config::hook::HookWire as serde_core::de::Deserialize>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map::<&mut <serde_saphyr::de::deserializer::YamlDeserializer as serde_core::de::Deserializer>::deserialize_map::MA>
0.0% 0.4% 48.7KiB annotate_snippets annotate_snippets::renderer::render::render
0.0% 0.3% 43.9KiB prek <<prek::config::Config as serde_core::de::Deserialize>::deserialize::__Visitor as serde_core::de::Visitor>::visit_map::<&mut <serde_saphyr::de::deserializer::YamlDeserializer as serde_core::de::Deserializer>::deserialize_map::MA>
0.0% 0.3% 41.2KiB prek prek::run::{closure#0}
0.0% 0.3% 40.3KiB prek <prek::cli::RunOptions as clap_builder::derive::Args>::augment_args
0.0% 0.3% 36.8KiB prek prek::cli::run::run::run::{closure#0}
0.0% 0.2% 28.8KiB serde_saphyr <granit_parser::scanner::Scanner<granit_parser::input::str::StrInput>>::fetch_more_tokens
0.0% 0.2% 28.6KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 28.4KiB serde_saphyr <granit_parser::scanner::Scanner<granit_parser::input::str::StrInput>>::fetch_more_tokens
0.0% 0.2% 28.0KiB aws_lc_sys aws_lc_0_45_0_edwards25519_scalarmuldouble_alt
0.0% 0.2% 27.5KiB aws_lc_sys aws_lc_0_45_0_edwards25519_scalarmuldouble
0.0% 0.2% 27.2KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 27.0KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 27.0KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 27.0KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 26.9KiB prek prek::archive::unpack::<fs_err::tokio::file::File, &std::path::PathBuf>::{closure#0}
0.0% 0.2% 24.5KiB prek <prek::workspace::Project>::init_hooks::{closure#0}
4.1% 86.6% 10.7MiB And 20201 smaller methods. Use -n N to show more.
4.7% 100.0% 12.3MiB .text section size, the file size is 263.5MiB
|
Sorry, something went wrong.
Copy link
Copy Markdown
⚡️ Hyperfine BenchmarksSummary: 0 regressions, 0 improvements above the 10% threshold. Environment
Benchmarking basic commands in the main repo: prek --version
prek list
prek validate-config .pre-commit-config.yaml⏭️ Skipped: .pre-commit-config.yaml not found prek sample-config
Comparing first run (cold) vs subsequent runs (warm cache): prek run --all-files (cold - no cache)
prek run --all-files (warm - with cache)
Running the builtin hook suite on the benchmark workspace: prek run --all-files (full builtin hook suite)
Benchmarking each hook individually on the test repo: prek run trailing-whitespace --all-files
prek run end-of-file-fixer --all-files
prek run check-json --all-files
prek run check-yaml --all-files
prek run check-toml --all-files
prek run check-xml --all-files
prek run detect-private-key --all-files
prek run fix-byte-order-marker --all-files
Benchmarking hook installation (fast path hooks skip Python setup): prek install-hooks (cold - no cache)
prek install-hooks (warm - with cache)
Testing different file selection modes: prek run (staged files only)
prek run --files '*.json' (specific file type)
Benchmarking hook discovery and initialization overhead: prek run --dry-run --all-files (measures init overhead)
Benchmarking meta hooks separately: prek run check-hooks-apply --all-files
prek run check-useless-excludes --all-files
prek run identity --all-files
|
Sorry, something went wrong.
ebuildy
force-pushed
the
check-signed-commit
branch
from
099b4e7 to
ed34c0c
Compare
September 15, 2026 05:01
ebuildy
force-pushed
the
check-signed-commit
branch
from
ed34c0c to
d8a22dc
Compare
September 15, 2026 05:19
ebuildy
marked this pull request as ready for review
September 15, 2026 05:29
ebuildy
marked this pull request as draft
September 15, 2026 05:30
ebuildy
marked this pull request as ready for review
September 18, 2026 14:09
Copy link
Copy Markdown
Author
|
Ready for review ! I am doing another PR related , to check the DCO signoff status |
Sorry, something went wrong.
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Reviewers
At least 1 approving review is required to merge this pull request.
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Verifying commit signatures before a push has no good option today. Wiring git verify-commit up as a local hook runs into the problems described in #2720:
How
Adds a builtin hook, check-signed-commit, that checks every non-merge commit in the range being pushed against Git's own signature status (%G? from git log), in a single git log call (no per-commit subprocess, no filename plumbing):
Example
check for commit signatures..............................................Failed - hook id: check-signed-commit - exit code: 1 a1b2c3d [N] no signature: fix typo Commit signature status codes: G good signature ...Closes #2720
🤖 Generated with Claude Code