fix(secrets-ui): Add refuses an existing name; restoring a tombstone needs a second submit (scadaproj#45)

Secrets-014: SecretEditor in add mode reads the name first. A live row is refused ("already exists. Use Rotate") with no write and no audit, so a pasted name can no longer replace a live credential and reset its description/content type under a "secret.add" audit. A tombstoned name is restored only when the operator submits the same name a second time after the warning. Claude-Session: https://claude.ai/code/session_01GT67AEHd2Acv1YPwGfXEoV
2026-11-09 15:17:59 -05:00
parent 849640f467
commit fe94b32d42
2 changed files with 138 additions and 0 deletions
@@ -129,6 +129,22 @@

            try
            {
+                if (!IsRotate)
+                {
+                    // Add must never silently overwrite (Secrets-014): the store's upsert replaces the
+                    // value, clears a tombstone and resets description/content type, and it would be
+                    // audited as "add". A live row is refused; a tombstone needs a second, deliberate
+                    // submit of the same name. (Check-then-write is not atomic; a concurrent add of the
+                    // same name in another session can still race — the window is the round trip.)
+                    StoredSecret? existing = await Store.GetAsync(name, _cts.Token);
+                    string? refusal = AddRefusal(name, existing);
+                    if (refusal is not null)
+                    {
+                        _error = refusal;
+                        return;
+                    }
+                }
+
                StoredSecret row = Cipher.Encrypt(name, _value, _contentType);
                row = row with
                {
@@ -177,6 +193,31 @@
        }
    }

+    // The name whose tombstone the operator has been warned about; a second submit of the SAME
+    // name restores it. Any other name starts over.
+    private string? _restoreConfirmedFor;
+
+    private string? AddRefusal(SecretName name, StoredSecret? existing)
+    {
+        if (existing is null)
+        {
+            return null;
+        }
+
+        if (!existing.IsDeleted)
+        {
+            return $"A secret named '{name.Value}' already exists. Use Rotate to change its value.";
+        }
+
+        if (string.Equals(_restoreConfirmedFor, name.Value, StringComparison.Ordinal))
+        {
+            return null;
+        }
+
+        _restoreConfirmedFor = name.Value;
+        return $"A deleted secret named '{name.Value}' exists. Select Add again to restore it with this value.";
+    }
+
    private async Task WriteSuccessAuditAsync(string actor, string action, string target)
    {
        try
@@ -0,0 +1,97 @@
+using Bunit;
+using Bunit.TestDoubles;
+using Microsoft.Extensions.DependencyInjection;
+using ZB.MOM.WW.Audit;
+using ZB.MOM.WW.Secrets.Abstractions;
+using ZB.MOM.WW.Secrets.Ui.Components;
+using ZB.MOM.WW.Secrets.Ui.Tests.Fakes;
+
+namespace ZB.MOM.WW.Secrets.Ui.Tests;
+
+/// <summary>
+/// Secrets-014: "Add secret" on a name that already exists must not silently overwrite it (value,
+/// description, content type) and audit the overwrite as <c>secret.add</c>. A live row is refused;
+/// a tombstoned one is restored only on a second, deliberate submit of the same name.
+/// </summary>
+public class SecretEditorAddExistingTests : TestContext
+{
+    private (RecordingSecretStore Store, CapturingAuditWriter Audit, FakeCipher Cipher) Setup()
+    {
+        var store = new RecordingSecretStore();
+        var audit = new CapturingAuditWriter();
+        var cipher = new FakeCipher();
+        Services.AddSingleton<ISecretStore>(store);
+        Services.AddSingleton<IAuditWriter>(audit);
+        Services.AddSingleton<ISecretCipher>(cipher);
+        var auth = this.AddTestAuthorization();
+        auth.SetAuthorized("carol");
+        auth.SetPolicies(SecretsAuthorization.ManagePolicy);
+        return (store, audit, cipher);
+    }
+
+    private static StoredSecret Existing(FakeCipher cipher, string name, bool deleted) =>
+        cipher.Encrypt(new SecretName(name), "live-credential", SecretContentType.ConnectionString) with
+        {
+            Description = "central db",
+            Revision = 4,
+            IsDeleted = deleted,
+            CreatedUtc = DateTimeOffset.UnixEpoch,
+            UpdatedUtc = DateTimeOffset.UnixEpoch,
+        };
+
+    private IRenderedComponent<SecretEditor> FillAdd(string name, string value)
+    {
+        var cut = RenderComponent<SecretEditor>(p => p.Add(c => c.OnSaved, () => { }));
+        cut.Find("[data-testid=secret-name]").Change(name);
+        cut.Find("[data-testid=secret-value]").Change(value);
+        return cut;
+    }
+
+    [Fact]
+    public void Adding_an_existing_live_name_is_refused_and_writes_nothing()
+    {
+        var (store, audit, cipher) = Setup();
+        store.Seed(Existing(cipher, "db/central", deleted: false));
+
+        var cut = FillAdd("db/central", "pasted-from-other-env");
+        cut.Find("[data-testid=editor-submit]").Click();
+
+        Assert.Empty(store.Upserts);
+        Assert.Empty(audit.Events);
+        Assert.Contains("already exists", cut.Find("[data-testid=editor-error]").TextContent);
+
+        // A second click does not get round the refusal.
+        cut.Find("[data-testid=editor-submit]").Click();
+        Assert.Empty(store.Upserts);
+    }
+
+    [Fact]
+    public void Adding_a_tombstoned_name_needs_a_second_submit_to_restore()
+    {
+        var (store, audit, cipher) = Setup();
+        store.Seed(Existing(cipher, "db/retired", deleted: true));
+
+        var cut = FillAdd("db/retired", "fresh-value");
+        cut.Find("[data-testid=editor-submit]").Click();
+
+        Assert.Empty(store.Upserts);
+        Assert.Contains("deleted secret", cut.Find("[data-testid=editor-error]").TextContent);
+
+        cut.Find("[data-testid=editor-submit]").Click();
+
+        StoredSecret row = Assert.Single(store.Upserts);
+        Assert.Equal("fresh-value", cipher.Decrypt(row));
+        Assert.Equal("secret.add", Assert.Single(audit.Events).Action);
+    }
+
+    [Fact]
+    public void Adding_a_new_name_still_works_in_one_submit()
+    {
+        var (store, _, _) = Setup();
+
+        var cut = FillAdd("db/new", "v");
+        cut.Find("[data-testid=editor-submit]").Click();
+
+        Assert.Single(store.Upserts);
+    }
+}