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

Finish support for notarization #3926

Closed
wants to merge 3 commits into from

Conversation

Kilian
Copy link
Contributor

@Kilian Kilian commented May 27, 2019

As per #3870, this PR adds two new options:

*gatekeeperAssess in macOptions (defaults to true) Allows people to disable the gatekeeper assessment while osx-sign fails on that (see electron/osx-sign#196)
*sign in dmgOptions (defaults to false) signing the dmg does not appear to be needed and actually prevents people from opening the dmg in 10.14.5, see #3870.

Together with the afterSign hook where people can call electron-notarize, these options let electron-builder user successfully create a notarized application in a DMG that does not show warning messages.

@Kilian
Copy link
Contributor Author

Kilian commented May 29, 2019

@develar Can you take a look at this? I don't really see why the appVeyor or travis builds would fail on this (failures seem unrelated too?)

@wagslane
Copy link

Looks like some dependency errors:

 TypeError: cb.apply is not a function
      at node_modules/graceful-fs/polyfills.js:285:20

https://ci.appveyor.com/project/stefanjudis/electron-builder/builds/24832175#L244

I would guess some updates are in order?

@wagslane
Copy link

wagslane commented Jun 1, 2019

I also just tried testing sections of this PR against travis and appveyor.. I have no idea why these errors are being thrown. Although they look like they are linked to fs-extra-p @develar

@wagslane
Copy link

wagslane commented Jun 2, 2019

Idk if this will be taken care of, but @Kilian I copied the "out" folder from app-builder-lib and dmg-builder in order to get this working for myself because its taking so long to get this merged.

I noticed that there is a bug in app-builder-lib where it is saying gatekeeperAssess is not a valid configuration option (on your branch). For myself I hard coded it but this may be a problem here.

@Kilian
Copy link
Contributor Author

Kilian commented Jun 4, 2019

@lane-c-wagner did you put gatekeeperAssess in the mac configuration or in the dmg configuration? It should go in the former.

@develar develar closed this Jun 4, 2019
@guanxun14
Copy link

Thank you @develar for the commit. As I understand the situation, this commit effectively solves the notarization situation as far as electron-build is concerned. May I just check if you will be tagging this as a release, or might there be more needed before it's ready? Thanks again! :)

@develar
Copy link
Member

develar commented Jun 4, 2019

20.43.0 released.

@raymondjacobson
Copy link

After bumping the version and trying this out, I'm seeing
Exception Type: EXC_BAD_ACCESS (Code Signature Invalid)

when trying to open the App. The actual disk image mounts fine.

codesign --verify -vvvv /Volumes/App/App.app

gives me

--prepared:/Volumes/App/App.app/Contents/Frameworks/App Helper.app
--validated:/Volumes/App/App.app/Contents/Frameworks/App Helper.app
--prepared:/Volumes/App/App.app/Contents/Frameworks/Electron Framework.framework/Versions/Current/.
--validated:/Volumes/App/App.app/Contents/Frameworks/Electron Framework.framework/Versions/Current/.
--prepared:/Volumes/App/App.app/Contents/Frameworks/Mantle.framework/Versions/Current/.
--validated:/Volumes/App/App.app/Contents/Frameworks/Mantle.framework/Versions/Current/.
--prepared:/Volumes/App/App.app/Contents/Frameworks/ReactiveCocoa.framework/Versions/Current/.
--validated:/Volumes/App/App.app/Contents/Frameworks/ReactiveCocoa.framework/Versions/Current/.
--prepared:/Volumes/App/App.app/Contents/Frameworks/Squirrel.framework/Versions/Current/.
--validated:/Volumes/App/App.app/Contents/Frameworks/Squirrel.framework/Versions/Current/.
/Volumes/App/App.app: valid on disk
/Volumes/App/App.app: satisfies its Designated Requirement

And
spctl -a -v /Volumes/App/App.app

gives

/Volumes/App/App.app: accepted
source=Notarized Developer ID

Has anyone seen this?

@wagslane
Copy link

wagslane commented Jun 5, 2019 via email

@raymondjacobson
Copy link

Looks that that was probably the issue :) Particularly this one, I think?
<key>com.apple.security.cs.allow-unsigned-executable-memory</key>

thx Lane!

@itsthisjustin
Copy link

itsthisjustin commented Jul 11, 2019

@raymondjacobson We only have 1 single entitlement, com.apple.security.files.user-selected.read-write and getting this on our app. Is there anything else that would cause it?

UPDATE:

Added an obscene amount of entitlements and now I don't get the signing crash, but our app exits on launch.

@raymondjacobson
Copy link

raymondjacobson commented Jul 12, 2019

@itsthisjustin Sorry, just saw this message. FWIW, this is what I've done

You may not need all of these.


    <key>com.apple.security.network.client</key>

    <true/>

    <key>com.apple.security.files.user-selected.read-only</key>

    <true/>

    <key>com.apple.security.files.user-selected.read-write</key>

    <true/>

    <key>com.apple.security.cs.allow-jit</key>

    <true/>

    <key>com.apple.security.cs.allow-unsigned-executable-memory</key>

    <true/>

    <key>com.apple.security.cs.allow-dyld-environment-variables</key>

    <true/>

@itsthisjustin
Copy link

@raymondjacobson Thanks. I finally got it working. Notarization has unfortunately caused a new issue that I'm wondering if you or @Kilian have come across.

Node-notifier has this weird issue where it comes default with the terminal icon and you have to manually copy your app's icon over top it in the node_modules folder. This has been working fine for us and the way we do it is by copying over a new info.plist and the icon so it shows as fresh and then unasar-ing it in the build command.

So, now with notarization, Apple gives us back this error:

The binary is not signed with a valid Developer ID certificate
with a link to the node notifier package.

Any tips on how to have my cake and eat it too?

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.

6 participants