feat(git): automate SSH signing key setup - #23
Conversation
- Add user info, signing key path, and allowedSignersFile to gitconfig - Create script/setup-git-signing to generate key, write allowed_signers, and upload to GitHub via gh ssh-key add --type signing - Add bootstrap_git_signing step to script/bootstrap - Check for existing key on GitHub to avoid duplicate uploads
There was a problem hiding this comment.
🟡 Changes recommended
The new setup script has a few concrete robustness issues (missing precondition checks and destructive allowed_signers writing) that can cause failures or unexpected local config loss.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds first-class support for Git commit signing via SSH in this dotfiles repo by committing the relevant Git configuration and providing a bootstrap-friendly setup script that generates an SSH signing key, configures local verification, and uploads the key to GitHub.
Changes:
- Add SSH signing-related
gitconfigentries (user.signingkey,gpg.format=ssh, andgpg.ssh.allowedSignersFile). - Introduce
script/setup-git-signingto generate an SSH signing key, write anallowed_signersfile, and upload the signing key to GitHub viagh. - Run the signing setup once during
script/bootstrapvia a newbootstrap_git_signingstep (gated by a config flag).
File summaries
| File | Description |
|---|---|
script/setup-git-signing |
Implements the end-to-end SSH signing key setup + GitHub upload workflow. |
script/bootstrap |
Adds a bootstrap step to run the signing setup once on new machines. |
gitconfig |
Commits the Git SSH signing configuration needed for signing and verification. |
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- Convert script/setup-git-signing to script/lib/git-signing.sh - Remove bootstrap_git_signing from script/bootstrap - Source library in script/setup and call after setup_rc() - Assume gh is installed by Brewfile (bootstrap runs first) - Create ~/.ssh directory if missing - Error if git user.email is unset - Append to allowed_signers only if entry missing (idempotent)
There was a problem hiding this comment.
🟡 Changes recommended
The new setup script has confirmed failure modes (missing gh, missing .pub file, unsafe regex matching for key lookup) that can break script/setup execution and should be hardened before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
script/lib/git-signing.sh:23
ensure_gh_authenticatedcallsghunconditionally; ifghis not installed (e.g., Brewfile install skipped/failed),script/setupwill exit due toset -e. Add an explicitcommand -v ghcheck and a clearer error before attempting auth.
ensure_gh_authenticated () {
# gh is installed by Brewfile; bootstrap_homebrew runs first in setup()
if ! gh auth status &>/dev/null 2>&1; then
msg_warn "==> GitHub CLI not authenticated"
gh auth login
fi
}
script/lib/git-signing.sh:63
- The key lookup uses
grep -q(regex mode) against base64 key material; this can produce false matches if any regex metacharacters appear in the key. Use fixed-string matching (grep -F) and--for safety.
existing_keys=$(gh api /user/ssh_signing_keys --jq '.[].key' 2>/dev/null || echo "")
if echo "$existing_keys" | grep -q "$pubkey_data"; then
msg_info "==> Signing key already on GitHub"
return
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
- Regenerate .pub from private key if missing in ensure_signing_key - Remove user.username from gitconfig (non-standard; use [github].user)
There was a problem hiding this comment.
🟡 Changes recommended
The committed config points user.signingkey at a .pub file and the setup script can hang when a .pub exists without its private key, both of which can break the intended “automated setup” flow.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
- Change user.signingkey from .pub to private key path (SSH signing requires private key) - Add check for orphaned .pub without private key (abort with clear error)
There was a problem hiding this comment.
🟡 Changes recommended
There are a few concrete correctness/security issues in the new signing setup script (key existence matching and default unencrypted key generation) that should be addressed before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
gitconfig:86
- The comment on the
[include]section later in this file says user identity is kept in a private config, but[user]is now set in this committed gitconfig. Update that comment to reflect that the include is for machine-specific overrides instead.
script/setup:23 - PR description says a git-signing step was added to
script/bootstrap, but the implementation here adds the step toscript/setupinstead (andscript/bootstrapcurrently has no git-signing logic). Either update the description or add the corresponding bootstrap step if that was the intent.
script/lib/git-signing.sh:21
ensure_gh_authenticatedassumesghexists; if it’s missing, thegh auth logincall will fail with a generic “command not found” and abort setup. Add an explicitcommand -v ghcheck with a clear error message before attempting auth.
ensure_gh_authenticated () {
# gh is installed by Brewfile; bootstrap_homebrew runs first in setup()
if ! gh auth status &>/dev/null 2>&1; then
msg_warn "==> GitHub CLI not authenticated"
gh auth login
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
- Remove -N "" to allow ssh-keygen to prompt for passphrase - Use jq to extract key material and grep -qxF for exact matching
There was a problem hiding this comment.
🔵 Needs a closer look
There are security/robustness issues in the new signing setup script (namespace restriction and clearer handling when gh is missing) and a mismatch between the PR description and what the code actually adds.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
script/lib/git-signing.sh:57
allowed_signersentries are written without a namespace restriction, which means the key is trusted for any SSH signature namespace. For Git commit/tag verification, it’s safer to restrict this entry to thegitnamespace.
script/setup:23- The PR description mentions a standalone
script/setup-git-signingscript and abootstrap_git_signingstep inscript/bootstrap, but the implementation in this PR appears to wire signing setup intoscript/setupviascript/lib/git-signing.shinstead. This mismatch can confuse reviewers/users; either update the PR description or add the described entrypoints.
script/lib/git-signing.sh:22
- If
ghis missing (e.g., Brew bundle didn’t run or failed), this will currently abort with a generic "command not found" due toset -e. Add an explicit presence check with a clearer error message before callinggh.
if ! gh auth status &>/dev/null 2>&1; then
msg_warn "==> GitHub CLI not authenticated"
gh auth login
fi
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Three moderate issues remain in SSH key handling and GitHub duplicate detection.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
script/lib/git-signing.sh:52
- Checking only for the existence of both files allows a stale or mismatched
.pubfile to pass through. The allowed-signers file and GitHub will then contain a different public key from the private key Git uses to sign, causing verification to fail; derive and compare the public key whenever the private key is present, or replace a mismatch.
This issue also appears on line 65 of the same file.
script/lib/git-signing.sh:65
- This check is substring-based, so a malformed or extended line containing
entrycan make the function skip adding the exact allowed-signer entry. Use whole-line matching here, as in the GitHub check below, so local verification cannot silently remain configured with the wrong line.
if [ -f "$ALLOWED_SIGNERS" ] && grep -qF "$entry" "$ALLOWED_SIGNERS"; then
script/lib/git-signing.sh:48
- Redirecting directly to
$KEY_PATH.pubcreates or truncates that file beforessh-keygenhas successfully derived the key. A failed passphrase/derivation therefore leaves an empty or partial.pub; on the next run both files exist, so line 52 skips regeneration and the invalid key is used for local verification and upload. Write to a temporary file and rename it only after success.
ssh-keygen -y -f "$KEY_PATH" > "$KEY_PATH.pub"
script/lib/git-signing.sh:78
- The GitHub list endpoint is paginated, but this request does not use
--paginate. If the matching signing key is beyond the first page, the duplicate check misses it andgh ssh-key addattempts to upload an existing key, making setup fail unnecessarily.
existing_key_data=$(gh api /user/ssh_signing_keys --jq '.[].key | split(" ")[1]' 2>/dev/null || true)
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
- Validate .pub matches private key; regenerate via temp file if mismatched - Use grep -qxF for exact line matching in allowed_signers - Add --paginate to GitHub API call for signing keys
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved critical and moderate issues affect signing configuration and setup reliability.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (3)
Previously missed (2) — in code that hasn't changed since the last review.
script/lib/git-signing.sh:27
gh auth statusonly proves that a credential exists; it does not ensure the token has theadmin:public_keyscope required bygh ssh-key add --type signing. A freshgh auth loginnormally grants the default scopes, so this path can create the local key and allowed-signers entry and then fail at the upload step, leaving setup incomplete. Request or refresh that scope (while handling tokens such asGH_TOKENthat cannot be refreshed) before attempting the upload.
script/lib/git-signing.sh:32- If
user.emailis absent,git config user.emailexits with status 1. Under the caller'sset -e, this assignment terminates the script before the empty-value check and its explanatory error messages run, so the validation path is unreachable for the missing-config case. Make the lookup non-fatal before testing$email.
script/lib/git-signing.sh:48
- For an existing encrypted private key,
ssh-keygen -yneeds to prompt for its passphrase, but redirecting stderr to/dev/nullhides the prompt (OpenSSH writes it to stderr). The setup can therefore appear to hang while waiting for input, undermining the passphrase support added here; leave this diagnostic/prompt stream visible or handle it separately.
derived_pubkey=$(ssh-keygen -y -f "$KEY_PATH" 2>/dev/null | awk '{print $2}')
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
There was a problem hiding this comment.
🔵 Needs a closer look
Unresolved findings affect key-path handling, GitHub authorization, and safe allowed_signers updates.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
script/lib/git-signing.sh:80
- When an existing
allowed_signersfile does not end with a newline, this append joins the new principal/key to the previous line and makes both entries invalid. Ensure the file is newline-terminated before appending, then useprintf '%s\n'for the new entry.
gitconfig:56
- Git does not treat
user.signingkeyas a path-valued config entry, so this literal~is passed to the SSH signer rather than expanded by the shell. Commits will fail to load the key (for example, with a~/.ssh/...file-not-found error). Use an absolute path in the committed config, or have setup write an expanded$KEY_PATHinto a local override.
signingkey = ~/.ssh/id_ed25519_signing
script/lib/git-signing.sh:26
gh auth statusonly proves that a token exists; it does not prove that the token has theadmin:public_keyscope required by/user/ssh_signing_keysandgh ssh-key add --type signing. A defaultgh auth loginon a fresh machine can therefore create the local key and then fail at upload. Request or validate that scope (for example withgh auth refresh -h github.com -s admin:public_key) before continuing.
if ! gh auth status &>/dev/null 2>&1; then
msg_warn "==> GitHub CLI not authenticated"
gh auth login
script/lib/git-signing.sh:6
- Because
rcrclistsconfiginSYMLINK_DIRS(rcrc:2), this resolves~/.configinto the dotfiles config tree during setup.mkdir -pand the append below therefore create an untrackedconfig/git/allowed_signersfile (there is no matching ignore rule), leaving the checkout dirty and making machine-local generated state easy to commit accidentally. Store this file outside the symlinked tree or add a dedicated ignore rule and keep the Git config path consistent.
ALLOWED_SIGNERS="$HOME/.config/git/allowed_signers"
- Files reviewed: 3/3 changed files
- Comments generated: 0 new
- Review effort level: Lite
- Move user.signingkey and gpg.ssh.allowedSignersFile to ~/.local/gitconfig - Ensure local directory exists in setup (symlink iCloud or create local dir) - Move allowed_signers to ~/.local/share/git/allowed_signers - Add admin:public_key scope refresh before GitHub upload - Ensure newline before appending to allowed_signers - Validate public key matches private key and regenerate if mismatched
There was a problem hiding this comment.
🟡 Changes recommended
Unresolved setup fallback and error-handling issues can cause setup failures or expose machine-specific data.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
gitconfig:58
- The new
[github]block leaves the include comment below inaccurate: it says the user name is kept in a private file, but the user identity is now committed in this file. Update that comment to describe only machine-specific and third-party settings so it does not document the old privacy model.
script/lib/git-signing.sh:45
- Because
script/setupenablesset -e,git config user.emailreturning 1 makes this assignment fail and exits the script before the[ -z "$email" ]check below. A missing email therefore produces no actionablegit user.email is not setmessage; make the lookup tolerate the missing-key status (for example, append|| true) before performing the explicit check.
email=$(git config user.email)
script/lib/git-signing.sh:90
- This early return skips the newline check when the existing entry is the final line without a terminating newline. That leaves
allowed_signersnon-newline-terminated despite this function's newline guarantee; append a newline before returning when the matching entry is present but unterminated.
if [ -f "$ALLOWED_SIGNERS" ] && grep -qxF "$entry" "$ALLOWED_SIGNERS"; then
msg_info "==> allowed_signers already contains this key"
return
script/setup:43
- The fallback directory is created inside the repository, but the repository's ignore rule is for
.local(gitignore:60), notlocal. The signing setup then writeslocal/gitconfigandlocal/share/git/allowed_signersthrough the~/.locallink, leaving machine-specific data visible togit statusand easy to stage accidentally. Addlocal/to the repository ignore rules before using this fallback.
else
mkdir -p "${DOT_DIR}/local"
msg_info "==> Created local directory for machine-specific files"
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
…ndling - Update gitconfig include comment to reflect current model - Make git config user.email lookups non-fatal under set -e - Ensure allowed_signers is newline-terminated before early return - Keep local gitignore pattern matching both symlinks and directories
There was a problem hiding this comment.
🔵 Needs a closer look
Address the three moderate setup reliability issues before approval.
Review details
Suppressed comments (3)
script/lib/git-signing.sh:107
|| trueconverts any failure while listing signing keys (such as a network or rate-limit error) into an empty result. If the key is already registered, the following add then fails as a duplicate, so setup is not reliably idempotent; propagate the listing error and stop before uploading.
existing_key_data=$(gh api /user/ssh_signing_keys --paginate --jq '.[].key | split(" ")[1]' 2>/dev/null || true)
script/setup:42
- When
secure_diris empty or points to a missing directory, this new fallback createsDOT_DIR/local, butsetup()still unconditionally callssetup_hosts(), which runshostile load "$(config_get secure_dir)/hosts"and therefore fails on/hostsor a nonexistent path. The fallback cannot complete setup in exactly the no-iCloud case it is intended to support; either skip host loading when the secure hosts file is unavailable or make this branch establish a valid hosts source.
if [ -n "$secure_dir" ] && [ -d "$secure_dir" ]; then
ln -s "$secure_dir" "${DOT_DIR}/local"
else
mkdir -p "${DOT_DIR}/local"
script/setup:34
- A dangling
localsymlink is not considered a directory by-d(for example, when iCloud is temporarily unavailable), butmkdir -pcannot replace that existing symlink. Withset -e, rerunning setup exits instead of creating the advertised local fallback; handle or remove dangling links before themkdir.
if [ -d "${DOT_DIR}/local" ]; then return; fi
- Files reviewed: 3/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
- Propagate gh API errors when listing signing keys instead of masking with || true - Handle dangling local symlinks in setup_secure_files - Skip host setup when secure hosts file is unavailable
There was a problem hiding this comment.
🔵 Needs a closer look
Three moderate issues remain in setup robustness, passphrase feedback, and GitHub permission validation.
Review details
Suppressed comments (3)
Previously missed (1) — in code that hasn't changed since the last review.
script/lib/git-signing.sh:61
- When an existing private key is encrypted,
ssh-keygen -yneeds to show its passphrase prompt and any failure. Redirecting stderr to/dev/nullhides that feedback, so rerunning setup (for example after a later upload failure) can appear to hang or exit without explaining why. Keep stderr visible for this derivation.
script/lib/git-signing.sh:34
- This GET request only establishes read access to the signing-key endpoint; a token with
read:public_keycan pass it while still lacking theadmin:public_keypermission required bygh ssh-key add. In that case the refresh is skipped and the setup fails at upload. Check the active token scopes (or otherwise verify the create permission) and refresh whenadmin:public_keyis absent.
if ! gh api /user/ssh_signing_keys --jq '.[0].id' &>/dev/null 2>&1; then
msg_warn "==> Refreshing GitHub CLI auth for admin:public_key scope"
gh auth refresh -h github.com -s admin:public_key
fi
script/setup:141
setup_secure_filesnow explicitly supports a missingsecure_dirby supplying a default, but this lookup still callsconfig_getwithout one. Withset -u, a fresh or manually configured setup that skipsbootstrap_secure_diraborts on an unset positional parameter instead of skipping host setup; read the optional value with a default and guard the empty path.
hosts_file="$(config_get secure_dir)/hosts"
- Files reviewed: 3/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
- Remove stderr suppression from ssh-keygen to show passphrase prompt - Check actual token scopes via API headers instead of endpoint access - Provide empty default for config_get secure_dir in setup_hosts
There was a problem hiding this comment.
🟡 Changes recommended
Three moderate issues remain unresolved in setup and authentication handling.
Get a fresh assessment by requesting another Copilot review.
Review details
Suppressed comments (2)
script/setup:38
- If the configured iCloud directory is temporarily unavailable, this deletes the symlink and creates a fallback directory; the
-dcheck then returns on every later run, even after iCloud is available again. That permanently prevents the configured secure directory from being relinked and leaves signing files local instead of synced. Reconcile or explicitly migrate the fallback directory before returning, rather than treating it as final state.
if [ -L "${DOT_DIR}/local" ] && [ ! -e "${DOT_DIR}/local" ]; then
msg_warn "==> Removing dangling local symlink"
rm "${DOT_DIR}/local"
elif [ -d "${DOT_DIR}/local" ]; then
return
script/setup:144
- When
secure_diris unset, this expands to/hosts; if that file exists, setup will runsudo hostile load /hostsinstead of skipping. Guard the empty configuration before constructing the path so the fallback local-directory setup cannot accidentally load an unrelated root-level file.
hosts_file="$(config_get secure_dir "")/hosts"
if [ ! -f "$hosts_file" ]; then
msg_warn "==> Hosts file not found at $hosts_file; skipping"
- Files reviewed: 3/4 changed files
- Comments generated: 1
- Review effort level: Lite
Add || true to scope extraction pipeline so missing x-oauth-scopes header doesn't terminate script before refresh logic runs.
There was a problem hiding this comment.
🔵 Needs a closer look
Moderate unresolved issues remain in account validation, setup failure handling, fallback switching, and empty-path guarding.
Review details
Suppressed comments (4)
script/lib/git-signing.sh:29
gh auth statusonly proves that some GitHub account is authenticated. Since the committed config now declaresgithub.user = jackboberg, a machine logged into a different account will pass this check and upload the signing key to the wrong profile, so GitHub will not verify commits for this configuration. Querygh api user --jq .loginand compare it withgit config github.user(or stop and require re-authentication) before uploading.
if ! gh auth status &>/dev/null 2>&1; then
msg_warn "==> GitHub CLI not authenticated"
gh auth login
fi
script/setup:39
- Once the fallback
localdirectory has been created, every later setup run returns here before recheckingsecure_dir. If iCloud is unavailable on the first run but becomes available later, this never switches the fallback to the configured iCloud symlink, so machine-specific files (including the signing config) remain local and are not synced. Recheck the configured directory when the existinglocalis the fallback directory, while preserving any non-empty local data.
elif [ -d "${DOT_DIR}/local" ]; then
return
fi
script/setup:23
- This runs after
setup_rchas installedgitconfig, which enablescommit.gpgsignwith SSH format. If authentication, key generation, or upload is cancelled/fails, setup exits whileuser.signingkeyandgpg.ssh.allowedSignersFilehave not been written, leaving Git commits unable to sign until the user manually repairs the config. Install/activate the signing configuration only after this step succeeds, or roll back/disable signing on failure.
setup_git_signing_step
script/setup:143
- When
secure_diris unset, this expansion produces/hostsrather than a path under a configured directory. If that file happens to exist, the new existence check will pass andsudo hostile loadwill load an unrelated system-level file; guard the emptysecure_dircase before constructinghosts_fileand skip setup.
hosts_file="$(config_get secure_dir "")/hosts"
if [ ! -f "$hosts_file" ]; then
- Files reviewed: 3/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
Changes
[user],[github], and[gpg]sections to committedgitconfig(non-machine-specific signing config)script/lib/git-signing.shwith reusable functions for SSH signing setup:ensure_gh_authenticated: checksghis installed, authenticated, and hasadmin:public_keyscopeensure_signing_key: generates key if missing, regenerates.pubif needed or mismatchedensure_local_gitconfig: writes machine-specific signing paths to~/.local/gitconfigwrite_allowed_signers: appends to~/.local/share/git/allowed_signersonly if entry is missing, ensures newlineupload_to_github: uploads key viagh ssh-key add --type signing, with paginated duplicate checkscript/setup(aftersetup_rc) viasetup_git_signing_stepsetup_secure_filesto ensurelocalalways exists: symlink from iCloud if available, otherwise create a local directoryuser.username(non-standard config key)user.signingkeyandgpg.ssh.allowedSignersFilefrom committed config; these are machine-specific and written to~/.local/gitconfig-N "")jq+grep -qxF) to avoid false positivesContext
Previously, signing config lived in the private
~/.local/gitconfig(not tracked). This moves the public parts (name, email, signing behavior) to the committed config while keeping machine-specific paths (signing key, allowed signers file) in~/.local/gitconfig, which may be synced via iCloud whenlocal/is symlinked from iCloud.The setup script handles the full workflow for new machines: generate key, configure local verification, write machine-specific git config, and upload to GitHub.