Skip to content

fix: Handle Empty String in iFrame Embedding Config options#2958

Open
prakhar-singh1928 wants to merge 2 commits intomainfrom
iframe-embedding-fix-2492
Open

fix: Handle Empty String in iFrame Embedding Config options#2958
prakhar-singh1928 wants to merge 2 commits intomainfrom
iframe-embedding-fix-2492

Conversation

@prakhar-singh1928
Copy link
Collaborator

@prakhar-singh1928 prakhar-singh1928 commented Feb 15, 2026

🔗 Related Issue

Closes #2492


📝 Summary

This PR fixes X_FRAME_OPTIONS related middleware behavior not properly handling empty string values.

Key changes:

  • Fixed middleware bug where empty string "" was falling through to the default case and being treated like DENY (adding frame-ancestors 'none'), when it should allow all embedding (no frame-ancestors directive)
  • Updated test cases in test_security_middleware_comprehensive.py to use None instead of "" when testing disabled iframe restrictions, matching the config validator's behavior that converts empty strings to None
  • Added test_x_frame_options_empty_string_returns_none

The root cause was:

  1. The middleware was incorrectly treating empty string "" as an unknown value, defaulting to DENY behavior(adding frame-ancestors 'none') (blocking embedding)

All X_FRAME_OPTIONS now exhibit expected behaviour

🏷️ Type of Change

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

🧪 Verification

Check Command Status
Lint suite make lint ✅ Passed (Pylint score: 10/10)
Unit tests make test ✅ Passed (all X_FRAME_OPTIONS tests)
Coverage ≥ 80% make coverage ✅ Passed (100% for modified files, 99%+ overal

✅ Checklist

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

📓 Notes (optional)

Targeted local verification executed:

  • pytest tests/unit/mcpgateway/test_config.py -v
  • pytest tests/security/test_security_middleware_comprehensive.py

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.

[TESTING][CONFIG]: iFrame Mode (X-Frame-Options) Test Plan

1 participant