microsoft / vscode-cpptools

Official repository for the Microsoft C/C++ extension for VS Code.
Other
5.52k stars 1.55k forks source link

Notification of breaking api change with v1.92 release of VS Code #12460

Open deepak1556 opened 3 months ago

deepak1556 commented 3 months ago

Hello from the VS Code team 👋

In our next release v1.92, we will update to Electron 30 which includes Node.js 20.14.0. This Node version contains a breaking change, in response to a CVE, which may affect you if you execute .bat or .cmd files on Windows. Based on a simple scan of your extension's source code, you may be impacted by this change. The stable VS Code that contains this update will be released in early August.

Action: please try out your extension on this month's VS Code Insiders on Windows. If you are affected by this change, you will encounter an EINVAL error when you try to spawn a bat/cmd file.

Node.js has added a section on batch file spawning to their documentation. To fix any issues:

  1. Find locations where you call child_process.spawn to execute a batch file on Windows
  2. Add shell: true or shell: process.platform === 'win32' to the options object
  3. If the batch script path may contain spaces, you will also need to wrap the path in quotation marks.

Please let us know if you run into issues or if you need clarification.

Happy coding!

Colengms commented 3 months ago

Hi @deepak1556 . Thanks for the heads-up.

If the batch script path may contain spaces, you will also need to wrap the path in quotation marks.

I'd suggest being careful about this detail when advising people about it. Correctly implementing shell command line escaping can be difficult to get right. A naive implementation is usually not correct. Some characters may need to be conditionally escaped. The rules differ between Windows and Linux/Mac. (And across shells, even potentially depending on what Windows binary is being executed, as parsing is done by the runtime of the target - usually VC runtime).

I can provide more resources on this, if interested.

https://learn.microsoft.com/en-us/archive/blogs/twistylittlepassagesallalike/everyone-quotes-command-line-arguments-the-wrong-way

https://daviddeley.com/autohotkey/parameters/parameters.htm

Colengms commented 3 months ago

It looks like most of our calls to spawn or spawnSync already use shell:true. The rest appear to be in test code or code paths that aren't used.

Code that may need attention, which calls spawn or spawnSync:

Extension\src\Utility\Process\process.ts

Extension\.scripts\code.ts
Extension\.scripts\test.ts
Extension\src\Utility\Process\commandLine.ts

(in comments)

Extension\.scripts\vscode.ts