make: sign the installed and dev binaries with the stable identity - #21
Conversation
lofty-cli had no code-signing at all, unlike the rest of the family. That matters because it stores its API credential in the macOS keychain (pk-cli-secrets): `cargo install` ad-hoc signs, giving ~/.cargo/bin/lofty a new code identity each time, and macOS scopes keychain "Always Allow" grants to the identity — so every reinstall silently revoked the grant and the next run re-prompted. Adds the SIGN-macro pattern shared by rpm-fl-cli, wabhoa, fpl, and tojfl: `install` and a new `dev` target both re-sign with pk-cli-codesign, `install` honours CARGO_INSTALL_ROOT, and `dev` joins `.PHONY`. Verified: both `make install` and `make dev` produce a binary with Authority=pk-cli-codesign. macOS-only; a no-op with a note elsewhere. self-update already re-signs via pk-cli-selfupdate.
piekstra-dev
left a comment
piekstra-dev
left a comment
There was a problem hiding this comment.
Automated PR Review
Reviewed commit: ebf26f545643
Profile: reviewer - Posting as: piekstra-dev
Summary
| Reviewer | Findings |
|---|---|
| automation:ci-release | 1 |
| policies:conventions | 0 |
automation:ci-release (1 finding)
Nits - Makefile:58
The new
devtarget callscargo builddirectly instead of$(CARGO) build, unlike every other target in this file (build,release,test,lint,fmt,install, etc.), which all go through the$(CARGO)variable. IfCARGOis ever overridden (e.g. a wrapped/pinned toolchain invocation),devwould silently use the wrong cargo binary while the rest of the file honors the override. Fix: use$(CARGO) buildfor consistency.
Reviewer Coverage
| Reviewer | Status | Inspected | Skipped | Constraints |
|---|---|---|---|---|
| automation:ci-release | complete_broad | Makefile | unavailable | unavailable |
| policies:conventions | complete_broad | Makefile | unavailable | No cli-common/docs (or ../cli-common sibling checkout) was present in the review context, so the SIGN-macro pattern this PR adopts from rpm-fl-cli/wabhoa/fpl/tojfl could not be diffed against a source-of-truth doc or sibling repo — only internal consistency of the Makefile change was checked. |
0 PR discussion threads considered. 0 summarized; 0 resolved.
Completed in 2m 18s | $0.71 | claude-sonnet-5 | cr 0.10.268
| Field | Value |
|---|---|
| Model | claude-sonnet-5 |
| Reviewers | automation:ci-release, policies:conventions |
| Engine | claude_cli · claude-sonnet-5 |
| Reviewed by | cr · piekstra-dev |
| Duration | 2m 18s wall · 3m 11s compute |
| Cost | $0.71 |
| Tokens | 50 in / 8.9k out |
Per-workstream usage
| Workstream | Model | In | Out | Cache read | Cache create | Cost | Duration |
|---|---|---|---|---|---|---|---|
| orchestrator-selection | claude-sonnet-5 | 6 | 1.0k | 49.4k | 10.8k | $0.10 | 16s |
| automation:ci-release | claude-sonnet-5 | 16 | 3.4k | 203.6k | 16.4k | $0.21 | 1m 50s |
| policies:conventions | claude-sonnet-5 | 22 | 4.0k | 333.2k | 23.1k | $0.30 | 55s |
| orchestrator-rollup | claude-sonnet-5 | 6 | 373 | 57.9k | 13.4k | $0.10 | 9s |
| # dev loop doesn't re-prompt either (see cli-common/scripts). | ||
| dev: SIGN_TARGET = target/debug/$(BIN) | ||
| dev: | ||
| cargo build |
There was a problem hiding this comment.
The new dev target calls cargo build directly instead of $(CARGO) build, unlike every other target in this file (build, release, test, lint, fmt, install, etc.), which all go through the $(CARGO) variable. If CARGO is ever overridden (e.g. a wrapped/pinned toolchain invocation), dev would silently use the wrong cargo binary while the rest of the file honors the override. Fix: use $(CARGO) build for consistency.
Reply inline to this comment.
Pre-empting the review finding on the sibling fpl PR: passing $(SIGN_TARGET) unquoted word-splits when CARGO_INSTALL_ROOT contains a space. Quoted in the dev-sign call.
2 participants
lofty-cli had no code-signing at all, unlike the rest of the family — and it stores its API credential in the macOS keychain (
pk-cli-secrets), so it needs it.cargo installad-hoc signs, giving~/.cargo/bin/loftya new code identity each time; macOS scopes keychain "Always Allow" grants to the code identity, so every reinstall silently revoked the grant and the next run re-prompted.Adds the SIGN-macro pattern now shared by rpm-fl-cli, wabhoa, fpl, and tojfl:
installand a newdevtarget both re-sign withpk-cli-codesign,installhonoursCARGO_INSTALL_ROOT, anddevjoins.PHONY.Verified locally: both
make installandmake devproduce a binary withAuthority=pk-cli-codesign. macOS-only — a no-op with a printed note elsewhere.self-updatealready re-signs viapk-cli-selfupdate.