Limit webhook request body size

CHANGELOG.md

@@ -6,6 +6,7 @@ This format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/)
 
 ## Unreleased
 
+- Limit the size of the request body, since otherwise there can be a large attack vector where random senders can spam the database with data and cause a denial of service. With background validation, this is one of the few cases where we want to reject the payload without persisting it.
 - Manage the `state` of the `ReceivedWebhook` from the background job itself. This frees up the handler to actually do the work associated with processing only. The job will manage the rest.
 - Use `valid?` in the background job instead of the controller. Most common configuration issue is an incorrectly specified signing secret, or an incorrectly implemented input validation. When these happen, it is better to allow the webhook to be reprocessed
 - Use instance methods in handlers instead of class methods, as they are shorter to define. Assume a handler module supports `.new` - with a module using singleton methods it may return `self` from `new`.

lib/munster.rb

@@ -21,4 +21,5 @@ class Munster::Configuration
   config_accessor(:processing_job_class, default: Munster::ProcessingJob)
   config_accessor(:active_handlers, default: {})
   config_accessor(:error_context, default: {})
+  config_accessor(:request_body_size_limit, default: 512.kilobytes)
 end

lib/munster/models/received_webhook.rb

@@ -37,7 +37,12 @@ def request=(action_dispatch_request)
       # ...and the raw request body - because we already save it separately
       headers.delete("RAW_POST_DATA")
 
+      # Verify the request body is not too large
       request_body_io = action_dispatch_request.env.fetch("rack.input")
+      if request_body_io.size > Munster.configuration.request_body_size_limit
+        raise "Cannot accept the webhook as the request body is larger than #{limit} bytes"
+      end
+
       write_attribute("body", request_body_io.read.force_encoding(Encoding::BINARY))
       write_attribute("request_headers", headers)
     ensure

test/munster_test.rb

@@ -108,6 +108,18 @@ def self.xtest(msg)
     assert_predicate tf.read, :empty?
   end
 
+  test "does not accept a test payload that is larger than the configured maximum size" do
+    Munster::ReceivedWebhook.delete_all
+
+    oversize = Munster.configuration.request_body_size_limit + 1
+    utf8_junk = Base64.strict_encode64(Random.bytes(oversize))
+    body = {isValid: true, filler: utf8_junk, raiseDuringProcessing: false, outputToFilename: "/tmp/nothing"}
+    body_json = body.to_json
+
+    post "/munster/test", params: body_json, headers: {"CONTENT_TYPE" => "application/json"}
+    assert_raises(ActiveRecord::RecordNotFound) { Munster::ReceivedWebhook.last! }
+  end
+
   test "does not try to process a webhook if it is not in `received' state" do
     Munster::ReceivedWebhook.delete_all