feat: sign nrq darwin binaries with a stable code-signing identity#129
Conversation
Add the darwin hooks.post to the nrq-darwin build (centralized codesign-darwin.sh via $CODESIGN_DARWIN_SCRIPT, fail-loud on a broken script), forward the four macos-cert-* secrets, and bump version.txt to 1.1 so the merge cuts a signed release. Closes #128
|
Codex PR review (9a): blockers=0 majors=0 minors=0 nits=0 — no findings. No findings. Verified against the actual repo files:
|
monit-reviewer
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: fe23326
Approved with 2 non-blocking suggestions below. Address at your discretion.
Summary
| Reviewer | Findings |
|---|---|
| harness-engineering:harness-architecture-reviewer | 1 |
| harness-engineering:harness-knowledge-reviewer | 1 |
harness-engineering:harness-architecture-reviewer (1 findings)
💡 Suggestion - .goreleaser.yml:51
The hook silently no-ops when CODESIGN_DARWIN_SCRIPT is unset, which is intentional for local builds. In CI, if the upstream macos-codesign-setup action fails to export the variable, GoReleaser succeeds and archives an unsigned darwin binary without any build-level failure. The only enforcement layer is the external darwin-gate check-signature step. If that gate is ever skipped or misconfigured, unsigned binaries can ship silently. A lightweight guard asserting CODESIGN_DARWIN_SCRIPT is set when CI=true and GORELEASER_CURRENT_TAG is set would close this gap at the build level.
harness-engineering:harness-knowledge-reviewer (1 findings)
💡 Suggestion - .goreleaser.yml:48
The error message 'CODESIGN_DARWIN_SCRIPT not executable' conflates two distinct failure modes: file-not-found and file-not-executable. A CI operator seeing this for a missing file will be misled. Consider splitting:
[ -e "$f" ] || { echo "CODESIGN_DARWIN_SCRIPT path does not exist: $f" >&2; exit 1; }; [ -x "$f" ] || { echo "CODESIGN_DARWIN_SCRIPT not executable: $f" >&2; exit 1; }.
1 PR discussion thread considered.
Completed in 2m 06s | $0.47 | sonnet | daemon 0.2.121 | Glorfindel
| Field | Value |
|---|---|
| Model | sonnet |
| Reviewers | hybrid-synthesis, harness-engineering:harness-architecture-reviewer, harness-engineering:harness-enforcement-reviewer, harness-engineering:harness-knowledge-reviewer |
| Engine | claude · sonnet |
| Reviewed by | pr-review-daemon · monit-pr-reviewer |
| Duration | 2m 06s wall · 4m 18s compute (Reviewers: 1m 50s · Synthesis: 13s) |
| Cost | $0.47 |
| Tokens | 130.1k in / 14.7k out |
| Turns | 8 |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost |
|---|---|---|---|---|---|---|
| hybrid-synthesis | sonnet | 30.7k | 715 | 18.6k | 12.0k (1h) | $0.07 |
| harness-engineering:harness-architecture-reviewer | sonnet | 30.0k | 6.2k | 16.2k | 13.8k (1h) | $0.15 |
| harness-engineering:harness-enforcement-reviewer | sonnet | 27.8k | 4.8k | 6.9k | 20.9k (1h) | $0.15 |
| harness-engineering:harness-knowledge-reviewer | sonnet | 41.6k | 3.1k | 30.9k | 10.8k (1h) | $0.10 |
Re-reviews only run when @monit-reviewer is re-requested as a reviewer — push as many commits as you need, then re-request when ready. PRs targeting branches other than main, master are skipped, even when @monit-reviewer is re-requested.
| hooks: | ||
| post: | ||
| - cmd: bash -c 'f="${CODESIGN_DARWIN_SCRIPT:-}"; if [ -z "$f" ]; then echo "skip codesign (CODESIGN_DARWIN_SCRIPT unset, local build)"; exit 0; fi; [ -x "$f" ] || { echo "CODESIGN_DARWIN_SCRIPT not executable ($f)" >&2; exit 1; }; exec "$f" "$0" "$1"' "{{ .Path }}" "{{ .Os }}" | ||
| - id: nrq-unix-win |
There was a problem hiding this comment.
🔵 Low (harness-engineering:harness-architecture-reviewer): The hook silently no-ops when CODESIGN_DARWIN_SCRIPT is unset, which is intentional for local builds. In CI, if the upstream macos-codesign-setup action fails to export the variable, GoReleaser succeeds and archives an unsigned darwin binary without any build-level failure. The only enforcement layer is the external darwin-gate check-signature step. If that gate is ever skipped or misconfigured, unsigned binaries can ship silently. A lightweight guard asserting CODESIGN_DARWIN_SCRIPT is set when CI=true and GORELEASER_CURRENT_TAG is set would close this gap at the build level.
Reply to this thread when addressed.
| # (cli-common distribution.md §2A). Logic + identity live in open-cli-collective/.github | ||
| # (macos-codesign-setup), which exports CODESIGN_DARWIN_SCRIPT (absolute). Unset in | ||
| # local builds → signing skipped. | ||
| hooks: |
There was a problem hiding this comment.
🔵 Low (harness-engineering:harness-knowledge-reviewer): The error message 'CODESIGN_DARWIN_SCRIPT not executable' conflates two distinct failure modes: file-not-found and file-not-executable. A CI operator seeing this for a missing file will be misled. Consider splitting: [ -e "$f" ] || { echo "CODESIGN_DARWIN_SCRIPT path does not exist: $f" >&2; exit 1; }; [ -x "$f" ] || { echo "CODESIGN_DARWIN_SCRIPT not executable: $f" >&2; exit 1; }.
Reply to this thread when addressed.
Gives
nrqa stable macOS code-signing identity so the Keychain "Always Allow" grant survivesbrew upgrade(convention:cli-commondistribution.md §2A; infra on@v1). Same pattern as the verified slck canary + gro..goreleaser.yml: darwinhooks.postonnrq-darwin→ centralizedcodesign-darwin.shvia absolute$CODESIGN_DARWIN_SCRIPT(fail-loud if set-but-broken; no-op when unset/local).release.yml: forward the fourmacos-cert-*secrets (explicit pass-through).version.txt→1.1: trips the dual-gate auto-release → signed release.darwin-gate check-signatureenforcescertificate leaf = H"42e1afd0…"/ nocdhashbefore publish. One-time final re-prompt expected.Closes #128