Use spawn with git to avoid shell script vulnerabilities (#14)

helpers/spawn/index.js

@@ -0,0 +1,38 @@
+const { spawn } = require('child_process');
+
+/**
+ * Executes git with given arguments string. resolves with the output message
+ * @param  {String} string
+ * @return {Promise}
+ */
+module.exports = string => new Promise(
+	(resolve, reject) => {
+		const git = spawn(
+			'git',
+			string.split(' '),
+		);
+
+		const outputs = { stdout: [], stderr: [] };
+
+		git.stdout.on(
+			'data',
+			data => outputs.stdout.push(data.toString()),
+		);
+
+		git.stderr.on(
+			'data',
+			data => outputs.stderr.push(data.toString()),
+		);
+
+		git.on('exit', code => {
+			switch (code) {
+				case 0:
+					resolve(outputs.stdout.join('').trim());
+					break;
+				default:
+					reject(outputs.stderr.join('').trim());
+					break;
+			}
+		});
+	},
+);

index.js

@@ -1,7 +1,6 @@
-const exec = require('async-execute');
+const spawn = require('./helpers/spawn');
 
 const {
-	list,
 	modified,
 	remote,
 	reset,
@@ -30,41 +29,41 @@ const formats = [
  * @type {Array[]}
  */
 const outputs = [
-	[ 'branch', 'git rev-parse --abbrev-ref HEAD' ],
-	[ 'origin', 'git remote get-url origin' ],
+	[ 'branch', 'rev-parse --abbrev-ref HEAD' ],
+	[ 'origin', 'remote get-url origin' ],
 ];
 
 /**
  * Lists to get from multi-line terminal output
  * @type {Array[]}
  */
 const lists = [
-	[ 'changed', 'git diff-tree --no-commit-id --name-only -r HEAD' ],
-	[ 'staged', 'git diff --name-only --cached' ],
-	[ 'tags', 'git tag' ],
-	[ 'unstaged', 'git diff --name-only' ],
-	[ 'untracked', 'git ls-files -o --exclude-standard' ],
+	[ 'changed', 'diff-tree --no-commit-id --name-only -r HEAD' ],
+	[ 'staged', 'diff --name-only --cached' ],
+	[ 'tags', 'tag' ],
+	[ 'unstaged', 'diff --name-only' ],
+	[ 'untracked', 'ls-files -o --exclude-standard' ],
 ];
 
 /**
  * Properties which will become (async) getters
  */
 const getters = Object.assign(
 	{
-		date: async () => new Date(parseInt(await exec('git show -s --format=%at')) * 1000),
+		date: async () => new Date(parseInt(await spawn('show -s --format=%at')) * 1000),
 		name: async () => (await remote()).name,
 		owner: async () => (await remote()).owner,
 		unadded,
-		version: async () => (await exec('git version')).split(' ').pop(),
+		version: async () => (await spawn('version')).split(' ').pop(),
 	},
 	...outputs.map(
-		([ key, value ]) => ({ [key]: exec.bind(null, value) }),
+		([ key, value ]) => ({ [key]: spawn.bind(null, value) }),
 	),
 	...lists.map(
-		([ key, value ]) => ({ [key]: list.bind(null, value) }),
+		([ key, value ]) => ({ [key]: async () => (await spawn(value)).split('\n') }),
 	),
 	...formats.map(
-		([ key, value ]) => ({ [key]: exec.bind(null, `git show -s --format=%${value}`) }),
+		([ key, value ]) => ({ [key]: spawn.bind(null, `show -s --format=%${value}`) }),
 	),
 );
 

lib/index.js

@@ -1,5 +1,4 @@
 module.exports = {
-	list: require('./list'),
 	modified: require('./modified'),
 	remote: require('./remote'),
 	reset: require('./reset'),

lib/modified/index.js

@@ -1,6 +1,6 @@
 const { resolve } = require('path');
-const exec = require('async-execute');
 const exist = require('@does/exist');
+const spawn = require('../../helpers/spawn');
 
 /**
  * Create and push a git tag with the last commit message
@@ -18,7 +18,7 @@ module.exports = async function(path) {
 		throw new Error(`Could not find file at path "${absolute}"`);
 	}
 
-	const ts = await exec(`git log -1 --format="%at" -- ${path}`);
+	const ts = await spawn(`log -1 --format="%at" -- ${path}`);
 
 	return new Date(Number(ts) * 1000);
 };

lib/remote/index.js

@@ -1,11 +1,11 @@
-const exec = require('async-execute');
+const spawn = require('../../helpers/spawn');
 
 /**
  * Get remote repo name and owner
  * @return {string{}}
  */
 module.exports = async function remote() {
-	const origin = await exec('git remote get-url origin');
+	const origin = await spawn('remote get-url origin');
 	const nosuffix = origin.replace(/\.git$/, '');
 	const [ match ] = nosuffix.match(/[\w-]*\/[\w-]+$/) || [ nosuffix ];
 

lib/remote/spec.js

@@ -2,7 +2,7 @@ const { clean, override } = abuser(__filename);
 
 function cleanup() {
 	clean('.');
-	clean('async-execute');
+	clean('../../helpers/spawn');
 }
 
 describe('lib/remote', () => {
@@ -23,7 +23,7 @@ describe('lib/remote', () => {
 		'ssh://[username@]repo_owner1/repo-name',
 	].forEach(
 		format => it(`should find owner and repo name in ${format}`, async () => {
-			override('async-execute', () => format);
+			override('../../helpers/spawn', () => format);
 
 			const remote = require('.');
 			const { owner, name } = await remote();
@@ -40,7 +40,7 @@ describe('lib/remote', () => {
 		format => it(
 			`Still returns reponame when no owner is found (${format})`,
 			async () => {
-				override('async-execute', async () => format);
+				override('../../helpers/spawn', async () => format);
 
 				const remote = require('.');
 				const { owner, name } = await remote();

lib/reset/index.js

@@ -1,4 +1,4 @@
-const exec = require('async-execute');
+const spawn = require('../../helpers/spawn');
 
 /**
  * Reset current HEAD to the specified state
@@ -8,11 +8,17 @@ const exec = require('async-execute');
  */
 module.exports = async function(destination, { hard = true } = {}) {
 	if (destination && typeof destination === 'string') {
-		return await exec(`git reset ${JSON.stringify(destination)} ${hard ? '--hard' : ''}`);
+		try {
+			await spawn(`check-ref-format ${destination}`);
+		} catch (error) {
+			throw new RangeError('can not reset to illegal ref "${destination}"');
+		}
+
+		return await spawn(`reset ${destination} ${hard ? '--hard' : ''}`);
 	}
 
 	if (destination && typeof destination === 'number') {
-		return await exec(`git reset HEAD~${Math.abs(destination)} ${hard ? '--hard' : ''}`);
+		return await spawn(`reset HEAD~${Math.abs(destination)} ${hard ? '--hard' : ''}`);
 	}
 
 	throw new TypeError(`No case for handling destination ${destination} (${typeof destination})`);

lib/reset/spec.js

@@ -6,7 +6,7 @@ describe('lib/reset', async() => {
 
 	before(() => {
 		clean('.');
-		override('async-execute', exec);
+		override('../../helpers/spawn', exec);
 		reset = require('.');
 	});
 	afterEach(() => exec.resetHistory());
@@ -31,22 +31,22 @@ describe('lib/reset', async() => {
 	});
 
 	it('Should hard reset to a given sha', async() => {
-		reset('shaid');
-		expect(exec.getCall(0).args[0]).to.equal('git reset "shaid" --hard');
+		await reset('shaid');
+		expect(exec.getCall(1).args[0]).to.equal('reset shaid --hard');
 	});
 
 	it('Should hard reset to n commits back', async() => {
-		reset(1);
-		expect(exec.getCall(0).args[0]).to.equal('git reset HEAD~1 --hard');
+		await reset(1);
+		expect(exec.getCall(0).args[0]).to.equal('reset HEAD~1 --hard');
 	});
 
 	it('Should hard reset to n commits back with negative value as well', async() => {
-		reset(-3);
-		expect(exec.getCall(0).args[0]).to.equal('git reset HEAD~3 --hard');
+		await reset(-3);
+		expect(exec.getCall(0).args[0]).to.equal('reset HEAD~3 --hard');
 	});
 
 	it('Should reset w/o hard argument', async() => {
-		reset('shaid', { hard: false });
-		expect(exec.getCall(0).args[0].trim()).to.equal('git reset "shaid"');
+		await reset('shaid', { hard: false });
+		expect(exec.getCall(1).args[0].trim()).to.equal('reset shaid');
 	});
 });

lib/tag/index.js

@@ -1,4 +1,4 @@
-const exec = require('async-execute');
+const spawn = require('../../helpers/spawn');
 
 /**
  * Create and push a git tag with the last commit message
@@ -13,9 +13,9 @@ module.exports = async function(tag) {
 	const { message, author, email } = this;
 
 	await Promise.all([
-		exec(`git config user.name "${await author}"`),
-		exec(`git config user.email "${await email}"`),
+		spawn(`config user.name "${await author}"`),
+		spawn(`config user.email "${await email}"`),
 	]);
-	await exec(`git tag -a ${JSON.stringify(tag)} -m "${await message}"`);
-	await exec(`git push origin ${JSON.stringify(`refs/tags/${tag}`)}`);
+	await spawn(`tag -a ${tag} -m "${await message}"`);
+	await spawn(`push origin refs/tags/${tag}`);
 };

lib/tag/spec.js

@@ -6,7 +6,7 @@ describe('lib/tag', async() => {
 
 	before(() => {
 		clean('.');
-		override('async-execute', (...args) => dummy.stub(...args));
+		override('../../helpers/spawn', (...args) => dummy.stub(...args));
 
 		gitTag = require('.').bind({
 			message: 'this is a message',
@@ -22,16 +22,16 @@ describe('lib/tag', async() => {
 		dummy.stub = command => lines.push(command);
 
 		await gitTag('1.1.1');
-		expect(lines).to.include('git config user.name "boaty mcboatface"');
-		expect(lines).to.include('git config user.email "boaty@boat.face"');
+		expect(lines).to.include('config user.name "boaty mcboatface"');
+		expect(lines).to.include('config user.email "boaty@boat.face"');
 	});
 
 	it('Should create a git tag with given message and push it', async() => {
 		const lines = [];
 		dummy.stub = command => lines.push(command);
 
 		await gitTag('1.1.1');
-		expect(lines).to.include('git tag -a "1.1.1" -m "this is a message"');
-		expect(lines).to.include('git push origin "refs/tags/1.1.1"');
+		expect(lines).to.include('tag -a 1.1.1 -m "this is a message"');
+		expect(lines).to.include('push origin refs/tags/1.1.1');
 	});
 });

lib/unadded/index.js

@@ -1,11 +1,11 @@
-const exec = require('async-execute');
+const spawn = require('../../helpers/spawn');
 
 /**
  * List all files that would be added by "add ."
  * @return {string[]}
  */
 module.exports = async function unadded() {
-	const output = await exec('git add --dry-run .');
+	const output = await spawn('add --dry-run .');
 
 	return output
 		.split('\n')

lib/unadded/spec.js

@@ -5,7 +5,7 @@ describe('lib/unadded', () => {
 
 	before(() => {
 		clean('.');
-		override('async-execute', () => `remove '.gitattributes'
+		override('../../helpers/spawn', () => `remove '.gitattributes'
 add 'apps/post/index.js'
 add 'new file.txt'
 `);