Security audit: Fix critical authentication bypass and class injection vulnerabilities

[Copilot]

This PR addresses critical security vulnerabilities found during a comprehensive security audit of the webhook server framework's authentication and handler loading mechanisms.

Critical Vulnerabilities Fixed

1. Authentication Bypass (CVE-level severity)

Issue: Missing secret_env_key in endpoint configuration completely bypassed authentication validation, allowing unauthenticated requests to be processed.

# Before: This would bypass auth entirely
endpoint_config = { 
  auth: { type: "hmac" }  # Missing secret_env_key
}

# After: Now properly validates and rejects

Fix: Enhanced validate_auth! to require proper authentication configuration and reject incomplete auth setups.

2. Dynamic Class Loading Vulnerability (RCE Risk)

Issue: Handler names from configuration were passed directly to Object.const_get(), enabling arbitrary class instantiation and potential remote code execution.

# Before: Attacker could set handler to dangerous classes
config = { handler: "File" }     # Could instantiate File class
config = { handler: "Kernel" }   # Could instantiate Kernel class
config = { handler: "Process" }  # Could instantiate Process class

# After: Strict validation prevents dangerous class names

Fix:

• Added whitelist-based handler name validation (alphanumeric + underscore, starting with uppercase)
• Blacklisted dangerous system classes (File, Kernel, Process, IO, Socket, etc.)
• Added inheritance validation to ensure handlers extend Hooks::Handlers::Base
• Implemented path traversal prevention in handler file loading

3. Information Disclosure

Issue: Error messages leaked environment variable names, potentially revealing sensitive configuration details.

# Before: "secret 'SUPER_SECRET_WEBHOOK_KEY' not found in environment"
# After: "authentication secret not configured"

Security Enhancements

Authentication Hardening

• Mandatory validation when auth configuration is present
• Generic error messages prevent information leakage
• Enhanced input sanitization for auth parameters
• Comprehensive edge case handling

Handler Security

• Configuration-level validation catches malicious handler names early
• Runtime validation prevents dangerous class instantiation
• File system path validation prevents directory traversal
• Type checking ensures proper inheritance hierarchy

Input Validation

• Strict pattern matching for handler names: /\A[A-Z][a-zA-Z0-9_]*\z/
• Rejection of control characters and special symbols
• Validation of auth type and secret configuration completeness
• Protection against null byte injection and whitespace manipulation

Test Coverage

Added 42 new security-focused test cases covering:

• Authentication bypass scenarios
• Dynamic class loading exploitation attempts
• Input sanitization and edge cases
• Configuration validation security
• Error message information disclosure prevention

Files Changed

• lib/hooks/app/auth/auth.rb: Fixed authentication bypass vulnerability
• lib/hooks/app/helpers.rb: Secured dynamic handler loading
• lib/hooks/core/config_validator.rb: Added handler name validation
• 4 new security test files with comprehensive coverage

Verification

All security tests pass and the framework maintains 81%+ test coverage. The fixes are minimal and focused, preserving existing functionality while closing critical security gaps.

Security Impact: These fixes prevent potential remote code execution, authentication bypass, and information disclosure vulnerabilities that could compromise webhook server deployments.

Fixes #9.

[Copilot]

Pull Request Overview

This PR strengthens security around authentication and handler loading by enforcing stricter configuration validation, sanitization, and class loading checks.

• Enforces presence and completeness of authentication config in validate_auth!, with generic error messages to avoid information disclosure.
• Implements whitelist-based handler name validation, directory traversal prevention, and inheritance checks in both the config validator and load_handler.
• Adds comprehensive security-focused specs for auth validators, endpoint config validation, and handler loading.

Reviewed Changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file

File	Description
lib/hooks/app/auth/auth.rb	Added checks for auth type and secret_env_key presence and generic error msgs
lib/hooks/app/helpers.rb	Added valid_handler_class_name?, path traversal prevention, and inheritance checks
lib/hooks/core/config_validator.rb	Introduced valid_handler_name? and integrated it into endpoint validation
spec/unit/lib/hooks/plugins/auth/security_validators_spec.rb	New security tests for HMAC and SharedSecret validators
spec/unit/lib/hooks/core/config_validator_security_spec.rb	New security tests for endpoint config validation
spec/unit/lib/hooks/app/helpers_security_spec.rb	New security tests for handler loading
spec/unit/lib/hooks/app/auth/auth_security_spec.rb	New security tests for validate_auth!

[Copilot]

[nitpick] The dangerous_classes list is duplicated in both helpers and the config validator. Extract this list into a shared constant or module to adhere to DRY and simplify future updates.

[Copilot]

[nitpick] This dangerous_classes array mirrors the one in Helpers. Consider extracting it into a shared constant to avoid duplication and ensure consistency.

Suggested change

	dangerous_classes = %w[
	File Dir Kernel Object Class Module Proc Method
	IO Socket TCPSocket UDPSocket BasicSocket
	Process Thread Fiber Mutex ConditionVariable
	Marshal YAML JSON Pathname
	]
	return false if dangerous_classes.include?(handler_name)
	return false if DANGEROUS_CLASSES.include?(handler_name)

[Copilot]

[nitpick] Client-side configuration errors (e.g., missing or invalid auth config) are more appropriately represented with a 400 series status code. Consider using 400 instead of 500 for these checks.

Suggested change

	error!("authentication configuration missing or invalid", 500)
	error!("authentication configuration missing or invalid", 400)