fix(secrets-cli): contain file-source key IO/access errors in doctor + bundle-import flows

2026-07-19 10:38:16 -04:00
parent 6dbb372adf
commit 980b4a37f6
4 changed files with 103 additions and 4 deletions
@@ -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;
            }
        }
@@ -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;
@@ -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()
    {
@@ -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()
    {