fix(api): fail loudly when Cors:Origins is empty outside Development by ilyanfraimbault · Pull Request #511 · ilyanfraimbault/TrueMain

ilyanfraimbault · 2026-06-15T12:53:55Z

What

Closes #209.

Api/Program.cs built the FrontendCors policy with AllowAnyHeader().AllowAnyMethod() and only added WithOrigins(...) when the configured array was non-empty. Since appsettings.json ships "Cors:Origins": [], production silently ran a no-op CORS policy: the frontend appeared to work locally (Development ships real origins) but broke in prod — a silent-failure class of bug.

Changes

New typed CorsOptions (Cors:*, with an Origins array) replacing the ad-hoc GetSection(...).Get<string[]>() read, so there is a single source for the origin list.
Startup validation (ValidateOnStart): empty Cors:Origins fails the boot loudly in any non-Development environment. The failure message names the section so the boot log is actionable.
Development: an empty list is tolerated but logs a warning after build (Development normally ships local origins, so this only fires when a local override is missing).
The FrontendCors policy is now built from the bound options.

Tests

TrueMainWebApplicationFactory now injects a default Cors:Origins entry — the integration suite runs under the non-Development Testing environment, so without it every test would now fail the new boot guard.
New CorsStartupIntegrationTests covers both paths: empty origins in a non-Development environment throws OptionsValidationException at startup, and a configured origin boots and is echoed back via Access-Control-Allow-Origin.

Note: no .NET SDK is available in this remote environment, so the build/tests were not run locally — relying on CI.

https://claude.ai/code/session_01Pz4RjCbSEgD36SaM5SavWg

Generated by Claude Code

appsettings.json ships "Cors:Origins": [], so the previous policy built AllowAnyHeader/AllowAnyMethod with no WithOrigins call — a no-op CORS policy that silently rejects the frontend in production while working locally (Development ships real origins). Promote the section to a typed CorsOptions class and validate it on start: empty Origins fails the boot in any non-Development environment, and only logs a warning under Development. Builds the FrontendCors policy from the bound options so there is a single source for the origin list. The test web factory now injects a default Cors:Origins entry (it runs under the non-Development "Testing" environment) and a new integration test covers both the fail-loud boot and the configured-origin path. Closes #209

claude

Corrige le bug Cors:Origins vide (#209). ValidateOnStart fait echouer le boot avant toute requete servie; TrueMainWebApplicationFactory mis a jour pour Testing; deux chemins critiques couverts par des tests. Aucun point bloquant. Suggestions non bloquantes dans les commentaires inline: (1) Program.cs double lecture de config, (2) CorsStartupIntegrationTests assertion sur le status code de /champions couple le test CORS a la logique metier.

- Fix CS1734: drop the <paramref> tag from the test factory's class doc comment (a class has no parameters to reference). - Fix IDE0005: remove the redundant TrueMain.TestKit using (it is a global using) and the now-unused System.Net using. - Consolidate the CORS config to a single source: bind CorsOptions once and build the FrontendCors policy from the bound options via Configure<IOptions<CorsOptions>>, instead of reading configuration a second time eagerly for AddCors. - Decouple the CORS integration test from /champions business logic by asserting the Access-Control-Allow-Origin header on a preflight request rather than on a 200 from the endpoint.

claude

Approuvé

Le correctif est solide. La cause racine était claire (politique CORS no-op silencieuse en production) et la solution adresse chaque couche correctement : options typées, validation au démarrage, avertissement en Development, et couverture de test des deux chemins.

Points positifs :

ValidateOnStart est l'outil idiomatique pour ce genre de guard ; fail-fast bien placé.
CorsStartupFactory, indépendante de TrueMainWebApplicationFactory, évite de polluer la base commune.
Le preflight OPTIONS exerce le middleware CORS directement, sans coupler au code métier d'un endpoint.
DefaultCorsOrigin injecté dans TrueMainWebApplicationFactory : la suite existante ne casse pas.

Suggestions (non bloquantes) :

CorsOptions.cs ligne 18 - Le nom CorsOptions force la qualification complète de Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions dans Program.cs. Renommer en FrontendCorsOptions éliminerait l'ambiguïté.
Program.cs ligne 65 - Le if (Origins.Length > 0) est un chemin mort en non-Development (garanti par ValidateOnStart). Un commentaire d'invariante éviterait une suppression erronée.
CorsStartupIntegrationTests.cs ligne 40 - WebApplicationFactory peut encapsuler l'OptionsValidationException ; si le test échoue en CI, envisager .WithInnerException pour cibler l'exception racine.

Address non-blocking review feedback: - Rename the typed options class to FrontendCorsOptions so it no longer collides with ASP.NET Core's Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions, removing the full-qualification burden for readers. - Document that the Origins.Length > 0 guard in the policy builder is a Development-only path: ValidateOnStart already guarantees a non-empty list everywhere else.

claude

Synthese: corrige le bug de silent-failure CORS (#209). appsettings.json livrait Cors:Origins vide, construisant une politique sans origines autorisees invisible en Development et cassee en prod. FrontendCorsOptions bien nommee, SectionName en source unique, ValidateOnStart echoue au demarrage hors Development, warning en Development apres app.Build() sans risque de lever une exception. Wiring DI correct. Tests : les deux chemins couverts, CorsStartupFactory intentionnellement separee de TrueMainWebApplicationFactory. Suggestions non bloquantes : AllowAnyHeader/AllowAnyMethod heritage a restreindre dans une iteration future ; test preflight sans assertion sur le statut 204 (voir commentaire inline).

develop added a readiness-health-check guard that fails fast on a missing ConnectionStrings:TrueMain only under Production. The test host injects the connection string after Program reads it at startup, so a Production host trips that guard before reaching the CORS validation. Use the Testing environment (still non-Development, so the CORS guard is exercised) which is exempt from the Production-only connection-string check, matching TrueMainWebApplicationFactory.

claude

Verdict : Approuve

Le PR corrige proprement le bug #209 (politique CORS no-op silencieuse en production). La logique est solide : validation au demarrage via le pattern Options, avertissement en Development, et tests d'integration qui couvrent les deux chemins (boot echoue / origine autorisee).

Aucun point bloquant identifie.

Suggestions (non bloquantes) postees en commentaires inline :

CorsStartupIntegrationTests.cs L34 - 'using' synchrone alors que l'autre test utilise 'await using'. Rendre le test async pour la coherence et la surete.
Program.cs L61 - nom complet qualifie pour CorsOptions ; un alias 'using' en tete de fichier ameliorerait la lisibilite.
Program.cs L211 - concatenation plus dans le template de log ; un seul litteral est preferable pour les analyseurs statiques.

- Program.cs: introduce an AspNetCorsOptions using-alias instead of the fully-qualified Microsoft.AspNetCore.Cors.Infrastructure.CorsOptions. - Program.cs: collapse the Development CORS warning into a single log template literal (friendlier to logging analyzers). - CorsStartupIntegrationTests: assert the preflight returns 204 No Content for clearer diagnostics, and make the empty-origins test async with await using for consistency with the other test.

claude · 2026-06-15T14:02:51Z

+    /// Development.
+    /// </summary>
+    public string[] Origins { get; set; } = [];
+}


NON BLOQUANT — set peut être remplacé par init : les options sont liées une seule fois au démarrage et ne doivent pas être mutées ensuite. Le binder d'options .NET 6+ supporte init sans problème.

Suggested change

}

public string[] Origins { get; init; } = [];

claude · 2026-06-15T14:02:54Z

+        // a missing connection string only under Production, and the test host
+        // injects that string after Program reads it.)
+        await using var factory = new CorsStartupFactory(_fixture, environment: "Testing", origin: null);
+


NON BLOQUANT — startup.Should().Throw<OptionsValidationException>() invoque la lambda de façon synchrone. Si la stack d'hébergement venait à envelopper l'exception dans une AggregateException (comportement possible selon la version d'hôte), l'assertion échouerait avec un message peu lisible. En pratique ValidateOnStart propage OptionsValidationException directement dans la version actuelle de l'hôte générique, mais il pourrait être utile d'ajouter .WithInnerExceptionExactly<OptionsValidationException>() comme filet de sécurité si ce comportement venait à changer.

claude

Revue

Le PR corrige correctement le bug silencieux de l'issue #209 : l'absence d'origines dans Cors:Origins construisait une politique CORS valide mais inerte, ce qui se traduisait par un comportement "fonctionne en local, cassé en prod". L'approche choisie (typed options + ValidateOnStart + avertissement Development) est robuste et bien testée.

Points vérifiés

Logique de validation : le prédicat isDevelopment || options.Origins.Length > 0 est correct — isDevelopment est capturé par valeur (bool), il ne peut pas dériver. Le ValidateOnStart est correctement chaîné.
Câblage CORS via AddOptions<AspNetCorsOptions>() : pattern non idiomatique mais techniquement correct — IOptions<FrontendCorsOptions> est résolu de façon paresseuse à la première invocation du middleware, après que ValidateOnStart a déjà réussi ou échoué. Le commentaire inline explique l'intention.
Avertissement post-build : limité à Development, résout IOptions<FrontendCorsOptions> depuis le conteneur (cohérent avec la source unique voulue par le PR).
Tests : Startup_InNonDevelopment_FailsWhenCorsOriginsEmpty couvre le chemin d'échec ; Startup_InNonDevelopment_AllowsConfiguredOriginWhenPresent vérifie le header Access-Control-Allow-Origin via une vraie requête preflight sans couplage aux endpoints. TrueMainWebApplicationFactory est correctement mis à jour pour ne pas casser la suite existante.
Sécurité : AllowAnyHeader().AllowAnyMethod() sans AllowCredentials() — comportement pré-existant, le PR ne l'aggrave pas.

Aucun point bloquant.

Suggestions (non bloquantes) listées en commentaires inline :

Origins { get; set; } → init pour exprimer l'immuabilité après liaison.
L'assertion Should().Throw<OptionsValidationException>() pourrait être fragilisée par une évolution du comportement de propagation de l'hôte générique.

ilyanfraimbault marked this pull request as ready for review June 15, 2026 12:54