diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/ReferenceAuditor.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/ReferenceAuditor.cs index a11428e..e99e311 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/ReferenceAuditor.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/ReferenceAuditor.cs @@ -24,6 +24,12 @@ public enum ReferenceStatus /// The row exists but does not decrypt under the session's KEK (wrong or rotated KEK, or tampering). Undecryptable, + + /// + /// The token names a value that is not a valid (illegal characters, an + /// unfilled placeholder like REPLACE ME, and so on), so it cannot be looked up at all. + /// + InvalidName, } /// @@ -45,55 +51,99 @@ public sealed class ReferenceAuditor /// /// Walks every configuration value, collects each distinct ${secret:NAME} reference with the - /// config key paths that use it, and classifies each name against the session's store. Findings are - /// ordered by name (ordinal); each finding's config paths are likewise sorted and de-duplicated. + /// config key paths that use it, and classifies each name against the session's store. References are + /// keyed by their normalized so case/whitespace variants of one + /// secret collapse to a single finding whose name matches what a get would resolve; a token that + /// is not a valid name is reported once as under its raw text + /// rather than aborting the whole audit. Findings are ordered by name (ordinal); each finding's config + /// paths are likewise sorted and de-duplicated. /// /// The open store session whose target configuration is audited. /// A token to cancel the store lookups. - /// One finding per distinct referenced secret name. + /// One finding per distinct referenced secret. public async Task> AuditAsync(SecretsSession session, CancellationToken ct) { ArgumentNullException.ThrowIfNull(session); - // name -> ordered, de-duplicated set of config key paths. SortedDictionary keeps findings ordered - // by name; each SortedSet keeps that name's paths deterministic. - var pathsByName = new SortedDictionary>(StringComparer.Ordinal); + // group key -> collected reference. The key is the normalized SecretName.Value for a valid name + // (so variants collapse), or the raw trimmed token text for an invalid one. SortedDictionary keeps + // findings ordered by name; each group's SortedSet keeps its paths deterministic. + var groups = new SortedDictionary(StringComparer.Ordinal); foreach (KeyValuePair entry in session.Target.Configuration.AsEnumerable()) { - if (entry.Value is not { } value) + // Mirror SecretReferenceExpander.IsCommentKey: an entry whose leaf key segment starts with '_' + // (the "_comment" convention) may legitimately document the ${secret:...} syntax with a literal + // example token and is never resolved — so the auditor must not report it either. + if (entry.Value is not { } value || IsCommentKey(entry.Key)) { continue; } foreach (Match match in TokenPattern.Matches(value)) { - string name = match.Groups[1].Value.Trim(); - if (!pathsByName.TryGetValue(name, out SortedSet? paths)) - { - paths = new SortedSet(StringComparer.Ordinal); - pathsByName[name] = paths; - } - - paths.Add(entry.Key); + string raw = match.Groups[1].Value.Trim(); + ReferenceGroup group = ResolveGroup(groups, raw); + group.Paths.Add(entry.Key); } } - var findings = new List(pathsByName.Count); - foreach ((string name, SortedSet paths) in pathsByName) + var findings = new List(groups.Count); + foreach ((string key, ReferenceGroup group) in groups) { - ReferenceStatus status = await ClassifyAsync(session, name, ct).ConfigureAwait(false); - findings.Add(new ReferenceFinding(name, status, [.. paths])); + ReferenceStatus status = group.Name is { } name + ? await ClassifyAsync(session, name, ct).ConfigureAwait(false) + : ReferenceStatus.InvalidName; + findings.Add(new ReferenceFinding(key, status, [.. group.Paths])); } return findings; } + // Finds (or creates) the group a raw token belongs to. A valid name groups under its normalized value + // and carries the parsed SecretName; an invalid token (illegal chars, an unfilled placeholder, ...) + // groups under its raw text with no name, so one bad token becomes one InvalidName finding instead of + // throwing and aborting the entire audit. + private static ReferenceGroup ResolveGroup( + SortedDictionary groups, string raw) + { + string key; + SecretName? name; + try + { + var parsed = new SecretName(raw); + key = parsed.Value; + name = parsed; + } + catch (ArgumentException) + { + key = raw; + name = null; + } + + if (!groups.TryGetValue(key, out ReferenceGroup? group)) + { + group = new ReferenceGroup(name); + groups[key] = group; + } + + return group; + } + + // A configuration key is a comment/documentation key when its leaf segment (after the last ':') starts + // with '_'. Duplicated from SecretReferenceExpander.IsCommentKey — keep the two in lockstep. + private static bool IsCommentKey(string key) + { + int lastSeparator = key.LastIndexOf(':'); + string leaf = lastSeparator >= 0 ? key[(lastSeparator + 1)..] : key; + return leaf.StartsWith('_'); + } + // Present ⇒ tombstone-check ⇒ (degraded ⇒ presence-only) ⇒ decrypt-probe. The decrypted plaintext is // discarded immediately (never stored in a finding); only the success/failure signal is kept. private static async Task ClassifyAsync( - SecretsSession session, string name, CancellationToken ct) + SecretsSession session, SecretName name, CancellationToken ct) { - StoredSecret? row = await session.Store.GetAsync(new SecretName(name), ct).ConfigureAwait(false); + StoredSecret? row = await session.Store.GetAsync(name, ct).ConfigureAwait(false); if (row is null) { return ReferenceStatus.Missing; @@ -119,4 +169,13 @@ public sealed class ReferenceAuditor return ReferenceStatus.Undecryptable; } } + + // A collated reference: the config key paths that use it, plus the parsed name (null when the token is + // not a valid SecretName — an InvalidName finding). + private sealed class ReferenceGroup(SecretName? name) + { + public SortedSet Paths { get; } = new(StringComparer.Ordinal); + + public SecretName? Name { get; } = name; + } } diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/ReferenceAuditorTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/ReferenceAuditorTests.cs index 6542255..a3d2eac 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/ReferenceAuditorTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/ReferenceAuditorTests.cs @@ -157,6 +157,50 @@ public sealed class ReferenceAuditorTests : IDisposable Assert.Equal(ReferenceStatus.Undecryptable, Assert.Single(findings).Status); } + [Fact] + public async Task Malformed_reference_yields_InvalidName_finding_without_aborting_audit() + { + SecretsSession session = await Open(EnvKey(), Config( + ("Bad:Ref", "${secret:REPLACE ME!}"), + ("Good:Ref", "${secret:api/key}"))); + + IReadOnlyList findings = + await new ReferenceAuditor().AuditAsync(session, CancellationToken.None); + + Assert.Equal(2, findings.Count); + Assert.Equal(ReferenceStatus.InvalidName, findings.Single(f => f.SecretName == "REPLACE ME!").Status); + Assert.Equal(ReferenceStatus.Missing, findings.Single(f => f.SecretName == "api/key").Status); + } + + [Fact] + public async Task Comment_keys_are_skipped_like_the_expander() + { + SecretsSession session = await Open(EnvKey(), Config( + ("Docs:_comment", "Reference a secret like ${secret:example/thing}"), + ("Real:Key", "${secret:api/key}"))); + + IReadOnlyList findings = + await new ReferenceAuditor().AuditAsync(session, CancellationToken.None); + + ReferenceFinding finding = Assert.Single(findings); + Assert.Equal("api/key", finding.SecretName); + } + + [Fact] + public async Task Case_variant_references_collapse_to_one_normalized_finding() + { + SecretsSession session = await Open(EnvKey(), Config( + ("A:Conn", "${secret:Db/Pw}"), + ("B:Conn", "${secret:db/pw}"))); + + IReadOnlyList findings = + await new ReferenceAuditor().AuditAsync(session, CancellationToken.None); + + ReferenceFinding finding = Assert.Single(findings); + Assert.Equal("db/pw", finding.SecretName); + Assert.Equal(["A:Conn", "B:Conn"], finding.ConfigPaths); + } + [Fact] public async Task Degraded_session_reports_PresenceOnly_status() {