Skip to content

test(eventviewerx): ✅ guard known-field string literals in rules #241

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
PrzemyslawKlys merged 1 commit into from
Mar 5, 2026
Merged

test(eventviewerx): ✅ guard known-field string literals in rules #241

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
45 changes: 45 additions & 0 deletions Sources/EventViewerX.Tests/TestRuleSourceGuards.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,13 +1,41 @@
using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Text.RegularExpressions;
using Xunit;

namespace EventViewerX.Tests;

public class TestRuleSourceGuards
{
private static readonly IReadOnlyList<string> KnownFieldKeys = new[]
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Cover all KnownEventField keys in literal guard

The new guard hard-codes KnownFieldKeys but omits existing KnownEventField members (Message, NoNameA0, NoNameA1, and TextPayload from Sources/EventViewerX/Definitions/KnownEventField.cs), so string-literal access to those canonical fields is not detected. I verified this gap by searching current rules, which already contain calls like GetValueFromDataDictionary("NoNameA0") and GetValueFromDataDictionary("#text"); this test still passes despite those known-field literals.

Useful? React with 👍 / 👎.

{
"SubjectUserName",
"SubjectDomainName",
"SubjectLogonId",
"TargetUserName",
"TargetDomainName",
"TargetSid",
"WorkstationName",
"IpAddress",
"IpPort",
"LogonType",
"Status",
"SubStatus",
"FailureReason",
"AuthenticationPackageName",
"LogonProcessName",
"LmPackageName",
"KeyLength",
"ProcessId",
"ProcessName",
"TransmittedServices",
"TicketOptions",
"TicketEncryptionType",
"PreAuthType"
};
Comment on lines +12 to +37
Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

KnownFieldKeys duplicates KnownEventField values but is manually maintained and currently omits enum-backed keys like Message/NoNameA0/NoNameA1 and the TextPayload mapping ("#text"). This can lead to false negatives (e.g., Rules/RuleHelpers.cs uses "NoNameA0" and "#text" as string literals). To avoid drift, build the key set from KnownEventField (Enum.GetValues/Enum.GetNames) and include the special ToEventFieldKey mapping for TextPayload.

Copilot uses AI. Check for mistakes.

[Fact]
public void RuleSources_DoNotUseDirectEventObjectDataIndexer()
{
Expand All @@ -34,6 +62,23 @@ public void RuleSources_DoNotUseDirectDataTryGetValueCalls()
string.Join(Environment.NewLine, offenders));
}

[Fact]
public void RuleSources_DoNotUseKnownFieldStringLiteralsInAccessors()
{
string knownKeysPattern = string.Join("|", KnownFieldKeys.Select(Regex.Escape));
Regex knownFieldLiteralPattern = new(
$@"\b(?:GetValueFromDataDictionary|GetDataValueOrEmpty|TryGetDataValue|TryGetDataEnum)\s*\([^)]*""(?:{knownKeysPattern})""",
Copy link

@chatgpt-codex-connector chatgpt-codex-connector bot Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Handle multiline accessor calls in regex guard

The pattern \s*\([^)]*"..." is applied to one line at a time, so it only catches calls where the method name and string literal are on the same line. A multiline form (for example, GetDataValueOrEmpty( on one line and the known key string on the next) will bypass this test entirely, which makes the new guard easy to evade through formatting.

Useful? React with 👍 / 👎.

Copy link

Copilot AI Mar 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The offender detection only tests each source line individually (FindRuleSourceOffenders uses File.ReadAllLines + pattern.IsMatch(line)), but this regex requires the method call and the string literal to appear on the same line. Multi-line accessor calls won’t be detected (e.g., Sources/EventViewerX/Rules/AzureAD/AADConnectStagingDisabled.cs uses GetValueFromDataDictionary(\n "SubjectUserName", ...)). Consider scanning the full file text (or buffering a statement until ')' / ';') so the guard also catches multi-line invocations, while still reporting an accurate line number for the match.

Suggested change
$@"\b(?:GetValueFromDataDictionary|GetDataValueOrEmpty|TryGetDataValue|TryGetDataEnum)\s*\([^)]*""(?:{knownKeysPattern})""",
$@"""(?:{knownKeysPattern})""",

Copilot uses AI. Check for mistakes.
RegexOptions.Compiled);

IReadOnlyList<string> offenders = FindRuleSourceOffenders(knownFieldLiteralPattern);

Assert.True(
offenders.Count == 0,
"Known event fields should use KnownEventField enum accessors instead of string literals. Offenders:" +
Environment.NewLine +
string.Join(Environment.NewLine, offenders));
}

private static IReadOnlyList<string> FindRuleSourceOffenders(Regex pattern)
{
string rulesDirectory = ResolveRulesDirectory();
Expand Down
Loading