Health Check: Add check for untrusted database constraints on SQL Server

src/Umbraco.Core/Constants-HealthChecks.cs

@@ -47,6 +47,17 @@ public static class LiveEnvironment
                 public const string RuntimeModeCheck = "https://docs.umbraco.com/umbraco-cms/fundamentals/setup/server-setup/runtime-modes";
             }
 
+            /// <summary>
+            ///     Contains documentation links for data health checks.
+            /// </summary>
+            public static class Data
+            {
+                /// <summary>
+                ///     The documentation link for untrusted database constraints check.
+                /// </summary>
+                public const string UntrustedConstraintsCheck = "https://umbra.co/healthchecks-untrusted-constraints";
+            }
+
             /// <summary>
             ///     Contains documentation links for configuration health checks.
             /// </summary>

src/Umbraco.Infrastructure/HealthChecks/Checks/Data/UntrustedDatabaseConstraintsCheck.cs

@@ -0,0 +1,105 @@
+using System.Net;
+using System.Text;
+using Umbraco.Cms.Core;
+using Umbraco.Cms.Core.HealthChecks;
+using Umbraco.Cms.Infrastructure.Persistence;
+
+namespace Umbraco.Cms.Infrastructure.HealthChecks.Checks.Data;
+
+/// <summary>
+///     Health check that detects untrusted foreign key and check constraints on SQL Server.
+/// </summary>
+/// <remarks>
+///     An untrusted constraint (<c>is_not_trusted = 1</c>) means SQL Server has not verified that
+///     all existing rows satisfy the constraint. This prevents the query optimizer from using the
+///     constraint for join elimination and cardinality estimation, and indicates that orphaned or
+///     otherwise invalid rows may exist. The <c>RetrustForeignKeyAndCheckConstraints</c> migration
+///     attempts to re-trust these constraints automatically but logs a warning and continues when
+///     existing data violates a constraint, leaving the issue for manual resolution.
+/// </remarks>
+[HealthCheck(
+    "0B1E71E4-8D37-4F9B-A9A4-86C5B9EA5B0B",
+    "Untrusted database constraints",
+    Description = "Checks for untrusted foreign key or check constraints on SQL Server, which indicate pre-existing data integrity issues that must be resolved manually.",
+    Group = "Data Integrity")]
+public class UntrustedDatabaseConstraintsCheck : HealthCheck
+{
+    private readonly IUmbracoDatabaseFactory _databaseFactory;
+
+    /// <summary>
+    ///     Initializes a new instance of the <see cref="UntrustedDatabaseConstraintsCheck" /> class.
+    /// </summary>
+    /// <param name="databaseFactory">The database factory used to create a connection for the check.</param>
+    public UntrustedDatabaseConstraintsCheck(IUmbracoDatabaseFactory databaseFactory)
+        => _databaseFactory = databaseFactory;
+
+    /// <inheritdoc />
+    public override Task<IEnumerable<HealthCheckStatus>> GetStatusAsync()
+        => Task.FromResult<IEnumerable<HealthCheckStatus>>([CheckConstraints()]);
+
+    private HealthCheckStatus CheckConstraints()
+    {
+        if (_databaseFactory.Configured is false || _databaseFactory.CanConnect is false)
+        {
+            return new HealthCheckStatus("Cannot connect to the database.")
+            {
+                ResultType = StatusResultType.Info,
+            };
+        }
+
+        using IUmbracoDatabase db = _databaseFactory.CreateDatabase();
+
+#pragma warning disable CS0618 // Type or member is obsolete - justification: in this case we do want to check the database type before running SQL Server-specific queries.
+        if (db.DatabaseType.IsSqlServer() is false)
+        {
+            return new HealthCheckStatus("Not applicable — this check only runs on SQL Server.")
+            {
+                ResultType = StatusResultType.Info,
+            };
+        }
+#pragma warning restore CS0618 // Type or member is obsolete
+
+        List<UntrustedConstraintsQuery.UntrustedConstraintDto> untrustedConstraints =
+            db.Fetch<UntrustedConstraintsQuery.UntrustedConstraintDto>(UntrustedConstraintsQuery.Sql);
+
+        if (untrustedConstraints.Count == 0)
+        {
+            return new HealthCheckStatus("All Umbraco database constraints are trusted.")
+            {
+                ResultType = StatusResultType.Success,
+            };
+        }
+
+        return new HealthCheckStatus(BuildMessage(untrustedConstraints))
+        {
+            ResultType = StatusResultType.Warning,
+            ReadMoreLink = Constants.HealthChecks.DocumentationLinks.Data.UntrustedConstraintsCheck,
+        };
+    }
+
+    private static string BuildMessage(IReadOnlyList<UntrustedConstraintsQuery.UntrustedConstraintDto> constraints)
+    {
+        var sb = new StringBuilder();
+        sb.Append("<p>Found ")
+            .Append(constraints.Count)
+            .AppendLine(" untrusted constraint(s). These indicate pre-existing data integrity issues that need to be resolved manually.</p>");
+        sb.AppendLine(
+            "<p>Untrusted constraints prevent the SQL Server query optimizer from using them and indicate that orphaned or invalid rows may exist. Resolve the underlying data issue, then re-trust the constraint with <code>ALTER TABLE ... WITH CHECK CHECK CONSTRAINT</code>.</p>");
+        sb.AppendLine("<ul>");
+        foreach (UntrustedConstraintsQuery.UntrustedConstraintDto constraint in constraints)
+        {
+            sb.Append("<li>")
+                .Append(WebUtility.HtmlEncode(constraint.ConstraintType))
+                .Append(" <code>")
+                .Append(WebUtility.HtmlEncode(constraint.ConstraintName))
+                .Append("</code> on <code>")
+                .Append(WebUtility.HtmlEncode(constraint.SchemaName))
+                .Append('.')
+                .Append(WebUtility.HtmlEncode(constraint.TableName))
+                .AppendLine("</code></li>");
+        }
+
+        sb.AppendLine("</ul>");
+        return sb.ToString();
+    }
+}

src/Umbraco.Infrastructure/Migrations/Upgrade/V_17_3_0/RetrustForeignKeyAndCheckConstraints.cs

@@ -51,28 +51,8 @@ protected override Task MigrateAsync()
 
     private void RetrustConstraints()
     {
-        // Only target Umbraco tables (prefixed "umbraco" or "cms") to avoid touching
-        // custom or third-party tables that share the same database.
-        List<UntrustedConstraintDto> untrustedConstraints = Database.Fetch<UntrustedConstraintDto>(
-            @"SELECT
-                s.name AS SchemaName,
-                OBJECT_NAME(fk.parent_object_id) AS TableName,
-                fk.name AS ConstraintName
-            FROM sys.foreign_keys fk
-            INNER JOIN sys.schemas s ON fk.schema_id = s.schema_id
-            WHERE fk.is_not_trusted = 1
-              AND (OBJECT_NAME(fk.parent_object_id) LIKE 'umbraco%' OR OBJECT_NAME(fk.parent_object_id) LIKE 'cms%')
-
-            UNION ALL
-
-            SELECT
-                s.name AS SchemaName,
-                OBJECT_NAME(cc.parent_object_id) AS TableName,
-                cc.name AS ConstraintName
-            FROM sys.check_constraints cc
-            INNER JOIN sys.schemas s ON cc.schema_id = s.schema_id
-            WHERE cc.is_not_trusted = 1
-              AND (OBJECT_NAME(cc.parent_object_id) LIKE 'umbraco%' OR OBJECT_NAME(cc.parent_object_id) LIKE 'cms%')");
+        List<UntrustedConstraintsQuery.UntrustedConstraintDto> untrustedConstraints =
+            Database.Fetch<UntrustedConstraintsQuery.UntrustedConstraintDto>(UntrustedConstraintsQuery.Sql);
 
         if (untrustedConstraints.Count == 0)
         {
@@ -95,7 +75,7 @@ FROM sys.check_constraints cc
         using IUmbracoDatabase db = _databaseFactory.CreateDatabase();
         EnsureLongCommandTimeout(db);
 
-        foreach (UntrustedConstraintDto constraint in untrustedConstraints)
+        foreach (UntrustedConstraintsQuery.UntrustedConstraintDto constraint in untrustedConstraints)
         {
             // Leading semicolon prevents NPoco's auto-select from prepending
             // "SELECT ... FROM []" based on the empty [TableName("")] attribute.
@@ -140,19 +120,6 @@ BEGIN CATCH
             untrustedConstraints.Count);
     }
 
-    [TableName("")]
-    private class UntrustedConstraintDto
-    {
-        [Column("SchemaName")]
-        public string SchemaName { get; set; } = null!;
-
-        [Column("TableName")]
-        public string TableName { get; set; } = null!;
-
-        [Column("ConstraintName")]
-        public string ConstraintName { get; set; } = null!;
-    }
-
     [TableName("")]
     private class RetrustResultDto
     {

src/Umbraco.Infrastructure/Persistence/UntrustedConstraintsQuery.cs

@@ -0,0 +1,69 @@
+using NPoco;
+
+namespace Umbraco.Cms.Infrastructure.Persistence;
+
+/// <summary>
+/// Shared SQL and DTO for identifying untrusted foreign key and check constraints on SQL Server.
+/// Consumed by the <c>RetrustForeignKeyAndCheckConstraints</c> migration and the
+/// <c>UntrustedDatabaseConstraintsCheck</c> health check.
+/// </summary>
+internal static class UntrustedConstraintsQuery
+{
+    /// <summary>
+    /// Returns one row per untrusted constraint (<c>is_not_trusted = 1</c>) on Umbraco tables
+    /// (those prefixed "umbraco" or "cms"), across both foreign keys and check constraints.
+    /// Other tables in the same database are deliberately excluded.
+    /// </summary>
+    public const string Sql = @"SELECT
+    'Foreign key' AS ConstraintType,
+    s.name AS SchemaName,
+    OBJECT_NAME(fk.parent_object_id) AS TableName,
+    fk.name AS ConstraintName
+FROM sys.foreign_keys fk
+INNER JOIN sys.schemas s ON fk.schema_id = s.schema_id
+WHERE fk.is_not_trusted = 1
+  AND (OBJECT_NAME(fk.parent_object_id) LIKE 'umbraco%' OR OBJECT_NAME(fk.parent_object_id) LIKE 'cms%')
+
+UNION ALL
+
+SELECT
+    'Check constraint' AS ConstraintType,
+    s.name AS SchemaName,
+    OBJECT_NAME(cc.parent_object_id) AS TableName,
+    cc.name AS ConstraintName
+FROM sys.check_constraints cc
+INNER JOIN sys.schemas s ON cc.schema_id = s.schema_id
+WHERE cc.is_not_trusted = 1
+  AND (OBJECT_NAME(cc.parent_object_id) LIKE 'umbraco%' OR OBJECT_NAME(cc.parent_object_id) LIKE 'cms%')";
+
+    /// <summary>
+    /// A row returned by <see cref="Sql"/>, describing a single untrusted constraint.
+    /// </summary>
+    [TableName("")]
+    public class UntrustedConstraintDto
+    {
+        /// <summary>
+        /// Gets or sets the kind of constraint — either <c>"Foreign key"</c> or <c>"Check constraint"</c>.
+        /// </summary>
+        [Column("ConstraintType")]
+        public string ConstraintType { get; set; } = null!;
+
+        /// <summary>
+        /// Gets or sets the schema name of the table the constraint belongs to (e.g. <c>"dbo"</c>).
+        /// </summary>
+        [Column("SchemaName")]
+        public string SchemaName { get; set; } = null!;
+
+        /// <summary>
+        /// Gets or sets the name of the table the constraint belongs to.
+        /// </summary>
+        [Column("TableName")]
+        public string TableName { get; set; } = null!;
+
+        /// <summary>
+        /// Gets or sets the name of the untrusted constraint.
+        /// </summary>
+        [Column("ConstraintName")]
+        public string ConstraintName { get; set; } = null!;
+    }
+}

tests/Umbraco.Tests.Integration/Umbraco.Infrastructure/HealthChecks/UntrustedDatabaseConstraintsCheckTests.cs

@@ -0,0 +1,89 @@
+using Microsoft.Extensions.DependencyInjection;
+using NUnit.Framework;
+using Umbraco.Cms.Core.HealthChecks;
+using Umbraco.Cms.Infrastructure.HealthChecks.Checks.Data;
+using Umbraco.Cms.Infrastructure.Persistence;
+using Umbraco.Cms.Tests.Common.Testing;
+using Umbraco.Cms.Tests.Integration.Testing;
+
+namespace Umbraco.Cms.Tests.Integration.Umbraco.Infrastructure.HealthChecks;
+
+[TestFixture]
+[UmbracoTest(Database = UmbracoTestOptions.Database.NewSchemaPerTest)]
+internal sealed class UntrustedDatabaseConstraintsCheckTests : UmbracoIntegrationTest
+{
+    private const string TestConstraintName = "FK_umbracoRelation_umbracoNode";
+
+    private UntrustedDatabaseConstraintsCheck CreateSut()
+        => new(Services.GetRequiredService<IUmbracoDatabaseFactory>());
+
+    [Test]
+    public async Task FreshDatabase_ReportsSuccessOnSqlServer()
+    {
+        if (BaseTestDatabase.IsSqlite())
+        {
+            Assert.Ignore("Untrusted constraints are a SQL Server concept and do not apply to SQLite.");
+            return;
+        }
+
+        HealthCheckStatus status = (await CreateSut().GetStatusAsync()).Single();
+
+        Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Success));
+    }
+
+    [Test]
+    public async Task UntrustedConstraint_ReportsWarningAndIncludesConstraintName()
+    {
+        if (BaseTestDatabase.IsSqlite())
+        {
+            Assert.Ignore("Untrusted constraints are a SQL Server concept and do not apply to SQLite.");
+            return;
+        }
+
+        MakeConstraintUntrusted(TestConstraintName);
+
+        try
+        {
+            HealthCheckStatus status = (await CreateSut().GetStatusAsync()).Single();
+
+            Assert.Multiple(() =>
+            {
+                Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Warning));
+                Assert.That(status.Message, Does.Contain(TestConstraintName));
+                Assert.That(status.ReadMoreLink, Is.Not.Null.And.Not.Empty);
+            });
+        }
+        finally
+        {
+            RetrustConstraint(TestConstraintName);
+        }
+    }
+
+    [Test]
+    public async Task SqliteDatabase_ReportsInfoShortCircuit()
+    {
+        if (BaseTestDatabase.IsSqlite() is false)
+        {
+            Assert.Ignore("This test verifies the SQLite short-circuit path.");
+            return;
+        }
+
+        HealthCheckStatus status = (await CreateSut().GetStatusAsync()).Single();
+
+        Assert.That(status.ResultType, Is.EqualTo(StatusResultType.Info));
+    }
+
+    private void MakeConstraintUntrusted(string constraintName)
+        => ExecuteNonQuery(
+            $"ALTER TABLE [umbracoRelation] NOCHECK CONSTRAINT [{constraintName}];" +
+            $"ALTER TABLE [umbracoRelation] WITH NOCHECK CHECK CONSTRAINT [{constraintName}];");
+
+    private void RetrustConstraint(string constraintName)
+        => ExecuteNonQuery($"ALTER TABLE [umbracoRelation] WITH CHECK CHECK CONSTRAINT [{constraintName}];");
+
+    private void ExecuteNonQuery(string sql)
+    {
+        using IUmbracoDatabase db = Services.GetRequiredService<IUmbracoDatabaseFactory>().CreateDatabase();
+        db.Execute(sql);
+    }
+}