diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/BundleImportFlow.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/BundleImportFlow.cs index 7cff49b..a77c987 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/BundleImportFlow.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/BundleImportFlow.cs @@ -85,12 +85,28 @@ public sealed class BundleImportFlow : IInteractiveFlow return; } + // The first KekId read is where an env/file source actually resolves the key: it lazily throws + // MasterKeyUnavailableException (an unset var, a missing file) or, for a file source, a raw + // IOException/UnauthorizedAccessException straight out of File.ReadAllBytes (e.g. a + // permission-denied key file), since FileMasterKeyProvider does not wrap those. All must render + // a friendly line and return here — not escape to the shell. + string sourceKekId; + try + { + sourceKekId = sourceKek.KekId; + } + catch (Exception ex) when (ex is MasterKeyUnavailableException or IOException or UnauthorizedAccessException) + { + console.MarkupLineInterpolated($"[red]Source KEK unavailable:[/] {ex.Message}"); + return; + } + // A valid-but-wrong key would otherwise silently land every row in SkippedForeignKek. Verify // the supplied key actually derives the bundle's source KEK id before importing. - if (!string.Equals(sourceKek.KekId, bundle.SourceKekId, StringComparison.Ordinal)) + if (!string.Equals(sourceKekId, bundle.SourceKekId, StringComparison.Ordinal)) { console.MarkupLineInterpolated( - $"[red]That key does not match this bundle's source KEK ('{bundle.SourceKekId}' expected, got '{sourceKek.KekId}').[/]"); + $"[red]That key does not match this bundle's source KEK ('{bundle.SourceKekId}' expected, got '{sourceKekId}').[/]"); return; } } diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/KekDoctorFlow.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/KekDoctorFlow.cs index cadbfba..74fbed6 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/KekDoctorFlow.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/Flows/KekDoctorFlow.cs @@ -108,7 +108,9 @@ public sealed class KekDoctorFlow : IInteractiveFlow { // 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 + // MasterKeyUnavailableException the moment KekId is read (an unset var, a missing file) — or, for + // a file source, a raw IOException/UnauthorizedAccessException straight out of File.ReadAllBytes + // (e.g. a permission-denied key file), since FileMasterKeyProvider does not wrap those. All 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; @@ -123,7 +125,7 @@ public sealed class KekDoctorFlow : IInteractiveFlow console.MarkupLineInterpolated($"[red]Invalid old KEK:[/] {ex.Message}"); return; } - catch (MasterKeyUnavailableException ex) + catch (Exception ex) when (ex is MasterKeyUnavailableException or IOException or UnauthorizedAccessException) { console.MarkupLineInterpolated($"[red]Old KEK unavailable:[/] {ex.Message}"); return; diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/BundleFlowTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/BundleFlowTests.cs index 3b888e3..8cd826a 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/BundleFlowTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/BundleFlowTests.cs @@ -232,6 +232,52 @@ public sealed class BundleFlowTests : IDisposable Assert.Null(await target.Store.GetAsync(new SecretName("svc/cross"), CancellationToken.None)); // untouched } + [Fact] + public async Task Import_source_key_file_permission_denied_renders_friendly_error_without_importing() + { + Skip.IfNot(!OperatingSystem.IsWindows(), "Unix permission bits are exercised via UnixFileMode."); + + LiteralMasterKeyProvider kekA = NewKek(out _); + LiteralMasterKeyProvider kekB = NewKek(out _); + + SecretsSession source = await NewSessionAsync("a.db", kekA); + await SealAsync(source, "svc/cross", "cross-value"); + + var exportConsole = NewConsole(); + exportConsole.Input.PushTextWithEnter(BundlePath); + exportConsole.Input.PushTextWithEnter("n"); + await new BundleExportFlow().RunAsync(exportConsole, source, CancellationToken.None); + + SecretsSession target = await NewSessionAsync("b.db", kekB); + + // A source-key file that exists (so FileMasterKeyProvider's File.Exists gate passes) but has no + // read permission: File.ReadAllBytes inside ResolveKeyBytes throws UnauthorizedAccessException, + // which FileMasterKeyProvider does not itself wrap into MasterKeyUnavailableException. + string keyFilePath = Path.Combine(_dir, $"nokey-{Guid.NewGuid():N}"); + await File.WriteAllBytesAsync(keyFilePath, new byte[32], CancellationToken.None); + File.SetUnixFileMode(keyFilePath, UnixFileMode.None); + try + { + TestConsole console = NewConsole(); + console.Input.PushTextWithEnter(BundlePath); // path + console.Input.PushKey(ConsoleKey.DownArrow); // source-key selection: skip "Paste base64 key" + console.Input.PushKey(ConsoleKey.DownArrow); // skip "Environment variable" + console.Input.PushKey(ConsoleKey.Enter); // choose "Key file path" + console.Input.PushTextWithEnter(keyFilePath); + + await new BundleImportFlow().RunAsync(console, target, CancellationToken.None); + + Assert.Contains("Source KEK unavailable", console.Output, StringComparison.Ordinal); // contained + Assert.DoesNotContain("Imported", console.Output, StringComparison.Ordinal); // no report rendered + Assert.Null(await target.Store.GetAsync(new SecretName("svc/cross"), CancellationToken.None)); // untouched + } + finally + { + File.SetUnixFileMode(keyFilePath, UnixFileMode.UserRead | UnixFileMode.UserWrite); + File.Delete(keyFilePath); + } + } + [Fact] public async Task Import_rejects_unknown_format_version_before_prompting() { diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/KekDoctorFlowTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/KekDoctorFlowTests.cs index 39be042..3331e61 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/KekDoctorFlowTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/Flows/KekDoctorFlowTests.cs @@ -226,6 +226,41 @@ public sealed class KekDoctorFlowTests : IAsyncLifetime, IDisposable Assert.Equal(_kekA.KekId, (await GetAsync("svc/a")).KekId); // nothing changed } + [Fact] + public async Task File_old_key_source_permission_denied_renders_friendly_error_and_still_re_diagnoses() + { + Skip.IfNot(!OperatingSystem.IsWindows(), "Unix permission bits are exercised via UnixFileMode."); + + await SeedAsync("svc/a", "value-a", _kekA); + SecretsSession session = Session(_kekB); + + // A key file that exists (so FileMasterKeyProvider's File.Exists gate passes) but has no read + // permission: File.ReadAllBytes inside ResolveKeyBytes throws UnauthorizedAccessException, which + // FileMasterKeyProvider does not itself wrap into MasterKeyUnavailableException. + string keyFilePath = Path.Combine(Path.GetTempPath(), $"zb-secrets-doctorflow-nokey-{Guid.NewGuid():N}"); + await File.WriteAllBytesAsync(keyFilePath, new byte[32], CancellationToken.None); + File.SetUnixFileMode(keyFilePath, UnixFileMode.None); + try + { + TestConsole console = NewConsole(); + console.Input.PushKey(ConsoleKey.Enter); // "Rewrap from old KEK" + console.Input.PushKey(ConsoleKey.DownArrow); // old-key source: skip "Environment variable" + console.Input.PushKey(ConsoleKey.Enter); // choose "Key file" (index 1) + console.Input.PushTextWithEnter(keyFilePath); + + 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 + } + finally + { + File.SetUnixFileMode(keyFilePath, UnixFileMode.UserRead | UnixFileMode.UserWrite); + File.Delete(keyFilePath); + } + } + [Fact] public async Task Corrupt_row_is_offered_reset_as_the_only_remedy() {