Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions lib/utils/error-message.js
Original file line number Diff line number Diff line change
Expand Up @@ -364,6 +364,20 @@ const errorMessage = (er, npm) => {
break
}

// Append a dependency "why is this here" explanation for resolution errors
// that carry structured explain() data from Arborist (ETARGET,
// EALLOWREMOTE/EALLOWGIT, tarball/network failures, EINCOMPLETEMANIFEST,
// ESTRICTALLOWSCRIPTS, etc.). ERESOLVE does its own richer formatting in the
// switch above, so it is excluded here.
const hasExplanation = er.explanation &&
(Array.isArray(er.explanation) ? er.explanation.length > 0 : true)
if (hasExplanation && er.code !== 'ERESOLVE') {
const { report } = require('./explain-dep-error.js')
const { explanation, file } = report(er, npm.logChalk, npm.noColorChalk)
detail.push(['', explanation])
files.push([`${String(er.code).toLowerCase()}-report.txt`, file])
}

return {
summary,
detail,
Expand Down
51 changes: 51 additions & 0 deletions lib/utils/explain-dep-error.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
// Formats the structured dependency `explanation` attached to install errors
// (ETARGET, EALLOWREMOTE/EALLOWGIT, tarball/network, EINCOMPLETEMANIFEST,
// EBADENGINE, EBADPLATFORM, ESTRICTALLOWSCRIPTS) into a human-readable
// "why is this package here" report. Mirrors explain-eresolve.js and reuses
// the same `npm explain` formatters.
//
// Arborist attaches one of two shapes to `er.explanation`:
// - an edge-explanation object (from `edge.explain()`) describing the single
// dependency request that failed to resolve/fetch, or
// - a node-explanation object (or array of them, from `node.explain()`) for
// a package that resolved but is unusable/flagged (bad platform/engine,
// unreviewed install scripts).
const { explainEdge, explainNode } = require('./explain-dep.js')

// Normalize `er.explanation` (single object, array, or nothing) to a list.
const toList = (expl) => Array.isArray(expl) ? expl : expl ? [expl] : []

// An edge-explanation carries a top-level `spec`; a node-explanation does not.
const explainOne = (expl, depth, chalk) =>
'spec' in expl
? explainEdge(expl, depth, chalk)
: explainNode(expl, depth, chalk)

const explain = (expl, chalk, depth) =>
toList(expl).map(e => explainOne(e, depth, chalk)).join('\n\n')

// Generate a depth-limited explanation for the terminal plus a full
// (depth=Infinity) report written to the logs folder, just like ERESOLVE.
const report = (er, chalk, noColorChalk) => {
const list = toList(er.explanation)
if (!list.length) {
return { explanation: '', file: '' }
}
// Edge-shaped explanations are genuine resolution failures; node-shaped ones
// describe a package that resolved but is unusable, where the actionable
// context is "why is it here" (removing that path avoids the error).
const heading = 'spec' in list[0]
? 'Could not resolve dependency:'
: list.length > 1
? 'These packages are installed because:'
: 'This package is installed because:'
return {
explanation: `${heading}\n${explain(er.explanation, chalk, 4)}`,
file: `# npm ${er.code} error report\n\n${heading}\n${explain(er.explanation, noColorChalk, Infinity)}`,
}
}

module.exports = {
explain,
report,
}
25 changes: 7 additions & 18 deletions lib/utils/strict-allow-scripts-preflight.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const checkAllowScripts = require('./check-allow-scripts.js')
const { trustedDisplay } = require('@npmcli/arborist/lib/script-allowed.js')
const { strictAllowScriptsError } = require('@npmcli/arborist/lib/unreviewed-scripts.js')
const { configSetAllowScripts } = require('./allow-scripts-remediation.js')

// Pre-flight check for `--strict-allow-scripts`. Call after arborist has
Expand Down Expand Up @@ -38,16 +39,6 @@ const strictAllowScriptsPreflight = async ({ arb, npm, idealTreeOpts }) => {
return
}

const lines = unreviewed.map(({ node, scripts }) => {
const events = Object.entries(scripts)
.map(([event, body]) => `${event}: ${body}`)
.join('; ')
const name = node.package?.name || node.name
const version = node.package?.version || ''
const label = version ? `${name}@${version}` : name
return ` ${label} (${events})`
}).join('\n')

// `npm approve-scripts` / `npm deny-scripts` write to a project
// package.json, which doesn't exist for global installs. Point global
// users at the `--allow-scripts` flag and `npm config set allow-scripts`,
Expand All @@ -63,14 +54,12 @@ const strictAllowScriptsPreflight = async ({ arb, npm, idealTreeOpts }) => {
'`npm deny-scripts`, or bypass this check with ' +
'`--dangerously-allow-all-scripts`.'

throw Object.assign(
new Error(
`--strict-allow-scripts: ${unreviewed.length} package(s) have install ` +
`scripts not covered by allowScripts:\n${lines}\n` +
remediation
),
{ code: 'ESTRICTALLOWSCRIPTS' }
)
// Build via the shared arborist helper so the thrown error carries the
// structured dependency `explanation` (one node.explain() per unreviewed
// package). This lets the CLI show *which path* pulled each package in —
// removing that path is how a user avoids the prompt. The message produced
// here is identical to the inline form this previously threw.
throw strictAllowScriptsError(unreviewed, { remediation })
}

module.exports = strictAllowScriptsPreflight
27 changes: 27 additions & 0 deletions tap-snapshots/test/lib/utils/error-message.js.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -1109,6 +1109,33 @@ Object {
}
`

exports[`test/lib/utils/error-message.js TAP explain dependency resolution errors > must match snapshot 1`] = `
Object {
"detail": Array [
Array [
"notarget",
"In most cases you or one of your dependencies are requesting a package version that doesn't exist.",
],
Array [
"",
"explanation",
],
],
"files": Array [
Array [
"etarget-report.txt",
"report",
],
],
"summary": Array [
Array [
"notarget",
"no matching version found for foo@9.9.9",
],
],
}
`

exports[`test/lib/utils/error-message.js TAP just simple messages > must match snapshot 1`] = `
Object {
"detail": Array [],
Expand Down
172 changes: 172 additions & 0 deletions tap-snapshots/test/lib/utils/explain-dep-error.js.test.cjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,172 @@
/* IMPORTANT
* This snapshot file is auto-generated, but designed for humans.
* It should be checked into source control and tracked carefully.
* Re-generate by setting TAP_SNAPSHOT=1 and running tests.
* Make sure to inspect the output below. Do not ignore changes!
*/
'use strict'
exports[`test/lib/utils/explain-dep-error.js TAP explain depth > strict scripts array, full depth 1`] = `
has-install-script@1.2.3
node_modules/has-install-script
has-install-script@"^1.0.0" from the root project

nested-install-script@4.5.6
node_modules/parent/node_modules/nested-install-script
nested-install-script@"^4.0.0" from parent@7.0.0
node_modules/parent
parent@"^7.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP explain depth > transitive, depth 2 1`] = `
foo@"^9.9.9" from bar@1.0.0
node_modules/bar
bar@"^1.0.0" from baz@2.0.0
node_modules/baz
`

exports[`test/lib/utils/explain-dep-error.js TAP explain depth > transitive, full depth 1`] = `
foo@"^9.9.9" from bar@1.0.0
node_modules/bar
bar@"^1.0.0" from baz@2.0.0
node_modules/baz
dev baz@"^2.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report directEdge > report file (no color) 1`] = `
# npm ETARGET error report

Could not resolve dependency:
foo@"^9.9.9" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report directEdge > report with color 1`] = `
Could not resolve dependency:
foo@"^9.9.9" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report directEdge > report with no color 1`] = `
Could not resolve dependency:
foo@"^9.9.9" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report incompatibleNode > report file (no color) 1`] = `
# npm EBADPLATFORM error report

This package is installed because:
fsevents@2.3.3
node_modules/fsevents
fsevents@"^2.3.0" from chokidar@3.6.0
node_modules/chokidar
dev chokidar@"^3.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report incompatibleNode > report with color 1`] = `
This package is installed because:
fsevents@2.3.3
node_modules/fsevents
fsevents@"^2.3.0" from chokidar@3.6.0
node_modules/chokidar
dev chokidar@"^3.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report incompatibleNode > report with no color 1`] = `
This package is installed because:
fsevents@2.3.3
node_modules/fsevents
fsevents@"^2.3.0" from chokidar@3.6.0
node_modules/chokidar
dev chokidar@"^3.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report remoteEdge > report file (no color) 1`] = `
# npm EALLOWREMOTE error report

Could not resolve dependency:
sketchy@"github:foo/sketchy" from middle@3.1.4
node_modules/middle
middle@"^3.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report remoteEdge > report with color 1`] = `
Could not resolve dependency:
sketchy@"github:foo/sketchy" from middle@3.1.4
node_modules/middle
middle@"^3.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report remoteEdge > report with no color 1`] = `
Could not resolve dependency:
sketchy@"github:foo/sketchy" from middle@3.1.4
node_modules/middle
middle@"^3.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report strictScripts > report file (no color) 1`] = `
# npm ESTRICTALLOWSCRIPTS error report

These packages are installed because:
has-install-script@1.2.3
node_modules/has-install-script
has-install-script@"^1.0.0" from the root project

nested-install-script@4.5.6
node_modules/parent/node_modules/nested-install-script
nested-install-script@"^4.0.0" from parent@7.0.0
node_modules/parent
parent@"^7.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report strictScripts > report with color 1`] = `
These packages are installed because:
has-install-script@1.2.3
node_modules/has-install-script
has-install-script@"^1.0.0" from the root project

nested-install-script@4.5.6
node_modules/parent/node_modules/nested-install-script
nested-install-script@"^4.0.0" from parent@7.0.0
node_modules/parent
parent@"^7.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report strictScripts > report with no color 1`] = `
These packages are installed because:
has-install-script@1.2.3
node_modules/has-install-script
has-install-script@"^1.0.0" from the root project

nested-install-script@4.5.6
node_modules/parent/node_modules/nested-install-script
nested-install-script@"^4.0.0" from parent@7.0.0
node_modules/parent
parent@"^7.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report transitiveEdge > report file (no color) 1`] = `
# npm ETARGET error report

Could not resolve dependency:
foo@"^9.9.9" from bar@1.0.0
node_modules/bar
bar@"^1.0.0" from baz@2.0.0
node_modules/baz
dev baz@"^2.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report transitiveEdge > report with color 1`] = `
Could not resolve dependency:
foo@"^9.9.9" from bar@1.0.0
node_modules/bar
bar@"^1.0.0" from baz@2.0.0
node_modules/baz
dev baz@"^2.0.0" from the root project
`

exports[`test/lib/utils/explain-dep-error.js TAP report transitiveEdge > report with no color 1`] = `
Could not resolve dependency:
foo@"^9.9.9" from bar@1.0.0
node_modules/bar
bar@"^1.0.0" from baz@2.0.0
node_modules/baz
dev baz@"^2.0.0" from the root project
`
Loading
Loading