Repository navigation
fix: verify helper code signature before bootstrapping dev daemon - #279
ethanndickson wants to merge 2 commits into
Conversation
The dev helper upsert script blindly bootstrapped whatever binary sat in the installed bundle. A stale or ad-hoc-signed helper (left behind when Xcode skips the CodeSignOnCopy embed phase, or when a build with code signing disabled writes into the installed bundle) would be installed silently, and only surface later as cryptic errSecCSReqFailed (-67050) XPC connection failures. Now the script verifies the helper against the app's XPC peer code signing requirement before installing it, failing the build loudly with remediation steps on mismatch.
Codex Review Summary: ✅ CompletedThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: e6c1081948
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| # errors. A stale or ad-hoc-signed helper can end up in the installed bundle | ||
| # when Xcode skips the "Embed Dependencies" (CodeSignOnCopy) phase, or when a | ||
| # build with code signing disabled (e.g. `make test`) writes into it. | ||
| if ! codesign --verify --strict -R="$XPC_PEER_REQUIREMENT" "$LAUNCH_DAEMON_BINARY_PATH"; then |
There was a problem hiding this comment.
On a local make test run with CI unset, the Makefile explicitly passes CODE_SIGNING_REQUIRED=NO and CODE_SIGNING_ALLOWED=NO, while the Coder Desktop test scheme builds the app target and its post-build script invokes this helper installer. The resulting helper is unsigned or ad-hoc signed and cannot satisfy the Apple anchor/team-certificate requirement, so this new exit 1 aborts xcodebuild before the tests run. This breaks the documented signing-free test workflow, particularly for external contributors without Coder's certificate; skip the installer/verification when signing is disabled or during test builds.
Useful? React with 👍 / 👎.
A signing-free build (e.g. `make test`, which passes CODE_SIGNING_ALLOWED=NO) can't produce a helper the app will accept over XPC, so there's no point bootstrapping one as a root daemon (and prompting for an admin password). Previously this silently replaced the installed helper with an ad-hoc signed one, breaking app<->helper XPC.
|
@codex review re: Skip signature enforcement for unsigned test builds I don't think we should skip the verification when signing is disabled; the unsigned build is exactly the case we want to catch. Today, a local make test with a stale stamp bootstraps an ad-hoc signed helper as a root LaunchDaemon, which the installed app then rejects over XPC with -67050. That's the bug this PR is trying to surface at build time instead of runtime. CI is unaffected, as the wrapper exits early when $CI is set. The real issue is that a signing-free test build shouldn't be installing a root daemon (and prompting for admin) at all, since it can't produce a helper the app will accept. I've added a guard to the wrapper that skips the installer entirely when CODE_SIGNING_ALLOWED=NO. |
|
Codex Review: Didn't find any major issues. Breezy! Reviewed commit: ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
If Codex has suggestions, it will comment; otherwise it will react with 👍. Codex can also answer questions or update the PR. Try commenting "@codex address that feedback". |
My local dev build suddenly couldn't talk to the helper over XPC, despite no code changes:
It turns out a stale, ad-hoc signed helper was sitting in the installed bundle. Xcode had skipped the embed phase that re-signs the helper on copy, and because the installed bundle doubles as a build output (
DEPLOYMENT_LOCATION), builds with signing disabled (e.g.make test) can also clobber it.upsert-dev-helper.shhappily bootstrapped it anyway, so the failure only surfaced at runtime as these cryptic errors.To fix, the script now verifies the helper satisfies
Validator.xpcPeerRequirementbefore installing it, and fails the build with remediation steps if it doesn't.