Skip to content

Guard audit reconciliation writes and fix audit value canonicalization - #575

Merged
cdcavell merged 2 commits into
mainfrom
fix/audit-reconciliation-concurrency-and-hash-canonicalization
Sep 21, 2026
Merged

cdcavell merged 2 commits into
mainfrom
fix/audit-reconciliation-concurrency-and-hash-canonicalization

Conversation

@cdcavell

Copy link
Copy Markdown
Collaborator

Summary

1. Audit reconciliation writes (HIGH)

ApplicationAuditReconciler.PersistCandidatesAsync:

  • Transactional: all inserts/updates for a run execute in one transaction inside CreateExecutionStrategy(), the same pattern RecordRemediationAsync uses. If the caller already owns a transaction, the run joins it.
  • Optimistic concurrency: every UPDATE now includes AND [ConcurrencyStamp] = @original. Zero rows affected → DbUpdateConcurrencyException, and the whole run rolls back.
  • Insert race: INSERT ... SELECT ... WHERE NOT EXISTS (FindingKey). If an interleaved run already inserted the key, the result is a concurrency conflict, not a unique-index violation. On SQL Server, a true simultaneous insert can still hit the unique index. The run still rolls back cleanly, and the hosted service retries on the next cycle.
  • Round trips: the two reads (active keys + resolvable findings) are merged into one query. Writes are still one statement per row because each row needs its own stamp guard, but they now share one transaction.

2. Audit value protection (HIGH)

  • (a) The Hash disposition is kept for compatibility (existing policies use it for high-entropy IDs). The enum and the method are now documented as an integrity / change-detection digest with no confidentiality for low-entropy values. They point to HmacSha256 instead.
  • (b) ToCanonicalString now handles byte[]/Memory<byte> as hex, DateTime/DateTimeOffset in round-trip O format (the old format dropped sub-second precision), other IFormattable values in invariant culture, and non-string IEnumerable as a JSON array of canonical elements. Binary columns no longer collapse to "System.Byte[]".

Behavior change: digests and truncated text for byte[], collection, and DateTime values differ from earlier releases. String and numeric digests are unchanged, which the existing golden-hash tests confirm. This is noted in the CHANGELOG.

Tests

  • Reconcile: a stale stamp throws and rolls back the full run (including the other batch's insert), a concurrent insert of the same key throws with no duplicate, and a caller-owned transaction is joined without committing.
  • Protector: distinct byte[] values give distinct Hash/HMAC digests, Truncate renders byte[] as hex, collections expand unambiguously, and formatting is culture-invariant with DateTime precision kept.
  • dotnet build -c Release (warnings as errors) passes; dotnet test: 455/455 passed.

🤖 Generated with Claude Code

Reconciliation persistence now runs in one transaction inside the execution
strategy (or joins a caller-owned transaction), guards every finding update
with its ConcurrencyStamp, inserts findings only when the key is absent, and
reads active and resolvable findings in a single query. Conflicts raise
DbUpdateConcurrencyException and roll back the whole run.

Audit value canonicalization now expands byte[] (hex), collections (JSON
array), and DateTime/DateTimeOffset (round-trip), so binary values no longer
hash or truncate to "System.Byte[]". The Hash disposition is documented as an
integrity-only digest with no confidentiality for low-entropy values.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 21, 2026 23:41
@cdcavell
cdcavell merged commit bd95f02 into main Sep 21, 2026
17 checks passed
@cdcavell
cdcavell deleted the fix/audit-reconciliation-concurrency-and-hash-canonicalization branch September 21, 2026 23:53
@cdcavell
cdcavell removed the request for review from Copilot September 22, 2026 00:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant