# NFR Design Patterns — U3 HTTP Security Headers & CSP **Unit**: U3 HTTP Security Headers & CSP **NFRs addressed**: NFR-01 (no server configuration), NFR-03 (startup cost), NFR-06 (testability), SECURITY-04, SECURITY-11, SECURITY-15 These patterns exist because U3 moves work that normally lives in nginx into the request pipeline. That changes the failure modes: a bad nginx config fails loudly at reload, whereas a bad middleware silently emits nothing. Every pattern below is chosen so that "wrong" is visible rather than quiet. --- ## Pattern 1 — Fail Closed at Startup: `ValidateOnStart` **Rule**: BR-U3-20 — an unknown policy name must stop the process. **Pattern**: Bind `SecurityHeadersOptions` through the options builder with a validator and `ValidateOnStart()`, so the failure happens during host start rather than on the first request. ```csharp services.AddOptions() .Bind(configuration.GetSection(SecurityHeadersOptions.SectionName)) .Validate(o => o.PathPolicies.All(r => CspPolicyCatalog.IsKnownPolicy(r.Policy)), "SecurityHeaders:PathPolicies contains an unknown policy name.") .Validate(o => CspPolicyCatalog.IsKnownPolicy(o.DefaultPolicy), "SecurityHeaders:DefaultPolicy is not a known policy.") .Validate(o => o.AllowedScriptOrigins.All(IsOrigin) && o.AllowedConnectOrigins.All(IsOrigin), "SecurityHeaders origins must be scheme-and-host only, without a path.") .ValidateOnStart(); ``` **Why `ValidateOnStart` and not a constructor check**: options are resolved lazily. Without `ValidateOnStart`, a typo in `PathPolicies` is discovered when the first request arrives — by which time the deployment has already been reported as successful and the health check is green. `ValidateOnStart` registers an `IStartupValidator` that runs inside `IHost.StartAsync`, so `dotnet SlpModularCms.Api.dll` exits non-zero and the release switch is visibly broken. **Consequence for the deploy workflow (U6)**: this makes a smoke request unnecessary to catch configuration typos — the process simply does not come up. Worth stating, because it is the difference between "deployment failed" and "deployment succeeded and the site is broken". --- ## Pattern 2 — Precomposed Immutable Policies: `ICspPolicyProvider` **Rule**: BR-U3-18 — policy strings are composed once and reused. ```csharp public interface ICspPolicyProvider { SecurityHeaderSet Get(string policyName); IReadOnlyCollection PolicyNames { get; } } public sealed record SecurityHeaderSet( string ContentSecurityPolicy, string FrameOptions, string ReferrerPolicy); ``` **Implementation shape**: a singleton whose constructor builds a `FrozenDictionary` from `CspPolicyCatalog` (the in-code policy definitions) plus the configured origin lists. Nothing is composed per request; the middleware performs one dictionary lookup and three header writes. **Why `FrozenDictionary`**: read-only after startup and read on every response. This is the exact workload it exists for, and the build cost is paid once against a two-entry dictionary. **Why the split into catalog and provider**: `CspPolicyCatalog` is a pure static function of `(policy name, origin lists) → directive list` with no dependencies, so the directive content is unit-testable without a service provider or configuration. The provider only caches it. --- ## Pattern 3 — Path Matching: `StartsWithSegments`, First Match Wins **Rule**: BR-U3-12. ```csharp foreach (var rule in _rules) // ordered, from configuration { if (path.StartsWithSegments(rule.Prefix, StringComparison.OrdinalIgnoreCase)) { return rule.Policy; } } return _defaultPolicy; ``` **`StartsWithSegments`, never `string.StartsWith`.** `"/administrator".StartsWith("/admin")` is `true`. A future route named `/admin-tools` or a public page at `/administration` would silently inherit the strict policy and lose its inline scripts — a failure that looks like a broken page with a console error and no server-side trace at all. `StartsWithSegments` compares whole path segments and does not match. **First match wins, not longest prefix.** The rule list is ordered, and order is meaningful. Longest-prefix matching would be more forgiving but would make the effective policy non-obvious from reading the configuration. Order is documented in the configuration comment and asserted in a test that puts a broader prefix before a narrower one and expects the broader one to win. **Prefixes are normalised at startup** — a configured `admin` or `/admin/` both become `/admin` — because `StartsWithSegments` requires a leading slash and treats a trailing slash inconsistently across overloads. --- ## Pattern 4 — Writing at Response Start Without a Per-Request Closure **Rule**: BR-U3-05 — the content type is unknown before the pipeline continues. ```csharp public async Task InvokeAsync(HttpContext context) { if (!_options.Enabled) { await _next(context); return; } var headerSet = _policyProvider.Get(_resolver.Resolve(context.Request.Path)); context.Response.OnStarting(WriteHeadersCallback, new HeaderWriteState(context.Response, headerSet)); await _next(context); } private static readonly Func WriteHeadersCallback = static state => { var s = (HeaderWriteState)state; SecurityHeaderWriter.Apply(s.Response.Headers, s.Response.ContentType, s.HeaderSet, s.SendHsts); return Task.CompletedTask; }; ``` **Why the `(callback, state)` overload with a `static` delegate**: the lambda overload allocates a closure and a delegate on **every response**, including every static asset of the public website. Static assets are the bulk of the traffic in this hosting model. A cached static delegate plus one small state object is the difference between two allocations per response and none beyond the state. **Why not `IHttpResponseFeature` interception or a `Stream` wrapper**: both are heavier, both risk interfering with `SendFileAsync` fast paths used by static-file middleware, and neither buys anything — headers are still mutable at response start. --- ## Pattern 5 — The Header Decision as a Pure Function (the testability pattern) **NFR-06**, and a trap worth naming. `DefaultHttpContext.Response.OnStarting(...)` **does nothing** — there is no response feature to trigger it. A unit test that builds a `DefaultHttpContext`, invokes the middleware and asserts on `context.Response.Headers` will find them empty, and the natural reaction is to assume the middleware is broken. The opposite mistake is worse: a test that asserts *nothing was set* passes for the wrong reason and keeps passing after the middleware is deleted. **Pattern**: all decision logic lives in a static, dependency-free writer. The middleware is glue. ```csharp internal static class SecurityHeaderWriter { public static void Apply( IHeaderDictionary headers, string? contentType, SecurityHeaderSet set, bool sendHsts) { headers.TryAdd("X-Content-Type-Options", "nosniff"); if (sendHsts) { headers.TryAdd("Strict-Transport-Security", HstsValue); } if (!IsHtml(contentType)) { return; } headers.TryAdd("Content-Security-Policy", set.ContentSecurityPolicy); headers.TryAdd("X-Frame-Options", set.FrameOptions); headers.TryAdd("Referrer-Policy", set.ReferrerPolicy); } } ``` | Component | Test style | Why | |---|---|---| | `CspPolicyCatalog` | Pure function over origin lists | Directive content, `'unsafe-inline'` placement, BR-U3-13 | | `SecurityHeaderWriter` | `new HeaderDictionary()` directly | Per-header scoping, HSTS gating, never-overwrite | | `PathPolicyResolver` | Pure function over `PathString` | `/administrator` vs `/admin`, ordering | | Options validation | `ValidateOnStart` via a real `ServiceProvider` | Startup failure on an unknown name | | Composed middleware | Carried to phase-level Build and Test | Needs a real response feature; `DefaultHttpContext` cannot express it | **`TryAdd`, not the indexer** (BR-U3-04): `headers["X-Frame-Options"] = value` overwrites. `TryAdd` is a no-op when the key exists, which is exactly the rule, and it states the intent in the call rather than in a surrounding `if`. --- ## Pattern 6 — Content-Type Detection ```csharp private static bool IsHtml(string? contentType) => contentType is not null && MediaTypeHeaderValue.TryParse(contentType, out var parsed) && parsed.MediaType.Equals("text/html", StringComparison.OrdinalIgnoreCase); ``` **Why parse rather than `Contains("text/html")`**: the real header is `text/html; charset=utf-8`, so a substring check happens to work — until something serves `application/xhtml+xml` or a type whose *parameter* contains the string. Parsing states the intent and costs nothing at this volume. **`null` is not HTML**, which is what makes the `304`, redirect and no-body cases in BR-U3's edge-case table fall out automatically rather than needing their own branches. --- ## Pattern 7 — Never Throw Into the Response Path **Rule**: BR-U3-07. An exception thrown from an `OnStarting` callback surfaces after the response has begun — too late for the exception handler, and it produces a truncated or malformed response instead of a diagnosable error. The callback therefore wraps the writer: ```csharp try { SecurityHeaderWriter.Apply(...); } catch (Exception ex) { s.Logger.LogError(ex, "Failed to apply security headers to the response."); } ``` **This is deliberately the one place that swallows.** The alternative — letting it propagate — converts a header-writing bug into a corrupted response for the user. The startup validation in Pattern 1 is what keeps this catch from hiding configuration errors: by the time a request arrives, the configuration is already known good, so anything reaching this catch is a code defect and belongs in the log (and, via U4, in Sentry at `Error`). --- ## Pattern 8 — HSTS Gating Resolved Once `sendHsts` is `!IsDevelopment()`, evaluated **once at startup** and stored on the middleware, not per request. `IWebHostEnvironment.IsDevelopment()` is a string comparison against `EnvironmentName` that cannot change while the process runs; evaluating it per response is pure waste on a static-file workload. The value is passed explicitly into the writer rather than read from ambient state, so the Development and non-Development cases are both directly testable (BR-U3-02, BR-U3-08). --- ## Pattern 9 — Startup Logging and the Umami Drift Check **Rules**: BR-U3-22, BR-U3-23, BR-U3-24. BR-U3-23 (log permitted origins) and BR-U3-24 (warn when disabled) are logged from `UseCmsSecurityHeaders()`, which runs once during pipeline construction and has `app.Services` available. No hosted service is needed. ### Refinement REF-U3-01 — BR-U3-22 cannot work as specified BR-U3-22 asks for a startup warning when "a Umami website ID is configured but its script origin is absent from the allowed origins". **The backend cannot perform that check.** The Umami website ID is a `VITE_` variable baked into the frontend bundle at build time; the running host never sees it. Comparing `AllowedScriptOrigins` against a value the process cannot read would either always warn or never warn. The check is therefore **moved to where both values are visible — the CI workflow (U5)**: | Where | What is checked | On failure | |---|---|---| | ~~Backend startup~~ | ~~Umami ID configured, origin missing from CSP~~ | Withdrawn — not implementable | | **U5 CI workflow** | If `VITE_UMAMI_SCRIPT_URL` is set for an environment's frontend build, its origin must appear in that environment's `SecurityHeaders:AllowedScriptOrigins` | **Pipeline fails** | | Backend startup (retained) | Permitted script and connect origins | Logged at `Information` (BR-U3-23) | This is stronger than the rule it replaces: a drift between the frontend build and the backend CSP now blocks the deployment instead of writing a warning into a log file on a Raspberry Pi that nobody reads until something is already broken. **Carried to U5** as a required gate. Recorded here rather than silently dropped, because "we warn about this" appearing in a design and not in the code is exactly the kind of gap that survives to production. --- ## Pattern 10 — Pipeline Position ``` UseExceptionHandler() ← already present UseCmsSecurityHeaders() ← NEW: first thing inside the exception handler UseRateLimiter() UseHttpsRedirection() UseCmsStaticContent() ← short-circuits; must be after us (BR-U3-06) UseCors() UseModules() ← availability gate; its 503 still gets headers UseAuthentication() / UseAuthorization() MapControllers() / MapCmsHealthChecks() / MapCmsSpaFallbacks() ``` Two ordering constraints, both load-bearing: - **Inside `UseExceptionHandler`** (BR-U3-09): the exception handler re-executes the pipeline from inside itself, so middleware registered outside it never observes the `ProblemDetails` response. Registering there means error responses carry the headers too. - **Before `UseCmsStaticContent`** (BR-U3-06): static-file middleware terminates the request. Everything registered after it is invisible to the public website — which is almost all of the HTML this host serves. **Ordering is verified by test, not by comment.** A phase-level Build and Test assertion requests a static website asset and a `503` from the availability gate and asserts on the headers of both. A misordered registration compiles, starts, passes every unit test, and serves the entire website without a CSP. --- ## Pattern 11 — Configuration Shape ```jsonc "SecurityHeaders": { "Enabled": true, "DefaultPolicy": "Relaxed", "PathPolicies": [ { "PathPrefix": "/admin", "Policy": "Strict" }, { "PathPrefix": "/api/v1", "Policy": "Strict" }, { "PathPrefix": "/health", "Policy": "Strict" } ], "AllowedScriptOrigins": [], "AllowedConnectOrigins": [] } ``` Follows the existing `IOptions` convention used by `JwtSettings`, `Availability`, `MasterModule`, `MasterPolling` and `RateLimiting`. No secret appears in this section — origins are public hostnames — so it is committed with real values per environment rather than supplied through environment variables (contrast D-16, which governs credentials). **The defaults are the production values.** An environment that supplies no `SecurityHeaders` section at all gets the strict policy on `/admin`, `/api/v1` and `/health` and the relaxed policy elsewhere. Omission cannot produce an unprotected host.