Skip to content

Commit

Permalink
Use spawn with git to avoid shell script vulnerabilities
Browse files Browse the repository at this point in the history
  • Loading branch information
omrilotan authored and undefined committed Jan 24, 2021
1 parent 20701c5 commit e4b7006
Show file tree
Hide file tree
Showing 17 changed files with 102 additions and 136 deletions.
38 changes: 38 additions & 0 deletions helpers/spawn/index.js
Original file line number Diff line number Diff line change
@@ -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;
}
});
},
);
27 changes: 13 additions & 14 deletions index.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
const exec = require('async-execute');
const spawn = require('./helpers/spawn');

const {
list,
modified,
remote,
reset,
Expand Down Expand Up @@ -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}`) }),
),
);

Expand Down
1 change: 0 additions & 1 deletion lib/index.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
module.exports = {
list: require('./list'),
modified: require('./modified'),
remote: require('./remote'),
reset: require('./reset'),
Expand Down
15 changes: 0 additions & 15 deletions lib/list/index.js

This file was deleted.

51 changes: 0 additions & 51 deletions lib/list/spec.js

This file was deleted.

4 changes: 2 additions & 2 deletions lib/modified/index.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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);
};
4 changes: 2 additions & 2 deletions lib/remote/index.js
Original file line number Diff line number Diff line change
@@ -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 ];

Expand Down
6 changes: 3 additions & 3 deletions lib/remote/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ const { clean, override } = abuser(__filename);

function cleanup() {
clean('.');
clean('async-execute');
clean('../../helpers/spawn');
}

describe('lib/remote', () => {
Expand All @@ -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();
Expand All @@ -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();
Expand Down
12 changes: 9 additions & 3 deletions lib/reset/index.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
const exec = require('async-execute');
const spawn = require('../../helpers/spawn');

/**
* Reset current HEAD to the specified state
Expand All @@ -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})`);
Expand Down
18 changes: 9 additions & 9 deletions lib/reset/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ describe('lib/reset', async() => {

before(() => {
clean('.');
override('async-execute', exec);
override('../../helpers/spawn', exec);
reset = require('.');
});
afterEach(() => exec.resetHistory());
Expand All @@ -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');
});
});
10 changes: 5 additions & 5 deletions lib/tag/index.js
Original file line number Diff line number Diff line change
@@ -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
Expand All @@ -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}`);
};
10 changes: 5 additions & 5 deletions lib/tag/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -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');
});
});
4 changes: 2 additions & 2 deletions lib/unadded/index.js
Original file line number Diff line number Diff line change
@@ -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')
Expand Down
2 changes: 1 addition & 1 deletion lib/unadded/spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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'
`);
Expand Down
Loading

0 comments on commit e4b7006

Please sign in to comment.