diff --git a/packages/dd-trace/src/plugins/util/exec.js b/packages/dd-trace/src/plugins/util/exec.js index 17b2dd56337..d55127d8b17 100644 --- a/packages/dd-trace/src/plugins/util/exec.js +++ b/packages/dd-trace/src/plugins/util/exec.js @@ -1,8 +1,8 @@ const cp = require('child_process') -const sanitizedExec = (cmd, options = {}) => { +const sanitizedExec = (cmd, flags, options = { stdio: 'pipe' }) => { try { - return cp.execSync(cmd, options).toString().replace(/(\r\n|\n|\r)/gm, '') + return cp.execFileSync(cmd, flags, options).toString().replace(/(\r\n|\n|\r)/gm, '') } catch (e) { return '' } diff --git a/packages/dd-trace/src/plugins/util/git.js b/packages/dd-trace/src/plugins/util/git.js index 8014828ab70..59143ea1031 100644 --- a/packages/dd-trace/src/plugins/util/git.js +++ b/packages/dd-trace/src/plugins/util/git.js @@ -1,6 +1,7 @@ -const { execSync } = require('child_process') +const { execFileSync } = require('child_process') const os = require('os') const path = require('path') +const fs = require('fs') const log = require('../../log') const { sanitizedExec } = require('./exec') @@ -21,26 +22,35 @@ const { const GIT_REV_LIST_MAX_BUFFER = 8 * 1024 * 1024 // 8MB +function isDirectory (path) { + try { + const stats = fs.statSync(path) + return stats.isDirectory() + } catch (e) { + return false + } +} + function isShallowRepository () { - return sanitizedExec('git rev-parse --is-shallow-repository', { stdio: 'pipe' }) === 'true' + return sanitizedExec('git', ['rev-parse', '--is-shallow-repository']) === 'true' } function unshallowRepository () { try { - execSync('git config remote.origin.partialclonefilter "blob:none"', { stdio: 'pipe' }) - execSync('git fetch --shallow-since="1 month ago" --update-shallow --refetch', { stdio: 'pipe' }) + sanitizedExec('git', ['config', 'remote.origin.partialclonefilter', '"blob:none"']) + sanitizedExec('git', ['fetch', '--shallow-since="1 month ago"', '--update-shallow', '--refetch']) } catch (err) { log.error(err) } } function getRepositoryUrl () { - return sanitizedExec('git config --get remote.origin.url', { stdio: 'pipe' }) + return sanitizedExec('git', ['config', '--get', 'remote.origin.url']) } function getLatestCommits () { try { - return execSync('git log --format=%H -n 1000 --since="1 month ago"', { stdio: 'pipe' }) + return execFileSync('git', ['log', '--format=%H', '-n 1000', '--since="1 month ago"'], { stdio: 'pipe' }) .toString() .split('\n') .filter(commit => commit) @@ -51,15 +61,21 @@ function getLatestCommits () { } function getCommitsToUpload (commitsToExclude) { - let gitCommandToGetCommitsToUpload = - 'git rev-list --objects --no-object-names --filter=blob:none --since="1 month ago" HEAD' - - commitsToExclude.forEach(commit => { - gitCommandToGetCommitsToUpload = `${gitCommandToGetCommitsToUpload} ^${commit}` - }) + const commitsToExcludeString = commitsToExclude.map(commit => `^${commit}`) try { - return execSync(gitCommandToGetCommitsToUpload, { stdio: 'pipe', maxBuffer: GIT_REV_LIST_MAX_BUFFER }) + return execFileSync( + 'git', + [ + 'rev-list', + '--objects', + '--no-object-names', + '--filter=blob:none', + '--since="1 month ago"', + 'HEAD', + ...commitsToExcludeString + ], + { stdio: 'pipe', maxBuffer: GIT_REV_LIST_MAX_BUFFER }) .toString() .split('\n') .filter(commit => commit) @@ -72,6 +88,11 @@ function getCommitsToUpload (commitsToExclude) { function generatePackFilesForCommits (commitsToUpload) { const tmpFolder = os.tmpdir() + if (!isDirectory(tmpFolder)) { + log.error(new Error('Provided path to generate packfiles is not a directory')) + return [] + } + const randomPrefix = String(Math.floor(Math.random() * 10000)) const temporaryPath = path.join(tmpFolder, randomPrefix) const cwdPath = path.join(process.cwd(), randomPrefix) @@ -79,14 +100,20 @@ function generatePackFilesForCommits (commitsToUpload) { // Generates pack files to upload and // returns the ordered list of packfiles' paths function execGitPackObjects (targetPath) { - return execSync( - `git pack-objects --compression=9 --max-pack-size=3m ${targetPath}`, - { input: commitsToUpload.join('\n') } + return execFileSync( + 'git', + [ + 'pack-objects', + '--compression=9', + '--max-pack-size=3m', + targetPath + ], + { stdio: 'pipe', input: commitsToUpload.join('\n') } ).toString().split('\n').filter(commit => commit).map(commit => `${targetPath}-${commit}.pack`) } try { - return execGitPackObjects(temporaryPath, commitsToUpload) + return execGitPackObjects(temporaryPath) } catch (err) { log.error(err) /** @@ -102,7 +129,7 @@ function generatePackFilesForCommits (commitsToUpload) { * TODO: fix issue and remove workaround. */ try { - return execGitPackObjects(cwdPath, commitsToUpload) + return execGitPackObjects(cwdPath) } catch (err) { log.error(err) } @@ -133,23 +160,23 @@ function getGitMetadata (ciMetadata) { committerName, committerEmail, committerDate - ] = sanitizedExec('git show -s --format=%an,%ae,%aI,%cn,%ce,%cI', { stdio: 'pipe' }).split(',') + ] = sanitizedExec('git', ['show', '-s', '--format=%an,%ae,%aI,%cn,%ce,%cI']).split(',') return { [GIT_REPOSITORY_URL]: - repositoryUrl || sanitizedExec('git ls-remote --get-url', { stdio: 'pipe' }), + repositoryUrl || sanitizedExec('git', ['ls-remote', '--get-url']), [GIT_COMMIT_MESSAGE]: - commitMessage || sanitizedExec('git show -s --format=%s', { stdio: 'pipe' }), + commitMessage || sanitizedExec('git', ['show', '-s', '--format=%s']), [GIT_COMMIT_AUTHOR_DATE]: authorDate, [GIT_COMMIT_AUTHOR_NAME]: ciAuthorName || authorName, [GIT_COMMIT_AUTHOR_EMAIL]: ciAuthorEmail || authorEmail, [GIT_COMMIT_COMMITTER_DATE]: committerDate, [GIT_COMMIT_COMMITTER_NAME]: committerName, [GIT_COMMIT_COMMITTER_EMAIL]: committerEmail, - [GIT_BRANCH]: branch || sanitizedExec('git rev-parse --abbrev-ref HEAD', { stdio: 'pipe' }), - [GIT_COMMIT_SHA]: commitSHA || sanitizedExec('git rev-parse HEAD', { stdio: 'pipe' }), + [GIT_BRANCH]: branch || sanitizedExec('git', ['rev-parse', '--abbrev-ref', 'HEAD']), + [GIT_COMMIT_SHA]: commitSHA || sanitizedExec('git', ['rev-parse', 'HEAD']), [GIT_TAG]: tag, - [CI_WORKSPACE_PATH]: ciWorkspacePath || sanitizedExec('git rev-parse --show-toplevel', { stdio: 'pipe' }) + [CI_WORKSPACE_PATH]: ciWorkspacePath || sanitizedExec('git', ['rev-parse', '--show-toplevel']) } } diff --git a/packages/dd-trace/test/plugins/util/git.spec.js b/packages/dd-trace/test/plugins/util/git.spec.js index d8807294bab..868d25a82fe 100644 --- a/packages/dd-trace/test/plugins/util/git.spec.js +++ b/packages/dd-trace/test/plugins/util/git.spec.js @@ -68,12 +68,12 @@ describe('git', () => { } ) expect(metadata[GIT_REPOSITORY_URL]).not.to.equal('ciRepositoryUrl') - expect(sanitizedExecStub).to.have.been.calledWith('git ls-remote --get-url', { stdio: 'pipe' }) - expect(sanitizedExecStub).to.have.been.calledWith('git show -s --format=%an,%ae,%aI,%cn,%ce,%cI', { stdio: 'pipe' }) - expect(sanitizedExecStub).not.to.have.been.calledWith('git show -s --format=%s', { stdio: 'pipe' }) - expect(sanitizedExecStub).not.to.have.been.calledWith('git rev-parse HEAD', { stdio: 'pipe' }) - expect(sanitizedExecStub).not.to.have.been.calledWith('git rev-parse --abbrev-ref HEAD', { stdio: 'pipe' }) - expect(sanitizedExecStub).not.to.have.been.calledWith('git rev-parse --show-toplevel', { stdio: 'pipe' }) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['ls-remote', '--get-url']) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['show', '-s', '--format=%an,%ae,%aI,%cn,%ce,%cI']) + expect(sanitizedExecStub).not.to.have.been.calledWith('git', ['show', '-s', '--format=%s']) + expect(sanitizedExecStub).not.to.have.been.calledWith('git', ['rev-parse', 'HEAD']) + expect(sanitizedExecStub).not.to.have.been.calledWith('git', ['rev-parse', '--abbrev-ref', 'HEAD']) + expect(sanitizedExecStub).not.to.have.been.calledWith('git', ['rev-parse', '--show-toplevel']) }) it('does not crash if git is not available', () => { sanitizedExecStub.returns('') @@ -121,12 +121,12 @@ describe('git', () => { [GIT_COMMIT_COMMITTER_NAME]: 'git committer', [CI_WORKSPACE_PATH]: 'ciWorkspacePath' }) - expect(sanitizedExecStub).to.have.been.calledWith('git ls-remote --get-url', { stdio: 'pipe' }) - expect(sanitizedExecStub).to.have.been.calledWith('git show -s --format=%s', { stdio: 'pipe' }) - expect(sanitizedExecStub).to.have.been.calledWith('git show -s --format=%an,%ae,%aI,%cn,%ce,%cI', { stdio: 'pipe' }) - expect(sanitizedExecStub).to.have.been.calledWith('git rev-parse HEAD', { stdio: 'pipe' }) - expect(sanitizedExecStub).to.have.been.calledWith('git rev-parse --abbrev-ref HEAD', { stdio: 'pipe' }) - expect(sanitizedExecStub).to.have.been.calledWith('git rev-parse --show-toplevel', { stdio: 'pipe' }) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['ls-remote', '--get-url']) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['show', '-s', '--format=%s']) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['show', '-s', '--format=%an,%ae,%aI,%cn,%ce,%cI']) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['rev-parse', 'HEAD']) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['rev-parse', '--abbrev-ref', 'HEAD']) + expect(sanitizedExecStub).to.have.been.calledWith('git', ['rev-parse', '--show-toplevel']) }) }) @@ -155,7 +155,7 @@ describe('getCommitsToUpload', () => { const { getCommitsToUpload } = proxyquire('../../../src/plugins/util/git', { 'child_process': { - 'execSync': (_, ...rest) => execSync(`head -c ${GIT_REV_LIST_MAX_BUFFER * 2} /dev/zero`, ...rest) + 'execFileSync': (_, ...rest) => execSync(`head -c ${GIT_REV_LIST_MAX_BUFFER * 2} /dev/zero`, ...rest) }, '../../log': { error: logErrorSpy @@ -169,39 +169,40 @@ describe('getCommitsToUpload', () => { }) describe('generatePackFilesForCommits', () => { + let tmpdirStub beforeEach(() => { sinon.stub(Math, 'random').returns('0.1234') - sinon.stub(os, 'tmpdir').returns('tmp') + tmpdirStub = sinon.stub(os, 'tmpdir').returns('/tmp') sinon.stub(process, 'cwd').returns('cwd') }) afterEach(() => { sinon.restore() }) it('creates pack files in temporary path', () => { - const execSyncSpy = sinon.stub().returns(['commitSHA']) + const execFileSyncSpy = sinon.stub().returns(['commitSHA']) const { generatePackFilesForCommits } = proxyquire('../../../src/plugins/util/git', { 'child_process': { - 'execSync': execSyncSpy + 'execFileSync': execFileSyncSpy } } ) - const temporaryPath = path.join('tmp', '1234') + const temporaryPath = path.join('/tmp', '1234') const packFilesToUpload = generatePackFilesForCommits(['commitSHA']) expect(packFilesToUpload).to.eql([`${temporaryPath}-commitSHA.pack`]) }) it('creates pack files in cwd if the temporary path fails', () => { - const execSyncSpy = sinon.stub().onCall(0).throws().onCall(1).returns(['commitSHA']) + const execFileSyncSpy = sinon.stub().onCall(0).throws().onCall(1).returns(['commitSHA']) const cwdPath = path.join('cwd', '1234') const { generatePackFilesForCommits } = proxyquire('../../../src/plugins/util/git', { 'child_process': { - 'execSync': execSyncSpy + 'execFileSync': execFileSyncSpy } } ) @@ -209,4 +210,20 @@ describe('generatePackFilesForCommits', () => { const packFilesToUpload = generatePackFilesForCommits(['commitSHA']) expect(packFilesToUpload).to.eql([`${cwdPath}-commitSHA.pack`]) }) + + it('does not work if tmpdir does not return a folder', () => { + tmpdirStub.restore() + sinon.stub(os, 'tmpdir').returns('; echo hey') + const execFileSyncSpy = sinon.stub().onCall(0).throws().onCall(1).returns(['commitSHA']) + + const { generatePackFilesForCommits } = proxyquire('../../../src/plugins/util/git', + { + 'child_process': { + 'execFileSync': execFileSyncSpy + } + } + ) + const packFilesToUpload = generatePackFilesForCommits(['commitSHA']) + expect(packFilesToUpload).to.eql([]) + }) })