Skip to content

ci: scaffold macOS/Windows code signing for gddy releases - #243

Merged
jpage-godaddy merged 2 commits into
mainfrom
code-signing
Sep 3, 2026
Merged

ci: scaffold macOS/Windows code signing for gddy releases#243
jpage-godaddy merged 2 commits into
mainfrom
code-signing

Conversation

@jpage-godaddy

@jpage-godaddy jpage-godaddy commented Sep 2, 2026

Copy link
Copy Markdown
Collaborator

Summary

  • Adds Developer ID codesign + notarization steps for macOS, and DigiCert KeyLocker Authenticode signing for Windows, to the release build.
  • macOS notarization and all of Windows signing are currently commented out pending secrets — only macOS codesign is live for now.
  • Signing alone fixes upgrades causing re-prompting for Keychain access.

Follow-ups tracked separately

  • DEVEX-1080 — wire up macOS notarization secrets, re-enable the commented-out notarize step
  • DEVEX-1081 — wire up Windows DigiCert KeyLocker secrets, re-enable the commented-out signing step

Test plan

  • cargo fmt --check, cargo check, cargo clippy -- -D warnings, cargo test all pass (no Rust source touched)
  • Workflow YAML validated
  • Real/dispatched release run confirms codesign --verify succeeds once SIGNING_CERTIFICATE_P12/_PASSWORD are set
  • Manual verification on a Mac that the Keychain "Always Allow" grant persists across two signed releases

Refs DEVEX-896

Adds Developer ID codesign/notarize steps (macOS) and DigiCert
KeyLocker Authenticode signing (Windows) to the release workflow, so
gddy stops re-prompting for Keychain access on every version bump.
Both signing paths are currently disabled pending secrets/vars on the
repo — macOS notarization is commented out (DEVEX-1080) and Windows
signing is commented out entirely (DEVEX-1081); only macOS codesign is
live once SIGNING_CERTIFICATE_P12/_PASSWORD are set.

Refs DEVEX-896
Copilot AI lite review requested due to automatic review settings September 2, 2026 22:06

Copilot AI left a comment

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.

🟡 Changes recommended

The macOS signing setup uses base64 -d, which is likely to fail on macos-latest and break signing once secrets are enabled.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR scaffolds release-time code signing for gddy across macOS and Windows by extending the GitHub Actions release workflow and adding a supporting Windows KeyLocker signing script, with macOS codesign enabled once secrets are configured and notarization/Windows signing intentionally left commented out pending follow-up work.

Changes:

  • Add macOS keychain setup + codesign verification steps to the release build job (notarization block present but commented out).
  • Add a PowerShell script to Authenticode-sign Windows artifacts using DigiCert KeyLocker (currently not invoked because the workflow block is commented out).
File summaries
File Description
.github/workflows/release.yml Adds macOS signing scaffolding (and a commented notarization block) into the release build pipeline.
.github/workflows/Sign-WithKeyLocker.ps1 Introduces a Windows KeyLocker signing helper script intended for later enablement in the release workflow.
Review details

Suppressed comments (2)

.github/workflows/release.yml:180

  • The (currently commented) notarization step also uses base64 -d, which is likely to fail on macOS when this block is re-enabled.
      #     KEY_PATH="$RUNNER_TEMP/notary_key.p8"
      #     printf '%s' "$API_KEY_BASE64" | base64 -d > "$KEY_PATH"
      #     chmod 600 "$KEY_PATH"

.github/workflows/Sign-WithKeyLocker.ps1:76

  • smctl windows certsync is a native command; if it fails, the script will currently continue and only fail later during signing/verification. Check $LASTEXITCODE and fail fast with a clear error.
smctl windows ksp register
smctl windows certsync --keypair-alias="$KeypairAlias"
  • Files reviewed: 2/2 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread .github/workflows/Sign-WithKeyLocker.ps1 Outdated
Comment thread .github/workflows/release.yml
Comment thread .github/workflows/Sign-WithKeyLocker.ps1 Outdated
Comment thread .github/workflows/Sign-WithKeyLocker.ps1 Outdated
…s, credential cleanup

- base64 -d is a GNU flag; macOS ships BSD base64, which requires -D.
  Both macOS-only decode sites (active keychain-import step and the
  still-disabled notarize block) now use -D.
- Sign-WithKeyLocker.ps1: check exit codes for curl.exe, msiexec, and
  smctl windows certsync so a failed download/install/certsync fails
  fast instead of surfacing as a confusing downstream error. ksp
  register gets a warning instead of a hard failure, since it can
  exit non-zero on the documented benign "already registered" case.
- Clean up the materialized KeyLocker client .p12 and the healthcheck
  log (which can contain partially-masked credentials) via a
  try/finally instead of leaving them on disk.

Copilot AI left a comment

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.

🔵 Needs a closer look

The new Windows signing script has verified argument/validation issues (empty-secret env vars and unquoted file paths) that can cause hard-to-diagnose signing failures.

Review details

Suppressed comments (2)

Previously missed (2) — in code that hasn't changed since the last review.

.github/workflows/Sign-WithKeyLocker.ps1:43

  • The required-env-var preflight only checks that the env var exists (via Test-Path env:...) but not that it has a non-empty value. In GitHub Actions, missing secrets often become empty-string env vars, so this can skip the intended early failure and lead to confusing downstream errors (e.g., curl against an empty SM_HOST).
    .github/workflows/Sign-WithKeyLocker.ps1:99
  • $full is passed to smctl sign and signtool verify without quoting. If a caller provides a path containing spaces, PowerShell will split it into multiple arguments and signing/verification will fail.
  • Files reviewed: 2/2 changed files
  • Comments generated: 0 new
  • Review effort level: Lite

@jpage-godaddy
jpage-godaddy merged commit fe4a94b into main Sep 3, 2026
6 checks passed
@jpage-godaddy
jpage-godaddy deleted the code-signing branch September 3, 2026 00:05
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.

3 participants