Merge pull request #42 from github/copilot/fix-41

task: implement header symbolization with opt-out configuration

Author: GrantBirki

Gemfile.lock

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

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
-  # @param headers [Hash<String, String>] HTTP headers
+  # @param payload [Hash, String] webhook payload (symbolized keys by default)
+  # @param headers [Hash] HTTP headers (symbolized keys by default)
   # @param config [Hash] Endpoint configuration
   # @return [Hash] Response data
   def call(payload:, headers:, config:)
@@ -61,27 +61,45 @@ It will be parsed and passed to the handler as:
 
 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.
 
-Here is an example of what the `headers` parameter might look like:
+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.
+
+**TL;DR**: The headers are almost always a Hash with symbolized keys, with hyphens converted to underscores.
+
+For example, if the client sends the following headers:
+
+```
+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>
+```
+
+They will be normalized and symbolized and passed to the handler as:
 
 ```ruby
-# example headers as a Hash
 {
-  "host" => "<HOSTNAME>", # e.g., "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" => "<HOSTNAME>", # e.g., "hooks.example.com"
-  "x-forwarded-proto" => "https",
-  "version" => "HTTP/1.1",
-  "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 **String keys** (not symbols). They are also normalized (lowercased and trimmed) to ensure consistency.
+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.
 
-You can disable this 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.
+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.
 
 ### `config` Parameter
 

lib/hooks/app/api.rb

@@ -105,10 +105,11 @@ def self.create(config:, endpoints:, log:)
                   payload = parse_payload(raw_body, headers, symbolize: config[:symbolize_payload])
                   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
 
                   response = handler.call(
                     payload:,
-                    headers: normalized_headers,
+                    headers: symbolized_headers,
                     config: endpoint_config
                   )
 

lib/hooks/core/config_loader.rb

@@ -21,7 +21,8 @@ class ConfigLoader
         endpoints_dir: "./config/endpoints",
         use_catchall_route: false,
         symbolize_payload: true,
-        normalize_headers: true
+        normalize_headers: true,
+        symbolize_headers: true
       }.freeze
 
       SILENCE_CONFIG_LOADER_MESSAGES = ENV.fetch(
@@ -143,6 +144,7 @@ def self.load_env_config
           "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
         }
 
@@ -154,7 +156,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
+          when :use_catchall_route, :symbolize_payload, :normalize_headers, :symbolize_headers
             # Convert string to boolean
             env_config[config_key] = %w[true 1 yes on].include?(value.downcase)
           else

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<String, String>] HTTP headers
+        # @param headers [Hash] HTTP headers (symbolized keys by default)
         # @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/utils/normalize.rb

@@ -58,6 +58,39 @@ def self.headers(headers)
         normalized
       end
 
+      # Symbolize header keys in a hash
+      #
+      # @param headers [Hash, #each] Headers hash or hash-like object
+      # @return [Hash] Hash with symbolized keys (hyphens converted to underscores)
+      #
+      # @example Header symbolization
+      #   headers = { "content-type" => "application/json", "x-github-event" => "push" }
+      #   symbolized = Normalize.symbolize_headers(headers)
+      #   # => { content_type: "application/json", x_github_event: "push" }
+      #
+      # @example Handle various input types
+      #   Normalize.symbolize_headers(nil)  # => nil
+      #   Normalize.symbolize_headers({})   # => {}
+      def self.symbolize_headers(headers)
+        # Handle nil input
+        return nil if headers.nil?
+
+        # Fast path for non-enumerable inputs
+        return {} unless headers.respond_to?(:each)
+
+        symbolized = {}
+
+        headers.each do |key, value|
+          next if key.nil?
+
+          # Convert key to symbol, replacing hyphens with underscores
+          symbolized_key = key.to_s.tr("-", "_").to_sym
+          symbolized[symbolized_key] = value
+        end
+
+        symbolized
+      end
+
       # Normalize a single HTTP header name
       #
       # @param header [String] Header name to normalize

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.4".freeze
+  VERSION = "0.0.5".freeze
 end

spec/integration/header_symbolization_spec.rb

@@ -0,0 +1,74 @@
+# frozen_string_literal: true
+
+ENV["HOOKS_SILENCE_CONFIG_LOADER_MESSAGES"] = "true" # Silence config loader messages in tests
+
+require_relative "../../lib/hooks"
+
+describe "Header Symbolization Integration" do
+  let(:config) do
+    {
+      symbolize_headers: true,
+      normalize_headers: true
+    }
+  end
+
+  let(:headers) do
+    {
+      "Content-Type" => "application/json",
+      "X-GitHub-Event" => "push",
+      "User-Agent" => "test-agent",
+      "Accept-Encoding" => "gzip, br"
+    }
+  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
+
+      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({
+        "content-type" => "application/json",
+        "x-github-event" => "push",
+        "user-agent" => "test-agent",
+        "accept-encoding" => "gzip, br"
+      })
+    end
+  end
+
+  context "when both symbolize_headers and normalize_headers are 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
+
+      expect(symbolized_headers).to eq(headers)
+    end
+  end
+end

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

@@ -20,7 +20,8 @@
           endpoints_dir: "./config/endpoints",
           use_catchall_route: false,
           symbolize_payload: true,
-          normalize_headers: true
+          normalize_headers: true,
+          symbolize_headers: true
         )
       end
     end
@@ -189,6 +190,7 @@
         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"
 
@@ -198,6 +200,7 @@
         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
@@ -370,6 +373,13 @@
           handler: "ValidHandler"
         )
       end
+      it "allows opt-out via environment variable" do
+        ENV["HOOKS_SYMBOLIZE_HEADERS"] = "false"
+
+        config = described_class.load
+
+        expect(config[:symbolize_headers]).to be false
+      end
     end
   end
 

spec/unit/lib/hooks/plugins/utils/normalize_spec.rb

@@ -117,4 +117,84 @@
       end
     end
   end
+
+  describe ".symbolize_headers" do
+    context "when input is a hash of headers" do
+      it "converts header keys to symbols and replaces hyphens with underscores" do
+        headers = {
+          "content-type" => "application/json",
+          "x-github-event" => "push",
+          "user-agent" => "test-agent",
+          "authorization" => "Bearer token123"
+        }
+
+        symbolized = described_class.symbolize_headers(headers)
+
+        expect(symbolized).to eq({
+          content_type: "application/json",
+          x_github_event: "push",
+          user_agent: "test-agent",
+          authorization: "Bearer token123"
+        })
+      end
+
+      it "handles mixed case and already symbolized keys" do
+        headers = {
+          "Content-Type" => "application/json",
+          "X-GitHub-Event" => "push",
+          :already_symbol => "value"
+        }
+
+        symbolized = described_class.symbolize_headers(headers)
+
+        expect(symbolized).to eq({
+          Content_Type: "application/json",
+          X_GitHub_Event: "push",
+          already_symbol: "value"
+        })
+      end
+
+      it "handles nil keys by skipping them" do
+        headers = {
+          "valid-header" => "value",
+          nil => "should-be-skipped"
+        }
+
+        symbolized = described_class.symbolize_headers(headers)
+
+        expect(symbolized).to eq({
+          valid_header: "value"
+        })
+      end
+
+      it "handles nil input" do
+        expect(described_class.symbolize_headers(nil)).to eq(nil)
+      end
+
+      it "handles empty hash input" do
+        expect(described_class.symbolize_headers({})).to eq({})
+      end
+
+      it "handles non-enumerable input" do
+        expect(described_class.symbolize_headers(123)).to eq({})
+        expect(described_class.symbolize_headers(true)).to eq({})
+      end
+
+      it "preserves header values unchanged" do
+        headers = {
+          "x-custom-header" => ["array", "values"],
+          "numeric-header" => 123,
+          "boolean-header" => true
+        }
+
+        symbolized = described_class.symbolize_headers(headers)
+
+        expect(symbolized).to eq({
+          x_custom_header: ["array", "values"],
+          numeric_header: 123,
+          boolean_header: true
+        })
+      end
+    end
+  end
 end