Merge branch 'id-limited-encoder' into 'master'

Limit webhook payload size to 25MB

See merge request gitlab-org/gitlab!38687

Gemfile

@@ -510,3 +510,4 @@ gem 'json-schema', '~> 2.8.0'
 gem 'json_schemer', '~> 0.2.12'
 gem 'oj', '~> 3.10.6'
 gem 'multi_json', '~> 1.14.1'
+gem 'yajl-ruby', '~> 1.4.1', require: 'yajl'

Gemfile.lock

@@ -1180,6 +1180,7 @@ GEM
     xml-simple (1.1.5)
     xpath (3.2.0)
       nokogiri (~> 1.8)
+    yajl-ruby (1.4.1)
     zeitwerk (2.3.0)
 
 PLATFORMS
@@ -1447,6 +1448,7 @@ DEPENDENCIES
   webmock (~> 3.5.1)
   webpack-rails (~> 0.9.10)
   wikicloth (= 0.8.1)
+  yajl-ruby (~> 1.4.1)
 
 BUNDLED WITH
    1.17.3

app/services/web_hook_service.rb

@@ -13,6 +13,7 @@ class WebHookService
     end
   end
 
+  REQUEST_BODY_SIZE_LIMIT = 25.megabytes
   GITLAB_EVENT_HEADER = 'X-Gitlab-Event'
 
   attr_accessor :hook, :data, :hook_name, :request_options
@@ -53,7 +54,7 @@ class WebHookService
       http_status: response.code,
       message: response.to_s
     }
-  rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep => e
+  rescue SocketError, OpenSSL::SSL::SSLError, Errno::ECONNRESET, Errno::ECONNREFUSED, Errno::EHOSTUNREACH, Net::OpenTimeout, Net::ReadTimeout, Gitlab::HTTP::BlockedUrlError, Gitlab::HTTP::RedirectionTooDeep, Gitlab::Json::LimitedEncoder::LimitExceeded => e
     log_execution(
       trigger: hook_name,
       url: hook.url,
@@ -83,7 +84,7 @@ class WebHookService
 
   def make_request(url, basic_auth = false)
     Gitlab::HTTP.post(url,
-      body: data.to_json,
+      body: Gitlab::Json::LimitedEncoder.encode(data, limit: REQUEST_BODY_SIZE_LIMIT),
       headers: build_headers(hook_name),
       verify: hook.enable_ssl_verification,
       basic_auth: basic_auth,

doc/administration/instance_limits.md

@@ -160,7 +160,7 @@ There is a limit when embedding metrics in GFM for performance reasons.
 
 ## Number of webhooks
 
-On GitLab.com, the [maximum number of webhooks](../user/gitlab_com/index.md#maximum-number-of-webhooks) per project, and per group, is limited.
+On GitLab.com, the [maximum number of webhooks and their size](../user/gitlab_com/index.md#webhooks) per project, and per group, is limited.
 
 To set this limit on a self-managed installation, run the following in the
 [GitLab Rails console](troubleshooting/debug.md#starting-a-rails-console-session):

doc/security/webhooks.md

@@ -5,7 +5,7 @@ type: concepts, reference, howto
 # Webhooks and insecure internal web services
 
 NOTE: **Note:**
-On GitLab.com the [maximum number of webhooks](../user/gitlab_com/index.md#maximum-number-of-webhooks) per project is limited.
+On GitLab.com, the [maximum number of webhooks and their size](../user/gitlab_com/index.md#webhooks) per project, and per group, is limited.
 
 If you have non-GitLab web services running on your GitLab server or within its
 local network, these may be vulnerable to exploitation via Webhooks.

doc/user/gitlab_com/index.md

@@ -116,12 +116,13 @@ All our runners are deployed into Google Cloud Platform (GCP) - any IP based
 firewall can be configured by looking up all
 [IP address ranges or CIDR blocks for GCP](https://cloud.google.com/compute/docs/faq#where_can_i_find_product_name_short_ip_ranges).
 
-## Maximum number of webhooks
+## Webhooks
 
 A limit of:
 
 - 100 webhooks applies to projects.
 - 50 webhooks applies to groups. **(BRONZE ONLY)**
+- Payload is limited to 25MB
 
 ## Shared Runners
 

doc/user/project/integrations/webhooks.md

@@ -35,7 +35,7 @@ Navigate to the webhooks page by going to your project's
 **Settings ➔ Webhooks**.
 
 NOTE: **Note:**
-On GitLab.com, the [maximum number of webhooks](../../../user/gitlab_com/index.md#maximum-number-of-webhooks) per project, and per group, is limited.
+On GitLab.com, the [maximum number of webhooks and their size](../../../user/gitlab_com/index.md#webhooks) per project, and per group, is limited.
 
 ## Version history
 

lib/gitlab/json.rb

@@ -220,5 +220,33 @@ module Gitlab
         end
       end
     end
+
+    class LimitedEncoder
+      LimitExceeded = Class.new(StandardError)
+
+      # Generates JSON for an object or raise an error if the resulting json string is too big
+      #
+      # @param object [Hash, Array, Object] must be hash, array, or an object that responds to .to_h or .to_json
+      # @param limit [Integer] max size of the resulting json string
+      # @return [String]
+      # @raise [LimitExceeded] if the resulting json string is bigger than the specified limit
+      def self.encode(object, limit: 25.megabytes)
+        return ::Gitlab::Json.dump(object) unless Feature.enabled?(:json_limited_encoder)
+
+        buffer = []
+        buffer_size = 0
+
+        ::Yajl::Encoder.encode(object) do |data_chunk|
+          chunk_size = data_chunk.bytesize
+
+          raise LimitExceeded if buffer_size + chunk_size > limit
+
+          buffer << data_chunk
+          buffer_size += chunk_size
+        end
+
+        buffer.join('')
+      end
+    end
   end
 end

spec/lib/gitlab/json_spec.rb

@@ -407,4 +407,36 @@ RSpec.describe Gitlab::Json do
       end
     end
   end
+
+  describe Gitlab::Json::LimitedEncoder do
+    subject { described_class.encode(obj, limit: 8.kilobytes) }
+
+    context 'when object size is acceptable' do
+      let(:obj) { { test: true } }
+
+      it 'returns json string' do
+        is_expected.to eq("{\"test\":true}")
+      end
+    end
+
+    context 'when object is too big' do
+      let(:obj) { [{ test: true }] * 1000 }
+
+      it 'raises LimitExceeded error' do
+        expect { subject }.to raise_error(
+          Gitlab::Json::LimitedEncoder::LimitExceeded
+        )
+      end
+    end
+
+    context 'when json_limited_encoder is disabled' do
+      let(:obj) { [{ test: true }] * 1000 }
+
+      it 'does not raise an error' do
+        stub_feature_flags(json_limited_encoder: false)
+
+        expect { subject }.not_to raise_error
+      end
+    end
+  end
 end

spec/services/web_hook_service_spec.rb

@@ -130,6 +130,14 @@ RSpec.describe WebHookService do
       end
     end
 
+    context 'when request body size is too big' do
+      it 'does not perform the request' do
+        stub_const("#{described_class}::REQUEST_BODY_SIZE_LIMIT", 10.bytes)
+
+        expect(service_instance.execute).to eq({ status: :error, message: "Gitlab::Json::LimitedEncoder::LimitExceeded" })
+      end
+    end
+
     it 'handles 200 status code' do
       stub_full_request(project_hook.url, method: :post).to_return(status: 200, body: 'Success')