fix(io,core): BinaryData::ToString decodes UTF-8, and the shared decoder moves to Core.Base (#2106)

The finding has two halves and they are answered differently, on purpose.

DECODING -- REPAIRED. ToString() copied its byte range verbatim, so it could
return a std::string that is not valid UTF-8: 0xFF came back as 0xFF. .NET's is
Encoding.UTF8.GetString(_bytes.Span) and always returns valid text. It now
substitutes U+FFFD, one replacement per ill-formed BYTE, because the decoder
resumes one byte later -- which is what stops a truncated sequence swallowing
the character after it. Well-formed UTF-8 is byte-identical.

ALIASING -- A DELIBERATE PERMANENT DEVIATION, not an unfinished repair. .NET's
BinaryData(byte[]) WRAPS its argument, so a later mutation of the caller's array
is observable. This port copies and will keep copying: reproducing the aliasing
means holding a reference to storage this object does not own, in a language
with no GC -- precisely the borrowed-reference defect CCF-019 exists to remove,
and that #1959, #2029, #2066, #2088, #2096 and #2134 have spent this programme
removing. Pinned in that direction.

The shared UTF-8 scalar decode moved from modules/text to Core.Base to make the
first half possible. modules/io does not depend on Text, so the options were a
SIXTH copy, a new public component edge, or moving the one definition somewhere
everything already depends on. System::Text::detail re-exports both names, the
moved bodies are byte-identical (verified by diff against HEAD), and the module
graph is unchanged at 41 modules / 92 edges. #2354 is rescoped accordingly, and
gains two further copies noticed on the way -- in XmlConvert.cpp and
Utf8JsonWriter.cpp.

A process defect in my own mutation loop is recorded rather than hidden: an
early mutation of the new, untracked header could not be reverted by git
checkout, and a later backup captured the mutated file -- so one gate reading
was taken with a mutation in the tree. The file was then verified byte-identical
and the gate RE-MEASURED. 17,266 is the re-measurement.

+1 net test. Three mutations: two caught, one confirmed equivalent.

Gate: 17,266 run, 0 failed, 38 executables -- green. Downstream, measured: zero
System::BinaryData sites in either consumer.

SR-AUD-185 -> remediated. Record: docs/Migration-BinaryDataUtf8Decoding.md.

Author: robertvokac

CLAUDE.md

@@ -196,7 +196,7 @@ implementation ticket.
 | [SR-AUD-182](modules/core/include/System/StringNormalizationExtensions.hpp.audit.md#sr-aud-182--medium--public-unicode-normalization-is-an-always-successful-no-op-outside-ascii) | medium | confirmed | `StringNormalizationExtensions.hpp`, `NormalizationForm.hpp`, `StringTests.cpp` | Every input is reported normalized and returned unchanged; `e` + U+0301 stays `65CC81` while current .NET FormC reports false and composes it to `C3A9`. **Reviewed 2026-08-12 (#2337); STILL CONFIRMED — its pin clause is REMEDIATED, its implementation clause is `blocked` (#2338).** **Consumer surface: none in production** — five tests in `StringTests.cpp`, no `String::Normalize`, no module call site. **Measured gap:** all five of those tests use ASCII or the empty string, so **every one passes identically before and after a real normalization implementation** — the divergence had no pin at all, the same gap #2022 found for SR-AUD-294. **#2337 (`done`, test only), +4 gated-behaviour pins:** the decomposed sequence the audit measured (UTF-8 `65 CC 81`, reported normalized for Form C and returned unchanged rather than composed to `C3 A9`), the composed U+00E9 under Form D, the compatibility ligature U+FB01 under Form KC/KD, and the finding's separate observation that an **undefined** `NormalizationForm` value is accepted as a normal success where .NET reports an argument error. Each must be inverted by the repair. No production file touched. **Deliberately not done:** throwing `PlatformNotSupportedException` for non-ASCII input — a runtime behaviour break on accepted input, unapproved, in a type whose disclosure says the opposite. **#2338 (`blocked`):** normalization needs canonical and compatibility decomposition mappings, canonical combining classes, composition exclusions and quick-check properties **and** a UAX #15 implementation — the only one of this repository's four Unicode-blocked tickets that is not a table lookup. The **data-source** decision is nonetheless the same **Approval F / #2018**, recorded as the gate with no second approval raised. `docs/CoreOwnedFindingsReviews2317.md`. |
 | [SR-AUD-183](modules/threading/include/System/Threading/WaitHandle.hpp.audit.md#sr-aud-183--medium--waitall-and-waitany-silently-accept-invalid-handle-collections-and-invalid-timeout-values) | medium | remediated | `WaitHandle.hpp`, `Batch9ThreadingTests.cpp` | Empty/null collections and invalid timeouts return true/258 or loop instead of managed argument diagnostics; C++/managed probes reproduce each divergence.  **Remediated (#1952, 2026-08-03, cause T-C of `docs/ThreadingNamespaceReviewPlan.md`):** all four static entries now validate in `WaitMultiple`'s order -- empty collection (`ArgumentException`, "Waithandle array may not be empty.", paramName `waitHandles`), then timeout (`ArgumentOutOfRangeException`, `millisecondsTimeout`), then each element (`ArgumentNullException`, paramName `waitHandles[i]`, "At least one element in the specified array was null."). **Three corrections measured by `build-probe/1952_probe1_waithandle_multiwait.cpp`:** (1) there are **three** non-terminating shapes, not one -- `WaitAny({})`, `WaitAny({}, -1)` and `WaitAny({nullptr})` all spun forever, because a null element was skipped and left the poll loop with nothing to observe; (2) a `-2` timeout gave **two different** wrong answers, `true` for an empty collection and `false` for a non-empty one, since the negative deadline was already past; (3) a mixed `{handle, nullptr}` collection returned a plausible index by silently skipping the null -- the one pre-fix call that produced a *usable* answer, and now the sharpest observable change. .NET's `MaxWaitHandles` (64) ceiling is **deliberately not** reproduced: it exists because Win32 `WaitForMultipleObjects` caps at 64 and this port waits sequentially, so adopting it would reject input that works; the report's note about the unrepresentable ceiling is answered by documenting it in the header. Valid-input behaviour, including both `-1` special cases, is unchanged. +7 tests. |
 | [SR-AUD-184](modules/threading/include/System/Threading/EventWaitHandle.hpp.audit.md#sr-aud-184--medium--eventwaithandle-accepts-invalid-eventresetmode-values-instead-of-rejecting-them-at-construction) | medium | remediated | `EventWaitHandle.hpp`, `EventResetMode.hpp`, `ThreadingRemainingTests.cpp` | C++ accepts underlying mode 42 and gives mixed auto/manual behavior; .NET rejects it with an argument exception.  **Remediated (#1954, 2026-08-03, cause T-C of `docs/ThreadingNamespaceReviewPlan.md`):** the constructor rejects any value that is neither `AutoReset` nor `ManualReset` with `System::ArgumentException("Value of flags is invalid.", "mode")`. Rejection rather than normalisation is deliberate and is **not** unified with `ReaderWriterLockSlim`'s normalisation in the same ticket, because .NET does not unify them -- `EventWaitHandle` validates `mode`, `ReaderWriterLockSlim` stores a bool and derives its property. Measured incoherence before the change (`build-probe/1954_probe1_argument_domain.cpp`): with mode 42 the handle stayed signalled after `WaitOne` (`first=1 second_without_set=1`), because `Set()` took the AutoReset branch *for not being ManualReset* while `WaitOne()` skipped the reset *for not being AutoReset*. **Recorded uncertainty:** the reference tree is absent from this environment and the audit's managed probe records only "an argument exception", so the port throws the **base** `ArgumentException` -- caught by any handler written for either .NET candidate -- and the tests assert the category, not a derived type. +2 tests. |
-| [SR-AUD-185](modules/io/include/System/BinaryData.hpp.audit.md#sr-aud-185--medium--binarydatatostring-returns-malformed-utf-8-bytes-unchanged-instead-of-utf-8-decoding-them) | medium | confirmed | `BinaryData.hpp`, `Batch7Tests.cpp`, `BinaryDataTests.cpp` | C++ returns invalid byte `FF` as text while .NET UTF-8 decoding returns U+FFFD (`EFBFBD`); all direct cases use valid ASCII. **OWNER: #2106, deferred** (`docs/SystemIONamespaceReviewPlan.md` §4, §16). Establishing .NET's exact decoding needs the reference tree, which is absent, so nothing is changed. **PINNED by #2104** (`BinaryDataDeferredBehaviourPins`, 3 cases): `ToString()` of `0xFF` returns one byte `0xFF` and explicitly not `EF BF BD`, a truncated `E2 82` is equally untouched, and the valid-UTF-8 control is unchanged — so a resolution must edit a test and cannot land silently. `BinaryData.hpp` now states the divergence at the class and at `ToString()` instead of claiming "UTF-8 decoding". |
+| [SR-AUD-185](modules/io/include/System/BinaryData.hpp.audit.md#sr-aud-185--medium--binarydatatostring-returns-malformed-utf-8-bytes-unchanged-instead-of-utf-8-decoding-them) | medium | remediated | `BinaryData.hpp`, `Batch7Tests.cpp`, `BinaryDataTests.cpp` | C++ returns invalid byte `FF` as text while .NET UTF-8 decoding returns U+FFFD (`EFBFBD`); all direct cases use valid ASCII. **OWNER: #2106, deferred** (`docs/SystemIONamespaceReviewPlan.md` §4, §16). Establishing .NET's exact decoding needs the reference tree, which is absent, so nothing is changed. **PINNED by #2104** (`BinaryDataDeferredBehaviourPins`, 3 cases): `ToString()` of `0xFF` returns one byte `0xFF` and explicitly not `EF BF BD`, a truncated `E2 82` is equally untouched, and the valid-UTF-8 control is unchanged — so a resolution must edit a test and cannot land silently. `BinaryData.hpp` now states the divergence at the class and at `ToString()` instead of claiming "UTF-8 decoding". **DECODING HALF REMEDIATED (#2106, 2026-08-17); the ALIASING half is a deliberate permanent deviation, and the two are answered differently on purpose.** `BinaryData::ToString()` copied its byte range **verbatim**, so it could return a `std::string` that is not valid UTF-8 — `0xFF` came back as `0xFF` — where .NET's is `Encoding.UTF8.GetString(_bytes.Span)` (`BinaryData.cs:419`) and always returns valid text. It now substitutes U+FFFD, one replacement per ill-formed **byte**, because the decoder resumes one byte later — which is what stops a truncated sequence swallowing the character after it. Well-formed UTF-8 is byte-identical. **The aliasing half will not be repaired**: .NET's `BinaryData(byte[])` *wraps* (`_bytes = data`, `:60-63`) so a later mutation of the caller's array is observable; this port copies, because reproducing the aliasing means holding a reference to storage the object does not own, in a language with no GC — precisely the borrowed-reference defect CCF-019 exists to remove and that #1959, #2029, #2066, #2088, #2096 and #2134 have spent this programme removing. Pinned in that direction. **The shared UTF-8 scalar decode moved from `modules/text` to `Core.Base`** to make this possible: `modules/io` does not depend on `Text`, so the options were a *sixth* copy of the decode, a new **public** component edge, or moving the one definition somewhere everything already depends on. `System::Text::detail::Utf8Scalar.hpp` re-exports both names so every existing caller is unchanged, the moved bodies are byte-identical (verified by diff), and the module graph is unchanged at 41 modules / 92 edges. +1 net test. Three mutations, two caught and one confirmed equivalent. Downstream measured: zero `System::BinaryData` sites in either consumer. `docs/Migration-BinaryDataUtf8Decoding.md`. |
 | [SR-AUD-186](modules/io/include/System/BinaryData.hpp.audit.md#sr-aud-186--medium--binarydatafrombytesreadonlymemory-snapshots-bytes-that-current-net-deliberately-wraps) | medium | confirmed | `BinaryData.hpp`, `Batch7Tests.cpp`, `BinaryDataTests.cpp` | C++ preserves byte 01 after its source changes to 02, whereas current .NET stores/wraps the supplied ReadOnlyMemory and observes later contents. **OWNER: #2106, deferred. PREMISE INVERTED** (`docs/SystemIONamespaceReviewPlan.md` §6.1): the behaviour is reproduced exactly, but the finding's implicit direction — that the port is wrong — is backwards. .NET's is the **aliasing** behaviour and this port's is the **defensive** one, so "fixing" it means making `BinaryData` alias caller memory it does not own, introducing a borrowed-pointer lifetime hazard of exactly the CCF-019 shape into a type that today has none. It also needs a decision about which overload is meant: `BinaryData(byte[])` copies, `BinaryData(ReadOnlyMemory<byte>)` wraps. Neither is settleable with `/rv` absent. **PINNED by #2104** across **all four** construction doors, not the one the finding names, plus a test showing the copy outliving a source that is gone entirely. Two `FromBytes` doc-comments that said "by **wrapping** the provided ReadOnlyMemory" while copying were corrected — the header was documenting .NET's behaviour, not its own. |
 | [SR-AUD-187](modules/threading/include/System/Threading/ThreadPool.hpp.audit.md#sr-aud-187--high--unsafequeueuserworkitem-releases-a-borrowed-work-item-to-a-detached-thread-without-retaining-its-lifetime) | high | remediated | `ThreadPool.hpp`, `IThreadPoolWorkItem.hpp`, `Batch9ThreadingTests.cpp` | Detached work captures a raw pointer; an ASan probe deletes it after Execute starts and reports heap-use-after-free in the queued work item. **REMEDIATED (#1959, 2026-08-17), by OWNERSHIP rather than by a waiting destructor.** `ThreadPool::UnsafeQueueUserWorkItem` now takes `std::shared_ptr<IThreadPoolWorkItem>`. The boundary #2134/#2066/#2088 gave the other CCF-019 async members — the owner's destructor waits — is **not available here**: a DETACHED thread has no owner whose destructor could wait for it. Holding a share IS the boundary, and it is the direct counterpart of the GC reference .NET's queue entry is. **Public source break**, landed under `docs/StandingApprovals.md` SA-2 with all five conditions: `docs/Migration-BorrowedCallbackOwnership.md`, a site in `test/consumer/threading_borrowed_callback_negative.cpp`, downstream ticket **#2353**, the full gate, and the measured downstream report (zero sites in `cna` and `mobile-eggbert`). The test asserts BOTH halves: the item survives the caller dropping its reference, **and** `std::weak_ptr::expired()` becomes true once `Execute()` returns, so the repair is not a leak dressed up as a boundary. |
 | [SR-AUD-188](modules/threading/include/System/Threading/RegisteredWaitHandle.hpp.audit.md#sr-aud-188--high--registerwaitforsingleobject-accepts-a-null-waithandle-and-starts-a-background-null-dereference) | high | remediated | `ThreadPool.hpp`, `RegisteredWaitHandle.hpp`, `Batch9ThreadingTests.cpp` | Null registration returns into a worker that dereferences null and terminates an isolated C++ process, while .NET rejects input synchronously.  **Remediated (#1953, 2026-08-03, cause T-C of `docs/ThreadingNamespaceReviewPlan.md`):** `RegisteredWaitHandle`'s private constructor now runs `ArgumentNullException("waitObject")` then `ArgumentNullException("callBack")` before the `std::thread` is created, and `ThreadPool::RegisterWaitForSingleObject` runs `ArgumentOutOfRangeException("millisecondsTimeOutInterval")` first -- exactly .NET's split, where the public overload does `ThrowIfLessThan(millisecondsTimeOutInterval, -1)` and the private one it delegates to does the two `ThrowIfNull`s. The empty callback and the `-2` timeout are the two extra gaps the owning per-file report records under "Other missing assertions" and asks to be repaired *with* the null-wait crash path; both are closed here rather than deferred. Probe `build-probe/1953_probe1_null_argument_crashes.cpp` (forked children under `alarm(3)`): `threadpool.register.null_waitobject` moved from `child-signal:11(SEGV)` to a synchronous `ArgumentNullException(waitObject)`; the valid-registration control still fires. ASan/UBSan/LSan clean. |

audit/AUDIT_FINDINGS_INDEX.md

@@ -0,0 +1,68 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — `BinaryData::ToString()` decodes UTF-8 (ticket #2106)
+
+*2026-08-17.* `BinaryData::ToString()` copied its byte range **verbatim**, so it could return a
+`std::string` that is not valid UTF-8. .NET's is `Encoding.UTF8.GetString(_bytes.Span)` and
+always returns valid text.
+
+Landed under `docs/StandingApprovals.md` SA-5. No public signature, layout, vtable or `noexcept`
+change.
+
+---
+
+## 1. What changed
+
+| Bytes | Was | Is |
+|---|---|---|
+| `FF` | `FF` — not valid UTF-8 | `EF BF BD` (U+FFFD) |
+| `E2 82` (truncated U+20AC) | `E2 82` | `EF BF BD EF BF BD` — one per ill-formed **byte** |
+| `FF 'a' E2 82 AC` | unchanged | `EF BF BD 'a' E2 82 AC` |
+| any well-formed UTF-8 | — | **byte-identical** |
+
+One replacement per ill-formed byte, because the decoder resumes one byte later. That is what
+stops a truncated sequence swallowing the character that follows it.
+
+## 2. The second half is a deliberate permanent deviation
+
+The finding has two halves, and they are answered **differently**.
+
+.NET's `BinaryData(byte[])` **wraps** its argument — `_bytes = data`, `BinaryData.cs:60-63` — so
+a later mutation of the caller's array is observable through the `BinaryData`. This port
+**copies**, and will keep copying.
+
+Reproducing the aliasing here would mean holding a reference to storage this object does not own,
+in a language with no GC to keep it alive. That is precisely the borrowed-reference defect
+CCF-019 exists to remove, and that #1959, #2029, #2066, #2088, #2096 and #2134 have spent this
+programme removing. A managed alias is safe because the runtime keeps the array reachable; a C++
+one is a use-after-free waiting for the caller's vector to leave scope.
+
+So it is a deviation with a reason rather than an unfinished repair, and it is pinned in that
+direction.
+
+## 3. The UTF-8 decoder moved to `Core.Base`
+
+`modules/io` does not depend on `Text`, where the one shared UTF-8 scalar decode lived (ticket
+#2014). The options were a **sixth** copy of that decode, a new **public** component edge from
+`io` to `Text`, or moving the single definition somewhere everything already depends on.
+
+It now lives at `modules/core/include/System/detail/Utf8Scalar.hpp`, and
+`System::Text::detail::Utf8Scalar.hpp` re-exports the two names, so **every existing caller is
+unchanged**. The moved bodies are byte-identical to the originals, verified by diff. The module
+graph is unchanged: 41 modules, 92 edges.
+
+## 4. To migrate
+
+If you fed `BinaryData` well-formed UTF-8 — which every call site in this repository does — the
+output is byte-identical.
+
+If you were using `ToString()` to retrieve arbitrary bytes, that was never what it meant. Use
+`ToArray()`, which is unchanged and returns the bytes.
+
+## 5. Downstream, measured
+
+Neither `cna` nor `mobile-eggbert` references `System::BinaryData` — **zero sites in both**.
+`cna` matches the substring three times, all in vendored Vulkan headers
+(`pVendorBinaryData`, `vkGetShaderBinaryDataEXT`), which are unrelated. Neither repository was
+modified.