chore: add BinSkim flags to winpty #767
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
deepak1556
reviewed
Mar 13, 2025
deepak1556
approved these changes
Mar 14, 2025
NorthernMan54
added a commit
to homebridge/node-pty-prebuilt-multiarch
that referenced
this pull request
Jun 9, 2025
* Port to NAPI (microsoft#644) * Port to NAPI The "5th pty bug" in microsoft#432 fixed also. * Fix help message in pty.cc * Move NAPI deps to devDependencies in package.json * Apply most of deepak1556's suggestions * Fix winpty * Fix conpty missing CloseHandle * Use unique_ptr to avoid `goto`s * Why macos failed? * fix: ci and minor cleanups * fix build failed on windows --------- Co-authored-by: deepak1556 <hop2deep@gmail.com> * build(deps): bump ip from 2.0.0 to 2.0.1 Bumps [ip](https://github.com/indutny/node-ip) from 2.0.0 to 2.0.1. - [Commits](indutny/node-ip@v2.0.0...v2.0.1) --- updated-dependencies: - dependency-name: ip dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * api scan * fix job name * chore: fix APIScan software name (microsoft#667) * fix: comment out invalid API call (microsoft#669) * fix: assertion on node environment shutdown (microsoft#672) * Upgrade node-gyp to fix microsoft#643 and microsoft#646 (microsoft#673) * chore: remove deprecated api `process.binding` (microsoft#653) * Remove deprecated API `process.binding` Originally designed to work with ancient node.js 0.12 and io.js * Fix node `net.Socket` limitations nodejs/node#37780 * chore: migrate pipeline to use 1ES template (microsoft#676) * build(deps): bump tar from 6.2.0 to 6.2.1 Bumps [tar](https://github.com/isaacs/node-tar) from 6.2.0 to 6.2.1. - [Release notes](https://github.com/isaacs/node-tar/releases) - [Changelog](https://github.com/isaacs/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v6.2.0...v6.2.1) --- updated-dependencies: - dependency-name: tar dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * docs: add a note on Spectre-mitigated libraries (microsoft#679) * build(deps): bump braces from 3.0.2 to 3.0.3 Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * Include termios.h on freebsd * Get conpty.dll conditionally loading * Add conpty dlls and node headers * Add useConptyDll option * Copy conpty.dll and openconsole.exe depending on arch in postinstall * Ship third_party folder with module This needs to be there to copy over the right binary on postinstall Part of microsoft/vscode#224488 * Don't run CI for node 16 * Disable APIScan task * Fix winpty resize and reduce test flakiness * Publish third_party folder * Improve can't find conpty.dll exception message Part of microsoft/vscode#224488 * Get path of conpty.node for conpty.dll Part of microsoft/vscode#224488 * Update to newer version of conpty * Fix correct method being loaded when not using conptydll Part of microsoft/vscode#224488 * Copy right dll/exe based on npm_config_arch Part of microsoft/vscode#224488 Part of microsoft/vscode#225355 * Bring back clear impl Fixes microsoft#711 * Close piClient.hThread handle Fixes microsoft#717 * Move buffer_ to heap Resolves warning: src\win\path_util.cc(54): warning C6262: Function uses '131804' bytes of stack: exceeds /analyze:stacksize '16384'. Consider moving some data to heap. * Catch possible exceptions in ~Agent Fixes warning: deps\winpty\src\agent\Agent.cc(231): warning C4722: 'Agent::~Agent': destructor never returns, potential memory leak * yarn.lock -> package-lock.json Fixes microsoft#712 * Regenerate dependencies * Fix typo in trace log Follow up microsoft#720 * Move to compile commands on postinstall for fixing intellisense Fixes microsoft#707 * chore: use node 20 and fix build (microsoft#730) * chore: use node 20 and fix build * chore: bump macOS deployment target * spec: reduce flakyness * Revert "chore: bump macOS deployment target" This reverts commit d9d18b3. --------- Co-authored-by: deepak1556 <hop2deep@gmail.com> * chore: add new publish pipeline (microsoft#727) Also allows the CI pipeline to use the Unofficial template * fix: ensure proper cleanup of console process on shutdown * fix: publish pipeline broken APIScan task (microsoft#756) * chore: improve cleanup with useConptyDll mode * chore: update tests * chore: auto-publish beta versions (microsoft#757) * chore: work around blocked release integration (microsoft#758) * Update to conpty v1.22 (microsoft#759) * Update to conpty v1.22 Fixes microsoft#490 * Add logs to show the problem happening * spec: increase exit delay * refactor: input and output handling with conpty * Close the input read and output write handles after creating the client process * Call ReleasePseudoConsole after creating the client process which will cause the output read handle to close when there is no more data from the session * For manual termination via Kill, we close the input write handle and call into ClosePseudoConsole, we should then drain the output handle NB: ideally draining the output handle should have been enough to cause the client process to close but it doesn't work, we call TerminateProcess to fix this case. * chore: restore legacy conpty path --------- Co-authored-by: deepak1556 <hop2deep@gmail.com> * chore: remove old publishing stage (microsoft#761) * fix: restore conpty non-dll path (microsoft#766) * chore: add BinSkim flags to winpty (microsoft#767) * chore: add BinSkim flags to winpty * Apply PR feedback * conpty@1.22.250204002 * Change buffer size to 128KiB Fixes microsoft#765 * chore: match trigger with perf-bot's (microsoft#773) * chore: match exclude with latest conpty (microsoft#774) * chore: use folder wildcard (microsoft#775) * Revert "conpty@1.22.250204002" This reverts commit 247ae7d. * fix: gate conpty-exclusive call behind conpty check (microsoft#778) * NodeJS-24 * Node 20 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: kkocdko <31189892+kkocdko@users.noreply.github.com> Co-authored-by: deepak1556 <hop2deep@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Co-authored-by: SteVen Batten <sbatten@microsoft.com> Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Co-authored-by: jpcastberg <jpcastberg@gmail.com>
NorthernMan54
added a commit
to homebridge/node-pty-prebuilt-multiarch
that referenced
this pull request
Jul 2, 2025
* upgrade conpty from 1.20 to 1.22 (#56) Signed-off-by: Chapman Pendery <cpendery@microsoft.com> * Update package.json * Node js 24 (#58) * Port to NAPI (microsoft#644) * Port to NAPI The "5th pty bug" in microsoft#432 fixed also. * Fix help message in pty.cc * Move NAPI deps to devDependencies in package.json * Apply most of deepak1556's suggestions * Fix winpty * Fix conpty missing CloseHandle * Use unique_ptr to avoid `goto`s * Why macos failed? * fix: ci and minor cleanups * fix build failed on windows --------- Co-authored-by: deepak1556 <hop2deep@gmail.com> * build(deps): bump ip from 2.0.0 to 2.0.1 Bumps [ip](https://github.com/indutny/node-ip) from 2.0.0 to 2.0.1. - [Commits](indutny/node-ip@v2.0.0...v2.0.1) --- updated-dependencies: - dependency-name: ip dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * api scan * fix job name * chore: fix APIScan software name (microsoft#667) * fix: comment out invalid API call (microsoft#669) * fix: assertion on node environment shutdown (microsoft#672) * Upgrade node-gyp to fix microsoft#643 and microsoft#646 (microsoft#673) * chore: remove deprecated api `process.binding` (microsoft#653) * Remove deprecated API `process.binding` Originally designed to work with ancient node.js 0.12 and io.js * Fix node `net.Socket` limitations nodejs/node#37780 * chore: migrate pipeline to use 1ES template (microsoft#676) * build(deps): bump tar from 6.2.0 to 6.2.1 Bumps [tar](https://github.com/isaacs/node-tar) from 6.2.0 to 6.2.1. - [Release notes](https://github.com/isaacs/node-tar/releases) - [Changelog](https://github.com/isaacs/node-tar/blob/main/CHANGELOG.md) - [Commits](isaacs/node-tar@v6.2.0...v6.2.1) --- updated-dependencies: - dependency-name: tar dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * docs: add a note on Spectre-mitigated libraries (microsoft#679) * build(deps): bump braces from 3.0.2 to 3.0.3 Bumps [braces](https://github.com/micromatch/braces) from 3.0.2 to 3.0.3. - [Changelog](https://github.com/micromatch/braces/blob/master/CHANGELOG.md) - [Commits](micromatch/braces@3.0.2...3.0.3) --- updated-dependencies: - dependency-name: braces dependency-type: indirect ... Signed-off-by: dependabot[bot] <support@github.com> * Include termios.h on freebsd * Get conpty.dll conditionally loading * Add conpty dlls and node headers * Add useConptyDll option * Copy conpty.dll and openconsole.exe depending on arch in postinstall * Ship third_party folder with module This needs to be there to copy over the right binary on postinstall Part of microsoft/vscode#224488 * Don't run CI for node 16 * Disable APIScan task * Fix winpty resize and reduce test flakiness * Publish third_party folder * Improve can't find conpty.dll exception message Part of microsoft/vscode#224488 * Get path of conpty.node for conpty.dll Part of microsoft/vscode#224488 * Update to newer version of conpty * Fix correct method being loaded when not using conptydll Part of microsoft/vscode#224488 * Copy right dll/exe based on npm_config_arch Part of microsoft/vscode#224488 Part of microsoft/vscode#225355 * Bring back clear impl Fixes microsoft#711 * Close piClient.hThread handle Fixes microsoft#717 * Move buffer_ to heap Resolves warning: src\win\path_util.cc(54): warning C6262: Function uses '131804' bytes of stack: exceeds /analyze:stacksize '16384'. Consider moving some data to heap. * Catch possible exceptions in ~Agent Fixes warning: deps\winpty\src\agent\Agent.cc(231): warning C4722: 'Agent::~Agent': destructor never returns, potential memory leak * yarn.lock -> package-lock.json Fixes microsoft#712 * Regenerate dependencies * Fix typo in trace log Follow up microsoft#720 * Move to compile commands on postinstall for fixing intellisense Fixes microsoft#707 * chore: use node 20 and fix build (microsoft#730) * chore: use node 20 and fix build * chore: bump macOS deployment target * spec: reduce flakyness * Revert "chore: bump macOS deployment target" This reverts commit d9d18b3. --------- Co-authored-by: deepak1556 <hop2deep@gmail.com> * chore: add new publish pipeline (microsoft#727) Also allows the CI pipeline to use the Unofficial template * fix: ensure proper cleanup of console process on shutdown * fix: publish pipeline broken APIScan task (microsoft#756) * chore: improve cleanup with useConptyDll mode * chore: update tests * chore: auto-publish beta versions (microsoft#757) * chore: work around blocked release integration (microsoft#758) * Update to conpty v1.22 (microsoft#759) * Update to conpty v1.22 Fixes microsoft#490 * Add logs to show the problem happening * spec: increase exit delay * refactor: input and output handling with conpty * Close the input read and output write handles after creating the client process * Call ReleasePseudoConsole after creating the client process which will cause the output read handle to close when there is no more data from the session * For manual termination via Kill, we close the input write handle and call into ClosePseudoConsole, we should then drain the output handle NB: ideally draining the output handle should have been enough to cause the client process to close but it doesn't work, we call TerminateProcess to fix this case. * chore: restore legacy conpty path --------- Co-authored-by: deepak1556 <hop2deep@gmail.com> * chore: remove old publishing stage (microsoft#761) * fix: restore conpty non-dll path (microsoft#766) * chore: add BinSkim flags to winpty (microsoft#767) * chore: add BinSkim flags to winpty * Apply PR feedback * conpty@1.22.250204002 * Change buffer size to 128KiB Fixes microsoft#765 * chore: match trigger with perf-bot's (microsoft#773) * chore: match exclude with latest conpty (microsoft#774) * chore: use folder wildcard (microsoft#775) * Revert "conpty@1.22.250204002" This reverts commit 247ae7d. * fix: gate conpty-exclusive call behind conpty check (microsoft#778) * NodeJS-24 * Node 20 --------- Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: kkocdko <31189892+kkocdko@users.noreply.github.com> Co-authored-by: deepak1556 <hop2deep@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Co-authored-by: SteVen Batten <sbatten@microsoft.com> Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Co-authored-by: jpcastberg <jpcastberg@gmail.com> * Working * Update validate for NodeJS 24 * Should now be working * Update Stage 2 - Validate.yml * Update Prepare prebuild environment.yml * Update Dockerfile.debian * Fix ? * Add git * Fix ? * Fixed * Update package-lock.json * Fix * Try again * Again * Update Stage 2 - Validate.yml * Windows fix * fix * Update updateABIRegistry.js * Update engines to match prebuilds --------- Signed-off-by: Chapman Pendery <cpendery@microsoft.com> Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: Chapman Pendery <35637443+cpendery@users.noreply.github.com> Co-authored-by: kkocdko <31189892+kkocdko@users.noreply.github.com> Co-authored-by: deepak1556 <hop2deep@gmail.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com> Co-authored-by: Daniel Imms <2193314+Tyriar@users.noreply.github.com> Co-authored-by: SteVen Batten <sbatten@microsoft.com> Co-authored-by: Raymond Zhao <7199958+rzhao271@users.noreply.github.com> Co-authored-by: jpcastberg <jpcastberg@gmail.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
This PR adds BinSkim flags to node-pty's winpty executables.
static_casteverywhere to turn the implicit conversions into explicit ones.This PR also adds Spectre mitigation to the executables.