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

[BUG] "license" key in lockfile is never populated #5532

Closed
2 tasks done
robcresswell opened this issue Sep 19, 2022 · 15 comments
Closed
2 tasks done

[BUG] "license" key in lockfile is never populated #5532

robcresswell opened this issue Sep 19, 2022 · 15 comments
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 8.x work is associated with a specific npm 8 release ws:arborist Related to the arborist workspace

Comments

@robcresswell
Copy link

Is there an existing issue for this?

  • I have searched the existing issues

This issue exists in the latest npm version

  • I am using the latest npm

Current Behavior

npm v8.19.2 does not populate the "license" key in a v3 package-lock.json, which appears to be documented here: https://docs.npmjs.com/cli/v8/configuring-npm/package-lock-json#packages

It's unclear to me whether that key is specific to the root package (under the "" key), or whether it should be populated for all keys.

Expected Behavior

I'd expect to the see npm's license information, judging by the docs

Steps To Reproduce

  1. MacOS, Node 18.9.0, npm 8.19.2

  2. package.json

    {
      "name": "lock-v3-fixture",
      "version": "1.0.0",
      "description": "",
      "main": "index.js",
      "private": true,
      "dependencies": {
        "@types/react": "^18.0.9"
      },
      "license": "MIT"
    }
    

    package-lock.json

    {
      "name": "lock-v3-fixture",
      "version": "1.0.0",
      "lockfileVersion": 3,
      "requires": true,
      "packages": {
        "": {
          "name": "lock-v3-fixture",
          "version": "1.0.0",
          "license": "MIT",
          "dependencies": {
            "@types/react": "^18.0.9"
          }
        },
        "node_modules/@types/prop-types": {
          "version": "15.7.5",
          "resolved": "https://registry.npmjs.org/@types/prop-types/-/prop-types-15.7.5.tgz",
          "integrity": "sha512-JCB8C6SnDoQf0cNycqd/35A7MjcnK+ZTqE7judS6o7utxUCg6imJg3QK2qzHKszlTjcj2cn+NwMB2i96ubpj7w=="
        },
        "node_modules/@types/react": {
          "version": "18.0.20",
          "resolved": "https://registry.npmjs.org/@types/react/-/react-18.0.20.tgz",
          "integrity": "sha512-MWul1teSPxujEHVwZl4a5HxQ9vVNsjTchVA+xRqv/VYGCuKGAU6UhfrTdF5aBefwD1BHUD8i/zq+O/vyCm/FrA==",
          "dependencies": {
            "@types/prop-types": "*",
            "@types/scheduler": "*",
            "csstype": "^3.0.2"
          }
        },
        "node_modules/@types/scheduler": {
          "version": "0.16.2",
          "resolved": "https://registry.npmjs.org/@types/scheduler/-/scheduler-0.16.2.tgz",
          "integrity": "sha512-hppQEBDmlwhFAXKJX2KnWLYu5yMfi91yazPb2l+lbJiwW+wdo1gNeRA+3RgNSO39WYX2euey41KEwnqesU2Jew=="
        },
        "node_modules/csstype": {
          "version": "3.1.1",
          "resolved": "https://registry.npmjs.org/csstype/-/csstype-3.1.1.tgz",
          "integrity": "sha512-DJR/VvkAvSZW9bTouZue2sSxDwdTN92uHjqeKVm+0dAqdfNykRzQ95tay8aXMBAAPpUiq4Qcug2L7neoRh2Egw=="
        }
      }
    }
    

Environment

  • npm: 8.19.2
  • Node.js: 18.9.0
  • OS Name: MacOS Monterey 12.6
@robcresswell robcresswell added Bug thing that needs fixing Needs Triage needs review for next steps Release 8.x work is associated with a specific npm 8 release labels Sep 19, 2022
@fritzy fritzy added Priority 2 secondary priority issue ws:arborist Related to the arborist workspace and removed Needs Triage needs review for next steps labels Sep 22, 2022
@amw
Copy link

amw commented Oct 11, 2022

I can confirm this issue. When I issue npm install all the license fields are removed from my lock file. This prevents me from updating my project as I cannot lose information about licenses my dependencies are using.

@robcresswell
Copy link
Author

Hey @fritzy is there some way I can chase this up? It seems like a pretty huge bug if my understanding is correct, but I'm unsure. I'd love to know if this is really as broken as it seems or if I'm misunderstanding the usage

@amw
Copy link

amw commented Dec 6, 2022

I was trying to investigate when has license field stopped being added to package-lock.json and could not downgrade to npm/node version that still had it. I went as far back as:

$ npm --version
6.14.17
$ node --version
v14.21.1

I've also tried node 16 and npm 7.0.0, 7.10, 7.20. Still the lockfile after npm install did not contain license keys.

I have no idea why the license fields used to be populated (I clearly have them in my repo history) or why they no longer are.

@robcresswell
Copy link
Author

robcresswell commented Dec 7, 2022

@amw Yeah, we have similar fixture data in Syft and cannot recreate it. I was really hoping someone from the npm side would be able to drop by and fill in the details here, but its been nearly 3 months so it doesn't sound like this is high priority. Maybe it was only in a particular npm version by accident.

@HeikoStudt
Copy link

HeikoStudt commented Jul 20, 2023

I am very confused by this finding which may be a workaround:

I have added "npm" as a (normal) dependency into my package.json (and removed node_modules/* and package-lock.json).

Then, In my package-lock.json file I got "license" entries for npm/node_modules/* in the first iteration. However, if you are reiterating npm install, it creates license entries for all the package-lock.json entries as well...?

(in my tests I removed both package-lock.json and the one in node_modules. I do not have any shrink or yarn lock file. I was both using npm 9.5.1 on a debian with the dependency version "~9.5.1".

I do not understand why this is happening. Can anyone else reproduce this workaround?

@HeikoStudt
Copy link

TL;DR: there is a bug in the npm install cli. Sometimes the licenses are written into the package-lock.json and sometimes not, so this is some weird timing or caching anomaly.

The problematic part is that the package-lock.json and node_modules/.package-lock.json files seem to make things worse by cementing the bad status until both being removed.

Yesterday, the workaround one posting back was working like 100%. Now, it does not anymore. After several times doing rm node_modules/.package-lock.json && rm package-lock.json && npm install it got the licenses again, even after removing NPM as a dependency.

My current hypotheses is that this may be an effect of cached IO and Promises not waited for. So, I tried with a removed node_modules/* and package-lock.json and waiting quite some time, so that the IO cache is surely be cleaned. In the first try, I could reproduce not getting the licenses.
After removing both package-lock files and redo npm install that information came back.

I hope these descriptions are helping someone with code knowledge to formulate a better hypotheses on what is actually happening.

@HeikoStudt
Copy link

Using https://www.baeldung.com/linux/empty-buffer-cache I could reproduce it (from my current point of view - hey loving timing!):

rm ../pl-first.json ../pl-second.json
rm -rf node_modules/* &&  rm package-lock.json
sudo bash -c "free && sync && echo 3 > /proc/sys/vm/drop_caches && free"
npm install
cp package-lock.json ../pl-first.json
rm -rf node_modules/.package-lock.json &&  rm package-lock.json
npm install
cp package-lock.json ../pl-second.json
meld ../pl-first.json ../pl-second.json

Another thing which might help in the bug search is the actual difference which tells that "resolved" and "integrity" is replaced with the "license", so we need to find the place where this is done.

4748,4749d4034
<       "resolved": "https://registry.npmjs.org/yocto-queue/-/yocto-queue-0.1.0.tgz",
<       "integrity": "sha512-rVksvsnNCdJ/ohGc6xgPwyN8eheCxsiLM8mxuE/t/mOVqJewPuO1miLpTHQiRgTKCLexL4MeAFVagts7HmNZ2Q==",
4750a4036
>       "license": "MIT",
4760,4761c4046
<       "resolved": "https://registry.npmjs.org/zrender/-/zrender-5.4.4.tgz",
<       "integrity": "sha512-0VxCNJ7AGOMCWeHVyTrGzUgrK4asT4ml9PEkeGirAkKNYXYzoPJCLvmyfdoOXcjTHPs10OZVMfD1Rwg16AZyYw==",
---
>       "license": "BSD-3-Clause",

@HeikoStudt
Copy link

I have tried the script above on a very small package.json (containing only luxon) and could reproduce the behavior and difference in the package-lock.json. every single time.

What is happening:

  • At the beginning, there is no node_modules directory
  • npm install creates the tree without considering node_modules directory (I do not understand where the recursive dependencies come from yet)
  • npm saves the two package-lock.json having the resolve information but without reading the actual package.json inside node_modules
  • From then on, npm uses the lock-file instead of reify the data from the node_module's package.json files

If I remove the two lock files, then this happens:

  • reify finds the package locally inside node_modules and reads the package.json from there
  • The lock contains the license information but not the resolve/integrity information as the package was found locally inside the node_modules directory

Why did the npm dependency help with the sub-dependencies?

  • As I have got npm installed, the sub-dependencies are found in the global node_modules and therefore the local package.json file got read

What was happening with your mumbling about IO cache, timing etc?

  • This was probably clutter from my IDEs, editors, system etc which locked, let some file stay alive, or something else. I am currently working with a very small-scale package.json having the same problem consistently. If there was anything more than clutter, it should be a separate bug.

Conclusion: We need to find out, why the license field is not found/used if loaded from the registry.

@HeikoStudt
Copy link

HeikoStudt commented Jul 21, 2023

TL;DR: Real Workaround included in last line!

Ok, I have found the culprit:

npmcli is using pacote to resolve / download the manifest (package.json) file. In pacote there are two different accept headers possible:

// Corgis are cute. 🐕🐶
const corgiDoc = 'application/vnd.npm.install-v1+json; q=1.0, application/json; q=0.8, */*'
const fullDoc = 'application/json'

EDIT: the corgi mime type declares "v1" which seems to resolve into manifest version 1 which is possibly filtered server-side to suffice this format. If I put v7, I get a transformed format including the "license" metadata.

This is influenced by two options: https://github.com/npm/pacote/blob/53cf17e02f54cb620de013081be063f24573d6ce/lib/fetcher.js#L93

According to that, we need one of the options fullMetadata or before set.
Luckily, "before" is some kind of configuration for NPM and allows some date.

# rm package-lock.json node_modules/.package-lock.json
# so that resolve/integrity are stored in the lock file:
# rm -r node_modules/* 
npm install --before=2050-01-01

@HeikoStudt
Copy link

The following are the reproduction steps for NPM:

  1. Simple package.json file (does not matter)
  2. Run npm install luxon
  3. package-lock.json contains NO "license" information, contains also integrity + resolved
  4. rm package-lock.json node_modules/.package-lock.json
  5. npm install
  6. package-lock.json contains "license" information, contains NO integrity + resolved
  7. rm -r package-lock.json node_modules/
  8. npm install --before=2050-01-01
  9. package-lock.json contains "license" information, contains also integrity + resolved

The easiest fix locally in npmcli is setting "fullMetadata" for any pacote call. However, this results into way more data to be downloaded from npmjs.com.
A more sophisticated approach is to create a new accept variant to be implemented by npmjs.com and pacote, e.g. application/vnd.npm.install-v2+json. However, every alternative registry implementation needs to understand this as well or default to the full metadata.

@fritzy This probably needs a decision/discussion within npm, can you please point the correct persons to this?

@robcresswell
Copy link
Author

Thanks for the investigation and workaround @HeikoStudt

Unfortunately it doesn't seem like the npm folks have much interest in acknowledging this issue 😞

@ljharb
Copy link
Contributor

ljharb commented Nov 23, 2023

Does this still occur in npm v9 and v10?

@HeikoStudt
Copy link

HeikoStudt commented May 5, 2024

EDIT: English (ups :))
Now I have tried https://nodejs.org/en/download/package-manager for testing having npm version 10.5.0. The Problem are still there.

However, the steps 8 & 9, i.e. the workaround utilizing the npmcli parameter "--before" does not work in this version: I still get resolved+integrity but not the license. I checked back using NPM 8.5.1 and the workaround is working there.

I checked in pacote source (GitHub npm/pacote) and did not find any changes to july, so that I believe that the repository server or npmcli have been modifed to undo the behavior of the workaround.

@robcresswell
Copy link
Author

I'm so sorry @ljharb I totally missed your reply. Yes, it still occurs in the latest versions of npm. Its unclear to me if it was ever supposed to work, but nobody has had the time to acknowledge or discuss it.

@lukekarrys
Copy link
Contributor

This is fixed by effe910 which will be in the next npm release.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug thing that needs fixing Priority 2 secondary priority issue Release 8.x work is associated with a specific npm 8 release ws:arborist Related to the arborist workspace
Projects
None yet
Development

No branches or pull requests

6 participants