fix: security and reliability improvements for maintainership

- Fix runString() using tmpdir reference instead of tmpdir() call (fixes #320)
- Replace exec() with execFile() to prevent command injection in
  checkSyntaxFile, getVersion, and getVersionSync
- Add temp file cleanup in runString() and checkSyntax() via .finally()
- Replace custom extend() with Object.assign
- Re-enable getVersion/getVersionSync tests (were disabled since #158)
- Add GitHub Actions CI matrix (Node 18/20/22, Python 3.10/3.12, 3 OSes)
- Update minimum Node.js engine from 0.10 to 16

Author: ianhandy

.github/workflows/ci.yml

@@ -0,0 +1,26 @@
+name: CI
+
+on:
+  push:
+    branches: [master]
+  pull_request:
+    branches: [master]
+
+jobs:
+  test:
+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-latest, macos-latest, windows-latest]
+        node-version: [18, 20, 22]
+        python-version: ['3.10', '3.12']
+    steps:
+      - uses: actions/checkout@v4
+      - uses: actions/setup-node@v4
+        with:
+          node-version: ${{ matrix.node-version }}
+      - uses: actions/setup-python@v5
+        with:
+          python-version: ${{ matrix.python-version }}
+      - run: npm install
+      - run: npm test

index.ts

@@ -3,13 +3,13 @@ import {
   ChildProcess,
   spawn,
   SpawnOptions,
-  exec,
-  execSync,
+  execFile,
+  execFileSync,
 } from 'child_process';
 import { EOL as newline, tmpdir } from 'os';
 import { join, sep } from 'path';
 import { Readable, Transform, TransformCallback, Writable } from 'stream';
-import { writeFile, writeFileSync } from 'fs';
+import { writeFile, writeFileSync, unlink } from 'fs';
 import { promisify } from 'util';
 
 function toArray<T>(source?: T | T[]): T[] {
@@ -21,28 +21,8 @@ function toArray<T>(source?: T | T[]): T[] {
   return source;
 }
 
-/**
- * adds arguments as properties to obj
- */
-function extend(obj: {}, ...args) {
-  Array.prototype.slice.call(arguments, 1).forEach(function (source) {
-    if (source) {
-      for (let key in source) {
-        obj[key] = source[key];
-      }
-    }
-  });
-  return obj;
-}
-
-/**
- * gets a random int from 0-10000000000
- */
-function getRandomInt() {
-  return Math.floor(Math.random() * 10000000000);
-}
-
-const execPromise = promisify(exec);
+const execFilePromise = promisify(execFile);
+const unlinkPromise = promisify(unlink);
 
 export interface Options extends SpawnOptions {
   /**
@@ -171,7 +151,7 @@ export class PythonShell extends EventEmitter {
     let errorData = '';
     EventEmitter.call(this);
 
-    options = <Options>extend({}, PythonShell.defaultOptions, options);
+    options = <Options>Object.assign({}, PythonShell.defaultOptions, options);
     let pythonPath: string;
     if (!options.pythonPath) {
       pythonPath = PythonShell.defaultPythonPath;
@@ -266,7 +246,7 @@ export class PythonShell extends EventEmitter {
             'process exited with code ' + self.exitCode,
           );
         }
-        err = <PythonShellError>extend(err, {
+        Object.assign(err, {
           executable: pythonPath,
           options: pythonOptions.length ? pythonOptions : null,
           script: self.scriptPath,
@@ -313,12 +293,14 @@ export class PythonShell extends EventEmitter {
    * @returns rejects promise w/ string error output if syntax failure
    */
   static async checkSyntax(code: string) {
-    const randomInt = getRandomInt();
-    const filePath = tmpdir() + sep + `pythonShellSyntaxCheck${randomInt}.py`;
+    const filePath =
+      tmpdir() + sep + `pythonShellSyntaxCheck${Date.now()}.py`;
 
     const writeFilePromise = promisify(writeFile);
     return writeFilePromise(filePath, code).then(() => {
-      return this.checkSyntaxFile(filePath);
+      return this.checkSyntaxFile(filePath).finally(() => {
+        unlinkPromise(filePath).catch(() => {});
+      });
     });
   }
 
@@ -334,8 +316,7 @@ export class PythonShell extends EventEmitter {
    */
   static async checkSyntaxFile(filePath: string) {
     const pythonPath = this.getPythonPath();
-    let compileCommand = `${pythonPath} -m py_compile ${filePath}`;
-    return execPromise(compileCommand);
+    return execFilePromise(pythonPath, ['-m', 'py_compile', filePath]);
   }
 
   /**
@@ -370,21 +351,22 @@ export class PythonShell extends EventEmitter {
    */
   static runString(code: string, options?: Options) {
     // put code in temp file
-    const randomInt = getRandomInt();
-    const filePath = tmpdir + sep + `pythonShellFile${randomInt}.py`;
+    const filePath = tmpdir() + sep + `pythonShellFile${Date.now()}.py`;
     writeFileSync(filePath, code);
 
-    return PythonShell.run(filePath, options);
+    return PythonShell.run(filePath, options).finally(() => {
+      unlinkPromise(filePath).catch(() => {});
+    });
   }
 
   static getVersion(pythonPath?: string) {
     if (!pythonPath) pythonPath = this.getPythonPath();
-    return execPromise(pythonPath + ' --version');
+    return execFilePromise(pythonPath, ['--version']);
   }
 
   static getVersionSync(pythonPath?: string) {
     if (!pythonPath) pythonPath = this.getPythonPath();
-    return execSync(pythonPath + ' --version').toString();
+    return execFileSync(pythonPath, ['--version']).toString();
   }
 
   /**

package-lock.json

@@ -40,7 +40,7 @@
     "email": "nicolas@extrabacon.net"
   },
   "engines": {
-    "node": ">=0.10"
+    "node": ">=16"
   },
   "license": "MIT"
 }

package.json

@@ -98,27 +98,24 @@ describe('PythonShell', function () {
     });
   });
 
-  // #158 these tests are failing on appveyor windows node 8/10 python 2/3
-  // but they work locally on my windows machine .....
-  // these methods are not that important so just commenting out tests untill someone fixes them
-  // describe("#getVersion", function(){
-  //     it('should return a string', function(done){
-  //         PythonShell.getVersion().then((out)=>{
-  //             const version = out.stdout
-  //             version.should.be.a.String();
-  //             version.length.should.be.greaterThan(0)
-  //             done()
-  //         })
-  //     })
-  // })
+  describe('#getVersion', function () {
+    it('should return a string', function (done) {
+      PythonShell.getVersion().then((out) => {
+        const version = out.stdout;
+        version.should.be.a.String();
+        version.length.should.be.greaterThan(0);
+        done();
+      });
+    });
+  });
 
-  // describe("#getVersionSync", function(){
-  //     it('should return a string', function(){
-  //         const version = PythonShell.getVersionSync()
-  //         version.should.be.a.String();
-  //         version.length.should.be.greaterThan(0)
-  //     })
-  // })
+  describe('#getVersionSync', function () {
+    it('should return a string', function () {
+      const version = PythonShell.getVersionSync();
+      version.should.be.a.String();
+      version.length.should.be.greaterThan(0);
+    });
+  });
 
   describe('#runString(script, options)', function () {
     before(() => {