contrib: replace deprecated --deep codesign flag, fix accidental --verify skip on ci #34914

1. 
   Sjors commented at 4:36 PM on March 24, 2026: member

   Replace the deprecated codesign --deep with explicit, and minimal, per-component signing of Frameworks, Plugins and the top-level bundle.

   The CI signature check introduced in #34787 is updated to use --strict.

   Can be tested with:

   cmake -B build -DBUILD_GUI=ON
   # delete artifacts before rebuilding the `deploy` target
   rm -rf build/Bitcoin-Qt.app build/bitcoin-macos-app.zip
   cmake --build build -t deploy
   codesign --verify --deep --strict --verbose=4 build/dist/Bitcoin-Qt.app
   

   Fixes #32486, supersedes #33592 (this is a condensed version)

   Additionally this PR modifies 03_test_script.sh to avoid modifying GOAL in place. That was causing the codesign --verify step to get skipped entirely.

2. DrahtBot added the label Scripts and tools on Mar 24, 2026
3. 
   DrahtBot commented at 4:37 PM on March 24, 2026: contributor

   <!--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/34914.

   <!--021abf342d371248e50ceaed478a90ca-->

   Reviews

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

   Type	Reviewers
   ACK	ArikShalom
   Concept ACK	hebasto

   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.

   <!--174a7506f384e20aa4161008e828411d-->

   Conflicts

   Reviewers, this pull request conflicts with the following ones:

   • #35468 (ci, iwyu: Request IPC file generation explicitly by hebasto)

   If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

   <!--5faf32d7da4f0f540f40219e4f7537a3-->

4. Sjors renamed this:
   contrib: remove deprecated --deep codesign flag
   contrib: replace deprecated --deep codesign flag with minimal signing
   on Mar 24, 2026
5. 
   hebasto commented at 4:44 PM on March 24, 2026: member

   Concept ACK. This is a smaller diff than in #33592 :)

6. fanquake added this to the milestone 32.0 on Mar 25, 2026
7. 
   fanquake commented at 3:07 AM on March 25, 2026: member

   codesign --verify --deep --strict --verbose=4

   You can also update the CI check to also use --strict: https://github.com/bitcoin/bitcoin/blob/2fe76ed8324af44c985b96455a05c3e8bec0a03e/ci/test/03_test_script.sh#L160

8. Sjors force-pushed on Mar 25, 2026
9. 
   Sjors commented at 8:29 AM on March 25, 2026: member

   You can also update the CI check to also use --strict

   Done

10. ci: avoid modifying GOAL in 03_test_script.sh

    The modification caused "codesign --verify" to be silently skipped.
    
    Introduce BUILD_TARGETS for the cmake target list so GOAL remains
    unmodified throughout the script.

    d79ef13c27
11. contrib: remove deprecated --deep codesign flag

    Replace the deprecated `codesign --deep` with explicit per-component
    signing of Frameworks, Plugins and the top-level bundle.
    
    CI is updated to verify with --strict.
    
    Can be verified with:
    codesign --verify --deep --strict --verbose=4 build/dist/Bitcoin-Qt.app
    
    Co-authored-by: amisha <amishhhaaaa@gmail.com>

    5a102a5fc2
12. Sjors force-pushed on Mar 25, 2026
13. 
    Sjors commented at 10:16 AM on March 25, 2026: member

    I noticed that codesign --verify --strict didn't appear in the log. Looks like we forgot to check that in #34787 review. The problem is that 03_test_script.sh modifies GOAL. Added a commit to reduce the chances of that happening again.

14. Sjors renamed this:
    contrib: replace deprecated --deep codesign flag with minimal signing
    contrib: replace deprecated --deep codesign flag, fix accidental sign --verify skip on ci
    on Mar 25, 2026
15. Sjors renamed this:
    contrib: replace deprecated --deep codesign flag, fix accidental sign --verify skip on ci
    contrib: replace deprecated --deep codesign flag, fix accidental --verify skip on ci
    on Mar 25, 2026
16. Sjors marked this as a draft on Mar 25, 2026
17. 
    Sjors commented at 10:36 AM on March 25, 2026: member

    The GOAL mutation was introduced here: https://github.com/bitcoin/bitcoin/pull/33810/changes/2c78814e0e182853ce44d9fd63d24ee6cab5223e

    It was trying to avoid passing all to cmake --build when GOAL=codegen, which this PR addresses by introducing BUILD_TARGETS instead.

18. Sjors marked this as ready for review on Mar 25, 2026
19. sedited requested review from hebasto on Apr 26, 2026
20. 
    ArikShalom commented at 5:51 PM on July 5, 2026: none

    Tested ACK 5a102a5fc2d1346f6a3aef2008610aea80aaefa7

    Tested on macOS 26 (Darwin 25.5.0), arm64, Apple clang 21, Qt 6 from Homebrew.

    Built the deploy target on this branch and on master (2063f02bd5). The PR contains no src/ changes, so this isolates the signing method:

    • This branch (per-component signing): codesign --verify --deep --strict --verbose=4 build/dist/Bitcoin-Qt.app → valid on disk / satisfies its Designated Requirement. All 32 nested components under Contents/Frameworks/ and Contents/PlugIns/ also pass individual codesign --verify --strict, and the signed binary runs.
    • master (--deep signing): the same checks also pass today — so no regression from switching; the gain is no longer relying on a flag deprecated since macOS 13, and signing inside-out (components first, bundle last) per Apple's guidance.

    For d79ef13c27, code review only (I did not run the CI scripts): the previous GOAL mutation meant the "install deploy" comparison could never match, silently skipping the codesign check — BUILD_TARGETS restores it correctly.

21. in ci/test/03_test_script.sh:159 in 5a102a5fc2

     157 | @@ -157,7 +158,7 @@ fi
     158 |  
     159 |  if [[ "$CI_OS_NAME" == "macos" && "${GOAL}" = "install deploy" ]]; then
    

    ---

    ---

    

    fanquake commented at 10:50 AM on August 26, 2026:

    Rather than your BUILD_TARGETS/GOAL changes, I think you could just do:

    if [[ "$CI_OS_NAME" == "macos" && "${RUN_FUZZ_TESTS}" = "false" ]]; then
    

    ---

    

    maflcko commented at 11:07 AM on August 26, 2026:

    Would using RUN_FUZZ_TESTS break when someone is running something like:

    # RUN_FUZZ_TESTS=false USER=dummy_user DANGER_RUN_CI_ON_HOST="1" MAKEJOBS="-j4" FILE_ENV="./ci/test/00_setup_env_mac_native_fuzz.sh" ./ci/test_run_all.sh 
    

    (not sure if this is possible)

    ---

    

    Sjors commented at 3:09 PM on August 26, 2026:

    @fanquake I think that makes the intention less clear, i.e. we only sign things that need to be signed. Otherwise we might as well also sign the fuzz build?

    ---

    

    fanquake commented at 3:24 PM on August 26, 2026:

    Otherwise we might as well also sign the fuzz build?

    Not sure what you mean, as it has no GUI or related deploy targets to sign? (The binaries it does have are also already implicitly (self) signed, otherwise the binaries wouldn't run).

    ---

    

    Sjors commented at 3:47 PM on August 26, 2026:

    Right, we don't sign because CI needs it, we sign to test part of the release process. That's why imo checking GOAL for install deploy is more clear than checking that we're not fuzzing (RUN_FUZZ_TESTS = "false").

    ---

    

    fanquake commented at 4:16 PM on August 26, 2026:

    (not sure if this is possible)

    I am not sure.

    checking GOAL for install deploy

    Ok. I feel like there should be a more targeted way to fix this, which doesn't require changing the CI globally, but also don't want to spend too much more time here. Will mark as resolved.

Contributors

SjorsDrahtBothebastofanquakeArikShalommaflcko

Review Requested
hebasto

Milestone
32.0