Sanitise the log in a shape CodeQL can see

[ivanvyd]

The log-forging fix in #32 worked, and CodeQL kept reporting it. Twelve cs/log-forging alerts stand against main right now, on code that is in fact fixed.

I got the verification wrong. I read refs/pull/32/merge reporting 0 alerts as proof the fix was recognised. That ref simply had no analysis — the 0 meant no data, not no findings. The scan of the merge commit itself reports 12.

Why the old shape wasn't recognised

if (!name.Any(char.IsControl)) { return name; }
return new string([.. name.Select(c => char.IsControl(c) ? '\uFFFD' : c)]);

CodeQL's C# taint tracking doesn't model a filter-inside-a-Select as sanitising the flow, so the tainted route value still reaches the log as far as the query is concerned.

The new shape

internal static string ForLog(string name) => name
    .Replace('\r', Redacted)
    .Replace('\n', Redacted)
    .Replace('\u001b', Redacted);

Chained string.Replace is a shape the query recognises. ESC joins CR and LF because a terminal tailing a log acts on escape sequences and a route can carry one.

This is a narrowing, deliberately. char.IsControl covered every control character; this covers the three that actually do something. The rest are ugly in a log rather than dangerous.

An alert nobody can close is an alert everybody learns to scroll past — that's the real cost of leaving twelve standing on correct code.

Verification

448 tests green on both net8.0 and net10.0. Assert.Same still holds for an ordinary name: string.Replace returns the same reference when nothing matches, so the common path still allocates nothing.

New test for the ESC case; the existing CR/LF test is unchanged and still passes.

The real check is CodeQL itself — whether the alert count on main goes from 12 to 0 after this merges. I'll confirm that rather than assert it.