Skip to content

fix(macos): configure release signing and notarization#422

Open
penso wants to merge 1 commit intomainfrom
ios-signatures
Open

fix(macos): configure release signing and notarization#422
penso wants to merge 1 commit intomainfrom
ios-signatures

Conversation

@penso
Copy link
Copy Markdown
Collaborator

@penso penso commented Mar 12, 2026

Summary

  • add macOS app signing configuration and entitlements for release builds
  • import the Apple certificate in the release workflow, sign the app, notarize it, staple the ticket, and repack the artifact
  • disable code signing in CI/local macOS validation builds and document the updated validation command

Validation

Completed

  • ./scripts/generate-swift-project.sh
  • git rebase origin/main
  • git push -u origin ios-signatures

Remaining

  • ./scripts/lint-swift.sh (fails on existing function_body_length violation in apps/macos/Sources/ConfigurationPane.swift:225)
  • xcodebuild -project apps/macos/Moltis.xcodeproj -scheme Moltis -configuration Release -destination "platform=macOS" -derivedDataPath apps/macos/.derivedData-local-validate CODE_SIGNING_ALLOWED=NO build (fails in this environment because apps/macos/Sources/Bridging-Header.h cannot find moltis_bridge.h)

Manual QA

  • Not run

@greptile-apps
Copy link
Copy Markdown
Contributor

greptile-apps bot commented Mar 12, 2026

Greptile Summary

This PR wires up macOS release signing and notarization for the Moltis macOS app — adding Apple certificate import, Developer ID Application signing, xcrun notarytool notarization with stapling, and keychain cleanup to the release workflow, while disabling code signing in CI and local-validate builds.

Key changes and concerns:

  • Release workflow (release.yml): The signing-specific xcodebuild flags (CODE_SIGN_STYLE=Manual, CODE_SIGN_IDENTITY, DEVELOPMENT_TEAM, OTHER_CODE_SIGN_FLAGS) are applied to the "Build macOS app" step unconditionally, but the "Import Apple certificate" step that populates the keychain is gated on RELEASE_DRY_RUN != 'true'. A dry run will reach the build step with no signing identity in any keychain and fail. The build step must be split or guarded to use CODE_SIGNING_ALLOWED=NO for dry runs.
  • project.yml: CODE_SIGN_STYLE: Manual and CODE_SIGN_IDENTITY: "Developer ID Application" are placed in the base settings block (applies to all configurations). This will cause Xcode Debug builds to fail for any developer who doesn't have the distribution certificate installed locally. These settings should be scoped to the Release configuration only.
  • Notarization flow: Submitting the zip for notarization and then stapling the original .app bundle before repacking is the correct approach per Apple guidelines.
  • CI / local-validate: CODE_SIGNING_ALLOWED=NO added correctly in both ci.yml and local-validate.sh; entitlements file is minimal and correct for Hardened Runtime.

Confidence Score: 2/5

  • Not safe to merge as-is — dry-run builds will be broken and the project.yml signing config will impact developer experience.
  • Two logic issues need addressing: the unconditional signing parameters in the build step will cause failures during dry runs (when no certificate is imported), and placing Manual code-signing settings in the base project configuration will break Xcode Debug builds for most contributors.
  • .github/workflows/release.yml (dry-run/signing conditionality) and apps/macos/project.yml (signing settings scoped to base instead of Release only)

Important Files Changed

Filename Overview
.github/workflows/release.yml Adds Apple certificate import, signed xcodebuild invocation, notarization, stapling, repack, and keychain cleanup. Two issues: (1) the signing build flags are applied unconditionally while the certificate import is gated on non-dry-run, causing dry-run builds to fail; (2) minor blank-line removal between steps.
apps/macos/project.yml Adds entitlements path, hardened runtime, and manual code-signing settings. The signing settings are placed in base (applies to Debug + Release), which will break Xcode Debug builds for developers without the "Developer ID Application" certificate.
apps/macos/Moltis.entitlements New empty entitlements plist required for Hardened Runtime; correct boilerplate for a minimal entitlements set.
.github/workflows/ci.yml Adds CODE_SIGNING_ALLOWED=NO to the CI macOS xcodebuild invocation; straightforward and correct.
CLAUDE.md Updates documented macOS local-validate command to include CODE_SIGNING_ALLOWED=NO; consistent with the script change.
scripts/local-validate.sh Adds CODE_SIGNING_ALLOWED=NO to the default macOS xcodebuild command; consistent with the CLAUDE.md and CI changes.

Sequence Diagram

sequenceDiagram
    participant GHA as GitHub Actions
    participant KC as macOS Keychain
    participant XC as xcodebuild
    participant NT as notarytool
    participant ST as stapler

    GHA->>KC: Import Apple certificate (if !dry_run)
    GHA->>XC: Build with CODE_SIGN_STYLE=Manual<br/>CODE_SIGN_IDENTITY="Developer ID Application"<br/>DEVELOPMENT_TEAM (always – even in dry_run ⚠️)
    XC-->>GHA: Moltis.app (signed)
    GHA->>GHA: ditto → moltis-VERSION-macos.app.zip
    GHA->>NT: notarytool submit zip --wait (if !dry_run)
    NT-->>GHA: Notarization approved
    GHA->>ST: stapler staple Moltis.app (if !dry_run)
    ST-->>GHA: Ticket stapled
    GHA->>GHA: rm zip, ditto → new zip (stapled)
    GHA->>GHA: Sign with Sigstore + upload artifact
    GHA->>KC: security delete-keychain (always)
Loading

Last reviewed commit: 50c1a06

Comment on lines 901 to 916
- name: Build macOS app
env:
DERIVED_DATA_DIR: apps/macos/.derivedData-ci
APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }}
run: |
xcodebuild \
-project apps/macos/Moltis.xcodeproj \
-scheme Moltis \
-configuration Release \
-destination "platform=macOS" \
-derivedDataPath "$DERIVED_DATA_DIR" \
CODE_SIGN_STYLE=Manual \
CODE_SIGN_IDENTITY="Developer ID Application" \
DEVELOPMENT_TEAM="$APPLE_TEAM_ID" \
OTHER_CODE_SIGN_FLAGS="--options runtime" \
build
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Signing parameters applied unconditionally while certificate import is conditional

The "Import Apple certificate" step is gated on RELEASE_DRY_RUN != 'true', but the "Build macOS app" step always runs — including the CODE_SIGN_STYLE=Manual, CODE_SIGN_IDENTITY="Developer ID Application", and DEVELOPMENT_TEAM overrides. During a dry run, no certificate is imported into the keychain, so xcodebuild will fail immediately with "code signing identity 'Developer ID Application' does not match any code signing certificate in your keychain".

You should guard the signing parameters with the same condition, or add a separate code path that builds with CODE_SIGNING_ALLOWED=NO for dry runs. For example:

      - name: Build macOS app (dry run – no signing)
        if: ${{ env.RELEASE_DRY_RUN == 'true' }}
        env:
          DERIVED_DATA_DIR: apps/macos/.derivedData-ci
        run: |
          xcodebuild \
            -project apps/macos/Moltis.xcodeproj \
            -scheme Moltis \
            -configuration Release \
            -destination "platform=macOS" \
            -derivedDataPath "$DERIVED_DATA_DIR" \
            CODE_SIGNING_ALLOWED=NO \
            build

      - name: Build macOS app (release – signed)
        if: ${{ env.RELEASE_DRY_RUN != 'true' }}
        env:
          DERIVED_DATA_DIR: apps/macos/.derivedData-ci
          APPLE_TEAM_ID: ${{ secrets.APPLE_TEAM_ID }}
        run: |
          xcodebuild \
            -project apps/macos/Moltis.xcodeproj \
            -scheme Moltis \
            -configuration Release \
            -destination "platform=macOS" \
            -derivedDataPath "$DERIVED_DATA_DIR" \
            CODE_SIGN_STYLE=Manual \
            CODE_SIGN_IDENTITY="Developer ID Application" \
            DEVELOPMENT_TEAM="$APPLE_TEAM_ID" \
            OTHER_CODE_SIGN_FLAGS="--options runtime" \
            build

Comment on lines +40 to +42
ENABLE_HARDENED_RUNTIME: YES
CODE_SIGN_STYLE: Manual
CODE_SIGN_IDENTITY: "Developer ID Application"
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manual signing identity in base settings breaks Debug builds for most developers

CODE_SIGN_STYLE: Manual and CODE_SIGN_IDENTITY: "Developer ID Application" are placed in the base settings block, which means they apply to both Debug and Release configurations. When a developer opens the project in Xcode and runs a normal Debug build without the CODE_SIGNING_ALLOWED=NO command-line override, Xcode will look for a "Developer ID Application" certificate in their keychain. Most contributors won't have this certificate (it requires a paid account with the specific distribution cert), causing the build to fail immediately in Xcode.

Consider scoping these signing settings to the Release configuration only:

    settings:
      base:
        PRODUCT_BUNDLE_IDENTIFIER: org.moltis.app
        GENERATE_INFOPLIST_FILE: YES
        SWIFT_EMIT_LOC_STRINGS: NO
        ASSETCATALOG_COMPILER_APPICON_NAME: AppIcon
        SWIFT_OBJC_BRIDGING_HEADER: Sources/Bridging-Header.h
        ENABLE_HARDENED_RUNTIME: YES
        OTHER_LDFLAGS:
          ...
      configs:
        Release:
          CODE_SIGN_STYLE: Manual
          CODE_SIGN_IDENTITY: "Developer ID Application"

Copy link
Copy Markdown

@chatgpt-codex-connector chatgpt-codex-connector bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 50c1a06fa0

ℹ️ About Codex in GitHub

Your team has set up Codex to 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 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment on lines +912 to +914
CODE_SIGN_STYLE=Manual \
CODE_SIGN_IDENTITY="Developer ID Application" \
DEVELOPMENT_TEAM="$APPLE_TEAM_ID" \
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P1 Badge Skip signing in dry-run macOS release builds

In .github/workflows/release.yml, this step now always forces manual Developer ID signing, but the certificate import is gated behind if: ${{ env.RELEASE_DRY_RUN != 'true' }}. For workflow_dispatch dry runs, no signing cert is installed while xcodebuild still requires one, so the macOS job fails before packaging/notarization logic can be exercised. This makes the workflow’s dry-run mode effectively unusable.

Useful? React with 👍 / 👎.

Comment on lines +41 to +42
CODE_SIGN_STYLE: Manual
CODE_SIGN_IDENTITY: "Developer ID Application"
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Restrict manual Developer ID signing to release builds

Adding manual signing at the target base settings level applies it to all generated macOS builds, not just release packaging. Local developer workflows (just swift-build / just swift-test, via scripts/build-swift.sh and scripts/test-swift.sh) call xcodebuild without CODE_SIGNING_ALLOWED=NO, so machines without a Developer ID certificate will now fail to build/test the app. This is a regression in day-to-day macOS development.

Useful? React with 👍 / 👎.

@codspeed-hq
Copy link
Copy Markdown
Contributor

codspeed-hq bot commented Mar 12, 2026

Merging this PR will not alter performance

✅ 39 untouched benchmarks
⏩ 5 skipped benchmarks1


Comparing ios-signatures (50c1a06) with main (742bce9)

Open in CodSpeed

Footnotes

  1. 5 benchmarks were skipped, so the baseline results were used instead. If they were deleted from the codebase, click here and archive them to remove them from the performance reports.

@codecov
Copy link
Copy Markdown

codecov bot commented Mar 12, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

@penso
Copy link
Copy Markdown
Collaborator Author

penso commented Mar 24, 2026

Summary

  • The PR moves the macOS release job toward proper Developer ID signing and notarization, and it correctly disables signing in CI/local validation paths that are meant to stay unsigned.
  • Two regressions remain: the documented release dry-run path now appears broken, and the shared macOS project now hardcodes release-only signing defaults that will break normal local app builds for contributors without the release certificate.

Risks

  • workflow_dispatch dry runs still execute the signed macOS build even though certificate import is skipped, so the build is likely to fail before packaging.
  • apps/macos/project.yml now requires manual Developer ID Application signing by default, which leaks release-only requirements into everyday just swift-build, just swift-run, and scripts/test-swift.sh flows.

Required fixes

  • In .github/workflows/release.yml, make the dry-run path skip the signed macOS build or force an unsigned build (CODE_SIGNING_ALLOWED=NO) when RELEASE_DRY_RUN == 'true'. Right now only the certificate/notary steps are gated, but the build step still requests manual signing.
  • In apps/macos/project.yml, remove the committed manual Developer ID defaults from the shared project spec, or make them opt-in via a release-only xcconfig/workflow override. Local builds should not require the release certificate.

Optional improvements

  • Add a post-build verification step for the signed artifact, for example codesign --verify and spctl --assess, so release failures are caught before upload.
  • Add coverage for the dry-run release path, since this PR changes behavior there and there is no validation evidence in the diff.

Verdict

request_changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant