From 8a8771dc2e64012d9a388c3b7c998a0531505672 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 04:44:58 +0000 Subject: [PATCH 01/10] Initial plan for issue From 49601be4af48b4943ef27ff5dc7bad7486bb2b65 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 04:49:20 +0000 Subject: [PATCH 02/10] Initial analysis and plan for production readiness audit 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 734772e508e0af2a5db0efa6df55586de0c6e438 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 12 Jun 2025 05:01:07 +0000 Subject: [PATCH 03/10] Implement production readiness and performance improvements Co-authored-by: GrantBirki <23362539+GrantBirki@users.noreply.github.com> --- docs/reliability.md | 262 ++++++++++++++++++++++++ lib/hooks.rb | 39 ++-- lib/hooks/app/helpers.rb | 54 ++++- lib/hooks/utils/retry.rb | 19 +- spec/unit/lib/hooks/app/helpers_spec.rb | 43 ++++ spec/unit/lib/hooks/utils/retry_spec.rb | 129 ++++++++++++ 6 files changed, 520 insertions(+), 26 deletions(-) create mode 100644 docs/reliability.md create mode 100644 spec/unit/lib/hooks/utils/retry_spec.rb diff --git a/docs/reliability.md b/docs/reliability.md new file mode 100644 index 00000000..10f34aab --- /dev/null +++ b/docs/reliability.md @@ -0,0 +1,262 @@ +# Production Reliability and Performance Guide + +This document outlines the reliability, performance, and security considerations for running the Hooks webhook server framework in production environments. + +## 🔍 Security Considerations + +### Dynamic Plugin Loading Security + +The framework includes comprehensive security measures for dynamic plugin loading: + +- **Class Name Validation**: All plugin class names are validated against safe patterns (`/\A[A-Z][a-zA-Z0-9_]*\z/`) +- **Dangerous Class Blacklist**: System classes like `File`, `Dir`, `Kernel`, `Object`, `Process`, etc. are blocked from being loaded as plugins +- **Path Traversal Protection**: Plugin file paths are normalized and validated to prevent loading files outside designated directories +- **Safe Constant Resolution**: Uses `Object.const_get` only after thorough validation + +### Request Processing Security + +- **Request Size Limits**: Configurable request body size limits (default enforcement via `request_limit` config) +- **JSON Parsing Protection**: JSON parsing includes security limits to prevent JSON bombs: + - Maximum nesting depth (configurable via `JSON_MAX_NESTING`, default: 20) + - Maximum payload size before parsing (configurable via `JSON_MAX_SIZE`, default: 10MB) + - Disabled object creation from JSON (`create_additions: false`) + - Uses plain Hash/Array classes to prevent object injection +- **Header Validation**: Multiple header format handling with safe fallbacks and optimized lookup order + +## ⚡ Performance Optimizations + +### Startup Performance + +The framework uses several strategies to optimize startup time: + +- **Explicit Module Loading**: Core modules are loaded explicitly rather than using `Dir.glob` patterns for better performance and security +- **Boot-time Plugin Loading**: All plugins are loaded once at startup rather than per-request +- **Plugin Caching**: Loaded plugins are cached in class-level registries for fast access +- **Sorted Directory Loading**: Plugin directories are processed in sorted order for consistent behavior + +### Runtime Performance + +- **Per-request Optimizations**: + - Plugin instances are reused across requests + - Request contexts use thread-local storage for efficient access + - Handler instances are created per-request but classes are cached + - Optimized header processing with common cases checked first + +- **Memory Management**: + - Plugin registries use hash-based lookups for O(1) access + - Thread-local contexts are properly cleaned up after requests + - Clear plugin loading separates concerns efficiently + +- **Security Limits**: + - Retry configuration includes bounds checking to prevent resource exhaustion + - JSON parsing has built-in limits to prevent JSON bombs and memory attacks + +### Recommended Production Configuration + +```yaml +# Example production configuration +log_level: "info" # Reduces debug overhead +request_limit: 1048576 # 1MB limit (adjust based on needs) +request_timeout: 30 # 30 second timeout +environment: "production" # Disables debug features like backtraces +normalize_headers: true # Consistent header processing +symbolize_payload: false # Reduced memory usage for large payloads +``` + +### Security Environment Variables + +Additional security can be configured via environment variables: + +```bash +# JSON Security Limits +JSON_MAX_NESTING=20 # Maximum JSON nesting depth (default: 20) +JSON_MAX_SIZE=10485760 # Maximum JSON size before parsing (default: 10MB) + +# Retry Safety Limits +DEFAULT_RETRY_SLEEP=1 # Sleep between retries 0-300 seconds (default: 1) +DEFAULT_RETRY_TRIES=10 # Number of retry attempts 1-50 (default: 10) +RETRY_LOG_RETRIES=false # Disable retry logging in production (default: true) +``` + +## 🔧 Monitoring and Observability + +### Health Check Endpoint + +The built-in health endpoint (`/health`) provides comprehensive status information: + +```json +{ + "status": "healthy", + "timestamp": "2025-01-01T12:00:00Z", + "version": "1.0.0", + "uptime_seconds": 3600, + "config_checksum": "abc123", + "endpoints_loaded": 5, + "plugins_loaded": 3 +} +``` + +### Lifecycle Hooks for Monitoring + +Use lifecycle plugins to add comprehensive monitoring: + +- **Request Metrics**: Track request counts, timing, and error rates +- **Error Reporting**: Capture and report exceptions with full context +- **Resource Monitoring**: Track memory usage, plugin load times, etc. + +### Recommended Instrumentation + +```ruby +# Example monitoring lifecycle plugin +class MonitoringLifecycle < Hooks::Plugins::Lifecycle + def on_request(env) + stats.increment("webhook.requests", { + handler: env["hooks.handler"], + endpoint: env["PATH_INFO"] + }) + end + + def on_response(env, response) + processing_time = Time.now - Time.parse(env["hooks.start_time"]) + stats.timing("webhook.processing_time", processing_time * 1000, { + handler: env["hooks.handler"] + }) + end + + def on_error(exception, env) + stats.increment("webhook.errors", { + error_type: exception.class.name, + handler: env["hooks.handler"] + }) + + failbot.report(exception, { + request_id: env["hooks.request_id"], + handler: env["hooks.handler"], + endpoint: env["PATH_INFO"] + }) + end +end +``` + +## 🚀 Production Deployment Best Practices + +### Server Configuration + +1. **Use Puma in Cluster Mode** for production: +```ruby +# config/puma.rb +workers ENV.fetch("WEB_CONCURRENCY", 2) +threads_count = ENV.fetch("MAX_THREADS", 5) +threads threads_count, threads_count +preload_app! +``` + +2. **Configure Resource Limits**: + - Set appropriate worker memory limits + - Configure worker restart thresholds + - Set connection pool sizes appropriately + +3. **Environment Variables**: +```bash +# Retry configuration +DEFAULT_RETRY_TRIES=3 # Reduced from default 10 +DEFAULT_RETRY_SLEEP=1 # 1 second between retries +RETRY_LOG_RETRIES=false # Reduce log noise in production + +# Logging +LOG_LEVEL=info # Reduce debug overhead +``` + +### Container Considerations + +```dockerfile +# Optimized production Dockerfile +FROM ruby:3.2-alpine AS builder +WORKDIR /app +COPY Gemfile* ./ +RUN bundle install --deployment --without development test + +FROM ruby:3.2-alpine +WORKDIR /app +COPY --from=builder /app/vendor ./vendor +COPY . . + +# Security: Run as non-root user +RUN addgroup -g 1001 -S appuser && \ + adduser -S appuser -u 1001 -G appuser +USER appuser + +# Health check +HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ + CMD curl -f http://localhost:3000/health || exit 1 + +EXPOSE 3000 +CMD ["bundle", "exec", "puma", "-C", "config/puma.rb"] +``` + +## 🛡️ Security Hardening + +### Input Validation + +- **Payload Size Limits**: Always configure `request_limit` appropriate for your use case +- **Timeout Configuration**: Set reasonable `request_timeout` values +- **Content Type Validation**: Implement strict content type checking if needed + +### Authentication + +- **HMAC Validation**: Always enable authentication for production endpoints +- **Secret Management**: Store webhook secrets in environment variables or secure secret management systems +- **Signature Validation**: Use time-based signature validation to prevent replay attacks + +### Network Security + +- **TLS Termination**: Always terminate TLS/SSL at load balancer or reverse proxy +- **IP Whitelisting**: Implement IP restrictions at network level when possible +- **Rate Limiting**: Implement rate limiting at reverse proxy/load balancer level + +## 📊 Performance Benchmarking + +### Load Testing Recommendations + +1. **Baseline Testing**: Test with minimal handlers and no lifecycle plugins +2. **Plugin Impact**: Measure performance impact of each lifecycle plugin +3. **Memory Profiling**: Monitor memory usage over extended periods +4. **Concurrency Testing**: Test with realistic concurrent webhook loads + +### Key Metrics to Monitor + +- **Request Processing Time**: P50, P95, P99 response times +- **Memory Usage**: RSS, heap size, GC frequency +- **Error Rates**: 4xx and 5xx response rates +- **Plugin Performance**: Individual plugin execution times +- **Resource Utilization**: CPU, memory, network I/O + +## 🔧 Troubleshooting + +### Common Performance Issues + +1. **High Memory Usage**: + - Check for plugin memory leaks + - Monitor payload sizes + - Review lifecycle plugin efficiency + +2. **Slow Request Processing**: + - Profile individual plugins + - Check JSON parsing performance + - Review handler implementation efficiency + +3. **Plugin Loading Issues**: + - Verify plugin directory permissions + - Check plugin class name formatting + - Review security validation errors + +### Debug Configuration + +For troubleshooting, temporarily enable debug logging: + +```yaml +log_level: "debug" +environment: "development" # Enables error backtraces +``` + +**Important**: Never run production with debug logging enabled long-term due to performance and security implications. \ No newline at end of file diff --git a/lib/hooks.rb b/lib/hooks.rb index d1ab8cea..6eaab38b 100644 --- a/lib/hooks.rb +++ b/lib/hooks.rb @@ -3,20 +3,35 @@ require_relative "hooks/version" require_relative "hooks/core/builder" -# Load all core components -Dir[File.join(__dir__, "hooks/core/**/*.rb")].sort.each do |file| - require file -end +# Load core components explicitly for better performance and security +require_relative "hooks/core/config_loader" +require_relative "hooks/core/config_validator" +require_relative "hooks/core/logger_factory" +require_relative "hooks/core/plugin_loader" +require_relative "hooks/core/global_components" +require_relative "hooks/core/log" +require_relative "hooks/core/failbot" +require_relative "hooks/core/stats" -# Load all plugins (auth plugins, handler plugins, lifecycle hooks, etc.) -Dir[File.join(__dir__, "hooks/plugins/**/*.rb")].sort.each do |file| - require file -end +# Load essential plugins explicitly +require_relative "hooks/plugins/auth/base" +require_relative "hooks/plugins/auth/hmac" +require_relative "hooks/plugins/auth/shared_secret" +require_relative "hooks/plugins/handlers/base" +require_relative "hooks/plugins/handlers/default" +require_relative "hooks/plugins/lifecycle" +require_relative "hooks/plugins/instruments/stats_base" +require_relative "hooks/plugins/instruments/failbot_base" +require_relative "hooks/plugins/instruments/stats" +require_relative "hooks/plugins/instruments/failbot" -# Load all utils -Dir[File.join(__dir__, "hooks/utils/**/*.rb")].sort.each do |file| - require file -end +# Load utils explicitly +require_relative "hooks/utils/normalize" +require_relative "hooks/utils/retry" + +# Load security module +require_relative "hooks/security" +require_relative "hooks/version" # Main module for the Hooks webhook server framework module Hooks diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index aa2e59da..5d1fb287 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -21,13 +21,15 @@ def uuid # @return [void] # @note Timeout enforcement should be handled at the server level (e.g., Puma) def enforce_request_limits(config) - # Check content length (handle different header formats and sources) - content_length = headers["Content-Length"] || headers["CONTENT_LENGTH"] || - headers["content-length"] || headers["HTTP_CONTENT_LENGTH"] || - env["CONTENT_LENGTH"] || env["HTTP_CONTENT_LENGTH"] + # Optimized content length check - check most common sources first + content_length = request.content_length if respond_to?(:request) && request.respond_to?(:content_length) - # Also try to get from request object directly - content_length ||= request.content_length if respond_to?(:request) && request.respond_to?(:content_length) + content_length ||= headers["Content-Length"] || + headers["CONTENT_LENGTH"] || + headers["content-length"] || + headers["HTTP_CONTENT_LENGTH"] || + env["CONTENT_LENGTH"] || + env["HTTP_CONTENT_LENGTH"] content_length = content_length&.to_i @@ -38,23 +40,29 @@ def enforce_request_limits(config) # Note: Timeout enforcement would typically be handled at the server level (Puma, etc.) end - # Parse request payload + # Parse request payload with security limits # # @param raw_body [String] The raw request body # @param headers [Hash] The request headers # @param symbolize [Boolean] Whether to symbolize keys in parsed JSON (default: true) # @return [Hash, String] Parsed JSON as Hash (optionally symbolized), or raw body if not JSON def parse_payload(raw_body, headers, symbolize: true) - content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] || headers["content-type"] || headers["HTTP_CONTENT_TYPE"] + # Optimized content type check - check most common header first + content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] || headers["content-type"] # Try to parse as JSON if content type suggests it or if it looks like JSON if content_type&.include?("application/json") || (raw_body.strip.start_with?("{", "[") rescue false) begin - parsed_payload = JSON.parse(raw_body) + # Security: Limit JSON parsing depth and complexity to prevent JSON bombs + parsed_payload = safe_json_parse(raw_body) parsed_payload = parsed_payload.transform_keys(&:to_sym) if symbolize && parsed_payload.is_a?(Hash) return parsed_payload - rescue JSON::ParserError - # If JSON parsing fails, return raw body + rescue JSON::ParserError, ArgumentError => e + # If JSON parsing fails or security limits exceeded, return raw body + # Log security violations at debug level to avoid log spam + if e.message.include?("nesting") || e.message.include?("depth") + log.debug("JSON parsing security limit exceeded: #{e.message}") + end end end @@ -79,6 +87,30 @@ def load_handler(handler_class_name) private + # Safely parse JSON with security limits to prevent JSON bombs + # + # @param json_string [String] The JSON string to parse + # @return [Hash, Array] Parsed JSON object + # @raise [JSON::ParserError] If JSON is invalid + # @raise [ArgumentError] If security limits are exceeded + def safe_json_parse(json_string) + # Security limits for JSON parsing + max_nesting = ENV.fetch("JSON_MAX_NESTING", "20").to_i + max_create_depth = ENV.fetch("JSON_MAX_CREATE_DEPTH", "15").to_i + + # Additional size check before parsing + if json_string.length > ENV.fetch("JSON_MAX_SIZE", "10485760").to_i # 10MB default + raise ArgumentError, "JSON payload too large for parsing" + end + + JSON.parse(json_string, { + max_nesting: max_nesting, + create_additions: false, # Security: Disable object creation from JSON + object_class: Hash, # Use plain Hash instead of custom classes + array_class: Array # Use plain Array instead of custom classes + }) + end + # Determine HTTP error code from exception # # @param exception [Exception] The exception to map to an HTTP status code diff --git a/lib/hooks/utils/retry.rb b/lib/hooks/utils/retry.rb index cad5fc59..d3ac82fc 100644 --- a/lib/hooks/utils/retry.rb +++ b/lib/hooks/utils/retry.rb @@ -9,11 +9,24 @@ module Retry # Should the number of retries be reached without success, the last exception will be raised # # @param log [Logger] The logger to use for retryable logging - # @raise [ArgumentError] If no logger is provided + # @raise [ArgumentError] If no logger is provided or configuration values are invalid # @return [void] def self.setup!(log: nil, log_retries: ENV.fetch("RETRY_LOG_RETRIES", "true") == "true") raise ArgumentError, "a logger must be provided" if log.nil? + # Security: Validate and bound retry configuration values + retry_sleep = ENV.fetch("DEFAULT_RETRY_SLEEP", "1").to_i + retry_tries = ENV.fetch("DEFAULT_RETRY_TRIES", "10").to_i + + # Bounds checking to prevent resource exhaustion + if retry_sleep < 0 || retry_sleep > 300 # Max 5 minutes between retries + raise ArgumentError, "DEFAULT_RETRY_SLEEP must be between 0 and 300 seconds, got: #{retry_sleep}" + end + + if retry_tries < 1 || retry_tries > 50 # Max 50 retries to prevent infinite loops + raise ArgumentError, "DEFAULT_RETRY_TRIES must be between 1 and 50, got: #{retry_tries}" + end + log_method = lambda do |retries, exception| # :nocov: if log_retries @@ -28,8 +41,8 @@ def self.setup!(log: nil, log_retries: ENV.fetch("RETRY_LOG_RETRIES", "true") == Retryable.configure do |config| config.contexts[:default] = { on: [StandardError], - sleep: ENV.fetch("DEFAULT_RETRY_SLEEP", "1").to_i, - tries: ENV.fetch("DEFAULT_RETRY_TRIES", "10").to_i, + sleep: retry_sleep, + tries: retry_tries, log_method: } end diff --git a/spec/unit/lib/hooks/app/helpers_spec.rb b/spec/unit/lib/hooks/app/helpers_spec.rb index c4179a85..e4aac5eb 100644 --- a/spec/unit/lib/hooks/app/helpers_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_spec.rb @@ -204,6 +204,49 @@ def error!(message, code) end end + context "with JSON security limits" do + it "handles deeply nested JSON within limits" do + headers = { "Content-Type" => "application/json" } + # Create a nested JSON structure within reasonable limits + nested_json = '{"level1": {"level2": {"level3": {"value": "test"}}}}' + + result = helper.parse_payload(nested_json, headers) + + expect(result).to eq({ level1: { "level2" => { "level3" => { "value" => "test" } } } }) + end + + it "returns raw body when JSON exceeds size limits" do + headers = { "Content-Type" => "application/json" } + + # Mock the safe_json_parse method to test the size limit behavior + allow(helper).to receive(:safe_json_parse).and_raise(ArgumentError, "JSON payload too large for parsing") + + # Create a JSON string + json_data = '{"data": "test"}' + + result = helper.parse_payload(json_data, headers) + + # Should return raw body when size limit exceeded + expect(result).to eq(json_data) + end + + it "logs debug message when JSON security limits are exceeded" do + headers = { "Content-Type" => "application/json" } + + # Mock logger to capture debug messages + logger = instance_double("Logger") + allow(helper).to receive(:log).and_return(logger) + expect(logger).to receive(:debug).with(/JSON parsing security limit exceeded/) + + # Mock the safe_json_parse method to simulate nesting limit exceeded + allow(helper).to receive(:safe_json_parse).and_raise(ArgumentError, "nesting exceeded") + + json_data = '{"data": "test"}' + result = helper.parse_payload(json_data, headers) + expect(result).to eq(json_data) + end + end + context "with non-JSON content" do it "returns raw body for plain text" do headers = { "Content-Type" => "text/plain" } diff --git a/spec/unit/lib/hooks/utils/retry_spec.rb b/spec/unit/lib/hooks/utils/retry_spec.rb new file mode 100644 index 00000000..2ab0c297 --- /dev/null +++ b/spec/unit/lib/hooks/utils/retry_spec.rb @@ -0,0 +1,129 @@ +# frozen_string_literal: true + +require_relative "../../../spec_helper" + +# Import the tested class +require_relative "../../../../../lib/hooks/utils/retry" + +describe Retry do + let(:logger) { instance_double("Logger") } + + before do + # Reset any previous configuration + Retryable.configuration.contexts.clear + end + + describe ".setup!" do + context "with valid parameters" do + it "sets up retry configuration with default values" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("1") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("10") + allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("true") + + Retry.setup!(log: logger) + + expect(Retryable.configuration.contexts[:default]).to include( + sleep: 1, + tries: 10 + ) + end + + it "accepts custom retry configuration within bounds" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("5") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("3") + allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("false") + + Retry.setup!(log: logger) + + expect(Retryable.configuration.contexts[:default]).to include( + sleep: 5, + tries: 3 + ) + end + end + + context "with invalid parameters" do + it "raises ArgumentError when no logger is provided" do + expect do + Retry.setup!(log: nil) + end.to raise_error(ArgumentError, "a logger must be provided") + end + + it "raises ArgumentError when retry sleep is negative" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("-1") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("10") + + expect do + Retry.setup!(log: logger) + end.to raise_error(ArgumentError, /DEFAULT_RETRY_SLEEP must be between 0 and 300/) + end + + it "raises ArgumentError when retry sleep exceeds maximum" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("301") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("10") + + expect do + Retry.setup!(log: logger) + end.to raise_error(ArgumentError, /DEFAULT_RETRY_SLEEP must be between 0 and 300/) + end + + it "raises ArgumentError when retry tries is zero" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("1") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("0") + + expect do + Retry.setup!(log: logger) + end.to raise_error(ArgumentError, /DEFAULT_RETRY_TRIES must be between 1 and 50/) + end + + it "raises ArgumentError when retry tries exceeds maximum" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("1") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("51") + + expect do + Retry.setup!(log: logger) + end.to raise_error(ArgumentError, /DEFAULT_RETRY_TRIES must be between 1 and 50/) + end + end + + context "with boundary values" do + it "accepts minimum valid values" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("0") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("1") + allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("true") + + expect do + Retry.setup!(log: logger) + end.not_to raise_error + + expect(Retryable.configuration.contexts[:default]).to include( + sleep: 0, + tries: 1 + ) + end + + it "accepts maximum valid values" do + allow(ENV).to receive(:fetch).and_call_original + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("300") + allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("50") + allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("true") + + expect do + Retry.setup!(log: logger) + end.not_to raise_error + + expect(Retryable.configuration.contexts[:default]).to include( + sleep: 300, + tries: 50 + ) + end + end + end +end From 61ece3fe2d695b5e3e7b1e07a5a3cabe5c22b7e3 Mon Sep 17 00:00:00 2001 From: Grant Birkinbine Date: Wed, 11 Jun 2025 22:10:05 -0700 Subject: [PATCH 04/10] Apply suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- lib/hooks/app/helpers.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index 5d1fb287..23bcb6f8 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -48,7 +48,7 @@ def enforce_request_limits(config) # @return [Hash, String] Parsed JSON as Hash (optionally symbolized), or raw body if not JSON def parse_payload(raw_body, headers, symbolize: true) # Optimized content type check - check most common header first - content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] || headers["content-type"] + content_type = headers["Content-Type"] || headers["CONTENT_TYPE"] || headers["content-type"] || headers["HTTP_CONTENT_TYPE"] # Try to parse as JSON if content type suggests it or if it looks like JSON if content_type&.include?("application/json") || (raw_body.strip.start_with?("{", "[") rescue false) From 925df621f8bb150b7fd1ac284c64755c488cc62b Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 11 Jun 2025 22:13:05 -0700 Subject: [PATCH 05/10] Refactor bundle configuration and improve logging level for JSON parsing errors --- .bundle/config | 3 +-- lib/hooks.rb | 8 -------- lib/hooks/app/helpers.rb | 2 +- spec/unit/lib/hooks/app/helpers_spec.rb | 2 +- 4 files changed, 3 insertions(+), 12 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" diff --git a/lib/hooks.rb b/lib/hooks.rb index 6eaab38b..c5a479d7 100644 --- a/lib/hooks.rb +++ b/lib/hooks.rb @@ -2,8 +2,6 @@ require_relative "hooks/version" require_relative "hooks/core/builder" - -# Load core components explicitly for better performance and security require_relative "hooks/core/config_loader" require_relative "hooks/core/config_validator" require_relative "hooks/core/logger_factory" @@ -12,8 +10,6 @@ require_relative "hooks/core/log" require_relative "hooks/core/failbot" require_relative "hooks/core/stats" - -# Load essential plugins explicitly require_relative "hooks/plugins/auth/base" require_relative "hooks/plugins/auth/hmac" require_relative "hooks/plugins/auth/shared_secret" @@ -24,12 +20,8 @@ require_relative "hooks/plugins/instruments/failbot_base" require_relative "hooks/plugins/instruments/stats" require_relative "hooks/plugins/instruments/failbot" - -# Load utils explicitly require_relative "hooks/utils/normalize" require_relative "hooks/utils/retry" - -# Load security module require_relative "hooks/security" require_relative "hooks/version" diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index 23bcb6f8..a8e30b49 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -61,7 +61,7 @@ def parse_payload(raw_body, headers, symbolize: true) # If JSON parsing fails or security limits exceeded, return raw body # Log security violations at debug level to avoid log spam if e.message.include?("nesting") || e.message.include?("depth") - log.debug("JSON parsing security limit exceeded: #{e.message}") + log.warn("JSON parsing security limit exceeded: #{e.message}") end end end diff --git a/spec/unit/lib/hooks/app/helpers_spec.rb b/spec/unit/lib/hooks/app/helpers_spec.rb index e4aac5eb..80b11c5a 100644 --- a/spec/unit/lib/hooks/app/helpers_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_spec.rb @@ -236,7 +236,7 @@ def error!(message, code) # Mock logger to capture debug messages logger = instance_double("Logger") allow(helper).to receive(:log).and_return(logger) - expect(logger).to receive(:debug).with(/JSON parsing security limit exceeded/) + expect(logger).to receive(:warn).with(/JSON parsing security limit exceeded/) # Mock the safe_json_parse method to simulate nesting limit exceeded allow(helper).to receive(:safe_json_parse).and_raise(ArgumentError, "nesting exceeded") From c4159c34d71a2f272c6db93649e8b10b463f854d Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 11 Jun 2025 22:16:12 -0700 Subject: [PATCH 06/10] Refactor JSON parsing comments and update warning messages for clarity --- lib/hooks/app/helpers.rb | 7 +++---- spec/unit/lib/hooks/app/helpers_spec.rb | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index a8e30b49..bf76003b 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -40,7 +40,7 @@ def enforce_request_limits(config) # Note: Timeout enforcement would typically be handled at the server level (Puma, etc.) end - # Parse request payload with security limits + # Parse request payload # # @param raw_body [String] The raw request body # @param headers [Hash] The request headers @@ -59,9 +59,8 @@ def parse_payload(raw_body, headers, symbolize: true) return parsed_payload rescue JSON::ParserError, ArgumentError => e # If JSON parsing fails or security limits exceeded, return raw body - # Log security violations at debug level to avoid log spam if e.message.include?("nesting") || e.message.include?("depth") - log.warn("JSON parsing security limit exceeded: #{e.message}") + log.warn("JSON parsing limit exceeded: #{e.message}") end end end @@ -87,7 +86,7 @@ def load_handler(handler_class_name) private - # Safely parse JSON with security limits to prevent JSON bombs + # Safely parse JSON # # @param json_string [String] The JSON string to parse # @return [Hash, Array] Parsed JSON object diff --git a/spec/unit/lib/hooks/app/helpers_spec.rb b/spec/unit/lib/hooks/app/helpers_spec.rb index 80b11c5a..d50572e7 100644 --- a/spec/unit/lib/hooks/app/helpers_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_spec.rb @@ -236,7 +236,7 @@ def error!(message, code) # Mock logger to capture debug messages logger = instance_double("Logger") allow(helper).to receive(:log).and_return(logger) - expect(logger).to receive(:warn).with(/JSON parsing security limit exceeded/) + expect(logger).to receive(:warn).with(/JSON parsing limit exceeded/) # Mock the safe_json_parse method to simulate nesting limit exceeded allow(helper).to receive(:safe_json_parse).and_raise(ArgumentError, "nesting exceeded") From 2102b7c58af7932a800fc009329ebe75f914f828 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 11 Jun 2025 22:16:53 -0700 Subject: [PATCH 07/10] Remove unused max_create_depth variable from safe_json_parse method --- lib/hooks/app/helpers.rb | 1 - 1 file changed, 1 deletion(-) diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index bf76003b..bbe1659a 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -95,7 +95,6 @@ def load_handler(handler_class_name) def safe_json_parse(json_string) # Security limits for JSON parsing max_nesting = ENV.fetch("JSON_MAX_NESTING", "20").to_i - max_create_depth = ENV.fetch("JSON_MAX_CREATE_DEPTH", "15").to_i # Additional size check before parsing if json_string.length > ENV.fetch("JSON_MAX_SIZE", "10485760").to_i # 10MB default From 07c84293d719e91de9df6d15928950c7f29a81a2 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 11 Jun 2025 22:19:23 -0700 Subject: [PATCH 08/10] revert --- lib/hooks/utils/retry.rb | 19 +++---------------- 1 file changed, 3 insertions(+), 16 deletions(-) diff --git a/lib/hooks/utils/retry.rb b/lib/hooks/utils/retry.rb index d3ac82fc..cad5fc59 100644 --- a/lib/hooks/utils/retry.rb +++ b/lib/hooks/utils/retry.rb @@ -9,24 +9,11 @@ module Retry # Should the number of retries be reached without success, the last exception will be raised # # @param log [Logger] The logger to use for retryable logging - # @raise [ArgumentError] If no logger is provided or configuration values are invalid + # @raise [ArgumentError] If no logger is provided # @return [void] def self.setup!(log: nil, log_retries: ENV.fetch("RETRY_LOG_RETRIES", "true") == "true") raise ArgumentError, "a logger must be provided" if log.nil? - # Security: Validate and bound retry configuration values - retry_sleep = ENV.fetch("DEFAULT_RETRY_SLEEP", "1").to_i - retry_tries = ENV.fetch("DEFAULT_RETRY_TRIES", "10").to_i - - # Bounds checking to prevent resource exhaustion - if retry_sleep < 0 || retry_sleep > 300 # Max 5 minutes between retries - raise ArgumentError, "DEFAULT_RETRY_SLEEP must be between 0 and 300 seconds, got: #{retry_sleep}" - end - - if retry_tries < 1 || retry_tries > 50 # Max 50 retries to prevent infinite loops - raise ArgumentError, "DEFAULT_RETRY_TRIES must be between 1 and 50, got: #{retry_tries}" - end - log_method = lambda do |retries, exception| # :nocov: if log_retries @@ -41,8 +28,8 @@ def self.setup!(log: nil, log_retries: ENV.fetch("RETRY_LOG_RETRIES", "true") == Retryable.configure do |config| config.contexts[:default] = { on: [StandardError], - sleep: retry_sleep, - tries: retry_tries, + sleep: ENV.fetch("DEFAULT_RETRY_SLEEP", "1").to_i, + tries: ENV.fetch("DEFAULT_RETRY_TRIES", "10").to_i, log_method: } end From d97e8b80d69161f21a949dd929e9a0f711bd3d10 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 11 Jun 2025 22:19:34 -0700 Subject: [PATCH 09/10] remove --- docs/reliability.md | 262 -------------------------------------------- 1 file changed, 262 deletions(-) delete mode 100644 docs/reliability.md diff --git a/docs/reliability.md b/docs/reliability.md deleted file mode 100644 index 10f34aab..00000000 --- a/docs/reliability.md +++ /dev/null @@ -1,262 +0,0 @@ -# Production Reliability and Performance Guide - -This document outlines the reliability, performance, and security considerations for running the Hooks webhook server framework in production environments. - -## 🔍 Security Considerations - -### Dynamic Plugin Loading Security - -The framework includes comprehensive security measures for dynamic plugin loading: - -- **Class Name Validation**: All plugin class names are validated against safe patterns (`/\A[A-Z][a-zA-Z0-9_]*\z/`) -- **Dangerous Class Blacklist**: System classes like `File`, `Dir`, `Kernel`, `Object`, `Process`, etc. are blocked from being loaded as plugins -- **Path Traversal Protection**: Plugin file paths are normalized and validated to prevent loading files outside designated directories -- **Safe Constant Resolution**: Uses `Object.const_get` only after thorough validation - -### Request Processing Security - -- **Request Size Limits**: Configurable request body size limits (default enforcement via `request_limit` config) -- **JSON Parsing Protection**: JSON parsing includes security limits to prevent JSON bombs: - - Maximum nesting depth (configurable via `JSON_MAX_NESTING`, default: 20) - - Maximum payload size before parsing (configurable via `JSON_MAX_SIZE`, default: 10MB) - - Disabled object creation from JSON (`create_additions: false`) - - Uses plain Hash/Array classes to prevent object injection -- **Header Validation**: Multiple header format handling with safe fallbacks and optimized lookup order - -## ⚡ Performance Optimizations - -### Startup Performance - -The framework uses several strategies to optimize startup time: - -- **Explicit Module Loading**: Core modules are loaded explicitly rather than using `Dir.glob` patterns for better performance and security -- **Boot-time Plugin Loading**: All plugins are loaded once at startup rather than per-request -- **Plugin Caching**: Loaded plugins are cached in class-level registries for fast access -- **Sorted Directory Loading**: Plugin directories are processed in sorted order for consistent behavior - -### Runtime Performance - -- **Per-request Optimizations**: - - Plugin instances are reused across requests - - Request contexts use thread-local storage for efficient access - - Handler instances are created per-request but classes are cached - - Optimized header processing with common cases checked first - -- **Memory Management**: - - Plugin registries use hash-based lookups for O(1) access - - Thread-local contexts are properly cleaned up after requests - - Clear plugin loading separates concerns efficiently - -- **Security Limits**: - - Retry configuration includes bounds checking to prevent resource exhaustion - - JSON parsing has built-in limits to prevent JSON bombs and memory attacks - -### Recommended Production Configuration - -```yaml -# Example production configuration -log_level: "info" # Reduces debug overhead -request_limit: 1048576 # 1MB limit (adjust based on needs) -request_timeout: 30 # 30 second timeout -environment: "production" # Disables debug features like backtraces -normalize_headers: true # Consistent header processing -symbolize_payload: false # Reduced memory usage for large payloads -``` - -### Security Environment Variables - -Additional security can be configured via environment variables: - -```bash -# JSON Security Limits -JSON_MAX_NESTING=20 # Maximum JSON nesting depth (default: 20) -JSON_MAX_SIZE=10485760 # Maximum JSON size before parsing (default: 10MB) - -# Retry Safety Limits -DEFAULT_RETRY_SLEEP=1 # Sleep between retries 0-300 seconds (default: 1) -DEFAULT_RETRY_TRIES=10 # Number of retry attempts 1-50 (default: 10) -RETRY_LOG_RETRIES=false # Disable retry logging in production (default: true) -``` - -## 🔧 Monitoring and Observability - -### Health Check Endpoint - -The built-in health endpoint (`/health`) provides comprehensive status information: - -```json -{ - "status": "healthy", - "timestamp": "2025-01-01T12:00:00Z", - "version": "1.0.0", - "uptime_seconds": 3600, - "config_checksum": "abc123", - "endpoints_loaded": 5, - "plugins_loaded": 3 -} -``` - -### Lifecycle Hooks for Monitoring - -Use lifecycle plugins to add comprehensive monitoring: - -- **Request Metrics**: Track request counts, timing, and error rates -- **Error Reporting**: Capture and report exceptions with full context -- **Resource Monitoring**: Track memory usage, plugin load times, etc. - -### Recommended Instrumentation - -```ruby -# Example monitoring lifecycle plugin -class MonitoringLifecycle < Hooks::Plugins::Lifecycle - def on_request(env) - stats.increment("webhook.requests", { - handler: env["hooks.handler"], - endpoint: env["PATH_INFO"] - }) - end - - def on_response(env, response) - processing_time = Time.now - Time.parse(env["hooks.start_time"]) - stats.timing("webhook.processing_time", processing_time * 1000, { - handler: env["hooks.handler"] - }) - end - - def on_error(exception, env) - stats.increment("webhook.errors", { - error_type: exception.class.name, - handler: env["hooks.handler"] - }) - - failbot.report(exception, { - request_id: env["hooks.request_id"], - handler: env["hooks.handler"], - endpoint: env["PATH_INFO"] - }) - end -end -``` - -## 🚀 Production Deployment Best Practices - -### Server Configuration - -1. **Use Puma in Cluster Mode** for production: -```ruby -# config/puma.rb -workers ENV.fetch("WEB_CONCURRENCY", 2) -threads_count = ENV.fetch("MAX_THREADS", 5) -threads threads_count, threads_count -preload_app! -``` - -2. **Configure Resource Limits**: - - Set appropriate worker memory limits - - Configure worker restart thresholds - - Set connection pool sizes appropriately - -3. **Environment Variables**: -```bash -# Retry configuration -DEFAULT_RETRY_TRIES=3 # Reduced from default 10 -DEFAULT_RETRY_SLEEP=1 # 1 second between retries -RETRY_LOG_RETRIES=false # Reduce log noise in production - -# Logging -LOG_LEVEL=info # Reduce debug overhead -``` - -### Container Considerations - -```dockerfile -# Optimized production Dockerfile -FROM ruby:3.2-alpine AS builder -WORKDIR /app -COPY Gemfile* ./ -RUN bundle install --deployment --without development test - -FROM ruby:3.2-alpine -WORKDIR /app -COPY --from=builder /app/vendor ./vendor -COPY . . - -# Security: Run as non-root user -RUN addgroup -g 1001 -S appuser && \ - adduser -S appuser -u 1001 -G appuser -USER appuser - -# Health check -HEALTHCHECK --interval=30s --timeout=10s --start-period=5s --retries=3 \ - CMD curl -f http://localhost:3000/health || exit 1 - -EXPOSE 3000 -CMD ["bundle", "exec", "puma", "-C", "config/puma.rb"] -``` - -## 🛡️ Security Hardening - -### Input Validation - -- **Payload Size Limits**: Always configure `request_limit` appropriate for your use case -- **Timeout Configuration**: Set reasonable `request_timeout` values -- **Content Type Validation**: Implement strict content type checking if needed - -### Authentication - -- **HMAC Validation**: Always enable authentication for production endpoints -- **Secret Management**: Store webhook secrets in environment variables or secure secret management systems -- **Signature Validation**: Use time-based signature validation to prevent replay attacks - -### Network Security - -- **TLS Termination**: Always terminate TLS/SSL at load balancer or reverse proxy -- **IP Whitelisting**: Implement IP restrictions at network level when possible -- **Rate Limiting**: Implement rate limiting at reverse proxy/load balancer level - -## 📊 Performance Benchmarking - -### Load Testing Recommendations - -1. **Baseline Testing**: Test with minimal handlers and no lifecycle plugins -2. **Plugin Impact**: Measure performance impact of each lifecycle plugin -3. **Memory Profiling**: Monitor memory usage over extended periods -4. **Concurrency Testing**: Test with realistic concurrent webhook loads - -### Key Metrics to Monitor - -- **Request Processing Time**: P50, P95, P99 response times -- **Memory Usage**: RSS, heap size, GC frequency -- **Error Rates**: 4xx and 5xx response rates -- **Plugin Performance**: Individual plugin execution times -- **Resource Utilization**: CPU, memory, network I/O - -## 🔧 Troubleshooting - -### Common Performance Issues - -1. **High Memory Usage**: - - Check for plugin memory leaks - - Monitor payload sizes - - Review lifecycle plugin efficiency - -2. **Slow Request Processing**: - - Profile individual plugins - - Check JSON parsing performance - - Review handler implementation efficiency - -3. **Plugin Loading Issues**: - - Verify plugin directory permissions - - Check plugin class name formatting - - Review security validation errors - -### Debug Configuration - -For troubleshooting, temporarily enable debug logging: - -```yaml -log_level: "debug" -environment: "development" # Enables error backtraces -``` - -**Important**: Never run production with debug logging enabled long-term due to performance and security implications. \ No newline at end of file From 3a2e0d12bbb84d68dbc37da379eb8661ed81d704 Mon Sep 17 00:00:00 2001 From: GrantBirki Date: Wed, 11 Jun 2025 22:20:19 -0700 Subject: [PATCH 10/10] remove --- spec/unit/lib/hooks/utils/retry_spec.rb | 129 ------------------------ 1 file changed, 129 deletions(-) delete mode 100644 spec/unit/lib/hooks/utils/retry_spec.rb diff --git a/spec/unit/lib/hooks/utils/retry_spec.rb b/spec/unit/lib/hooks/utils/retry_spec.rb deleted file mode 100644 index 2ab0c297..00000000 --- a/spec/unit/lib/hooks/utils/retry_spec.rb +++ /dev/null @@ -1,129 +0,0 @@ -# frozen_string_literal: true - -require_relative "../../../spec_helper" - -# Import the tested class -require_relative "../../../../../lib/hooks/utils/retry" - -describe Retry do - let(:logger) { instance_double("Logger") } - - before do - # Reset any previous configuration - Retryable.configuration.contexts.clear - end - - describe ".setup!" do - context "with valid parameters" do - it "sets up retry configuration with default values" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("1") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("10") - allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("true") - - Retry.setup!(log: logger) - - expect(Retryable.configuration.contexts[:default]).to include( - sleep: 1, - tries: 10 - ) - end - - it "accepts custom retry configuration within bounds" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("5") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("3") - allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("false") - - Retry.setup!(log: logger) - - expect(Retryable.configuration.contexts[:default]).to include( - sleep: 5, - tries: 3 - ) - end - end - - context "with invalid parameters" do - it "raises ArgumentError when no logger is provided" do - expect do - Retry.setup!(log: nil) - end.to raise_error(ArgumentError, "a logger must be provided") - end - - it "raises ArgumentError when retry sleep is negative" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("-1") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("10") - - expect do - Retry.setup!(log: logger) - end.to raise_error(ArgumentError, /DEFAULT_RETRY_SLEEP must be between 0 and 300/) - end - - it "raises ArgumentError when retry sleep exceeds maximum" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("301") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("10") - - expect do - Retry.setup!(log: logger) - end.to raise_error(ArgumentError, /DEFAULT_RETRY_SLEEP must be between 0 and 300/) - end - - it "raises ArgumentError when retry tries is zero" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("1") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("0") - - expect do - Retry.setup!(log: logger) - end.to raise_error(ArgumentError, /DEFAULT_RETRY_TRIES must be between 1 and 50/) - end - - it "raises ArgumentError when retry tries exceeds maximum" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("1") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("51") - - expect do - Retry.setup!(log: logger) - end.to raise_error(ArgumentError, /DEFAULT_RETRY_TRIES must be between 1 and 50/) - end - end - - context "with boundary values" do - it "accepts minimum valid values" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("0") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("1") - allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("true") - - expect do - Retry.setup!(log: logger) - end.not_to raise_error - - expect(Retryable.configuration.contexts[:default]).to include( - sleep: 0, - tries: 1 - ) - end - - it "accepts maximum valid values" do - allow(ENV).to receive(:fetch).and_call_original - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_SLEEP", "1").and_return("300") - allow(ENV).to receive(:fetch).with("DEFAULT_RETRY_TRIES", "10").and_return("50") - allow(ENV).to receive(:fetch).with("RETRY_LOG_RETRIES", "true").and_return("true") - - expect do - Retry.setup!(log: logger) - end.not_to raise_error - - expect(Retryable.configuration.contexts[:default]).to include( - sleep: 300, - tries: 50 - ) - end - end - end -end