From 56aa386784409e3ac0cd4ac8fdaba8fd6ed6ffae Mon Sep 17 00:00:00 2001 From: Joseph Doherty Date: Thu, 22 Oct 2026 09:21:28 -0400 Subject: [PATCH] fix(transport): serialise the export content once and pack those bytes (review 2026-10-21 04/P8a) --- .../Export/BundleExporter.cs | 8 +++-- .../Serialization/BundleSerializer.cs | 23 ++++++++++++- .../Serialization/BundleSerializerTests.cs | 32 +++++++++++++++++++ 3 files changed, 60 insertions(+), 3 deletions(-) diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs index 0f9c8b8d..b83825f6 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Export/BundleExporter.cs @@ -159,6 +159,10 @@ public sealed class BundleExporter : IBundleExporter SaltB64: string.Empty, IvB64: string.Empty); + // Review 2026-10-21 04/P8a: serialise the content graph ONCE — the same bytes + // seed the manifest hash and are what Pack writes (or encrypts). + var contentBytes = _bundleSerializer.SerializeContentBytes(contentDto); + var templateManifest = _manifestBuilder.Build( sourceEnvironment: sourceEnvironment, exportedBy: user, @@ -170,11 +174,11 @@ public sealed class BundleExporter : IBundleExporter // (either fresh ciphertext or the JSON it just serialized), so the // upstream hash here only matters for the plaintext path — Pack // trusts the supplied hash and the serializer is deterministic. - contentBytes: _bundleSerializer.SerializeContentBytes(contentDto)); + contentBytes: contentBytes); // 5. Pack the zip; with a passphrase, Pack re-encrypts and re-stamps // EncryptionMetadata (fresh salt/iv) + ContentHash. - var zipStream = _bundleSerializer.Pack(contentDto, templateManifest, passphrase, _encryptor); + var zipStream = _bundleSerializer.PackContentBytes(contentBytes, templateManifest, passphrase, _encryptor); // 6. Audit — entityId is the bundle's overall SHA-256 over the zip bytes // (opaque to caller but uniquely identifies this export). Action diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/BundleSerializer.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/BundleSerializer.cs index b8e4b67f..86a1ba38 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/BundleSerializer.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Serialization/BundleSerializer.cs @@ -57,6 +57,28 @@ public sealed class BundleSerializer BundleSecretEncryptor? encryptor) { ArgumentNullException.ThrowIfNull(content); + return PackContentBytes(SerializeContentBytes(content), manifest, passphrase, encryptor); + } + + /// + /// Packs already-serialised content (the output of ) + /// and the manifest into a ZIP stream, optionally encrypting the content. Lets a caller + /// that has already serialised the DTO — e.g. to hash it for the manifest — pack those + /// same bytes instead of serialising the whole graph a second time (review 2026-10-21 + /// 04/P8a). + /// + /// The serialised bundle content JSON. + /// The bundle manifest describing the content and encryption metadata. + /// Passphrase used for encryption; null produces a plaintext bundle. + /// Encryptor required when is supplied. + /// A seeked-to-start containing the bundle ZIP archive. + public Stream PackContentBytes( + byte[] contentBytes, + BundleManifest manifest, + string? passphrase, + BundleSecretEncryptor? encryptor) + { + ArgumentNullException.ThrowIfNull(contentBytes); ArgumentNullException.ThrowIfNull(manifest); if (passphrase is not null && encryptor is null) @@ -68,7 +90,6 @@ public sealed class BundleSerializer throw new ArgumentException("Manifest must carry EncryptionMetadata when packing an encrypted bundle.", nameof(manifest)); } - var contentBytes = SerializeContentBytes(content); byte[] payload; string payloadEntryName; BundleManifest finalManifest; diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/BundleSerializerTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/BundleSerializerTests.cs index 916961fa..c203dd98 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/BundleSerializerTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Serialization/BundleSerializerTests.cs @@ -50,6 +50,38 @@ public sealed class BundleSerializerTests Assert.Null(archive.GetEntry("content.enc")); } + /// + /// Review 2026-10-21 04/P8a: the exporter serialises the content DTO once and packs + /// those bytes — the pre-serialised overload writes them verbatim as the plaintext + /// payload and they round-trip, encrypted or not. + /// + /// Null for a plaintext bundle, otherwise the passphrase. + [Theory] + [InlineData(null)] + [InlineData("pass")] + public void PackContentBytes_packs_preserialised_content_verbatim(string? passphrase) + { + var sut = new BundleSerializer(); + var encryptor = new BundleSecretEncryptor(); + var contentJson = sut.SerializeContentBytes(SampleContent()); + var manifest = passphrase is null + ? BuildManifestFor(contentJson) + : BuildManifestFor(contentJson, encryption: new EncryptionMetadata( + "AES-256-GCM", "PBKDF2-SHA256", TestIterations, string.Empty, string.Empty)); + + using var stream = sut.PackContentBytes(contentJson, manifest, passphrase, encryptor); + + var readManifest = sut.ReadManifest(stream); + stream.Position = 0; + var payload = sut.ReadContentBytes(stream, readManifest); + if (passphrase is null) + { + Assert.Equal(contentJson, payload); + } + var unpacked = sut.UnpackContent(payload, readManifest, passphrase, encryptor); + Assert.Equal("util", Assert.Single(unpacked.SharedScripts).Name); + } + /// Verifies that packing with a passphrase emits a manifest and encrypted content. [Fact] public void Pack_emits_manifest_and_content_enc_when_passphrase_supplied()