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

fix: address npm audit security report #42

Merged
merged 7 commits into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from 3 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
2 changes: 1 addition & 1 deletion .eslintrc
Original file line number Diff line number Diff line change
@@ -1,3 +1,3 @@
{
"extends": "groupon/node4"
"extends": "groupon/node6"
}
3 changes: 1 addition & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
/yarn.lock
/package-lock.json
/.nyc_output
node_modules/
/tmp
npm-debug.log
Expand Down
10 changes: 5 additions & 5 deletions .travis.yml
Original file line number Diff line number Diff line change
@@ -1,12 +1,12 @@
language: node_js
node_js:
- 4.6.1
- 6.11.5
- 8.9.0
- 6.14.3
- 8.11.3
- 10.5.0
deploy:
- provider: script
script: ./bin/nlm.js release
script: ./node_modules/.bin/nlm release
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assume that won't work

skip_cleanup: true
'on':
branch: master
node: 8.9.0
node: 10.5.0
1 change: 1 addition & 0 deletions lib/cli.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,6 +97,7 @@ if (argv.version) {
} else {
const cwd = process.cwd();
const packageJsonFile = path.join(cwd, 'package.json');
// eslint-disable-next-line import/no-dynamic-require
const pkg = require(packageJsonFile);
command(cwd, pkg, pkg.nlm ? _.merge({}, pkg.nlm, argv) : argv)
.catch(prettyPrintErrorAndExit)
Expand Down
6 changes: 3 additions & 3 deletions lib/git/commits.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ const run = require('../run');

const SEPARATOR = '---nlm-split---';
const GIT_LOG_FORMAT = `--format=%H %P\n%B${SEPARATOR}`;
const PR_MERGE_PATTERN = /^Merge pull request #(\d+) from ([^/]+)\/([\S]+)/;
const PR_MERGE_PATTERN = /^Merge pull request #(\d+) from ([^\/]+)\/([\S]+)/;

function parseCommit(commit) {
const metaEndIdx = commit.indexOf('\n');
Expand All @@ -54,14 +54,14 @@ function parseCommit(commit) {
const parentSha = meta.shift() || null;

const data = commitParser.sync(message, {
issuePrefixes: ['#', 'https?://\\w[\\w.-]*[\\w/-]+?'],
issuePrefixes: ['#', 'https?://\\w[\\w.-]*[\\D/-]+'],
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

\D includes literally anything that isn't a digit, including e.g. ]

});
const prMatch = message.match(PR_MERGE_PATTERN);
if (prMatch) {
const prId = prMatch[1];
data.type = 'pr';
data.pullId = prId;
data.references.push({
Object.assign(data.references[0], {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you explain this a bit?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

No problem. After updating the conventional-commits-parser library it seemed that there was already an array index of 0 present, and this test was expecting the original code to push those values into an empty array.
I thought it best to not disturb the original functionality

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I parse it, the test was expecting there to be no references, and hence our push()ed one is [0].

What's changed is that there now are some... I'd vote:

  1. figure out why (what are the references we're getting now that we weren't before)
  2. if they're good things, change the test to search for our push()ed item, rather than assuming it's at [0]
  3. if they're things we don't want, delete them and push ours

action: 'Merges',
owner: prMatch[2],
repository: null,
Expand Down
4 changes: 2 additions & 2 deletions lib/git/ensure-tag.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,12 +52,12 @@ function fetchTag(cwd, tag) {
function ensureTag(cwd, tag) {
if (tag === 'v0.0.0') {
// There is no such thing (most likely)
return undefined;
return null;
}

const tagFile = path.join(cwd, '.git', 'refs', 'tags', tag);
try {
fs.readFileSync(tagFile);
return fs.readFileSync(tagFile);
} catch (error) {
if (error.code !== 'ENOENT') {
throw error;
Expand Down
2 changes: 1 addition & 1 deletion lib/github/setup-labels.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const REQUIRED_LABELS = [
];

function findMissingLabels(labels) {
return _.reject(REQUIRED_LABELS, function exists(label) {
return _.reject(REQUIRED_LABELS, label => {
return _.find(labels, { name: label.name });
});
}
Expand Down
8 changes: 4 additions & 4 deletions lib/license/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ const COMMENT_TYPES = {
getLicenseHeader: function getLicenseHeader(licenseText) {
const body = licenseText
.split('\n')
.map(function prefixLine(line) {
.map(line => {
return ` ${`* ${line}`.trim()}`;
})
.join('\n');
Expand Down Expand Up @@ -78,7 +78,7 @@ function collectFiles(cwd, whitelist, optionalExclude) {

return Bluebird.map(whitelist || ['.'], scanDirectory)
.then(_.flatten)
.map(function loadFile(relFilename) {
.map(relFilename => {
const filename = path.join(cwd, relFilename);
return Bluebird.props({
filename: filename,
Expand All @@ -103,7 +103,7 @@ function addMissingLicenseHeaders(licenseText, files) {
'.coffee': COMMENT_TYPES['.coffee'].getLicenseHeader(licenseText),
};
return files
.map(function buildLicenseHeader(file) {
.map(file => {
file.licenseHeader = licenseHeaders[path.extname(file.filename)];
return file;
})
Expand All @@ -114,7 +114,7 @@ function addMissingLicenseHeaders(licenseText, files) {
function getLicenseText(cwd) {
return readFileAsync(path.join(cwd, 'LICENSE'), 'utf8')
.then(_.trim)
.catch(function catchNotFound(error) {
.catch(error => {
if (error.code === 'ENOENT') {
return null;
}
Expand Down
8 changes: 4 additions & 4 deletions lib/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,17 +40,17 @@ const _ = require('lodash');

module.exports = function run(command, args, options) {
debug(command, args, _.omit(options, 'env'));
return new Bluebird(function resolveExec(resolve, reject) {
return new Bluebird((resolve, reject) => {
function onExecDone(error, stdout) {
if (error) return reject(error);
resolve(stdout);
return resolve(stdout);
}

const child = childProcess.execFile(command, args, options, onExecDone);
child.stdout.on('data', function forwardStdOut(chunk) {
child.stdout.on('data', chunk => {
debug('stdout', chunk.toString().trim());
});
child.stderr.on('data', function forwardStdErr(chunk) {
child.stderr.on('data', chunk => {
debug('stderr', chunk.toString().trim());
});
});
Expand Down
24 changes: 11 additions & 13 deletions lib/steps/changelog.js
Original file line number Diff line number Diff line change
Expand Up @@ -44,19 +44,19 @@ function addPullRequestCommits(pkg, commits, pr) {
github.pull.get(pr.pullId),
github.pull.commits(pr.pullId),
])
.spread(function expandCommitInfo(info, prCommits) {
.spread((info, prCommits) => {
pr.author = {
name: info.user.login,
href: info.user.html_url,
};
pr.href = info.html_url;
pr.title = info.title || info.header;
const shas = (pr.shas = _.map(prCommits, 'sha'));
pr.commits = commits.filter(function isPartOfPR(commit) {
pr.commits = commits.filter(commit => {
return shas.indexOf(commit.sha) !== -1;
});
})
.catch(function handle404(err) {
.catch(err => {
if (err.statusCode !== 404) throw err;
// If the PR doesn't exist, handle it gracefully.
pr.commits = pr.shas = null;
Expand All @@ -65,7 +65,7 @@ function addPullRequestCommits(pkg, commits, pr) {

function removePRCommits(commits, prs) {
const prShas = _.flatten(_.map(prs, 'shas'));
return _.filter(commits, function isNotInAnyPR(commit) {
return _.filter(commits, commit => {
return commit.type !== 'pr' && prShas.indexOf(commit.sha) === -1;
});
}
Expand All @@ -74,16 +74,14 @@ function extractBreakingChanges(commit) {
if (!commit.notes || !commit.notes.length) {
return [];
}
return _.filter(commit.notes, { title: 'BREAKING CHANGE' }).map(
function buildChangeNote(note) {
return { text: note.text, commit: commit };
}
);
return _.filter(commit.notes, { title: 'BREAKING CHANGE' }).map(note => {
return { text: note.text, commit: commit };
});
}

function removeInvalidPRs(prs) {
// Warning: We're doing something evil here and mutate the input array.
const filtered = prs.filter(function allCommitsFound(pr) {
const filtered = prs.filter(pr => {
return pr.shas && pr.shas.length === pr.commits.length;
});
prs.length = filtered.length;
Expand Down Expand Up @@ -152,7 +150,7 @@ function generateChangeLog(cwd, pkg, options) {
}

function formatPR(pr) {
const changes = pr.commits.map(formatCommit).map(function withDashes(line) {
const changes = pr.commits.map(formatCommit).map(line => {
return ` - ${line}`;
});

Expand All @@ -172,7 +170,7 @@ function generateChangeLog(cwd, pkg, options) {
const changes = prs
.map(formatPR)
.concat(orphans.map(formatCommit))
.map(function star(line) {
.map(line => {
return `* ${line}`;
});

Expand All @@ -184,7 +182,7 @@ function generateChangeLog(cwd, pkg, options) {
.then(_.partial(removePRCommits, commits, prs))
.then(formatCommits)
.then(prependBreakingChanges)
.then(function setChangelog(changelog) {
.then(changelog => {
options.changelog = changelog;
return changelog;
});
Expand Down
2 changes: 1 addition & 1 deletion lib/steps/pending-changes.js
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ function normalizeReferences(meta, commit) {

function getPendingChanges(cwd, pkg, options) {
const meta = parseRepository(pkg.repository);
return getCommits(cwd, `v${pkg.version}`).then(function setCommits(commits) {
return getCommits(cwd, `v${pkg.version}`).then(commits => {
options.commits = commits.map(_.partial(normalizeReferences, meta));
});
}
Expand Down
8 changes: 4 additions & 4 deletions lib/steps/publish-to-npm.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ function checkPublishRequired(cwd, pkg, options) {
cwd: cwd,
env: options.npmEnv,
})
.then(function parseNpmList(content) {
.then(content => {
// If we get an empty response, we'll assume it was not found.
if (content.trim() === '') {
return 'publish';
Expand All @@ -87,7 +87,7 @@ function checkPublishRequired(cwd, pkg, options) {
}
return 'none';
})
.catch(function handle404(error) {
.catch(error => {
if (error.message.indexOf('ERR! 404') !== -1) {
return 'publish';
}
Expand Down Expand Up @@ -176,7 +176,7 @@ function publishToNPM(cwd, pkg, options) {
checkPublishRequired(cwd, pkg, options),
getCurrentCommit(cwd),
])
.spread(function checkAndPublish(publishRequired, currentCommit) {
.spread((publishRequired, currentCommit) => {
if (currentCommit !== `v${pkg.version}`) {
console.log(
'[nlm] Skipping publish, not a version commit:',
Expand Down Expand Up @@ -204,7 +204,7 @@ function publishToNPM(cwd, pkg, options) {
return null;
}
})
.finally(function removeTmpRcFile() {
.finally(() => {
fs.unlinkSync(rcFile);
});
}
Expand Down
28 changes: 13 additions & 15 deletions lib/steps/tag-pr.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,19 @@ function tagPullRequest(cwd, pkg, options) {
return null;
}
const github = Github.forRepository(pkg.repository);
return github.labels
.listByIssue(options.pr)
.then(function checkAndChangeLabels(labels) {
const releaseType = options.releaseType;
const name = `semver-${releaseType}`;
if (_.find(labels, { name: name })) {
debug('Already tagged with %j', name);
return null;
}
const newLabels = _.map(labels, 'name')
.filter(nonSemverTag)
.concat(name);
return github.labels.listByIssue(options.pr).then(labels => {
const releaseType = options.releaseType;
const name = `semver-${releaseType}`;
if (_.find(labels, { name: name })) {
debug('Already tagged with %j', name);
return null;
}
const newLabels = _.map(labels, 'name')
.filter(nonSemverTag)
.concat(name);

debug('Tagging %s', options.pr, newLabels);
return github.labels.setForIssue(options.pr, newLabels);
});
debug('Tagging %s', options.pr, newLabels);
return github.labels.setForIssue(options.pr, newLabels);
});
}
module.exports = tagPullRequest;
2 changes: 1 addition & 1 deletion lib/steps/version-commit.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,7 +85,7 @@ function createVersionCommit(cwd, pkg, options) {
return addFiles(cwd)
.then(_.partial(commit, cwd, `v${pkg.version}`))
.then(_.partial(getHEAD, cwd))
.then(function setVersionCommitSha(output) {
.then(output => {
options.versionCommitSha = output.trim();
return options.versionCommitSha;
});
Expand Down
Loading