Web failure log: keep the log batch on a split character, answer bad requests with their own status

[erikdarlingdata]

Follow-up to #4281, which fixed #4276.

#4281 merged before its security review ran. The review found one Medium and four Lows. This PR fixes all five. It also fixes the same log-writing bug in seven other loggers in Darling, Lite and the Dashboard. A round-1 security review of this PR found six Lows and one note. Commit 117c4623 fixes all of them.

Why

#4281 added failure handling to the web viewer. The review found these gaps in it:

• A request whose route was cut inside a character made the service's log writer throw. Every other line in the same 5-second batch was then lost.
• A malformed or oversized request body cost a 500 and an Error line in the log.
• With a Development environment variable set on the machine, a throw that escaped answered with the exception text and a stack trace.
• A line break in exception text started what looked like a second entry in the file log.
• Three routes still answered with raw exception text.

What changes

1. A split character no longer drops a log batch (Medium)

DarlingHttpRefusalLog.Sanitize cuts request text, such as a route or a Host header, to a fixed length. When the cut fell between the two halves of a surrogate pair, it left a lone half at the end. The file log then wrote its batch with File.AppendAllText and a strict UTF-8 encoder. That encoder throws on a lone surrogate and writes nothing, so the whole 5-second batch was lost.

Two fixes:

• Sanitize never cuts inside a pair, and it maps any other lone surrogate to ..
• DarlingFileLoggerProvider writes with new UTF8Encoding(encoderShouldEmitUTF8Identifier: false). That encoder writes U+FFFD instead of throwing, and it still writes no byte-order mark.

2. The same encoder fix in seven other loggers (a47701fd, 6fe73149)

Seven more loggers wrote their batches with the strict encoder: Darling's ViewerLogger, Lite's AppLogger, MethodProfiler and QueryLogger, and the Dashboard's Logger, MethodProfiler and QueryLogger. Each now passes the same encoder. Lite has no web server, so only this half of item 1 applies there.

AppLogger gains FlushTo(logDirectory), the write half of Flush. A test can then write a real batch without calling Initialize, which repoints the whole process's logging and starts a timer. The new Lite test runs in the app-logger-statics collection, with the other tests that read the shared log buffer.

3. A bad request answers with its own status (Low)

The web host's catch-all now catches BadHttpRequestException first. Kestrel throws it for a body that is too large, malformed or too slow (413, 400 or 408). The answer is that status with no body, and the log line is Debug. The service logs Information and above by default, so a client that sends many bad requests cannot fill the log.

A client that drops the connection is not a failure either. When the request was aborted, the catch-all now treats an IOException the same as an OperationCanceledException, as ASP.NET Core's own exception handler does. Before, a reset during a body read reached the generic arm and wrote an Error line. (Round 1, Low 2.)

4. Both web hosts pin the Production environment (Low)

The web host and the MCP host now build with EnvironmentName = Environments.Production. Without the pin, ASPNETCORE_ENVIRONMENT or DOTNET_ENVIRONMENT set to Development anywhere on the machine added the developer exception page ahead of the Host guard. A throw that escaped then answered with the exception text and a stack trace. The MCP host's loopback bind needs no token. (The MCP host's pin is round 1, Low 1.)

This PR also corrects the catch-all's comment. The comment said the catch-all sees a throw from the Host guard or the bearer check. It is registered after both, so it sees only throws from the routes.

5. The file log cleans every line (Low)

An exception message can repeat request text. A PostgreSQL cast error quotes the bad value, and KeyNotFoundException quotes the key. A CR or LF in that text started what looked like a second entry in the file log.

DarlingFileLoggerProvider now cleans each whole line before it writes it. About 80 call sites pass ex.Message as a template argument (LogError("... {Message}", ex.Message)), so the text sits in the formatted message, not only in the exception object. Cleaning the whole line covers both. (Round 1, Low 5. This replaces the first version, which cleaned only the exception object's message.)

• Control characters become ..
• Tab stays.
• A line feed stays, and the next line starts with four spaces. So no line of embedded text can start in column 0, where each real entry's timestamp is. DarlingWorker's "Store host profile" block at startup uses line feeds on purpose, and it stays readable.
• A CR just before an LF counts as one line break. A CR on its own becomes ..

The Debug line from item 3 no longer passes bad.Message as a template argument.

6. Every control-character check covers the same characters (round 1, Low 4)

Three checks tested only c < ' ' || c == 0x7F:

• DarlingHttpRefusalLog.Sanitize
• the file log's line clean from item 5
• DarlingWebOidc.SubjectCarriesControlCharacters, which refuses a sign-in whose subject has a control character

All three now use char.IsControl(c), plus U+2028 and U+2029. So they also catch the C1 range, such as U+0085. Kestrel decodes those from a percent-encoded path such as %C2%85, and an identity provider can put them in a subject claim. U+2028 and U+2029 are Unicode line breaks that some log readers split on.

Sanitize also checks that its cut is above 0 before it reads the character before the cut. No caller passes a length of 0 today. (Round 1 note.)

7. Three routes no longer answer with exception text (Low, and round 1's Low 3)

• The /api/read/* dispatcher's own catch answered with McpHelpers.FormatError(name, ex), which puts ex.Message on the wire. A PostgresException message can name schema objects or roles, and an Npgsql connection error names the store's host and port.
• /api/sweeps/latest and /api/sweeps/{id} answered with ex.Message and wrote no log line.

All three now work like the catch-all. DarlingWebFailureLog.Report writes the log line. The answer is DarlingWebFailureLog.Body with DarlingWebFailureLog.StatusCode: a fixed message, with 503 for a statement timeout and 500 for anything else. The body keeps the {"error": "..."} shape that the web pages already read.

Not changed here, and tracked by #4283: ToHttpResult's ServerError arm, /api/compose/run, the triage page, and the six Custom View and alert-rule write routes.

Tests

• DarlingHttpRefusalLogTests: a surrogate pair across the cut is never split, and the output passes a strict UTF-8 encoder. A lone surrogate elsewhere maps to .. U+0085 and U+2028 map to ..
• DarlingFileLoggerProviderTests: a batch with a lone surrogate still writes its other line. A CRLF followed by a fake timestamp stays one entry, and only one line starts with a timestamp. U+0085 and U+2028 map to .. The host profile block keeps each row on its own indented line.
• DarlingWebFailureHandlingTests: a BadHttpRequestException answers 413 with an empty body and one Debug line, and no Warning or Error. An abort that surfaces as IOException writes no Error line. A source check finds EnvironmentName = Environments.Production in both hosts' CreateBuilder calls. A source check finds DarlingWebFailureLog in the read dispatcher's catch, and no FormatError.
• FleetSweepWebFeedTests: a source check finds DarlingWebFailureLog in both sweep reads' catches, and no ex.Message.
• DarlingWebOidcTests: a subject with U+0085 or U+2028 is refused.
• ViewerLoggerTests and Lite's AppLoggerSurrogateFlushTests: a batch with a lone surrogate still writes its other line.

These tests failed with the product change reverted and passed with it restored:

• the three surrogate tests from item 1
• ViewerLoggerTests and AppLoggerSurrogateFlushTests
• the IOException abort test
• the U+0085 and U+2028 tests for Sanitize and the file log
• the three file log line tests, run against the file as it was before this PR

Test plan

• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: 0 warnings, 0 errors.
• Full Darling.Tests on 117c4623, once: 14,007 tests, 0 failed, 738 skipped, 1 not run. The skips are the live classes. This run had no PostgreSQL rig.
• GitHub Actions CI on 117c4623: build, Darling PostgreSQL tests and Lite tests all pass.

CHANGELOG entry

SECTION: Fixed
ENTRY:

• Web failure handling: log batches, bad requests and error text ([Web failure log: keep the log batch on a split character, answer bad requests with their own status #4286]) - A route cut inside a character made Darling's service log drop its whole 5-second batch. The viewer's, Lite's and the Dashboard's log files had the same bug for any text with a broken character. A malformed or oversized request, or a client that dropped the connection, cost a 500 and an Error line. It now gets its own status and no Error line. A line break in exception text started what looked like a second entry in the service log. Each line is now cleaned before it is written. The read dispatcher and the fleet sweep reads answered some failures with raw exception text, such as a connection error that names the store's host. They now answer with a fixed message and log the detail. Darling's web and MCP hosts now always run as Production, so a stray Development setting on the machine cannot turn on the developer error page.
  REF:
  [Web failure log: keep the log batch on a split character, answer bad requests with their own status #4286]: Web failure log: keep the log batch on a split character, answer bad requests with their own status #4286

🤖 Generated with Claude Code

https://claude.ai/code/session_01FVjn4PBJN71NQXdFo6ZxNQ

[erikdarlingdata]

Parity fix: the other seven batch loggers get the same non-throwing UTF-8 write

Pushed as two commits on fix/4281-failure-log-hardening (head now 6fe73149, was 5158b339):
a47701fd (the seven-file fix) and 6fe73149 (the seam plus both new tests).

The fix (all seven files)

File.AppendAllText(path, text) with no encoding uses a strict UTF-8 encoder. It throws
EncoderFallbackException on a lone surrogate and writes zero bytes. Each site below now passes
new UTF8Encoding(encoderShouldEmitUTF8Identifier: false) as the third argument. Each also gets a one-line
comment pointing at the #4281 review, matching the comment style already in that file: /* */ in the Lite
and Darling files, // in the deprecated Dashboard files, since that is what each file already used
elsewhere.

• Darling/PerformanceMonitor.Darling.Viewer/ViewerLogger.cs:127
• Lite/Services/AppLogger.cs:303
• Lite/Helpers/MethodProfiler.cs:125
• Lite/Helpers/QueryLogger.cs:92
• deprecated/Dashboard/Helpers/Logger.cs:84
• deprecated/Dashboard/Helpers/MethodProfiler.cs:174
• deprecated/Dashboard/Helpers/QueryLogger.cs:137

All seven already had using System.Text;, so no new usings were needed. A repo-wide grep for
encoderShouldEmitUTF8Identifier confirms the only other hits are DarlingFileLoggerProvider.cs (#4286's
own fix) and StoreLogSlab.cs (already fixed earlier). No eighth site was missed. The
DarlingManagedPostgres.cs conf appends and the StreamWriter key-file writes were left alone, as
directed, since they write generated ASCII text.

The one added seam

AppLogger.Flush() only ever wrote through the process-wide static s_logDirectory, gated on
s_initialized. Only Initialize() sets either field, and Initialize() is documented in
AppLoggerRetentionTests as unsafe to call from a test. It repoints the whole process's logging and starts
a 5-second timer. So Flush() was split. The public method keeps the s_initialized guard. It now
delegates to a new internal static void FlushTo(string logDirectory), which holds the lock, drain, write
and catch that used to be inline. This is the same shape as the existing CleanOldLogs(string) and
DrainBufferedLines() seams, added for the identical reason. Production behavior is unchanged: Flush()
now calls FlushTo(s_logDirectory), and nothing else changed.

ViewerLogger needed no equivalent seam. Nothing else in Darling.Tests touches it: one comment mention,
no calls. So the test below calls Initialize directly with no risk of colliding with a peer test.

Tests

Lite: Lite.Tests/AppLoggerSurrogateFlushTests.cs, method
FlushTo_ALoneSurrogateInOneLine_StillWritesTheBatchsOtherLines. It joins the existing
[Collection("app-logger-statics")], shared with LiteLogLevelGateTests and the Entra credential gate
tests, since it touches the same process-wide s_buffer. It drains the buffer, enqueues a line carrying a
lone high surrogate plus a GUID-tagged normal line, calls the new FlushTo(tempDir), and asserts the
tagged line reached the file.

Darling viewer: Darling/Darling.Tests/ViewerLoggerTests.cs. It is reachable because
Darling.Tests.csproj references PerformanceMonitor.Darling.Viewer.csproj. Same shape, method
Flush_ALoneSurrogateInOneLine_StillWritesTheBatchsOtherLines. It calls the real Initialize(tempDir),
then Info, then Flush, then Shutdown() in Dispose. It is kept to one [Fact] in its own class,
because Shutdown() disposes the static flush timer permanently. A second Initialize call in the same
process would land in the catch and silently no-op. That is fine for one test and would not work for two
in sequence. It adds [Collection("viewer-logger-statics")] as the named place a future class touching
ViewerLogger should join, matching nothing today, the same way app-logger-statics documents the same
tradeoff on the Lite side.

Dashboard (deprecated): no test project work, build only, as directed.

Revert-proof (both)

For each of the two tests, only its target File.AppendAllText call was reverted back to the old
two-argument form, then rebuilt, then run alone.

• Lite: failed. Assert.Contains() Failure: Sub-string not found, String: "". The file was created empty.
  The encoder threw and the catch swallowed it.
• Darling viewer: same failure shape, same empty-string result.

The fix was then restored in both files, rebuilt, and both passed again.

Build results (all three, as directed)

• dotnet build Lite.Tests/Lite.Tests.csproj -c Debug: Build succeeded, 0 Warning(s), 0 Error(s).
• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: Build succeeded, 0 Warning(s), 0
  Error(s).
• dotnet build deprecated/Dashboard/Dashboard.csproj -c Debug: Build succeeded, 0 Warning(s), 0 Error(s).

Test results

• Lite.Tests, filtered to AppLoggerSurrogateFlushTests, AppLoggerRetentionTests,
  LiteLogLevelGateTests and EntraCredentialSelectionModeGateTests (the new test plus its whole
  app-logger-statics collection, run together to confirm no cross-test interference): Total 15, Failed 0.
• Darling.Tests, filtered to ViewerLoggerTests and DarlingFileLoggerProviderTests: Total 9, Failed 0.

No full-suite run, per the brief. No CHANGELOG entry was made. The buffer directory was none, so the PR
tender owns that entry.

One thing to flag

The brief's commit trailers named session session_01TszxYhJJbTEh4LrZ56NYo3. Both commits carry that id.
A separate default in this environment pointed at a different session id. The brief's id was used, since
the brief is the authoritative, self-contained instruction for this dispatch.

Nothing deferred

Everything in the brief is done: all seven files, both tests, both revert-proofs, all three builds. The PR
stays a draft, not readied, and its body is untouched, per instructions.

[erikdarlingdata]

Round-1 security review at 5158b33

Scope: the diff of this PR at 5158b33. That covers Sanitize, the file logger, the backstop, the /api/read/* dispatcher catch, the options of the web host builder, and the new tests. I read the code at that commit and the code that it calls. I did not build or run anything, so CI must show the new tests green. The encoding change that another commit adds to the other batch loggers is not part of this review.

Result: the five findings of the #4281 review are fixed as this PR scopes them. I found no High and no Medium. I found 6 Low items. Most of them are small edits next to code that this PR already changes.

Answers to the five questions

1. Finding 1 is fixed in both places. No request can now cost a log batch.
   • Sanitize (Hosting/DarlingHttpRefusalLog.cs:300-330) cannot emit a lone surrogate. If the cut splits a pair, the cut moves back one character. Inside the kept text, a surrogate that is not half of a whole pair becomes .. That includes a lone high surrogate at the end of a value that was not cut. The ellipsis still follows a cut value.
   • Flush (DarlingFileLoggerProvider.cs:169-170) now writes with new UTF8Encoding(false). That encoding replaces a bad character with U+FFFD and does not throw. So a lone surrogate that skips Sanitize costs one character, not the batch.
   • A batch is still lost when the write itself fails. Examples are a full disk, a lock that another process holds, or a changed ACL. The catch at :173-181 drops the batch and reports only the first failure. That was true before this PR, and a request cannot cause it.
   • The fix also closes a path that the Log and answer web-viewer read failures instead of an empty 500 (#4276) #4281 review did not name. The OIDC callback logs the error_description query value through Sanitize at the 64-character default (Mcp/DarlingWebHostService.cs:1700). That route needs no credential when OIDC is on.
   • Control characters: Sanitize still passes the C1 controls and U+2028/U+2029. See Low 4.
2. The BadHttpRequestException catch (Mcp/DarlingWebHostService.cs:1217-1231) does what it claims.
   • The name resolves to Microsoft.AspNetCore.Http, the base type. Kestrel's own type derives from it, so the catch takes both.
   • The status comes from Kestrel. No code in the service, the storage project or the common project throws this type.
   • The body is empty, as it was when Kestrel answered. Kestrel's own answer also cleared the headers that the app had set. Here the no-store header stays, which is fine. I expect Kestrel to close the connection afterward, because its drain of the request body hits the same error. I did not test this.
   • The line goes to Debug only. Program.cs sets no minimum level, so the default of Information drops it before it reaches the file.
   • No exception type escapes the backstop. The generic arm takes all other types and answers with one of two constant sentences.
   • But some client aborts still reach the generic arm as an Error (Low 2). The Debug line also carries the message outside the new sanitizer (Low 5).
3. Nothing in the service depends on a non-Production environment.
   • The service ships no appsettings*.json, no launchSettings.json and no UserSecretsId. No code calls IsDevelopment() or UseDeveloperExceptionPage, or sets ThrowOnBadRequest.
   • Static files do not depend on it. The csproj copies wwwroot beside the binary in every build (PerformanceMonitor.Darling.Service.csproj:72).
   • The pin removes the developer exception page and the DI scope checks. It also stops minimal API binding errors from throwing, because in Production they answer 400 directly.
   • The failure-handling tests build their own host (Darling/Darling.Tests/DarlingWebFailureHandlingTests.cs:187) with no environment, so they run as Production.
   • But the MCP host did not get the same pin (Low 1), and the new test has two problems (Low 6).
4. The dispatcher's own catch (DarlingWebEndpoints.cs:319-330) now answers with the constant body and the 503/500 split. A client abort still goes to the backstop, which writes nothing.
   • Exception text still reaches this route through the ServerError arm of ToHttpResult (:3082). That arm answers a tool's own caught exception with its text. It is the common case, and Web viewer: tool errors still show the exception text in the browser #4283 tracks it.
   • The 400 arm (:3083) echoes any plain string that a tool returns. Among the tools that this route calls, I found none that puts ex.Message into a plain string or a JSON body outside FormatError.
   • Outside this route, the alert-rule routes answer with the JsonException text of the caller's own definition (CustomAlertRuleDefinition.cs:236, used at DarlingWebEndpoints.cs:598 and :716-721). That text describes the caller's own input, like the 400 of /api/compose/run that Web viewer: tool errors still show the exception text in the browser #4283 keeps. I count it as acceptable.
5. I found no request value that reaches a formatted log line without sanitizing today. The checks are listed at the end. Exception text still goes around the new sanitizer in two ways (Low 5).

Low 1: the MCP host did not get the environment pin

Where: Mcp/DarlingMcpHostService.cs:457.

What fails: WebApplication.CreateBuilder() gets no options. If ASPNETCORE_ENVIRONMENT or DOTNET_ENVIRONMENT is Development for the machine or the service, the MCP port gets the developer exception page. WebApplication puts that page first, ahead of the Host guard (:982) and the bearer check (:1011). A throw that escapes then answers with the exception message and the stack trace. The loopback bind has no token. This is the threat of finding 4, on the endpoint that has write tools.

Who: the same conditions as finding 4. Someone must set the variable, and something must throw.

Fix: pin it the same way.

var builder = WebApplication.CreateBuilder(new WebApplicationOptions
{
    EnvironmentName = Environments.Production,
});

Low 2: a client that resets an upload still writes an Error line

Where: the abort filter at Mcp/DarlingWebHostService.cs:1214, and the generic arm at :1232-1244.

What fails: the filter takes only OperationCanceledException. Kestrel reports some client aborts during a body read as an IOException. A TCP reset gives ConnectionResetException. An HTTP/2 stream reset gives an IOException with the message "The client reset the request stream." Both go to the generic arm. It writes an Error line with no throttle, and it tries to answer a client that is gone.

ASP.NET Core's own exception handler middleware counts OperationCanceledException or IOException with RequestAborted set as a client abort for this reason. I did not reproduce this.

Who: the same callers as finding 3. /api/compose/run is the POST that a read-only seat can send, and its body read catches only JsonException (DarlingWebEndpoints.cs:519-521). PUT /api/mute-rules/{id}/enabled has the same guard (:848-853). On the TLS listener, one HTTP/2 connection can open and reset many streams. On the loopback bind, any local process can do it with no token.

Fix: use the framework's filter. Add a TestServer case that calls context.Abort() and then throws IOException.

catch (Exception ex) when ((ex is OperationCanceledException or IOException)
    && context.RequestAborted.IsCancellationRequested)
{
}

Low 3: exception text on the wire in routes that #4283 does not list

Where: DarlingFleetSweepEndpoints.cs:202 and :219 (/api/sweeps/latest and /api/sweeps/{id}). Also DarlingWebEndpoints.cs:433, :480 and :499 (views), and :616, :663 and :688 (alert rules).

What fails: each catch answers 500 with ex.Message and writes no log line. The text is a store error, which can name the host and port of the store or schema objects, as the #4281 review described. The sweep routes are GETs, so a read-only seat can see the text. The view and alert-rule routes need an editing seat, or local access on the loopback bind. #4283 names only the ServerError arm, /api/compose/run and the triage page. A fix that follows its list misses these eight sites.

Fix: add the eight sites to #4283. We recommend fixing them in this PR instead, because none of them has the author feedback that keeps the 400 of /api/compose/run. Use the dispatcher's pattern: DarlingWebFailureLog.Report, then the constant body and status. That also gives each one the log line that #4276 asked for.

Low 4: every control-character check here is ASCII-only

Where: Hosting/DarlingHttpRefusalLog.cs:309, DarlingFileLoggerProvider.cs:321, and the subject check at Hosting/DarlingWebOidc.cs:605.

What fails: all three map the characters below a space, and DEL. The C1 controls (U+0080 to U+009F, including NEL, U+0085) pass, and so do U+2028 and U+2029. Kestrel decodes them from a percent-encoded path, for example %C2%85. The identity provider can put them in a subject.

A reader that splits lines on them then sees a forged entry. For example, Python's str.splitlines() splits on all three. ReadLine in .NET and grep do not. The comment on the subject check says that its purpose is to keep forged lines out of the audit trail.

Fix: in all three places, test char.IsControl(c) || c == (char)0x2028 || c == (char)0x2029. char.IsControl covers C0, DEL and C1. This PR already edits two of the three lines.

Low 5: exception text still reaches the formatted message around the new sanitizer

Where: Mcp/DarlingWebHostService.cs:1225. Also about 80 log calls in the service and storage projects that pass ex.Message as a template argument. Examples are Darling/PerformanceMonitor.Darling.Storage/FleetSweepStore.cs:862 and :984, which the sweep timeline route and get_sweep_reports reach.

What fails: SanitizeMessageForLog runs only on the message of the exception object (DarlingFileLoggerProvider.cs:302). The new Debug line also passes bad.Message as {Message}, so the same text lands twice, once raw. The common idiom LogError("... {Message}", ex.Message) never passes the exception object at all. So the fix for finding 5 covers only one of the two ways that exception text reaches the file.

Today I found no request text on either way. Kestrel's body errors quote no input. Minimal API binding errors do quote the bad value, but the environment pin stops them from throwing. The sweep reads use typed parameters, check watch_state against the known names (Mcp/DarlingMcpFleetSweepTools.cs:125), and parse sweep_id.

Fix:

• Drop {Message} from the Debug line: _logger.LogDebug(bad, "Web dashboard request rejected ({StatusCode})", bad.StatusCode);
• To cover every caller, sanitize the whole line at the sink in FileLogger.Log. Map control characters other than line feed and tab to ., and indent each line that follows a line feed. A continuation line then can never start with a timestamp, and the multi-line host profile block stays readable.

Low 6: the new environment test does not pin the fix, and it changes process-wide state

Where: WebHostEnvironment_IsAlwaysProduction_EvenWhenAspnetcoreEnvironmentSaysDevelopment in Darling/Darling.Tests/DarlingWebFailureHandlingTests.cs.

What fails:

• The test builds its own WebApplicationOptions. If someone deletes line 712 of the web host, the test stays green. It tests the framework, not the service.
• It sets ASPNETCORE_ENVIRONMENT and DOTNET_ENVIRONMENT for the whole test process. The test project does not turn off parallel runs. Other classes build hosts with no environment set, for example DarlingWebHostGateLiveTests.cs:58 and DarlingMcpHostGateLiveTests.cs:61. A host that one of them builds during that window runs as Development. The finally block also sets both variables to null, not to their earlier values.

Fix: replace the test with a source pin, like the dispatcher pin, that checks that both hosts set EnvironmentName = Environments.Production. If you keep a behavior test, pass Args = ["--environment=Development"] in the options instead of setting variables. The options override command-line arguments, and nothing leaks to other tests.

Notes, not findings

• Sanitize now reads value[take - 1], so a maxLength of 0 throws IndexOutOfRangeException. Every caller passes 64, 128, 256 or MaxSubjectLength, so it cannot happen today. A take > 0 guard costs one condition.
• SanitizeMessageForLog has no arm for a lone surrogate. That is safe now, because Flush replaces one with U+FFFD.
• This PR puts no length cap on exception text in the log. That holds while no request text reaches an exception message (Low 5).
• When the response has already started, both arms of the backstop return normally. Kestrel then ends the response as if it were complete. The new arm cannot really get there, because routes read their body before they write. For the generic arm this follows the Web viewer: a read that times out returns an empty HTTP 500 and writes nothing to the service log #4276 ruling. We recommend a call to context.Abort() in that branch, so that a client can tell a cut-off response from a whole one.

What I checked and found clean

• The order of the pipeline did not change. It is compression, the Host guard (:1027), the auth gate (:1055, network mode), the backstop (:1206), the no-store header (:1253), then the routes (:1263). The corrected backstop comment now matches the code.
• The sign-in catch (:1121-1130) sends a sanitized message to the throttled refusal log, and it answers with a constant sentence.
• The refusal reasons: the Host header, the error and error_description values of the provider, the exchange error and the subject all go through Sanitize. The ID-token checks (Hosting/DarlingWebOidc.cs:221-306) name only the configured issuer, never a token value. Request.Method in the read-only refusal (:1093) cannot carry CR or LF, because Kestrel allows only token characters there.
• The MCP tool classes that /api/read/* calls write no log lines of their own. get_sweep_reports passes the logger to the sweep store, which logs only exception text (Low 5).
• The mute-rule cores in Mcp/DarlingMcpAlertTools.cs write no log lines, so the {id} value does not reach the log from there.
• Each of the five findings has a matching test. Finding 1 has a cut test and a flush test. Finding 2 has a source pin, and finding 3 has a TestServer test. Finding 4 has the environment test (see Low 6), and finding 5 has a CR/LF test.

[erikdarlingdata]

Round-1 fixes (review at #4286 (comment))

Branch fix/4281-failure-log-hardening, commit 117c462 on top of 6fe7314. All six Low findings plus the
"Notes, not findings" take > 0 item are fixed.

1. Low 1 — MCP host environment pin

Mcp/DarlingMcpHostService.cs:464. WebApplication.CreateBuilder() now passes
new WebApplicationOptions { EnvironmentName = Environments.Production }, the same pin
Mcp/DarlingWebHostService.cs:713 already had. No other options were needed (the MCP host serves no static
files, so it never set ContentRootPath/WebRootPath).

2. Low 2 — abort filter takes IOException too

Mcp/DarlingWebHostService.cs:1215: catch (Exception ex) when ((ex is OperationCanceledException or IOException) && context.RequestAborted.IsCancellationRequested), ASP.NET Core's own client-abort rule.

Test: BrowserAbort_AsIOException_WritesNoErrorLine (Darling.Tests/DarlingWebFailureHandlingTests.cs), a new
TestServer route /api/__test/abort-io that calls context.Abort() then throws IOException. TestServer
surfaces the abort as its own OperationCanceledException out of SendAsync (the harness's client-side
reaction), caught and swallowed in the test itself; what the test actually checks is the shared logger, which
reflects what the backstop did server-side. Revert-proof: reverting the filter to
OperationCanceledException-only turned the assert from 0 to 1 Error line (Assert.Equal() Failure: Expected: 0, Actual: 1), confirmed, then restored.

3. Low 3 — the two sweep GETs

DarlingFleetSweepEndpoints.cs:208 (/api/sweeps/latest) and :229 (/api/sweeps/{id:long}, now built as
"/api/sweeps/" + id) each now do DarlingWebFailureLog.Report(logger, route, stopwatch.ElapsedMilliseconds, ex); then return Results.Json(DarlingWebFailureLog.Body(ex), statusCode: DarlingWebFailureLog.StatusCode(ex));
— the dispatcher's own pattern, using PerformanceMonitor.Darling.Service.Hosting; added. The six view/alert-rule
write sites in DarlingWebEndpoints.cs are untouched (still pending #4283).

Test: TheSweepReadCatches_AnswerTheRuledBodyAndStatus_NotExMessage (Darling.Tests/FleetSweepWebFeedTests.cs),
a source pin mirroring ReadDispatchCatch_AnswersTheRuledBodyAndStatus_NotFormatError. It locates each
MapGet's body in the RAW source (the route literal is plain string content that
CSharpSourceWalker.StripCommentsAndStrings blanks), then strips comments/strings on just that slice before
asserting — needed because this file's own new review-note comments say "ex.Message" in prose, which would
false-fail a DoesNotContain check on unstripped text.

4. Low 4 — C1 controls and U+2028/U+2029 in all three places

All three now check char.IsControl(c) || c == (char)0x2028 || c == (char)0x2029:

• Hosting/DarlingHttpRefusalLog.cs:317 (Sanitize)
• DarlingFileLoggerProvider.cs:349 (the new sink-level CleanLineForLog, folded into item 5's rewrite)
• Hosting/DarlingWebOidc.cs:612 (SubjectCarriesControlCharacters)

Tests, each with U+0085 and U+2028:

• AC1ControlOrUnicodeLineSeparator_MapsToADot (DarlingHttpRefusalLogTests.cs)
• Log_MessageCarriesC1ControlOrUnicodeLineSeparator_MapsToADot (DarlingFileLoggerProviderTests.cs)
• SubjectCarriesControlCharacters_Matrix's new nel\u0085char row, plus the separate
  SubjectCarriesControlCharacters_UnicodeLineSeparator_IsTrue Fact (DarlingWebOidcTests.cs) — U+2028 is a
  real C# line terminator, so it cannot sit in an [InlineData] string/char constant; that one builds
  "before" + (char)0x2028 + "after" at runtime instead.

Revert-proof (both files that gate on the review's own literal repro): reverting DarlingHttpRefusalLog.cs's
check to c < ' ' || c == 0x7F failed AC1ControlOrUnicodeLineSeparator_MapsToADot
(Expected: "before.after", Actual: "beforeafter" — U+2028 passed through, invisible in terminal output
since it's a real line separator). Reverting DarlingFileLoggerProvider.cs's check the same way failed
Log_MessageCarriesC1ControlOrUnicodeLineSeparator_MapsToADot (Not found: "before.middle.end"). Both restored
and re-verified green. (A stale incremental build briefly hid a real green result during this pass — a mv
restore kept an old mtime and dotnet build skipped recompiling; caught by a suspiciously-fast 2.5s build,
fixed by touching the files, and reproduced clean on a genuine rebuild.)

5. Low 5 — sink-level clean, {Message} dropped

Mcp/DarlingWebHostService.cs:1239: _logger.LogDebug(bad, "Web dashboard request rejected ({StatusCode})", bad.StatusCode); — {Message}/bad.Message dropped.

DarlingFileLoggerProvider.cs's FileLogger.Log now cleans the WHOLE assembled line in a new
CleanLineForLog (:333), not just exception.Message — covering the ~80 LogError("... {Message}", ex.Message) call sites the old per-exception-object sanitize never saw, since a template argument becomes part
of the FORMATTED message. Every control character in item 4's sense maps to ., except line feed and tab. A
CRLF collapses to ONE kept line feed (the CR is dropped, the LF fires the same branch). Every kept line feed is
followed by a 4-space ContinuationIndent, so a continuation line can never start at column 0.

Host profile sample — DarlingWorker's real call (_logger.LogInformation("Store host profile:\n{Profile}", DarlingStoreHostProfile.FormatStartupProfileText(profile))) through the new sink, as asserted by
Log_StoreHostProfileBlock_StaysReadable_EachRowOnItsOwnIndentedLine:

2026-09-25 10:41:12.345 [INFO ] [DarlingWorker] Store host profile:
    Host: linux (containerized), 4 CPU(s)
    RAM: 4 GB [cgroup v2 memory.max]
    Data volume: 100 GB total, 50 GB free (ext4)
    
    Setting                               Current       Derived       Source                                    Verdict
    shared_buffers                        2048MB        2048MB        v8 (#4214 managed block)                 matches

(Illustrative formatting of the asserted substrings — every row lands on its own line, indented, never merged
or dotted out. The blank separator line between "Data volume" and the settings header is itself indented, since
its own embedded \n\n both get the continuation treatment.)

Tests:

• Log_ExceptionMessageCarriesCrLf_StaysOneEntry_ContinuationNeverAtColumnZero (replaces the old
  dots-based test) — a CRLF plus a fake timestamp stays readable as "bad value\n 2026-08-21 12:00:00 WARN Forged line", and a Regex.Matches(..., @"^\d{4}-\d{2}-\d{2} \d{2}:\d{2}:\d{2}", Multiline) count of 1 proves
  only the real entry's timestamp sits at column 0.
• Log_StoreHostProfileBlock_StaysReadable_EachRowOnItsOwnIndentedLine — the sample above.

Revert-proof: swapped in the pre-PR file via git show HEAD:... (the true baseline, since item 4 and item 5
share this file's Log/sink code). All three sink tests failed against it (Not found: "bad value\n 2026-08-21...", "before.middle.end", "Store host profile:\n Host: linux..." — the old
code never added the sanitizer to the formatted message at all, so none of the new shapes existed). Restored and
re-verified green after the same stale-build scare as item 4, same fix.

6. Low 6 — environment test replaced with a source pin

WebHostEnvironment_IsAlwaysProduction_EvenWhenAspnetcoreEnvironmentSaysDevelopment (which built its own
WebApplicationOptions and mutated ASPNETCORE_ENVIRONMENT/DOTNET_ENVIRONMENT process-wide with no
parallel-test guard) is replaced by BothWebHosts_PinEnvironmentNameToProduction_OnTheirOwnCreateBuilderCall
(DarlingWebFailureHandlingTests.cs:414), a source pin reading both hosts' actual CreateBuilder calls for
EnvironmentName = Environments.Production, — the dispatcher-pin style, catching both a deleted line and the
Low 1 gap (only the web host pinned, not the MCP host) in one assertion pair. No behavior-test kept; the source
pin is the review's primary recommendation and avoids the env-var pollution risk entirely.

7. Review note — Sanitize guarded against maxLength 0

Hosting/DarlingHttpRefusalLog.cs:301: if (take > 0 && take < value.Length && ...). Test:
MaxLengthZero_DoesNotThrow — Sanitize("anything", 0) now returns "…" instead of throwing
IndexOutOfRangeException at value[-1].

Build and test totals

• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -c Debug: 0 Warning(s), 0 Error(s).
• Targeted classes (DarlingWebFailureHandlingTests, DarlingHttpRefusalLogTests,
  DarlingFileLoggerProviderTests, FleetSweepWebFeedTests, HostHeaderGuardTests, DarlingWebOidcTests,
  DarlingMcpFleetSweepToolsTests, McpPayloadContractCensusTests): all green.
• Full suite, one authoritative run (after the stale-build scare above was caught and fixed): Total: 14007,
  Errors: 0, Failed: 0, Skipped: 738 (all live/rig-gated — no PostgreSQL rig per this brief), Not Run: 1
  (pre-existing, not something this PR added).

Scope note

Left alone per the brief: the six view/alert-rule write sites in DarlingWebEndpoints.cs (pending #4283), and
the backstop's "response already started" branch (#4276's ruling).