[Bug?]: yarn berry showing extra dependency that yarn 1.22 doesn't show.

PhilipAbed commented 3 years ago

Self-service

[ ] I'd be willing to implement a fix

Describe the bug

1) "yarn set version berry" 2) i've done yarn install on my package.json i had in my package.json:

  "devDependencies": {
    "b": "^2.0.1",
    "v": "^0.3.0"
  }

according to the yarn.lock

"node-addon-api@npm:^1.7.1":
  version: 1.7.2
  resolution: "node-addon-api@npm:1.7.2"
  dependencies:
    node-gyp: latest
  checksum: 938922b3d7cb34ee137c5ec39df6289a3965e8cab9061c6848863324c21a778a81ae3bc955554c56b6b86962f6ccab2043dd5fa3f33deab633636bd28039333f
  languageName: node
  linkType: hard

but that doesn't make sense to me, in yarn1 before migration the same version was installed without dependencies! Old yarn.lock:

node-addon-api@^1.7.1:
  version "1.7.2"
  resolved "https://registry.yarnpkg.com/node-addon-api/-/node-addon-api-1.7.2.tgz#3df30b95720b53c24e59948b49532b662444f54d"
  integrity sha512-ibPK3iA+vaY1eEjESkQkM0BbCqFOaZMiXRTtdB0u7b4djtY6JnsjvPdUHVMg6xQt3B8fpTTWHI9A+ADjM9frzg==

now i checked this in npmjs just to make sure. https://www.npmjs.com/package/node-addon-api and i can see it has "0" dependencies! zero!.

To reproduce

await expect(
    packageJsonAndInstall({
        dependencies: {
            'node-addon-api': '1.7.2',
        },
    })
).resolves.not.toContain('node-gyp');

Environment

System: OS: Windows

Binaries: Yarn: 3.0.1 npm: 7.20.3 Node: v16.6.1

Additional context

No response

yarnbot commented 3 years ago

This issue reproduces on master:

Error: expect(received).resolves.not.toContain(expected) // indexOf

Expected substring: not "node-gyp"
Received string:        "➤ YN0000: ┌ Resolution step
::group::Resolution step
➤ YN0032: │ node-addon-api@npm:1.7.2: Implicit dependencies on node-gyp are discouraged
::endgroup::
➤ YN0000: └ Completed in 1s 888ms
➤ YN0000: ┌ Fetch step
::group::Fetch step
➤ YN0013: │ No packages were cached - 99 packages had to be fetched
::endgroup::
➤ YN0000: └ Completed in 1s 833ms
➤ YN0000: ┌ Link step
::group::Link step
::endgroup::
➤ YN0000: └ Completed in 0s 328ms
➤ YN0000: Done with warnings in 4s 76ms
"
    at Object.toContain (/github/workspace/.yarn/cache/expect-npm-24.8.0-8c7640c562-0c0da74930.zip/node_modules/expect/build/index.js:202:20)
    at module.exports (evalmachine.<anonymous>:8:16)
    at /github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.2-91650a2501-627bee24a7.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:56:19
    at executeInTempDirectory (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.2-91650a2501-627bee24a7.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:17:22)
    at Object.executeRepro (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.2-91650a2501-627bee24a7.zip/node_modules/@arcanis/sherlock/lib/executeRepro.js:24:18)
    at ExecCommand.execute (/github/workspace/.yarn/cache/@arcanis-sherlock-npm-2.0.2-91650a2501-627bee24a7.zip/node_modules/@arcanis/sherlock/lib/commands/exec.js:25:59)
    at async ExecCommand.validateAndExecute (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-91cf93ba72.zip/node_modules/clipanion/lib/advanced/Command.js:161:26)
    at async Cli.run (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-91cf93ba72.zip/node_modules/clipanion/lib/advanced/Cli.js:74:24)
    at async Cli.runExit (/github/workspace/.yarn/cache/clipanion-npm-2.0.0-rc.16-b9444aaf89-91cf93ba72.zip/node_modules/clipanion/lib/advanced/Cli.js:83:28)

merceyz commented 3 years ago

The dependency is injected here https://github.com/yarnpkg/berry/blob/863c48b4caf2af842746f48b1cca828b00ce93ee/packages/plugin-npm/sources/NpmSemverResolver.ts#L135-L147 See https://yarnpkg.com/advanced/error-codes/#yn0032---node_gyp_injected for the documentation for that behaviour, though in this case it's a false-positive

PhilipAbed commented 3 years ago

does that mean that if one of the sub-dependencies of that node_gyp is Vulnerable it will show in my vulnerability report? even if its a false positive? will this be fixed? or will it stay this way?

Kurt-von-Laven commented 2 years ago

@PhilipAbed, yes, CVE-2021-3807 exposed more publicly that Yarn v2 considers node-gyp a dependency of fsevents, but Yarn v1 and npm do not. As you say, the consequence was a false positive from security auditing tools because of a vulnerability in a transitive dependency of node-gyp. (See related discussion at facebook/jest#11939.) In addition to injecting node-gyp, Yarn maintains a centralized list of other implicit dependencies, which makes me wonder if one solution might be a comparable exception list to prevent injection of node-gyp in certain packages.

As for whether the security auditing consequence outweighs the "legacy reason" for injecting node-gyp, I don't have a clue and assume it was too complex to warrant elaboration in the linked document.

yarnpkg / berry