Decode pg_read_binary_file log tail in the connected database's server_encoding (#4062)

[erikdarlingdata]

Part of #4058. Closes #4062.

What changed

The binary log route (pg_read_binary_file → bytea) previously decoded the tail as UTF-8 unconditionally
and PgReadBinaryFileCapability's probe gated the whole route to UTF8 and SQL_ASCII databases via a CASE
in the SQL text. On any other server encoding, the text route (pg_read_file) could still be blinded by one
planted byte (EUC_* → 22021, WIN1252 → 22P05), and there was no fix available even with the grant.

1. New PerformanceMonitor.Collectors/PgServerEncoding.cs: an explicit map of PostgreSQL SERVER encoding
   name → .NET code page (UTF8, SQL_ASCII, every LATIN/ISO_8859 variant listed in the brief, WIN866/874/1250–1258,
   KOI8R/KOI8U, EUC_JP/EUC_CN/EUC_KR). EUC_TW, EUC_JIS_2004, LATIN6, LATIN8, LATIN10, MULE_INTERNAL,
   and anything this runtime cannot resolve stay unmapped — no UTF-8 fallback. Every mapped Encoding is built
   with EncoderFallback.ReplacementFallback/DecoderFallback.ReplacementFallback (never throws), and
   CodePagesEncodingProvider.Instance is registered in the static constructor.
2. PgReadBinaryFileCapability.cs: ProbeSql now selects current_setting('server_encoding') plus the raw
   has_function_privilege check, no CASE gate — C# decides. IsGrantedAsync combines granted-and-mapped into
   the returned bool; a new TryGetCachedEncoding exposes the mapped Encoding from the same cache entry (no
   second round trip). IsCachedAsUnsupportedEncoding now means "granted, but the encoding is one
   PgServerEncoding does not map."
3. CollectorContext.PgLogEncoding (new, beside PgReadBinaryFileGranted): set in
   DarlingCollectorRunner.ResolvePgReadBinaryFileGrantAsync, always null unless the grant flag is true.
4. PgBinaryTailText.DecodeWhole/UnescapeAndDecode take an Encoding parameter instead of hard-coding
   UTF-8. PgLogEventsCollector, PgDeadlocksCollector, PgPlanCaptureCollector pass
   context.PgLogEncoding ?? Encoding.UTF8 (the ?? UTF8 is defensive; the grant/encoding invariant means it's
   never actually needed at the call sites, since PgReadBinaryFileGranted is only true when PgLogEncoding is
   set).
5. Text: Darling/README.md:774's paragraph now names the mapped-encoding list and the unmapped exceptions
   instead of "UTF8 or SQL_ASCII only." DarlingWorker.LogTailUndecodableByteExplanation's "grant does not help"
   sentence now names the unmapped case by encoding families rather than by issue number (no #nnnn in served
   text, per house rule — dropped the #4062 reference the sentence used to carry, and updated the two tests
   that asserted on it).

Tests

• New PgServerEncodingTests: every mapped name resolves; an unmapped name (including null) returns false;
  WIN1252 byte 0x81 decodes without throwing (see note below); LATIN1 0xE9 → é; EUC_JP two-byte round-trip.
• PgBinaryTailTextTests: kept the UTF-8 cases as-is (now passing Encoding.UTF8 explicitly), added WIN1252 and
  EUC_JP cases for both DecodeWhole and UnescapeAndDecode.
• PgReadBinaryFileCapabilityTests: replaced the "NULL for non-UTF8" pins with granted+mapped / granted+unmapped
  / ungranted-regardless-of-encoding cases; added TryGetCachedEncoding coverage; fixed the probe-shape assertion
  (no CASE, still asks for both the encoding and the privilege).
• PgReadBinaryFileAdvisoryTests and PostgresFaultOutcomeTests: updated to the new probe scalar shape
  ("<encoding>:<bool>") and the no-issue-number sentence.
• Live test (extending PgReadBinaryFileCapabilityLiveTests per the brief) was NOT added. I ran out of the
  30-minute budget before writing it — see "Not verified" below. The unit-level coverage above exercises the same
  paths (PgServerEncoding.TryGet, the probe scalar parsing, the cache) without a live database.

One correction to the brief's own test list

PgServerEncodingTests's WIN1252-0x81 case does not assert a U+FFFD outcome, unlike the brief's phrasing.
Checked against a throwaway .NET program: .NET's Windows-1252 code page maps byte 0x81 to U+0081 (a reserved-but-mapped
C1 control point), not to the replacement character — there is no byte value in a single-byte .NET code page that
lacks some mapping, so DecoderReplacementFallback is never actually invoked for a single stray byte in WIN1252.
The test instead asserts the byte decodes without throwing and produces exactly one character, which is the
property #4062 actually needs (a captured log line must never fail the whole read over one byte). Both new tests
(PgServerEncodingTests.Win1252UnassignedByte_DecodesWithoutThrowing,
PgBinaryTailTextTests.DecodeWhole_Win1252UnassignedByte_DecodesWithoutThrowing) are named and commented to say so.

Known limitation (per the brief — not fixed here)

One server log file mixes lines from backends in different databases, each in its own database's encoding.
Decoding with the connected database's encoding can still mis-decode (replace, never fail) lines from a database
with a different encoding. The text route has the same property. Documented in Darling/README.md.

What I ran and what it showed

• dotnet build on PerformanceMonitor.Collectors/PerformanceMonitor.Collectors.csproj alone: clean, no warnings
  (confirmed System.Text.Encoding.CodePages needs no explicit PackageReference under net10.0 — the SDK already
  supplies it; NU1510 fired when I added one, so I removed it and the Directory.Packages.props entry).
• dotnet build Darling/Darling.Tests/Darling.Tests.csproj -p:EnableWindowsTargeting=true: builds the whole
  solution clean, 0 errors, 0 warnings.
• Targeted classes while iterating (PgServerEncodingTests, PgBinaryTailTextTests,
  PgReadBinaryFileCapabilityTests, PgReadBinaryFileAdvisoryTests, PostgresFaultOutcomeTests,
  PgServerLogTailBinaryRouteTests, PgLogEventsPipelineTests, PgPlanCaptureLiveTests): 234 total, 0 failed,
  2 skipped (live, no DARLING_TEST_PG).
• Full suite, once, with Microsoft.WindowsDesktop.App stripped from the test runtimeconfig per _COMMON.md:
  Total: 13356, Errors: 0, Failed: 217, Skipped: 714. dev's known baseline on this Mac is 214 Windows-only
  failures; the extra 3 above baseline were PgReadBinaryFileAdvisoryTests tests using the OLD bool scalar
  shape against the NEW probe's "encoding:bool" string scalar — fixed in this branch, then reconfirmed 0 failed
  on that class alone (7/7 passed). I did not re-run the full 13356-test suite a second time after that fix (time
  budget); the class-level rerun is the verification for that specific change, and every other file I touched was
  already covered by the full run before the fix.
• Not verified: the live PgReadBinaryFileCapabilityLiveTests extension the brief asked for (creating a
  WIN1252-encoded database, connecting, and reading a planted 0x81 byte through pg_read_binary_file) was not
  written — ran out of time before landing it. The lane's docker container (lane4062-pg, port 55890) was never
  started, so no live proof against a real server ran. This is the one item I'd flag as a fast follow if the
  coordinator wants it in-lane rather than as a separate ticket; it's a genuine gap against the brief's own test
  list, not a design question.
• scrubcheck on git diff origin/dev...HEAD: 0 hits, 17/17 controls fired.

Coordinator verification (branch head after 1929d37)

• Live PostgreSQL 18 (a throwaway container):
  • a new PgReadBinaryFileCapabilityLiveTests.IsGrantedAsync_OnAWin1252Database_GrantsTheRoute_AndCachesItsEncoding: on a WIN1252 database, the probe grants the binary route and caches code page 1252, and pg_read_binary_file then decodes. Before this PR the probe's CASE gate answered NULL there. The lane didn't write this test; the coordinator did.
  • The whole class 3/3; plus PgPlanCaptureGuardedCastLiveTests 3/3 and PgDeadlockCandidatePatternLiveTests 2/2.
• Unit: PgServerEncodingTests 40, PgBinaryTailTextTests 15, PgReadBinaryFileCapabilityTests 31, PgReadBinaryFileAdvisoryTests 7, PostgresFaultOutcomeTests 90, PgLogEventsPipelineTests 49, PgServerLogTailBinaryRouteTests 7. All pass.
• Full Darling suite on macOS, compared with dev on the same rig: 3 failures dev doesn't have, all fixed in the last commit.
  • DocCommentHygieneTests ×2: DecoderReplacementFallback now resolves, so its entry leaves the unresolved-cref inventory.
  • LiveCleanupConversionRatchetTests: the new live test now tears down through LiveStoreCleanup.
  • Both classes pass (77 and 15).
• Note: in .NET's code page 1252, the five undefined bytes (0x81, 0x8D, 0x8F, 0x90, 0x9D) decode to the matching C1 control characters, not U+FFFD. Nothing throws, which is the point.

CHANGELOG entry

• Fixed: the pg_read_binary_file log-tail route now decodes bytes in the connected database's own
  server_encoding (every LATIN/ISO_8859 variant, WIN866/874/1250–1258, KOI8R/KOI8U, EUC_JP/EUC_CN/EUC_KR, plus
  UTF8/SQL_ASCII) instead of hard-coded UTF-8, so a database in one of those encodings no longer needs to fall
  back to the text route's byte-blinding failure mode. (Postgres log readers: decode the byte route in the database's own encoding, so EUC and WIN1252 targets are not blindable #4062)

Round-1 review fixes

Review: #4106 (comment)

1. Medium — a planted EUC lead byte swallows the next newline. Fixed in PerformanceMonitor.Collectors/PgBinaryTailText.cs:
   DecodeWhole and UnescapeAndDecode now split the raw bytes on 0x0A before decoding, decode each segment with the target
   encoding, and join with '\n'. This is safe for every mapped encoding — 0x0A is never a trail byte in EUC-JP, EUC-KR or GBK,
   and single-byte encodings are unaffected — so valid text decodes to exactly the same string as before. Commit: 4062 review: split on 0x0A before decoding so a planted EUC/DBCS lead byte cannot eat the newline that follows it.
   New tests: DecodeWhole_PlantedEucLeadByteBeforeNewline_DoesNotSwallowTheLineBreak and
   UnescapeAndDecode_PlantedEucLeadByteBeforeNewline_DoesNotSwallowTheLineBreak, both [Theory] over EUC_JP/EUC_CN/EUC_KR
   with 0xA4 0x0A 'x' (and the escape()-reversed form \244\nx) — the result contains '\n' and 'x' survives on the next line.
2. Low — the replacement char was ?, not U+FFFD. PerformanceMonitor.Collectors/PgServerEncoding.cs now builds every mapped
   code page with new DecoderReplacementFallback("\uFFFD") instead of the default DecoderFallback.ReplacementFallback
   (whose string is a plain "?"). UTF8/SQL_ASCII stayed UTF8Encoding(false, false), which already yields U+FFFD. Commit:
   4062 review: replacement char is U+FFFD, not '?', and the EUC map is approximate, not exact. Fixed a dangling cref this left
   in PgServerEncodingTests.cs (the removed DecoderFallback.ReplacementFallback symbol was still cref'd in a test comment) —
   caught by DocCommentHygieneTests, now green again.
3. Low — approximate mappings. Reworded PgServerEncoding.cs's header comment: it no longer claims every mapped charset is
   exact. EUC_CN → 936 is GBK (a GB2312 superset: an unmapped lead byte can pair with an ASCII byte in 0x40–0x7E and swallow
   it as one CJK character); EUC_JP → 51932 does not decode 3-byte JIS X 0212 sequences; EUC_KR → 51949 goes through the same
   DBCS table path and is approximate for the same reason. None of the three throws. The other mapped entries (LATIN1–4/5/7/9,
   ISO_8859_5..8, WIN866/874/125x, KOI8R/KOI8U) are called out as exact. The map itself is unchanged. Same commit as item 2.
4. Info — the served sentence was garbled. Darling/PerformanceMonitor.Darling.Service/DarlingWorker.cs ~9146: "which this
   database's encoding is not one this collector maps" → "but this collector does not map this database's encoding", per the
   review's suggested wording. No issue refs in served text. PostgresFaultOutcomeTests did not pin the old wording; ran the
   class after the change — still 90/90 green. Commit: 4062 review: reword the garbled served sentence about an unmapped encoding.
5. Tests run: PgBinaryTailTextTests (21/21, up from 15 — this lane's 6 new cases), PgServerEncodingTests (40/40),
   PgReadBinaryFileCapabilityTests (31/31), PostgresFaultOutcomeTests (90/90), DocCommentHygieneTests (77/77). Then the
   full Darling suite once (on this Mac, dotnet Darling.Tests.dll with Microsoft.WindowsDesktop.App stripped from
   runtimeconfig, restored after): 13363 total, 214 failed, 715 skipped — 214 is exactly dev's stated Windows-only baseline
   for this rig; no failure is in a class this lane touched or added.
6. Lows accepted as-is, not fixed here (the review calls them known and bounded):
   • The encoding cache is keyed per server, not per database, so two log-tail collectors on one server connecting to
     databases with different encodings could share a stale encoding for up to an hour. Wrong text, not an exception.
   • escape()'s cost: legitimate non-ASCII text on EUC/WIN/LATIN databases now expands up to 4× every cycle through
     encode(…,'escape'), not only when bytes are planted. Still bounded (~16 MB) and under the Postgres log readers: the planted-input paths the #4051 review left open #4058 item 1 guard.
   • An unparseable or NULL probe result caches Granted = null, read as "unsupported encoding" — harmless today, worth a
     comment rather than a behavior change.

PR stays un-armed (draft/ready state untouched); the coordinator readies it.

[erikdarlingdata]

Security review round 1 (automated reviewer)

Verdict: no High findings. Fix the Medium first (small change) or accept it in writing. The rest can merge as is or go to a follow-up.

Scope: the diff origin/dev...origin/fix/4062-binary-route-server-encoding under PerformanceMonitor.Collectors and Darling/PerformanceMonitor.Darling.Service, plus the README paragraph. I read the .NET DBCSCodePageEncoding, EUCJPEncoding and DecoderReplacementFallback sources (dotnet/runtime main) to check decoder behaviour. I ran nothing.

Can a planted byte make the decode throw? No.

• PgServerEncoding.cs:86-87 builds every code page with Encoding.GetEncoding(cp, EncoderFallback.ReplacementFallback, DecoderFallback.ReplacementFallback). That overload replaces the provider's default best-fit fallback, so best-fit never applies. UTF8/SQL_ASCII (:79-80) are new UTF8Encoding(false, false), so invalid bytes are replaced and nothing throws.
• ReplacementFallback's string is a plain "?" with no surrogates, so the fallback buffer's surrogate and recursion checks cannot fire. GetString on a byte array of 4 MB or less cannot overflow.
• UnescapeAndDecode (PgBinaryTailText.cs:53-87): the output buffer is escaped.Length and each input char writes at most one byte, so no index overrun. The (byte) casts are unchecked, and I found no CheckForOverflowUnderflow in the repo. A \777 escape therefore truncates without throwing, and escape() never emits one.
• Registration order is safe. RegisterProvider runs first in the static constructor (:77), and the only way to reach an Encoding is through TryGet, which runs that constructor first. No other assembly needs the provider on this path.

Medium: a planted DBCS lead byte eats the next byte, including \n

• For 51932, 936 and 51949, DBCSCodePageEncoding.GetChars joins a lead byte with whatever byte follows it (iBytes <<= 8; iBytes |= *bytes). If that pair is unmapped, both bytes become one ?. So 0xA4 0x0A decodes to ? and the newline is gone. .NET's UTF-8 decoder never consumes an ASCII byte this way, so this property is new with this PR.
• pg_log_events is exposed. DecodeWhole (PgBinaryTailText.cs:31) decodes the whole tail before C# splits lines. A lead byte right before a real \n merges the next real line into the attacker's line. Depending on how PgLogEntryAssembler treats a line without a prefix, that line is hidden or turned into attacker-controlled continuation text.
• pg_deadlocks and pg_plan_capture are less exposed. Their regex runs server-side over encode(…,'escape') bytes, so which rows match is decided before decoding. Only the captured text that the C# parser receives can lose a newline.
• Can an attacker reach it? I did not prove this. In the unauthenticated FATAL/DETAIL shapes I checked, the planted role or database name is followed by " or a space, never by the newline. That byte is eaten instead ("X\xA4" does not exist decodes to "X? does not exist). A message shape that puts the planted byte last on its line (for example a PG18 connection line, or an extension's message) would turn this into line merging.
• Fix (cheap): split on byte 0x0A before decoding and decode each segment. EUC trail bytes are always 0xA1 or higher, so this changes no valid text and brings back the UTF-8 property that ASCII control bytes are never consumed. Pin it with a test: EUC_JP bytes …\xA4\n<next line> must give two lines.

Low

• Docs say U+FFFD, but the code writes ?. DecoderFallback.ReplacementFallback is new DecoderReplacementFallback(), whose string is "?", not U+FFFD. The claims are wrong at PgServerEncoding.cs:28-32, PgBinaryTailText.cs:22-25, and the served sentence at DarlingWorker.cs:9158 ("shows as U+FFFD"). As a result, an undecodable or planted byte cannot be told apart from a real ? in stored text and hashes. Fix: new DecoderReplacementFallback("\uFFFD") for the code pages, which keeps one marker on every route.
• The mapping is not exact everywhere, but it never throws. The class comment (PgServerEncoding.cs:21-22) says "Only exact charsets … not an approximation". Two entries are not exact:
  • EUC_CN → 936 is GBK, a superset of GB2312. Valid EUC_CN decodes correctly. However, a planted lead byte followed by an ASCII byte in 0x40–0x7E becomes one CJK character and swallows that letter.
  • EUC_JP → 51932: .NET's own source says "WE DON'T USE JIS 0212". It treats 0x8F as a single byte, so PG's 3-byte JIS X 0212 sequences decode wrong.
  • EUC_KR → 51949 also goes through the DBCS table path. Treat it as approximate unless a test pins it.
  • These entries are exact: LATIN1–4 → 28591–28594, LATIN5 → 28599 (ISO-8859-9), LATIN7 → 28603 (8859-13), LATIN9 → 28605 (8859-15), ISO_8859_5..8 → 28595–28598, WIN866 → 866, WIN874/125x, KOI8R → 20866, KOI8U → 21866.
  • Please reword the comment to "approximate for the EUC family".
• The cache is keyed per server, but encoding is a per-database property. See DarlingCollectorRunner.cs:1124-1134, 1144. If two log-tail collectors on one server connect to databases with different encodings, the first probe's encoding decodes for both for up to an hour. That is wrong text, not an exception. The grant verdict already had this property because EXECUTE ACLs are per database, so this is a correctness issue, not a new security one. Consider adding the database name to the key, or say in the doc why it is safe.
• More databases now take the escape() expansion in normal operation. On EUC/WIN/LATIN databases, legitimate non-ASCII text is all at or above 0x80. encode(…,'escape') now expands it up to 4× on every cycle, not only when bytes are planted. The expansion is still bounded (about 16 MB) and still under the Postgres log readers: the planted-input paths the #4051 review left open #4058 item 1 guard, so this is a cost, not a new exposure.

Info

• Probe (PgReadBinaryFileCapability.cs ProbeSql, parse at :116-142): :: binds tighter than ||, so the cast applies to the boolean as intended. bool::text returns true/false, and bool.TryParse accepts both. PG encoding names come from the fixed pg_enc2name table ([A-Z0-9_]), so none contains :. server_encoding is PGC_INTERNAL, fixed at initdb and not client-settable. The shape is valid on 14+. LastIndexOf is fine either way.
• An unparseable or NULL probe result reads as "unsupported encoding". Such a result caches Granted = null, which IsCachedAsUnsupportedEncoding reads as "the grant doesn't help". That also stops DarlingCollectorRunner.cs:1174 from dropping the verdict on a 22021. Harmless today; worth a comment.
• Garbled sentence at DarlingWorker.cs:9145-9148: "which this database's encoding is not one this collector maps". Suggest: "…decodes the log in the database's own server_encoding, but this collector does not map this database's encoding (…)".
• No new SQL injection surface. The probe is a constant and the encoding name only reaches a dictionary lookup.