Skip to content

Make CORS allow_credentials configurable instead of hardcoded True#62658

Open
Vamsi-klu wants to merge 1 commit intoapache:mainfrom
Vamsi-klu:fix/m3-cors-credentials-configurable
Open

Make CORS allow_credentials configurable instead of hardcoded True#62658
Vamsi-klu wants to merge 1 commit intoapache:mainfrom
Vamsi-klu:fix/m3-cors-credentials-configurable

Conversation

@Vamsi-klu
Copy link
Contributor

@Vamsi-klu Vamsi-klu commented Mar 1, 2026

Summary

  • Replace hardcoded allow_credentials=True in CORSMiddleware with configurable [api] access_control_allow_credentials option (default: False)
  • Log a warning when allow_credentials=True is used with wildcard (*) origins, as this creates CSRF risk
  • Add config option to config.yml template

Co-contributors : @codingrealitylabs @girlcoder-gaming

Test plan

  • Verify default behavior: allow_credentials=False when option not set
  • Verify setting access_control_allow_credentials = True enables credentials
  • Verify warning is logged when credentials + wildcard origins are configured
  • Run pytest tests/api_fastapi/core_api/test_app.py -v

Note: This is a breaking change for deployments that rely on CORS credentials being enabled by default.

🤖 Generated with Claude Code

@Vamsi-klu Vamsi-klu marked this pull request as ready for review March 1, 2026 07:36
@Vamsi-klu
Copy link
Contributor Author

cc @kaxil @vincbeck @ashb — Would appreciate your review. This replaces the hardcoded allow_credentials=True in CORSMiddleware with a configurable option (default: False). Note this is a breaking change for deployments relying on CORS credentials being enabled by default.

Add [api] access_control_allow_credentials config option (default: False)
to replace the hardcoded allow_credentials=True in CORSMiddleware. Also
log a warning when credentials are enabled with wildcard origins, as
this creates CSRF risk.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Contributor

@vincbeck vincbeck left a comment

Choose a reason for hiding this comment

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

AI generated PR with no explanation on why doing this change. Plus, it introduces a breaking change. Please provide more details about this PR and why you want to make that change, otherwise I'll close it

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.

2 participants