Skip to content

feat(security): add MIME type restrictions for resources (US-2)#3847

Merged
crivetimihai merged 1 commit intomainfrom
feat/security-restrict-content-types
Apr 3, 2026
Merged

feat(security): add MIME type restrictions for resources (US-2)#3847
crivetimihai merged 1 commit intomainfrom
feat/security-restrict-content-types

Conversation

@msureshkumar88
Copy link
Copy Markdown
Collaborator

🔗 Related Issue

Partially closed #538 (US-2: MIME Type Restrictions for Resources)


📝 Summary

Implements comprehensive content security validation for the MCP Context Forge gateway, adding content size limits and MIME type restrictions for resources and prompts. This addresses US-1 and US-2 from issue #538.

Key Features:

  • ✅ Content size validation (100KB for resources, 10KB for prompts)
  • ✅ MIME type allowlist validation (18 safe types by default)
  • ✅ Flexible enforcement modes (strict reject or log-only)
  • ✅ HTTP 413/415 error responses with detailed context
  • ✅ Prometheus metrics for security violations
  • ✅ PII-safe logging (hashed emails, masked IPs)
  • ✅ Support for vendor MIME types and suffixes (+json, +xml)

🏷️ Type of Change

  • Feature / Enhancement
  • Bug fix
  • Documentation
  • Refactor
  • Chore (deps, CI, tooling)
  • Other (describe below)

🧪 Verification

Check Command Status
Lint suite make lint ✅ Pass
Unit tests make test ✅ Pass
Coverage ≥ 80% make coverage ✅ 95%+

Test Coverage:

  • 526 lines of unit tests in test_content_security.py
  • Integration tests for size limits
  • All validation scenarios covered (size, MIME, strict/log-only modes)
  • Thread safety and singleton pattern tested

✅ Checklist

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

Documentation Added:

  • US-2-IMPLEMENTATION-PLAN.md (825 lines) - Complete implementation plan
  • docs/docs/architecture/security-features.md - Architecture documentation
  • docs/docs/manage/content-security.md (394 lines) - Operational guide

📓 Notes

Implementation Details

New Files:

  • mcpgateway/services/content_security.py (378 lines) - Core validation service
  • tests/unit/mcpgateway/services/test_content_security.py (526 lines) - Comprehensive tests

Modified Files:

  • mcpgateway/main.py - Added HTTP 413/415 exception handlers
  • mcpgateway/config.py - Added 4 new configuration settings
  • mcpgateway/services/resource_service.py - Integrated size & MIME validation
  • mcpgateway/services/prompt_service.py - Integrated size validation

Configuration

Default settings (safe for production):

CONTENT_MAX_RESOURCE_SIZE=102400  # 100KB
CONTENT_MAX_PROMPT_SIZE=10240     # 10KB
CONTENT_STRICT_MIME_VALIDATION=false  # Log-only mode
CONTENT_ALLOWED_RESOURCE_MIMETYPES=text/plain,text/markdown,...  # 18 types

Deployment Strategy

Phase 1 (Recommended): Deploy with CONTENT_STRICT_MIME_VALIDATION=false

  • Monitor Prometheus metrics for 1-2 weeks
  • Analyze violation patterns
  • Adjust MIME allowlist if needed

Phase 2: Enable strict mode with CONTENT_STRICT_MIME_VALIDATION=true

Metrics

Monitor these Prometheus counters:

  • content_security_size_violations_total{entity_type="resource|prompt"}
  • content_security_mime_violations_total

Remaining Work (Issue #538)

This PR completes 40% of issue #538 (US-1 and US-2). Remaining user stories:

  • US-3: Malicious pattern detection
  • US-4: Template syntax validation
  • US-5: Rate limiting

@msureshkumar88 msureshkumar88 changed the title Feat/security restrict content types feat(security): security restrict content types Mar 25, 2026
@msureshkumar88 msureshkumar88 force-pushed the feat/security-restrict-content-types branch from c77cb69 to f7a38a6 Compare March 25, 2026 15:04
@crivetimihai crivetimihai changed the title feat(security): security restrict content types feat(security): add MIME type restrictions for resources (US-2) Mar 29, 2026
@crivetimihai crivetimihai added enhancement New feature or request SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release security Improves security labels Mar 29, 2026
@crivetimihai crivetimihai added this to the Release 1.0.0 milestone Mar 29, 2026
@crivetimihai
Copy link
Copy Markdown
Member

Thanks @msureshkumar88. Comprehensive implementation of US-2 from #538 — the feature-flagged approach with log-only mode is a good migration strategy. Given the size (+1899 lines, 17 files), please ensure all tests pass and check for merge conflicts. One note: the README appears to have a duplicate "Content Security" section heading — please consolidate.

@msureshkumar88 msureshkumar88 force-pushed the feat/security-restrict-content-types branch 2 times, most recently from 7dddd2c to 7e0e18c Compare March 31, 2026 09:41
@msureshkumar88
Copy link
Copy Markdown
Collaborator Author

msureshkumar88 commented Mar 31, 2026

@crivetimihai
All the requested changes are done

  • Resolve merge conflict
  • Removed the duplicate "Content Security" section (previously at lines 758-767)
  • Updated the consolidated section's note to include all relevant information from both sections
  • The consolidated note now reads: "Size limits apply only to new create/update operations. Existing content is not retroactively validated

Comment thread tests/integration/test_content_size_limits.py Outdated
Comment thread tests/unit/mcpgateway/services/test_content_security.py Outdated
Comment thread tests/unit/mcpgateway/services/test_content_security.py
@msureshkumar88
Copy link
Copy Markdown
Collaborator Author

msureshkumar88 commented Mar 31, 2026

@madhu-mohan-jaishankar
Requested changes are done and ready to review again.

Comment thread tests/e2e/test_admin_apis.py Outdated
Comment thread tests/unit/mcpgateway/services/test_content_security.py Outdated
Comment thread tests/unit/mcpgateway/services/test_content_security.py Outdated
Comment thread tests/unit/mcpgateway/services/test_content_security.py Outdated
Comment thread tests/unit/mcpgateway/services/test_content_security.py Outdated
Comment thread tests/unit/mcpgateway/test_main.py Outdated
Comment thread tests/unit/mcpgateway/test_main.py Outdated
Comment thread tests/unit/mcpgateway/test_main_error_handlers.py Outdated
Comment thread tests/unit/mcpgateway/test_main_error_handlers.py Outdated
Comment thread tests/unit/mcpgateway/test_main_error_handlers.py Outdated
Copy link
Copy Markdown
Collaborator

@madhu-mohan-jaishankar madhu-mohan-jaishankar left a comment

Choose a reason for hiding this comment

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

LGTM.

@msureshkumar88 msureshkumar88 added the release-fix Critical bugfix required for the release label Apr 2, 2026
@msureshkumar88 msureshkumar88 force-pushed the feat/security-restrict-content-types branch 2 times, most recently from fd5e924 to 845b7d5 Compare April 2, 2026 14:44
Copy link
Copy Markdown
Collaborator

@madhu-mohan-jaishankar madhu-mohan-jaishankar left a comment

Choose a reason for hiding this comment

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

LGTM

@crivetimihai
Copy link
Copy Markdown
Member

Review & Rebase Summary

Rebased onto current main, squashed 27 commits into one clean commit preserving original author, and addressed several issues found during review.

Bugs fixed

# Issue Severity
1 TOOL_DESCRIPTION_FORBIDDEN_PATTERNS was removed from _normalize_env_list_vars instead of adding CONTENT_ALLOWED_RESOURCE_MIMETYPES alongside it — CSV-format env vars for tool description forbidden patterns would silently break High
2 .env.example documented CONTENT_STRICT_MIME_VALIDATION default as true but config.py default is False Medium
3 security-features.md said vendor/suffix types are "automatically permitted" but code explicitly rejects them in strict mode Medium
4 README.md linked to non-existent docs/MIGRATION_CONTENT_LIMITS.md Low
5 application/octet-stream in default MIME allowlist defeats the purpose of MIME restrictions (catch-all binary type) Medium
6 3 pairs of duplicate test methods in test_main.py (Python shadows earlier definitions) High
7 test_update_prompt_validation_and_integrity_errors body was truncated — test passed vacuously High
8 2 unrelated preserves_team_id_when_not_in_form tests were deleted from test_admin.py High

Security fixes

# Issue Severity
9 MIME priority inversion — URL-detected type overrode user-declared, allowing attackers to launder dangerous types via friendly filenames (e.g., application/x-executable stored as text/plain by naming the URI payload.txt). Fixed: user-provided > URI-detected > content fallback High
10 Log-only mode was a no-opCONTENT_STRICT_MIME_VALIDATION=false returned immediately without checking the allowlist, logging, or incrementing metrics. The entire monitoring/rollout phase was useless. Fixed: always checks, logs WARNING + increments counter; only the raise is gated on strict High
11 MIME enforcement bypassable via update without contentupdate_resource only validated MIME type inside if content is not None, so updating just the MIME type field skipped validation entirely High
12 Dirty session state on rejectionupdate_resource mutated tracked model fields (uri, name, etc.) before MIME validation. If validation rejected, the session was dirty with no rollback. Fixed: MIME type resolved into local variable, validated, then assigned; db.rollback() restored in both ContentSizeError and ContentTypeError handlers High
13 Unbounded Prometheus cardinalitycontent_type_violations_counter used raw rejected MIME strings as a label. In log-only mode (default), an attacker could generate arbitrary label values without being blocked, causing cardinality explosion. Fixed: removed mime_type label (raw type is in WARNING logs) Medium
14 Bulk registration validated but didn't persist resolved MIMEbulk_mime_type was computed and validated correctly, but resource.mime_type (original user value) was written to DB on all 3 persist paths (new, update, rename) Medium
15 Admin edit resource missing rollbackadmin_edit_resource exception handler returned 415/413 without rolling back dirty session state, unlike admin_add_resource which had the rollback pattern Medium

Tests added/fixed

  • MIME parameter stripping (text/plain; charset=utf-8)
  • Prometheus counter increment verification (size + MIME)
  • URI change without MIME type triggers detection
  • Update MIME type without content still validates (bypass regression test)
  • Session stays clean on rejection assertion
  • User-provided MIME priority tests (replacing old URL-detection-wins tests)
  • Restored broken/deleted tests from original PR

All 1857 affected tests passing, 100% coverage on content_security.py.

crivetimihai
crivetimihai previously approved these changes Apr 3, 2026
Copy link
Copy Markdown
Member

@crivetimihai crivetimihai left a comment

Choose a reason for hiding this comment

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

Reviewed, rebased, and fixed 15 issues (8 bugs, 7 security). All tests passing. LGTM.

Add content security validation for MIME types on resources:

- ContentSecurityService.validate_resource_mime_type() with configurable
  allowlist (CONTENT_ALLOWED_RESOURCE_MIMETYPES) and strict/log-only modes
  (CONTENT_STRICT_MIME_VALIDATION, default: false)
- Log-only mode checks the allowlist and logs violations at WARNING level
  with Prometheus metrics, enabling monitoring before enforcement
- ContentTypeError exception with HTTP 415 global handler
- URL-detected MIME type priority over user-provided values
- Prometheus counters for size and MIME type violations
- PII-safe audit logging (hashed emails, masked IPs)
- Validation in register_resource, update_resource (including MIME-only
  updates without content changes), and bulk import

Partially closes #538

Signed-off-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
@crivetimihai crivetimihai force-pushed the feat/security-restrict-content-types branch from 141cc7a to 6eed931 Compare April 3, 2026 11:22
@crivetimihai crivetimihai merged commit fa18388 into main Apr 3, 2026
25 checks passed
@crivetimihai crivetimihai deleted the feat/security-restrict-content-types branch April 3, 2026 11:24
jonpspri pushed a commit that referenced this pull request Apr 10, 2026
Add content security validation for MIME types on resources:

- ContentSecurityService.validate_resource_mime_type() with configurable
  allowlist (CONTENT_ALLOWED_RESOURCE_MIMETYPES) and strict/log-only modes
  (CONTENT_STRICT_MIME_VALIDATION, default: false)
- Log-only mode checks the allowlist and logs violations at WARNING level
  with Prometheus metrics, enabling monitoring before enforcement
- ContentTypeError exception with HTTP 415 global handler
- URL-detected MIME type priority over user-provided values
- Prometheus counters for size and MIME type violations
- PII-safe audit logging (hashed emails, masked IPs)
- Validation in register_resource, update_resource (including MIME-only
  updates without content changes), and bulk import

Partially closes #538

Signed-off-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
claudia-gray pushed a commit that referenced this pull request Apr 13, 2026
Add content security validation for MIME types on resources:

- ContentSecurityService.validate_resource_mime_type() with configurable
  allowlist (CONTENT_ALLOWED_RESOURCE_MIMETYPES) and strict/log-only modes
  (CONTENT_STRICT_MIME_VALIDATION, default: false)
- Log-only mode checks the allowlist and logs violations at WARNING level
  with Prometheus metrics, enabling monitoring before enforcement
- ContentTypeError exception with HTTP 415 global handler
- URL-detected MIME type priority over user-provided values
- Prometheus counters for size and MIME type violations
- PII-safe audit logging (hashed emails, masked IPs)
- Validation in register_resource, update_resource (including MIME-only
  updates without content changes), and bulk import

Partially closes #538

Signed-off-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
Signed-off-by: Mihai Criveti <crivetimihai@gmail.com>
Co-authored-by: Suresh Kumar Moharajan <suresh.kumar.m@ibm.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request release-fix Critical bugfix required for the release security Improves security SHOULD P2: Important but not vital; high-value items that are not crucial for the immediate release

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FEATURE][SECURITY]: Content size and type security limits for resources and prompts

3 participants