fix(inbound): prune expired ApiMethodCache entries; pin exact-name scopes (review 2026-10-21 05/M3)
This commit is contained in:
@@ -34,6 +34,8 @@ public sealed class ApiMethodCache
|
||||
private sealed record Entry(ApiMethod? Method, DateTimeOffset ExpiresAtUtc);
|
||||
|
||||
private readonly ConcurrentDictionary<string, Entry> _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;
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// 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
|
||||
/// <c>EndpointExtensions</c> 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
|
||||
/// <c>ZB.MOM.WW.Auth.ApiKeys</c>; pinned by <c>EndpointExtensionsTests</c>). Should a
|
||||
/// wildcard scope ever be added, every probed path would otherwise be a permanent
|
||||
/// negative entry — this prune is the safety net.
|
||||
/// </summary>
|
||||
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();
|
||||
}
|
||||
|
||||
/// <summary>Drops the cached entry for one method name. Safe to call for an unknown name.</summary>
|
||||
/// <param name="methodName">The method name to invalidate.</param>
|
||||
public void Invalidate(string methodName) => _entries.TryRemove(methodName, out _);
|
||||
|
||||
@@ -122,4 +122,24 @@ public class ApiMethodCacheTests
|
||||
/// <inheritdoc />
|
||||
public override DateTimeOffset GetUtcNow() => _now;
|
||||
}
|
||||
|
||||
/// <summary>Verifies that expired entries are pruned on a miss once the cache is large.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[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<ApiMethod?> Fetch(CancellationToken _) => Task.FromResult<ApiMethod?>(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);
|
||||
}
|
||||
}
|
||||
|
||||
@@ -327,6 +327,27 @@ public sealed class EndpointExtensionsTests : IDisposable
|
||||
Assert.Equal(NotApprovedBodyJson, body);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that scopes are exact method names, not prefixes or wildcards.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[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);
|
||||
}
|
||||
|
||||
/// <summary>Verifies that an unknown method returns 403 with a body identical to the not-in-scope body.</summary>
|
||||
/// <returns>A task that represents the asynchronous operation.</returns>
|
||||
[Fact]
|
||||
|
||||
Reference in New Issue
Block a user