fix: use execFile instead of exec for security reason (#26)

Thanks Douglas Hall (douglas_hall) to report this security bug.

.travis.yml

@@ -3,7 +3,7 @@ language: node_js
 node_js:
   - '6'
   - '8'
-  - '9'
+  - '10'
 install:
   - npm i npminstall && npminstall
 script:

appveyor.yml

@@ -2,7 +2,7 @@ environment:
   matrix:
     - nodejs_version: '6'
     - nodejs_version: '8'
-    - nodejs_version: '9'
+    - nodejs_version: '10'
 
 install:
   - ps: Install-Product node $env:nodejs_version

lib/cmd/start.js

@@ -4,7 +4,7 @@ const path = require('path');
 
 const Command = require('../command');
 const debug = require('debug')('egg-script:start');
-const { exec } = require('mz/child_process');
+const { execFile } = require('mz/child_process');
 const fs = require('mz/fs');
 const homedir = require('node-homedir');
 const mkdirp = require('mz-modules/mkdirp');
@@ -232,12 +232,15 @@ class StartCommand extends Command {
 
     if (hasError) {
       try {
-        const [ stdout ] = yield exec('tail -n 100 ' + stderr);
+        const args = [ '-n', '100', stderr ];
+        this.logger.error('tail %s', args.join(' '));
+        const [ stdout ] = yield execFile('tail', args);
         this.logger.error('Got error when startup: ');
         this.logger.error(stdout);
-      } catch (_) {
-        // nothing
+      } catch (err) {
+        this.logger.error('ignore tail error: %s', err);
       }
+
       isSuccess = ignoreStdErr;
       this.logger.error('Start got error, see %s', stderr);
       this.logger.error('Or use `--ignore-stderr` to ignore stderr at startup.');

package.json

@@ -50,7 +50,7 @@
     "bin"
   ],
   "ci": {
-    "version": "6, 8, 9"
+    "version": "6, 8, 10"
   },
   "bug": {
     "url": "https://github.com/eggjs/egg/issues"

test/start.test.js

@@ -307,6 +307,7 @@ describe('test/start.test.js', () => {
       before(function* () {
         yield utils.cleanup(fixturePath);
         yield rimraf(logDir);
+        yield rimraf(path.join(fixturePath, 'start-fail'));
         yield mkdirp(logDir);
       });
 
@@ -315,6 +316,7 @@ describe('test/start.test.js', () => {
         yield utils.cleanup(fixturePath);
         yield rimraf(path.join(fixturePath, 'stdout.log'));
         yield rimraf(path.join(fixturePath, 'stderr.log'));
+        yield rimraf(path.join(fixturePath, 'start-fail'));
       });
 
       it('should start', function* () {
@@ -332,6 +334,30 @@ describe('test/start.test.js', () => {
         content = yield fs.readFile(stderr, 'utf-8');
         assert(content === '');
       });
+
+      it('should start with insecurity --stderr argument', function* () {
+        const cwd = path.join(__dirname, 'fixtures/status');
+        mm(process.env, 'ERROR', 'error message');
+
+        const stdout = path.join(fixturePath, 'start-fail/stdout.log');
+        const stderr = path.join(fixturePath, 'start-fail/stderr.log');
+        const malicious = path.join(fixturePath, 'start-fail/malicious');
+        app = coffee.fork(eggBin, [
+          'start', '--workers=1', '--daemon', `--stdout=${stdout}`,
+          `--stderr=${stderr}; touch ${malicious}`,
+          cwd,
+        ]);
+        // app.debug();
+
+        yield sleep(waitTime);
+
+        const content = yield fs.readFile(stdout, 'utf-8');
+        assert(!content.match(/custom-framework started on http:\/\/127\.0\.0\.1:7001/));
+        let exists = yield fs.exists(stderr);
+        assert(!exists);
+        exists = yield fs.exists(malicious);
+        assert(!exists);
+      });
     });
 
     describe('read cluster config', () => {
@@ -520,10 +546,11 @@ describe('test/start.test.js', () => {
       if (isWin) stderr = stderr.replace(/\\/g, '\\\\');
 
       const app = coffee.fork(eggBin, [ 'start', '--daemon', '--workers=1' ], { cwd });
-      // app.debug()
+      // app.debug();
       // TODO: find a windows replacement for tail command
       if (!isWin) app.expect('stderr', /nodejs.Error: error message/);
       yield app.expect('stderr', new RegExp(`Start got error, see ${stderr}`))
+        .expect('stderr', /Got error when startup/)
         .expect('code', 1)
         .end();
     });