Skip to content

task: implement header symbolization with opt-out configuration #42

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 6 commits into from
Jun 13, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 13, 2025

This PR implements header symbolization for webhook handlers, following the same pattern as payload symbolization. Headers are now symbolized by default with hyphens converted to underscores, making them consistent with other symbolized parameters.

Changes Made

Core Implementation

  • Added symbolize_headers: true to default configuration
  • Added HOOKS_SYMBOLIZE_HEADERS environment variable for opt-out
  • Created Hooks::Utils::Normalize.symbolize_headers method that converts header keys to symbols and replaces hyphens with underscores
  • Updated API request processing to apply header symbolization after normalization

Example Transformation

# Before (string keys)
{
  "content-type" => "application/json",
  "x-github-event" => "push",
  "user-agent" => "GitHub-Hookshot/abc123"
}

# After (symbolized keys) 
{
  content_type: "application/json",
  x_github_event: "push", 
  user_agent: "GitHub-Hookshot/abc123"
}

Opt-out Options

Users can disable header symbolization via:

  • Environment variable: HOOKS_SYMBOLIZE_HEADERS=false
  • Global config: symbolize_headers: false

Documentation Updates

  • Updated handler base class YARD documentation
  • Updated docs/handler_plugins.md with new symbolized headers behavior
  • Added examples showing the transformation and opt-out configuration

Testing

  • Added comprehensive unit tests for header symbolization
  • Added integration tests for end-to-end functionality
  • Added configuration tests for environment variable opt-out
  • All existing tests continue to pass with 100% coverage maintained

Backward Compatibility

  • Auth validation continues to receive raw headers for proper signature validation
  • Default behavior is symbolization (enabled by default)
  • Easy opt-out available for users who need string keys

Fixes #41.


💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.

@Copilot Copilot AI changed the title [WIP] task: header symbolization task: implement header symbolization with opt-out configuration Jun 13, 2025
@Copilot Copilot AI requested a review from GrantBirki June 13, 2025 00:59
Copilot finished work on behalf of GrantBirki June 13, 2025 00:59
@GrantBirki GrantBirki marked this pull request as ready for review June 13, 2025 04:09
@Copilot Copilot AI review requested due to automatic review settings June 13, 2025 04:09
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds header symbolization to webhook handlers with opt-out configuration, ensuring headers are consistent with other symbolized parameters.

  • Introduces symbolize_headers utility and integrates it into the request handling flow.
  • Extends the config loader with a new symbolize_headers option (default true) and corresponding HOOKS_SYMBOLIZE_HEADERS environment variable.
  • Updates documentation and tests (unit and integration) to cover header symbolization behavior.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
spec/unit/lib/hooks/plugins/utils/normalize_spec.rb Added unit tests covering symbolize_headers for various inputs
spec/unit/lib/hooks/core/config_loader_spec.rb Updated config loader specs to include symbolize_headers default and opt-out
spec/integration/header_symbolization_spec.rb Added end-to-end integration tests for header symbolization
lib/hooks/utils/normalize.rb Implemented Normalize.symbolize_headers and added YARD docs
lib/hooks/plugins/handlers/base.rb Updated handler base class docs to reflect symbolized headers
lib/hooks/core/config_loader.rb Added symbolize_headers to default config and env mapping
lib/hooks/app/api.rb Integrated symbolization into API request processing
docs/handler_plugins.md Expanded documentation to describe header symbolization and opt-out
.bundle/config Bundler config paths updated (review portability)
Comments suppressed due to low confidence (4)

lib/hooks/utils/normalize.rb:64

  • Update the @return annotation to reflect that symbolize_headers may return nil for nil input or an empty Hash for non-enumerable inputs (e.g., @return [Hash, nil]).
@return [Hash] Hash with symbolized keys (hyphens converted to underscores)

lib/hooks/plugins/handlers/base.rb:18

  • [nitpick] Consider refining the type annotation to something like Hash<Symbol, Object> to clarify that keys are symbols and values can be strings, arrays, booleans, etc.
# @param headers [Hash] HTTP headers (symbolized keys by default)

docs/handler_plugins.md:100

  • [nitpick] There's duplicate guidance on disabling symbolization and normalization nearby; consider consolidating these instructions for clarity and to avoid repetition.
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.

.bundle/config:3

  • The Bundler config contains an absolute CI path, which can hinder portability; consider reverting to a project-local or relative path, or omitting .bundle/config from version control.
BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle"

@GrantBirki GrantBirki merged commit d1ed74f into main Jun 13, 2025
22 checks passed
@GrantBirki GrantBirki deleted the copilot/fix-41 branch June 13, 2025 04:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

task: header symbolization
2 participants