Merge pull request #181 from coinbase/parse_yarn_audit

parse yarn audit text output into json

lib/salus/scanners/yarn_audit.rb

@@ -7,14 +7,80 @@
 
 module Salus::Scanners
   class YarnAudit < NodeAudit
-    AUDIT_COMMAND = 'yarn audit --json'.freeze
+    # the command was previously 'yarn audit --json', which had memory allocation issues
+    # see https://github.com/yarnpkg/yarn/issues/7404
+    AUDIT_COMMAND = 'yarn audit --no-color'.freeze
 
     def should_run?
       @repository.yarn_lock_present?
     end
 
+    def run
+      shell_return = Dir.chdir(@repository.path_to_repo) do
+        command = "#{AUDIT_COMMAND} #{scan_deps}"
+        shell_return = run_shell(command)
+
+        excpts = @config.fetch('exceptions', []).map { |e| e["advisory_id"].to_i }
+        report_info(:ignored_cves, excpts)
+        return report_success if shell_return.success?
+
+        stdout_lines = shell_return.stdout.split("\n")
+        table_start_pos = stdout_lines.index { |l| l.start_with?("┌─") && l.end_with?("─┐") }
+        table_end_pos = stdout_lines.rindex { |l| l.start_with?("└─") && l.end_with?("─┘") }
+
+        # if no table in output
+        if table_start_pos.nil? || table_end_pos.nil?
+          report_error(shell_return.stderr, status: shell_return.status)
+          report_stderr(shell_return.stderr)
+          return report_failure
+        end
+
+        table_lines = stdout_lines[table_start_pos..table_end_pos]
+        # lines contain 1 or more vuln tables
+
+        vulns = parse_output(table_lines)
+        vuln_ids = vulns.map { |v| v['id'] }
+        report_info(:vulnerabilities, vuln_ids.uniq)
+
+        vulns.reject! { |v| excpts.include?(v['id']) }
+        # vulns were all whitelisted
+        return report_success if vulns.empty?
+
+        log(format_vulns(vulns))
+        report_stdout(vulns.to_json)
+        report_failure
+      end
+    end
+
     private
 
+    def parse_output(lines)
+      vulns = []
+
+      i = 0
+      while i < lines.size
+        if lines[i].start_with?("┌─") && lines[i].end_with?("─┐")
+          vuln = {}
+        elsif lines[i].start_with? "│ "
+          line_split = lines[i].split("│")
+          curr_key = line_split[1].strip
+          val = line_split[2].strip
+
+          if curr_key != ""
+            vuln[curr_key] = val
+            prev_key = curr_key
+          else
+            vuln[prev_key] += ' ' + val
+          end
+        elsif lines[i].start_with?("└─") && lines[i].end_with?("─┘")
+          vulns.push vuln
+        end
+        i += 1
+      end
+
+      vulns.each { |vln| normalize_vuln(vln) }
+    end
+
     def scan_deps
       dep_types = @config.fetch('exclude_groups', [])
 
@@ -35,19 +101,41 @@ def scan_deps
       command << 'optionalDependencies ' unless dep_types.include?('optionalDependencies')
     end
 
-    def scan_for_cves
-      # Yarn gives us a new-line separated list of JSON blobs.
-      # But the last JSON blob is a summary that we can discard.
-      # We must also pluck out only the standard advisory hashes.
-      command = "#{AUDIT_COMMAND} #{scan_deps}"
-      command_output = run_shell(command)
+    # severity and vuln title in the yarn output looks like
+    # | low           | Prototype Pollution                                          |
+    # which are stored in the vuln hash as "low" ==> "Prototype Pollution"
+    # need to update that to
+    #     1) "severity" => "low"
+    #     2) "title" => "Prototype Pollution"
+    #
+    # Also, add a separate id field
+    def normalize_vuln(vuln)
+      sev_levels = %w[info low moderate high critical]
+
+      sev_levels.each do |sev|
+        if vuln[sev]
+          vuln['severity'] = sev
+          vuln['title'] = vuln[sev]
+          vuln.delete(sev)
+          break
+        end
+      end
 
-      report_stdout(command_output.stdout)
+      # "More info" looks like https://www.npmjs.com/advisories/1179
+      # need to extract the id at the end
+      id = vuln["More info"].split("https://www.npmjs.com/advisories/")[1]
+      vuln['id'] = id.to_i
+    end
 
-      command_output.stdout.split("\n")[0..-2].map do |raw_advisory|
-        advisory_hash = JSON.parse(raw_advisory, symbolize_names: true)
-        advisory_hash[:data][:advisory]
+    def format_vulns(vulns)
+      str = ""
+      vulns.each do |vul|
+        vul.each do |k, v|
+          str += "#{k}: #{v}\n"
+        end
+        str += "\n"
       end
+      str
     end
   end
 end

spec/fixtures/yarn_audit/failure-3/package.json

@@ -0,0 +1,24 @@
+{
+  "name": "www",
+  "version": "1.0.0",
+  "description": "Test package.json file.",
+  "main": "index.js",
+  "engines": {
+    "npm": ">= 4.0.0",
+    "node": ">= 6.9"
+  },
+  "scripts": {
+    "test": "test"
+  },
+  "repository": {
+    "type": "git",
+    "url": "test"
+  },
+  "author": "",
+  "license": "ISC",
+  "dependencies": {
+    "classnames-repo-does-not-exist": "^2.2.5",
+    "mobx": "^3.2.1",
+    "uglify-js": "1.2.3"
+  }
+}

spec/fixtures/yarn_audit/failure-3/yarn.lock

@@ -0,0 +1,18 @@
+# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY.
+# yarn lockfile v1
+
+
+classnames@^2.2.5:
+  version "2.2.6"
+  resolved "https://registry.yarnpkg.com/classnames/-/classnames-2.2.6.tgz#43935bffdd291f326dad0a205309b38d00f650ce"
+  integrity sha512-JR/iSQOSt+LQIWwrwEzJ9uk0xfN3mTVYMwt1Ir5mUcSN6pU+V4zQFFaJsclJbPuAUQH+yfWef6tm7l1quW3C8Q==
+
+mobx@^3.2.1:
+  version "3.6.2"
+  resolved "https://registry.yarnpkg.com/mobx/-/mobx-3.6.2.tgz#fb9f5ff5090539a1ad54e75dc4c098b602693320"
+  integrity sha512-Dq3boJFLpZEvuh5a/MbHLUIyN9XobKWIb0dBfkNOJffNkE3vtuY0C9kSDVpfH8BB0BPkVw8g22qCv7d05LEhKg==
+
+[email protected]:
+  version "1.2.3"
+  resolved "https://registry.yarnpkg.com/uglify-js/-/uglify-js-1.2.3.tgz#3b0ce6631a28dcaa64302b893123b20876bdc536"
+  integrity sha1-OwzmYxoo3KpkMCuJMSOyCHa9xTY=

spec/lib/salus/scanners/node_audit_spec.rb

@@ -24,15 +24,23 @@
           expect(scanner.report.passed?).to eq(false)
           info = scanner.report.to_h.fetch(:info)
           expect(info.key?(:stdout)).to eq(true)
-          expect(info).to include(
-            prod_advisories: %w[39 48],
-            dev_advisories: [],
-            unexcepted_prod_advisories: %w[39 48],
-            exceptions: [],
-            prod_exceptions: [],
-            dev_exceptions: [],
-            useless_exceptions: []
-          )
+
+          if klass_str == 'NPMAudit'
+            expect(info).to include(
+              prod_advisories: %w[39 48],
+              dev_advisories: [],
+              unexcepted_prod_advisories: %w[39 48],
+              exceptions: [],
+              prod_exceptions: [],
+              dev_exceptions: [],
+              useless_exceptions: []
+            )
+          else # YarnAudit
+            expect(info).to include(
+              vulnerabilities: [39, 48],
+              ignored_cves: []
+            )
+          end
         end
 
         it 'should fail, recording advisory ids and npm output' do
@@ -43,15 +51,22 @@
           expect(scanner.report.passed?).to eq(false)
           info = scanner.report.to_h.fetch(:info)
           expect(info.key?(:stdout)).to eq(true)
-          expect(info).to include(
-            prod_advisories: %w[39 48 722],
-            dev_advisories: [],
-            unexcepted_prod_advisories: %w[39 48 722],
-            exceptions: [],
-            prod_exceptions: [],
-            dev_exceptions: [],
-            useless_exceptions: []
-          )
+          if klass_str == 'NPMAudit'
+            expect(info).to include(
+              prod_advisories: %w[39 48 722],
+              dev_advisories: [],
+              unexcepted_prod_advisories: %w[39 48 722],
+              exceptions: [],
+              prod_exceptions: [],
+              dev_exceptions: [],
+              useless_exceptions: []
+            )
+          else # YarnAudit
+            expect(info).to include(
+              vulnerabilities: [39, 48, 722],
+              ignored_cves: []
+            )
+          end
         end
       end
 
@@ -63,12 +78,19 @@
           expect(scanner.report.passed?).to eq(true)
 
           info = scanner.report.to_h.fetch(:info)
-          expect(info.key?(:stdout)).to eq(true)
-          expect(info).to include(
-            prod_advisories: [],
-            dev_advisories: [],
-            unexcepted_prod_advisories: []
-          )
+          if klass_str == 'NPMAudit'
+            expect(info.key?(:stdout)).to eq(true)
+            expect(info).to include(
+              prod_advisories: [],
+              dev_advisories: [],
+              unexcepted_prod_advisories: []
+            )
+          else # YarnAudit
+            expect(info.key?(:stdout)).to eq(false)
+            expect(info).to include(
+              ignored_cves: []
+            )
+          end
         end
       end
 
@@ -85,16 +107,25 @@
 
           expect(scanner.report.passed?).to eq(true)
           info = scanner.report.to_h.fetch(:info)
-          expect(info.key?(:stdout)).to eq(true)
-          expect(info).to include(
-            prod_advisories: %w[39 48],
-            dev_advisories: [],
-            unexcepted_prod_advisories: [],
-            exceptions: %w[39 48],
-            prod_exceptions: %w[39 48],
-            dev_exceptions: [],
-            useless_exceptions: []
-          )
+          if klass_str == 'NPMAudit'
+            expect(info.key?(:stdout)).to eq(true)
+            expect(info).to include(
+              prod_advisories: %w[39 48],
+              dev_advisories: [],
+              unexcepted_prod_advisories: [],
+              exceptions: %w[39 48],
+              prod_exceptions: %w[39 48],
+              dev_exceptions: [],
+              useless_exceptions: []
+            )
+          else # YarnAudit
+            # YarnAudit no longer displays vulns that have been whitelisted
+            expect(info.key?(:stdout)).to eq(false)
+            expect(info).to include(
+              ignored_cves: [39, 48],
+              vulnerabilities: [39, 48]
+            )
+          end
         end
       end
     end

spec/lib/salus/scanners/yarn_audit_spec.rb

@@ -26,6 +26,26 @@
       scanner.run
 
       expect(scanner.report.to_h.fetch(:passed)).to eq(false)
+      vulns = JSON.parse(scanner.report.to_h[:info][:stdout])
+      expect(vulns.size).to eq(2)
+      vuln0 = { "Package" => "uglify-js",
+                "Patched in" => ">= 2.4.24",
+                "Dependency of" => "uglify-js",
+                "Path" => "uglify-js",
+                "More info" => "https://www.npmjs.com/advisories/39",
+                "severity" => "low",
+                "title" => "Incorrect Handling of Non-Boolean Comparisons During Minification",
+                "id" => 39 }
+      vuln1 = { "Package" => "uglify-js",
+                "Patched in" => ">=2.6.0",
+                "Dependency of" => "uglify-js",
+                "Path" => "uglify-js",
+                "More info" => "https://www.npmjs.com/advisories/48",
+                "severity" => "low",
+                "title" => "Regular Expression Denial of Service",
+                "id" => 48 }
+      expect(vulns[0]).to eq(vuln0)
+      expect(vulns[1]).to eq(vuln1)
 
       repo = Salus::Repo.new('spec/fixtures/yarn_audit/failure-2')
       scanner = Salus::Scanners::YarnAudit.new(repository: repo, config: {})
@@ -34,6 +54,17 @@
       expect(scanner.report.to_h.fetch(:passed)).to eq(false)
     end
 
+    it 'should fail with error if there are errors' do
+      repo = Salus::Repo.new('spec/fixtures/yarn_audit/failure-3')
+      scanner = Salus::Scanners::YarnAudit.new(repository: repo, config: {})
+      scanner.run
+
+      report = scanner.report.to_h
+      expect(report.fetch(:passed)).to eq(false)
+      info = scanner.report.to_h.fetch(:info)
+      expect(info[:stderr]).to include("classnames-repo-does-not-exist: Not found")
+    end
+
     it 'should pass if vulnerable devDependencies are excluded' do
       repo = Salus::Repo.new('spec/fixtures/yarn_audit/success_with_exclusions')
       scanner = Salus::Scanners::YarnAudit.new(repository: repo, config: {})