contrib: reject divergent verify-commits history #35980

Problem: verify-commits.py checks a Git commit's history for trusted signatures and tree hashes back to configured roots. The documented workflow runs this check after fetching a commit and before checkout, proceeding only when the script succeeds. A commit that is an ancestor of a configured root is intentionally accepted without checking earlier history. The script also takes this success path after Git errors or for divergent commits, even though neither establishes that relationship.

Fix: Require Git to prove the ancestor relationship before taking this success path.

Reproducers: Each commit can be validated manually.

<details><summary>Manual reproducer: Git error</summary>

Run this on master and at this PR's head:

contrib/verify-commits/verify-commits.py 0000000000000000000000000000000000000000 && echo 🐛 || echo 🛡️

master exits successfully without verifying the missing commit, while the PR head rejects the Git error.

</details>

<details><summary>Manual reproducer: divergent history</summary>

On master and at this PR's head, create an unreferenced sibling of the trusted root and run the verifier:

root=$(head -n1 contrib/verify-commits/trusted-git-root)
divergent_commit=$(git commit-tree "$root^{tree}" -p "$root^" -m 'divergent commit')
contrib/verify-commits/verify-commits.py "$divergent_commit" && echo 🐛 || echo 🛡️

master exits successfully without verifying the sibling commit, while the PR head rejects divergent history.

</details>

This issue was also found and disclosed responsibly by the Red Team 🟥.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35980.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline and AI policy for information on the review process.

If your review is incorrectly listed, please copy-paste <code>&lt;!--meta-tag:bot-skip--&gt;</code> into the comment that the bot should ignore.

<!--5faf32d7da4f0f540f40219e4f7537a3-->

tACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f

Ran the two reproducers from the PR on master and on tip. Master exits 0 for both the missing commit and the divergent sibling; tip fails both. Trusted root and root^ still exit 0.

Also checked a divergent sibling of the Tree-SHA512 root (rejected), a shallow clone (tip fails closed; master still exits 0), and a short signed walk on master with the tip script (sigs + Tree-SHA512 + clean-merge still pass).

tACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f

Ran both reproducers on both sides. On master, an unsigned divergent commit (commit-tree, parent root^) passes with exit 0 and the script even says it "predates the trusted root", which it doesn't. On this head it's rejected with exit 1. The usual paths are unchanged.

ACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f

ACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f

For the steps to test the second commit:

I don't think it is required to manually create a divergent commit. There are already plenty. E.g:

./contrib/verify-commits/verify-commits.py v30.1 && echo 🐛 || echo 🛡️ 

nit: Also the x and v emojis were a bit confusing. I think it would be better to use the bug-emoji (🐛) to indicate the bug and the shield-emoji (🛡️ ) to indicate a successful reject.

I wonder if this needs a brief release note, given that some projects have this script copy-pasted? Also, I wonder if the docs should be updated to educate users about how to verify older diverging tags from master?

Edit: To clarify:

• https://github.com/bitcoin-core/bitcoincore.org/blob/master/contrib/verify-commits/verify-commits.sh (not sure if this needs a broader website system change)

But those are just nits/questions. Nice find and fix.

review ACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f 🥜

<details><summary>Show signature</summary>

Signature:

untrusted comment: signature from minisign secret key on empty file; verify via: minisign -Vm "${path_to_any_empty_file}" -P RWTRmVTMeKV5noAMqVlsMugDDCyyTSbA3Re5AkUrhvLVln0tSaFWglOw -x "${path_to_this_whole_four_line_signature_blob}"
RUTRmVTMeKV5npGrKx1nqXCw5zeVHdtdYURB/KlyA/LMFgpNCs+SkW9a8N95d+U4AP1RJMi+krxU1A3Yux4bpwZNLvVBKy0wLgM=
trusted comment: review ACK 465bca734ebf22dc27dd8667224f1cf2b590fc9f 🥜
Jnmnp9JQyY2a7oW/Ox87LakFE4v/gM6gBis+dEUqz+BObrQYwzSkDZerGvoc9Noyz/ShqQYXMvmhbfcWMl8YAg==

</details>

I think it would be better to use the bug-emoji (🐛) to indicate the bug and the shield-emoji (🛡️ ) to indicate a successful reject

Love it, updated the PR title post-merge, I'll use these next time!

Also, I wonder if the docs should be updated to educate users about how to verify older diverging tags from master?

I guess the docs could be updated to something like this?

diff --git a/contrib/verify-commits/README.md b/contrib/verify-commits/README.md
index 020890c..a257505 100644
--- a/contrib/verify-commits/README.md
+++ b/contrib/verify-commits/README.md
@@ -27,6 +27,10 @@ Note that the above isn't a good UI/UX yet, and needs significant improvements
 to make it more convenient and reduce the chance of errors; pull-reqs
 improving this process would be much appreciated.
 
+If you run into a failure about diverging histories, when trying to verify an
+older release branch or release tag, you may have to first check out or apply
+an earlier trusted version of the trusted roots.
+
 Unless `--clean-merge 0` is specified, `verify-commits.py` will attempt to verify that
 each merge commit applies cleanly (with some exceptions). This requires using at least
 git v2.38.0.