From 79ebd14baa782ac13ee726df59de807b803d683c Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Wed, 15 Jul 2026 16:55:24 -0400 Subject: [PATCH] feat(secrets): default resolver with TTL cache + audit (never logs plaintext) --- .../ISecretActorAccessor.cs | 17 ++ .../DefaultSecretResolver.cs | 140 ++++++++++++++++ .../DefaultSecretResolverTests.cs | 158 ++++++++++++++++++ .../Fakes/CapturingAuditWriter.cs | 24 +++ .../Fakes/CountingSecretStore.cs | 51 ++++++ .../Fakes/FixedActorAccessor.cs | 16 ++ .../Fakes/MutableTimeProvider.cs | 29 ++++ 7 files changed, 435 insertions(+) create mode 100644 ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Abstractions/ISecretActorAccessor.cs create mode 100644 ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/DefaultSecretResolver.cs create mode 100644 ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/DefaultSecretResolverTests.cs create mode 100644 ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CapturingAuditWriter.cs create mode 100644 ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CountingSecretStore.cs create mode 100644 ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/FixedActorAccessor.cs create mode 100644 ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/MutableTimeProvider.cs diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Abstractions/ISecretActorAccessor.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Abstractions/ISecretActorAccessor.cs new file mode 100644 index 0000000..9f8f1dc --- /dev/null +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Abstractions/ISecretActorAccessor.cs @@ -0,0 +1,17 @@ +namespace ZB.MOM.WW.Secrets.Abstractions; + +/// +/// Resolves the current principal (the ZB.MOM.WW.Auth identity) for audit attribution when a +/// secret is resolved. A consumer wires this to whatever surfaces the ambient caller — an HTTP +/// context accessor, an actor context, a CLI identity, etc. A null +/// (or no accessor at all) is treated as the "system" actor by the resolver, so keyless / +/// background resolutions are still attributed rather than left blank. +/// +public interface ISecretActorAccessor +{ + /// + /// The current principal to record as the audit Actor, or null when no + /// principal is available (treated as "system"). + /// + string? CurrentActor { get; } +} 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 new file mode 100644 index 0000000..5325258 --- /dev/null +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/DefaultSecretResolver.cs @@ -0,0 +1,140 @@ +using System.Collections.Concurrent; +using ZB.MOM.WW.Audit; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.Secrets; + +/// +/// The default : loads an encrypted row from the store, decrypts it +/// on demand, and serves it from a short-lived in-memory cache. Every resolution — hit, miss, or +/// tombstone — emits an recording which secret was accessed and the +/// outcome. The plaintext value is never placed in any audit field. +/// +/// +/// +/// Security note — the cache holds decrypted secret material in process memory. The TTL is +/// deliberately kept short (configured by the caller / DI) so plaintext lives only briefly and a +/// rotation or revocation is picked up quickly. lets the write path evict +/// an entry immediately after a rotate or delete. The cache is keyed by the normalized +/// and is safe for concurrent use. +/// +/// +/// 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. +/// +/// +public sealed class DefaultSecretResolver : ISecretResolver +{ + private const string SystemActor = "system"; + private const string AuditCategory = "Secrets"; + private const string ResolveAction = "secret.resolve"; + private const string NotFoundAction = "secret.resolve.not-found"; + + private readonly ISecretStore _store; + private readonly ISecretCipher _cipher; + private readonly IAuditWriter _audit; + private readonly TimeSpan _cacheTtl; + private readonly ISecretActorAccessor? _actorAccessor; + private readonly TimeProvider _timeProvider; + + // Keyed by SecretName.Value. Holds DECRYPTED plaintext — kept only for the short TTL. + private readonly ConcurrentDictionary _cache = new(StringComparer.Ordinal); + + /// + /// Creates a resolver over the given store, cipher, and audit writer. + /// + /// The encrypted-row store to read from. + /// The cipher used to decrypt a loaded row. + /// The audit sink; one event is written per resolve (never the value). + /// + /// How long a decrypted value is cached in memory. Keep this short — it is plaintext secret + /// material. A non-positive value effectively disables caching (every entry is already expired). + /// + /// + /// Optional accessor for the current principal to attribute the audit to; a null accessor + /// (or a null ) is recorded as + /// "system". + /// + /// The clock (defaults to ). + public DefaultSecretResolver( + ISecretStore store, + ISecretCipher cipher, + IAuditWriter audit, + TimeSpan cacheTtl, + ISecretActorAccessor? actorAccessor = null, + TimeProvider? timeProvider = null) + { + _store = store ?? throw new ArgumentNullException(nameof(store)); + _cipher = cipher ?? throw new ArgumentNullException(nameof(cipher)); + _audit = audit ?? throw new ArgumentNullException(nameof(audit)); + _cacheTtl = cacheTtl; + _actorAccessor = actorAccessor; + _timeProvider = timeProvider ?? TimeProvider.System; + } + + /// + public async Task GetAsync(SecretName name, CancellationToken ct) + { + string key = name.Value; + DateTimeOffset now = _timeProvider.GetUtcNow(); + + // 1) Serve from cache while the entry is still live. + if (_cache.TryGetValue(key, out CacheEntry cached) && now < cached.ExpiresUtc) + { + await AuditSuccessAsync(name, source: "cache", ct).ConfigureAwait(false); + return cached.Plaintext; + } + + // 2) Load the encrypted row. + StoredSecret? row = await _store.GetAsync(name, ct).ConfigureAwait(false); + if (row is null || row.IsDeleted) + { + await AuditNotFoundAsync(name, ct).ConfigureAwait(false); + return null; + } + + // Decrypt on demand and cache the plaintext for the short TTL. + string plaintext = _cipher.Decrypt(row); + _cache[key] = new CacheEntry(plaintext, now + _cacheTtl, row.Revision); + + await AuditSuccessAsync(name, source: "store", ct).ConfigureAwait(false); + return plaintext; + } + + /// + /// Evicts the cached plaintext for , if any. The admin / write path + /// calls this immediately after a rotate or delete so a stale value is not served for the + /// remainder of its TTL. Not part of ; cheap and idempotent. + /// + /// The secret whose cache entry to remove. + public void Invalidate(SecretName name) => _cache.TryRemove(name.Value, out _); + + private Task AuditSuccessAsync(SecretName name, string source, CancellationToken ct) => + WriteAuditAsync(name, ResolveAction, AuditOutcome.Success, $"{{\"source\":\"{source}\"}}", ct); + + private Task AuditNotFoundAsync(SecretName name, CancellationToken ct) => + WriteAuditAsync(name, NotFoundAction, AuditOutcome.Failure, "{\"reason\":\"not-found\"}", ct); + + private Task WriteAuditAsync( + SecretName name, string action, AuditOutcome outcome, string detailsJson, CancellationToken ct) + { + // NOTE: only the secret NAME and the outcome are recorded — never the decrypted value. + var evt = new AuditEvent + { + EventId = Guid.NewGuid(), + OccurredAtUtc = _timeProvider.GetUtcNow(), + Actor = _actorAccessor?.CurrentActor ?? SystemActor, + Action = action, + Outcome = outcome, + Category = AuditCategory, + Target = name.Value, + DetailsJson = detailsJson, + }; + + return _audit.WriteAsync(evt, ct); + } + + /// A cached decrypted secret and its expiry (revision kept for future invalidation). + private readonly record struct CacheEntry(string Plaintext, DateTimeOffset ExpiresUtc, long Revision); +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/DefaultSecretResolverTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/DefaultSecretResolverTests.cs new file mode 100644 index 0000000..e7202c5 --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/DefaultSecretResolverTests.cs @@ -0,0 +1,158 @@ +using System.Text.Json; +using ZB.MOM.WW.Audit; +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; + +public class DefaultSecretResolverTests +{ + private static readonly TimeSpan Ttl = TimeSpan.FromSeconds(30); + + /// + /// Builds a real encrypted row for using the real cipher, then + /// stamps a revision + timestamps (the cipher leaves those at defaults) and optionally tombstones + /// it, so tests exercise the true decrypt path rather than a fake. + /// + private static StoredSecret MakeRow( + AesGcmEnvelopeCipher cipher, SecretName name, string plaintext, bool deleted = false) + { + StoredSecret row = cipher.Encrypt(name, plaintext, SecretContentType.Text); + return row with + { + Revision = 1, + CreatedUtc = DateTimeOffset.UnixEpoch, + UpdatedUtc = DateTimeOffset.UnixEpoch, + IsDeleted = deleted, + DeletedUtc = deleted ? DateTimeOffset.UnixEpoch : null, + }; + } + + private static (DefaultSecretResolver resolver, CountingSecretStore store, CapturingAuditWriter audit, + AesGcmEnvelopeCipher cipher, MutableTimeProvider clock) NewSut(ISecretActorAccessor? actor = null) + { + var cipher = new AesGcmEnvelopeCipher(new FakeMasterKeyProvider("k1")); + var store = new CountingSecretStore(); + var audit = new CapturingAuditWriter(); + var clock = new MutableTimeProvider(DateTimeOffset.UnixEpoch); + var resolver = new DefaultSecretResolver(store, cipher, audit, Ttl, actor, clock); + return (resolver, store, audit, cipher, clock); + } + + [Fact] + public async Task Get_ReturnsDecryptedPlaintext_AndAuditsSuccess() + { + var (resolver, store, audit, cipher, _) = NewSut(); + var name = new SecretName("sql/foo"); + store.Seed(MakeRow(cipher, name, "hunter2")); + + string? value = await resolver.GetAsync(name, CancellationToken.None); + + Assert.Equal("hunter2", value); + AuditEvent evt = Assert.Single(audit.Events); + Assert.Equal(AuditOutcome.Success, evt.Outcome); + Assert.Equal("sql/foo", evt.Target); + + // Hard guarantee: the plaintext must NEVER appear in any serialized audit field. + foreach (AuditEvent captured in audit.Events) + { + string json = JsonSerializer.Serialize(captured); + Assert.DoesNotContain("hunter2", json, StringComparison.Ordinal); + } + } + + [Fact] + public async Task Get_CachesWithinTtl() + { + var (resolver, store, _, cipher, _) = NewSut(); + var name = new SecretName("sql/foo"); + store.Seed(MakeRow(cipher, name, "hunter2")); + + string? first = await resolver.GetAsync(name, CancellationToken.None); + string? second = await resolver.GetAsync(name, CancellationToken.None); + + Assert.Equal("hunter2", first); + Assert.Equal("hunter2", second); + Assert.Equal(1, store.GetCount); // second call served from the TTL cache + } + + [Fact] + public async Task Get_ReloadsAfterTtlExpiry() + { + var (resolver, store, _, cipher, clock) = NewSut(); + var name = new SecretName("sql/foo"); + store.Seed(MakeRow(cipher, name, "hunter2")); + + await resolver.GetAsync(name, CancellationToken.None); + clock.Advance(Ttl + TimeSpan.FromSeconds(1)); // past the TTL + await resolver.GetAsync(name, CancellationToken.None); + + Assert.Equal(2, store.GetCount); + } + + [Fact] + public async Task Get_MissingSecret_ReturnsNull_AuditsFailure() + { + var (resolver, _, audit, _, _) = NewSut(); + + string? value = await resolver.GetAsync(new SecretName("sql/absent"), CancellationToken.None); + + Assert.Null(value); + AuditEvent evt = Assert.Single(audit.Events); + Assert.Equal(AuditOutcome.Failure, evt.Outcome); + Assert.Contains("not-found", evt.Action, StringComparison.Ordinal); + } + + [Fact] + public async Task Get_TombstonedSecret_ReturnsNull() + { + var (resolver, store, audit, cipher, _) = NewSut(); + var name = new SecretName("sql/dead"); + store.Seed(MakeRow(cipher, name, "hunter2", deleted: true)); + + string? value = await resolver.GetAsync(name, CancellationToken.None); + + Assert.Null(value); + AuditEvent evt = Assert.Single(audit.Events); + Assert.Equal(AuditOutcome.Failure, evt.Outcome); + } + + [Fact] + public async Task Invalidate_RemovesCacheEntry() + { + var (resolver, store, _, cipher, _) = NewSut(); + var name = new SecretName("sql/foo"); + store.Seed(MakeRow(cipher, name, "hunter2")); + + await resolver.GetAsync(name, CancellationToken.None); // caches (count 1) + resolver.Invalidate(name); + await resolver.GetAsync(name, CancellationToken.None); // cache gone → reload (count 2) + + Assert.Equal(2, store.GetCount); + } + + [Fact] + public async Task Get_DefaultActorIsSystem_WhenNoAccessor() + { + var (resolver, store, audit, cipher, _) = NewSut(actor: null); + var name = new SecretName("sql/foo"); + store.Seed(MakeRow(cipher, name, "hunter2")); + + await resolver.GetAsync(name, CancellationToken.None); + + Assert.Equal("system", Assert.Single(audit.Events).Actor); + } + + [Fact] + public async Task Get_UsesActorFromAccessor() + { + var (resolver, store, audit, cipher, _) = NewSut(new FixedActorAccessor("alice")); + var name = new SecretName("sql/foo"); + store.Seed(MakeRow(cipher, name, "hunter2")); + + await resolver.GetAsync(name, CancellationToken.None); + + Assert.Equal("alice", Assert.Single(audit.Events).Actor); + } +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CapturingAuditWriter.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CapturingAuditWriter.cs new file mode 100644 index 0000000..0da2d7d --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CapturingAuditWriter.cs @@ -0,0 +1,24 @@ +using System.Collections.Concurrent; +using ZB.MOM.WW.Audit; + +namespace ZB.MOM.WW.Secrets.Tests.Fakes; + +/// +/// An test double that collects every written, +/// so a test can assert on the audit trail (outcome, target, and — critically — that no plaintext +/// ever appears in any field). +/// +public sealed class CapturingAuditWriter : IAuditWriter +{ + private readonly ConcurrentQueue _events = new(); + + /// The events captured so far, in write order. + public IReadOnlyList Events => _events.ToArray(); + + /// + public Task WriteAsync(AuditEvent evt, CancellationToken ct = default) + { + _events.Enqueue(evt); + return Task.CompletedTask; + } +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CountingSecretStore.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CountingSecretStore.cs new file mode 100644 index 0000000..1ffd006 --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/CountingSecretStore.cs @@ -0,0 +1,51 @@ +using System.Collections.Concurrent; +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.Secrets.Tests.Fakes; + +/// +/// An in-memory test double that counts calls so +/// a test can prove a resolver cache hit (or miss). Only the read path exercised by +/// DefaultSecretResolver is meaningfully implemented; the mutation / replication members +/// throw so an accidental dependency on them fails loudly rather than silently no-ops. +/// +public sealed class CountingSecretStore : ISecretStore +{ + private readonly ConcurrentDictionary _rows = new(StringComparer.Ordinal); + private int _getCount; + + /// The number of times has been invoked. + public int GetCount => Volatile.Read(ref _getCount); + + /// Seeds (or overwrites) a row keyed by its normalized name. + /// The encrypted row to seed. + public void Seed(StoredSecret row) => _rows[row.Name.Value] = row; + + /// + public Task GetAsync(SecretName name, CancellationToken ct) + { + Interlocked.Increment(ref _getCount); + _rows.TryGetValue(name.Value, out StoredSecret? row); + return Task.FromResult(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(); +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/FixedActorAccessor.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/FixedActorAccessor.cs new file mode 100644 index 0000000..15b1f21 --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/FixedActorAccessor.cs @@ -0,0 +1,16 @@ +using ZB.MOM.WW.Secrets.Abstractions; + +namespace ZB.MOM.WW.Secrets.Tests.Fakes; + +/// +/// An test double that reports a fixed (or null) actor. +/// +public sealed class FixedActorAccessor : ISecretActorAccessor +{ + /// Creates an accessor that always reports . + /// The actor to report, or null. + public FixedActorAccessor(string? actor) => CurrentActor = actor; + + /// + public string? CurrentActor { get; } +} diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/MutableTimeProvider.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/MutableTimeProvider.cs new file mode 100644 index 0000000..592e60d --- /dev/null +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Fakes/MutableTimeProvider.cs @@ -0,0 +1,29 @@ +namespace ZB.MOM.WW.Secrets.Tests.Fakes; + +/// +/// A test double with a settable , so a test can +/// advance the clock past a cache TTL deterministically without sleeping. +/// +public sealed class MutableTimeProvider : TimeProvider +{ + private DateTimeOffset _utcNow; + + /// Creates a provider starting at the given instant (defaults to the Unix epoch). + /// The initial value. + public MutableTimeProvider(DateTimeOffset? start = null) => + _utcNow = start ?? DateTimeOffset.UnixEpoch; + + /// Gets or sets the current UTC instant this provider reports. + public DateTimeOffset UtcNow + { + get => _utcNow; + set => _utcNow = value; + } + + /// Moves the clock forward by . + /// The amount to advance. + public void Advance(TimeSpan delta) => _utcNow += delta; + + /// + public override DateTimeOffset GetUtcNow() => _utcNow; +}