Skip to content

Commit

Permalink
fix: prevent improper clobbering of man/bin links
Browse files Browse the repository at this point in the history
This uses the updated version of gentle-fs which has the binLink method,
so that we don't need to fork based on OS, and will always check to
ensure that the bin or cmd-shim is a reference into the folder being
linked.

Also perform a similar check on linked man pages.

Fix #11
  • Loading branch information
isaacs committed Dec 11, 2019
1 parent bc69419 commit 1bb7310
Show file tree
Hide file tree
Showing 4 changed files with 270 additions and 18 deletions.
30 changes: 21 additions & 9 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,9 @@
const path = require('path')
const fs = require('graceful-fs')
const BB = require('bluebird')
const linkIfExists = BB.promisify(require('gentle-fs').linkIfExists)
const cmdShimIfExists = BB.promisify(require('cmd-shim').ifExists)
const gentleFs = require('gentle-fs')
const linkIfExists = BB.promisify(gentleFs.linkIfExists)
const gentleFsBinLink = BB.promisify(gentleFs.binLink)
const open = BB.promisify(fs.open)
const close = BB.promisify(fs.close)
const read = BB.promisify(fs.read, {multiArgs: true})
Expand Down Expand Up @@ -42,9 +43,9 @@ function isHashbangFile (file) {
return read(fileHandle, Buffer.alloc(2), 0, 2, 0).spread((_, buf) => {
if (!hasHashbang(buf)) return []
return read(fileHandle, Buffer.alloc(2048), 0, 2048, 0)
}).spread((_, buf) => buf && hasCR(buf), () => false)
}).spread((_, buf) => buf && hasCR(buf), /* istanbul ignore next */ () => false)
.finally(() => close(fileHandle))
}).catch(() => false)
}).catch(/* istanbul ignore next */ () => false)
}

function hasHashbang (buf) {
Expand Down Expand Up @@ -109,18 +110,19 @@ function linkBins (pkg, folder, parent, gtop, opts) {
opts.log.showProgress()
}
}).catch(err => {
/* istanbul ignore next */
if (err.code === 'ENOENT' && opts.ignoreScripts) return
throw err
})
})
}

function linkBin (from, to, opts) {
if (process.platform !== 'win32') {
return linkIfExists(from, to, opts)
} else {
return cmdShimIfExists(from, to)
// do not clobber global bins
if (opts.globalBin && to.indexOf(opts.globalBin) === 0) {
opts.clobberLinkGently = true
}
return gentleFsBinLink(from, to, opts)
}

function linkMans (pkg, folder, parent, gtop, opts) {
Expand All @@ -132,17 +134,22 @@ function linkMans (pkg, folder, parent, gtop, opts) {
// make sure that the mans are unique.
// otherwise, if there are dupes, it'll fail with EEXIST
var set = pkg.man.reduce(function (acc, man) {
if (typeof man !== 'string') {
return acc
}
const cleanMan = path.join('/', man).replace(/\\|:/g, '/').substr(1)
acc[path.basename(man)] = cleanMan
return acc
}, {})
var manpages = pkg.man.filter(function (man) {
if (typeof man !== 'string') {
return false
}
const cleanMan = path.join('/', man).replace(/\\|:/g, '/').substr(1)
return set[path.basename(man)] === cleanMan
})

return BB.map(manpages, man => {
if (typeof man !== 'string') return
opts.log.silly('linkMans', 'preparing to link', man)
var parseMan = man.match(/(.*\.([0-9]+)(\.gz)?)$/)
if (!parseMan) {
Expand All @@ -165,6 +172,11 @@ function linkMans (pkg, folder, parent, gtop, opts) {

var manDest = path.join(manRoot, 'man' + sxn, bn)

// man pages should always be clobbering gently, because they are
// only installed for top-level global packages, so never destroy
// a link if it doesn't point into the folder we're linking
opts.clobberLinkGently = true

return linkIfExists(manSrc, manDest, getLinkOpts(opts, gtop && folder))
})
}
36 changes: 30 additions & 6 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

6 changes: 3 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@
"scripts": {
"prerelease": "npm t",
"postrelease": "npm publish && git push --follow-tags",
"pretest": "standard",
"posttest": "standard",
"release": "standard-version -s",
"test": "tap -J --nyc-arg=--all --coverage test/*.js",
"test": "tap -J --nyc-arg=--all --coverage test/*.js --100",
"update-coc": "weallbehave -o . && git add CODE_OF_CONDUCT.md && git commit -m 'docs(coc): updated CODE_OF_CONDUCT.md'",
"update-contrib": "weallcontribute -o . && git add CONTRIBUTING.md && git commit -m 'docs(contributing): updated CONTRIBUTING.md'"
},
Expand All @@ -30,7 +30,7 @@
"dependencies": {
"bluebird": "^3.5.3",
"cmd-shim": "^3.0.0",
"gentle-fs": "^2.0.1",
"gentle-fs": "^2.3.0",
"graceful-fs": "^4.1.15",
"npm-normalize-package-bin": "^1.0.0",
"write-file-atomic": "^2.3.0"
Expand Down
216 changes: 216 additions & 0 deletions test/link-bins.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,216 @@
const t = require('tap')
const BB = require('bluebird')
const binLinks = BB.promisify(require('../'))

// forking between cmd-shims and symlinks is already handled by
// the gentle-fs.binLink module. just test the unix handling here.
const _FAKE_PLATFORM_ = process.platform === 'win32' ? 'unix' : null

const fs = require('fs')
const mkdirp = require('mkdirp').sync
const rimraf = require('rimraf').sync
const {basename, resolve} = require('path')
const me = resolve(__dirname, basename(__filename, '.js'))
rimraf(me)
mkdirp(me)
const globalDir = resolve(me, 'node_modules')
t.teardown(() => rimraf(me))

const log = {
clearProgress () {},
info () {},
showProgress () {},
silly () {},
verbose () {}
}

// create some stuff that's already in the bin/man folders
const globalBin = resolve(me, 'bin')
mkdirp(globalBin)
fs.writeFileSync(globalBin + '/foo', 'pre-existing foo bin')
mkdirp(me + '/node_modules/bar/bin')
fs.writeFileSync(me + '/node_modules/bar/bin/bar.js', 'original bar')
fs.symlinkSync(me + '/node_modules/bar/bin/bar.js', me + '/bin/bar')

const prefixes = [
me,
globalBin,
globalDir,
resolve(me, 'share/man/man1')
]

mkdirp(me + '/share/man/man1')
fs.writeFileSync(me + '/share/man/man1/foo.1', 'pre-existing foo man')
mkdirp(me + '/node_modules/bar/man')
fs.writeFileSync(me + '/node_modules/bar/man/bar.1', 'original bar man')
fs.symlinkSync(me + '/node_modules/bar/man/bar.1', me + '/share/man/man1/bar.1')

t.test('foo package cannot link, pre-existing stuff there', t => {
const foo = resolve(me, 'node_modules/foo')
mkdirp(foo)
const pkg = {
name: 'foo',
version: '1.2.3',
bin: 'foo.js',
man: ['foo.1', { not: 'a manpage string' }]
}
fs.writeFileSync(foo + '/package.json', JSON.stringify(pkg))
fs.writeFileSync(foo + '/foo.1', 'how to foo\n')
fs.writeFileSync(foo + '/foo.js', '#!/usr/bin/env node\nconsole.log("foo")')

// might get it on the bin or the man, but one of these will EEXIST
return t.rejects(binLinks(pkg, foo, true, {
prefix: me,
global: true,
globalBin,
globalDir,
log,
pkgId: `${pkg.name}@${pkg.version}`,
name: pkg.name,
_FAKE_PLATFORM_,
prefixes: prefixes.concat(foo)
}), { code: 'EEXIST' }).then(() => {
// ensure we still have our preexisting bits
t.equal(fs.readFileSync(me + '/bin/foo', 'utf8'), 'pre-existing foo bin')
t.equal(fs.readFileSync(me + '/share/man/man1/foo.1', 'utf8'), 'pre-existing foo man')
})
})

t.test('foo package can link with --force link', t => {
const foo = resolve(me, 'node_modules/foo')
mkdirp(foo)
const pkg = {
name: 'foo',
version: '1.2.3',
bin: 'foo.js',
man: ['foo.1']
}
fs.writeFileSync(foo + '/package.json', JSON.stringify(pkg))
fs.writeFileSync(foo + '/foo.1', 'how to foo\n')
fs.writeFileSync(foo + '/foo.js', '#!/usr/bin/env node\nconsole.log("foo")')

return binLinks(pkg, foo, true, {
prefix: me,
global: true,
globalBin,
globalDir,
log,
pkgId: `${pkg.name}@${pkg.version}`,
name: pkg.name,
_FAKE_PLATFORM_,
force: true,
prefixes: prefixes.concat(foo)
}).then(() => {
// ensure we got our links made
t.equal(fs.readFileSync(me + '/bin/foo', 'utf8'), '#!/usr/bin/env node\nconsole.log("foo")')
if (process.platform !== 'win32') {
t.equal(fs.readFileSync(me + '/share/man/man1/foo.1', 'utf8'), 'how to foo\n')
}
})
})

t.test('bar package can update, links are ours', t => {
const bar = resolve(me, 'node_modules/bar')
mkdirp(bar)
const pkg = {
name: 'bar',
version: '1.2.3',
bin: 'bar.js',
man: ['bar.1']
}
fs.writeFileSync(bar + '/package.json', JSON.stringify(pkg))
fs.writeFileSync(bar + '/bar.1', 'how to bar\n')
fs.writeFileSync(bar + '/bar.js', '#!/usr/bin/env node\nconsole.log("bar")')

return binLinks(pkg, bar, true, {
prefix: me,
parseable: true,
global: true,
globalBin,
globalDir,
log,
pkgId: `${pkg.name}@${pkg.version}`,
name: pkg.name,
_FAKE_PLATFORM_,
prefixes: prefixes.concat(bar, bar + '/man')
}).then(() => {
// ensure we got our links made
t.equal(fs.readFileSync(me + '/bin/bar', 'utf8'), '#!/usr/bin/env node\nconsole.log("bar")')
if (process.platform !== 'win32') {
t.equal(fs.readFileSync(me + '/share/man/man1/bar.1', 'utf8'), 'how to bar\n')
}
})
})

t.test('cannot overwrite with another package with the same bin', t => {
const baz = resolve(me, 'node_modules/@scope/baz')
mkdirp(baz)
const pkg = {
name: '@scope/baz',
version: '1.2.3',
bin: { bar: 'baz.js' }
}
fs.writeFileSync(baz + '/package.json', JSON.stringify(pkg))
fs.writeFileSync(baz + '/baz.js', '#!/usr/bin/env node\nconsole.log("baz")')

return t.rejects(binLinks(pkg, baz, true, {
global: true,
prefix: me,
globalBin,
globalDir,
log,
pkgId: `${pkg.name}@${pkg.version}`,
_FAKE_PLATFORM_,
name: pkg.name,
prefixes: prefixes.concat(baz)
}), { code: 'EEXIST' }).then(() => {
// ensure bar is still intact
t.equal(fs.readFileSync(me + '/bin/bar', 'utf8'), '#!/usr/bin/env node\nconsole.log("bar")')
})
})

t.test('nothing to link', t => {
const qux = resolve(me, 'node_modules/qux')
mkdirp(qux)
const pkg = {
name: 'qux',
version: '1.2.3'
}
fs.writeFileSync(qux + '/package.json', JSON.stringify(pkg))

return binLinks(pkg, qux, true, {
global: true,
prefix: me,
globalBin,
globalDir,
log,
pkgId: `${pkg.name}@${pkg.version}`,
_FAKE_PLATFORM_,
name: pkg.name,
prefixes: prefixes.concat(qux)
})
})

t.test('invalid man page name', t => {
const badman = resolve(me, 'node_modules/badman')
mkdirp(badman)
const pkg = {
name: 'badman',
version: '1.2.3',
man: ['manpage']
}
fs.writeFileSync(badman + '/package.json', JSON.stringify(pkg))
fs.writeFileSync(badman + '/manpage', JSON.stringify(pkg))

return t.rejects(binLinks(pkg, badman, true, {
global: true,
prefix: me,
globalBin,
globalDir,
log,
pkgId: `${pkg.name}@${pkg.version}`,
_FAKE_PLATFORM_,
name: pkg.name,
prefixes: prefixes.concat(badman)
}), { message: 'manpage is not a valid name for a man file.' })
})

0 comments on commit 1bb7310

Please sign in to comment.