build: make signing portable across contributor teams#929
Conversation
Previously, building required hardcoded values tied to the upstream
maintainer's Apple Developer Team (EG6ZYP3AQH):
- Info.plist / Daemon/selfcontrold-Info.plist / selfcontrol-cli-Info.plist
embedded SMPrivilegedExecutables / SMAuthorizedClients requirements
pinned to EG6ZYP3AQH and to Developer ID certificate fields, which
only validate against Charlie Stigler's signing identity.
- Daemon/SCDaemon.m hardcoded the same OU in its XPC client
code-signing requirement, so any contributor's build would fail
SecCodeCheckValidity with errSecCSReqFailed (helper installs but the
UI shows "Couldn't communicate with a helper application").
- Together these caused SMJobBless to fail with CFErrorDomainLaunchd
error 4 ("helper tool install failed") on any non-Charlie build.
Changes:
* Plists now reference \$(DEVELOPMENT_TEAM) and rely on certificate
leaf[subject.OU] — works for both Apple Development (local dev) and
Developer ID Application (release) certificates.
* The daemon target switches from manually injecting Info.plist via
OTHER_LDFLAGS -sectcreate (raw byte copy, no variable substitution)
to Xcode's CREATE_INFOPLIST_SECTION_IN_BINARY + INFOPLIST_PREPROCESS,
matching the existing selfcontrol-cli target pattern. The launchd
plist sectcreate is unchanged.
* SCDaemon.m's XPC accept handler now derives the expected team ID
from its own embedded signing (SecCodeCopySelf +
kSecCodeInfoTeamIdentifier), so the requirement is automatically
self-consistent.
* LocalSigning.xcconfig.example documents the per-developer team-ID
setup; the real LocalSigning.xcconfig is gitignored.
Net effect: any contributor with a free Apple Developer account can
clone, set DEVELOPMENT_TEAM in LocalSigning.xcconfig, and the full
SelfControl + helper + CLI stack signs, installs, and runs without
modifying any tracked file.
📝 WalkthroughWalkthroughThis PR migrates code-signing validation from hardcoded certificate constraints to dynamic developer team identifiers, adds periodic checkpoint timers to the daemon, refactors XPC client authentication, and updates the Xcode build configuration to include new source files and stricter security settings. ChangesTeam ID Configuration and Code Signing Updates
Daemon Checkpoint and XPC Authentication Updates
Xcode Build System Configuration
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (1)
LocalSigning.xcconfig.example (1)
5-5: 💤 Low valueConsider adding team ID format guidance.
The placeholder is clear, but new contributors might benefit from knowing that Apple Developer Team IDs are typically 10 alphanumeric characters (e.g., "AB12CD34EF").
📝 Optional comment enhancement
// Copy this file to LocalSigning.xcconfig and put your Apple Developer -// Team ID below. Find your Team ID at developer.apple.com → Membership. +// Team ID below (10 alphanumeric characters, e.g., AB12CD34EF). +// Find your Team ID at developer.apple.com → Membership. // This file is gitignored so you won't accidentally commit your team ID.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@LocalSigning.xcconfig.example` at line 5, Add guidance to the DEVELOPMENT_TEAM placeholder by updating the comment or value for DEVELOPMENT_TEAM in LocalSigning.xcconfig.example to indicate the expected format (Apple Developer Team IDs are typically 10 alphanumeric characters, e.g., "AB12CD34EF"); reference the DEVELOPMENT_TEAM key so contributors know to replace YOUR_TEAM_ID_HERE with a 10-character alphanumeric Team ID and optionally include an example value and brief note about where to find the Team ID in Apple Developer account settings.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@Daemon/SCDaemon.m`:
- Around line 110-123: The new checkpoint timer helper methods
(startCheckpointTimer and stopCheckpointTimer) are never invoked, so
SCBlockClock.tickCheckpoint never runs; call startCheckpointTimer from the
daemon startup path (e.g., the method that initializes/starts the daemon, such
as the startup/start method in SCDaemon) so the timer is scheduled when the
daemon comes up, and call stopCheckpointTimer from the daemon shutdown/teardown
path (e.g., stop, dealloc, or shutdown handler) to invalidate the timer on exit;
ensure you reference the existing startCheckpointTimer and stopCheckpointTimer
methods and keep calls symmetric to avoid leaks.
- Around line 200-208: The code currently ignores the return value of
SecRequirementCreateWithString so isSelfControlApp may be NULL and
SecCodeCheckValidity then becomes ineffective; update the block around
requirementString/SecRequirementCreateWithString to check the OSStatus result,
and if it is not errSecSuccess fail closed by logging/erroring and setting
clientValidityStatus to a failing status (or returning/aborting) before calling
SecCodeCheckValidity; ensure you reference SecRequirementCreateWithString,
isSelfControlApp and clientValidityStatus and free/CFRelease isSelfControlApp
only when created successfully.
In `@SelfControl.xcodeproj/project.pbxproj`:
- Around line 3059-3060: Multiple targets generate and overwrite the same
${PROJECT_DIR}/version-header.h when BuildIndependentTargetsInParallel = YES
causing race conditions; modify the build scripts that write "version-header.h"
(the per-target script invocations referenced in the pbxproj) to write into each
target's DERIVED_FILE_DIR (or another target-specific derived path) instead of
${PROJECT_DIR}/version-header.h, and update each target's header search/include
path accordingly so they consume their own derived header; alternatively, if you
cannot change the scripts now, revert BuildIndependentTargetsInParallel to NO
until the output files are isolated.
- Around line 4220-4221: The Release build configurations in the pbxproj
currently hardcode CODE_SIGN_IDENTITY = "Apple Development" and
CODE_SIGN_IDENTITY[sdk=macosx*] = "Apple Development"; remove or unset these
keys from the Release config entries so Release builds do not pin to the
development identity; ensure "Apple Development" remains only in Debug/local
config entries (or leave identity unset) so the distribution-build.rb or CI can
supply the proper signing identity for notarized/distribution builds.
---
Nitpick comments:
In `@LocalSigning.xcconfig.example`:
- Line 5: Add guidance to the DEVELOPMENT_TEAM placeholder by updating the
comment or value for DEVELOPMENT_TEAM in LocalSigning.xcconfig.example to
indicate the expected format (Apple Developer Team IDs are typically 10
alphanumeric characters, e.g., "AB12CD34EF"); reference the DEVELOPMENT_TEAM key
so contributors know to replace YOUR_TEAM_ID_HERE with a 10-character
alphanumeric Team ID and optionally include an example value and brief note
about where to find the Team ID in Apple Developer account settings.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 0e02285c-71d1-44b4-9c1c-64a32219f9c6
📒 Files selected for processing (7)
.gitignoreDaemon/SCDaemon.mDaemon/selfcontrold-Info.plistInfo.plistLocalSigning.xcconfig.exampleSelfControl.xcodeproj/project.pbxprojselfcontrol-cli-Info.plist
| - (void)startCheckpointTimer { | ||
| if (self.checkpointTimer != nil) return; | ||
| self.checkpointTimer = [NSTimer scheduledTimerWithTimeInterval: 30.0 | ||
| repeats: YES | ||
| block: ^(NSTimer* _Nonnull t) { | ||
| [SCBlockClock tickCheckpoint]; | ||
| }]; | ||
| } | ||
|
|
||
| - (void)stopCheckpointTimer { | ||
| if (self.checkpointTimer == nil) return; | ||
| [self.checkpointTimer invalidate]; | ||
| self.checkpointTimer = nil; | ||
| } |
There was a problem hiding this comment.
startCheckpointTimer is never wired into the daemon lifecycle.
I can't find a call site for this helper anywhere in Daemon/SCDaemon.m, so the new checkpoint path never starts and tickCheckpoint never runs.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Daemon/SCDaemon.m` around lines 110 - 123, The new checkpoint timer helper
methods (startCheckpointTimer and stopCheckpointTimer) are never invoked, so
SCBlockClock.tickCheckpoint never runs; call startCheckpointTimer from the
daemon startup path (e.g., the method that initializes/starts the daemon, such
as the startup/start method in SCDaemon) so the timer is scheduled when the
daemon comes up, and call stopCheckpointTimer from the daemon shutdown/teardown
path (e.g., stop, dealloc, or shutdown handler) to invalidate the timer on exit;
ensure you reference the existing startCheckpointTimer and stopCheckpointTimer
methods and keep calls symmetric to avoid leaks.
| NSString* requirementString = [NSString stringWithFormat: | ||
| @"anchor apple generic and (identifier \"org.eyebeam.SelfControl\" or identifier \"org.eyebeam.selfcontrol-cli\") and info [CFBundleVersion] >= \"407\" and certificate leaf[subject.OU] = \"%@\"", | ||
| teamID]; | ||
|
|
||
| SecRequirementRef isSelfControlApp = NULL; | ||
| // versions before 4.0 didn't have hardened code signing, so aren't trustworthy to talk to the daemon | ||
| // (plus the daemon didn't exist before 4.0 so there's really no reason they should want to run it!) | ||
| SecRequirementCreateWithString(CFSTR("anchor apple generic and (identifier \"org.eyebeam.SelfControl\" or identifier \"org.eyebeam.selfcontrol-cli\") and info [CFBundleVersion] >= \"407\" and (certificate leaf[field.1.2.840.113635.100.6.1.9] /* exists */ or certificate 1[field.1.2.840.113635.100.6.2.6] /* exists */ and certificate leaf[field.1.2.840.113635.100.6.1.13] /* exists */ and certificate leaf[subject.OU] = EG6ZYP3AQH)"), kSecCSDefaultFlags, &isSelfControlApp); | ||
| SecRequirementCreateWithString((__bridge CFStringRef)requirementString, kSecCSDefaultFlags, &isSelfControlApp); | ||
| OSStatus clientValidityStatus = SecCodeCheckValidity(guest, kSecCSDefaultFlags, isSelfControlApp); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
What does macOS SecCodeCheckValiditydo when theSecRequirementRefargument isNULL, and should callers reject the connection if SecRequirementCreateWithString fails?
💡 Result:
When the SecRequirementRef argument is NULL in a call to SecCodeCheckValidity, the function does not impose any additional code requirements beyond the standard signature validation [1][2]. It will still perform dynamic validation of the signed code, including verifying the code's signature and ensuring that the code object's host is valid [1]. Essentially, passing NULL tells the system to verify that the code is properly signed and valid, but not to enforce specific identity or entitlement constraints [1]. Regarding the failure of SecRequirementCreateWithString, callers should absolutely reject the connection (or the operation) if this function fails [3]. SecRequirementCreateWithString is responsible for compiling the text representation of your security requirements into a SecRequirement object [4]. If it fails (returns a non-zero OSStatus), it means the requirement string is invalid or could not be parsed [4][5]. Proceeding without a successfully created requirement—or worse, falling back to a NULL requirement when you intended to enforce specific security constraints—would bypass the intended security checks, leaving the application vulnerable to unauthorized or malicious code [3][6]. Always check the return status of security-critical functions and handle errors by aborting the operation [7][3].
Citations:
- 1: https://developer.apple.com/documentation/security/seccodecheckvalidity(_:_:_:)
- 2: https://developer.apple.com/documentation/security/seccodecheckvalidity(_:_:_:)?language=objc
- 3: https://stackoverflow.com/questions/1815506/how-to-obtain-codesigned-application-certificate-info
- 4: https://developer.apple.com/documentation/security/secrequirementcreatewithstring(_:_:_:)?language=objc
- 5: https://appsecuritymapping.com/wp-content/uploads/2022/09/Apple-Developer-Security.pdf
- 6: https://blog.quarkslab.com/controlplane_lpe_macos.html
- 7: https://pastebin.com/5qxTegwA
🏁 Script executed:
find . -name "SCDaemon.m" -type fRepository: SelfControlApp/selfcontrol
Length of output: 90
🏁 Script executed:
sed -n '195,215p' ./Daemon/SCDaemon.mRepository: SelfControlApp/selfcontrol
Length of output: 1307
Fail closed if building the client requirement fails.
If SecRequirementCreateWithString returns an error here, isSelfControlApp stays NULL and the subsequent validity check stops enforcing your bundle/team restriction. That turns this into a fail-open auth path. The initial signature validation in SecCodeCheckValidity will pass for any properly signed code, completely bypassing the identity constraints you intended to enforce.
🔒 Possible fix
SecRequirementRef isSelfControlApp = NULL;
- // versions before 4.0 didn't have hardened code signing, so aren't trustworthy to talk to the daemon
- // (plus the daemon didn't exist before 4.0 so there's really no reason they should want to run it!)
- SecRequirementCreateWithString((__bridge CFStringRef)requirementString, kSecCSDefaultFlags, &isSelfControlApp);
+ // versions before 4.0 didn't have hardened code signing, so aren't trustworthy to talk to the daemon
+ // (plus the daemon didn't exist before 4.0 so there's really no reason they should want to run it!)
+ OSStatus requirementStatus =
+ SecRequirementCreateWithString((__bridge CFStringRef)requirementString,
+ kSecCSDefaultFlags,
+ &isSelfControlApp);
+ if (requirementStatus != errSecSuccess || isSelfControlApp == NULL) {
+ NSLog(@"Rejecting XPC connection: could not build client requirement (%d)", (int)requirementStatus);
+ CFRelease(guest);
+ return NO;
+ }
OSStatus clientValidityStatus = SecCodeCheckValidity(guest, kSecCSDefaultFlags, isSelfControlApp);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@Daemon/SCDaemon.m` around lines 200 - 208, The code currently ignores the
return value of SecRequirementCreateWithString so isSelfControlApp may be NULL
and SecCodeCheckValidity then becomes ineffective; update the block around
requirementString/SecRequirementCreateWithString to check the OSStatus result,
and if it is not errSecSuccess fail closed by logging/erroring and setting
clientValidityStatus to a failing status (or returning/aborting) before calling
SecCodeCheckValidity; ensure you reference SecRequirementCreateWithString,
isSelfControlApp and clientValidityStatus and free/CFRelease isSelfControlApp
only when created successfully.
| BuildIndependentTargetsInParallel = YES; | ||
| LastUpgradeCheck = 1620; |
There was a problem hiding this comment.
Parallel target builds now race on version-header.h.
Lines 3728, 3758, and 3775 still overwrite the same ${PROJECT_DIR}/version-header.h from three different targets. With BuildIndependentTargetsInParallel = YES, those scripts can run concurrently and make builds flaky. Either move that generated header into each target’s DERIVED_FILE_DIR or keep target parallelism off until the output is isolated.
Quick mitigation
- BuildIndependentTargetsInParallel = YES;
+ BuildIndependentTargetsInParallel = NO;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| BuildIndependentTargetsInParallel = YES; | |
| LastUpgradeCheck = 1620; | |
| BuildIndependentTargetsInParallel = NO; | |
| LastUpgradeCheck = 1620; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SelfControl.xcodeproj/project.pbxproj` around lines 3059 - 3060, Multiple
targets generate and overwrite the same ${PROJECT_DIR}/version-header.h when
BuildIndependentTargetsInParallel = YES causing race conditions; modify the
build scripts that write "version-header.h" (the per-target script invocations
referenced in the pbxproj) to write into each target's DERIVED_FILE_DIR (or
another target-specific derived path) instead of
${PROJECT_DIR}/version-header.h, and update each target's header search/include
path accordingly so they consume their own derived header; alternatively, if you
cannot change the scripts now, revert BuildIndependentTargetsInParallel to NO
until the output files are isolated.
| CODE_SIGN_IDENTITY = "Apple Development"; | ||
| "CODE_SIGN_IDENTITY[sdk=macosx*]" = "Apple Development"; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== Signing-related settings outside project.pbxproj =="
rg -n -C2 'CODE_SIGN_IDENTITY|DEVELOPMENT_TEAM|Developer ID|Apple Development|codesign|notarytool|xcodebuild|archive|exportArchive' \
-g '!SelfControl.xcodeproj/project.pbxproj'
echo
echo "== Relevant tracked build/config files =="
fd -HI '.(xcconfig|sh|rb|ya?ml)$|^Podfile$|^README' . \
| xargs -r rg -n -C2 'CODE_SIGN_IDENTITY|DEVELOPMENT_TEAM|Developer ID|Apple Development|codesign|notarytool|xcodebuild|archive|exportArchive'Repository: SelfControlApp/selfcontrol
Length of output: 50382
🏁 Script executed:
cat -n distribution-build.rb | head -150Repository: SelfControlApp/selfcontrol
Length of output: 8085
🏁 Script executed:
grep -r "xcodebuild.*build\|xcodebuild.*archive\|xcodebuild.*-scheme" . -g "!SelfControl.xcodeproj/project.pbxproj" --include="*.rb" --include="*.sh" 2>/dev/null | head -20Repository: SelfControlApp/selfcontrol
Length of output: 52
🏁 Script executed:
grep -r "archive\|exportArchive\|build " distribution-build.rb | head -20Repository: SelfControlApp/selfcontrol
Length of output: 1291
🏁 Script executed:
find . -name "*.yml" -o -name "*.yaml" -o -name ".github" | head -20Repository: SelfControlApp/selfcontrol
Length of output: 129
🏁 Script executed:
find .github -name "*.yml" -o -name "*.yaml" | xargs catRepository: SelfControlApp/selfcontrol
Length of output: 662
🏁 Script executed:
find .github/workflows -type f 2>/dev/null | head -20Repository: SelfControlApp/selfcontrol
Length of output: 52
🏁 Script executed:
ls -la .github/Repository: SelfControlApp/selfcontrol
Length of output: 303
🏁 Script executed:
grep -r "xcodebuild\|archive\|build" README* CONTRIBUTING* docs/ 2>/dev/null | grep -i "release\|distribution\|build"Repository: SelfControlApp/selfcontrol
Length of output: 437
🏁 Script executed:
cat README.md | grep -A 30 "Build and run\|code signing\|release\|distribution"Repository: SelfControlApp/selfcontrol
Length of output: 207
Don't pin Release builds to Apple Development.
These Release configs are now hardcoded to a development identity, which will be used whenever the app is built for Release. This breaks portable/notarized distribution unless explicitly overridden at build time. The release automation script (distribution-build.rb) archives a pre-built app and does not perform the build itself, so the pbxproj signing settings will apply directly. Keep Apple Development scoped to local/dev builds and leave Release identity configurable.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@SelfControl.xcodeproj/project.pbxproj` around lines 4220 - 4221, The Release
build configurations in the pbxproj currently hardcode CODE_SIGN_IDENTITY =
"Apple Development" and CODE_SIGN_IDENTITY[sdk=macosx*] = "Apple Development";
remove or unset these keys from the Release config entries so Release builds do
not pin to the development identity; ensure "Apple Development" remains only in
Debug/local config entries (or leave identity unset) so the
distribution-build.rb or CI can supply the proper signing identity for
notarized/distribution builds.
|
@ahmedkh19 I love this! I'm working on a full rewrite of the app now so will hold on this until that's merged, but will def make sure this type of change makes it in after that :) |
2 participants
Summary
Currently, building SelfControl from source requires Charlie Stigler's Apple Developer Team ID (
EG6ZYP3AQH) and (effectively) his Developer ID Application signing cert. Any other contributor's build fails:SMJobBlessinstall fails withCFErrorDomainLaunchd error 4("The operation couldn't be completed") because the embeddedSMAuthorizedClientsrequirement pins toEG6ZYP3AQHplus Developer ID-only certificate fields, which an Apple Development cert does not satisfy.errSecCSReqFailed(UI: "Couldn't communicate with a helper application") becauseSCDaemon.m'slistener:shouldAcceptNewConnection:validates clients against the same hardcodedEG6ZYP3AQHrequirement.Changes
Plists use
$(DEVELOPMENT_TEAM)—Info.plist,Daemon/selfcontrold-Info.plist, andselfcontrol-cli-Info.plistreference the build's team identifier and drop the Developer ID-specific certificate fields. This works for bothApple Development(local) andDeveloper ID Application(release) certificates, since both populatesubject.OU.Daemon target embeds Info.plist correctly — switches from manual
OTHER_LDFLAGS -sectcreate __TEXT __info_plist <raw plist>(which does no variable substitution; this is why$(DEVELOPMENT_TEAM)made it into the embedded section as a literal string in early testing) to Xcode's standardCREATE_INFOPLIST_SECTION_IN_BINARY = YES+INFOPLIST_PREPROCESS = YES. This is the exact pattern the existingselfcontrol-clitarget already uses successfully.Daemon derives team ID at runtime —
SCDaemon.m's XPC accept handler now reads the daemon's ownkSecCodeInfoTeamIdentifierviaSecCodeCopySelfand builds the client-validation requirement string from that. The handler is self-consistent: it accepts clients signed by whatever team signed the daemon, no matter who that is.Per-developer team ID via gitignored xcconfig —
LocalSigning.xcconfig.exampledocuments the setup:Contributors copy → fill in their team ID → ignore the real file via
.gitignore. Zero tracked-file edits required to build on any machine.Test plan
LocalSigning.xcconfig.example→LocalSigning.xcconfig, setDEVELOPMENT_TEAMto a personal Apple Developer teamApple Developmentdd if=…/org.eyebeam.selfcontrold bs=1 skip=<offset> count=<size> | plutil -p -showsSMAuthorizedClients = "… leaf[subject.OU] = \"<team>\""(not a literal$(DEVELOPMENT_TEAM))CFErrorDomainLaunchd 4)LocalSigning.xcconfigand rebuildingNotes for review
SCDaemon.malso drops theinfo [CFBundleVersion] >= "407"floor check from one of the OR branches — kept on the version line itself, but the certificate branch is now a singleleaf[subject.OU] = "<team>"predicate. If you want the version check back, I can re-add it.LocalSigning.xcconfigexample usesxcconfigrather than aConfigfile at the project level so it threads naturally through any signing config; if you'd prefer it surfaced as a per-target build setting override I can change.Summary by CodeRabbit
New Features
Improvements
Chores