Skip to content

./script/sign cleanup #10599

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
andyfeller merged 2 commits into from
Mar 19, 2025
Merged

./script/sign cleanup #10599

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
f3e4976
`./script/sign` cleanup
iamazeem Mar 14, 2025
342e3cd
More cleanup
iamazeem Mar 14, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions .goreleaser.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,7 @@ builds:
goarch: [386, amd64, arm64]
hooks:
post:
- cmd: >-
{{ if eq .Runtime.Goos "windows" }}pwsh .\script\sign.ps1{{ else }}./script/sign{{ end }} '{{ .Path }}'
- cmd: pwsh .\script\sign.ps1 '{{ .Path }}'
output: true
binary: bin/gh
main: ./cmd/gh
Expand Down
3 changes: 0 additions & 3 deletions docs/release-process-deep-dive.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -428,9 +428,6 @@ Breaking this command down:
* `/dlib` points to the previously extracted DLL
* `/dmdf` points to the previously created metadata file

> [!WARNING]
> The [`GoReleaser` signing hook](https://github.com/cli/cli/blob/756f4ec04abdc9fdbab3fef35b182c546ef1dd17/.goreleaser.yml#L43) can currently call `./script/sign` on a non-windows machine, but this is an artifact from pre-HSM that should be removed.

## <a id="release">[release](https://github.com/cli/cli/blob/756f4ec04abdc9fdbab3fef35b182c546ef1dd17/.github/workflows/deployment.yml#L250-L395)</a>

<details>
Expand Down
49 changes: 9 additions & 40 deletions script/sign
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,36 +1,12 @@
#!/bin/bash
# usage: script/sign <file>
#
# Signs macOS binaries using codesign, notarizes macOS zip archives using notarytool, and signs
# Windows EXE and MSI files using osslsigncode.
# Signs macOS binaries using codesign, notarizes macOS zip archives using notarytool
#
set -e

sign_windows() {
if [ -z "$CERT_FILE" ]; then
echo "skipping Windows code-signing; CERT_FILE not set" >&2
return 0
fi

if [ ! -f "$CERT_FILE" ]; then
echo "error Windows code-signing; file '$CERT_FILE' not found" >&2
return 1
fi

if [ -z "$CERT_PASSWORD" ]; then
echo "error Windows code-signing; no value for CERT_PASSWORD" >&2
return 1
fi

osslsigncode sign -n "GitHub CLI" -t http://timestamp.digicert.com \
-pkcs12 "$CERT_FILE" -readpass <(printf "%s" "$CERT_PASSWORD") -h sha256 \
-in "$1" -out "$1"~

mv "$1"~ "$1"
}

sign_macos() {
if [ -z "$APPLE_DEVELOPER_ID" ]; then
if [[ -z "$APPLE_DEVELOPER_ID" ]]; then

@andyfeller andyfeller Mar 19, 2025

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.

As far as I know, this should work without problems, but the only way to test this is during an actual release.

This is a little outside the scope of the issue's work but verified separately that these shouldn't be impactful.

echo "skipping macOS code-signing; APPLE_DEVELOPER_ID not set" >&2
return 0
fi
Expand All @@ -42,24 +18,17 @@ sign_macos() {
fi
}

if [ $# -eq 0 ]; then
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

for input_file; do
case "$input_file" in
*.exe | *.msi )
sign_windows "$input_file"
;;
* )
if [ "$platform" = "Darwin" ]; then
sign_macos "$input_file"
else
printf "warning: don't know how to sign %s on %s\n" "$1", "$platform" >&2
fi
;;
esac
done
sign_macos "$input_file"
done