GeezerVison

The World Wide Web, restored to 800 pixels of glory.

GeezerVison is showing github.com — doc: add steps about signing the binary in single-executable docs by RaisinTen · Pull Request #46764 · nodejs/node

Conversation

RaisinTen commented Feb 22, 2023

Copy link

Copy Markdown

Member

We didn't catch this in #45038 because the binary wasn't signed by default unlike the official Node.js binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75

cc @nodejs/single-executable @targos @ShenHongFei

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ovflowd reacted with thumbs up emoji

All reactions

👍 1 reaction


          doc: add steps about signing the binary in single-executable docs

ce1284b

We didn't catch this in nodejs#45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <[email protected]>

nodejs-github-bot added the doc Issues and PRs related to Node.js documentation. label

Feb 22, 2023

This was referenced Feb 22, 2023

SEA created following Node.js documentation doesn't work nodejs/postject#76

Closed

Weired output message The signature seems corrupted! on Windows nodejs/postject#75

Closed

targos approved these changes

Feb 22, 2023

View reviewed changes

targos left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM for the macOS part.

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

RaisinTen commented Feb 22, 2023

Copy link

Copy Markdown

Member Author

@nodejs/platform-windows can someone please review the Windows part?

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

tony-go commented Feb 22, 2023

Copy link

Copy Markdown

Member

Let me start my windows machine ^^

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

cjihrig approved these changes

Feb 22, 2023

View reviewed changes

RaisinTen added the author ready PRs with CI started, the required approvals, and no outstanding review comments. label

Feb 22, 2023

tony-go approved these changes

Feb 22, 2023

View reviewed changes

tony-go left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM for mac

Not able to test on my windows machine due to some reasons :/

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

aymen94 approved these changes

Feb 22, 2023

View reviewed changes

RaisinTen added the commit-queue PRs queued for automated landing through the Commit Queue. label

Feb 23, 2023

ShenHongFei commented Feb 23, 2023

Copy link

Copy Markdown

Contributor

@RaisinTen
For windows part, I tried it according to the process in the document, and I needed to change hello to hello.exe (executable programs on windows need a .exe suffix to run).

I executed the following command in powershell:

$ echo 'console.log(`Hello, ${process.argv[2]}!`);' > hello.js
$ cp e:/sdk/nodejs/node.exe hello.exe
$ &"C:/Program Files (x86)/Windows Kits/10/bin/10.0.22621.0/x64/signtool.exe" remove /s hello.exe
$ npx postject hello.exe NODE_JS_CODE hello.js --sentinel-fuse NODE_JS_FUSE_fce680ab2cc467b6e072b8b5df1996b2
$ &"C:/Program Files (x86)/Windows Kits/10/bin/10.0.22621.0/x64/signtool.exe" sign /fd SHA256 hello.exe

The last step went wrong:
SignTool Error: No certificates were found that met all the given criteria.

It may be that I don't have a local certificate that can be used for digital signatures, but the generated unsigned hello.exe can also run normally

Using signtools (https://learn.microsoft.com/en-us/windows/win32/seccrypto/signtool) in Windows system to remove node.exe signature, and to sign the final generated hello.exe, is not Required steps, and users need to manually install windows sdk (https://developer.microsoft.com/en-us/windows/downloads/windows-sdk/)

I think it would be more convenient to inform users that they can ignore the warnings output by postject, or make the method of deleting and re-signing as an optional step, explaining how to obtain and use signtools to sign.

ranajitphadatare-eaton reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

mhdawson approved these changes

Feb 23, 2023

View reviewed changes

mhdawson left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

LGTM

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

jasnell approved these changes

Feb 24, 2023

View reviewed changes

RaisinTen removed the commit-queue PRs queued for automated landing through the Commit Queue. label

Feb 24, 2023


          doc: clarify that signing is optional on Windows

70effbb

Refs: nodejs#46764 (comment)
Signed-off-by: Darshan Sen <[email protected]>

RaisinTen added the commit-queue-squash PRs the Commit Queue should land as one squashed commit. label

Feb 24, 2023

RaisinTen commented Feb 24, 2023

Copy link

Copy Markdown

Member Author

@ShenHongFei updated the docs to clarify that. PTAL reviewers.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

ovflowd approved these changes

Feb 24, 2023

View reviewed changes

ovflowd left a comment

Copy link

Copy Markdown

Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Docs seem legit for me for both Windows and macOS :)

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

ShenHongFei commented Feb 26, 2023

Copy link

Copy Markdown

Contributor

@ShenHongFei updated the docs to clarify that. PTAL reviewers.

LGTM 😄

RaisinTen reacted with thumbs up emoji

All reactions

👍 1 reaction

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

RaisinTen added a commit that referenced this pull request

Feb 26, 2023


          doc: add steps about signing the binary in single-executable docs

4cde39e

We didn't catch this in #45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: #46764
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>

RaisinTen commented Feb 26, 2023

Copy link

Copy Markdown

Member Author

Landed in 4cde39e

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

RaisinTen closed this

Feb 26, 2023

RaisinTen deleted the doc-add-steps-about-signing-the-binary-in-single-executable-docs branch

February 26, 2023 06:59

targos pushed a commit that referenced this pull request

Mar 13, 2023


          doc: add steps about signing the binary in single-executable docs

33b5c3e

We didn't catch this in #45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: #46764
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>

targos pushed a commit that referenced this pull request

Mar 14, 2023


          doc: add steps about signing the binary in single-executable docs

48cf984

We didn't catch this in #45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: #46764
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>

targos mentioned this pull request

Mar 14, 2023

v19.8.0 release proposal #47087

Merged

danielleadams added the backport-blocked-v18.x label

Apr 3, 2023

danielleadams commented Apr 3, 2023

Copy link

Copy Markdown

Contributor

Blocked by #45038

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

RaisinTen added the single-executable Issues and PRs related to single-executable applications. label

May 5, 2023

targos pushed a commit that referenced this pull request

Nov 10, 2023


          doc: add steps about signing the binary in single-executable docs

15280fb

We didn't catch this in #45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: #46764
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>

targos mentioned this pull request

Nov 28, 2023

v18.19.0 release proposal #50953

Merged

sercher added a commit to sercher/graaljs that referenced this pull request

Apr 25, 2024


          doc: add steps about signing the binary in single-executable

f1c9eaf

We didn't catch this in nodejs/node#45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs/node#46764
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>

sercher added a commit to sercher/graaljs that referenced this pull request

Apr 25, 2024


          doc: add steps about signing the binary in single-executable

9f44b0c

We didn't catch this in nodejs/node#45038
because the binary wasn't signed by default unlike the official Node.js
binary, which is signed by the Node.js Foundation identity by default.

Refs: nodejs/postject#76 (macOS arm64 part only)
Fixes: nodejs/postject#75
Signed-off-by: Darshan Sen <[email protected]>
PR-URL: nodejs/node#46764
Reviewed-By: Michaël Zasso <[email protected]>
Reviewed-By: Colin Ihrig <[email protected]>
Reviewed-By: Michael Dawson <[email protected]>
Reviewed-By: James M Snell <[email protected]>

Reviewers

jasnell

jasnell approved these changes

cjihrig

cjihrig approved these changes

ovflowd

ovflowd approved these changes

+4 more reviewers

targos

targos approved these changes

mhdawson

mhdawson approved these changes

aymen94

aymen94 approved these changes

tony-go

tony-go approved these changes

Reviewers whose approvals may not affect merge requirements

Uh oh!

There was an error while loading. Please reload this page.

Assignees

No one assigned

Labels

author ready

PRs with CI started, the required approvals, and no outstanding review comments.

commit-queue-squash

PRs the Commit Queue should land as one squashed commit.

doc

Issues and PRs related to Node.js documentation.

single-executable

Issues and PRs related to single-executable applications.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Weired output message The signature seems corrupted! on Windows

Uh oh!

There was an error while loading. Please reload this page.

11 participants

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.

You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.