From e236552ad3675ecccc26ef29ffef3144bc988114 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 13:39:13 -0700 Subject: [PATCH 01/25] first draft --- .gitignore | 5 +- bin/hooks | 181 ++++++++++ config.ru | 9 + config/config.yaml | 17 + config/endpoints/github.yaml | 16 + config/endpoints/team1.yaml | 16 + config/puma.rb | 18 + demo/demo.rb | 121 +++++++ demo/simple_test.rb | 23 ++ handlers/github_handler.rb | 162 +++++++++ handlers/team1_handler.rb | 95 +++++ handlers/test_handler.rb | 12 + hooks.gemspec | 5 +- lib/hooks.rb | 29 ++ lib/hooks/app/api_factory.rb | 331 ++++++++++++++++++ lib/hooks/core/builder.rb | 95 +++++ lib/hooks/core/config_loader.rb | 140 ++++++++ lib/hooks/core/config_validator.rb | 91 +++++ lib/hooks/core/logger_factory.rb | 89 +++++ lib/hooks/core/signal_handler.rb | 53 +++ lib/hooks/handlers/base.rb | 21 ++ lib/hooks/plugins/lifecycle.rb | 33 ++ lib/hooks/plugins/signature_validator/base.rb | 24 ++ .../signature_validator/hmac_sha256.rb | 46 +++ script/server | 13 +- spec/integration/hooks_integration_spec.rb | 165 +++++++++ 26 files changed, 1806 insertions(+), 4 deletions(-) create mode 100755 bin/hooks create mode 100644 config.ru create mode 100644 config/config.yaml create mode 100644 config/endpoints/github.yaml create mode 100644 config/endpoints/team1.yaml create mode 100644 config/puma.rb create mode 100755 demo/demo.rb create mode 100644 demo/simple_test.rb create mode 100644 handlers/github_handler.rb create mode 100644 handlers/team1_handler.rb create mode 100644 handlers/test_handler.rb create mode 100644 lib/hooks.rb create mode 100644 lib/hooks/app/api_factory.rb create mode 100644 lib/hooks/core/builder.rb create mode 100644 lib/hooks/core/config_loader.rb create mode 100644 lib/hooks/core/config_validator.rb create mode 100644 lib/hooks/core/logger_factory.rb create mode 100644 lib/hooks/core/signal_handler.rb create mode 100644 lib/hooks/handlers/base.rb create mode 100644 lib/hooks/plugins/lifecycle.rb create mode 100644 lib/hooks/plugins/signature_validator/base.rb create mode 100644 lib/hooks/plugins/signature_validator/hmac_sha256.rb create mode 100644 spec/integration/hooks_integration_spec.rb diff --git a/.gitignore b/.gitignore index 82edbbeb..5ebc4e9d 100644 --- a/.gitignore +++ b/.gitignore @@ -1,4 +1,7 @@ -bin/ +# Ignore binstubs but do commit the one specific for this code. +bin/* +!bin/hooks + coverage/ logs/ tmp/ diff --git a/bin/hooks b/bin/hooks new file mode 100755 index 00000000..fa317544 --- /dev/null +++ b/bin/hooks @@ -0,0 +1,181 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Development CLI script for the hooks framework + +# Add lib directory to load path so we can require our code +lib_dir = File.expand_path("../lib", __dir__) +$LOAD_PATH.unshift(lib_dir) unless $LOAD_PATH.include?(lib_dir) + +# Set bundle gemfile +ENV["BUNDLE_GEMFILE"] ||= File.expand_path("../Gemfile", __dir__) + +require "bundler/setup" +require "optparse" +require "hooks" +require "yaml" + +# CLI implementation +class HooksCLI + def initialize + @options = { + config_file: "hooks.yaml", + port: 4567, + host: "0.0.0.0", + environment: "development", + threads: "5:5" + } + end + + def run(args = ARGV) + # Handle version and help flags before parsing other options + if args.include?("--version") || args.include?("-v") + puts Hooks::VERSION + exit + end + + if args.include?("--help") || args.include?("-h") || args.include?("help") + show_help + exit + end + + parse_options(args) + + case args.first + when "start", nil + start_server + when "version" + puts Hooks::VERSION + else + puts "Unknown command: #{args.first}" + show_help + exit 1 + end + end + + private + + def parse_options(args) + OptionParser.new do |opts| + opts.banner = "Usage: hooks [command] [options]" + + opts.on("-c", "--config FILE", "Configuration file (default: hooks.yaml)") do |file| + @options[:config_file] = file + end + + opts.on("-p", "--port PORT", Integer, "Port to listen on (default: 4567)") do |port| + @options[:port] = port + end + + opts.on("-H", "--host HOST", "Host to bind to (default: 0.0.0.0)") do |host| + @options[:host] = host + end + + opts.on("-e", "--environment ENV", "Environment (default: development)") do |env| + @options[:environment] = env + end + + opts.on("-t", "--threads THREADS", "Puma thread pool size (default: 5:5)") do |threads| + @options[:threads] = threads + end + + opts.on("-h", "--help", "Show this help message") do + show_help + exit + end + + opts.on("-v", "--version", "Show version") do + puts Hooks::VERSION + exit + end + end.parse!(args) + end + + def start_server + puts "Starting Hooks webhook server..." + puts "Config file: #{@options[:config_file]}" + puts "Host: #{@options[:host]}" + puts "Port: #{@options[:port]}" + puts "Environment: #{@options[:environment]}" + puts "Threads: #{@options[:threads]}" + puts + + # parse the configuration file + if File.exist?(@options[:config_file]) + begin + config = YAML.load_file(@options[:config_file]) + rescue Psych::SyntaxError => e + puts "Error parsing configuration file: #{e.message}" + exit 1 + end + else + puts "Configuration file #{@options[:config_file]} not found. Using defaults." + config = {} + end + + # Merge CLI options into config + config.merge!({ + "host" => @options[:host], + "port" => @options[:port], + "environment" => @options[:environment], + "threads" => @options[:threads] + }) + + # Build the application with framework-level config + app = Hooks.build(config:) + + # Start the server with CLI options + require "rack" + require "rack/handler/puma" + require "puma" + + Rack::Handler::Puma.run( + app, + Host: @options[:host], + Port: @options[:port], + Threads: @options[:threads], + environment: @options[:environment] + ) + rescue Interrupt + puts "\nShutting down gracefully..." + exit 0 + rescue => e + puts "Error starting server: #{e.message}" + puts e.backtrace if @options[:environment] == "development" + exit 1 + end + + def show_help + puts <<~HELP + Hooks - A Pluggable Webhook Server Framework + + Usage: + hooks [start] Start the webhook server (default) + hooks version Show version information + hooks help Show this help message + + Options: + -c, --config FILE Configuration file (default: hooks.yaml) + -p, --port PORT Port to listen on (default: 4567) + -H, --host HOST Host to bind to (default: 0.0.0.0) + -e, --environment ENV Environment (default: development) + -t, --threads THREADS Puma thread pool size (default: 5:5) + -h, --help Show this help message + -v, --version Show version + + Examples: + hooks Start server with default settings + hooks start -p 8080 Start server on port 8080 + hooks -c custom.yaml -e production Start with custom config in production mode + hooks -t 10:10 Start with 10 threads + hooks version Show version information + + For more information, see the README.md file. + HELP + end +end + +# Run the CLI if this file is executed directly +if __FILE__ == $0 + HooksCLI.new.run +end diff --git a/config.ru b/config.ru new file mode 100644 index 00000000..15fbca86 --- /dev/null +++ b/config.ru @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +require_relative "lib/hooks" + +# Build the Hooks application with default config +app = Hooks.build(config: "./config/config.yaml") + +# Run the application +run app diff --git a/config/config.yaml b/config/config.yaml new file mode 100644 index 00000000..4c570393 --- /dev/null +++ b/config/config.yaml @@ -0,0 +1,17 @@ +# Sample configuration for Hooks webhook server +handler_dir: ./handlers +log_level: info + +# Request handling +request_limit: 1048576 # 1MB max body size +request_timeout: 15 # 15 seconds timeout + +# Path configuration +root_path: /webhooks +health_path: /health +metrics_path: /metrics +version_path: /version + +# Runtime behavior +environment: development +endpoints_dir: ./config/endpoints diff --git a/config/endpoints/github.yaml b/config/endpoints/github.yaml new file mode 100644 index 00000000..4f053159 --- /dev/null +++ b/config/endpoints/github.yaml @@ -0,0 +1,16 @@ +# Sample endpoint configuration for GitHub webhooks +path: /github +handler: GitHubHandler + +# GitHub uses HMAC SHA256 signature validation +verify_signature: + type: default + secret_env_key: GITHUB_WEBHOOK_SECRET + header: X-Hub-Signature-256 + algorithm: sha256 + +# Options for GitHub webhook handling +opts: + repository: "my-org/my-repo" + events: ["push", "pull_request", "issues"] + branch_filter: ["main", "develop"] diff --git a/config/endpoints/team1.yaml b/config/endpoints/team1.yaml new file mode 100644 index 00000000..73dd2419 --- /dev/null +++ b/config/endpoints/team1.yaml @@ -0,0 +1,16 @@ +# Sample endpoint configuration for Team 1 +path: /team1 +handler: Team1Handler + +# Signature validation (optional) +# verify_signature: +# type: default +# secret_env_key: TEAM1_SECRET +# header: X-Hub-Signature-256 +# algorithm: sha256 + +# Custom options passed to handler +opts: + env: staging + teams: ["infra", "billing"] + notify_channels: ["#team1-alerts"] diff --git a/config/puma.rb b/config/puma.rb new file mode 100644 index 00000000..b4cb4bae --- /dev/null +++ b/config/puma.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +require "time" +require "json" + +bind "tcp://0.0.0.0:8080" +# single mode: https://github.com/puma/puma/blob/master/docs/deployment.md#single-vs-cluster-mode +workers 0 + +log_formatter do |msg| + timestamp = Time.now.strftime("%Y-%m-%dT%H:%M:%S.%L%z") + { + time: timestamp, + level: ENV.fetch("LOG_LEVEL", "INFO").upcase, + progname: "puma", + msg: msg.rstrip + }.to_json +end diff --git a/demo/demo.rb b/demo/demo.rb new file mode 100755 index 00000000..d0bdc229 --- /dev/null +++ b/demo/demo.rb @@ -0,0 +1,121 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +# Demo script to test the Hooks webhook server + +require_relative "../lib/hooks" +require "net/http" +require "json" +require "uri" + +puts "๐Ÿช Hooks Webhook Server Demo" +puts "=" * 40 + +# Create a simple test config +test_config = { + handler_dir: "./handlers", + log_level: "info", + request_limit: 1048576, + request_timeout: 15, + root_path: "/webhooks", + health_path: "/health", + metrics_path: "/metrics", + version_path: "/version", + environment: "development", + endpoints_dir: "./config/endpoints" +} + +puts "Building Hooks application..." +app = Hooks.build(config: test_config) + +puts "โœ… Application built successfully!" +puts "๐Ÿ“Š Version: #{Hooks::VERSION}" + +# Test with a simple Rack request +puts "\n๐Ÿงช Testing operational endpoints..." + +# Create a simple mock request environment +def mock_request(method, path, body = nil, headers = {}) + env = { + "REQUEST_METHOD" => method, + "PATH_INFO" => path, + "SCRIPT_NAME" => "", + "QUERY_STRING" => "", + "SERVER_NAME" => "localhost", + "SERVER_PORT" => "3000", + "rack.version" => [1, 3], + "rack.url_scheme" => "http", + "rack.input" => StringIO.new(body || ""), + "rack.errors" => $stderr, + "rack.multithread" => false, + "rack.multiprocess" => true, + "rack.run_once" => false + } + + headers.each { |k, v| env["HTTP_#{k.upcase.gsub('-', '_')}"] = v } + env +end + +# Test health endpoint +puts "Testing /health..." +begin + status, headers, body = app.call(mock_request("GET", "/health")) + response_body = body.join("") + parsed = JSON.parse(response_body) + puts " Status: #{status}" + puts " Response: #{parsed['status']} (v#{parsed['version']})" +rescue => e + puts " โŒ Error: #{e.message}" +end + +# Test version endpoint +puts "\nTesting /version..." +begin + status, headers, body = app.call(mock_request("GET", "/version")) + response_body = body.join("") + parsed = JSON.parse(response_body) + puts " Status: #{status}" + puts " Version: #{parsed['version']}" +rescue => e + puts " โŒ Error: #{e.message}" +end + +# Test hello endpoint +puts "\nTesting /webhooks/hello..." +begin + status, headers, body = app.call(mock_request("GET", "/webhooks/hello")) + response_body = body.join("") + parsed = JSON.parse(response_body) + puts " Status: #{status}" + puts " Message: #{parsed['message']}" +rescue => e + puts " โŒ Error: #{e.message}" +end + +# Test webhook endpoint with default handler +puts "\nTesting /webhooks/demo with default handler..." +begin + test_payload = { event: "demo", timestamp: Time.now.iso8601 } + status, headers, body = app.call(mock_request( + "POST", + "/webhooks/demo", + test_payload.to_json, + { "Content-Type" => "application/json" } + )) + response_body = body.join("") + parsed = JSON.parse(response_body) + puts " Status: #{status}" + puts " Handler: #{parsed['handler']}" + puts " Message: #{parsed['message']}" +rescue => e + puts " โŒ Error: #{e.message}" +end + +puts "\nโœ… Demo completed!" +puts "\n๐Ÿš€ To start the server:" +puts " ./bin/hooks serve -p 3000 -c ./config/config.yaml" +puts "\n๐Ÿ“š Example webhook endpoints:" +puts " POST http://localhost:3000/webhooks/team1" +puts " POST http://localhost:3000/webhooks/github" +puts " GET http://localhost:3000/health" +puts " GET http://localhost:3000/metrics" diff --git a/demo/simple_test.rb b/demo/simple_test.rb new file mode 100644 index 00000000..51550f01 --- /dev/null +++ b/demo/simple_test.rb @@ -0,0 +1,23 @@ +#!/usr/bin/env ruby +# frozen_string_literal: true + +require_relative "../lib/hooks" + +puts "Testing Hooks framework..." + +begin + # Test basic instantiation + puts "Creating Hooks app..." + app = Hooks.build + puts "โœ“ App created successfully" + puts "App class: #{app.class}" + + # Test with config + puts "\nTesting with config file..." + app_with_config = Hooks.build(config: "./config/config.yaml") + puts "โœ“ App with config created successfully" + +rescue => e + puts "โœ— Error: #{e.class}: #{e.message}" + puts e.backtrace.first(5).join("\n") +end diff --git a/handlers/github_handler.rb b/handlers/github_handler.rb new file mode 100644 index 00000000..1b978c9c --- /dev/null +++ b/handlers/github_handler.rb @@ -0,0 +1,162 @@ +# frozen_string_literal: true + +require_relative "../lib/hooks/handlers/base" + +# Example handler for GitHub webhooks +class GitHubHandler < Hooks::Handlers::Base + # Process GitHub webhook + # + # @param payload [Hash, String] GitHub webhook payload + # @param headers [Hash] HTTP headers + # @param config [Hash] Endpoint configuration + # @return [Hash] Response data + def call(payload:, headers:, config:) + # GitHub sends event type in header + event_type = headers["X-GitHub-Event"] || "unknown" + + puts "GitHubHandler: Received #{event_type} event" + + return handle_raw_payload(payload, config) unless payload.is_a?(Hash) + + case event_type + when "push" + handle_push_event(payload, config) + when "pull_request" + handle_pull_request_event(payload, config) + when "issues" + handle_issues_event(payload, config) + when "ping" + handle_ping_event(payload, config) + else + handle_unknown_event(payload, event_type, config) + end + end + + private + + # Handle raw string payload + # + # @param payload [String] Raw payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_raw_payload(payload, config) + { + status: "raw_payload_processed", + handler: "GitHubHandler", + payload_size: payload.length, + repository: config.dig(:opts, :repository), + timestamp: Time.now.iso8601 + } + end + + # Handle push events + # + # @param payload [Hash] Push event payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_push_event(payload, config) + ref = payload["ref"] + branch = ref&.split("/")&.last + commits_count = payload.dig("commits")&.length || 0 + + # Check if branch is in filter + branch_filter = config.dig(:opts, :branch_filter) + if branch_filter && !branch_filter.include?(branch) + return { + status: "ignored", + handler: "GitHubHandler", + reason: "branch_not_in_filter", + branch: branch, + filter: branch_filter, + timestamp: Time.now.iso8601 + } + end + + { + status: "push_processed", + handler: "GitHubHandler", + repository: payload.dig("repository", "full_name"), + branch: branch, + commits_count: commits_count, + pusher: payload.dig("pusher", "name"), + timestamp: Time.now.iso8601 + } + end + + # Handle pull request events + # + # @param payload [Hash] Pull request event payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_pull_request_event(payload, config) + action = payload["action"] + pr_number = payload.dig("pull_request", "number") + pr_title = payload.dig("pull_request", "title") + + { + status: "pull_request_processed", + handler: "GitHubHandler", + action: action, + repository: payload.dig("repository", "full_name"), + pr_number: pr_number, + pr_title: pr_title, + author: payload.dig("pull_request", "user", "login"), + timestamp: Time.now.iso8601 + } + end + + # Handle issues events + # + # @param payload [Hash] Issues event payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_issues_event(payload, config) + action = payload["action"] + issue_number = payload.dig("issue", "number") + issue_title = payload.dig("issue", "title") + + { + status: "issue_processed", + handler: "GitHubHandler", + action: action, + repository: payload.dig("repository", "full_name"), + issue_number: issue_number, + issue_title: issue_title, + author: payload.dig("issue", "user", "login"), + timestamp: Time.now.iso8601 + } + end + + # Handle ping events (webhook test) + # + # @param payload [Hash] Ping event payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_ping_event(payload, config) + { + status: "ping_acknowledged", + handler: "GitHubHandler", + repository: payload.dig("repository", "full_name"), + hook_id: payload.dig("hook", "id"), + zen: payload["zen"], + timestamp: Time.now.iso8601 + } + end + + # Handle unknown events + # + # @param payload [Hash] Event payload + # @param event_type [String] Event type + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_unknown_event(payload, event_type, config) + { + status: "unknown_event_processed", + handler: "GitHubHandler", + event_type: event_type, + repository: payload.dig("repository", "full_name"), + supported_events: config.dig(:opts, :events), + timestamp: Time.now.iso8601 + } + end +end diff --git a/handlers/team1_handler.rb b/handlers/team1_handler.rb new file mode 100644 index 00000000..b605f374 --- /dev/null +++ b/handlers/team1_handler.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require_relative "../lib/hooks/handlers/base" + +# Example handler for Team 1 webhooks +class Team1Handler < Hooks::Handlers::Base + # Process Team 1 webhook + # + # @param payload [Hash, String] Webhook payload + # @param headers [Hash] HTTP headers + # @param config [Hash] Endpoint configuration + # @return [Hash] Response data + def call(payload:, headers:, config:) + # Log the webhook receipt + puts "Team1Handler: Received webhook for #{config.dig(:opts, :teams)&.join(', ')}" + + # Process the payload based on type + if payload.is_a?(Hash) + event_type = payload["event_type"] || "unknown" + + case event_type + when "deployment" + handle_deployment(payload, config) + when "alert" + handle_alert(payload, config) + else + handle_generic(payload, config) + end + else + # Handle raw string payload + { + status: "processed", + handler: "Team1Handler", + message: "Raw payload processed", + payload_size: payload.length, + environment: config.dig(:opts, :env), + timestamp: Time.now.iso8601 + } + end + end + + private + + # Handle deployment events + # + # @param payload [Hash] Deployment payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_deployment(payload, config) + { + status: "deployment_processed", + handler: "Team1Handler", + deployment_id: payload["deployment_id"], + environment: payload["environment"] || config.dig(:opts, :env), + teams_notified: config.dig(:opts, :teams), + timestamp: Time.now.iso8601 + } + end + + # Handle alert events + # + # @param payload [Hash] Alert payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_alert(payload, config) + alert_level = payload["level"] || "info" + + # In a real implementation, you might send notifications here + # notify_teams(payload, config.dig(:opts, :notify_channels)) + + { + status: "alert_processed", + handler: "Team1Handler", + alert_id: payload["alert_id"], + level: alert_level, + channels_notified: config.dig(:opts, :notify_channels), + timestamp: Time.now.iso8601 + } + end + + # Handle generic events + # + # @param payload [Hash] Generic payload + # @param config [Hash] Configuration + # @return [Hash] Response + def handle_generic(payload, config) + { + status: "generic_processed", + handler: "Team1Handler", + event_type: payload["event_type"], + environment: config.dig(:opts, :env), + timestamp: Time.now.iso8601 + } + end +end diff --git a/handlers/test_handler.rb b/handlers/test_handler.rb new file mode 100644 index 00000000..b8e548c9 --- /dev/null +++ b/handlers/test_handler.rb @@ -0,0 +1,12 @@ +require_relative "../lib/hooks/handlers/base" + +class TestHandler < Hooks::Handlers::Base + def call(payload:, headers:, config:) + { + status: "test_success", + payload_received: payload, + config_opts: config[:opts], + timestamp: Time.now.iso8601 + } + end +end diff --git a/hooks.gemspec b/hooks.gemspec index 3f9fefe4..33fa654f 100644 --- a/hooks.gemspec +++ b/hooks.gemspec @@ -27,7 +27,10 @@ Gem::Specification.new do |spec| spec.required_ruby_version = Gem::Requirement.new(">= 3.0.0") - spec.files = %w[LICENSE README.md hooks.gemspec] + spec.files = %w[LICENSE README.md hooks.gemspec config.ru] spec.files += Dir.glob("lib/**/*.rb") + spec.files += Dir.glob("bin/*") + spec.bindir = "bin" + spec.executables = ["hooks"] spec.require_paths = ["lib"] end diff --git a/lib/hooks.rb b/lib/hooks.rb new file mode 100644 index 00000000..ddbd2ff3 --- /dev/null +++ b/lib/hooks.rb @@ -0,0 +1,29 @@ +# frozen_string_literal: true + +require_relative "hooks/version" +require_relative "hooks/core/builder" +require_relative "hooks/handlers/base" +require_relative "hooks/plugins/lifecycle" +require_relative "hooks/plugins/signature_validator/base" +require_relative "hooks/plugins/signature_validator/hmac_sha256" + +# Main module for the Hooks webhook server framework +module Hooks + # Build a Rack-compatible webhook server application + # + # @param config [String, Hash] Path to config file or config hash + # @param log [Logger] Custom logger instance (optional) + # @param request_limit [Integer] Maximum request body size in bytes + # @param request_timeout [Integer] Request timeout in seconds + # @param root_path [String] Base path for webhook endpoints + # @return [Object] Rack-compatible application + def self.build(config: nil, log: nil, request_limit: nil, request_timeout: nil, root_path: nil) + Core::Builder.new( + config: config, + log: log, + request_limit: request_limit, + request_timeout: request_timeout, + root_path: root_path + ).build + end +end diff --git a/lib/hooks/app/api_factory.rb b/lib/hooks/app/api_factory.rb new file mode 100644 index 00000000..5c124ee2 --- /dev/null +++ b/lib/hooks/app/api_factory.rb @@ -0,0 +1,331 @@ +# frozen_string_literal: true + +require "grape" +require "json" +require "securerandom" +require_relative "../handlers/base" +require_relative "../plugins/signature_validator/hmac_sha256" +require_relative "../core/logger_factory" + +module Hooks + module App + # Factory for creating configured Grape API classes + class APIFactory + # Create a new configured API class + def self.create(config:, endpoints:, logger:, signal_handler:) + # Store startup time for uptime calculation + start_time = Time.now + + # Capture values in local variables for closure + captured_config = config + captured_endpoints = endpoints + captured_logger = logger + _captured_signal_handler = signal_handler + captured_start_time = start_time + + # Create the API class with dynamic routes + api_class = Class.new(Grape::API) do + # Accept all content types but don't auto-parse + content_type :json, "application/json" + content_type :txt, "text/plain" + content_type :xml, "application/xml" + content_type :any, "*/*" + format :txt # Use text format so no automatic parsing happens + default_format :txt + end + + # Use class_eval to dynamically define routes + api_class.class_eval do + # Define helper methods first, before routes + helpers do + # Enforce request size and timeout limits + def enforce_request_limits(config) + # Check content length (handle different header formats and sources) + content_length = headers["Content-Length"] || headers["CONTENT_LENGTH"] || + headers["content-length"] || headers["HTTP_CONTENT_LENGTH"] || + env["CONTENT_LENGTH"] || env["HTTP_CONTENT_LENGTH"] + + # Also try to get from request object directly + content_length ||= request.content_length if respond_to?(:request) && request.respond_to?(:content_length) + + content_length = content_length&.to_i + + if content_length && content_length > config[:request_limit] + error!("Request body too large", 413) + end + + # Note: Timeout enforcement would typically be handled at the server level (Puma, etc.) + end + + # Validate request signature + def validate_signature(payload, headers, endpoint_config) + signature_config = endpoint_config[:verify_signature] + validator_type = signature_config[:type] || "default" + secret_env_key = signature_config[:secret_env_key] + + return unless secret_env_key + + secret = ENV[secret_env_key] + unless secret + error!("Secret not found in environment", 500) + end + + # Use default validator or load custom + validator_class = if validator_type == "default" + Plugins::SignatureValidator::HmacSha256 + else + # In a full implementation, we'd dynamically load custom validators + error!("Custom validators not implemented in POC", 500) + end + + unless validator_class.valid?( + payload: payload, + headers: headers, + secret: secret, + config: endpoint_config + ) + error!("Invalid signature", 401) + end + end + + # Parse request payload + def parse_payload(raw_body, headers) + content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] + + # Try to parse as JSON if content type suggests it or if it looks like JSON + if content_type&.include?("application/json") || (raw_body.strip.start_with?("{", "[") rescue false) + begin + return JSON.parse(raw_body) + rescue JSON::ParserError + # If JSON parsing fails, return raw body + end + end + + # Return raw body for all other cases + raw_body + end + + # Load handler class + def load_handler(handler_class_name, handler_dir) + # Convert class name to file name (e.g., Team1Handler -> team1_handler.rb) + file_name = handler_class_name.gsub(/([A-Z])/, '_\1').downcase.sub(/^_/, "") + ".rb" + file_path = File.join(handler_dir, file_name) + + if File.exist?(file_path) + require file_path + Object.const_get(handler_class_name).new + else + # Create a default handler for POC + DefaultHandler.new + end + rescue => e + error!("Failed to load handler #{handler_class_name}: #{e.message}", 500) + end + + # Determine HTTP error code from exception + def determine_error_code(exception) + case exception + when ArgumentError then 400 + when SecurityError then 401 + when NotImplementedError then 501 + else 500 + end + end + end + + # Define operational endpoints + get captured_config[:health_path] do + content_type "application/json" + { + status: "healthy", + timestamp: Time.now.iso8601, + version: Hooks::VERSION, + uptime_seconds: (Time.now - captured_start_time).to_i + }.to_json + end + + get captured_config[:metrics_path] do + content_type "application/json" + { message: "Metrics functionality removed for simplification" }.to_json + end + + get captured_config[:version_path] do + content_type "application/json" + { + version: Hooks::VERSION, + timestamp: Time.now.iso8601 + }.to_json + end + + # Hello world demo endpoint + get "#{captured_config[:root_path]}/hello" do + content_type "application/json" + { + message: "Hooks is working!", + version: Hooks::VERSION, + timestamp: Time.now.iso8601 + }.to_json + end + + # Define webhook endpoints dynamically + captured_endpoints.each do |endpoint_config| + full_path = "#{captured_config[:root_path]}#{endpoint_config[:path]}" + handler_class_name = endpoint_config[:handler] + + # Use send to dynamically create POST route + send(:post, full_path) do + request_id = SecureRandom.uuid + start_time = Time.now + + # Use captured values + config = captured_config + logger = captured_logger + + # Set request context for logging + request_context = { + request_id: request_id, + path: full_path, + handler: handler_class_name + } + + Core::LogContext.with(request_context) do + begin + # Enforce request limits + enforce_request_limits(config) + + # Get raw body for signature validation + request.body.rewind + raw_body = request.body.read + + # Validate signature if configured + validate_signature(raw_body, headers, endpoint_config) if endpoint_config[:verify_signature] + + # Parse payload + payload = parse_payload(raw_body, headers) + + # Load and instantiate handler + handler = load_handler(handler_class_name, config[:handler_dir]) + + # Call handler + response = handler.call( + payload: payload, + headers: headers, + config: endpoint_config + ) + + logger.info "Request processed successfully" + + # Return response as JSON string when using txt format + status 200 # Explicitly set status to 200 + content_type "application/json" + (response || { status: "ok" }).to_json + + rescue => e + logger.error "Request failed: #{e.message}" + + # Return error response + error_response = { + error: e.message, + code: determine_error_code(e), + request_id: request_id + } + + # Add backtrace in development + if config[:environment] == "development" + error_response[:backtrace] = e.backtrace + end + + status error_response[:code] + content_type "application/json" + error_response.to_json + end + end + end + end + + # Catch-all route for unknown endpoints - use default handler + post "#{captured_config[:root_path]}/*path" do + request_id = SecureRandom.uuid + start_time = Time.now + + # Use captured values + config = captured_config + logger = captured_logger + + # Set request context for logging + request_context = { + request_id: request_id, + path: "/#{params[:path]}", + handler: "DefaultHandler" + } + + Core::LogContext.with(request_context) do + begin + # Enforce request limits + enforce_request_limits(config) + + # Get raw body for payload parsing + request.body.rewind + raw_body = request.body.read + + # Parse payload + payload = parse_payload(raw_body, headers) + + # Use default handler + handler = DefaultHandler.new + + # Call handler + response = handler.call( + payload: payload, + headers: headers, + config: {} + ) + + logger.info "Request processed successfully with default handler" + + # Return response as JSON string when using txt format + status 200 + content_type "application/json" + (response || { status: "ok" }).to_json + + rescue => e + logger.error "Request failed: #{e.message}" + + # Return error response + error_response = { + error: e.message, + code: determine_error_code(e), + request_id: request_id + } + + # Add backtrace in development + if config[:environment] == "development" + error_response[:backtrace] = e.backtrace + end + + status error_response[:code] + content_type "application/json" + error_response.to_json + end + end + end + end + + # Return the configured API class + api_class + end + + # Default handler for POC when no custom handler is found + class DefaultHandler < Handlers::Base + def call(payload:, headers:, config:) + { + message: "Webhook received", + handler: "DefaultHandler", + payload_size: payload.is_a?(String) ? payload.length : payload.to_s.length, + timestamp: Time.now.iso8601 + } + end + end + end + end +end diff --git a/lib/hooks/core/builder.rb b/lib/hooks/core/builder.rb new file mode 100644 index 00000000..f8fa3dd1 --- /dev/null +++ b/lib/hooks/core/builder.rb @@ -0,0 +1,95 @@ +# frozen_string_literal: true + +require_relative "config_loader" +require_relative "config_validator" +require_relative "logger_factory" +require_relative "signal_handler" +require_relative "../app/api_factory" + +module Hooks + module Core + # Main builder that orchestrates the webhook server setup + class Builder + # Initialize builder with configuration options + # + # @param config [String, Hash] Path to config file or config hash + # @param log [Logger] Custom logger instance + # @param request_limit [Integer] Maximum request body size in bytes + # @param request_timeout [Integer] Request timeout in seconds + # @param root_path [String] Base path for webhook endpoints + def initialize(config: nil, log: nil, request_limit: nil, request_timeout: nil, root_path: nil) + @config_input = config + @custom_logger = log + @programmatic_overrides = { + request_limit: request_limit, + request_timeout: request_timeout, + root_path: root_path + }.compact + end + + # Build and return Rack-compatible application + # + # @return [Object] Rack-compatible application + def build + # Load and validate configuration + config = load_and_validate_config + + # Create logger + logger = LoggerFactory.create( + log_level: config[:log_level], + custom_logger: @custom_logger + ) + + # Setup signal handler for graceful shutdown + signal_handler = SignalHandler.new(logger) + + # Load endpoints + endpoints = load_endpoints(config) + + # Log startup + logger.info "Starting Hooks webhook server v#{Hooks::VERSION}" + logger.info "Config: #{endpoints.size} endpoints loaded" + + # Build and return Grape API class + Hooks::App::APIFactory.create( + config: config, + endpoints: endpoints, + logger: logger, + signal_handler: signal_handler + ) + end + + private + + # Load and validate all configuration + # + # @return [Hash] Validated global configuration + def load_and_validate_config + # Load base config from file/hash and environment + config = ConfigLoader.load(config_path: @config_input) + + # Apply programmatic overrides (highest priority) + config.merge!(@programmatic_overrides) + + # Validate global configuration + ConfigValidator.validate_global_config(config) + rescue ConfigValidator::ValidationError => e + raise ConfigurationError, "Configuration validation failed: #{e.message}" + end + + # Load and validate endpoint configurations + # + # @param config [Hash] Global configuration + # @return [Array] Array of validated endpoint configurations + def load_endpoints(config) + endpoints = ConfigLoader.load_endpoints(config[:endpoints_dir]) + ConfigValidator.validate_endpoints(endpoints) + rescue ConfigValidator::ValidationError => e + raise ConfigurationError, "Endpoint validation failed: #{e.message}" + end + end + + # Configuration error + class ConfigurationError < StandardError; end + end +end diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb new file mode 100644 index 00000000..0d29c759 --- /dev/null +++ b/lib/hooks/core/config_loader.rb @@ -0,0 +1,140 @@ +# frozen_string_literal: true + +require "yaml" +require "json" + +module Hooks + module Core + # Loads and merges configuration from files and environment variables + class ConfigLoader + DEFAULT_CONFIG = { + handler_dir: "./handlers", + log_level: "info", + request_limit: 1_048_576, + request_timeout: 15, + root_path: "/webhooks", + health_path: "/health", + metrics_path: "/metrics", + version_path: "/version", + environment: "production", + endpoints_dir: "./config/endpoints" + }.freeze + + # Load and merge configuration from various sources + # + # @param config_path [String, Hash] Path to config file or config hash + # @return [Hash] Merged configuration + def self.load(config_path: nil) + config = DEFAULT_CONFIG.dup + + # Load from file if path provided + if config_path.is_a?(String) && File.exist?(config_path) + file_config = load_config_file(config_path) + config.merge!(file_config) if file_config + elsif config_path.is_a?(Hash) + config.merge!(config_path) + end + + # Override with environment variables + config.merge!(load_env_config) + + # Convert string keys to symbols for consistency + symbolize_keys(config) + end + + # Load endpoint configurations from directory + # + # @param endpoints_dir [String] Directory containing endpoint config files + # @return [Array] Array of endpoint configurations + def self.load_endpoints(endpoints_dir) + return [] unless endpoints_dir && Dir.exist?(endpoints_dir) + + endpoints = [] + files = Dir.glob(File.join(endpoints_dir, "*.{yml,yaml,json}")) + + files.each do |file| + endpoint_config = load_config_file(file) + if endpoint_config + endpoints << symbolize_keys(endpoint_config) + end + end + + endpoints + end + + private + + # Load configuration from YAML or JSON file + # + # @param file_path [String] Path to config file + # @return [Hash, nil] Parsed configuration or nil if error + def self.load_config_file(file_path) + content = File.read(file_path) + + result = case File.extname(file_path).downcase + when ".json" + JSON.parse(content) + when ".yml", ".yaml" + YAML.safe_load(content, permitted_classes: [Symbol]) + else + nil + end + + result + rescue => e + # In production, we'd log this error + nil + end + + # Load configuration from environment variables + # + # @return [Hash] Configuration from ENV vars + def self.load_env_config + env_config = {} + + env_mappings = { + "HOOKS_HANDLER_DIR" => :handler_dir, + "HOOKS_LOG_LEVEL" => :log_level, + "HOOKS_REQUEST_LIMIT" => :request_limit, + "HOOKS_REQUEST_TIMEOUT" => :request_timeout, + "HOOKS_ROOT_PATH" => :root_path, + "HOOKS_HEALTH_PATH" => :health_path, + "HOOKS_METRICS_PATH" => :metrics_path, + "HOOKS_VERSION_PATH" => :version_path, + "HOOKS_ENVIRONMENT" => :environment, + "HOOKS_ENDPOINTS_DIR" => :endpoints_dir + } + + env_mappings.each do |env_key, config_key| + value = ENV[env_key] + next unless value + + # Convert numeric values + case config_key + when :request_limit, :request_timeout + env_config[config_key] = value.to_i + else + env_config[config_key] = value + end + end + + env_config + end + + # Recursively convert string keys to symbols + # + # @param obj [Hash, Array, Object] Object to convert + # @return [Hash, Array, Object] Converted object + def self.symbolize_keys(obj) + case obj + when Hash + obj.transform_keys(&:to_sym).transform_values { |v| symbolize_keys(v) } + when Array + obj.map { |v| symbolize_keys(v) } + else + obj + end + end + end + end +end diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb new file mode 100644 index 00000000..ef7f1afc --- /dev/null +++ b/lib/hooks/core/config_validator.rb @@ -0,0 +1,91 @@ +# frozen_string_literal: true + +require "dry-schema" + +module Hooks + module Core + # Validates configuration using Dry::Schema + class ConfigValidator + # Global configuration schema + GLOBAL_CONFIG_SCHEMA = Dry::Schema.Params do + optional(:handler_dir).filled(:string) + optional(:log_level).filled(:string, included_in?: %w[debug info warn error]) + optional(:request_limit).filled(:integer, gt?: 0) + optional(:request_timeout).filled(:integer, gt?: 0) + optional(:root_path).filled(:string) + optional(:health_path).filled(:string) + optional(:metrics_path).filled(:string) + optional(:version_path).filled(:string) + optional(:environment).filled(:string, included_in?: %w[development production]) + optional(:endpoints_dir).filled(:string) + end + + # Endpoint configuration schema + ENDPOINT_CONFIG_SCHEMA = Dry::Schema.Params do + required(:path).filled(:string) + required(:handler).filled(:string) + + optional(:verify_signature).hash do + optional(:type).filled(:string) + optional(:secret_env_key).filled(:string) + optional(:header).filled(:string) + optional(:algorithm).filled(:string) + end + + optional(:opts).hash + end + + # Validate global configuration + # + # @param config [Hash] Configuration to validate + # @return [Hash] Validated configuration + # @raise [ValidationError] if validation fails + def self.validate_global_config(config) + result = GLOBAL_CONFIG_SCHEMA.call(config) + + if result.failure? + raise ValidationError, "Invalid global configuration: #{result.errors.to_h}" + end + + result.to_h + end + + # Validate endpoint configuration + # + # @param config [Hash] Endpoint configuration to validate + # @return [Hash] Validated configuration + # @raise [ValidationError] if validation fails + def self.validate_endpoint_config(config) + result = ENDPOINT_CONFIG_SCHEMA.call(config) + + if result.failure? + raise ValidationError, "Invalid endpoint configuration: #{result.errors.to_h}" + end + + result.to_h + end + + # Validate array of endpoint configurations + # + # @param endpoints [Array] Array of endpoint configurations + # @return [Array] Array of validated configurations + # @raise [ValidationError] if any validation fails + def self.validate_endpoints(endpoints) + validated_endpoints = [] + + endpoints.each_with_index do |endpoint, index| + begin + validated_endpoints << validate_endpoint_config(endpoint) + rescue ValidationError => e + raise ValidationError, "Endpoint #{index}: #{e.message}" + end + end + + validated_endpoints + end + end + + # Custom validation error + class ValidationError < StandardError; end + end +end diff --git a/lib/hooks/core/logger_factory.rb b/lib/hooks/core/logger_factory.rb new file mode 100644 index 00000000..147815e7 --- /dev/null +++ b/lib/hooks/core/logger_factory.rb @@ -0,0 +1,89 @@ +# frozen_string_literal: true + +require "logger" +require "json" +require "securerandom" + +module Hooks + module Core + # Factory for creating structured JSON loggers + class LoggerFactory + # Create a structured JSON logger + # + # @param log_level [String] Log level (debug, info, warn, error) + # @param custom_logger [Logger] Custom logger instance (optional) + # @return [Logger] Configured logger instance + def self.create(log_level: "info", custom_logger: nil) + return custom_logger if custom_logger + + logger = Logger.new($stdout) + logger.level = parse_log_level(log_level) + logger.formatter = json_formatter + logger + end + + private + + # Parse string log level to Logger constant + # + # @param level [String] Log level string + # @return [Integer] Logger level constant + def self.parse_log_level(level) + case level.to_s.downcase + when "debug" then Logger::DEBUG + when "info" then Logger::INFO + when "warn" then Logger::WARN + when "error" then Logger::ERROR + else Logger::INFO + end + end + + # JSON formatter for structured logging + # + # @return [Proc] Formatter procedure + def self.json_formatter + proc do |severity, datetime, progname, msg| + log_entry = { + timestamp: datetime.iso8601, + level: severity.downcase, + message: msg + } + + # Add request context if available in thread local storage + if Thread.current[:hooks_request_context] + log_entry.merge!(Thread.current[:hooks_request_context]) + end + + "#{log_entry.to_json}\n" + end + end + end + + # Helper for setting request context in logs + module LogContext + # Set request context for current thread + # + # @param context [Hash] Request context data + def self.set(context) + Thread.current[:hooks_request_context] = context + end + + # Clear request context for current thread + def self.clear + Thread.current[:hooks_request_context] = nil + end + + # Execute block with request context + # + # @param context [Hash] Request context data + # @yield Block to execute with context + def self.with(context) + old_context = Thread.current[:hooks_request_context] + Thread.current[:hooks_request_context] = context + yield + ensure + Thread.current[:hooks_request_context] = old_context + end + end + end +end diff --git a/lib/hooks/core/signal_handler.rb b/lib/hooks/core/signal_handler.rb new file mode 100644 index 00000000..d9c5b62f --- /dev/null +++ b/lib/hooks/core/signal_handler.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Hooks + module Core + # Handles graceful shutdown signals + class SignalHandler + # Initialize signal handler + # + # @param logger [Logger] Logger instance + # @param graceful_timeout [Integer] Seconds to wait for graceful shutdown + def initialize(logger, graceful_timeout: 30) + @logger = logger + @graceful_timeout = graceful_timeout + @shutdown_requested = false + # Don't setup signal traps - let Puma handle them + # setup_signal_traps + end + + # Check if shutdown has been requested + # + # @return [Boolean] true if shutdown requested + def shutdown_requested? + @shutdown_requested + end + + # Manually request shutdown (for testing or programmatic shutdown) + def request_shutdown + @shutdown_requested = true + @logger.info "Shutdown requested" + end + + private + + # Setup signal traps for graceful shutdown + # NOTE: Disabled for now to let Puma handle signals properly + def setup_signal_traps + %w[SIGINT SIGTERM].each do |signal| + Signal.trap(signal) do + # Don't use logger in signal trap - just set the flag + unless @shutdown_requested + @shutdown_requested = true + + # Use STDERR for immediate output since logger might not work in trap context + $stderr.puts "Received #{signal}, initiating graceful shutdown (timeout: #{@graceful_timeout}s)" + + # Don't start a timeout thread - let Puma handle the shutdown + end + end + end + end + end + end +end diff --git a/lib/hooks/handlers/base.rb b/lib/hooks/handlers/base.rb new file mode 100644 index 00000000..dff85b5c --- /dev/null +++ b/lib/hooks/handlers/base.rb @@ -0,0 +1,21 @@ +# frozen_string_literal: true + +module Hooks + module Handlers + # Base class for all webhook handlers + # + # All custom handlers must inherit from this class and implement the #call method + class Base + # Process a webhook request + # + # @param payload [Hash, String] Parsed request body (JSON Hash) or raw string + # @param headers [Hash] HTTP headers + # @param config [Hash] Merged endpoint configuration including opts section + # @return [Hash, String, nil] Response body (will be auto-converted to JSON) + # @raise [NotImplementedError] if not implemented by subclass + def call(payload:, headers:, config:) + raise NotImplementedError, "Handler must implement #call method" + end + end + end +end diff --git a/lib/hooks/plugins/lifecycle.rb b/lib/hooks/plugins/lifecycle.rb new file mode 100644 index 00000000..c9b54f68 --- /dev/null +++ b/lib/hooks/plugins/lifecycle.rb @@ -0,0 +1,33 @@ +# frozen_string_literal: true + +module Hooks + module Plugins + # Base class for global lifecycle plugins + # + # Plugins can hook into request/response/error lifecycle events + class Lifecycle + # Called before handler execution + # + # @param env [Hash] Rack environment + def on_request(env) + # Override in subclass for pre-processing logic + end + + # Called after successful handler execution + # + # @param env [Hash] Rack environment + # @param response [Hash] Handler response + def on_response(env, response) + # Override in subclass for post-processing logic + end + + # Called when any error occurs during request processing + # + # @param exception [Exception] The raised exception + # @param env [Hash] Rack environment + def on_error(exception, env) + # Override in subclass for error handling logic + end + end + end +end diff --git a/lib/hooks/plugins/signature_validator/base.rb b/lib/hooks/plugins/signature_validator/base.rb new file mode 100644 index 00000000..9c536db6 --- /dev/null +++ b/lib/hooks/plugins/signature_validator/base.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Hooks + module Plugins + module SignatureValidator + # Abstract base class for signature validators + # + # All custom signature validators must inherit from this class + class Base + # Validate request signature + # + # @param payload [String] Raw request body + # @param headers [Hash] HTTP headers + # @param secret [String] Secret key for validation + # @param config [Hash] Endpoint configuration + # @return [Boolean] true if signature is valid + # @raise [NotImplementedError] if not implemented by subclass + def self.valid?(payload:, headers:, secret:, config:) + raise NotImplementedError, "Validator must implement .valid? class method" + end + end + end + end +end diff --git a/lib/hooks/plugins/signature_validator/hmac_sha256.rb b/lib/hooks/plugins/signature_validator/hmac_sha256.rb new file mode 100644 index 00000000..7c77bc8b --- /dev/null +++ b/lib/hooks/plugins/signature_validator/hmac_sha256.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +require "openssl" +require "rack/utils" +require_relative "base" + +module Hooks + module Plugins + module SignatureValidator + # Default HMAC SHA256 signature validator + # + # Validates GitHub-style webhook signatures using HMAC SHA256 + class HmacSha256 < Base + # Validate HMAC SHA256 signature + # + # @param payload [String] Raw request body + # @param headers [Hash] HTTP headers + # @param secret [String] Secret key for HMAC validation + # @param config [Hash] Endpoint configuration with signature settings + # @return [Boolean] true if signature is valid + def self.valid?(payload:, headers:, secret:, config:) + return false if secret.nil? || secret.empty? + + signature_header = config.dig(:verify_signature, :header) || "X-Hub-Signature-256" + algorithm = config.dig(:verify_signature, :algorithm) || "sha256" + + provided_signature = headers[signature_header] + return false if provided_signature.nil? || provided_signature.empty? + + # Compute expected signature + computed_signature = "#{algorithm}=" + OpenSSL::HMAC.hexdigest( + OpenSSL::Digest.new(algorithm), + secret, + payload + ) + + # Use secure comparison to prevent timing attacks + Rack::Utils.secure_compare(computed_signature, provided_signature) + rescue => e + # Log error in production implementation + false + end + end + end + end +end diff --git a/script/server b/script/server index b3e3dbf6..87bf9e38 100755 --- a/script/server +++ b/script/server @@ -1,7 +1,16 @@ #! /usr/bin/env bash +# usage: script/server [--dev] + set -e -source script/env "$@" +DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && cd .. && pwd )" +cd "$DIR" + +# check for --dev flag +if [[ "$1" == "--dev" ]]; then + source script/dev + shift # remove the --dev flag from the arguments +fi -bundle exec ruby lib/cli.rb "$@" +bundle exec puma --tag hooks diff --git a/spec/integration/hooks_integration_spec.rb b/spec/integration/hooks_integration_spec.rb new file mode 100644 index 00000000..5a29c8a4 --- /dev/null +++ b/spec/integration/hooks_integration_spec.rb @@ -0,0 +1,165 @@ +# frozen_string_literal: true + +require_relative "../../lib/hooks" +require "rack/test" +require "json" +require "fileutils" +require "yaml" + +RSpec.describe "Hooks Integration" do + include Rack::Test::Methods + + def app + @app ||= Hooks.build( + config: { + handler_dir: "./handlers", + log_level: "error", # Reduce noise in tests + request_limit: 1048576, + request_timeout: 15, + root_path: "/webhooks", + health_path: "/health", + metrics_path: "/metrics", + version_path: "/version", + environment: "development", + endpoints_dir: "./spec/fixtures/endpoints" + } + ) + end + + before(:all) do + # Create test endpoint config + FileUtils.mkdir_p("./spec/fixtures/endpoints") + File.write("./spec/fixtures/endpoints/test.yaml", { + path: "/test", + handler: "TestHandler", + opts: { test_mode: true } + }.to_yaml) + + # Create test handler + FileUtils.mkdir_p("./handlers") + File.write("./handlers/test_handler.rb", <<~RUBY) + require_relative "../lib/hooks/handlers/base" + + class TestHandler < Hooks::Handlers::Base + def call(payload:, headers:, config:) + { + status: "test_success", + payload_received: payload, + config_opts: config[:opts], + timestamp: Time.now.iso8601 + } + end + end + RUBY + end + + after(:all) do + # Clean up test files + FileUtils.rm_rf("./spec/fixtures") + end + + describe "operational endpoints" do + it "responds to health check" do + get "/health" + expect(last_response.status).to eq(200) + + body = JSON.parse(last_response.body) + expect(body["status"]).to eq("healthy") + expect(body["version"]).to eq(Hooks::VERSION) + expect(body).to have_key("timestamp") + expect(body).to have_key("uptime_seconds") + end + + it "responds to version endpoint" do + get "/version" + expect(last_response.status).to eq(200) + + body = JSON.parse(last_response.body) + expect(body["version"]).to eq(Hooks::VERSION) + expect(body).to have_key("timestamp") + end + end + + describe "hello world endpoint" do + it "responds to hello endpoint" do + get "/webhooks/hello" + expect(last_response.status).to eq(200) + + body = JSON.parse(last_response.body) + expect(body["message"]).to eq("Hooks is working!") + expect(body["version"]).to eq(Hooks::VERSION) + expect(body).to have_key("timestamp") + end + end + + describe "webhook endpoints" do + it "processes JSON webhook with custom handler" do + payload = { event: "test_event", data: "test_data" } + + post "/webhooks/test", payload.to_json, { + "CONTENT_TYPE" => "application/json" + } + + expect(last_response.status).to eq(200) + + body = JSON.parse(last_response.body) + expect(body["status"]).to eq("test_success") + expect(body["payload_received"]).to eq(payload.stringify_keys) + expect(body["config_opts"]).to eq({ "test_mode" => true }) + end + + it "handles raw string payload" do + payload = "raw webhook data" + + post "/webhooks/test", payload, { + "CONTENT_TYPE" => "text/plain" + } + + expect(last_response.status).to eq(200) + + body = JSON.parse(last_response.body) + expect(body["status"]).to eq("test_success") + expect(body["payload_received"]).to eq(payload) + end + + it "uses default handler for unknown endpoint" do + payload = { test: "data" } + + post "/webhooks/unknown", payload.to_json, { + "CONTENT_TYPE" => "application/json" + } + + expect(last_response.status).to eq(200) + + body = JSON.parse(last_response.body) + expect(body["message"]).to eq("Webhook received") + expect(body["handler"]).to eq("DefaultHandler") + end + end + + describe "request validation" do + it "rejects requests that are too large" do + large_payload = "x" * 2_000_000 # 2MB, larger than 1MB limit + + post "/webhooks/test", large_payload, { + "CONTENT_TYPE" => "text/plain", + "CONTENT_LENGTH" => large_payload.length.to_s + } + + expect(last_response.status).to eq(413) + end + end + + describe "error handling" do + it "returns structured error response" do + # Send invalid JSON as text/plain to avoid Grape's automatic parsing + post "/webhooks/test", "invalid json", { + "CONTENT_TYPE" => "text/plain" + } + + expect(last_response.status).to eq(200) # Our handler accepts any payload + body = JSON.parse(last_response.body) + expect(body["payload_received"]).to eq("invalid json") + end + end +end From b1dc24aec18d4a37dac5c4e07d1dc054c5a370ae Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 13:40:50 -0700 Subject: [PATCH 02/25] rename to just api --- lib/hooks/app/{api_factory.rb => api.rb} | 2 +- lib/hooks/core/builder.rb | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) rename lib/hooks/app/{api_factory.rb => api.rb} (99%) diff --git a/lib/hooks/app/api_factory.rb b/lib/hooks/app/api.rb similarity index 99% rename from lib/hooks/app/api_factory.rb rename to lib/hooks/app/api.rb index 5c124ee2..41766ade 100644 --- a/lib/hooks/app/api_factory.rb +++ b/lib/hooks/app/api.rb @@ -10,7 +10,7 @@ module Hooks module App # Factory for creating configured Grape API classes - class APIFactory + class API # Create a new configured API class def self.create(config:, endpoints:, logger:, signal_handler:) # Store startup time for uptime calculation diff --git a/lib/hooks/core/builder.rb b/lib/hooks/core/builder.rb index f8fa3dd1..edd056ef 100644 --- a/lib/hooks/core/builder.rb +++ b/lib/hooks/core/builder.rb @@ -4,7 +4,7 @@ require_relative "config_validator" require_relative "logger_factory" require_relative "signal_handler" -require_relative "../app/api_factory" +require_relative "../app/api" module Hooks module Core @@ -51,7 +51,7 @@ def build logger.info "Config: #{endpoints.size} endpoints loaded" # Build and return Grape API class - Hooks::App::APIFactory.create( + Hooks::App::API.create( config: config, endpoints: endpoints, logger: logger, From d1e89978b93ba1bd14b5057b0872468b684b3fc5 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 14:11:47 -0700 Subject: [PATCH 03/25] improvements --- config/config.yaml | 1 - demo/demo.rb | 2 -- lib/hooks.rb | 8 +---- lib/hooks/app/api.rb | 41 ++++++++++------------ lib/hooks/core/builder.rb | 19 +++------- lib/hooks/core/config_loader.rb | 17 ++++++--- lib/hooks/core/config_validator.rb | 7 ++-- spec/integration/hooks_integration_spec.rb | 5 ++- 8 files changed, 41 insertions(+), 59 deletions(-) diff --git a/config/config.yaml b/config/config.yaml index 4c570393..425620ea 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -9,7 +9,6 @@ request_timeout: 15 # 15 seconds timeout # Path configuration root_path: /webhooks health_path: /health -metrics_path: /metrics version_path: /version # Runtime behavior diff --git a/demo/demo.rb b/demo/demo.rb index d0bdc229..3fafacb1 100755 --- a/demo/demo.rb +++ b/demo/demo.rb @@ -19,7 +19,6 @@ request_timeout: 15, root_path: "/webhooks", health_path: "/health", - metrics_path: "/metrics", version_path: "/version", environment: "development", endpoints_dir: "./config/endpoints" @@ -118,4 +117,3 @@ def mock_request(method, path, body = nil, headers = {}) puts " POST http://localhost:3000/webhooks/team1" puts " POST http://localhost:3000/webhooks/github" puts " GET http://localhost:3000/health" -puts " GET http://localhost:3000/metrics" diff --git a/lib/hooks.rb b/lib/hooks.rb index ddbd2ff3..d8f1afc8 100644 --- a/lib/hooks.rb +++ b/lib/hooks.rb @@ -13,17 +13,11 @@ module Hooks # # @param config [String, Hash] Path to config file or config hash # @param log [Logger] Custom logger instance (optional) - # @param request_limit [Integer] Maximum request body size in bytes - # @param request_timeout [Integer] Request timeout in seconds - # @param root_path [String] Base path for webhook endpoints # @return [Object] Rack-compatible application - def self.build(config: nil, log: nil, request_limit: nil, request_timeout: nil, root_path: nil) + def self.build(config: nil, log: nil) Core::Builder.new( config: config, log: log, - request_limit: request_limit, - request_timeout: request_timeout, - root_path: root_path ).build end end diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 41766ade..fdaf7a75 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -51,7 +51,7 @@ def enforce_request_limits(config) content_length = content_length&.to_i if content_length && content_length > config[:request_limit] - error!("Request body too large", 413) + error!("request body too large", 413) end # Note: Timeout enforcement would typically be handled at the server level (Puma, etc.) @@ -67,7 +67,7 @@ def validate_signature(payload, headers, endpoint_config) secret = ENV[secret_env_key] unless secret - error!("Secret not found in environment", 500) + error!("secret '#{secret_env_key}' not found in environment", 500) end # Use default validator or load custom @@ -119,7 +119,7 @@ def load_handler(handler_class_name, handler_dir) DefaultHandler.new end rescue => e - error!("Failed to load handler #{handler_class_name}: #{e.message}", 500) + error!("failed to load handler #{handler_class_name}: #{e.message}", 500) end # Determine HTTP error code from exception @@ -144,11 +144,6 @@ def determine_error_code(exception) }.to_json end - get captured_config[:metrics_path] do - content_type "application/json" - { message: "Metrics functionality removed for simplification" }.to_json - end - get captured_config[:version_path] do content_type "application/json" { @@ -161,7 +156,7 @@ def determine_error_code(exception) get "#{captured_config[:root_path]}/hello" do content_type "application/json" { - message: "Hooks is working!", + message: "hooks is working!", version: Hooks::VERSION, timestamp: Time.now.iso8601 }.to_json @@ -183,7 +178,7 @@ def determine_error_code(exception) # Set request context for logging request_context = { - request_id: request_id, + request_id:, path: full_path, handler: handler_class_name } @@ -208,12 +203,12 @@ def determine_error_code(exception) # Call handler response = handler.call( - payload: payload, - headers: headers, + payload:, + headers:, config: endpoint_config ) - logger.info "Request processed successfully" + logger.info "request processed successfully (id: #{request_id}, handler: #{handler_class_name})" # Return response as JSON string when using txt format status 200 # Explicitly set status to 200 @@ -221,7 +216,7 @@ def determine_error_code(exception) (response || { status: "ok" }).to_json rescue => e - logger.error "Request failed: #{e.message}" + logger.error "request failed: #{e.message} (id: #{request_id}, handler: #{handler_class_name})" # Return error response error_response = { @@ -230,8 +225,8 @@ def determine_error_code(exception) request_id: request_id } - # Add backtrace in development - if config[:environment] == "development" + # Add backtrace in all environments except production + unless config[:production] == true error_response[:backtrace] = e.backtrace end @@ -281,15 +276,15 @@ def determine_error_code(exception) config: {} ) - logger.info "Request processed successfully with default handler" + logger.info "request processed successfully with default handler (id: #{request_id})" # Return response as JSON string when using txt format status 200 content_type "application/json" (response || { status: "ok" }).to_json - rescue => e - logger.error "Request failed: #{e.message}" + rescue StandardError => e + logger.error "request failed: #{e.message} (id: #{request_id})" # Return error response error_response = { @@ -298,8 +293,8 @@ def determine_error_code(exception) request_id: request_id } - # Add backtrace in development - if config[:environment] == "development" + # Add backtrace in all environments except production + unless config[:production] == true error_response[:backtrace] = e.backtrace end @@ -315,11 +310,11 @@ def determine_error_code(exception) api_class end - # Default handler for POC when no custom handler is found + # Default handler when no custom handler is found class DefaultHandler < Handlers::Base def call(payload:, headers:, config:) { - message: "Webhook received", + message: "webhook received", handler: "DefaultHandler", payload_size: payload.is_a?(String) ? payload.length : payload.to_s.length, timestamp: Time.now.iso8601 diff --git a/lib/hooks/core/builder.rb b/lib/hooks/core/builder.rb index edd056ef..b964fcc9 100644 --- a/lib/hooks/core/builder.rb +++ b/lib/hooks/core/builder.rb @@ -14,17 +14,9 @@ class Builder # # @param config [String, Hash] Path to config file or config hash # @param log [Logger] Custom logger instance - # @param request_limit [Integer] Maximum request body size in bytes - # @param request_timeout [Integer] Request timeout in seconds - # @param root_path [String] Base path for webhook endpoints - def initialize(config: nil, log: nil, request_limit: nil, request_timeout: nil, root_path: nil) + def initialize(config: nil, log: nil) @config_input = config @custom_logger = log - @programmatic_overrides = { - request_limit: request_limit, - request_timeout: request_timeout, - root_path: root_path - }.compact end # Build and return Rack-compatible application @@ -47,8 +39,10 @@ def build endpoints = load_endpoints(config) # Log startup - logger.info "Starting Hooks webhook server v#{Hooks::VERSION}" - logger.info "Config: #{endpoints.size} endpoints loaded" + logger.info "starting hooks server v#{Hooks::VERSION}" + logger.info "config: #{endpoints.size} endpoints loaded" + logger.info "environment: #{config[:environment]}" + logger.info "available endpoints: #{endpoints.map { |e| e[:path] }.join(', ')}" # Build and return Grape API class Hooks::App::API.create( @@ -68,9 +62,6 @@ def load_and_validate_config # Load base config from file/hash and environment config = ConfigLoader.load(config_path: @config_input) - # Apply programmatic overrides (highest priority) - config.merge!(@programmatic_overrides) - # Validate global configuration ConfigValidator.validate_global_config(config) rescue ConfigValidator::ValidationError => e diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index 0d29c759..9bab5033 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -11,12 +11,12 @@ class ConfigLoader handler_dir: "./handlers", log_level: "info", request_limit: 1_048_576, - request_timeout: 15, + request_timeout: 30, root_path: "/webhooks", health_path: "/health", - metrics_path: "/metrics", version_path: "/version", environment: "production", + production: true, endpoints_dir: "./config/endpoints" }.freeze @@ -39,7 +39,15 @@ def self.load(config_path: nil) config.merge!(load_env_config) # Convert string keys to symbols for consistency - symbolize_keys(config) + config = symbolize_keys(config) + + if config[:environment] == "production" + config[:production] = true + else + config[:production] = false + end + + return config end # Load endpoint configurations from directory @@ -81,7 +89,7 @@ def self.load_config_file(file_path) end result - rescue => e + rescue => _e # In production, we'd log this error nil end @@ -99,7 +107,6 @@ def self.load_env_config "HOOKS_REQUEST_TIMEOUT" => :request_timeout, "HOOKS_ROOT_PATH" => :root_path, "HOOKS_HEALTH_PATH" => :health_path, - "HOOKS_METRICS_PATH" => :metrics_path, "HOOKS_VERSION_PATH" => :version_path, "HOOKS_ENVIRONMENT" => :environment, "HOOKS_ENDPOINTS_DIR" => :endpoints_dir diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index ef7f1afc..aa02101f 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -6,6 +6,9 @@ module Hooks module Core # Validates configuration using Dry::Schema class ConfigValidator + # Custom validation error + class ValidationError < StandardError; end + # Global configuration schema GLOBAL_CONFIG_SCHEMA = Dry::Schema.Params do optional(:handler_dir).filled(:string) @@ -14,7 +17,6 @@ class ConfigValidator optional(:request_timeout).filled(:integer, gt?: 0) optional(:root_path).filled(:string) optional(:health_path).filled(:string) - optional(:metrics_path).filled(:string) optional(:version_path).filled(:string) optional(:environment).filled(:string, included_in?: %w[development production]) optional(:endpoints_dir).filled(:string) @@ -84,8 +86,5 @@ def self.validate_endpoints(endpoints) validated_endpoints end end - - # Custom validation error - class ValidationError < StandardError; end end end diff --git a/spec/integration/hooks_integration_spec.rb b/spec/integration/hooks_integration_spec.rb index 5a29c8a4..acab72b8 100644 --- a/spec/integration/hooks_integration_spec.rb +++ b/spec/integration/hooks_integration_spec.rb @@ -18,7 +18,6 @@ def app request_timeout: 15, root_path: "/webhooks", health_path: "/health", - metrics_path: "/metrics", version_path: "/version", environment: "development", endpoints_dir: "./spec/fixtures/endpoints" @@ -86,7 +85,7 @@ def call(payload:, headers:, config:) expect(last_response.status).to eq(200) body = JSON.parse(last_response.body) - expect(body["message"]).to eq("Hooks is working!") + expect(body["message"]).to eq("hooks is working!") expect(body["version"]).to eq(Hooks::VERSION) expect(body).to have_key("timestamp") end @@ -132,7 +131,7 @@ def call(payload:, headers:, config:) expect(last_response.status).to eq(200) body = JSON.parse(last_response.body) - expect(body["message"]).to eq("Webhook received") + expect(body["message"]).to eq("webhook received") expect(body["handler"]).to eq("DefaultHandler") end end From 97f2b5a338195755df66afd2e2061bca71aee959 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 14:16:28 -0700 Subject: [PATCH 04/25] only expose the catch-all route if `use_catchall_route` is set globally to `true` --- config/config.yaml | 3 + lib/hooks/app/api.rb | 125 +++++++++++---------- lib/hooks/core/config_loader.rb | 3 +- lib/hooks/core/config_validator.rb | 1 + spec/integration/hooks_integration_spec.rb | 3 +- 5 files changed, 72 insertions(+), 63 deletions(-) diff --git a/config/config.yaml b/config/config.yaml index 425620ea..5ab28841 100644 --- a/config/config.yaml +++ b/config/config.yaml @@ -14,3 +14,6 @@ version_path: /version # Runtime behavior environment: development endpoints_dir: ./config/endpoints + +# Optional features +# use_catchall_route: false # Set to true to enable catch-all route for unknown endpoints diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index fdaf7a75..4670d364 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -239,68 +239,71 @@ def determine_error_code(exception) end # Catch-all route for unknown endpoints - use default handler - post "#{captured_config[:root_path]}/*path" do - request_id = SecureRandom.uuid - start_time = Time.now - - # Use captured values - config = captured_config - logger = captured_logger - - # Set request context for logging - request_context = { - request_id: request_id, - path: "/#{params[:path]}", - handler: "DefaultHandler" - } - - Core::LogContext.with(request_context) do - begin - # Enforce request limits - enforce_request_limits(config) - - # Get raw body for payload parsing - request.body.rewind - raw_body = request.body.read - - # Parse payload - payload = parse_payload(raw_body, headers) - - # Use default handler - handler = DefaultHandler.new - - # Call handler - response = handler.call( - payload: payload, - headers: headers, - config: {} - ) - - logger.info "request processed successfully with default handler (id: #{request_id})" - - # Return response as JSON string when using txt format - status 200 - content_type "application/json" - (response || { status: "ok" }).to_json - - rescue StandardError => e - logger.error "request failed: #{e.message} (id: #{request_id})" - - # Return error response - error_response = { - error: e.message, - code: determine_error_code(e), - request_id: request_id - } - - # Add backtrace in all environments except production - unless config[:production] == true - error_response[:backtrace] = e.backtrace - end + # Only create if explicitly enabled in config + if captured_config[:use_catchall_route] + post "#{captured_config[:root_path]}/*path" do + request_id = SecureRandom.uuid + start_time = Time.now + + # Use captured values + config = captured_config + logger = captured_logger - status error_response[:code] - content_type "application/json" - error_response.to_json + # Set request context for logging + request_context = { + request_id: request_id, + path: "/#{params[:path]}", + handler: "DefaultHandler" + } + + Core::LogContext.with(request_context) do + begin + # Enforce request limits + enforce_request_limits(config) + + # Get raw body for payload parsing + request.body.rewind + raw_body = request.body.read + + # Parse payload + payload = parse_payload(raw_body, headers) + + # Use default handler + handler = DefaultHandler.new + + # Call handler + response = handler.call( + payload: payload, + headers: headers, + config: {} + ) + + logger.info "request processed successfully with default handler (id: #{request_id})" + + # Return response as JSON string when using txt format + status 200 + content_type "application/json" + (response || { status: "ok" }).to_json + + rescue StandardError => e + logger.error "request failed: #{e.message} (id: #{request_id})" + + # Return error response + error_response = { + error: e.message, + code: determine_error_code(e), + request_id: request_id + } + + # Add backtrace in all environments except production + unless config[:production] == true + error_response[:backtrace] = e.backtrace + end + + status error_response[:code] + content_type "application/json" + error_response.to_json + end end end end diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index 9bab5033..b3ccb65c 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -17,7 +17,8 @@ class ConfigLoader version_path: "/version", environment: "production", production: true, - endpoints_dir: "./config/endpoints" + endpoints_dir: "./config/endpoints", + use_catchall_route: false }.freeze # Load and merge configuration from various sources diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index aa02101f..0f68b942 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -20,6 +20,7 @@ class ValidationError < StandardError; end optional(:version_path).filled(:string) optional(:environment).filled(:string, included_in?: %w[development production]) optional(:endpoints_dir).filled(:string) + optional(:use_catchall_route).filled(:bool) end # Endpoint configuration schema diff --git a/spec/integration/hooks_integration_spec.rb b/spec/integration/hooks_integration_spec.rb index acab72b8..0d9636a8 100644 --- a/spec/integration/hooks_integration_spec.rb +++ b/spec/integration/hooks_integration_spec.rb @@ -20,7 +20,8 @@ def app health_path: "/health", version_path: "/version", environment: "development", - endpoints_dir: "./spec/fixtures/endpoints" + endpoints_dir: "./spec/fixtures/endpoints", + use_catchall_route: true # Enable catch-all route for testing } ) end From 0cd54e1b5d5124fae4668366cec9f08ccb6a1c2c Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 14:25:59 -0700 Subject: [PATCH 05/25] cleanup and move into acceptance dir --- config.ru | 5 +- demo/demo.rb | 119 ------------------ demo/simple_test.rb | 23 ---- script/server | 2 +- .../acceptance/config}/endpoints/github.yaml | 0 .../acceptance/config}/endpoints/team1.yaml | 0 .../acceptance/config/hooks.yaml | 7 +- {config => spec/acceptance/config}/puma.rb | 0 .../acceptance/handlers}/github_handler.rb | 2 - .../acceptance/handlers}/team1_handler.rb | 2 - .../acceptance/handlers}/test_handler.rb | 2 +- 11 files changed, 8 insertions(+), 154 deletions(-) delete mode 100755 demo/demo.rb delete mode 100644 demo/simple_test.rb rename {config => spec/acceptance/config}/endpoints/github.yaml (100%) rename {config => spec/acceptance/config}/endpoints/team1.yaml (100%) rename config/config.yaml => spec/acceptance/config/hooks.yaml (68%) rename {config => spec/acceptance/config}/puma.rb (100%) rename {handlers => spec/acceptance/handlers}/github_handler.rb (98%) rename {handlers => spec/acceptance/handlers}/team1_handler.rb (98%) rename {handlers => spec/acceptance/handlers}/test_handler.rb (83%) diff --git a/config.ru b/config.ru index 15fbca86..25bc151b 100644 --- a/config.ru +++ b/config.ru @@ -2,8 +2,5 @@ require_relative "lib/hooks" -# Build the Hooks application with default config -app = Hooks.build(config: "./config/config.yaml") - -# Run the application +app = Hooks.build(config: "./spec/acceptance/config/hooks.yaml") run app diff --git a/demo/demo.rb b/demo/demo.rb deleted file mode 100755 index 3fafacb1..00000000 --- a/demo/demo.rb +++ /dev/null @@ -1,119 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -# Demo script to test the Hooks webhook server - -require_relative "../lib/hooks" -require "net/http" -require "json" -require "uri" - -puts "๐Ÿช Hooks Webhook Server Demo" -puts "=" * 40 - -# Create a simple test config -test_config = { - handler_dir: "./handlers", - log_level: "info", - request_limit: 1048576, - request_timeout: 15, - root_path: "/webhooks", - health_path: "/health", - version_path: "/version", - environment: "development", - endpoints_dir: "./config/endpoints" -} - -puts "Building Hooks application..." -app = Hooks.build(config: test_config) - -puts "โœ… Application built successfully!" -puts "๐Ÿ“Š Version: #{Hooks::VERSION}" - -# Test with a simple Rack request -puts "\n๐Ÿงช Testing operational endpoints..." - -# Create a simple mock request environment -def mock_request(method, path, body = nil, headers = {}) - env = { - "REQUEST_METHOD" => method, - "PATH_INFO" => path, - "SCRIPT_NAME" => "", - "QUERY_STRING" => "", - "SERVER_NAME" => "localhost", - "SERVER_PORT" => "3000", - "rack.version" => [1, 3], - "rack.url_scheme" => "http", - "rack.input" => StringIO.new(body || ""), - "rack.errors" => $stderr, - "rack.multithread" => false, - "rack.multiprocess" => true, - "rack.run_once" => false - } - - headers.each { |k, v| env["HTTP_#{k.upcase.gsub('-', '_')}"] = v } - env -end - -# Test health endpoint -puts "Testing /health..." -begin - status, headers, body = app.call(mock_request("GET", "/health")) - response_body = body.join("") - parsed = JSON.parse(response_body) - puts " Status: #{status}" - puts " Response: #{parsed['status']} (v#{parsed['version']})" -rescue => e - puts " โŒ Error: #{e.message}" -end - -# Test version endpoint -puts "\nTesting /version..." -begin - status, headers, body = app.call(mock_request("GET", "/version")) - response_body = body.join("") - parsed = JSON.parse(response_body) - puts " Status: #{status}" - puts " Version: #{parsed['version']}" -rescue => e - puts " โŒ Error: #{e.message}" -end - -# Test hello endpoint -puts "\nTesting /webhooks/hello..." -begin - status, headers, body = app.call(mock_request("GET", "/webhooks/hello")) - response_body = body.join("") - parsed = JSON.parse(response_body) - puts " Status: #{status}" - puts " Message: #{parsed['message']}" -rescue => e - puts " โŒ Error: #{e.message}" -end - -# Test webhook endpoint with default handler -puts "\nTesting /webhooks/demo with default handler..." -begin - test_payload = { event: "demo", timestamp: Time.now.iso8601 } - status, headers, body = app.call(mock_request( - "POST", - "/webhooks/demo", - test_payload.to_json, - { "Content-Type" => "application/json" } - )) - response_body = body.join("") - parsed = JSON.parse(response_body) - puts " Status: #{status}" - puts " Handler: #{parsed['handler']}" - puts " Message: #{parsed['message']}" -rescue => e - puts " โŒ Error: #{e.message}" -end - -puts "\nโœ… Demo completed!" -puts "\n๐Ÿš€ To start the server:" -puts " ./bin/hooks serve -p 3000 -c ./config/config.yaml" -puts "\n๐Ÿ“š Example webhook endpoints:" -puts " POST http://localhost:3000/webhooks/team1" -puts " POST http://localhost:3000/webhooks/github" -puts " GET http://localhost:3000/health" diff --git a/demo/simple_test.rb b/demo/simple_test.rb deleted file mode 100644 index 51550f01..00000000 --- a/demo/simple_test.rb +++ /dev/null @@ -1,23 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -require_relative "../lib/hooks" - -puts "Testing Hooks framework..." - -begin - # Test basic instantiation - puts "Creating Hooks app..." - app = Hooks.build - puts "โœ“ App created successfully" - puts "App class: #{app.class}" - - # Test with config - puts "\nTesting with config file..." - app_with_config = Hooks.build(config: "./config/config.yaml") - puts "โœ“ App with config created successfully" - -rescue => e - puts "โœ— Error: #{e.class}: #{e.message}" - puts e.backtrace.first(5).join("\n") -end diff --git a/script/server b/script/server index 87bf9e38..8e1648bd 100755 --- a/script/server +++ b/script/server @@ -13,4 +13,4 @@ if [[ "$1" == "--dev" ]]; then shift # remove the --dev flag from the arguments fi -bundle exec puma --tag hooks +bundle exec puma -C spec/acceptance/config/puma.rb --tag hooks diff --git a/config/endpoints/github.yaml b/spec/acceptance/config/endpoints/github.yaml similarity index 100% rename from config/endpoints/github.yaml rename to spec/acceptance/config/endpoints/github.yaml diff --git a/config/endpoints/team1.yaml b/spec/acceptance/config/endpoints/team1.yaml similarity index 100% rename from config/endpoints/team1.yaml rename to spec/acceptance/config/endpoints/team1.yaml diff --git a/config/config.yaml b/spec/acceptance/config/hooks.yaml similarity index 68% rename from config/config.yaml rename to spec/acceptance/config/hooks.yaml index 5ab28841..b5ea376a 100644 --- a/config/config.yaml +++ b/spec/acceptance/config/hooks.yaml @@ -1,5 +1,5 @@ # Sample configuration for Hooks webhook server -handler_dir: ./handlers +handler_dir: ./spec/acceptance/handlers log_level: info # Request handling @@ -13,7 +13,10 @@ version_path: /version # Runtime behavior environment: development -endpoints_dir: ./config/endpoints + +# Available endpoints +# Each endpoint configuration file should be placed in the endpoints directory +endpoints_dir: ./spec/acceptance/config/endpoints # Optional features # use_catchall_route: false # Set to true to enable catch-all route for unknown endpoints diff --git a/config/puma.rb b/spec/acceptance/config/puma.rb similarity index 100% rename from config/puma.rb rename to spec/acceptance/config/puma.rb diff --git a/handlers/github_handler.rb b/spec/acceptance/handlers/github_handler.rb similarity index 98% rename from handlers/github_handler.rb rename to spec/acceptance/handlers/github_handler.rb index 1b978c9c..c5684345 100644 --- a/handlers/github_handler.rb +++ b/spec/acceptance/handlers/github_handler.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative "../lib/hooks/handlers/base" - # Example handler for GitHub webhooks class GitHubHandler < Hooks::Handlers::Base # Process GitHub webhook diff --git a/handlers/team1_handler.rb b/spec/acceptance/handlers/team1_handler.rb similarity index 98% rename from handlers/team1_handler.rb rename to spec/acceptance/handlers/team1_handler.rb index b605f374..574ca831 100644 --- a/handlers/team1_handler.rb +++ b/spec/acceptance/handlers/team1_handler.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require_relative "../lib/hooks/handlers/base" - # Example handler for Team 1 webhooks class Team1Handler < Hooks::Handlers::Base # Process Team 1 webhook diff --git a/handlers/test_handler.rb b/spec/acceptance/handlers/test_handler.rb similarity index 83% rename from handlers/test_handler.rb rename to spec/acceptance/handlers/test_handler.rb index b8e548c9..5c0fd05b 100644 --- a/handlers/test_handler.rb +++ b/spec/acceptance/handlers/test_handler.rb @@ -1,4 +1,4 @@ -require_relative "../lib/hooks/handlers/base" +# frozen_string_literal: true class TestHandler < Hooks::Handlers::Base def call(payload:, headers:, config:) From 924911e363d95950089c948ef3ad3645246c0f11 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 15:13:26 -0700 Subject: [PATCH 06/25] setting up support for github webhook hmac signatures --- Dockerfile | 43 ----------- docs/design.md | 4 +- handlers/test_handler.rb | 12 +++ lib/hooks.rb | 7 +- lib/hooks/app/api.rb | 36 ++++----- lib/hooks/core/config_validator.rb | 4 +- .../{hmac_sha256.rb => github_webhooks.rb} | 14 ++-- script/acceptance | 41 +++++++++++ spec/acceptance/Dockerfile | 24 ++++++ spec/acceptance/acceptance_tests.rb | 73 +++++++++++++++++++ spec/acceptance/config/endpoints/github.yaml | 12 ++- spec/acceptance/config/endpoints/team1.yaml | 2 +- spec/acceptance/docker-compose.yml | 16 ++++ 13 files changed, 207 insertions(+), 81 deletions(-) delete mode 100644 Dockerfile create mode 100644 handlers/test_handler.rb rename lib/hooks/plugins/signature_validator/{hmac_sha256.rb => github_webhooks.rb} (74%) create mode 100755 script/acceptance create mode 100644 spec/acceptance/Dockerfile create mode 100644 spec/acceptance/acceptance_tests.rb create mode 100644 spec/acceptance/docker-compose.yml diff --git a/Dockerfile b/Dockerfile deleted file mode 100644 index 86d5fcd8..00000000 --- a/Dockerfile +++ /dev/null @@ -1,43 +0,0 @@ -ARG RUBY_VERSION=3 -FROM ruby:${RUBY_VERSION}-slim AS base - -# create a nonroot user -RUN useradd -m nonroot - -WORKDIR /app - -# install system dependencies -RUN apt-get -qq update && apt-get --no-install-recommends install -y \ - build-essential \ - git && \ - rm -rf /var/lib/apt/lists/* - -# set the BUNDLE_APP_CONFIG environment variable -ENV BUNDLE_APP_CONFIG=/app/.bundle - -# copy bundler config -COPY --chown=nonroot:nonroot .bundle ./.bundle - -# install core scripts -COPY --chown=nonroot:nonroot script ./script - -# copy core ruby files first -COPY --chown=nonroot:nonroot .ruby-version Gemfile Gemfile.lock ./ - -# copy vendored gems -COPY --chown=nonroot:nonroot vendor ./vendor - -# bootstrap the ruby environment -RUN RUBY_ENV=production script/bootstrap - -# copy the rest of the application -COPY --chown=nonroot:nonroot . . - -# change ownership of /app directory to nonroot user -RUN chown -R nonroot:nonroot /app - -# switch to the nonroot user -USER nonroot - -# set the environment to production -ENV RUBY_ENV=production diff --git a/docs/design.md b/docs/design.md index 7446351c..498d934c 100644 --- a/docs/design.md +++ b/docs/design.md @@ -167,7 +167,7 @@ path: /team1 # Mounted at /team1 handler: Team1Handler # Class in handler_dir # Signature validation -verify_signature: +verify_request: type: default # 'default' uses HMACSHA256, or a custom class name secret_env_key: TEAM1_SECRET header: X-Hub-Signature @@ -613,7 +613,7 @@ path: string # Endpoint path (mounted under root_path) handler: string # Handler class name # Optional signature validation -verify_signature: +verify_request: type: string # 'default' or custom validator class name secret_env_key: string # ENV key containing secret header: string # Header containing signature (default: X-Hub-Signature) diff --git a/handlers/test_handler.rb b/handlers/test_handler.rb new file mode 100644 index 00000000..b8e548c9 --- /dev/null +++ b/handlers/test_handler.rb @@ -0,0 +1,12 @@ +require_relative "../lib/hooks/handlers/base" + +class TestHandler < Hooks::Handlers::Base + def call(payload:, headers:, config:) + { + status: "test_success", + payload_received: payload, + config_opts: config[:opts], + timestamp: Time.now.iso8601 + } + end +end diff --git a/lib/hooks.rb b/lib/hooks.rb index d8f1afc8..627d0d7c 100644 --- a/lib/hooks.rb +++ b/lib/hooks.rb @@ -4,8 +4,11 @@ require_relative "hooks/core/builder" require_relative "hooks/handlers/base" require_relative "hooks/plugins/lifecycle" -require_relative "hooks/plugins/signature_validator/base" -require_relative "hooks/plugins/signature_validator/hmac_sha256" + +# Load all signature validators +Dir[File.join(__dir__, "hooks/plugins/signature_validator/**/*.rb")].sort.each do |file| + require file +end # Main module for the Hooks webhook server framework module Hooks diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 4670d364..c2e74fba 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -4,7 +4,6 @@ require "json" require "securerandom" require_relative "../handlers/base" -require_relative "../plugins/signature_validator/hmac_sha256" require_relative "../core/logger_factory" module Hooks @@ -57,11 +56,11 @@ def enforce_request_limits(config) # Note: Timeout enforcement would typically be handled at the server level (Puma, etc.) end - # Validate request signature - def validate_signature(payload, headers, endpoint_config) - signature_config = endpoint_config[:verify_signature] - validator_type = signature_config[:type] || "default" - secret_env_key = signature_config[:secret_env_key] + # Verify the incoming request + def verify_request(payload, headers, endpoint_config) + verify_request_config = endpoint_config[:verify_request] + validator_type = verify_request_config[:type] || "default" + secret_env_key = verify_request_config[:secret_env_key] return unless secret_env_key @@ -70,18 +69,19 @@ def validate_signature(payload, headers, endpoint_config) error!("secret '#{secret_env_key}' not found in environment", 500) end - # Use default validator or load custom - validator_class = if validator_type == "default" - Plugins::SignatureValidator::HmacSha256 + validator_class = nil + + case validator_type + when "GitHubWebhooks" + validator_class = Plugins::SignatureValidator::GitHubWebhooks else - # In a full implementation, we'd dynamically load custom validators error!("Custom validators not implemented in POC", 500) end unless validator_class.valid?( - payload: payload, - headers: headers, - secret: secret, + payload:, + headers:, + secret:, config: endpoint_config ) error!("Invalid signature", 401) @@ -192,8 +192,8 @@ def determine_error_code(exception) request.body.rewind raw_body = request.body.read - # Validate signature if configured - validate_signature(raw_body, headers, endpoint_config) if endpoint_config[:verify_signature] + # Verify/validate request if configured + verify_request(raw_body, headers, endpoint_config) if endpoint_config[:verify_request] # Parse payload payload = parse_payload(raw_body, headers) @@ -222,7 +222,7 @@ def determine_error_code(exception) error_response = { error: e.message, code: determine_error_code(e), - request_id: request_id + request_id: } # Add backtrace in all environments except production @@ -251,7 +251,7 @@ def determine_error_code(exception) # Set request context for logging request_context = { - request_id: request_id, + request_id:, path: "/#{params[:path]}", handler: "DefaultHandler" } @@ -292,7 +292,7 @@ def determine_error_code(exception) error_response = { error: e.message, code: determine_error_code(e), - request_id: request_id + request_id: } # Add backtrace in all environments except production diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index 0f68b942..c3fc7117 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -28,8 +28,8 @@ class ValidationError < StandardError; end required(:path).filled(:string) required(:handler).filled(:string) - optional(:verify_signature).hash do - optional(:type).filled(:string) + optional(:verify_request).hash do + required(:type).filled(:string) optional(:secret_env_key).filled(:string) optional(:header).filled(:string) optional(:algorithm).filled(:string) diff --git a/lib/hooks/plugins/signature_validator/hmac_sha256.rb b/lib/hooks/plugins/signature_validator/github_webhooks.rb similarity index 74% rename from lib/hooks/plugins/signature_validator/hmac_sha256.rb rename to lib/hooks/plugins/signature_validator/github_webhooks.rb index 7c77bc8b..eb3b394b 100644 --- a/lib/hooks/plugins/signature_validator/hmac_sha256.rb +++ b/lib/hooks/plugins/signature_validator/github_webhooks.rb @@ -7,11 +7,13 @@ module Hooks module Plugins module SignatureValidator - # Default HMAC SHA256 signature validator + # GitHub webhook signature validator # # Validates GitHub-style webhook signatures using HMAC SHA256 - class HmacSha256 < Base - # Validate HMAC SHA256 signature + class GitHubWebhooks < Base + # Validate HMAC SHA256 signature from GitHub webhooks + # + # official docs: https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries # # @param payload [String] Raw request body # @param headers [Hash] HTTP headers @@ -21,8 +23,8 @@ class HmacSha256 < Base def self.valid?(payload:, headers:, secret:, config:) return false if secret.nil? || secret.empty? - signature_header = config.dig(:verify_signature, :header) || "X-Hub-Signature-256" - algorithm = config.dig(:verify_signature, :algorithm) || "sha256" + signature_header = config.dig(:verify_request, :header) || "X-Hub-Signature-256" + algorithm = config.dig(:verify_request, :algorithm) || "sha256" provided_signature = headers[signature_header] return false if provided_signature.nil? || provided_signature.empty? @@ -36,7 +38,7 @@ def self.valid?(payload:, headers:, secret:, config:) # Use secure comparison to prevent timing attacks Rack::Utils.secure_compare(computed_signature, provided_signature) - rescue => e + rescue => _e # Log error in production implementation false end diff --git a/script/acceptance b/script/acceptance new file mode 100755 index 00000000..71265715 --- /dev/null +++ b/script/acceptance @@ -0,0 +1,41 @@ +#! /usr/bin/env bash + +set -e # prevent any kind of script failures + +source script/env "$@" + +KEEP_UP=false +COMPOSE_FILE="spec/acceptance/docker-compose.yml" + +# check for the --keep flag +# if it exists, we will keep the docker processes up after the tests run +for arg in "$@"; do + if [ "$arg" == "--keep" ]; then + KEEP_UP=true + break + fi +done + +# check to ensure docker is running +if ! docker info &> /dev/null; then + echo -e "${RED}Docker is not running. Please start Docker and try again.${OFF}" + exit 1 +fi + +echo -e "${PURPLE}[#]${OFF} ${BLUE}Killing old docker processes${OFF}" +docker compose -f $COMPOSE_FILE down --remove-orphans -v -t 1 +docker network prune --force +docker compose -f $COMPOSE_FILE up --build -d + +echo -e "${PURPLE}[#]${OFF} ${BLUE}Running acceptance tests${OFF}" +bundle exec rspec spec/acceptance/acceptance_tests.rb + +if [ "$KEEP_UP" == "false" ]; then + echo -e "${PURPLE}[#]${OFF} ${BLUE}Stopping docker processes${OFF}" + docker compose -f $COMPOSE_FILE down --remove-orphans -v -t 1 + docker network prune --force +else + echo -e "${PURPLE}[#]${OFF} ${BLUE}Keeping docker processes up${OFF}" +fi + +echo -e "${PURPLE}[#]${OFF} ${GREEN}Acceptance tests passed${OFF}" diff --git a/spec/acceptance/Dockerfile b/spec/acceptance/Dockerfile new file mode 100644 index 00000000..d2bad372 --- /dev/null +++ b/spec/acceptance/Dockerfile @@ -0,0 +1,24 @@ +FROM ruby:3.4.2-slim@sha256:342bfeb04d3660045ceba063197d22baafec6b163f019714ddf8fc83c59aabee + +RUN apt-get update && \ + apt-get install -y --no-install-recommends build-essential libyaml-dev && \ + apt-get clean && \ + rm -rf /var/lib/apt/lists/* + +WORKDIR /app + +# create nonroot user +RUN useradd -m nonroot + +COPY --chown=nonroot:nonroot lib/hooks/version.rb ./lib/hooks/version.rb +COPY --chown=nonroot:nonroot .ruby-version Gemfile Gemfile.lock hooks.gemspec ./ +COPY --chown=nonroot:nonroot vendor/cache ./vendor/cache +COPY --chown=nonroot:nonroot script ./script +COPY --chown=nonroot:nonroot .bundle ./.bundle + +RUN script/bootstrap + +COPY --chown=nonroot:nonroot . . + +# switch to the nonroot user +USER nonroot diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb new file mode 100644 index 00000000..98f133e6 --- /dev/null +++ b/spec/acceptance/acceptance_tests.rb @@ -0,0 +1,73 @@ +# frozen_string_literal: true + +require "rspec" +require "net/http" +require "json" + +MAX_WAIT_TIME = 30 # how long to wait for the server to start + +describe "Hooks" do + let(:http) { Net::HTTP.new("127.0.0.1", 8080) } + + before(:all) do + start_time = Time.now + loop do + begin + response = Net::HTTP.new("127.0.0.1", 8080).get("/health") + break if response.is_a?(Net::HTTPSuccess) + rescue Errno::ECONNREFUSED, SocketError + # Server not ready yet, continue waiting + end + + if Time.now - start_time > MAX_WAIT_TIME + raise "Server did not return a 200 within #{MAX_WAIT_TIME} seconds" + end + + sleep 1 + end + end + + describe "operational endpoints" do + it "responds to the /health check" do + response = http.get("/health") + expect(response).to be_a(Net::HTTPSuccess) + + body = JSON.parse(response.body) + expect(body["status"]).to eq("healthy") + expect(body["version"]).to eq(Hooks::VERSION) + expect(body).to have_key("timestamp") + expect(body).to have_key("uptime_seconds") + end + + it "responds to the /version endpoint" do + response = http.get("/version") + expect(response).to be_a(Net::HTTPSuccess) + + body = JSON.parse(response.body) + expect(body["version"]).to eq(Hooks::VERSION) + expect(body).to have_key("timestamp") + end + end + + describe "endpoints" do + describe "team1" do + it "responds to the /webhooks/team1 endpoint" do + response = http.get("/webhooks/team1") + expect(response).to be_a(Net::HTTPMethodNotAllowed) + expect(response.body).to include("405 Not Allowed") + end + + it "processes a POST request with JSON payload" do + payload = { event: "test_event", data: "test_data", event_type: "alert" } + response = http.post("/webhooks/team1", payload.to_json, { "Content-Type" => "application/json" }) + expect(response).to be_a(Net::HTTPSuccess) + + body = JSON.parse(response.body) + expect(body["status"]).to eq("alert_processed") + expect(body["handler"]).to eq("Team1Handler") + expect(body["channels_notified"]).to include("#team1-alerts") + expect(body).to have_key("timestamp") + end + end + end +end diff --git a/spec/acceptance/config/endpoints/github.yaml b/spec/acceptance/config/endpoints/github.yaml index 4f053159..4632aa96 100644 --- a/spec/acceptance/config/endpoints/github.yaml +++ b/spec/acceptance/config/endpoints/github.yaml @@ -3,14 +3,12 @@ path: /github handler: GitHubHandler # GitHub uses HMAC SHA256 signature validation -verify_signature: - type: default +verify_request: + type: GitHubWebhooks secret_env_key: GITHUB_WEBHOOK_SECRET - header: X-Hub-Signature-256 - algorithm: sha256 + # header: X-Hub-Signature-256 # Optional, defaults to X-Hub-Signature-256 + # algorithm: sha256 # Optional, defaults to sha256 # Options for GitHub webhook handling opts: - repository: "my-org/my-repo" - events: ["push", "pull_request", "issues"] - branch_filter: ["main", "develop"] + slack_channel: "#github-webhooks" diff --git a/spec/acceptance/config/endpoints/team1.yaml b/spec/acceptance/config/endpoints/team1.yaml index 73dd2419..a2306162 100644 --- a/spec/acceptance/config/endpoints/team1.yaml +++ b/spec/acceptance/config/endpoints/team1.yaml @@ -3,7 +3,7 @@ path: /team1 handler: Team1Handler # Signature validation (optional) -# verify_signature: +# verify_request: # type: default # secret_env_key: TEAM1_SECRET # header: X-Hub-Signature-256 diff --git a/spec/acceptance/docker-compose.yml b/spec/acceptance/docker-compose.yml new file mode 100644 index 00000000..96c581b9 --- /dev/null +++ b/spec/acceptance/docker-compose.yml @@ -0,0 +1,16 @@ +services: + hooks: + container_name: hooks + build: + context: ../../ + dockerfile: ./spec/acceptance/Dockerfile + ports: + - "8080:8080" + environment: + LOG_LEVEL: DEBUG + command: ["script/server"] + healthcheck: + test: ["CMD", "curl", "-f", "http://0.0.0.0:8080/health"] + interval: 30s + timeout: 10s + retries: 3 From 2edf280b740540878274fe21124392fb2bcad7aa Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 15:36:07 -0700 Subject: [PATCH 07/25] rename to request validator --- docs/design.md | 4 ++-- lib/hooks.rb | 5 ++--- lib/hooks/app/api.rb | 14 +++++++------- lib/hooks/core/config_validator.rb | 2 +- .../base.rb | 10 +++++----- .../github_webhooks.rb | 8 ++++---- spec/acceptance/config/endpoints/github.yaml | 2 +- spec/acceptance/config/endpoints/team1.yaml | 2 +- 8 files changed, 23 insertions(+), 24 deletions(-) rename lib/hooks/plugins/{signature_validator => request_validator}/base.rb (70%) rename lib/hooks/plugins/{signature_validator => request_validator}/github_webhooks.rb (85%) diff --git a/docs/design.md b/docs/design.md index 498d934c..95ba8b48 100644 --- a/docs/design.md +++ b/docs/design.md @@ -167,7 +167,7 @@ path: /team1 # Mounted at /team1 handler: Team1Handler # Class in handler_dir # Signature validation -verify_request: +request_validator: type: default # 'default' uses HMACSHA256, or a custom class name secret_env_key: TEAM1_SECRET header: X-Hub-Signature @@ -613,7 +613,7 @@ path: string # Endpoint path (mounted under root_path) handler: string # Handler class name # Optional signature validation -verify_request: +request_validator: type: string # 'default' or custom validator class name secret_env_key: string # ENV key containing secret header: string # Header containing signature (default: X-Hub-Signature) diff --git a/lib/hooks.rb b/lib/hooks.rb index 627d0d7c..b557ceab 100644 --- a/lib/hooks.rb +++ b/lib/hooks.rb @@ -3,10 +3,9 @@ require_relative "hooks/version" require_relative "hooks/core/builder" require_relative "hooks/handlers/base" -require_relative "hooks/plugins/lifecycle" -# Load all signature validators -Dir[File.join(__dir__, "hooks/plugins/signature_validator/**/*.rb")].sort.each do |file| +# Load all plugins (request validators, lifecycle hooks, etc.) +Dir[File.join(__dir__, "hooks/plugins/**/*.rb")].sort.each do |file| require file end diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index c2e74fba..09080b04 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -57,10 +57,10 @@ def enforce_request_limits(config) end # Verify the incoming request - def verify_request(payload, headers, endpoint_config) - verify_request_config = endpoint_config[:verify_request] - validator_type = verify_request_config[:type] || "default" - secret_env_key = verify_request_config[:secret_env_key] + def request_validator(payload, headers, endpoint_config) + request_validator_config = endpoint_config[:request_validator] + validator_type = request_validator_config[:type] || "default" + secret_env_key = request_validator_config[:secret_env_key] return unless secret_env_key @@ -73,7 +73,7 @@ def verify_request(payload, headers, endpoint_config) case validator_type when "GitHubWebhooks" - validator_class = Plugins::SignatureValidator::GitHubWebhooks + validator_class = Plugins::RequestValidator::GitHubWebhooks else error!("Custom validators not implemented in POC", 500) end @@ -84,7 +84,7 @@ def verify_request(payload, headers, endpoint_config) secret:, config: endpoint_config ) - error!("Invalid signature", 401) + error!("request validation failed", 401) end end @@ -193,7 +193,7 @@ def determine_error_code(exception) raw_body = request.body.read # Verify/validate request if configured - verify_request(raw_body, headers, endpoint_config) if endpoint_config[:verify_request] + validate_request(raw_body, headers, endpoint_config) if endpoint_config[:request_validator] # Parse payload payload = parse_payload(raw_body, headers) diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index c3fc7117..05b0577e 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -28,7 +28,7 @@ class ValidationError < StandardError; end required(:path).filled(:string) required(:handler).filled(:string) - optional(:verify_request).hash do + optional(:request_validator).hash do required(:type).filled(:string) optional(:secret_env_key).filled(:string) optional(:header).filled(:string) diff --git a/lib/hooks/plugins/signature_validator/base.rb b/lib/hooks/plugins/request_validator/base.rb similarity index 70% rename from lib/hooks/plugins/signature_validator/base.rb rename to lib/hooks/plugins/request_validator/base.rb index 9c536db6..e3df7341 100644 --- a/lib/hooks/plugins/signature_validator/base.rb +++ b/lib/hooks/plugins/request_validator/base.rb @@ -2,18 +2,18 @@ module Hooks module Plugins - module SignatureValidator - # Abstract base class for signature validators + module RequestValidator + # Abstract base class for request validators # - # All custom signature validators must inherit from this class + # All custom request validators must inherit from this class class Base - # Validate request signature + # Validate request # # @param payload [String] Raw request body # @param headers [Hash] HTTP headers # @param secret [String] Secret key for validation # @param config [Hash] Endpoint configuration - # @return [Boolean] true if signature is valid + # @return [Boolean] true if request is valid # @raise [NotImplementedError] if not implemented by subclass def self.valid?(payload:, headers:, secret:, config:) raise NotImplementedError, "Validator must implement .valid? class method" diff --git a/lib/hooks/plugins/signature_validator/github_webhooks.rb b/lib/hooks/plugins/request_validator/github_webhooks.rb similarity index 85% rename from lib/hooks/plugins/signature_validator/github_webhooks.rb rename to lib/hooks/plugins/request_validator/github_webhooks.rb index eb3b394b..89dcfe1d 100644 --- a/lib/hooks/plugins/signature_validator/github_webhooks.rb +++ b/lib/hooks/plugins/request_validator/github_webhooks.rb @@ -6,8 +6,8 @@ module Hooks module Plugins - module SignatureValidator - # GitHub webhook signature validator + module RequestValidator + # GitHub webhook request validator # # Validates GitHub-style webhook signatures using HMAC SHA256 class GitHubWebhooks < Base @@ -23,8 +23,8 @@ class GitHubWebhooks < Base def self.valid?(payload:, headers:, secret:, config:) return false if secret.nil? || secret.empty? - signature_header = config.dig(:verify_request, :header) || "X-Hub-Signature-256" - algorithm = config.dig(:verify_request, :algorithm) || "sha256" + signature_header = config.dig(:request_validator, :header) || "X-Hub-Signature-256" + algorithm = config.dig(:request_validator, :algorithm) || "sha256" provided_signature = headers[signature_header] return false if provided_signature.nil? || provided_signature.empty? diff --git a/spec/acceptance/config/endpoints/github.yaml b/spec/acceptance/config/endpoints/github.yaml index 4632aa96..d7748d85 100644 --- a/spec/acceptance/config/endpoints/github.yaml +++ b/spec/acceptance/config/endpoints/github.yaml @@ -3,7 +3,7 @@ path: /github handler: GitHubHandler # GitHub uses HMAC SHA256 signature validation -verify_request: +request_validator: type: GitHubWebhooks secret_env_key: GITHUB_WEBHOOK_SECRET # header: X-Hub-Signature-256 # Optional, defaults to X-Hub-Signature-256 diff --git a/spec/acceptance/config/endpoints/team1.yaml b/spec/acceptance/config/endpoints/team1.yaml index a2306162..c69fbd23 100644 --- a/spec/acceptance/config/endpoints/team1.yaml +++ b/spec/acceptance/config/endpoints/team1.yaml @@ -3,7 +3,7 @@ path: /team1 handler: Team1Handler # Signature validation (optional) -# verify_request: +# request_validator: # type: default # secret_env_key: TEAM1_SECRET # header: X-Hub-Signature-256 From 6bf9d0fa8c9ec25eff4ccd516008217dfe776fdf Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 15:36:53 -0700 Subject: [PATCH 08/25] add acceptance ci job --- .github/workflows/acceptance.yml | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 .github/workflows/acceptance.yml diff --git a/.github/workflows/acceptance.yml b/.github/workflows/acceptance.yml new file mode 100644 index 00000000..af26557e --- /dev/null +++ b/.github/workflows/acceptance.yml @@ -0,0 +1,29 @@ +name: acceptance + +on: + push: + branches: + - main + pull_request: + +permissions: + contents: read + +jobs: + acceptance: + name: acceptance + runs-on: ubuntu-latest + + steps: + - name: checkout + uses: actions/checkout@v4 + + - uses: ruby/setup-ruby@13e7a03dc3ac6c3798f4570bfead2aed4d96abfb # pin@v1.244.0 + with: + bundler-cache: true + + - name: bootstrap + run: script/bootstrap + + - name: acceptance + run: script/acceptance From 19bc900384f6ee45c7a9831ff1df1c791fd1f3c0 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 16:46:04 -0700 Subject: [PATCH 09/25] working github webhook auth --- lib/hooks/app/api.rb | 10 +- .../request_validator/github_webhooks.rb | 3 + lib/hooks/plugins/utils/normalize.rb | 85 ++++++++++ script/server | 8 +- spec/acceptance/acceptance_tests.rb | 33 ++++ spec/acceptance/config/endpoints/github.yaml | 2 +- spec/acceptance/config/hooks.yaml | 2 +- spec/acceptance/docker-compose.yml | 1 + spec/acceptance/handlers/github_handler.rb | 150 +----------------- .../lib/hooks/plugins/utils/normalize_spec.rb | 120 ++++++++++++++ spec/spec_helper.rb | 3 + 11 files changed, 257 insertions(+), 160 deletions(-) create mode 100644 lib/hooks/plugins/utils/normalize.rb create mode 100644 spec/lib/hooks/plugins/utils/normalize_spec.rb diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 09080b04..b9ffc51b 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -57,9 +57,9 @@ def enforce_request_limits(config) end # Verify the incoming request - def request_validator(payload, headers, endpoint_config) + def validate_request(payload, headers, endpoint_config) request_validator_config = endpoint_config[:request_validator] - validator_type = request_validator_config[:type] || "default" + validator_type = request_validator_config[:type] secret_env_key = request_validator_config[:secret_env_key] return unless secret_env_key @@ -115,8 +115,7 @@ def load_handler(handler_class_name, handler_dir) require file_path Object.const_get(handler_class_name).new else - # Create a default handler for POC - DefaultHandler.new + raise LoadError, "Handler #{handler_class_name} not found at #{file_path}" end rescue => e error!("failed to load handler #{handler_class_name}: #{e.message}", 500) @@ -193,6 +192,9 @@ def determine_error_code(exception) raw_body = request.body.read # Verify/validate request if configured + logger.info "validating request (id: #{request_id}, handler: #{handler_class_name})" + logger.debug "raw body: #{raw_body.inspect}" + logger.debug "headers: #{headers.inspect}" validate_request(raw_body, headers, endpoint_config) if endpoint_config[:request_validator] # Parse payload diff --git a/lib/hooks/plugins/request_validator/github_webhooks.rb b/lib/hooks/plugins/request_validator/github_webhooks.rb index 89dcfe1d..e4fb8bc5 100644 --- a/lib/hooks/plugins/request_validator/github_webhooks.rb +++ b/lib/hooks/plugins/request_validator/github_webhooks.rb @@ -26,6 +26,9 @@ def self.valid?(payload:, headers:, secret:, config:) signature_header = config.dig(:request_validator, :header) || "X-Hub-Signature-256" algorithm = config.dig(:request_validator, :algorithm) || "sha256" + signature_header = Utils::Normalize.header(signature_header) + headers = Utils::Normalize.headers(headers) + provided_signature = headers[signature_header] return false if provided_signature.nil? || provided_signature.empty? diff --git a/lib/hooks/plugins/utils/normalize.rb b/lib/hooks/plugins/utils/normalize.rb new file mode 100644 index 00000000..0ff51ab9 --- /dev/null +++ b/lib/hooks/plugins/utils/normalize.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Hooks + module Plugins + module Utils + # Utility class for normalizing HTTP headers + # + # Provides a robust method to consistently format HTTP headers + # across the application, handling various edge cases and formats. + class Normalize + # Normalize a hash of HTTP headers + # + # @param headers [Hash, #each] Headers hash or hash-like object + # @return [Hash] Normalized headers hash with downcased keys and trimmed values + # + # @example Hash of headers normalization + # headers = { "Content-Type" => " application/json ", "X-GitHub-Event" => "push" } + # normalized = Normalize.headers(headers) + # # => { "content-type" => "application/json", "x-github-event" => "push" } + # + # @example Handle various input types + # Normalize.headers(nil) # => nil + # Normalize.headers({}) # => {} + # Normalize.headers({ "KEY" => ["a", "b"] }) # => { "key" => "a" } + # Normalize.headers({ "Key" => 123 }) # => { "key" => "123" } + def self.headers(headers) + # Handle nil input + return nil if headers.nil? + + # Fast path for non-enumerable inputs (numbers, etc.) + return {} unless headers.respond_to?(:each) + + normalized = {} + + headers.each do |key, value| + # Skip nil keys or values entirely + next if key.nil? || value.nil? + + # Convert key to string, downcase, and strip in one operation + normalized_key = key.to_s.downcase.strip + next if normalized_key.empty? + + # Handle different value types efficiently + normalized_value = case value + when String + value.strip + when Array + # Take first non-empty element for multi-value headers + first_valid = value.find { |v| v && !v.to_s.strip.empty? } + first_valid ? first_valid.to_s.strip : nil + else + value.to_s.strip + end + + # Only add if we have a non-empty value + normalized[normalized_key] = normalized_value if normalized_value && !normalized_value.empty? + end + + normalized + end + + # Normalize a single HTTP header name + # + # @param header [String] Header name to normalize + # @return [String, nil] Normalized header name (downcased and trimmed), or nil if input is nil + # + # @example Single header normalization + # Normalize.header(" Content-Type ") # => "content-type" + # Normalize.header("X-GitHub-Event") # => "x-github-event" + # Normalize.header("") # => "" + # Normalize.header(nil) # => nil + # + # @raise [ArgumentError] If input is not a String or nil + def self.header(header) + return nil if header.nil? + if header.is_a?(String) + header.downcase.strip + else + raise ArgumentError, "Expected a String for header normalization" + end + end + end + end + end +end diff --git a/script/server b/script/server index 8e1648bd..3f3d3c3e 100755 --- a/script/server +++ b/script/server @@ -1,16 +1,10 @@ #! /usr/bin/env bash -# usage: script/server [--dev] +# usage: script/server set -e DIR="$( cd "$( dirname "${BASH_SOURCE[0]}" )" && cd .. && pwd )" cd "$DIR" -# check for --dev flag -if [[ "$1" == "--dev" ]]; then - source script/dev - shift # remove the --dev flag from the arguments -fi - bundle exec puma -C spec/acceptance/config/puma.rb --tag hooks diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 98f133e6..593938b6 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require_relative "../spec_helper" + require "rspec" require "net/http" require "json" @@ -69,5 +71,36 @@ expect(body).to have_key("timestamp") end end + + describe "github" do + it "receives a POST request but contains an invalid HMAC signature" do + payload = { action: "push", repository: { name: "test-repo" } } + headers = { "Content-Type" => "application/json", "X-Hub-Signature-256" => "sha256=invalidsignature" } + response = http.post("/webhooks/github", payload.to_json, headers) + + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("request validation failed") + end + + it "receives a POST request but there is no HMAC related header" do + payload = { action: "push", repository: { name: "test-repo" } } + headers = { "Content-Type" => "application/json" } + response = http.post("/webhooks/github", payload.to_json, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("request validation failed") + end + + it "successfully processes a valid POST request with HMAC signature" do + payload = { action: "push", repository: { name: "test-repo" } } + headers = { + "Content-Type" => "application/json", + "X-Hub-Signature-256" => "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_HMAC_SECRET, payload.to_json) + } + response = http.post("/webhooks/github", payload.to_json, headers) + expect(response).to be_a(Net::HTTPSuccess) + body = JSON.parse(response.body) + expect(body["status"]).to eq("success") + end + end end end diff --git a/spec/acceptance/config/endpoints/github.yaml b/spec/acceptance/config/endpoints/github.yaml index d7748d85..e2f002ee 100644 --- a/spec/acceptance/config/endpoints/github.yaml +++ b/spec/acceptance/config/endpoints/github.yaml @@ -1,6 +1,6 @@ # Sample endpoint configuration for GitHub webhooks path: /github -handler: GitHubHandler +handler: GithubHandler # GitHub uses HMAC SHA256 signature validation request_validator: diff --git a/spec/acceptance/config/hooks.yaml b/spec/acceptance/config/hooks.yaml index b5ea376a..453cbcdd 100644 --- a/spec/acceptance/config/hooks.yaml +++ b/spec/acceptance/config/hooks.yaml @@ -1,6 +1,6 @@ # Sample configuration for Hooks webhook server handler_dir: ./spec/acceptance/handlers -log_level: info +log_level: debug # Request handling request_limit: 1048576 # 1MB max body size diff --git a/spec/acceptance/docker-compose.yml b/spec/acceptance/docker-compose.yml index 96c581b9..8222315d 100644 --- a/spec/acceptance/docker-compose.yml +++ b/spec/acceptance/docker-compose.yml @@ -8,6 +8,7 @@ services: - "8080:8080" environment: LOG_LEVEL: DEBUG + GITHUB_WEBHOOK_SECRET: "octoawesome-secret" command: ["script/server"] healthcheck: test: ["CMD", "curl", "-f", "http://0.0.0.0:8080/health"] diff --git a/spec/acceptance/handlers/github_handler.rb b/spec/acceptance/handlers/github_handler.rb index c5684345..ed2f8f6c 100644 --- a/spec/acceptance/handlers/github_handler.rb +++ b/spec/acceptance/handlers/github_handler.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true # Example handler for GitHub webhooks -class GitHubHandler < Hooks::Handlers::Base +class GithubHandler < Hooks::Handlers::Base # Process GitHub webhook # # @param payload [Hash, String] GitHub webhook payload @@ -9,152 +9,8 @@ class GitHubHandler < Hooks::Handlers::Base # @param config [Hash] Endpoint configuration # @return [Hash] Response data def call(payload:, headers:, config:) - # GitHub sends event type in header - event_type = headers["X-GitHub-Event"] || "unknown" - - puts "GitHubHandler: Received #{event_type} event" - - return handle_raw_payload(payload, config) unless payload.is_a?(Hash) - - case event_type - when "push" - handle_push_event(payload, config) - when "pull_request" - handle_pull_request_event(payload, config) - when "issues" - handle_issues_event(payload, config) - when "ping" - handle_ping_event(payload, config) - else - handle_unknown_event(payload, event_type, config) - end - end - - private - - # Handle raw string payload - # - # @param payload [String] Raw payload - # @param config [Hash] Configuration - # @return [Hash] Response - def handle_raw_payload(payload, config) - { - status: "raw_payload_processed", - handler: "GitHubHandler", - payload_size: payload.length, - repository: config.dig(:opts, :repository), - timestamp: Time.now.iso8601 - } - end - - # Handle push events - # - # @param payload [Hash] Push event payload - # @param config [Hash] Configuration - # @return [Hash] Response - def handle_push_event(payload, config) - ref = payload["ref"] - branch = ref&.split("/")&.last - commits_count = payload.dig("commits")&.length || 0 - - # Check if branch is in filter - branch_filter = config.dig(:opts, :branch_filter) - if branch_filter && !branch_filter.include?(branch) - return { - status: "ignored", - handler: "GitHubHandler", - reason: "branch_not_in_filter", - branch: branch, - filter: branch_filter, - timestamp: Time.now.iso8601 - } - end - - { - status: "push_processed", - handler: "GitHubHandler", - repository: payload.dig("repository", "full_name"), - branch: branch, - commits_count: commits_count, - pusher: payload.dig("pusher", "name"), - timestamp: Time.now.iso8601 - } - end - - # Handle pull request events - # - # @param payload [Hash] Pull request event payload - # @param config [Hash] Configuration - # @return [Hash] Response - def handle_pull_request_event(payload, config) - action = payload["action"] - pr_number = payload.dig("pull_request", "number") - pr_title = payload.dig("pull_request", "title") - - { - status: "pull_request_processed", - handler: "GitHubHandler", - action: action, - repository: payload.dig("repository", "full_name"), - pr_number: pr_number, - pr_title: pr_title, - author: payload.dig("pull_request", "user", "login"), - timestamp: Time.now.iso8601 - } - end - - # Handle issues events - # - # @param payload [Hash] Issues event payload - # @param config [Hash] Configuration - # @return [Hash] Response - def handle_issues_event(payload, config) - action = payload["action"] - issue_number = payload.dig("issue", "number") - issue_title = payload.dig("issue", "title") - - { - status: "issue_processed", - handler: "GitHubHandler", - action: action, - repository: payload.dig("repository", "full_name"), - issue_number: issue_number, - issue_title: issue_title, - author: payload.dig("issue", "user", "login"), - timestamp: Time.now.iso8601 - } - end - - # Handle ping events (webhook test) - # - # @param payload [Hash] Ping event payload - # @param config [Hash] Configuration - # @return [Hash] Response - def handle_ping_event(payload, config) - { - status: "ping_acknowledged", - handler: "GitHubHandler", - repository: payload.dig("repository", "full_name"), - hook_id: payload.dig("hook", "id"), - zen: payload["zen"], - timestamp: Time.now.iso8601 - } - end - - # Handle unknown events - # - # @param payload [Hash] Event payload - # @param event_type [String] Event type - # @param config [Hash] Configuration - # @return [Hash] Response - def handle_unknown_event(payload, event_type, config) - { - status: "unknown_event_processed", - handler: "GitHubHandler", - event_type: event_type, - repository: payload.dig("repository", "full_name"), - supported_events: config.dig(:opts, :events), - timestamp: Time.now.iso8601 + return { + status: "success" } end end diff --git a/spec/lib/hooks/plugins/utils/normalize_spec.rb b/spec/lib/hooks/plugins/utils/normalize_spec.rb new file mode 100644 index 00000000..0b2db006 --- /dev/null +++ b/spec/lib/hooks/plugins/utils/normalize_spec.rb @@ -0,0 +1,120 @@ +# frozen_string_literal: true + +describe Hooks::Plugins::Utils::Normalize do + describe ".header" do + context "when input is a single string" do + it "normalizes header name to lowercase and trims whitespace" do + expect(described_class.header(" Content-Type ")).to eq("content-type") + expect(described_class.header("X-GitHub-Event")).to eq("x-github-event") + expect(described_class.header("AUTHORIZATION")).to eq("authorization") + end + + it "handles empty and nil string inputs" do + expect(described_class.header("")).to eq("") + expect(described_class.header(" ")).to eq("") + expect(described_class.header(nil)).to eq(nil) + end + + it "raises ArgumentError for non-string inputs" do + expect { described_class.header(123) }.to raise_error(ArgumentError, "Expected a String for header normalization") + expect { described_class.header({}) }.to raise_error(ArgumentError, "Expected a String for header normalization") + expect { described_class.header([]) }.to raise_error(ArgumentError, "Expected a String for header normalization") + end + end + end + + describe ".headers" do + + context "when input is a hash of headers" do + it "normalizes header keys to lowercase and trims values" do + headers = { + "Content-Type" => " application/json ", + "X-GitHub-Event" => "push", + "AUTHORIZATION" => " Bearer token123 " + } + + normalized = described_class.headers(headers) + + expect(normalized).to eq({ + "content-type" => "application/json", + "x-github-event" => "push", + "authorization" => "Bearer token123" + }) + end + + it "handles nil and empty values" do + headers = { + "valid-header" => "value", + "empty-header" => "", + "nil-header" => nil, + nil => "should-be-skipped" + } + + normalized = described_class.headers(headers) + + expect(normalized).to eq({ + "valid-header" => "value" + }) + end + + it "handles array values by taking the first non-empty element" do + headers = { + "multi-value" => ["first", "second", "third"], + "empty-array" => [], + "nil-in-array" => [nil, "", "valid"], + "all-empty-array" => ["", " ", nil] + } + + normalized = described_class.headers(headers) + + expect(normalized).to eq({ + "multi-value" => "first", + "nil-in-array" => "valid" + }) + end + + it "handles non-string values by converting to string" do + headers = { + "numeric-header" => 123, + "boolean-header" => true, + "symbol-header" => :symbol_value + } + + normalized = described_class.headers(headers) + + expect(normalized).to eq({ + "numeric-header" => "123", + "boolean-header" => "true", + "symbol-header" => "symbol_value" + }) + end + + it "skips headers with empty keys after normalization" do + headers = { + " " => "should-be-skipped", + "" => "also-skipped", + "valid-key" => "kept" + } + + normalized = described_class.headers(headers) + + expect(normalized).to eq({ + "valid-key" => "kept" + }) + end + + it "handles nil input" do + expect(described_class.headers(nil)).to eq(nil) + end + + it "handles empty hash input" do + expect(described_class.headers({})).to eq({}) + end + + it "handles non-enumerable input" do + expect(described_class.headers(123)).to eq({}) + expect(described_class.headers(true)).to eq({}) + end + end + end +end diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index a808337d..359b8609 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -6,6 +6,9 @@ require "time" TIME_MOCK = "2025-01-01T00:00:00Z" +FAKE_HMAC_SECRET = "octoawesome-secret" + +ENV["GITHUB_WEBHOOK_SECRET"] = FAKE_HMAC_SECRET COV_DIR = File.expand_path("../coverage", File.dirname(__FILE__)) From c69d2a7a4ec96f78a7752b75185b2d6a0f022920 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 16:47:37 -0700 Subject: [PATCH 10/25] notes --- lib/hooks/app/api.rb | 2 ++ 1 file changed, 2 insertions(+) diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index b9ffc51b..ebfae084 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -108,6 +108,8 @@ def parse_payload(raw_body, headers) # Load handler class def load_handler(handler_class_name, handler_dir) # Convert class name to file name (e.g., Team1Handler -> team1_handler.rb) + # E.g.2: GithubHandler -> github_handler.rb + # E.g.3: GitHubHandler -> git_hub_handler.rb file_name = handler_class_name.gsub(/([A-Z])/, '_\1').downcase.sub(/^_/, "") + ".rb" file_path = File.join(handler_dir, file_name) From bab9d24f62f3b4c04771261e13cff0a5c93a26cf Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 16:52:38 -0700 Subject: [PATCH 11/25] log instead of logger --- lib/hooks/app/api.rb | 20 +++++++++----------- lib/hooks/core/builder.rb | 26 ++++++++++++++------------ 2 files changed, 23 insertions(+), 23 deletions(-) diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index ebfae084..1bbb6db8 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -11,14 +11,14 @@ module App # Factory for creating configured Grape API classes class API # Create a new configured API class - def self.create(config:, endpoints:, logger:, signal_handler:) + def self.create(config:, endpoints:, log:, signal_handler:) # Store startup time for uptime calculation start_time = Time.now # Capture values in local variables for closure captured_config = config captured_endpoints = endpoints - captured_logger = logger + captured_logger = log _captured_signal_handler = signal_handler captured_start_time = start_time @@ -175,7 +175,7 @@ def determine_error_code(exception) # Use captured values config = captured_config - logger = captured_logger + log = captured_logger # Set request context for logging request_context = { @@ -194,9 +194,7 @@ def determine_error_code(exception) raw_body = request.body.read # Verify/validate request if configured - logger.info "validating request (id: #{request_id}, handler: #{handler_class_name})" - logger.debug "raw body: #{raw_body.inspect}" - logger.debug "headers: #{headers.inspect}" + log.info "validating request (id: #{request_id}, handler: #{handler_class_name})" validate_request(raw_body, headers, endpoint_config) if endpoint_config[:request_validator] # Parse payload @@ -212,7 +210,7 @@ def determine_error_code(exception) config: endpoint_config ) - logger.info "request processed successfully (id: #{request_id}, handler: #{handler_class_name})" + log.info "request processed successfully (id: #{request_id}, handler: #{handler_class_name})" # Return response as JSON string when using txt format status 200 # Explicitly set status to 200 @@ -220,7 +218,7 @@ def determine_error_code(exception) (response || { status: "ok" }).to_json rescue => e - logger.error "request failed: #{e.message} (id: #{request_id}, handler: #{handler_class_name})" + log.error "request failed: #{e.message} (id: #{request_id}, handler: #{handler_class_name})" # Return error response error_response = { @@ -251,7 +249,7 @@ def determine_error_code(exception) # Use captured values config = captured_config - logger = captured_logger + log = captured_logger # Set request context for logging request_context = { @@ -282,7 +280,7 @@ def determine_error_code(exception) config: {} ) - logger.info "request processed successfully with default handler (id: #{request_id})" + log.info "request processed successfully with default handler (id: #{request_id})" # Return response as JSON string when using txt format status 200 @@ -290,7 +288,7 @@ def determine_error_code(exception) (response || { status: "ok" }).to_json rescue StandardError => e - logger.error "request failed: #{e.message} (id: #{request_id})" + log.error "request failed: #{e.message} (id: #{request_id})" # Return error response error_response = { diff --git a/lib/hooks/core/builder.rb b/lib/hooks/core/builder.rb index b964fcc9..877bbc5c 100644 --- a/lib/hooks/core/builder.rb +++ b/lib/hooks/core/builder.rb @@ -15,8 +15,8 @@ class Builder # @param config [String, Hash] Path to config file or config hash # @param log [Logger] Custom logger instance def initialize(config: nil, log: nil) + @log = log @config_input = config - @custom_logger = log end # Build and return Rack-compatible application @@ -26,29 +26,31 @@ def build # Load and validate configuration config = load_and_validate_config - # Create logger - logger = LoggerFactory.create( - log_level: config[:log_level], - custom_logger: @custom_logger - ) + # Create logger unless a custom logger is provided + if @log.nil? + @log = LoggerFactory.create( + log_level: config[:log_level], + custom_logger: @custom_logger + ) + end # Setup signal handler for graceful shutdown - signal_handler = SignalHandler.new(logger) + signal_handler = SignalHandler.new(@log) # Load endpoints endpoints = load_endpoints(config) # Log startup - logger.info "starting hooks server v#{Hooks::VERSION}" - logger.info "config: #{endpoints.size} endpoints loaded" - logger.info "environment: #{config[:environment]}" - logger.info "available endpoints: #{endpoints.map { |e| e[:path] }.join(', ')}" + @log.info "starting hooks server v#{Hooks::VERSION}" + @log.info "config: #{endpoints.size} endpoints loaded" + @log.info "environment: #{config[:environment]}" + @log.info "available endpoints: #{endpoints.map { |e| e[:path] }.join(', ')}" # Build and return Grape API class Hooks::App::API.create( config: config, endpoints: endpoints, - logger: logger, + log: @log, signal_handler: signal_handler ) end From b229697df2040ae1a1c2552b3fbf4a11fa419bec Mon Sep 17 00:00:00 2001 From: Grant Birkinbine Date: Mon, 9 Jun 2025 16:54:21 -0700 Subject: [PATCH 12/25] Update script/acceptance Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- script/acceptance | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/script/acceptance b/script/acceptance index 71265715..650fdfd2 100755 --- a/script/acceptance +++ b/script/acceptance @@ -23,16 +23,16 @@ if ! docker info &> /dev/null; then fi echo -e "${PURPLE}[#]${OFF} ${BLUE}Killing old docker processes${OFF}" -docker compose -f $COMPOSE_FILE down --remove-orphans -v -t 1 +docker compose -f "$COMPOSE_FILE" down --remove-orphans -v -t 1 docker network prune --force -docker compose -f $COMPOSE_FILE up --build -d +docker compose -f "$COMPOSE_FILE" up --build -d echo -e "${PURPLE}[#]${OFF} ${BLUE}Running acceptance tests${OFF}" bundle exec rspec spec/acceptance/acceptance_tests.rb if [ "$KEEP_UP" == "false" ]; then echo -e "${PURPLE}[#]${OFF} ${BLUE}Stopping docker processes${OFF}" - docker compose -f $COMPOSE_FILE down --remove-orphans -v -t 1 + docker compose -f "$COMPOSE_FILE" down --remove-orphans -v -t 1 docker network prune --force else echo -e "${PURPLE}[#]${OFF} ${BLUE}Keeping docker processes up${OFF}" From 8875c5c04de54ded6d784e1b510265aa70597cb3 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 21:07:53 -0700 Subject: [PATCH 13/25] make it so that payloads are symbolized by default and headers are also normalized by default as well --- lib/hooks/app/api.rb | 15 ++++++++++----- lib/hooks/core/config_loader.rb | 4 +++- lib/hooks/core/config_validator.rb | 2 ++ spec/acceptance/handlers/team1_handler.rb | 2 +- 4 files changed, 16 insertions(+), 7 deletions(-) diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 1bbb6db8..7ea99013 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -89,13 +89,15 @@ def validate_request(payload, headers, endpoint_config) end # Parse request payload - def parse_payload(raw_body, headers) - content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] + def parse_payload(raw_body, headers, symbolize: true) + content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] || headers["content-type"] || headers["HTTP_CONTENT_TYPE"] # Try to parse as JSON if content type suggests it or if it looks like JSON if content_type&.include?("application/json") || (raw_body.strip.start_with?("{", "[") rescue false) begin - return JSON.parse(raw_body) + parsed_payload = JSON.parse(raw_body) + parsed_payload = parsed_payload.transform_keys(&:to_sym) if symbolize && parsed_payload.is_a?(Hash) + return parsed_payload rescue JSON::ParserError # If JSON parsing fails, return raw body end @@ -197,12 +199,15 @@ def determine_error_code(exception) log.info "validating request (id: #{request_id}, handler: #{handler_class_name})" validate_request(raw_body, headers, endpoint_config) if endpoint_config[:request_validator] - # Parse payload - payload = parse_payload(raw_body, headers) + # Parse payload (symbolize_payload is true by default) + payload = parse_payload(raw_body, headers, symbolize: config[:symbolize_payload]) # Load and instantiate handler handler = load_handler(handler_class_name, config[:handler_dir]) + # Normalize the headers based on the endpoint configuration (normalization is the default) + headers = Hooks::Plugins::Utils::Normalize.headers(headers) if config[:normalize_headers] + # Call handler response = handler.call( payload:, diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index b3ccb65c..bbd31887 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -18,7 +18,9 @@ class ConfigLoader environment: "production", production: true, endpoints_dir: "./config/endpoints", - use_catchall_route: false + use_catchall_route: false, + symbolize_payload: true, + normalize_headers: true }.freeze # Load and merge configuration from various sources diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index 05b0577e..5b2088c8 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -21,6 +21,8 @@ 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 # Endpoint configuration schema diff --git a/spec/acceptance/handlers/team1_handler.rb b/spec/acceptance/handlers/team1_handler.rb index 574ca831..a4c52ece 100644 --- a/spec/acceptance/handlers/team1_handler.rb +++ b/spec/acceptance/handlers/team1_handler.rb @@ -14,7 +14,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" From 8dd83384e22bc516d17cae3d7e2cde42896af18d Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 21:08:35 -0700 Subject: [PATCH 14/25] cleanup --- lib/hooks.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/hooks.rb b/lib/hooks.rb index b557ceab..615fa5bb 100644 --- a/lib/hooks.rb +++ b/lib/hooks.rb @@ -18,8 +18,8 @@ module Hooks # @return [Object] Rack-compatible application def self.build(config: nil, log: nil) Core::Builder.new( - config: config, - log: log, + config:, + log:, ).build end end From d614c36c29251c2b5701dffc2b6510b2691c481e Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 21:13:21 -0700 Subject: [PATCH 15/25] move utils to its own dir --- lib/hooks.rb | 5 ++ lib/hooks/app/api.rb | 2 +- .../request_validator/github_webhooks.rb | 4 +- lib/hooks/plugins/utils/normalize.rb | 85 ------------------- lib/hooks/utils/normalize.rb | 83 ++++++++++++++++++ .../lib/hooks/plugins/utils/normalize_spec.rb | 2 +- 6 files changed, 92 insertions(+), 89 deletions(-) delete mode 100644 lib/hooks/plugins/utils/normalize.rb create mode 100644 lib/hooks/utils/normalize.rb diff --git a/lib/hooks.rb b/lib/hooks.rb index 615fa5bb..6d3ee7d4 100644 --- a/lib/hooks.rb +++ b/lib/hooks.rb @@ -9,6 +9,11 @@ require file end +# Load all utils +Dir[File.join(__dir__, "hooks/utils/**/*.rb")].sort.each do |file| + require file +end + # Main module for the Hooks webhook server framework module Hooks # Build a Rack-compatible webhook server application diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 7ea99013..3be4051c 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -206,7 +206,7 @@ def determine_error_code(exception) handler = load_handler(handler_class_name, config[:handler_dir]) # Normalize the headers based on the endpoint configuration (normalization is the default) - headers = Hooks::Plugins::Utils::Normalize.headers(headers) if config[:normalize_headers] + headers = Hooks::Utils::Normalize.headers(headers) if config[:normalize_headers] # Call handler response = handler.call( diff --git a/lib/hooks/plugins/request_validator/github_webhooks.rb b/lib/hooks/plugins/request_validator/github_webhooks.rb index e4fb8bc5..4f630629 100644 --- a/lib/hooks/plugins/request_validator/github_webhooks.rb +++ b/lib/hooks/plugins/request_validator/github_webhooks.rb @@ -26,8 +26,8 @@ def self.valid?(payload:, headers:, secret:, config:) signature_header = config.dig(:request_validator, :header) || "X-Hub-Signature-256" algorithm = config.dig(:request_validator, :algorithm) || "sha256" - signature_header = Utils::Normalize.header(signature_header) - headers = Utils::Normalize.headers(headers) + signature_header = Hooks::Utils::Normalize.header(signature_header) + headers = Hooks::Utils::Normalize.headers(headers) provided_signature = headers[signature_header] return false if provided_signature.nil? || provided_signature.empty? diff --git a/lib/hooks/plugins/utils/normalize.rb b/lib/hooks/plugins/utils/normalize.rb deleted file mode 100644 index 0ff51ab9..00000000 --- a/lib/hooks/plugins/utils/normalize.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -module Hooks - module Plugins - module Utils - # Utility class for normalizing HTTP headers - # - # Provides a robust method to consistently format HTTP headers - # across the application, handling various edge cases and formats. - class Normalize - # Normalize a hash of HTTP headers - # - # @param headers [Hash, #each] Headers hash or hash-like object - # @return [Hash] Normalized headers hash with downcased keys and trimmed values - # - # @example Hash of headers normalization - # headers = { "Content-Type" => " application/json ", "X-GitHub-Event" => "push" } - # normalized = Normalize.headers(headers) - # # => { "content-type" => "application/json", "x-github-event" => "push" } - # - # @example Handle various input types - # Normalize.headers(nil) # => nil - # Normalize.headers({}) # => {} - # Normalize.headers({ "KEY" => ["a", "b"] }) # => { "key" => "a" } - # Normalize.headers({ "Key" => 123 }) # => { "key" => "123" } - def self.headers(headers) - # Handle nil input - return nil if headers.nil? - - # Fast path for non-enumerable inputs (numbers, etc.) - return {} unless headers.respond_to?(:each) - - normalized = {} - - headers.each do |key, value| - # Skip nil keys or values entirely - next if key.nil? || value.nil? - - # Convert key to string, downcase, and strip in one operation - normalized_key = key.to_s.downcase.strip - next if normalized_key.empty? - - # Handle different value types efficiently - normalized_value = case value - when String - value.strip - when Array - # Take first non-empty element for multi-value headers - first_valid = value.find { |v| v && !v.to_s.strip.empty? } - first_valid ? first_valid.to_s.strip : nil - else - value.to_s.strip - end - - # Only add if we have a non-empty value - normalized[normalized_key] = normalized_value if normalized_value && !normalized_value.empty? - end - - normalized - end - - # Normalize a single HTTP header name - # - # @param header [String] Header name to normalize - # @return [String, nil] Normalized header name (downcased and trimmed), or nil if input is nil - # - # @example Single header normalization - # Normalize.header(" Content-Type ") # => "content-type" - # Normalize.header("X-GitHub-Event") # => "x-github-event" - # Normalize.header("") # => "" - # Normalize.header(nil) # => nil - # - # @raise [ArgumentError] If input is not a String or nil - def self.header(header) - return nil if header.nil? - if header.is_a?(String) - header.downcase.strip - else - raise ArgumentError, "Expected a String for header normalization" - end - end - end - end - end -end diff --git a/lib/hooks/utils/normalize.rb b/lib/hooks/utils/normalize.rb new file mode 100644 index 00000000..a9f395cc --- /dev/null +++ b/lib/hooks/utils/normalize.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Hooks + module Utils + # Utility class for normalizing HTTP headers + # + # Provides a robust method to consistently format HTTP headers + # across the application, handling various edge cases and formats. + class Normalize + # Normalize a hash of HTTP headers + # + # @param headers [Hash, #each] Headers hash or hash-like object + # @return [Hash] Normalized headers hash with downcased keys and trimmed values + # + # @example Hash of headers normalization + # headers = { "Content-Type" => " application/json ", "X-GitHub-Event" => "push" } + # normalized = Normalize.headers(headers) + # # => { "content-type" => "application/json", "x-github-event" => "push" } + # + # @example Handle various input types + # Normalize.headers(nil) # => nil + # Normalize.headers({}) # => {} + # Normalize.headers({ "KEY" => ["a", "b"] }) # => { "key" => "a" } + # Normalize.headers({ "Key" => 123 }) # => { "key" => "123" } + def self.headers(headers) + # Handle nil input + return nil if headers.nil? + + # Fast path for non-enumerable inputs (numbers, etc.) + return {} unless headers.respond_to?(:each) + + normalized = {} + + headers.each do |key, value| + # Skip nil keys or values entirely + next if key.nil? || value.nil? + + # Convert key to string, downcase, and strip in one operation + normalized_key = key.to_s.downcase.strip + next if normalized_key.empty? + + # Handle different value types efficiently + normalized_value = case value + when String + value.strip + when Array + # Take first non-empty element for multi-value headers + first_valid = value.find { |v| v && !v.to_s.strip.empty? } + first_valid ? first_valid.to_s.strip : nil + else + value.to_s.strip + end + + # Only add if we have a non-empty value + normalized[normalized_key] = normalized_value if normalized_value && !normalized_value.empty? + end + + normalized + end + + # Normalize a single HTTP header name + # + # @param header [String] Header name to normalize + # @return [String, nil] Normalized header name (downcased and trimmed), or nil if input is nil + # + # @example Single header normalization + # Normalize.header(" Content-Type ") # => "content-type" + # Normalize.header("X-GitHub-Event") # => "x-github-event" + # Normalize.header("") # => "" + # Normalize.header(nil) # => nil + # + # @raise [ArgumentError] If input is not a String or nil + def self.header(header) + return nil if header.nil? + if header.is_a?(String) + header.downcase.strip + else + raise ArgumentError, "Expected a String for header normalization" + end + end + end + end +end diff --git a/spec/lib/hooks/plugins/utils/normalize_spec.rb b/spec/lib/hooks/plugins/utils/normalize_spec.rb index 0b2db006..b75fa3e2 100644 --- a/spec/lib/hooks/plugins/utils/normalize_spec.rb +++ b/spec/lib/hooks/plugins/utils/normalize_spec.rb @@ -1,6 +1,6 @@ # frozen_string_literal: true -describe Hooks::Plugins::Utils::Normalize do +describe Hooks::Utils::Normalize do describe ".header" do context "when input is a single string" do it "normalizes header name to lowercase and trims whitespace" do From bd21031e61fe09ea8c16a04f51857c83afc66855 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 21:26:31 -0700 Subject: [PATCH 16/25] remove old file --- handlers/test_handler.rb | 12 ------------ 1 file changed, 12 deletions(-) delete mode 100644 handlers/test_handler.rb diff --git a/handlers/test_handler.rb b/handlers/test_handler.rb deleted file mode 100644 index b8e548c9..00000000 --- a/handlers/test_handler.rb +++ /dev/null @@ -1,12 +0,0 @@ -require_relative "../lib/hooks/handlers/base" - -class TestHandler < Hooks::Handlers::Base - def call(payload:, headers:, config:) - { - status: "test_success", - payload_received: payload, - config_opts: config[:opts], - timestamp: Time.now.iso8601 - } - end -end From 90496efb5bb17a89fa06495cb85b6f6aae96b7b0 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 21:52:14 -0700 Subject: [PATCH 17/25] making hmac generic and not github specific --- .gitignore | 1 + lib/hooks/app/api.rb | 8 +- .../request_validator/github_webhooks.rb | 51 ----- lib/hooks/plugins/request_validator/hmac.rb | 188 ++++++++++++++++++ spec/acceptance/config/endpoints/github.yaml | 7 +- spec/integration/hooks_integration_spec.rb | 16 +- 6 files changed, 205 insertions(+), 66 deletions(-) delete mode 100644 lib/hooks/plugins/request_validator/github_webhooks.rb create mode 100644 lib/hooks/plugins/request_validator/hmac.rb diff --git a/.gitignore b/.gitignore index 5ebc4e9d..f9ddad4c 100644 --- a/.gitignore +++ b/.gitignore @@ -5,6 +5,7 @@ bin/* coverage/ logs/ tmp/ +spec/integration/tmp/ tarballs/ vendor/gems/ .idea diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 3be4051c..39b8936d 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -59,7 +59,7 @@ def enforce_request_limits(config) # Verify the incoming request def validate_request(payload, headers, endpoint_config) request_validator_config = endpoint_config[:request_validator] - validator_type = request_validator_config[:type] + validator_type = request_validator_config[:type].downcase secret_env_key = request_validator_config[:secret_env_key] return unless secret_env_key @@ -72,8 +72,8 @@ def validate_request(payload, headers, endpoint_config) validator_class = nil case validator_type - when "GitHubWebhooks" - validator_class = Plugins::RequestValidator::GitHubWebhooks + when "hmac" + validator_class = Plugins::RequestValidator::HMAC else error!("Custom validators not implemented in POC", 500) end @@ -196,7 +196,7 @@ def determine_error_code(exception) raw_body = request.body.read # Verify/validate request if configured - log.info "validating request (id: #{request_id}, handler: #{handler_class_name})" + log.info "validating request (id: #{request_id}, handler: #{handler_class_name})" if endpoint_config[:request_validator] validate_request(raw_body, headers, endpoint_config) if endpoint_config[:request_validator] # Parse payload (symbolize_payload is true by default) diff --git a/lib/hooks/plugins/request_validator/github_webhooks.rb b/lib/hooks/plugins/request_validator/github_webhooks.rb deleted file mode 100644 index 4f630629..00000000 --- a/lib/hooks/plugins/request_validator/github_webhooks.rb +++ /dev/null @@ -1,51 +0,0 @@ -# frozen_string_literal: true - -require "openssl" -require "rack/utils" -require_relative "base" - -module Hooks - module Plugins - module RequestValidator - # GitHub webhook request validator - # - # Validates GitHub-style webhook signatures using HMAC SHA256 - class GitHubWebhooks < Base - # Validate HMAC SHA256 signature from GitHub webhooks - # - # official docs: https://docs.github.com/en/webhooks/using-webhooks/validating-webhook-deliveries - # - # @param payload [String] Raw request body - # @param headers [Hash] HTTP headers - # @param secret [String] Secret key for HMAC validation - # @param config [Hash] Endpoint configuration with signature settings - # @return [Boolean] true if signature is valid - def self.valid?(payload:, headers:, secret:, config:) - return false if secret.nil? || secret.empty? - - signature_header = config.dig(:request_validator, :header) || "X-Hub-Signature-256" - algorithm = config.dig(:request_validator, :algorithm) || "sha256" - - signature_header = Hooks::Utils::Normalize.header(signature_header) - headers = Hooks::Utils::Normalize.headers(headers) - - provided_signature = headers[signature_header] - return false if provided_signature.nil? || provided_signature.empty? - - # Compute expected signature - computed_signature = "#{algorithm}=" + OpenSSL::HMAC.hexdigest( - OpenSSL::Digest.new(algorithm), - secret, - payload - ) - - # Use secure comparison to prevent timing attacks - Rack::Utils.secure_compare(computed_signature, provided_signature) - rescue => _e - # Log error in production implementation - false - end - end - end - end -end diff --git a/lib/hooks/plugins/request_validator/hmac.rb b/lib/hooks/plugins/request_validator/hmac.rb new file mode 100644 index 00000000..48dd9c58 --- /dev/null +++ b/lib/hooks/plugins/request_validator/hmac.rb @@ -0,0 +1,188 @@ +# frozen_string_literal: true + +require "openssl" +require "rack/utils" +require "time" +require_relative "base" + +module Hooks + module Plugins + module RequestValidator + # Generic HMAC signature validator for webhooks + # + # This validator supports multiple webhook providers with different signature formats: + # - GitHub: X-Hub-Signature-256: sha256=abc123... + # - Shopify: X-Shopify-Hmac-Sha256: abc123... (hash only) + # - Slack: X-Slack-Signature: v0=abc123... (with timestamp validation) + # - And any other HMAC-based webhook provider + # + # @example Basic GitHub-style configuration + # request_validator: + # type: HMAC + # secret_env_key: WEBHOOK_SECRET + # header: X-Hub-Signature-256 + # algorithm: sha256 + # format: "algorithm=signature" + # + # @example Slack-style with timestamp validation + # request_validator: + # type: HMAC + # secret_env_key: SLACK_SIGNING_SECRET + # header: X-Slack-Signature + # timestamp_header: X-Slack-Request-Timestamp + # timestamp_tolerance: 300 # 5 minutes + # algorithm: sha256 + # format: "version=signature" + # version_prefix: "v0" + # payload_template: "{version}:{timestamp}:{body}" + class HMAC < Base + # Default configuration values + DEFAULT_CONFIG = { + algorithm: "sha256", + format: "algorithm=signature", # GitHub default + timestamp_tolerance: 300, # 5 minutes for Slack + version_prefix: "v0" # Slack default + }.freeze + + # Supported signature formats + FORMATS = { + "algorithm=signature" => :github_style, # "sha256=abc123..." + "signature_only" => :shopify_style, # "abc123..." + "version=signature" => :slack_style # "v0=abc123..." + }.freeze + + # Validate HMAC signature from webhook requests + # + # @param payload [String] Raw request body + # @param headers [Hash] HTTP headers + # @param secret [String] Secret key for HMAC validation + # @param config [Hash] Endpoint configuration with signature settings + # @return [Boolean] true if signature is valid + def self.valid?(payload:, headers:, secret:, config:) + return false if secret.nil? || secret.empty? + + validator_config = build_config(config) + normalized_headers = normalize_headers(headers) + + # Get signature from headers + signature_header = validator_config[:header] + provided_signature = normalized_headers[signature_header.downcase] + return false if provided_signature.nil? || provided_signature.empty? + + # Validate timestamp if required (for Slack and others) + if validator_config[:timestamp_header] + return false unless valid_timestamp?(normalized_headers, validator_config) + end + + # Compute expected signature + computed_signature = compute_signature( + payload: payload, + headers: normalized_headers, + secret: secret, + config: validator_config + ) + + # Use secure comparison to prevent timing attacks + Rack::Utils.secure_compare(computed_signature, provided_signature) + rescue StandardError => _e + # Log error in production - for now just return false + false + end + + private + + # Build final configuration by merging defaults with provided config + def self.build_config(config) + validator_config = config.dig(:request_validator) || {} + + DEFAULT_CONFIG.merge({ + header: validator_config[:header] || "X-Signature", + timestamp_header: validator_config[:timestamp_header], + timestamp_tolerance: validator_config[:timestamp_tolerance] || DEFAULT_CONFIG[:timestamp_tolerance], + algorithm: validator_config[:algorithm] || DEFAULT_CONFIG[:algorithm], + format: validator_config[:format] || DEFAULT_CONFIG[:format], + version_prefix: validator_config[:version_prefix] || DEFAULT_CONFIG[:version_prefix], + payload_template: validator_config[:payload_template] + }) + end + + # Normalize headers using the Utils::Normalize class + def self.normalize_headers(headers) + Utils::Normalize.headers(headers) || {} + end + + # Validate timestamp if timestamp validation is configured + def self.valid_timestamp?(headers, config) + timestamp_header = config[:timestamp_header].downcase + timestamp_value = headers[timestamp_header] + + return false unless timestamp_value + + timestamp = timestamp_value.to_i + current_time = Time.now.to_i + tolerance = config[:timestamp_tolerance] + + (current_time - timestamp).abs <= tolerance + end + + # Compute HMAC signature based on provider requirements + def self.compute_signature(payload:, headers:, secret:, config:) + # Determine what to sign based on payload template + signing_payload = build_signing_payload( + payload: payload, + headers: headers, + config: config + ) + + # Compute HMAC hash + algorithm = config[:algorithm] + computed_hash = OpenSSL::HMAC.hexdigest( + OpenSSL::Digest.new(algorithm), + secret, + signing_payload + ) + + # Format according to provider requirements + format_signature(computed_hash, config) + end + + # Build the payload string to sign (handles Slack's special requirements) + def self.build_signing_payload(payload:, headers:, config:) + template = config[:payload_template] + + if template + # Slack-style: "v0:timestamp:body" + timestamp = headers[config[:timestamp_header].downcase] + template + .gsub("{version}", config[:version_prefix]) + .gsub("{timestamp}", timestamp.to_s) + .gsub("{body}", payload) + else + # Standard: just the payload + payload + end + end + + # Format the computed signature based on provider requirements + def self.format_signature(hash, config) + format_style = FORMATS[config[:format]] + + case format_style + when :github_style + # GitHub: "sha256=abc123..." + "#{config[:algorithm]}=#{hash}" + when :shopify_style + # Shopify: just the hash + hash + when :slack_style + # Slack: "v0=abc123..." + "#{config[:version_prefix]}=#{hash}" + else + # Default to GitHub style + "#{config[:algorithm]}=#{hash}" + end + end + end + end + end +end diff --git a/spec/acceptance/config/endpoints/github.yaml b/spec/acceptance/config/endpoints/github.yaml index e2f002ee..8a8b1569 100644 --- a/spec/acceptance/config/endpoints/github.yaml +++ b/spec/acceptance/config/endpoints/github.yaml @@ -4,10 +4,11 @@ handler: GithubHandler # GitHub uses HMAC SHA256 signature validation request_validator: - type: GitHubWebhooks + type: hmac secret_env_key: GITHUB_WEBHOOK_SECRET - # header: X-Hub-Signature-256 # Optional, defaults to X-Hub-Signature-256 - # algorithm: sha256 # Optional, defaults to sha256 + header: X-Hub-Signature-256 + algorithm: sha256 + format: "algorithm=signature" # produces "sha256=abc123..." # Options for GitHub webhook handling opts: diff --git a/spec/integration/hooks_integration_spec.rb b/spec/integration/hooks_integration_spec.rb index 0d9636a8..cdccaaa9 100644 --- a/spec/integration/hooks_integration_spec.rb +++ b/spec/integration/hooks_integration_spec.rb @@ -12,7 +12,7 @@ def app @app ||= Hooks.build( config: { - handler_dir: "./handlers", + handler_dir: "./spec/integration/tmp/handlers", log_level: "error", # Reduce noise in tests request_limit: 1048576, request_timeout: 15, @@ -20,7 +20,7 @@ def app health_path: "/health", version_path: "/version", environment: "development", - endpoints_dir: "./spec/fixtures/endpoints", + endpoints_dir: "./spec/integration/tmp/endpoints", use_catchall_route: true # Enable catch-all route for testing } ) @@ -28,17 +28,17 @@ def app before(:all) do # Create test endpoint config - FileUtils.mkdir_p("./spec/fixtures/endpoints") - File.write("./spec/fixtures/endpoints/test.yaml", { + FileUtils.mkdir_p("./spec/integration/tmp/endpoints") + File.write("./spec/integration/tmp/endpoints/test.yaml", { path: "/test", handler: "TestHandler", opts: { test_mode: true } }.to_yaml) # Create test handler - FileUtils.mkdir_p("./handlers") - File.write("./handlers/test_handler.rb", <<~RUBY) - require_relative "../lib/hooks/handlers/base" + FileUtils.mkdir_p("./spec/integration/tmp/handlers") + File.write("./spec/integration/tmp/handlers/test_handler.rb", <<~RUBY) + require_relative "../../../../lib/hooks/handlers/base" class TestHandler < Hooks::Handlers::Base def call(payload:, headers:, config:) @@ -55,7 +55,7 @@ def call(payload:, headers:, config:) after(:all) do # Clean up test files - FileUtils.rm_rf("./spec/fixtures") + FileUtils.rm_rf("./spec/integration/tmp") end describe "operational endpoints" do From dc156bfb32c4a9d3a9418c25f2f95cc550aa47c6 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 22:10:24 -0700 Subject: [PATCH 18/25] update hmac module --- lib/hooks/plugins/request_validator/hmac.rb | 174 +++++++++++++++----- 1 file changed, 134 insertions(+), 40 deletions(-) diff --git a/lib/hooks/plugins/request_validator/hmac.rb b/lib/hooks/plugins/request_validator/hmac.rb index 48dd9c58..bd43d55a 100644 --- a/lib/hooks/plugins/request_validator/hmac.rb +++ b/lib/hooks/plugins/request_validator/hmac.rb @@ -10,13 +10,10 @@ module Plugins module RequestValidator # Generic HMAC signature validator for webhooks # - # This validator supports multiple webhook providers with different signature formats: - # - GitHub: X-Hub-Signature-256: sha256=abc123... - # - Shopify: X-Shopify-Hmac-Sha256: abc123... (hash only) - # - Slack: X-Slack-Signature: v0=abc123... (with timestamp validation) - # - And any other HMAC-based webhook provider + # This validator supports multiple webhook providers with different signature formats. + # It provides flexible configuration options to handle various HMAC-based authentication schemes. # - # @example Basic GitHub-style configuration + # @example Basic configuration with algorithm prefix # request_validator: # type: HMAC # secret_env_key: WEBHOOK_SECRET @@ -24,40 +21,71 @@ module RequestValidator # algorithm: sha256 # format: "algorithm=signature" # - # @example Slack-style with timestamp validation + # @example Configuration with timestamp validation # request_validator: # type: HMAC - # secret_env_key: SLACK_SIGNING_SECRET - # header: X-Slack-Signature - # timestamp_header: X-Slack-Request-Timestamp + # secret_env_key: WEBHOOK_SECRET + # header: X-Signature + # timestamp_header: X-Request-Timestamp # timestamp_tolerance: 300 # 5 minutes # algorithm: sha256 # format: "version=signature" # version_prefix: "v0" # payload_template: "{version}:{timestamp}:{body}" class HMAC < Base - # Default configuration values + # Default configuration values for HMAC validation + # + # @return [Hash] Default configuration settings + # @note These values provide sensible defaults for most webhook implementations DEFAULT_CONFIG = { algorithm: "sha256", - format: "algorithm=signature", # GitHub default - timestamp_tolerance: 300, # 5 minutes for Slack - version_prefix: "v0" # Slack default + format: "algorithm=signature", # Format: algorithm=hash + timestamp_tolerance: 300, # 5 minutes tolerance for timestamp validation + version_prefix: "v0" # Default version prefix for versioned signatures }.freeze - # Supported signature formats + # Mapping of signature format strings to internal format symbols + # + # @return [Hash] Format string to symbol mapping + # @note Supports three common webhook signature formats: + # - algorithm=signature: "sha256=abc123..." (GitHub, GitLab style) + # - signature_only: "abc123..." (Shopify style) + # - version=signature: "v0=abc123..." (Slack style) FORMATS = { - "algorithm=signature" => :github_style, # "sha256=abc123..." - "signature_only" => :shopify_style, # "abc123..." - "version=signature" => :slack_style # "v0=abc123..." + "algorithm=signature" => :algorithm_prefixed, # "sha256=abc123..." + "signature_only" => :hash_only, # "abc123..." + "version=signature" => :version_prefixed # "v0=abc123..." }.freeze # Validate HMAC signature from webhook requests # - # @param payload [String] Raw request body - # @param headers [Hash] HTTP headers - # @param secret [String] Secret key for HMAC validation - # @param config [Hash] Endpoint configuration with signature settings - # @return [Boolean] true if signature is valid + # Performs comprehensive HMAC signature validation with support for multiple + # signature formats and optional timestamp validation. Uses secure comparison + # to prevent timing attacks. + # + # @param payload [String] Raw request body to validate + # @param headers [Hash] HTTP headers from the request + # @param secret [String] Secret key for HMAC computation + # @param config [Hash] Endpoint configuration containing validator settings + # @option config [Hash] :request_validator Validator-specific configuration + # @option config [String] :header ('X-Signature') Header containing the signature + # @option config [String] :timestamp_header Header containing timestamp (optional) + # @option config [Integer] :timestamp_tolerance (300) Timestamp tolerance in seconds + # @option config [String] :algorithm ('sha256') HMAC algorithm to use + # @option config [String] :format ('algorithm=signature') Signature format + # @option config [String] :version_prefix ('v0') Version prefix for versioned signatures + # @option config [String] :payload_template Template for payload construction + # @return [Boolean] true if signature is valid, false otherwise + # @raise [StandardError] Rescued internally, returns false on any error + # @note This method is designed to be safe and will never raise exceptions + # @note Uses Rack::Utils.secure_compare to prevent timing attacks + # @example Basic validation + # HMAC.valid?( + # payload: request_body, + # headers: request.headers, + # secret: ENV['WEBHOOK_SECRET'], + # config: { request_validator: { header: 'X-Signature' } } + # ) def self.valid?(payload:, headers:, secret:, config:) return false if secret.nil? || secret.empty? @@ -69,16 +97,16 @@ def self.valid?(payload:, headers:, secret:, config:) provided_signature = normalized_headers[signature_header.downcase] return false if provided_signature.nil? || provided_signature.empty? - # Validate timestamp if required (for Slack and others) + # Validate timestamp if required (for services that include timestamp validation) if validator_config[:timestamp_header] return false unless valid_timestamp?(normalized_headers, validator_config) end # Compute expected signature computed_signature = compute_signature( - payload: payload, + payload:, headers: normalized_headers, - secret: secret, + secret:, config: validator_config ) @@ -92,6 +120,14 @@ def self.valid?(payload:, headers:, secret:, config:) private # Build final configuration by merging defaults with provided config + # + # Combines default configuration values with user-provided settings, + # ensuring all required configuration keys are present with sensible defaults. + # + # @param config [Hash] Raw endpoint configuration + # @return [Hash] Merged configuration with defaults applied + # @note Missing configuration values are filled with DEFAULT_CONFIG values + # @api private def self.build_config(config) validator_config = config.dig(:request_validator) || {} @@ -107,11 +143,29 @@ def self.build_config(config) end # Normalize headers using the Utils::Normalize class + # + # Converts header hash to normalized format with lowercase keys for + # case-insensitive header matching. + # + # @param headers [Hash] Raw HTTP headers + # @return [Hash] Normalized headers with lowercase keys + # @note Returns empty hash if headers is nil + # @api private def self.normalize_headers(headers) Utils::Normalize.headers(headers) || {} end # Validate timestamp if timestamp validation is configured + # + # Checks if the provided timestamp is within the configured tolerance + # of the current time. This prevents replay attacks using old requests. + # + # @param headers [Hash] Normalized HTTP headers + # @param config [Hash] Validator configuration + # @return [Boolean] true if timestamp is valid or not required, false otherwise + # @note Returns false if timestamp header is missing when required + # @note Tolerance is applied as absolute difference (past or future) + # @api private def self.valid_timestamp?(headers, config) timestamp_header = config[:timestamp_header].downcase timestamp_value = headers[timestamp_header] @@ -125,13 +179,24 @@ def self.valid_timestamp?(headers, config) (current_time - timestamp).abs <= tolerance end - # Compute HMAC signature based on provider requirements + # Compute HMAC signature based on configuration requirements + # + # Generates the expected HMAC signature for the given payload using the + # specified algorithm and formatting rules. + # + # @param payload [String] Raw request body + # @param headers [Hash] Normalized HTTP headers + # @param secret [String] Secret key for HMAC computation + # @param config [Hash] Validator configuration + # @return [String] Formatted HMAC signature + # @note The returned signature format depends on the configured format style + # @api private def self.compute_signature(payload:, headers:, secret:, config:) # Determine what to sign based on payload template signing_payload = build_signing_payload( - payload: payload, - headers: headers, - config: config + payload:, + headers:, + config: ) # Compute HMAC hash @@ -146,12 +211,28 @@ def self.compute_signature(payload:, headers:, secret:, config:) format_signature(computed_hash, config) end - # Build the payload string to sign (handles Slack's special requirements) + # Build the payload string to sign (handles templated payload requirements) + # + # Constructs the signing payload based on configuration. Some webhook services + # require specific payload formats that include metadata like timestamps. + # + # @param payload [String] Raw request body + # @param headers [Hash] Normalized HTTP headers + # @param config [Hash] Validator configuration + # @return [String] Payload string ready for HMAC computation + # @note When payload_template is provided, it supports variable substitution: + # - {version}: Replaced with version_prefix + # - {timestamp}: Replaced with timestamp from headers + # - {body}: Replaced with the raw payload + # @example Template usage + # template: "{version}:{timestamp}:{body}" + # result: "v0:1609459200:{"event":"push"}" + # @api private def self.build_signing_payload(payload:, headers:, config:) template = config[:payload_template] if template - # Slack-style: "v0:timestamp:body" + # Templated payload format (e.g., "v0:timestamp:body" for timestamp-based validation) timestamp = headers[config[:timestamp_header].downcase] template .gsub("{version}", config[:version_prefix]) @@ -163,22 +244,35 @@ def self.build_signing_payload(payload:, headers:, config:) end end - # Format the computed signature based on provider requirements + # Format the computed signature based on configuration requirements + # + # Applies the appropriate formatting to the computed HMAC hash based on + # the configured signature format style. + # + # @param hash [String] Raw HMAC hash (hexadecimal string) + # @param config [Hash] Validator configuration + # @return [String] Formatted signature string + # @note Supported formats: + # - :algorithm_prefixed: "sha256=abc123..." (GitHub style) + # - :hash_only: "abc123..." (Shopify style) + # - :version_prefixed: "v0=abc123..." (Slack style) + # @note Defaults to algorithm_prefixed format for unknown format styles + # @api private def self.format_signature(hash, config) format_style = FORMATS[config[:format]] case format_style - when :github_style - # GitHub: "sha256=abc123..." + when :algorithm_prefixed + # Algorithm-prefixed format: "sha256=abc123..." (used by GitHub, GitLab, etc.) "#{config[:algorithm]}=#{hash}" - when :shopify_style - # Shopify: just the hash + when :hash_only + # Hash-only format: "abc123..." (used by Shopify, etc.) hash - when :slack_style - # Slack: "v0=abc123..." + when :version_prefixed + # Version-prefixed format: "v0=abc123..." (used by Slack, etc.) "#{config[:version_prefix]}=#{hash}" else - # Default to GitHub style + # Default to algorithm-prefixed format "#{config[:algorithm]}=#{hash}" end end From 9ba064634345e371026ddb9c9d6b20e9d3d5b4da Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 22:15:03 -0700 Subject: [PATCH 19/25] hmac improvements --- lib/hooks/plugins/request_validator/hmac.rb | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/lib/hooks/plugins/request_validator/hmac.rb b/lib/hooks/plugins/request_validator/hmac.rb index bd43d55a..0df6b262 100644 --- a/lib/hooks/plugins/request_validator/hmac.rb +++ b/lib/hooks/plugins/request_validator/hmac.rb @@ -131,11 +131,14 @@ def self.valid?(payload:, headers:, secret:, config:) def self.build_config(config) validator_config = config.dig(:request_validator) || {} + algorithm = validator_config[:algorithm] || DEFAULT_CONFIG[:algorithm] + tolerance = validator_config[:timestamp_tolerance] || DEFAULT_CONFIG[:timestamp_tolerance] + DEFAULT_CONFIG.merge({ header: validator_config[:header] || "X-Signature", timestamp_header: validator_config[:timestamp_header], - timestamp_tolerance: validator_config[:timestamp_tolerance] || DEFAULT_CONFIG[:timestamp_tolerance], - algorithm: validator_config[:algorithm] || DEFAULT_CONFIG[:algorithm], + timestamp_tolerance: tolerance, + algorithm: algorithm, format: validator_config[:format] || DEFAULT_CONFIG[:format], version_prefix: validator_config[:version_prefix] || DEFAULT_CONFIG[:version_prefix], payload_template: validator_config[:payload_template] @@ -167,12 +170,19 @@ def self.normalize_headers(headers) # @note Tolerance is applied as absolute difference (past or future) # @api private def self.valid_timestamp?(headers, config) - timestamp_header = config[:timestamp_header].downcase + timestamp_header = config[:timestamp_header] + return false unless timestamp_header + + timestamp_header = timestamp_header.downcase timestamp_value = headers[timestamp_header] return false unless timestamp_value timestamp = timestamp_value.to_i + + # Ensure timestamp is a valid integer + return false unless timestamp.is_a?(Integer) && timestamp > 0 + current_time = Time.now.to_i tolerance = config[:timestamp_tolerance] From bcdf6869e4e41b5cc4d912c7388d4bd26690de7a Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 22:23:34 -0700 Subject: [PATCH 20/25] add spec tests for hmac --- .../plugins/request_validator/hmac_spec.rb | 231 ++++++++++++++++++ 1 file changed, 231 insertions(+) create mode 100644 spec/lib/hooks/plugins/request_validator/hmac_spec.rb diff --git a/spec/lib/hooks/plugins/request_validator/hmac_spec.rb b/spec/lib/hooks/plugins/request_validator/hmac_spec.rb new file mode 100644 index 00000000..50313822 --- /dev/null +++ b/spec/lib/hooks/plugins/request_validator/hmac_spec.rb @@ -0,0 +1,231 @@ +# frozen_string_literal: true + +describe Hooks::Plugins::RequestValidator::HMAC do + let(:secret) { "supersecret" } + let(:payload) { '{"foo":"bar"}' } + let(:default_header) { "X-Hub-Signature-256" } + let(:default_algorithm) { "sha256" } + let(:default_config) do + { + request_validator: { + header: default_header, + algorithm: default_algorithm, + format: "algorithm=signature" + } + } + end + + def valid_with(args = {}) + args = { config: default_config }.merge(args) + described_class.valid?(payload:, secret:, **args) + end + + describe ".valid?" do + context "with algorithm-prefixed format" do + let(:signature) { "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) } + let(:headers) { { default_header => signature } } + + it "returns true for a valid signature" do + expect(valid_with(headers:)).to be true + end + + it "returns false for an invalid signature" do + bad_headers = { default_header => "sha256=bad" } + expect(valid_with(headers: bad_headers)).to be false + end + + it "returns false if signature header is missing" do + expect(valid_with(headers: {})).to be false + end + + it "returns false if secret is nil or empty" do + expect(valid_with(headers:, secret: nil)).to be false + expect(valid_with(headers:, secret: "")).to be false + end + + it "normalizes header names to lowercase" do + upcase_headers = { default_header.upcase => signature } + expect(valid_with(headers: upcase_headers)).to be true + end + end + + context "with hash-only format" do + let(:header) { "X-Signature-Hash" } + let(:config) do + { + request_validator: { + header: header, + algorithm: "sha256", + format: "signature_only" + } + } + end + let(:signature) { OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) } + let(:headers) { { header => signature } } + + it "returns true for a valid hash-only signature" do + expect(valid_with(headers:, config:)).to be true + end + + it "returns false for an invalid hash-only signature" do + bad_headers = { header => "bad" } + expect(valid_with(headers: bad_headers, config:)).to be false + end + end + + context "with version-prefixed format and timestamp" do + let(:header) { "X-Signature-Versioned" } + let(:timestamp_header) { "X-Request-Timestamp" } + let(:timestamp) { Time.now.to_i.to_s } + let(:payload_template) { "v0:{timestamp}:{body}" } + let(:signing_payload) { "v0:#{timestamp}:#{payload}" } + let(:signature) { "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) } + let(:headers) { { header => signature, timestamp_header => timestamp } } + let(:config) do + { + request_validator: { + header: header, + timestamp_header: timestamp_header, + algorithm: "sha256", + format: "version=signature", + version_prefix: "v0", + payload_template: payload_template, + timestamp_tolerance: 300 + } + } + end + + it "returns true for a valid versioned signature with valid timestamp" do + expect(valid_with(headers:, config:)).to be true + end + + it "returns false for an expired timestamp" do + old_timestamp = (Time.now.to_i - 1000).to_s + old_signing_payload = "v0:#{old_timestamp}:#{payload}" + old_signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, old_signing_payload) + bad_headers = { header => old_signature, timestamp_header => old_timestamp } + expect(valid_with(headers: bad_headers, config:)).to be false + end + + it "returns false if timestamp header is missing" do + bad_headers = { header => signature } + expect(valid_with(headers: bad_headers, config:)).to be false + end + + it "returns false if timestamp is not an integer string" do + bad_headers = { header => signature, timestamp_header => "notanumber" } + expect(valid_with(headers: bad_headers, config:)).to be false + end + end + + context "with unsupported algorithm" do + let(:header) { "X-Unsupported-Alg" } + let(:config) do + { + request_validator: { + header: header, + algorithm: "md5", + format: "algorithm=signature" + } + } + end + let(:signature) { "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) } + let(:headers) { { header => signature } } + + it "returns false for unsupported algorithm" do + expect(valid_with(headers:, config:)).to be false + end + end + + context "with missing config values" do + let(:headers) { { "X-Signature" => "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) } } + let(:config) { {} } + + it "uses defaults and validates correctly" do + expect(valid_with(headers:, config:)).to be true + end + end + + context "with tampered payload" do + let(:signature) { "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) } + let(:headers) { { default_header => signature } } + let(:tampered_payload) { '{"foo":"evil"}' } + + it "returns false if payload does not match signature" do + expect(valid_with(payload: tampered_payload, headers:)).to be false + end + end + + context "with nil headers" do + let(:headers) { nil } + it "returns false" do + expect(valid_with(headers:)).to be false + end + end + + context "with invalid config structure" do + let(:headers) { { default_header => "sha256=bad" } } + let(:config) { { not_validator: true } } + it "returns false" do + expect(valid_with(headers:, config:)).to be false + end + end + end + + describe ".build_signing_payload" do + let(:headers) { { "x-timestamp" => "12345" } } + it "substitutes variables in template" do + template = "v0:{timestamp}:{body}" + config = { version_prefix: "v0", timestamp_header: "x-timestamp", payload_template: template } + result = described_class.send(:build_signing_payload, payload:, headers:, config:) + expect(result).to eq("v0:12345:{\"foo\":\"bar\"}") + end + it "returns payload if no template" do + config = {} + result = described_class.send(:build_signing_payload, payload:, headers:, config:) + expect(result).to eq(payload) + end + end + + describe ".format_signature" do + it "formats algorithm-prefixed" do + config = { algorithm: "sha256", format: "algorithm=signature" } + expect(described_class.send(:format_signature, "abc123", config)).to eq("sha256=abc123") + end + it "formats hash-only" do + config = { format: "signature_only" } + expect(described_class.send(:format_signature, "abc123", config)).to eq("abc123") + end + it "formats version-prefixed" do + config = { version_prefix: "v0", format: "version=signature" } + expect(described_class.send(:format_signature, "abc123", config)).to eq("v0=abc123") + end + it "defaults to algorithm-prefixed" do + config = { algorithm: "sha256", format: "unknown" } + expect(described_class.send(:format_signature, "abc123", config)).to eq("sha256=abc123") + end + end + + describe ".normalize_headers" do + it "returns empty hash for nil headers" do + expect(described_class.send(:normalize_headers, nil)).to eq({}) + end + it "downcases header keys" do + headers = { "X-FOO" => "bar" } + expect(described_class.send(:normalize_headers, headers)).to eq({ "x-foo" => "bar" }) + end + end + + describe ".build_config" do + it "applies defaults when config is missing" do + expect(described_class.send(:build_config, {})).to include(:algorithm, :format, :timestamp_tolerance, :version_prefix) + end + it "overrides defaults with provided config" do + config = { request_validator: { algorithm: "sha512", format: "signature_only", header: "X-My-Sig" } } + result = described_class.send(:build_config, config) + expect(result[:algorithm]).to eq("sha512") + expect(result[:format]).to eq("signature_only") + expect(result[:header]).to eq("X-My-Sig") + end + end +end From 6342342db7c63d713195cf321f61feba200772ac Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 22:32:41 -0700 Subject: [PATCH 21/25] hmac hardening --- lib/hooks/plugins/request_validator/hmac.rb | 33 +- .../plugins/request_validator/hmac_spec.rb | 341 ++++++++++++++++++ 2 files changed, 369 insertions(+), 5 deletions(-) diff --git a/lib/hooks/plugins/request_validator/hmac.rb b/lib/hooks/plugins/request_validator/hmac.rb index 0df6b262..c5ca2ee2 100644 --- a/lib/hooks/plugins/request_validator/hmac.rb +++ b/lib/hooks/plugins/request_validator/hmac.rb @@ -90,12 +90,32 @@ def self.valid?(payload:, headers:, secret:, config:) return false if secret.nil? || secret.empty? validator_config = build_config(config) - normalized_headers = normalize_headers(headers) - # Get signature from headers + # Security: Check raw headers BEFORE normalization to detect tampering + return false unless headers.respond_to?(:each) + signature_header = validator_config[:header] + + # Find the signature header with case-insensitive matching but preserve original value + raw_signature = nil + headers.each do |key, value| + if key.to_s.downcase == signature_header.downcase + raw_signature = value.to_s + break + end + end + + return false if raw_signature.nil? || raw_signature.empty? + + # Security: Reject signatures with leading/trailing whitespace + return false if raw_signature != raw_signature.strip + + # Security: Reject signatures containing null bytes or other control characters + return false if raw_signature.match?(/[\u0000-\u001f\u007f-\u009f]/) + + # Now we can safely normalize headers for the rest of the validation + normalized_headers = normalize_headers(headers) provided_signature = normalized_headers[signature_header.downcase] - return false if provided_signature.nil? || provided_signature.empty? # Validate timestamp if required (for services that include timestamp validation) if validator_config[:timestamp_header] @@ -178,10 +198,13 @@ def self.valid_timestamp?(headers, config) return false unless timestamp_value + # Security: Strict timestamp validation - must be only digits with no leading zeros + return false unless timestamp_value.match?(/\A[1-9]\d*\z/) || timestamp_value == "0" + timestamp = timestamp_value.to_i - # Ensure timestamp is a valid integer - return false unless timestamp.is_a?(Integer) && timestamp > 0 + # Ensure timestamp is a positive integer (reject zero and negative) + return false unless timestamp > 0 current_time = Time.now.to_i tolerance = config[:timestamp_tolerance] diff --git a/spec/lib/hooks/plugins/request_validator/hmac_spec.rb b/spec/lib/hooks/plugins/request_validator/hmac_spec.rb index 50313822..6f9dbbe7 100644 --- a/spec/lib/hooks/plugins/request_validator/hmac_spec.rb +++ b/spec/lib/hooks/plugins/request_validator/hmac_spec.rb @@ -170,6 +170,319 @@ def valid_with(args = {}) expect(valid_with(headers:, config:)).to be false end end + + context "security and edge cases" do + let(:signature) { "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) } + let(:headers) { { default_header => signature } } + + it "returns false for empty signature header value" do + empty_headers = { default_header => "" } + expect(valid_with(headers: empty_headers)).to be false + end + + it "returns false for whitespace-only signature" do + whitespace_headers = { default_header => " " } + expect(valid_with(headers: whitespace_headers)).to be false + end + + it "returns false for signature with only algorithm prefix" do + incomplete_headers = { default_header => "sha256=" } + expect(valid_with(headers: incomplete_headers)).to be false + end + + it "returns false for malformed signature format" do + malformed_headers = { default_header => "sha256" } + expect(valid_with(headers: malformed_headers)).to be false + end + + it "returns false for signature with wrong algorithm prefix" do + wrong_algo_headers = { default_header => "sha1=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha1"), secret, payload) } + expect(valid_with(headers: wrong_algo_headers)).to be false + end + + it "returns false for signature with extra characters" do + tampered_headers = { default_header => signature + "extra" } + expect(valid_with(headers: tampered_headers)).to be false + end + + it "returns false for signature with leading/trailing whitespace" do + whitespace_headers = { default_header => " #{signature} " } + expect(valid_with(headers: whitespace_headers)). to be false + end + + it "returns false for case-sensitive signature tampering" do + case_tampered = signature.upcase + case_headers = { default_header => case_tampered } + expect(valid_with(headers: case_headers)).to be false + end + + it "returns false for null byte injection in signature" do + null_byte_headers = { default_header => signature + "\x00" } + expect(valid_with(headers: null_byte_headers)).to be false + end + + it "returns false for unicode normalization attacks" do + # Using similar-looking unicode characters + unicode_headers = { default_header => signature.gsub('a', 'ะฐ') } # Cyrillic 'a' + expect(valid_with(headers: unicode_headers)).to be false + end + + it "returns false when secret contains null bytes" do + null_secret = "secret\x00injection" + expect(valid_with(headers:, secret: null_secret)).to be false + end + + it "returns false when payload is modified with invisible characters" do + invisible_payload = payload + "\u200b" # Zero-width space + expect(valid_with(payload: invisible_payload, headers:)).to be false + end + + it "handles very long signatures gracefully" do + long_signature = "sha256=" + ("a" * 10000) + long_headers = { default_header => long_signature } + expect(valid_with(headers: long_headers)).to be false + end + + it "handles very long payloads" do + long_payload = "a" * 100000 + long_signature = "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, long_payload) + long_headers = { default_header => long_signature } + expect(valid_with(payload: long_payload, headers: long_headers)).to be true + end + end + + context "format mismatch attacks" do + let(:base_payload) { '{"data":"test"}' } + + it "fails when server expects algorithm-prefixed but receives hash-only" do + # Server configured for algorithm-prefixed format + server_config = { + request_validator: { + header: "X-Signature", + algorithm: "sha256", + format: "algorithm=signature" + } + } + + # Attacker sends hash-only format + hash_only_sig = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, base_payload) + attacker_headers = { "X-Signature" => hash_only_sig } + + expect(valid_with(payload: base_payload, headers: attacker_headers, config: server_config)).to be false + end + + it "fails when server expects hash-only but receives algorithm-prefixed" do + # Server configured for hash-only format + server_config = { + request_validator: { + header: "X-Signature", + algorithm: "sha256", + format: "signature_only" + } + } + + # Attacker sends algorithm-prefixed format + algo_prefixed_sig = "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, base_payload) + attacker_headers = { "X-Signature" => algo_prefixed_sig } + + expect(valid_with(payload: base_payload, headers: attacker_headers, config: server_config)).to be false + end + + it "fails when server expects version-prefixed but receives algorithm-prefixed" do + # Server configured for version-prefixed format with timestamp + timestamp = Time.now.to_i.to_s + server_config = { + request_validator: { + header: "X-Signature", + timestamp_header: "X-Timestamp", + algorithm: "sha256", + format: "version=signature", + version_prefix: "v0", + payload_template: "v0:{timestamp}:{body}", + timestamp_tolerance: 300 + } + } + + # Attacker sends algorithm-prefixed format (ignoring timestamp) + algo_sig = "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, base_payload) + attacker_headers = { + "X-Signature" => algo_sig, + "X-Timestamp" => timestamp + } + + expect(valid_with(payload: base_payload, headers: attacker_headers, config: server_config)).to be false + end + + it "fails when algorithm in config differs from signature prefix" do + # Server configured for sha512 + server_config = { + request_validator: { + header: "X-Signature", + algorithm: "sha512", + format: "algorithm=signature" + } + } + + # Attacker sends sha256 signature + sha256_sig = "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, base_payload) + attacker_headers = { "X-Signature" => sha256_sig } + + expect(valid_with(payload: base_payload, headers: attacker_headers, config: server_config)).to be false + end + + it "fails when version prefix in config differs from signature" do + timestamp = Time.now.to_i.to_s + signing_payload = "v1:#{timestamp}:#{base_payload}" + + # Server configured for v0 prefix + server_config = { + request_validator: { + header: "X-Signature", + timestamp_header: "X-Timestamp", + algorithm: "sha256", + format: "version=signature", + version_prefix: "v0", + payload_template: "v0:{timestamp}:{body}", + timestamp_tolerance: 300 + } + } + + # Attacker sends v1 signature + v1_sig = "v1=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + attacker_headers = { + "X-Signature" => v1_sig, + "X-Timestamp" => timestamp + } + + expect(valid_with(payload: base_payload, headers: attacker_headers, config: server_config)).to be false + end + end + + context "timestamp validation edge cases" do + let(:header) { "X-Signature" } + let(:timestamp_header) { "X-Timestamp" } + let(:base_config) do + { + request_validator: { + header: header, + timestamp_header: timestamp_header, + algorithm: "sha256", + format: "version=signature", + version_prefix: "v0", + payload_template: "v0:{timestamp}:{body}", + timestamp_tolerance: 300 + } + } + end + + it "returns false for negative timestamp" do + negative_timestamp = "-1" + signing_payload = "v0:#{negative_timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + headers = { header => signature, timestamp_header => negative_timestamp } + + expect(valid_with(headers:, config: base_config)).to be false + end + + it "returns false for zero timestamp" do + zero_timestamp = "0" + signing_payload = "v0:#{zero_timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + headers = { header => signature, timestamp_header => zero_timestamp } + + expect(valid_with(headers:, config: base_config)).to be false + end + + it "returns false for timestamp with decimal point" do + decimal_timestamp = "#{Time.now.to_i}.5" + signing_payload = "v0:#{decimal_timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + headers = { header => signature, timestamp_header => decimal_timestamp } + + expect(valid_with(headers:, config: base_config)).to be false + end + + it "returns false for timestamp with leading zeros" do + padded_timestamp = "00#{Time.now.to_i}" + signing_payload = "v0:#{padded_timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + headers = { header => signature, timestamp_header => padded_timestamp } + + expect(valid_with(headers:, config: base_config)).to be false + end + + it "returns false for timestamp with embedded null bytes" do + null_timestamp = "#{Time.now.to_i}\x00123" + signing_payload = "v0:#{null_timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + headers = { header => signature, timestamp_header => null_timestamp } + + expect(valid_with(headers:, config: base_config)).to be false + end + + it "returns false for very large timestamp (year 2100+)" do + future_timestamp = "4000000000" # Year ~2096 + signing_payload = "v0:#{future_timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + headers = { header => signature, timestamp_header => future_timestamp } + + expect(valid_with(headers:, config: base_config)).to be false + end + + it "returns false when timestamp header name case differs" do + timestamp = Time.now.to_i.to_s + signing_payload = "v0:#{timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + + # Use different case for timestamp header + headers = { header => signature, "X-TIMESTAMP" => timestamp } + + expect(valid_with(headers:, config: base_config)).to be true # Should work due to normalization + end + end + + context "error handling and resilience" do + it "returns false when OpenSSL raises an error" do + allow(OpenSSL::HMAC).to receive(:hexdigest).and_raise(OpenSSL::OpenSSLError, "Invalid algorithm") + + signature = "sha256=fakesignature" + headers = { default_header => signature } + + expect(valid_with(headers:)).to be false + end + + it "returns false when Rack::Utils.secure_compare raises an error" do + signature = "sha256=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, payload) + headers = { default_header => signature } + + allow(Rack::Utils).to receive(:secure_compare).and_raise(StandardError, "Comparison error") + + expect(valid_with(headers:)).to be false + end + + it "returns false when Time.now raises an error" do + config = { + request_validator: { + header: "X-Signature", + timestamp_header: "X-Timestamp", + algorithm: "sha256", + format: "version=signature", + version_prefix: "v0", + payload_template: "v0:{timestamp}:{body}", + timestamp_tolerance: 300 + } + } + + timestamp = Time.now.to_i.to_s + signing_payload = "v0:#{timestamp}:#{payload}" + signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) + headers = { "X-Signature" => signature, "X-Timestamp" => timestamp } + + allow(Time).to receive(:now).and_raise(StandardError, "Time error") + + expect(valid_with(headers:, config:)).to be false + end + end end describe ".build_signing_payload" do @@ -228,4 +541,32 @@ def valid_with(args = {}) expect(result[:header]).to eq("X-My-Sig") end end + + describe ".valid_timestamp?" do + let(:config) do + { + timestamp_header: "X-Timestamp", + timestamp_tolerance: 300 + } + end + + it "returns false when timestamp header is nil in config" do + no_header_config = { timestamp_tolerance: 300 } + headers = { "x-timestamp" => Time.now.to_i.to_s } + + expect(described_class.send(:valid_timestamp?, headers, no_header_config)).to be false + end + + it "returns false when timestamp value contains non-digits" do + headers = { "x-timestamp" => "123abc" } + + expect(described_class.send(:valid_timestamp?, headers, config)).to be false + end + + it "returns false when timestamp is empty string" do + headers = { "x-timestamp" => "" } + + expect(described_class.send(:valid_timestamp?, headers, config)). to be false + end + end end From 42dae3150632e85364cf89c78550ebb653f2cd71 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 23:09:51 -0700 Subject: [PATCH 22/25] fixes --- lib/hooks/core/config_validator.rb | 5 ++++ spec/acceptance/acceptance_tests.rb | 26 ++++++++++++++++++- spec/acceptance/config/endpoints/slack.yaml | 13 ++++++++++ spec/acceptance/docker-compose.yml | 1 + spec/acceptance/handlers/slack_handler.rb | 9 +++++++ spec/acceptance/handlers/team1_handler.rb | 3 --- .../plugins/request_validator/hmac_spec.rb | 21 ++++++++++++++- spec/spec_helper.rb | 2 ++ 8 files changed, 75 insertions(+), 5 deletions(-) create mode 100644 spec/acceptance/config/endpoints/slack.yaml create mode 100644 spec/acceptance/handlers/slack_handler.rb diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index 5b2088c8..a488ab9d 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -35,6 +35,11 @@ class ValidationError < StandardError; end optional(:secret_env_key).filled(:string) optional(:header).filled(:string) optional(:algorithm).filled(:string) + optional(:timestamp_header).filled(:string) + optional(:timestamp_tolerance).filled(:integer, gt?: 0) + optional(:format).filled(:string) + optional(:version_prefix).filled(:string) + optional(:payload_template).filled(:string) end optional(:opts).hash diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 593938b6..e5969d3f 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true -require_relative "../spec_helper" +FAKE_HMAC_SECRET = "octoawesome-secret" +FAKE_ALT_HMAC_SECRET = "octoawesome-2-secret" require "rspec" require "net/http" @@ -90,6 +91,17 @@ expect(response.body).to include("request validation failed") end + it "receives a POST request but it uses the wrong algo" do + payload = { action: "push", repository: { name: "test-repo" } } + headers = { + "Content-Type" => "application/json", + "X-Hub-Signature-256" => "sha512=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha512"), FAKE_HMAC_SECRET, payload.to_json) + } + response = http.post("/webhooks/github", payload.to_json, headers) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("request validation failed") + end + it "successfully processes a valid POST request with HMAC signature" do payload = { action: "push", repository: { name: "test-repo" } } headers = { @@ -102,5 +114,17 @@ expect(body["status"]).to eq("success") end end + + describe "slack" do + it "receives a POST request but contains an invalid HMAC signature" do + payload = { text: "Hello, Slack!" } + digest = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), FAKE_ALT_HMAC_SECRET, payload.to_json) + headers = { "Content-Type" => "application/json", "Signature-256" => "sha256=#{digest}" } + response = http.post("/webhooks/slack", payload.to_json, headers) + + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.body).to include("request validation failed") + end + end end end diff --git a/spec/acceptance/config/endpoints/slack.yaml b/spec/acceptance/config/endpoints/slack.yaml new file mode 100644 index 00000000..c20a3934 --- /dev/null +++ b/spec/acceptance/config/endpoints/slack.yaml @@ -0,0 +1,13 @@ +path: /slack +handler: SlackHandler + +request_validator: + type: hmac + secret_env_key: ALT_WEBHOOK_SECRET + header: Signature-256 + algorithm: sha256 + format: "version=signature" # produces "v0=abc123..." + timestamp_header: "X-Timestamp" + version_prefix: "v0" + payload_template: "v0:{timestamp}:{body}" + timestamp_tolerance: 300 diff --git a/spec/acceptance/docker-compose.yml b/spec/acceptance/docker-compose.yml index 8222315d..74da9e09 100644 --- a/spec/acceptance/docker-compose.yml +++ b/spec/acceptance/docker-compose.yml @@ -9,6 +9,7 @@ services: environment: LOG_LEVEL: DEBUG GITHUB_WEBHOOK_SECRET: "octoawesome-secret" + ALT_WEBHOOK_SECRET: "octoawesome-too-secret" command: ["script/server"] healthcheck: test: ["CMD", "curl", "-f", "http://0.0.0.0:8080/health"] diff --git a/spec/acceptance/handlers/slack_handler.rb b/spec/acceptance/handlers/slack_handler.rb new file mode 100644 index 00000000..6fe61c41 --- /dev/null +++ b/spec/acceptance/handlers/slack_handler.rb @@ -0,0 +1,9 @@ +# frozen_string_literal: true + +class SlackHandler < Hooks::Handlers::Base + def call(payload:, headers:, config:) + return { + status: "success" + } + end +end diff --git a/spec/acceptance/handlers/team1_handler.rb b/spec/acceptance/handlers/team1_handler.rb index a4c52ece..0d51d6ef 100644 --- a/spec/acceptance/handlers/team1_handler.rb +++ b/spec/acceptance/handlers/team1_handler.rb @@ -9,9 +9,6 @@ class Team1Handler < Hooks::Handlers::Base # @param config [Hash] Endpoint configuration # @return [Hash] Response data def call(payload:, headers:, config:) - # Log the webhook receipt - puts "Team1Handler: Received webhook for #{config.dig(:opts, :teams)&.join(', ')}" - # Process the payload based on type if payload.is_a?(Hash) event_type = payload[:event_type] || "unknown" diff --git a/spec/lib/hooks/plugins/request_validator/hmac_spec.rb b/spec/lib/hooks/plugins/request_validator/hmac_spec.rb index 6f9dbbe7..44ed5a92 100644 --- a/spec/lib/hooks/plugins/request_validator/hmac_spec.rb +++ b/spec/lib/hooks/plugins/request_validator/hmac_spec.rb @@ -313,6 +313,25 @@ def valid_with(args = {}) expect(valid_with(payload: base_payload, headers: attacker_headers, config: server_config)).to be false end + it "fails when server expects algorithm-prefixed but receives version-prefixed" do + # Server configured for algorithm-prefixed format + server_config = { + request_validator: { + header: "X-Signature", + algorithm: "sha256", + format: "algorithm=signature" + } + } + # Attacker sends version-prefixed format + timestamp = Time.now.to_i.to_s + versioned_sig = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, "v0:#{timestamp}:#{base_payload}") + attacker_headers = { + "X-Signature" => versioned_sig, + "X-Timestamp" => timestamp + } + expect(valid_with(payload: base_payload, headers: attacker_headers, config: server_config)).to be false + end + it "fails when algorithm in config differs from signature prefix" do # Server configured for sha512 server_config = { @@ -429,7 +448,7 @@ def valid_with(args = {}) expect(valid_with(headers:, config: base_config)).to be false end - it "returns false when timestamp header name case differs" do + it "returns true when timestamp header name case differs due to normalization" do timestamp = Time.now.to_i.to_s signing_payload = "v0:#{timestamp}:#{payload}" signature = "v0=" + OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha256"), secret, signing_payload) diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 359b8609..7b1951d5 100644 --- a/spec/spec_helper.rb +++ b/spec/spec_helper.rb @@ -7,8 +7,10 @@ TIME_MOCK = "2025-01-01T00:00:00Z" FAKE_HMAC_SECRET = "octoawesome-secret" +FAKE_ALT_HMAC_SECRET = "octoawesome-2-secret" ENV["GITHUB_WEBHOOK_SECRET"] = FAKE_HMAC_SECRET +ENV["ALT_WEBHOOK_SECRET"] = FAKE_ALT_HMAC_SECRET COV_DIR = File.expand_path("../coverage", File.dirname(__FILE__)) From dd2d46085fb79a2544223f229a86112aa790a658 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 23:11:29 -0700 Subject: [PATCH 23/25] add note --- .github/copilot-instructions.md | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index d6482214..0bcaef89 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -73,6 +73,7 @@ The linter is powered by `rubocop` with its config file located at `.rubocop.yml - The use of whitespace (newlines) over compactness of files. - Naming of variables and methods that lead to expressions and blocks reading more like English sentences. - Less lines of code over more. Keep changes minimal and focused. +- The `docs/design.md` file is the main design document for the project. It might be out-of-date but it should still contain a general high-level overview of the project. ## Pull Request Requirements From b04aa8efd938b8dd63ec3fd29cb85c1520ae5e1f Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 23:19:14 -0700 Subject: [PATCH 24/25] use 0.0.0.0 --- spec/acceptance/acceptance_tests.rb | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index e5969d3f..35daa7d8 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -10,13 +10,13 @@ MAX_WAIT_TIME = 30 # how long to wait for the server to start describe "Hooks" do - let(:http) { Net::HTTP.new("127.0.0.1", 8080) } + let(:http) { Net::HTTP.new("0.0.0.0", 8080) } before(:all) do start_time = Time.now loop do begin - response = Net::HTTP.new("127.0.0.1", 8080).get("/health") + response = Net::HTTP.new("0.0.0.0", 8080).get("/health") break if response.is_a?(Net::HTTPSuccess) rescue Errno::ECONNREFUSED, SocketError # Server not ready yet, continue waiting From 491e4e0997390a7f08c7e9901e418d5ebbb5fdcd Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Mon, 9 Jun 2025 23:23:34 -0700 Subject: [PATCH 25/25] try again --- spec/acceptance/acceptance_tests.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 35daa7d8..8e7bde4b 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -18,7 +18,7 @@ begin response = Net::HTTP.new("0.0.0.0", 8080).get("/health") break if response.is_a?(Net::HTTPSuccess) - rescue Errno::ECONNREFUSED, SocketError + rescue Errno::ECONNREFUSED, SocketError, StandardError # Server not ready yet, continue waiting end