fix!: harden dapper providers and skip keys with null xml

AGENTS.md

@@ -50,9 +50,9 @@ ASP.NET Core Data Protection (IXmlRepository)
 
 - **Core:** `PersistKeysToStorage<TStorage>()` registers a singleton `IConfigureOptions<KeyManagementOptions>` that sets `XmlRepository` to a `StorageWrapper<TStorage>`, or to a `DeletableStorageWrapper<TStorage>` when `TStorage` is assignable to `IDeletableDataProtectionStorage` (so `XmlKeyManager.CanDeleteKeys` reflects the storage). It does **not** register `TStorage`; the caller must.
 - `DeletableStorageWrapper.DeleteElements` offers the parsed keys (malformed rows skipped), then passes the chosen `GetAll()` instances, ordered by `DeletionOrder`, to a single `Delete()` call. A storage exception is logged (`ErrorDeletingKeysFromStorage`) and returns `false`. Note: in ASP.NET Core 10.0, `XmlKeyManager` exposes `DeleteKeys` but does not implement `IDeletableKeyManager`.
-- `StorageWrapper` (internal) creates a new DI scope and resolves `TStorage` on every `GetAllElements` / `StoreElement` call, so scoped storage implementations work. It trims `FriendlyName`, normalizes empty or whitespace-only names to `null`, and wraps any `Insert` exception in `KeyInsertException`. `GetAllElements` logs and skips rows with malformed XML (`SkippingMalformedKey`) instead of failing the whole key ring.
+- `StorageWrapper` (internal) creates a new DI scope and resolves `TStorage` on every `GetAllElements` / `StoreElement` call, so scoped storage implementations work. It trims `FriendlyName`, normalizes empty or whitespace-only names to `null`, and wraps any `Insert` exception in `KeyInsertException`. `GetAllElements` logs and skips rows with malformed or `null` XML (`SkippingMalformedKey`) instead of failing the whole key ring.
 - Never log or expose key XML (it may contain unencrypted key material); this includes `XmlException` messages, which can quote the input.
-- **DB providers:** `PersistKeysWithDapperInSqlServer(...)` / `PersistKeysWithDapperInPostgreSQL(...)` configure `IOptions<DapperDataProtectionConfig>` with an internal per-provider `DapperDataProtectionConfigValidator` and `ValidateOnStart()`, register the connection source as a singleton (`SqlConnectionFactory`; `NpgsqlDataSource` as a keyed singleton under the internal `Constants.DataSourceServiceKey`, created by a factory so the container disposes it, and so it never collides with the application's own data source), register the repository as scoped `IDbDataProtectionStorage`, then call `PersistKeysToStorage<IDbDataProtectionStorage>()`.
+- **DB providers:** `PersistKeysWithDapperInSqlServer(...)` / `PersistKeysWithDapperInPostgreSQL(...)` configure `IOptions<DapperDataProtectionConfig>` with an internal per-provider `DapperDataProtectionConfigValidator` and `ValidateOnStart()`, register the connection source as a keyed singleton, so it never collides with one registered by the application (`SqlConnectionFactory` under the internal `Constants.ConnectionFactoryServiceKey`; `NpgsqlDataSource` under the internal `Constants.DataSourceServiceKey`, created by a factory so the container disposes it), register the repository as scoped `IDbDataProtectionStorage`, then call `PersistKeysToStorage<IDbDataProtectionStorage>()`.
 - `IServiceProvider.UseDapperDataProtection()` (in the shared Dapper project) must be called at startup. It resolves the registrations and runs `InitializeDb()` when `DapperDataProtectionConfig.InitializeTable` is `true` (the default). Reading the options there also triggers validation when no host runs `ValidateOnStart`.
 
 ### Database Implementations
@@ -67,9 +67,9 @@ Both repositories use raw SQL via Dapper. They differ in naming conventions:
 
 - The PostgreSQL `SELECT` aliases each snake_case column to its `DataProtectionKeyEntity` property name (`AS "{nameof(...)}"`). Never set the process-wide `Dapper.DefaultTypeMap.MatchNamesWithUnderscores`: it changes column mapping for every Dapper query in the host application.
 - `Delete()` requires `DataProtectionKeyEntity` instances (as returned by `GetAll()`) and deletes by `Id`, row by row in order, in one transaction.
-- Schema and table names are validated at startup (SQL Server: `^[A-Za-z_][A-Za-z0-9_]*`, schema <= 128, table <= 112 to fit `IX_{table}_FriendlyName`; PostgreSQL: lowercase only, <= 63) and always quoted (`[...]` / `"..."`, with the closing quote escaped). PostgreSQL names are lowercase because earlier versions did not quote them: a mixed-case name would silently target a new, empty table.
+- Schema and table names are validated at startup (SQL Server: `^[A-Za-z_][A-Za-z0-9_]*`, schema <= 128, table <= 112 to fit `IX_{table}_FriendlyName`; PostgreSQL: lowercase only, schema + table <= 45 to fit `ix_{schema}_{table}_friendly_name` in 63) and always quoted (`[...]` / `"..."`, with the closing quote escaped). PostgreSQL names are lowercase because earlier versions did not quote them: a mixed-case name would silently target a new, empty table.
 - SQL Server passes names as parameters wherever T-SQL accepts them (existence checks, `QUOTENAME`, the duplicate-index error message).
-- `InitializeDb()` creates a missing schema. It checks existence first (`SCHEMA_ID` / `pg_namespace`), because PostgreSQL's `CREATE SCHEMA IF NOT EXISTS` requires the database CREATE privilege even when the schema exists.
+- `InitializeDb()` creates the schema, the table and the index only when missing. On PostgreSQL it checks existence first (`pg_namespace`, `pg_tables`, `pg_indexes`), because PostgreSQL checks privileges before `IF NOT EXISTS`: a user without CREATE on the database or schema, or without ownership of the table, would otherwise fail on every start.
 - Defaults are in each provider's `Constants.cs`. Keep the table DDL in sync with the "Database schema" section of `README.md`.
 
 ### Testing

README.md

@@ -101,6 +101,10 @@ app.Services.UseDapperDataProtection();
 app.Run();
 ```
 
+The provider registers its `SqlConnectionFactory` as a keyed service. A `SqlConnectionFactory`
+registered by the application is neither replaced nor used for the keys, whatever the registration
+order.
+
 ### PostgreSQL
 
 ```csharp
@@ -146,8 +150,9 @@ builder.Services.AddDataProtection()
 `UseDapperDataProtection()` is an extension on `IServiceProvider`. Call it once at startup
 (`app.Services.UseDapperDataProtection()`): it validates the service registrations and, when
 `InitializeTable` is `true`, creates the schema, the table and the friendly-name unique index if
-they are missing. The schema is created only when it does not exist, so the database user needs the
-permission to create schemas only in that case. The index is also created on an existing table that lacks it; if that table already
+they are missing. Each object is created only when it does not exist, so the database user needs the
+permission to create it only in that case: once the schema, the table and the index exist, a user
+that can only read, insert and delete rows of the table is enough. The index is also created on an existing table that lacks it; if that table already
 contains duplicate non-null friendly names, initialization fails and the duplicates must be
 removed before retrying. Initialization is safe when several instances start at the same time:
 it runs in a transaction that holds a database-wide lock (`sp_getapplock` on SQL Server,
@@ -164,7 +169,7 @@ the option and the rule:
 | | SQL Server | PostgreSQL |
 |---|---|---|
 | Allowed characters | letters, digits and `_`; must not start with a digit | lowercase letters, digits and `_`; must not start with a digit |
-| Maximum length | 128 for the schema, 112 for the table (room for the `IX_{TableName}_FriendlyName` index name) | 63 |
+| Maximum length | 128 for the schema, 112 for the table (room for the `IX_{TableName}_FriendlyName` index name) | 45 for the schema and the table together (room for the `ix_{SchemaName}_{TableName}_friendly_name` index name within the 63-character identifier limit) |
 
 The names are always quoted in the generated SQL, so reserved words such as `order` or `user` are
 allowed.
@@ -345,7 +350,7 @@ Protection key repository. The adapter maps the framework's XML-element calls to
 service provider for every operation. When `TStorage` implements `IDeletableDataProtectionStorage`,
 the adapter also implements the framework's `IDeletableXmlRepository` and maps deletions to `Delete()`.
 
-When the key ring is loaded, a stored key whose XML is malformed is skipped and logged as an error
+When the key ring is loaded, a stored key whose XML is malformed or missing is skipped and logged as an error
 (event `SkippingMalformedKey`, with the friendly name and the parse position; the XML itself is never
 logged). The remaining keys stay usable, but data protected with the skipped key can no longer be
 unprotected, and if no valid key remains, Data Protection generates a new one.
@@ -389,6 +394,12 @@ with an `OptionsValidationException`. The rules are listed in
 - **PostgreSQL:** names must be lowercase. v1 did not quote the names, so PostgreSQL folded them to
   lowercase: a table configured as `DpKeys` was created as `dpkeys`. Replace the configured name
   with its lowercase form; the database does not need to change.
+- **PostgreSQL:** the schema and table names together must be at most 45 characters long. v1
+  accepted longer names, but PostgreSQL silently truncated the derived index name to 63
+  characters, so two tables could end up with the same index name. Rename the table
+  (`ALTER TABLE ... RENAME TO ...`) and its index to `ix_{SchemaName}_{TableName}_friendly_name`
+  (`ALTER INDEX ... RENAME TO ...`), then update the configuration. If the index is not renamed,
+  `InitializeDb()` creates a second unique index with the new name.
 
 #### The PostgreSQL provider no longer enables Dapper's underscore mapping
 
@@ -422,6 +433,24 @@ v1 registered its `NpgsqlDataSource` as an unkeyed singleton. v2 registers it as
 A malformed connection string now throws when the data source is first resolved, in
 `UseDapperDataProtection()`, instead of in `PersistKeysWithDapperInPostgreSQL()`.
 
+#### The SQL Server provider registers its connection factory as a keyed service
+
+v1 registered its `SqlConnectionFactory` as an unkeyed singleton. v2 registers it as a keyed service
+(see [SQL Server](#sql-server)). This affects:
+
+- Applications that resolve `SqlConnectionFactory` from dependency injection and received the keys
+  connection factory: register your own `SqlConnectionFactory`.
+- Applications that register `SQLServerDataProtectionRepository` themselves instead of calling
+  `PersistKeysWithDapperInSqlServer()`: the constructor now resolves the provider's keyed connection
+  factory. Call `PersistKeysWithDapperInSqlServer()`, or register the repository with a factory
+  that passes the connection factory explicitly:
+
+  ```csharp
+  builder.Services.AddScoped<IDbDataProtectionStorage>(sp => new SQLServerDataProtectionRepository(
+      new SqlConnectionFactory(connectionString),
+      sp.GetRequiredService<IOptionsSnapshot<DapperDataProtectionConfig>>()));
+  ```
+
 #### `StorageWrapper<TStorage>` is internal
 
 Affects code that created `StorageWrapper<TStorage>` directly, for example to assign
@@ -461,9 +490,12 @@ These need no code change, but can affect a running application:
 - **Missing schema created.** When `InitializeTable` is `true`, `InitializeDb()` creates a missing
   schema instead of failing. The database user needs the permission to create schemas only in
   that case.
+- **PostgreSQL initialization with a restricted user.** `InitializeDb()` creates the table and the
+  index only when they are missing. In v1, `CREATE TABLE IF NOT EXISTS` failed on every start for a
+  user without the `CREATE` privilege on the schema, even when the table existed.
 - **Blank friendly names stored as `NULL`.** An empty or whitespace-only friendly name is passed to
   `Insert` as `null`; v1 passed an empty string. Custom storages must accept `null`.
-- **Malformed keys skipped.** A stored key whose XML is malformed is logged
+- **Malformed keys skipped.** A stored key whose XML is malformed or `null` is logged
   (`SkippingMalformedKey`) and skipped; in v1 it made loading the whole key ring fail.
 - **No key XML in `KeyInsertException`.** The exception message names the key by friendly name
   only; the storage exception is the `InnerException`.

src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/DapperDataProtectionConfigValidator.cs

@@ -19,23 +19,40 @@ internal sealed partial class DapperDataProtectionConfigValidator : IValidateOpt
     /// </summary>
     internal const int MaxNameLength = 63;
 
+    /// <summary>
+    /// Maximum combined length of the schema and table names, which leaves room for the derived index name
+    /// <c>ix_{SchemaName}_{TableName}_friendly_name</c>, the longest name derived from them.
+    /// PostgreSQL would silently truncate a longer name, so two tables could get the same index name.
+    /// </summary>
+    internal static readonly int MaxCombinedNameLength =
+        MaxNameLength - PostgreSQLDataProtectionRepository.GetIndexName(string.Empty, string.Empty).Length;
+
     /// <inheritdoc />
     public ValidateOptionsResult Validate(string? name, DapperDataProtectionConfig options)
     {
         var failures = new List<string>();
         Validate(nameof(DapperDataProtectionConfig.SchemaName), options.SchemaName, failures);
         Validate(nameof(DapperDataProtectionConfig.TableName), options.TableName, failures);
+        if (failures.Count == 0 && options.SchemaName.Length + options.TableName.Length > MaxCombinedNameLength)
+        {
+            failures.Add(
+                $"{nameof(DapperDataProtectionConfig)}.{nameof(DapperDataProtectionConfig.SchemaName)} '{options.SchemaName}' " +
+                $"and {nameof(DapperDataProtectionConfig)}.{nameof(DapperDataProtectionConfig.TableName)} '{options.TableName}' " +
+                $"are too long: together they must be at most {MaxCombinedNameLength} characters long, so that the " +
+                $"derived index name fits the PostgreSQL limit of {MaxNameLength} characters.");
+        }
+
         return failures.Count == 0 ? ValidateOptionsResult.Success : ValidateOptionsResult.Fail(failures);
     }
 
     private static void Validate(string optionName, string? value, List<string> failures)
     {
-        if (value is null || value.Length > MaxNameLength || !IdentifierRegex().IsMatch(value))
+        if (value is null || !IdentifierRegex().IsMatch(value))
         {
             failures.Add(
                 $"{nameof(DapperDataProtectionConfig)}.{optionName} '{value}' is not a valid PostgreSQL name: " +
-                "it must start with a lowercase letter or an underscore, contain only lowercase letters, digits " +
-                $"and underscores, and be at most {MaxNameLength} characters long.");
+                "it must start with a lowercase letter or an underscore and contain only lowercase letters, digits " +
+                "and underscores.");
         }
     }
 

src/AspNetCore.DataProtection.CustomStorage.Dapper.PostgreSQL/PostgreSQLDataProtectionRepository.cs

@@ -123,8 +123,9 @@ public void InitializeDb()
         connection.Execute("SELECT pg_advisory_xact_lock(hashtext(@LockName));",
             new { LockName = InitializeDbLockName },
             transaction);
-        // CREATE SCHEMA IF NOT EXISTS requires the CREATE privilege on the database even when the schema
-        // exists, so it is executed only when the schema is missing.
+        // PostgreSQL checks privileges before IF NOT EXISTS: CREATE SCHEMA requires the CREATE privilege on the
+        // database, CREATE TABLE the CREATE privilege on the schema and CREATE INDEX the ownership of the table,
+        // even when the object exists. Each object is therefore created only when it is missing.
         var schemaExists = connection.ExecuteScalar<bool>(
             "SELECT EXISTS(SELECT 1 FROM pg_catalog.pg_namespace WHERE nspname = @SchemaName);",
             new { _config.SchemaName },
@@ -135,24 +136,49 @@ public void InitializeDb()
                 transaction: transaction);
         }
 
-        connection.Execute($"""
-             CREATE TABLE IF NOT EXISTS {_tableName}
-              (
-                 id INTEGER generated always as identity,
-                 insert_date timestamp with time zone NOT NULL default NOW(),
-                 friendly_name character varying(256) COLLATE pg_catalog."default" NULL,
-                 xml text COLLATE pg_catalog."default" NOT NULL,
-                 CONSTRAINT {QuoteIdentifier($"pk_{_config.SchemaName}_{_config.TableName}")} PRIMARY KEY (id)
-              );
-
-              CREATE UNIQUE INDEX IF NOT EXISTS {QuoteIdentifier($"ix_{_config.SchemaName}_{_config.TableName}_friendly_name")}
-              ON {_tableName} USING btree
-              (friendly_name COLLATE pg_catalog."default" ASC NULLS LAST)
-              TABLESPACE pg_default;
-             """,
-            transaction: transaction);
+        var tableExists = connection.ExecuteScalar<bool>(
+            "SELECT EXISTS(SELECT 1 FROM pg_catalog.pg_tables WHERE schemaname = @SchemaName AND tablename = @TableName);",
+            new { _config.SchemaName, _config.TableName },
+            transaction);
+        if (!tableExists)
+        {
+            connection.Execute($"""
+                 CREATE TABLE IF NOT EXISTS {_tableName}
+                  (
+                     id INTEGER generated always as identity,
+                     insert_date timestamp with time zone NOT NULL default NOW(),
+                     friendly_name character varying(256) COLLATE pg_catalog."default" NULL,
+                     xml text COLLATE pg_catalog."default" NOT NULL,
+                     CONSTRAINT {QuoteIdentifier($"pk_{_config.SchemaName}_{_config.TableName}")} PRIMARY KEY (id)
+                  );
+                 """,
+                transaction: transaction);
+        }
+
+        var indexName = GetIndexName(_config.SchemaName, _config.TableName);
+        var indexExists = connection.ExecuteScalar<bool>(
+            "SELECT EXISTS(SELECT 1 FROM pg_catalog.pg_indexes WHERE schemaname = @SchemaName AND indexname = @IndexName);",
+            new { _config.SchemaName, IndexName = indexName },
+            transaction);
+        if (!indexExists)
+        {
+            connection.Execute($"""
+                 CREATE UNIQUE INDEX IF NOT EXISTS {QuoteIdentifier(indexName)}
+                 ON {_tableName} USING btree
+                 (friendly_name COLLATE pg_catalog."default" ASC NULLS LAST)
+                 TABLESPACE pg_default;
+                 """,
+                transaction: transaction);
+        }
+
         transaction.Commit();
     }
 
+    /// <summary>
+    /// Returns the name of the friendly-name unique index, the longest name derived from the schema and table names.
+    /// </summary>
+    internal static string GetIndexName(string schemaName, string tableName) =>
+        $"ix_{schemaName}_{tableName}_friendly_name";
+
     private static string QuoteIdentifier(string name) => $"\"{name.Replace("\"", "\"\"")}\"";
 }

src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/Constants.cs

@@ -13,4 +13,9 @@ public static class Constants
     /// default keys table name
     /// </summary>
     public const string TableName = "DataProtectionKeys";
+    /// <summary>
+    /// service key of the <see cref="SqlConnectionFactory"/> registered by the provider,
+    /// so that it does not collide with a connection factory registered by the application
+    /// </summary>
+    internal const string ConnectionFactoryServiceKey = "AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer";
 }

src/AspNetCore.DataProtection.CustomStorage.Dapper.SQLServer/DapperDataProtectionExtensions.cs

@@ -41,7 +41,9 @@ public static IDataProtectionBuilder PersistKeysWithDapperInSqlServer(this IData
         builder.Services.TryAddEnumerable(ServiceDescriptor
             .Singleton<IValidateOptions<DapperDataProtectionConfig>, DapperDataProtectionConfigValidator>());
 
-        builder.Services.AddSingleton(_=>new SqlConnectionFactory(connectionString));
+        // Keyed, so that a connection factory registered by the application is neither replaced nor used.
+        builder.Services.AddKeyedSingleton(Constants.ConnectionFactoryServiceKey,
+            (_, _) => new SqlConnectionFactory(connectionString));
         builder.Services.AddScoped<IDbDataProtectionStorage, SQLServerDataProtectionRepository>();
         builder.PersistKeysToStorage<IDbDataProtectionStorage>();
         return builder;