fix(transport): pin a compressed copy of the decrypted bundle on the import session, release the upload buffer early, document the footprint (review 2026-10-21 04/M2)

This commit is contained in:
Joseph Doherty
2026-10-22 09:20:31 -04:00
parent 55cebf2397
commit 93fbd2e030
5 changed files with 180 additions and 76 deletions
+1 -1
View File
@@ -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.
@@ -16,7 +16,12 @@ public sealed class BundleSession
public Guid SessionId { get; init; }
/// <summary>Parsed manifest from the uploaded bundle.</summary>
public BundleManifest Manifest { get; init; } = null!;
/// <summary>Decrypted bundle content bytes; empty until the bundle is successfully unlocked.</summary>
/// <summary>
/// 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.
/// </summary>
public byte[] DecryptedContent { get; init; } = Array.Empty<byte>();
/// <summary>UTC timestamp after which this session is considered expired and must be re-uploaded.</summary>
public DateTimeOffset ExpiresAt { get; init; }
@@ -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);
}
/// <summary>
/// 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).
/// </summary>
/// <param name="bundleStream">The uploaded bundle stream (need not be seekable).</param>
/// <param name="ct">Cancellation token.</param>
/// <returns>The parsed manifest and the content entry's bytes (ciphertext when encrypted).</returns>
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);
}
/// <summary>
/// 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<BundleContentDto>(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<ImportPreviewItem>();
@@ -1317,16 +1345,7 @@ public sealed class BundleImporter : IBundleImporter
throw new InvalidOperationException($"Bundle session {sessionId} is locked.");
}
BundleContentDto content;
try
{
content = JsonSerializer.Deserialize<BundleContentDto>(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
/// </summary>
private void BumpGraphWatermark() => _watermark?.BumpAll();
/// <summary>
/// Review 2026-10-21 04/M2: compresses decrypted bundle content JSON into the
/// form pinned on a <see cref="BundleSession"/>. The caller owns (and zeroes)
/// <paramref name="plaintext"/>.
/// </summary>
/// <param name="plaintext">The decrypted content JSON.</param>
/// <returns>The Brotli-compressed content.</returns>
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();
}
/// <summary>
/// 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.
/// </summary>
/// <param name="session">The open bundle session.</param>
/// <returns>The bundle content DTO.</returns>
/// <exception cref="InvalidDataException">The session content is not a valid <see cref="BundleContentDto"/>.</exception>
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<BundleContentDto>(brotli, ContentJsonOptions)
?? throw new InvalidDataException("Session content deserialized to null.");
}
catch (JsonException ex)
{
throw new InvalidDataException("Session content is not a valid BundleContentDto.", ex);
}
}
/// <summary>
/// T-007: zeros the session's <see cref="BundleSession.DecryptedContent"/>
/// buffer in place so any caller still holding a reference observes the
@@ -30,9 +30,20 @@ public sealed class TransportOptions
public int MaxBundleEntryCompressionRatio { get; set; } = 50;
/// <summary>
/// Maximum number of concurrently-open import sessions. Each open
/// session pins a fully-decrypted bundle (up to <see cref="MaxBundleSizeMb"/>
/// 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).
/// <para>
/// Worst-case resident footprint: the decrypted content is bounded by
/// <see cref="MaxBundleEntryDecompressedMb"/> (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 <c>LoadAsync</c> additionally peaks at
/// roughly the uploaded zip (≤ <see cref="MaxBundleSizeMb"/>, released once the
/// entries are extracted) plus the content entry and its decrypted copy (each
/// ≤ <see cref="MaxBundleEntryDecompressedMb"/>), and each <c>PreviewAsync</c>/
/// <c>ApplyAsync</c> materialises one content DTO graph while it runs.
/// </para>
/// </summary>
public int MaxConcurrentImportSessions { get; set; } = 8;
/// <summary>Gets or sets the maximum number of failed passphrase unlock attempts before a session is locked.</summary>
@@ -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<BundleContentDto>(
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.
// ─────────────────────────────────────────────────────────────────────
/// <summary>
/// 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.
/// </summary>
/// <returns>A task that represents the asynchronous operation.</returns>
[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.
// ─────────────────────────────────────────────────────────────────────