Guard audit reconciliation writes and fix audit value canonicalization

CHANGELOG.md

@@ -11,6 +11,23 @@ This project follows Semantic Versioning using the format `MAJOR.MINOR.PATCH`.
 * Completed the AsiBackbone 6.0 and Learning 1.0 alignment review, updated
   current terminology, and confirmed that NCAT remains compatible with its 2.x
   public and generated-template contracts.
+* **Behavior change:** audit value canonicalization used by the `Hash`,
+  `HmacSha256`, and `Truncate` dispositions now renders `byte[]` values as hex,
+  collections as JSON arrays of canonical elements, and `DateTime` /
+  `DateTimeOffset` values in round-trip (`O`) format. Digests for these types
+  differ from earlier releases; string and numeric digests are unchanged.
+* Clarified that the `Hash` disposition is an unkeyed integrity digest with no
+  confidentiality for low-entropy values; use `HmacSha256` instead.
+
+### Fixed
+
+* Audit reconciliation runs now persist findings in a single transaction inside
+  the execution strategy (or join a caller-owned transaction), guard every
+  finding update with its `ConcurrencyStamp`, and insert findings only when the
+  key is absent. A concurrent writer now causes a `DbUpdateConcurrencyException`
+  and a full rollback instead of a last-writer-wins or partially applied run.
+* `byte[]` audit values no longer canonicalize to the literal `System.Byte[]`,
+  which made every binary value hash and truncate to the same constant.
 
 ## 2.10.0 - 2026-09-19
 

src/ProjectTemplate.Infrastructure/Data/Auditing/ApplicationAuditReconciler.cs

@@ -438,22 +438,74 @@ private async Task PersistCandidatesAsync(
         IReadOnlyCollection<ApplicationAuditCompletionOutboxEntry> completionEntries,
         DateTime now,
         CancellationToken cancellationToken)
+    {
+        // When the caller already owns an EF Core transaction, join it and let the caller decide whether
+        // the reconciliation writes commit.
+        if (_dbContext.Database.CurrentTransaction is not null)
+        {
+            await PersistCandidatesCoreAsync(candidates, auditBatchIds, completionEntries, now, cancellationToken)
+                .ConfigureAwait(false);
+            return;
+        }
+
+        // Otherwise persist every finding change in one transaction inside the execution strategy, so a run
+        // applies all of its inserts and guarded updates or none of them, and a retrying provider replays
+        // the reads together with the writes.
+        IExecutionStrategy executionStrategy = _dbContext.Database.CreateExecutionStrategy();
+
+        await executionStrategy.ExecuteAsync(
+                async strategyCancellationToken =>
+                {
+                    await using IDbContextTransaction transaction = await _dbContext.Database
+                        .BeginTransactionAsync(strategyCancellationToken)
+                        .ConfigureAwait(false);
+
+                    await PersistCandidatesCoreAsync(
+                            candidates,
+                            auditBatchIds,
+                            completionEntries,
+                            now,
+                            strategyCancellationToken)
+                        .ConfigureAwait(false);
+
+                    await transaction.CommitAsync(strategyCancellationToken).ConfigureAwait(false);
+                },
+                cancellationToken)
+            .ConfigureAwait(false);
+    }
+
+    private async Task PersistCandidatesCoreAsync(
+        IReadOnlyCollection<ApplicationAuditReconciliationCandidate> candidates,
+        IReadOnlyCollection<string> auditBatchIds,
+        IReadOnlyCollection<ApplicationAuditCompletionOutboxEntry> completionEntries,
+        DateTime now,
+        CancellationToken cancellationToken)
     {
         string[] keys = [.. candidates.Select(candidate => candidate.FindingKey)];
+        string[] scopeBatchIds = [.. auditBatchIds
+            .Concat(completionEntries.Select(entry => entry.MutationBatchId))
+            .Where(batchId => !string.IsNullOrWhiteSpace(batchId))
+            .Distinct(StringComparer.Ordinal)];
+
+        // Read the active and the resolvable findings in one round trip. Each row's stamp guards its update.
         List<ApplicationAuditReconciliationFinding> existing = await _dbContext
             .ApplicationAuditReconciliationFindings
             .AsNoTracking()
-            .Where(finding => keys.Contains(finding.FindingKey))
+            .Where(finding => keys.Contains(finding.FindingKey) ||
+                (scopeBatchIds.Contains(finding.MutationBatchId) &&
+                    finding.RemediationStatus != ApplicationAuditReconciliationRemediationStatuses.Resolved))
             .ToListAsync(cancellationToken)
             .ConfigureAwait(false);
         var existingByKey = existing
             .ToDictionary(finding => finding.FindingKey, StringComparer.Ordinal);
+        var activeKeys = new HashSet<string>(keys, StringComparer.Ordinal);
+        var scopeBatchIdSet = new HashSet<string>(scopeBatchIds, StringComparer.Ordinal);
 
         foreach (ApplicationAuditReconciliationCandidate candidate in candidates)
         {
             if (existingByKey.TryGetValue(candidate.FindingKey, out ApplicationAuditReconciliationFinding? finding))
             {
-                await _dbContext.Database.ExecuteSqlInterpolatedAsync($$"""
+                int updatedCount = await _dbContext.Database.ExecuteSqlInterpolatedAsync($$"""
                     UPDATE [ApplicationAuditReconciliationFindings]
                     SET [Severity] = {{candidate.Severity}},
                         [Guidance] = {{candidate.Guidance}},
@@ -462,44 +514,55 @@ UPDATE [ApplicationAuditReconciliationFindings]
                         [ResolvedUtc] = {{(DateTime?)null}},
                         [ConcurrencyStamp] = {{Guid.NewGuid().ToString("N")}}
                     WHERE [Id] = {{finding.Id}}
+                        AND [ConcurrencyStamp] = {{finding.ConcurrencyStamp}}
                     """, cancellationToken).ConfigureAwait(false);
+
+                EnsureSingleRowWritten(updatedCount, candidate.FindingKey);
             }
             else
             {
+                // Insert only when no row holds the key, so a run that interleaved and inserted the same finding
+                // surfaces as a concurrency conflict (and a rollback) rather than a unique index violation.
                 var id = Guid.NewGuid();
-                await _dbContext.Database.ExecuteSqlInterpolatedAsync($$"""
+                int insertedCount = await _dbContext.Database.ExecuteSqlInterpolatedAsync($$"""
                     INSERT INTO [ApplicationAuditReconciliationFindings]
                         ([Id], [SchemaVersion], [FindingKey], [ReasonCode], [Severity], [MutationBatchId], [Destination], [Guidance], [RemediationStatus], [FirstObservedUtc], [LastObservedUtc], [ResolvedUtc], [ConcurrencyStamp])
-                    VALUES
-                        ({{id}}, {{ApplicationAuditReconciliationFinding.CurrentSchemaVersion}}, {{candidate.FindingKey}}, {{candidate.ReasonCode}}, {{candidate.Severity}}, {{candidate.MutationBatchId}}, {{candidate.Destination}}, {{candidate.Guidance}}, {{ApplicationAuditReconciliationRemediationStatuses.Open}}, {{now}}, {{now}}, {{(DateTime?)null}}, {{Guid.NewGuid().ToString("N")}})
+                    SELECT
+                        {{id}}, {{ApplicationAuditReconciliationFinding.CurrentSchemaVersion}}, {{candidate.FindingKey}}, {{candidate.ReasonCode}}, {{candidate.Severity}}, {{candidate.MutationBatchId}}, {{candidate.Destination}}, {{candidate.Guidance}}, {{ApplicationAuditReconciliationRemediationStatuses.Open}}, {{now}}, {{now}}, {{(DateTime?)null}}, {{Guid.NewGuid().ToString("N")}}
+                    WHERE NOT EXISTS (
+                        SELECT 1 FROM [ApplicationAuditReconciliationFindings]
+                        WHERE [FindingKey] = {{candidate.FindingKey}})
                     """, cancellationToken).ConfigureAwait(false);
+
+                EnsureSingleRowWritten(insertedCount, candidate.FindingKey);
             }
         }
 
-        string[] scopeBatchIds = [.. auditBatchIds
-            .Concat(completionEntries.Select(entry => entry.MutationBatchId))
-            .Where(batchId => !string.IsNullOrWhiteSpace(batchId))
-            .Distinct(StringComparer.Ordinal)];
-        string[] activeKeys = keys;
-
-        List<ApplicationAuditReconciliationFinding> resolved = await _dbContext
-            .ApplicationAuditReconciliationFindings
-            .AsNoTracking()
-            .Where(finding => scopeBatchIds.Contains(finding.MutationBatchId) &&
-                finding.RemediationStatus != ApplicationAuditReconciliationRemediationStatuses.Resolved &&
-                !activeKeys.Contains(finding.FindingKey))
-            .ToListAsync(cancellationToken)
-            .ConfigureAwait(false);
-
-        foreach (ApplicationAuditReconciliationFinding finding in resolved)
+        foreach (ApplicationAuditReconciliationFinding finding in existing.Where(finding =>
+                     !activeKeys.Contains(finding.FindingKey) &&
+                     scopeBatchIdSet.Contains(finding.MutationBatchId) &&
+                     finding.RemediationStatus != ApplicationAuditReconciliationRemediationStatuses.Resolved))
         {
-            await _dbContext.Database.ExecuteSqlInterpolatedAsync($$"""
+            int resolvedCount = await _dbContext.Database.ExecuteSqlInterpolatedAsync($$"""
                 UPDATE [ApplicationAuditReconciliationFindings]
                 SET [RemediationStatus] = {{ApplicationAuditReconciliationRemediationStatuses.Resolved}},
                     [ResolvedUtc] = {{now}},
                     [ConcurrencyStamp] = {{Guid.NewGuid().ToString("N")}}
                 WHERE [Id] = {{finding.Id}}
+                    AND [ConcurrencyStamp] = {{finding.ConcurrencyStamp}}
                 """, cancellationToken).ConfigureAwait(false);
+
+            EnsureSingleRowWritten(resolvedCount, finding.FindingKey);
+        }
+    }
+
+    private static void EnsureSingleRowWritten(int affectedCount, string findingKey)
+    {
+        if (affectedCount != 1)
+        {
+            throw new DbUpdateConcurrencyException(
+                $"Audit reconciliation finding '{findingKey}' was modified by another writer after it was read. " +
+                "No reconciliation changes were committed; the next reconciliation run re-evaluates the finding.");
         }
     }
 

src/ProjectTemplate.Infrastructure/Data/Auditing/ApplicationAuditValueDisposition.cs

@@ -16,10 +16,11 @@ public enum ApplicationAuditValueDisposition
     Mask = 1,
 
     /// <summary>
-    /// Records an unsalted SHA-256 hash of the value.
-    /// This does not protect low-entropy values such as email addresses because
-    /// an attacker holding the audit table can brute-force candidate values.
-    /// Use this only for high-entropy values where correlation matters more than secrecy.
+    /// Records an unsalted, unkeyed SHA-256 hash of the value for integrity and change detection only.
+    /// This is not a confidentiality control: low-entropy values such as email addresses, phone numbers,
+    /// national identifiers, booleans, enum names, and small numbers are trivially recovered by a
+    /// dictionary attack against the audit table. Use <see cref="HmacSha256" /> when the value must stay
+    /// confidential, and use this only for high-entropy values where correlation matters more than secrecy.
     /// </summary>
     Hash = 2,
 

src/ProjectTemplate.Infrastructure/Data/Auditing/ApplicationAuditValueProtector.cs

@@ -1,6 +1,8 @@
+using System.Collections;
 using System.Globalization;
 using System.Security.Cryptography;
 using System.Text;
+using System.Text.Json;
 
 namespace ProjectTemplate.Infrastructure.Data.Auditing;
 
@@ -47,6 +49,9 @@ internal static bool TryProtect(
         }
     }
 
+    // Unkeyed SHA-256 is an integrity / change-detection digest only. It provides no confidentiality for
+    // low-entropy values (email addresses, phone numbers, identifiers, booleans, enum names, small numbers),
+    // which a holder of the audit table can recover by dictionary attack. HmacSha256 is the confidential option.
     private static string Hash(object? value)
     {
         byte[] canonicalValue = Encoding.UTF8.GetBytes(ToCanonicalString(value));
@@ -67,9 +72,24 @@ private static string HmacSha256(object? value, string? key)
         return Convert.ToHexString(hash);
     }
 
-    private static string ToCanonicalString(object? value)
+    // Produces a culture-invariant representation that is distinct for distinct values. Types whose
+    // Object.ToString() is only the type name (byte[], collections) are expanded, so change detection on
+    // binary and collection columns does not collapse to a single constant digest.
+    internal static string ToCanonicalString(object? value)
     {
-        return Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty;
+        return value switch
+        {
+            null => string.Empty,
+            string text => text,
+            byte[] bytes => Convert.ToHexString(bytes),
+            ReadOnlyMemory<byte> memory => Convert.ToHexString(memory.Span),
+            Memory<byte> memory => Convert.ToHexString(memory.Span),
+            DateTime dateTime => dateTime.ToString("O", CultureInfo.InvariantCulture),
+            DateTimeOffset dateTimeOffset => dateTimeOffset.ToString("O", CultureInfo.InvariantCulture),
+            IFormattable formattable => formattable.ToString(null, CultureInfo.InvariantCulture),
+            IEnumerable sequence => JsonSerializer.Serialize(sequence.Cast<object?>().Select(ToCanonicalString).ToArray()),
+            _ => Convert.ToString(value, CultureInfo.InvariantCulture) ?? string.Empty,
+        };
     }
 
     private static string Truncate(object? value, int? maximumLength)

tests/ProjectTemplate.Web.Tests/ApplicationAuditReconciliationTests.cs

@@ -134,6 +134,114 @@ await database.Context.ApplicationAuditReconciliationFindings
             finding => finding.ReasonCode == ApplicationAuditReconciliationReasonCodes.DuplicateCompletion);
     }
 
+    [Fact]
+    public async Task ReconcileAsync_FindingChangedAfterRead_ThrowsConcurrencyExceptionAndRollsBackRun()
+    {
+        var interceptor = new NonQueryInterceptor();
+        await using TestDatabase database = await TestDatabase.CreateAsync(interceptor: interceptor);
+        ApplicationAuditReconciliationFinding finding = await CreateOpenFindingAsync(database, "reconcile-concurrency-batch");
+        database.Context.AuditRecords.Add(CreateAuditRecord("reconcile-concurrency-new-batch"));
+        _ = await database.Context.SaveChangesAsync(TestContext.Current.CancellationToken);
+
+        bool simulatedConcurrentWrite = false;
+        interceptor.OnNonQueryExecuting = async (command, cancellationToken) =>
+        {
+            if (simulatedConcurrentWrite ||
+                !command.CommandText.Contains("UPDATE [ApplicationAuditReconciliationFindings]", StringComparison.Ordinal))
+            {
+                return;
+            }
+
+            simulatedConcurrentWrite = true;
+
+            // Simulate another reconciler run or remediation changing the finding after this run read it.
+            await using DbCommand concurrentWrite = command.Connection!.CreateCommand();
+            concurrentWrite.Transaction = command.Transaction;
+            concurrentWrite.CommandText =
+                "UPDATE [ApplicationAuditReconciliationFindings] SET [ConcurrencyStamp] = 'concurrent-writer'";
+            _ = await concurrentWrite.ExecuteNonQueryAsync(cancellationToken);
+        };
+
+        _ = await Assert.ThrowsAsync<DbUpdateConcurrencyException>(() => database.Reconciler
+            .ReconcileAsync(TestContext.Current.CancellationToken));
+
+        interceptor.OnNonQueryExecuting = null;
+
+        Assert.True(simulatedConcurrentWrite);
+        Assert.Null(database.Context.Database.CurrentTransaction);
+        ApplicationAuditReconciliationFinding current = Assert.Single(
+            await database.Context.ApplicationAuditReconciliationFindings
+                .AsNoTracking()
+                .ToListAsync(TestContext.Current.CancellationToken));
+        Assert.Equal(finding.Id, current.Id);
+        Assert.Equal(finding.ConcurrencyStamp, current.ConcurrencyStamp);
+    }
+
+    [Fact]
+    public async Task ReconcileAsync_ConcurrentRunInsertedSameFinding_ThrowsConcurrencyExceptionWithoutDuplicate()
+    {
+        var interceptor = new NonQueryInterceptor();
+        await using TestDatabase database = await TestDatabase.CreateAsync(interceptor: interceptor);
+        database.Context.AuditRecords.Add(CreateAuditRecord("reconcile-insert-race-batch"));
+        _ = await database.Context.SaveChangesAsync(TestContext.Current.CancellationToken);
+
+        bool simulatedConcurrentInsert = false;
+        interceptor.OnNonQueryExecuting = async (command, cancellationToken) =>
+        {
+            if (simulatedConcurrentInsert ||
+                !command.CommandText.Contains("INSERT INTO [ApplicationAuditReconciliationFindings]", StringComparison.Ordinal))
+            {
+                return;
+            }
+
+            simulatedConcurrentInsert = true;
+
+            // Simulate an interleaved run inserting the same finding key between this run's read and insert.
+            await using DbCommand concurrentInsert = command.Connection!.CreateCommand();
+            concurrentInsert.Transaction = command.Transaction;
+            concurrentInsert.CommandText = command.CommandText;
+            foreach (DbParameter parameter in command.Parameters)
+            {
+                _ = concurrentInsert.Parameters.Add(new SqliteParameter(parameter.ParameterName, parameter.Value));
+            }
+
+            concurrentInsert.Parameters[0].Value = Guid.NewGuid().ToString().ToUpperInvariant();
+            _ = await concurrentInsert.ExecuteNonQueryAsync(cancellationToken);
+        };
+
+        _ = await Assert.ThrowsAsync<DbUpdateConcurrencyException>(() => database.Reconciler
+            .ReconcileAsync(TestContext.Current.CancellationToken));
+
+        interceptor.OnNonQueryExecuting = null;
+
+        Assert.True(simulatedConcurrentInsert);
+        Assert.Empty(await database.Context.ApplicationAuditReconciliationFindings
+            .AsNoTracking()
+            .ToListAsync(TestContext.Current.CancellationToken));
+    }
+
+    [Fact]
+    public async Task ReconcileAsync_CallerOwnedTransaction_JoinsTransactionWithoutCommitting()
+    {
+        await using TestDatabase database = await TestDatabase.CreateAsync();
+        database.Context.AuditRecords.Add(CreateAuditRecord("reconcile-caller-transaction-batch"));
+        _ = await database.Context.SaveChangesAsync(TestContext.Current.CancellationToken);
+
+        await using (IDbContextTransaction transaction = await database.Context.Database
+            .BeginTransactionAsync(TestContext.Current.CancellationToken))
+        {
+            _ = await database.Reconciler.ReconcileAsync(TestContext.Current.CancellationToken);
+
+            Assert.Same(transaction, database.Context.Database.CurrentTransaction);
+
+            await transaction.RollbackAsync(TestContext.Current.CancellationToken);
+        }
+
+        Assert.Empty(await database.Context.ApplicationAuditReconciliationFindings
+            .AsNoTracking()
+            .ToListAsync(TestContext.Current.CancellationToken));
+    }
+
     [Fact]
     public async Task RecordRemediationAsync_AppendsEvidenceAndResolvesFinding()
     {