Refactor script/sign: add comprehensive env validation and improve error handling

[Copilot]

The script/sign script lacked upfront environment variable validation and proper error handling. It would silently skip signing if APPLE_DEVELOPER_ID was unset, making failures hard to diagnose.

Changes

• Environment validation: Added check_env_vars() that validates all required env vars (APPLE_DEVELOPER_ID, APPLE_ID, APPLE_ID_PASSWORD) upfront and reports all missing vars at once
• Fail-fast behavior: Changed from silent skipping to explicit failure when env vars are missing
• Variable safety: Properly quoted all variable expansions ("$input_file", "${APPLE_ID}")
• Code clarity: Made loop explicit (for input_file in "$@") and extracted file parameter in sign_macos() to local var

Before

sign_macos() {
  if [[ -z "$APPLE_DEVELOPER_ID" ]]; then
    echo "skipping macOS code-signing; APPLE_DEVELOPER_ID not set" >&2
    return 0  # Silent skip
  fi
  # ... uses $1 directly, other vars checked with ${VAR?}
}

After

check_env_vars() {
  local missing=()
  for var in "${required_env_vars[@]}"; do
    [[ -z "${!var}" ]] && missing+=("$var")
  done
  (( ${#missing[@]} )) && {
    echo "Error: Missing required environment variables: ${missing[*]}" >&2
    exit 1
  }
}

sign_macos() {
  local input_file="$1"
  # ... all env vars validated before execution reaches here
}

Breaking change: Script now fails instead of skipping when env vars are unset.

Original prompt

Problem Statement

Improve the script/sign script to follow Bash best practices and add better error handling.

File to modify: script/sign
Base commit: cc4164d

Required Changes

Refactor the script/sign script with the following improvements:

1. Environment Variable Validation

• Add a function check_env_vars() that validates ALL required environment variables before proceeding
• Required variables: APPLE_DEVELOPER_ID, APPLE_ID, APPLE_ID_PASSWORD
• Report all missing variables at once (not one-by-one)
• Exit with error code 1 if any are missing

2. Improved Error Handling

• Add set -e at the top of the script to exit on errors
• Add better error messages that write to stderr using >&2
• Exit with error code 1 when no arguments are provided
• Exit with error code 1 when not running on macOS (instead of silently skipping)

3. Code Safety & Best Practices

• Quote all variable expansions for safety (especially "$1", "$input_file", "$@")
• Make the argument loop more explicit: for input_file in "$@"; do
• Use [[ ]] for conditionals instead of [ ]

4. User Experience

• Add usage message when no arguments provided: echo "usage: script/sign <file>" >&2
• Improve platform check error message to be clearer

5. Code Structure

• Extract signing logic into a sign_macos() function that takes the input file as a parameter
• Keep the existing logic for checking file extension (.zip vs other files)
• Maintain the same codesign and notarytool commands

Implementation Reference

#!/bin/bash
# usage: script/sign <file>
#
# Signs macOS binaries using codesign, notarizes macOS zip archives using notarytool

set -e

required_env_vars=("APPLE_DEVELOPER_ID" "APPLE_ID" "APPLE_ID_PASSWORD")

check_env_vars() {
  local missing=()
  for var in "${required_env_vars[@]}"; do
    if [[ -z "${!var}" ]]; then
      missing+=("$var")
    fi
  done
  if (( ${#missing[@]} )); then
    echo "Error: Missing required environment variables: ${missing[*]}" >&2
    exit 1
  fi
}

sign_macos() {
  local input_file="$1"
  if [[ $input_file == *.zip ]]; then
    xcrun notarytool submit "$input_file" \
      --apple-id "${APPLE_ID}" \
      --team-id "${APPLE_DEVELOPER_ID}" \
      --password "${APPLE_ID_PASSWORD}"
  else
    codesign --timestamp --options=runtime \
      -s "${APPLE_DEVELOPER_ID}" -v "$input_file"
  fi
}

if [[ $# -eq 0 ]]; then
  echo "usage: script/sign <file>" >&2
  exit 1
fi

platform="$(uname -s)"
if [[ $platform != "Darwin" ]]; then
  echo "error: must run on macOS; skipping codesigning/notarization" >&2
  exit 1
fi

check_env_vars

for input_file in "$@"; do
  sign_macos "$input_file"
done

Expected Outcome

The script should:

• Be more robust with comprehensive environment variable checking
• Follow Bash best practices with proper quoting and error handling
• Provide clearer error messages to users
• Maintain backward compatibility with existing functionality
• Have better code organization with helper functions

This pull request was created from Copilot chat.

---

💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.

---

Continue Tasks: ❌ 1 failed — View all

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 394fed153c

ℹ️ 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".

[chatgpt-codex-connector]

Validate notarization vars only for zip inputs

Running check_env_vars unconditionally here makes APPLE_ID and APPLE_ID_PASSWORD mandatory even when every input is a non-zip file that only reaches the codesign branch. That is a functional regression from the prior behavior, where non-zip signing only depended on APPLE_DEVELOPER_ID, and it breaks binary-only signing workflows (for example local signing setups without notarization credentials).

Useful? React with 👍 / 👎.

[Copilot]

Pull request overview

This PR refactors the script/sign shell script to add comprehensive upfront environment variable validation and improve error handling. The script is used in the GitHub Actions deployment workflow to sign macOS binaries and notarize zip archives for production releases.

Changes:

• Added upfront validation for all required environment variables (APPLE_DEVELOPER_ID, APPLE_ID, APPLE_ID_PASSWORD) with clear error reporting
• Changed from silent skip to fail-fast behavior when environment variables are missing
• Enhanced error handling with set -e, proper error messages to stderr, and explicit argument validation
• Improved code safety with proper variable quoting and explicit loop syntax

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.