diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/InteractiveShell.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/InteractiveShell.cs index b7e4491..6911323 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/InteractiveShell.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/InteractiveShell.cs @@ -48,6 +48,24 @@ public sealed class InteractiveShell ArgumentNullException.ThrowIfNull(sessions); ArgumentNullException.ThrowIfNull(flows); + // A flow's Title is the menu routing key (BuildMenu emits it; the loop dispatches with a + // .First() title match), so a duplicate — or a collision with a reserved built-in label — + // would silently misroute selections. Reject both at construction rather than at click time. + HashSet seenTitles = new(StringComparer.Ordinal); + foreach (IInteractiveFlow flow in flows) + { + if (flow.Title is SwitchLabel or QuitLabel) + { + throw new ArgumentException( + $"Flow title '{flow.Title}' collides with a reserved menu action.", nameof(flows)); + } + + if (!seenTitles.Add(flow.Title)) + { + throw new ArgumentException($"Duplicate flow title '{flow.Title}'.", nameof(flows)); + } + } + _console = console; _recents = recents; _reader = reader; @@ -60,23 +78,37 @@ public sealed class InteractiveShell /// 0 on a clean quit. public async Task RunAsync(CancellationToken ct) { - while (true) + try { - StoreTarget? target = PickTarget(); - if (target is null) + while (true) { - return 0; // operator cancelled the picker (Ctrl-C) → exit + if (ct.IsCancellationRequested) + { + return 0; + } + + StoreTarget? target = PickTarget(); + if (target is null) + { + return 0; // operator cancelled the picker (Ctrl-C) → exit + } + + SecretsSession session = await _sessions.OpenAsync(target, overrideKek: null, ct).ConfigureAwait(false); + RenderWarnings(session); + + if (await RunMenuLoopAsync(session, ct).ConfigureAwait(false) == MenuOutcome.Quit) + { + return 0; + } + + // MenuOutcome.SwitchTarget → fall through and re-open the picker. } - - SecretsSession session = await _sessions.OpenAsync(target, overrideKek: null, ct).ConfigureAwait(false); - RenderWarnings(session); - - if (await RunMenuLoopAsync(session, ct).ConfigureAwait(false) == MenuOutcome.Quit) - { - return 0; - } - - // MenuOutcome.SwitchTarget → fall through and re-open the picker. + } + catch (OperationCanceledException) + { + // Cooperative cancellation of the shell's token (Ctrl-C, or a flow/host cancelling it) is a + // clean shutdown, not an error — surfaced e.g. from a pre-cancelled session open. + return 0; } } @@ -91,6 +123,14 @@ public sealed class InteractiveShell { while (true) { + // Honor cooperative cancellation between actions. Spectre's blocking prompt cannot observe + // the token itself (TestConsole likewise cannot inject a Ctrl-C into a prompt), so this + // top-of-loop check is what turns a token cancelled during a flow into a clean exit. + if (ct.IsCancellationRequested) + { + return MenuOutcome.Quit; + } + RenderHeader(session); string choice; @@ -140,10 +180,13 @@ public sealed class InteractiveShell // The store/key faults a flow may realistically surface. SecretDecryptionException and // MasterKeyUnavailableException both derive from InvalidOperationException; DbException (base of both - // Microsoft.Data.Sqlite and Microsoft.Data.SqlClient exceptions) covers store-provider faults. A - // blanket catch is deliberately avoided so genuinely unexpected faults still surface. + // Microsoft.Data.Sqlite and Microsoft.Data.SqlClient exceptions) covers store-provider faults; + // UnauthorizedAccessException (a permission-denied key file or appsettings — NOT an IOException + // subtype) is exactly the lockout path this console exists to recover, so it must be contained too. + // A blanket catch is deliberately avoided so genuinely unexpected faults still surface. private static bool IsContainable(Exception ex) => - ex is InvalidOperationException or IOException or ArgumentException or DbException; + ex is InvalidOperationException or IOException or ArgumentException or DbException + or UnauthorizedAccessException; // Presents the target picker: recents newest-first, then the two entry options. Returns the resolved // target, or null if the operator cancelled the selection (Ctrl-C). Re-prompts on a recoverable diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/InteractiveShellTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/InteractiveShellTests.cs index 5e61497..957abdb 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/InteractiveShellTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/InteractiveShellTests.cs @@ -256,6 +256,66 @@ public sealed class InteractiveShellTests : IDisposable Assert.Contains("KEK doctor", console.Output, StringComparison.Ordinal); } + [Fact] + public async Task Flow_throwing_UnauthorizedAccess_is_contained_and_returns_to_menu() + { + var flow = new FakeFlow( + "Read key file", + requiresKek: false, + onRun: _ => throw new UnauthorizedAccessException("permission denied")); + + TestConsole console = NewConsole(); + InteractiveShell shell = NewShell(console, NewRecents(), flow); + + ScriptManualEntry(console, UnsetEnvVarName()); + SelectIndex(console, 0); // run the throwing flow + SelectIndex(console, 2); // loop returned to menu → Quit + + int result = await shell.RunAsync(CancellationToken.None); + + Assert.Equal(0, result); + Assert.Equal(1, flow.InvocationCount); + Assert.Contains("KEK doctor", console.Output, StringComparison.Ordinal); + } + + [Fact] + public void Constructor_rejects_duplicate_flow_titles() + { + var first = new FakeFlow("Same title"); + var second = new FakeFlow("Same title"); + + Assert.Throws(() => NewShell(NewConsole(), NewRecents(), first, second)); + } + + [Fact] + public void Constructor_rejects_flow_title_colliding_with_a_builtin() + { + var reserved = new FakeFlow("Quit"); + + Assert.Throws(() => NewShell(NewConsole(), NewRecents(), reserved)); + } + + [Fact] + public async Task Cancelled_token_exits_menu_loop_cleanly_with_zero() + { + using var cts = new CancellationTokenSource(); + // The flow cancels the shell's own token mid-action; the menu loop's top-of-loop cancellation + // check must then exit 0 without prompting for (unscripted) further input or throwing. This is + // the testable stand-in for a menu-level Ctrl-C, which TestConsole cannot inject into a prompt. + var flow = new FakeFlow("Cancel the shell", requiresKek: false, onRun: _ => cts.Cancel()); + + TestConsole console = NewConsole(); + InteractiveShell shell = NewShell(console, NewRecents(), flow); + + ScriptManualEntry(console, UnsetEnvVarName()); + SelectIndex(console, 0); // run the flow, which cancels the token — no more input is scripted + + int result = await shell.RunAsync(cts.Token); + + Assert.Equal(0, result); + Assert.Equal(1, flow.InvocationCount); + } + [Fact] public async Task Switch_target_reopens_picker() {