From 5b681ee59b58e0712ae512f9ec50ae13d1008b64 Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Fri, 7 Aug 2026 06:10:14 -0400 Subject: [PATCH] fix(SEC-31,SEC-32): identify a probe-slot reservation by version, not by timestamp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ReleaseProbe recognised its own reservation by comparing NextProbeAtTicks to now + _probeIntervalTicks. RecordInto's rearm-on-trip writes that identical expression, so a concurrent RecordFailure on the same WindowState whose `now` lands on the claimer's tick — routine at ~1 ms clock resolution under load — was mistaken for the caller's own claim. The release then stomped the legitimate fresh re-arm back to the stale previousProbeAtTicks, which is already due, handing the next arrival a free probe the re-arm had just closed. WindowState gains a monotonic ProbeVersion bumped by every writer of NextProbeAtTicks (TryConsumeProbe's claim and RecordInto's re-arm alike). TryConsumeProbe returns the stamp it set as part of a ProbeClaim; ReleaseProbe restores the previous value only while the state's version still equals that stamp, checking and restoring in one lock(state) section and bumping the version again on restore so no other stale release can match either. Test: ProbeSlotRestore_DoesNotStompConcurrentRearmAtSameTick, with the clock held still so the claim and the interleaved failure necessarily share a tick. Making it deterministic needed a seam — the claim-to-release window is a few nanoseconds and racing threads do not hit it (an earlier thread-based attempt passed against the defective guard three runs out of three, and its end state was ordering-dependent rather than correctness-dependent, so it was dropped rather than shipped as theatre). The seam is an internal ProbeReleaseInterleaveHook, null in production, costing one null check on the already-refused path. Verified as a genuine red against the timestamp guard: Expected ThrottledByPeer, Actual ProbeAdmitted. --- .../2026-07-12/remediation/00-tracking.md | 2 +- docs/Authorization.md | 2 +- .../Authorization/ApiKeyFailureLimiter.cs | 63 +++++++++++++++---- .../ApiKeyFailureLimiterTests.cs | 48 ++++++++++++++ 4 files changed, 102 insertions(+), 13 deletions(-) diff --git a/archreview/2026-07-12/remediation/00-tracking.md b/archreview/2026-07-12/remediation/00-tracking.md index bc9c721..fe9be4f 100644 --- a/archreview/2026-07-12/remediation/00-tracking.md +++ b/archreview/2026-07-12/remediation/00-tracking.md @@ -161,4 +161,4 @@ Sequence these together rather than piecemeal — several are one change set spa | 2026-07-13 | Operator bring-up complete: dedicated CI ed25519 key installed in windev `administrators_authorized_keys` (authorized into `dohertj2`, which owns the working MXAccess/toolchain env — a fresh OS account would break the build; the key is independently revocable), Gitea secrets `WINDEV_SSH_KEY`/`WINDEV_SSH_KNOWN_HOSTS` + variable `WINDEV_SSH_USER=dohertj2` stored, runner→`10.100.0.48:22` egress verified on the `traefik` net, issue-write confirmed. **TST-25/TST-26 → `Done`:** credentialed `windows-x86` ran GREEN on `d769244` (Gitea run #37) — Linux runner SSHed windev, checked out the SHA in `C:\build\mxaccessgw-ci` under lock, ran the x86 Worker build + `Worker.Tests`, exit 0; `nightly-windev` correctly skipped on the push event. Branch merged to `main`. Follow-ups (old tracker): revisit **TST-05** (scheduled live smoke — now covered by `nightly-windev`) and **TST-24** (client wire tests) which this unlocks. | | 2026-07-13 | Ran the TST-25 acceptance checks (scripts/ci/README.md) — they caught **two real CI defects, both fixed** on `fix/tst-25-ci-key-log-leak`: (1) **CI SSH key leaked in cleartext** in the `windows-x86` step env echo (Gitea's line-oriented masker missed the multiline PEM) — rotated the CI key on windev (old pubkey revoked), stored the key **base64-encoded** so the masker redacts it to `***` (confirmed on run #38), taught `run-windev-ci.sh` to decode, dropped the redundant public known-hosts secret from the job env; (2) **bootstrap lock race** — `run-windev-ci.sh`'s pre-hand-off `git fetch`/`checkout` ran outside the worktree lock, so concurrent runs collided on `.git/index.lock`; the bootstrap now holds the lock (ps1 re-uses it via `MXGW_CI_LOCK_HELD`), retest confirmed clean serialization. Also **deflaked** `SessionManagerTests` fail-fast timing assertions (absolute `<100ms` wall-clock bound flaked under CI load; now anchored to the configured timeout / dropped for the zero-timeout case). Checks passed: unreachable-host fast-fail (exit 255/15s), deliberate-red propagation (Worker.Tests failure → exit 1), lock concurrency (2nd run waits), no-key-in-logs (masked). Merge target `df7e20d` verified GREEN via the local windev path (Worker build + 356 tests); merged to `main` `19cbf7b`. Check 6 (forced-failure nightly issue): issue endpoint+token proven live at bring-up (#124); in-CI forced-failure probe abandoned to shared-runner congestion (residual `if: failure()` gating is standard Actions). | | 2026-07-13 | New finding **TST-30** (`Low`/`P2`) added — surfaced during TST-25 acceptance: CI runs on a single shared `gitea-runner` (`maxParallel=1`, co-located `10.100.0.35`) interleaved with `dohertj2/lmxopcua`, and Gitea 1.26 exposes no run cancel/delete, so queue latency is unbounded under cross-repo contention and the runner is a single point of failure. Design: add a second/labelled runner + document the no-cancel reality and the `run-windev-ci.sh` bypass. Roll-ups updated (Testing Low 2→3, total 47→48; P2 9→10). | -| 2026-08-07 | **SEC-31 + SEC-32 → `Done`** (branch `fix/sec-31-32-limiter`, one change set as planned). `ApiKeyFailureLimiter` reworked from `IsBlocked/RecordFailure/Reset(string peer)` to a partition-pair API (`Check/RecordFailure/Reset(ApiKeyThrottlePartition)` returning `ApiKeyThrottleDecision`): layer 1 is the composite `(transport peer, key id)` partition, layer 2 a per-key-id aggregate across peers (`ApiKeyFailureAggregateLimit`, default 30), and an over-limit state now admits one probe per `ApiKeyFailureProbeIntervalSeconds` (default 5) instead of blocking absolutely — so a success can reset the state while throttled, killing the 10-packets-per-minute lockout. SEC-32 rides along: the interceptor validates token shape (`mxgw` prefix, ≥3 non-empty `_` segments, key id ≤ 64 chars) before minting a key-id partition, each peer may mint at most 32 of them (overflow collapses to its fallback partition), and eviction prefers expired windows, never dropping an over-limit partition below a 2× transient overshoot ceiling. New counter `mxgateway.auth.throttled` tagged `stage=peer\|aggregate` only (no key material — `/metrics` is still unauthenticated per open SEC-14). Docs updated in the same commit (`docs/GatewayConfiguration.md` limiter rows + two new keys, `docs/Authentication.md` hot-path paragraph, `docs/Authorization.md` SEC-11 section, limiter/`SecurityOptions` XML remarks). Evidence: `dotnet build …Server` clean; `--filter ~ApiKeyFailureLimiter` 11/11 passed (new `ApiKeyFailureLimiterTests`), `--filter ~GatewayGrpcAuthorizationInterceptor` 20/20 passed (incl. the four SEC-31 contract tests and `NonMxgwToken_FallsBackToTransportPeerPartition`), `--filter ~GatewayOptionsValidator` 66/66 passed. Full suite on macOS: 804 passed / 44 failed — all 44 are the pre-existing named-pipe fake-worker classes (`WorkerClientTests`, `FakeWorkerHarnessTests`, `SessionWorkerClientFactoryFakeWorkerTests`, `GatewayEndToEnd*`), verified identical (44) on the unmodified tree. Follow-up unchanged: the new `MxGateway:Security` keys belong in old **SEC-24**'s effective-config projection when that is picked up. **Code review of the branch found two defects in the first pass, both fixed before merge:** (1) probe admission was check-then-act across two lock scopes, so a burst arriving at an interval boundary could all observe "due" and all be admitted — the claim is now a single critical section (`TryConsumeProbe`), and because the two layers are claimed one at a time, a slot claimed on the partition is compensated (`ReleaseProbe`) when the aggregate then refuses; (2) `Reset` on a success whose key id had been collapsed into the address's shared fallback partition removed that shared partition, letting one authentication wipe an in-progress spray from the same address — it is now left to decay by window expiry, while the key's aggregate is still cleared. Tests added: `ProbeAdmission_UnderConcurrentArrivals_GrantsExactlyOneSlot`, `ProbeAdmission_WhenAggregateRefuses_ReturnsTheClaimedPeerSlot`, `Reset_WithOverCapKeyId_DoesNotClearSharedFallbackPartition` (limiter suite 11 → 14). | +| 2026-08-07 | **SEC-31 + SEC-32 → `Done`** (branch `fix/sec-31-32-limiter`, one change set as planned). `ApiKeyFailureLimiter` reworked from `IsBlocked/RecordFailure/Reset(string peer)` to a partition-pair API (`Check/RecordFailure/Reset(ApiKeyThrottlePartition)` returning `ApiKeyThrottleDecision`): layer 1 is the composite `(transport peer, key id)` partition, layer 2 a per-key-id aggregate across peers (`ApiKeyFailureAggregateLimit`, default 30), and an over-limit state now admits one probe per `ApiKeyFailureProbeIntervalSeconds` (default 5) instead of blocking absolutely — so a success can reset the state while throttled, killing the 10-packets-per-minute lockout. SEC-32 rides along: the interceptor validates token shape (`mxgw` prefix, ≥3 non-empty `_` segments, key id ≤ 64 chars) before minting a key-id partition, each peer may mint at most 32 of them (overflow collapses to its fallback partition), and eviction prefers expired windows, never dropping an over-limit partition below a 2× transient overshoot ceiling. New counter `mxgateway.auth.throttled` tagged `stage=peer\|aggregate` only (no key material — `/metrics` is still unauthenticated per open SEC-14). Docs updated in the same commit (`docs/GatewayConfiguration.md` limiter rows + two new keys, `docs/Authentication.md` hot-path paragraph, `docs/Authorization.md` SEC-11 section, limiter/`SecurityOptions` XML remarks). Evidence: `dotnet build …Server` clean; `--filter ~ApiKeyFailureLimiter` 11/11 passed (new `ApiKeyFailureLimiterTests`), `--filter ~GatewayGrpcAuthorizationInterceptor` 20/20 passed (incl. the four SEC-31 contract tests and `NonMxgwToken_FallsBackToTransportPeerPartition`), `--filter ~GatewayOptionsValidator` 66/66 passed. Full suite on macOS: 804 passed / 44 failed — all 44 are the pre-existing named-pipe fake-worker classes (`WorkerClientTests`, `FakeWorkerHarnessTests`, `SessionWorkerClientFactoryFakeWorkerTests`, `GatewayEndToEnd*`), verified identical (44) on the unmodified tree. Follow-up unchanged: the new `MxGateway:Security` keys belong in old **SEC-24**'s effective-config projection when that is picked up. **Code review of the branch found two defects in the first pass, both fixed before merge:** (1) probe admission was check-then-act across two lock scopes, so a burst arriving at an interval boundary could all observe "due" and all be admitted — the claim is now a single critical section (`TryConsumeProbe`), and because the two layers are claimed one at a time, a slot claimed on the partition is compensated (`ReleaseProbe`) when the aggregate then refuses; (2) `Reset` on a success whose key id had been collapsed into the address's shared fallback partition removed that shared partition, letting one authentication wipe an in-progress spray from the same address — it is now left to decay by window expiry, while the key's aggregate is still cleared. Tests added: `ProbeAdmission_UnderConcurrentArrivals_GrantsExactlyOneSlot`, `ProbeAdmission_WhenAggregateRefuses_ReturnsTheClaimedPeerSlot`, `Reset_WithOverCapKeyId_DoesNotClearSharedFallbackPartition`. **A second review pass found a residual defect in that compensation path:** the release identified its own reservation by comparing `NextProbeAtTicks` to `now + interval`, the identical expression a failure re-arm writes — so a concurrent `RecordFailure` on the same state sharing a clock tick (routine at ~1 ms resolution) was mistaken for the caller's own claim and stomped back to the stale, already-due value, prematurely reopening the probe slot. Replaced with a monotonic per-state `ProbeVersion` bumped by every writer of `NextProbeAtTicks` (claim and re-arm alike); the release restores only when the version still matches the one its claim stamped, and bumps it again on restore so no other stale release can match. Covered by `ProbeSlotRestore_DoesNotStompConcurrentRearmAtSameTick`, made deterministic by a new `internal ProbeReleaseInterleaveHook` test seam (null in production, one null check on the refused path) because the claim-to-release window is nanoseconds wide and racing threads cannot hit it reliably — verified as a genuine red against the timestamp guard (`Expected: ThrottledByPeer / Actual: ProbeAdmitted`). Limiter suite 11 → 15. | diff --git a/docs/Authorization.md b/docs/Authorization.md index 52f1765..abb8dc3 100644 --- a/docs/Authorization.md +++ b/docs/Authorization.md @@ -96,7 +96,7 @@ Before the verification store read, the helper asks a cheap in-process failure c - **Composite `(transport peer, key id)` partitions.** Reaching `MxGateway:Security:ApiKeyFailureLimit` failures binds the throttle to the address that produced them. The key id alone is never the partition: key ids are not secret — they ride in every token and are listed on the dashboard — so keying on them let any network peer deny a key to its legitimate holder. The key id joins the partition only after a token-shape check (literal `mxgw` prefix, at least three non-empty `_` segments, key id of at most 64 characters), and one address may mint at most 32 key-id partitions before the overflow collapses onto that address's fallback partition. - **A per-key-id aggregate** across all peers (`ApiKeyFailureAggregateLimit`, default 30), which bounds a distributed or source-rotating sprayer that never trips any single partition. -An over-limit state is a valve, not a wall: one request per `ApiKeyFailureProbeIntervalSeconds` (default 5 s) is admitted through to the real verifier, and everything else is refused with `StatusCode.ResourceExhausted` before the store read. The slot is claimed atomically, so a burst arriving together at an interval boundary still yields exactly one admission. A successful verification resets both layers — which is why the reset path stays reachable while a key is under active spray. One exception: when the caller's key id was collapsed into its address's shared fallback partition by the per-peer cap, a success clears the key's aggregate but leaves that shared partition alone, since it also holds failures contributed by other key ids from the same address. The tracked partitions form a bounded LRU (`ApiKeyFailureTrackedPeers`) whose eviction prefers fully expired windows and never removes an over-limit partition below a 2x transient overshoot ceiling, so the cap bounds memory without becoming a reset button for an active block. `ResourceExhausted` reveals only that throttling is in effect, not whether any particular secret was valid, preserving the opaque-failure property. Refusals increment `mxgateway.auth.throttled`, tagged `stage=peer|aggregate` and nothing else — `/metrics` is unauthenticated, so neither key ids nor peer addresses may appear there. +An over-limit state is a valve, not a wall: one request per `ApiKeyFailureProbeIntervalSeconds` (default 5 s) is admitted through to the real verifier, and everything else is refused with `StatusCode.ResourceExhausted` before the store read. The slot is claimed atomically, so a burst arriving together at an interval boundary still yields exactly one admission, and a slot claimed for a request that a later layer then refuses is handed back under a per-state version stamp — never by timestamp comparison, which collides whenever a concurrent failure re-arms the same state on the same clock tick. A successful verification resets both layers — which is why the reset path stays reachable while a key is under active spray. One exception: when the caller's key id was collapsed into its address's shared fallback partition by the per-peer cap, a success clears the key's aggregate but leaves that shared partition alone, since it also holds failures contributed by other key ids from the same address. The tracked partitions form a bounded LRU (`ApiKeyFailureTrackedPeers`) whose eviction prefers fully expired windows and never removes an over-limit partition below a 2x transient overshoot ceiling, so the cap bounds memory without becoming a reset button for an active block. `ResourceExhausted` reveals only that throttling is in effect, not whether any particular secret was valid, preserving the opaque-failure property. Refusals increment `mxgateway.auth.throttled`, tagged `stage=peer|aggregate` and nothing else — `/metrics` is unauthenticated, so neither key ids nor peer addresses may appear there. The dashboard login surface is throttled independently: `POST /auth/login` carries a fixed-window ASP.NET Core rate-limiter policy keyed per remote IP (`MxGateway:Security:LoginRateLimit*`), rejecting a burst with HTTP 429 before the LDAP bind is relayed to the directory. See [GatewayConfiguration](./GatewayConfiguration.md#security-options). diff --git a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs index db5131d..ad28237 100644 --- a/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs +++ b/src/ZB.MOM.WW.MxGateway.Server/Security/Authorization/ApiKeyFailureLimiter.cs @@ -112,6 +112,18 @@ public sealed class ApiKeyFailureLimiter /// Gets the number of tracked per-key-id aggregates. Test seam. internal int TrackedAggregateCount => _aggregates.Count; + /// + /// Test seam invoked between a probe claim and its compensating release; + /// in production, where it costs one null check on the already-refused path. + /// + /// + /// The window this straddles is a few nanoseconds wide, so the interleaving it exists to cover — + /// a concurrent failure re-arming the same state between the two — cannot be produced reliably + /// by racing threads. Without this seam the version guard in would be + /// verifiable only by inspection. + /// + internal Action? ProbeReleaseInterleaveHook { get; set; } + /// Decides whether an authentication attempt may reach the verifier. /// The throttle partition derived from the request. /// The admission decision for this attempt. @@ -150,11 +162,11 @@ public sealed class ApiKeyFailureLimiter // Every over-limit layer must yield its probe slot for the request to pass. The slots are // claimed one at a time (holding two per-state locks at once would need a global ordering to // stay deadlock-free), so a claim is reserved and then compensated if a later layer refuses. - long peerProbeRestore = 0; + ProbeClaim peerClaim = default; bool peerProbeClaimed = false; if (peerOver) { - if (!TryConsumeProbe(peerState!, now, out peerProbeRestore)) + if (!TryConsumeProbe(peerState!, now, out peerClaim)) { return ApiKeyThrottleDecision.ThrottledByPeer; } @@ -166,7 +178,8 @@ public sealed class ApiKeyFailureLimiter { if (peerProbeClaimed) { - ReleaseProbe(peerState!, now, peerProbeRestore); + ProbeReleaseInterleaveHook?.Invoke(); + ReleaseProbe(peerState!, peerClaim); } return ApiKeyThrottleDecision.ThrottledByAggregate; @@ -264,10 +277,13 @@ public sealed class ApiKeyFailureLimiter state.LastActivityTicks = now; // Arm (or push out) the probe slot whenever the state is at or over its limit, so the - // attempt that trips the limit is not itself followed by an immediate free probe. + // attempt that trips the limit is not itself followed by an immediate free probe. This + // is a write of NextProbeAtTicks, so it bumps the version that identifies a probe claim + // — otherwise a release could mistake this re-arm for its own reservation. if (limit > 0 && state.FailureTicks.Count >= limit) { state.NextProbeAtTicks = now + _probeIntervalTicks; + state.ProbeVersion++; } } } @@ -292,35 +308,47 @@ public sealed class ApiKeyFailureLimiter /// arriving at the interval boundary observe "due" and all be admitted, which is exactly the /// unbounded-guessing burst the probe interval exists to prevent. /// - private bool TryConsumeProbe(WindowState state, long now, out long previousProbeAtTicks) + private bool TryConsumeProbe(WindowState state, long now, out ProbeClaim claim) { lock (state) { - previousProbeAtTicks = state.NextProbeAtTicks; + long previousProbeAtTicks = state.NextProbeAtTicks; if (now < previousProbeAtTicks) { + claim = default; return false; } state.NextProbeAtTicks = now + _probeIntervalTicks; state.LastActivityTicks = now; + claim = new ProbeClaim(previousProbeAtTicks, ++state.ProbeVersion); return true; } } /// /// Returns a probe slot claimed for a request that a later layer then refused, so the wasted - /// reservation does not cost the next arrival its slot. Only the caller's own reservation is - /// undone — a slot re-granted or re-armed in the meantime wins. + /// reservation does not cost the next arrival its slot. /// - private void ReleaseProbe(WindowState state, long now, long previousProbeAtTicks) + /// + /// The claim is identified by the per-state version stamped when it was made, never by the + /// timestamp it wrote. Every writer of NextProbeAtTicks bumps that version, so a re-arm + /// from a concurrent failure — which writes the identical now + interval expression, and + /// at ~1 ms clock resolution routinely lands on the same tick — cannot be mistaken for the + /// caller's own reservation and stomped back to a stale, already-due value. Restoring bumps the + /// version again so no other stale release can match either. + /// + private static void ReleaseProbe(WindowState state, ProbeClaim claim) { lock (state) { - if (state.NextProbeAtTicks == now + _probeIntervalTicks) + if (state.ProbeVersion != claim.Version) { - state.NextProbeAtTicks = previousProbeAtTicks; + return; } + + state.NextProbeAtTicks = claim.PreviousProbeAtTicks; + state.ProbeVersion++; } } @@ -504,8 +532,21 @@ public sealed class ApiKeyFailureLimiter public long LastActivityTicks; public long NextProbeAtTicks; + + /// + /// Monotonic stamp bumped by every writer of (probe claim and + /// failure re-arm alike). It is what lets a compensating release recognise its own + /// reservation without comparing timestamps, which collide whenever two writers share a + /// clock tick. + /// + public long ProbeVersion; } + /// A probe slot reservation: what to restore, and the stamp proving it is still ours. + /// The slot value replaced when the claim was made. + /// The stamped by this claim. + private readonly record struct ProbeClaim(long PreviousProbeAtTicks, long Version); + private sealed class PeerKeyIds { /// Key ids this transport peer has minted a partition for. diff --git a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs index 95d7935..d843b83 100644 --- a/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs +++ b/src/ZB.MOM.WW.MxGateway.Tests/Security/Authorization/ApiKeyFailureLimiterTests.cs @@ -219,6 +219,54 @@ public sealed class ApiKeyFailureLimiterTests Assert.Equal(ApiKeyThrottleDecision.ProbeAdmitted, limiter.Check(holder)); } + /// + /// A compensating release must never undo a re-arm written by a concurrent failure on the same + /// partition. Both writers store the identical now + interval value when they share a + /// clock tick, so identifying the caller's own reservation by timestamp would let the release + /// stomp a fresh re-arm back to an already-due value and reopen the probe slot early. The clock + /// is deliberately held still here, which forces exactly that collision. + /// + [Fact] + public void ProbeSlotRestore_DoesNotStompConcurrentRearmAtSameTick() + { + ManualTimeProvider clock = new(DateTimeOffset.UnixEpoch); + ApiKeyFailureLimiter limiter = CreateLimiter(clock, limit: 3, aggregateLimit: 5); + ApiKeyThrottlePartition holder = new("ipv4:10.0.0.1:1", "victim"); + ApiKeyThrottlePartition other = new("ipv4:10.0.0.2:1", "victim"); + + // Trip both layers, then push the aggregate's slot one interval past the partition's, so + // every Check below claims the partition's slot and is then refused by the aggregate — the + // claim-and-compensate path under test. + RecordFailures(limiter, holder, 5); + clock.Advance(TimeSpan.FromSeconds(3)); + limiter.RecordFailure(other); + clock.Advance(TimeSpan.FromSeconds(2)); + + // Land a failure on the same partition inside the claim-to-release window — the interleaving + // a concurrent RecordFailure produces, forced here so the assertion is deterministic. It + // shares the frozen clock tick with the claim, so both write the identical slot value. + int interleaved = 0; + limiter.ProbeReleaseInterleaveHook = () => + { + if (Interlocked.Exchange(ref interleaved, 1) == 0) + { + limiter.RecordFailure(holder); + } + }; + + Assert.Equal(ApiKeyThrottleDecision.ThrottledByAggregate, limiter.Check(holder)); + Assert.Equal(1, interleaved); + limiter.ProbeReleaseInterleaveHook = null; + + // Drop the aggregate so the next decision reflects the composite partition alone. + limiter.Reset(other); + + // The interleaved failure pushed the slot one interval past the (still unadvanced) clock, so + // no probe may be due. Restoring over it would leave the already-due earlier value and hand + // the next arrival a free probe. + Assert.Equal(ApiKeyThrottleDecision.ThrottledByPeer, limiter.Check(holder)); + } + /// A zero probe interval restores absolute blocking (documented as not recommended). [Fact] public void ProbeIntervalZero_BlocksAbsolutely()