fix(secrets-cli): doctor flow — contained old-key errors, corrupt-row + rewrap-failure coverage

2026-07-19 10:27:54 -04:00
parent 1b65e820fb
commit 7e67bd61c7
2 changed files with 127 additions and 4 deletions
@@ -106,11 +106,31 @@ public sealed class KekDoctorFlow : IInteractiveFlow
    private static async Task RunRewrapAsync(
        IAnsiConsole console, SecretsSession session, KekDoctor doctor, KekDoctorReport report, CancellationToken ct)
    {
-        IMasterKeyProvider oldKek = PromptForOldKek(console);
+        // Acquire the old KEK and resolve its id under their OWN guard: a malformed pasted key throws
+        // ArgumentException from LiteralMasterKeyProvider, and an env/file source lazily throws
+        // MasterKeyUnavailableException the moment KekId is read (an unset var, a missing file). Both must
+        // render a friendly line and return here — not escape to the shell — so the closing re-diagnosis
+        // in RunAsync still runs. This is distinct from the rewrap guard below (a wrong key vs a valid one).
+        IMasterKeyProvider oldKek;
+        string oldKekId;
+        try
+        {
+            oldKek = PromptForOldKek(console);
+            oldKekId = oldKek.KekId;
+        }
+        catch (ArgumentException ex)
+        {
+            console.MarkupLineInterpolated($"[red]Invalid old KEK:[/] {ex.Message}");
+            return;
+        }
+        catch (MasterKeyUnavailableException ex)
+        {
+            console.MarkupLineInterpolated($"[red]Old KEK unavailable:[/] {ex.Message}");
+            return;
+        }

-        // Prompt and confirm OUTSIDE the guard: only the rewrap call itself is caught, so a stray
+        // Confirm OUTSIDE the rewrap guard: only the rewrap call itself is caught there, so a stray
        // ArgumentException from setup/prompt code is never mis-reported as a "rewrap failed" message.
-        string oldKekId = oldKek.KekId;
        int moving = report.Rows.Count(r => string.Equals(r.RowKekId, oldKekId, StringComparison.Ordinal));

        string confirmText =
@@ -222,7 +242,9 @@ public sealed class KekDoctorFlow : IInteractiveFlow
        SecretContentType contentType = console.Prompt(
            new SelectionPrompt<SecretContentType>().Title("  Content type").AddChoices(Enum.GetValues<SecretContentType>()));

-        StoredSecret row = session.Cipher!.Encrypt(name, value, contentType) with
+        ISecretCipher cipher = session.Cipher
+            ?? throw new InvalidOperationException("flow requires an unlocked KEK session");
+        StoredSecret row = cipher.Encrypt(name, value, contentType) with
        {
            CreatedBy = FlowPrompts.Actor,
            UpdatedBy = FlowPrompts.Actor,
@@ -31,6 +31,9 @@ public sealed class KekDoctorFlowTests : IAsyncLifetime, IDisposable
    // retained so a test can paste KEK-A at the prompt and assert it never echoes back to the terminal.
    private readonly string _kekABase64 = Convert.ToBase64String(FillKey(0xA1));
    private readonly string _kekBBase64 = Convert.ToBase64String(FillKey(0xB2));
+
+    // A third, valid-format 32-byte key never used to seal a row — a "wrong but well-formed" paste.
+    private readonly string _kekCBase64 = Convert.ToBase64String(FillKey(0xC3));
    private readonly LiteralMasterKeyProvider _kekA;
    private readonly LiteralMasterKeyProvider _kekB;

@@ -184,6 +187,104 @@ public sealed class KekDoctorFlowTests : IAsyncLifetime, IDisposable
        Assert.DoesNotContain("NEW-A-VALUE", console.Output, StringComparison.Ordinal); // value never echoed
    }

+    [Fact]
+    public async Task Malformed_pasted_old_key_renders_friendly_error_and_still_re_diagnoses()
+    {
+        await SeedAsync("svc/a", "value-a", _kekA);
+        SecretsSession session = Session(_kekB);
+
+        TestConsole console = NewConsole();
+        console.Input.PushKey(ConsoleKey.Enter); // "Rewrap from old KEK"
+        console.Input.PushKey(ConsoleKey.DownArrow);
+        console.Input.PushKey(ConsoleKey.DownArrow);
+        console.Input.PushKey(ConsoleKey.Enter); // "Paste base64 key"
+        console.Input.PushTextWithEnter("!!!not-valid-base64!!!"); // malformed → ArgumentException
+
+        await new KekDoctorFlow().RunAsync(console, session, CancellationToken.None);
+
+        Assert.Contains("Invalid old KEK", console.Output, StringComparison.Ordinal); // contained, not shell-escaped
+        Assert.Contains("Re-diagnosis", console.Output, StringComparison.Ordinal); // closing summary still runs
+        Assert.Equal(_kekA.KekId, (await GetAsync("svc/a")).KekId); // nothing changed
+    }
+
+    [Fact]
+    public async Task Env_var_old_key_source_naming_unset_var_renders_friendly_error_and_still_re_diagnoses()
+    {
+        await SeedAsync("svc/a", "value-a", _kekA);
+        SecretsSession session = Session(_kekB);
+
+        TestConsole console = NewConsole();
+        console.Input.PushKey(ConsoleKey.Enter);        // "Rewrap from old KEK"
+        console.Input.PushKey(ConsoleKey.Enter);        // old-key source: "Environment variable" (index 0)
+        // An env var guaranteed unset → KekId read lazily throws MasterKeyUnavailableException.
+        console.Input.PushTextWithEnter($"ZB_TEST_UNSET_{Guid.NewGuid():N}");
+
+        await new KekDoctorFlow().RunAsync(console, session, CancellationToken.None);
+
+        Assert.Contains("Old KEK unavailable", console.Output, StringComparison.Ordinal); // contained
+        Assert.Contains("Re-diagnosis", console.Output, StringComparison.Ordinal); // closing summary still runs
+        Assert.Equal(_kekA.KekId, (await GetAsync("svc/a")).KekId); // nothing changed
+    }
+
+    [Fact]
+    public async Task Corrupt_row_is_offered_reset_as_the_only_remedy()
+    {
+        // Seed under the session KEK, then tamper the DEK wrap tag: kek_id still matches the session KEK,
+        // but the decrypt probe fails closed → the row diagnoses Corrupt (not WrongKek).
+        await SeedAsync("svc/corrupt", "original", _kekA);
+        StoredSecret seeded = await GetAsync("svc/corrupt");
+        byte[] tag = (byte[])seeded.WrapTag.Clone();
+        tag[0] ^= 0xFF;
+        await _store.UpsertAsync(seeded with { WrapTag = tag }, CancellationToken.None);
+
+        SecretsSession session = Session(_kekA); // same KEK — so the row is Corrupt, not WrongKek
+
+        TestConsole console = NewConsole();
+        console.Input.PushKey(ConsoleKey.DownArrow); // remedy menu: move to
+        console.Input.PushKey(ConsoleKey.Enter);     // "Old KEK is lost — re-set affected secrets"
+        console.Input.PushTextWithEnter("y");        // accept re-set of the corrupt row
+        console.Input.PushTextWithEnter("RESEEDED-VALUE"); // masked new value
+        console.Input.PushKey(ConsoleKey.Enter);     // content type: Text
+
+        await new KekDoctorFlow().RunAsync(console, session, CancellationToken.None);
+
+        Assert.Contains("corrupt", console.Output, StringComparison.Ordinal); // the diagnosis surfaced it
+        Assert.Contains("only remedy", console.Output, StringComparison.Ordinal); // the re-set note
+
+        // The corrupt row was re-seeded and now decrypts cleanly under the session KEK.
+        var cipherA = new AesGcmEnvelopeCipher(_kekA);
+        StoredSecret after = await GetAsync("svc/corrupt");
+        Assert.Equal("RESEEDED-VALUE", cipherA.Decrypt(after));
+        Assert.DoesNotContain("RESEEDED-VALUE", console.Output, StringComparison.Ordinal); // never echoed
+    }
+
+    [Fact]
+    public async Task Wrong_but_wellformed_old_key_fails_the_rewrap_and_still_re_diagnoses()
+    {
+        // Rows under KEK-A; session on KEK-B. The operator pastes KEK-C — valid format, but neither the
+        // old (A) nor the session (B) KEK → RewrapAllAsync aborts with SecretDecryptionException.
+        await SeedAsync("svc/a", "value-a", _kekA);
+        await SeedAsync("svc/b", "value-b", _kekA);
+        SecretsSession session = Session(_kekB);
+
+        TestConsole console = NewConsole();
+        console.Input.PushKey(ConsoleKey.Enter); // "Rewrap from old KEK"
+        console.Input.PushKey(ConsoleKey.DownArrow);
+        console.Input.PushKey(ConsoleKey.DownArrow);
+        console.Input.PushKey(ConsoleKey.Enter); // "Paste base64 key"
+        console.Input.PushTextWithEnter(_kekCBase64); // valid format, wrong key
+        console.Input.PushTextWithEnter("y"); // confirm the rewrap
+
+        await new KekDoctorFlow().RunAsync(console, session, CancellationToken.None);
+
+        Assert.Contains("Rewrap failed", console.Output, StringComparison.Ordinal); // the SecretDecryptionException path
+        Assert.Contains("Re-diagnosis", console.Output, StringComparison.Ordinal); // closing summary still runs
+
+        // The pass aborted on the first row → both rows remain under KEK-A.
+        Assert.Equal(_kekA.KekId, (await GetAsync("svc/a")).KekId);
+        Assert.Equal(_kekA.KekId, (await GetAsync("svc/b")).KekId);
+    }
+
    public void Dispose()
    {
        SqliteConnection.ClearAllPools();