Merge pull request #44 from github/copilot/fix-43

feat: refactor `payload` and `headers` into pure JSON

Author: GrantBirki

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    hooks-ruby (0.0.5)
+    hooks-ruby (0.0.6)
       dry-schema (~> 1.14, >= 1.14.1)
       grape (~> 2.3)
       puma (~> 6.6)

docs/configuration.md

@@ -95,6 +95,12 @@ When set to `true`, enables a catch-all route that will handle requests to unkno
 **Default:** `false`  
 **Example:** `false`
 
+### `normalize_headers`
+
+When set to `true`, normalizes incoming HTTP headers by lowercasing and trimming them. This ensures consistency in header names and values.
+
+**Default:** `true`  
+
 ## Endpoint Options
 
 ### `path`

docs/handler_plugins.md

@@ -15,8 +15,8 @@ Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base
 class Example < Hooks::Plugins::Handlers::Base
   # Process a webhook payload
   #
-  # @param payload [Hash, String] webhook payload (symbolized keys by default)
-  # @param headers [Hash] HTTP headers (symbolized keys by default)
+  # @param payload [Hash, String] webhook payload (pure JSON with string keys)
+  # @param headers [Hash] HTTP headers (string keys, optionally normalized - default is normalized)
   # @param config [Hash] Endpoint configuration
   # @return [Hash] Response data
   def call(payload:, headers:, config:)
@@ -31,9 +31,9 @@ end
 
 The `payload` parameter can be a Hash or a String. If the payload is a String, it will be parsed as JSON. If it is a Hash, it will be passed directly to the handler. The payload can contain any data that the webhook sender wants to send.
 
-By default, the payload is parsed as JSON (if it can be) and then symbolized. This means that the keys in the payload will be converted to symbols. You can disable this auto-symbolization of the payload by setting the environment variable `HOOKS_SYMBOLIZE_PAYLOAD` to `false` or by setting the `symbolize_payload` option to `false` in the global configuration file.
+The payload is parsed as JSON (if it can be) and returned as a pure Ruby hash with string keys, maintaining the original JSON structure. This ensures that the payload is always a valid JSON representation that can be easily serialized and processed.
 
-**TL;DR**: The payload is almost always a Hash with symbolized keys, regardless of whether the original payload was a Hash or a JSON String.
+**TL;DR**: The payload is almost always a Hash with string keys, regardless of whether the original payload was a Hash or a JSON String.
 
 For example, if the client sends the following JSON payload:
 
@@ -50,24 +50,24 @@ It will be parsed and passed to the handler as:
 
 ```ruby
 {
-  hello: "world",
-  foo: ["bar", "baz"],
-  truthy: true,
-  coffee: {is: "good"}
+  "hello" => "world",
+  "foo" => ["bar", "baz"],
+  "truthy" => true,
+  "coffee" => {"is" => "good"}
 }
 ```
 
 ### `headers` Parameter
 
 The `headers` parameter is a Hash that contains the HTTP headers that were sent with the webhook request. It includes standard headers like `host`, `user-agent`, `accept`, and any custom headers that the webhook sender may have included.
 
-By default, the headers are normalized (lowercased and trimmed) and then symbolized. This means that the keys in the headers will be converted to symbols, and any hyphens (`-`) in header names are converted to underscores (`_`). You can disable header symbolization by setting the environment variable `HOOKS_SYMBOLIZE_HEADERS` to `false` or by setting the `symbolize_headers` option to `false` in the global configuration file.
+By default, the headers are normalized (lowercased and trimmed) but kept as string keys to maintain their JSON representation. Header keys are always strings, and any normalization simply ensures consistent formatting (lowercasing and trimming whitespace). You can disable header normalization by setting the environment variable `HOOKS_NORMALIZE_HEADERS` to `false` or by setting the `normalize_headers` option to `false` in the global configuration file.
 
-**TL;DR**: The headers are almost always a Hash with symbolized keys, with hyphens converted to underscores.
+**TL;DR**: The headers are always a Hash with string keys, optionally normalized for consistency.
 
 For example, if the client sends the following headers:
 
-```
+```text
 Host: hooks.example.com
 User-Agent: foo-client/1.0
 Accept: application/json, text/plain, */*
@@ -79,25 +79,23 @@ X-Forwarded-Proto: https
 Authorization: Bearer <TOKEN>
 ```
 
-They will be normalized and symbolized and passed to the handler as:
+They will be normalized and passed to the handler as:
 
 ```ruby
 {
-  host: "hooks.example.com",
-  user_agent: "foo-client/1.0",
-  accept: "application/json, text/plain, */*",
-  accept_encoding: "gzip, compress, deflate, br",
-  client_name: "foo",
-  x_forwarded_for: "<IP_ADDRESS>",
-  x_forwarded_host: "hooks.example.com",
-  x_forwarded_proto: "https",
-  authorization: "Bearer <TOKEN>" # a careful reminder that headers *can* contain sensitive information!
+  "host" => "hooks.example.com",
+  "user-agent" => "foo-client/1.0",
+  "accept" => "application/json, text/plain, */*",
+  "accept-encoding" => "gzip, compress, deflate, br",
+  "client-name" => "foo",
+  "x-forwarded-for" => "<IP_ADDRESS>",
+  "x-forwarded-host" => "hooks.example.com",
+  "x-forwarded-proto" => "https",
+  "authorization" => "Bearer <TOKEN>" # a careful reminder that headers *can* contain sensitive information!
 }
 ```
 
-It should be noted that the `headers` parameter is a Hash with **symbolized keys** (not strings) by default. They are also normalized (lowercased and trimmed) to ensure consistency.
-
-You can disable header symbolization by either setting the environment variable `HOOKS_SYMBOLIZE_HEADERS` to `false` or by setting the `symbolize_headers` option to `false` in the global configuration file.
+It should be noted that the `headers` parameter is a Hash with **string keys** (not symbols). They are optionally normalized (lowercased and trimmed) to ensure consistency.
 
 You can disable header normalization by either setting the environment variable `HOOKS_NORMALIZE_HEADERS` to `false` or by setting the `normalize_headers` option to `false` in the global configuration file.
 

lib/hooks/app/api.rb

@@ -102,14 +102,13 @@ def self.create(config:, endpoints:, log:)
                     validate_auth!(raw_body, headers, endpoint_config, config)
                   end
 
-                  payload = parse_payload(raw_body, headers, symbolize: config[:symbolize_payload])
+                  payload = parse_payload(raw_body, headers, symbolize: false)
                   handler = load_handler(handler_class_name)
-                  normalized_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers
-                  symbolized_headers = config[:symbolize_headers] ? Hooks::Utils::Normalize.symbolize_headers(normalized_headers) : normalized_headers
+                  processed_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers
 
                   response = handler.call(
                     payload:,
-                    headers: symbolized_headers,
+                    headers: processed_headers,
                     config: endpoint_config
                   )
 

lib/hooks/app/helpers.rb

@@ -44,9 +44,9 @@ def enforce_request_limits(config)
       #
       # @param raw_body [String] The raw request body
       # @param headers [Hash] The request headers
-      # @param symbolize [Boolean] Whether to symbolize keys in parsed JSON (default: true)
-      # @return [Hash, String] Parsed JSON as Hash (optionally symbolized), or raw body if not JSON
-      def parse_payload(raw_body, headers, symbolize: true)
+      # @param symbolize [Boolean] Whether to symbolize keys in parsed JSON (default: false)
+      # @return [Hash, String] Parsed JSON as Hash with string keys, or raw body if not JSON
+      def parse_payload(raw_body, headers, symbolize: false)
         # Optimized content type check - check most common header first
         content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] || headers["content-type"] || headers["HTTP_CONTENT_TYPE"]
 
@@ -55,6 +55,7 @@ def parse_payload(raw_body, headers, symbolize: true)
           begin
             # Security: Limit JSON parsing depth and complexity to prevent JSON bombs
             parsed_payload = safe_json_parse(raw_body)
+            # Note: symbolize parameter is kept for backward compatibility but defaults to false
             parsed_payload = parsed_payload.transform_keys(&:to_sym) if symbolize && parsed_payload.is_a?(Hash)
             return parsed_payload
           rescue JSON::ParserError, ArgumentError => e

lib/hooks/core/config_loader.rb

@@ -20,9 +20,7 @@ class ConfigLoader
         production: true,
         endpoints_dir: "./config/endpoints",
         use_catchall_route: false,
-        symbolize_payload: true,
-        normalize_headers: true,
-        symbolize_headers: true
+        normalize_headers: true
       }.freeze
 
       SILENCE_CONFIG_LOADER_MESSAGES = ENV.fetch(
@@ -142,9 +140,7 @@ def self.load_env_config
           "HOOKS_ENVIRONMENT" => :environment,
           "HOOKS_ENDPOINTS_DIR" => :endpoints_dir,
           "HOOKS_USE_CATCHALL_ROUTE" => :use_catchall_route,
-          "HOOKS_SYMBOLIZE_PAYLOAD" => :symbolize_payload,
           "HOOKS_NORMALIZE_HEADERS" => :normalize_headers,
-          "HOOKS_SYMBOLIZE_HEADERS" => :symbolize_headers,
           "HOOKS_SOME_STRING_VAR" => :some_string_var # Added for test
         }
 
@@ -156,7 +152,7 @@ def self.load_env_config
           case config_key
           when :request_limit, :request_timeout
             env_config[config_key] = value.to_i
-          when :use_catchall_route, :symbolize_payload, :normalize_headers, :symbolize_headers
+          when :use_catchall_route, :normalize_headers
             # Convert string to boolean
             env_config[config_key] = %w[true 1 yes on].include?(value.downcase)
           else

lib/hooks/core/config_validator.rb

@@ -26,7 +26,6 @@ class ValidationError < StandardError; end
         optional(:environment).filled(:string, included_in?: %w[development production])
         optional(:endpoints_dir).filled(:string)
         optional(:use_catchall_route).filled(:bool)
-        optional(:symbolize_payload).filled(:bool)
         optional(:normalize_headers).filled(:bool)
       end
 

lib/hooks/plugins/handlers/base.rb

@@ -15,7 +15,7 @@ class Base
         # Process a webhook request
         #
         # @param payload [Hash, String] Parsed request body (JSON Hash) or raw string
-        # @param headers [Hash] HTTP headers (symbolized keys by default)
+        # @param headers [Hash] HTTP headers (string keys, optionally normalized - default is normalized)
         # @param config [Hash] Merged endpoint configuration including opts section (symbolized keys)
         # @return [Hash, String, nil] Response body (will be auto-converted to JSON)
         # @raise [NotImplementedError] if not implemented by subclass

lib/hooks/version.rb

@@ -4,5 +4,5 @@
 module Hooks
   # Current version of the Hooks webhook framework
   # @return [String] The version string following semantic versioning
-  VERSION = "0.0.5".freeze
+  VERSION = "0.0.6".freeze
 end

spec/acceptance/plugins/handlers/team1_handler.rb

@@ -25,7 +25,7 @@ def call(payload:, headers:, config:)
 
     # Process the payload based on type
     if payload.is_a?(Hash)
-      event_type = payload[:event_type] || "unknown"
+      event_type = payload["event_type"] || "unknown"
 
       case event_type
       when "deployment"