fix(core): ArraySegment::CopyTo rejects a short destination (#2328, SR-AUD-055)

CopyTo(std::vector<T>&, index) RESIZED the destination whenever it was too
short. .NET's body is Array.Copy (ArraySegment.cs:106-110) and .NET arrays
cannot grow, so it raises ArgumentException(Arg_LongerThanDestArray).

Two independent reasons, both from the review. It diverged from .NET -- a caller
who passed the wrong buffer got a silently enlarged one instead of a diagnostic,
and a caller who sized a buffer deliberately had that size overwritten. And it
was THE ONE PLACE IN THE REPOSITORY THAT BROKE ITS OWN CONVENTION: twenty of the
twenty-six CopyTo(std::vector&, ...) overloads already rejected a short
destination.

The check is written out locally rather than shared, because
requireValidCopyDestination lives in Collections.Core and Core.Base must not
depend on it. The message is .NET's own, so the two cannot drift apart in
wording.

THE REVIEW'S PREMISE CORRECTION WAS RIGHT AND STILL INCOMPLETE. It found four
tests pinning the resize where the finding named one; there were five. The
fifth, NonDefaultSegmentsAreUnaffected, was found by the FULL GATE after a
filtered run had already passed -- which is exactly why the gate is run over the
whole repository rather than over the suites a change looks like it touches.

CopyTo_VectorWithOffset_ExpandsDest was named for the behaviour and is INVERTED
rather than patched. #2214 had deliberately preserved the resize while repairing
this file's default-state and overlap defects; that preservation was correct
then and is superseded now, and the comment saying so is updated rather than
deleted.

Three mutations, all caught. Gate 17,300 run, 0 failed.

Author: robertvokac

CLAUDE.md

@@ -66,7 +66,7 @@ implementation ticket.
 | [SR-AUD-052](modules/core/include/System/Array.hpp.audit.md#sr-aud-052--medium--array-delegate-overloads-do-not-validate-empty-stdfunction-at-the-public-boundary) | medium | remediated | `Array.hpp` | Empty `std::function` inputs yield `bad_function_call` only if reached, or a silent normal result for empty arrays, instead of a deterministic argument error. |
 | [SR-AUD-053](modules/core/include/System/Array.hpp.audit.md#sr-aud-053--low--arraymaxlengthproperty-reports-int32max-not-the-net-runtime-limit) | low | remediated | `Array.hpp` | `MaxLengthProperty()` exposes `INT32_MAX`, 56 higher than current .NET `Array.MaxLength`, without a documented vector-adaptation decision. **Reviewed 2026-08-12 (#2327); STILL CONFIRMED, nothing implemented — `needs_user`.** Live: `Array.hpp:38` returns `std::numeric_limits<intcs>::max()` = 2,147,483,647 against .NET's `0x7FFFFFC7` = 2,147,483,591; the gap is 56. **Measured consumer surface: nothing uses it** — across `modules/`, `tests/`, `test/` and `bench/` the name occurs once outside its own definition, `ArrayTests.cpp:386` `EXPECT_GT(..., 0)`; no production call site, no `static_assert`, no array sizing, no allocation or indexing path, no serialized form. **Premise addition the finding does not state: the port already disagrees with itself.** `System::Buffers::MemoryPool<T>::MaxArrayLength` (`MemoryPool.hpp:42`) is a **public** `static constexpr intcs = 0x7FFFFFC7` documented "Matches .NET's Array.MaxLength (Array.cs)" and pinned to that literal by `MemoryPoolTests.MaxBufferSize_MatchesArrayMaxLength`; `ArrayBufferWriter<T>::MaxArrayLength` is the same value, private. The .NET number is already the repository's answer everywhere the limit is *enforced*; the only place publishing a different one is the property named after the .NET constant, where it is enforced nowhere. **Why it is still a decision:** the value is a public `constexpr` in a shipped header, so lowering it changes a downstream `constexpr` context's answer and makes a downstream `n > MaxLengthProperty()` guard newly reject 56 values — a surface this repository cannot measure (CNA/mobile-eggbert are outside its boundary), and the precedent for a public constant's value changing is `needs_user` (SR-AUD-130/#2326). The finding's own remediation is a disjunction, matching `CLAUDE.md` checklist item 5. **Deliberately not done:** pinning `== INT32_MAX`, which would encode the undecided branch as the tested contract. Options priced in #2327. `docs/CoreOwnedFindingsReviews2317.md`. **REMEDIATED (#2327, 2026-08-18) under SA-8.** `Array::MaxLengthProperty()` returned `int.MaxValue`; .NET's `Array.MaxLength` is `0x7FFFFFC7` = 2147483591 (`Array.cs:2641-2643`), **56 less**. The gap is not arbitrary and was not this port's to choose — .NET's own comment says *"Keep in sync with `inline SIZE_T MaxArrayLength()` from gchelpers"*, so the number is the GC's allocation ceiling and the documented contract is that *all* larger allocations fail. A port answering `int.MaxValue` promises 56 elements the reference refuses. **Measured: nothing uses it** — one occurrence outside the definition in the whole repository, and zero sites in either consumer. That single assertion, `EXPECT_GT(..., 0)`, is *why the divergence survived*: it passes for both values. It is replaced by the exact number and the 56-element gap stated directly. The value stays `constexpr`, so this is a value change rather than a signature change, and a `static_assert` pins that. One mutation, caught at compile time. |
 | [SR-AUD-054](modules/core/include/System/ArraySegment.hpp.audit.md#sr-aud-054--high--default-arraysegment-operations-silently-succeed-or-dereference-null-instead-of-throwing) | high | remediated | `ArraySegment.hpp` | Default-segment operations omit .NET's invalid-state guard; `ToArray` can silently succeed and `Slice(0)` reaches sanitizer-confirmed null dereference. **Residual (#2215, needs_user):** `begin()`/`end()`, the `GetEnumerator()` counterpart, still iterate a default segment zero times because guarding them needs an approved `noexcept` drop. **RESIDUAL NOW REMEDIATED TOO (#2215, 2026-08-18).** #2214 guarded ten of the type's array-touching doors and could not touch `begin()`/`end()`, because the guard requires dropping their `noexcept` — an exception-specification change, now covered by `docs/StandingApprovals.md` SA-10. All four enumeration doors are guarded, so a **default** segment raises `InvalidOperationException` where a range-`for` used to perform **zero iterations silently**, which is the same defect shape the other ten had: a clean, plausible, wrong result instead of a diagnostic. **The guard asks whether the array is present, never whether the count is zero** — an empty-but-real segment still iterates zero times, including one over an empty `std::vector` where `data()` may itself be null, and a test pins that distinction. SA-2's five conditions all discharged, including a four-site negative consumer fixture whose fourth site is the shape that breaks *silently* — a helper whose own `noexcept` is **computed** from the door. Three mutations: two caught, and the third **caught as a crash rather than a failure, inherently** — it lets a default segment reach `array_->data()` through a null pointer, which is the very SEGV this finding recorded, and no test can assert on undefined behaviour. Downstream measured: **zero** `ArraySegment` sites in either consumer (#2365). `docs/Migration-ArraySegmentEnumerationGuard.md`. |
-| [SR-AUD-055](modules/core/include/System/ArraySegment.hpp.audit.md) | medium | confirmed | `ArraySegment.hpp` | Vector `CopyTo` resizes an undersized destination rather than preserving ArraySegment's fixed-capacity destination/error contract. **Reviewed 2026-08-12 (#2328); STILL CONFIRMED — `needs_user`; one doc clause landed as #2329.** **Premise correction: four tests pin the resize, not one.** Besides `CopyTo_VectorWithOffset_ExpandsDest`, `ArraySegmentTests.CopyTo_Vector_CopiesAllElements` and `CopyTo_Vector_PartialSegment` both copy into an **empty** `dest` through the one-argument overload the finding treats only as "forwards to that behavior", and `CoreMemorySafetyOverlapTests.NonOverlappingCopiesKeepTheirPreviousResults` (`CoreMemorySafetyTests.cpp:837-838`) carries the comment `// SR-AUD-055's resize, unchanged` — ticket #2214 repaired this file's default-state and overlap defects and **deliberately preserved** the resize. **Premise addition: there is a repository convention and this is the one place that breaks it.** Of the **26** `void CopyTo(std::vector<...>&, ...)` overloads in `modules/`, **20 reject** a short destination — 13 with their own guard, five through the shared `System::Collections::detail::requireValidCopyDestination`, and `ImmutableList`'s two forwarding forms — and `ArraySegment`'s two-argument overload is the **only** one that grows a caller-sized destination. (`BitArray`'s two forms *replace* the destination outright: a third shape, a different .NET member, not this finding.) `modules/core` may not depend on `Collections`, so a repair writes its own guard rather than reusing the helper. **Why it is a decision:** the reject branch is a runtime behaviour break on currently accepted input — calls that succeed today start throwing and four first-party tests invert — the class this repository has always escalated. Both options priced in #2328. **#2329 (`done`, documentation only):** the one-argument overload's comment required "capacity for at least Count elements" and described writes "via `push_back`", both false and directly contradicted by the overload it forwards to; it now states what the code does and records that the resize-versus-reject question is open. `docs/CoreOwnedFindingsReviews2317.md`. |
+| [SR-AUD-055](modules/core/include/System/ArraySegment.hpp.audit.md) | medium | remediated | `ArraySegment.hpp` | Vector `CopyTo` resizes an undersized destination rather than preserving ArraySegment's fixed-capacity destination/error contract. **Reviewed 2026-08-12 (#2328); STILL CONFIRMED — `needs_user`; one doc clause landed as #2329.** **Premise correction: four tests pin the resize, not one.** Besides `CopyTo_VectorWithOffset_ExpandsDest`, `ArraySegmentTests.CopyTo_Vector_CopiesAllElements` and `CopyTo_Vector_PartialSegment` both copy into an **empty** `dest` through the one-argument overload the finding treats only as "forwards to that behavior", and `CoreMemorySafetyOverlapTests.NonOverlappingCopiesKeepTheirPreviousResults` (`CoreMemorySafetyTests.cpp:837-838`) carries the comment `// SR-AUD-055's resize, unchanged` — ticket #2214 repaired this file's default-state and overlap defects and **deliberately preserved** the resize. **Premise addition: there is a repository convention and this is the one place that breaks it.** Of the **26** `void CopyTo(std::vector<...>&, ...)` overloads in `modules/`, **20 reject** a short destination — 13 with their own guard, five through the shared `System::Collections::detail::requireValidCopyDestination`, and `ImmutableList`'s two forwarding forms — and `ArraySegment`'s two-argument overload is the **only** one that grows a caller-sized destination. (`BitArray`'s two forms *replace* the destination outright: a third shape, a different .NET member, not this finding.) `modules/core` may not depend on `Collections`, so a repair writes its own guard rather than reusing the helper. **Why it is a decision:** the reject branch is a runtime behaviour break on currently accepted input — calls that succeed today start throwing and four first-party tests invert — the class this repository has always escalated. Both options priced in #2328. **#2329 (`done`, documentation only):** the one-argument overload's comment required "capacity for at least Count elements" and described writes "via `push_back`", both false and directly contradicted by the overload it forwards to; it now states what the code does and records that the resize-versus-reject question is open. `docs/CoreOwnedFindingsReviews2317.md`. **REMEDIATED (#2328, 2026-08-18) under SA-8.** `CopyTo(std::vector<T>&, index)` **resized** the destination whenever it was too short; .NET's body is `Array.Copy` (`ArraySegment.cs:106-110`) and .NET arrays cannot grow, so it raises `ArgumentException(Arg_LongerThanDestArray)`. A caller who passed the wrong buffer got a silently enlarged one instead of a diagnostic, and a caller who sized a buffer deliberately had that size overwritten. **It was also the one place in this repository that broke its own convention** — twenty of the twenty-six `CopyTo(std::vector&, …)` overloads already rejected a short destination. The check is written out locally because the shared helper lives in `Collections.Core` and `Core.Base` must not depend on it; the **message** is .NET's own so the two cannot drift. **The review's premise correction was right and still incomplete**: it found four tests pinning the resize where the finding named one, and there were **five** — the fifth was found by the **full gate** after a filtered run had already passed, which is why the gate covers the whole repository rather than the suites a change looks like it touches. The test named for the behaviour is **inverted**, not patched, and #2214's deliberate preservation of the resize is recorded as superseded rather than deleted. Three mutations, all caught. Downstream: zero sites in either consumer. `docs/Migration-ArraySegmentCopyToRejectsShortDestination.md`. |
 | [SR-AUD-056](modules/core/tests/System/InterfaceTests2.cpp.audit.md#sr-aud-056--medium--observable-fixture-discards-the-subscription-handle-and-permits-post-completion-delivery) | medium | remediated | `InterfaceTests2.cpp` | The direct observable fixture returns a null subscription and permits `OnNext` after completion, leaving required unsubscription and terminal-state behavior unasserted. **Remediated (#2301 review, #2302 implementation, 2026-08-11)** by all five of the report's own suggested assertions plus the fixture behaviour they require. This is a test-contract defect, so the fixture and its assertions are the whole of it and **no production file was touched**. Re-measured first: the production interfaces were the right oracle all along - `IObservable<T>::Subscribe` already returns `std::shared_ptr<IDisposable>` and documents what it is for, and `IObserver<T>` already documents that no `OnNext` or `OnCompleted` follows a terminal call; only the fixture contradicted them, and the finding's own note that no first-party `IObservable<T>` implementation exists still holds, so nothing shipped was wrong. `IntObservable2::Subscribe` now returns a real `Subscription` that unsubscribes exactly its own observer, rejects a null observer with `System::ArgumentNullException`, and is a no-op on a second `Dispose()`; the provider records a terminal state so `Emit`/`Complete`/`Fail` deliver nothing after one, and `Fail` supplies the `OnError` path the report notes was never covered. Two documented design choices: the observer list lives in a `State` the provider and every subscription **co-own** - the `SortedSet<T>`/#1786 pattern - so disposal is safe in any destruction order and there is **no ownership cycle**, answering the last "other missing assertions" bullet; and `~Subscription()` deliberately does **not** dispose, because unsubscription is tied to `Dispose()` rather than to the handle's lifetime - measured, since an RAII destructor made both pre-existing cases fail, both discarding the handle. **Seven cases added, none retired** (2 -> 9); the two pre-existing keep their names, with a non-null subscription assertion added to the first. Both defects the finding names were re-introduced as mutations, rebuilt and relinked: returning `nullptr` from `Subscribe` fails `Subscribe_ReturnsADisposableSubscription` **and** the pre-existing `Subscribe_AndReceiveValues`; removing the terminal guard from `Emit` fails `NoValueIsDeliveredAfterCompletion` **and** `OnError_IsTerminalToo` - so the report's "a future implementation can copy this fixture's wrong behavior without a failing regression" risk is closed in both directions. Deliberately **not** pinned: subscription ordering, which no interface documents. Still open and outside this finding: the `ParseableInt` and provider-`nullptr` bullets. `docs/CoreObservableFixtureContractPlan.md`. |
 | [SR-AUD-057](modules/core/include/System/Index.hpp.audit.md#sr-aud-057--high--unchecked-net-index-offset-semantics-use-signed-c-overflow) | high | remediated | `Index.hpp`, `Range.hpp` | The intentionally unvalidated index/range offset path executes signed C++ overflow for a maximal from-end Index and `INT_MIN` length rather than implementing .NET's defined unchecked arithmetic. |
 | [SR-AUD-058](modules/core/include/System/Progress.hpp.audit.md#sr-aud-058--medium--empty-progress-event-subscription-becomes-a-delayed-stdbad_function_call) | medium | remediated | `Progress.hpp` | An empty added progress callback is stored and later throws `std::bad_function_call`; .NET's nullable event subscription cannot create that delayed invocation failure. |

audit/AUDIT_FINDINGS_INDEX.md

@@ -0,0 +1,85 @@
+<!-- SPDX-License-Identifier: MIT -->
+<!-- Copyright (c) Robert Vokac and contributors -->
+
+# Migration — `ArraySegment::CopyTo` rejects a short destination (ticket #2328)
+
+*2026-08-18.* `ArraySegment<T>::CopyTo(std::vector<T>&, index)` **resized** the destination
+whenever it was too short. .NET's body is `Array.Copy`, and .NET arrays cannot grow.
+
+Landed under `docs/StandingApprovals.md` **SA-8**. A **narrowing**: a call that used to succeed by
+enlarging the destination now throws. No signature, layout, vtable or `noexcept` change.
+
+---
+
+## 1. What changed
+
+| Call | Was | Is |
+|---|---|---|
+| `seg.CopyTo(dest)` with `dest.size() < count` | **grew `dest`** | **`ArgumentException`**, `dest` untouched |
+| `seg.CopyTo(dest, 3)` with an empty `dest` | grew `dest` to `3 + count` | **`ArgumentException`**, `dest` untouched |
+| destination exactly large enough | worked | **unchanged** |
+| a default segment | `InvalidOperationException` | **unchanged**, and still checked **first** |
+| negative `destinationIndex` | `ArgumentOutOfRangeException` | **unchanged** |
+| `CopyTo(ArraySegment<T>&)` | already rejected | **unchanged** |
+
+The message is `Strings.resx:478-480`, transcribed: *"Destination array was not long enough. Check
+the destination index, length, and the array's lower bounds."*
+
+## 2. Why
+
+Two independent reasons, and both matter.
+
+**It diverged from .NET.** The body is `Array.Copy(_array, _offset, destination,
+destinationIndex, _count)` (`ArraySegment.cs:106-110`), which raises
+`ArgumentException(SR.Arg_LongerThanDestArray)` when the destination is short. A caller who passed
+the wrong buffer got a silently enlarged one instead of a diagnostic, and a caller who sized a
+buffer deliberately had that size overwritten.
+
+**It was the one place in this repository that broke its own convention.** Of the twenty-six
+`CopyTo(std::vector<T>&, …)` overloads in `modules/`, twenty already rejected a short destination —
+thirteen with their own guard, five through
+`System::Collections::detail::requireValidCopyDestination`, and `ImmutableList`'s two forwarding
+forms through its four-argument body.
+
+The check is written out in `ArraySegment.hpp` rather than shared, because that helper lives in
+`Collections.Core` and `Core.Base` must not depend on it. The **message** is .NET's own, so the two
+cannot drift apart in wording.
+
+## 3. To migrate
+
+Size the destination first:
+
+```cpp
+// before
+std::vector<int> dest;
+seg.CopyTo(dest);
+
+// after
+std::vector<int> dest(seg.getCountProperty());
+seg.CopyTo(dest);
+
+// with an index, the index counts against the room
+std::vector<int> dest(3 + seg.getCountProperty());
+seg.CopyTo(dest, 3);
+```
+
+Or use `ToArray()`, which allocates for you and is unchanged.
+
+## 4. First party
+
+Five tests asserted the resize, not the one the finding named. Four were predicted by the review
+(`CopyTo_Vector_CopiesAllElements`, `CopyTo_Vector_PartialSegment`,
+`CopyTo_VectorWithOffset_ExpandsDest`, and `NonOverlappingCopiesKeepTheirPreviousResults`, which
+carried the comment `// SR-AUD-055's resize, unchanged`). **A fifth,
+`NonDefaultSegmentsAreUnaffected`, was not** — it was found by the full gate after a filtered run
+had already passed, which is why the gate is run on the whole repository rather than on the
+suites a change looks like it touches.
+
+`CopyTo_VectorWithOffset_ExpandsDest` was named for the behaviour and is **inverted** rather than
+patched. #2214 had deliberately *preserved* the resize while repairing this file's default-state
+and overlap defects; that preservation was correct then and is superseded now.
+
+## 5. Downstream, measured
+
+Neither `cna` nor `mobile-eggbert` references `ArraySegment` — **zero sites in both** (measured for
+#2215 on the same date, and re-checked). Neither repository was modified.