Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support breaking changes in NPM v7 #165

Closed
quinnturner opened this issue Oct 14, 2020 · 8 comments
Closed

Support breaking changes in NPM v7 #165

quinnturner opened this issue Oct 14, 2020 · 8 comments
Assignees
Labels
enhancement New feature or request

Comments

@quinnturner
Copy link
Member

With the release of NPM v7, the output of npm audit --json has changed significantly. Under the hood, audit-ci uses the --json flag.

Support for both NPM v7 and NPM v6 is critical.

For the full list of breaking changes, see below.

Breaking changes

Despite the massive overhaul to the internals of npm, the team has worked tirelessly to ensure that there will be minimal disruptions to most workflows. That said, some breaking changes are necessary to improve the overall developer experience. Breaking changes in npm 7.0.0 include:

  • Automatically installing peer dependencies (while this feature is something we think is desirable new behavior, it does potentially break certain workflows).
  • npm uses the package.exports field making it no longer possible to require() npm’s internal modules.
  • npx has been completely rewritten to use the npm exec command. There are various changes in functionality, most noticeable being a prompt if the module you are trying to run is not yet installed.
  • The output of npm audit has significantly changed both in the human-readable and --json output styles.

To learn more about the breaking changes in npm 7.0.0 please check out our in-depth post on the npmjs.com blog.

@quinnturner quinnturner added the enhancement New feature or request label Oct 14, 2020
@quinnturner
Copy link
Member Author

Also, NPM v7 can now use yarn.lock as a source of package metadata and resolution guidance:

package-lock v2 and support for yarn.lock: Our new package-lock format will unlock the ability to do deterministically reproducible builds and includes everything npm will need to fully build the package tree. Prior to npm 7 yarn.lock files were ignored, the npm cli can now use yarn.lock as source of package metadata and resolution guidance.

One test case that will have to be accounted for is NPM 7 with:

  • yarn.lock
  • no package-lock.json
  • Using the NPM registry for auditing

Currently, the behaviour for determining whether we use Yarn or NPM for auditing is:

  1. If a specific package manager is specified in the arguments, use that package manager
  2. If a package-lock.json is found, use NPM
  3. If an npm-shrinkwrap.json is found, use NPM
  4. If yarn.lock is found, use Yarn
  5. Else, throw error

Now, with NPM 7, audit-ci might be able to use the npm audit --json with yarn.lock. Further investigation is required.

@quinnturner quinnturner self-assigned this Oct 14, 2020
@quinnturner
Copy link
Member Author

For reference:

Here's an example of the new v7 audit (when using the existing package-lock.json format):
{
"auditReportVersion": 2,
"vulnerabilities": {
  "base64url": {
    "name": "base64url",
    "severity": "moderate",
    "via": [
      {
        "source": 658,
        "name": "base64url",
        "dependency": "base64url",
        "title": "Out-of-bounds Read",
        "url": "https://npmjs.com/advisories/658",
        "severity": "moderate",
        "range": "<3.0.0"
      }
    ],
    "effects": [],
    "range": "<3.0.0",
    "nodes": [
      "node_modules/base64url"
    ],
    "fixAvailable": {
      "name": "base64url",
      "version": "3.0.1",
      "isSemVerMajor": true
    }
  }
},
"metadata": {
  "vulnerabilities": {
    "info": 0,
    "low": 0,
    "moderate": 1,
    "high": 0,
    "critical": 0,
    "total": 1
  },
  "dependencies": {
    "prod": 1,
    "dev": 0,
    "optional": 0,
    "peer": 0,
    "peerOptional": 0,
    "total": 1
  }
}
}
Here's NPM v6:
{
  "actions": [
    {
      "isMajor": true,
      "action": "install",
      "resolves": [
        {
          "id": 658,
          "path": "base64url",
          "dev": false,
          "optional": false,
          "bundled": false
        }
      ],
      "module": "base64url",
      "target": "3.0.1"
    }
  ],
  "advisories": {
    "658": {
      "findings": [
        {
          "version": "2.0.0",
          "paths": [
            "base64url"
          ]
        }
      ],
      "id": 658,
      "created": "2018-05-16T19:16:37.120Z",
      "updated": "2018-05-16T19:17:38.115Z",
      "deleted": null,
      "title": "Out-of-bounds Read",
      "found_by": {
        "name": "Сковорода Никита Андреевич"
      },
      "reported_by": {
        "name": "Сковорода Никита Андреевич"
      },
      "module_name": "base64url",
      "cves": [],
      "vulnerable_versions": "<3.0.0",
      "patched_versions": ">=3.0.0",
      "overview": "Versions of `base64url` before 3.0.0 are vulnerable to to out-of-bounds reads as it allocates uninitialized Buffers when number is passed in input on Node.js 4.x and below.",
      "recommendation": "Update to version 3.0.0 or later.",
      "references": "- [HackerOne Report](https://hackerone.com/reports/321687)\n- [PR #25](https://github.com/brianloveswords/base64url/pull/25\")",
      "access": "public",
      "severity": "moderate",
      "cwe": "CWE-125",
      "metadata": {
        "module_type": "",
        "exploitability": 2,
        "affected_components": ""
      },
      "url": "https://npmjs.com/advisories/658"
    }
  },
  "muted": [],
  "metadata": {
    "vulnerabilities": {
      "info": 0,
      "low": 0,
      "moderate": 1,
      "high": 0,
      "critical": 0
    },
    "dependencies": 1,
    "devDependencies": 0,
    "optionalDependencies": 0,
    "totalDependencies": 1
  },
  "runId": "420e00ad-8e08-4850-bc69-3840e7581217"
}

@quinnturner
Copy link
Member Author

quinnturner commented Oct 14, 2020

Consider integrating https://github.com/npm/arborist for auditing.

Here's how NPM handles auditing in their CLI. It might be useful to follow a similar manner.
const audit = async args => {
  const arb = new Arborist({
    ...npm.flatOptions,
    audit: true,
    path: npm.prefix
  })
  const fix = args[0] === 'fix'
  await arb.audit({ fix })
  if (fix) {
    reifyOutput(arb)
  } else {
    const reporter = npm.flatOptions.json ? 'json' : 'detail'
    const result = auditReport(arb.auditReport, {
      ...npm.flatOptions,
      reporter
    })
    process.exitCode = process.exitCode || result.exitCode
    output(result.report)
  }
}

The main advantage of using arborist directly is that we bypass a cycle of serialization of the audit response.

@sricks
Copy link

sricks commented May 11, 2021

Hi, thanks very much for this package! 😁

I see that you've released a new version 8 days ago to support npm7, but I'm having issues:

$ npm exec audit-ci -p npm
audit-ci version: 4.0.0
NPM audit report results:
{
  "advisories": {},
  "metadata": {
    "vulnerabilities": {
      "info": 0,
      "low": 0,
      "moderate": 80,
      "high": 0,
      "critical": 0,
      "total": 80
    },
    "dependencies": {
      "prod": 2127,
      "dev": 31,
      "optional": 31,
      "peer": 0,
      "peerOptional": 0,
      "total": 2188
    }
  }
}
Cannot create property 'paths' on string 'postcss'
Exiting...

$ node -v
v14.16.0
$ npm -v
7.12.1

Any thoughts? Downgrading to npm 6 makes everything work as expected.

@sricks
Copy link

sricks commented May 11, 2021

By the way, the npm advisory causing this issue is https://www.npmjs.com/advisories/1693

https://uko.codes/dealing-with-npm-v7-audit-changes

Based on this blog post, I'm thinking it's probably caused by the via array - in this case, it's probably just the string postcss instead of an advisory object.

@sricks
Copy link

sricks commented Oct 7, 2021

I'm not sure if this was fixed by a PR, or why this issue was closed, but this package is working for me today on the same repository.

npx audit-ci --critical

@quinnturner
Copy link
Member Author

Hi @sricks, let's open a fresh issue to capture the paths error. This got closed when, on another PR, I said: "Closes #PR#", since NPM 7 support was released.

@sricks
Copy link

sricks commented Oct 12, 2021

@quinnturner Thanks, although I don't have a reproduction case now. If I run into the issue again, I will file a new issue!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants