fix: secure and unixify paths discovered via directories.bin

wraithgar · wraithgar · commit a8b1cc9d7339 · 2025-07-15T12:58:22.000-07:00
Also tweaked logging statement to help w/ debugging
diff --git a/lib/index.js b/lib/index.js
@@ -113,7 +113,7 @@ class PackageJson {
     return p.prepare(opts)
   }
 
-  // read-package-json-fast.normalize compatible behavior (distinct from just read-package-json-fast
+  // read-package-json-fast compatible behavior
   static async normalize (path, opts) {
     const p = new PackageJson()
     await p.load(path)
diff --git a/lib/normalize.js b/lib/normalize.js
@@ -67,7 +67,7 @@ function normalizePackageBin (pkg, changes) {
           changes?.push(`"bin[${binKey}]" was renamed to "bin[${base}]"`)
         }
         if (binTarget !== pkg.bin[binKey]) {
-          changes?.push(`"bin[${base}]" script name was cleaned`)
+          changes?.push(`"bin[${base}]" script name ${binTarget} was invalid and removed`)
         }
         pkg.bin[base] = binTarget
       }
@@ -373,22 +373,19 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })
     normalizePackageMan(data, changes)
   }
 
-  if (steps.includes('bin') || steps.includes('binDir') || steps.includes('binRefs')) {
-    normalizePackageBin(data, changes)
-  }
-
   // expand "directories.bin"
   if (steps.includes('binDir') && data.directories?.bin && !data.bin) {
-    const binsDir = path.resolve(pkg.path, secureAndUnixifyPath(data.directories.bin))
-    const bins = await lazyLoadGlob()('**', { cwd: binsDir })
+    const binPath = secureAndUnixifyPath(data.directories.bin)
+    const bins = await lazyLoadGlob()('**', { cwd: path.resolve(pkg.path, binPath) })
     data.bin = bins.reduce((acc, binFile) => {
       if (binFile && !binFile.startsWith('.')) {
         const binName = path.basename(binFile)
-        acc[binName] = path.join(data.directories.bin, binFile)
+        // binPath is already cleaned and unixified, no need to path.join here.
+        acc[binName] = `${binPath}/${secureAndUnixifyPath(binFile)}`
       }
       return acc
     }, {})
-    // *sigh*
+  } else if (steps.includes('bin') || steps.includes('binDir') || steps.includes('binRefs')) {
     normalizePackageBin(data, changes)
   }
 
diff --git a/tap-snapshots/test/fix.js.test.cjs b/tap-snapshots/test/fix.js.test.cjs
@@ -29,7 +29,7 @@ exports[`test/fix.js TAP with changes binRefs scoped name > must match snapshot
 Array [
   "\\"bin\\" was converted to an object",
   "\\"bin[@npmcli/test-package]\\" was renamed to \\"bin[test-package]\\"",
-  "\\"bin[test-package]\\" script name was cleaned",
+  "\\"bin[test-package]\\" script name @npmcli/test-package was invalid and removed",
 ]
 `
 
diff --git a/test/fix.js b/test/fix.js
@@ -217,7 +217,7 @@ for (const [name, testFix] of Object.entries(testMethods)) {
     t.test('scriptpath', async t => {
       t.test('non-object scripts', async t => {
         const testdir = {
-          'package.json': pkg({ scripts: '@npmcil/test-package' }),
+          'package.json': pkg({ scripts: '@npmcli/test-package' }),
         }
         const { content } = await testFix(t, testdir)
         t.notHas(content, 'scripts')
@@ -263,17 +263,17 @@ for (const [name, testFix] of Object.entries(testMethods)) {
     t.test('binRefs', async t => {
       t.test('scoped name', async t => {
         const testdir = {
-          'package.json': pkg({ bin: '@npmcil/test-package' }),
+          'package.json': pkg({ bin: '@npmcli/test-package' }),
         }
         const { content } = await testFix(t, testdir)
-        t.strictSame(content.bin, { 'test-package': '@npmcil/test-package' })
+        t.strictSame(content.bin, { 'test-package': '@npmcli/test-package' })
       })
       t.test('array', async t => {
         const testdir = {
-          'package.json': pkg({ bin: ['@npmcil/test-package'] }),
+          'package.json': pkg({ bin: ['@npmcli/test-package'] }),
         }
         const { content } = await testFix(t, testdir)
-        t.strictSame(content.bin, { 'test-package': '@npmcil/test-package' })
+        t.strictSame(content.bin, { 'test-package': '@npmcli/test-package' })
       })
       t.test('no bin target', async t => {
         const testdir = {

Original file line number	Diff line number	Diff line change
`@@ -113,7 +113,7 @@ class PackageJson {`
`113`	`113`	`return p.prepare(opts)`
`114`	`114`	`}`
`115`	`115`
`116`		`- // read-package-json-fast.normalize compatible behavior (distinct from just read-package-json-fast`
	`116`	`+ // read-package-json-fast compatible behavior`
`117`	`117`	`static async normalize (path, opts) {`
`118`	`118`	`const p = new PackageJson()`
`119`	`119`	`await p.load(path)`
Original file line number	Diff line number	Diff line change
`@@ -67,7 +67,7 @@ function normalizePackageBin (pkg, changes) {`
`67`	`67`	changes?.push(`"bin[${binKey}]" was renamed to "bin[${base}]"`)
`68`	`68`	`}`
`69`	`69`	`if (binTarget !== pkg.bin[binKey]) {`
`70`		- changes?.push(`"bin[${base}]" script name was cleaned`)
	`70`	+ changes?.push(`"bin[${base}]" script name ${binTarget} was invalid and removed`)
`71`	`71`	`}`
`72`	`72`	`pkg.bin[base] = binTarget`
`73`	`73`	`}`
`@@ -373,22 +373,19 @@ const normalize = async (pkg, { strict, steps, root, changes, allowLegacyCase })`
`373`	`373`	`normalizePackageMan(data, changes)`
`374`	`374`	`}`
`375`	`375`
`376`		`- if (steps.includes('bin') \|\| steps.includes('binDir') \|\| steps.includes('binRefs')) {`
`377`		`- normalizePackageBin(data, changes)`
`378`		`- }`
`379`		`-`
`380`	`376`	`// expand "directories.bin"`
`381`	`377`	`if (steps.includes('binDir') && data.directories?.bin && !data.bin) {`
`382`		`- const binsDir = path.resolve(pkg.path, secureAndUnixifyPath(data.directories.bin))`
`383`		`- const bins = await lazyLoadGlob()('**', { cwd: binsDir })`
	`378`	`+ const binPath = secureAndUnixifyPath(data.directories.bin)`
	`379`	`+ const bins = await lazyLoadGlob()('**', { cwd: path.resolve(pkg.path, binPath) })`
`384`	`380`	`data.bin = bins.reduce((acc, binFile) => {`
`385`	`381`	`if (binFile && !binFile.startsWith('.')) {`
`386`	`382`	`const binName = path.basename(binFile)`
`387`		`- acc[binName] = path.join(data.directories.bin, binFile)`
	`383`	`+ // binPath is already cleaned and unixified, no need to path.join here.`
	`384`	+ acc[binName] = `${binPath}/${secureAndUnixifyPath(binFile)}`
`388`	`385`	`}`
`389`	`386`	`return acc`
`390`	`387`	`}, {})`
`391`		`- // sigh`
	`388`	`+ } else if (steps.includes('bin') \|\| steps.includes('binDir') \|\| steps.includes('binRefs')) {`
`392`	`389`	`normalizePackageBin(data, changes)`
`393`	`390`	`}`
`394`	`391`
Original file line number	Diff line number	Diff line change
@@ -29,7 +29,7 @@ exports[`test/fix.js TAP with changes binRefs scoped name > must match snapshot
`29`	`29`	`Array [`
`30`	`30`	`"\\"bin\\" was converted to an object",`
`31`	`31`	`"\\"bin[@npmcli/test-package]\\" was renamed to \\"bin[test-package]\\"",`
`32`		`- "\\"bin[test-package]\\" script name was cleaned",`
	`32`	`+ "\\"bin[test-package]\\" script name @npmcli/test-package was invalid and removed",`
`33`	`33`	`]`
`34`	`34`	`
`35`	`35`