Add limit to number of test cases parsed

This adds a hardcoded value for now, but in succeeding issues will be
replaced with an actual application limit.

We can't rely on test_suite.total_count because it counts test cases
uniquely by their classname + name. So if there's a malicious file
that are just a bunch of duplicate test cases, we won't be able to
catch it.

app/models/ci/build.rb

@@ -898,7 +898,11 @@ module Ci
     def collect_test_reports!(test_reports)
       test_reports.get_suite(group_name).tap do |test_suite|
         each_report(Ci::JobArtifact::TEST_REPORT_FILE_TYPES) do |file_type, blob|
-          Gitlab::Ci::Parsers.fabricate!(file_type).parse!(blob, test_suite, job: self)
+          Gitlab::Ci::Parsers.fabricate!(file_type).parse!(
+            blob,
+            test_suite,
+            job: self
+          )
         end
       end
     end
@@ -964,6 +968,12 @@ module Ci
       status_commit_hooks.push(block)
     end
 
+    def max_test_cases_per_report
+      # NOTE: This is temporary and will be replaced later by a value
+      # that would come from an actual application limit.
+      ::Gitlab.com? ? 500_000 : 0
+    end
+
     protected
 
     def run_status_commit_hooks!

changelogs/unreleased/eb-limit-test-cases-parsed.yml

+---
+title: Add limit to number of test cases parsed by JUnit parser
+merge_request: 44615
+author:
+type: changed

doc/user/gitlab_com/index.md

@@ -96,6 +96,7 @@ Below are the current settings regarding [GitLab CI/CD](../../ci/README.md).
 | [Max pipeline schedules in projects](../../administration/instance_limits.md#number-of-pipeline-schedules) | `10` for Free tier, `50` for all paid tiers | Unlimited |
 | [Max number of instance level variables](../../administration/instance_limits.md#number-of-instance-level-variables) | `25` | `25` |
 | [Scheduled Job Archival](../../user/admin_area/settings/continuous_integration.md#archive-jobs) | 3 months | Never |
+| Max test cases per [unit test report](../../ci/unit_test_reports.md) | `500_000` | Unlimited |
 
 ## Account and limit settings
 

lib/gitlab/ci/parsers/test/junit.rb

@@ -8,12 +8,17 @@ module Gitlab
           JunitParserError = Class.new(Gitlab::Ci::Parsers::ParserError)
           ATTACHMENT_TAG_REGEX = /\[\[ATTACHMENT\|(?<path>.+?)\]\]/.freeze
 
-          def parse!(xml_data, test_suite, **args)
+          def parse!(xml_data, test_suite, job:)
             root = Hash.from_xml(xml_data)
+            total_parsed = 0
+            max_test_cases = job.max_test_cases_per_report
 
             all_cases(root) do |test_case|
-              test_case = create_test_case(test_case, test_suite, args)
+              test_case = create_test_case(test_case, test_suite, job)
               test_suite.add_test_case(test_case)
+              total_parsed += 1
+
+              ensure_test_cases_limited!(total_parsed, max_test_cases)
             end
           rescue Nokogiri::XML::SyntaxError => e
             test_suite.set_suite_error("JUnit XML parsing failed: #{e}")
@@ -23,6 +28,12 @@ module Gitlab
 
           private
 
+          def ensure_test_cases_limited!(total_parsed, limit)
+            return unless limit > 0 && total_parsed > limit
+
+            raise JunitParserError.new("number of test cases exceeded the limit of #{limit}")
+          end
+
           def all_cases(root, parent = nil, &blk)
             return unless root.present?
 
@@ -50,7 +61,7 @@ module Gitlab
             end
           end
 
-          def create_test_case(data, test_suite, args)
+          def create_test_case(data, test_suite, job)
             if data.key?('failure')
               status = ::Gitlab::Ci::Reports::TestCase::STATUS_FAILED
               system_output = data['failure']
@@ -75,7 +86,7 @@ module Gitlab
               status: status,
               system_output: system_output,
               attachment: attachment,
-              job: args.fetch(:job)
+              job: job
             )
           end
 

spec/lib/gitlab/ci/parsers/test/junit_spec.rb

@@ -4,11 +4,12 @@ require 'fast_spec_helper'
 
 RSpec.describe Gitlab::Ci::Parsers::Test::Junit do
   describe '#parse!' do
-    subject { described_class.new.parse!(junit, test_suite, **args) }
+    subject { described_class.new.parse!(junit, test_suite, job: job) }
 
     let(:test_suite) { Gitlab::Ci::Reports::TestSuite.new('rspec') }
     let(:test_cases) { flattened_test_cases(test_suite) }
-    let(:args) { { job: { id: 1, project: "project" } } }
+    let(:job) { double(max_test_cases_per_report: max_test_cases) }
+    let(:max_test_cases) { 0 }
 
     context 'when data is JUnit style XML' do
       context 'when there are no <testcases> in <testsuite>' do
@@ -230,6 +231,55 @@ RSpec.describe Gitlab::Ci::Parsers::Test::Junit do
           )
         end
       end
+
+      context 'when number of test cases exceeds the max_test_cases limit' do
+        let(:max_test_cases) { 1 }
+
+        shared_examples_for 'rejecting too many test cases' do
+          it 'attaches an error to the TestSuite object' do
+            expect { subject }.not_to raise_error
+            expect(test_suite.suite_error).to eq("JUnit data parsing failed: number of test cases exceeded the limit of #{max_test_cases}")
+          end
+        end
+
+        context 'and test cases are unique' do
+          let(:junit) do
+            <<-EOF.strip_heredoc
+            <testsuites>
+              <testsuite>
+                <testcase classname='Calculator' name='sumTest1' time='0.01'></testcase>
+                <testcase classname='Calculator' name='sumTest2' time='0.02'></testcase>
+              </testsuite>
+              <testsuite>
+                <testcase classname='Statemachine' name='happy path' time='100'></testcase>
+                <testcase classname='Statemachine' name='unhappy path' time='200'></testcase>
+              </testsuite>
+            </testsuites>
+            EOF
+          end
+
+          it_behaves_like 'rejecting too many test cases'
+        end
+
+        context 'and test cases are duplicates' do
+          let(:junit) do
+            <<-EOF.strip_heredoc
+            <testsuites>
+              <testsuite>
+                <testcase classname='Calculator' name='sumTest1' time='0.01'></testcase>
+                <testcase classname='Calculator' name='sumTest2' time='0.02'></testcase>
+              </testsuite>
+              <testsuite>
+                <testcase classname='Calculator' name='sumTest1' time='0.01'></testcase>
+                <testcase classname='Calculator' name='sumTest2' time='0.02'></testcase>
+              </testsuite>
+            </testsuites>
+            EOF
+          end
+
+          it_behaves_like 'rejecting too many test cases'
+        end
+      end
     end
 
     context 'when data is not JUnit style XML' do
@@ -316,9 +366,7 @@ RSpec.describe Gitlab::Ci::Parsers::Test::Junit do
         expect(test_cases[0].has_attachment?).to be_truthy
         expect(test_cases[0].attachment).to eq("some/path.png")
 
-        expect(test_cases[0].job).to be_present
-        expect(test_cases[0].job[:id]).to eq(1)
-        expect(test_cases[0].job[:project]).to eq("project")
+        expect(test_cases[0].job).to eq(job)
       end
     end