add MAX_PAYLOAD_SIZE and MAX_SIGNATURE_LENGTH checks to the default hmac auth plugin

Author: GrantBirki

.rubocop.yml

@@ -15,6 +15,7 @@ AllCops:
 GitHub/InsecureHashAlgorithm:
   Exclude:
     - "spec/unit/lib/hooks/plugins/auth/hmac_spec.rb"
+    - "spec/acceptance/acceptance_tests.rb"
 
 GitHub/AvoidObjectSendWithDynamicMethod:
   Exclude:

docs/design.md

@@ -100,7 +100,7 @@ Core configuration options can be provided via environment variables:
 export HOOKS_CONFIG=./config/config.yaml
 
 # Runtime settings (override config file)
-export HOOKS_REQUEST_LIMIT=1048576
+export HOOKS_REQUEST_LIMIT=1048576 # 1 MB
 export HOOKS_REQUEST_TIMEOUT=15
 export HOOKS_GRACEFUL_SHUTDOWN_TIMEOUT=30
 export HOOKS_ROOT_PATH="/webhooks"

lib/hooks/plugins/auth/hmac.rb

@@ -46,6 +46,10 @@ module Auth
       #     payload_template: "{timestamp}.{body}"
       #     timestamp_tolerance: 300  # 5 minutes
       class HMAC < Base
+        # Security constants
+        MAX_SIGNATURE_LENGTH = ENV.fetch("HOOKS_MAX_SIGNATURE_LENGTH", 1024).to_i # Prevent DoS attacks via large signatures
+        MAX_PAYLOAD_SIZE = ENV.fetch("MAX_PAYLOAD_SIZE", 10 * 1024 * 1024).to_i # 10MB limit for payload validation
+
         # Default configuration values for HMAC validation
         #
         # @return [Hash<Symbol, String|Integer>] Default configuration settings
@@ -121,6 +125,11 @@ def self.valid?(payload:, headers:, config:)
           # Find the signature header with case-insensitive matching
           raw_signature = find_header_value(headers, signature_header)
 
+          if payload && payload.bytesize > MAX_PAYLOAD_SIZE
+            log.warn("Auth::HMAC validation failed: Payload size exceeds maximum limit of #{MAX_PAYLOAD_SIZE} bytes")
+            return false
+          end
+
           if raw_signature.nil? || raw_signature.empty?
             log.warn("Auth::HMAC validation failed: Missing or empty signature header '#{signature_header}'")
             return false
@@ -132,6 +141,11 @@ def self.valid?(payload:, headers:, config:)
             return false
           end
 
+          if raw_signature.length > MAX_SIGNATURE_LENGTH
+            log.warn("Auth::HMAC validation failed: Signature length exceeds maximum limit of #{MAX_SIGNATURE_LENGTH} characters")
+            return false
+          end
+
           # Security: Reject signatures containing null bytes or other control characters
           if raw_signature.match?(/[\u0000-\u001f\u007f-\u009f]/)
             log.warn("Auth::HMAC validation failed: Signature contains control characters")

spec/unit/lib/hooks/plugins/auth/hmac_spec.rb

@@ -23,6 +23,7 @@
 
   before(:each) do
     Hooks::Log.instance = log
+    allow(ENV).to receive(:[]).and_call_original
     allow(ENV).to receive(:[]).with("HMAC_TEST_SECRET").and_return(secret)
   end
 
@@ -273,16 +274,34 @@ def create_timestamped_signature(timestamp, version = "v0")
       it "handles very long signatures gracefully" do
         long_signature = "sha256=" + ("a" * 10000)
         long_headers = { default_header => long_signature }
+        expect(log).to receive(:warn).with(/Signature length exceeds maximum limit/)
         expect(valid_with(headers: long_headers)).to be false
       end
 
+      it "returns false for signatures exceeding maximum length limit" do
+        # Create signature larger than MAX_SIGNATURE_LENGTH (1024 + 1 characters)
+        oversized_signature = "sha256=" + ("a" * (1024 - 7 + 1)) # -7 for "sha256=" prefix
+        oversized_headers = { default_header => oversized_signature }
+        expect(log).to receive(:warn).with(/Signature length exceeds maximum limit/)
+        expect(valid_with(headers: oversized_headers)).to be false
+      end
+
       it "handles very long payloads" do
         long_payload = "a" * 100000
         long_signature = create_algorithm_prefixed_signature(long_payload)
         long_headers = { default_header => long_signature }
         expect(valid_with(payload: long_payload, headers: long_headers)).to be true
       end
 
+      it "returns false for payloads exceeding maximum size limit" do
+        # Create payload larger than MAX_PAYLOAD_SIZE (10MB + 1 byte)
+        oversized_payload = "a" * (10 * 1024 * 1024 + 1)
+        signature = create_algorithm_prefixed_signature(payload) # Use regular payload for signature
+        headers_with_signature = { default_header => signature }
+        expect(log).to receive(:warn).with(/Payload size exceeds maximum limit/)
+        expect(valid_with(payload: oversized_payload, headers: headers_with_signature)).to be false
+      end
+
       it "returns false and logs for signature containing non-null control characters" do
         control_char = "\x01"
         headers_with_control = { default_header => signature + control_char }