Add xUnit test suite covering ConfigService, CryptServerClient, and cert strategies

[rodchristiansen]

Summary

• Adds tests/CryptEscrow.Tests/ — an xUnit test project covering ConfigService priority chains, CryptServerClient HTTP behavior, CredentialManager round-trip, and the mTLS cert-strategy ordering added in Allow for .pem and .key client cert authentication #2.
• Wires dotnet test into .github/workflows/ci.yml so PRs fail fast before publishing.
• 37 tests, all pass locally in ~800ms, no admin required.

Note

Depends on #2 — this branch is based on the state of aysiu-patch-1 so the new tests can compile against the PFX / PEM / Credential Manager code added there. Until #2 merges, the PR diff will include aysiu's changes too; once #2 lands, a rebase against main will shrink the diff to just the test additions and the small production-code seams below.

Production code changes (minimal, non-behavioral)

• src/CryptEscrow/CryptEscrow.csproj: <InternalsVisibleTo Include=\"CryptEscrow.Tests\" />.
• src/CryptEscrow/Services/CryptServerClient.cs:
  • New internal test constructor accepting an HttpMessageHandler directly, so RichardSzalay.MockHttp can intercept traffic without a real TLS stack.
  • GetClientCertificate promoted from private to internal so strategy ordering can be tested directly.
• src/CryptEscrow/Services/ConfigService.cs:
  • internal static string? ConfigPathOverride — redirects YAML reads to per-test temp files.
  • internal static Func<string, string?>? RegistryReaderOverride — redirects registry reads to per-test HKCU subkeys (tests run without admin).
  • Both default to null. Production behavior is unchanged.

Test coverage (37 tests)

Area	Tests	What's covered
ConfigServiceTests	15	Env → registry → YAML → default priority for GetApiKey, GetUseMtls, GetCertificateSubject, GetClientCertPath/KeyPath, GetPfxPath, GetPfxPasswordCredential; GetAuthConfig composition from mixed sources.
CredentialManagerTests	6	ASCII round trip, Unicode round trip, missing target, empty/null/whitespace target guard.
GetClientCertificateTests	8	PFX wins over PEM, PEM-only loads, unprotected PFX loads, missing credential falls through to PEM, missing PFX file falls through to PEM, missing PEM files return null, no config returns null.
CryptServerClientTests	8	Checkin success / 401 / 403 (no retry), form-url-encoded body fields, default + custom X-API-Key header, verify success / 401.

Test fixtures (tests/CryptEscrow.Tests/Fixtures/)

• EnvironmentSnapshot — scoped env-var save/restore.
• TempRegistryKey — creates HKCU\Software\CryptEscrowTest\<guid>, installs the reader override, deletes on dispose.
• TempConfigFile — writes YAML to a unique %TEMP% subdir and points ConfigPathOverride at it.
• SelfSignedCertFactory — generates ephemeral RSA certs via CertificateRequest and serializes to PEM/key and PFX temp files.
• CredentialManagerWriter — test-only advapi32!CredWriteW helper for seeding entries that the production reader consumes. Uses DllImport (not LibraryImport) because the CREDENTIAL struct contains FILETIME, which isn't compatible with DisableRuntimeMarshalling.

Framework choices

• xUnit + FluentAssertions + RichardSzalay.MockHttp. No mocking framework — code under test is mostly static classes, concrete types, and file I/O, so ambient-state fixtures + InternalsVisibleTo are sufficient (and we avoid the Moq SponsorLink controversy).
• FluentAssertions pinned to 6.12.1. Versions 7+ moved to a commercial license; 6.x remains free and API-stable.
• TargetFramework: net10.0-windows (must match main project — uses Windows-only APIs like Microsoft.Win32.Registry).

CI updates

- name: Restore
  run: dotnet restore CryptEscrow.sln

- name: Test
  run: dotnet test CryptEscrow.sln --configuration Release --no-restore --logger "trx;LogFileName=test-results.trx" --results-directory ./TestResults

- name: Upload test results
  if: always()
  uses: actions/upload-artifact@v4
  with:
    name: test-results
    path: ./TestResults/*.trx

Runs before the existing publish/package steps. If tests fail, the workflow short-circuits before producing an MSI.

Out of scope (follow-up PRs)

• Command-level tests for EscrowCommand / RotateCommand / VerifyCommand — need a mocking abstraction over BitLocker / WMI / manage-bde.
• Clock seam for retry backoff tests — current Task.Delay calls are real.
• Coverage threshold enforcement in CI (e.g. coverlet + 80% gate).
• Integration tests against a real Crypt Server.

Test plan

• dotnet build CryptEscrow.sln — zero warnings, zero errors
• dotnet test CryptEscrow.sln -c Release — 37 passed, 0 failed, ~800ms
• Manually break one line of ConfigService.GetApiKey — the corresponding test fails (proves the test actually exercises the code)
• CI run on this PR shows green tests and uploads test-results.trx
• After Allow for .pem and .key client cert authentication #2 merges, rebase this branch on main and confirm the diff shrinks to just the test additions

[Copilot]

Pull request overview

Adds a new xUnit-based test suite for CryptEscrow (config precedence, HTTP client behavior, Credential Manager, and mTLS cert strategy selection) and wires dotnet test into CI, plus introduces a few small internal seams to make the production code testable.

Changes:

• Add tests/CryptEscrow.Tests test project with fixtures and ~37 unit tests.
• Add CI steps to run dotnet test and upload TRX results.
• Introduce test seams/internal access in ConfigService and CryptServerClient (and InternalsVisibleTo), plus add Credential Manager support.

Reviewed changes

Copilot reviewed 17 out of 18 changed files in this pull request and generated 5 comments.

Show a summary per file

File	Description
tests/CryptEscrow.Tests/Services/ConfigServiceTests.cs	Tests config priority chain + GetAuthConfig composition.
tests/CryptEscrow.Tests/Services/CryptServerClientTests.cs	Verifies HTTP behavior via injected HttpMessageHandler.
tests/CryptEscrow.Tests/Services/CredentialManagerTests.cs	Exercises CredentialManager.ReadGenericPassword behavior.
tests/CryptEscrow.Tests/Services/GetClientCertificateTests.cs	Verifies mTLS client-cert strategy selection and fall-through.
tests/CryptEscrow.Tests/Fixtures/*	Env/registry/config/cert/credential test isolation helpers.
tests/CryptEscrow.Tests/CryptEscrow.Tests.csproj	Adds xUnit + FluentAssertions + MockHttp dependencies.
src/CryptEscrow/Services/ConfigService.cs	Adds internal test seams + new auth getters for PEM/PFX.
src/CryptEscrow/Services/CryptServerClient.cs	Adds internal test constructor + exposes/expands cert loading strategies.
src/CryptEscrow/Services/CredentialManager.cs	Adds Credential Manager reader used by PFX strategy.
src/CryptEscrow/CryptEscrow.csproj	Adds InternalsVisibleTo and enables unsafe blocks.
.github/workflows/ci.yml	Runs tests on CI and uploads TRX artifacts.
README.md	Documents new mTLS configuration options and env/registry keys.
CryptEscrow.sln	Adds test project to solution.
.gitignore	Ignores test result directories.

Comments suppressed due to low confidence (1)

src/CryptEscrow/Services/ConfigService.cs:66

• GetRegistryValue currently reads registry data using key?.GetValue(valueName) as string, which silently drops non-string policy values (e.g., DWORDs). This breaks the documented String/DWORD registry support (and means GetRegistryBool/GetRegistryInt won’t work for DWORD values in real HKLM policy keys). Consider switching to var raw = key?.GetValue(valueName) and handling string + numeric types consistently (e.g., int, long) before returning.

            // Try standard Group Policy path first
            using var key = Registry.LocalMachine.OpenSubKey(RegistryBasePath);
            var value = key?.GetValue(valueName) as string;

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[rodchristiansen]

Copilot review — response

Addressed four of the five comments in 917376e. Summary:

#	Comment	Resolution
1	Test parallelization mutating global state	Fixed — ConfigServiceTests joins new GlobalState collection with DisableParallelization = true. Defensive today, cheap insurance for future test additions.
2	ConfigPathOverride edge cases (empty string, relative path)	Fixed — added a shared ResolvedOverrideDir helper that gates all three path accessors through a single IsNullOrWhiteSpace check. Invalid overrides fall back to defaults consistently.
3	mTLS strategy ordering contradicts README	Fixed — reordered GetClientCertificate to Cert Store → PFX → PEM, matching the security ranking. LoadFromCertStore short-circuits when no store identifiers are set, so file-only users pay no cost and don't get misleading error logs.
4	README strategy ordering	Addressed by #3 — code now matches the README; no README change needed.
5	AllowUnsafeBlocks in main csproj	Declined — required by LibraryImport source generator (SYSLIB1062). See reply on the comment thread.

New tests added

• CertStoreWinsOverPfxWhenBothConfigured — installs a self-signed cert in CurrentUser\My via a new TransientStoreCert fixture (no admin required), configures AuthConfig with both a store thumbprint AND a PfxPath, asserts the store cert wins.
• FallsThroughToPfxWhenCertStoreThumbprintNotFound — verifies that a missing store thumbprint doesn't short-circuit the chain; PFX still loads.

Test count: 37 → 39, all passing locally in ~1s.

Suppressed low-confidence comment

Copilot also flagged a pre-existing issue in ConfigService.GetRegistryValue where as string silently drops non-string policy values (e.g., REG_DWORD). This would break GetRegistryBool / GetRegistryInt when policies are deployed via Intune CSP with DWORD data types, not string equivalents. It's a real bug but pre-exists this PR — filing as a follow-up rather than expanding scope here.