feat(arborist): warn on incomplete lockfile provenance

workspaces/arborist/lib/shrinkwrap.js

@@ -1222,6 +1222,34 @@ class Shrinkwrap {
 
     // This must be called before the lockfile conversion check below since it sets properties as part of `commit()`
     const json = this.toString(options)
+    if (!this.hiddenLockfile && this.tree) {
+      let missingProvenance = 0
+      for (const node of this.tree.root.inventory.values()) {
+        if (node.isProjectRoot || node.isLink || node.inBundle ||
+            !node.isRegistryDependency) {
+          continue
+        }
+        const meta = this.data.packages[relpath(this.path, node.path)]
+        // A registry version may resolve to a Git artifact named in its packument.
+        // Git sources do not carry registry tarball integrity metadata.
+        const resolvedSpec = meta?.resolved ? specFromResolved(meta.resolved) : null
+        if (resolvedSpec?.type === 'git' && resolvedSpec.gitCommittish) {
+          continue
+        }
+        if (meta && (!meta.integrity ||
+            (!this.resolveOptions.omitLockfileRegistryResolved && !meta.resolved))) {
+          missingProvenance++
+        }
+      }
+      if (missingProvenance) {
+        log.warn(
+          'shrinkwrap',
+          `Missing resolved or integrity metadata for ${missingProvenance} non-bundled registry ` +
+          `${missingProvenance === 1 ? 'dependency' : 'dependencies'}. ` +
+          'Review the lockfile before relying on it for reproducible installs.'
+        )
+      }
+    }
     if (
       !this.hiddenLockfile
       && this.originalLockfileVersion !== undefined

workspaces/arborist/test/arborist/build-ideal-tree.js

@@ -55,6 +55,12 @@ This is a one-time fix-up, please be patient...
 `,
 ]
 
+const missingProvenanceWarning = [
+  'warn',
+  'shrinkwrap',
+  'Missing resolved or integrity metadata for 1 non-bundled registry dependency. Review the lockfile before relying on it for reproducible installs.',
+]
+
 const cwd = normalizePath(process.cwd())
 t.cleanSnapshot = s => s.split(cwd).join('{CWD}')
 
@@ -1248,7 +1254,7 @@ t.test('no old lockfile warning with no package-lock', async t => {
   const registry = createRegistry(t, false)
   registry.audit()
   await newArb(fixt).reify()
-  t.strictSame(warnings, [])
+  t.strictSame(warnings, [missingProvenanceWarning])
 })
 
 t.test('no old lockfile warning with a conflict package-lock', async t => {
@@ -1275,7 +1281,7 @@ t.test('no old lockfile warning with a conflict package-lock', async t => {
   const registry = createRegistry(t, false)
   registry.audit()
   await newArb(fixt).reify()
-  t.strictSame(warnings, [])
+  t.strictSame(warnings, [missingProvenanceWarning])
 })
 
 t.test('override a conflict with the root dep (with force)', async t => {

workspaces/arborist/test/shrinkwrap-provenance.js

@@ -0,0 +1,270 @@
+const t = require('tap')
+const procLog = require('proc-log')
+const Node = require('../lib/node.js')
+const Link = require('../lib/link.js')
+const Arborist = require('../lib/index.js')
+const calcDepFlags = require('../lib/calc-dep-flags.js')
+const { readFile, writeFile, mkdir } = require('node:fs/promises')
+const { resolve } = require('node:path')
+
+const warnings = []
+const Shrinkwrap = t.mock('../lib/shrinkwrap.js', {
+  'proc-log': {
+    ...procLog,
+    log: { ...procLog.log, warn: (...args) => warnings.push(args) },
+  },
+})
+
+const resolved = 'https://registry.example/dep/-/dep-1.0.0.tgz'
+const integrity = 'sha512-dep'
+const getWarnings = () => warnings.splice(0)
+
+const makeTree = async (t, options = {}) => {
+  const dir = t.testdir()
+  const sw = await Shrinkwrap.load({
+    path: dir,
+    lockfileVersion: options.lockfileVersion || 3,
+    hiddenLockfile: options.hiddenLockfile,
+    resolveOptions: { omitLockfileRegistryResolved: options.omitResolved },
+  })
+  const root = new Node({
+    path: dir,
+    meta: sw,
+    pkg: {
+      name: 'root',
+      version: '1.0.0',
+      dependencies: { dep: options.spec || '1.0.0' },
+      ...(options.bundled ? { bundleDependencies: ['dep'] } : {}),
+    },
+  })
+  const dep = new Node({
+    pkg: { name: 'dep', version: '1.0.0' },
+    resolved: options.resolved,
+    integrity: options.integrity,
+    parent: root,
+  })
+  calcDepFlags(root)
+  sw.add(root)
+  return { root, dep, sw }
+}
+
+const assertWarning = (t, count = 1) => {
+  const messages = getWarnings()
+  t.equal(messages.length, 1, 'one aggregate warning')
+  t.equal(messages[0]?.[0], 'shrinkwrap')
+  t.match(messages[0]?.[1], `metadata for ${count} non-bundled registry`)
+  t.notMatch(messages[0]?.[1], /registry\.example|node_modules|sha512|root/,
+    'diagnostic contains no source or package data')
+}
+
+for (const lockfileVersion of [1, 2, 3, 4]) {
+  t.test(`public v${lockfileVersion} lockfile provenance warnings`, async t => {
+    const cases = [
+      [{}, 'both fields missing'],
+      [{ resolved }, 'integrity missing'],
+      [{ integrity }, 'resolved missing'],
+      [{ omitResolved: true }, 'integrity missing with resolved omitted'],
+    ]
+    for (const [options, name] of cases) {
+      await t.test(name, async t => {
+        const { sw } = await makeTree(t, { ...options, lockfileVersion })
+        const expected = sw.toString()
+        await sw.save()
+        assertWarning(t)
+        t.equal(await readFile(sw.filename, 'utf8'), expected,
+          'warning leaves serialized data unchanged')
+      })
+    }
+  })
+}
+
+t.test('complete and deliberately omitted metadata do not warn', async t => {
+  for (const options of [{ resolved, integrity }, { resolved, integrity, omitResolved: true }]) {
+    await t.test(JSON.stringify(options), async t => {
+      const { sw } = await makeTree(t, options)
+      await sw.save()
+      t.same(getWarnings(), [])
+      const data = JSON.parse(await readFile(sw.filename, 'utf8'))
+      t.equal(data.packages['node_modules/dep'].resolved,
+        options.omitResolved ? undefined : resolved)
+    })
+  }
+})
+
+t.test('multiple incomplete dependencies produce one warning', async t => {
+  const { sw, root } = await makeTree(t)
+  root.package = { ...root.package, dependencies: { dep: '1.0.0', other: '1.0.0' } }
+  new Node({ pkg: { name: 'other', version: '1.0.0' }, parent: root })
+  calcDepFlags(root)
+  await sw.save()
+  assertWarning(t, 2)
+})
+
+t.test('non-registry sources and bundled packages are excluded', async t => {
+  const cases = [
+    { spec: 'git+https://example.com/dep.git' },
+    { spec: 'file:../dep.tgz' },
+    { spec: 'file:../dep' },
+    { spec: 'https://example.com/dep.tgz' },
+    { bundled: true },
+  ]
+  for (const options of cases) {
+    await t.test(JSON.stringify(options), async t => {
+      const { sw } = await makeTree(t, options)
+      await sw.save()
+      t.ok(sw.data.packages['node_modules/dep'], 'excluded package is serialized')
+      t.same(getWarnings(), [])
+    })
+  }
+})
+
+t.test('registry version resolving to Git has no registry tarball warning', async t => {
+  const gitResolved = 'git+ssh://git@customgit.com:a/b/c.git#lkjadflkjasdf'
+  const { sw, dep } = await makeTree(t, {
+    spec: '1.0.0',
+    resolved: gitResolved,
+  })
+  t.equal(dep.isRegistryDependency, true, 'incoming version spec uses a registry')
+  await sw.save()
+  t.equal(sw.data.packages['node_modules/dep'].resolved, gitResolved)
+  t.notOk(sw.data.packages['node_modules/dep'].integrity,
+    'Git artifact has no registry tarball integrity')
+  t.same(getWarnings(), [], 'Git artifact is excluded from registry tarball warning')
+})
+
+t.test('registry version resolving to unpinned Git warns', async t => {
+  const gitResolved = 'git+ssh://git@customgit.com:a/b/c.git'
+  const { sw, dep } = await makeTree(t, {
+    spec: '1.0.0',
+    resolved: gitResolved,
+  })
+  t.equal(dep.isRegistryDependency, true, 'incoming version spec uses a registry')
+  await sw.save()
+  t.equal(sw.data.packages['node_modules/dep'].resolved, gitResolved)
+  t.notOk(sw.data.packages['node_modules/dep'].integrity,
+    'integrity is absent')
+  assertWarning(t)
+})
+
+t.test('registry version resolving to Git warns when serialized provenance is omitted', async t => {
+  const { sw, dep } = await makeTree(t, {
+    spec: '1.0.0',
+    resolved: 'git+ssh://git@customgit.com:a/b/c.git#lkjadflkjasdf',
+    omitResolved: true,
+  })
+  t.equal(dep.isRegistryDependency, true, 'incoming version spec uses a registry')
+  await sw.save()
+  t.notOk(sw.data.packages['node_modules/dep'].resolved,
+    'resolved source is omitted from serialized lockfile')
+  t.notOk(sw.data.packages['node_modules/dep'].integrity,
+    'integrity is also absent')
+  assertWarning(t)
+})
+
+t.test('workspace links and their targets are excluded', async t => {
+  const { sw, root, dep } = await makeTree(t)
+  dep.parent = null
+  const path = resolve(root.path, 'packages/dep')
+  root.workspaces = new Map([['dep', path]])
+  const target = new Node({ path, pkg: { name: 'dep', version: '1.0.0' }, root })
+  const link = new Link({ path: resolve(root.path, 'node_modules/dep'), target, parent: root })
+  calcDepFlags(root)
+  t.equal(link.isLink, true)
+  t.equal(target.isWorkspace, true)
+  await sw.save()
+  t.ok(sw.data.packages['packages/dep'])
+  t.ok(sw.data.packages['node_modules/dep'].link)
+  t.same(getWarnings(), [])
+})
+
+t.test('root and packages with no incoming spec are excluded', async t => {
+  const { sw, root, dep } = await makeTree(t)
+  root.package = { name: 'root', version: '1.0.0' }
+  calcDepFlags(root)
+  t.equal(dep.isRegistryDependency, false)
+  await sw.save()
+  t.ok(sw.data.packages['node_modules/dep'])
+  t.same(getWarnings(), [])
+})
+
+t.test('inert packages omitted from serialization do not warn', async t => {
+  const { sw, dep } = await makeTree(t)
+  dep.package = { name: 'dep' }
+  dep.inert = true
+  await sw.save()
+  t.notOk(sw.data.packages['node_modules/dep'])
+  t.same(getWarnings(), [])
+})
+
+t.test('hidden lockfiles and metadata-only saves do not warn', async t => {
+  const { sw } = await makeTree(t, { hiddenLockfile: true })
+  await mkdir(resolve(sw.path, 'node_modules'))
+  await sw.save()
+  t.ok(JSON.parse(await readFile(sw.filename, 'utf8')).packages['node_modules/dep'])
+  t.same(getWarnings(), [])
+  sw.hiddenLockfile = false
+  sw.tree = null
+  await sw.save()
+  t.same(getWarnings(), [])
+})
+
+t.test('root links use the project inventory', async t => {
+  const { sw, root } = await makeTree(t)
+  const link = new Link({ path: resolve(root.path, '../project-link'), realpath: root.path })
+  link.target = root
+  t.equal(root.root, link)
+  t.equal(root.inventory.size, 0)
+  t.ok(link.inventory.size > 0)
+  await sw.save()
+  assertWarning(t)
+})
+
+t.test('save errors are preserved', async t => {
+  t.throws(() => new Shrinkwrap().save(), Error('run load() before saving data'))
+  const { sw } = await makeTree(t)
+  sw.filename = resolve(sw.path, 'missing/package-lock.json')
+  await t.rejects(sw.save(), { code: 'ENOENT' })
+  assertWarning(t)
+})
+
+t.test('reify warns when rebuilding a lockfile from an installed tree', async t => {
+  const dir = t.testdir()
+  const project = resolve(dir, 'project')
+  await mkdir(resolve(project, 'node_modules/dep'), { recursive: true })
+  await writeFile(resolve(project, 'package.json'), JSON.stringify({
+    name: 'root',
+    version: '1.0.0',
+    dependencies: { dep: '1.0.0' },
+  }))
+  await writeFile(resolve(project, 'node_modules/dep/package.json'), JSON.stringify({
+    name: 'dep',
+    version: '1.0.0',
+  }))
+
+  const seen = []
+  const onlog = (...args) => args[0] === 'warn' && seen.push(args)
+  process.on('log', onlog)
+  try {
+    await new Arborist({
+      path: project,
+      cache: resolve(dir, 'cache'),
+      offline: true,
+      ignoreScripts: true,
+      audit: false,
+      fund: false,
+    }).reify()
+  } finally {
+    process.removeListener('log', onlog)
+  }
+
+  const lock = JSON.parse(await readFile(resolve(project, 'package-lock.json'), 'utf8'))
+  const dep = lock.packages['node_modules/dep']
+  t.ok(dep, 'the installed dependency is saved to the public lockfile')
+  t.equal(dep.resolved, undefined, 'the original source URL remains unknown')
+  t.equal(dep.integrity, undefined, 'the original integrity remains unknown')
+  t.equal(seen.length, 1, 'one aggregate warning reaches the reify caller')
+  if (seen[0]) {
+    t.same(seen[0].slice(0, 2), ['warn', 'shrinkwrap'])
+    t.match(seen[0][2], /Missing resolved or integrity metadata for 1 non-bundled registry dependency/)
+  }
+})