diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/DefaultSecretResolver.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/DefaultSecretResolver.cs
index 7152b91..1cfb44e 100644
--- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/DefaultSecretResolver.cs
+++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/DefaultSecretResolver.cs
@@ -1,4 +1,5 @@
using System.Collections.Concurrent;
+using System.Runtime.CompilerServices;
using ZB.MOM.WW.Audit;
using ZB.MOM.WW.Secrets.Abstractions;
@@ -20,6 +21,14 @@ namespace ZB.MOM.WW.Secrets;
/// and is safe for concurrent use.
///
///
+/// A resolve that was already reading the store when ran never re-caches
+/// what it read: each key carries an invalidation generation, and an entry inserted under a stale
+/// generation is withdrawn. Expired entries are removed when they are next looked up and by a
+/// periodic sweep (every TTL, at least once a second), so plaintext resolved once — at startup
+/// expansion, say — does not stay referenced for the life of the process. A non-positive TTL caches
+/// nothing. The resolver is only to stop that sweep timer.
+///
+///
/// A missing or tombstoned secret is not an exception: returns
/// null and audits a with a not-found action, so a
/// consumer can distinguish "absent" from "present" without a throw on the hot path.
@@ -32,7 +41,7 @@ namespace ZB.MOM.WW.Secrets;
/// ciphertext and any value are never included.
///
///
-public sealed class DefaultSecretResolver : ISecretResolver, ISecretCacheInvalidator
+public sealed class DefaultSecretResolver : ISecretResolver, ISecretCacheInvalidator, IDisposable
{
private const string SystemActor = "system";
private const string AuditCategory = "Secrets";
@@ -50,6 +59,14 @@ public sealed class DefaultSecretResolver : ISecretResolver, ISecretCacheInvalid
// Keyed by SecretName.Value. Holds DECRYPTED plaintext — kept only for the short TTL.
private readonly ConcurrentDictionary _cache = new(StringComparer.Ordinal);
+ // Per-key invalidation generation; bumped by Invalidate before it evicts. Grows with the set of
+ // names ever invalidated (bounded by the store), holds no secret material.
+ private readonly ConcurrentDictionary> _generations = new(StringComparer.Ordinal);
+
+ private readonly object _timerGate = new();
+ private ITimer? _sweepTimer;
+ private bool _disposed;
+
///
/// Creates a resolver over the given store, cipher, and audit writer.
///
@@ -89,12 +106,20 @@ public sealed class DefaultSecretResolver : ISecretResolver, ISecretCacheInvalid
DateTimeOffset now = _timeProvider.GetUtcNow();
// 1) Serve from cache while the entry is still live.
- if (_cache.TryGetValue(key, out CacheEntry cached) && now < cached.ExpiresUtc)
+ if (_cache.TryGetValue(key, out CacheEntry cached))
{
- await AuditSuccessAsync(name, source: "cache", ct).ConfigureAwait(false);
- return cached.Plaintext;
+ if (now < cached.ExpiresUtc)
+ {
+ await AuditSuccessAsync(name, source: "cache", ct).ConfigureAwait(false);
+ return cached.Plaintext;
+ }
+
+ // Expired: drop it now (only this exact entry — a concurrent fresh insert survives).
+ _cache.TryRemove(new KeyValuePair(key, cached));
}
+ long generation = Volatile.Read(ref Generation(key).Value);
+
// 2) Load the encrypted row.
StoredSecret? row = await _store.GetAsync(name, ct).ConfigureAwait(false);
if (row is null || row.IsDeleted)
@@ -117,7 +142,23 @@ public sealed class DefaultSecretResolver : ISecretResolver, ISecretCacheInvalid
throw;
}
- _cache[key] = new CacheEntry(plaintext, now + _cacheTtl, row.Revision);
+ if (_cacheTtl > TimeSpan.Zero)
+ {
+ var entry = new CacheEntry(plaintext, now + _cacheTtl, row.Revision);
+ _cache[key] = entry;
+
+ // Invalidate bumps the generation BEFORE it evicts, so either its eviction runs after
+ // this insert, or this check sees the new generation: a value read before an
+ // invalidation is never left in the cache after it.
+ if (Volatile.Read(ref Generation(key).Value) != generation)
+ {
+ _cache.TryRemove(new KeyValuePair(key, entry));
+ }
+ else
+ {
+ EnsureSweepTimer();
+ }
+ }
await AuditSuccessAsync(name, source: "store", ct).ConfigureAwait(false);
return plaintext;
@@ -129,7 +170,61 @@ public sealed class DefaultSecretResolver : ISecretResolver, ISecretCacheInvalid
/// so a stale value is not served for the remainder of its TTL. Cheap and idempotent.
///
/// The secret whose cache entry to remove.
- public void Invalidate(SecretName name) => _cache.TryRemove(name.Value, out _);
+ public void Invalidate(SecretName name)
+ {
+ Interlocked.Increment(ref Generation(name.Value).Value);
+ _cache.TryRemove(name.Value, out _);
+ }
+
+ /// Stops the periodic expired-entry sweep. Idempotent.
+ public void Dispose()
+ {
+ lock (_timerGate)
+ {
+ _disposed = true;
+ _sweepTimer?.Dispose();
+ _sweepTimer = null;
+ }
+ }
+
+ /// Removes every expired entry. Run by the sweep timer; internal for tests.
+ internal void SweepExpired()
+ {
+ DateTimeOffset now = _timeProvider.GetUtcNow();
+ foreach (KeyValuePair pair in _cache)
+ {
+ if (now >= pair.Value.ExpiresUtc)
+ {
+ _cache.TryRemove(pair);
+ }
+ }
+ }
+
+ /// The number of cached entries (expired or not); internal for tests.
+ internal int CachedCount => _cache.Count;
+
+ private StrongBox Generation(string key) =>
+ _generations.GetOrAdd(key, static _ => new StrongBox(0));
+
+ private void EnsureSweepTimer()
+ {
+ if (Volatile.Read(ref _sweepTimer) is not null)
+ {
+ return;
+ }
+
+ lock (_timerGate)
+ {
+ if (_sweepTimer is not null || _disposed)
+ {
+ return;
+ }
+
+ TimeSpan period = _cacheTtl < TimeSpan.FromSeconds(1) ? TimeSpan.FromSeconds(1) : _cacheTtl;
+ _sweepTimer = _timeProvider.CreateTimer(
+ static state => ((DefaultSecretResolver)state!).SweepExpired(), this, period, period);
+ }
+ }
private Task AuditSuccessAsync(SecretName name, string source, CancellationToken ct) =>
WriteAuditAsync(name, ResolveAction, AuditOutcome.Success, $"{{\"source\":\"{source}\"}}", ct);
diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/DefaultSecretResolverCacheTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/DefaultSecretResolverCacheTests.cs
new file mode 100644
index 0000000..3056b6a
--- /dev/null
+++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/DefaultSecretResolverCacheTests.cs
@@ -0,0 +1,174 @@
+using ZB.MOM.WW.Secrets.Abstractions;
+using ZB.MOM.WW.Secrets.Crypto;
+using ZB.MOM.WW.Secrets.Tests.Fakes;
+
+namespace ZB.MOM.WW.Secrets.Tests;
+
+///
+/// Secrets-006: an that lands while a resolve is
+/// reading the store must not be undone by that resolve re-caching the old value, and expired
+/// plaintext must not stay referenced for the life of the process.
+///
+public class DefaultSecretResolverCacheTests
+{
+ private static readonly TimeSpan Ttl = TimeSpan.FromSeconds(30);
+
+ private static StoredSecret Row(AesGcmEnvelopeCipher cipher, SecretName name, string value, long revision) =>
+ cipher.Encrypt(name, value, SecretContentType.Text) with
+ {
+ Revision = revision,
+ CreatedUtc = DateTimeOffset.UnixEpoch,
+ UpdatedUtc = DateTimeOffset.UnixEpoch,
+ };
+
+ /// A store whose next read is held until the test releases it.
+ private sealed class GatedStore : ISecretStore
+ {
+ private readonly CountingSecretStore _inner = new();
+
+ public TaskCompletionSource ReadStarted { get; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+ public TaskCompletionSource Release { get; private set; } = new(TaskCreationOptions.RunContinuationsAsynchronously);
+
+ public bool Gate { get; set; }
+
+ public int Reads => _inner.GetCount;
+
+ public void Seed(StoredSecret row) => _inner.Seed(row);
+
+ public async Task GetAsync(SecretName name, CancellationToken ct)
+ {
+ StoredSecret? row = await _inner.GetAsync(name, ct);
+ if (Gate)
+ {
+ Gate = false;
+ ReadStarted.TrySetResult();
+ await Release.Task;
+ }
+
+ return row;
+ }
+
+ public Task UpsertAsync(StoredSecret row, CancellationToken ct) => throw new NotSupportedException();
+ public Task DeleteAsync(SecretName name, string? actor, CancellationToken ct) => throw new NotSupportedException();
+ public Task> ListAsync(bool includeDeleted, CancellationToken ct) => throw new NotSupportedException();
+ public Task> GetManifestAsync(CancellationToken ct) => throw new NotSupportedException();
+ public Task ApplyReplicatedAsync(StoredSecret row, CancellationToken ct) => throw new NotSupportedException();
+ public Task ApplyRewrapAsync(StoredSecret rewrappedRow, byte[] expectedCurrentWrappedDek, CancellationToken ct) => throw new NotSupportedException();
+ }
+
+ /// A clock whose timers fire only when the test says so.
+ private sealed class ManualTimerTimeProvider : TimeProvider
+ {
+ public DateTimeOffset Now { get; set; } = DateTimeOffset.UnixEpoch;
+
+ public List<(TimerCallback Callback, object? State)> Timers { get; } = new();
+
+ public override DateTimeOffset GetUtcNow() => Now;
+
+ public override ITimer CreateTimer(TimerCallback callback, object? state, TimeSpan dueTime, TimeSpan period)
+ {
+ Timers.Add((callback, state));
+ return new NullTimer();
+ }
+
+ public void FireAll()
+ {
+ foreach ((TimerCallback callback, object? state) in Timers)
+ {
+ callback(state);
+ }
+ }
+
+ private sealed class NullTimer : ITimer
+ {
+ public bool Change(TimeSpan dueTime, TimeSpan period) => true;
+ public void Dispose() { }
+ public ValueTask DisposeAsync() => ValueTask.CompletedTask;
+ }
+ }
+
+ [Fact]
+ public async Task An_invalidate_during_an_in_flight_resolve_is_not_undone_by_it()
+ {
+ var cipher = new AesGcmEnvelopeCipher(new FakeMasterKeyProvider("k1"));
+ var store = new GatedStore();
+ var name = new SecretName("db/central");
+ store.Seed(Row(cipher, name, "old-password", revision: 3));
+ using var resolver = new DefaultSecretResolver(store, cipher, new CapturingAuditWriter(), Ttl);
+
+ // A request thread misses the cache and reads revision 3 ...
+ store.Gate = true;
+ Task inFlight = resolver.GetAsync(name, CancellationToken.None);
+ await store.ReadStarted.Task;
+
+ // ... the operator rotates to revision 4 and the write path invalidates ...
+ store.Seed(Row(cipher, name, "new-password", revision: 4));
+ resolver.Invalidate(name);
+
+ // ... and only then does the request thread finish with the old value.
+ store.Release.TrySetResult();
+ Assert.Equal("old-password", await inFlight);
+
+ // The next resolve must go back to the store, not serve revision 3 for another TTL.
+ Assert.Equal("new-password", await resolver.GetAsync(name, CancellationToken.None));
+ Assert.Equal(2, store.Reads);
+ }
+
+ [Fact]
+ public async Task An_expired_entry_is_dropped_when_it_is_looked_up()
+ {
+ var cipher = new AesGcmEnvelopeCipher(new FakeMasterKeyProvider("k1"));
+ var store = new CountingSecretStore();
+ var clock = new MutableTimeProvider(DateTimeOffset.UnixEpoch);
+ var name = new SecretName("db/central");
+ store.Seed(Row(cipher, name, "hunter2", 1));
+ using var resolver = new DefaultSecretResolver(store, cipher, new CapturingAuditWriter(), Ttl, null, clock);
+
+ await resolver.GetAsync(name, CancellationToken.None);
+ Assert.Equal(1, resolver.CachedCount);
+
+ // The secret is deleted out-of-band; the expired entry must not outlive the lookup.
+ store.Seed(Row(cipher, name, "hunter2", 2) with { IsDeleted = true });
+ clock.Advance(Ttl + TimeSpan.FromSeconds(1));
+ Assert.Null(await resolver.GetAsync(name, CancellationToken.None));
+ Assert.Equal(0, resolver.CachedCount);
+ }
+
+ [Fact]
+ public async Task The_sweep_drops_expired_plaintext_that_is_never_looked_up_again()
+ {
+ var cipher = new AesGcmEnvelopeCipher(new FakeMasterKeyProvider("k1"));
+ var store = new CountingSecretStore();
+ var clock = new ManualTimerTimeProvider();
+ var name = new SecretName("db/central");
+ store.Seed(Row(cipher, name, "hunter2", 1));
+ using var resolver = new DefaultSecretResolver(store, cipher, new CapturingAuditWriter(), Ttl, null, clock);
+
+ await resolver.GetAsync(name, CancellationToken.None); // e.g. startup expansion
+ Assert.Single(clock.Timers);
+
+ clock.FireAll();
+ Assert.Equal(1, resolver.CachedCount); // positive control: not yet expired
+
+ clock.Now += Ttl;
+ clock.FireAll();
+ Assert.Equal(0, resolver.CachedCount);
+ }
+
+ [Fact]
+ public async Task A_non_positive_ttl_caches_nothing()
+ {
+ var cipher = new AesGcmEnvelopeCipher(new FakeMasterKeyProvider("k1"));
+ var store = new CountingSecretStore();
+ var name = new SecretName("db/central");
+ store.Seed(Row(cipher, name, "hunter2", 1));
+ using var resolver = new DefaultSecretResolver(store, cipher, new CapturingAuditWriter(), TimeSpan.Zero);
+
+ await resolver.GetAsync(name, CancellationToken.None);
+ await resolver.GetAsync(name, CancellationToken.None);
+
+ Assert.Equal(0, resolver.CachedCount);
+ Assert.Equal(2, store.GetCount);
+ }
+}