Files

T

SluijsensandClaude Opus 5 980dc80701 Records that the key-ring move leaves older ciphertext unreadable

Starting both hosts after U2 fails to decrypt a stored slave API key: the
row was encrypted under the old file-based key ring, and the database key
ring holds a different, freshly generated key. The key ring working
correctly is exactly why the old key is not found.

U2 moved the ring without a path for ciphertext that predates it. Confirmed
with the user that nothing is deployed yet, so no migration ships and the
first deploy meets an empty database. ASM-08 records the condition and makes
it blocking for U6 if it ever stops holding.

Also notes that the master lets the CryptographicException escape while the
slave catches it and reports "master API key rejected" instead — the
ambiguity U4's event was designed around, showing up on day one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01HHoJpxYXzHACSQguHrC5fw

2026-07-28 11:58:19 +02:00

7.3 KiB

Raw Blame History

Code Generation Summary — U2 Data Durability

Date: 2026-07-27 Requirements: FR-11, FR-12

Files Created

Path	Purpose
`src/SlpModularCms.Core/Hosting/DataProtectionExtensions.cs`	`AddCmsDataProtection()` — database key ring, fixed discriminator
`src/SlpModularCms.Core/Hosting/DatabaseMigrationExtensions.cs`	`MigrateCoreDatabase()` — startup migration with failure classification
`src/SlpModularCms.Core/Migrations/20260727203036_AddDataProtectionKeys.cs`	Keys table migration
`src/SlpModularCms.Core.Tests/Hosting/DataProtectionExtensionsTests.cs`	4 tests
`src/SlpModularCms.Core.Tests/Hosting/DatabaseMigrationExtensionsTests.cs`	7 tests
`src/SlpModularCms.Modules.Availability.Tests/AvailabilityModuleDataProtectionTests.cs`	1 test
`src/SlpModularCms.Modules.Master.Tests/MasterModuleDataProtectionTests.cs`	1 test

Files Modified

Path	Change
`src/SlpModularCms.Core/SlpModularCms.Core.csproj`	`Microsoft.AspNetCore.DataProtection.EntityFrameworkCore` 10.0.9
`src/SlpModularCms.Core/Data/ApplicationDbContext.cs`	Implements `IDataProtectionKeyContext`; `DataProtectionKeys` set
`src/SlpModularCms.Modules.Availability/AvailabilityModule.cs`	`AddDataProtection()` removed
`src/SlpModularCms.Modules.Master/MasterModule.cs`	`AddDataProtection()` removed
`src/SlpModularCms.Api/Program.cs`	`AddCmsDataProtection()` before module registration; `MigrateCoreDatabase()` after build
`src/SlpModularCms.Api.Slave/Program.cs`	Same

No duplicate or parallel files were created.

Implementation Decisions

The removals are the point of this unit, so they are commented in place

Deleting services.AddDataProtection() from two modules looks like a regression to anyone who does not know the ordering issue. Both call sites therefore carry a comment explaining that the host owns Data Protection and that a bare call here would silently discard the persistent key store.

The tests assert configuration, not registration

A test asserting "Data Protection is registered" passes in both the broken and fixed cases, because IDataProtector resolves either way. Every test here inspects the resulting configuration instead:

KeyManagementOptions.XmlRepository is EntityFrameworkCoreXmlRepository<ApplicationDbContext> — not the filesystem default
DataProtectionOptions.ApplicationDiscriminator is the fixed constant — not the path-derived default
A protected value survives a simulated restart from a different release directory, which is the property that actually matters
The discriminator contains no path separator, guarding against a future "improvement" that makes it computed or configurable

The module-level tests live in each module's own test project rather than in Core.Tests, because Core.Tests does not reference the modules. Each registers the host's Data Protection first and the module second — the real ordering — and asserts the EF repository survives.

`SqlException` is produced genuinely, not faked

SqlException has no public constructor. Rather than substituting a stand-in type, the test provokes a real one by opening a connection to an unreachable host with a one-second timeout. The classifier is therefore exercised against the exact type it will meet in production.

Wrong credentials are classified as a connection failure

Distinguishing bad credentials from an unreachable server would add branching for no benefit: retries are exhausted and the process does not start either way. The simpler classification is the honest one.

Migration verified as additive

The generated migration only creates a table — no dropped or narrowed columns. Rollback by redeploying an earlier release therefore stays safe, which BR-U2-16 requires and which the whole rollback strategy depends on.

Operational Constraints Enforced by Documentation, Not Code

Both were decided deliberately (U2 FD Q4 = C, Q5 = C). They must appear in the Operations deployment instructions:

Constraint	Why it is not enforced in code
The `DataProtectionKeys` table must never be pruned. Deleting a key makes every value encrypted with it permanently unreadable, including stored slave API keys.	Nothing in the application deletes these rows; the risk comes from a human treating the table as housekeeping. It is the single most destructive maintenance action available against this system, and it looks harmless.
Only one instance may migrate a given database at a time.	One instance per database holds by design today — the Master and each slave have their own. A distributed migration lock would add failure modes without removing any. If the deployment model ever changes to multiple instances sharing a database, automatic startup migration must be revisited before that change is made.

Also recorded for Operations: DEV-05 — keys are stored unencrypted at rest, with TLS on the database connection and a non-public database as the compensating controls (BR-U2-08). These are not optional extras; they are what makes the deviation acceptable.

Verification

Check	Result
`dotnet build SlpModularCms.sln -c Release`	✅ 0 errors
`SlpModularCms.Core.Tests`	✅ 83 passed
`SlpModularCms.Modules.Availability.Tests`	✅ 82 passed
`SlpModularCms.Modules.Identity.Tests`	✅ 37 passed
`SlpModularCms.Modules.Master.Tests`	✅ 51 passed
Migration is purely additive	✅ Inspected — creates one table, drops nothing

No failures occurred during generation.

Not verifiable at this stage: the composed startup path (MigrateCoreDatabase against a real database, and both hosts actually starting) requires SQL Server. Carried to the phase-level Build and Test stage.

Follow-up found in local testing (2026-07-28)

Starting both hosts after U2 produced, on the master:

CryptographicException: The key {2ad1bf17-...} was not found in the key ring
  at ApiKeyProtector.Unprotect
  at CmsInstanceService.VerifyIntegrityAsync

Not a defect in this unit — it is this unit's change meeting data written before it. The stored MasterCmsInstances.ApiKey had been encrypted under the old file-based key ring in %LOCALAPPDATA%\ASP.NET\DataProtection-Keys; both databases now hold their own freshly generated single key, which cannot decrypt it. The key ring working correctly is precisely why the old key is not found.

The gap this unit left: no one-time path for ciphertext that predates the move. Recorded as ASM-08 in requirements.md. Confirmed with the user on 2026-07-28 that the CMS is not deployed anywhere yet, so the first deploy meets an empty database and no migration is shipped. If that ever stops being true, U6 must not deploy until either the affected rows are cleared and the instances re-registered, or this unit gains a fallback that reads the old ring and re-encrypts.

Resolved locally by deleting the one stale row on each side and re-registering the slave.

Worth noting for U4's alerting: the two sides fail differently. Modules.Master/ApiKeyProtector lets the exception escape, while Modules.Availability/MasterApiKeyProtector catches it and returns null, so the slave reports "master API key rejected" instead. That is exactly the ambiguity U4's MasterApiKeyRejected event documents — an intruder, or an unreadable key ring — occurring in practice.

7.3 KiB Raw Blame History