[pull] latest from npm:latest

lib/utils/queryable.js

@@ -2,6 +2,8 @@ const util = require('node:util')
 const _delete = Symbol('delete')
 const _append = Symbol('append')
 
+const FORBIDDEN_KEYS = new Set(['__proto__', 'constructor', 'prototype'])
+
 const sqBracketsMatcher = str => str.match(/(.+)\[([^\]]+)\]\.?(.*)$/)
 
 // replaces any occurrence of an empty-brackets (e.g: []) with a special Symbol(append) to represent it
@@ -122,6 +124,9 @@ const setter = ({ data, key, value, force }) => {
   // e.g: ['foo', 'bar', 'baz'] -> { foo: { bar: { baz:  {} } }
   const keys = parseKeys(key)
   const setKeys = (_data, _key) => {
+    if (FORBIDDEN_KEYS.has(String(_key))) {
+      throw Object.assign(new Error(`Forbidden key: "${_key}"`), { code: 'EFORBIDDENKEY' })
+    }
     // handles array indexes, converting valid integers to numbers
     // note that occurrences of Symbol(append) will throw so we just ignore these for now
     let maybeIndex = Number.NaN

test/lib/utils/queryable.js

@@ -963,3 +963,42 @@ t.test('bracket lovers', async t => {
     'any top-level item cannot be parsed with square bracket notation'
   )
 })
+
+t.test('forbidden keys', async t => {
+  // defensive cleanup in case any assertion below unexpectedly mutates the prototype
+  t.teardown(() => {
+    delete Object.prototype.scripts
+    delete Object.prototype.polluted
+  })
+
+  for (const key of ['__proto__', 'constructor', 'prototype']) {
+    t.throws(
+      () => new Queryable({ name: 'demo' }).set(`${key}.scripts.postinstall`, 'cmd'),
+      { code: 'EFORBIDDENKEY' },
+      `should throw EFORBIDDENKEY when '${key}' is used as a top-level key`
+    )
+  }
+
+  t.throws(
+    () => new Queryable({}).set('a.__proto__.scripts.postinstall', 'cmd'),
+    { code: 'EFORBIDDENKEY' },
+    'should throw when __proto__ appears nested in a dotted path'
+  )
+
+  t.throws(
+    () => new Queryable({}).set('foo[__proto__].scripts.postinstall', 'cmd'),
+    { code: 'EFORBIDDENKEY' },
+    'should throw when __proto__ is used via square bracket notation'
+  )
+
+  t.throws(
+    () => new Queryable({ scripts: { postinstall: 'x' } }).delete('__proto__.scripts'),
+    { code: 'EFORBIDDENKEY' },
+    'should block forbidden keys via delete() as well as set()'
+  )
+
+  t.notOk(
+    Object.prototype.scripts,
+    'Object.prototype must not be polluted by any forbidden-key attempt'
+  )
+})

workspaces/arborist/lib/install-scripts.js

@@ -1,4 +1,5 @@
 const { isNodeGypPackage } = require('@npmcli/node-gyp')
+const PackageJson = require('@npmcli/package-json')
 
 // Returns the install-relevant lifecycle scripts that would run for a
 // given arborist Node, or `{}` if there are none.
@@ -70,15 +71,40 @@ const getInstallScripts = async (node) => {
     collected.install = 'node-gyp rebuild'
   }
 
-  // Lockfile-only nodes (e.g. `npm ci` before reify) carry
-  // `hasInstallScript: true` but no enumerated scripts: the lockfile
-  // records the presence flag but never the script bodies. Without this
-  // fallback the strict-allow-scripts preflight would miss them entirely
-  // and let postinstall run. We can't recover the real script body
-  // without fetching the manifest, so emit a sentinel describing that
-  // install scripts are present.
+  // Lockfile-only nodes carry `hasInstallScript: true` but no enumerated
+  // scripts: the lockfile records the presence flag, not the script bodies,
+  // so `node.package.scripts` is empty on a lockfile-driven install (`npm ci`,
+  // a repeat `npm install`). Before giving up, read the installed
+  // package.json from disk to recover the real script bodies. Builder#addToBuildSet
+  // does the same disk read to decide what to run, but unlike that path this
+  // one is read-only: we never mutate `node.package`.
   if (Object.keys(collected).length === 0 && node.hasInstallScript === true) {
-    collected.install = '(install scripts present)'
+    const { content } = await PackageJson.normalize(node.path)
+      .catch(() => ({ content: {} }))
+    /* istanbul ignore next: normalize resolves to an object with a scripts
+       object, or our catch fallback returns {}; defensive guard only. */
+    const diskScripts = content?.scripts || {}
+
+    if (diskScripts.preinstall) {
+      collected.preinstall = diskScripts.preinstall
+    }
+    if (diskScripts.install) {
+      collected.install = diskScripts.install
+    }
+    if (diskScripts.postinstall) {
+      collected.postinstall = diskScripts.postinstall
+    }
+    if (diskScripts.prepare && hasNonRegistryShape(node)) {
+      collected.prepare = diskScripts.prepare
+    }
+
+    // Still nothing. The package isn't on disk yet (e.g. `npm ci` before
+    // reify) or its package.json is unreadable. Emit a sentinel so the
+    // advisory and the strict-allow-scripts preflight still surface that
+    // install scripts are present.
+    if (Object.keys(collected).length === 0) {
+      collected.install = '(install scripts present)'
+    }
   }
 
   return collected

workspaces/arborist/test/install-scripts.js

@@ -182,7 +182,7 @@ t.test('lockfile-only node with hasInstallScript=true emits a sentinel', async t
 
 t.test('sentinel is not emitted when scripts are already enumerated', async t => {
   // If `hasInstallScript: true` coexists with a real `scripts` map, we
-  // surface the real names — the sentinel must not overwrite them.
+  // surface the real names, and the sentinel must not overwrite them.
   const getInstallScripts = mockGetInstallScripts(t)
   const node = {
     resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz',
@@ -206,3 +206,178 @@ t.test('sentinel is not emitted when hasInstallScript is absent', async t => {
   }
   t.strictSame(await getInstallScripts(node), {})
 })
+
+t.test('lockfile-only node hydrates real scripts from package.json on disk', async t => {
+  // The common lockfile-driven case (`npm ci`, a repeat `npm install`):
+  // `node.package.scripts` is empty but the package is unpacked on disk.
+  // We must read the installed package.json and surface the real script
+  // body instead of the sentinel.
+  const getInstallScripts = mockGetInstallScripts(t)
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'pkg',
+      version: '1.0.0',
+      scripts: { postinstall: 'node -e "console.log(1)"' },
+    }),
+  })
+  const lockfileNode = {
+    resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz',
+    path,
+    isRegistryDependency: true,
+    hasInstallScript: true,
+    package: { name: 'pkg', version: '1.0.0' },
+  }
+  t.strictSame(
+    await getInstallScripts(lockfileNode),
+    { postinstall: 'node -e "console.log(1)"' }
+  )
+})
+
+t.test('lockfile-only node hydrates an explicit install script from disk', async t => {
+  // A native package such as fsevents that ships a prebuilt binary (no
+  // binding.gyp, so synthetic gyp detection finds nothing) but declares an
+  // explicit `install: node-gyp rebuild`. The disk read must recover it
+  // rather than emitting the sentinel.
+  const getInstallScripts = mockGetInstallScripts(t)
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'fsevents',
+      version: '2.3.3',
+      scripts: { install: 'node-gyp rebuild' },
+    }),
+  })
+  const lockfileNode = {
+    resolved: 'https://registry.npmjs.org/fsevents/-/fsevents-2.3.3.tgz',
+    path,
+    isRegistryDependency: true,
+    hasInstallScript: true,
+    package: { name: 'fsevents', version: '2.3.3' },
+  }
+  t.strictSame(
+    await getInstallScripts(lockfileNode),
+    { install: 'node-gyp rebuild' }
+  )
+})
+
+t.test('lockfile-only node hydrates a preinstall script from disk', async t => {
+  // Cover the disk `preinstall` recovery path.
+  const getInstallScripts = mockGetInstallScripts(t)
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'pkg',
+      version: '1.0.0',
+      scripts: { preinstall: 'node pre.js' },
+    }),
+  })
+  const lockfileNode = {
+    resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz',
+    path,
+    isRegistryDependency: true,
+    hasInstallScript: true,
+    package: { name: 'pkg', version: '1.0.0' },
+  }
+  t.strictSame(
+    await getInstallScripts(lockfileNode),
+    { preinstall: 'node pre.js' }
+  )
+})
+
+t.test('lockfile-only non-registry node hydrates prepare from disk', async t => {
+  // A git/file dependency installed from a lockfile: `prepare` runs for
+  // non-registry sources, so the disk read must recover it.
+  const getInstallScripts = mockGetInstallScripts(t)
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'pkg',
+      version: '1.0.0',
+      scripts: { prepare: 'node build.js' },
+    }),
+  })
+  const lockfileNode = {
+    resolved: 'git+ssh://git@github.com/foo/bar.git#abc',
+    path,
+    isRegistryDependency: false,
+    hasInstallScript: true,
+    package: { name: 'pkg', version: '1.0.0' },
+  }
+  t.strictSame(
+    await getInstallScripts(lockfileNode),
+    { prepare: 'node build.js' }
+  )
+})
+
+t.test('hydrated prepare is ignored for registry deps', async t => {
+  // Hydration reuses the same registry/non-registry boundary as the
+  // in-memory path: a registry dep whose on-disk package.json only has a
+  // `prepare` script falls through to the sentinel, since `prepare` never
+  // runs for registry installs.
+  const getInstallScripts = mockGetInstallScripts(t)
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'pkg',
+      version: '1.0.0',
+      scripts: { prepare: 'build' },
+    }),
+  })
+  const lockfileNode = {
+    resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz',
+    path,
+    isRegistryDependency: true,
+    hasInstallScript: true,
+    package: { name: 'pkg', version: '1.0.0' },
+  }
+  t.strictSame(
+    await getInstallScripts(lockfileNode),
+    { install: '(install scripts present)' }
+  )
+})
+
+t.test('falls back to sentinel when on-disk package.json has no install scripts', async t => {
+  // The lockfile flagged install scripts but the on-disk package.json has
+  // none we recognise (e.g. only lifecycle scripts like `test`). Keep the
+  // sentinel so the advisory still surfaces that something was flagged.
+  const getInstallScripts = mockGetInstallScripts(t)
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'pkg',
+      version: '1.0.0',
+      scripts: { test: 'tap' },
+    }),
+  })
+  const lockfileNode = {
+    resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz',
+    path,
+    isRegistryDependency: true,
+    hasInstallScript: true,
+    package: { name: 'pkg', version: '1.0.0' },
+  }
+  t.strictSame(
+    await getInstallScripts(lockfileNode),
+    { install: '(install scripts present)' }
+  )
+})
+
+t.test('disk hydration does not mutate node.package', async t => {
+  // The enumeration helper is read-only: recovering scripts from disk must
+  // not write them back onto the node (unlike Builder#addToBuildSet, which
+  // owns the node and hydrates it deliberately).
+  const getInstallScripts = mockGetInstallScripts(t)
+  const path = t.testdir({
+    'package.json': JSON.stringify({
+      name: 'pkg',
+      version: '1.0.0',
+      scripts: { postinstall: 'echo hi' },
+    }),
+  })
+  const pkg = { name: 'pkg', version: '1.0.0' }
+  const lockfileNode = {
+    resolved: 'https://registry.npmjs.org/pkg/-/pkg-1.0.0.tgz',
+    path,
+    isRegistryDependency: true,
+    hasInstallScript: true,
+    package: pkg,
+  }
+  await getInstallScripts(lockfileNode)
+  t.strictSame(pkg, { name: 'pkg', version: '1.0.0' }, 'node.package untouched')
+  t.notOk(pkg.scripts, 'no scripts written back onto node.package')
+})

workspaces/libnpmpublish/README.md

@@ -44,8 +44,13 @@ A couple of options of note:
   defaults to `latest`.
 
 * `opts.access` - tells the registry whether this package should be
-  published as `public` or `restricted`. Only applies to scoped
-  packages.  Defaults to `public`.
+  published as `'public'` or `'restricted'`. May also be `null`, which
+  preserves the existing access level on already-published packages and
+  defers to the registry's default for new packages (the registry treats
+  scoped packages as `restricted` and unscoped packages as `public` by
+  default). Only `'restricted'` and `null` are meaningful for scoped
+  packages; `'restricted'` is rejected for unscoped packages. Defaults to
+  `null`.
 
 * `opts.token` - can be passed in and will be used as the authentication
   token for the registry. For other ways to pass in auth details, see the

workspaces/libnpmpublish/lib/publish.js

@@ -23,7 +23,7 @@ Remove the 'private' field from the package.json to publish it.`),
   // spec is used to pick the appropriate registry/auth combo
   const spec = npa.resolve(manifest.name, manifest.version)
   opts = {
-    access: 'public',
+    access: null,
     algorithms: ['sha512'],
     defaultTag: 'latest',
     ...opts,

workspaces/libnpmpublish/test/publish.js

@@ -57,7 +57,7 @@ t.test('basic publish - no npmVersion', async t => {
         },
       },
     },
-    access: 'public',
+    access: null,
     _attachments: {
       'libnpmpublish-test-1.0.0.tgz': {
         content_type: 'application/octet-stream',
@@ -110,7 +110,7 @@ t.test('scoped publish', async t => {
         },
       },
     },
-    access: 'public',
+    access: null,
     _attachments: {
       '@npmcli/libnpmpublish-test-1.0.0.tgz': {
         content_type: 'application/octet-stream',
@@ -302,7 +302,7 @@ t.test('other error code', async t => {
   const packument = {
     name: 'libnpmpublish',
     description: 'some stuff',
-    access: 'public',
+    access: null,
     _id: 'libnpmpublish',
     'dist-tags': {
       latest: '1.0.0',
@@ -546,6 +546,7 @@ t.test('publish existing package with provenance in gha', async t => {
 
   const ret = await publish(manifest, tarData, {
     ...opts,
+    access: 'public',
     provenance: true,
     fulcioURL: fulcioURL,
     rekorURL: rekorURL,
@@ -766,7 +767,7 @@ t.test('user-supplied provenance - success', async t => {
         },
       },
     },
-    access: 'public',
+    access: null,
     _attachments: {
       '@npmcli/libnpmpublish-test-1.0.0.tgz': {
         content_type: 'application/octet-stream',
@@ -1091,6 +1092,7 @@ t.test('publish existing package with provenance in gitlab', async t => {
 
   const ret = await publish(manifest, tarData, {
     ...opts,
+    access: 'public',
     provenance: true,
     fulcioURL: fulcioURL,
     rekorURL: rekorURL,