feat(parser): add the EvtxECmd map engine - #541
Merged
Conversation
Every event ID carries a different EventData shape, which is why event logs resist tabular display. EvtxECmd solved this with community maps that project each event's fields into a fixed set of columns. Implementing that schema means the existing upstream corpus works unmodified, and maps written here work in EvtxECmd and Timeline Explorer. The grammar was measured, not assumed. Across all 468 upstream maps there are 1,837 value expressions in six shapes: named EventData (1,441), nested UserData (204), bare Data (176), System paths including Correlation/@ActivityID (12), indexed Data (3), and the EventData container (1). So this implements an absolute element path with optional attribute-equality or 1-based index predicates and an optional trailing attribute selector, rather than a general XPath engine the corpus does not justify. Two boundaries keep the crate pure and wasm32-compatible, and both add zero dependencies: - No XML crate. Callers convert what they already hold into EventNode. - No YAML crate. The schema derives serde::Deserialize, so format-specific loading belongs in the host layer. Upstream maps are YAML; the vendored fixtures are the same maps as JSON. Resolution is non-fatal by design. Maps are written against a provider's superset of fields, so an individual event legitimately omits some. A missing field is reported on MappedValue::unresolved and leaves its %placeholder% visible rather than blanking it, which would present a partial value as a complete one. A malformed path is a defect in the map file, so it is reported separately from a missing field. Two corpus details are handled deliberately: four upstream maps spell the target "Username" rather than "UserName", so Property parsing is ASCII case-insensitive; and unknown targets are preserved as MapProperty::Other so a map contributed against a newer schema is not silently discarded. Fixtures are unmodified upstream maps (MIT, attributed in the fixtures README) converted from YAML to JSON, so the engine is tested against the real schema rather than invented examples. Open question recorded in path.rs: whether EvtxECmd joins repeated unnamed <Data> elements or takes the first. This takes the first; to be confirmed against the tool on the lab host. Refs #539. Gates: 2,203 tests pass, clippy -D warnings clean, rustfmt clean, and cargo check --target wasm32-unknown-unknown still succeeds. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThe pull request adds a Windows Event Log platform across the parser, Tauri host, and React workspace. It adds event-map and provider enrichment, XML parsing, filtering, streaming diagnostics, exports, saved filters, grouping, dynamic columns, and unified timelines. ChangesEvent log platform
Estimated code review effort: 5 (Critical) | ~120 minutes Mergeability Score: 🟠 High · up to This PR changes event-log loading and refresh behavior alongside the parser work. At the current head, users may see empty or incomplete logs, stale error state, or a workspace that remains loading after failures, with some inputs capable of triggering a crash or time-window overflow. These concrete correctness and availability risks should be fixed or explicitly accepted before merge. Sequence Diagram(s)sequenceDiagram
participant EventLogWorkspace
participant EvtxFilterBar
participant TauriCommands
participant EventLogParser
participant MapRegistry
participant ProviderStore
participant UnifiedTimelineView
EventLogWorkspace->>EvtxFilterBar: select filters and display settings
EvtxFilterBar->>TauriCommands: query channels or parse files
TauriCommands->>EventLogParser: parse records with registries
EventLogParser->>MapRegistry: apply event mappings
EventLogParser->>ProviderStore: resolve provider metadata
EventLogParser-->>TauriCommands: records and coverage diagnostics
TauriCommands-->>EvtxFilterBar: streamed records and scan results
EvtxFilterBar->>TauriCommands: build unified timeline
TauriCommands-->>UnifiedTimelineView: timeline items and unplaced entries
Possibly related issues
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an EvtxECmd-compatible “map engine” to cmtraceopen-parser to normalize Windows Event Log XML into stable columns (provider/channel/event-id keyed), without introducing XML/YAML dependencies to the parser crate (keeping it pure + wasm32-friendly).
Changes:
- Introduces
cmtraceopen_parser::eventmapwith a minimalEventNodetree, a tiny EvtxECmd path grammar (ValuePath), and anapply_mapengine that reports unresolved vs invalid paths. - Implements the EvtxECmd map schema (
EventMap,MapEntry,Lookup,MapProperty) with serde deserialization and case-insensitive handling where required by the upstream corpus. - Adds real upstream-derived JSON fixtures plus a corpus-driven test suite validating mapping, lookups, registry identity matching, and non-fatal resolution behavior.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/cmtraceopen-parser/src/lib.rs | Exposes the new eventmap module from the parser crate. |
| crates/cmtraceopen-parser/src/eventmap/mod.rs | Public API surface + MapRegistry for resolving maps by (channel, provider, event_id). |
| crates/cmtraceopen-parser/src/eventmap/model.rs | Serde-deserializable EvtxECmd schema types (map entries, properties, lookups). |
| crates/cmtraceopen-parser/src/eventmap/node.rs | Defines the XML-free EventNode structure used by the engine. |
| crates/cmtraceopen-parser/src/eventmap/path.rs | Implements the small EvtxECmd “XPath-like” path parser/evaluator. |
| crates/cmtraceopen-parser/src/eventmap/apply.rs | Applies maps to events, performs placeholder substitution, tracks unresolved/invalid paths, and applies lookups. |
| crates/cmtraceopen-parser/tests/eventmap_corpus.rs | End-to-end corpus-driven tests against real converted fixtures. |
| crates/cmtraceopen-parser/tests/fixtures/eventmap/README.md | Documents fixture provenance and why each is included. |
| crates/cmtraceopen-parser/tests/fixtures/eventmap/shell-core-9701.json | Fixture covering bare /Event/EventData/Data extraction. |
| crates/cmtraceopen-parser/tests/fixtures/eventmap/security-4624.json | Fixture covering multi-binding templates and common targets (UserName, RemoteHost, etc.). |
| crates/cmtraceopen-parser/tests/fixtures/eventmap/ntfs-146-lookups.json | Fixture covering lookup-table translation with default fallback. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+62
to
+65
| /// Returns the child elements called `name`, in document order. | ||
| pub fn children_named<'a>(&'a self, name: &'a str) -> impl Iterator<Item = &'a EventNode> { | ||
| self.children.iter().filter(move |child| child.name == name) | ||
| } |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cmtraceopen-parser/src/eventmap/apply.rs`:
- Around line 118-131: Update remaining_placeholders to resume scanning at the
rejected candidate’s closing % when the extracted name is empty or contains
whitespace, rather than advancing past it. Preserve the existing advance
behavior for accepted placeholders so subsequent valid names are detected and
is_complete() remains accurate.
- Around line 75-101: Update the event-text substitution flow around the binding
loop to resolve all binding values first, then render the original template in
one pass. Ensure placeholder matching is performed only against the untouched
template and each placeholder is replaced from the precomputed resolutions, so
placeholder-like content in resolved values is never re-scanned or substituted.
In `@crates/cmtraceopen-parser/tests/fixtures/eventmap/README.md`:
- Around line 3-5: Update the fixture README description to replace
“byte-faithful to the upstream corpus” with wording that states the keys,
values, and structure are preserved, while retaining the existing note about
YAML-to-JSON conversion and the MIT license.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: e4d1b7f6-3510-467a-9c71-3f8d59a30e36
📒 Files selected for processing (11)
crates/cmtraceopen-parser/src/eventmap/apply.rscrates/cmtraceopen-parser/src/eventmap/mod.rscrates/cmtraceopen-parser/src/eventmap/model.rscrates/cmtraceopen-parser/src/eventmap/node.rscrates/cmtraceopen-parser/src/eventmap/path.rscrates/cmtraceopen-parser/src/lib.rscrates/cmtraceopen-parser/tests/eventmap_corpus.rscrates/cmtraceopen-parser/tests/fixtures/eventmap/README.mdcrates/cmtraceopen-parser/tests/fixtures/eventmap/ntfs-146-lookups.jsoncrates/cmtraceopen-parser/tests/fixtures/eventmap/security-4624.jsoncrates/cmtraceopen-parser/tests/fixtures/eventmap/shell-core-9701.json
…iour A bare step such as /Event/EventData/Data took the first matching element. That was a guess, flagged as an open question in the previous commit, and it was wrong. Verified against EvtxECmd 1.5.2 on a Windows 11 lab host rather than reasoned about. A probe map binding /Event/EventData/Data was run over a real ESENT event ID 326 record carrying nine unnamed <Data> children, with a maps directory containing only that map. The emitted PayloadData1 was 1,712 characters longer than the first element alone, and the bytes between the first and second element were 44 and 32, so repeated elements are joined with ", ". Only the final step can select a repeated set, because joining containers has no meaning, and an attribute selector still reads a single element since joining attribute values across siblings would be meaningless too. A single match keeps its previous behaviour exactly, so paths that resolve one element, which is every named-Data expression and 78% of the corpus, are unaffected. evaluate now returns Cow<str> so the common single-match case still borrows and only a genuine join allocates. Gates: 2,230 tests pass, clippy -D warnings clean, rustfmt clean, wasm32 target still builds. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
Open question resolved: EvtxECmd joins, it does not take the firstThe PR flagged one unverified assumption: whether EvtxECmd joins repeated unnamed How it was verifiedMeasured against EvtxECmd 1.5.2 on the Windows 11 lab host, not reasoned about.
EventId: 326
Channel: Application
Provider: ESENT
Maps:
-
Property: PayloadData1
PropertyValue: "%Probe%"
Values:
- Name: Probe
Value: "/Event/EventData/Data"
Result
against source elements Two independent confirmations, because a first-glance read of a long string is not evidence:
What changedOnly the final step joins, because joining containers has no meaning. An attribute selector still reads a single element, since joining attribute values across siblings would be meaningless too. A single match keeps its previous behaviour exactly, so every named-Data expression, which is 78% of the corpus, is unaffected.
Gates after the fix: 2,230 tests pass, clippy Worth noting for the recordThis is the second time today a guess I encoded turned out to be backwards, after the |
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
crates/cmtraceopen-parser/src/eventmap/path.rs (1)
77-85: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winReject empty path segments and empty attribute selectors.
filter(|s| !s.is_empty())converts/Event//EventData/Datainto/Event/EventData/Data. The parser then resolves a different expression instead of reporting a malformed path. The same block accepts/Event/@and treats it as a missing attribute.Preserve the existing
Emptyresult for/. Reject empty interior segments and empty terminal attribute names withPathError.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@crates/cmtraceopen-parser/src/eventmap/path.rs` around lines 77 - 85, Update the path parsing logic around segments in the path parser to preserve empty components instead of filtering them out, while retaining the existing Empty result for “/”. Reject any empty interior segment with the appropriate PathError, and reject terminal attribute selectors where strip_prefix('@') yields an empty name. Keep valid path and attribute parsing unchanged.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@crates/cmtraceopen-parser/src/eventmap/path.rs`:
- Around line 77-85: Update the path parsing logic around segments in the path
parser to preserve empty components instead of filtering them out, while
retaining the existing Empty result for “/”. Reject any empty interior segment
with the appropriate PathError, and reject terminal attribute selectors where
strip_prefix('@') yields an empty name. Keep valid path and attribute parsing
unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 415d27b2-d858-4304-9520-19ab93c49872
📒 Files selected for processing (2)
crates/cmtraceopen-parser/src/eventmap/apply.rscrates/cmtraceopen-parser/src/eventmap/path.rs
Host-side adapter for the map engine. The schema and engine stay in cmtraceopen-parser, which is pure and wasm32-compatible and therefore carries no YAML dependency; serde_norway is added to src-tauri only, where I/O already lives. The parser crate is unchanged by this commit and still builds for wasm32-unknown-unknown. Two behaviours were verified against EvtxECmd 1.5.2 on a Windows 11 host rather than inferred from its documentation: - First loaded wins. Two maps claiming the same identity were placed in one directory; EvtxECmd rejected the second with "An item with the same key has already been added. Key: 326-APPLICATION-ESENT", reported "Maps loaded: 1", and kept the 1_-prefixed file. That is the opposite of MapRegistry::insert, which is last-wins, so the loader checks ownership before inserting rather than relying on registry semantics. - Identity is case-insensitive, and that same key shows channel and provider uppercased, matching how MapRegistry compares them. Upstream .map files are UTF-8 with a byte order mark, which YAML parsers reject as an unexpected character, so the BOM is stripped before deserializing. Failures and supersessions are reported rather than dropped. A map that did not load means events of that type render unmapped, which is a coverage gap an operator needs to see, not a silent omission. Gates: 1,095 src-tauri tests pass, clippy -D warnings clean, rustfmt clean, tsc clean, cargo audit exits 0 for the new dependency, and the parser crate still builds for wasm32. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/src/event_log/maps.rs`:
- Around line 106-111: Update the sorting closure in the file-listing flow to
use a deterministic secondary key after the lowercased filename, such as the
original filename or full path. Preserve the existing case-insensitive primary
ordering while ensuring names with identical lowercase keys have a stable order.
- Around line 95-97: Update the directory-entry loading flow that builds files
from ReadDir so item-level enumeration errors are not discarded by
filter_map(Result::ok). Propagate each ReadDir error or record a directory-level
MapLoadFailure, ensuring partial results cannot report outcome.is_clean() ==
true.
- Around line 91-120: Update load_maps_from_dir to be async, using tokio::fs for
directory enumeration and byte reads, and update every caller and test to await
it. Preserve directory-entry errors instead of discarding them, decode bytes
with encoding_rs::UTF_8 and fall back to WINDOWS_1252 on decode errors, and
remove a leading UTF-8 BOM before parsing while retaining all existing
map-loading behavior.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: bc5b7286-a500-48f2-8566-64f89c1195b0
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock,!Cargo.lock
📒 Files selected for processing (3)
src-tauri/Cargo.tomlsrc-tauri/src/event_log/maps.rssrc-tauri/src/event_log/mod.rs
Addresses three review findings on the map engine, two of them correctness defects with concrete repro cases now covered by tests. Substituted values were re-scanned as templates. apply_entry tested text.contains against the partially rendered output and then used str::replace, so a resolved value that itself contained %Name% became a substitution target for a later binding. Event field content is untrusted: with security-4624 and SubjectDomainName set to the literal "%user%", UserName rendered as "adam\adam" instead of "%user%\adam". Bindings are now resolved against the original template and the result is rendered left to right in one pass, so substituted text is appended and never revisited. The placeholder scanner lost a name after rejecting a candidate. On finding a non-placeholder between two percent signs it advanced past the closing one, consuming the opening delimiter of the next real placeholder. "50% off %Cost%" therefore reported nothing unresolved while the text still read %Cost%, contradicting the documented contract of MappedValue::unresolved. The renderer now resumes immediately after the opening percent so the closing one stays available. EventNode::children_named tied the name's lifetime to the node borrow, which forced path::select_one to reimplement the filter inline. The name now carries its own lifetime and select_one uses the helper. Gates: clippy -D warnings clean, rustfmt clean, wasm32 target builds, and the crate's tests pass including four new cases covering percent handling. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Filtering happens either inside the Event Log service or in the client after every matching event has been fetched and rendered. FullEventLogView pushes only Event ID down and evaluates level, provider, time, and description client-side, which is why its default "last 7 days" costs a walk of every channel. Our live path currently sends the literal "*" and filters afterwards, so it has the same shape. This builds the query instead: relative and absolute time windows, levels, Event IDs with ranges, providers, and a keyword mask, include or exclude. It is pure string construction with no Windows dependency, so it lives in the parser crate and is unit-testable off Windows. Two constraints come from the service rather than from taste. Expression count is capped, so a large include list is split across several Select nodes inside one QueryList. Each node repeats the other predicates, because the service unions nodes rather than intersecting them; without that repetition the second node would match every level and silently widen the result. Exclusion lists are never split, since "not (a or b)" spread across unioned nodes becomes "not a or not b" and matches almost everything. Provider names reach this from user input and from event data, so interpolated values are escaped at both the XML and XPath layers. An unescaped apostrophe would terminate the string literal and let the remainder be read as query syntax; a test asserts "Evil' or '1'='1" cannot survive as syntax. Gates: 20 focused tests, clippy -D warnings clean, rustfmt clean, wasm32 target builds. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Four changes to the live query path, all aimed at the complaint that started this work: a time window should cost a bounded query, not a walk of every channel. The query is now built rather than hardcoded. The path sent the literal "*" and filtered afterwards, which is the same shape as FullEventLogView and has the same cost. It now compiles the caller's filter to XPath via cmtraceopen_parser::event_query, so non-matching events are never fetched, rendered, or transferred. query_channel_filtered exposes this; the existing entry points keep their behaviour by passing an empty filter. EvtNext fetches 256 handles per call instead of 16. Each call is a round trip to the service and is the dominant cost of a scan. The API accepts up to 1024; 256 cuts round trips by 16x against the previous value while keeping the per-call array modest. FullEventLogView hardcodes 1. EvtQueryTolerateQueryErrors is now set. Without it a single element the service cannot evaluate, such as a provider not registered on this machine, aborts the whole channel and the result silently looks empty rather than partial. Channels are queried concurrently with rayon. Each channel is an independent conversation that spends nearly all its time waiting on RPC, so serializing them left the machine idle. Results are collected per channel and ordered afterwards, so concurrency cannot affect output. This is structurally impossible for FullEventLogView, which imports no threading primitives at all. A failed channel still reports 0 events so the coverage gap stays visible instead of reading as a channel that had nothing in it. Windows-only code, so compilation is verified by the Windows CI job rather than locally; measurement against the benchmark gate still has to happen on the lab host. macOS side: clippy -D warnings clean, rustfmt clean, 1,095 tests pass, parser crate still builds for wasm32. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
There was a problem hiding this comment.
Actionable comments posted: 5
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@crates/cmtraceopen-parser/src/event_query/mod.rs`:
- Around line 114-133: Update escape and its call sites around the provider
XPath construction to preserve apostrophes rather than deleting them. Select
single or double XPath delimiters based on which delimiter the value does not
contain, reject only values containing both, and apply the corresponding XML
escaping for the chosen delimiter; adjust the usages at the referenced locations
consistently.
- Around line 225-230: Update the event-ID splitting logic around needs_split
and select_body to count emitted XPath expressions rather than selectors,
weighting Range selectors as two expressions and other selectors as one. Apply
this expression-aware chunking to Include mode so compound filters stay within
the service limit; handle Exclude mode through the module’s Suppress-node path
instead of unioned Select chunks, preserving the documented exclusion semantics.
- Around line 179-196: Update the exclusion branches in the event-query
predicate construction to emit XPath function syntax as not(...) for both event
ID and provider clauses, explicitly wrapping the provider clause. Adjust the
expectations near the tests at lines 352, 430, and 472 to match the new query
strings.
- Around line 205-211: Update build_query and the predicate builders to keep
operators raw when constructing bare XPath, and XML-escape the complete select
body only when embedding it inside the structured <QueryList> document. Ensure
select_body and related helpers do not escape predicate values prematurely, then
adjust affected tests to expect raw operators for XPath and XML entities only
within structured queries.
- Around line 232-241: Update the query builder around the loop in build_query
to assign each split <Query> a unique Id and ensure every <Select> has a defined
channel Path. Preserve the existing build_query String API only when its XPath
and structured-query path contract is documented; otherwise introduce a typed
API for the channel path while keeping build_query backward-compatible.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fedd8152-e7d4-40cb-a84d-d9c549f479fd
📒 Files selected for processing (2)
crates/cmtraceopen-parser/src/event_query/mod.rscrates/cmtraceopen-parser/src/lib.rs
Completes the path from the UI to the Event Log service so the query work is actually reachable. The filter now crosses IPC and reaches EvtQuery as XPath. EventQueryFilter gains serde derives with a camelCase wire shape, so the same type is the Rust API and the TypeScript contract, and a round-trip test asserts the compiled query is identical on both sides of the boundary. Every field defaults, because the frontend sends only what the operator set. The live view gains a time window control offering the last hour, 24 hours, 7 days, 30 days, or all time, defaulting to 24 hours. That default is the point of the exercise: FullEventLogView loads seven days on startup and filters time client-side, so the window costs a walk of every channel. Here the window is a service-side predicate, so events outside it are never fetched, rendered, or transferred, and widening it to 30 days is cheap rather than punitive. Changing the window refetches, since a server-side predicate cannot be applied to records already in memory. The control only appears for live sources, where it means something. Level and provider deliberately stay client-side for now. Their existing controls filter records already loaded, and moving them server-side changes what a reload fetches. That is a behavioural change worth making deliberately rather than smuggling in alongside this one. Gates: tsc clean, clippy -D warnings clean on both crates, rustfmt clean, wasm32 target builds. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…e tree The map engine takes an already-parsed tree because cmtraceopen-parser is wasm32-compatible and carries no XML reader. This is the host-side adapter that produces one from the XML both EvtRender and the evtx crate emit, so maps can finally be applied to real events. quick-xml moves from the intune-diagnostics feature to event-log as well, since that is now where it is needed. Three details are handled deliberately, each found by a failing test rather than by inspection. Namespace prefixes are stripped. Event XML declares a default namespace and some providers emit prefixed elements, while map paths are written unprefixed as /Event/EventData/Data. Keeping prefixes would make every such map silently match nothing. Entity references arrive as their own event in quick-xml 0.41. Ignoring them drops every '&', '<' and '>' from event data, which is common in command lines and file paths; "a & b <c>" was parsing as "abc". Text is not trimmed globally. Trimming strips meaningful spaces from field values, and because entities are separate events the fragments of "a & b" were being reassembled from individually trimmed pieces. Whitespace-only text is instead dropped when an element with children closes, which removes pretty-printing without touching real content. A well-formed but unusual document yields whatever tree it describes, so an unexpected provider shape degrades to unmapped columns rather than a failed query. Only malformed XML is an error. Gates: 10 focused tests, clippy -D warnings clean, rustfmt clean, full src-tauri suite passes. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
clippy::ptr_arg. close() only reads the stack top; it never pushes or pops, so &mut [EventNode] is the honest signature. Caught by re-running the gate after committing, which is the wrong order and the reason the previous commit shipped with clippy red. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Adds Task, Opcode, Process ID, Thread ID, user SID, and Keywords to the record model and the details pane. FullEventLogView exposes all of these; we exposed none of them. These are worth more than map-derived columns for coverage. A mapped column only exists where someone has written a map, and the upstream corpus has 13 maps touching the channels our users open. System fields are present on every event from every provider. Extraction is shared. Both the live path and the file path already carry the raw XML, so both now parse it once through the node adapter and read the System block the same way, rather than each growing its own extraction. Every field is optional, and an omitted one stays absent rather than defaulting to zero. Providers legitimately write no Task element, and Security carries no UserID for events raised outside a user context; rendering those as 0 and S-1-0-0 would claim the provider said something it did not. The details pane renders each field only when present, for the same reason. The user SID is deliberately not resolved to an account name. That needs LookupAccountSidW plus a cache, is only meaningful on a machine that knows the domain, and would silently produce different output on a workstation than on the originating host. It is a separate concern rather than something to bolt on here. Gates: 46 event_log tests pass, full src-tauri suite green, clippy -D warnings clean, rustfmt clean, tsc clean. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ent XML
FullEventLogView offers nine export formats and we offered none, which made
every analysis dead-end inside the app. This adds the formats that carry the
data losslessly enough to be worth having.
The export writes what is on screen. A shared selectVisibleRecords is now the
single definition of "visible", used by both the list and the export, because
recomputing the predicate at the export site would let the two drift and a file
that quietly differs from the view is worse than no export.
CSV escaping is the part worth reviewing. Values containing the delimiter,
quotes, or newlines are quoted, and embedded quotes are doubled, so a multi-line
event description cannot break the row structure. Values are quoted only where
needed, so exports stay diffable.
Leading =, +, -, and @ are neutralized with an apostrophe. Excel and LibreOffice
treat those as formula starts, event descriptions and command lines routinely
begin with them, and event content is attacker-influenceable, so an analyst
opening the export would otherwise execute it. A test asserts
"=cmd|'/c calc'!A1" cannot survive as a formula.
Absent optionals render empty rather than 0, consistent with the record model:
claiming opcode 0 when the provider wrote none invents evidence. The XML export
passes the provider's own representation through untouched, since re-encoding it
would change what the source said, and an export is evidence.
Two pieces of tidying fell out. parseEventIdFilter existed only inside the
timeline component; it now lives in a Tauri-free module alongside the selector,
so both are unit-testable without a Tauri runtime, and it gained range support
("4-6") and space separators to match what the incumbent tools accept. The store
subscribes to Tauri events at module scope, so importing it from a test fired
that subscription and exited non-zero; the pure helpers moved out rather than
mocking around it.
Gates: 13 export tests, 8 filter tests, 1,123 Rust tests, clippy -D warnings
clean, rustfmt clean, tsc clean, vitest exits 0.
Refs #539.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Closes the loop on the map engine. Maps could be parsed and applied in isolation, but nothing loaded them into a running app or put the results in front of anyone. The registry is process-wide rather than threaded through every call. Both the live and file record paths already parse each event once for the System block, so applying maps at that point costs no second parse. An empty registry is the normal state until maps are loaded and simply yields no columns. Records gain a mapped list, and the details pane renders it only when a map covers that event type, so no empty heading appears for the overwhelming majority of events that have no map. A column whose map referenced a field the event did not carry is marked incomplete and shown in warning colour with its unresolved placeholder intact, rather than being quietly blanked. Two commands are added: one to load a directory of maps and report what loaded, what was superseded, and what failed, and one to report how many are in effect. An operator who wonders why an event type is not being mapped can see the answer instead of guessing. An end-to-end test proves the whole chain: YAML on disk, into the registry, applied to XML parsed by the host adapter, out as columns, with a non-matching event id confirming no map means no columns rather than a guess. Gates: 62 event_log tests pass, full src-tauri suite green, clippy -D warnings clean, rustfmt clean, tsc clean, vitest exits 0. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Running the benchmark against a real service found three defects that no shape-only unit test could catch. Every assertion had been about the string I expected, not about what the service parses, so the builder was confidently producing queries that fail at runtime. Escaping was applied in the wrong place. A bare XPath must carry raw "<=" and ">="; the same operators inside a QueryList document must be XML-escaped. The builder escaped unconditionally, so every query without a large Event ID set, which is the common case including the new time window, was rejected with "The specified query is invalid". Predicates are now built raw and the expression is escaped only where it becomes XML text. There is no negation in this XPath subset. "not(...)" is rejected outright, so exclusion is now expressed with "!=" joined by "and", and an excluded range becomes its complement. The != form and the documented <Suppress> element return identical result sets on the same channel, 10,587 events each, so this is the right construction rather than merely an accepted one. Excluding several ids joined with "or" would have matched everything, since "EventID!=1 or EventID!=2" is true for every event. Exclusion joins with "and". A new test module pins the exact strings that were executed against Windows 11 build 26200 and accepted, so a future change that looks reasonable but is rejected at runtime fails in CI instead of in front of a user. All twelve forms the builder emits were validated: zero invalid. The benchmark that surfaced all this also produced the first real number behind the performance claim. Same channel, same 121 matching events, median of three runs on a 4-core host: server-side XPath predicate 1,053 ms client-side filtering 23,152 ms 22x, with identical result sets. That measures where the filter is evaluated, which is the change this work makes; it is not a measurement of our binary against FullEventLogView, and the commit messages should not be read as one. Refs #539. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
✅ Action performedReview finished.
|
The live path scanned the rendered XML for itself: six System fields by substring search, and EventData by regular expression, on top of the parse it was already doing for the map engine. That regex required a Name attribute, could not match a value containing a newline, and never looked at UserData, so three classes of event field were dropped from the live view with nothing on screen indicating a field was missing. The file path never had those bugs because it read the tree, and it is the divergence between the two implementations that let them drift. The translation moves to event_log::rendered, which is not gated on Windows. It is pure - a string in, a record out - and gating it meant it could only be tested on the one platform that can produce its input, which is why it had no tests at all. extract_event_data moves next to extract_system_fields so both paths share one extractor. Also: the loop now parses each event once and hands the tree to the record builder rather than parsing again inside it; an event whose XML will not parse is counted and reported instead of pushed as a record with every field defaulted, which rendered as a real event at the epoch with no provider; and the message is only requested for an event that actually named a provider. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Phase 1 of the event viewer epic carries a gate: no performance claim ships without a reproducible scenario and recorded numbers. There was no way to produce either. This adds the scan half - channel enumeration and a windowed query across every channel, with wall clock and per-event cost - so a claim can be checked rather than argued from reading the code. Channels that fail to read are counted rather than skipped, because treating an unreadable channel as zero events reports a faster scan of a smaller corpus as an improvement. Peak working set is deliberately left to the caller: a process cannot sample its own peak reliably. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
queryChannels sent every selected channel in a single request. The backend collects a whole request's records into one vector before replying, so asking for forty channels held every event of every channel in memory, twice, before anything reached the screen. The two other load paths in this store already query per channel; this one was the outlier, and it is the path that loads the channels a user explicitly selects. A single request also fails as a whole, so one unreadable channel discarded the results of every channel queried alongside it and left the view empty. Each channel now succeeds or fails on its own, and a channel that could not be read is recorded as a coverage gap rather than only as a loadError string that the next load replaces. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
query_channel documented "capped at max_events (default 1000)". There is no default: None becomes usize::MAX, and every caller in the application passes None. A bound that is documented but not enforced is worse than either having it or not, because it invites a caller to rely on it. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The first run of the harness showed the working set climbing past 690MB on an all-channels seven-day scan, which is the constraint that actually matters here and was not being attributed to anything. Every record carries the whole rendered XML it was built from, and that string is serialized to the frontend and held there too, so if it dominates the record it dominates three copies. Reports raw_xml bytes against message and field bytes so that claim is measured rather than assumed, plus the widest channel, since a single channel deciding the peak is a different problem from the total being large. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Measuring a full seven-day scan surfaced EvtNext failing with RPC_S_INVALID_BOUND (0x800706c6) on one channel out of roughly twelve hundred. The service was refusing the size of the request, not its contents, and the loop's response was to stop reading and return what it already had. The caller received Ok, counted the events, and showed the channel as fully loaded. This is the failure this view exists to avoid: events that were never fetched look exactly like events that do not exist. It was reachable only because the EvtNext batch was raised from 16 to 256 without the measurement the epic asked for, and the larger request is what some channels reject. The batch now halves down to a floor of 8 and retries, which reads the channel rather than abandoning it. If a read still fails, the records already gathered are returned, and the reason is carried alongside them: query_channel_inner and its four wrappers return ChannelScan, a set of records plus the gaps in it, so a caller cannot take the records without also being handed what is missing. Unparsable events are reported the same way instead of only being logged. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…t it The rule for responding to a failed EvtNext - end of channel, retry smaller, or report truncation - was written inside the Windows-only query loop, so no CI runner could execute it. That is the wrong place for it. Getting it wrong does not crash: it returns a partly read channel that the caller presents as whole, which is the failure mode this view exists to avoid, and it is exactly the kind of quiet wrong answer that needs a test running everywhere. classify_fetch_failure is a pure function of the Win32 code, the current batch and the floor, covered by six tests including one that follows the decisions from a full batch down to the floor to prove the retry loop terminates. The two single-code predicates it replaces are gone rather than left behind unused. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
One channel is most of a scan. Security measured 286,401 of 404,769 events and 191.8 seconds of a 267 second scan, so querying channels concurrently and delivering them one at a time - both already done - do nothing for it. An operator waited three minutes on a single blocking call with an empty list. query_channel_inner now hands each fetched batch to a callback as it is built. A caller that takes the records never holds more than one batch; a caller that ignores the argument gets the channel whole, as before, so the collecting wrappers and their tests are unchanged. ChannelScan carries `delivered` separately from `records`, because a streaming caller empties the vector and counting its length would report a fully read channel as holding nothing. The command emits each batch with a per-channel sequence number, and the store assembles the view from them. An event channel promises no delivery, so the store checks what it assembled against the count the reply states and against the sequence run, and reports either shortfall as a coverage gap. Without that check a dropped batch is indistinguishable from events that never happened, which is the failure this workspace exists to avoid. totalRecords moves into the validated reply shape for the same reason, with an absent count staying distinguishable from zero. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/workspaces/event-log/evtx-store.ts (1)
223-237: 🗄️ Data Integrity & Integration | 🔴 Critical | ⚡ Quick winOnly
queryChannelsconsumes the streamed record batches, so the other two live-query paths show an empty view.src-tauri/src/event_log/commands.rsalways callsquery_channel_streamedand emits every record as anevtx-record-batchevent, leaving the reply'srecordsempty. Any path that mergesresult.recordswithout callingdrainStreamedRecordstherefore records zero events while reporting the channel as loaded.
src/workspaces/event-log/evtx-store.ts#L223-L237: callresetStreamedRecords(availableCore)before the requests, then mergedrainStreamedRecords(ch).recordstogether withresult.recordsinmergeResult, and push a gap whenmissingSequencesis non-empty.src/workspaces/event-log/evtx-store.ts#L385-L399: callresetStreamedRecords(loaded)before the requests, then mergedrainStreamedRecords(ch).recordstogether withresult.records, and record a coverage gap in thecatchinstead of only callingconsole.warn.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workspaces/event-log/evtx-store.ts` around lines 223 - 237, Update queryChannels at src/workspaces/event-log/evtx-store.ts:223-237 to call resetStreamedRecords(availableCore) before requests, merge drainStreamedRecords(ch).records with result.records via mergeResult, and push a gap when missingSequences is non-empty. Apply the same streamed-record merge in the live-query path at src/workspaces/event-log/evtx-store.ts:385-399 after resetStreamedRecords(loaded), and record a coverage gap in its catch instead of only logging with console.warn.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src-tauri/examples/evtx_scan.rs`:
- Around line 56-60: Validate the `days` input before constructing
`EventQueryFilter`, using checked arithmetic for the day-to-millisecond
conversion so overflow cannot panic or produce an incorrect window. Return an
input error when the checked calculation fails, while preserving the existing
`TimeWindow::Last` behavior for valid values.
In `@src-tauri/src/event_log/live.rs`:
- Around line 358-363: Update the XML preview expression in the unparsable-event
warning within the event-log parsing flow to truncate by characters rather than
slicing the UTF-8 byte string at index 300. Match the safe character-based
approach already used by the rendered event-log path, preserving the
300-character preview and ensuring malformed or non-ASCII event data cannot
panic.
- Around line 347-348: Update the event-processing loop around render_event_xml
so an EvtRender failure does not propagate via ?, preserving already collected
records and continuing the scan. Add an unrenderable counter beside unparsable,
increment it when rendering fails, skip that event, and include the count as a
gap alongside the existing unparsable gap handling.
In `@src-tauri/src/event_log/rendered.rs`:
- Around line 145-163: Update the file-path fallback in parser.rs to call
build_event_data_summary so it matches the live-path EvtxRecord::message
behavior for long field values. Remove the now-unused build_message helper and
its associated test, while preserving the existing summary formatting and
truncation behavior.
In `@src/workspaces/event-log/evtx-store.ts`:
- Around line 288-322: Wrap each per-channel processing iteration in the loop
after the invoke handling with try/catch/finally so failures from
assertParseResultShape or drainStreamedRecords are handled; in catch, record the
channel failure in coverageGaps and set loadError, and in finally ensure
isLoading is set to false before queryChannels can reject or return.
- Around line 569-576: Replace the Math.max spread in the pending sequence
handling with a reduction or iterative calculation that determines the highest
sequence without expanding the set into function arguments. Preserve the
existing missingSequences loop and returned records while ensuring large
sequence sets do not exceed engine argument limits.
---
Outside diff comments:
In `@src/workspaces/event-log/evtx-store.ts`:
- Around line 223-237: Update queryChannels at
src/workspaces/event-log/evtx-store.ts:223-237 to call
resetStreamedRecords(availableCore) before requests, merge
drainStreamedRecords(ch).records with result.records via mergeResult, and push a
gap when missingSequences is non-empty. Apply the same streamed-record merge in
the live-query path at src/workspaces/event-log/evtx-store.ts:385-399 after
resetStreamedRecords(loaded), and record a coverage gap in its catch instead of
only logging with console.warn.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 72c21269-7798-424b-9382-98d938bbbfef
📒 Files selected for processing (12)
src-tauri/Cargo.tomlsrc-tauri/examples/evtx_scan.rssrc-tauri/src/event_log/commands.rssrc-tauri/src/event_log/event_node.rssrc-tauri/src/event_log/fetch.rssrc-tauri/src/event_log/live.rssrc-tauri/src/event_log/mod.rssrc-tauri/src/event_log/parser.rssrc-tauri/src/event_log/rendered.rssrc/workspaces/event-log/evtx-coverage.tssrc/workspaces/event-log/evtx-store-coverage.test.tssrc/workspaces/event-log/evtx-store.ts
A single handle that failed EvtRender propagated Err for the whole channel, discarding every record already read; the caller then reported the channel as holding no events. It is now counted as an unrenderable gap and the scan continues, matching how an unparsable document is already treated. The unparsable-warning preview sliced the XML by byte offset, which panics when byte 300 lands inside a multi-byte character. It is now sliced by character.
The file path fell back to its own build_message, which never truncated long field values, while the live path used build_event_data_summary, which does. The same event therefore rendered a different message depending on how it was opened. The file path now shares the live path's summary and build_message is gone.
The per-channel processing loop after the invoke call was unguarded, so a malformed reply (assertParseResultShape throws by design) or a drain failure rejected queryChannels before isLoading was cleared, leaving a stuck spinner with no message. Each iteration is now wrapped so the failure becomes a gap and a load error. drainStreamedRecords spread the whole sequence set into Math.max(...), which throws RangeError once a channel produces more batches than the engine accepts as arguments. The highest sequence is now found by reduction.
The harness multiplied days by milliseconds without a bound, so a large --days panicked in debug and scanned the wrong window in release. It now uses checked arithmetic and exits with an input error on overflow.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
src/workspaces/event-log/evtx-store.ts (1)
392-395: 🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick winA failed refresh clears the gaps and reports nothing in their place.
Line 394 clears
coverageGapsbecause the records they describe are replaced. The per-channelcatchat Line 430 then only callsconsole.warn. It sets noloadErrorand merges no gap. So a channel whose refresh request fails contributes zero records to the replaced view, and the view reports full coverage.
queryChannelshandles the same failure at Line 363 by merging${channel}: not read (${message}). Refresh must do the same, otherwise the time-window control can silently drop a whole channel.🐛 Proposed fix
} catch (e) { - console.warn(`[evtx] Refresh failed for ${ch}:`, e); + const message = e instanceof Error ? e.message : String(e); + console.warn(`[evtx] Refresh failed for ${ch}: ${message}`); + // Recorded, not only logged. The refresh cleared the previous gaps, so a silent failure + // here presents a view that is missing a whole channel as complete. + set((s) => ({ + coverageGaps: mergeCoverageGaps(s.coverageGaps, [`${ch}: not read (${message})`]), + loadError: s.loadError ?? `${ch}: ${message}`, + })); }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/workspaces/event-log/evtx-store.ts` around lines 392 - 395, Update the refresh flow around coverageGaps and its per-channel catch to preserve a gap for each failed channel after clearing the previous results. Reuse the same `${channel}: not read (${message})` gap format and merging behavior already implemented by queryChannels, while retaining the existing warning.src-tauri/examples/evtx_scan.rs (1)
82-90: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
channels_with_gapscounts gap entries, not channels.Line 90 adds
scan.gaps.len()for every channel. One channel can report several gaps at once: a truncated fetch, unparsable records, and unrenderable records are separate entries insrc-tauri/src/event_log/live.rs. The printed value at Line 125 then exceedschannels_scannedand reads as more affected channels than exist. Count channels separately, or rename the metric.♻️ Proposed fix
- let mut gap_reports = 0usize; + let mut gap_reports = 0usize; + let mut channels_with_gaps = 0usize;Ok(scan) => { let records = scan.records; gap_reports += scan.gaps.len(); + if !scan.gaps.is_empty() { + channels_with_gaps += 1; + }- println!("channels_with_gaps={gap_reports}"); + println!("channels_with_gaps={channels_with_gaps}"); + println!("gap_reports={gap_reports}");🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src-tauri/examples/evtx_scan.rs` around lines 82 - 90, Update the gap metric in the loop over channels in the example’s scan flow: do not accumulate scan.gaps.len() as a channel count. Increment the metric at most once for each channel with one or more gaps, or rename the variable and output to explicitly report gap entries; keep the printed metric consistent with its meaning.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src-tauri/examples/evtx_scan.rs`:
- Around line 82-90: Update the gap metric in the loop over channels in the
example’s scan flow: do not accumulate scan.gaps.len() as a channel count.
Increment the metric at most once for each channel with one or more gaps, or
rename the variable and output to explicitly report gap entries; keep the
printed metric consistent with its meaning.
In `@src/workspaces/event-log/evtx-store.ts`:
- Around line 392-395: Update the refresh flow around coverageGaps and its
per-channel catch to preserve a gap for each failed channel after clearing the
previous results. Reuse the same `${channel}: not read (${message})` gap format
and merging behavior already implemented by queryChannels, while retaining the
existing warning.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 95eff654-db05-4a19-8542-d4a3b75353b1
📒 Files selected for processing (5)
src-tauri/examples/evtx_scan.rssrc-tauri/src/event_log/live.rssrc-tauri/src/event_log/parser.rssrc/workspaces/event-log/evtx-store-coverage.test.tssrc/workspaces/event-log/evtx-store.ts
refreshLoadedChannels cleared coverageGaps with the records it replaced, but its per-channel catch only logged. A channel whose refresh request failed then contributed zero records to the replaced view while it reported full coverage. The failure is now recorded as a gap and a load error, matching queryChannels.
The harness summed gap entries and printed the total as channels_with_gaps. A channel can report several gaps at once, so the number overstated how many channels came back incomplete. Channels and entries are now counted apart.
Owner
Author
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/workspaces/event-log/evtx-store-coverage.test.ts`:
- Around line 118-123: Extend the refresh failure test assertions around
useEvtxStore.getState() to verify that state.loadError contains both
“Application” and “access denied”, and that state.isLoading is false after the
failed refresh. Keep the existing coverageGaps assertion unchanged.
In `@src/workspaces/event-log/evtx-store.ts`:
- Around line 431-438: Update the refresh-start state update in
refreshLoadedChannels to set loadError to null alongside clearing records and
coverageGaps. Keep the existing s.loadError ?? ... assignment in the failure
callback so only the first failure from the current refresh is retained.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: 0b50661b-362e-4f1d-8a74-04ffc71aaece
📒 Files selected for processing (3)
src-tauri/examples/evtx_scan.rssrc/workspaces/event-log/evtx-store-coverage.test.tssrc/workspaces/event-log/evtx-store.ts
refreshLoadedChannels cleared records and coverage gaps but not loadError, so a stale error from an earlier load survived a successful refresh and could hide a later failure. It is now cleared at refresh start, and the failure test asserts the full state: the gap, the loadError message, and that isLoading is false.
3 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Phase 3 groundwork for #539.
Why
Every event ID carries a different
EventDatashape. That is the actual reason event logs resist tabular display, and it is why no event viewer in the interactive class offers grouping or pivoting. EvtxECmd solved it with community maps that project each event's fields into a fixed set of columns.Implementing that schema means the existing upstream corpus works unmodified, and maps written here work in EvtxECmd and Timeline Explorer. It also means the device-management corpus (MDM, Autopilot, ESP, ConfigMgr), which is currently unwritten by anyone, becomes ours to author in a format the DFIR community already reads.
The grammar was measured, not assumed
Across all 468 upstream maps there are 1,837 value expressions, in six shapes:
/Event/EventData/Data[@Name="X"]/Event/UserData/<Container>/<Field>/Event/EventData/Data/Event/System/..., incl.Correlation/@ActivityID/Event/EventData/Data[N]/Event/EventDataSo this implements an absolute element path with optional attribute-equality or 1-based index predicates and an optional trailing attribute selector. A general XPath engine would be far more machinery than the corpus justifies.
Two boundaries, zero new dependencies
The parser crate must stay pure and wasm32-compatible:
EventNode.serde::Deserialize, so format-specific loading belongs in the host layer. Upstream maps are YAML; the vendored fixtures are the same maps as JSON.That second one also defers the YAML-crate choice, which matters: the maintained options are all either stale or carry governance concerns, and that decision belongs in
src-tauriwhere I/O already lives.Resolution is non-fatal by design
Maps are written against a provider's superset of fields, so an individual event legitimately omits some. A missing field is reported on
MappedValue::unresolvedand leaves its%placeholder%visible rather than blanking it, which would present a partial value as a complete one. A malformed path is a defect in the map file, not the event, so it is reported separately viainvalid_paths.Two corpus details handled deliberately
Usernamerather thanUserName. Treating those as unknown would silently drop the account column, soPropertyparsing is ASCII case-insensitive.MapProperty::Otherso a map contributed against a newer schema is not silently discarded.Verification
Fixtures are unmodified upstream maps (MIT, attributed in
tests/fixtures/eventmap/README.md) converted YAML to JSON, so the engine is tested against the real schema rather than invented examples.cargo testcargo clippy --all-targets -- -D warningsrustfmt --checkcargo check --target wasm32-unknown-unknownOne open question, recorded in the code
Whether EvtxECmd joins repeated unnamed
<Data>elements or takes the first. This takes the first, andpath.rssays so at the point of the decision. EvtxECmd is installed on the lab host, so this is resolvable by comparison rather than by guessing, and I would rather confirm it than quietly encode an assumption.Not included
Loading
.mapfiles from disk. That is YAML plus I/O, so it belongs insrc-taurias the next slice.🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Documentation