Skip to content

feat(config): make CORS allowed origins configurable via env#697

Open
raulanatol wants to merge 4 commits into
plastic-labs:mainfrom
raulanatol:feat/configurable-cors-origins
Open

feat(config): make CORS allowed origins configurable via env#697
raulanatol wants to merge 4 commits into
plastic-labs:mainfrom
raulanatol:feat/configurable-cors-origins

Conversation

@raulanatol
Copy link
Copy Markdown

@raulanatol raulanatol commented May 17, 2026

Summary

  • Adds a new CORSSettings block (env prefix CORS_) and exposes it as settings.CORS.ORIGINS.
  • src/main.py now feeds allow_origins from settings.CORS.ORIGINS instead of a hardcoded list. Defaults preserve the prior values (http://localhost, http://127.0.0.1:8000, https://api.honcho.dev) so existing deployments behave identically.
  • Documented under a new CORS Settings section in .env.template.

Motivation: self-hosted deployments behind custom domains currently have to patch src/main.py to whitelist their frontend. With this change a single env var is enough:

CORS_ORIGINS=["https://app.example.com","https://staging.example.com"]

Scope is intentionally limited to origins. The allow_methods / allow_headers wildcards flagged in #394 are left as-is to keep this PR focused on the env-configurability hole; happy to follow up in a separate PR if maintainers want them tightened too.

Test plan

  • uv run ruff check src/
  • uv run ruff format --check src/
  • uv run basedpyright src/main.py src/config.py — 0 errors
  • Verified defaults load unchanged: uv run python -c "from src.config import settings; print(settings.CORS.ORIGINS)"
  • Verified env override: CORS_ORIGINS='["http://foo.test","https://bar.test"]' uv run python -c ...
  • Maintainers: smoke test that CORS preflight from a custom origin works once CORS_ORIGINS is set

Summary by CodeRabbit

  • New Features

    • Allowed origins for cross-origin requests are now configurable at runtime via environment settings instead of using a built-in hardcoded list.
  • Chores

    • Environment template updated with a CORS section and example allowlist entries for local and production setups.

Review Change Stack

Replaces the hardcoded `origins` list in `src/main.py` with a new
`CORSSettings` block (env prefix `CORS_`), exposed as `settings.CORS.ORIGINS`.
Defaults match the prior hardcoded values, so self-hosted deployments behind
custom domains can now whitelist their frontend without editing source.

Documented in `.env.template` under a new CORS Settings section.
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 17, 2026

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 6ceca92f-2310-40ef-8606-492bcd48a672

📥 Commits

Reviewing files that changed from the base of the PR and between 4db285c and 596cd28.

📒 Files selected for processing (2)
  • src/config.py
  • src/main.py

Walkthrough

Adds a new AppSettings field CORS_ORIGINS containing a default origins list, updates FastAPI CORSMiddleware to use settings.CORS_ORIGINS for allow_origins, and documents the CORS_ORIGINS JSON-array environment variable in .env.template.

Changes

CORS settings

Layer / File(s) Summary
AppSettings CORS field and middleware wiring
src/config.py, src/main.py, .env.template
Adds AppSettings.CORS_ORIGINS (default allowlist), substitutes settings.CORS_ORIGINS for the previous hardcoded origins when calling app.add_middleware(CORSMiddleware, allow_origins=...), and documents the CORS_ORIGINS env var in .env.template.

Sequence Diagram

sequenceDiagram
  participant Application
  participant AppSettings
  participant CORSMiddleware
  Application->>AppSettings: read CORS_ORIGINS
  Application->>CORSMiddleware: add_middleware(..., allow_origins=settings.CORS_ORIGINS)
  CORSMiddleware->>CORSMiddleware: enforce CORS using allow_origins
Loading

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 I nudged the origins into a list,

No hardcoded strings to be missed.
Env var sings where defaults once stood,
Devs can add the hosts they should.
🥕🌐

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately describes the main change: making CORS allowed origins configurable via environment variables, which is the core objective across all three modified files.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown
Collaborator

@Rajat-Ahuja1997 Rajat-Ahuja1997 left a comment

Choose a reason for hiding this comment

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

looks good, one suggestion regarding placement

Comment thread src/config.py Outdated
return self


class CORSSettings(HonchoSettings):
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

instead of adding a separate CORSSetting block, it would be simpler to just add a CORS_ORIGINS field to AppSettings. if we ever see the need to add other CORS_* values then we can promote this to a nested class.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Good call, thanks! I just pushed a commit that drops the dedicated CORSSettings block and adds CORS_ORIGINS directly to AppSettings. The env var (CORS_ORIGINS) keeps working as-is since AppSettings has no env prefix. If more CORS_* knobs come up later we can promote it back to a nested class.

Drop the dedicated CORSSettings nested model and expose CORS_ORIGINS
directly on AppSettings. The CORS_ORIGINS env var keeps working as
before since AppSettings has no env prefix.
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