diff --git a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ApiMethodCache.cs b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ApiMethodCache.cs index 0d4c4a14..dab26a3e 100644 --- a/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ApiMethodCache.cs +++ b/src/ZB.MOM.WW.ScadaBridge.InboundAPI/ApiMethodCache.cs @@ -34,6 +34,8 @@ public sealed class ApiMethodCache private sealed record Entry(ApiMethod? Method, DateTimeOffset ExpiresAtUtc); private readonly ConcurrentDictionary _entries = new(StringComparer.Ordinal); + private const int PruneThreshold = 256; + private const int HardCap = 4096; private readonly TimeSpan _ttl; private readonly TimeProvider _timeProvider; @@ -73,10 +75,34 @@ public sealed class ApiMethodCache } var method = await fetch(cancellationToken).ConfigureAwait(false); + if (_entries.Count >= PruneThreshold) + PruneExpired(now); _entries[methodName] = new Entry(method, now + _ttl); return method; } + /// + /// Opportunistically removes expired entries on a miss once the dictionary is large, and + /// clears it outright at the hard cap. Growth is bounded today by the invariant that + /// EndpointExtensions reaches this cache only after the in-memory scope check, and + /// a key's scopes are EXACT method names (no wildcard/prefix scope exists in + /// ZB.MOM.WW.Auth.ApiKeys; pinned by EndpointExtensionsTests). Should a + /// wildcard scope ever be added, every probed path would otherwise be a permanent + /// negative entry — this prune is the safety net. + /// + private void PruneExpired(DateTimeOffset now) + { + foreach (var kv in _entries) + { + if (kv.Value.ExpiresAtUtc <= now) + _entries.TryRemove(kv); + } + + // Still at the cap with nothing expired: drop everything (entries are re-fetchable). + if (_entries.Count >= HardCap) + _entries.Clear(); + } + /// Drops the cached entry for one method name. Safe to call for an unknown name. /// The method name to invalidate. public void Invalidate(string methodName) => _entries.TryRemove(methodName, out _); diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ApiMethodCacheTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ApiMethodCacheTests.cs index b4980ce9..dbeb0188 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ApiMethodCacheTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/ApiMethodCacheTests.cs @@ -122,4 +122,24 @@ public class ApiMethodCacheTests /// public override DateTimeOffset GetUtcNow() => _now; } + + /// Verifies that expired entries are pruned on a miss once the cache is large. + /// A task that represents the asynchronous operation. + [Fact] + public async Task GetOrFetchAsync_Miss_PrunesExpiredEntries() + { + // Review 2026-10-21 05/M3: expired entries for methods no longer called were never evicted. + var time = new FakeTimeProvider(DateTimeOffset.UtcNow); + var cache = new ApiMethodCache(TimeSpan.FromSeconds(1), time); + Task Fetch(CancellationToken _) => Task.FromResult(null); + + for (var i = 0; i < 300; i++) + await cache.GetOrFetchAsync("m" + i, Fetch, CancellationToken.None); + Assert.Equal(300, cache.Count); + + time.Advance(TimeSpan.FromSeconds(2)); + await cache.GetOrFetchAsync("fresh", Fetch, CancellationToken.None); + + Assert.Equal(1, cache.Count); + } } diff --git a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs index b17e6237..00f2ac98 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.InboundAPI.Tests/EndpointExtensionsTests.cs @@ -327,6 +327,27 @@ public sealed class EndpointExtensionsTests : IDisposable Assert.Equal(NotApprovedBodyJson, body); } + /// Verifies that scopes are exact method names, not prefixes or wildcards. + /// A task that represents the asynchronous operation. + [Theory] + [InlineData("*")] + [InlineData("sec*")] + [InlineData("sec")] + public async Task ValidKey_WildcardOrPrefixScope_DoesNotGrantMethod(string scope) + { + // Review 2026-10-21 05/M3: ApiMethodCache bounds its growth on the invariant that scopes + // are exact names; pin it so a future wildcard scope has to revisit the cache. + var method = SeedMethod(1, "secured", "return 1;"); + + using var host = await BuildHostAsync(method); + var token = await SeedKeyAsync(host, "key1", "caller", new[] { scope }); + var client = host.GetTestClient(); + + var response = await client.SendAsync(BuildPost("secured", "{}", token)); + + Assert.Equal(HttpStatusCode.Forbidden, response.StatusCode); + } + /// Verifies that an unknown method returns 403 with a body identical to the not-in-scope body. /// A task that represents the asynchronous operation. [Fact]