fix(secrets-cli): bundle import — verbatim ApplyReplicated writes, format gate, entry-aware errors

2026-07-19 09:35:39 -04:00
parent 4576f73c4c
commit 138e2c1006
3 changed files with 171 additions and 13 deletions
@@ -80,6 +80,7 @@ public sealed class BundleService
    /// Imports the bundle at <paramref name="path"/> into <paramref name="session"/>'s store.
    /// </summary>
    /// <remarks>
+    /// <para>
    /// Per row: a row wrapped under a foreign KEK is re-wrapped under the target KEK when
    /// <paramref name="sourceKek"/> matches its <see cref="StoredSecret.KekId"/>, otherwise it is
    /// skipped (<see cref="BundleImportReport.SkippedForeignKek"/>). A row whose name does not yet
@@ -87,6 +88,19 @@ public sealed class BundleService
    /// <paramref name="conflictOverride"/> when supplied, else by <see cref="SecretLastWriterWins"/>;
    /// the winner is written and the loser skipped (<see cref="BundleImportReport.SkippedOlder"/>).
    /// No plaintext is ever handled — the bundle is ciphertext only.
+    /// </para>
+    /// <para>
+    /// Writes go through the store's <see cref="ISecretStore.ApplyReplicatedAsync"/> so the row lands
+    /// <b>verbatim</b> — its own revision, timestamps, and tombstone flag preserved — under a
+    /// serializable transaction with the shared last-writer-wins gate built in. This keeps LWW metadata
+    /// intact across further hops (a second import, or cluster anti-entropy) and means an imported
+    /// tombstone stays a tombstone. The one exception is a <paramref name="conflictOverride"/> that
+    /// forces the incoming row against the LWW ordering (operator adopts an <em>older</em> row): a
+    /// verbatim replicate would be silently rejected by the LWW gate, so that single case is written
+    /// through <see cref="ISecretStore.UpsertAsync"/> instead — a local-write that restamps the row as
+    /// current, which is exactly what makes the operator's forced choice durable rather than flipped
+    /// back on the next reconciliation.
+    /// </para>
    /// </remarks>
    /// <param name="session">The full session to import into.</param>
    /// <param name="path">Source bundle path.</param>
@@ -118,11 +132,31 @@ public sealed class BundleService
        string json = await File.ReadAllTextAsync(path, ct).ConfigureAwait(false);
        SecretBundle bundle = SecretBundleCodec.Deserialize(json);

+        if (bundle.FormatVersion != SecretBundle.CurrentFormatVersion)
+        {
+            throw new InvalidOperationException(
+                $"Bundle '{path}' has unsupported format version {bundle.FormatVersion}; " +
+                $"this build supports version {SecretBundle.CurrentFormatVersion}.");
+        }
+
        int imported = 0, skippedOlder = 0, skippedForeignKek = 0, conflicts = 0;

-        foreach (BundleEntry entry in bundle.Entries)
+        for (int i = 0; i < bundle.Entries.Count; i++)
        {
-            StoredSecret row = SecretBundleCodec.ToRow(entry);
+            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.
+            StoredSecret row;
+            try
+            {
+                row = SecretBundleCodec.ToRow(entry);
+            }
+            catch (Exception ex) when (ex is FormatException or ArgumentException)
+            {
+                throw new InvalidOperationException(
+                    $"Bundle '{path}' entry #{i} ('{entry.Name}') is malformed: {ex.Message}", ex);
+            }

            // Foreign KEK: re-wrap under the target KEK if the source KEK is available, else skip.
            if (!string.Equals(row.KekId, targetKek.KekId, StringComparison.Ordinal))
@@ -141,25 +175,36 @@ public sealed class BundleService
            StoredSecret? existing = await session.Store.GetAsync(row.Name, ct).ConfigureAwait(false);
            if (existing is null)
            {
-                await session.Store.UpsertAsync(row, ct).ConfigureAwait(false);
+                // No local row: ApplyReplicatedAsync inserts it verbatim (its LWW read finds nothing).
+                await session.Store.ApplyReplicatedAsync(row, ct).ConfigureAwait(false);
                imported++;
                continue;
            }

            conflicts++;
-            bool takeIncoming = conflictOverride is not null
-                ? conflictOverride(existing, row)
-                : SecretLastWriterWins.IsNewer(row.UpdatedUtc, row.Revision, existing.UpdatedUtc, existing.Revision);
+            bool lwwWin = SecretLastWriterWins.IsNewer(
+                row.UpdatedUtc, row.Revision, existing.UpdatedUtc, existing.Revision);
+            bool takeIncoming = conflictOverride is not null ? conflictOverride(existing, row) : lwwWin;

-            if (takeIncoming)
+            if (!takeIncoming)
            {
-                await session.Store.UpsertAsync(row, ct).ConfigureAwait(false);
-                imported++;
+                skippedOlder++;
+                continue;
+            }
+
+            if (lwwWin)
+            {
+                // Natural (or override-agreed) winner: apply verbatim, keeping LWW metadata intact.
+                await session.Store.ApplyReplicatedAsync(row, ct).ConfigureAwait(false);
            }
            else
            {
-                skippedOlder++;
+                // 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);
@@ -14,8 +14,11 @@ namespace ZB.MOM.WW.Secrets.Cli.Interactive;
 /// </summary>
 public sealed record SecretBundle
 {
+    /// <summary>The only bundle format version this build can read or write.</summary>
+    public const int CurrentFormatVersion = 1;
+
    /// <summary>The on-disk bundle format version (currently <c>1</c>).</summary>
-    public int FormatVersion { get; init; } = 1;
+    public int FormatVersion { get; init; } = CurrentFormatVersion;

    /// <summary>When the bundle was exported (UTC).</summary>
    public DateTimeOffset ExportedUtc { get; init; }
@@ -77,7 +77,13 @@ public sealed class BundleServiceTests : IDisposable
    {
        LiteralMasterKeyProvider kek = NewKek();
        SecretsSession source = BuildSession(Db("a.db"), kek);
-        await SealAsync(source, "svc/one", "value-one");
+
+        // Seed non-trivial metadata: created_by/updated_by set, and a second update so Revision == 1
+        // and created_utc != updated_utc — makes the "verbatim" assertion below actually load-bearing.
+        StoredSecret seed = source.Cipher!.Encrypt(new SecretName("svc/one"), "value-one", SecretContentType.Text)
+            with { Description = "the one", CreatedBy = "creator", UpdatedBy = "updater" };
+        await source.Store.UpsertAsync(seed, CancellationToken.None);
+        await source.Store.UpsertAsync(seed, CancellationToken.None);
        await SealAsync(source, "svc/two", "value-two");

        var service = new BundleService();
@@ -98,7 +104,7 @@ public sealed class BundleServiceTests : IDisposable
            Assert.NotNull(src);
            Assert.NotNull(dst);

-            // Fields UpsertAsync preserves verbatim.
+            // Crypto + identity fields.
            Assert.Equal(src!.Name.Value, dst!.Name.Value);
            Assert.Equal(src.ContentType, dst.ContentType);
            Assert.Equal(src.Description, dst.Description);
@@ -110,9 +116,113 @@ public sealed class BundleServiceTests : IDisposable
            Assert.Equal(src.WrapNonce, dst.WrapNonce);
            Assert.Equal(src.WrapTag, dst.WrapTag);

+            // Verbatim import: LWW metadata + audit stamps survive unchanged (ApplyReplicatedAsync).
+            Assert.Equal(src.Revision, dst.Revision);
+            Assert.Equal(src.CreatedUtc, dst.CreatedUtc);
+            Assert.Equal(src.UpdatedUtc, dst.UpdatedUtc);
+            Assert.Equal(src.IsDeleted, dst.IsDeleted);
+            Assert.Equal(src.DeletedUtc, dst.DeletedUtc);
+            Assert.Equal(src.CreatedBy, dst.CreatedBy);
+            Assert.Equal(src.UpdatedBy, dst.UpdatedBy);
+
            // And it decrypts to the same plaintext under the same KEK.
            Assert.Equal(value, target.Cipher!.Decrypt(dst));
        }
+
+        // The twice-updated row genuinely exercised non-default metadata.
+        StoredSecret one = (await target.Store.GetAsync(new SecretName("svc/one"), CancellationToken.None))!;
+        Assert.Equal(1, one.Revision);
+        Assert.Equal("creator", one.CreatedBy);
+        Assert.Equal("updater", one.UpdatedBy);
+    }
+
+    [Fact]
+    public async Task Import_preserves_tombstones_verbatim()
+    {
+        LiteralMasterKeyProvider kek = NewKek();
+        SecretsSession source = BuildSession(Db("a.db"), kek);
+        await SealAsync(source, "svc/dead", "dead-value");
+        await source.Store.DeleteAsync(new SecretName("svc/dead"), actor: "remover", CancellationToken.None);
+        StoredSecret src = (await source.Store.GetAsync(new SecretName("svc/dead"), CancellationToken.None))!;
+
+        var service = new BundleService();
+        int exported = await service.ExportAsync(source, BundlePath, includeDeleted: true, CancellationToken.None);
+        Assert.Equal(1, exported);
+
+        SecretsSession target = BuildSession(Db("b.db"), kek);
+        BundleImportReport report =
+            await service.ImportAsync(target, BundlePath, sourceKek: null, conflictOverride: null, CancellationToken.None);
+        Assert.Equal(1, report.Imported);
+
+        // The imported row stays a tombstone (no resurrection) with its delete metadata intact.
+        StoredSecret dst = (await target.Store.GetAsync(new SecretName("svc/dead"), CancellationToken.None))!;
+        Assert.True(dst.IsDeleted);
+        Assert.Equal(src.DeletedUtc, dst.DeletedUtc);
+        Assert.Equal(src.Revision, dst.Revision);
+        Assert.Equal(src.UpdatedUtc, dst.UpdatedUtc);
+
+        // And it is not listed as a live secret.
+        IReadOnlyList<SecretMetadata> live = await target.Store.ListAsync(includeDeleted: false, CancellationToken.None);
+        Assert.Empty(live);
+    }
+
+    [Fact]
+    public async Task Import_rejects_unknown_format_version()
+    {
+        LiteralMasterKeyProvider kek = NewKek();
+        SecretsSession target = BuildSession(Db("b.db"), kek);
+
+        const string json = """
+            {
+              "FormatVersion": 99,
+              "ExportedUtc": "2026-01-01T00:00:00+00:00",
+              "SourceKekId": "kek",
+              "Entries": []
+            }
+            """;
+        await File.WriteAllTextAsync(BundlePath, json, CancellationToken.None);
+
+        var service = new BundleService();
+        InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
+            () => service.ImportAsync(target, BundlePath, sourceKek: null, conflictOverride: null, CancellationToken.None));
+        Assert.Contains("99", ex.Message, StringComparison.Ordinal);
+        Assert.Contains("1", ex.Message, StringComparison.Ordinal);
+    }
+
+    [Fact]
+    public async Task Import_malformed_entry_fails_with_entry_context()
+    {
+        LiteralMasterKeyProvider kek = NewKek();
+        SecretsSession target = BuildSession(Db("b.db"), kek);
+
+        // A well-formed bundle envelope carrying one entry with non-base64 ciphertext.
+        const string json = """
+            {
+              "FormatVersion": 1,
+              "ExportedUtc": "2026-01-01T00:00:00+00:00",
+              "SourceKekId": "kek",
+              "Entries": [
+                {
+                  "Name": "svc/broken", "Description": null, "ContentType": "Text",
+                  "Ciphertext": "@@not-base64@@", "Nonce": "AAAA", "Tag": "AAAA",
+                  "WrappedDek": "AAAA", "WrapNonce": "AAAA", "WrapTag": "AAAA",
+                  "KekId": "kek", "Revision": 0, "IsDeleted": false, "DeletedUtc": null,
+                  "CreatedUtc": "2026-01-01T00:00:00+00:00", "UpdatedUtc": "2026-01-01T00:00:00+00:00",
+                  "CreatedBy": null, "UpdatedBy": null
+                }
+              ]
+            }
+            """;
+        await File.WriteAllTextAsync(BundlePath, json, CancellationToken.None);
+
+        var service = new BundleService();
+        InvalidOperationException ex = await Assert.ThrowsAsync<InvalidOperationException>(
+            () => service.ImportAsync(target, BundlePath, sourceKek: null, conflictOverride: null, CancellationToken.None));
+        Assert.Contains("svc/broken", ex.Message, StringComparison.Ordinal);
+        Assert.Contains(BundlePath, ex.Message, StringComparison.Ordinal);
+
+        // The store was left untouched.
+        Assert.Null(await target.Store.GetAsync(new SecretName("svc/broken"), CancellationToken.None));
    }

    [Fact]