Merge pull request #395 from sass/child-process-deprecation

nex3 · web-flow · commit 3afea5cdd180 · 2025-10-31T07:38:14.000-07:00
Fix Node.js deprecation warnings about passing args to child_process
diff --git a/bin/sass.ts b/bin/sass.ts
@@ -2,25 +2,31 @@
 
 import * as child_process from 'child_process';
 import * as path from 'path';
+
 import {compilerCommand} from '../lib/src/compiler-path';
 
 // TODO npm/cmd-shim#152 and yarnpkg/berry#6422 - If and when the package
 // managers support it, we should make this a proper shell script rather than a
 // JS wrapper.
 
 try {
-  child_process.execFileSync(
-    compilerCommand[0],
-    [...compilerCommand.slice(1), ...process.argv.slice(2)],
-    {
-      // Node blocks launching .bat and .cmd without a shell due to CVE-2024-27980
-      shell: ['.bat', '.cmd'].includes(
-        path.extname(compilerCommand[0]).toLowerCase(),
-      ),
-      stdio: 'inherit',
-      windowsHide: true,
-    },
-  );
+  let command = compilerCommand[0];
+  let args = [...compilerCommand.slice(1), ...process.argv.slice(2)];
+  const options: child_process.ExecFileSyncOptions = {
+    stdio: 'inherit',
+    windowsHide: true,
+  };
+
+  // Node forbids launching .bat and .cmd without a shell due to CVE-2024-27980,
+  // and DEP0190 forbids passing an argument list *with* shell: true. To work
+  // around this, we have to manually concatenate the arguments.
+  if (['.bat', '.cmd'].includes(path.extname(command).toLowerCase())) {
+    command = `${command} ${args.join(' ')}`;
+    args = [];
+    options.shell = true;
+  }
+
+  child_process.execFileSync(command, args, options);
 } catch (error) {
   if (error.code) {
     throw error;
diff --git a/lib/index.mjs b/lib/index.mjs
@@ -47,7 +47,7 @@ function defaultExportDeprecation() {
   printedDefaultExportDeprecation = true;
   console.error(
     "`import sass from 'sass'` is deprecated.\n" +
-      "Please use `import * as sass from 'sass'` instead."
+      "Please use `import * as sass from 'sass'` instead.",
   );
 }
 
diff --git a/lib/src/compiler/async.ts b/lib/src/compiler/async.ts
@@ -2,7 +2,8 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
-import {spawn} from 'child_process';
+import * as child_process from 'child_process';
+
 import {Observable} from 'rxjs';
 import {takeUntil} from 'rxjs/operators';
 
@@ -36,21 +37,28 @@ const initFlag = Symbol();
 /** An asynchronous wrapper for the embedded Sass compiler */
 export class AsyncCompiler {
   /** The underlying process that's being wrapped. */
-  private readonly process = spawn(
-    compilerCommand[0],
-    [...compilerCommand.slice(1), '--embedded'],
-    {
+  private readonly process = (() => {
+    let command = compilerCommand[0];
+    let args = [...compilerCommand.slice(1), '--embedded'];
+    const options: child_process.SpawnOptions = {
       // Use the command's cwd so the compiler survives the removal of the
       // current working directory.
       // https://github.com/sass/embedded-host-node/pull/261#discussion_r1438712923
       cwd: path.dirname(compilerCommand[0]),
-      // Node blocks launching .bat and .cmd without a shell due to CVE-2024-27980
-      shell: ['.bat', '.cmd'].includes(
-        path.extname(compilerCommand[0]).toLowerCase(),
-      ),
       windowsHide: true,
-    },
-  );
+    };
+
+    // Node forbids launching .bat and .cmd without a shell due to CVE-2024-27980,
+    // and DEP0190 forbids passing an argument list *with* shell: true. To work
+    // around this, we have to manually concatenate the arguments.
+    if (['.bat', '.cmd'].includes(path.extname(command).toLowerCase())) {
+      command = `${command} ${args!.join(' ')}`;
+      args = [];
+      options.shell = true;
+    }
+
+    return child_process.spawn(command, args, options);
+  })();
 
   /** The next compilation ID. */
   private compilationId = 1;
@@ -73,17 +81,17 @@ export class AsyncCompiler {
 
   /** The buffers emitted by the child process's stdout. */
   private readonly stdout$ = new Observable<Buffer>(observer => {
-    this.process.stdout.on('data', buffer => observer.next(buffer));
+    this.process.stdout!.on('data', buffer => observer.next(buffer));
   }).pipe(takeUntil(this.exit$));
 
   /** The buffers emitted by the child process's stderr. */
   private readonly stderr$ = new Observable<Buffer>(observer => {
-    this.process.stderr.on('data', buffer => observer.next(buffer));
+    this.process.stderr!.on('data', buffer => observer.next(buffer));
   }).pipe(takeUntil(this.exit$));
 
   /** Writes `buffer` to the child process's stdin. */
   private writeStdin(buffer: Buffer): void {
-    this.process.stdin.write(buffer);
+    this.process.stdin!.write(buffer);
   }
 
   /** Guards against using a disposed compiler. */
@@ -190,7 +198,7 @@ export class AsyncCompiler {
   async dispose(): Promise<void> {
     this.disposed = true;
     await Promise.all(this.compilations);
-    this.process.stdin.end();
+    this.process.stdin!.end();
     await this.exit$;
   }
 }
diff --git a/lib/src/compiler/sync.ts b/lib/src/compiler/sync.ts
@@ -2,10 +2,11 @@
 // MIT-style license that can be found in the LICENSE file or at
 // https://opensource.org/licenses/MIT.
 
+import * as path from 'path';
+
 import {Subject} from 'rxjs';
-import {SyncChildProcess} from 'sync-child-process';
+import * as sync_child_process from 'sync-child-process';
 
-import * as path from 'path';
 import {
   OptionsWithLegacy,
   createDispatcher,
@@ -36,21 +37,28 @@ const initFlag = Symbol();
 /** A synchronous wrapper for the embedded Sass compiler */
 export class Compiler {
   /** The underlying process that's being wrapped. */
-  private readonly process = new SyncChildProcess(
-    compilerCommand[0],
-    [...compilerCommand.slice(1), '--embedded'],
-    {
+  private readonly process = (() => {
+    let command = compilerCommand[0];
+    let args = [...compilerCommand.slice(1), '--embedded'];
+    const options: sync_child_process.Options = {
       // Use the command's cwd so the compiler survives the removal of the
       // current working directory.
       // https://github.com/sass/embedded-host-node/pull/261#discussion_r1438712923
       cwd: path.dirname(compilerCommand[0]),
-      // Node blocks launching .bat and .cmd without a shell due to CVE-2024-27980
-      shell: ['.bat', '.cmd'].includes(
-        path.extname(compilerCommand[0]).toLowerCase(),
-      ),
       windowsHide: true,
-    },
-  );
+    };
+
+    // Node forbids launching .bat and .cmd without a shell due to CVE-2024-27980,
+    // and DEP0190 forbids passing an argument list *with* shell: true. To work
+    // around this, we have to manually concatenate the arguments.
+    if (['.bat', '.cmd'].includes(path.extname(command).toLowerCase())) {
+      command = `${command} ${args!.join(' ')}`;
+      args = [];
+      options.shell = true;
+    }
+
+    return new sync_child_process.SyncChildProcess(command, args, options);
+  })();
 
   /** The next compilation ID. */
   private compilationId = 1;
diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "sass-embedded",
-  "version": "1.93.2",
+  "version": "1.93.3-dev",
   "protocol-version": "3.2.0",
   "compiler-version": "1.93.2",
   "description": "Node.js library that communicates with Embedded Dart Sass using the Embedded Sass protocol",
@@ -28,12 +28,15 @@
   },
   "scripts": {
     "init": "ts-node ./tool/init.ts",
-    "check": "npm-run-all check:gts check:tsc",
+    "check": "npm-run-all check:gts check:tsc check:mjs",
     "check:gts": "gts check",
     "check:tsc": "tsc --noEmit",
+    "check:mjs": "prettier -0check **/*.mjs",
     "clean": "gts clean",
     "compile": "tsc -p tsconfig.build.json && cp lib/index.mjs dist/lib/index.mjs && cp -r lib/src/vendor/sass/ dist/lib/src/vendor/sass && cp dist/lib/src/vendor/sass/index.d.ts dist/lib/src/vendor/sass/index.m.d.ts",
-    "fix": "gts fix",
+    "fix": "npm-run-all fix:ts fix:mjs",
+    "fix:ts": "gts fix",
+    "fix:mjs": "prettier --write **/*.mjs",
     "prepublishOnly": "npm run clean && ts-node ./tool/prepare-release.ts",
     "test": "jest"
   },
diff --git a/test/after-compile-test.mjs b/test/after-compile-test.mjs
@@ -18,7 +18,13 @@ const cjs = await import('../dist/lib/index.js');
 const esm = await import('../dist/lib/index.mjs');
 
 for (const [name, value] of Object.entries(cjs)) {
-  if (name === '__esModule' || name === 'default') continue;
+  if (
+    name === '__esModule' ||
+    name === 'default' ||
+    name === 'module.exports'
+  ) {
+    continue;
+  }
   if (!esm[name]) {
     throw new Error(`ESM module is missing export ${name}.`);
   } else if (esm[name] !== value) {

Original file line number	Diff line number	Diff line change
`@@ -47,7 +47,7 @@ function defaultExportDeprecation() {`
`47`	`47`	`printedDefaultExportDeprecation = true;`
`48`	`48`	`console.error(`
`49`	`49`	"`import sass from 'sass'` is deprecated.\n" +
`50`		- "Please use `import * as sass from 'sass'` instead."
	`50`	+ "Please use `import * as sass from 'sass'` instead.",
`51`	`51`	`);`
`52`	`52`	`}`
`53`	`53`