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

pull Sjors wants to merge 2 commits into bitcoin:master from Sjors:2026/03/deep-sign changing 2 files +18 −5
  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 willcl-ark, fanquake
    Concept ACK hebasto
    Stale ACK ArikShalom

    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

    No conflicts as of last run.

    <!--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. Sjors force-pushed on Mar 25, 2026
  11. 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.

  12. 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
  13. 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
  14. Sjors marked this as a draft on Mar 25, 2026
  15. 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.

  16. Sjors marked this as ready for review on Mar 25, 2026
  17. sedited requested review from hebasto on Apr 26, 2026
  18. 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.

  19. in ci/test/03_test_script.sh:159 in 5a102a5fc2 outdated 
     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.

  20. fanquake commented at 11:21 AM on September 9, 2026: member

    This needs a rebase.

  21. DrahtBot added the label Needs rebase on Sep 9, 2026
  22. 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.
    ad4eeaf859
  23. Sjors force-pushed on Sep 9, 2026
  24. Sjors commented at 4:39 PM on September 9, 2026: member

    Rebased after #35468 (just context).

  25. DrahtBot removed the label Needs rebase on Sep 9, 2026
  26. in ci/test/03_test_script.sh:186 in 98738c56dd 
     182 | @@ -182,7 +183,7 @@ fi
 183 |  
 184 |  if [[ "$CI_OS_NAME" == "macos" && "${GOAL}" = "install deploy" ]]; then
 185 |    unzip "${BASE_BUILD_DIR}/bitcoin-macos-app.zip" -d "${BASE_BUILD_DIR}/deploy"
 186 | -  if ! ( codesign --verify "${BASE_BUILD_DIR}/deploy/Bitcoin-Qt.app" ); then
 187 | +  if ! ( codesign --verify --strict "${BASE_BUILD_DIR}/deploy/Bitcoin-Qt.app" ); then
    willcl-ark commented at 8:04 AM on September 10, 2026:

    Reading https://developer.apple.com/library/archive/technotes/tn2206/_index.html#//apple_ref/doc/uid/DTS40007919-CH1-TNTAG404

    ISTM that we might want to add --deep here?

    Using the codesign Tool's --deep Option Correctly

    When verifying signatures, add --deep to perform recursive validation of nested code. Without --deep, validation will be shallow: it will check the immediate nested content but not check that fully. Note that Gatekeeper always performs --deep style validation.

    Important: While the --deep option can be applied to a signing operation, this is not recommended. We recommend that you sign code inside out in individual stages (as Xcode does automatically). Signing with --deep is for emergency repairs and temporary adjustments only.

    Note that signing with the combination --deep --force will forcibly re-sign all code in a bundle.

    Sjors commented at 11:56 AM on September 10, 2026:

    Let me try that...

  27. 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 --deep --strict.

Can be verified with:
codesign --verify --deep --strict --verbose=4 build/dist/Bitcoin-Qt.app

Co-authored-by: amisha <amishhhaaaa@gmail.com>
    da7d7dbc7c
  28. Sjors force-pushed on Sep 10, 2026
  29. sedited requested review from willcl-ark on Sep 11, 2026
  30. willcl-ark approved
  31. willcl-ark commented at 8:42 AM on September 11, 2026: member

    Light ACK da7d7dbc7c0040d5e91fe21e19fdc1cb3b8ea7b2

    Not an area of the codebase I am that familiar with, but the changes seem reasonable to me.

    I tested this on MacOS 26.6.2 (rebased on master with merge-base 1a4e034ddb) and it worked.

    I ran:

    cmake -B build -DBUILD_GUI=ON
cmake --build build -j12
rm -rf build/Bitcoin-Qt.app build/dist build/deploy build/bitcoin-macos-app.zip
cmake --build build --target deploy -j12

unzip build/bitcoin-macos-app.zip -d build/deploy
codesign --verify --deep --strict --verbose=4 build/deploy/Bitcoin-Qt.app

    Which got me:

    <snip>
build/deploy/Bitcoin-Qt.app/: valid on disk
build/deploy/Bitcoin-Qt.app/: satisfies its Designated Requirement

    This seems to be the expected output.

    Signing frameworks and plugins before signing the containing app follows apples recommended approach (vs using --deep).

    I guess one thing is that now we need to make sure is that we don't add anything which isn't covered by the explicit target list (Frameworks// Plugins//) but asking ChatGPT these seem to be things like "Login items" and "XPC Services"? so it seems like we'll probably be safe here.

  32. Sjors commented at 9:47 AM on September 11, 2026: member

    but asking ChatGPT these seem to be things like "Login items" and "XPC Services"? so it seems like we'll probably be safe here.

    I think so too, and CI would catch that, since the --verify step does use --deep (rather than our narrow list).

  33. fanquake commented at 9:55 AM on September 11, 2026: member

    ACK da7d7dbc7c0040d5e91fe21e19fdc1cb3b8ea7b2 - I think we should try and cleanup macdeployqtplus somewhat, but that can happen in future.

  34. fanquake merged this on Sep 11, 2026
  35. fanquake closed this on Sep 11, 2026
  36. Sjors deleted the branch on Sep 11, 2026
Contributors
SjorsDrahtBothebastofanquakeArikShalommaflckowillcl-ark


hebasto

Labels
Scripts and tools

Milestone
32.0

Linked (view graph)
#32486 build: deprecated arg usage in macOS deploy script#33592 contrib: remove deprecated --deep signing from macdeployqtplus#34787 build: fix native macOS deployment
github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-09-15 18:51 UTC
This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me