Merge branch 'configurable-yml-limits' into 'master'

Add configurable maximum YAML file size and depth

See merge request gitlab-org/gitlab!68593

app/helpers/application_settings_helper.rb

@@ -263,6 +263,8 @@ module ApplicationSettingsHelper
       :max_attachment_size,
       :max_import_size,
       :max_pages_size,
+      :max_yaml_size_bytes,
+      :max_yaml_depth,
       :metrics_method_call_threshold,
       :minimum_password_length,
       :mirror_available,

app/models/application_setting.rb

@@ -343,6 +343,8 @@ class ApplicationSetting < ApplicationRecord
 
   validates :snippet_size_limit, numericality: { only_integer: true, greater_than: 0 }
   validates :wiki_page_max_content_bytes, numericality: { only_integer: true, greater_than_or_equal_to: 1.kilobytes }
+  validates :max_yaml_size_bytes, numericality: { only_integer: true, greater_than: 0 }, presence: true
+  validates :max_yaml_depth, numericality: { only_integer: true, greater_than: 0 }, presence: true
 
   validates :email_restrictions, untrusted_regexp: true
 

app/models/application_setting_implementation.rb

@@ -109,6 +109,8 @@ module ApplicationSettingImplementation
         max_artifacts_size: Settings.artifacts['max_size'],
         max_attachment_size: Settings.gitlab['max_attachment_size'],
         max_import_size: 0,
+        max_yaml_size_bytes: 1.megabyte,
+        max_yaml_depth: 100,
         minimum_password_length: DEFAULT_MINIMUM_PASSWORD_LENGTH,
         mirror_available: true,
         notes_create_limit: 300,

db/migrate/20210817172214_add_yaml_limits_application_setting.rb

+# frozen_string_literal: true
+
+class AddYamlLimitsApplicationSetting < ActiveRecord::Migration[6.1]
+  DOWNTIME = false
+
+  def change
+    add_column :application_settings, :max_yaml_size_bytes, :bigint, default: 1.megabyte, null: false
+    add_column :application_settings, :max_yaml_depth, :integer, default: 100, null: false
+  end
+end

db/migrate/20210830154358_add_yaml_limit_constraints.rb

+# frozen_string_literal: true
+
+# See https://docs.gitlab.com/ee/development/migration_style_guide.html
+# for more information on how to write migrations for GitLab.
+
+class AddYamlLimitConstraints < ActiveRecord::Migration[6.1]
+  include Gitlab::Database::MigrationHelpers
+
+  DOWNTIME = false
+
+  SIZE_CONSTRAINT_NAME = 'app_settings_yaml_max_size_positive'
+  DEPTH_CONSTRAINT_NAME = 'app_settings_yaml_max_depth_positive'
+
+  disable_ddl_transaction!
+
+  def up
+    add_check_constraint :application_settings, 'max_yaml_size_bytes > 0', SIZE_CONSTRAINT_NAME
+    add_check_constraint :application_settings, 'max_yaml_depth > 0', DEPTH_CONSTRAINT_NAME
+  end
+
+  def down
+    remove_check_constraint :application_settings, SIZE_CONSTRAINT_NAME
+    remove_check_constraint :application_settings, DEPTH_CONSTRAINT_NAME
+  end
+end

db/schema_migrations/20210817172214

+d6dd6ce802beeea380e0eb1c564f6a5cbc6d30cb3488a3cb91935e1302a4c387
\ No newline at end of file

db/schema_migrations/20210830154358

+04a44d0e261b26cc7f39b81a4c59ea8e4903d6d7bf73c2004b426204db4491bc
\ No newline at end of file

db/structure.sql

@@ -9606,6 +9606,8 @@ CREATE TABLE application_settings (
     encrypted_customers_dot_jwt_signing_key bytea,
     encrypted_customers_dot_jwt_signing_key_iv bytea,
     pypi_package_requests_forwarding boolean DEFAULT true NOT NULL,
+    max_yaml_size_bytes bigint DEFAULT 1048576 NOT NULL,
+    max_yaml_depth integer DEFAULT 100 NOT NULL,
     throttle_unauthenticated_files_api_requests_per_period integer DEFAULT 125 NOT NULL,
     throttle_unauthenticated_files_api_period_in_seconds integer DEFAULT 15 NOT NULL,
     throttle_authenticated_files_api_requests_per_period integer DEFAULT 500 NOT NULL,
@@ -9615,6 +9617,8 @@ CREATE TABLE application_settings (
     CONSTRAINT app_settings_container_reg_cleanup_tags_max_list_size_positive CHECK ((container_registry_cleanup_tags_service_max_list_size >= 0)),
     CONSTRAINT app_settings_ext_pipeline_validation_service_url_text_limit CHECK ((char_length(external_pipeline_validation_service_url) <= 255)),
     CONSTRAINT app_settings_registry_exp_policies_worker_capacity_positive CHECK ((container_registry_expiration_policies_worker_capacity >= 0)),
+    CONSTRAINT app_settings_yaml_max_depth_positive CHECK ((max_yaml_depth > 0)),
+    CONSTRAINT app_settings_yaml_max_size_positive CHECK ((max_yaml_size_bytes > 0)),
     CONSTRAINT check_17d9558205 CHECK ((char_length((kroki_url)::text) <= 1024)),
     CONSTRAINT check_2dba05b802 CHECK ((char_length(gitpod_url) <= 255)),
     CONSTRAINT check_51700b31b5 CHECK ((char_length(default_branch_name) <= 255)),

doc/administration/instance_limits.md

@@ -513,6 +513,29 @@ Update `ci_jobs_trace_size_limit` with the new value in megabytes:
 Plan.default.actual_limits.update!(ci_jobs_trace_size_limit: 125)
 ```
 
+### Maximum size and depth of CI/CD configuration YAML files
+
+The default maximum size of a CI/CD configuration YAML file is 1 megabyte and the default depth is 100.
+
+You can change these limits in the [GitLab Rails console](operations/rails_console.md#starting-a-rails-console-session).
+Update `max_yaml_size_bytes` with the new value in megabytes:
+
+```ruby
+ApplicationSetting.update!(max_yaml_size_bytes: 2.megabytes)
+```
+
+Update `max_yaml_depth` with the new value in megabytes:
+
+```ruby
+ApplicationSetting.update!(max_yaml_depth: 125)
+```
+
+To disable this limitation entirely, disable the feature flag in the console:
+
+```ruby
+Feature.disable(:ci_yaml_limit_size)
+```
+
 ## Instance monitoring and metrics
 
 ### Limit inbound incident management alerts

lib/gitlab/config/loader/yaml.rb

@@ -9,9 +9,6 @@ module Gitlab
 
         include Gitlab::Utils::StrongMemoize
 
-        MAX_YAML_SIZE = 1.megabyte
-        MAX_YAML_DEPTH = 100
-
         def initialize(config, additional_permitted_classes: [])
           @config = YAML.safe_load(config,
             permitted_classes: [Symbol, *additional_permitted_classes],
@@ -52,8 +49,8 @@ module Gitlab
         def deep_size
           strong_memoize(:deep_size) do
             Gitlab::Utils::DeepSize.new(@config,
-              max_size: MAX_YAML_SIZE,
-              max_depth: MAX_YAML_DEPTH)
+              max_size: Gitlab::CurrentSettings.current_application_settings.max_yaml_size_bytes,
+              max_depth: Gitlab::CurrentSettings.current_application_settings.max_yaml_depth)
           end
         end
       end

spec/lib/gitlab/config/loader/yaml_spec.rb

@@ -15,6 +15,24 @@ RSpec.describe Gitlab::Config::Loader::Yaml do
     YAML
   end
 
+  context 'when max yaml size and depth are set in ApplicationSetting' do
+    let(:yaml_size) { 2.megabytes }
+    let(:yaml_depth) { 200 }
+
+    before do
+      stub_application_setting(max_yaml_size_bytes: yaml_size, max_yaml_depth: yaml_depth)
+    end
+
+    it 'uses ApplicationSetting values rather than the defaults' do
+      expect(Gitlab::Utils::DeepSize)
+        .to receive(:new)
+        .with(any_args, { max_size: yaml_size, max_depth: yaml_depth })
+        .and_call_original
+
+      loader.load!
+    end
+  end
+
   context 'when yaml syntax is correct' do
     let(:yml) { 'image: ruby:2.7' }
 

spec/models/application_setting_spec.rb

@@ -79,6 +79,10 @@ RSpec.describe ApplicationSetting do
     it { is_expected.to validate_numericality_of(:wiki_page_max_content_bytes).only_integer.is_greater_than_or_equal_to(1024) }
     it { is_expected.to validate_presence_of(:max_artifacts_size) }
     it { is_expected.to validate_numericality_of(:max_artifacts_size).only_integer.is_greater_than(0) }
+    it { is_expected.to validate_presence_of(:max_yaml_size_bytes) }
+    it { is_expected.to validate_numericality_of(:max_yaml_size_bytes).only_integer.is_greater_than(0) }
+    it { is_expected.to validate_presence_of(:max_yaml_depth) }
+    it { is_expected.to validate_numericality_of(:max_yaml_depth).only_integer.is_greater_than(0) }
     it { is_expected.to validate_presence_of(:max_pages_size) }
     it 'ensures max_pages_size is an integer greater than 0 (or equal to 0 to indicate unlimited/maximum)' do
       is_expected.to validate_numericality_of(:max_pages_size).only_integer.is_greater_than_or_equal_to(0)