Skip to content

fix: macOS notarization and PTY4J signing#99

Merged
kshivang merged 3 commits intomasterfrom
dev
Dec 8, 2025
Merged

fix: macOS notarization and PTY4J signing#99
kshivang merged 3 commits intomasterfrom
dev

Conversation

@kshivang
Copy link
Copy Markdown
Owner

@kshivang kshivang commented Dec 8, 2025

Summary

  • Add signPty4jBinaries Gradle task for macOS notarization
  • Fix PTY4J native executable signing with hardened runtime
  • Add missing entitlements for JVM apps (disable-library-validation, debugger)
  • Create runtime-entitlements.plist for JVM runtime
  • Add detailed notarization error logging in CI
  • Fix partial bottom row rendering at certain window sizes

Problem

Apple notarization was failing because:

  1. pty4j-unix-spawn-helper inside the pty4j JAR doesn't have hardened runtime enabled
  2. Missing entitlements required for JVM apps with native libraries

Solution

The new signPty4jBinaries Gradle task:

  1. Extracts pty4j JAR from the built app bundle
  2. Signs all native binaries (.dylib, spawn-helper) with --options runtime
  3. Repackages the JAR with signed binaries
  4. Re-signs the entire app bundle with entitlements

Task dependencies:

  • Runs automatically after createDistributable (via finalizedBy)
  • Runs before packageDmg (via mustRunAfter)

Test plan

  • Run ./gradlew :compose-ui:createDistributable locally
  • Verify signPty4jBinaries task executes and signs PTY4J natives
  • Run ./gradlew :compose-ui:packageDmg to create DMG
  • Submit to notarization via GitHub Actions release workflow

🤖 Generated with Claude Code

kshivang and others added 3 commits December 8, 2025 02:45
The pty4j library contains native executables (pty4j-unix-spawn-helper)
that need to be signed with hardened runtime for Apple notarization.

Changes:
- Split build into createDistributable + packageDmg steps
- Add step to extract, sign, and repackage JARs with native executables
- Sign all Mach-O binaries with hardened runtime and entitlements
- Re-sign entire app bundle after JAR modifications

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
The pty4j library contains native executables (pty4j-unix-spawn-helper)
that must be signed with hardened runtime for Apple notarization.

This task:
- Extracts pty4j JAR from the built app bundle
- Signs all native binaries (.dylib and spawn-helper) with hardened runtime
- Repackages the JAR with signed binaries
- Re-signs the entire app bundle with entitlements

Task dependencies:
- Runs after createDistributable (via finalizedBy)
- Runs before packageDmg (via mustRunAfter)

Based on working implementation from boss-main project.

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
@kshivang kshivang merged commit 8cb7af6 into master Dec 8, 2025
7 checks passed
@claude
Copy link
Copy Markdown

claude Bot commented Dec 8, 2025

Code Review: macOS Notarization and PTY4J Signing

Overall Assessment

This PR addresses a critical issue with macOS notarization by properly signing PTY4J native binaries with hardened runtime. The implementation is solid and well-documented, with a few suggestions for improvement.

✅ Strengths

  1. Clear Problem Statement: The PR description and code comments clearly explain why this is needed (notarization failures due to unsigned PTY4J native binaries)

  2. Comprehensive Solution:

    • Both Gradle task and CI workflow implementations cover the same ground
    • Proper use of hardened runtime flags (--options runtime)
    • Correct re-signing of the entire app bundle after JAR modifications
  3. Good Error Handling: Fallback to - identity and conditional execution based on environment

  4. Modern Gradle Practices: Using @Inject for ExecOperations instead of deprecated project.exec()

🔍 Issues & Concerns

1. Duplication Between Gradle Task and CI Workflow (Minor)

The signPty4jBinaries Gradle task (compose-ui/build.gradle.kts:241-417) and the "Sign Native Executables in JARs" CI step (.github/workflows/release.yml:142-196) implement nearly identical logic. This creates maintenance burden.

Recommendation: Choose one approach:

  • Option A: Keep only the Gradle task and call it from CI
  • Option B: Keep only the CI workflow step and remove the Gradle task

The Gradle task is preferable because it works for local development and is easier to test.

2. SIGNED_SOMETHING Variable Issue (Critical)

In the CI workflow (line 166), SIGNED_SOMETHING is set inside a subshell (the while read loop), so the check at line 172 will never see the modified value. The pipeline creates a subshell, making variable assignment ineffective.

Fix: Use process substitution or restructure the logic to avoid subshell issues.

3. Silent Failure in CI Workflow (Moderate)

Line 171 in the CI workflow has || echo "Warning: Failed to sign" which swallows errors. If signing fails, the build continues with unsigned binaries, defeating the purpose.

Recommendation: Fail the build on signing errors instead of just printing warnings.

4. Missing Error Handling in Gradle Task (Moderate)

The Gradle task wraps signing in try-catch but continues on failure (line 292). This could lead to partially signed JARs. Consider tracking failures and throwing a GradleException if any signing operations fail.

5. Jar Extraction/Repackaging Could Break Signatures (Moderate)

The CI workflow uses unzip + zip (lines 160, 177-179) which doesn't preserve extended attributes or code signatures. The Gradle task uses jar xf/cf which is better but may still have issues.

Recommendation: Test that signed binaries remain properly signed after repackaging. Consider using ditto for preservation on macOS.

6. File Detection Logic Could Be More Robust (Minor)

Lines 164-165 (CI) and similar logic in Gradle use basic pattern matching. A Mach-O binary check using the file command would be more reliable to verify files are actually executables before attempting to sign.

📊 Performance Considerations

  • Extracting and repackaging JARs during build will add time (estimate: 10-30 seconds per JAR)
  • The tempDir.deleteRecursively() in finally block (line 318) is good cleanup
  • Consider adding progress indicators for long operations

🔒 Security Considerations

  1. Entitlements Are Appropriate: Both files grant necessary permissions for a JVM terminal app

    • com.apple.security.cs.allow-jit: Required for JVM
    • com.apple.security.cs.disable-library-validation: Required for loading PTY4J natives
    • com.apple.security.cs.debugger: Standard for dev tools
  2. Identity Handling: Using environment variables for signing identity is correct

  3. No Hardcoded Secrets: ✅ Good use of GitHub secrets

🧪 Test Coverage

Missing:

  • No automated tests for the signing process
  • No verification that signed binaries actually pass notarization

Recommendations:

  1. Add a verification step that runs codesign -vvv --deep --strict on the final app
  2. Consider adding a test that extracts and checks signatures on PTY4J binaries
  3. Add CI check that fails if Mach-O binaries are found unsigned

📝 Code Quality

  1. Comments: Excellent inline documentation explaining the "why"
  2. Structure: Clean separation of concerns
  3. Naming: Clear variable names
  4. Logging: Good use of emojis for visual scanning ✅

Minor Nits

  1. Line 235: Comment mentions "Replaces deprecated project.exec()" but doesn't specify Gradle version - add "for Gradle 9.0+"
  2. Line 305: Consider extracting magic strings like ".jar", ".dylib" to constants
  3. Line 378: Remove duplicated println that's already in line 369

🎯 Action Items

Must Fix (Breaking Issues):

Should Fix (Before Merge):

Nice to Have:


Verdict: ⚠️ Changes Requested

This PR solves an important problem, but has a critical bug (#2) and several moderate issues that should be addressed before merging. The implementation approach is sound, but needs refinement in error handling and de-duplication.

Great work on tackling the notarization challenge! The documentation and code structure are excellent.


🤖 Generated with Claude Code

kshivang added a commit that referenced this pull request Dec 12, 2025
fix: macOS notarization and PTY4J signing
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