Skip to content

Implement application-level IP filtering with allowlist/blocklist support #57

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 12 commits into from
Jun 18, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 17, 2025

This PR implements comprehensive application-level IP filtering functionality for the Hooks service, allowing both global and endpoint-specific IP-based access control.

Overview

The implementation adds IP filtering as a "pre-flight" operation in the request processing pipeline, supporting both allowlist and blocklist configurations with CIDR notation support.

Key Features

  • Allowlist and Blocklist Support: Configure allowed and denied IP addresses/ranges
  • CIDR Notation: Support for network ranges (e.g., 192.168.1.0/24)
  • Configurable IP Headers: Defaults to X-Forwarded-For but supports custom headers
  • Global and Endpoint-Level Configuration: Apply filtering globally or per-endpoint
  • Blocklist Precedence: Blocklist rules take precedence over allowlist rules
  • Comprehensive Error Handling: Returns HTTP 403 with detailed error messages

Configuration Examples

Endpoint-Level Configuration

# config/endpoints/example.yml
path: /secure-endpoint
handler: example_handler

ip_filtering:
  ip_header: X-Real-IP  # Optional, defaults to X-Forwarded-For
  allowlist:
    - "127.0.0.1"
    - "192.168.1.0/24"
  blocklist:
    - "192.168.1.100"

Global Configuration

# hooks.yml
ip_filtering:
  allowlist:
    - "10.0.0.0/8"
  blocklist:
    - "10.0.0.1"

Implementation Details

  • Core Logic: lib/hooks/app/network/ip_filtering.rb - Main IP filtering functionality
  • Integration: Added to request processing pipeline in lib/hooks/app/api.rb
  • Configuration: Updated schemas in lib/hooks/core/config_validator.rb to validate IP filtering config
  • Error Handling: Uses existing Hooks::Plugins::Handlers::Error for consistent error responses

Security Considerations

⚠️ Important: This filtering operates at the application layer and trusts proxy headers like X-Forwarded-For. It should only be used by those who understand the security implications and have proper network-level controls in place. For production security, consider implementing IP filtering at the network or load balancer level.

Testing

  • 25 Unit Tests: Comprehensive coverage of all IP filtering scenarios
  • 7 Acceptance Tests: End-to-end validation in Docker environment
  • Edge Cases: Handles invalid IPs, missing headers, empty configurations, and malformed data
  • Coverage: 99.9% test coverage maintained

Example Usage

# Allowed IP - succeeds
curl -H "X-Forwarded-For: 192.168.1.50" http://localhost:8080/webhooks/secure-endpoint

# Blocked IP - returns 403
curl -H "X-Forwarded-For: 192.168.1.100" http://localhost:8080/webhooks/secure-endpoint
# Response: {"error":"ip_filtering_failed","message":"IP address not allowed","request_id":"..."}

Fixes #56.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] task: implement application based IP filtering Implement application-level IP filtering with allowlist/blocklist support Jun 17, 2025
@Copilot Copilot AI requested a review from GrantBirki June 17, 2025 22:14
@GrantBirki GrantBirki marked this pull request as ready for review June 18, 2025 01:29
@Copilot Copilot AI review requested due to automatic review settings June 18, 2025 01:29
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds comprehensive application-level IP filtering to the Hooks service, enabling both global and endpoint-specific allowlist/blocklist support with CIDR notation and configurable headers.

  • Introduces a new IpFiltering class with core allowlist/blocklist logic and CIDR support
  • Integrates IP filtering into the API request pipeline
  • Updates configuration schemas and adds extensive unit and acceptance tests

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
lib/hooks/app/network/ip_filtering.rb Core IP filtering logic with allowlist, blocklist, CIDR support
lib/hooks/app/api.rb Hooks IP filtering into request pipeline
lib/hooks/core/config_validator.rb Adds ip_filtering schema for global and endpoint configs
lib/hooks/app/helpers.rb Adds helper method to invoke IP filtering
spec/unit/app/network/ip_filtering_spec.rb Unit tests covering IP filtering scenarios
spec/acceptance/config/endpoints/*.yml Acceptance configs for direct and custom-header examples
spec/acceptance/acceptance_tests.rb Acceptance tests validating IP filtering behavior
.bundle/config CI bundle path/deployment settings update
Comments suppressed due to low confidence (2)

lib/hooks/app/network/ip_filtering.rb:21

  • The docstring says missing or invalid config raises an error, but the code simply returns for missing config and skips invalid patterns. Update the comment to reflect the actual behavior.
        # If the IP filtering configuration is missing or invalid, it raises an error.

.bundle/config:3

  • This CI-specific bundle configuration may not be relevant to all contributors. Consider removing or git-ignoring .bundle/config changes to avoid environmental drift.
BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle"

@GrantBirki
Copy link
Member

@copilot please pickup where you left off. Everything looks great but test coverage is not at 100%. Run script/test and fix the results. Also, please update the docs/ip_filtering.md with the cumulative summary of these changes being made, and how to use them.

Copy link
Contributor Author

Copilot AI commented Jun 18, 2025

@copilot please pickup where you left off. Everything looks great but test coverage is not at 100%. Run script/test and fix the results. Also, please update the docs/ip_filtering.md with the cumul...

Done! Fixed the test coverage issue and completed the documentation.

Changes made:

  1. 100% Test Coverage: Added unit test for the ip_filtering! helper method in helpers_spec.rb - this was the missing line causing 99.9% coverage
  2. Comprehensive Documentation: Created complete documentation in docs/ip_filtering.md with:
    • Configuration examples (global and endpoint-level)
    • CIDR notation support details
    • Security considerations and best practices
    • Troubleshooting guide
    • Migration guide from custom auth plugins

All tests pass, coverage is now 100% (971/971 lines), and linting passes. The IP filtering feature is fully documented and ready for use.

Commit: 05a6dc8

Copilot finished work on behalf of GrantBirki June 18, 2025 01:45
@GrantBirki GrantBirki merged commit 6c5329d into main Jun 18, 2025
22 checks passed
@GrantBirki GrantBirki deleted the copilot/fix-56 branch June 18, 2025 04:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

task: implement application based IP filtering
2 participants