From 7d3ab47b9f83fdadcf3cb1c29fc2ab929f40a879 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Jun 2025 21:50:28 +0000 Subject: [PATCH 01/12] Initial plan for issue From e6141337cef24c9972b52c125bf585706f5ba680 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Jun 2025 21:53:30 +0000 Subject: [PATCH 02/12] Initial exploration and planning for IP filtering implementation 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 09263598bab0fcdc4572c12c51c70f4e84764c71 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 17 Jun 2025 22:13:14 +0000 Subject: [PATCH 03/12] Implement application-level IP filtering with allowlist/blocklist support Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- lib/hooks/app/api.rb | 10 +- lib/hooks/app/helpers.rb | 23 ++ lib/hooks/app/network/ip_filtering.rb | 112 +++++ lib/hooks/core/config_validator.rb | 12 + spec/acceptance/acceptance_tests.rb | 81 ++++ .../endpoints/ip_filtering_custom_header.yml | 7 + .../config/endpoints/ip_filtering_direct.yml | 9 + spec/unit/app/network/ip_filtering_spec.rb | 390 ++++++++++++++++++ 8 files changed, 639 insertions(+), 5 deletions(-) create mode 100644 lib/hooks/app/network/ip_filtering.rb create mode 100644 spec/acceptance/config/endpoints/ip_filtering_custom_header.yml create mode 100644 spec/acceptance/config/endpoints/ip_filtering_direct.yml create mode 100644 spec/unit/app/network/ip_filtering_spec.rb diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 0e3ea7b3..912fff78 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -4,7 +4,7 @@ require "json" require "securerandom" require_relative "helpers" -#require_relative "network/ip_filtering" +require_relative "network/ip_filtering" require_relative "auth/auth" require_relative "rack_env_builder" require_relative "../plugins/handlers/base" @@ -83,12 +83,12 @@ def self.create(config:, endpoints:, log:) plugin.on_request(rack_env) end - # TODO: IP filtering before processing the request if defined + # IP filtering before processing the request if defined # If IP filtering is enabled at either global or endpoint level, run the filtering rules # before processing the request - #if config[:ip_filtering] || endpoint_config[:ip_filtering] - #ip_filtering!(headers, endpoint_config, config, request_context, rack_env) - #end + if config[:ip_filtering] || endpoint_config[:ip_filtering] + ip_filtering!(headers, endpoint_config, config, request_context, rack_env) + end enforce_request_limits(config, request_context) request.body.rewind diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index 0b32ca5d..b9502b18 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -3,6 +3,7 @@ require "securerandom" require_relative "../security" require_relative "../core/plugin_loader" +require_relative "network/ip_filtering" module Hooks module App @@ -88,6 +89,28 @@ def load_handler(handler_class_name) return handler_class.new end + # Verifies the incoming request passes the configured IP filtering rules. + # + # This method assumes that the client IP address is available in the request headers (e.g., `X-Forwarded-For`). + # The headers that is used is configurable via the endpoint configuration. + # It checks the IP address against the allowed and denied lists defined in the endpoint configuration. + # If the IP address is not allowed, it instantly returns an error response via the `error!` method. + # If the IP filtering configuration is missing or invalid, it raises an error. + # If IP filtering is configured at the global level, it will also check against the global configuration first, + # and then against the endpoint-specific configuration. + # + # @param headers [Hash] The request headers. + # @param endpoint_config [Hash] The endpoint configuration, must include :ip_filtering key. + # @param global_config [Hash] The global configuration (optional, for compatibility). + # @param request_context [Hash] Context for the request, e.g. request ID, path, handler (optional). + # @param env [Hash] The Rack environment + # @raise [StandardError] Raises error if IP filtering fails or is misconfigured. + # @return [void] + # @note This method will halt execution with an error if IP filtering rules fail. + def ip_filtering!(headers, endpoint_config, global_config, request_context, env) + Network::IpFiltering.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + end + private # Safely parse JSON diff --git a/lib/hooks/app/network/ip_filtering.rb b/lib/hooks/app/network/ip_filtering.rb new file mode 100644 index 00000000..5c1063a9 --- /dev/null +++ b/lib/hooks/app/network/ip_filtering.rb @@ -0,0 +1,112 @@ +# frozen_string_literal: true + +require "ipaddr" +require_relative "../../plugins/handlers/error" + +module Hooks + module App + module Network + # Application-level IP filtering functionality + # Provides both allowlist and blocklist filtering with CIDR support + class IpFiltering + # Default IP header to check for client IP + DEFAULT_IP_HEADER = "X-Forwarded-For" + + # Verifies the incoming request passes the configured IP filtering rules. + # + # This method assumes that the client IP address is available in the request headers (e.g., `X-Forwarded-For`). + # The headers that is used is configurable via the endpoint configuration. + # It checks the IP address against the allowed and denied lists defined in the endpoint configuration. + # If the IP address is not allowed, it instantly returns an error response via the `error!` method. + # If the IP filtering configuration is missing or invalid, it raises an error. + # If IP filtering is configured at the global level, it will also check against the global configuration first, + # and then against the endpoint-specific configuration. + # + # @param headers [Hash] The request headers. + # @param endpoint_config [Hash] The endpoint configuration, must include :ip_filtering key. + # @param global_config [Hash] The global configuration (optional, for compatibility). + # @param request_context [Hash] Context for the request, e.g. request ID, path, handler (optional). + # @param env [Hash] The Rack environment + # @raise [StandardError] Raises error if IP filtering fails or is misconfigured. + # @return [void] + # @note This method will halt execution with an error if IP filtering rules fail. + def self.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + # Determine which IP filtering configuration to use + ip_config = resolve_ip_config(endpoint_config, global_config) + return unless ip_config # No IP filtering configured + + # Extract client IP from headers + client_ip = extract_client_ip(headers, ip_config) + return unless client_ip # No client IP found + + # Validate IP against filtering rules + unless ip_allowed?(client_ip, ip_config) + request_id = request_context&.dig(:request_id) || request_context&.dig("request_id") + error_msg = { + error: "ip_filtering_failed", + message: "IP address not allowed", + request_id: request_id + } + raise Hooks::Plugins::Handlers::Error.new(error_msg, 403) + end + end + + private_class_method def self.resolve_ip_config(endpoint_config, global_config) + # Endpoint-level configuration takes precedence over global configuration + endpoint_config[:ip_filtering] || global_config[:ip_filtering] + end + + private_class_method def self.extract_client_ip(headers, ip_config) + # Use configured header or default to X-Forwarded-For + ip_header = ip_config[:ip_header] || DEFAULT_IP_HEADER + + # Case-insensitive header lookup + headers.each do |key, value| + if key.to_s.downcase == ip_header.downcase + # X-Forwarded-For can contain multiple IPs, take the first one (original client) + client_ip = value.to_s.split(",").first&.strip + return client_ip unless client_ip.nil? || client_ip.empty? + end + end + + nil + end + + private_class_method def self.ip_allowed?(client_ip, ip_config) + # Parse client IP + begin + client_addr = IPAddr.new(client_ip) + rescue IPAddr::InvalidAddressError + return false # Invalid IP format + end + + # Check blocklist first (if IP is blocked, deny immediately) + if ip_config[:blocklist]&.any? + return false if ip_matches_list?(client_addr, ip_config[:blocklist]) + end + + # Check allowlist (if defined, IP must be in allowlist) + if ip_config[:allowlist]&.any? + return ip_matches_list?(client_addr, ip_config[:allowlist]) + end + + # If no allowlist is defined and IP is not in blocklist, allow + true + end + + private_class_method def self.ip_matches_list?(client_addr, ip_list) + ip_list.each do |ip_pattern| + begin + pattern_addr = IPAddr.new(ip_pattern.to_s) + return true if pattern_addr.include?(client_addr) + rescue IPAddr::InvalidAddressError + # Skip invalid IP patterns + next + end + end + false + end + end + end + end +end \ No newline at end of file diff --git a/lib/hooks/core/config_validator.rb b/lib/hooks/core/config_validator.rb index e680d9d3..c53fe651 100644 --- a/lib/hooks/core/config_validator.rb +++ b/lib/hooks/core/config_validator.rb @@ -27,6 +27,12 @@ class ValidationError < StandardError; end optional(:endpoints_dir).filled(:string) optional(:use_catchall_route).filled(:bool) optional(:normalize_headers).filled(:bool) + + optional(:ip_filtering).hash do + optional(:ip_header).filled(:string) + optional(:allowlist).array(:string) + optional(:blocklist).array(:string) + end end # Endpoint configuration schema @@ -52,6 +58,12 @@ class ValidationError < StandardError; end optional(:key_value_separator).filled(:string) end + optional(:ip_filtering).hash do + optional(:ip_header).filled(:string) + optional(:allowlist).array(:string) + optional(:blocklist).array(:string) + end + optional(:opts).hash end diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 7bd4dc38..1f2d0100 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -587,5 +587,86 @@ def expired_unix_timestamp(seconds_ago = 600) expect_response(response, Net::HTTPUnauthorized, "authentication failed") end end + + describe "application-level IP filtering" do + it "allows requests from IPs in allowlist" do + payload = {}.to_json + headers = { "Content-Type" => "application/json", "X-Forwarded-For" => "127.0.0.1" } + response = make_request(:post, "/webhooks/ip_filtering_direct", payload, headers) + + expect_response(response, Net::HTTPSuccess) + body = parse_json_response(response) + expect(body["status"]).to eq("success") + end + + it "allows requests from IPs in CIDR range" do + payload = {}.to_json + headers = { "Content-Type" => "application/json", "X-Forwarded-For" => "192.168.1.50" } + response = make_request(:post, "/webhooks/ip_filtering_direct", payload, headers) + + expect_response(response, Net::HTTPSuccess) + body = parse_json_response(response) + expect(body["status"]).to eq("success") + end + + it "blocks requests from IPs in blocklist even if in allowlist" do + payload = {}.to_json + headers = { "Content-Type" => "application/json", "X-Forwarded-For" => "192.168.1.100" } + response = make_request(:post, "/webhooks/ip_filtering_direct", payload, headers) + + expect_response(response, Net::HTTPForbidden) + body = parse_json_response(response) + expect(body["error"]).to eq("ip_filtering_failed") + expect(body["message"]).to eq("IP address not allowed") + end + + it "blocks requests from IPs not in allowlist" do + payload = {}.to_json + headers = { "Content-Type" => "application/json", "X-Forwarded-For" => "203.0.113.1" } + response = make_request(:post, "/webhooks/ip_filtering_direct", payload, headers) + + expect_response(response, Net::HTTPForbidden) + body = parse_json_response(response) + expect(body["error"]).to eq("ip_filtering_failed") + end + + it "uses custom IP header when configured" do + payload = {}.to_json + headers = { + "Content-Type" => "application/json", + "X-Real-IP" => "10.0.0.1", + "X-Forwarded-For" => "203.0.113.1" + } + response = make_request(:post, "/webhooks/ip_filtering_custom_header", payload, headers) + + expect_response(response, Net::HTTPSuccess) + body = parse_json_response(response) + expect(body["status"]).to eq("success") + end + + it "blocks requests when custom IP header has disallowed IP" do + payload = {}.to_json + headers = { + "Content-Type" => "application/json", + "X-Real-IP" => "203.0.113.1", + "X-Forwarded-For" => "10.0.0.1" + } + response = make_request(:post, "/webhooks/ip_filtering_custom_header", payload, headers) + + expect_response(response, Net::HTTPForbidden) + body = parse_json_response(response) + expect(body["error"]).to eq("ip_filtering_failed") + end + + it "allows requests when no IP filtering is configured" do + payload = {}.to_json + headers = { "Content-Type" => "application/json", "X-Forwarded-For" => "203.0.113.1" } + response = make_request(:post, "/webhooks/hello", payload, headers) + + expect_response(response, Net::HTTPSuccess) + body = parse_json_response(response) + expect(body["status"]).to eq("success") + end + end end end diff --git a/spec/acceptance/config/endpoints/ip_filtering_custom_header.yml b/spec/acceptance/config/endpoints/ip_filtering_custom_header.yml new file mode 100644 index 00000000..e3fbfa13 --- /dev/null +++ b/spec/acceptance/config/endpoints/ip_filtering_custom_header.yml @@ -0,0 +1,7 @@ +path: /ip_filtering_custom_header +handler: hello + +ip_filtering: + ip_header: X-Real-IP + allowlist: + - "10.0.0.0/8" \ No newline at end of file diff --git a/spec/acceptance/config/endpoints/ip_filtering_direct.yml b/spec/acceptance/config/endpoints/ip_filtering_direct.yml new file mode 100644 index 00000000..1618f59e --- /dev/null +++ b/spec/acceptance/config/endpoints/ip_filtering_direct.yml @@ -0,0 +1,9 @@ +path: /ip_filtering_direct +handler: hello + +ip_filtering: + allowlist: + - "127.0.0.1" + - "192.168.1.0/24" + blocklist: + - "192.168.1.100" \ No newline at end of file diff --git a/spec/unit/app/network/ip_filtering_spec.rb b/spec/unit/app/network/ip_filtering_spec.rb new file mode 100644 index 00000000..be3e9fca --- /dev/null +++ b/spec/unit/app/network/ip_filtering_spec.rb @@ -0,0 +1,390 @@ +# frozen_string_literal: true + +require_relative "../../../unit/spec_helper" +require_relative "../../../../lib/hooks/app/network/ip_filtering" + +RSpec.describe Hooks::App::Network::IpFiltering do + describe ".ip_filtering!" do + let(:headers) { { "X-Forwarded-For" => "192.168.1.100" } } + let(:endpoint_config) { {} } + let(:global_config) { {} } + let(:request_context) { { request_id: "test-123" } } + let(:env) { {} } + + context "when no IP filtering is configured" do + it "does not raise an error" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with allowlist configuration" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100", "10.0.0.0/8"] + } + } + end + + context "when client IP is in allowlist" do + it "does not raise an error" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "when client IP is in CIDR range" do + let(:headers) { { "X-Forwarded-For" => "10.1.2.3" } } + + it "does not raise an error" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "when client IP is not in allowlist" do + let(:headers) { { "X-Forwarded-For" => "203.0.113.1" } } + + it "raises an error with 403 status" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.to raise_error(Hooks::Plugins::Handlers::Error) do |error| + expect(error.status).to eq(403) + expect(error.body[:error]).to eq("ip_filtering_failed") + expect(error.body[:message]).to eq("IP address not allowed") + expect(error.body[:request_id]).to eq("test-123") + end + end + end + end + + context "with blocklist configuration" do + let(:endpoint_config) do + { + ip_filtering: { + blocklist: ["192.168.1.100", "203.0.113.0/24"] + } + } + end + + context "when client IP is not in blocklist" do + let(:headers) { { "X-Forwarded-For" => "10.0.0.1" } } + + it "does not raise an error" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "when client IP is in blocklist" do + it "raises an error with 403 status" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.to raise_error(Hooks::Plugins::Handlers::Error) do |error| + expect(error.status).to eq(403) + expect(error.body[:error]).to eq("ip_filtering_failed") + end + end + end + + context "when client IP is in blocklist CIDR range" do + let(:headers) { { "X-Forwarded-For" => "203.0.113.50" } } + + it "raises an error with 403 status" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.to raise_error(Hooks::Plugins::Handlers::Error) do |error| + expect(error.status).to eq(403) + end + end + end + end + + context "with both allowlist and blocklist" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.0/24"], + blocklist: ["192.168.1.100"] + } + } + end + + context "when IP is in allowlist but also in blocklist" do + it "raises an error (blocklist takes precedence)" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.to raise_error(Hooks::Plugins::Handlers::Error) do |error| + expect(error.status).to eq(403) + end + end + end + + context "when IP is in allowlist and not in blocklist" do + let(:headers) { { "X-Forwarded-For" => "192.168.1.50" } } + + it "does not raise an error" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + end + + context "with custom IP header" do + let(:endpoint_config) do + { + ip_filtering: { + ip_header: "X-Real-IP", + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "X-Real-IP" => "192.168.1.100", "X-Forwarded-For" => "203.0.113.1" } } + + it "uses the custom header instead of X-Forwarded-For" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with case-insensitive headers" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "x-forwarded-for" => "192.168.1.100" } } + + it "finds the header regardless of case" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with multiple IPs in X-Forwarded-For" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "X-Forwarded-For" => "192.168.1.100, 10.0.0.1, 203.0.113.1" } } + + it "uses the first IP (original client)" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with global configuration" do + let(:global_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + + it "uses global configuration when endpoint config is not set" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with endpoint configuration overriding global" do + let(:global_config) do + { + ip_filtering: { + allowlist: ["10.0.0.1"] + } + } + end + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + + it "uses endpoint configuration over global" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with invalid IP addresses" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "X-Forwarded-For" => "invalid-ip" } } + + it "raises an error for invalid client IP" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.to raise_error(Hooks::Plugins::Handlers::Error) do |error| + expect(error.status).to eq(403) + end + end + end + + context "when no IP header is present" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { {} } + + it "does not raise an error (no filtering applied)" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "when custom IP header is not found" do + let(:endpoint_config) do + { + ip_filtering: { + ip_header: "X-Real-IP", + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "X-Forwarded-For" => "192.168.1.100" } } + + it "does not raise an error (no filtering applied)" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "when IP header has empty value" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "X-Forwarded-For" => "" } } + + it "does not raise an error (no filtering applied)" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "when IP header has whitespace only value" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "X-Forwarded-For" => " " } } + + it "does not raise an error (no filtering applied)" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with invalid IP patterns in configuration" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100", "invalid-pattern", "10.0.0.0/8"] + } + } + end + let(:headers) { { "X-Forwarded-For" => "10.0.0.1" } } + + it "skips invalid patterns and processes valid ones" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "with string keys in request_context" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:request_context) { { "request_id" => "test-string-key" } } + let(:headers) { { "X-Forwarded-For" => "203.0.113.1" } } + + it "raises an error and extracts request_id correctly" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.to raise_error(Hooks::Plugins::Handlers::Error) do |error| + expect(error.status).to eq(403) + expect(error.body[:request_id]).to eq("test-string-key") + end + end + end + + context "with blocklist only configuration" do + let(:endpoint_config) do + { + ip_filtering: { + blocklist: ["203.0.113.0/24"] + } + } + end + let(:headers) { { "X-Forwarded-For" => "192.168.1.100" } } + + it "allows IPs not in blocklist when no allowlist is defined" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + + context "when IP header has nil value after split" do + let(:endpoint_config) do + { + ip_filtering: { + allowlist: ["192.168.1.100"] + } + } + end + let(:headers) { { "X-Forwarded-For" => "," } } + + it "does not raise an error (no filtering applied)" do + expect { + described_class.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + }.not_to raise_error + end + end + end +end \ No newline at end of file From 8913aed6c59f6e4883260fd2421a33f92dc012cf Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 18:32:43 -0700 Subject: [PATCH 04/12] lint --- lib/hooks/app/network/ip_filtering.rb | 6 +++--- spec/acceptance/acceptance_tests.rb | 8 ++++---- spec/unit/app/network/ip_filtering_spec.rb | 2 +- 3 files changed, 8 insertions(+), 8 deletions(-) diff --git a/lib/hooks/app/network/ip_filtering.rb b/lib/hooks/app/network/ip_filtering.rb index 5c1063a9..455ccd66 100644 --- a/lib/hooks/app/network/ip_filtering.rb +++ b/lib/hooks/app/network/ip_filtering.rb @@ -59,7 +59,7 @@ def self.ip_filtering!(headers, endpoint_config, global_config, request_context, private_class_method def self.extract_client_ip(headers, ip_config) # Use configured header or default to X-Forwarded-For ip_header = ip_config[:ip_header] || DEFAULT_IP_HEADER - + # Case-insensitive header lookup headers.each do |key, value| if key.to_s.downcase == ip_header.downcase @@ -68,7 +68,7 @@ def self.ip_filtering!(headers, endpoint_config, global_config, request_context, return client_ip unless client_ip.nil? || client_ip.empty? end end - + nil end @@ -109,4 +109,4 @@ def self.ip_filtering!(headers, endpoint_config, global_config, request_context, end end end -end \ No newline at end of file +end diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 1f2d0100..00aa4276 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -632,8 +632,8 @@ def expired_unix_timestamp(seconds_ago = 600) it "uses custom IP header when configured" do payload = {}.to_json - headers = { - "Content-Type" => "application/json", + headers = { + "Content-Type" => "application/json", "X-Real-IP" => "10.0.0.1", "X-Forwarded-For" => "203.0.113.1" } @@ -646,8 +646,8 @@ def expired_unix_timestamp(seconds_ago = 600) it "blocks requests when custom IP header has disallowed IP" do payload = {}.to_json - headers = { - "Content-Type" => "application/json", + headers = { + "Content-Type" => "application/json", "X-Real-IP" => "203.0.113.1", "X-Forwarded-For" => "10.0.0.1" } diff --git a/spec/unit/app/network/ip_filtering_spec.rb b/spec/unit/app/network/ip_filtering_spec.rb index be3e9fca..f89ee3f6 100644 --- a/spec/unit/app/network/ip_filtering_spec.rb +++ b/spec/unit/app/network/ip_filtering_spec.rb @@ -387,4 +387,4 @@ end end end -end \ No newline at end of file +end From 37d86afb8a7e89f4d65fe32a9d9b5642302a781c Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 18:36:09 -0700 Subject: [PATCH 05/12] docs --- docs/auth_plugins.md | 2 ++ docs/ip_filtering.md | 3 +++ 2 files changed, 5 insertions(+) create mode 100644 docs/ip_filtering.md diff --git a/docs/auth_plugins.md b/docs/auth_plugins.md index f821b904..7087fdfc 100644 --- a/docs/auth_plugins.md +++ b/docs/auth_plugins.md @@ -465,3 +465,5 @@ opts: - "" - "" ``` + +To use the built-in IP filtering feature (rather than trying to implement your own like this example above), check out the [IP Filtering documentation](ip_filtering.md). diff --git a/docs/ip_filtering.md b/docs/ip_filtering.md new file mode 100644 index 00000000..38a0be8f --- /dev/null +++ b/docs/ip_filtering.md @@ -0,0 +1,3 @@ +# IP Filtering + +TODO From 05a6dc8a41c66ec14bc25f2624c899661e3096c8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Wed, 18 Jun 2025 01:44:17 +0000 Subject: [PATCH 06/12] Fix test coverage to 100% and add comprehensive IP filtering documentation Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- docs/ip_filtering.md | 297 +++++++++++++++++++++++- spec/unit/lib/hooks/app/helpers_spec.rb | 20 ++ 2 files changed, 316 insertions(+), 1 deletion(-) diff --git a/docs/ip_filtering.md b/docs/ip_filtering.md index 38a0be8f..159b9a99 100644 --- a/docs/ip_filtering.md +++ b/docs/ip_filtering.md @@ -1,3 +1,298 @@ # IP Filtering -TODO +The Hooks service provides comprehensive application-level IP filtering functionality that allows you to control access to your webhooks based on client IP addresses. This feature supports both allowlist (whitelist) and blocklist (blacklist) configurations with CIDR notation support. + +## Overview + +IP filtering operates as a "pre-flight" check in the request processing pipeline, validating incoming requests before they reach your webhook handlers. The filtering can be configured both globally (for all endpoints) and at the individual endpoint level. + +## ⚠️ Security Considerations + +**Important**: This IP filtering operates at the application layer and relies on HTTP headers (like `X-Forwarded-For`) to determine client IP addresses. This approach has important security implications: + +1. **Header Trust**: The service trusts proxy headers, which can be spoofed by malicious clients +2. **Network-Level Protection**: For production security, consider implementing IP filtering at the network or load balancer level +3. **Proper Proxy Configuration**: Ensure your reverse proxy/load balancer is properly configured to set accurate IP headers +4. **Defense in Depth**: Use this feature as part of a broader security strategy, not as the sole protection mechanism + +## Configuration + +### Global Configuration + +Configure IP filtering globally to apply rules to all endpoints: + +```yaml +# hooks.yml or your main configuration file +ip_filtering: + ip_header: X-Forwarded-For # Optional, defaults to X-Forwarded-For + allowlist: + - "10.0.0.0/8" # Allow entire private network + - "172.16.0.0/12" # Allow another private range + - "192.168.1.100" # Allow specific IP + blocklist: + - "192.168.1.200" # Block specific IP even if in allowlist + - "203.0.113.0/24" # Block entire subnet +``` + +### Endpoint-Level Configuration + +Configure IP filtering for specific endpoints: + +```yaml +# config/endpoints/secure-endpoint.yml +path: /secure-webhook +handler: my_secure_handler + +ip_filtering: + ip_header: X-Real-IP # Optional, defaults to X-Forwarded-For + allowlist: + - "127.0.0.1" # Allow localhost + - "192.168.1.0/24" # Allow local network + blocklist: + - "192.168.1.100" # Block specific IP in the allowed range +``` + +## Configuration Options + +### `ip_header` (optional) +- **Default**: `X-Forwarded-For` +- **Description**: HTTP header to check for the client IP address +- **Common alternatives**: `X-Real-IP`, `CF-Connecting-IP`, `X-Client-IP` + +### `allowlist` (optional) +- **Type**: Array of strings +- **Description**: List of allowed IP addresses or CIDR ranges +- **Behavior**: If specified, only IPs in this list are allowed access +- **Format**: Individual IPs (`192.168.1.1`) or CIDR notation (`192.168.1.0/24`) + +### `blocklist` (optional) +- **Type**: Array of strings +- **Description**: List of blocked IP addresses or CIDR ranges +- **Behavior**: IPs in this list are denied access, even if they appear in the allowlist +- **Format**: Individual IPs (`192.168.1.1`) or CIDR notation (`192.168.1.0/24`) + +## Filtering Logic + +The IP filtering follows this precedence order: + +1. **Extract Client IP**: Get the client IP from the configured header (case-insensitive lookup) +2. **Check Blocklist**: If the IP matches any entry in the blocklist, deny immediately +3. **Check Allowlist**: If an allowlist is configured, the IP must match an entry to be allowed +4. **Default Allow**: If no allowlist is configured and IP is not blocked, allow the request + +### Precedence Rules + +- **Endpoint-level configuration** takes precedence over global configuration +- **Blocklist rules** take precedence over allowlist rules +- **First IP in comma-separated list** is used (e.g., in `X-Forwarded-For: 192.168.1.1, 10.0.0.1`, only `192.168.1.1` is checked) + +## CIDR Notation Support + +The service supports CIDR (Classless Inter-Domain Routing) notation for specifying IP ranges: + +```yaml +ip_filtering: + allowlist: + - "192.168.1.0/24" # Allows 192.168.1.1 through 192.168.1.254 + - "10.0.0.0/8" # Allows 10.0.0.1 through 10.255.255.254 + - "172.16.0.0/12" # Allows 172.16.0.1 through 172.31.255.254 + blocklist: + - "192.168.1.100/32" # Blocks specific IP (equivalent to 192.168.1.100) + - "203.0.113.0/24" # Blocks entire test network range +``` + +## Examples + +### Example 1: Basic Allowlist + +```yaml +# Allow only specific IPs +path: /secure-webhook +handler: secure_handler + +ip_filtering: + allowlist: + - "127.0.0.1" + - "192.168.1.50" +``` + +### Example 2: CIDR Range with Exceptions + +```yaml +# Allow local network but block specific troublemaker +path: /internal-webhook +handler: internal_handler + +ip_filtering: + allowlist: + - "192.168.1.0/24" + blocklist: + - "192.168.1.100" # Block this specific IP +``` + +### Example 3: Custom IP Header + +```yaml +# Use Cloudflare's connecting IP header +path: /cloudflare-webhook +handler: cf_handler + +ip_filtering: + ip_header: CF-Connecting-IP + allowlist: + - "203.0.113.0/24" +``` + +### Example 4: Multiple CIDR Ranges + +```yaml +# Allow multiple office networks +path: /office-webhook +handler: office_handler + +ip_filtering: + allowlist: + - "192.168.1.0/24" # Main office + - "192.168.2.0/24" # Branch office + - "10.0.100.0/24" # VPN range + blocklist: + - "192.168.1.200" # Compromised machine +``` + +## Error Responses + +When IP filtering fails, the service returns an HTTP 403 Forbidden response: + +```json +{ + "error": "ip_filtering_failed", + "message": "IP address not allowed", + "request_id": "550e8400-e29b-41d4-a716-446655440000" +} +``` + +## Testing Your Configuration + +You can test your IP filtering configuration using curl: + +```bash +# Test with allowed IP +curl -H "X-Forwarded-For: 192.168.1.50" \ + -H "Content-Type: application/json" \ + -d '{"test": "data"}' \ + http://localhost:8080/webhooks/secure-endpoint + +# Test with blocked IP +curl -H "X-Forwarded-For: 192.168.1.100" \ + -H "Content-Type: application/json" \ + -d '{"test": "data"}' \ + http://localhost:8080/webhooks/secure-endpoint +``` + +## Common Use Cases + +### 1. Restrict to Corporate Network + +```yaml +ip_filtering: + allowlist: + - "10.0.0.0/8" # Private network + - "172.16.0.0/12" # Private network + - "192.168.0.0/16" # Private network +``` + +### 2. Allow Specific Service Providers + +```yaml +ip_filtering: + allowlist: + - "140.82.112.0/20" # GitHub webhook IPs (example) + - "192.30.252.0/22" # GitHub webhook IPs (example) +``` + +### 3. Block Known Bad Actors + +```yaml +ip_filtering: + blocklist: + - "203.0.113.0/24" # Example bad network + - "198.51.100.50" # Specific bad IP +``` + +### 4. Development vs Production + +```yaml +# Development - allow local testing +ip_filtering: + allowlist: + - "127.0.0.1" + - "192.168.1.0/24" + +# Production - restrict to known sources +ip_filtering: + allowlist: + - "10.0.0.0/8" + blocklist: + - "10.0.0.100" # Compromised internal host +``` + +## Best Practices + +1. **Start Restrictive**: Begin with a narrow allowlist and expand as needed +2. **Monitor Logs**: Watch for legitimate traffic being blocked +3. **Layer Security**: Use IP filtering alongside other security measures +4. **Document Changes**: Keep track of why specific IPs or ranges are allowed/blocked +5. **Regular Review**: Periodically review and update your IP filtering rules +6. **Test Thoroughly**: Always test configuration changes in a non-production environment first +7. **Consider Automation**: For dynamic environments, consider using network-level controls instead + +## Troubleshooting + +### Common Issues + +1. **Wrong IP Header**: Verify your proxy/load balancer sets the correct IP header +2. **CIDR Notation**: Ensure CIDR ranges are correctly formatted +3. **Header Case**: The service performs case-insensitive header lookup +4. **Multiple IPs**: Only the first IP in comma-separated headers is checked +5. **IPv6 Support**: The service supports IPv6 addresses and ranges + +### Debug Steps + +1. Check server logs for IP filtering messages +2. Verify the client IP being detected matches expectations +3. Test with known good/bad IPs +4. Confirm endpoint vs global configuration precedence +5. Validate CIDR range calculations + +## Performance Considerations + +- IP filtering adds minimal overhead to request processing +- CIDR matching is optimized for common use cases +- Consider the number of rules - hundreds of rules may impact performance +- Invalid IP patterns in configuration are silently skipped + +## Migration from Auth Plugins + +If you're currently using custom auth plugins for IP filtering, you can migrate to the built-in IP filtering feature: + +**Before (Custom Auth Plugin):** +```yaml +path: /webhook +handler: my_handler +auth: + type: my_ip_filtering_plugin +opts: + allowed_ips: + - "192.168.1.1" +``` + +**After (Built-in IP Filtering):** +```yaml +path: /webhook +handler: my_handler +ip_filtering: + allowlist: + - "192.168.1.1" +``` + +The built-in IP filtering provides better performance, more features (CIDR support, blocklists), and consistent error handling across all endpoints. diff --git a/spec/unit/lib/hooks/app/helpers_spec.rb b/spec/unit/lib/hooks/app/helpers_spec.rb index 66feb99e..0cbab7f5 100644 --- a/spec/unit/lib/hooks/app/helpers_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_spec.rb @@ -345,4 +345,24 @@ def error!(message, code) expect(helper.send(:determine_error_code, error)).to eq(500) end end + + describe "#ip_filtering!" do + let(:headers) { { "X-Forwarded-For" => "192.168.1.1" } } + let(:endpoint_config) { {} } + let(:global_config) { {} } + let(:request_context) { { request_id: "test-request-id" } } + let(:env) { {} } + + it "delegates to Network::IpFiltering.ip_filtering!" do + expect(Hooks::App::Network::IpFiltering).to receive(:ip_filtering!).with( + headers, + endpoint_config, + global_config, + request_context, + env + ) + + helper.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + end + end end From 5ae862806e173a8f141e651dab6ef96a96974035 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 20:54:27 -0700 Subject: [PATCH 07/12] 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 eaa06e4c7858872e0539834e9709c82db2849eeb Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 20:58:04 -0700 Subject: [PATCH 08/12] add docs --- docs/ip_filtering.md | 117 +++---------------------------------------- 1 file changed, 7 insertions(+), 110 deletions(-) diff --git a/docs/ip_filtering.md b/docs/ip_filtering.md index 159b9a99..7e75bb66 100644 --- a/docs/ip_filtering.md +++ b/docs/ip_filtering.md @@ -1,6 +1,6 @@ # IP Filtering -The Hooks service provides comprehensive application-level IP filtering functionality that allows you to control access to your webhooks based on client IP addresses. This feature supports both allowlist (whitelist) and blocklist (blacklist) configurations with CIDR notation support. +The Hooks service provides comprehensive application-level IP filtering functionality that allows you to control access to your webhooks based on client IP addresses. This feature supports both allowlist and blocklist configurations with CIDR notation support. ## Overview @@ -38,6 +38,8 @@ ip_filtering: Configure IP filtering for specific endpoints: +> If a global configuration is set, endpoint-level settings will override it. + ```yaml # config/endpoints/secure-endpoint.yml path: /secure-webhook @@ -55,17 +57,20 @@ ip_filtering: ## Configuration Options ### `ip_header` (optional) + - **Default**: `X-Forwarded-For` - **Description**: HTTP header to check for the client IP address - **Common alternatives**: `X-Real-IP`, `CF-Connecting-IP`, `X-Client-IP` ### `allowlist` (optional) + - **Type**: Array of strings - **Description**: List of allowed IP addresses or CIDR ranges - **Behavior**: If specified, only IPs in this list are allowed access - **Format**: Individual IPs (`192.168.1.1`) or CIDR notation (`192.168.1.0/24`) ### `blocklist` (optional) + - **Type**: Array of strings - **Description**: List of blocked IP addresses or CIDR ranges - **Behavior**: IPs in this list are denied access, even if they appear in the allowlist @@ -167,7 +172,7 @@ When IP filtering fails, the service returns an HTTP 403 Forbidden response: { "error": "ip_filtering_failed", "message": "IP address not allowed", - "request_id": "550e8400-e29b-41d4-a716-446655440000" + "request_id": "" } ``` @@ -188,111 +193,3 @@ curl -H "X-Forwarded-For: 192.168.1.100" \ -d '{"test": "data"}' \ http://localhost:8080/webhooks/secure-endpoint ``` - -## Common Use Cases - -### 1. Restrict to Corporate Network - -```yaml -ip_filtering: - allowlist: - - "10.0.0.0/8" # Private network - - "172.16.0.0/12" # Private network - - "192.168.0.0/16" # Private network -``` - -### 2. Allow Specific Service Providers - -```yaml -ip_filtering: - allowlist: - - "140.82.112.0/20" # GitHub webhook IPs (example) - - "192.30.252.0/22" # GitHub webhook IPs (example) -``` - -### 3. Block Known Bad Actors - -```yaml -ip_filtering: - blocklist: - - "203.0.113.0/24" # Example bad network - - "198.51.100.50" # Specific bad IP -``` - -### 4. Development vs Production - -```yaml -# Development - allow local testing -ip_filtering: - allowlist: - - "127.0.0.1" - - "192.168.1.0/24" - -# Production - restrict to known sources -ip_filtering: - allowlist: - - "10.0.0.0/8" - blocklist: - - "10.0.0.100" # Compromised internal host -``` - -## Best Practices - -1. **Start Restrictive**: Begin with a narrow allowlist and expand as needed -2. **Monitor Logs**: Watch for legitimate traffic being blocked -3. **Layer Security**: Use IP filtering alongside other security measures -4. **Document Changes**: Keep track of why specific IPs or ranges are allowed/blocked -5. **Regular Review**: Periodically review and update your IP filtering rules -6. **Test Thoroughly**: Always test configuration changes in a non-production environment first -7. **Consider Automation**: For dynamic environments, consider using network-level controls instead - -## Troubleshooting - -### Common Issues - -1. **Wrong IP Header**: Verify your proxy/load balancer sets the correct IP header -2. **CIDR Notation**: Ensure CIDR ranges are correctly formatted -3. **Header Case**: The service performs case-insensitive header lookup -4. **Multiple IPs**: Only the first IP in comma-separated headers is checked -5. **IPv6 Support**: The service supports IPv6 addresses and ranges - -### Debug Steps - -1. Check server logs for IP filtering messages -2. Verify the client IP being detected matches expectations -3. Test with known good/bad IPs -4. Confirm endpoint vs global configuration precedence -5. Validate CIDR range calculations - -## Performance Considerations - -- IP filtering adds minimal overhead to request processing -- CIDR matching is optimized for common use cases -- Consider the number of rules - hundreds of rules may impact performance -- Invalid IP patterns in configuration are silently skipped - -## Migration from Auth Plugins - -If you're currently using custom auth plugins for IP filtering, you can migrate to the built-in IP filtering feature: - -**Before (Custom Auth Plugin):** -```yaml -path: /webhook -handler: my_handler -auth: - type: my_ip_filtering_plugin -opts: - allowed_ips: - - "192.168.1.1" -``` - -**After (Built-in IP Filtering):** -```yaml -path: /webhook -handler: my_handler -ip_filtering: - allowlist: - - "192.168.1.1" -``` - -The built-in IP filtering provides better performance, more features (CIDR support, blocklists), and consistent error handling across all endpoints. From 363c0b4ea4c45a8f1f9d0a042e11e371ce22fd93 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 21:04:42 -0700 Subject: [PATCH 09/12] Enhance IP filtering documentation with detailed method descriptions and examples --- lib/hooks/app/network/ip_filtering.rb | 200 +++++++++++++++++++++++--- 1 file changed, 179 insertions(+), 21 deletions(-) diff --git a/lib/hooks/app/network/ip_filtering.rb b/lib/hooks/app/network/ip_filtering.rb index 455ccd66..4f0e44a2 100644 --- a/lib/hooks/app/network/ip_filtering.rb +++ b/lib/hooks/app/network/ip_filtering.rb @@ -6,30 +6,71 @@ module Hooks module App module Network - # Application-level IP filtering functionality - # Provides both allowlist and blocklist filtering with CIDR support + # Application-level IP filtering functionality for HTTP requests. + # + # This class provides robust IP filtering capabilities supporting both allowlist + # and blocklist filtering with CIDR notation support. It can extract client IP + # addresses from various HTTP headers and validate them against configured rules. + # + # The filtering logic follows these rules: + # 1. If a blocklist is configured and the IP matches, access is denied + # 2. If an allowlist is configured, the IP must match to be allowed + # 3. If no allowlist is configured and IP is not blocked, access is allowed + # + # @example Basic usage with endpoint configuration + # config = { + # ip_filtering: { + # allowlist: ["192.168.1.0/24", "10.0.0.1"], + # blocklist: ["192.168.1.100"], + # ip_header: "X-Real-IP" + # } + # } + # IpFiltering.ip_filtering!(headers, config, {}, {}, env) + # + # @note This class is designed to work with Rack-based applications and + # expects headers to be in a Hash format. class IpFiltering - # Default IP header to check for client IP + # Default HTTP header to check for client IP address. + # @return [String] the default header name DEFAULT_IP_HEADER = "X-Forwarded-For" - # Verifies the incoming request passes the configured IP filtering rules. - # - # This method assumes that the client IP address is available in the request headers (e.g., `X-Forwarded-For`). - # The headers that is used is configurable via the endpoint configuration. - # It checks the IP address against the allowed and denied lists defined in the endpoint configuration. - # If the IP address is not allowed, it instantly returns an error response via the `error!` method. - # If the IP filtering configuration is missing or invalid, it raises an error. - # If IP filtering is configured at the global level, it will also check against the global configuration first, - # and then against the endpoint-specific configuration. - # - # @param headers [Hash] The request headers. - # @param endpoint_config [Hash] The endpoint configuration, must include :ip_filtering key. - # @param global_config [Hash] The global configuration (optional, for compatibility). - # @param request_context [Hash] Context for the request, e.g. request ID, path, handler (optional). - # @param env [Hash] The Rack environment - # @raise [StandardError] Raises error if IP filtering fails or is misconfigured. - # @return [void] - # @note This method will halt execution with an error if IP filtering rules fail. + # Verifies that an incoming request passes the configured IP filtering rules. + # + # This method extracts the client IP address from request headers and validates + # it against configured allowlist and blocklist rules. The method will halt + # execution by raising an error if the IP filtering rules fail. + # + # The IP filtering configuration can be defined at both global and endpoint levels, + # with endpoint configuration taking precedence. If no IP filtering is configured, + # the method returns early without performing any checks. + # + # The client IP is extracted from HTTP headers, with support for configurable + # header names. The default header is X-Forwarded-For, which can contain multiple + # comma-separated IPs (the first IP is used as the original client). + # + # @param headers [Hash] The request headers as key-value pairs + # @param endpoint_config [Hash] The endpoint-specific configuration containing :ip_filtering + # @param global_config [Hash] The global configuration (optional, for compatibility) + # @param request_context [Hash] Context information for the request (e.g., request_id, path, handler) + # @param env [Hash] The Rack environment hash + # + # @raise [Hooks::Plugins::Handlers::Error] Raises a 403 error if IP filtering rules fail + # @return [void] Returns nothing if IP filtering passes or is not configured + # + # @example Successful IP filtering + # headers = { "X-Forwarded-For" => "192.168.1.50" } + # config = { ip_filtering: { allowlist: ["192.168.1.0/24"] } } + # IpFiltering.ip_filtering!(headers, config, {}, { request_id: "123" }, env) + # + # @example IP filtering failure + # headers = { "X-Forwarded-For" => "10.0.0.1" } + # config = { ip_filtering: { allowlist: ["192.168.1.0/24"] } } + # # Raises Hooks::Plugins::Handlers::Error with 403 status + # IpFiltering.ip_filtering!(headers, config, {}, { request_id: "123" }, env) + # + # @note This method assumes that the client IP address is available in the request headers + # @note If the IP filtering configuration is missing or invalid, it raises an error + # @note This method will halt execution with an error if IP filtering rules fail def self.ip_filtering!(headers, endpoint_config, global_config, request_context, env) # Determine which IP filtering configuration to use ip_config = resolve_ip_config(endpoint_config, global_config) @@ -51,11 +92,62 @@ def self.ip_filtering!(headers, endpoint_config, global_config, request_context, end end + # Resolves the IP filtering configuration to use for the current request. + # + # This method determines which IP filtering configuration should be applied + # by checking endpoint-specific configuration first, then falling back to + # global configuration. This allows for flexible configuration inheritance + # with endpoint-level overrides. + # + # @param endpoint_config [Hash] The endpoint-specific configuration + # @param global_config [Hash] The global application configuration + # + # @return [Hash, nil] The IP filtering configuration hash, or nil if none configured + # + # @example With endpoint configuration + # endpoint_config = { ip_filtering: { allowlist: ["192.168.1.0/24"] } } + # global_config = { ip_filtering: { allowlist: ["10.0.0.0/8"] } } + # resolve_ip_config(endpoint_config, global_config) + # # => { allowlist: ["192.168.1.0/24"] } + # + # @example With only global configuration + # endpoint_config = {} + # global_config = { ip_filtering: { allowlist: ["10.0.0.0/8"] } } + # resolve_ip_config(endpoint_config, global_config) + # # => { allowlist: ["10.0.0.0/8"] } + # + # @note Endpoint-level configuration takes precedence over global configuration private_class_method def self.resolve_ip_config(endpoint_config, global_config) # Endpoint-level configuration takes precedence over global configuration endpoint_config[:ip_filtering] || global_config[:ip_filtering] end + # Extracts the client IP address from request headers. + # + # This method looks for the client IP in the specified header (or default + # X-Forwarded-For header). It performs case-insensitive header matching + # and handles comma-separated IP lists by taking the first IP address, + # which represents the original client in proxy chains. + # + # @param headers [Hash] The request headers as key-value pairs + # @param ip_config [Hash] The IP filtering configuration containing :ip_header + # + # @return [String, nil] The client IP address, or nil if not found or empty + # + # @example Extracting from X-Forwarded-For + # headers = { "X-Forwarded-For" => "192.168.1.50, 10.0.0.1" } + # ip_config = { ip_header: "X-Forwarded-For" } + # extract_client_ip(headers, ip_config) + # # => "192.168.1.50" + # + # @example Extracting from custom header + # headers = { "X-Real-IP" => "203.0.113.45" } + # ip_config = { ip_header: "X-Real-IP" } + # extract_client_ip(headers, ip_config) + # # => "203.0.113.45" + # + # @note Case-insensitive header lookup is performed + # @note For comma-separated IP lists, only the first IP is returned private_class_method def self.extract_client_ip(headers, ip_config) # Use configured header or default to X-Forwarded-For ip_header = ip_config[:ip_header] || DEFAULT_IP_HEADER @@ -72,6 +164,40 @@ def self.ip_filtering!(headers, endpoint_config, global_config, request_context, nil end + # Determines if a client IP address is allowed based on filtering rules. + # + # This method implements the core IP filtering logic by checking the client + # IP against configured blocklist and allowlist rules. The filtering follows + # these precedence rules: + # 1. If blocklist exists and IP matches, deny access (return false) + # 2. If allowlist exists, IP must match to be allowed (return true/false) + # 3. If no allowlist exists and IP not blocked, allow access (return true) + # + # @param client_ip [String] The client IP address to validate + # @param ip_config [Hash] The IP filtering configuration containing :blocklist and/or :allowlist + # + # @return [Boolean] true if IP is allowed, false if blocked or invalid + # + # @example IP allowed by allowlist + # client_ip = "192.168.1.50" + # ip_config = { allowlist: ["192.168.1.0/24"] } + # ip_allowed?(client_ip, ip_config) + # # => true + # + # @example IP blocked by blocklist + # client_ip = "192.168.1.100" + # ip_config = { blocklist: ["192.168.1.100"] } + # ip_allowed?(client_ip, ip_config) + # # => false + # + # @example Invalid IP format + # client_ip = "invalid-ip" + # ip_config = { allowlist: ["192.168.1.0/24"] } + # ip_allowed?(client_ip, ip_config) + # # => false + # + # @note Invalid IP addresses are automatically denied + # @note Blocklist rules take precedence over allowlist rules private_class_method def self.ip_allowed?(client_ip, ip_config) # Parse client IP begin @@ -94,6 +220,38 @@ def self.ip_filtering!(headers, endpoint_config, global_config, request_context, true end + # Checks if a client IP address matches any pattern in an IP list. + # + # This method iterates through a list of IP patterns (which can include + # individual IPs or CIDR ranges) and determines if the client IP matches + # any of them. It uses Ruby's IPAddr class for robust IP address and + # CIDR range matching, with error handling for invalid IP patterns. + # + # @param client_addr [IPAddr] The client IP address as an IPAddr object + # @param ip_list [Array] Array of IP patterns (IPs or CIDR ranges) + # + # @return [Boolean] true if client IP matches any pattern in the list, false otherwise + # + # @example Matching individual IP + # client_addr = IPAddr.new("192.168.1.50") + # ip_list = ["192.168.1.50", "10.0.0.1"] + # ip_matches_list?(client_addr, ip_list) + # # => true + # + # @example Matching CIDR range + # client_addr = IPAddr.new("192.168.1.50") + # ip_list = ["192.168.1.0/24", "10.0.0.0/8"] + # ip_matches_list?(client_addr, ip_list) + # # => true + # + # @example No match found + # client_addr = IPAddr.new("203.0.113.45") + # ip_list = ["192.168.1.0/24", "10.0.0.0/8"] + # ip_matches_list?(client_addr, ip_list) + # # => false + # + # @note Invalid IP patterns in the list are silently skipped + # @note Supports both IPv4 and IPv6 addresses and ranges private_class_method def self.ip_matches_list?(client_addr, ip_list) ip_list.each do |ip_pattern| begin From b92ff2260b12543202edf98bd663fa392a24304d Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 21:12:40 -0700 Subject: [PATCH 10/12] Refactor IP filtering module structure and update references in helpers and specs --- lib/hooks/app/api.rb | 2 +- lib/hooks/app/helpers.rb | 4 ++-- lib/hooks/{app => core}/network/ip_filtering.rb | 2 +- spec/unit/app/network/ip_filtering_spec.rb | 4 ++-- spec/unit/lib/hooks/app/helpers_spec.rb | 2 +- 5 files changed, 7 insertions(+), 7 deletions(-) rename lib/hooks/{app => core}/network/ip_filtering.rb (99%) diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 912fff78..0bf9d579 100644 --- a/lib/hooks/app/api.rb +++ b/lib/hooks/app/api.rb @@ -4,7 +4,7 @@ require "json" require "securerandom" require_relative "helpers" -require_relative "network/ip_filtering" +require_relative "../core/network/ip_filtering" require_relative "auth/auth" require_relative "rack_env_builder" require_relative "../plugins/handlers/base" diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index b9502b18..7c3b89a7 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -3,7 +3,7 @@ require "securerandom" require_relative "../security" require_relative "../core/plugin_loader" -require_relative "network/ip_filtering" +require_relative "../core/network/ip_filtering" module Hooks module App @@ -108,7 +108,7 @@ def load_handler(handler_class_name) # @return [void] # @note This method will halt execution with an error if IP filtering rules fail. def ip_filtering!(headers, endpoint_config, global_config, request_context, env) - Network::IpFiltering.ip_filtering!(headers, endpoint_config, global_config, request_context, env) + Hooks::Core::Network::IpFiltering.ip_filtering!(headers, endpoint_config, global_config, request_context, env) end private diff --git a/lib/hooks/app/network/ip_filtering.rb b/lib/hooks/core/network/ip_filtering.rb similarity index 99% rename from lib/hooks/app/network/ip_filtering.rb rename to lib/hooks/core/network/ip_filtering.rb index 4f0e44a2..060d7be5 100644 --- a/lib/hooks/app/network/ip_filtering.rb +++ b/lib/hooks/core/network/ip_filtering.rb @@ -4,7 +4,7 @@ require_relative "../../plugins/handlers/error" module Hooks - module App + module Core module Network # Application-level IP filtering functionality for HTTP requests. # diff --git a/spec/unit/app/network/ip_filtering_spec.rb b/spec/unit/app/network/ip_filtering_spec.rb index f89ee3f6..e016e093 100644 --- a/spec/unit/app/network/ip_filtering_spec.rb +++ b/spec/unit/app/network/ip_filtering_spec.rb @@ -1,9 +1,9 @@ # frozen_string_literal: true require_relative "../../../unit/spec_helper" -require_relative "../../../../lib/hooks/app/network/ip_filtering" +require_relative "../../../../lib/hooks/core/network/ip_filtering" -RSpec.describe Hooks::App::Network::IpFiltering do +describe Hooks::Core::Network::IpFiltering do describe ".ip_filtering!" do let(:headers) { { "X-Forwarded-For" => "192.168.1.100" } } let(:endpoint_config) { {} } diff --git a/spec/unit/lib/hooks/app/helpers_spec.rb b/spec/unit/lib/hooks/app/helpers_spec.rb index 0cbab7f5..21cb7cd6 100644 --- a/spec/unit/lib/hooks/app/helpers_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_spec.rb @@ -354,7 +354,7 @@ def error!(message, code) let(:env) { {} } it "delegates to Network::IpFiltering.ip_filtering!" do - expect(Hooks::App::Network::IpFiltering).to receive(:ip_filtering!).with( + expect(Hooks::Core::Network::IpFiltering).to receive(:ip_filtering!).with( headers, endpoint_config, global_config, From 313ac9dba2948b3fbb880874700580473ca9c1c1 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 21:20:17 -0700 Subject: [PATCH 11/12] bump version --- Gemfile.lock | 2 +- lib/hooks/version.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index c3752188..f4b7cded 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ PATH remote: . specs: - hooks-ruby (0.3.0) + hooks-ruby (0.3.1) 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 a506b8d9..a50c58a1 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.0".freeze + VERSION = "0.3.1".freeze end From aca58fac02795f56ce30eda8e393d615f6ff4caa Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Tue, 17 Jun 2025 21:21:41 -0700 Subject: [PATCH 12/12] fmt --- spec/acceptance/config/endpoints/ip_filtering_custom_header.yml | 2 +- spec/acceptance/config/endpoints/ip_filtering_direct.yml | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/spec/acceptance/config/endpoints/ip_filtering_custom_header.yml b/spec/acceptance/config/endpoints/ip_filtering_custom_header.yml index e3fbfa13..73e24246 100644 --- a/spec/acceptance/config/endpoints/ip_filtering_custom_header.yml +++ b/spec/acceptance/config/endpoints/ip_filtering_custom_header.yml @@ -4,4 +4,4 @@ handler: hello ip_filtering: ip_header: X-Real-IP allowlist: - - "10.0.0.0/8" \ No newline at end of file + - "10.0.0.0/8" diff --git a/spec/acceptance/config/endpoints/ip_filtering_direct.yml b/spec/acceptance/config/endpoints/ip_filtering_direct.yml index 1618f59e..f12acb29 100644 --- a/spec/acceptance/config/endpoints/ip_filtering_direct.yml +++ b/spec/acceptance/config/endpoints/ip_filtering_direct.yml @@ -6,4 +6,4 @@ ip_filtering: - "127.0.0.1" - "192.168.1.0/24" blocklist: - - "192.168.1.100" \ No newline at end of file + - "192.168.1.100"