-
Notifications
You must be signed in to change notification settings - Fork 68
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(NODE-6265): add Spectre Mitigation and CFG #190
fix(NODE-6265): add Spectre Mitigation and CFG #190
Conversation
There is an existing patch(es) for this commit SHA: Please note that the status that is posted is not in the context of this PR but rather the (latest) existing patch and that may affect some tests that may depend on the particular PR. If your tests do not rely on any PR-specific values (like base or head branch name) then your tests will report the same status. If you would like a patch to run in the context of this PR and abort the other(s), comment 'evergreen retry'. |
evergreen retry |
There is an existing patch(es) for this commit SHA that will be aborted: The status reported will be corresponding to this PR rather than the previous existing ones. If you would like a patch to run for another PR and to abort this one, comment 'evergreen retry' on the corresponding PR. |
Hi @rzhao271 thanks for the help here. The |
The libraries are only required during the build step, when node-gyp calls into MSVC to build the C++ files. |
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
Description
What is changing?
This PR adds Spectre mitigation and control flow guards to the generated binaries by adding those flags to the binding.gyp file.
Is there new documentation needed for these changes?
Potentially. Enabling Spectre mitigation requires that package maintainers and contributors install Spectre-mitigated libraries for Visual Studio, assuming that Windows developers need to run
node-gyp rebuild
. In my case, it seems that the prebuild-install script runs and skips over the rebuild command. Non-Windows OSes should not be affected by this PR in general.What is the motivation for this change?
BinSkim (a Microsoft binary analyzer) has identified that kerberos.node is missing Spectre mitigation and control flow guard flags. This issue affects the compliance of Visual Studio Code downstream along with the general security of the kerberos.node binary.
Release Highlight
Double check the following
npm run check:lint
scripttype(NODE-xxxx)[!]: description
feat(NODE-1234)!: rewriting everything in coffeescript