Skip to content

[FEATURE][SECURITY]: Implement IP-based access control (#536)#3226

Open
kcostell06 wants to merge 4 commits intoIBM:mainfrom
kcostell06:feature/ip-control-536
Open

[FEATURE][SECURITY]: Implement IP-based access control (#536)#3226
kcostell06 wants to merge 4 commits intoIBM:mainfrom
kcostell06:feature/ip-control-536

Conversation

@kcostell06
Copy link
Copy Markdown

@kcostell06 kcostell06 commented Feb 24, 2026 •
edited
Loading

Related Issue

Closes #536

Summary

Implements a generic IP-based access control system with:

  • Allowlist/blocklist modes with configurable default behavior
  • CIDR notation support for IPv4 and IPv6 address matching
  • Path-scoped rules using regex patterns for granular control
  • Priority-based evaluation where first matching rule wins
  • Temporary IP blocks with automatic expiration and cleanup
  • In-memory caching with configurable TTL and size-based eviction
  • Admin REST API for managing rules, blocks, testing IPs, and viewing status
  • Middleware integration with proxy header trust, log-only (dry-run) mode, and skip paths
  • 8 configuration settings via environment variables (disabled by default)

Files added

  • mcpgateway/middleware/ip_control.py - Request middleware with IP extraction and enforcement
  • mcpgateway/services/ip_control_service.py - Core service with evaluation, caching, and CRUD
  • mcpgateway/routers/ip_control_router.py - Admin API endpoints (rules, blocks, test, status, cache)
  • mcpgateway/alembic/versions/z9j0k1l2m3n4_add_ip_access_control_tables.py - Migration
  • Test files for all three layers (88 tests, 99% coverage)

Files modified

  • mcpgateway/config.py - Added 8 ip_control_* settings
  • mcpgateway/db.py - Added IPRule and IPBlock ORM models
  • mcpgateway/schemas.py - Added 9 Pydantic schemas
  • mcpgateway/main.py - Conditional middleware and router registration
  • .env.example - Added IP_CONTROL_* environment variables

Type of Change

  • Feature / Enhancement

Verification

Check Command Status
Lint suite make lint Pass
Unit tests make test Pass
Coverage >= 80% make coverage 99%

Checklist

  • Code formatted (make black isort pre-commit)
  • Tests added/updated for changes
  • No secrets or credentials committed

@claude
feat: implement IP-based access control (IBM#536)
e0ffd55 
Add generic IP-based access control system with allowlist/blocklist modes,
CIDR matching, path-scoped rules, temporary blocks, and admin API.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kelly Costello <kellycostello@Kellys-MacBook-Air.local>
Kelly Costello and others added 2 commits February 24, 2026 16:25
@claude
test: increase IP control test coverage to 99%
abe171e 
Add tests for service CRUD methods, evaluate_ip session management,
error/rollback paths, test_ip diagnostics, get_status, cleanup_expired_blocks,
router get_db error handling, list_blocks endpoint, and middleware unknown
client IP fallback.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kelly Costello <kellycostello@Kellys-MacBook-Air.local>
@claude
fix: add nosec B110 to ip_control_service hit count rollback
e56268b 
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kelly Costello <kellycostello@Kellys-MacBook-Air.local>
@crivetimihai crivetimihai changed the title feat: implement IP-based access control (#536) [FEATURE][SECURITY]: Implement IP-based access control (#536) Feb 25, 2026
@crivetimihai crivetimihai added enhancement New feature or request security Improves security COULD P3: Nice-to-have features with minimal impact if left out; included if time permits labels Feb 25, 2026
@crivetimihai crivetimihai added this to the Release 1.2.0 milestone Feb 25, 2026
@crivetimihai
Copy link
Copy Markdown
Member

crivetimihai commented Feb 25, 2026

Thanks for this @kcostell06. This is a comprehensive implementation with solid test coverage and clean separation across middleware/service/router layers. The feature-flag approach (disabled by default) is the right call. A few items: (1) full CI hasn't run yet — only DCO passed so far, please verify the full suite, (2) the migration filename z9j0k1l2m3n4_... should use an Alembic-generated revision ID (run alembic revision --autogenerate), (3) please verify the migration's down_revision points to the current head. Looking forward to iterating on this.

@claude
fix: use alembic-generated revision ID for IP control migration
bcc0507 
Replace manually-created revision z9j0k1l2m3n4 with alembic-generated
1d27bc0a81f5. Verified down_revision points to current head b2d9c6e4f1a7.
Also adds nosec B110 to hit count rollback pass.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Signed-off-by: Kelly Costello <kellycostello@Kellys-MacBook-Air.local>
@kcostell06
Copy link
Copy Markdown
Author

kcostell06 commented Feb 26, 2026

Thanks for the review! All three items have been addressed:

1. Migration revision ID — Replaced the manually-created z9j0k1l2m3n4 with Alembic-generated 1d27bc0a81f5 (via alembic revision -m "add_ip_access_control_tables"). The old file was removed and the new one carries the same idempotent upgrade/downgrade logic.

2. down_revision verified — Alembic itself confirmed b2d9c6e4f1a7 as the current head. Tested the full cycle: upgrade → downgrade → re-upgrade all succeed. No new migrations have landed on main since we branched, so there's no conflict.

3. Full test suite verified locally — 12,145 passed, 254 skipped, 0 failed (excluding the pre-existing test_translate_stdio_endpoint failure due to missing jq binary). IP control coverage is at 99% (88 tests across middleware/service/router). Bandit, flake8, interrogate, and pylint all pass clean.

New commits:

  • test: increase IP control test coverage to 99% — 670 lines of additional tests covering CRUD methods, error/rollback paths, session management, diagnostics, and edge cases
  • fix: add nosec B110 to ip_control_service hit count rollback — Bandit suppression for the intentional try/except/pass on best-effort hit count update
  • fix: use alembic-generated revision ID for IP control migration — Addresses the migration filename feedback

@kcostell06
Copy link
Copy Markdown
Author

kcostell06 commented Feb 26, 2026

The latest push should also trigger the full CI pipeline — previously only the DCO check had run. All changes are now pushed and ready for CI validation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@crivetimihai crivetimihai Awaiting requested review from crivetimihai crivetimihai is a code owner

@kevalmahajan kevalmahajan Awaiting requested review from kevalmahajan kevalmahajan is a code owner

@madhav165 madhav165 Awaiting requested review from madhav165 madhav165 is a code owner

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

COULD P3: Nice-to-have features with minimal impact if left out; included if time permits enhancement New feature or request security Improves security

Projects

None yet

Milestone

Release 1.1.0

Development

Successfully merging this pull request may close these issues.

[FEATURE][SECURITY]: Generic IP-based access control (allowlist)

2 participants

@kcostell06 @crivetimihai