Add comprehensive tests for security, log, handlers and endpoints

Co-authored-by: GrantBirki <[email protected]>

Author: Copilot

spec/unit/lib/hooks/app/endpoints/health_spec.rb

@@ -0,0 +1,77 @@
+# frozen_string_literal: true
+
+require "rack/test"
+
+describe Hooks::App::HealthEndpoint do
+  include Rack::Test::Methods
+
+  def app
+    described_class
+  end
+
+  before do
+    # Mock API start_time for consistent uptime calculation
+    allow(Hooks::App::API).to receive(:start_time).and_return(Time.parse("2024-12-31T23:59:00Z"))
+  end
+
+  describe "GET /" do
+    it "returns health status as JSON" do
+      get "/"
+
+      expect(last_response.status).to eq(200)
+      expect(last_response.headers["Content-Type"]).to include("application/json")
+    end
+
+    it "includes health status in response" do
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["status"]).to eq("healthy")
+    end
+
+    it "includes timestamp in ISO8601 format" do
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["timestamp"]).to eq(TIME_MOCK)
+    end
+
+    it "includes version information" do
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["version"]).to eq(Hooks::VERSION)
+    end
+
+    it "includes uptime in seconds" do
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["uptime_seconds"]).to be_a(Integer)
+      expect(response_data["uptime_seconds"]).to eq(60) # 1 minute difference
+    end
+
+    it "returns valid JSON structure" do
+      get "/"
+      
+      expect { JSON.parse(last_response.body) }.not_to raise_error
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data).to have_key("status")
+      expect(response_data).to have_key("timestamp")
+      expect(response_data).to have_key("version")
+      expect(response_data).to have_key("uptime_seconds")
+    end
+
+    it "calculates uptime correctly" do
+      # Test with different start time
+      different_start = Time.parse("2024-12-31T23:58:30Z")
+      allow(Hooks::App::API).to receive(:start_time).and_return(different_start)
+      
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["uptime_seconds"]).to eq(90) # 1.5 minutes difference
+    end
+  end
+end

spec/unit/lib/hooks/app/endpoints/version_spec.rb

@@ -0,0 +1,51 @@
+# frozen_string_literal: true
+
+require "rack/test"
+
+describe Hooks::App::VersionEndpoint do
+  include Rack::Test::Methods
+
+  def app
+    described_class
+  end
+
+  describe "GET /" do
+    it "returns version information as JSON" do
+      get "/"
+
+      expect(last_response.status).to eq(200)
+      expect(last_response.headers["Content-Type"]).to include("application/json")
+    end
+
+    it "includes version number in response" do
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["version"]).to eq(Hooks::VERSION)
+    end
+
+    it "includes timestamp in ISO8601 format" do
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["timestamp"]).to eq(TIME_MOCK)
+    end
+
+    it "returns valid JSON structure" do
+      get "/"
+      
+      expect { JSON.parse(last_response.body) }.not_to raise_error
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data).to have_key("version")
+      expect(response_data).to have_key("timestamp")
+    end
+
+    it "version matches expected format" do
+      get "/"
+      
+      response_data = JSON.parse(last_response.body)
+      expect(response_data["version"]).to match(/^\d+\.\d+\.\d+$/)
+    end
+  end
+end

spec/unit/lib/hooks/core/log_spec.rb

@@ -0,0 +1,36 @@
+# frozen_string_literal: true
+
+describe Hooks::Log do
+  describe ".instance" do
+    it "can be set and retrieved" do
+      logger = instance_double(Logger)
+      described_class.instance = logger
+      
+      expect(described_class.instance).to eq(logger)
+    end
+
+    it "can be set to nil" do
+      described_class.instance = nil
+      
+      expect(described_class.instance).to be_nil
+    end
+
+    it "maintains the same instance when set" do
+      logger = instance_double(Logger)
+      described_class.instance = logger
+      
+      expect(described_class.instance).to be(logger)
+    end
+
+    it "can be overridden" do
+      first_logger = instance_double(Logger)
+      second_logger = instance_double(Logger)
+      
+      described_class.instance = first_logger
+      expect(described_class.instance).to eq(first_logger)
+      
+      described_class.instance = second_logger
+      expect(described_class.instance).to eq(second_logger)
+    end
+  end
+end

spec/unit/lib/hooks/handlers/default_spec.rb

@@ -0,0 +1,99 @@
+# frozen_string_literal: true
+
+describe DefaultHandler do
+  let(:log) { instance_double(Logger).as_null_object }
+  let(:payload) { { "action" => "opened", "number" => 1 } }
+  let(:headers) { { "X-GitHub-Event" => "pull_request" } }
+  let(:config) { { environment: "test" } }
+  let(:handler) { described_class.new }
+
+  before do
+    allow(handler).to receive(:log).and_return(log)
+  end
+
+  describe "#call" do
+    context "with valid parameters" do
+      let(:result) { handler.call(payload:, headers:, config:) }
+
+      it "logs that the default handler was invoked" do
+        result
+
+        expect(log).to have_received(:info).with("🔔 Default handler invoked for webhook 🔔")
+      end
+
+      it "logs payload debug information when payload is present" do
+        result
+
+        expect(log).to have_received(:debug).with("received payload: #{payload.inspect}")
+      end
+
+      it "returns a success response hash" do
+        expect(result).to be_a(Hash)
+        expect(result).to include(
+          message: "webhook processed successfully",
+          handler: "DefaultHandler",
+          timestamp: TIME_MOCK
+        )
+      end
+
+      it "includes timestamp in ISO8601 format" do
+        expect(result[:timestamp]).to eq(TIME_MOCK)
+      end
+    end
+
+    context "with nil payload" do
+      let(:payload) { nil }
+      let(:result) { handler.call(payload:, headers:, config:) }
+
+      it "does not log payload debug information" do
+        result
+
+        expect(log).not_to have_received(:debug)
+      end
+
+      it "still returns a success response" do
+        expect(result).to include(
+          message: "webhook processed successfully",
+          handler: "DefaultHandler"
+        )
+      end
+    end
+
+    context "with empty payload" do
+      let(:payload) { {} }
+      let(:result) { handler.call(payload:, headers:, config:) }
+
+      it "logs the empty payload" do
+        result
+
+        expect(log).to have_received(:debug).with("received payload: #{payload.inspect}")
+      end
+    end
+
+    context "with complex payload" do
+      let(:payload) do
+        {
+          "action" => "opened",
+          "pull_request" => {
+            "id" => 123,
+            "title" => "Test PR",
+            "body" => "This is a test"
+          }
+        }
+      end
+      let(:result) { handler.call(payload:, headers:, config:) }
+
+      it "logs the complex payload structure" do
+        result
+
+        expect(log).to have_received(:debug).with("received payload: #{payload.inspect}")
+      end
+    end
+  end
+
+  describe "inheritance" do
+    it "inherits from Hooks::Handlers::Base" do
+      expect(described_class.superclass).to eq(Hooks::Handlers::Base)
+    end
+  end
+end

spec/unit/lib/hooks/security_spec.rb

@@ -0,0 +1,48 @@
+# frozen_string_literal: true
+
+describe Hooks::Security do
+  describe "DANGEROUS_CLASSES" do
+    it "is frozen to prevent modification" do
+      expect(described_class::DANGEROUS_CLASSES).to be_frozen
+    end
+
+    it "contains system-access classes" do
+      expected_classes = %w[
+        File Dir Kernel Object Class Module Proc Method
+        IO Socket TCPSocket UDPSocket BasicSocket
+        Process Thread Fiber Mutex ConditionVariable
+        Marshal YAML JSON Pathname
+      ]
+
+      expect(described_class::DANGEROUS_CLASSES).to match_array(expected_classes)
+    end
+
+    it "contains file system classes" do
+      expect(described_class::DANGEROUS_CLASSES).to include("File", "Dir", "Pathname")
+    end
+
+    it "contains network classes" do
+      expect(described_class::DANGEROUS_CLASSES).to include("Socket", "TCPSocket", "UDPSocket", "BasicSocket")
+    end
+
+    it "contains process control classes" do
+      expect(described_class::DANGEROUS_CLASSES).to include("Process", "Thread", "Fiber")
+    end
+
+    it "contains serialization classes" do
+      expect(described_class::DANGEROUS_CLASSES).to include("Marshal", "YAML", "JSON")
+    end
+
+    it "contains core Ruby classes that provide system access" do
+      expect(described_class::DANGEROUS_CLASSES).to include("Kernel", "Object", "Class", "Module")
+    end
+
+    it "prevents empty string attacks" do
+      expect(described_class::DANGEROUS_CLASSES).not_to include("")
+    end
+
+    it "prevents nil attacks" do
+      expect(described_class::DANGEROUS_CLASSES).not_to include(nil)
+    end
+  end
+end