From 3d764d1846335fb67c870660501dc56484413647 Mon Sep 17 00:00:00 2001 From: kchindam-infy Date: Wed, 16 Oct 2024 12:34:02 -0700 Subject: [PATCH 1/4] fix: npm pack marks the wrong files as executable --- lib/util/is-package-bin.js | 23 +++++++++++++++++------ 1 file changed, 17 insertions(+), 6 deletions(-) diff --git a/lib/util/is-package-bin.js b/lib/util/is-package-bin.js index 49a3f73f..46d6e7d3 100644 --- a/lib/util/is-package-bin.js +++ b/lib/util/is-package-bin.js @@ -1,3 +1,4 @@ +const path = require('path').posix // Function to determine whether a path is in the package.bin set. // Used to prevent issues when people publish a package from a // windows machine, and then install with --no-bin-links. @@ -10,16 +11,26 @@ const binObj = (name, bin) => typeof bin === 'string' ? { [name]: bin } : bin -const hasBin = (pkg, path) => { +const hasBin = (pkg, filePath) => { const bin = binObj(pkg.name, pkg.bin) - const p = path.replace(/^[^\\/]*\//, '') - for (const kv of Object.entries(bin)) { - if (kv[1] === p) { + + // Remove 'package/' prefix if present + const relativeFilePath = filePath.startsWith('package/') + ? filePath.slice(8) + : filePath + + const normalizedFilePath = path.normalize(relativeFilePath); + + for (const binName in bin) { + const binPath = bin[binName] + const normalizedBinPath = path.normalize(binPath); + + if (normalizedFilePath === normalizedBinPath) { return true } } return false } -module.exports = (pkg, path) => - pkg && pkg.bin ? hasBin(pkg, path) : false +module.exports = (pkg, filePath) => + pkg && pkg.bin ? hasBin(pkg, filePath) : false From 189322894aa19e5c9022ddd21e0fbb2469b1232c Mon Sep 17 00:00:00 2001 From: kchindam-infy Date: Wed, 16 Oct 2024 12:40:29 -0700 Subject: [PATCH 2/4] lintfix --- lib/util/is-package-bin.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/util/is-package-bin.js b/lib/util/is-package-bin.js index 46d6e7d3..bb28f53f 100644 --- a/lib/util/is-package-bin.js +++ b/lib/util/is-package-bin.js @@ -19,11 +19,11 @@ const hasBin = (pkg, filePath) => { ? filePath.slice(8) : filePath - const normalizedFilePath = path.normalize(relativeFilePath); + const normalizedFilePath = path.normalize(relativeFilePath) for (const binName in bin) { const binPath = bin[binName] - const normalizedBinPath = path.normalize(binPath); + const normalizedBinPath = path.normalize(binPath) if (normalizedFilePath === normalizedBinPath) { return true From 5efc3efc634c8e379874aaa85a1d664ec8243edd Mon Sep 17 00:00:00 2001 From: kchindam-infy Date: Thu, 24 Oct 2024 13:11:06 -0700 Subject: [PATCH 3/4] added test case for faulty ispackagebin --- lib/util/is-package-bin.js | 4 +--- test/util/is-package-bin.js | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 3 deletions(-) diff --git a/lib/util/is-package-bin.js b/lib/util/is-package-bin.js index bb28f53f..beb26860 100644 --- a/lib/util/is-package-bin.js +++ b/lib/util/is-package-bin.js @@ -15,9 +15,7 @@ const hasBin = (pkg, filePath) => { const bin = binObj(pkg.name, pkg.bin) // Remove 'package/' prefix if present - const relativeFilePath = filePath.startsWith('package/') - ? filePath.slice(8) - : filePath + const relativeFilePath = path.relative('package', filePath) const normalizedFilePath = path.normalize(relativeFilePath) diff --git a/test/util/is-package-bin.js b/test/util/is-package-bin.js index 77bb9f08..ddb5ed0a 100644 --- a/test/util/is-package-bin.js +++ b/test/util/is-package-bin.js @@ -1,8 +1,32 @@ const t = require('tap') const isPackageBin = require('../../lib/util/is-package-bin.js') +const path = require('path').posix t.ok(isPackageBin({ bin: 'foo' }, 'package/foo'), 'finds string') t.ok(isPackageBin({ bin: { bar: 'foo' } }, 'package/foo'), 'finds in obj') t.notOk(isPackageBin(null, 'anything'), 'return false if pkg is not') t.notOk(isPackageBin({ bin: 'foo' }, 'package/bar'), 'not the bin string') t.notOk(isPackageBin({ bin: { bar: 'foo' } }, 'package/bar'), 'not in obj') + +t.test('bin file not recognized without prefix removal', t => { + const testPkg = { name: 'my-package', bin: 'bin/index.js' } + const testFilePath = 'package/bin/index.js' // includes 'package/' prefix + + // Simulate isPackageBin without prefix removal + const faultyIsPackageBin = (pkg, filePath) => { + const bin = typeof pkg.bin === 'string' ? { [pkg.name]: pkg.bin } : pkg.bin + const normalizedFilePath = path.normalize(filePath) // No prefix removal + for (const binPath of Object.values(bin)) { + const normalizedBinPath = path.normalize(binPath) + if (normalizedFilePath === normalizedBinPath) { + return true + } + } + return false + } + + t.notOk(faultyIsPackageBin(testPkg, testFilePath), + 'fails to recognize bin file without prefix removal') + t.ok(isPackageBin(testPkg, testFilePath), 'correctly recognizes bin file with prefix removal') + t.end() +}) From 3cf79cc57a9aa25a29d513af77a9116cb3b28470 Mon Sep 17 00:00:00 2001 From: kchindam-infy Date: Mon, 9 Dec 2024 04:11:13 -0800 Subject: [PATCH 4/4] fix: add optional prefix removal toggle for bin path resolution --- lib/util/is-package-bin.js | 21 ++++++--------------- lib/util/tar-create-options.js | 2 +- test/util/is-package-bin.js | 22 +++------------------- 3 files changed, 10 insertions(+), 35 deletions(-) diff --git a/lib/util/is-package-bin.js b/lib/util/is-package-bin.js index beb26860..dd7a8cf9 100644 --- a/lib/util/is-package-bin.js +++ b/lib/util/is-package-bin.js @@ -1,4 +1,3 @@ -const path = require('path').posix // Function to determine whether a path is in the package.bin set. // Used to prevent issues when people publish a package from a // windows machine, and then install with --no-bin-links. @@ -11,24 +10,16 @@ const path = require('path').posix const binObj = (name, bin) => typeof bin === 'string' ? { [name]: bin } : bin -const hasBin = (pkg, filePath) => { +const hasBin = (pkg, path, replace = true) => { const bin = binObj(pkg.name, pkg.bin) - - // Remove 'package/' prefix if present - const relativeFilePath = path.relative('package', filePath) - - const normalizedFilePath = path.normalize(relativeFilePath) - - for (const binName in bin) { - const binPath = bin[binName] - const normalizedBinPath = path.normalize(binPath) - - if (normalizedFilePath === normalizedBinPath) { + const p = replace ? path.replace(/^[^\\/]*\//, '') : path + for (const kv of Object.entries(bin)) { + if (kv[1] === p) { return true } } return false } -module.exports = (pkg, filePath) => - pkg && pkg.bin ? hasBin(pkg, filePath) : false +module.exports = (pkg, path, replace) => + pkg && pkg.bin ? hasBin(pkg, path, replace) : false diff --git a/lib/util/tar-create-options.js b/lib/util/tar-create-options.js index d070f0f7..b905de55 100644 --- a/lib/util/tar-create-options.js +++ b/lib/util/tar-create-options.js @@ -17,7 +17,7 @@ const tarCreateOptions = manifest => ({ // anything that is not a regular file, ignored by // .npmignore or package.json "files", etc. filter: (path, stat) => { - if (isPackageBin(manifest, path)) { + if (isPackageBin(manifest, path, false)) { stat.mode |= 0o111 } return true diff --git a/test/util/is-package-bin.js b/test/util/is-package-bin.js index ddb5ed0a..eae6095b 100644 --- a/test/util/is-package-bin.js +++ b/test/util/is-package-bin.js @@ -1,6 +1,5 @@ const t = require('tap') const isPackageBin = require('../../lib/util/is-package-bin.js') -const path = require('path').posix t.ok(isPackageBin({ bin: 'foo' }, 'package/foo'), 'finds string') t.ok(isPackageBin({ bin: { bar: 'foo' } }, 'package/foo'), 'finds in obj') @@ -10,23 +9,8 @@ t.notOk(isPackageBin({ bin: { bar: 'foo' } }, 'package/bar'), 'not in obj') t.test('bin file not recognized without prefix removal', t => { const testPkg = { name: 'my-package', bin: 'bin/index.js' } - const testFilePath = 'package/bin/index.js' // includes 'package/' prefix - - // Simulate isPackageBin without prefix removal - const faultyIsPackageBin = (pkg, filePath) => { - const bin = typeof pkg.bin === 'string' ? { [pkg.name]: pkg.bin } : pkg.bin - const normalizedFilePath = path.normalize(filePath) // No prefix removal - for (const binPath of Object.values(bin)) { - const normalizedBinPath = path.normalize(binPath) - if (normalizedFilePath === normalizedBinPath) { - return true - } - } - return false - } - - t.notOk(faultyIsPackageBin(testPkg, testFilePath), - 'fails to recognize bin file without prefix removal') - t.ok(isPackageBin(testPkg, testFilePath), 'correctly recognizes bin file with prefix removal') + const testFilePath = 'bin/index.js' // includes 'package/' prefix + t.ok(isPackageBin(testPkg, testFilePath, false), + 'correctly recognizes bin file with prefix removal') t.end() })