IEP-1720: ci(macos) switch to release-sign action for code signing#1406
Open
kolipakakondal wants to merge 1 commit intomasterfrom
Open
IEP-1720: ci(macos) switch to release-sign action for code signing#1406kolipakakondal wants to merge 1 commit intomasterfrom
kolipakakondal wants to merge 1 commit intomasterfrom
Conversation
📝 WalkthroughWalkthroughThe PR refactors the macOS code-signing workflow by replacing inline signing scripts with the dedicated Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Possibly related PRs
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches
🧪 Generate unit tests (beta)
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: 1
🧹 Nitpick comments (2)
.github/workflows/ci.yml (2)
123-123: Rename duplicated step name for log clarity.Line 123 reuses
Codesign Espressif-IDE; use a distinct name (for example,Codesign DMG artifacts) to keep CI logs unambiguous.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml at line 123, The CI step name "Codesign Espressif-IDE" is duplicated; update the step's name string in the workflow (the step that currently shows name: "Codesign Espressif-IDE") to a distinct, descriptive value such as "Codesign DMG artifacts" so logs are unambiguous and each step can be distinguished in CI output.
123-130: Add explicit DMG signature verification before upload.After signing, verify both DMGs before Lines 131-143 upload steps to avoid publishing unsigned/broken artifacts.
✅ Suggested follow-up step
+ - name: Verify DMG signatures + run: | + codesign --verify --deep --strict --verbose=2 releng/ide-dmg-builder/Espressif-IDE-macosx-cocoa-x86_64.dmg + codesign --verify --deep --strict --verbose=2 releng/ide-dmg-builder/Espressif-IDE-macosx-cocoa-aarch64.dmg🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In @.github/workflows/ci.yml around lines 123 - 130, Add an explicit verification step after the "Codesign Espressif-IDE" action to ensure both DMG artifacts produced by releng/ide-dmg-builder are correctly signed before upload: locate the "Codesign Espressif-IDE" step and insert a new job/step that finds the two DMG files, runs hdiutil verify <dmg> and codesign -v --deep --strict <dmg> (and optionally spctl --assess --type execute <dmg>) for each DMG, fails the workflow if any verification command returns non-zero, and outputs clear error messages identifying which DMG failed; reference the step name "Codesign Espressif-IDE" and the artifact producer "releng/ide-dmg-builder" to place this verification immediately after signing and before the upload steps.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In @.github/workflows/ci.yml:
- Line 109: The workflow currently references the action string "uses:
espressif/release-sign@master", which is mutable; update both occurrences of
that exact string to pin to a specific commit SHA (e.g., "uses:
espressif/release-sign@<commit-sha>") so the release-sign action is immutable
and reproducible—replace both instances in the workflow.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Line 123: The CI step name "Codesign Espressif-IDE" is duplicated; update the
step's name string in the workflow (the step that currently shows name:
"Codesign Espressif-IDE") to a distinct, descriptive value such as "Codesign DMG
artifacts" so logs are unambiguous and each step can be distinguished in CI
output.
- Around line 123-130: Add an explicit verification step after the "Codesign
Espressif-IDE" action to ensure both DMG artifacts produced by
releng/ide-dmg-builder are correctly signed before upload: locate the "Codesign
Espressif-IDE" step and insert a new job/step that finds the two DMG files, runs
hdiutil verify <dmg> and codesign -v --deep --strict <dmg> (and optionally spctl
--assess --type execute <dmg>) for each DMG, fails the workflow if any
verification command returns non-zero, and outputs clear error messages
identifying which DMG failed; reference the step name "Codesign Espressif-IDE"
and the artifact producer "releng/ide-dmg-builder" to place this verification
immediately after signing and before the upload steps.
| /usr/bin/codesign --options runtime --force -s "ESPRESSIF SYSTEMS (SHANGHAI) CO., LTD. (QWXF6GB4AV)" $PWD/releng/ide-dmg-builder/Espressif-IDE-macosx-cocoa-aarch64.dmg -v | ||
| /usr/bin/codesign -v -vvv --deep $PWD/releng/ide-dmg-builder/Espressif-IDE-macosx-cocoa-aarch64.dmg | ||
|
|
||
| uses: espressif/release-sign@master |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# First, let's check the file exists and view the relevant lines
cat -n .github/workflows/ci.yml | sed -n '105,130p'Repository: espressif/idf-eclipse-plugin
Length of output: 1472
🏁 Script executed:
# Also verify the exact pattern at lines 109 and 124
sed -n '109p;124p' .github/workflows/ci.ymlRepository: espressif/idf-eclipse-plugin
Length of output: 155
🏁 Script executed:
# Search for all uses of espressif/release-sign in the file
rg -n 'espressif/release-sign' .github/workflows/ci.ymlRepository: espressif/idf-eclipse-plugin
Length of output: 163
🏁 Script executed:
# Search for all action uses in the workflow
rg -n 'uses:' .github/workflows/ci.yml | head -30Repository: espressif/idf-eclipse-plugin
Length of output: 742
🏁 Script executed:
# Check what versions/tags are available for espressif/release-sign
# First let's see if there are any pinned versions in other workflows
fd -t f '\.yml$' -o '\.yaml$' .github/workflows/ -exec grep -l 'espressif/release-sign' {} \;Repository: espressif/idf-eclipse-plugin
Length of output: 205
🏁 Script executed:
# Check for GitHub Actions best practices - search for pinned vs unpinned in entire workflow
rg '@(master|main|latest|develop|v[0-9])' .github/workflows/ci.yml | wc -lRepository: espressif/idf-eclipse-plugin
Length of output: 74
Pin espressif/release-sign to a specific commit SHA.
Lines 109 and 124 use @master, which creates a mutable, non-reproducible release-signing path. All other actions in this workflow are pinned to specific versions. Replace both with a pinned commit SHA.
Suggested change
- uses: espressif/release-sign@master
+ uses: espressif/release-sign@<commit-sha>Apply to both line 109 and line 124.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In @.github/workflows/ci.yml at line 109, The workflow currently references the
action string "uses: espressif/release-sign@master", which is mutable; update
both occurrences of that exact string to pin to a specific commit SHA (e.g.,
"uses: espressif/release-sign@<commit-sha>") so the release-sign action is
immutable and reproducible—replace both instances in the workflow.
kolipakakondal
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
ci(macos): switch to release-sign action for code signing
Fixes # (IEP-1720)
Type of change
How has this been tested?
Please describe the tests that you ran to verify your changes. Provide instructions so we can reproduce. Please also list any relevant details for your test configuration
Test Configuration:
Dependent components impacted by this PR:
Checklist
Summary by CodeRabbit