diff --git a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/AuditLogAppendOnlyGuardTests.cs b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/AuditLogAppendOnlyGuardTests.cs index edfdd96a..4ff29cf7 100644 --- a/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/AuditLogAppendOnlyGuardTests.cs +++ b/tests/ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests/AuditLogAppendOnlyGuardTests.cs @@ -13,8 +13,8 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests; /// /// Matching rule (see ContainsAuditLogMutation for full detail) /// A line is flagged as a violation iff it matches the DML-syntax pattern: -/// • UPDATE\s+(?:dbo\.)?AuditLog\b — UPDATE targeting AuditLog -/// • DELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b — DELETE targeting AuditLog +/// • UPDATE\s+(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b — UPDATE targeting AuditLog +/// • DELETE\s+(?:FROM\s+)?(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b — DELETE targeting AuditLog /// /// These tight DML-syntax patterns naturally exclude false positives: /// - DENY UPDATE ON dbo.AuditLog … → "DENY" comes before UPDATE; the regex @@ -25,6 +25,12 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests; /// - Comments like "// AuditLog … UPDATE …" → UPDATE is not immediately followed /// by AuditLog (there are intervening words). /// - DELETE FROM Notifications … → AuditLog not present. +/// +/// Known limitations: This guard scans only raw SQL strings — EF Core methods +/// such as ExecuteDeleteAsync, ExecuteUpdateAsync, and RemoveRange +/// targeting the AuditLog entity are NOT covered and must never be introduced. +/// Additionally, the scan is line-oriented: DML where the keyword and table name appear +/// on separate lines is an accepted, undetected edge case. /// public class AuditLogAppendOnlyGuardTests { @@ -72,10 +78,11 @@ public class AuditLogAppendOnlyGuardTests /// /// Matching rule. The regex requires the DML keyword to be /// immediately followed (possibly via FROM) by the optional schema prefix - /// dbo. and then the table name AuditLog as a whole word: + /// (dbo. or [dbo].) and then the table name AuditLog + /// or [AuditLog] as a whole word: /// - /// UPDATE\s+(?:dbo\.)?AuditLog\b - /// DELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b + /// UPDATE\s+(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b + /// DELETE\s+(?:FROM\s+)?(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b /// /// This tight DML-syntax pattern naturally excludes false positives without /// any additional keyword checks: @@ -106,23 +113,31 @@ public class AuditLogAppendOnlyGuardTests } // DML-syntax pattern: the UPDATE or DELETE keyword must be directly followed - // (optionally via FROM) by the optional "dbo." schema qualifier and then the - // table name "AuditLog" as a whole word. + // (optionally via FROM) by the optional schema qualifier and then the table name. // - // UPDATE\s+(?:dbo\.)?AuditLog\b - // matches: "UPDATE AuditLog …", "UPDATE dbo.AuditLog …" - // does NOT match: "DENY UPDATE ON dbo.AuditLog" (UPDATE is followed by ON, not AuditLog) + // Schema sub-pattern : (?:\[?dbo\]?\.)? + // matches: nothing, "dbo.", "[dbo]." // - // DELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b - // matches: "DELETE FROM AuditLog …", "DELETE FROM dbo.AuditLog …", "DELETE dbo.AuditLog …" - // does NOT match: "DENY DELETE ON dbo.AuditLog" (DELETE is followed by ON, not FROM/AuditLog) - const string dmlPattern = - @"\bUPDATE\s+(?:dbo\.)?AuditLog\b" + - @"|\bDELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b"; - - return Regex.IsMatch(text, dmlPattern, RegexOptions.IgnoreCase); + // Table sub-pattern : \[?AuditLog\]? + // matches: "AuditLog", "[AuditLog]" + // + // UPDATE\s+(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b + // matches: "UPDATE AuditLog", "UPDATE dbo.AuditLog", + // "UPDATE [AuditLog]", "UPDATE [dbo].[AuditLog]" + // does NOT match: "DENY UPDATE ON dbo.AuditLog" (UPDATE is followed by ON) + // + // DELETE\s+(?:FROM\s+)?(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b + // matches: "DELETE FROM AuditLog", "DELETE FROM dbo.AuditLog", + // "DELETE FROM [AuditLog]", "DELETE FROM [dbo].[AuditLog]" + // does NOT match: "DENY DELETE ON dbo.AuditLog" (DELETE is followed by ON) + return AuditLogMutationPattern.IsMatch(text); } + private static readonly Regex AuditLogMutationPattern = new( + @"\bUPDATE\s+(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b" + + @"|\bDELETE\s+(?:FROM\s+)?(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b", + RegexOptions.IgnoreCase | RegexOptions.Compiled); + // --------------------------------------------------------------------------- // Guard test: scan every *.cs file in ConfigurationDatabase (excluding // Designer/Snapshot EF artefacts and the obj/ directory). @@ -248,9 +263,28 @@ public class AuditLogAppendOnlyGuardTests Assert.True(ContainsAuditLogMutation( "update dbo.AuditLog set Actor = 'system' where Actor is null;")); - // AuditLog first, UPDATE second (reverse order — still a violation). + // AuditLog mentioned earlier in the line (e.g. in a comment prefix), with a real + // UPDATE dbo.AuditLog DML following — the DML occurrence must still be caught. Assert.True(ContainsAuditLogMutation( "-- AuditLog: UPDATE dbo.AuditLog SET x = 1")); + + // ---- Bracketed identifier forms (SSMS-generated SQL) ---- + + // UPDATE [dbo].[AuditLog] — bracketed schema and bracketed table. + Assert.True(ContainsAuditLogMutation( + "UPDATE [dbo].[AuditLog] SET DetailsJson = @json WHERE EventId = @id;")); + + // UPDATE [AuditLog] — bracketed table, no schema prefix. + Assert.True(ContainsAuditLogMutation( + "UPDATE [AuditLog] SET Status = 'Corrected' WHERE EventId = @id;")); + + // DELETE FROM [dbo].[AuditLog] — bracketed schema and bracketed table. + Assert.True(ContainsAuditLogMutation( + "DELETE FROM [dbo].[AuditLog] WHERE OccurredAtUtc < @threshold;")); + + // DELETE FROM [AuditLog] — bracketed table, no schema prefix. + Assert.True(ContainsAuditLogMutation( + "DELETE FROM [AuditLog] WHERE OccurredAtUtc < @threshold;")); } [Fact] @@ -273,6 +307,10 @@ public class AuditLogAppendOnlyGuardTests Assert.False(ContainsAuditLogMutation( "DELETE FROM dbo.Notifications WHERE CompletedAtUtc < @cutoff;")); + // Notifications DELETE using bracketed identifiers — AuditLog not present: + Assert.False(ContainsAuditLogMutation( + "DELETE FROM [dbo].[Notifications] WHERE CompletedAtUtc < @cutoff;")); + // SiteCalls DELETE (legitimate; AuditLog not present on the line): Assert.False(ContainsAuditLogMutation( "DELETE FROM dbo.SiteCalls WHERE TerminalAtUtc < @cutoff;"));