fix(secrets-cli): bundle import — source-key verification, format gate up front, access-denied containment

This commit is contained in:
Joseph Doherty
2026-07-19 10:00:59 -04:00
parent 4549b93d2a
commit 04018aab6e
2 changed files with 91 additions and 7 deletions
@@ -45,19 +45,29 @@ public sealed class BundleImportFlow : IInteractiveFlow
return; return;
} }
// Read + parse FIRST so the source KEK can be inspected before any write is attempted. // Read + parse FIRST so the format version and source KEK can be inspected before any write
// or any prompting — is attempted.
SecretBundle bundle; SecretBundle bundle;
try try
{ {
string json = await File.ReadAllTextAsync(path, ct).ConfigureAwait(false); string json = await File.ReadAllTextAsync(path, ct).ConfigureAwait(false);
bundle = SecretBundleCodec.Deserialize(json); bundle = SecretBundleCodec.Deserialize(json);
} }
catch (Exception ex) when (ex is InvalidOperationException or JsonException or IOException) catch (Exception ex) when (ex is InvalidOperationException or JsonException or IOException or UnauthorizedAccessException)
{ {
console.MarkupLineInterpolated($"[red]Could not read bundle '{path}': {ex.Message}[/]"); console.MarkupLineInterpolated($"[red]Could not read bundle '{path}': {ex.Message}[/]");
return; return;
} }
// Format gate up front: reject an unreadable version before consuming any operator input. The
// service applies the same gate as defense-in-depth, but failing here keeps the flow honest.
if (bundle.FormatVersion != SecretBundle.CurrentFormatVersion)
{
console.MarkupLineInterpolated(
$"[red]Bundle '{path}' has unsupported format version {bundle.FormatVersion}; this build supports version {SecretBundle.CurrentFormatVersion}.[/]");
return;
}
// Foreign-KEK detection: a bundle exported under another KEK must be re-wrapped, which needs the // Foreign-KEK detection: a bundle exported under another KEK must be re-wrapped, which needs the
// source key. Prompt for it up front; without it every row would be skipped as foreign-KEK. // source key. Prompt for it up front; without it every row would be skipped as foreign-KEK.
IMasterKeyProvider? sourceKek = null; IMasterKeyProvider? sourceKek = null;
@@ -74,6 +84,15 @@ public sealed class BundleImportFlow : IInteractiveFlow
console.MarkupLine("[yellow]Import cancelled.[/]"); console.MarkupLine("[yellow]Import cancelled.[/]");
return; 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))
{
console.MarkupLineInterpolated(
$"[red]That key does not match this bundle's source KEK ('{bundle.SourceKekId}' expected, got '{sourceKek.KekId}').[/]");
return;
}
} }
bool perConflict = console.Prompt( bool perConflict = console.Prompt(
@@ -97,7 +116,7 @@ public sealed class BundleImportFlow : IInteractiveFlow
return; return;
} }
RenderReport(console, report); RenderReport(console, report, bundle.SourceKekId);
} }
// Builds the source-KEK provider the operator selects: a pasted base64 key (masked), an environment // Builds the source-KEK provider the operator selects: a pasted base64 key (masked), an environment
@@ -166,16 +185,23 @@ public sealed class BundleImportFlow : IInteractiveFlow
return console.Prompt(new ConfirmationPrompt(" Take the bundle row?") { DefaultValue = false }); return console.Prompt(new ConfirmationPrompt(" Take the bundle row?") { DefaultValue = false });
} }
// A compact tally table of the import outcome. // A compact tally table of the import outcome. When rows were skipped as foreign-KEK, the bundle's
private static void RenderReport(IAnsiConsole console, BundleImportReport report) // source KEK id is appended so a wrong-key paste is self-diagnosable from the report alone.
private static void RenderReport(IAnsiConsole console, BundleImportReport report, string sourceKekId)
{ {
var table = new Table() var table = new Table()
.AddColumn("Result") .AddColumn("Result")
.AddColumn("Rows"); .AddColumn("Rows");
string foreign = report.SkippedForeignKek.ToString(CultureInfo.InvariantCulture);
if (report.SkippedForeignKek > 0)
{
foreign += $" (source KEK {Markup.Escape(sourceKekId)})";
}
table.AddRow("Imported", report.Imported.ToString(CultureInfo.InvariantCulture)); table.AddRow("Imported", report.Imported.ToString(CultureInfo.InvariantCulture));
table.AddRow("Skipped (older)", report.SkippedOlder.ToString(CultureInfo.InvariantCulture)); table.AddRow("Skipped (older)", report.SkippedOlder.ToString(CultureInfo.InvariantCulture));
table.AddRow("Skipped (foreign KEK)", report.SkippedForeignKek.ToString(CultureInfo.InvariantCulture)); table.AddRow("Skipped (foreign KEK)", foreign);
table.AddRow("Conflicts", report.Conflicts.ToString(CultureInfo.InvariantCulture)); table.AddRow("Conflicts", report.Conflicts.ToString(CultureInfo.InvariantCulture));
console.Write(table); console.Write(table);
@@ -100,7 +100,7 @@ public sealed class BundleFlowTests : IDisposable
Assert.True(new BundleExportFlow().RequiresKek); Assert.True(new BundleExportFlow().RequiresKek);
Assert.True(File.Exists(outPath)); Assert.True(File.Exists(outPath));
Assert.Contains("2", console.Output, StringComparison.Ordinal); Assert.Contains("Exported 2 row(s)", console.Output, StringComparison.Ordinal);
// The offered default filename carries the target name and the .json extension. // The offered default filename carries the target name and the .json extension.
Assert.Contains($"secrets-bundle-{session.Target.Name}", console.Output, StringComparison.Ordinal); Assert.Contains($"secrets-bundle-{session.Target.Name}", console.Output, StringComparison.Ordinal);
Assert.Contains(".json", console.Output, StringComparison.Ordinal); Assert.Contains(".json", console.Output, StringComparison.Ordinal);
@@ -201,4 +201,62 @@ public sealed class BundleFlowTests : IDisposable
Assert.DoesNotContain(base64A, console.Output, StringComparison.Ordinal); // pasted key never echoed Assert.DoesNotContain(base64A, console.Output, StringComparison.Ordinal); // pasted key never echoed
} }
[Fact]
public async Task Import_wrong_source_key_is_rejected_without_importing()
{
LiteralMasterKeyProvider kekA = NewKek(out _);
LiteralMasterKeyProvider kekB = NewKek(out _);
LiteralMasterKeyProvider wrong = NewKek(out string wrongBase64); // valid format, unrelated key
Assert.NotEqual(kekA.KekId, wrong.KekId);
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);
TestConsole console = NewConsole();
console.Input.PushTextWithEnter(BundlePath); // path
console.Input.PushKey(ConsoleKey.Enter); // source-key selection: paste base64
console.Input.PushTextWithEnter(wrongBase64); // a valid-format but wrong key
await new BundleImportFlow().RunAsync(console, target, CancellationToken.None);
Assert.Contains("does not match this bundle's source KEK", console.Output, StringComparison.Ordinal);
Assert.DoesNotContain("Imported", console.Output, StringComparison.Ordinal); // no report rendered
Assert.Null(await target.Store.GetAsync(new SecretName("svc/cross"), CancellationToken.None)); // untouched
}
[Fact]
public async Task Import_rejects_unknown_format_version_before_prompting()
{
LiteralMasterKeyProvider kek = NewKek(out _);
SecretsSession target = await NewSessionAsync("b.db", kek);
// A hand-built bundle envelope on an unreadable format version. Only the path is scripted: if the
// flow prompted for anything past the format gate, the missing input would fail the test.
const string json = """
{
"FormatVersion": 99,
"ExportedUtc": "2026-01-01T00:00:00+00:00",
"SourceKekId": "some-kek",
"Entries": []
}
""";
await File.WriteAllTextAsync(BundlePath, json, CancellationToken.None);
TestConsole console = NewConsole();
console.Input.PushTextWithEnter(BundlePath); // path only — no further prompt must be consumed
await new BundleImportFlow().RunAsync(console, target, CancellationToken.None);
Assert.Contains("99", console.Output, StringComparison.Ordinal);
Assert.Contains("format version", console.Output, StringComparison.Ordinal);
Assert.DoesNotContain("Imported", console.Output, StringComparison.Ordinal); // no report rendered
}
} }