coinbase/salus

YarnAudit fails for large yarn.lock files

GMartinez-Sisti opened this issue · 10 comments

Hi,

We started receiving the following error when executing Salus YarnAudit scan agains our repository:

==== YarnAudit: FAILED in 120.72s

 ~~ Errors:

  [
    {
      "message": "Unhandled exception running YarnAudit: JSON::ParserError: 751: unexpected token at
   ''",
      "error_class": "JSON::ParserError",
      "backtrace": [
        "/usr/local/lib/ruby/2.4.0/json/common.rb:156:in `parse'",
        "/usr/local/lib/ruby/2.4.0/json/common.rb:156:in `parse'",
        "/home/lib/salus/scanners/yarn_audit.rb:48:in `block in scan_for_cves'",
        "/home/lib/salus/scanners/yarn_audit.rb:47:in `map'",
        "/home/lib/salus/scanners/yarn_audit.rb:47:in `scan_for_cves'"
      ]
    }
  ]

After a lot of troubleshooting, I believe the issue is caused by our yarn.lock file that has 12k+ lines and the size of the stdout buffer.

I parsed the yarn.lock file, with this script to understand the output of the file.

Here are our tests:

yarn.lock lines Parsed json file size Salus YarnAudit result
~12k 903K Fails with above error
~9k 474K Fails with above error
~8k 422K Scan succeeds

I hope this information helps.

Please let me know if you need more information, or if you want me to test a specific branch.

Regards

That is a massive yarn.lock file. My guess is that Salus is currently loading the entire file into memory causing the issues above.

Could a temporary fix be to increase the amount of memory allocated to the running container?

@nishils unfortunately doesn't look like it's a matter of memory allocation.

Also note that if we perform yarn audit directly it works, so it's something in the Salus logic that is causing it. I believe it's related to the fact that is using stdout to send yarn.lock and consuming from there to do the analysis.

Interesting. Thanks for raising this issue.

This is a part of the codebase I haven't touched yet so I'm unfamiliar why we decided to go down this route instead of copying the file or reading the original file. It doesn't make much sense to me. I'll have to look into why we made this decision. Optimistically, a patch towards the end of next week is the best outcome. If you have any other insight you can share or a test yarn.lock, then that would be helpful!

xolaz commented

@GMartinez-Sisti where are you running Salus from when you get this error? Your local environment or on your CI tool or something else?

xolaz commented

@GMartinez-Sisti another question, what output if any did you get when you ran yarn audit?

@xolaz yarn audit by itself provides the proper output you would expect from a successful run. We're running this on CI, but the tests were performed locally, both with the docker image.

xolaz commented

@GMartinez-Sisti Thanks for the info! Can you copy/paste output please 🙏? By docker image do you mean Salus's docker image or a docker image for your project?

For context, we are only able to reproduce this error in a CI when yarn audit returns CVEs results.

@xolaz Sharing the entire yarn audit result in here wouldn't be helpful, the output is very long:

→ yarn audit | wc -l
64796

The end of it is:

Severity: 1 Moderate | 4095 High | 1 Critical
✨  Done in 13.53s.

Maybe the problem is not with the file but with the results.

@GMartinez-Sisti I have a repo that has a yarn.lock that's over 12K lines, but could not reproduce the problem. Is your repo public? Or can you share your yarn.lock?

@ghbren I just tried this with Salus 2.7.3 and it's not happening anymore. The errors were reported with 2.7.1 I believe.

I'm going to close this then. Thank you for all your help!