fix(secrets): malformed ${secret:} references fail closed; README sample validates (scadaproj#45)

Secrets-009: SecretReferenceExpander.ExpandAsync throws FormatException when a value holds a
${secret: marker that is not a complete reference with a non-blank name (${secret:}, an
unterminated ${secret:name, a nested marker), instead of passing the literal text through as the
credential. ExpandConfigurationAsync names the offending key (never the value) and writes nothing.
The README wiring sample drops the relative "SqlitePath": "secrets.db" (which failed validation)
for the per-user default and shows an absolute example; a test validates the sample.

Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV

ZB.MOM.WW.Secrets/README.md

@@ -84,7 +84,10 @@ await expander.ExpandConfigurationAsync((IConfigurationRoot)builder.Configuratio
 ```jsonc
 // appsettings.json
 "Secrets": {
-  "SqlitePath": "secrets.db",
+  // SqlitePath omitted: the per-user default (<LocalApplicationData>/ZB.MOM.WW/secrets.db) applies.
+  // A service sets an ABSOLUTE path OUTSIDE its install directory instead, e.g.
+  // "SqlitePath": "E:\\ZB\\data\\secrets.db" — a relative path, or one under the content root,
+  // fails startup validation.
   "MasterKey": { "Source": "Environment", "EnvVarName": "ZB_SECRETS_MASTER_KEY" },
   "RunMigrationsOnStartup": true,
   "ResolveCacheTtl": "00:00:30"

ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets/Configuration/SecretReferenceExpander.cs

@@ -10,7 +10,10 @@ namespace ZB.MOM.WW.Secrets.Configuration;
 /// referenced secret through an <see cref="ISecretResolver"/>. Expansion is <em>fail-closed</em>:
 /// a token that names an absent secret throws <see cref="SecretNotFoundException"/> rather than
 /// leaving the placeholder (or a blank) in place, so a misconfigured deployment fails loudly at
-/// startup instead of silently running with an empty credential.
+/// startup instead of silently running with an empty credential. A <b>malformed</b> reference — an
+/// empty name (<c>${secret:}</c>) or a missing closing brace (<c>${secret:name</c>) — is equally
+/// fail-closed: it throws <see cref="FormatException"/> instead of passing the literal text through as
+/// the credential.
 /// </summary>
 public sealed class SecretReferenceExpander
 {
@@ -37,6 +40,10 @@ public sealed class SecretReferenceExpander
     /// no tokens.
     /// </returns>
     /// <exception cref="SecretNotFoundException">A referenced secret does not exist (fail-closed).</exception>
+    /// <exception cref="FormatException">
+    /// <paramref name="raw"/> contains a <c>${secret:</c> marker that is not a complete reference with
+    /// a non-blank name (fail-closed). The message never includes the value.
+    /// </exception>
     public async Task<string> ExpandAsync(string raw, CancellationToken ct)
     {
         if (raw is null)
@@ -45,6 +52,20 @@ public sealed class SecretReferenceExpander
         }
 
         MatchCollection matches = TokenPattern.Matches(raw);
+
+        // Every marker must start a complete, named reference. "${secret:}" and an unterminated
+        // "${secret:name" do not match the pattern at all, and a marker swallowed inside another
+        // match ("${secret:a${secret:b}") is not one reference per marker — left alone, each would
+        // pass through verbatim and be used as the credential.
+        int markers = CountMarkers(raw);
+        if (markers != matches.Count
+            || matches.Any(m => string.IsNullOrWhiteSpace(m.Groups[1].Value)))
+        {
+            throw new FormatException(
+                "Malformed ${secret:...} reference: every ${secret: must be followed by a non-blank "
+                + "secret name and a closing '}'.");
+        }
+
         if (matches.Count == 0)
         {
             return raw;
@@ -122,6 +143,7 @@ public sealed class SecretReferenceExpander
     /// <param name="config">The loaded configuration root to expand.</param>
     /// <param name="ct">A token to cancel the operation.</param>
     /// <exception cref="SecretNotFoundException">A referenced secret does not exist (fail-closed).</exception>
+    /// <exception cref="FormatException">A value holds a malformed reference (fail-closed); names the key.</exception>
     public async Task ExpandConfigurationAsync(IConfigurationRoot config, CancellationToken ct)
     {
         ArgumentNullException.ThrowIfNull(config);
@@ -143,7 +165,15 @@ public sealed class SecretReferenceExpander
         var expanded = new Dictionary<string, string>(StringComparer.OrdinalIgnoreCase);
         foreach (KeyValuePair<string, string> entry in pending)
         {
-            expanded[entry.Key] = await ExpandAsync(entry.Value, ct).ConfigureAwait(false);
+            try
+            {
+                expanded[entry.Key] = await ExpandAsync(entry.Value, ct).ConfigureAwait(false);
+            }
+            catch (FormatException ex)
+            {
+                // Name the key (never the value) so the operator can find the typo.
+                throw new FormatException($"Configuration key '{entry.Key}': {ex.Message}", ex);
+            }
         }
 
         if (expanded.Count == 0)
@@ -161,6 +191,19 @@ public sealed class SecretReferenceExpander
         }
     }
 
+    private static int CountMarkers(string value)
+    {
+        int count = 0;
+        for (int at = value.IndexOf(TokenMarker, StringComparison.Ordinal);
+             at >= 0;
+             at = value.IndexOf(TokenMarker, at + TokenMarker.Length, StringComparison.Ordinal))
+        {
+            count++;
+        }
+
+        return count;
+    }
+
     // ConfigurationManager: append (or reuse) the overlay source. It is never reloaded, so it keeps
     // winning over the file providers beneath it whatever they reload to.
     private static void PinInOverlay(IConfigurationBuilder builder, Dictionary<string, string> expanded)

ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/ShippedCliSettingsTests.cs

@@ -9,6 +9,7 @@ namespace ZB.MOM.WW.Secrets.Tests.Cli;
 /// scadaproj#49: the <c>appsettings.json</c> the CLI ships must pass the store-path validator. It
 /// used to set <c>"SqlitePath": "secrets.db"</c>, which the validator rejects as relative, so the
 /// headless CLI could not start from its own directory unless an environment variable overrode it.
+/// Same check for the README's wiring sample (Secrets-009).
 /// </summary>
 public sealed class ShippedCliSettingsTests
 {
@@ -44,4 +45,21 @@ public sealed class ShippedCliSettingsTests
 
         Assert.True(Path.IsPathFullyQualified(options.SqlitePath));
     }
+
+    [Fact]
+    public void The_readme_wiring_sample_passes_store_path_validation()
+    {
+        string readme = File.ReadAllText(RepoFile("README.md"));
+        int start = readme.IndexOf("\"Secrets\": {", StringComparison.Ordinal);
+        Assert.True(start >= 0, "README no longer has a Secrets sample block");
+        int end = readme.IndexOf("\n},", start, StringComparison.Ordinal);
+        string block = "{" + readme[start..(end + 2)].TrimEnd(',') + "}";
+        string json = string.Join('\n', block.Split('\n').Where(l => !l.TrimStart().StartsWith("//", StringComparison.Ordinal)));
+
+        IConfiguration config = new ConfigurationBuilder()
+            .AddJsonStream(new MemoryStream(System.Text.Encoding.UTF8.GetBytes(json)))
+            .Build();
+
+        Validate(config);
+    }
 }

ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Configuration/SecretReferenceExpanderTests.cs

@@ -129,4 +129,47 @@ public sealed class SecretReferenceExpanderTests
         // Real key still expanded.
         Assert.Equal("Server=.;Password=hunter2", config["ConnectionStrings:Db"]);
     }
+
+    // ---------- Secrets-009: malformed references are fail-closed ----------
+
+    [Theory]
+    [InlineData("pw=${secret:}")]
+    [InlineData("pw=${secret:   }")]
+    [InlineData("pw=${secret:sql/foo")]
+    [InlineData("pw=${secret:sql/foo};x=${secret:")]
+    [InlineData("pw=${secret:a${secret:sql/foo}")]
+    public async Task ExpandAsync_rejects_a_malformed_reference_instead_of_passing_it_through(string raw)
+    {
+        SecretReferenceExpander expander = Expander(("sql/foo", "hunter2"));
+
+        var ex = await Assert.ThrowsAsync<FormatException>(() => expander.ExpandAsync(raw, CancellationToken.None));
+        Assert.DoesNotContain("hunter2", ex.Message);
+    }
+
+    [Fact]
+    public async Task ExpandConfigurationAsync_names_the_key_holding_a_malformed_reference_and_writes_nothing()
+    {
+        IConfigurationRoot config = new ConfigurationBuilder()
+            .AddInMemoryCollection(new Dictionary<string, string?>
+            {
+                ["Db:Good"] = "pw=${secret:sql/foo}",
+                ["Db:Typo"] = "Password=${secret:}",
+            })
+            .Build();
+
+        var ex = await Assert.ThrowsAsync<FormatException>(
+            () => Expander(("sql/foo", "hunter2")).ExpandConfigurationAsync(config, CancellationToken.None));
+
+        Assert.Contains("Db:Typo", ex.Message);
+        Assert.DoesNotContain("Password", ex.Message);
+        Assert.Equal("pw=${secret:sql/foo}", config["Db:Good"]); // nothing half-expanded
+    }
+
+    [Fact]
+    public async Task ExpandAsync_text_without_a_marker_is_still_returned_unchanged()
+    {
+        // Positive control: the guard is about the marker, not about braces or dollar signs.
+        string raw = "a=${env:X};b={secret};c=$secret:";
+        Assert.Equal(raw, await Expander().ExpandAsync(raw, CancellationToken.None));
+    }
 }