Feature: Sign build artifacts #71
Conversation
# Conflicts: # .github/workflows/build-mac.yml # .github/workflows/build-win.yml
WalkthroughThe pull request introduces significant modifications to the GitHub Actions workflows for building Java applications on both macOS and Windows platforms. In the macOS workflow ( In the Windows workflow ( Possibly related PRs
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![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
🧹 Outside diff range and nitpick comments (3)
.github/workflows/build-win.yml (1)
84-100: Enhance temporary directory handling and path securityWhile the DLL extraction logic is sound, consider these improvements:
- Add cleanup of the temporary extraction directory after signing
- Use a more secure temporary path with a unique identifier
Consider this enhanced version:
- $jarExtractDir = New-Item -Path ".\target\jar-extract" -ItemType Directory + $tempId = [System.Guid]::NewGuid().ToString() + $jarExtractDir = New-Item -Path ".\target\jar-extract-$tempId" -ItemType Directory -ForceAnd add cleanup at the end of the workflow:
Remove-Item -Path $jarExtractDir -Recurse -Force.github/workflows/build-mac.yml (2)
69-69: Consider making Xcode version more maintainableThe hardcoded Xcode paths (
Xcode_16.appandXcode_15.2.app) might require frequent updates as new Xcode versions are released.Consider:
- Using environment variables for Xcode versions
- Or using a matrix strategy to define Xcode versions
- xcode-path: /Applications/Xcode_16.app + xcode-path: /Applications/Xcode_${{ env.XCODE_VERSION }}.appAlso applies to: 73-73
224-224: Add newline at end of fileYAML files should end with a newline character.
cryptomator-cli-*.asc +🧰 Tools
🪛 yamllint (1.35.1)
[error] 224-224: no new line character at the end of file
(new-line-at-end-of-file)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
dist/mac/cryptomator-cli.entitlementsis excluded by!**/dist/**
📒 Files selected for processing (2)
.github/workflows/build-mac.yml(6 hunks).github/workflows/build-win.yml(2 hunks)
🧰 Additional context used
🪛 actionlint (1.7.4)
.github/workflows/build-mac.yml
112-112: shellcheck reported issue in this script: SC2086:info:6:53: Double quote to prevent globbing and word splitting
(shellcheck)
112-112: shellcheck reported issue in this script: SC2086:info:9:57: Double quote to prevent globbing and word splitting
(shellcheck)
112-112: shellcheck reported issue in this script: SC2086:info:10:41: Double quote to prevent globbing and word splitting
(shellcheck)
112-112: shellcheck reported issue in this script: SC2086:info:11:57: Double quote to prevent globbing and word splitting
(shellcheck)
112-112: shellcheck reported issue in this script: SC2086:info:14:17: Double quote to prevent globbing and word splitting
(shellcheck)
112-112: shellcheck reported issue in this script: SC2086:info:14:81: Double quote to prevent globbing and word splitting
(shellcheck)
112-112: shellcheck reported issue in this script: SC2086:info:15:35: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:2:104: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:3:170: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:5:100: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2044:warning:6:17: For loops over find output are fragile. Use find -exec or a while read loop
(shellcheck)
133-133: shellcheck reported issue in this script: SC2006:style:6:17: Use $(...) notation instead of legacy backticks ...
(shellcheck)
133-133: shellcheck reported issue in this script: SC2006:style:7:9: Use $(...) notation instead of legacy backticks ...
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:7:19: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:8:29: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:11:14: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:11:29: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:12:10: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:12:67: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:13:10: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:13:68: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:14:8: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:15:11: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:16:16: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2035:info:16:32: Use ./glob or -- glob so names with dashes won't become options
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:18:11: Double quote to prevent globbing and word splitting
(shellcheck)
133-133: shellcheck reported issue in this script: SC2086:info:24:95: Double quote to prevent globbing and word splitting
(shellcheck)
173-173: shellcheck reported issue in this script: SC2086:info:4:48: Double quote to prevent globbing and word splitting
(shellcheck)
173-173: shellcheck reported issue in this script: SC2086:info:5:41: Double quote to prevent globbing and word splitting
(shellcheck)
173-173: shellcheck reported issue in this script: SC2086:info:6:48: Double quote to prevent globbing and word splitting
(shellcheck)
195-195: shellcheck reported issue in this script: SC2086:info:2:26: Double quote to prevent globbing and word splitting
(shellcheck)
🪛 yamllint (1.35.1)
.github/workflows/build-mac.yml
[error] 224-224: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (6)
.github/workflows/build-win.yml (3)
81-83: LGTM: Permission fix is necessary for code signing
The removal of the read-only attribute is required to allow the subsequent code signing process to modify the executable.
135-141: LGTM: Artifact upload configuration is well-structured
The artifact upload step correctly handles both the binary and its signature, with appropriate error conditions.
Line range hint 1-141: Verify workflow integration with other build configurations
Let's ensure this workflow aligns with other build configurations and doesn't have conflicting settings.
✅ Verification successful
Windows build workflow is properly integrated with other platform configurations
The verification shows consistent implementation across all platform build workflows:
- Version handling (
semVerStr,semVerNum) is uniform across Linux, macOS, and Windows builds - Artifact naming follows the same pattern:
cryptomator-cli-{version}-{platform}-{arch}.zip - Code signing is properly configured for Windows, while other platforms use their respective signing mechanisms
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for consistency across build workflows and verify signing configurations
# Check for consistent version handling across workflows
echo "Checking version handling consistency:"
rg -l "semVerStr|semVerNum" .github/workflows/
# Check for other signing configurations that might conflict
echo "Checking for other signing configurations:"
rg -l "codesign|sign|certificate" .github/workflows/
# Check for consistent artifact naming patterns
echo "Checking artifact naming consistency:"
rg -l "artifact-name|cryptomator-cli-.*\.zip" .github/workflows/
Length of output: 731
Script:
#!/bin/bash
# Let's examine the version handling and artifact naming patterns in detail
echo "Checking version handling patterns:"
rg "semVerStr|semVerNum" .github/workflows/build-{win,mac,linux}.yml -A 2 -B 2
echo -e "\nChecking artifact naming patterns:"
rg "artifact-name.*cryptomator-cli-.*" .github/workflows/build-{win,mac,linux}.yml
echo -e "\nChecking signing configurations:"
rg "uses:.*code-sign-action|certificate:|timestampUrl:" .github/workflows/build-{win,mac,linux}.yml
Length of output: 7775
.github/workflows/build-mac.yml (3)
11-14: LGTM: Well-structured workflow input and prepare job changes
The changes properly set up the foundation for app notarization:
- Optional boolean input for controlling notarization
- Revision number output for bundle versioning
- Full history fetch for accurate commit counting
Also applies to: 35-35, 37-39
166-199: LGTM: Well-implemented notarization process
The notarization implementation follows Apple's best practices:
- Proper credential handling using keychain
- Conditional execution based on user input
- Cleanup ensures sensitive data is removed
🧰 Tools
🪛 actionlint (1.7.4)
173-173: shellcheck reported issue in this script: SC2086:info:4:48: Double quote to prevent globbing and word splitting
(shellcheck)
173-173: shellcheck reported issue in this script: SC2086:info:5:41: Double quote to prevent globbing and word splitting
(shellcheck)
173-173: shellcheck reported issue in this script: SC2086:info:6:48: Double quote to prevent globbing and word splitting
(shellcheck)
195-195: shellcheck reported issue in this script: SC2086:info:2:26: Double quote to prevent globbing and word splitting
(shellcheck)
202-215: Verify GPG key availability
The workflow uses a specific GPG key (615D449FE6E6A235) for signing. Ensure this key is properly maintained and backed up.
✅ Verification successful
The initial results show the key is used across all build workflows. Let's verify the context to ensure consistent usage pattern.
GPG key usage is consistent across build workflows
The GPG key (615D449FE6E6A235) is properly implemented across all build workflows (Linux, Mac, and Windows) with consistent configuration:
- Uses the same key ID in all workflows
- Implements identical signing process with batch mode
- Utilizes the same secrets (RELEASES_GPG_PRIVATE_KEY and RELEASES_GPG_PASSPHRASE)
- Followed by artifact upload steps in each workflow
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify if the GPG key is used in other workflows
rg -l "615D449FE6E6A235" .github/workflows/
Length of output: 141
Script:
#!/bin/bash
# Check the context of GPG key usage in all workflows
rg -A 5 "615D449FE6E6A235" .github/workflows/
Length of output: 2568
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
.github/workflows/build-win.yml (1)
84-100: Add error handling for ZIP operationsWhile the extraction logic is correct, consider adding error handling for robustness:
- Directory creation might fail
- ZIP operations could throw exceptions
$jarExtractDir = New-Item -Path ".\target\jar-extract" -ItemType Directory +if (-not $?) { + Write-Error "Failed to create extraction directory" + exit 1 +} Get-ChildItem -Path $jarFolder -Filter "*.jar" | ForEach-Object { + try { $jar = [Io.compression.zipfile]::OpenRead($_.FullName) if (@($jar.Entries | Where-Object {$_.Name.ToString().EndsWith(".dll")} | Select-Object -First 1).Count -gt 0) { Set-Location $jarExtractDir Expand-Archive -Path $_.FullName } + } catch { + Write-Error "Failed to process JAR: $_" + exit 1 + } finally { $jar.Dispose() + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
.github/workflows/build-win.yml(2 hunks)
🔇 Additional comments (4)
.github/workflows/build-win.yml (4)
101-110: LGTM: Secure code signing configuration
The code signing configuration correctly:
- Uses secrets for sensitive data
- Uses variables for certificate SHA1
- Includes proper timestamping
111-124: LGTM: DLL replacement logic is correct
The implementation correctly updates JARs with signed DLLs using proper path resolution.
135-141: LGTM: Artifact upload properly configured
The artifact upload correctly includes both the binary and its signature, with appropriate error handling.
81-83: LGTM: Permission fix is necessary for code signing
The step correctly removes the read-only attribute from the executable before signing.
Let's verify if the file exists and is writable after this step:
✅ Verification successful
Permission fix step is correctly placed and implemented
The step is properly sequenced after the jpackage build and before the DLL extraction/signing process. The PowerShell command to remove the read-only attribute is correct and necessary to prepare the executable for subsequent signing operations.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if the target exe exists in the workflow file
rg -A 2 "target/cryptomator-cli/cryptomator-cli.exe" .github/workflows/
Length of output: 306
Script:
#!/bin/bash
# Let's verify the complete context around this file operation
# and ensure it's properly sequenced in the workflow
rg -B 5 -A 10 "Fix permissions" .github/workflows/build-win.yml
Length of output: 804
This PR adds signing to the build binaries.