Skip to content
This repository has been archived by the owner on Aug 11, 2022. It is now read-only.

Commit

Permalink
install: Refactor module validation for ealier/better checks
Browse files Browse the repository at this point in the history
This does a few things:

1. It fixes the calls to npm-install-checks– we were passing a
read-package-tree node, and it wants package.json data.

2. This moves checks to be at "install" time (resolveWithNewModule), so
they happen earlier.

3. Arguments passed in from the command line are checked even earlier.
This is where "don't install yourself" checks are done.

PR-URL: #9039
Fixes: #8637
Fixes: #8921
  • Loading branch information
iarna committed Jul 24, 2015
1 parent f0dd8bf commit b181fa3
Show file tree
Hide file tree
Showing 10 changed files with 352 additions and 56 deletions.
5 changes: 2 additions & 3 deletions lib/fetch-package-metadata.js
Original file line number Diff line number Diff line change
Expand Up @@ -172,18 +172,17 @@ module.exports.addShrinkwrap = function addShrinkwrap (pkg, next) {
}
if (er) return next(er)
var foundShrinkwrap = false
log.silly('addShrinkwrap', 'Adding shrinkwrap to ' + pkgname)
untar.on('entry', function (entry) {
if (!/^(?:[^\/]+[\/])npm-shrinkwrap.json$/.test(entry.path)) return
log.silly('addShrinkwrap', 'Found shrinkwrap at ' + entry.path)
log.silly('addShrinkwrap', 'Found shrinkwrap in ' + pkgname + ' ' + entry.path)
foundShrinkwrap = true
var shrinkwrap = ''
entry.on('data', function (chunk) {
shrinkwrap += chunk
})
entry.on('end', function () {
untar.close()
log.silly('addShrinkwrap', 'Done reading shrinkwrap')
log.silly('addShrinkwrap', 'Completed reading shrinkwrap in ' + pkgname)
try {
pkg._shrinkwrap = parseJSON(shrinkwrap)
} catch (ex) {
Expand Down
2 changes: 2 additions & 0 deletions lib/install.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ var checkPermissions = require('./install/check-permissions.js')
var decomposeActions = require('./install/decompose-actions.js')
var filterInvalidActions = require('./install/filter-invalid-actions.js')
var validateTree = require('./install/validate-tree.js')
var validateArgs = require('./install/validate-args.js')
var saveRequested = require('./install/save.js').saveRequested
var getSaveType = require('./install/save.js').getSaveType
var doSerialActions = require('./install/actions.js').doSerial
Expand Down Expand Up @@ -358,6 +359,7 @@ Installer.prototype.loadAllDepsIntoIdealTree = function (cb) {
var steps = []

if (installNewModules) {
steps.push([validateArgs, this.idealTree, this.args])
steps.push([loadRequestedDeps, this.args, this.idealTree, saveDeps, cg.newGroup('loadRequestedDeps')])
} else {
steps.push(
Expand Down
10 changes: 10 additions & 0 deletions lib/install/deps.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ var flatName = require('./flatten-tree.js').flatName
var createChild = require('./node.js').create
var resetMetadata = require('./node.js').reset
var andIgnoreErrors = require('./and-ignore-errors.js')
var isInstallable = require('./validate-args.js').isInstallable
var getPackageId = require('./get-package-id.js')

// The export functions in this module mutate a dependency tree, adding
// items to them.
Expand Down Expand Up @@ -381,6 +383,14 @@ function resolveWithNewModule (pkg, tree, log, next) {
}))
}

if (!pkg.installable) {
log.silly('resolveWithNewModule', getPackageId(pkg), 'checking installable status')
return isInstallable(pkg, iferr(next, function () {
pkg.installable = true
resolveWithNewModule(pkg, tree, log, next)
}))
}

if (!pkg._from) {
pkg._from = pkg._requested.name + '@' + pkg._requested.spec
}
Expand Down
44 changes: 44 additions & 0 deletions lib/install/validate-args.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
'use strict'
var validate = require('aproba')
var asyncMap = require('slide').asyncMap
var chain = require('slide').chain
var npmInstallChecks = require('npm-install-checks')
var checkEngine = npmInstallChecks.checkEngine
var checkPlatform = npmInstallChecks.checkPlatform
var npm = require('../npm.js')

module.exports = function (idealTree, args, next) {
validate('OAF', arguments)
var force = npm.config.get('force')

asyncMap(args, function (pkg, done) {
chain([
[checkSelf, idealTree, pkg, force],
[isInstallable, pkg]
], done)
}, next)
}

var isInstallable = module.exports.isInstallable = function (pkg, next) {
var force = npm.config.get('force')
var nodeVersion = npm.config.get('node-version')
var strict = npm.config.get('engine-strict')
chain([
[checkEngine, pkg, npm.version, nodeVersion, force, strict],
[checkPlatform, pkg, force]
], next)
}

function checkSelf (idealTree, pkg, force, next) {
if (idealTree.package.name !== pkg.name) return next()
if (force) {
var warn = new Error("Wouldn't install " + pkg.name + ' as a dependency of itself, but being forced')
warn.code = 'ENOSELF'
idealTree.warnings.push(warn)
next()
} else {
var er = new Error('Refusing to install ' + pkg.name + ' as a dependency of itself')
er.code = 'ENOSELF'
next(er)
}
}
82 changes: 33 additions & 49 deletions lib/install/validate-tree.js
Original file line number Diff line number Diff line change
@@ -1,12 +1,10 @@
'use strict'
var path = require('path')
var validate = require('aproba')
var npmInstallChecks = require('npm-install-checks')
var checkEngine = npmInstallChecks.checkEngine
var checkPlatform = npmInstallChecks.checkPlatform
var checkGit = npmInstallChecks.checkGit
var asyncMap = require('slide').asyncMap
var chain = require('slide').chain
var npmInstallChecks = require('npm-install-checks')
var checkGit = npmInstallChecks.checkGit
var clone = require('lodash.clonedeep')
var normalizePackageData = require('normalize-package-data')
var npm = require('../npm.js')
Expand All @@ -18,63 +16,49 @@ var getPackageId = require('./get-package-id.js')
module.exports = function (idealTree, log, next) {
validate('OOF', arguments)
var moduleMap = flattenTree(idealTree)
var force = npm.config.get('force')
var nodeVersion = npm.config.get('node-version')
var strict = npm.config.get('engine-strict')

var modules = Object.keys(moduleMap).map(function (name) { return moduleMap[name] })

asyncMap(modules, function (mod, done) {
chain([
[checkEngine, mod, npm.version, nodeVersion, force, strict],
[checkPlatform, mod, force],
mod.parent && !isInLink(mod) && [checkGit, mod.realpath],
[checkErrors, mod, idealTree]
chain([
[asyncMap, modules, function (mod, done) {
chain([
mod.parent && !mod.isLink && [checkGit, mod.realpath],
[checkErrors, mod, idealTree]
], done)
}, andValidateAllPeerDeps(idealTree, andCheckTop(idealTree, andFinishTracker(log, next))))
}

function isInLink (mod) {
if (!mod.parent) return false
if (mod.path !== mod.realpath) return true
return isInLink(mod.parent)
}],
[thenValidateAllPeerDeps, idealTree],
[thenCheckTop, idealTree]
], andFinishTracker(log, next))
}

function checkErrors (mod, idealTree, next) {
if (mod.error && (mod.parent || path.resolve(npm.globalDir, '..') !== mod.path)) idealTree.warnings.push(mod.error)
next()
}

function andValidateAllPeerDeps (idealTree, next) {
function thenValidateAllPeerDeps (idealTree, next) {
validate('OF', arguments)
return function (er) {
validateAllPeerDeps(idealTree, function (tree, pkgname, version) {
var warn = new Error(getPackageId(tree) + ' requires a peer of ' + pkgname + '@' +
version + ' but none was installed.')
warn.code = 'EPEERINVALID'
idealTree.warnings.push(warn)
})
next(er)
}
validateAllPeerDeps(idealTree, function (tree, pkgname, version) {
var warn = new Error(getPackageId(tree) + ' requires a peer of ' + pkgname + '@' +
version + ' but none was installed.')
warn.code = 'EPEERINVALID'
idealTree.warnings.push(warn)
})
next()
}

function andCheckTop (idealTree, next) {
function thenCheckTop (idealTree, next) {
validate('OF', arguments)
if (idealTree.package.error) {
return next
} else {
return function (er) {
// FIXME: when we replace read-package-json with something less magic,
// this should done elsewhere.
// As it is, the package has already been normalized and thus some
// errors are suppressed.
var pkg = clone(idealTree.package)
normalizePackageData(pkg, function (warn) {
var warnObj = new Error(getPackageId(idealTree) + ' ' + warn)
warnObj.code = 'EPACKAGEJSON'
idealTree.warnings.push(warnObj)
}, false)
next(er)
}
}
if (idealTree.package.error) return next()

// FIXME: when we replace read-package-json with something less magic,
// this should done elsewhere.
// As it is, the package has already been normalized and thus some
// errors are suppressed.
var pkg = clone(idealTree.package)
normalizePackageData(pkg, function (warn) {
var warnObj = new Error(getPackageId(idealTree) + ' ' + warn)
warnObj.code = 'EPACKAGEJSON'
idealTree.warnings.push(warnObj)
}, false)
next()
}
63 changes: 63 additions & 0 deletions test/tap/check-cpu-reqs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
'use strict'
var path = require('path')
var fs = require('fs')
var test = require('tap').test
var osenv = require('osenv')
var mkdirp = require('mkdirp')
var rimraf = require('rimraf')
var common = require('../common-tap.js')

var base = path.join(__dirname, path.basename(__filename, '.js'))
var installFrom = path.join(base, 'from')
var installIn = path.join(base, 'in')

var json = {
name: 'check-cpu-reqs',
version: '0.0.1',
description: 'fixture',
cpu: ['fake-cpu']
}

test('setup', function (t) {
setup()
t.end()
})

var INSTALL_OPTS = ['--loglevel', 'silly']
var EXEC_OPTS = {cwd: installIn}

test('install bad cpu', function (t) {
common.npm(['install', installFrom].concat(INSTALL_OPTS), EXEC_OPTS, function (err, code) {
t.ifError(err, 'npm ran without issue')
t.is(code, 1, 'npm install refused to install a package in itself')
t.end()
})
})
test('force install bad cpu', function (t) {
common.npm(['install', '--force', installFrom].concat(INSTALL_OPTS), EXEC_OPTS, function (err, code) {
t.ifError(err, 'npm ran without issue')
t.is(code, 0, 'npm install happily installed a package in itself with --force')
t.end()
})
})

test('cleanup', function (t) {
cleanup()
t.end()
})

function cleanup () {
process.chdir(osenv.tmpdir())
rimraf.sync(base)
}

function setup () {
cleanup()
mkdirp.sync(path.resolve(installFrom, 'node_modules'))
fs.writeFileSync(
path.join(installFrom, 'package.json'),
JSON.stringify(json, null, 2)
)
mkdirp.sync(path.resolve(installIn, 'node_modules'))
process.chdir(base)
}
65 changes: 65 additions & 0 deletions test/tap/check-engine-reqs.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
'use strict'
var path = require('path')
var fs = require('fs')
var test = require('tap').test
var osenv = require('osenv')
var mkdirp = require('mkdirp')
var rimraf = require('rimraf')
var common = require('../common-tap.js')

var base = path.join(__dirname, path.basename(__filename, '.js'))
var installFrom = path.join(base, 'from')
var installIn = path.join(base, 'in')

var json = {
name: 'check-engine-reqs',
version: '0.0.1',
description: 'fixture',
engines: {
node: '1.0.0-not-a-real-version'
}
}

test('setup', function (t) {
setup()
t.end()
})

var INSTALL_OPTS = ['--loglevel', 'silly']
var EXEC_OPTS = {cwd: installIn}

test('install bad engine', function (t) {
common.npm(['install', '--engine-strict', installFrom].concat(INSTALL_OPTS), EXEC_OPTS, function (err, code) {
t.ifError(err, 'npm ran without issue')
t.is(code, 1, 'npm install refused to install a package in itself')
t.end()
})
})
test('force install bad engine', function (t) {
common.npm(['install', '--engine-strict', '--force', installFrom].concat(INSTALL_OPTS), EXEC_OPTS, function (err, code) {
t.ifError(err, 'npm ran without issue')
t.is(code, 0, 'npm install happily installed a package in itself with --force')
t.end()
})
})

test('cleanup', function (t) {
cleanup()
t.end()
})

function cleanup () {
process.chdir(osenv.tmpdir())
rimraf.sync(base)
}

function setup () {
cleanup()
mkdirp.sync(path.resolve(installFrom, 'node_modules'))
fs.writeFileSync(
path.join(installFrom, 'package.json'),
JSON.stringify(json, null, 2)
)
mkdirp.sync(path.resolve(installIn, 'node_modules'))
process.chdir(base)
}
Loading

0 comments on commit b181fa3

Please sign in to comment.