fix: close the file descriptor when reading a shebang fails

.eslintrc.js

@@ -0,0 +1,24 @@
+'use strict';
+
+// Unit tests in `lib/` must get the same config as `test/.eslintrc`, but ESLint 5 doesn't support
+// `extends` in `overrides`, so we apply what `es9` adds on top of `es6` manually
+const es7 = require('eslint-config-moxy/es7');
+const es8 = require('eslint-config-moxy/es8');
+const es9 = require('eslint-config-moxy/es9');
+
+module.exports = {
+    root: true,
+    extends: [
+        'eslint-config-moxy/es6',
+        'eslint-config-moxy/addons/node',
+        // Only enabled for test files, such as `lib/**/*.test.js`
+        'eslint-config-moxy/addons/jest',
+    ],
+    overrides: [
+        {
+            files: ['lib/**/*.test.js'],
+            parserOptions: es9.parserOptions,
+            rules: Object.assign({}, es7.rules, es8.rules, es9.rules),
+        },
+    ],
+};

lib/util/readShebang.js

@@ -13,8 +13,14 @@ function readShebang(command) {
     try {
         fd = fs.openSync(command, 'r');
         fs.readSync(fd, buffer, 0, size, 0);
-        fs.closeSync(fd);
-    } catch (e) { /* Empty */ }
+    } catch (e) { /* Empty */ } finally {
+        // Always close the file descriptor, even if reading failed
+        if (fd !== undefined) {
+            try {
+                fs.closeSync(fd);
+            } catch (e) { /* Empty */ }
+        }
+    }
 
     // Attempt to extract shebang (null is returned if not a shebang)
     return shebangCommand(buffer.toString());

lib/util/readShebang.test.js

@@ -0,0 +1,64 @@
+'use strict';
+
+const fs = require('fs');
+const path = require('path');
+const readShebang = require('./readShebang');
+
+describe('readShebang', () => {
+    afterEach(() => jest.restoreAllMocks());
+
+    it('should read the shebang command', () => {
+        const close = jest.spyOn(fs, 'closeSync');
+
+        expect(readShebang(path.join(__dirname, '../../test/fixtures/shebang'))).toBe('node');
+        expect(close).toHaveBeenCalledTimes(1);
+    });
+
+    it('should return null if the file has no shebang', () => {
+        expect(readShebang(path.join(__dirname, '../../test/fixtures/echo.js'))).toBe(null);
+    });
+
+    it('should close the file descriptor when reading fails, including descriptor zero', () => {
+        jest.spyOn(fs, 'openSync').mockReturnValue(0);
+        jest.spyOn(fs, 'readSync').mockImplementation(() => {
+            throw new Error('read failed');
+        });
+        const close = jest.spyOn(fs, 'closeSync').mockImplementation(() => {});
+
+        expect(readShebang('command')).toBe(null);
+        expect(close).toHaveBeenCalledTimes(1);
+        expect(close).toHaveBeenCalledWith(0);
+    });
+
+    it('should close the file descriptor when the file is a directory', () => {
+        const open = jest.spyOn(fs, 'openSync');
+        const close = jest.spyOn(fs, 'closeSync');
+
+        expect(readShebang(__dirname)).toBe(null);
+
+        // Opening a directory fails on Windows, but succeeds on other platforms and then reading fails
+        expect(close).toHaveBeenCalledTimes(open.mock.results.filter((result) => result.type === 'return').length);
+    });
+
+    it('should not close a file descriptor when opening fails', () => {
+        jest.spyOn(fs, 'openSync').mockImplementation(() => {
+            throw new Error('open failed');
+        });
+        const close = jest.spyOn(fs, 'closeSync').mockImplementation(() => {});
+
+        expect(readShebang('command')).toBe(null);
+        expect(close).not.toHaveBeenCalled();
+    });
+
+    it('should return null when closing also fails', () => {
+        jest.spyOn(fs, 'openSync').mockReturnValue(0);
+        jest.spyOn(fs, 'readSync').mockImplementation(() => {
+            throw new Error('read failed');
+        });
+        jest.spyOn(fs, 'closeSync').mockImplementation(() => {
+            throw new Error('close failed');
+        });
+
+        expect(readShebang('command')).toBe(null);
+    });
+});

package.json

@@ -21,7 +21,8 @@
   "license": "MIT",
   "main": "index.js",
   "files": [
-    "lib"
+    "lib",
+    "!lib/**/*.test.js"
   ],
   "scripts": {
     "lint": "eslint .",
@@ -36,6 +37,13 @@
       "pre-commit": "lint-staged"
     }
   },
+  "jest": {
+    "collectCoverageFrom": [
+      "index.js",
+      "lib/**/*.js",
+      "!lib/**/*.test.js"
+    ]
+  },
   "lint-staged": {
     "*.js": [
       "eslint --fix",