Escape control characters and Unicode line separators (audit S1)

[khaines]

What & why

Implements the audit's S1 finding — the one wire-format change, deliberately separated from PR #89. Closes a log-forgery + terminal-escape-injection vector: the value encoder previously emitted control characters (incl. ESC) and the Unicode line separators U+2028/U+2029/U+0085 (NEL) raw.

Encoder (Logger.AppendValueField)

• Any char.IsControl character (C0/C1, including TAB and NEL U+0085) and U+2028/U+2029 are now quoted and escaped — TAB as \t, the rest as \uXXXX — instead of raw.
• Closes the forgery vector: a value with U+2028/U+2029/NEL could forge a second record in consumers that treat them as line terminators (e.g. .NET ReadOnlySpan.EnumerateLines() / ReplaceLineEndings()); and the terminal-escape vector (raw ESC and other C0 controls reaching a terminal log viewer).

Parser (LogfmtParser.ParseQuotedValue)

• Decodes \uXXXX (4 hex digits, case-insensitive) back to the character, with graceful handling of malformed/truncated escapes — so encode → parse round-trips exactly.

⚠️ Breaking change (wire format)

Like #75/#82, this changes the emitted logfmt for values containing control characters / line separators / TAB (now escaped). Values without such characters are unaffected. This belongs in the next release's CHANGELOG as a [CHANGE] (deferred to the release-cut PR, alongside #89).

Validation

• 201/201 tests on net8.0 and net10.0; dotnet build -c Release → 0 warnings.
• New tests: control chars + line separators are escaped-not-raw and round-trip; the parser decodes \uXXXX and tolerates malformed/truncated escapes. The tab/mixed/unicode-separator tests were updated to the new escaped output.
• Every hunk mutation-verified: dropping IsEscapable from the first pass, the \uXXXX-escape in the switch default, or the parser \u decode case each turns the corresponding test RED.

Self-authored → informational review; AI does not merge.

[khaines]

🤖 Review-Fix Loop — Final Report

PR: #90 — Escape control characters and Unicode line separators (audit S1)
Branch: fix/escape-control-and-unicode-separators
HEAD SHA: 7e1c9c7
Rounds completed: 2
Final rating: 5/5 ⭐ — Exceptional
Gate: ✅ PASS — unanimous 5/5 across all voting seats + decorrelated red-team NO-MISS-CERTIFIED
Termination reason: Target achieved (PASS gate)

Council run via the deltasharp review-pr / review-fix-loop skills against this repo (logfmt canon substituted for Delta/Spark/K8s). Self-authored PR → informational COMMENT review; AI never merges. This is the wire-format finding (S1) from the security/quality/red-team audit, deliberately separated from the never-throw PR (#89).

Progression

Round	Rating	🔴 Critical	🟠 High	🟡 Medium	🔵 Low	ℹ️ Info	Fixed
R1 (0b3acc5)	4/5	0	0	0	1	3	—
R2 (7e1c9c7)	5/5 · ✅ NO-MISS	0	0	0	0	0	1

Council Composition Audit

Round	Slot	agent_type	model	Dispatch HEAD	Verification
R1	Architect	general-purpose	claude-opus-4.8	0b3acc5	✓ 5/5
R1	Balanced	general-purpose	claude-opus-4.8	0b3acc5	✓ 5/5
R1	Security	general-purpose	claude-opus-4.8	0b3acc5	✓ 5/5
R1	Quality	general-purpose	gemini-3.1-pro-preview	0b3acc5	✓ 4/5
R2	Quality	general-purpose	gemini-3.1-pro-preview	7e1c9c7	✓ 5/5
R2	Red-Team	general-purpose	gemini-3.1-pro-preview	7e1c9c7	✓ NO-MISS-CERTIFIED (decorrelated from Opus spine)

Red-team note (transparency): the intended GPT-family red-team (gpt-5.6-sol) rendered an empty final message on all three attempts for this PR (it executed tool calls but emitted no report — a model-instance/turn-budget issue). The decorrelated adversarial gate was therefore fulfilled by a Gemini red-team (decorrelated from the Opus voting spine) plus an orchestrator-run adversarial battery, which agree on NO-MISS (evidence below). R2 is test-only, so the R1 Architect/Balanced/Security 5/5 carry forward on the unchanged production.

What the PR does

Closes the audit's F1 log-forgery + terminal-escape-injection vector. The value encoder previously emitted control characters (incl. ESC) and the Unicode line separators U+2028/U+2029/U+0085 (NEL) raw.

• Encoder (Logger.AppendValueField): any char.IsControl character (C0/C1, incl. TAB and NEL) and U+2028/U+2029 are now quoted and escaped — TAB as \t, the rest as \uXXXX — instead of raw.
• Parser (LogfmtParser.ParseQuotedValue): decodes \uXXXX (4 hex, case-insensitive, bounds-checked, malformed kept literal) so encode → parse round-trips exactly.

Findings Addressed

• [Low → R2, Quality] No test for literal \u text. A literal \u0041 user string must round-trip as literal (not decode to A). Fixed: added LiteralBackslashUTextRoundTripsWithoutDecoding (mutation-proven: if the encoder didn't double the backslash the literal would decode to A → test RED).

Findings Dismissed / Deferred / Inherent

• [Info, Architect] Bidi format chars (Cf / "Trojan-Source") are not escaped — a source-rendering threat, not record-forgery/terminal-escape; out of S1's stated scope.
• [Info, Security/Architect] \uXXXX is BMP-only (UTF-16 char). Astral chars are two non-control surrogate chars emitted raw individually — not line-terminators or C0/C1 controls, so no forgery/escape risk.
• [Info] Older parsers reading new \uXXXX degrade to the inert literal \uXXXX (safe; no corruption/forgery).

Validation

• Tests: dotnet test 202/202 on net8.0 and 202/202 on net10.0; dotnet build -c Release → 0 warnings (StyleCop-as-lint clean). CI: Analyze C#, CodeQL, build, dependency-review all green.
• F1 forgery CLOSED (before/after, Security): base a2701f5 — a U+2028 value produced EnumerateLines=2 / ReplaceLineEndings=2 lines (real forged record); HEAD — rawSepInOutput=False, EnumerateLines=1, round-trip true. Terminal-escape: base emitted raw ESC/C0; HEAD escapes all (anyRawControlByte=False).
• Round-trip (encode→parse), fuzzed by 3 seats + orchestrator: Balanced 339/0, Architect exhaustive BMP + 200k random / 0 mismatch, Gemini red-team 13/13, orchestrator 274/0 — covering every control char, LS/PS/NEL, TAB, =/"/\, literal \u0041/\\u0041, surrogate pairs, combining marks, empty, and 5–10K strings.
• Parser \u abuse (Security + red-team): k="\u0022 x=y", k="\uD83D" (lone surrogate), k="\u", k="\u12", k="\uGGGG", uppercase — no crash, no over-read, each stays a single field k (a decoded "/=/space is never re-fed to delimiter handling).
• Mutation coverage (C1), each hunk → named test RED (Quality + red-team): first-pass IsEscapable → ControlCharactersAreEscapedAsUnicodeAndRoundTrip + UnicodeLineSeparatorsAreEscapedAndNeverRaw; switch \u-default → same; parser case 'u' → ParseDecodesUnicodeEscape + UnicodeSeparatorsInValueDoNotBreakRecordOrInjectField; encoder backslash-doubling → LiteralBackslashUTextRoundTripsWithoutDecoding. No test weakened — the 3 updated tests (TabCharacterInValueCausesQuoting, MixedSpecialCharsInValueAllEscaped, UnicodeSeparatorsInValueDoNotBreakRecordOrInjectField) assert the new escaped form + round-trip.
• C6 hygiene: final diff is the 5 intended files; no scratch artifacts; dotnet format correctly dismissed. No issue closed (audit follow-up).

⚠️ Breaking change

Like #75/#82, this changes the emitted logfmt for values containing control characters / line separators / TAB (now escaped). Values without such characters are unaffected. Belongs in the next release's CHANGELOG as a [CHANGE] (deferred to the release-cut PR, alongside #89).

Commits

• 0b3acc5 — Escape control characters and Unicode line separators (audit S1)
• 7e1c9c7 — Review fix (R2): add literal backslash-u round-trip test

Recommendation: APPROVE (5/5). AI does not merge — awaiting your review/merge.