Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use spawn with git to avoid shell script vulnerabilities #14

Merged
merged 1 commit into from
Jan 24, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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