diff --git a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/BundleService.cs b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/BundleService.cs index f243b29..deff938 100644 --- a/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/BundleService.cs +++ b/ZB.MOM.WW.Secrets/src/ZB.MOM.WW.Secrets.Cli/Interactive/BundleService.cs @@ -144,12 +144,17 @@ public sealed class BundleService int imported = 0, skippedOlder = 0, skippedForeignKek = 0, conflicts = 0; + // Phase 1 — validate EVERYTHING before writing anything (Secrets-018). Rehydrate every entry + // and re-wrap every foreign-KEK row up front, so a malformed or tampered entry anywhere in the + // bundle fails the import with the store untouched, instead of after the rows before it have + // already been written (and possibly won conflicts against local values). + var prepared = new List(bundle.Entries.Count); for (int i = 0; i < bundle.Entries.Count; i++) { BundleEntry entry = bundle.Entries[i]; - // Rehydrate the row; a malformed entry (bad base64, unknown content type, illegal name) - // surfaces as an entry-aware failure rather than a bare FormatException/ArgumentException. + // A malformed entry (bad base64, unknown content type, illegal name) surfaces as an + // entry-aware failure rather than a bare FormatException/ArgumentException. StoredSecret row; try { @@ -158,7 +163,7 @@ public sealed class BundleService catch (Exception ex) when (ex is FormatException or ArgumentException) { throw new InvalidOperationException( - $"Bundle '{path}' entry #{i} ('{entry.Name}') is malformed: {ex.Message}", ex); + $"Bundle '{path}' entry #{i} ('{entry.Name}') is malformed: {ex.Message} Nothing was imported.", ex); } // Foreign KEK: re-wrap under the target KEK if the source KEK is available, else skip. @@ -166,7 +171,16 @@ public sealed class BundleService { if (sourceKek is not null && string.Equals(sourceKek.KekId, row.KekId, StringComparison.Ordinal)) { - row = cipher.Rewrap(row, sourceKek, targetKek); + try + { + row = cipher.Rewrap(row, sourceKek, targetKek); + } + catch (SecretDecryptionException ex) + { + throw new InvalidOperationException( + $"Bundle '{path}' entry #{i} ('{entry.Name}') could not be re-wrapped under the " + + $"target KEK: {ex.Message} Nothing was imported.", ex); + } } else { @@ -175,39 +189,60 @@ public sealed class BundleService } } - StoredSecret? existing = await session.Store.GetAsync(row.Name, ct).ConfigureAwait(false); - if (existing is null) + prepared.Add(row); + } + + // Phase 2 — apply. A store fault here can still leave the import partial (the store has no + // multi-row transaction), so the failure says exactly which rows were already written. + var written = new List(); + foreach (StoredSecret row in prepared) + { + try { - // No local row: ApplyReplicatedAsync inserts it verbatim (its LWW read finds nothing). - await session.Store.ApplyReplicatedAsync(row, ct).ConfigureAwait(false); + StoredSecret? existing = await session.Store.GetAsync(row.Name, ct).ConfigureAwait(false); + if (existing is null) + { + // No local row: ApplyReplicatedAsync inserts it verbatim (its LWW read finds nothing). + await session.Store.ApplyReplicatedAsync(row, ct).ConfigureAwait(false); + imported++; + written.Add(row.Name.Value); + continue; + } + + conflicts++; + bool lwwWin = SecretLastWriterWins.IsNewer( + row.UpdatedUtc, row.Revision, existing.UpdatedUtc, existing.Revision); + bool takeIncoming = conflictOverride is not null ? conflictOverride(existing, row) : lwwWin; + + if (!takeIncoming) + { + skippedOlder++; + continue; + } + + if (lwwWin) + { + // Natural (or override-agreed) winner: apply verbatim, keeping LWW metadata intact. + await session.Store.ApplyReplicatedAsync(row, ct).ConfigureAwait(false); + } + else + { + // Override forced an older row against LWW. A verbatim replicate would be rejected by + // the LWW gate, so adopt it as a local write — restamped current so the choice sticks. + await session.Store.UpsertAsync(row, ct).ConfigureAwait(false); + } + imported++; - continue; + written.Add(row.Name.Value); } - - conflicts++; - bool lwwWin = SecretLastWriterWins.IsNewer( - row.UpdatedUtc, row.Revision, existing.UpdatedUtc, existing.Revision); - bool takeIncoming = conflictOverride is not null ? conflictOverride(existing, row) : lwwWin; - - if (!takeIncoming) + catch (Exception ex) when (ex is not OperationCanceledException) { - skippedOlder++; - continue; + string already = written.Count == 0 + ? "No row had been written." + : $"{written.Count} row(s) were ALREADY written: {string.Join(", ", written)}."; + throw new InvalidOperationException( + $"Bundle '{path}' import stopped at '{row.Name.Value}': {ex.Message} {already}", ex); } - - if (lwwWin) - { - // Natural (or override-agreed) winner: apply verbatim, keeping LWW metadata intact. - await session.Store.ApplyReplicatedAsync(row, ct).ConfigureAwait(false); - } - else - { - // Override forced an older row against LWW. A verbatim replicate would be rejected by - // the LWW gate, so adopt it as a local write — restamped current so the choice sticks. - await session.Store.UpsertAsync(row, ct).ConfigureAwait(false); - } - - imported++; } return new BundleImportReport(imported, skippedOlder, skippedForeignKek, conflicts); diff --git a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/BundleServiceTests.cs b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/BundleServiceTests.cs index da6efef..ccccb83 100644 --- a/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/BundleServiceTests.cs +++ b/ZB.MOM.WW.Secrets/tests/ZB.MOM.WW.Secrets.Tests/Cli/Interactive/BundleServiceTests.cs @@ -374,4 +374,93 @@ public sealed class BundleServiceTests : IDisposable StoredSecret? dst = await target.Store.GetAsync(new SecretName("svc/cross"), CancellationToken.None); Assert.Null(dst); } + + // ---------- Secrets-018: a bad entry anywhere fails the import before anything is written ---------- + + [Fact] + public async Task A_tampered_foreign_kek_entry_late_in_the_bundle_leaves_the_store_untouched() + { + LiteralMasterKeyProvider kekA = NewKek(); + LiteralMasterKeyProvider kekB = NewKek(); + SecretsSession source = BuildSession(Db("a.db"), kekA); + await SealAsync(source, "svc/first", "v1"); + await SealAsync(source, "svc/second", "v2"); + StoredSecret first = (await source.Store.GetAsync(new SecretName("svc/first"), CancellationToken.None))!; + StoredSecret second = (await source.Store.GetAsync(new SecretName("svc/second"), CancellationToken.None))!; + byte[] tamperedTag = (byte[])second.WrapTag.Clone(); + tamperedTag[0] ^= 0xFF; + WriteBundle(kekA.KekId, first, second with { WrapTag = tamperedTag }); + + SecretsSession target = BuildSession(Db("b.db"), kekB); + InvalidOperationException ex = await Assert.ThrowsAsync( + () => new BundleService().ImportAsync(target, BundlePath, sourceKek: kekA, conflictOverride: null, CancellationToken.None)); + + Assert.Contains("svc/second", ex.Message, StringComparison.Ordinal); + Assert.Contains("Nothing was imported", ex.Message, StringComparison.Ordinal); + Assert.Null(await target.Store.GetAsync(new SecretName("svc/first"), CancellationToken.None)); + } + + [Fact] + public async Task A_malformed_entry_after_good_ones_leaves_the_store_untouched() + { + LiteralMasterKeyProvider kek = NewKek(); + SecretsSession source = BuildSession(Db("a.db"), kek); + await SealAsync(source, "svc/good", "v"); + StoredSecret good = (await source.Store.GetAsync(new SecretName("svc/good"), CancellationToken.None))!; + WriteBundle(kek.KekId, good, good with { Name = new SecretName("svc/bad") }); + + // Corrupt the second entry's ciphertext base64 in the file. + string json = await File.ReadAllTextAsync(BundlePath); + int at = json.LastIndexOf("\"Ciphertext\"", StringComparison.Ordinal); + int open = json.IndexOf('"', json.IndexOf(':', at)) + 1; + json = json[..open] + "@@not-base64@@" + json[json.IndexOf('"', open)..]; + await File.WriteAllTextAsync(BundlePath, json); + + SecretsSession target = BuildSession(Db("b.db"), kek); + await Assert.ThrowsAsync( + () => new BundleService().ImportAsync(target, BundlePath, sourceKek: null, conflictOverride: null, CancellationToken.None)); + + Assert.Null(await target.Store.GetAsync(new SecretName("svc/good"), CancellationToken.None)); + } + + private sealed class FailOnNameStore(ISecretStore inner, string failName) : ISecretStore + { + public Task GetAsync(SecretName name, CancellationToken ct) => inner.GetAsync(name, ct); + public Task UpsertAsync(StoredSecret row, CancellationToken ct) => inner.UpsertAsync(row, ct); + public Task DeleteAsync(SecretName name, string? actor, CancellationToken ct) => inner.DeleteAsync(name, actor, ct); + public Task> ListAsync(bool includeDeleted, CancellationToken ct) => inner.ListAsync(includeDeleted, ct); + public Task> GetManifestAsync(CancellationToken ct) => inner.GetManifestAsync(ct); + public Task ApplyReplicatedAsync(StoredSecret row, CancellationToken ct) => + row.Name.Value == failName ? throw new IOException("disk full") : inner.ApplyReplicatedAsync(row, ct); + public Task ApplyRewrapAsync(StoredSecret rewrappedRow, byte[] expectedCurrentWrappedDek, CancellationToken ct) => + inner.ApplyRewrapAsync(rewrappedRow, expectedCurrentWrappedDek, ct); + } + + [Fact] + public async Task A_store_fault_mid_import_reports_which_rows_were_already_written() + { + LiteralMasterKeyProvider kek = NewKek(); + SecretsSession source = BuildSession(Db("a.db"), kek); + await SealAsync(source, "svc/a", "1"); + await SealAsync(source, "svc/b", "2"); + await SealAsync(source, "svc/c", "3"); + await new BundleService().ExportAsync(source, BundlePath, includeDeleted: false, CancellationToken.None); + + SecretsSession built = BuildSession(Db("b.db"), kek); + var target = new SecretsSession + { + Target = built.Target, + Store = new FailOnNameStore(built.Store, "svc/b"), + Migrator = built.Migrator, + MasterKey = built.MasterKey, + Cipher = built.Cipher, + StoreKind = built.StoreKind, + }; + + InvalidOperationException ex = await Assert.ThrowsAsync( + () => new BundleService().ImportAsync(target, BundlePath, sourceKek: null, conflictOverride: null, CancellationToken.None)); + + Assert.Contains("stopped at 'svc/b'", ex.Message, StringComparison.Ordinal); + Assert.Contains("ALREADY written: svc/a", ex.Message, StringComparison.Ordinal); + } }