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

fix: check notarytool response status #173

Merged
merged 2 commits into from Jan 24, 2024

Conversation

rcekanauskas
Copy link
Contributor

I tried to sign and notarize my application, but kept getting error The staple and validate action failed! Error 65.. It is not obvious why this happens and what does it mean. After some digging I found that the problem is with my certificate. I understand that this is not directly related to this package, but this package could save some time by providing additional information.

Here is the output:

  ...
  electron-notarize:notarytool zip succeeded, attempting to upload to Apple +10s
  electron-notarize:spawn spawning cmd: xcrun args: [
  'notarytool',
  'submit',
  '/var/folders/jk/arst/T/electron-notarize-QH83Xm/my-app.zip',
  '--apple-id',
  '*********',
  '--password',
  '*********',
  '--team-id',
  '*********',
  '--wait',
  '--output-format',
  'json'
] opts: {} +0ms
  electron-notarize:spawn cmd xcrun terminated with code: 0 +1m
  electron-notarize:notarytool result code 0, output: {"id":"arst-astt-1234","status":"Invalid","message":"Processing complete"} +1m // this line temporarily added by me for debugging
  electron-notarize:notarytool notarization success +0ms
  electron-notarize:helpers work succeeded +1m
  electron-notarize:staple attempting to staple app: /var/folders/jk/arst/T/electron-packager/tmp-R6J7Wo/my-app.app +0ms
  electron-notarize:spawn spawning cmd: xcrun args: [ 'stapler', 'staple', '-v', 'my-app.app' ] opts: {
  cwd: '/var/folders/jk/arst/T/electron-packager/tmp-R6J7Wo'
} +6ms
  electron-notarize:spawn cmd xcrun terminated with code: 65 +291ms

As you can see on line electron-notarize:notarytool result code 0, output, notary tool exited with code 0 which is treated as success, but the output contains status Invalid. Currently code checks only exit code and discards output, so notarization process continues even though it will definitely fail. And we and up with final error message which does not tell whats wrong and let us think that problem is with staple step:

CloudKit query for my-app.app (2/123456789456123) failed due to "Record not found".
Could not find base64 encoded ticket in response for 2/123456789456123
The staple and validate action failed! Error 65.

With provided change we get this result:

An unhandled rejection has occurred inside Forge:
Error: Failed to notarize via notarytool

{"message":"Processing complete","status":"Invalid","id":"arst-arst-1234"}

It does not explicitly tell what's wrong, but using DEBUG we can get all the information from notarization log:

  ...
  electron-notarize:spawn spawning cmd: xcrun args: [
  'notarytool',
  'log',
  'arst-arst-1234',
  '--apple-id',
  '*********',
  '--password',
  '*********',
  '--team-id',
  '*********'
] opts: {} +0ms
  electron-notarize:spawn cmd xcrun terminated with code: 0 +2s
  electron-notarize:notarytool notarization log {
  "logFormatVersion": 1,
  "jobId": "arst-arst-1234",
  "status": "Invalid",
  "statusSummary": "Archive contains critical validation errors",
  "statusCode": 4000,
  ...

Thank you.

@rcekanauskas rcekanauskas requested a review from a team as a code owner December 19, 2023 10:46
Copy link
Member

@dsanders11 dsanders11 left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! Since result.output is JSON, can we refactor this to move the JSON.parse to line 79, and change the if statement to check parsed.status !== 'Accepted'?

According to the man page for notarytool: "Only return from notarytool once the Apple notary service has responded with a status of "Accepted", "Invalid", "Rejected", or if a fatal error has occurred during submission."

So the current change in this PR wouldn't catch "Rejected", which we also want to handle.

@rcekanauskas
Copy link
Contributor Author

Thank you for your insights!

Do you think we should go even more defensive by wrapping const parsed = JSON.parse(result.output.trim()); into a try block? In theory it is possible for the output to be invalid json as we do not control notarize. What do you think?

@dsanders11
Copy link
Member

I think it's fine to let it throw an error in that case, since it would mean there's a bug and it'll be easier to debug if we let it throw directly.

@dsanders11 dsanders11 changed the title feat: add status check after notarization fix: check notarytool response status Dec 22, 2023
@ckerr
Copy link
Member

ckerr commented Jan 24, 2024

@electron/wg-ecosystem, OK to merge? This has been sitting approved + unmerged for a few weeks now.

@dsanders11 dsanders11 merged commit fa2cb22 into electron:main Jan 24, 2024
4 checks passed
Copy link

🎉 This PR is included in version 2.2.1 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

None yet

3 participants