Skip to content

Update Makefile and pyproject.toml with packaging steps#3

Merged
crivetimihai merged 1 commit intomainfrom
linting
May 27, 2025
Merged

Update Makefile and pyproject.toml with packaging steps#3
crivetimihai merged 1 commit intomainfrom
linting

Conversation

@crivetimihai
Copy link
Copy Markdown
Member

No description provided.

@crivetimihai crivetimihai merged commit 7d2cc6f into main May 27, 2025
0 of 3 checks passed
@crivetimihai crivetimihai deleted the linting branch June 2, 2025 06:14
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
Update Makefile and pyproject.toml with packaging steps

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 14, 2025
Update Makefile and pyproject.toml with packaging steps
vk-playground pushed a commit to vk-playground/mcp-context-forge that referenced this pull request Sep 16, 2025
Update Makefile and pyproject.toml with packaging steps
Signed-off-by: Vicky Kuo <vicky.kuo@ibm.com>
crivetimihai added a commit that referenced this pull request Feb 3, 2026
Critical fixes for load test failures at 4000 concurrent users:

Issue #1 - Transaction leak in streamablehttp_transport.py (CRITICAL):
- Add explicit asyncio.CancelledError handling in get_db() context manager
- When MCP handlers are cancelled (client disconnect, timeout), the finally
  block may not execute properly, leaving transactions "idle in transaction"
- Now explicitly rollback and close before re-raising CancelledError
- Add rollback in direct SessionLocal usage at line ~1425

Issue #2 - Missing db parameter in admin routes (HIGH):
- Add `db: Session = Depends(get_db)` to 73 remaining admin routes
- Routes with @require_permission but no db param caused decorator to
  create fresh session via fresh_db_session() for EVERY permission check
- This doubled connection usage for affected routes under load

Issue #3 - Slow recovery from transaction leaks (MEDIUM):
- Reduce IDLE_TRANSACTION_TIMEOUT from 300s to 30s in docker-compose.yml
- Reduce CLIENT_IDLE_TIMEOUT from 300s to 60s
- Leaked transactions now killed faster, preventing pool exhaustion

Root cause confirmed: list_resources() MCP handler was primary source,
with 155+ connections stuck on `SELECT resources.*` for up to 273 seconds.

See todo/rca2.md for full analysis including live test data showing
connection leak progression and 606+ idle transaction timeout errors.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
crivetimihai added a commit that referenced this pull request Feb 3, 2026
* feat(api): standardize gateway response format

- Set *_unmasked fields to null in GatewayRead.masked()
- Apply masking consistently across all gateway return paths
- Mask credentials on cache reads
- Update admin UI to indicate stored secrets are write-only
- Update tests to verify masking behavior

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* delete artifact sbom

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(gateway): add configurable URL validation for gateway endpoints

Add comprehensive URL validation with configurable network access controls
for gateway and tool URL endpoints. This allows operators to control which
network ranges are accessible based on their deployment environment.

New configuration options:
- SSRF_PROTECTION_ENABLED: Master switch for URL validation (default: true)
- SSRF_ALLOW_LOCALHOST: Allow localhost/loopback (default: true for dev)
- SSRF_ALLOW_PRIVATE_NETWORKS: Allow RFC 1918 ranges (default: true)
- SSRF_DNS_FAIL_CLOSED: Reject unresolvable hostnames (default: false)
- SSRF_BLOCKED_NETWORKS: CIDR ranges to always block
- SSRF_BLOCKED_HOSTS: Hostnames to always block

Features:
- Validates all resolved IP addresses (A and AAAA records)
- Normalizes hostnames (case-insensitive, trailing dot handling)
- Blocks cloud metadata endpoints by default (169.254.169.254, etc.)
- Dev-friendly defaults with strict mode available for production
- Full documentation and Helm chart support

Also includes minor admin UI formatting improvements.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(auth): add token-scoped filtering for list endpoints and gateway forwarding

- Add token_teams parameter to list_servers and list_gateways endpoints
  for proper scoping based on JWT token team claims
- Update server_service.list_servers() and gateway_service.list_gateways()
  to filter results by token scope (public-only, team-scoped, or unrestricted)
- Skip caching for token-scoped queries to prevent cross-user data leakage
- Update gateway forwarding (_forward_request_to_all) to respect token team scope
- Fix public-only token handling in create endpoints (tools, resources, prompts,
  servers, gateways, A2A agents) to reject team/private visibility
- Preserve None vs [] distinction in SSE/WebSocket for proper admin bypass
- Update get_team_from_token to distinguish missing teams (legacy fallback)
  from explicit empty teams (public-only access)
- Add request.state.token_teams storage in all auth paths for downstream access

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(auth): add normalize_token_teams for consistent token scoping

Introduces a centralized `normalize_token_teams()` function in auth.py
that provides consistent token team normalization across all code paths:

- Missing teams key → empty list (public-only access)
- Explicit null teams + admin flag → None (admin bypass)
- Explicit null teams without admin → empty list (public-only)
- Empty teams array → empty list (public-only)
- Team list → normalized string IDs (team-scoped)

Additional changes:
- Update _get_token_teams_from_request() to use normalized teams
- Fix caching in server/gateway services to only cache public-only queries
- Fix server creation visibility parameter precedence
- Update token_scoping middleware to use normalize_token_teams()
- Add comprehensive unit tests for token normalization behavior

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(websocket): forward auth credentials to /rpc endpoint

The WebSocket /ws endpoint now propagates authentication credentials
when making internal requests to /rpc:

- Forward JWT token as Authorization header when present
- Forward proxy user header when trust_proxy_auth is enabled
- Enables WebSocket transport to work with AUTH_REQUIRED=true

Also adds unit tests to verify auth credential forwarding behavior.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(rbac): add granular permission checks to all admin routes

- Add @require_permission decorators to all 177 admin routes with
  allow_admin_bypass=False to enforce explicit permission checks
- Add allow_admin_bypass parameter to require_permission and
  require_any_permission decorators for configurable admin bypass
- Add has_admin_permission() method to PermissionService for checking
  admin-level access (is_admin, *, or admin.* permissions)
- Update AdminAuthMiddleware to use has_admin_permission() for
  coarse-grained admin UI access control
- Create shared test fixtures in tests/unit/mcpgateway/conftest.py
  for mocking PermissionService across unit tests
- Update test files to use proper user context dict format

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* docs(rbac): comprehensive update to authentication and RBAC documentation

Update documentation to accurately reflect the two-layer security model
(Token Scoping + RBAC) and correct token scoping behavior.

rbac.md:
- Rewrite overview with two-layer security model explanation
- Fix token scoping matrix (missing teams key = PUBLIC-ONLY, not UNRESTRICTED)
- Add admin bypass requirements warning (requires BOTH teams:null AND is_admin:true)
- Add public-only token limitations (cannot access private resources even if owned)
- Add Permission System section with categories and fallback permissions
- Add Configuration Safety section (AUTH_REQUIRED, TRUST_PROXY_AUTH warnings)
- Update enforcement points matrix with Token Scoping and RBAC columns

multitenancy.md:
- Add Token Scoping Model section with secure-first defaults
- Add Two-Layer Security Model section with request flow diagram
- Add Enforcement Points Matrix
- Add Token Scoping Invariants
- Document multi-team token behavior (first team used for request.state.team_id)

oauth-design.md & oauth-authorization-code-ui-design.md:
- Add scope clarification notes (gateway OAuth delegation vs user auth)
- Add Token Verification section
- Add cross-references to RBAC and multitenancy docs

AGENTS.md:
- Add Authentication & RBAC Overview section with quick reference

llms/mcpgateway.md & llms/api.md:
- Add token scoping quick reference and examples
- Add links to full documentation

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(rbac): add explicit db dependency to RBAC-protected routes

Address load test findings from RCA #1 and #2:

- Add `db: Session = Depends(get_db)` to routes in email_auth.py,
  llm_config_router.py, and teams.py that use @require_permission
- Fix test files to pass mock_db parameter after signature changes
- Add shm_size: 256m to PostgreSQL in docker-compose.yml
- Remove non-serializable content from resource update events
- Disable CircuitBreaker plugin for consistent load testing

These changes fix the NoneType errors (~33,700) observed under 4000
concurrent users where current_user_ctx["db"] was always None.

Remaining critical issue: Transaction leak in streamablehttp_transport.py
causing idle-in-transaction connections (see todo/rca2.md for details).

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(db): resolve transaction leak and connection pool exhaustion

Critical fixes for load test failures at 4000 concurrent users:

Issue #1 - Transaction leak in streamablehttp_transport.py (CRITICAL):
- Add explicit asyncio.CancelledError handling in get_db() context manager
- When MCP handlers are cancelled (client disconnect, timeout), the finally
  block may not execute properly, leaving transactions "idle in transaction"
- Now explicitly rollback and close before re-raising CancelledError
- Add rollback in direct SessionLocal usage at line ~1425

Issue #2 - Missing db parameter in admin routes (HIGH):
- Add `db: Session = Depends(get_db)` to 73 remaining admin routes
- Routes with @require_permission but no db param caused decorator to
  create fresh session via fresh_db_session() for EVERY permission check
- This doubled connection usage for affected routes under load

Issue #3 - Slow recovery from transaction leaks (MEDIUM):
- Reduce IDLE_TRANSACTION_TIMEOUT from 300s to 30s in docker-compose.yml
- Reduce CLIENT_IDLE_TIMEOUT from 300s to 60s
- Leaked transactions now killed faster, preventing pool exhaustion

Root cause confirmed: list_resources() MCP handler was primary source,
with 155+ connections stuck on `SELECT resources.*` for up to 273 seconds.

See todo/rca2.md for full analysis including live test data showing
connection leak progression and 606+ idle transaction timeout errors.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): use consistent user context format across all endpoints

- Update request_to_join_team and leave_team to use dict-based user context
- Fix teams router to use get_current_user_with_permissions consistently
- Move /discover route before /{team_id} to prevent route shadowing
- Update test fixtures to use mock_user_context dict format
- Add transaction commits in resource_service to prevent connection leaks
- Add missing docstring parameters for flake8 compliance

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(db): add explicit db.commit/close to prevent transaction leaks

Add explicit db.commit(); db.close() calls to 100+ endpoints across
all routers to prevent PostgreSQL connection leaks under high load.

Problem: Under high concurrency, FastAPI's Depends(get_db) cleanup
runs after response serialization, causing transactions to remain
in 'idle in transaction' state for 20-30+ seconds, exhausting the
connection pool.

Solution: Explicitly commit and close database sessions immediately
after database operations complete, before response serialization.

Routers fixed:
- tokens.py: 10 endpoints (create, list, get, update, revoke, usage, admin, team tokens)
- llm_config_router.py: 14 endpoints (provider/model CRUD, health, gateway models)
- sso.py: 5 endpoints (SSO provider CRUD)
- email_auth.py: 3 endpoints (user create/update/delete)
- oauth_router.py: 1 endpoint (delete_registered_client)
- teams.py: 18 endpoints (team CRUD, members, invitations, join requests)
- rbac.py: 12 endpoints (roles, user roles, permissions)
- main.py: 14 CUD + 3 list + 7 RPC handlers

Also fixes:
- admin.py: Rename 21 unused db params to _db (pylint W0613)
- test_teams*.py: Add mock_db fixture to tests calling router functions directly
- Add llms/audit-db-transaction-management.md for future audits

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* ci(coverage): lower doctest coverage threshold to 30%

Reduce the required doctest coverage from 34% to 30% to accommodate
current coverage levels (32.17%).

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(rpc): fix list_gateways tuple unpacking and add token scoping

The RPC list_gateways handler had two bugs:
1. Did not unpack the tuple (gateways, next_cursor) returned by
   gateway_service.list_gateways(), causing 'list' object has no
   attribute 'model_dump' error
2. Was missing token scoping via _get_rpc_filter_context(), which
   was the original R-02 security fix

Also fixed all callers of list_gateways that expected a list but
now receive a tuple:
- mcpgateway/admin.py: get_gateways_section()
- mcpgateway/services/import_service.py: 3 call sites

Updated test mocks to return (list, None) tuples instead of lists.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): build response before db.close() to avoid lazy-load errors

The teams router was calling db.commit(); db.close() before building
the TeamResponse, but TeamResponse includes team.get_member_count()
which needs an active session. When the session is closed, the fallback
in get_member_count() tries to access self.members (lazy-loaded),
causing "Parent instance is not bound to a Session" errors.

Fixed by building TeamResponse BEFORE calling db.close() in:
- create_team
- get_team
- update_team

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): fix update_team expecting team object but getting bool

The service's update_team() returns bool, but the router was treating
the return value as a team object and trying to access .id, .name, etc.

Fixed by:
1. Checking the boolean return value for success
2. Fetching the team again after successful update to build the response

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): fix update_member_role return type mismatch

The service's update_member_role() returns bool, but the router
treated it as a member object. Fixed by:
1. Checking the boolean success
2. Added get_member() method to TeamManagementService
3. Fetching the updated member to build the response

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* Fix teams return

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
hughhennelly pushed a commit to hughhennelly/mcp-context-forge that referenced this pull request Feb 8, 2026
* feat(api): standardize gateway response format

- Set *_unmasked fields to null in GatewayRead.masked()
- Apply masking consistently across all gateway return paths
- Mask credentials on cache reads
- Update admin UI to indicate stored secrets are write-only
- Update tests to verify masking behavior

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* delete artifact sbom

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(gateway): add configurable URL validation for gateway endpoints

Add comprehensive URL validation with configurable network access controls
for gateway and tool URL endpoints. This allows operators to control which
network ranges are accessible based on their deployment environment.

New configuration options:
- SSRF_PROTECTION_ENABLED: Master switch for URL validation (default: true)
- SSRF_ALLOW_LOCALHOST: Allow localhost/loopback (default: true for dev)
- SSRF_ALLOW_PRIVATE_NETWORKS: Allow RFC 1918 ranges (default: true)
- SSRF_DNS_FAIL_CLOSED: Reject unresolvable hostnames (default: false)
- SSRF_BLOCKED_NETWORKS: CIDR ranges to always block
- SSRF_BLOCKED_HOSTS: Hostnames to always block

Features:
- Validates all resolved IP addresses (A and AAAA records)
- Normalizes hostnames (case-insensitive, trailing dot handling)
- Blocks cloud metadata endpoints by default (169.254.169.254, etc.)
- Dev-friendly defaults with strict mode available for production
- Full documentation and Helm chart support

Also includes minor admin UI formatting improvements.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(auth): add token-scoped filtering for list endpoints and gateway forwarding

- Add token_teams parameter to list_servers and list_gateways endpoints
  for proper scoping based on JWT token team claims
- Update server_service.list_servers() and gateway_service.list_gateways()
  to filter results by token scope (public-only, team-scoped, or unrestricted)
- Skip caching for token-scoped queries to prevent cross-user data leakage
- Update gateway forwarding (_forward_request_to_all) to respect token team scope
- Fix public-only token handling in create endpoints (tools, resources, prompts,
  servers, gateways, A2A agents) to reject team/private visibility
- Preserve None vs [] distinction in SSE/WebSocket for proper admin bypass
- Update get_team_from_token to distinguish missing teams (legacy fallback)
  from explicit empty teams (public-only access)
- Add request.state.token_teams storage in all auth paths for downstream access

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(auth): add normalize_token_teams for consistent token scoping

Introduces a centralized `normalize_token_teams()` function in auth.py
that provides consistent token team normalization across all code paths:

- Missing teams key → empty list (public-only access)
- Explicit null teams + admin flag → None (admin bypass)
- Explicit null teams without admin → empty list (public-only)
- Empty teams array → empty list (public-only)
- Team list → normalized string IDs (team-scoped)

Additional changes:
- Update _get_token_teams_from_request() to use normalized teams
- Fix caching in server/gateway services to only cache public-only queries
- Fix server creation visibility parameter precedence
- Update token_scoping middleware to use normalize_token_teams()
- Add comprehensive unit tests for token normalization behavior

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(websocket): forward auth credentials to /rpc endpoint

The WebSocket /ws endpoint now propagates authentication credentials
when making internal requests to /rpc:

- Forward JWT token as Authorization header when present
- Forward proxy user header when trust_proxy_auth is enabled
- Enables WebSocket transport to work with AUTH_REQUIRED=true

Also adds unit tests to verify auth credential forwarding behavior.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(rbac): add granular permission checks to all admin routes

- Add @require_permission decorators to all 177 admin routes with
  allow_admin_bypass=False to enforce explicit permission checks
- Add allow_admin_bypass parameter to require_permission and
  require_any_permission decorators for configurable admin bypass
- Add has_admin_permission() method to PermissionService for checking
  admin-level access (is_admin, *, or admin.* permissions)
- Update AdminAuthMiddleware to use has_admin_permission() for
  coarse-grained admin UI access control
- Create shared test fixtures in tests/unit/mcpgateway/conftest.py
  for mocking PermissionService across unit tests
- Update test files to use proper user context dict format

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* docs(rbac): comprehensive update to authentication and RBAC documentation

Update documentation to accurately reflect the two-layer security model
(Token Scoping + RBAC) and correct token scoping behavior.

rbac.md:
- Rewrite overview with two-layer security model explanation
- Fix token scoping matrix (missing teams key = PUBLIC-ONLY, not UNRESTRICTED)
- Add admin bypass requirements warning (requires BOTH teams:null AND is_admin:true)
- Add public-only token limitations (cannot access private resources even if owned)
- Add Permission System section with categories and fallback permissions
- Add Configuration Safety section (AUTH_REQUIRED, TRUST_PROXY_AUTH warnings)
- Update enforcement points matrix with Token Scoping and RBAC columns

multitenancy.md:
- Add Token Scoping Model section with secure-first defaults
- Add Two-Layer Security Model section with request flow diagram
- Add Enforcement Points Matrix
- Add Token Scoping Invariants
- Document multi-team token behavior (first team used for request.state.team_id)

oauth-design.md & oauth-authorization-code-ui-design.md:
- Add scope clarification notes (gateway OAuth delegation vs user auth)
- Add Token Verification section
- Add cross-references to RBAC and multitenancy docs

AGENTS.md:
- Add Authentication & RBAC Overview section with quick reference

llms/mcpgateway.md & llms/api.md:
- Add token scoping quick reference and examples
- Add links to full documentation

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(rbac): add explicit db dependency to RBAC-protected routes

Address load test findings from RCA #1 and IBM#2:

- Add `db: Session = Depends(get_db)` to routes in email_auth.py,
  llm_config_router.py, and teams.py that use @require_permission
- Fix test files to pass mock_db parameter after signature changes
- Add shm_size: 256m to PostgreSQL in docker-compose.yml
- Remove non-serializable content from resource update events
- Disable CircuitBreaker plugin for consistent load testing

These changes fix the NoneType errors (~33,700) observed under 4000
concurrent users where current_user_ctx["db"] was always None.

Remaining critical issue: Transaction leak in streamablehttp_transport.py
causing idle-in-transaction connections (see todo/rca2.md for details).

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(db): resolve transaction leak and connection pool exhaustion

Critical fixes for load test failures at 4000 concurrent users:

Issue #1 - Transaction leak in streamablehttp_transport.py (CRITICAL):
- Add explicit asyncio.CancelledError handling in get_db() context manager
- When MCP handlers are cancelled (client disconnect, timeout), the finally
  block may not execute properly, leaving transactions "idle in transaction"
- Now explicitly rollback and close before re-raising CancelledError
- Add rollback in direct SessionLocal usage at line ~1425

Issue IBM#2 - Missing db parameter in admin routes (HIGH):
- Add `db: Session = Depends(get_db)` to 73 remaining admin routes
- Routes with @require_permission but no db param caused decorator to
  create fresh session via fresh_db_session() for EVERY permission check
- This doubled connection usage for affected routes under load

Issue IBM#3 - Slow recovery from transaction leaks (MEDIUM):
- Reduce IDLE_TRANSACTION_TIMEOUT from 300s to 30s in docker-compose.yml
- Reduce CLIENT_IDLE_TIMEOUT from 300s to 60s
- Leaked transactions now killed faster, preventing pool exhaustion

Root cause confirmed: list_resources() MCP handler was primary source,
with 155+ connections stuck on `SELECT resources.*` for up to 273 seconds.

See todo/rca2.md for full analysis including live test data showing
connection leak progression and 606+ idle transaction timeout errors.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): use consistent user context format across all endpoints

- Update request_to_join_team and leave_team to use dict-based user context
- Fix teams router to use get_current_user_with_permissions consistently
- Move /discover route before /{team_id} to prevent route shadowing
- Update test fixtures to use mock_user_context dict format
- Add transaction commits in resource_service to prevent connection leaks
- Add missing docstring parameters for flake8 compliance

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(db): add explicit db.commit/close to prevent transaction leaks

Add explicit db.commit(); db.close() calls to 100+ endpoints across
all routers to prevent PostgreSQL connection leaks under high load.

Problem: Under high concurrency, FastAPI's Depends(get_db) cleanup
runs after response serialization, causing transactions to remain
in 'idle in transaction' state for 20-30+ seconds, exhausting the
connection pool.

Solution: Explicitly commit and close database sessions immediately
after database operations complete, before response serialization.

Routers fixed:
- tokens.py: 10 endpoints (create, list, get, update, revoke, usage, admin, team tokens)
- llm_config_router.py: 14 endpoints (provider/model CRUD, health, gateway models)
- sso.py: 5 endpoints (SSO provider CRUD)
- email_auth.py: 3 endpoints (user create/update/delete)
- oauth_router.py: 1 endpoint (delete_registered_client)
- teams.py: 18 endpoints (team CRUD, members, invitations, join requests)
- rbac.py: 12 endpoints (roles, user roles, permissions)
- main.py: 14 CUD + 3 list + 7 RPC handlers

Also fixes:
- admin.py: Rename 21 unused db params to _db (pylint W0613)
- test_teams*.py: Add mock_db fixture to tests calling router functions directly
- Add llms/audit-db-transaction-management.md for future audits

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* ci(coverage): lower doctest coverage threshold to 30%

Reduce the required doctest coverage from 34% to 30% to accommodate
current coverage levels (32.17%).

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(rpc): fix list_gateways tuple unpacking and add token scoping

The RPC list_gateways handler had two bugs:
1. Did not unpack the tuple (gateways, next_cursor) returned by
   gateway_service.list_gateways(), causing 'list' object has no
   attribute 'model_dump' error
2. Was missing token scoping via _get_rpc_filter_context(), which
   was the original R-02 security fix

Also fixed all callers of list_gateways that expected a list but
now receive a tuple:
- mcpgateway/admin.py: get_gateways_section()
- mcpgateway/services/import_service.py: 3 call sites

Updated test mocks to return (list, None) tuples instead of lists.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): build response before db.close() to avoid lazy-load errors

The teams router was calling db.commit(); db.close() before building
the TeamResponse, but TeamResponse includes team.get_member_count()
which needs an active session. When the session is closed, the fallback
in get_member_count() tries to access self.members (lazy-loaded),
causing "Parent instance is not bound to a Session" errors.

Fixed by building TeamResponse BEFORE calling db.close() in:
- create_team
- get_team
- update_team

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): fix update_team expecting team object but getting bool

The service's update_team() returns bool, but the router was treating
the return value as a team object and trying to access .id, .name, etc.

Fixed by:
1. Checking the boolean return value for success
2. Fetching the team again after successful update to build the response

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): fix update_member_role return type mismatch

The service's update_member_role() returns bool, but the router
treated it as a member object. Fixed by:
1. Checking the boolean success
2. Added get_member() method to TeamManagementService
3. Fetching the updated member to build the response

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* Fix teams return

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Signed-off-by: hughhennnelly <hughhennelly06@gmail.com>
AbdulR11 pushed a commit to AbdulR11/mcp-context-forge that referenced this pull request Feb 11, 2026
- Add EmbeddingProvider abstract base class
- Add DummyProvider for deterministic test embeddings
- Add OpenAIProvider skeleton (not yet implemented)

Closes IBM#3

Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Signed-off-by: josephhegarty4 <hegartjo@tcd.ie>
AbdulR11 pushed a commit to AbdulR11/mcp-context-forge that referenced this pull request Feb 11, 2026
feat(embedding): add initial embedding service implementation

Closes IBM#3

See merge request aodonne8/sweng26_group19_ibm_mcp_conversational_gateway!4
hughhennelly added a commit to hughhennelly/mcp-context-forge that referenced this pull request Feb 12, 2026
1. Fix broken imports (Issue #1):
   - Change from ..database to ..db
   - Fix unified_pdp imports to use plugins.unified_pdp
   - Update in routes, services, schemas, and tests

2. Register sandbox router in main.py (Issue IBM#2):
   - Add import and app.include_router call

3. Fix XSS vulnerability (Issue IBM#3):
   - Replace f-string HTML with Jinja2 template
   - Create sandbox_simulate_results.html template
   - Add Request parameter for template access

4. Add authentication (Issue IBM#4):
   - Add Depends(get_current_user) to simulate endpoint

5. Remove scratch files (Issue IBM#5):
   - Delete sandbox_header.txt and sandbox_new_header.txt

6. Resolve schemas conflict (Issue IBM#6):
   - Merge schemas/sandbox.py into schemas.py
   - Remove conflicting schemas/ directory
   - Update imports in routes and services

All changes tested and ready for review.

Related to IBM#2226

Signed-off-by: hughhennelly <hughhennelly06@gmail.com>
yiannis2804 added a commit to yiannis2804/mcp-context-forge that referenced this pull request Feb 19, 2026
…BM#3)

Address code review feedback from @jonpspri:

Problem: When allow_admin_bypass=False, admins still bypassed permission
checks because PolicyEngine.check_access() had its own unconditional
admin bypass at Step 1.

Solution:
- Added allow_admin_bypass parameter to check_access() method
- Updated Step 1 admin bypass: if subject.is_admin AND allow_admin_bypass
- Removed workaround of setting subject.is_admin = False in decorator
- Properly pass allow_admin_bypass from decorator to check_access()

Result:
- When allow_admin_bypass=False, admins must have explicit permissions
- No security regression - behavior matches old decorator
- Cleaner implementation without subject mutation

Testing:
- Verified admin bypass works when allow_admin_bypass=True
- Verified admin bypass blocked when allow_admin_bypass=False
- All 262 admin tests still passing

Related: PR IBM#2682 Phase 1 Code Review Item IBM#3
Signed-off-by: yiannis2804 <yiannis2804@gmail.com>
crivetimihai pushed a commit that referenced this pull request Feb 24, 2026
)

Address code review feedback from @jonpspri:

Problem: When allow_admin_bypass=False, admins still bypassed permission
checks because PolicyEngine.check_access() had its own unconditional
admin bypass at Step 1.

Solution:
- Added allow_admin_bypass parameter to check_access() method
- Updated Step 1 admin bypass: if subject.is_admin AND allow_admin_bypass
- Removed workaround of setting subject.is_admin = False in decorator
- Properly pass allow_admin_bypass from decorator to check_access()

Result:
- When allow_admin_bypass=False, admins must have explicit permissions
- No security regression - behavior matches old decorator
- Cleaner implementation without subject mutation

Testing:
- Verified admin bypass works when allow_admin_bypass=True
- Verified admin bypass blocked when allow_admin_bypass=False
- All 262 admin tests still passing

Related: PR #2682 Phase 1 Code Review Item #3
Signed-off-by: yiannis2804 <yiannis2804@gmail.com>
crivetimihai pushed a commit that referenced this pull request Feb 24, 2026
1. Fix broken imports (Issue #1):
   - Change from ..database to ..db
   - Fix unified_pdp imports to use plugins.unified_pdp
   - Update in routes, services, schemas, and tests

2. Register sandbox router in main.py (Issue #2):
   - Add import and app.include_router call

3. Fix XSS vulnerability (Issue #3):
   - Replace f-string HTML with Jinja2 template
   - Create sandbox_simulate_results.html template
   - Add Request parameter for template access

4. Add authentication (Issue #4):
   - Add Depends(get_current_user) to simulate endpoint

5. Remove scratch files (Issue #5):
   - Delete sandbox_header.txt and sandbox_new_header.txt

6. Resolve schemas conflict (Issue #6):
   - Merge schemas/sandbox.py into schemas.py
   - Remove conflicting schemas/ directory
   - Update imports in routes and services

All changes tested and ready for review.

Related to #2226

Signed-off-by: hughhennelly <hughhennelly06@gmail.com>
kcostell06 pushed a commit to kcostell06/mcp-context-forge that referenced this pull request Feb 24, 2026
* feat(api): standardize gateway response format

- Set *_unmasked fields to null in GatewayRead.masked()
- Apply masking consistently across all gateway return paths
- Mask credentials on cache reads
- Update admin UI to indicate stored secrets are write-only
- Update tests to verify masking behavior

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* delete artifact sbom

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(gateway): add configurable URL validation for gateway endpoints

Add comprehensive URL validation with configurable network access controls
for gateway and tool URL endpoints. This allows operators to control which
network ranges are accessible based on their deployment environment.

New configuration options:
- SSRF_PROTECTION_ENABLED: Master switch for URL validation (default: true)
- SSRF_ALLOW_LOCALHOST: Allow localhost/loopback (default: true for dev)
- SSRF_ALLOW_PRIVATE_NETWORKS: Allow RFC 1918 ranges (default: true)
- SSRF_DNS_FAIL_CLOSED: Reject unresolvable hostnames (default: false)
- SSRF_BLOCKED_NETWORKS: CIDR ranges to always block
- SSRF_BLOCKED_HOSTS: Hostnames to always block

Features:
- Validates all resolved IP addresses (A and AAAA records)
- Normalizes hostnames (case-insensitive, trailing dot handling)
- Blocks cloud metadata endpoints by default (169.254.169.254, etc.)
- Dev-friendly defaults with strict mode available for production
- Full documentation and Helm chart support

Also includes minor admin UI formatting improvements.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(auth): add token-scoped filtering for list endpoints and gateway forwarding

- Add token_teams parameter to list_servers and list_gateways endpoints
  for proper scoping based on JWT token team claims
- Update server_service.list_servers() and gateway_service.list_gateways()
  to filter results by token scope (public-only, team-scoped, or unrestricted)
- Skip caching for token-scoped queries to prevent cross-user data leakage
- Update gateway forwarding (_forward_request_to_all) to respect token team scope
- Fix public-only token handling in create endpoints (tools, resources, prompts,
  servers, gateways, A2A agents) to reject team/private visibility
- Preserve None vs [] distinction in SSE/WebSocket for proper admin bypass
- Update get_team_from_token to distinguish missing teams (legacy fallback)
  from explicit empty teams (public-only access)
- Add request.state.token_teams storage in all auth paths for downstream access

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(auth): add normalize_token_teams for consistent token scoping

Introduces a centralized `normalize_token_teams()` function in auth.py
that provides consistent token team normalization across all code paths:

- Missing teams key → empty list (public-only access)
- Explicit null teams + admin flag → None (admin bypass)
- Explicit null teams without admin → empty list (public-only)
- Empty teams array → empty list (public-only)
- Team list → normalized string IDs (team-scoped)

Additional changes:
- Update _get_token_teams_from_request() to use normalized teams
- Fix caching in server/gateway services to only cache public-only queries
- Fix server creation visibility parameter precedence
- Update token_scoping middleware to use normalize_token_teams()
- Add comprehensive unit tests for token normalization behavior

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(websocket): forward auth credentials to /rpc endpoint

The WebSocket /ws endpoint now propagates authentication credentials
when making internal requests to /rpc:

- Forward JWT token as Authorization header when present
- Forward proxy user header when trust_proxy_auth is enabled
- Enables WebSocket transport to work with AUTH_REQUIRED=true

Also adds unit tests to verify auth credential forwarding behavior.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* feat(rbac): add granular permission checks to all admin routes

- Add @require_permission decorators to all 177 admin routes with
  allow_admin_bypass=False to enforce explicit permission checks
- Add allow_admin_bypass parameter to require_permission and
  require_any_permission decorators for configurable admin bypass
- Add has_admin_permission() method to PermissionService for checking
  admin-level access (is_admin, *, or admin.* permissions)
- Update AdminAuthMiddleware to use has_admin_permission() for
  coarse-grained admin UI access control
- Create shared test fixtures in tests/unit/mcpgateway/conftest.py
  for mocking PermissionService across unit tests
- Update test files to use proper user context dict format

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* docs(rbac): comprehensive update to authentication and RBAC documentation

Update documentation to accurately reflect the two-layer security model
(Token Scoping + RBAC) and correct token scoping behavior.

rbac.md:
- Rewrite overview with two-layer security model explanation
- Fix token scoping matrix (missing teams key = PUBLIC-ONLY, not UNRESTRICTED)
- Add admin bypass requirements warning (requires BOTH teams:null AND is_admin:true)
- Add public-only token limitations (cannot access private resources even if owned)
- Add Permission System section with categories and fallback permissions
- Add Configuration Safety section (AUTH_REQUIRED, TRUST_PROXY_AUTH warnings)
- Update enforcement points matrix with Token Scoping and RBAC columns

multitenancy.md:
- Add Token Scoping Model section with secure-first defaults
- Add Two-Layer Security Model section with request flow diagram
- Add Enforcement Points Matrix
- Add Token Scoping Invariants
- Document multi-team token behavior (first team used for request.state.team_id)

oauth-design.md & oauth-authorization-code-ui-design.md:
- Add scope clarification notes (gateway OAuth delegation vs user auth)
- Add Token Verification section
- Add cross-references to RBAC and multitenancy docs

AGENTS.md:
- Add Authentication & RBAC Overview section with quick reference

llms/mcpgateway.md & llms/api.md:
- Add token scoping quick reference and examples
- Add links to full documentation

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(rbac): add explicit db dependency to RBAC-protected routes

Address load test findings from RCA IBM#1 and IBM#2:

- Add `db: Session = Depends(get_db)` to routes in email_auth.py,
  llm_config_router.py, and teams.py that use @require_permission
- Fix test files to pass mock_db parameter after signature changes
- Add shm_size: 256m to PostgreSQL in docker-compose.yml
- Remove non-serializable content from resource update events
- Disable CircuitBreaker plugin for consistent load testing

These changes fix the NoneType errors (~33,700) observed under 4000
concurrent users where current_user_ctx["db"] was always None.

Remaining critical issue: Transaction leak in streamablehttp_transport.py
causing idle-in-transaction connections (see todo/rca2.md for details).

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(db): resolve transaction leak and connection pool exhaustion

Critical fixes for load test failures at 4000 concurrent users:

Issue IBM#1 - Transaction leak in streamablehttp_transport.py (CRITICAL):
- Add explicit asyncio.CancelledError handling in get_db() context manager
- When MCP handlers are cancelled (client disconnect, timeout), the finally
  block may not execute properly, leaving transactions "idle in transaction"
- Now explicitly rollback and close before re-raising CancelledError
- Add rollback in direct SessionLocal usage at line ~1425

Issue IBM#2 - Missing db parameter in admin routes (HIGH):
- Add `db: Session = Depends(get_db)` to 73 remaining admin routes
- Routes with @require_permission but no db param caused decorator to
  create fresh session via fresh_db_session() for EVERY permission check
- This doubled connection usage for affected routes under load

Issue IBM#3 - Slow recovery from transaction leaks (MEDIUM):
- Reduce IDLE_TRANSACTION_TIMEOUT from 300s to 30s in docker-compose.yml
- Reduce CLIENT_IDLE_TIMEOUT from 300s to 60s
- Leaked transactions now killed faster, preventing pool exhaustion

Root cause confirmed: list_resources() MCP handler was primary source,
with 155+ connections stuck on `SELECT resources.*` for up to 273 seconds.

See todo/rca2.md for full analysis including live test data showing
connection leak progression and 606+ idle transaction timeout errors.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): use consistent user context format across all endpoints

- Update request_to_join_team and leave_team to use dict-based user context
- Fix teams router to use get_current_user_with_permissions consistently
- Move /discover route before /{team_id} to prevent route shadowing
- Update test fixtures to use mock_user_context dict format
- Add transaction commits in resource_service to prevent connection leaks
- Add missing docstring parameters for flake8 compliance

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(db): add explicit db.commit/close to prevent transaction leaks

Add explicit db.commit(); db.close() calls to 100+ endpoints across
all routers to prevent PostgreSQL connection leaks under high load.

Problem: Under high concurrency, FastAPI's Depends(get_db) cleanup
runs after response serialization, causing transactions to remain
in 'idle in transaction' state for 20-30+ seconds, exhausting the
connection pool.

Solution: Explicitly commit and close database sessions immediately
after database operations complete, before response serialization.

Routers fixed:
- tokens.py: 10 endpoints (create, list, get, update, revoke, usage, admin, team tokens)
- llm_config_router.py: 14 endpoints (provider/model CRUD, health, gateway models)
- sso.py: 5 endpoints (SSO provider CRUD)
- email_auth.py: 3 endpoints (user create/update/delete)
- oauth_router.py: 1 endpoint (delete_registered_client)
- teams.py: 18 endpoints (team CRUD, members, invitations, join requests)
- rbac.py: 12 endpoints (roles, user roles, permissions)
- main.py: 14 CUD + 3 list + 7 RPC handlers

Also fixes:
- admin.py: Rename 21 unused db params to _db (pylint W0613)
- test_teams*.py: Add mock_db fixture to tests calling router functions directly
- Add llms/audit-db-transaction-management.md for future audits

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* ci(coverage): lower doctest coverage threshold to 30%

Reduce the required doctest coverage from 34% to 30% to accommodate
current coverage levels (32.17%).

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(rpc): fix list_gateways tuple unpacking and add token scoping

The RPC list_gateways handler had two bugs:
1. Did not unpack the tuple (gateways, next_cursor) returned by
   gateway_service.list_gateways(), causing 'list' object has no
   attribute 'model_dump' error
2. Was missing token scoping via _get_rpc_filter_context(), which
   was the original R-02 security fix

Also fixed all callers of list_gateways that expected a list but
now receive a tuple:
- mcpgateway/admin.py: get_gateways_section()
- mcpgateway/services/import_service.py: 3 call sites

Updated test mocks to return (list, None) tuples instead of lists.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): build response before db.close() to avoid lazy-load errors

The teams router was calling db.commit(); db.close() before building
the TeamResponse, but TeamResponse includes team.get_member_count()
which needs an active session. When the session is closed, the fallback
in get_member_count() tries to access self.members (lazy-loaded),
causing "Parent instance is not bound to a Session" errors.

Fixed by building TeamResponse BEFORE calling db.close() in:
- create_team
- get_team
- update_team

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): fix update_team expecting team object but getting bool

The service's update_team() returns bool, but the router was treating
the return value as a team object and trying to access .id, .name, etc.

Fixed by:
1. Checking the boolean return value for success
2. Fetching the team again after successful update to build the response

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(teams): fix update_member_role return type mismatch

The service's update_member_role() returns bool, but the router
treated it as a member object. Fixed by:
1. Checking the boolean success
2. Added get_member() method to TeamManagementService
3. Fetching the updated member to build the response

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* Fix teams return

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
aidbutlr referenced this pull request in aidbutlr/mcp-context-forge Mar 3, 2026
CYFR-280 Updates to align with .github/workflows/ build scripts
rakdutta added a commit that referenced this pull request Mar 17, 2026
…re in apijsonpath parsing

Address code review feedback with three security and performance improvements:

1. Specific Exception Handling (Issue #1)
   - Catch specific exceptions (json.JSONDecodeError, ValueError, ValidationError)
   - Separate user errors (400) from system errors (500)
   - Log unexpected errors with full context for debugging

2. Sanitize Exception Messages (Issue #2)
   - Return generic error messages in production (non-DEBUG mode)
   - Prevent information disclosure about internal types and stack traces
   - Detailed messages only shown when LOG_LEVEL=DEBUG for troubleshooting

3. Optimize Debug Logging (Issue #3)
   - Check if debug logging is enabled before building log message
   - Avoid string formatting overhead in production
   - Improves performance in high-traffic scenarios

Updated test assertions to verify generic error messages (security improvement).
All tests pass (18 jsonpath tests verified).

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>
ecthelion77 pushed a commit to ecthelion77/mcp-context-forge that referenced this pull request Mar 18, 2026
…orm_user role

Team scoping fixes (admin UI + backend):

1. verify_team_for_user: return consistent types (str|None), raise
   PermissionError instead of silently returning [] on unauthorized team.
   Add proper type annotations.

2. All edit handlers (server, tool, gateway, prompt, resource, a2a):
   stop reading team_id from the form. The URL team selector may point
   to a different team than the entity's actual owner. Set team_id=None
   on all Update schemas so the service layer preserves the existing
   entity's team_id. This prevents accidental team ownership changes.

3. Resource edit handler: was the only edit handler missing
   verify_team_for_user and team_id handling entirely -- now consistent
   with all other handlers (Bug IBM#3).

4. All create handlers: wrap verify_team_for_user calls with
   PermissionError handling to return 403 on unauthorized team access
   instead of silently falling back.

5. Frontend edit modals (editTool, editServer, editGateway, editPrompt,
   editResource, editA2AAgent): use entity.team_id from the fetched JSON
   instead of the URL team selector for the hidden team_id input and
   visibility coercion checks.

Role scoping fix:

6. Add new 'platform_user' role (global scope) with servers.use +
   tools.execute + llm.invoke permissions. Fills the gap between
   platform_viewer (read-only, no servers.use) and developer (team-scoped
   full CRUD). Suitable as SSO_ENTRA_DEFAULT_ROLE for users who need to
   use virtual servers across all teams without team-level CRUD.
crivetimihai pushed a commit that referenced this pull request Mar 18, 2026
…re in apijsonpath parsing

Address code review feedback with three security and performance improvements:

1. Specific Exception Handling (Issue #1)
   - Catch specific exceptions (json.JSONDecodeError, ValueError, ValidationError)
   - Separate user errors (400) from system errors (500)
   - Log unexpected errors with full context for debugging

2. Sanitize Exception Messages (Issue #2)
   - Return generic error messages in production (non-DEBUG mode)
   - Prevent information disclosure about internal types and stack traces
   - Detailed messages only shown when LOG_LEVEL=DEBUG for troubleshooting

3. Optimize Debug Logging (Issue #3)
   - Check if debug logging is enabled before building log message
   - Avoid string formatting overhead in production
   - Improves performance in high-traffic scenarios

Updated test assertions to verify generic error messages (security improvement).
All tests pass (18 jsonpath tests verified).

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>
crivetimihai added a commit that referenced this pull request Mar 18, 2026
…or handling (#3159)

* fix(api): change apijsonpath from Body to Query parameter for GET endpoints

- Move apijsonpath parameter from request body to query string for list_tools and get_tool endpoints
- Add _parse_apijsonpath() helper to handle both JSON string and JsonPathModifier inputs
- Return ORJSONResponse to bypass FastAPI response_model validation when using JSONPath modifiers
- Add comprehensive error handling with proper HTTPException status codes
- Add test coverage for Query parameter handling and error cases

This change resolves issue #2848 by making GET requests RESTful-compliant (no request body).

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* fix(security): improve error handling and reduce information disclosure in apijsonpath parsing

Address code review feedback with three security and performance improvements:

1. Specific Exception Handling (Issue #1)
   - Catch specific exceptions (json.JSONDecodeError, ValueError, ValidationError)
   - Separate user errors (400) from system errors (500)
   - Log unexpected errors with full context for debugging

2. Sanitize Exception Messages (Issue #2)
   - Return generic error messages in production (non-DEBUG mode)
   - Prevent information disclosure about internal types and stack traces
   - Detailed messages only shown when LOG_LEVEL=DEBUG for troubleshooting

3. Optimize Debug Logging (Issue #3)
   - Check if debug logging is enabled before building log message
   - Avoid string formatting overhead in production
   - Improves performance in high-traffic scenarios

Updated test assertions to verify generic error messages (security improvement).
All tests pass (18 jsonpath tests verified).

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* doc

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* except

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* test coverage

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* pagination

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* review

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* head remove

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* flake

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>

* fix: validate apijsonpath before DB query and add missing test coverage

- Move _parse_apijsonpath() call before the database query in list_tools
  to fail fast on invalid input without wasting a DB round-trip
- Add tests for the ValidationError path (extra fields rejected by
  extra="forbid" on JsonPathModifier)
- Run black/isort on test file

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix: correct nextCursor wire key, sanitize JSONPath in debug log, add e2e tests

- Fix pagination+JSONPath response to use "nextCursor" (camelCase) matching
  the CursorPaginatedToolsResponse alias contract. The hand-built dict
  bypassed Pydantic aliasing via ORJSONResponse, emitting "next_cursor"
  which broke any client relying on the documented field name.
  Confirmed regression against live server at localhost:8080.

- Sanitize user-supplied JSONPath expression in debug log via
  SecurityValidator.sanitize_log_message() to prevent CWE-117 log
  forging (newlines, ANSI escapes in crafted jsonpath values).

- Add end-to-end HTTP tests (TestApijsonpathHTTP) exercising the
  apijsonpath query parameter through FastAPI TestClient, covering
  the full HTTP binding path that __wrapped__() unit tests skip.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Rakhi Dutta <rakhibiswas@yahoo.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Rakhi Dutta <rakhibiswas@yahoo.com>
crivetimihai added a commit that referenced this pull request Mar 22, 2026
1. Remove MAX_TEMPLATE_LENGTH check from SecurityValidator.validate_template
   so the configurable CONTENT_MAX_PROMPT_SIZE is not silently capped at 64KB
   by the schema-level validator (finding #1)

2. Add db.rollback() to ContentSizeError handlers in resource_service to
   prevent partial metadata mutations from being committed when content
   size validation fails after ORM fields are already updated (finding #2)

3. Include actual_size and max_size in admin route 413 responses to match
   the structured error contract used by API routes (finding #3)

4. Update security and validator tests that expected schema-level length
   rejection — size enforcement now happens at service layer

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
crivetimihai added a commit that referenced this pull request Mar 22, 2026
…fixes #538 US-1) (#3251)

* feat(security): enforce content size limits for resources and prompts

Implement configurable content size validation to prevent DoS attacks
via oversized content submissions. Resources are limited to 100KB and
prompt templates to 10KB by default (configurable via environment
variables CONTENT_MAX_RESOURCE_SIZE and CONTENT_MAX_PROMPT_SIZE).

Validation occurs at the service layer before database operations,
returning 413 Payload Too Large with structured error details.

Closes #538

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(security): review fixes for content size limits PR

- Fix ResourceCreate.validate_content to remove MAX_CONTENT_LENGTH
  schema-level check (now handled by service layer, consistent with
  ResourceUpdate)
- Fix broken test assertions checking raw bytes in formatted messages
  (TestErrorMessageClarity was asserting "200000" in human-readable
  "195.3 KB" strings)
- Fix broken test_update_prompt_validation_and_integrity_errors which
  had its body accidentally deleted by the original PR
- Restore accidentally deleted assertions in test_list_resources and
  test_create_resource_endpoint
- Add global exception handler for ContentSizeError (defense-in-depth,
  consistent with existing handlers for ValidationError/IntegrityError)
- Add reset_content_security_service() for test isolation
- Remove "Made with Bob" watermark from test file
- Remove duplicate pytest import in integration tests
- Fix broken README link to non-existent migration guide
- Fix double-commented lines in .env.example

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix: resolve flake8 lint issues in content size limits PR

- Move noqa:DUO138 to the line flake8 reports (validators.py)
- Add Args/Returns to content_size_exception_handler docstring (DAR101/DAR201)

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(tests): update security tests for service-layer size validation

Content size validation moved from Pydantic schema to service layer
(returns 413 instead of 422). Update test_resource_create_content_validation
and test_zip_bomb_prevention to reflect that ResourceCreate no longer
rejects oversized content at schema level.

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* fix(security): address codex review findings for content size limits

1. Remove MAX_TEMPLATE_LENGTH check from SecurityValidator.validate_template
   so the configurable CONTENT_MAX_PROMPT_SIZE is not silently capped at 64KB
   by the schema-level validator (finding #1)

2. Add db.rollback() to ContentSizeError handlers in resource_service to
   prevent partial metadata mutations from being committed when content
   size validation fails after ORM fields are already updated (finding #2)

3. Include actual_size and max_size in admin route 413 responses to match
   the structured error contract used by API routes (finding #3)

4. Update security and validator tests that expected schema-level length
   rejection — size enforcement now happens at service layer

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* chore: fix trailing whitespace in test files

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

* docs: add content size limits to configuration docs, securing guide, and Helm values

- docs/docs/manage/configuration.md: add Content Security section with
  env var table, scope note, and error response example
- docs/docs/manage/securing.md: add section 9 (Content Size Limits)
  to production security checklist, renumber subsequent sections
- charts/mcp-stack/values.yaml: add CONTENT_MAX_RESOURCE_SIZE and
  CONTENT_MAX_PROMPT_SIZE to gateway env config

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>

---------

Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
Co-authored-by: Mihai Criveti <crivetimihai@gmail.com>
SiddheshBangar added a commit to SiddheshBangar/mcp-context-forge that referenced this pull request Mar 26, 2026
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.

1 participant