From c18954239a0baec01f85af71f7b301562e02f791 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Jun 2025 05:38:14 +0000 Subject: [PATCH 01/11] Initial plan for issue From 09a8393e8cfb9ade006f13a49420905a39ae5cda Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Jun 2025 05:43:14 +0000 Subject: [PATCH 02/11] Initial plan: Default to JSON in and JSON out Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- .bundle/config | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.bundle/config b/.bundle/config index 0146a1ce..f9263841 100644 --- a/.bundle/config +++ b/.bundle/config @@ -1,7 +1,8 @@ --- BUNDLE_BIN: "bin" -BUNDLE_PATH: "vendor/gems" +BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle" BUNDLE_CACHE_PATH: "vendor/cache" BUNDLE_CACHE_ALL: "true" BUNDLE_SPECIFIC_PLATFORM: "true" BUNDLE_NO_INSTALL: "true" +BUNDLE_DEPLOYMENT: "true" From 2d675236813f5189fc415545a8ab33c38fec9f10 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Jun 2025 05:55:05 +0000 Subject: [PATCH 03/11] Implement JSON default format with configuration support Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- lib/hooks/app/api.rb | 14 ++++++-------- lib/hooks/app/endpoints/catchall.rb | 14 ++++++++++---- lib/hooks/app/endpoints/health.rb | 11 +++++++++-- lib/hooks/app/endpoints/version.rb | 11 +++++++++-- lib/hooks/core/config_loader.rb | 9 ++++++++- spec/unit/lib/hooks/core/config_loader_spec.rb | 14 +++++++++++++- 6 files changed, 55 insertions(+), 18 deletions(-) diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 131ff79c..e293dcdb 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -39,8 +39,8 @@ def self.create(config:, endpoints:, log:) content_type :xml, "application/xml" content_type :any, "*/*" - format :txt # TODO: make this configurable via config[:format] (defaults to :json in the future) - default_format :txt # TODO: make this configurable via config[:default_format] (defaults to :json in the future) + format config[:format] || :json + default_format config[:default_format] || :json end api_class.class_eval do @@ -118,8 +118,7 @@ def self.create(config:, endpoints:, log:) log.info("successfully processed webhook event with handler: #{handler_class_name}") log.debug("processing duration: #{Time.now - start_time}s") status 200 - content_type "application/json" - response.to_json + response rescue Hooks::Plugins::Handlers::Error => e # Handler called error! method - immediately return error response and exit the request log.debug("handler #{handler_class_name} called `error!` method") @@ -132,8 +131,8 @@ def self.create(config:, endpoints:, log:) content_type "text/plain" error_response = e.body else - content_type "application/json" - error_response = e.body.to_json + # Let Grape handle JSON conversion with the default format + error_response = e.body end return error_response @@ -164,8 +163,7 @@ def self.create(config:, endpoints:, log:) error_response[:handler] = handler_class_name unless config[:production] status determine_error_code(e) - content_type "application/json" - error_response.to_json + error_response end end end diff --git a/lib/hooks/app/endpoints/catchall.rb b/lib/hooks/app/endpoints/catchall.rb index e240e2ce..b1cb2a4c 100644 --- a/lib/hooks/app/endpoints/catchall.rb +++ b/lib/hooks/app/endpoints/catchall.rb @@ -17,6 +17,14 @@ module App class CatchallEndpoint < Grape::API include Hooks::App::Helpers + # Set up content types and default format to JSON to match main API + content_type :json, "application/json" + content_type :txt, "text/plain" + content_type :xml, "application/xml" + content_type :any, "*/*" + format :json + default_format :json + def self.mount_path(config) # :nocov: "#{config[:root_path]}/*path" @@ -81,8 +89,7 @@ def self.route_block(captured_config, captured_logger) log.info("successfully processed webhook event with handler: #{handler_class_name}") log.debug("processing duration: #{Time.now - start_time}s") status 200 - content_type "application/json" - response.to_json + response rescue StandardError => e err_msg = "Error processing webhook event with handler: #{handler_class_name} - #{e.message} " \ "- request_id: #{request_id} - path: #{full_path} - method: #{http_method} - " \ @@ -102,8 +109,7 @@ def self.route_block(captured_config, captured_logger) error_response[:handler] = handler_class_name unless config[:production] status determine_error_code(e) - content_type "application/json" - error_response.to_json + error_response end end end diff --git a/lib/hooks/app/endpoints/health.rb b/lib/hooks/app/endpoints/health.rb index e8c27ba6..0a4d27c4 100644 --- a/lib/hooks/app/endpoints/health.rb +++ b/lib/hooks/app/endpoints/health.rb @@ -6,14 +6,21 @@ module Hooks module App class HealthEndpoint < Grape::API + # Set up content types and default format to JSON + content_type :json, "application/json" + content_type :txt, "text/plain" + content_type :xml, "application/xml" + content_type :any, "*/*" + format :json + default_format :json + get do - content_type "application/json" { status: "healthy", timestamp: Time.now.utc.iso8601, version: Hooks::VERSION, uptime_seconds: (Time.now - Hooks::App::API.server_start_time).to_i - }.to_json + } end end end diff --git a/lib/hooks/app/endpoints/version.rb b/lib/hooks/app/endpoints/version.rb index d88dbbcb..7b7dd431 100644 --- a/lib/hooks/app/endpoints/version.rb +++ b/lib/hooks/app/endpoints/version.rb @@ -6,12 +6,19 @@ module Hooks module App class VersionEndpoint < Grape::API + # Set up content types and default format to JSON + content_type :json, "application/json" + content_type :txt, "text/plain" + content_type :xml, "application/xml" + content_type :any, "*/*" + format :json + default_format :json + get do - content_type "application/json" { version: Hooks::VERSION, timestamp: Time.now.utc.iso8601 - }.to_json + } end end end diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index 69fd677a..d77f3278 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -20,7 +20,9 @@ class ConfigLoader production: true, endpoints_dir: "./config/endpoints", use_catchall_route: false, - normalize_headers: true + normalize_headers: true, + format: :json, + default_format: :json }.freeze SILENCE_CONFIG_LOADER_MESSAGES = ENV.fetch( @@ -141,6 +143,8 @@ def self.load_env_config "HOOKS_ENDPOINTS_DIR" => :endpoints_dir, "HOOKS_USE_CATCHALL_ROUTE" => :use_catchall_route, "HOOKS_NORMALIZE_HEADERS" => :normalize_headers, + "HOOKS_FORMAT" => :format, + "HOOKS_DEFAULT_FORMAT" => :default_format, "HOOKS_SOME_STRING_VAR" => :some_string_var # Added for test } @@ -155,6 +159,9 @@ def self.load_env_config when :use_catchall_route, :normalize_headers # Convert string to boolean env_config[config_key] = %w[true 1 yes on].include?(value.downcase) + when :format, :default_format + # Convert string to symbol + env_config[config_key] = value.to_sym else env_config[config_key] = value end diff --git a/spec/unit/lib/hooks/core/config_loader_spec.rb b/spec/unit/lib/hooks/core/config_loader_spec.rb index bab47139..ca583faa 100644 --- a/spec/unit/lib/hooks/core/config_loader_spec.rb +++ b/spec/unit/lib/hooks/core/config_loader_spec.rb @@ -19,7 +19,9 @@ production: true, endpoints_dir: "./config/endpoints", use_catchall_route: false, - normalize_headers: true + normalize_headers: true, + format: :json, + default_format: :json ) end end @@ -197,6 +199,16 @@ expect(config[:normalize_headers]).to be true expect(config[:some_string_var]).to eq("test_value") # Check the string var end + + it "converts format environment variables to symbols" do + ENV["HOOKS_FORMAT"] = "txt" + ENV["HOOKS_DEFAULT_FORMAT"] = "xml" + + config = described_class.load + + expect(config[:format]).to eq(:txt) + expect(config[:default_format]).to eq(:xml) + end end context "with auth plugin directory configuration" do From eaaf3a5270fee7111197ce4283db39c9622006bf Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Jun 2025 06:02:15 +0000 Subject: [PATCH 04/11] Complete JSON-first implementation with updated tests and documentation Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- docs/configuration.md | 18 +++++++- lib/hooks/app/api.rb | 5 +-- lib/hooks/app/endpoints/health.rb | 2 +- spec/acceptance/acceptance_tests.rb | 65 ++++++++++++++++++----------- 4 files changed, 60 insertions(+), 30 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 3f719a22..b972b379 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -103,7 +103,23 @@ When set to `true`, enables a catch-all route that will handle requests to unkno When set to `true`, normalizes incoming HTTP headers by lowercasing and trimming them. This ensures consistency in header names and values. -**Default:** `true` +**Default:** `true` + +### `format` + +Sets the request/response format for the webhook server. This determines how incoming requests are parsed and how responses are formatted. + +**Default:** `json` +**Valid values:** `json`, `txt`, `xml`, `any` +**Example:** `json` + +### `default_format` + +Sets the default response format when no specific format is requested. This works in conjunction with the `format` setting to determine response formatting. + +**Default:** `json` +**Valid values:** `json`, `txt`, `xml`, `any` +**Example:** `json` ## Endpoint Options diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index e293dcdb..756e292c 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -123,12 +123,11 @@ def self.create(config:, endpoints:, log:) # Handler called error! method - immediately return error response and exit the request log.debug("handler #{handler_class_name} called `error!` method") - error_response = nil - status e.status case e.body when String - content_type "text/plain" + # Even string errors are now JSON-encoded with the default JSON format + content_type "application/json" error_response = e.body else # Let Grape handle JSON conversion with the default format diff --git a/lib/hooks/app/endpoints/health.rb b/lib/hooks/app/endpoints/health.rb index 0a4d27c4..642c56d3 100644 --- a/lib/hooks/app/endpoints/health.rb +++ b/lib/hooks/app/endpoints/health.rb @@ -8,7 +8,7 @@ module App class HealthEndpoint < Grape::API # Set up content types and default format to JSON content_type :json, "application/json" - content_type :txt, "text/plain" + content_type :txt, "text/plain" content_type :xml, "application/xml" content_type :any, "*/*" format :json diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 00aa4276..c3a411fa 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -28,6 +28,18 @@ def expect_response(response, expected_type, expected_body_content = nil) expect(response.body).to include(expected_body_content) if expected_body_content end + def expect_json_auth_failure(response, expected_request_id: nil) + expect(response).to be_a(Net::HTTPUnauthorized) + expect(response.content_type).to eq("application/json") + + body = parse_json_response(response) + expect(body["error"]).to eq("authentication_failed") + expect(body["message"]).to eq("authentication failed") + expect(body).to have_key("request_id") + expect(body["request_id"]).to be_a(String) + expect(body["request_id"]).to eq(expected_request_id) if expected_request_id + end + def parse_json_response(response) JSON.parse(response.body) end @@ -140,13 +152,13 @@ def expired_unix_timestamp(seconds_ago = 600) headers = json_headers("X-Hub-Signature-256" => "sha256=invalidsignature") response = make_request(:post, "/webhooks/github", payload.to_json, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "receives a POST request but there is no HMAC related header" do payload = { action: "push", repository: { name: "test-repo" } } response = make_request(:post, "/webhooks/github", payload.to_json, json_headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "receives a POST request but it uses the wrong algo" do @@ -155,7 +167,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_hmac_signature(json_payload, FAKE_HMAC_SECRET, "sha512", "sha512=") headers = json_headers("X-Hub-Signature-256" => signature) response = make_request(:post, "/webhooks/github", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "successfully processes a valid POST request with HMAC signature" do @@ -212,7 +224,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_hmac_with_timestamp(json_payload, "bad-hmac-secret", timestamp) headers = json_headers("X-HMAC-Signature" => signature, "X-HMAC-Timestamp" => timestamp) response = make_request(:post, "/webhooks/hmac_with_timestamp", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "fails due to missing timestamp header" do @@ -221,7 +233,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_hmac_with_timestamp(json_payload, FAKE_ALT_HMAC_SECRET, current_timestamp) headers = json_headers("X-HMAC-Signature" => signature) response = make_request(:post, "/webhooks/hmac_with_timestamp", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "fails due to invalid timestamp format" do @@ -231,7 +243,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_hmac_with_timestamp(json_payload, FAKE_ALT_HMAC_SECRET, invalid_timestamp) headers = json_headers("X-HMAC-Signature" => signature, "X-HMAC-Timestamp" => invalid_timestamp) response = make_request(:post, "/webhooks/hmac_with_timestamp", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "rejects request with timestamp manipulation attack" do @@ -244,7 +256,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_hmac_with_timestamp(json_payload, FAKE_ALT_HMAC_SECRET, original_timestamp) headers = json_headers("X-HMAC-Signature" => signature, "X-HMAC-Timestamp" => manipulated_timestamp) response = make_request(:post, "/webhooks/hmac_with_timestamp", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "fails because the timestamp is too old" do @@ -254,7 +266,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_hmac_with_timestamp(json_payload, FAKE_ALT_HMAC_SECRET, expired_ts) headers = json_headers("X-HMAC-Signature" => signature, "X-HMAC-Timestamp" => expired_ts) response = make_request(:post, "/webhooks/hmac_with_timestamp", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "fails because the wrong HMAC algorithm is used" do @@ -265,7 +277,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = signature.gsub("sha256=", "sha512=") headers = json_headers("X-HMAC-Signature" => signature, "X-HMAC-Timestamp" => timestamp) response = make_request(:post, "/webhooks/hmac_with_timestamp", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end end @@ -289,7 +301,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_slack_signature(json_payload, FAKE_ALT_HMAC_SECRET, expired_ts) headers = json_headers("Signature-256" => signature, "X-Timestamp" => expired_ts) response = make_request(:post, "/webhooks/slack", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "rejects request with missing timestamp header" do @@ -299,7 +311,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_slack_signature(json_payload, FAKE_ALT_HMAC_SECRET, timestamp) headers = json_headers("Signature-256" => signature) response = make_request(:post, "/webhooks/slack", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "rejects request with invalid timestamp format" do @@ -309,7 +321,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_slack_signature(json_payload, FAKE_ALT_HMAC_SECRET, invalid_timestamp) headers = json_headers("Signature-256" => signature, "X-Timestamp" => invalid_timestamp) response = make_request(:post, "/webhooks/slack", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "successfully processes request with ISO 8601 UTC timestamp" do @@ -355,7 +367,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_slack_signature(json_payload, FAKE_ALT_HMAC_SECRET, non_utc_timestamp) headers = json_headers("Signature-256" => signature, "X-Timestamp" => non_utc_timestamp) response = make_request(:post, "/webhooks/slack", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "rejects request with timestamp manipulation attack" do @@ -368,7 +380,7 @@ def expired_unix_timestamp(seconds_ago = 600) signature = generate_slack_signature(json_payload, FAKE_ALT_HMAC_SECRET, original_timestamp) headers = json_headers("Signature-256" => signature, "X-Timestamp" => manipulated_timestamp) response = make_request(:post, "/webhooks/slack", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end end @@ -377,7 +389,7 @@ def expired_unix_timestamp(seconds_ago = 600) payload = { event: "user.login", user: { id: "12345" } } headers = json_headers("Authorization" => "badvalue") response = make_request(:post, "/webhooks/okta", payload.to_json, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "successfully processes a valid POST request with shared secret" do @@ -420,14 +432,14 @@ def expired_unix_timestamp(seconds_ago = 600) payload = {}.to_json headers = { "Authorization" => "Bearer wrong-secret" } response = make_request(:post, "/webhooks/with_custom_auth_plugin", payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "rejects requests with missing credentials using custom auth plugin" do payload = {}.to_json headers = {} response = make_request(:post, "/webhooks/with_custom_auth_plugin", payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end end @@ -518,12 +530,15 @@ def expired_unix_timestamp(seconds_ago = 600) it "sends a POST request to the /webhooks/boomtown_with_error endpoint and it explodes with a simple text error" do payload = { boom_simple_text: true }.to_json response = make_request(:post, "/webhooks/boomtown_with_error", payload, json_headers) - expect_response(response, Net::HTTPInternalServerError, "boomtown_with_error: the payload triggered a simple text boomtown error") + expect_response(response, Net::HTTPInternalServerError) - body = response.body - expect(body).to eq("boomtown_with_error: the payload triggered a simple text boomtown error") - expect(response.content_type).to eq("text/plain") + # With JSON default format, even string errors are JSON-encoded + expect(response.content_type).to eq("application/json") expect(response.code).to eq("500") + + # The error body should be the JSON-encoded string + body = parse_json_response(response) + expect(body).to eq("boomtown_with_error: the payload triggered a simple text boomtown error") end end @@ -546,14 +561,14 @@ def expired_unix_timestamp(seconds_ago = 600) json_payload = payload.to_json headers = json_headers("Tailscale-Webhook-Signature" => "t=1663781880,v1=invalidsignature") response = make_request(:post, "/webhooks/tailscale", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "rejects request with missing signature header" do payload = { event: "user.login", user: { id: "12345" } } json_payload = payload.to_json response = make_request(:post, "/webhooks/tailscale", json_payload, json_headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end it "rejects request with wrong signature algorithm" do @@ -565,7 +580,7 @@ def expired_unix_timestamp(seconds_ago = 600) wrong_signature = OpenSSL::HMAC.hexdigest(OpenSSL::Digest.new("sha1"), FAKE_ALT_HMAC_SECRET, signing_payload) headers = json_headers("Tailscale-Webhook-Signature" => "t=#{timestamp},v1=#{wrong_signature}") response = make_request(:post, "/webhooks/tailscale", json_payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end end @@ -584,7 +599,7 @@ def expired_unix_timestamp(seconds_ago = 600) payload = {}.to_json headers = { "Content-Type" => "application/json", "X-Forwarded-For" => "123.456.789.000" } response = make_request(:post, "/webhooks/ip_filtering_example", payload, headers) - expect_response(response, Net::HTTPUnauthorized, "authentication failed") + expect_json_auth_failure(response) end end From 24217a478bae5e2ce7431230434f02550f2af741 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Jun 2025 06:38:23 +0000 Subject: [PATCH 05/11] Update handler plugin docs to reflect JSON-first default behavior Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- docs/handler_plugins.md | 66 +++++++++++++++++++++++++++++++++++------ 1 file changed, 57 insertions(+), 9 deletions(-) diff --git a/docs/handler_plugins.md b/docs/handler_plugins.md index fef7e3f6..9aee8671 100644 --- a/docs/handler_plugins.md +++ b/docs/handler_plugins.md @@ -4,13 +4,17 @@ This document provides in-depth information about handler plugins and how you ca ## Writing a Handler Plugin -Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base` class. They are used to process webhook payloads and can do anything you want. They follow a very simple interface that allows you to define a `call` method that takes three parameters: `payload`, `headers`, and `config`. The `call` method should return a hash with the response data. The hash that this method returns will be sent back to the webhook sender as a JSON response. +Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base` class. They are used to process webhook payloads and can do anything you want. They follow a very simple interface that allows you to define a `call` method that takes four parameters: `payload`, `headers`, `env`, and `config`. + +**Important:** The `call` method should return a hash by default. Since the server now defaults to JSON format, any hash returned by the handler will be automatically converted to JSON with the correct `Content-Type: application/json` headers set by Grape. This ensures consistent API responses and proper JSON serialization. - `payload`: The webhook payload, which can be a Hash or a String. This is the data that the webhook sender sends to your endpoint. - `headers`: A Hash of HTTP headers that were sent with the webhook request. - `env`: A modified Rack environment that contains a lot of context about the request. This includes information about the request method, path, query parameters, and more. See [`rack_env_builder.rb`](../lib/hooks/app/rack_env_builder.rb) for the complete list of available keys. - `config`: A Hash containing the endpoint configuration. This can include any additional settings or parameters that you want to use in your handler. Most of the time, this won't be used but sometimes endpoint configs add `opts` that can be useful for the handler. +The method should return a **hash** that will be automatically serialized to JSON format with appropriate headers. The server defaults to JSON format for both input and output processing. + ```ruby # example file path: plugins/handlers/example.rb class Example < Hooks::Plugins::Handlers::Base @@ -18,12 +22,15 @@ class Example < Hooks::Plugins::Handlers::Base # # @param payload [Hash, String] webhook payload (pure JSON with string keys) # @param headers [Hash] HTTP headers (string keys, optionally normalized - default is normalized) - # @param env [Hash] A modifed Rack environment that contains a lot of context about the request + # @param env [Hash] A modified Rack environment that contains a lot of context about the request # @param config [Hash] Endpoint configuration - # @return [Hash] Response data + # @return [Hash] Response data (automatically converted to JSON) def call(payload:, headers:, env:, config:) + # Return a hash - it will be automatically converted to JSON with proper headers return { - status: "success" + status: "success", + message: "webhook processed successfully", + timestamp: Time.now.iso8601 } end end @@ -43,6 +50,30 @@ It should be noted that the `handler:` key in the endpoint configuration file sh - `MyCustomHandler` -> `my_custom_handler` - `Cool2Handler` -> `cool_2_handler` +## Default JSON Format + +By default, the Hooks server uses JSON format for both input and output processing. This means: + +- **Input**: Webhook payloads are parsed as JSON and passed to handlers as Ruby hashes +- **Output**: Handler return values (hashes) are automatically converted to JSON responses with `Content-Type: application/json` headers +- **Error Responses**: Authentication failures and handler errors return structured JSON responses + +**Best Practice**: Always return a hash from your handler's `call` method. The hash will be automatically serialized to JSON and sent to the webhook sender with proper headers. This ensures consistent API responses and proper JSON formatting. + +Example response format: +```json +{ + "status": "success", + "message": "webhook processed successfully", + "data": { + "processed_at": "2023-10-01T12:34:56Z", + "items_processed": 5 + } +} +``` + +> **Note**: The JSON format behavior can be configured using the `format` and `default_format` options in your global configuration. See the [Configuration documentation](./configuration.md) for more details. + ### `payload` Parameter The `payload` parameter can be a Hash or a String. If the payload is a String, it will be parsed as JSON. If it is a Hash, it will be passed directly to the handler. The payload can contain any data that the webhook sender wants to send. @@ -159,6 +190,8 @@ The `log.debug`, `log.info`, `log.warn`, and `log.error` methods are available i All handler plugins have access to the `error!` method, which is used to raise an error with a specific message and HTTP status code. This is useful for returning error responses to the webhook sender. +When using `error!` with the default JSON format, both hash and string responses are handled appropriately: + ```ruby class Example < Hooks::Plugins::Handlers::Base # Example webhook handler @@ -167,11 +200,12 @@ class Example < Hooks::Plugins::Handlers::Base # @param headers [Hash] HTTP headers # @param env [Hash] A modified Rack environment that contains a lot of context about the request # @param config [Hash] Endpoint configuration - # @return [Hash] Response data + # @return [Hash] Response data (automatically converted to JSON) def call(payload:, headers:, env:, config:) if payload.nil? || payload.empty? log.error("Payload is empty or nil") + # String errors are JSON-encoded with default format error!("Payload cannot be empty or nil", 400) end @@ -182,7 +216,7 @@ class Example < Hooks::Plugins::Handlers::Base end ``` -You can also use the `error!` method to return a JSON response as well: +**Recommended approach**: Use hash-based error responses for consistent JSON structure: ```ruby class Example < Hooks::Plugins::Handlers::Base @@ -190,13 +224,14 @@ class Example < Hooks::Plugins::Handlers::Base if payload.nil? || payload.empty? log.error("Payload is empty or nil") + # Hash errors are automatically converted to JSON error!({ error: "payload_empty", message: "the payload cannot be empty or nil", success: false, custom_value: "some_custom_value", request_id: env["hooks.request_id"] - }, 500) + }, 400) end return { @@ -206,6 +241,17 @@ class Example < Hooks::Plugins::Handlers::Base end ``` +This will return a properly formatted JSON error response: +```json +{ + "error": "payload_empty", + "message": "the payload cannot be empty or nil", + "success": false, + "custom_value": "some_custom_value", + "request_id": "uuid-here" +} +``` + ### `#Retryable.with_context(:default)` This method uses a default `Retryable` context to handle retries. It is used to wrap the execution of a block of code that may need to be retried in case of failure. @@ -220,7 +266,7 @@ class Example < Hooks::Plugins::Handlers::Base # @param headers [Hash] HTTP headers # @param env [Hash] A modified Rack environment that contains a lot of context about the request # @param config [Hash] Endpoint configuration - # @return [Hash] Response data + # @return [Hash] Response data (automatically converted to JSON) def call(payload:, headers:, env:, config:) result = Retryable.with_context(:default) do some_operation_that_might_fail() @@ -229,7 +275,9 @@ class Example < Hooks::Plugins::Handlers::Base log.debug("operation result: #{result}") return { - status: "success" + status: "success", + operation_result: result, + processed_at: Time.now.iso8601 } end end From 530f5fd0a6e09859505ebb3585859312bb40f840 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 18 Jun 2025 09:54:48 -0700 Subject: [PATCH 06/11] fix bundle --- .bundle/config | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/.bundle/config b/.bundle/config index f9263841..0146a1ce 100644 --- a/.bundle/config +++ b/.bundle/config @@ -1,8 +1,7 @@ --- BUNDLE_BIN: "bin" -BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle" +BUNDLE_PATH: "vendor/gems" BUNDLE_CACHE_PATH: "vendor/cache" BUNDLE_CACHE_ALL: "true" BUNDLE_SPECIFIC_PLATFORM: "true" BUNDLE_NO_INSTALL: "true" -BUNDLE_DEPLOYMENT: "true" From 98cf31a4ab62c744b9db6e1b87b59185317a5f60 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 18 Jun 2025 09:55:31 -0700 Subject: [PATCH 07/11] fmt --- docs/handler_plugins.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/docs/handler_plugins.md b/docs/handler_plugins.md index 9aee8671..ba521f77 100644 --- a/docs/handler_plugins.md +++ b/docs/handler_plugins.md @@ -4,7 +4,7 @@ This document provides in-depth information about handler plugins and how you ca ## Writing a Handler Plugin -Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base` class. They are used to process webhook payloads and can do anything you want. They follow a very simple interface that allows you to define a `call` method that takes four parameters: `payload`, `headers`, `env`, and `config`. +Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base` class. They are used to process webhook payloads and can do anything you want. They follow a very simple interface that allows you to define a `call` method that takes four parameters: `payload`, `headers`, `env`, and `config`. **Important:** The `call` method should return a hash by default. Since the server now defaults to JSON format, any hash returned by the handler will be automatically converted to JSON with the correct `Content-Type: application/json` headers set by Grape. This ensures consistent API responses and proper JSON serialization. @@ -61,6 +61,7 @@ By default, the Hooks server uses JSON format for both input and output processi **Best Practice**: Always return a hash from your handler's `call` method. The hash will be automatically serialized to JSON and sent to the webhook sender with proper headers. This ensures consistent API responses and proper JSON formatting. Example response format: + ```json { "status": "success", @@ -242,6 +243,7 @@ end ``` This will return a properly formatted JSON error response: + ```json { "error": "payload_empty", From 39d571759080996cfad851b007989799d76f4c80 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 18 Jun 2025 10:12:33 -0700 Subject: [PATCH 08/11] bump --- Gemfile.lock | 2 +- lib/hooks/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 5461111b..0de8f5fe 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - hooks-ruby (0.3.2) + hooks-ruby (0.4.0) dry-schema (~> 1.14, >= 1.14.1) grape (~> 2.3) puma (~> 6.6) diff --git a/lib/hooks/version.rb b/lib/hooks/version.rb index 7dba2953..0420f040 100644 --- a/lib/hooks/version.rb +++ b/lib/hooks/version.rb @@ -4,5 +4,5 @@ module Hooks # Current version of the Hooks webhook framework # @return [String] The version string following semantic versioning - VERSION = "0.3.2".freeze + VERSION = "0.4.0".freeze end From cc56829cd62393c80fab04bb5a4774f64949a6d2 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 18 Jun 2025 10:43:22 -0700 Subject: [PATCH 09/11] Change error response content type to text/plain for string errors --- lib/hooks/app/api.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 756e292c..a1647c92 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -126,8 +126,9 @@ def self.create(config:, endpoints:, log:) status e.status case e.body when String - # Even string errors are now JSON-encoded with the default JSON format - content_type "application/json" + # if error! was called with a string, we assume it's a simple text error + # example: error!("simple text error", 400) -> should return a plain text response + content_type "text/plain" error_response = e.body else # Let Grape handle JSON conversion with the default format From cbce221b1e755ffb27e326cef19ae70409bdf7c7 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 18 Jun 2025 10:43:30 -0700 Subject: [PATCH 10/11] Remove `format` configuration option and update related documentation --- docs/configuration.md | 10 +--------- docs/handler_plugins.md | 2 +- lib/hooks/app/api.rb | 1 - lib/hooks/app/endpoints/catchall.rb | 1 - lib/hooks/app/endpoints/health.rb | 1 - lib/hooks/app/endpoints/version.rb | 1 - lib/hooks/core/config_loader.rb | 4 +--- lib/hooks/core/config_validator.rb | 1 + spec/acceptance/acceptance_tests.rb | 7 ++----- spec/unit/lib/hooks/core/config_loader_spec.rb | 3 --- 10 files changed, 6 insertions(+), 25 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index b972b379..fc17d9c0 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -105,17 +105,9 @@ When set to `true`, normalizes incoming HTTP headers by lowercasing and trimming **Default:** `true` -### `format` - -Sets the request/response format for the webhook server. This determines how incoming requests are parsed and how responses are formatted. - -**Default:** `json` -**Valid values:** `json`, `txt`, `xml`, `any` -**Example:** `json` - ### `default_format` -Sets the default response format when no specific format is requested. This works in conjunction with the `format` setting to determine response formatting. +Sets the default response format when no specific format is requested. **Default:** `json` **Valid values:** `json`, `txt`, `xml`, `any` diff --git a/docs/handler_plugins.md b/docs/handler_plugins.md index ba521f77..01457475 100644 --- a/docs/handler_plugins.md +++ b/docs/handler_plugins.md @@ -73,7 +73,7 @@ Example response format: } ``` -> **Note**: The JSON format behavior can be configured using the `format` and `default_format` options in your global configuration. See the [Configuration documentation](./configuration.md) for more details. +> **Note**: The JSON format behavior can be configured using the `default_format` option in your global configuration. See the [Configuration documentation](./configuration.md) for more details. ### `payload` Parameter diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index a1647c92..5ee0c0de 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -39,7 +39,6 @@ def self.create(config:, endpoints:, log:) content_type :xml, "application/xml" content_type :any, "*/*" - format config[:format] || :json default_format config[:default_format] || :json end diff --git a/lib/hooks/app/endpoints/catchall.rb b/lib/hooks/app/endpoints/catchall.rb index b1cb2a4c..897c24d1 100644 --- a/lib/hooks/app/endpoints/catchall.rb +++ b/lib/hooks/app/endpoints/catchall.rb @@ -22,7 +22,6 @@ class CatchallEndpoint < Grape::API content_type :txt, "text/plain" content_type :xml, "application/xml" content_type :any, "*/*" - format :json default_format :json def self.mount_path(config) diff --git a/lib/hooks/app/endpoints/health.rb b/lib/hooks/app/endpoints/health.rb index 642c56d3..468bdc70 100644 --- a/lib/hooks/app/endpoints/health.rb +++ b/lib/hooks/app/endpoints/health.rb @@ -11,7 +11,6 @@ class HealthEndpoint < Grape::API content_type :txt, "text/plain" content_type :xml, "application/xml" content_type :any, "*/*" - format :json default_format :json get do diff --git a/lib/hooks/app/endpoints/version.rb b/lib/hooks/app/endpoints/version.rb index 7b7dd431..414c1d8a 100644 --- a/lib/hooks/app/endpoints/version.rb +++ b/lib/hooks/app/endpoints/version.rb @@ -11,7 +11,6 @@ class VersionEndpoint < Grape::API content_type :txt, "text/plain" content_type :xml, "application/xml" content_type :any, "*/*" - format :json default_format :json get do diff --git a/lib/hooks/core/config_loader.rb b/lib/hooks/core/config_loader.rb index d77f3278..3125b081 100644 --- a/lib/hooks/core/config_loader.rb +++ b/lib/hooks/core/config_loader.rb @@ -21,7 +21,6 @@ class ConfigLoader endpoints_dir: "./config/endpoints", use_catchall_route: false, normalize_headers: true, - format: :json, default_format: :json }.freeze @@ -143,7 +142,6 @@ def self.load_env_config "HOOKS_ENDPOINTS_DIR" => :endpoints_dir, "HOOKS_USE_CATCHALL_ROUTE" => :use_catchall_route, "HOOKS_NORMALIZE_HEADERS" => :normalize_headers, - "HOOKS_FORMAT" => :format, "HOOKS_DEFAULT_FORMAT" => :default_format, "HOOKS_SOME_STRING_VAR" => :some_string_var # Added for test } @@ -159,7 +157,7 @@ def self.load_env_config when :use_catchall_route, :normalize_headers # Convert string to boolean env_config[config_key] = %w[true 1 yes on].include?(value.downcase) - when :format, :default_format + when :default_format # Convert string to symbol env_config[config_key] = value.to_sym else diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index c53fe651..0e7807b9 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -27,6 +27,7 @@ class ValidationError < StandardError; end optional(:endpoints_dir).filled(:string) optional(:use_catchall_route).filled(:bool) optional(:normalize_headers).filled(:bool) + optional(:default_format).filled(:symbol, included_in?: %i[json txt xml any]) optional(:ip_filtering).hash do optional(:ip_header).filled(:string) diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index c3a411fa..c3e7e3f7 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -532,13 +532,10 @@ def expired_unix_timestamp(seconds_ago = 600) response = make_request(:post, "/webhooks/boomtown_with_error", payload, json_headers) expect_response(response, Net::HTTPInternalServerError) - # With JSON default format, even string errors are JSON-encoded - expect(response.content_type).to eq("application/json") + expect(response.content_type).to eq("text/plain") expect(response.code).to eq("500") - # The error body should be the JSON-encoded string - body = parse_json_response(response) - expect(body).to eq("boomtown_with_error: the payload triggered a simple text boomtown error") + expect(response.body).to match(/boomtown_with_error: the payload triggered a simple text boomtown error/) end end diff --git a/spec/unit/lib/hooks/core/config_loader_spec.rb b/spec/unit/lib/hooks/core/config_loader_spec.rb index ca583faa..95c2072f 100644 --- a/spec/unit/lib/hooks/core/config_loader_spec.rb +++ b/spec/unit/lib/hooks/core/config_loader_spec.rb @@ -20,7 +20,6 @@ endpoints_dir: "./config/endpoints", use_catchall_route: false, normalize_headers: true, - format: :json, default_format: :json ) end @@ -201,12 +200,10 @@ end it "converts format environment variables to symbols" do - ENV["HOOKS_FORMAT"] = "txt" ENV["HOOKS_DEFAULT_FORMAT"] = "xml" config = described_class.load - expect(config[:format]).to eq(:txt) expect(config[:default_format]).to eq(:xml) end end From e7b67a5296ea5f875854339e695cf989611a12da Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 18 Jun 2025 10:44:06 -0700 Subject: [PATCH 11/11] Add default_format configuration option for response format --- spec/acceptance/config/hooks.yaml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/spec/acceptance/config/hooks.yaml b/spec/acceptance/config/hooks.yaml index 474691d9..831614e9 100644 --- a/spec/acceptance/config/hooks.yaml +++ b/spec/acceptance/config/hooks.yaml @@ -6,6 +6,8 @@ instruments_plugin_dir: ./spec/acceptance/plugins/instruments log_level: debug +default_format: json # default response format for the server + # Request handling request_limit: 1048576 # 1MB max body size request_timeout: 15 # 15 seconds timeout