fix(secrets-cli): CRUD flows — escaped validation markup, shared prompt helpers, delete existence check

This commit is contained in:
Joseph Doherty
2026-07-19 09:47:10 -04:00
parent 6ad710bdbd
commit e5a4d0276a
5 changed files with 98 additions and 50 deletions
@@ -23,7 +23,15 @@ public sealed class DeleteSecretFlow : IInteractiveFlow
ArgumentNullException.ThrowIfNull(session); ArgumentNullException.ThrowIfNull(session);
var name = new SecretName(console.Prompt( var name = new SecretName(console.Prompt(
new TextPrompt<string>("Secret [green]name[/]:").Validate(NameValidation))); new TextPrompt<string>("Secret [green]name[/]:").Validate(FlowPrompts.ValidateSecretName)));
// Check existence first, so an unknown name is reported without a pointless confirm prompt.
StoredSecret? existing = await session.Store.GetAsync(name, ct).ConfigureAwait(false);
if (existing is null || existing.IsDeleted)
{
console.MarkupLineInterpolated($"[red]No secret named '{name.Value}'.[/]");
return;
}
if (!console.Prompt(new ConfirmationPrompt($"Delete (tombstone) '{name.Value}'?") { DefaultValue = false })) if (!console.Prompt(new ConfirmationPrompt($"Delete (tombstone) '{name.Value}'?") { DefaultValue = false }))
{ {
@@ -31,25 +39,10 @@ public sealed class DeleteSecretFlow : IInteractiveFlow
return; return;
} }
string actor = string.IsNullOrWhiteSpace(Environment.UserName) ? "cli" : Environment.UserName; bool removed = await session.Store.DeleteAsync(name, FlowPrompts.Actor, ct).ConfigureAwait(false);
bool removed = await session.Store.DeleteAsync(name, actor, ct).ConfigureAwait(false);
console.MarkupLine(removed console.MarkupLine(removed
? $"[green]Tombstoned '{Markup.Escape(name.Value)}'.[/]" ? $"[green]Tombstoned '{Markup.Escape(name.Value)}'.[/]"
: $"[red]No secret named '{Markup.Escape(name.Value)}'.[/]"); : $"[red]No secret named '{Markup.Escape(name.Value)}'.[/]");
} }
// Re-prompts on an invalid name by surfacing the SecretName constructor's own diagnostic.
private static ValidationResult NameValidation(string raw)
{
try
{
_ = new SecretName(raw);
return ValidationResult.Success();
}
catch (ArgumentException ex)
{
return ValidationResult.Error(ex.Message);
}
}
} }
@@ -0,0 +1,40 @@
using Spectre.Console;
using ZB.MOM.WW.Secrets.Abstractions;
namespace ZB.MOM.WW.Secrets.Cli.Interactive.Flows;
/// <summary>
/// Shared prompt/console helpers reused across the CRUD flows: a re-prompting secret-name validator and
/// the CLI actor-stamp fallback. Centralized so the validation and audit-actor conventions stay identical
/// in every flow.
/// </summary>
internal static class FlowPrompts
{
/// <summary>
/// A <see cref="TextPrompt{T}"/> validation callback that accepts a raw name iff it is a legal
/// <see cref="SecretName"/>, otherwise re-prompts with the constructor's own diagnostic. The
/// diagnostic is <see cref="Markup.Escape(string)"/>d because it embeds bracketed text (e.g. the
/// allowed-character class <c>[a-z0-9._/-]</c>) that Spectre would otherwise parse as markup and throw on.
/// </summary>
/// <param name="raw">The raw name entered at the prompt.</param>
/// <returns>Success when <paramref name="raw"/> is a valid name; an escaped error otherwise.</returns>
public static ValidationResult ValidateSecretName(string raw)
{
try
{
_ = new SecretName(raw);
return ValidationResult.Success();
}
catch (ArgumentException ex)
{
return ValidationResult.Error(Markup.Escape(ex.Message));
}
}
/// <summary>
/// The principal recorded as <c>created_by</c>/<c>updated_by</c> for CLI-originated writes — the OS
/// user name, or the literal <c>"cli"</c> when it is unavailable (mirrors <c>Program.cs</c>).
/// </summary>
public static string Actor =>
string.IsNullOrWhiteSpace(Environment.UserName) ? "cli" : Environment.UserName;
}
@@ -23,8 +23,11 @@ public sealed class RevealSecretFlow : IInteractiveFlow
ArgumentNullException.ThrowIfNull(console); ArgumentNullException.ThrowIfNull(console);
ArgumentNullException.ThrowIfNull(session); ArgumentNullException.ThrowIfNull(session);
ISecretCipher cipher = session.Cipher
?? throw new InvalidOperationException("flow requires an unlocked KEK session");
var name = new SecretName(console.Prompt( var name = new SecretName(console.Prompt(
new TextPrompt<string>("Secret [green]name[/]:").Validate(NameValidation))); new TextPrompt<string>("Secret [green]name[/]:").Validate(FlowPrompts.ValidateSecretName)));
StoredSecret? row = await session.Store.GetAsync(name, ct).ConfigureAwait(false); StoredSecret? row = await session.Store.GetAsync(name, ct).ConfigureAwait(false);
if (row is null || row.IsDeleted) if (row is null || row.IsDeleted)
@@ -38,20 +41,6 @@ public sealed class RevealSecretFlow : IInteractiveFlow
return; return;
} }
console.WriteLine(session.Cipher!.Decrypt(row)); console.WriteLine(cipher.Decrypt(row));
}
// Re-prompts on an invalid name by surfacing the SecretName constructor's own diagnostic.
private static ValidationResult NameValidation(string raw)
{
try
{
_ = new SecretName(raw);
return ValidationResult.Success();
}
catch (ArgumentException ex)
{
return ValidationResult.Error(ex.Message);
}
} }
} }
@@ -23,8 +23,11 @@ public sealed class SetSecretFlow : IInteractiveFlow
ArgumentNullException.ThrowIfNull(console); ArgumentNullException.ThrowIfNull(console);
ArgumentNullException.ThrowIfNull(session); ArgumentNullException.ThrowIfNull(session);
ISecretCipher cipher = session.Cipher
?? throw new InvalidOperationException("flow requires an unlocked KEK session");
var name = new SecretName(console.Prompt( var name = new SecretName(console.Prompt(
new TextPrompt<string>("Secret [green]name[/]:").Validate(NameValidation))); new TextPrompt<string>("Secret [green]name[/]:").Validate(FlowPrompts.ValidateSecretName)));
StoredSecret? existing = await session.Store.GetAsync(name, ct).ConfigureAwait(false); StoredSecret? existing = await session.Store.GetAsync(name, ct).ConfigureAwait(false);
if (existing is { IsDeleted: false } && if (existing is { IsDeleted: false } &&
@@ -40,8 +43,8 @@ public sealed class SetSecretFlow : IInteractiveFlow
string description = console.Prompt( string description = console.Prompt(
new TextPrompt<string>("Description [grey](optional)[/]:").AllowEmpty()); new TextPrompt<string>("Description [grey](optional)[/]:").AllowEmpty());
string actor = string.IsNullOrWhiteSpace(Environment.UserName) ? "cli" : Environment.UserName; string actor = FlowPrompts.Actor;
StoredSecret row = session.Cipher!.Encrypt(name, value, contentType) with StoredSecret row = cipher.Encrypt(name, value, contentType) with
{ {
Description = string.IsNullOrWhiteSpace(description) ? null : description, Description = string.IsNullOrWhiteSpace(description) ? null : description,
CreatedBy = actor, CreatedBy = actor,
@@ -51,18 +54,4 @@ public sealed class SetSecretFlow : IInteractiveFlow
console.MarkupLineInterpolated($"[green]Sealed '{name.Value}'.[/]"); console.MarkupLineInterpolated($"[green]Sealed '{name.Value}'.[/]");
} }
// Re-prompts on an invalid name by surfacing the SecretName constructor's own diagnostic.
private static ValidationResult NameValidation(string raw)
{
try
{
_ = new SecretName(raw);
return ValidationResult.Success();
}
catch (ArgumentException ex)
{
return ValidationResult.Error(ex.Message);
}
}
} }
@@ -122,6 +122,28 @@ public sealed class CrudFlowTests : IDisposable
Assert.DoesNotContain("TOPSECRET-PLAINTEXT", console.Output, StringComparison.Ordinal); // never echoed Assert.DoesNotContain("TOPSECRET-PLAINTEXT", console.Output, StringComparison.Ordinal); // never echoed
} }
[Fact]
public async Task Invalid_name_reprompts_then_completes()
{
SecretsSession session = await NewFullSessionAsync();
TestConsole console = NewConsole();
console.Input.PushTextWithEnter("bad name"); // invalid (space) → must re-prompt, not throw
console.Input.PushTextWithEnter("good/name"); // valid retry
console.Input.PushTextWithEnter("VALUE-AFTER-REPROMPT"); // value
console.Input.PushKey(ConsoleKey.Enter); // content-type: Text
console.Input.PushKey(ConsoleKey.Enter); // description: empty
// The bug this pins: SecretName's error text embeds "[a-z0-9._/-]", which Spectre parses as
// markup and throws InvalidOperationException on unless the validator escapes it. Reaching the
// assertion at all proves the re-prompt happened cleanly.
await new SetSecretFlow().RunAsync(console, session, CancellationToken.None);
StoredSecret? row = await session.Store.GetAsync(new SecretName("good/name"), CancellationToken.None);
Assert.NotNull(row);
Assert.Equal("VALUE-AFTER-REPROMPT", session.Cipher!.Decrypt(row!));
}
[Fact] [Fact]
public async Task Existing_name_confirms_overwrite() public async Task Existing_name_confirms_overwrite()
{ {
@@ -201,6 +223,21 @@ public sealed class CrudFlowTests : IDisposable
Assert.True(row!.IsDeleted); Assert.True(row!.IsDeleted);
} }
[Fact]
public async Task Unknown_name_reports_not_found_without_confirming()
{
SecretsSession session = await NewFullSessionAsync();
TestConsole console = NewConsole();
console.Input.PushTextWithEnter("never/existed"); // name only — no confirm input is scripted
// If the flow prompted for a confirm before checking existence, the missing input would fail.
await new DeleteSecretFlow().RunAsync(console, session, CancellationToken.None);
Assert.Contains("No secret named", console.Output, StringComparison.Ordinal);
StoredSecret? row = await session.Store.GetAsync(new SecretName("never/existed"), CancellationToken.None);
Assert.Null(row);
}
[Fact] [Fact]
public async Task Decline_leaves_row() public async Task Decline_leaves_row()
{ {