OPS-157454: Upload test coverage to Codacy from CI

.github/workflows/ci.yml

@@ -39,6 +39,68 @@ jobs:
         name: packages
         path: ./artifacts/*.nupkg
 
+  # Test coverage, uploaded to Codacy (OPS-157454).
+  # pdl-public, never a GitHub-hosted runner and never pdl-prod-cluster: this repository is public,
+  # so a fork pull request is untrusted code, and pdl-public is the hardened scale set that may run
+  # it (no dind, no privileged container, no hostPath, nothing shared between jobs).
+  coverage:
+    runs-on: [pdl-public]
+
+    env:
+      DOTNET_NOLOGO: 1
+      DOTNET_CLI_TELEMETRY_OPTOUT: 1
+      # The actions-runner image does not offer the Node version several of these actions request
+      # by default; without this they fail before their first step runs.
+      FORCE_JAVASCRIPT_ACTIONS_TO_NODE24: true
+
+    steps:
+    - uses: actions/checkout@v7
+      with:
+        fetch-depth: 0 # avoid shallow clone so nbgv can do its work.
+
+    - name: Setup .NET
+      uses: actions/setup-dotnet@v6
+      # The runner user cannot write /usr/share/dotnet, so install the SDK somewhere it can.
+      env:
+        DOTNET_INSTALL_DIR: ${{ runner.temp }}/dotnet
+      with:
+        dotnet-version: 10.0.x
+
+    - name: Build tests
+      run: dotnet build PanoramicData.Encryption.Test/PanoramicData.Encryption.Test.csproj --configuration Debug
+
+    # The test project is a Microsoft.Testing.Platform application, so coverage is collected by
+    # running the test executable directly (Microsoft.Testing.Extensions.CodeCoverage) rather than
+    # through `dotnet test`. On Linux the executable has no .exe extension.
+    #
+    # Every test in this repository runs offline with no credentials, so no filter is applied.
+    - name: Run tests with coverage
+      run: >-
+        ./PanoramicData.Encryption.Test/bin/Debug/net10.0/PanoramicData.Encryption.Test
+        --coverage
+        --coverage-settings "$GITHUB_WORKSPACE/coverage.config"
+        --coverage-output-format cobertura
+        --coverage-output coverage.cobertura.xml
+
+    # continue-on-error: a Codacy outage must not turn a passing test run red. The trade-off is that
+    # an expired or missing project token fails silently, so check the log for
+    # "Coverage received successfully" when in doubt.
+    - name: Upload coverage to Codacy
+      continue-on-error: true
+      uses: codacy/codacy-coverage-reporter-action@v1
+      with:
+        project-token: ${{ secrets.CODACY_PROJECT_TOKEN }}
+        coverage-reports: PanoramicData.Encryption.Test/bin/Debug/net10.0/TestResults/coverage.cobertura.xml
+
+    - name: Upload coverage artifact
+      if: always()
+      continue-on-error: true
+      uses: actions/upload-artifact@v7
+      with:
+        name: coverage
+        path: PanoramicData.Encryption.Test/bin/Debug/net10.0/TestResults/coverage.cobertura.xml
+        retention-days: 7
+
   publish:
     needs: build
     runs-on: ubuntu-latest

PanoramicData.Encryption.Test/ConcurrencyTests.cs

@@ -0,0 +1,98 @@
+using System.Collections.Concurrent;
+
+namespace PanoramicData.Encryption.Test;
+
+/// <summary>
+/// Regression tests for OPS-157454: EncryptionService previously shared a single static SHA256
+/// instance across threads, which intermittently threw "Concurrent operations from multiple
+/// threads on this type are not supported" and could silently return wrong hashes.
+/// Each test runs many operations concurrently and asserts every result equals the
+/// single-threaded result.
+/// </summary>
+public class ConcurrencyTests
+{
+	private const int Iterations = 5000;
+	private const string EncryptionKey = "00112233445566778899AABBCCDDEEFF00112233445566778899AABBCCDDEEFF";
+	private const string Salt = "0f0e0d0c0b0a09080706050403020100";
+
+	private static readonly ParallelOptions Options = new()
+	{
+		MaxDegreeOfParallelism = Math.Max(8, Environment.ProcessorCount * 2)
+	};
+
+	private static readonly string[] Inputs =
+	[
+		string.Empty,
+		"abc",
+		"The quick brown fox jumps over the lazy dog",
+		"café € 100 🔒",
+		new string('x', 10_000),
+	];
+
+	/// <summary>
+	/// Hashing from many threads at once returns the single-threaded result every time.
+	/// </summary>
+	[Fact]
+	public void GetHash_Concurrent_MatchesSingleThreadedResult()
+	{
+		var expected = Inputs.Select(EncryptionService.GetHash).ToArray();
+		var actual = new string[Iterations];
+
+		Parallel.For(0, Iterations, Options, i => actual[i] = EncryptionService.GetHash(Inputs[i % Inputs.Length]));
+
+		for (var i = 0; i < Iterations; i++)
+		{
+			actual[i].Should().Be(expected[i % Inputs.Length], "iteration {0} must match the single-threaded hash", i);
+		}
+	}
+
+	/// <summary>
+	/// Encrypting and decrypting from many threads at once, on one shared service instance and
+	/// across instances, returns the single-threaded result every time.
+	/// </summary>
+	[Fact]
+	public void EncryptDecrypt_Concurrent_MatchesSingleThreadedResult()
+	{
+		var shared = new EncryptionService(EncryptionKey);
+		var expected = Inputs.Select(input => shared.Encrypt(input, Salt).cipherText).ToArray();
+		var cipherTexts = new string[Iterations];
+		var plainTexts = new string[Iterations];
+
+		Parallel.For(0, Iterations, Options, i =>
+		{
+			var service = i % 2 == 0 ? shared : new EncryptionService(EncryptionKey);
+			var input = Inputs[i % Inputs.Length];
+			cipherTexts[i] = service.Encrypt(input, Salt).cipherText;
+			plainTexts[i] = service.Decrypt(cipherTexts[i], Salt);
+		});
+
+		for (var i = 0; i < Iterations; i++)
+		{
+			cipherTexts[i].Should().Be(expected[i % Inputs.Length], "iteration {0} must match the single-threaded cipher text", i);
+			plainTexts[i].Should().Be(Inputs[i % Inputs.Length], "iteration {0} must round-trip", i);
+		}
+	}
+
+	/// <summary>
+	/// Randomly generated salts produced from many threads at once are well formed, distinct,
+	/// and round-trip.
+	/// </summary>
+	[Fact]
+	public void Encrypt_RandomSaltConcurrent_ProducesDistinctSaltsThatRoundTrip()
+	{
+		var service = new EncryptionService(EncryptionKey);
+		var salts = new ConcurrentBag<string>();
+
+		Parallel.For(0, Iterations, Options, i =>
+		{
+			var input = Inputs[i % Inputs.Length];
+			var (cipherText, salt) = service.Encrypt(input);
+			service.Decrypt(cipherText, salt).Should().Be(input);
+			salts.Add(salt);
+		});
+
+		salts.Should().HaveCount(Iterations);
+		salts.Should().OnlyHaveUniqueItems();
+		salts.Should().AllSatisfy(salt => salt.Should().MatchRegex("^[0-9a-f]{32}$"));
+	}
+}

PanoramicData.Encryption.Test/PanoramicData.Encryption.Test.csproj

@@ -2,6 +2,7 @@
 <Project Sdk="Microsoft.NET.Sdk">
   <PropertyGroup>
     <TargetFramework>net10.0</TargetFramework>
+    <UseMicrosoftTestingPlatformRunner>true</UseMicrosoftTestingPlatformRunner>
     <AnalysisLevel>latest-recommended</AnalysisLevel>
     <IsPackable>false</IsPackable>
     <ImplicitUsings>enable</ImplicitUsings>

PanoramicData.Encryption.Test/PinnedOutputTests.cs

@@ -0,0 +1,72 @@
+namespace PanoramicData.Encryption.Test;
+
+/// <summary>
+/// Pins the exact output of the library for known inputs, so that any change to the input
+/// encoding, the algorithm, the padding or the output format (hex, casing) fails a test.
+/// The expected values are published SHA-256 test vectors where one exists, and otherwise were
+/// computed independently with the .NET BCL (SHA256.HashData / Aes.EncryptCbc over UTF-8).
+/// </summary>
+public class PinnedOutputTests
+{
+	private const string EncryptionKey = "00112233445566778899AABBCCDDEEFF00112233445566778899AABBCCDDEEFF";
+	private const string Salt = "0f0e0d0c0b0a09080706050403020100";
+
+	// "café € 100 🔒": two-, three- and four-byte UTF-8 sequences, including a surrogate pair.
+	private const string UnicodeText = "café € 100 🔒";
+
+	/// <summary>
+	/// GetHash returns the uppercase hex SHA-256 of the UTF-8 bytes of the input.
+	/// </summary>
+	[Theory]
+	[InlineData("", "E3B0C44298FC1C149AFBF4C8996FB92427AE41E4649B934CA495991B7852B855")]
+	[InlineData("abc", "BA7816BF8F01CFEA414140DE5DAE2223B00361A396177A9CB410FF61F20015AD")]
+	[InlineData("The quick brown fox jumps over the lazy dog", "D7A8FBB307D7809469CA9ABCB0082E4F8D5651E46D3CDB762D02D0BF37C9E592")]
+	[InlineData(UnicodeText, "91FD866D1271995C1AD7BC61B1970441991C32696B88F2ED75CB874B996D88F7")]
+	public void GetHash_KnownInput_ReturnsPinnedValue(string input, string expected)
+		=> EncryptionService.GetHash(input).Should().Be(expected);
+
+	/// <summary>
+	/// Encrypting with a fixed key and salt returns pinned lowercase hex cipher text (AES-256-CBC,
+	/// PKCS7 padding, UTF-8 plaintext), and the salt is returned unchanged in lowercase hex.
+	/// </summary>
+	[Theory]
+	[InlineData("", "2258814885f97e0e539cc0613346d8f4")]
+	[InlineData("Hello, World!", "3ce17eea6f106637d64736e7ed1280f1")]
+	[InlineData(UnicodeText, "08514b73e16093df81a1feea30eaae85efe306b05dc057f56172f0fbf88b3766")]
+	public void Encrypt_KnownInput_ReturnsPinnedValue(string plainText, string expectedCipherText)
+	{
+		var service = new EncryptionService(EncryptionKey);
+
+		var (cipherText, salt) = service.Encrypt(plainText, Salt);
+
+		cipherText.Should().Be(expectedCipherText);
+		salt.Should().Be(Salt);
+		service.Decrypt(cipherText, salt).Should().Be(plainText);
+	}
+
+	/// <summary>
+	/// Decrypting pinned cipher text returns the original plaintext, so data encrypted by earlier
+	/// versions of the library remains readable.
+	/// </summary>
+	[Theory]
+	[InlineData("2258814885f97e0e539cc0613346d8f4", "")]
+	[InlineData("3ce17eea6f106637d64736e7ed1280f1", "Hello, World!")]
+	[InlineData("08514B73E16093DF81A1FEEA30EAAE85EFE306B05DC057F56172F0FBF88B3766", UnicodeText)]
+	public void Decrypt_PinnedCipherText_ReturnsPlainText(string cipherText, string expectedPlainText)
+		=> new EncryptionService(EncryptionKey)
+			.Decrypt(cipherText, Salt)
+			.Should()
+			.Be(expectedPlainText);
+
+	/// <summary>
+	/// A randomly generated salt is a 32 character lowercase hex string (16 bytes).
+	/// </summary>
+	[Fact]
+	public void Encrypt_RandomSalt_IsLowercaseHexOf16Bytes()
+	{
+		var (_, salt) = new EncryptionService(EncryptionKey).Encrypt("Hello, World!");
+
+		salt.Should().HaveLength(32);
+		salt.Should().MatchRegex("^[0-9a-f]{32}$");
+	}
+}

PanoramicData.Encryption/EncryptionService.cs

@@ -11,10 +11,11 @@ public class EncryptionService
 {
 	private readonly byte[] _key;
 
-	private readonly static RandomNumberGenerator _random = RandomNumberGenerator.Create();
+	// UTF8Encoding is stateless for GetBytes/GetString and is safe to share across threads.
+	// No HashAlgorithm, SymmetricAlgorithm, ICryptoTransform or RandomNumberGenerator instance is
+	// shared: those types are not thread-safe (OPS-157454), so each call uses a static one-shot
+	// API or its own short-lived instance.
 	private readonly static UTF8Encoding _encoder = new();
-	private readonly static Aes _aes = Aes.Create();
-	private readonly static SHA256 _hashAlgorithm = SHA256.Create();
 
 
 	/// <summary>
@@ -33,9 +34,7 @@ public EncryptionService(string encryptionKey)
 
 	private static byte[] GenerateVector()
 	{
-		var vector = new byte[16];
-		_random.GetBytes(vector);
-		return vector;
+		return RandomNumberGenerator.GetBytes(16);
 	}
 
 	#pragma warning disable CA5401 // Symmetric encryption uses non-default initialization vector, which could be potentially repeatable
@@ -47,7 +46,8 @@ private static byte[] GenerateVector()
 	public (string cipherText, string salt) Encrypt(string unencrypted)
 	{
 		var vector = GenerateVector();
-		var encryptor = _aes.CreateEncryptor(_key, vector);
+		using var aes = Aes.Create();
+		using var encryptor = aes.CreateEncryptor(_key, vector);
 		return (ByteArrayToHexString(Transform(_encoder.GetBytes(unencrypted), encryptor)), ByteArrayToHexString(vector));
 	}
 
@@ -69,7 +69,8 @@ private static byte[] GenerateVector()
 		}
 
 		var vector = HexStringToByteArray(salt);
-		var encryptor = _aes.CreateEncryptor(_key, vector);
+		using var aes = Aes.Create();
+		using var encryptor = aes.CreateEncryptor(_key, vector);
 		return (ByteArrayToHexString(Transform(_encoder.GetBytes(unencrypted), encryptor)), ByteArrayToHexString(vector));
 	}
 #pragma warning restore CA5401
@@ -94,7 +95,8 @@ public string Decrypt(string encryptedString, string salt)
 
 		var encrypted = HexStringToByteArray(encryptedString);
 
-		var decryptor = _aes.CreateDecryptor(_key, vector);
+		using var aes = Aes.Create();
+		using var decryptor = aes.CreateDecryptor(_key, vector);
 		var decrypt = _encoder.GetString(Transform(encrypted, decryptor));
 		return decrypt;
 	}
@@ -140,12 +142,9 @@ public static string GetHash(string inputString)
 	{
 		ArgumentNullException.ThrowIfNull(inputString, nameof(inputString));
 
-		var sb = new StringBuilder();
-		foreach (var @byte in _hashAlgorithm.ComputeHash(Encoding.UTF8.GetBytes(inputString))) {
-			sb.Append(@byte.ToString("X2", CultureInfo.InvariantCulture));
-		}
-		var result = sb.ToString();
-		return result;
+		// SHA256.HashData is a stateless one-shot API and is safe to call from any thread.
+		// Convert.ToHexString emits uppercase hex, identical to the previous "X2" formatting.
+		return Convert.ToHexString(SHA256.HashData(Encoding.UTF8.GetBytes(inputString)));
 	}
 
 	/// <summary>

coverage.config

@@ -0,0 +1,33 @@
+<?xml version="1.0" encoding="utf-8"?>
+<!--
+  Microsoft.Testing.Extensions.CodeCoverage settings.
+
+  The attribute exclusions are not optional if figures are to stay comparable across the estate: the
+  Microsoft collector includes generated code by default, where coverlet excluded it. Porting them is
+  worth several points of apparent coverage on a repository with generated clients.
+-->
+<Configuration>
+  <CodeCoverage>
+    <ModulePaths>
+      <Exclude>
+        <ModulePath>.*Test\.dll$</ModulePath>
+        <ModulePath>.*Tests\.dll$</ModulePath>
+      </Exclude>
+    </ModulePaths>
+    <Attributes>
+      <Exclude>
+        <Attribute>^System\.Diagnostics\.CodeAnalysis\.ExcludeFromCodeCoverageAttribute$</Attribute>
+        <Attribute>^System\.Runtime\.CompilerServices\.CompilerGeneratedAttribute$</Attribute>
+        <Attribute>^System\.CodeDom\.Compiler\.GeneratedCodeAttribute$</Attribute>
+        <Attribute>^System\.ObsoleteAttribute$</Attribute>
+      </Exclude>
+    </Attributes>
+    <Sources>
+      <Exclude>
+        <Source>.*\\obj\\.*</Source>
+      </Exclude>
+    </Sources>
+    <UseVerifiableInstrumentation>False</UseVerifiableInstrumentation>
+    <CollectFromChildProcesses>True</CollectFromChildProcesses>
+  </CodeCoverage>
+</Configuration>