fix(editor): execute oxc.path.server in win32 with shell, when its the node_module binary

Author: Sysix

editors/vscode/client/PathValidator.ts

@@ -8,7 +8,7 @@
  *
  * The check for malicious characters is not needed, but it's an additional layer of security.
  * When using `shell: true` in `LanguageClient.ServerOptions`, it can be vulnerable to command injection.
- * Even though we are not using `shell: true`, it's a good practice to validate the input.
+ * We are using `shell: true` only on Windows when the paths ends with `node_modules/.bin/oxc_language_server`.
  */
 export function validateSafeBinaryPath(binary: string): boolean {
   // Check for path traversal (including Windows variants)

editors/vscode/client/extension.ts

@@ -137,8 +137,7 @@ export async function activate(context: ExtensionContext) {
     outputChannel,
   );
 
-  async function findBinary(): Promise<string> {
-    let bin = configService.getUserServerBinPath();
+  async function findBinary(bin: string | undefined): Promise<string> {
     if (workspace.isTrusted && bin) {
       try {
         await fsPromises.access(bin);
@@ -155,10 +154,20 @@ export async function activate(context: ExtensionContext) {
     );
   }
 
-  const command = await findBinary();
+  const userDefinedBinary = configService.getUserServerBinPath();
+  const command = await findBinary(userDefinedBinary);
   const run: Executable = {
     command: command!,
     options: {
+      // On Windows we need to run the binary in a shell to be able to execute the shell npm bin script.
+      // This is only needed when the user explicitly configures the binary to point to the npm bin script.
+      // The extension is shipped with the `.exe` file, we don't need to run it in a shell.
+      // Searching for the right `.exe` file inside `node_modules/` is not reliable as it depends on
+      // the package manager used (npm, yarn, pnpm, etc) and the package version.
+      // The npm bin script is a shell script that points to the actual binary.
+      // Security: We validated the userDefinedBinary in `configService.getUserServerBinPath()`.
+      shell: process.platform === 'win32' && command === userDefinedBinary &&
+        userDefinedBinary?.endsWith('node_modules\\.bin\\oxc_language_server'),
       env: {
         ...process.env,
         RUST_LOG: process.env.RUST_LOG || 'info',

editors/vscode/tests/ConfigService.spec.ts

@@ -2,6 +2,7 @@ import { strictEqual } from 'assert';
 import { workspace } from 'vscode';
 import { ConfigService } from '../client/ConfigService.js';
 import { testSingleFolderMode, WORKSPACE_FOLDER } from './test-helpers.js';
+import path = require('path/win32');
 
 const conf = workspace.getConfiguration('oxc');
 
@@ -18,20 +19,55 @@ suite('ConfigService', () => {
     await Promise.all(keys.map(key => conf.update(key, undefined)));
   });
 
-  testSingleFolderMode('resolves relative server path with workspace folder', async () => {
-    const service = new ConfigService();
-    const nonDefinedServerPath = service.getUserServerBinPath();
+  const getWorkspaceFolderPlatformSafe = () => {
+    let workspace_path = WORKSPACE_FOLDER.uri.path;
+    if (process.platform === 'win32') {
+      workspace_path = workspace_path.replaceAll('/', '\\');
+      if (workspace_path.startsWith('\\')) {
+        workspace_path = workspace_path.slice(1);
+      }
+    }
+    return workspace_path;
+  };
 
-    strictEqual(nonDefinedServerPath, undefined);
+  suite('getUserServerBinPath', () => {
+    testSingleFolderMode('resolves relative server path with workspace folder', async () => {
+      const service = new ConfigService();
+      const nonDefinedServerPath = service.getUserServerBinPath();
 
-    await conf.update('path.server', '/absolute/oxc_language_server');
-    const absoluteServerPath = service.getUserServerBinPath();
+      strictEqual(nonDefinedServerPath, undefined);
 
-    strictEqual(absoluteServerPath, '/absolute/oxc_language_server');
+      await conf.update('path.server', '/absolute/oxc_language_server');
+      const absoluteServerPath = service.getUserServerBinPath();
 
-    await conf.update('path.server', './relative/oxc_language_server');
-    const relativeServerPath = service.getUserServerBinPath();
+      strictEqual(absoluteServerPath, '/absolute/oxc_language_server');
 
-    strictEqual(relativeServerPath, WORKSPACE_FOLDER.uri.path + '/relative/oxc_language_server');
+      await conf.update('path.server', './relative/oxc_language_server');
+      const relativeServerPath = service.getUserServerBinPath();
+
+      let workspace_path = getWorkspaceFolderPlatformSafe();
+      strictEqual(relativeServerPath, `${workspace_path}/relative/oxc_language_server`);
+    });
+
+    testSingleFolderMode('returns undefined for unsafe server path', async () => {
+      const service = new ConfigService();
+      await conf.update('path.server', '../unsafe/oxc_language_server');
+      const unsafeServerPath = service.getUserServerBinPath();
+
+      strictEqual(unsafeServerPath, undefined);
+    });
+
+   testSingleFolderMode('returns backslashes path on Windows', async () => {
+      if (process.platform !== 'win32') {
+        return;
+      }
+      const service = new ConfigService();
+      await conf.update('path.server', './relative/oxc_language_server');
+      const relativeServerPath = service.getUserServerBinPath();
+      let workspace_path = getWorkspaceFolderPlatformSafe();
+
+      strictEqual(workspace_path[1], ':', 'The test workspace folder must be an absolute path with a drive letter on Windows');
+      strictEqual(relativeServerPath, `${workspace_path}\\relative\\oxc_language_server`);
+    });
   });
 });