diff --git a/docs/requirements/Component-Transport.md b/docs/requirements/Component-Transport.md index 2bf921ed..b1ef7fdc 100644 --- a/docs/requirements/Component-Transport.md +++ b/docs/requirements/Component-Transport.md @@ -134,7 +134,7 @@ flowchart TD class T muted ``` -The component is central-hosted. It is registered in `ZB.MOM.WW.ScadaBridge.Host` for central roles only, never for site roles — even when a bundle carries site-scoped config (T18), Transport only writes the central configuration database, never the site clusters. All persistence flows through existing audited repository interfaces in `ZB.MOM.WW.ScadaBridge.ConfigurationDatabase` — the component does not call `DbContext.SaveChangesAsync` directly. `BundleSessionStore` is in-process on the active central node (matching Blazor Server circuit affinity): 30-minute TTL, eviction on expiry, 3-strike passphrase lockout per session, and a `MaxConcurrentImportSessions` cap (default 8; arch-review 05 Task 24 — startup-validated as `> 0` by `TransportOptionsValidator`, since a zero/negative cap would reject every import session forever) bounding the decrypted-content footprint since each open session pins a fully-decrypted bundle in memory. **The cap is observable (register row 53):** open (unexpired) sessions are published as the `scadabridge.central.transport.import_sessions.open` gauge on the `ZB.MOM.WW.ScadaBridge` meter — already in the `ObservedMeters` allowlist, so no telemetry config change is needed — and a rejection at the cap logs at Warning with the current count, the cap and the TTL. Reclaiming a store full of *expired* sessions is not a cap hit and logs nothing. **A log line is not alertable, so register row 66 adds the counter `scadabridge.central.transport.import_sessions.rejected`** on the same meter, incremented at the same single genuine-rejection point (the post-eviction recheck — never on the reclaim-then-succeed path, and never when re-opening an already-open session id). Alert on the counter; interpret it against the gauge — rejections rising while the gauge stays below the cap means genuine concurrency (raise the cap), whereas rejections rising while the gauge is pinned at the cap means a leaked session (raising the cap only defers it). +The component is central-hosted. It is registered in `ZB.MOM.WW.ScadaBridge.Host` for central roles only, never for site roles — even when a bundle carries site-scoped config (T18), Transport only writes the central configuration database, never the site clusters. All persistence flows through existing audited repository interfaces in `ZB.MOM.WW.ScadaBridge.ConfigurationDatabase` — the component does not call `DbContext.SaveChangesAsync` directly. `BundleSessionStore` is in-process on the active central node (matching Blazor Server circuit affinity): 30-minute TTL, eviction on expiry, 3-strike passphrase lockout per session, and a `MaxConcurrentImportSessions` cap (default 8; arch-review 05 Task 24 — startup-validated as `> 0` by `TransportOptionsValidator`, since a zero/negative cap would reject every import session forever) bounding the decrypted-content footprint since each open session pins its decrypted content in memory. **Resident footprint (review 2026-10-21 04/M2):** the session holds a Brotli-compressed copy of the decrypted content JSON — not the plaintext, and not a parsed DTO (the `SecretsBlock` values are plaintext once the content is decrypted, and immutable strings cannot be zeroed; a byte buffer can) — and `LoadAsync` zeroes the plaintext as soon as it is compressed; `PreviewAsync`/`ApplyAsync` deserialise straight from the compressed stream. The buffered upload zip is released once the manifest and content entry are extracted. Worst case per session is still bounded only by `MaxBundleEntryDecompressedMb` (200 MB — an adversarial incompressible payload), so the hard ceiling is `MaxConcurrentImportSessions` × ~200 MB (1.6 GB at the default 8); a real export compresses ~5–10x, so typically 8 × 20–40 MB at the largest bundle size. One in-flight `LoadAsync` additionally peaks at roughly the zip (≤ `MaxBundleSizeMb`) plus the content entry and its decrypted copy (each ≤ `MaxBundleEntryDecompressedMb`). **The cap is observable (register row 53):** open (unexpired) sessions are published as the `scadabridge.central.transport.import_sessions.open` gauge on the `ZB.MOM.WW.ScadaBridge` meter — already in the `ObservedMeters` allowlist, so no telemetry config change is needed — and a rejection at the cap logs at Warning with the current count, the cap and the TTL. Reclaiming a store full of *expired* sessions is not a cap hit and logs nothing. **A log line is not alertable, so register row 66 adds the counter `scadabridge.central.transport.import_sessions.rejected`** on the same meter, incremented at the same single genuine-rejection point (the post-eviction recheck — never on the reclaim-then-succeed path, and never when re-opening an already-open session id). Alert on the counter; interpret it against the gauge — rejections rising while the gauge stays below the cap means genuine concurrency (raise the cap), whereas rejections rising while the gauge is pinned at the cap means a leaked session (raising the cap only defers it). **Session release is explicit, not TTL-only (register row 43).** `ApplyAsync` releases the session it applied (zero-then-remove, T-007), and `IBundleImporter.Abandon(sessionId)` does the same for a session that will never be applied. Every abandonment path calls it: the Central UI wizard on new upload / Back-to-Upload / `ResetSessionState` / lockout reset / `Dispose` (navigation away or circuit teardown), the CLI `transport preview` handler (a preview never applies), and the CLI `transport import` handler for the aborts that precede apply (blocker rows, unresolved mappings). `Abandon` is idempotent — double release, an unknown id, and an already-expired id are all silent no-ops — and since session ids are random GUIDs handed only to their opener, abandoning the id you hold can never evict another operator's in-progress import. Relying on the TTL alone was an availability *and* secret-lifetime defect: eight abandoned wizards exhausted `MaxConcurrentImportSessions` and locked out every import on the active node for 30 minutes, each still holding its decrypted bundle content. **TTL expiry zeroes too (review 2026-10-21 04/C3):** a session dropped because it expired — lazily on `Get`, or by the `EvictExpired` sweep — has its content buffer zeroed by `BundleSessionStore` as it is removed, so every release path (apply, abandon, expiry) follows the same zero-then-remove discipline. diff --git a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/BundleSession.cs b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/BundleSession.cs index 7a2c3a5b..f525dadf 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/BundleSession.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Commons/Types/Transport/BundleSession.cs @@ -16,7 +16,12 @@ public sealed class BundleSession public Guid SessionId { get; init; } /// Parsed manifest from the uploaded bundle. public BundleManifest Manifest { get; init; } = null!; - /// Decrypted bundle content bytes; empty until the bundle is successfully unlocked. + /// + /// The decrypted bundle content, held Brotli-compressed by the importer (review + /// 2026-10-21 04/M2) — still sensitive (it decompresses to the plaintext content, + /// secret values included), so every release path zeroes it before dropping the + /// session. Empty until the bundle is successfully unlocked. + /// public byte[] DecryptedContent { get; init; } = Array.Empty(); /// UTC timestamp after which this session is considered expired and must be re-uploaded. public DateTimeOffset ExpiresAt { get; init; } diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs index 9f7438cc..dd56c664 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/Import/BundleImporter.cs @@ -184,54 +184,11 @@ public sealed class BundleImporter : IBundleImporter { ArgumentNullException.ThrowIfNull(bundleStream); - // Copy to a seekable buffer — manifest + content readers each open a - // fresh ZipArchive over the same bytes, so the upstream stream needs to - // be seekable. A caller-supplied FileStream is seekable but a Kestrel - // request stream is not, so we always normalise to MemoryStream. - var ms = new MemoryStream(); - await bundleStream.CopyToAsync(ms, ct).ConfigureAwait(false); - ms.Position = 0; - - // Size cap is in MB; multiply in long arithmetic so the comparison - // doesn't overflow at the int boundary for large MaxBundleSizeMb. - var maxBytes = _options.Value.MaxBundleSizeMb * 1024L * 1024L; - if (ms.Length > maxBytes) - { - throw new InvalidOperationException( - $"Bundle exceeds maximum allowed size of {_options.Value.MaxBundleSizeMb} MB."); - } - - // T-006: zip-bomb / decompression-bomb defences. We enforce three caps - // BEFORE any entry is decompressed by ReadManifest / ReadContentBytes: - // 1) total entry count - // 2) per-entry decompressed length - // 3) per-entry compression ratio (Length / CompressedLength) - // Each cap is configurable via TransportOptions so an operator can tune - // for an environment with legitimately large or unusually compressible - // bundles. Using ZipArchiveEntry.Length / .CompressedLength avoids - // reading the entry payload at all. - ms.Position = 0; - ValidateArchiveEnvelope(ms); - - BundleManifest manifest; - try - { - ms.Position = 0; - manifest = _bundleSerializer.ReadManifest(ms); - } - catch (InvalidDataException) - { - // Preserve the serializer's specific "manifest missing/null" message - // — the caller wants to surface a precise diagnostic to the operator. - throw; - } - catch (Exception ex) - { - throw new InvalidDataException("Bundle is missing or has a malformed manifest.json.", ex); - } - - ms.Position = 0; - var contentBytes = _bundleSerializer.ReadContentBytes(ms, manifest); + // Review 2026-10-21 04/M2: the whole uploaded zip (≤ MaxBundleSizeMb) is + // buffered inside ReadEnvelopeAsync only, so it becomes unreachable as soon + // as the manifest and the content entry have been extracted, rather than + // staying live alongside the decrypted copy for the rest of this method. + var (manifest, contentBytes) = await ReadEnvelopeAsync(bundleStream, ct).ConfigureAwait(false); // Validate format version + content-hash + manifest shape. Reject paths // surface as distinct exceptions so the UI can disambiguate the cause. @@ -338,17 +295,97 @@ public sealed class BundleImporter : IBundleImporter decryptedContent = contentBytes; } + // Review 2026-10-21 04/M2: the session pins a Brotli-compressed copy of the + // decrypted content JSON, not the plaintext itself — bundle JSON (script + // bodies, config) typically compresses ~5–10x, and the session lives up to + // BundleSessionTtlMinutes. A parsed DTO was rejected as the resident form: + // the bundle's SecretsBlock values are plaintext strings once the content is + // decrypted, and immutable strings cannot be zeroed — a byte[] can, so the + // zero-then-remove discipline (T-007, 04/C3) still covers the only resident + // copy. The plaintext buffer is zeroed as soon as it has been compressed. + byte[] sessionContent; + try + { + sessionContent = CompressSessionContent(decryptedContent); + } + finally + { + Array.Clear(decryptedContent, 0, decryptedContent.Length); + } + var ttl = TimeSpan.FromMinutes(_options.Value.BundleSessionTtlMinutes); var session = new BundleSession { SessionId = Guid.NewGuid(), Manifest = manifest, - DecryptedContent = decryptedContent, + DecryptedContent = sessionContent, ExpiresAt = _timeProvider.GetUtcNow() + ttl, }; return _sessionStore.Open(session); } + /// + /// Buffers the uploaded bundle, enforces the size and zip-envelope caps, and + /// extracts the manifest plus the raw content entry. The buffered zip lives only + /// for the duration of this call (review 2026-10-21 04/M2). + /// + /// The uploaded bundle stream (need not be seekable). + /// Cancellation token. + /// The parsed manifest and the content entry's bytes (ciphertext when encrypted). + private async Task<(BundleManifest Manifest, byte[] ContentBytes)> ReadEnvelopeAsync( + Stream bundleStream, CancellationToken ct) + { + // Copy to a seekable buffer — manifest + content readers each open a + // fresh ZipArchive over the same bytes, so the upstream stream needs to + // be seekable. A caller-supplied FileStream is seekable but a Kestrel + // request stream is not, so we always normalise to MemoryStream. + var ms = new MemoryStream(); + await bundleStream.CopyToAsync(ms, ct).ConfigureAwait(false); + ms.Position = 0; + + // Size cap is in MB; multiply in long arithmetic so the comparison + // doesn't overflow at the int boundary for large MaxBundleSizeMb. + var maxBytes = _options.Value.MaxBundleSizeMb * 1024L * 1024L; + if (ms.Length > maxBytes) + { + throw new InvalidOperationException( + $"Bundle exceeds maximum allowed size of {_options.Value.MaxBundleSizeMb} MB."); + } + + // T-006: zip-bomb / decompression-bomb defences. We enforce three caps + // BEFORE any entry is decompressed by ReadManifest / ReadContentBytes: + // 1) total entry count + // 2) per-entry decompressed length + // 3) per-entry compression ratio (Length / CompressedLength) + // Each cap is configurable via TransportOptions so an operator can tune + // for an environment with legitimately large or unusually compressible + // bundles. Using ZipArchiveEntry.Length / .CompressedLength avoids + // reading the entry payload at all. + ms.Position = 0; + ValidateArchiveEnvelope(ms); + + BundleManifest manifest; + try + { + ms.Position = 0; + manifest = _bundleSerializer.ReadManifest(ms); + } + catch (InvalidDataException) + { + // Preserve the serializer's specific "manifest missing/null" message + // — the caller wants to surface a precise diagnostic to the operator. + throw; + } + catch (Exception ex) + { + throw new InvalidDataException("Bundle is missing or has a malformed manifest.json.", ex); + } + + ms.Position = 0; + var contentBytes = _bundleSerializer.ReadContentBytes(ms, manifest); + return (manifest, contentBytes); + } + /// /// T-006: validates the zip envelope against the configured caps BEFORE any /// entry payload is decompressed. Reads only the central-directory headers @@ -410,16 +447,7 @@ public sealed class BundleImporter : IBundleImporter throw new InvalidOperationException($"Bundle session {sessionId} is locked."); } - BundleContentDto content; - try - { - content = JsonSerializer.Deserialize(session.DecryptedContent, ContentJsonOptions) - ?? throw new InvalidDataException("Session content deserialized to null."); - } - catch (JsonException ex) - { - throw new InvalidDataException("Session content is not a valid BundleContentDto.", ex); - } + var content = ReadSessionContent(session); var items = new List(); @@ -1317,16 +1345,7 @@ public sealed class BundleImporter : IBundleImporter throw new InvalidOperationException($"Bundle session {sessionId} is locked."); } - BundleContentDto content; - try - { - content = JsonSerializer.Deserialize(session.DecryptedContent, ContentJsonOptions) - ?? throw new InvalidDataException("Session content deserialized to null."); - } - catch (JsonException ex) - { - throw new InvalidDataException("Session content is not a valid BundleContentDto.", ex); - } + var content = ReadSessionContent(session); var bundleImportId = Guid.NewGuid(); // Set the correlation BEFORE the transaction so any audit writes @@ -1749,6 +1768,45 @@ public sealed class BundleImporter : IBundleImporter /// private void BumpGraphWatermark() => _watermark?.BumpAll(); + /// + /// Review 2026-10-21 04/M2: compresses decrypted bundle content JSON into the + /// form pinned on a . The caller owns (and zeroes) + /// . + /// + /// The decrypted content JSON. + /// The Brotli-compressed content. + internal static byte[] CompressSessionContent(byte[] plaintext) + { + using var output = new MemoryStream(capacity: Math.Max(256, plaintext.Length / 4)); + using (var brotli = new BrotliStream(output, CompressionLevel.Fastest, leaveOpen: true)) + { + brotli.Write(plaintext, 0, plaintext.Length); + } + return output.ToArray(); + } + + /// + /// Review 2026-10-21 04/M2: deserializes a session's content straight from its + /// compressed form, so no plaintext copy of the content JSON is materialised. + /// + /// The open bundle session. + /// The bundle content DTO. + /// The session content is not a valid . + internal static BundleContentDto ReadSessionContent(BundleSession session) + { + try + { + using var compressed = new MemoryStream(session.DecryptedContent, writable: false); + using var brotli = new BrotliStream(compressed, CompressionMode.Decompress); + return JsonSerializer.Deserialize(brotli, ContentJsonOptions) + ?? throw new InvalidDataException("Session content deserialized to null."); + } + catch (JsonException ex) + { + throw new InvalidDataException("Session content is not a valid BundleContentDto.", ex); + } + } + /// /// T-007: zeros the session's /// buffer in place so any caller still holding a reference observes the diff --git a/src/ZB.MOM.WW.ScadaBridge.Transport/TransportOptions.cs b/src/ZB.MOM.WW.ScadaBridge.Transport/TransportOptions.cs index 6b6c9051..a94ee16b 100644 --- a/src/ZB.MOM.WW.ScadaBridge.Transport/TransportOptions.cs +++ b/src/ZB.MOM.WW.ScadaBridge.Transport/TransportOptions.cs @@ -30,9 +30,20 @@ public sealed class TransportOptions public int MaxBundleEntryCompressionRatio { get; set; } = 50; /// /// Maximum number of concurrently-open import sessions. Each open - /// session pins a fully-decrypted bundle (up to - /// of plaintext) in memory until it expires or is applied/cancelled, so this - /// bounds the N×~200 MB decrypted-content footprint the central node can hold. + /// session pins a Brotli-compressed copy of its decrypted content JSON in memory + /// until it expires or is applied/cancelled (review 2026-10-21 04/M2). + /// + /// Worst-case resident footprint: the decrypted content is bounded by + /// (200 MB), and an adversarial, + /// incompressible payload can keep its compressed copy close to that size, so + /// the hard ceiling is N × ~200 MB (1.6 GB at the default 8). A real export + /// compresses ~5–10x, so the typical footprint is N × 20–40 MB at the largest + /// bundle size. A single in-flight LoadAsync additionally peaks at + /// roughly the uploaded zip (≤ , released once the + /// entries are extracted) plus the content entry and its decrypted copy (each + /// ≤ ), and each PreviewAsync/ + /// ApplyAsync materialises one content DTO graph while it runs. + /// /// public int MaxConcurrentImportSessions { get; set; } = 8; /// Gets or sets the maximum number of failed passphrase unlock attempts before a session is locked. diff --git a/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Import/BundleImporterLoadTests.cs b/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Import/BundleImporterLoadTests.cs index a0bbad00..7ff07877 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Import/BundleImporterLoadTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.Transport.Tests/Import/BundleImporterLoadTests.cs @@ -197,8 +197,7 @@ public sealed class BundleImporterLoadTests Assert.NotEmpty(session.DecryptedContent); // The decrypted payload must round-trip back to the original DTO so the // PreviewAsync phase can deserialize it directly from the session. - var roundTripped = JsonSerializer.Deserialize( - session.DecryptedContent, BundleJsonOptions); + var roundTripped = BundleImporter.ReadSessionContent(session); Assert.NotNull(roundTripped); Assert.Single(roundTripped!.Templates); Assert.Equal("Pump", roundTripped.Templates[0].Name); @@ -718,6 +717,37 @@ public sealed class BundleImporterLoadTests Assert.NotNull(otherSession); } + // ───────────────────────────────────────────────────────────────────── + // Review 2026-10-21 04/M2 — resident session footprint. + // ───────────────────────────────────────────────────────────────────── + + /// + /// Verifies that an open session pins a compressed copy of the decrypted content + /// rather than the raw plaintext JSON, and that the copy round-trips to the DTO. + /// + /// A task that represents the asynchronous operation. + [Fact] + public async Task LoadAsync_pins_a_compressed_copy_of_the_content_on_the_session() + { + var rig = BuildRig(); + var bigDescription = string.Concat(Enumerable.Repeat("pump station telemetry ", 20_000)); + var content = SmallContent() with + { + Templates = new[] { SmallContent().Templates[0] with { Description = bigDescription } }, + }; + var plaintextLength = JsonSerializer.SerializeToUtf8Bytes(content, BundleJsonOptions).Length; + using var packed = PackEncryptedBundle( + rig.Serializer, rig.ManifestBuilder, rig.Encryptor, content, "secret123"); + + var session = await rig.Importer.LoadAsync(packed, passphrase: "secret123"); + + Assert.True( + session.DecryptedContent.Length < plaintextLength / 4, + $"session pins {session.DecryptedContent.Length} bytes for {plaintextLength} bytes of plaintext"); + var roundTripped = BundleImporter.ReadSessionContent(session); + Assert.Equal(bigDescription, roundTripped.Templates[0].Description); + } + // ───────────────────────────────────────────────────────────────────── // Register row 43 — abandoned-session release. // ─────────────────────────────────────────────────────────────────────