Skip to content

add workaround for npm workspaces #63

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

Open
vrohatgi opened this issue Feb 6, 2023 · 7 comments
Open

add workaround for npm workspaces #63

vrohatgi opened this issue Feb 6, 2023 · 7 comments

Comments

@vrohatgi
Copy link

vrohatgi commented Feb 6, 2023

  • snyk -v: 1.1052.0
  • Command run: snyk test

Expected behaviour

snyk-bulk should be able to recursively scan npm workspaces.

Actual behaviour

npm workspaces do not have their own package-lock.json file; instead, a single lock file in the root of the project is used for all workspaces in the project. snyk-bulk does not support this project layout and fails when trying to recursively scan each workspace.


If applicable, please append the --debug flag on your command and include the output here **ensuring to remove any sensitive/personal details or tokens.

@github-actions
Copy link

github-actions bot commented Feb 6, 2023

Issue: CS-189

@scott-es
Copy link
Contributor

scott-es commented Feb 6, 2023

Hello @vrohatgi! --

snyk does not support NPM workspaces directly, and so it was also not considered for use case in snyk-bulk, however I believe its similar to yarn workspaces, and we used a trick to add support for it, as shown here

The idea here was that we had one yarn.lock at the workspace root which snyk uses to resolve the transitives and versions, and by simply creating a symbolic link from the root yarn.lock to each package (next to where the package.json sits in each subfolder) then snyk can then iterate through the directories and test each package.json as its own project and end up with the same results as shown in the screen shot in the PR that has since been merged.

Because snyk uses the package.json as a manifest (it will only include in the dependency tree what is being brought in through the direct dependencies in the package.son) while also using the lock file to ensure accuracy of the transitives and versions, this happens to work well.

Do you see the same approach will be viable here? If so it will be trivial to add support as its almost identical to the yarn workspace support, however we're not very familiar with it so sanity checking this approach will be helpful.

@chris-walz
Copy link

I did a little testing with npm workspaces and I think I found a trick works. Not all workspace directories will have a node_modules folder (since they can use ones from the root) and if you try to run Snyk on a package.json file without that folder it will error out with Missing node_modules folder: we can't test without dependencies.
BUT...

If you make an empty node_modules folder and THEN run snyk monitor --file=package.json it works fine, I'm guessing because snyk is just using npm list or something similar and npm knows how to find the workspace dependencies.

So to get it to work, I think all you'd need to do is run npm install at the root, then list the workspaces (npm --workspaces list) and for each one create a node_modules folder if it doesn't already exist.

The package-lock.json trick doesn't seem to work. If one of the workspaces has a vulnerable dependency and I link the root packag-lock.json in the subfolder, Snyk doesn't find the vulnerable dependency.

@scott-es
Copy link
Contributor

scott-es commented Feb 7, 2023

Hi @chris-walz 👋 ! -- thats too bad it doesn't work like the yarn.lock does. Does the npm version matter at all? I'll have to test a solution with npm 7, 8, and 9 I guess at this point. I'll see if your trick works in my own testing as well and see if we can get a working solution here.

@chris-walz
Copy link

Thanks @scott-es ! Good point about the npm versions. I was testing on 9. I would think that it would work the same on any version that supports workspaces, but haven't tested that.

@vrohatgi vrohatgi changed the title add workaround for NPM workspaces add workaround for npm workspaces Feb 7, 2023
@vrohatgi
Copy link
Author

Hey @scott-es, any updates on the testing?

@chris-walz
Copy link

@scott-es I made a PR that I think should work. It checks each package.json file to see if it has workspaces and then creates an empty node_modules folder for each one.

I created a test repo with a couple workspaces and some dependencies and it seems to be working!

#64

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

No branches or pull requests

3 participants