Skip to content

fix: preserve Git verification keys and support macOS Bash - #871

Merged
DevSecNinja merged 1 commit into
mainfrom
devsecninja-chezmoi-mac-update
Sep 28, 2026
Merged

DevSecNinja merged 1 commit into
mainfrom
devsecninja-chezmoi-mac-update

Conversation

@DevSecNinja

@DevSecNinja DevSecNinja commented Sep 28, 2026 •

Copy link
Copy Markdown
Owner

A macOS dotfiles update exposed Bash 3.2 startup errors and proposed replacing existing YubiKey verification entries with the 1Password key. Switching signing backends should not discard trust in previously signed commits.

Changes

  • Render all enrolled YubiKey public keys, including legacy filenames, alongside the configured 1Password key in allowed_signers, regardless of useYubiKey. Selection of the signer for new commits and tags is unchanged.
  • Replace Bash 4-only lowercasing with a Bash 3.2-compatible directory match.
  • Load third-party Homebrew completions only on Bash 4.4+, avoiding ykman's unsupported complete -o nosort option on macOS's built-in Bash. Homebrew environment setup and the dotfiles' own completion initializers remain enabled.
  • Add regression coverage and document these behaviors.

Compatibility notes

YubiKey .pub files must remain in ~/.ssh/ so chezmoi can regenerate their verification entries; the hardware does not need to be connected. This does not change installed files until the updated source is applied.

Validation

  • 24 focused regression tests passed, covering retained verification keys, signer precedence, and shell startup behavior.
  • All-file lefthook checks and commit hooks passed.
  • An earlier full Bats run was not green on macOS. Failures included GNU-tar-only options, hardcoded /usr/bin/chmod, and a missing timeout command, among other environment-sensitive failures. Those broader issues are outside this PR.

Summary by CodeRabbit

  • Bug Fixes

    • Public keys for both YubiKey and 1Password signing remain trusted for commit verification, even when switching signing methods. YubiKey verification does not require the key to be plugged in.
    • Homebrew completions now load only in Bash 4.4 or newer; dotfiles’ own completion initializers continue to run on older Bash versions.
    • Project-directory matching works with macOS’s built-in Bash 3.2.
  • Documentation

    • Clarified which signing keys are trusted and how Homebrew completions behave across Bash versions.

Retain enrolled YubiKey and configured 1Password verification keys across signing-mode changes. Keep new-commit signer selection unchanged.

Use Bash 3.2-compatible startup directory matching and gate third-party Homebrew completions on Bash 4.4+. Add regression coverage and document both behaviors.

Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
@coderabbitai

coderabbitai Bot commented Sep 28, 2026 •

Copy link
Copy Markdown

Review in Change Stack →

Navigate logical layers of code changes, visualize relationships, and explore their blast radius.

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Advanced

Run ID: b3244f32-b7e9-4025-8bfe-7cd7fba319b6

📥 Commits

Reviewing files that changed from the base of the PR and between d937687 and d3f30e6.

📒 Files selected for processing (9)
  • docs/git-signing.md
  • docs/yubikey.md
  • home/dot_config/git/allowed_signers.tmpl
  • home/dot_config/shell/completions.d/00-homebrew.bash
  • home/dot_config/shell/completions.d/README.md
  • home/dot_config/shell/config.bash
  • tests/README.md
  • tests/bash/git-allowed-signers.bats
  • tests/bash/test-shell-startup-logic.bats

Included review availability: This review used your included allowance. Your plan provides up to 10 included reviews per hour; 9 remain after this review.


📝 Walkthrough

Walkthrough

The change keeps enrolled YubiKey and configured 1Password public keys in allowed_signers across signing modes, with tests and documentation. It also updates Bash startup path matching and limits Homebrew completion loading to Bash 4.4 or newer, with corresponding tests and guidance.

Changes

Signing-Key Verification

Layer / File(s) Summary
Allowed-signers generation and coverage
docs/git-signing.md, docs/yubikey.md, home/dot_config/git/allowed_signers.tmpl, tests/bash/git-allowed-signers.bats, tests/README.md
The template emits enrolled and legacy YubiKey keys and the configured 1Password key independently of the active signing mode. It shows missing-key guidance only when YubiKey signing is selected. Documentation and tests describe and check signer generation and verification.

Bash Startup Compatibility

Layer / File(s) Summary
Project path matching
home/dot_config/shell/config.bash, tests/bash/test-shell-startup-logic.bats
The project-directory check uses a case-insensitive glob. Runtime tests cover mixed-case project paths, VS Code sessions, and whether ~/projects exists.
Homebrew completion version gate
home/dot_config/shell/completions.d/00-homebrew.bash, home/dot_config/shell/completions.d/README.md, tests/bash/test-shell-startup-logic.bats
Homebrew completion loading requires Bash 4.4 or newer. Tests check loading across Bash versions, and the documentation describes the behavior for older Bash versions.

Priority: ⬇️ Low

Estimated code review effort: 2 (Simple) | ~15 minutes

Change: Bug fix

Merge Risk: ⚪ Minimal · up to d3f30

No actionable issue remains in the supplied review evidence; the change is mergeable after normal checks.

Security Architecture Review

Security architecture risk: 🔵 Low · up to d3f30

Preserving older verification keys maintains access to signed history, but it also means switching signing methods no longer withdraws trust from those keys. The effect is limited to machines using this generated Git configuration; accepting a forged signature would still require control of a trusted signing key.

Retained concerns

  • Low · security · inferred: Switching signing backends no longer removes an inactive key from local verification trust. A retired or compromised key remains accepted while its public file or configured key remains in the generated list; the PR does not establish a tested revocation and reapplication path.
Security review details

Security Blast Radius

  • inferred — The broadened trust applies to Git verification on machines that apply this dotfiles configuration and retain the relevant public keys. The evidence does not show a change to remote repository hosting or other users' verification policies.

Security Findings and Attack Paths

  • inferred — If an attacker can sign with a retained key—for example, after compromise of that key—switching to the other signing backend will no longer prevent its signatures from verifying locally. No such compromise or successful forgery is evidenced.

Trust Boundaries and Controls

  • observed — Public keys and the configured email determine allowed-signers entries; selecting the signing backend still determines the key used for new signatures. The 1Password signing path delegates private-key use to its signer rather than placing that private key in the template.

Resilience and Maintainability Implications

  • inferred — Successful rendering reflects currently present public-key files, but the tests do not check removal followed by application, failed or interrupted application, or actual Git verification after a key is retired.

Hardening Proposals

  • proposed — Document an explicit lost-key retirement procedure and check it end to end: remove the key from its template inputs, apply the configuration, and confirm Git no longer accepts signatures from that key.
🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (5 skipped: 5 … Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely summarizes the two main changes: preserving Git verification keys and supporting macOS Bash compatibility.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Full details: Docstring Coverage

Explanation

Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 4 functions across 4 files. (5 skipped: 5 unsupported.)

  • Fix all pre-merge checks with AI
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Commit to this branch
  • Create a new PR
🧪 Generate unit tests (beta)
  • Commit to this branch
  • Create a new PR

Comment @coderabbitai help to get the list of available commands.

@DevSecNinja
DevSecNinja enabled auto-merge (squash) September 28, 2026 13:22
@DevSecNinja
DevSecNinja merged commit 532c84c into main Sep 28, 2026
20 checks passed
@DevSecNinja
DevSecNinja deleted the devsecninja-chezmoi-mac-update branch September 28, 2026 13:25
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.

1 participant