diff --git a/code-reviews/NotificationOutbox/findings.md b/code-reviews/NotificationOutbox/findings.md
index 75e2951a..f2edff0a 100644
--- a/code-reviews/NotificationOutbox/findings.md
+++ b/code-reviews/NotificationOutbox/findings.md
@@ -439,7 +439,7 @@ _Unresolved._
|--|--|
| Severity | Medium |
| Category | Documentation & comments |
-| Status | Open |
+| Status | Resolved |
| Location | `src/ScadaLink.NotificationOutbox/NotificationOutboxActor.cs:469-477` |
**Description**
diff --git a/code-reviews/README.md b/code-reviews/README.md
index b8a3989c..8b56cf28 100644
--- a/code-reviews/README.md
+++ b/code-reviews/README.md
@@ -40,10 +40,10 @@ module file and counted in **Total**.
| Severity | Open findings |
|----------|---------------|
| Critical | 0 |
-| High | 13 |
-| Medium | 56 |
+| High | 12 |
+| Medium | 52 |
| Low | 90 |
-| **Total** | **159** |
+| **Total** | **154** |
## Module Status
@@ -63,7 +63,7 @@ module file and counted in **Total**.
| [Host](Host/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/2/5 | 7 | 22 |
| [InboundAPI](InboundAPI/findings.md) | 2026-05-28 | `1eb6e97` | 0/1/3/4 | 8 | 25 |
| [ManagementService](ManagementService/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/2/2 | 4 | 23 |
-| [NotificationOutbox](NotificationOutbox/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/3/3 | 6 | 10 |
+| [NotificationOutbox](NotificationOutbox/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/2/3 | 5 | 10 |
| [NotificationService](NotificationService/findings.md) | 2026-05-28 | `1eb6e97` | 0/1/2/3 | 6 | 25 |
| [Security](Security/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/0/2 | 2 | 21 |
| [SiteCallAudit](SiteCallAudit/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/2/4 | 6 | 6 |
@@ -71,7 +71,7 @@ module file and counted in **Total**.
| [SiteRuntime](SiteRuntime/findings.md) | 2026-05-28 | `1eb6e97` | 0/0/4/3 | 7 | 26 |
| [StoreAndForward](StoreAndForward/findings.md) | 2026-05-28 | `1eb6e97` | 0/1/3/3 | 7 | 24 |
| [TemplateEngine](TemplateEngine/findings.md) | 2026-05-28 | `1eb6e97` | 0/1/4/1 | 6 | 22 |
-| [Transport](Transport/findings.md) | 2026-05-28 | `1eb6e97` | 0/3/5/4 | 12 | 12 |
+| [Transport](Transport/findings.md) | 2026-05-28 | `1eb6e97` | 0/2/2/4 | 8 | 12 |
## Pending Findings
@@ -84,7 +84,7 @@ description, location, recommendation — lives in the module's `findings.md`.
_None open._
-### High (13)
+### High (12)
| ID | Module | Title |
|----|--------|-------|
@@ -100,9 +100,8 @@ _None open._
| TemplateEngine-017 | [TemplateEngine](TemplateEngine/findings.md) | Revision hash and diff both ignore `Description` and `Connections`, defeating staleness detection for real deployment changes |
| Transport-001 | [Transport](Transport/findings.md) | Template Overwrite never syncs attributes / alarms / scripts |
| Transport-002 | [Transport](Transport/findings.md) | ExternalSystem Overwrite never syncs methods |
-| Transport-003 | [Transport](Transport/findings.md) | Unlock lockout is enforced only client-side; server session is never marked Locked |
-### Medium (56)
+### Medium (52)
| ID | Module | Title |
|----|--------|-------|
@@ -139,7 +138,6 @@ _None open._
| ManagementService-021 | [ManagementService](ManagementService/findings.md) | Transport bundle handlers have zero test coverage |
| NotificationOutbox-005 | [NotificationOutbox](NotificationOutbox/findings.md) | Ingest persistence inherits the CD-015 check-then-act race; under contention the second writer throws and the site retries |
| NotificationOutbox-007 | [NotificationOutbox](NotificationOutbox/findings.md) | `NotificationOutboxOptions.DispatchBatchSize`, `DeliveredKpiWindow`, and `PurgeInterval` are not in the design document |
-| NotificationOutbox-010 | [NotificationOutbox](NotificationOutbox/findings.md) | Comment claims `PipeTo` is not used "because the writer never throws"; the surrounding try/catch is dead-letter for the documented failure mode |
| NotificationService-020 | [NotificationService](NotificationService/findings.md) | NS-001 fix superseded; `AkkaHostedService` would register two competing `Notification` S&F handlers if both code paths ran |
| NotificationService-024 | [NotificationService](NotificationService/findings.md) | No test affirms the central-only invariant; the orphaned-path tests give a false coverage signal |
| SiteCallAudit-001 | [SiteCallAudit](SiteCallAudit/findings.md) | SupervisorStrategy override is dead code; XML claims Resume that is not enforced |
@@ -158,9 +156,6 @@ _None open._
| TemplateEngine-020 | [TemplateEngine](TemplateEngine/findings.md) | `Create*` audit entries are written with `EntityId = "0"` before `SaveChangesAsync` populates the real key |
| TemplateEngine-021 | [TemplateEngine](TemplateEngine/findings.md) | `MoveTemplateAsync` skips folder cycle and sibling-name-collision validation |
| Transport-004 | [Transport](Transport/findings.md) | `MaxUnlockAttemptsPerIpPerHour` option is declared but never enforced |
-| Transport-005 | [Transport](Transport/findings.md) | Manifest fields outside `ContentHash` are not bound to the encrypted payload |
-| Transport-006 | [Transport](Transport/findings.md) | Bundle ZIP read has no per-entry size cap or entry-count cap (zip-bomb / decompression-bomb) |
-| Transport-007 | [Transport](Transport/findings.md) | Failed import sessions retain decrypted plaintext for the full 30-minute TTL |
| Transport-010 | [Transport](Transport/findings.md) | Critical Overwrite + cross-cutting paths uncovered by tests |
### Low (90)
diff --git a/code-reviews/Transport/findings.md b/code-reviews/Transport/findings.md
index 8d4bf376..22751c65 100644
--- a/code-reviews/Transport/findings.md
+++ b/code-reviews/Transport/findings.md
@@ -121,7 +121,7 @@ _Unresolved._
|--|--|
| Severity | High |
| Category | Security |
-| Status | Open |
+| Status | Resolved |
| Location | `src/ScadaLink.Transport/Import/BundleImporter.cs:184-203`, `src/ScadaLink.CentralUI/Components/Pages/Design/TransportImport.razor.cs:267-309`, `src/ScadaLink.Commons/Types/Transport/BundleSession.cs:14-16` |
**Description**
@@ -190,7 +190,7 @@ _Unresolved._
|--|--|
| Severity | Medium |
| Category | Security |
-| Status | Open |
+| Status | Resolved |
| Location | `src/ScadaLink.Transport/Encryption/BundleSecretEncryptor.cs:31-49`, `src/ScadaLink.Transport/Serialization/ManifestValidator.cs:29-53` |
**Description**
@@ -226,7 +226,7 @@ _Unresolved._
|--|--|
| Severity | Medium |
| Category | Security |
-| Status | Open |
+| Status | Resolved |
| Location | `src/ScadaLink.Transport/Serialization/BundleSerializer.cs:121-156`, `src/ScadaLink.Transport/Import/BundleImporter.cs:132-143` |
**Description**
@@ -259,7 +259,7 @@ _Unresolved._
|--|--|
| Severity | Medium |
| Category | Performance & resource management |
-| Status | Open |
+| Status | Resolved |
| Location | `src/ScadaLink.Transport/Import/BundleImporter.cs:614-696`, `src/ScadaLink.Transport/Import/BundleSessionStore.cs:67-93` |
**Description**
diff --git a/src/ScadaLink.CentralUI/Components/Pages/Design/TransportImport.razor.cs b/src/ScadaLink.CentralUI/Components/Pages/Design/TransportImport.razor.cs
index f18394c6..81492135 100644
--- a/src/ScadaLink.CentralUI/Components/Pages/Design/TransportImport.razor.cs
+++ b/src/ScadaLink.CentralUI/Components/Pages/Design/TransportImport.razor.cs
@@ -243,9 +243,15 @@ public partial class TransportImport : ComponentBase
// ============================================================
///
- /// Submits the entered passphrase. On
- /// increments the per-session counter; once the configured threshold is
- /// reached the wizard resets to Step 1 with an explanatory error.
+ /// Submits the entered passphrase.
+ ///
+ /// T-003: lockout enforcement is now server-side and keyed by the bundle's
+ /// content hash. means "wrong passphrase,
+ /// try again"; a means the importer has
+ /// observed enough failures against this bundle to lock it (the count is
+ /// shared across tabs / CLI / circuits). The Razor counter is kept ONLY for
+ /// display ("3 of N attempts used") — it is no longer the source of truth.
+ ///
///
private async Task SubmitPassphraseAsync()
{
@@ -264,37 +270,28 @@ public partial class TransportImport : ComponentBase
await LoadPreviewAndAdvanceAsync();
}
}
+ catch (BundleLockedException ex)
+ {
+ // T-003: server-side lockout reached. Emit a final audit row so the
+ // lockout is visible in the audit log, reset the wizard, and surface
+ // the typed message verbatim.
+ _passphrase = string.Empty;
+ _failedUnlockAttempts = ex.FailedAttempts;
+ await EmitUnlockFailedAuditRowAsync(ex.BundleContentHash, ex.FailedAttempts, ex.Message);
+ _errorMessage = ex.Message;
+ ResetSessionState();
+ _step = ImportWizardStep.Upload;
+ }
catch (CryptographicException ex)
{
_failedUnlockAttempts++;
_passphrase = string.Empty;
- // Emit audit row for every wrong-passphrase attempt (BundleImportUnlockFailed).
- // Best-effort — audit failure must never abort the user-facing action.
- try
- {
- var user = await Auth.GetCurrentUsernameAsync();
- var entityId = _session?.Manifest.ContentHash ?? "";
- var entityName = _session?.Manifest.SourceEnvironment ?? "";
- await AuditService.LogAsync(
- user: user,
- action: "BundleImportUnlockFailed",
- entityType: "Bundle",
- entityId: entityId,
- entityName: entityName,
- afterState: new
- {
- AttemptNumber = _failedUnlockAttempts,
- Reason = ex.Message,
- },
- cancellationToken: CancellationToken.None);
- await DbContext.SaveChangesAsync();
- }
- catch
- {
- // Audit failure is non-fatal — swallow and continue.
- }
+ var entityId = _session?.Manifest.ContentHash ?? "";
+ await EmitUnlockFailedAuditRowAsync(entityId, _failedUnlockAttempts, ex.Message);
+ // The server tracks the authoritative counter; the local count is
+ // kept in sync for the Razor display only.
if (_failedUnlockAttempts >= Options.Value.MaxUnlockAttemptsPerSession)
{
_errorMessage =
@@ -318,6 +315,37 @@ public partial class TransportImport : ComponentBase
}
}
+ ///
+ /// T-003: best-effort audit row for a wrong-passphrase attempt. Audit failure
+ /// must never abort the user-facing action — same defensive pattern as the
+ /// original page used.
+ ///
+ private async Task EmitUnlockFailedAuditRowAsync(string entityId, int attemptNumber, string reason)
+ {
+ try
+ {
+ var user = await Auth.GetCurrentUsernameAsync();
+ var entityName = _session?.Manifest.SourceEnvironment ?? "";
+ await AuditService.LogAsync(
+ user: user,
+ action: "BundleImportUnlockFailed",
+ entityType: "Bundle",
+ entityId: entityId,
+ entityName: entityName,
+ afterState: new
+ {
+ AttemptNumber = attemptNumber,
+ Reason = reason,
+ },
+ cancellationToken: CancellationToken.None);
+ await DbContext.SaveChangesAsync();
+ }
+ catch
+ {
+ // Audit failure is non-fatal — swallow and continue.
+ }
+ }
+
private void BackToUpload()
{
_step = ImportWizardStep.Upload;
diff --git a/src/ScadaLink.Commons/Interfaces/Transport/IBundleSessionStore.cs b/src/ScadaLink.Commons/Interfaces/Transport/IBundleSessionStore.cs
index cea04d63..666bcbc4 100644
--- a/src/ScadaLink.Commons/Interfaces/Transport/IBundleSessionStore.cs
+++ b/src/ScadaLink.Commons/Interfaces/Transport/IBundleSessionStore.cs
@@ -15,4 +15,29 @@ public interface IBundleSessionStore
void Remove(Guid sessionId);
/// Removes all sessions whose expiry has passed.
void EvictExpired();
+
+ ///
+ /// T-003: returns the current unlock-failure count for a bundle keyed by its
+ /// content hash. The counter is server-owned so a second tab / CLI caller
+ /// cannot side-step the lockout by re-uploading the same bytes.
+ ///
+ /// SHA-256 hex from BundleManifest.ContentHash.
+ /// Number of recorded failures for this bundle (0 if none, or if any record has expired).
+ int GetUnlockFailureCount(string bundleContentHash);
+
+ ///
+ /// T-003: atomically increments the unlock-failure counter for a bundle and
+ /// returns the new count. Tracking is scoped by content hash so retries
+ /// against identical bundle bytes are throttled regardless of client.
+ ///
+ /// SHA-256 hex from BundleManifest.ContentHash.
+ int IncrementUnlockFailureCount(string bundleContentHash);
+
+ ///
+ /// T-003: clears the unlock-failure counter for a bundle (called on a
+ /// successful unlock so a legitimate operator who eventually types the
+ /// right passphrase is not penalised for earlier typos).
+ ///
+ /// SHA-256 hex from BundleManifest.ContentHash.
+ void ClearUnlockFailures(string bundleContentHash);
}
diff --git a/src/ScadaLink.Commons/Types/Transport/BundleSession.cs b/src/ScadaLink.Commons/Types/Transport/BundleSession.cs
index defed650..82942684 100644
--- a/src/ScadaLink.Commons/Types/Transport/BundleSession.cs
+++ b/src/ScadaLink.Commons/Types/Transport/BundleSession.cs
@@ -10,8 +10,18 @@ public sealed class BundleSession
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; }
- /// Number of failed passphrase unlock attempts for this session.
+ ///
+ /// T-003: legacy per-session unlock-attempt counter. The unlock lockout is now
+ /// owned by IBundleSessionStore and keyed by BundleManifest.ContentHash
+ /// so retries from a second tab / CLI caller share the counter. A successful
+ /// LoadAsync never increments this — it stays 0 on every opened session,
+ /// and is unreachable on a session returned by the store.
+ /// Retained as a compatibility shim for callers/tests that still set it directly.
+ ///
public int FailedUnlockAttempts { get; set; }
- /// True when three or more unlock attempts have failed, locking further attempts.
+ ///
+ /// T-003 legacy: always false on a session returned by LoadAsync
+ /// because lockout enforcement moved server-side; see .
+ ///
public bool Locked => FailedUnlockAttempts >= 3;
}
diff --git a/src/ScadaLink.Transport/Encryption/BundleManifestAad.cs b/src/ScadaLink.Transport/Encryption/BundleManifestAad.cs
new file mode 100644
index 00000000..2b84a215
--- /dev/null
+++ b/src/ScadaLink.Transport/Encryption/BundleManifestAad.cs
@@ -0,0 +1,56 @@
+using System.Security.Cryptography;
+using System.Text.Json;
+using System.Text.Json.Serialization;
+using ScadaLink.Commons.Types.Transport;
+
+namespace ScadaLink.Transport.Encryption;
+
+///
+/// T-005: computes the AES-GCM Associated Authenticated Data (AAD) for a bundle's
+/// encrypted payload. AAD is the SHA-256 of the manifest after normalising the two
+/// derivative fields (ContentHash, Encryption) to known sentinel
+/// values — those depend on the ciphertext and the IV, so they cannot themselves
+/// be authenticated, but every OTHER manifest field (SourceEnvironment,
+/// ExportedBy, ScadaLinkVersion, Summary, Contents,
+/// CreatedAtUtc, …) participates in the GCM tag.
+///
+/// Threading this byte array through AesGcm.Encrypt / AesGcm.Decrypt
+/// makes the Step-4 "type the source environment name to confirm" gate
+/// tamper-evident: a flipped SourceEnvironment on a stolen bundle yields
+/// an AuthenticationTagMismatchException on decrypt instead of producing
+/// a valid plaintext with a forged origin label.
+///
+///
+public static class BundleManifestAad
+{
+ ///
+ /// JSON options matching BundleSerializer.JsonOptions so the AAD bytes
+ /// are stable across the encrypt + decrypt side.
+ ///
+ private static readonly JsonSerializerOptions JsonOptions = new()
+ {
+ WriteIndented = false,
+ DefaultIgnoreCondition = JsonIgnoreCondition.WhenWritingNull,
+ Converters = { new JsonStringEnumConverter() },
+ };
+
+ ///
+ /// Computes the AAD bytes for the supplied manifest. The two derivative
+ /// fields are normalised to fixed sentinels so the AAD is independent of the
+ /// ciphertext / IV that will eventually populate them in the on-disk
+ /// manifest.
+ ///
+ /// The manifest whose non-derivative fields should be authenticated.
+ /// SHA-256 digest of the canonicalised manifest bytes.
+ public static byte[] Compute(BundleManifest manifest)
+ {
+ ArgumentNullException.ThrowIfNull(manifest);
+ var canonical = manifest with
+ {
+ ContentHash = string.Empty,
+ Encryption = null,
+ };
+ var canonicalBytes = JsonSerializer.SerializeToUtf8Bytes(canonical, JsonOptions);
+ return SHA256.HashData(canonicalBytes);
+ }
+}
diff --git a/src/ScadaLink.Transport/Encryption/BundleSecretEncryptor.cs b/src/ScadaLink.Transport/Encryption/BundleSecretEncryptor.cs
index 73b36680..5a728422 100644
--- a/src/ScadaLink.Transport/Encryption/BundleSecretEncryptor.cs
+++ b/src/ScadaLink.Transport/Encryption/BundleSecretEncryptor.cs
@@ -22,11 +22,20 @@ public sealed class BundleSecretEncryptor
/// The data to encrypt.
/// The passphrase used to derive the encryption key.
/// PBKDF2 iteration count for key derivation.
+ ///
+ /// T-005: optional AES-GCM Associated Authenticated Data — typically
+ /// (manifest). Binds the manifest's
+ /// non-derivative fields (source environment, exporter, summary, …) to the
+ /// ciphertext so any tampering of those fields yields an authentication-tag
+ /// mismatch on decrypt. Pass for the
+ /// legacy no-AAD format; the decrypter must mirror the choice.
+ ///
/// The ciphertext (with appended GCM tag) and the encryption metadata needed to decrypt.
public (byte[] Ciphertext, EncryptionMetadata Metadata) Encrypt(
ReadOnlySpan plaintext,
string passphrase,
- int iterations)
+ int iterations,
+ ReadOnlySpan associatedData = default)
{
var salt = RandomNumberGenerator.GetBytes(SaltBytes);
var nonce = RandomNumberGenerator.GetBytes(NonceBytes);
@@ -35,7 +44,7 @@ public sealed class BundleSecretEncryptor
var ciphertext = new byte[plaintext.Length];
var tag = new byte[TagBytes];
using var aes = new AesGcm(key, TagBytes);
- aes.Encrypt(nonce, plaintext, ciphertext, tag);
+ aes.Encrypt(nonce, plaintext, ciphertext, tag, associatedData);
// Format: ciphertext || tag.
var output = new byte[ciphertext.Length + TagBytes];
@@ -54,8 +63,18 @@ public sealed class BundleSecretEncryptor
/// The ciphertext with appended GCM tag.
/// Encryption metadata carrying the algorithm, KDF, salt, nonce, and iterations.
/// The passphrase used to derive the decryption key.
+ ///
+ /// T-005: AAD that was passed to . Must match exactly —
+ /// any tampering of the manifest used to derive AAD yields an authentication-
+ /// tag mismatch (surfaces as , specifically
+ /// AuthenticationTagMismatchException on .NET 10).
+ ///
/// The decrypted plaintext bytes.
- public byte[] Decrypt(ReadOnlySpan payload, EncryptionMetadata metadata, string passphrase)
+ public byte[] Decrypt(
+ ReadOnlySpan payload,
+ EncryptionMetadata metadata,
+ string passphrase,
+ ReadOnlySpan associatedData = default)
{
ArgumentNullException.ThrowIfNull(metadata);
@@ -79,7 +98,7 @@ public sealed class BundleSecretEncryptor
var plaintext = new byte[ctLen];
using var aes = new AesGcm(key, TagBytes);
- aes.Decrypt(nonce, ciphertext, tag, plaintext);
+ aes.Decrypt(nonce, ciphertext, tag, plaintext, associatedData);
return plaintext;
}
diff --git a/src/ScadaLink.Transport/Import/BundleImporter.cs b/src/ScadaLink.Transport/Import/BundleImporter.cs
index 28366730..4e9d3cb4 100644
--- a/src/ScadaLink.Transport/Import/BundleImporter.cs
+++ b/src/ScadaLink.Transport/Import/BundleImporter.cs
@@ -1,3 +1,4 @@
+using System.IO.Compression;
using System.Security.Cryptography;
using System.Text.Json;
using System.Text.Json.Serialization;
@@ -142,6 +143,18 @@ public sealed class BundleImporter : IBundleImporter
$"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
{
@@ -181,12 +194,16 @@ public sealed class BundleImporter : IBundleImporter
throw new InvalidDataException($"Unrecognised manifest validation result: {validation}.");
}
- // Decrypt when the manifest carries EncryptionMetadata. AES-GCM tag
- // mismatch surfaces as a CryptographicException (or its
- // AuthenticationTagMismatchException subclass on .NET 10+) — bubble it
- // unchanged so the caller can detect wrong-passphrase via type check
- // and increment the lockout counter on the (about-to-be-rejected)
- // session reference. The session is not opened on the failure path.
+ // Decrypt when the manifest carries EncryptionMetadata.
+ //
+ // T-003: lockout enforcement is server-side and keyed by ContentHash so a
+ // second tab / CLI caller re-uploading the same bundle bytes cannot
+ // side-step the limit by skipping the Razor page. The counter is
+ // consulted BEFORE attempting decrypt (rejects further attempts on an
+ // already-locked bundle) and incremented on a CryptographicException
+ // from _encryptor.Decrypt; a successful decrypt clears the counter so a
+ // legitimate operator who eventually types the right passphrase is not
+ // penalised for earlier typos.
byte[] decryptedContent;
if (manifest.Encryption is not null)
{
@@ -195,7 +212,39 @@ public sealed class BundleImporter : IBundleImporter
throw new ArgumentException(
"Passphrase required for encrypted bundle.", nameof(passphrase));
}
- decryptedContent = _encryptor.Decrypt(contentBytes, manifest.Encryption, passphrase);
+
+ var maxAttempts = _options.Value.MaxUnlockAttemptsPerSession;
+ var priorFailures = _sessionStore.GetUnlockFailureCount(manifest.ContentHash);
+ if (priorFailures >= maxAttempts)
+ {
+ throw new BundleLockedException(manifest.ContentHash, priorFailures);
+ }
+
+ // T-005: bind the manifest's non-derivative fields into AES-GCM AAD so
+ // a tampered SourceEnvironment / ExportedBy / etc. yields an
+ // authentication-tag mismatch (surfaced as CryptographicException) on
+ // decrypt — preventing a forged origin label from slipping past the
+ // Step-4 typo-resistant confirmation gate.
+ var aad = Encryption.BundleManifestAad.Compute(manifest);
+ try
+ {
+ decryptedContent = _encryptor.Decrypt(contentBytes, manifest.Encryption, passphrase, aad);
+ }
+ catch (CryptographicException)
+ {
+ var newCount = _sessionStore.IncrementUnlockFailureCount(manifest.ContentHash);
+ if (newCount >= maxAttempts)
+ {
+ // Surface the lockout as the typed exception so the caller can
+ // distinguish "wrong passphrase, try again" from "no more attempts".
+ throw new BundleLockedException(manifest.ContentHash, newCount);
+ }
+ // Otherwise rebubble the CryptographicException so the UI's
+ // wrong-passphrase audit + retry path continues to work unchanged.
+ throw;
+ }
+
+ _sessionStore.ClearUnlockFailures(manifest.ContentHash);
}
else
{
@@ -213,6 +262,57 @@ public sealed class BundleImporter : IBundleImporter
return _sessionStore.Open(session);
}
+ ///
+ /// T-006: validates the zip envelope against the configured caps BEFORE any
+ /// entry payload is decompressed. Reads only the central-directory headers
+ /// ( / )
+ /// so a hostile bundle can't OOM the central node through this method itself.
+ ///
+ /// Buffered bundle bytes. Position is preserved.
+ private void ValidateArchiveEnvelope(MemoryStream bundleBytes)
+ {
+ var opts = _options.Value;
+ var maxEntries = opts.MaxBundleEntryCount;
+ var maxEntryDecompressed = opts.MaxBundleEntryDecompressedMb * 1024L * 1024L;
+ var maxRatio = opts.MaxBundleEntryCompressionRatio;
+
+ var savedPosition = bundleBytes.Position;
+ try
+ {
+ bundleBytes.Position = 0;
+ using var archive = new ZipArchive(bundleBytes, ZipArchiveMode.Read, leaveOpen: true);
+
+ if (archive.Entries.Count > maxEntries)
+ {
+ throw new InvalidDataException(
+ $"Bundle contains {archive.Entries.Count} zip entries; the configured maximum is {maxEntries}.");
+ }
+
+ foreach (var entry in archive.Entries)
+ {
+ if (entry.Length > maxEntryDecompressed)
+ {
+ throw new InvalidDataException(
+ $"Bundle entry '{entry.FullName}' declares a decompressed size of {entry.Length} bytes; "
+ + $"the configured maximum is {maxEntryDecompressed} bytes "
+ + $"({opts.MaxBundleEntryDecompressedMb} MB).");
+ }
+
+ // CompressedLength of 0 means store-only or empty — skip ratio check.
+ if (entry.CompressedLength > 0 && entry.Length / entry.CompressedLength > maxRatio)
+ {
+ throw new InvalidDataException(
+ $"Bundle entry '{entry.FullName}' has compression ratio "
+ + $"{entry.Length / entry.CompressedLength}x; the configured maximum is {maxRatio}x.");
+ }
+ }
+ }
+ finally
+ {
+ bundleBytes.Position = savedPosition;
+ }
+ }
+
///
public async Task PreviewAsync(Guid sessionId, CancellationToken ct = default)
{
@@ -611,6 +711,12 @@ public sealed class BundleImporter : IBundleImporter
await _dbContext.SaveChangesAsync(ct).ConfigureAwait(false);
await tx.CommitAsync(ct).ConfigureAwait(false);
+ // T-007: zero out the decrypted plaintext BEFORE remove so any
+ // caller-held reference (e.g. the Razor page that built the
+ // ImportPreview) sees the cleared buffer too. Remove drops the
+ // dictionary entry; together they release the secrets immediately
+ // instead of leaving them in process memory for the full TTL.
+ ZeroDecryptedContent(session);
_sessionStore.Remove(sessionId);
return new ImportResult(
@@ -692,6 +798,14 @@ public sealed class BundleImporter : IBundleImporter
// any failure here so the original exception below propagates
// unchanged rather than being masked by an audit-layer fault.
}
+
+ // T-007: a failed apply used to leave the BundleSession (with its
+ // decrypted secrets) in the in-memory store for the full 30-minute
+ // TTL — 10 failed 100 MB imports = 1 GB of plaintext still rooted.
+ // Drop the session here too so the secrets are released as soon as
+ // the failure surfaces, not when the next Get() happens to evict.
+ ZeroDecryptedContent(session);
+ _sessionStore.Remove(sessionId);
throw;
}
finally
@@ -704,6 +818,20 @@ public sealed class BundleImporter : IBundleImporter
}
}
+ ///
+ /// T-007: zeros the session's
+ /// buffer in place so any caller still holding a reference observes the
+ /// cleared bytes. Best-effort — a null/empty buffer is a no-op.
+ ///
+ private static void ZeroDecryptedContent(BundleSession session)
+ {
+ var buf = session.DecryptedContent;
+ if (buf is { Length: > 0 })
+ {
+ Array.Clear(buf, 0, buf.Length);
+ }
+ }
+
/// Mutable per-apply counter struct, accumulated through every helper.
private sealed class ImportSummary
{
diff --git a/src/ScadaLink.Transport/Import/BundleLockedException.cs b/src/ScadaLink.Transport/Import/BundleLockedException.cs
new file mode 100644
index 00000000..4e40137c
--- /dev/null
+++ b/src/ScadaLink.Transport/Import/BundleLockedException.cs
@@ -0,0 +1,31 @@
+namespace ScadaLink.Transport.Import;
+
+///
+/// T-003: thrown by when an encrypted bundle has
+/// exceeded the configured failed-unlock attempt limit
+/// (). The lockout is tracked
+/// server-side keyed by BundleManifest.ContentHash, so a second tab / CLI caller
+/// re-uploading the same bytes hits the same counter and cannot side-step the limit.
+///
+public sealed class BundleLockedException : Exception
+{
+ /// Number of recorded unlock failures for this bundle.
+ public int FailedAttempts { get; }
+
+ /// SHA-256 (hex) of the bundle's content bytes, the lockout's tracking key.
+ public string BundleContentHash { get; }
+
+ ///
+ /// Initializes a new .
+ ///
+ /// SHA-256 hex from BundleManifest.ContentHash.
+ /// Number of failures recorded against this bundle.
+ public BundleLockedException(string bundleContentHash, int failedAttempts)
+ : base(
+ $"Bundle is locked after {failedAttempts} failed unlock attempts. "
+ + "Wait for the lockout window to expire or re-export the bundle to obtain a new content hash.")
+ {
+ BundleContentHash = bundleContentHash ?? throw new ArgumentNullException(nameof(bundleContentHash));
+ FailedAttempts = failedAttempts;
+ }
+}
diff --git a/src/ScadaLink.Transport/Import/BundleSessionEvictionService.cs b/src/ScadaLink.Transport/Import/BundleSessionEvictionService.cs
new file mode 100644
index 00000000..5715c7a3
--- /dev/null
+++ b/src/ScadaLink.Transport/Import/BundleSessionEvictionService.cs
@@ -0,0 +1,55 @@
+using Microsoft.Extensions.Hosting;
+using Microsoft.Extensions.Logging;
+using ScadaLink.Commons.Interfaces.Transport;
+
+namespace ScadaLink.Transport.Import;
+
+///
+/// T-007: periodic background sweep that drives
+/// so abandoned import sessions clear from memory on their own, without needing a
+/// new to trigger lazy eviction. Each session
+/// owns the decrypted bundle content (potentially up to ~100 MB of secrets — DB
+/// connection strings, SMTP credentials, external-system auth configs), and the
+/// design contract is "bundles are not retained server-side after ApplyAsync
+/// commits". This service keeps abandoned / failed sessions from pinning that
+/// plaintext for the full 30-minute TTL when no other traffic flows.
+///
+internal sealed class BundleSessionEvictionService : BackgroundService
+{
+ private static readonly TimeSpan SweepInterval = TimeSpan.FromMinutes(1);
+
+ private readonly IBundleSessionStore _sessionStore;
+ private readonly ILogger _logger;
+
+ public BundleSessionEvictionService(
+ IBundleSessionStore sessionStore,
+ ILogger logger)
+ {
+ _sessionStore = sessionStore ?? throw new ArgumentNullException(nameof(sessionStore));
+ _logger = logger ?? throw new ArgumentNullException(nameof(logger));
+ }
+
+ protected override async Task ExecuteAsync(CancellationToken stoppingToken)
+ {
+ while (!stoppingToken.IsCancellationRequested)
+ {
+ try
+ {
+ await Task.Delay(SweepInterval, stoppingToken).ConfigureAwait(false);
+ }
+ catch (OperationCanceledException) when (stoppingToken.IsCancellationRequested)
+ {
+ return;
+ }
+
+ try
+ {
+ _sessionStore.EvictExpired();
+ }
+ catch (Exception ex)
+ {
+ _logger.LogWarning(ex, "Bundle session sweep failed; will retry on next interval.");
+ }
+ }
+ }
+}
diff --git a/src/ScadaLink.Transport/Import/BundleSessionStore.cs b/src/ScadaLink.Transport/Import/BundleSessionStore.cs
index ab644b70..a14fc585 100644
--- a/src/ScadaLink.Transport/Import/BundleSessionStore.cs
+++ b/src/ScadaLink.Transport/Import/BundleSessionStore.cs
@@ -36,19 +36,35 @@ namespace ScadaLink.Transport.Import;
public sealed class BundleSessionStore : IBundleSessionStore
{
private readonly ConcurrentDictionary _sessions = new();
- private readonly TimeProvider _timeProvider;
- // Options are accepted to honor the documented constructor contract and to
- // be ready for future per-store knobs (e.g. max in-flight sessions). The
- // current store does not read any field — TTL is on the session itself.
+ ///
+ /// T-003: per-bundle unlock-failure counters, keyed by
+ /// (SHA-256 hex of the bundle's content bytes). Failures are tracked here — not on
+ /// — so retries against the same bundle bytes from a
+ /// second tab / CLI caller share the counter and cannot side-step the lockout. Entries
+ /// expire on the same TTL as a session.
+ ///
+ private readonly ConcurrentDictionary _unlockFailures = new(StringComparer.OrdinalIgnoreCase);
+
+ private readonly TimeProvider _timeProvider;
+ private readonly IOptions _options;
+
+ /// T-003: per-bundle unlock-failure entry with expiry.
+ private sealed class UnlockFailureRecord
+ {
+ public int Count;
+ public DateTimeOffset ExpiresAt;
+ }
+
///
/// Initializes a new .
///
- /// Transport options (reserved for future per-store configuration).
+ /// Transport options. is also used as the TTL for the T-003 per-bundle unlock-failure tracker.
/// Time provider used to evaluate session expiry.
public BundleSessionStore(IOptions options, TimeProvider timeProvider)
{
ArgumentNullException.ThrowIfNull(options);
+ _options = options;
_ = options.Value;
_timeProvider = timeProvider ?? throw new ArgumentNullException(nameof(timeProvider));
}
@@ -90,5 +106,70 @@ public sealed class BundleSessionStore : IBundleSessionStore
_sessions.TryRemove(kv.Key, out _);
}
}
+
+ // T-003: also expire stale per-bundle unlock-failure entries so a bundle
+ // that was previously locked clears once the lockout window passes.
+ foreach (var kv in _unlockFailures)
+ {
+ if (kv.Value.ExpiresAt <= now)
+ {
+ _unlockFailures.TryRemove(kv.Key, out _);
+ }
+ }
+ }
+
+ ///
+ public int GetUnlockFailureCount(string bundleContentHash)
+ {
+ ArgumentException.ThrowIfNullOrEmpty(bundleContentHash);
+ if (!_unlockFailures.TryGetValue(bundleContentHash, out var record))
+ {
+ return 0;
+ }
+
+ // Lazy expiry — if the entry has aged past its window treat it as cleared.
+ if (record.ExpiresAt <= _timeProvider.GetUtcNow())
+ {
+ _unlockFailures.TryRemove(bundleContentHash, out _);
+ return 0;
+ }
+
+ return record.Count;
+ }
+
+ ///
+ public int IncrementUnlockFailureCount(string bundleContentHash)
+ {
+ ArgumentException.ThrowIfNullOrEmpty(bundleContentHash);
+ var ttl = TimeSpan.FromMinutes(_options.Value.BundleSessionTtlMinutes);
+ var now = _timeProvider.GetUtcNow();
+ var record = _unlockFailures.AddOrUpdate(
+ bundleContentHash,
+ _ => new UnlockFailureRecord { Count = 1, ExpiresAt = now + ttl },
+ (_, existing) =>
+ {
+ // Treat an expired record as a fresh start so a legitimate operator
+ // returning hours later does not face a stale lockout.
+ if (existing.ExpiresAt <= now)
+ {
+ existing.Count = 1;
+ }
+ else
+ {
+ existing.Count++;
+ }
+
+ existing.ExpiresAt = now + ttl;
+ return existing;
+ });
+
+ return record.Count;
+ }
+
+ ///
+ public void ClearUnlockFailures(string bundleContentHash)
+ {
+ ArgumentException.ThrowIfNullOrEmpty(bundleContentHash);
+ _unlockFailures.TryRemove(bundleContentHash, out _);
}
}
diff --git a/src/ScadaLink.Transport/ScadaLink.Transport.csproj b/src/ScadaLink.Transport/ScadaLink.Transport.csproj
index a1ac6db9..a2e1e03a 100644
--- a/src/ScadaLink.Transport/ScadaLink.Transport.csproj
+++ b/src/ScadaLink.Transport/ScadaLink.Transport.csproj
@@ -8,6 +8,8 @@
+
+
diff --git a/src/ScadaLink.Transport/Serialization/BundleSerializer.cs b/src/ScadaLink.Transport/Serialization/BundleSerializer.cs
index d0ba97e3..cee0b78d 100644
--- a/src/ScadaLink.Transport/Serialization/BundleSerializer.cs
+++ b/src/ScadaLink.Transport/Serialization/BundleSerializer.cs
@@ -82,7 +82,17 @@ public sealed class BundleSerializer
// count but rebuild ContentHash + Encryption fields against the bytes we
// actually write. The non-encryption manifest fields (source env, exported
// by, summary, contents, version) are preserved verbatim.
- var (cipher, freshMeta) = encryptor.Encrypt(contentBytes, passphrase, manifest.Encryption.Iterations);
+ //
+ // T-005: bind the manifest's non-derivative fields into the AES-GCM AAD
+ // so a tampered SourceEnvironment / ExportedBy / etc. on a stolen bundle
+ // yields an authentication-tag mismatch on decrypt instead of a forged
+ // origin label slipping past the Step-4 confirmation gate. AAD is
+ // computed over a manifest normalised to empty ContentHash + null
+ // Encryption (those fields are derivative of the ciphertext / IV and
+ // cannot themselves be authenticated).
+ var aad = BundleManifestAad.Compute(manifest);
+ var (cipher, freshMeta) = encryptor.Encrypt(
+ contentBytes, passphrase, manifest.Encryption.Iterations, aad);
payload = cipher;
payloadEntryName = ContentEncEntryName;
finalManifest = manifest with
@@ -178,7 +188,10 @@ public sealed class BundleSerializer
{
throw new ArgumentException("Encrypted bundle requires both passphrase and encryptor.");
}
- plaintext = encryptor.Decrypt(contentBytes, manifest.Encryption, passphrase);
+ // T-005: mirror the encrypt side — AAD is derived from the manifest's
+ // non-derivative fields. Tampering yields an authentication-tag mismatch.
+ var aad = BundleManifestAad.Compute(manifest);
+ plaintext = encryptor.Decrypt(contentBytes, manifest.Encryption, passphrase, aad);
}
else
{
diff --git a/src/ScadaLink.Transport/ServiceCollectionExtensions.cs b/src/ScadaLink.Transport/ServiceCollectionExtensions.cs
index 74a43ad8..a0a436ec 100644
--- a/src/ScadaLink.Transport/ServiceCollectionExtensions.cs
+++ b/src/ScadaLink.Transport/ServiceCollectionExtensions.cs
@@ -35,6 +35,9 @@ public static class ServiceCollectionExtensions
services.AddScoped();
services.AddScoped();
services.AddSingleton();
+ // T-007: periodic eviction sweep so abandoned sessions clear without
+ // needing a fresh Get() to trigger lazy eviction.
+ services.AddHostedService();
// SemanticValidator is a stateless utility used by ApplyAsync; use
// TryAdd so a host that already calls AddTemplateEngine() (which
// registers the same type as Transient) wins. Either registration
diff --git a/src/ScadaLink.Transport/TransportOptions.cs b/src/ScadaLink.Transport/TransportOptions.cs
index fb7e9ed9..be11c896 100644
--- a/src/ScadaLink.Transport/TransportOptions.cs
+++ b/src/ScadaLink.Transport/TransportOptions.cs
@@ -6,6 +6,28 @@ public sealed class TransportOptions
public int BundleSessionTtlMinutes { get; set; } = 30;
/// Gets or sets the maximum allowed bundle size in megabytes.
public int MaxBundleSizeMb { get; set; } = 100;
+ ///
+ /// T-006: maximum allowed decompressed size of any single zip entry in megabytes.
+ /// A 100 MB DEFLATE-compressed bundle can decompress to gigabytes; this cap
+ /// stops a malicious bundle from OOM-ing the central node before its entries
+ /// are decompressed.
+ ///
+ public int MaxBundleEntryDecompressedMb { get; set; } = 200;
+ ///
+ /// T-006: maximum permitted number of entries inside a bundle zip. A well-formed
+ /// bundle has exactly two (manifest.json plus content.json or
+ /// content.enc); a small upper bound limits the surface a zip-bomb can
+ /// exploit without rejecting future schema additions out of hand.
+ ///
+ public int MaxBundleEntryCount { get; set; } = 4;
+ ///
+ /// T-006: maximum permitted compression ratio (uncompressed length / compressed
+ /// length) per zip entry. Defence-in-depth against decompression bombs whose
+ /// declared is
+ /// trustworthy on read; legitimate JSON compresses around 5–10x, so 50x has
+ /// generous headroom for unusually compressible bundles.
+ ///
+ public int MaxBundleEntryCompressionRatio { get; set; } = 50;
/// Gets or sets the maximum number of failed passphrase unlock attempts before a session is locked.
public int MaxUnlockAttemptsPerSession { get; set; } = 3;
/// Gets or sets the maximum number of unlock attempts allowed per IP address per hour.
diff --git a/tests/ScadaLink.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs b/tests/ScadaLink.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs
index e6c54ea6..2aab5a40 100644
--- a/tests/ScadaLink.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs
+++ b/tests/ScadaLink.Transport.IntegrationTests/Import/BundleImporterApplyTests.cs
@@ -310,6 +310,40 @@ public sealed class BundleImporterApplyTests : IDisposable
Assert.Equal(0, await ctx.Templates.CountAsync());
Assert.True(await ctx.AuditLogEntries.AnyAsync(a => a.Action == "BundleImportFailed"));
}
+
+ // T-007: a failed apply used to keep the BundleSession (and its decrypted
+ // secrets) in the in-memory store for the full 30-minute TTL. The session
+ // must now be removed immediately so the plaintext is released.
+ var sessionStore = _provider.GetRequiredService();
+ Assert.Null(sessionStore.Get(sessionId));
+ }
+
+ [Fact]
+ public async Task ApplyAsync_removes_session_on_success_path_too()
+ {
+ // T-007: companion to the failed-apply test — the success path must also
+ // remove the session (it was already doing so before T-007, but the new
+ // test asserts the contract explicitly so a future refactor cannot
+ // accidentally leave plaintext in the store).
+ await using (var scope = _provider.CreateAsyncScope())
+ {
+ var ctx = scope.ServiceProvider.GetRequiredService();
+ ctx.Templates.Add(new Template("PumpForT007") { Description = "fresh" });
+ await ctx.SaveChangesAsync();
+ }
+ var sessionId = await ExportAndLoadAsync();
+ await WipeContentAsync();
+
+ await using (var scope = _provider.CreateAsyncScope())
+ {
+ var importer = scope.ServiceProvider.GetRequiredService();
+ await importer.ApplyAsync(sessionId,
+ new List { new("Template", "PumpForT007", ResolutionAction.Add, null) },
+ user: "alice");
+ }
+
+ var sessionStore = _provider.GetRequiredService();
+ Assert.Null(sessionStore.Get(sessionId));
}
[Fact]
diff --git a/tests/ScadaLink.Transport.Tests/Encryption/BundleSecretEncryptorTests.cs b/tests/ScadaLink.Transport.Tests/Encryption/BundleSecretEncryptorTests.cs
index e01ff96d..579367df 100644
--- a/tests/ScadaLink.Transport.Tests/Encryption/BundleSecretEncryptorTests.cs
+++ b/tests/ScadaLink.Transport.Tests/Encryption/BundleSecretEncryptorTests.cs
@@ -57,6 +57,37 @@ public sealed class BundleSecretEncryptorTests
Assert.NotEqual(meta1.SaltB64, meta2.SaltB64);
}
+ [Fact]
+ public void Decrypt_with_mismatched_AAD_throws_CryptographicException()
+ {
+ // T-005: AES-GCM AAD binds extra context (e.g. manifest non-derivative fields)
+ // into the auth tag. Decrypting with different AAD yields a tag mismatch even
+ // when the passphrase and ciphertext are correct.
+ var sut = new BundleSecretEncryptor();
+ var plaintext = Encoding.UTF8.GetBytes("secret payload");
+ var aadOnEncrypt = Encoding.UTF8.GetBytes("manifest-canonical-hash-A");
+ var aadOnDecrypt = Encoding.UTF8.GetBytes("manifest-canonical-hash-B");
+
+ var (ciphertext, metadata) = sut.Encrypt(plaintext, "pass", TestIterations, aadOnEncrypt);
+
+ Assert.ThrowsAny(
+ () => sut.Decrypt(ciphertext, metadata, "pass", aadOnDecrypt));
+ }
+
+ [Fact]
+ public void Decrypt_with_matching_AAD_recovers_plaintext()
+ {
+ // T-005: round-trip with non-empty AAD must succeed when both sides match.
+ var sut = new BundleSecretEncryptor();
+ var plaintext = Encoding.UTF8.GetBytes("secret payload");
+ var aad = Encoding.UTF8.GetBytes("manifest-canonical-hash");
+
+ var (ciphertext, metadata) = sut.Encrypt(plaintext, "pass", TestIterations, aad);
+ var recovered = sut.Decrypt(ciphertext, metadata, "pass", aad);
+
+ Assert.Equal(plaintext, recovered);
+ }
+
[Fact]
public void Encrypt_emits_metadata_matching_decryption_inputs()
{
diff --git a/tests/ScadaLink.Transport.Tests/Import/BundleImporterLoadTests.cs b/tests/ScadaLink.Transport.Tests/Import/BundleImporterLoadTests.cs
index ca34c252..e39456b8 100644
--- a/tests/ScadaLink.Transport.Tests/Import/BundleImporterLoadTests.cs
+++ b/tests/ScadaLink.Transport.Tests/Import/BundleImporterLoadTests.cs
@@ -207,6 +207,225 @@ public sealed class BundleImporterLoadTests
() => rig.Importer.LoadAsync(stream, passphrase: "wrong"));
}
+ [Fact]
+ public async Task LoadAsync_locks_bundle_after_three_wrong_passphrases_even_across_callers()
+ {
+ // T-003: the lockout is server-side and keyed by ContentHash, so replaying
+ // the SAME bundle bytes from a second caller (different stream, different
+ // session) must hit the same counter. After MaxUnlockAttemptsPerSession (3)
+ // failures the importer throws BundleLockedException, not another
+ // CryptographicException — and the lock survives a fresh LoadAsync from a
+ // pristine caller that has no idea about the prior attempts.
+ var rig = BuildRig();
+ using var packed = PackEncryptedBundle(
+ rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
+ var bundleBytes = ((MemoryStream)packed).ToArray();
+
+ // First two wrong-passphrase attempts surface as CryptographicException.
+ for (var attempt = 1; attempt <= 2; attempt++)
+ {
+ await Assert.ThrowsAnyAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: "wrong"));
+ }
+
+ // Third wrong-passphrase attempt crosses the threshold and surfaces as
+ // BundleLockedException — even though this is a fresh stream / a caller
+ // that never saw the earlier failures.
+ var locked = await Assert.ThrowsAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: "wrong"));
+ Assert.Equal(3, locked.FailedAttempts);
+
+ // Fourth attempt — even with the CORRECT passphrase — is still locked,
+ // because the lockout sticks until the TTL expires or the bundle is
+ // re-exported with a new content hash.
+ await Assert.ThrowsAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: "correct"));
+ }
+
+ [Fact]
+ public async Task LoadAsync_rejects_bundle_with_too_many_entries()
+ {
+ // T-006: a malicious bundle could pad the archive with arbitrary entries to
+ // exhaust per-entry handles or to slip an unexpected payload past the
+ // serializer. The envelope check rejects any archive whose entry count
+ // exceeds MaxBundleEntryCount BEFORE any decompression.
+ var rig = BuildRig(opts => opts.MaxBundleEntryCount = 2);
+ using var packed = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, EmptyContent());
+ var bytes = ((MemoryStream)packed).ToArray();
+ var paddedBytes = AppendExtraZipEntry(bytes, "extra.bin", new byte[8]);
+
+ await Assert.ThrowsAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(paddedBytes), passphrase: null));
+ }
+
+ [Fact]
+ public async Task LoadAsync_rejects_bundle_with_oversized_entry()
+ {
+ // T-006: caller-declared decompressed Length above the configured cap is
+ // a hostile bundle; reject without decompressing.
+ var rig = BuildRig(opts => opts.MaxBundleEntryDecompressedMb = 1);
+ using var packed = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, EmptyContent());
+ var bytes = ((MemoryStream)packed).ToArray();
+ // Replace content.json with a 2 MB entry of compressible zeros — uncompressed Length > cap.
+ var bigPayload = new byte[2 * 1024 * 1024];
+ var oversizedBytes = ReplaceZipEntry(bytes, "content.json", bigPayload);
+
+ await Assert.ThrowsAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(oversizedBytes), passphrase: null));
+ }
+
+ [Fact]
+ public async Task LoadAsync_rejects_bundle_whose_entry_exceeds_compression_ratio()
+ {
+ // T-006: defence-in-depth — even if Length is within the per-entry cap, an
+ // extreme compression ratio is a hallmark of a decompression bomb and is
+ // rejected outright.
+ var rig = BuildRig(opts =>
+ {
+ opts.MaxBundleEntryDecompressedMb = 100;
+ opts.MaxBundleEntryCompressionRatio = 10;
+ });
+ using var packed = PackPlainBundle(rig.Serializer, rig.ManifestBuilder, EmptyContent());
+ var bytes = ((MemoryStream)packed).ToArray();
+ // 1 MB of zeros compresses extremely well (>100x ratio) — well over the
+ // configured 10x cap.
+ var compressible = new byte[1024 * 1024];
+ var bombBytes = ReplaceZipEntry(bytes, "content.json", compressible);
+
+ await Assert.ThrowsAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(bombBytes), passphrase: null));
+ }
+
+ ///
+ /// T-006 helper: rewrites an existing zip to add a fresh entry alongside the
+ /// originals. Used by the "too many entries" test.
+ ///
+ private static byte[] AppendExtraZipEntry(byte[] zipBytes, string newEntryName, byte[] payload)
+ {
+ var output = new MemoryStream();
+ using (var src = new MemoryStream(zipBytes))
+ using (var srcZip = new ZipArchive(src, ZipArchiveMode.Read))
+ using (var dstZip = new ZipArchive(output, ZipArchiveMode.Create, leaveOpen: true))
+ {
+ foreach (var entry in srcZip.Entries)
+ {
+ var dst = dstZip.CreateEntry(entry.FullName);
+ using var inStream = entry.Open();
+ using var outStream = dst.Open();
+ inStream.CopyTo(outStream);
+ }
+ var extra = dstZip.CreateEntry(newEntryName);
+ using (var s = extra.Open()) { s.Write(payload); }
+ }
+ return output.ToArray();
+ }
+
+ ///
+ /// T-006 helper: rewrites an existing zip, replacing one entry's bytes with
+ /// the supplied payload while preserving every other entry verbatim.
+ ///
+ private static byte[] ReplaceZipEntry(byte[] zipBytes, string entryToReplace, byte[] newPayload)
+ {
+ var output = new MemoryStream();
+ using (var src = new MemoryStream(zipBytes))
+ using (var srcZip = new ZipArchive(src, ZipArchiveMode.Read))
+ using (var dstZip = new ZipArchive(output, ZipArchiveMode.Create, leaveOpen: true))
+ {
+ foreach (var entry in srcZip.Entries)
+ {
+ var dst = dstZip.CreateEntry(entry.FullName);
+ using var outStream = dst.Open();
+ if (entry.FullName == entryToReplace)
+ {
+ outStream.Write(newPayload);
+ }
+ else
+ {
+ using var inStream = entry.Open();
+ inStream.CopyTo(outStream);
+ }
+ }
+ }
+ return output.ToArray();
+ }
+
+ [Fact]
+ public async Task LoadAsync_rejects_bundle_with_tampered_manifest_field_even_with_correct_passphrase()
+ {
+ // T-005: a stolen bundle whose plaintext manifest fields (SourceEnvironment,
+ // ExportedBy, …) have been edited must fail decryption with a tag mismatch.
+ // Without AAD an attacker could rewrite the SourceEnvironment label and slip
+ // past the Step-4 typo-resistant confirmation gate. We tamper the field by
+ // re-packing the manifest (with everything else, including the original
+ // ciphertext, unchanged) into a fresh ZIP and verify decrypt fails.
+ var rig = BuildRig();
+ using var packed = PackEncryptedBundle(
+ rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
+ var bytes = ((MemoryStream)packed).ToArray();
+
+ // Read manifest + ciphertext from the legit bundle, mutate SourceEnvironment,
+ // and re-pack with the SAME ciphertext bytes — the cipher is fine, only the
+ // plaintext manifest is changed.
+ BundleManifest originalManifest;
+ byte[] cipherBytes;
+ using (var src = new MemoryStream(bytes))
+ using (var srcZip = new ZipArchive(src, ZipArchiveMode.Read))
+ {
+ using var ms = new MemoryStream();
+ srcZip.GetEntry("manifest.json")!.Open().CopyTo(ms);
+ originalManifest = JsonSerializer.Deserialize(ms.ToArray(), BundleJsonOptions)!;
+
+ using var ctMs = new MemoryStream();
+ srcZip.GetEntry("content.enc")!.Open().CopyTo(ctMs);
+ cipherBytes = ctMs.ToArray();
+ }
+
+ var tampered = originalManifest with { SourceEnvironment = "prod-spoofed" };
+ var tamperedManifestBytes = JsonSerializer.SerializeToUtf8Bytes(tampered, BundleJsonOptions);
+
+ var tamperedZipBytes = new MemoryStream();
+ using (var outZip = new ZipArchive(tamperedZipBytes, ZipArchiveMode.Create, leaveOpen: true))
+ {
+ var mEntry = outZip.CreateEntry("manifest.json");
+ using (var s = mEntry.Open()) { s.Write(tamperedManifestBytes); }
+ var cEntry = outZip.CreateEntry("content.enc");
+ using (var s = cEntry.Open()) { s.Write(cipherBytes); }
+ }
+ tamperedZipBytes.Position = 0;
+
+ // ContentHash check is the FIRST thing that catches the tamper here because
+ // ContentHash is sealed against the cipher bytes and the cipher is unchanged
+ // BUT the manifest's plaintext fields are mutated — wait, ContentHash is
+ // against the cipher which we kept, so it still matches. The defence is
+ // the AES-GCM AAD: the tag check fails because AAD differs.
+ await Assert.ThrowsAnyAsync(
+ () => rig.Importer.LoadAsync(tamperedZipBytes, passphrase: "correct"));
+ }
+
+ [Fact]
+ public async Task LoadAsync_resets_unlock_counter_on_correct_passphrase()
+ {
+ // T-003: a legitimate operator who typos once or twice before getting it
+ // right must not be penalised on the next bundle. A successful LoadAsync
+ // clears the per-bundle counter.
+ var rig = BuildRig();
+ using var packed = PackEncryptedBundle(
+ rig.Serializer, rig.ManifestBuilder, rig.Encryptor, EmptyContent(), "correct");
+ var bundleBytes = ((MemoryStream)packed).ToArray();
+
+ await Assert.ThrowsAnyAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: "wrong"));
+
+ var session = await rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: "correct");
+ Assert.NotNull(session);
+
+ // Counter cleared — a subsequent wrong-passphrase attempt starts from 0
+ // and surfaces as CryptographicException, not BundleLockedException.
+ await Assert.ThrowsAnyAsync(
+ () => rig.Importer.LoadAsync(new MemoryStream(bundleBytes), passphrase: "wrong"));
+ Assert.Equal(1, rig.SessionStore.GetUnlockFailureCount(session.Manifest.ContentHash));
+ }
+
[Fact]
public async Task LoadAsync_throws_NotSupportedException_when_bundleFormatVersion_unsupported()
{
diff --git a/tests/ScadaLink.Transport.Tests/Import/BundleSessionStoreTests.cs b/tests/ScadaLink.Transport.Tests/Import/BundleSessionStoreTests.cs
index 271de055..3954c27b 100644
--- a/tests/ScadaLink.Transport.Tests/Import/BundleSessionStoreTests.cs
+++ b/tests/ScadaLink.Transport.Tests/Import/BundleSessionStoreTests.cs
@@ -96,8 +96,13 @@ public sealed class BundleSessionStoreTests
}
[Fact]
- public void Three_failed_unlock_attempts_locks_session()
+ public void Legacy_FailedUnlockAttempts_field_still_round_trips_via_shared_reference()
{
+ // T-003 legacy guard: the per-session FailedUnlockAttempts / Locked fields on
+ // BundleSession are no longer the source of truth (lockout moved to the store
+ // keyed by ContentHash), but the fields remain as a compatibility shim for
+ // callers/tests that still set them directly. The store hands out the shared
+ // reference so mutations made on one Get() are observable from another.
var clock = new TestTimeProvider(DateTimeOffset.UtcNow);
var sut = new BundleSessionStore(Options(), clock);
var session = SessionExpiringAt(clock.GetUtcNow().AddMinutes(30));
@@ -120,6 +125,62 @@ public sealed class BundleSessionStoreTests
Assert.True(refetch!.Locked);
}
+ [Fact]
+ public void UnlockFailureCount_TracksPerBundleAndResets()
+ {
+ // T-003: the per-bundle unlock-failure counter is keyed by ContentHash and
+ // shared across sessions, so a second tab / CLI caller cannot side-step the
+ // lockout by re-uploading the same bytes. Increment must be atomic and
+ // ClearUnlockFailures must drop the entry so a legitimate operator who
+ // eventually types the right passphrase is not stuck on a stale count.
+ var clock = new TestTimeProvider(DateTimeOffset.UtcNow);
+ var sut = new BundleSessionStore(Options(), clock);
+ const string contentHashA = "sha-A";
+ const string contentHashB = "sha-B";
+
+ Assert.Equal(0, sut.GetUnlockFailureCount(contentHashA));
+ Assert.Equal(1, sut.IncrementUnlockFailureCount(contentHashA));
+ Assert.Equal(2, sut.IncrementUnlockFailureCount(contentHashA));
+
+ // Per-bundle isolation: another bundle's counter is unaffected.
+ Assert.Equal(0, sut.GetUnlockFailureCount(contentHashB));
+
+ sut.ClearUnlockFailures(contentHashA);
+ Assert.Equal(0, sut.GetUnlockFailureCount(contentHashA));
+ }
+
+ [Fact]
+ public void UnlockFailures_ExpireOnTtlAndGetReturnsZero()
+ {
+ // T-003: failure records share the session TTL so a bundle that was locked
+ // hours ago clears on its own. Get must lazily expire stale entries.
+ var clock = new TestTimeProvider(DateTimeOffset.UtcNow);
+ var sut = new BundleSessionStore(Options(), clock);
+ const string contentHash = "sha-expiring";
+
+ sut.IncrementUnlockFailureCount(contentHash);
+ sut.IncrementUnlockFailureCount(contentHash);
+ Assert.Equal(2, sut.GetUnlockFailureCount(contentHash));
+
+ // Advance beyond the configured TTL (default 30 min).
+ clock.Advance(TimeSpan.FromMinutes(31));
+ Assert.Equal(0, sut.GetUnlockFailureCount(contentHash));
+ }
+
+ [Fact]
+ public void UnlockFailures_EvictExpired_ClearsStaleEntries()
+ {
+ // T-003: EvictExpired sweep cleans the per-bundle counters too, not just sessions.
+ var clock = new TestTimeProvider(DateTimeOffset.UtcNow);
+ var sut = new BundleSessionStore(Options(), clock);
+ sut.IncrementUnlockFailureCount("sha-old");
+
+ clock.Advance(TimeSpan.FromMinutes(31));
+ sut.EvictExpired();
+
+ Assert.Equal(0, sut.GetUnlockFailureCount("sha-old"));
+ }
+
///
/// Minimal in-test with a mutable clock. Used in
/// place of Microsoft.Extensions.TimeProvider.Testing (not in the