Remove symbolization options and update to pure JSON format

Co-authored-by: GrantBirki <[email protected]>

Author: Copilot

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)
   # @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,20 +50,20 @@ 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:
 
@@ -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
 

spec/integration/header_symbolization_spec.rb

@@ -4,10 +4,9 @@
 
 require_relative "../../lib/hooks"
 
-describe "Header Symbolization Integration" do
+describe "Header Normalization Integration" do
   let(:config) do
     {
-      symbolize_headers: true,
       normalize_headers: true
     }
   end
@@ -21,33 +20,11 @@
     }
   end
 
-  context "when symbolize_headers is enabled (default)" do
-    it "normalizes and symbolizes headers" do
-      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
+  context "when normalize_headers is enabled (default)" do
+    it "normalizes headers but keeps string keys" do
+      processed_headers = config[:normalize_headers] ? Hooks::Utils::Normalize.headers(headers) : headers
 
-      expect(symbolized_headers).to eq({
-        content_type: "application/json",
-        x_github_event: "push",
-        user_agent: "test-agent",
-        accept_encoding: "gzip, br"
-      })
-    end
-  end
-
-  context "when symbolize_headers is disabled" do
-    let(:config) do
-      {
-        symbolize_headers: false,
-        normalize_headers: true
-      }
-    end
-
-    it "normalizes but does not symbolize headers" do
-      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
-
-      expect(symbolized_headers).to eq({
+      expect(processed_headers).to eq({
         "content-type" => "application/json",
         "x-github-event" => "push",
         "user-agent" => "test-agent",
@@ -56,19 +33,17 @@
     end
   end
 
-  context "when both symbolize_headers and normalize_headers are disabled" do
+  context "when normalize_headers is disabled" do
     let(:config) do
       {
-        symbolize_headers: false,
         normalize_headers: false
       }
     end
 
     it "passes headers through unchanged" do
-      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
 
-      expect(symbolized_headers).to eq(headers)
+      expect(processed_headers).to eq(headers)
     end
   end
 end

spec/unit/lib/hooks/app/helpers_spec.rb

@@ -121,7 +121,7 @@ def error!(message, code)
 
         result = helper.parse_payload(body, headers)
 
-        expect(result).to eq({ key: "value" })
+        expect(result).to eq({ "key" => "value" })
       end
 
       it "parses JSON that looks like JSON without content type" do
@@ -130,7 +130,7 @@ def error!(message, code)
 
         result = helper.parse_payload(body, headers)
 
-        expect(result).to eq({ key: "value" })
+        expect(result).to eq({ "key" => "value" })
       end
 
       it "parses JSON arrays" do
@@ -142,15 +142,15 @@ def error!(message, code)
         expect(result).to eq([{ "key" => "value" }])
       end
 
-      it "symbolizes keys by default" do
+      it "does not symbolize keys by default" do
         headers = { "Content-Type" => "application/json" }
         body = '{"string_key": "value", "nested": {"inner_key": "inner_value"}}'
 
         result = helper.parse_payload(body, headers)
 
         expect(result).to eq({
-          string_key: "value",
-          nested: { "inner_key" => "inner_value" } # Only top level is symbolized
+          "string_key" => "value",
+          "nested" => { "inner_key" => "inner_value" }
         })
       end
 
@@ -171,7 +171,7 @@ def error!(message, code)
 
         result = helper.parse_payload(body, headers)
 
-        expect(result).to eq({ key: "value" })
+        expect(result).to eq({ "key" => "value" })
       end
 
       it "handles lowercase content-type" do
@@ -180,7 +180,7 @@ def error!(message, code)
 
         result = helper.parse_payload(body, headers)
 
-        expect(result).to eq({ key: "value" })
+        expect(result).to eq({ "key" => "value" })
       end
 
       it "handles HTTP_CONTENT_TYPE" do
@@ -189,7 +189,7 @@ def error!(message, code)
 
         result = helper.parse_payload(body, headers)
 
-        expect(result).to eq({ key: "value" })
+        expect(result).to eq({ "key" => "value" })
       end
     end
 
@@ -212,7 +212,7 @@ def error!(message, code)
 
         result = helper.parse_payload(nested_json, headers)
 
-        expect(result).to eq({ level1: { "level2" => { "level3" => { "value" => "test" } } } })
+        expect(result).to eq({ "level1" => { "level2" => { "level3" => { "value" => "test" } } } })
       end
 
       it "returns raw body when JSON exceeds size limits" do

spec/unit/lib/hooks/core/config_loader_spec.rb

@@ -19,9 +19,7 @@
           production: true,
           endpoints_dir: "./config/endpoints",
           use_catchall_route: false,
-          symbolize_payload: true,
-          normalize_headers: true,
-          symbolize_headers: true
+          normalize_headers: true
         )
       end
     end
@@ -188,19 +186,15 @@
 
       it "converts boolean environment variables correctly" do
         ENV["HOOKS_USE_CATCHALL_ROUTE"] = "true"
-        ENV["HOOKS_SYMBOLIZE_PAYLOAD"] = "1"
         ENV["HOOKS_NORMALIZE_HEADERS"] = "yes"
-        ENV["HOOKS_SYMBOLIZE_HEADERS"] = "on"
         # Add a non-boolean var to ensure it's not misinterpreted
         ENV["HOOKS_SOME_STRING_VAR"] = "test_value"
 
 
         config = described_class.load
 
         expect(config[:use_catchall_route]).to be true
-        expect(config[:symbolize_payload]).to be true
         expect(config[:normalize_headers]).to be true
-        expect(config[:symbolize_headers]).to be true
         expect(config[:some_string_var]).to eq("test_value") # Check the string var
       end
     end
@@ -373,12 +367,12 @@
           handler: "ValidHandler"
         )
       end
-      it "allows opt-out via environment variable" do
-        ENV["HOOKS_SYMBOLIZE_HEADERS"] = "false"
+      it "allows environment variable setup" do
+        ENV["HOOKS_NORMALIZE_HEADERS"] = "false"
 
         config = described_class.load
 
-        expect(config[:symbolize_headers]).to be false
+        expect(config[:normalize_headers]).to be false
       end
     end
   end

spec/unit/lib/hooks/core/config_validator_spec.rb

@@ -15,7 +15,6 @@
           environment: "development",
           endpoints_dir: "./custom/endpoints",
           use_catchall_route: true,
-          symbolize_payload: false,
           normalize_headers: false
         }
 
@@ -115,14 +114,12 @@
       it "coerces boolean-like string values" do
         config = {
           use_catchall_route: "true",
-          symbolize_payload: "false",
           normalize_headers: "1"
         }
 
         result = described_class.validate_global_config(config)
 
         expect(result[:use_catchall_route]).to be true
-        expect(result[:symbolize_payload]).to be false
         expect(result[:normalize_headers]).to be true # "1" gets coerced to true
       end