test(configdb): M2.10 review fix — catch bracketed AuditLog identifiers; document EF/multi-line scan limits (#18)
Extends ContainsAuditLogMutation regex to match T-SQL bracketed forms ([AuditLog], [dbo].[AuditLog]) that SSMS-generated SQL produces; the prior optional-schema pattern only matched bare/dbo-prefixed names, silently missing these real violation forms. Changes: - Schema sub-pattern (?:dbo\.)? → (?:\[?dbo\]?\.)? (matches dbo. and [dbo].) - Table sub-pattern AuditLog\b → \[?AuditLog\]?\b (matches AuditLog and [AuditLog]) - Pattern compiled as static readonly Regex field for clarity/performance - Adds 4 new planted-positive cases: UPDATE [dbo].[AuditLog], UPDATE [AuditLog], DELETE FROM [dbo].[AuditLog], DELETE FROM [AuditLog] - Retains all existing negatives; adds DELETE FROM [dbo].[Notifications] negative - Fixes misleading "reverse order" comment on the comment-prefix positive case - Documents scan limitations (EF Core bulk methods; multi-line DML) in class XML doc
This commit is contained in:
+57
-19
@@ -13,8 +13,8 @@ namespace ZB.MOM.WW.ScadaBridge.ConfigurationDatabase.Tests;
|
|||||||
///
|
///
|
||||||
/// <b>Matching rule (see <c>ContainsAuditLogMutation</c> for full detail)</b>
|
/// <b>Matching rule (see <c>ContainsAuditLogMutation</c> for full detail)</b>
|
||||||
/// A line is flagged as a violation iff it matches the DML-syntax pattern:
|
/// A line is flagged as a violation iff it matches the DML-syntax pattern:
|
||||||
/// • <c>UPDATE\s+(?:dbo\.)?AuditLog\b</c> — UPDATE targeting AuditLog
|
/// • <c>UPDATE\s+(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b</c> — UPDATE targeting AuditLog
|
||||||
/// • <c>DELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b</c> — DELETE targeting AuditLog
|
/// • <c>DELETE\s+(?:FROM\s+)?(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b</c> — DELETE targeting AuditLog
|
||||||
///
|
///
|
||||||
/// These tight DML-syntax patterns naturally exclude false positives:
|
/// These tight DML-syntax patterns naturally exclude false positives:
|
||||||
/// - DENY UPDATE ON dbo.AuditLog … → "DENY" comes before UPDATE; the regex
|
/// - 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
|
/// - Comments like "// AuditLog … UPDATE …" → UPDATE is not immediately followed
|
||||||
/// by AuditLog (there are intervening words).
|
/// by AuditLog (there are intervening words).
|
||||||
/// - DELETE FROM Notifications … → AuditLog not present.
|
/// - DELETE FROM Notifications … → AuditLog not present.
|
||||||
|
///
|
||||||
|
/// <b>Known limitations:</b> This guard scans only raw SQL strings — EF Core methods
|
||||||
|
/// such as <c>ExecuteDeleteAsync</c>, <c>ExecuteUpdateAsync</c>, and <c>RemoveRange</c>
|
||||||
|
/// 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.
|
||||||
/// </summary>
|
/// </summary>
|
||||||
public class AuditLogAppendOnlyGuardTests
|
public class AuditLogAppendOnlyGuardTests
|
||||||
{
|
{
|
||||||
@@ -72,10 +78,11 @@ public class AuditLogAppendOnlyGuardTests
|
|||||||
///
|
///
|
||||||
/// <b>Matching rule.</b> The regex requires the DML keyword to be
|
/// <b>Matching rule.</b> The regex requires the DML keyword to be
|
||||||
/// immediately followed (possibly via FROM) by the optional schema prefix
|
/// immediately followed (possibly via FROM) by the optional schema prefix
|
||||||
/// <c>dbo.</c> and then the table name <c>AuditLog</c> as a whole word:
|
/// (<c>dbo.</c> or <c>[dbo].</c>) and then the table name <c>AuditLog</c>
|
||||||
|
/// or <c>[AuditLog]</c> as a whole word:
|
||||||
/// <code>
|
/// <code>
|
||||||
/// UPDATE\s+(?:dbo\.)?AuditLog\b
|
/// UPDATE\s+(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b
|
||||||
/// DELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b
|
/// DELETE\s+(?:FROM\s+)?(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b
|
||||||
/// </code>
|
/// </code>
|
||||||
/// This tight DML-syntax pattern naturally excludes false positives without
|
/// This tight DML-syntax pattern naturally excludes false positives without
|
||||||
/// any additional keyword checks:
|
/// any additional keyword checks:
|
||||||
@@ -106,23 +113,31 @@ public class AuditLogAppendOnlyGuardTests
|
|||||||
}
|
}
|
||||||
|
|
||||||
// DML-syntax pattern: the UPDATE or DELETE keyword must be directly followed
|
// DML-syntax pattern: the UPDATE or DELETE keyword must be directly followed
|
||||||
// (optionally via FROM) by the optional "dbo." schema qualifier and then the
|
// (optionally via FROM) by the optional schema qualifier and then the table name.
|
||||||
// table name "AuditLog" as a whole word.
|
|
||||||
//
|
//
|
||||||
// UPDATE\s+(?:dbo\.)?AuditLog\b
|
// Schema sub-pattern : (?:\[?dbo\]?\.)?
|
||||||
// matches: "UPDATE AuditLog …", "UPDATE dbo.AuditLog …"
|
// matches: nothing, "dbo.", "[dbo]."
|
||||||
// does NOT match: "DENY UPDATE ON dbo.AuditLog" (UPDATE is followed by ON, not AuditLog)
|
|
||||||
//
|
//
|
||||||
// DELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b
|
// Table sub-pattern : \[?AuditLog\]?
|
||||||
// matches: "DELETE FROM AuditLog …", "DELETE FROM dbo.AuditLog …", "DELETE dbo.AuditLog …"
|
// matches: "AuditLog", "[AuditLog]"
|
||||||
// does NOT match: "DENY DELETE ON dbo.AuditLog" (DELETE is followed by ON, not FROM/AuditLog)
|
//
|
||||||
const string dmlPattern =
|
// UPDATE\s+(?:\[?dbo\]?\.)?(?:\[?AuditLog\]?)\b
|
||||||
@"\bUPDATE\s+(?:dbo\.)?AuditLog\b" +
|
// matches: "UPDATE AuditLog", "UPDATE dbo.AuditLog",
|
||||||
@"|\bDELETE\s+(?:FROM\s+)?(?:dbo\.)?AuditLog\b";
|
// "UPDATE [AuditLog]", "UPDATE [dbo].[AuditLog]"
|
||||||
|
// does NOT match: "DENY UPDATE ON dbo.AuditLog" (UPDATE is followed by ON)
|
||||||
return Regex.IsMatch(text, dmlPattern, RegexOptions.IgnoreCase);
|
//
|
||||||
|
// 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
|
// Guard test: scan every *.cs file in ConfigurationDatabase (excluding
|
||||||
// Designer/Snapshot EF artefacts and the obj/ directory).
|
// Designer/Snapshot EF artefacts and the obj/ directory).
|
||||||
@@ -248,9 +263,28 @@ public class AuditLogAppendOnlyGuardTests
|
|||||||
Assert.True(ContainsAuditLogMutation(
|
Assert.True(ContainsAuditLogMutation(
|
||||||
"update dbo.AuditLog set Actor = 'system' where Actor is null;"));
|
"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(
|
Assert.True(ContainsAuditLogMutation(
|
||||||
"-- AuditLog: UPDATE dbo.AuditLog SET x = 1"));
|
"-- 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]
|
[Fact]
|
||||||
@@ -273,6 +307,10 @@ public class AuditLogAppendOnlyGuardTests
|
|||||||
Assert.False(ContainsAuditLogMutation(
|
Assert.False(ContainsAuditLogMutation(
|
||||||
"DELETE FROM dbo.Notifications WHERE CompletedAtUtc < @cutoff;"));
|
"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):
|
// SiteCalls DELETE (legitimate; AuditLog not present on the line):
|
||||||
Assert.False(ContainsAuditLogMutation(
|
Assert.False(ContainsAuditLogMutation(
|
||||||
"DELETE FROM dbo.SiteCalls WHERE TerminalAtUtc < @cutoff;"));
|
"DELETE FROM dbo.SiteCalls WHERE TerminalAtUtc < @cutoff;"));
|
||||||
|
|||||||
Reference in New Issue
Block a user