Skip to content

Release 1.3.1: fix prod CORS startup validation#566

Merged
ilyanfraimbault merged 1 commit into
masterfrom
develop
Jun 18, 2026
Merged

Release 1.3.1: fix prod CORS startup validation#566
ilyanfraimbault merged 1 commit into
masterfrom
develop

Conversation

@ilyanfraimbault

Copy link
Copy Markdown
Owner

Hotfix on top of 1.3.0.

The running prod still needs the env applied to its divergent compose + an api restart (config, not image).

@ilyanfraimbault ilyanfraimbault merged commit 528368b into master Jun 18, 2026
18 checks passed

@claude claude Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Review — Release 1.3.1: fix prod CORS startup validation

Hotfix is correct and well-scoped. The root cause (1.3.0 added ValidateOnStart on Cors:Origins for non-Development environments but no compose file supplied it) is properly addressed by injecting the origins at the compose layer.

What was checked

  • Binding conventionCors__Origins__0/1/2 is the correct ASP.NET Core env-var syntax for indexed array elements; FrontendCorsOptions binds from Cors:* and is mapped as string[]. Format is valid.
  • Scheme correctness — prod uses HTTPS, local (compose.yaml) and QA (compose.qa.yaml) use HTTP on localhost. Matches the actual deployments.
  • Test coverageCorsStartupIntegrationTests already covers both the empty-list boot failure and the allow-listed-origin path; no new tests needed for a compose-only change.
  • compose.dev.yaml — correctly left untouched: it sets ASPNETCORE_ENVIRONMENT: Development, which skips the strict guard (only a warning is logged).

Suggestions (non-bloquantes)

  • compose.prod.yaml comment — the sentence "The public site is same-origin via the web SSR proxy, but the policy must still list every real frontend host" is slightly misleading: if SSR truly proxies all browser requests, CORS wouldn't apply to those requests at all. The real reason to list the domains is for any direct-to-API call (admin panel, mobile, etc.). Minor wording nit.
  • compose.qa.yaml — a single http://localhost:3001 is fine for CI/manual QA, but if the QA stack ever gets a TLS-terminated host, the origin will need updating. Low risk since it's a dev environment.

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