Repository navigation
feat: Add macOS code signing and notarization - #1528
Conversation
- Add entitlements file for macOS web server capabilities - Update release workflow to sign and notarize macOS binaries - Add local signing script for testing on macOS - Add documentation for Apple Developer setup The workflow now: 1. Imports Apple Developer ID certificate 2. Signs the macOS binary with entitlements 3. Submits for notarization to Apple 4. Staples the notarization ticket Users will no longer see Gatekeeper warnings when running SQLPage on macOS. Required GitHub Secrets: - APPLE_SIGNING_CERTIFICATE_P12_BASE64 - APPLE_SIGNING_CERTIFICATE_PASSWORD - APPLE_SIGNING_IDENTITY - APPLE_NOTARIZATION_APPLE_ID - APPLE_NOTARIZATION_PASSWORD (App-Specific Password) - APPLE_NOTARIZATION_TEAM_ID
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
Prevent accidental commit of: - .env files containing secrets - Apple Developer certificates (*.cer, *.p12, *.key, *.csr, *.pem) - Generated signing files (sqlpage.key, sqlpage.p12, etc.)
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 1aa283d66f
ℹ️ 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".
| --wait | ||
|
|
||
| # Staple the notarization ticket | ||
| xcrun stapler staple target/${{ matrix.target }}/superoptimized/sqlpage |
There was a problem hiding this comment.
In the build-macos-windows job, this targets stapler at the standalone Mach-O executable. The stapler(1) supported-formats documentation limits stapling to UDIF images, signed flat packages, and executable bundles such as .app, so this command returns nonzero even after successful notarization and prevents the macOS matrix job—and therefore every tagged release—from completing. Package the binary in a supported signed container and staple that, or omit this unsupported step for the standalone executable.
Useful? React with 👍 / 👎.
| - name: Upload artifact (non-Windows) | ||
| if: matrix.os != 'windows-latest' | ||
| uses: actions/upload-artifact@v7 |
There was a problem hiding this comment.
In the Windows leg of build-macos-windows, this step now runs after the signed binary was already uploaded under the same sqlpage windows-latest artifact name. Upload artifacts are immutable and an upload fails by default when that name already exists, as documented by actions/upload-artifact, so the second upload fails the Windows matrix job and blocks create_release; it would also select the unsigned build output rather than the SignPath result. Restore the non-Windows condition.
Useful? React with 👍 / 👎.
| if: matrix.os == 'macos-latest' | ||
| run: | | ||
| # Sign the binary with entitlements | ||
| codesign --force --options runtime --entitlements sqlpage.entitlements --sign "${{ secrets.APPLE_SIGNING_IDENTITY }}" --timestamp target/${{ matrix.target }}/superoptimized/sqlpage |
There was a problem hiding this comment.
For macOS users connecting through ODBC, --options runtime enables library validation, while the new entitlements omit com.apple.security.cs.disable-library-validation. The release binary statically includes only the unixODBC manager and still loads the separately installed database driver, so drivers not signed by Apple or SQLPage's Team ID will be rejected at runtime; Apple explicitly requires the exception for third-party plug-ins and libraries. Add the exception or otherwise arrange for compatible signing of every supported driver.
AGENTS.md reference: AGENTS.md:L30-L35
Useful? React with 👍 / 👎.
| - name: Sign macOS binary | ||
| if: matrix.os == 'macos-latest' |
There was a problem hiding this comment.
This adds signing and notarization specifically to remove Gatekeeper warnings for downloaded macOS releases, which is a user-visible feature, but the commit does not add a corresponding CHANGELOG.md entry. Record the impact under the current unreleased version as required by the repository's release-note convention.
AGENTS.md reference: AGENTS.md:L91-L91
Useful? React with 👍 / 👎.
| exit 1 | ||
| fi | ||
|
|
||
| if ! command -v notarytool &> /dev/null; then |
There was a problem hiding this comment.
On a standard Xcode or Command Line Tools installation, notarytool is selected from the active developer directory rather than necessarily appearing directly on PATH; Apple documents invoking it with xcrun and locating it with xcrun --find notarytool. Consequently this command -v check can abort the helper even though the later xcrun notarytool invocation would work; the analogous stapler check has the same problem. Validate with xcrun --find instead.
Useful? React with 👍 / 👎.
|
|
||
| # Build the binary | ||
| echo "Building SQLPage for aarch64-apple-darwin..." | ||
| cargo build --profile superoptimized --locked --target aarch64-apple-darwin --features "odbc-static" |
There was a problem hiding this comment.
On a fresh checkout, this helper reaches cargo build without running the frontend setup used by the release workflow. frontend/dist is ignored, while build.rs unconditionally opens frontend/dist/tabler-sprite.svg and aborts with instructions to run npm ci && npm run build, so the documented local signing command cannot build its input binary unless the developer happens to have stale generated assets. Run the frontend build in this script or document it as a prerequisite.
AGENTS.md reference: AGENTS.md:L147-L147
Useful? React with 👍 / 👎.
| # Final verification | ||
| echo "Final verification..." | ||
| codesign --verify --deep --strict --verbose=2 "$BINARY_PATH" | ||
| spctl -a -v "$BINARY_PATH" |
There was a problem hiding this comment.
When any notarization credential is absent, the script explicitly takes the supported signing-only path and prints that notarization is being skipped, but it then unconditionally assesses the binary with spctl. A Developer ID-signed but unnotarized executable is denied as Unnotarized Developer ID, and spctl returns exit code 3 for a denied assessment; with set -e, the helper therefore fails instead of completing the advertised signing-only run. Only perform this assessment after successful notarization, or treat the expected signing-only denial separately.
Useful? React with 👍 / 👎.
| <key>com.apple.security.cs.allow-unsigned-executable-memory</key> | ||
| <true/> | ||
| <key>com.apple.security.cs.allow-jit</key> | ||
| <true/> |
There was a problem hiding this comment.
The new signature grants both JIT execution and unrestricted unsigned executable memory to every macOS release, although SQLPage does not embed a native JIT or use the rare legacy mechanisms that require these exceptions. In particular, Apple warns that allow-unsigned-executable-memory exposes applications to common vulnerabilities in memory-unsafe code and recommends enabling only essential hardened-runtime exceptions. Remove these entitlements unless a concrete dependency is verified to require them.
Useful? React with 👍 / 👎.
| openssl pkcs12 -export -out sqlpage.p12 -inkey sqlpage.key -in sqlpage.pem | ||
| ``` | ||
|
|
||
| ## Step 2: Create an App Store Connect API Key for Notarization |
There was a problem hiding this comment.
This setup step tells maintainers to create and download an App Store Connect API key, but neither the workflow nor the local script accepts the key ID, issuer ID, or downloaded .p8 file; both authenticate exclusively with an Apple ID and app-specific password configured in the next section. Anyone following the guide creates an unused privileged credential and still must configure the unrelated password flow. Remove this API-key step or update the commands and secrets to actually use API-key authentication.
Useful? React with 👍 / 👎.
.env files should never be committed to version control. All secrets are now stored securely in GitHub Actions secrets.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 726b5ea591
ℹ️ 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".
| /playwright-report/ | ||
|
|
||
| # Environment and secrets | ||
| .env |
There was a problem hiding this comment.
Ignoring and deleting the checked-in .env removes the only default COMPOSE_PROFILES value. On a fresh checkout, docker compose up enables the unprofiled web service while its ${COMPOSE_PROFILES-postgres} dependency resolves to the profile-gated postgres service; Compose does not automatically activate profiles for referenced dependencies and therefore rejects this model (Docker Compose profiles documentation). Keep a non-secret default profile or make the default database service unprofiled before removing this file.
AGENTS.md reference: AGENTS.md:L106-L106
Useful? React with 👍 / 👎.
|
|
||
| # Build the binary | ||
| echo "Building SQLPage for aarch64-apple-darwin..." | ||
| cargo build --profile superoptimized --locked --target aarch64-apple-darwin --features "odbc-static" |
There was a problem hiding this comment.
On an Intel Mac with a fresh Rust installation, this hard-coded aarch64-apple-darwin build fails before signing because rustup installs only the host platform's standard library, and rust-toolchain.toml does not request the ARM target. The documented local procedure applies to any macOS machine and does not tell Intel users to install it; add rustup target add aarch64-apple-darwin, declare the target in the toolchain file, or build for the detected host architecture as appropriate (rustup cross-compilation documentation).
Useful? React with 👍 / 👎.
|
|
||
| # Sign the binary | ||
| echo "Signing the binary..." | ||
| codesign --force --options runtime --entitlements sqlpage.entitlements --sign "$APPLE_SIGNING_IDENTITY" --timestamp "$BINARY_PATH" |
There was a problem hiding this comment.
On a Mac following the new setup guide from scratch, the certificate and private key exist only in sqlpage.p12: neither the guide nor this script imports that identity into a keychain. Consequently codesign --sign "$APPLE_SIGNING_IDENTITY" cannot find the identity and the documented local test stops here; code-signing identities must be available in an accessible keychain (Apple code-signing guide). Import the P12 before signing or add an explicit Keychain Access/import step to the guide.
Useful? React with 👍 / 👎.
- Move entitlements to .github/macos/entitlements.plist (out of repo root) - Remove docs/macos-signing-setup.md (setup instructions moved to PR description) - Combine macOS signing steps into a single concise step in release.yml - Use a temporary keychain instead of apple-actions/import-codesign-certs - Fix stapler: staple the zip container, not the bare Mach-O executable - Add disable-library-validation entitlement for third-party ODBC drivers - Remove unnecessary JIT/unsigned-memory entitlements - Restore .env with safe defaults (COMPOSE_PROFILES) for docker-compose - Update sign-macos.sh: use xcrun --find, build frontend first, import P12 to keychain - Add CHANGELOG.md entry for the user-visible signing change - Clean up .gitignore (remove *.key/*.pem blanket patterns that could hide real keys)
The macOS install tutorial now links directly to the sqlpage-macos.tgz download and notes that it is code-signed and notarized by Apple, so no Gatekeeper warning appears on first run.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ba7a8cb3ea
ℹ️ 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".
| --password "$APPLE_NOTARIZATION_PASSWORD" \ | ||
| --team-id "$APPLE_NOTARIZATION_TEAM_ID" \ | ||
| --wait | ||
| xcrun stapler staple sqlpage-macos.zip |
There was a problem hiding this comment.
The revised code now targets the generated ZIP instead of the bare executable, but Apple explicitly states that a ZIP may be notarized but cannot be stapled directly; stapler only supports app bundles, disk images, and flat installer packages (Apple notarization documentation). Consequently this command returns nonzero under set -euo pipefail, failing every macOS release job before artifact upload; the same unsupported ZIP stapling is also present in scripts/sign-macos.sh.
Useful? React with 👍 / 👎.
| security create-keychain -p "" /tmp/signing.keychain | ||
| security set-keychain-settings -lut 3600 /tmp/signing.keychain | ||
| security unlock-keychain -p "" /tmp/signing.keychain | ||
| security import /tmp/signing.p12 -k /tmp/signing.keychain -P "${{ secrets.APPLE_SIGNING_CERTIFICATE_PASSWORD }}" -T /usr/bin/codesign |
There was a problem hiding this comment.
If the certificate password contains shell-significant characters such as ", $, backticks, or a newline, GitHub substitutes the secret directly into the generated Bash source before execution, so the password can be altered, trigger expansion, or make the script invalid; with set -u, even an ordinary $name fragment can abort the release. GitHub recommends assigning secrets to an intermediate environment variable and quoting that variable in the command (GitHub secrets documentation).
Useful? React with 👍 / 👎.
The .env file with COMPOSE_PROFILES=mariadb must remain tracked in git. CI runs 'docker compose up --wait postgres' which requires the web service's depends_on to resolve correctly. Without .env, COMPOSE_PROFILES is unset and the web service depends on 'postgres' which is profile-gated, causing: service "web" depends on undefined service "postgres": invalid compose project Secrets should go in .env.local (which is gitignored), not .env.
- Remove stapler step: it does not support bare Mach-O executables or zip archives. Notarization alone is sufficient for standalone binaries — Gatekeeper checks Apple's servers online. - Pass P12 base64 and password through environment variables instead of inline secret interpolation to avoid shell injection issues with special characters. - Update sign-macos.sh to match: remove stapler, add explanatory comment.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ebc0629172
ℹ️ 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".
| xcrun notarytool submit sqlpage-macos.zip \ | ||
| --apple-id "$APPLE_NOTARIZATION_APPLE_ID" \ | ||
| --password "$APPLE_NOTARIZATION_PASSWORD" \ | ||
| --team-id "$APPLE_NOTARIZATION_TEAM_ID" \ | ||
| --wait |
There was a problem hiding this comment.
When Apple finishes processing with Invalid or Rejected, notarytool submit --wait can still exit successfully; --wait is documented to return for all terminal states, including Accepted, Invalid, and Rejected (notarytool manual). The following codesign --verify checks only the local signature, so this job can upload and release an unnotarized binary despite the workflow and documentation claiming success. Request structured output and explicitly fail unless the returned status is Accepted.
Useful? React with 👍 / 👎.
| <!DOCTYPE plist PUBLIC "-//Apple//DTD PLIST 1.0//EN" "http://www.apple.com/DTDs/PropertyList-1.0.dtd"> | ||
| <plist version="1.0"> | ||
| <dict> | ||
| <!-- Allow loading third-party ODBC drivers (not signed by Apple or our Team ID) --> |
There was a problem hiding this comment.
This plist is passed directly to codesign, but it contains XML comments here and again on lines 8 and 14. Apple requires distribution-signing entitlement files to use standard XML with no comments because the embedded entitlement data may otherwise be rejected by macOS; its guidance recommends normalizing the file with plutil when necessary (Apple distribution-signing documentation). Remove all comments from the plist or generate a normalized comment-free copy before signing so the released executable is not rejected at launch.
Useful? React with 👍 / 👎.
| > [the precompiled binaries for Apple silicon](https://github.com/sqlpage/SQLPage/releases/latest), | ||
| > [docker](https://hub.docker.com/repository/docker/lovasoa/SQLPage/general), | ||
| > [the precompiled binary for Apple silicon](https://github.com/sqlpage/SQLPage/releases/latest/download/sqlpage-macos.tgz). | ||
| > The download is code-signed and notarized by Apple, so macOS will not show a security warning when you first run it. |
There was a problem hiding this comment.
When this archive is downloaded through a quarantine-aware mechanism such as Safari, macOS still requests the user's approval the first time the executable is opened even if its Developer ID signature and notarization are valid; Apple explicitly documents that first-launch confirmation for software distributed outside the App Store (Apple Gatekeeper guidance). Signing and notarization remove the blocking unidentified-developer/malware warning, but not every security prompt, so this sentence—and the equivalent changelog claim—sets an expectation the delivered artifact cannot meet.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 98a04e5d51
ℹ️ 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".
| APPLE_SIGNING_IDENTITY: ${{ secrets.APPLE_SIGNING_IDENTITY }} | ||
| APPLE_NOTARIZATION_APPLE_ID: ${{ secrets.APPLE_NOTARIZATION_APPLE_ID }} | ||
| APPLE_NOTARIZATION_PASSWORD: ${{ secrets.APPLE_NOTARIZATION_PASSWORD }} | ||
| APPLE_NOTARIZATION_TEAM_ID: ${{ secrets.APPLE_NOTARIZATION_TEAM_ID }} | ||
| P12_BASE64: ${{ secrets.APPLE_SIGNING_CERTIFICATE_P12_BASE64 }} |
There was a problem hiding this comment.
For any same-repository pull request touching one of the filtered files, this job checks out the PR merge commit and executes its scripts/sign-macos.sh while the production P12, its password, and notarization credentials are in the environment. An unreviewed change to that script can therefore exfiltrate the Developer ID private key or use it to sign arbitrary code; GitHub confirms that pull_request workflows from branches in the same repository receive repository secrets and run code from the merge branch (GitHub security guidance). Limit this credentialed job to trusted post-merge pushes, or protect the secrets with an environment requiring approval.
Useful? React with 👍 / 👎.
| if ! xcrun --find notarytool &> /dev/null; then | ||
| echo "notarytool not found. Please install Xcode command line tools:" | ||
| echo " xcode-select --install" | ||
| exit 1 | ||
| fi |
There was a problem hiding this comment.
When a developer intentionally uses the supported signing-only path by omitting notarization credentials, the script still exits here if notarytool is unavailable, even though that tool is never needed in the later else branch. This affects older local Xcode installations: Apple documents that notarytool first shipped with Xcode 13 and otherwise has to be copied separately (TN3147). Move this check inside the credentials-present branch so signing-only runs can complete as advertised.
Useful? React with 👍 / 👎.
| BIN=target/${{ matrix.target }}/superoptimized/sqlpage | ||
| # Import the signing certificate into a temporary keychain | ||
| umask 077 | ||
| trap 'rm -f /tmp/signing.pem' EXIT |
There was a problem hiding this comment.
On every successful macOS release run, this trap removes only the plaintext PEM; /tmp/signing.p12 and the unlocked, empty-password /tmp/signing.keychain remain available while subsequent actions execute. Because the imported private key is still accessible through that keychain, a compromised later action could export or misuse the long-lived Developer ID identity even though signing has finished. Delete the keychain and P12 in the exit trap, as the separate signing workflow already does; Apple documents that security delete-keychain also removes it from the search list (security(1)).
Useful? React with 👍 / 👎.
1 participant
Summary
This PR adds support for signing and notarizing SQLPage macOS binaries, eliminating Gatekeeper warnings for users.
Changes
sqlpage.entitlements): Defines the macOS capabilities needed for SQLPage to run as a web server.github/workflows/release.yml): Updated to:scripts/sign-macos.sh): Helper script for testing signing locally on macOSdocs/macos-signing-setup.md): Step-by-step guide for setting up Apple Developer credentialsRequired GitHub Secrets
The following secrets have been added to the repository:
APPLE_SIGNING_CERTIFICATE_P12_BASE64- Base64-encoded P12 certificateAPPLE_SIGNING_CERTIFICATE_PASSWORD- Password for the P12 certificateAPPLE_SIGNING_IDENTITY- Developer ID Application signing identityAPPLE_NOTARIZATION_APPLE_ID- Apple ID email (ophir@sql-page.com)APPLE_NOTARIZATION_TEAM_ID- Team ID (8U83MM69B4)APPLE_NOTARIZATION_PASSWORD- App-Specific Password (needs to be added manually)Before merging, please add the final secret:
SQLPage NotarizationAPPLE_NOTARIZATION_PASSWORDTesting
Once merged, the next release (tagged with v*) will automatically:
Benefits