Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: add codesign verification and assessment prior to notarizing #152

Merged
merged 8 commits into from
Nov 14, 2023

Conversation

jonluca
Copy link
Contributor

@jonluca jonluca commented Aug 8, 2023

Prior to stapling, we should run an spctl --assess and codesign check to verify that the stapling will work. This caused major issues for us twice - electron builder would modify some files in the build, and that was causing stapling and notarization to fail. This will verify that the app is signed properly.

One potential change is to only run these if stapling fails - open to suggestions on that.

@jonluca jonluca requested a review from a team as a code owner August 8, 2023 22:03
@dsanders11 dsanders11 changed the title Add codesign verification and assessment prior to stapling fix: add codesign verification and assessment prior to stapling Sep 1, 2023
.gitignore Outdated Show resolved Hide resolved
src/index.ts Outdated Show resolved Hide resolved
@erickzhao
Copy link
Member

Merged in the main branch to try to kick CI

@dsanders11 dsanders11 changed the title fix: add codesign verification and assessment prior to stapling fix: add codesign verification and assessment prior to notarizing Nov 8, 2023
src/check-signature.ts Outdated Show resolved Hide resolved
@dsanders11
Copy link
Member

Looks like lint is failing. I also pushed a change to turn an import type into just import - I think the version of prettier on this repo is too old to understand that syntax.

@dsanders11
Copy link
Member

@jonluca, I think you need to log out of CircleCI and back in again, then push something to trigger CI (empty commit should do it). This is the CircleCI run associated with your last push. Happens from time to time with CircleCI unfortunately.

⚠️ Could not find a usable config.yml, you may have revoked the CircleCI OAuth app.
Please sign out of CircleCI and log back in with your VCS before triggering a new pipeline.

@jonluca
Copy link
Contributor Author

jonluca commented Nov 14, 2023

done!

src/index.ts Outdated Show resolved Hide resolved
src/check-signature.ts Show resolved Hide resolved
src/check-signature.ts Outdated Show resolved Hide resolved
Copy link
Member

@felixrieseberg felixrieseberg left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good!

@jonluca Thanks for adding this 🙇

@felixrieseberg felixrieseberg changed the title fix: add codesign verification and assessment prior to notarizing feat: add codesign verification and assessment prior to notarizing Nov 14, 2023
@felixrieseberg felixrieseberg merged commit b1b2ca1 into electron:main Nov 14, 2023
4 checks passed
Copy link

🎉 This PR is included in version 2.2.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

@DanielMcAssey
Copy link
Contributor

This has broken our .pkg signing, it throws an error, where it was working before (Using a patch to sign pkg's and dmg's)

@jonluca
Copy link
Contributor Author

jonluca commented Nov 15, 2023

@DanielMcAssey what is the error? If spctl or codesign return non-zero exit codes that implies there were issues with the bundle, so I'd be interested if it's a warning or something idiosyncratic

I'll prep a revert in the meantime though

@DanielMcAssey
Copy link
Contributor

So its a non-zero error, error code 1 ***.pkg: code object is not signed at all.

@erickzhao
Copy link
Member

@DanielMcAssey is the patch that you have the same as #154?

@DanielMcAssey
Copy link
Contributor

DanielMcAssey commented Nov 15, 2023

@DanielMcAssey is the patch that you have the same as #154?

See attachment, pretty much, slight variation, it specifically checks for .dmg and .pkg, instead of doing fs.stat.

NOTE: patch below also removes the sig check, temporarily.

@electron+notarize+2.2.0.patch

@dlmartens
Copy link

I'm getting the same error for my signed installer ".pkg." (code object is not signed at all)

It looks like codesign cannot verify the signature of an installer package. Without this check, my installer is able to be notarized without a problem. I can verify its signature with pkgutil --check-signature installer.pkg, which returns a non-zero exit code on verification error.

This same installer package can be notarized without issue with "@electron/notarize": "2.1.0".

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants