Skip to content

feat(eventviewerx): QueryLogXPath for custom XPath consumers#220

Merged
PrzemyslawKlys merged 1 commit intomasterfrom
refactor/eventviewerx-live-query-api
Feb 10, 2026
Merged

feat(eventviewerx): QueryLogXPath for custom XPath consumers#220
PrzemyslawKlys merged 1 commit intomasterfrom
refactor/eventviewerx-live-query-api

Conversation

@PrzemyslawKlys
Copy link
Member

Adds SearchEvents.QueryLogXPath(...) to stream EventObject from a log name with a caller-provided XPath filter, reusing the existing session warm-up + reader timeout logic. This is intended to let hosts/tools stay thin (no direct EventLogReader loops).

@claude
Copy link

claude bot commented Feb 10, 2026

Claude finished @PrzemyslawKlys's task —— View job


Code Review Complete

Reviewing PR #220: feat(eventviewerx): QueryLogXPath for custom XPath consumers

Todo List:

  • Read repository guidelines (CLAUDE.md) - No repo-specific guidelines found
  • Analyze the modified code file
  • Review code quality and best practices
  • Check for potential bugs or issues
  • Assess performance considerations
  • Evaluate security concerns
  • Review test coverage implications
  • Provide comprehensive feedback

Review Summary

This PR successfully adds the QueryLogXPath method and performs a valuable refactoring to extract common query execution logic. The implementation is well-designed and maintains consistency with existing patterns.

Strengths

Code Quality & Design

  • Excellent refactoring: Extracted QueryLogFromQuery to eliminate ~100 lines of code duplication between QueryLogEnumerable and the new QueryLogXPath method
  • Consistent API design: New method follows existing patterns with similar parameters (machineName, maxEvents, cancellationToken, sessionTimeoutMs)
  • Good documentation: Clear XML documentation explaining the method's purpose and parameters
  • Proper validation: Handles null/empty XPath by defaulting to "*"

Performance & Reliability

  • Reuses existing infrastructure: Leverages all existing timeout handling, session warm-up, and error recovery logic
  • Maintains timeout behavior: Preserves sophisticated timeout handling for both session creation and event reading
  • Memory efficient: Uses yield return for streaming, avoiding loading all events into memory

🔍 Areas for Consideration

Security & Input Validation

  • XPath injection risk: The xpath parameter is passed directly to EventLogQuery without validation or sanitization. While Windows Event Log XPath has limited attack surface, consider:
    // Consider adding basic XPath validation
    if (!string.IsNullOrWhiteSpace(xpath) && xpath.Contains("..")) {
        throw new ArgumentException("Invalid XPath expression", nameof(xpath));
    }

Error Handling

  • XPath syntax errors: Invalid XPath expressions will cause EventLogReader creation to fail, but error messages may not be user-friendly. The current error handling in CreateEventLogReader (lines 72-81) will catch and log these appropriately.

Test Coverage

  • Missing test coverage: No tests found for the new QueryLogXPath method. Consider adding tests similar to existing ones in TestSearchingEvents.cs:
    [Fact]
    public void QueryLogXPath_CustomFilter() {
        if (!OperatingSystem.IsWindows()) return;
        var xpath = "*[System[EventID=4624]]"; // Successful logon
        var events = SearchEvents.QueryLogXPath("Security", xpath, maxEvents: 10);
        foreach (var evt in events) {
            Assert.Equal(4624, evt.Id);
        }
    }

📋 Recommendations

  1. Add comprehensive tests for the new method covering:

    • Valid XPath expressions
    • Invalid XPath handling
    • Empty/null XPath handling
    • Remote machine scenarios
  2. Consider XPath validation for common injection patterns, though Windows Event Log XPath is relatively constrained

  3. Update documentation to include XPath syntax examples in the method documentation

🎯 Overall Assessment

Approved - This is a well-implemented feature that provides valuable flexibility for consumers while maintaining code quality through smart refactoring. The security risks are minimal given the Windows Event Log context, and the existing error handling provides good protection against malformed queries.

The refactoring to extract QueryLogFromQuery significantly improves maintainability, and the new method integrates seamlessly with existing infrastructure.

@PrzemyslawKlys PrzemyslawKlys merged commit 1bc1872 into master Feb 10, 2026
5 checks passed
@PrzemyslawKlys PrzemyslawKlys deleted the refactor/eventviewerx-live-query-api branch February 10, 2026 17:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant