Add sealed ETL denial analysis - #699
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
ea1d76a to
b885815
Compare
b885815 to
00ca994
Compare
There was a problem hiding this comment.
Pull request overview
Adds cross-platform denial-analysis primitives and a Windows sealed-ETL decoder for learning-mode captures.
Changes:
- Defines denial models, framing, summaries, analysis, and emission APIs.
- Decodes and normalizes Windows ETW denial events.
- Adds the
lm_analyzediagnostic utility and workspace integration.
Reviewed changes
Copilot reviewed 15 out of 16 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
src/core/learning_mode_core/Cargo.toml |
Defines the core crate. |
src/core/learning_mode_core/src/lib.rs |
Exposes core APIs. |
src/core/learning_mode_core/src/analyze.rs |
Defines analyzer abstraction and errors. |
src/core/learning_mode_core/src/model.rs |
Defines normalized denial types. |
src/core/learning_mode_core/src/summary.rs |
Defines terminal summaries. |
src/core/learning_mode_core/src/frame.rs |
Defines framed wire records. |
src/core/learning_mode_core/src/emit.rs |
Emits RFC 7464 records. |
src/Cargo.toml |
Registers the new crate. |
src/Cargo.lock |
Records dependency changes. |
src/backends/learning_mode/windows/Cargo.toml |
Adds the core dependency. |
src/backends/learning_mode/windows/src/lib.rs |
Exports ETL analysis APIs. |
src/backends/learning_mode/windows/src/etl_decode.rs |
Processes ETLs and deduplicates denials. |
src/backends/learning_mode/windows/src/tdh_decode.rs |
Decodes TDH event properties. |
src/backends/learning_mode/windows/src/extractors.rs |
Classifies denial events. |
src/backends/learning_mode/windows/src/path_norm.rs |
Normalizes Windows kernel paths. |
src/backends/learning_mode/windows/examples/lm_analyze.rs |
Adds manual ETL inspection. |
Comments suppressed due to low confidence (1)
src/backends/learning_mode/windows/src/etl_decode.rs:197
- A TDH failure is silently discarded, so a missing provider manifest or malformed payload can make analysis return
Ok([])rather thanAnalyzeError::Decode. Track decode failures for target learning-mode events in the accumulator and fail analysis when they prevent a reliable result.
if let Some(parts) = unsafe { tdh_decode::decode_event_parts(event_record) } {
acc.events.push(CollectedEvent {
pid: header.ProcessId,
filetime: header.TimeStamp as u64,
parts,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (4)
src/backends/learning_mode/windows/src/etl_decode.rs:153
- This raw diagnostic path buffers every event and all decoded property strings until EOF. Long ETLs can therefore consume multiple GB; the existing PLM parser explicitly streams for this reason (
src/host/plm/src/event_parser.rs:257-263). Stream raw events to a callback/writer duringProcessTrace, retaining only the histogram needed bylm_analyze.
pub fn decode_raw_events(source_path: &Path) -> Result<Vec<DecodedEventParts>, AnalyzeError> {
let accumulator = process_trace_file(source_path, CollectionMode::Raw)?;
if let Some(error) = accumulator.decode_error {
return Err(AnalyzeError::Decode(error));
}
Ok(accumulator.raw_events)
src/backends/learning_mode/windows/src/tdh_decode.rs:365
TDH_INTYPE_POINTERwidth is determined by the emitting event'sEventHeader.Flags, not by the analyzer process. A 32-bit event uses four bytes even when this x64/ARM64 decoder reads it; consuming eight shifts every following property and can misdecode or omit denials. Thread the event's 4/8-byte pointer size fromdecode_event_partsinto property formatting.
TDH_INTYPE_POINTER if available >= 8 => (
// 64-bit pointer; on 32-bit hosts this would be 4 bytes but
// we only target x64. Unaligned read because ETW payload
// alignment is not guaranteed.
format!("{:#x}", unsafe { (data.cast::<u64>()).read_unaligned() }),
8,
src/core/learning_mode_core/src/emit.rs:70
- The stream contract says
totalDenialsequals the number of preceding denial frames, but this public function accepts and emits a mismatched summary unchanged. That produces a malformed stream consumers cannot reliably validate. Reject a mismatched count before writing any bytes.
) -> io::Result<()> {
src/backends/learning_mode/windows/src/etl_decode.rs:225
ERROR_CANCELLEDmeans trace processing stopped before EOF (for example, because a buffer callback returnedFALSE); it is not a successful completed file trace. Accepting it can return a partial denial set while leavingdeniedResourcesTruncatedfalse. OnlyERROR_SUCCESSshould produce an analysis result.
// ERROR_SUCCESS (0) and ERROR_CANCELLED (1223) are both acceptable
// terminal states for a completed file trace.
if status.0 != 0 && status.0 != 1223 {
83ea5f8 to
14ae3c4
Compare
14ae3c4 to
516746f
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (6)
src/backends/learning_mode/windows/src/tdh_decode.rs:287
UserDatais an unaligned ETW byte buffer, so casting the current property address to*const u16and forming au16slice is undefined behavior whenever a Unicode property starts at an odd address. Decode UTF-16 fromchunks_exact(2)(aswide_str_atalready does) instead of creating an aligned slice.
let wchars = unsafe { std::slice::from_raw_parts(data.cast::<u16>(), max_wchars) };
src/backends/learning_mode/windows/src/tdh_decode.rs:364
- ETW pointer width is determined by
EVENT_HEADER_FLAG_32_BIT_HEADER/EVENT_HEADER_FLAG_64_BIT_HEADER, not by the analyzer process architecture. A 32-bit producer on 64-bit Windows encodes this property in four bytes; consuming eight shifts every following property and can turn valid events into false denials or decode failures. Thread the record's header flags into the formatter and consume 4 or 8 bytes accordingly.
TDH_INTYPE_POINTER if available >= 8 => (
// 64-bit pointer; on 32-bit hosts this would be 4 bytes but
// we only target x64. Unaligned read because ETW payload
// alignment is not guaranteed.
format!("{:#x}", unsafe { (data.cast::<u64>()).read_unaligned() }),
src/core/learning_mode_core/src/model.rs:102
- A normal Windows FILETIME (including the example value in this file) is far above JavaScript's
Number.MAX_SAFE_INTEGER. Because serde emits thisu64as a JSON number while the public contract says SDK consumers parse it, Node consumers will silently lose timestamp precision. Serialize it as a decimal string (or choose another lossless timestamp representation) in the wire format.
pub filetime: u64,
src/backends/learning_mode/windows/src/lib.rs:48
decode_raw_eventsis public, but itsDecodedEventPartsreturn type lives in the privateextractorsmodule and is not re-exported. External callers can infer values but cannot name the type in their own signatures or wrappers; re-export the diagnostic record alongside the function.
pub use etl_decode::{decode_raw_events, EtlDenialAnalyzer};
src/backends/learning_mode/windows/src/path_norm.rs:46
- This only strips the NT
\??\prefix, but the same EventID 14 paths can use Win32 verbatim/DOS-device prefixes (\\?\and\\.\); the existing parser explicitly normalizes all three insrc/host/plm/src/access_failure.rs:120-141. Leaving those forms intact violates this module's user-visible-path contract and prevents equivalent path forms from deduplicating.
if let Some(rest) = kernel_path.strip_prefix(r"\??\") {
src/backends/learning_mode/windows/src/etl_decode.rs:225
- This analyzer never requests cancellation, so
ERROR_CANCELLEDmeans the file was not processed to normal end-of-file. Treating it as success can emit a silently partial denial set withdenied_resources_truncated == false; onlyERROR_SUCCESSshould be accepted here.
// ERROR_SUCCESS (0) and ERROR_CANCELLED (1223) are both acceptable
// terminal states for a completed file trace.
if status.0 != 0 && status.0 != 1223 {
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (9)
src/backends/learning_mode/windows/src/tdh_decode.rs:289
- ETW explicitly does not guarantee that
UserDatais aligned for its payload types. Constructing a&[u16]from this pointer therefore has undefined behavior when a Unicode property starts at an odd/un-aligned offset (particularly relevant on ARM64). Read UTF-16 units from byte pairs or withread_unalignedinstead.
let max_wchars = byte_len / 2;
// SAFETY: data is valid for `available` bytes; max_wchars
// bounds the slice.
let wchars = unsafe { std::slice::from_raw_parts(data.cast::<u16>(), max_wchars) };
let len = wchars.iter().position(|&c| c == 0).unwrap_or(max_wchars);
let s = String::from_utf16_lossy(&wchars[..len]);
src/backends/learning_mode/windows/src/extractors.rs:361
- The file write mask omits
FILE_DELETE_CHILD(0x40), even though the API documentation says delete rights fold intoWrite. A directory denial requesting this right alone is currently reported asUnknown(or underreported as read/execute when combined), which can lead policy generation to grant the wrong access type.
FILE_WRITE_DATA
| FILE_APPEND_DATA
| FILE_WRITE_EA
| FILE_WRITE_ATTRIBUTES
| standard_write
| GENERIC_WRITE
| GENERIC_ALL,
src/backends/learning_mode/windows/src/etl_decode.rs:225
ERROR_CANCELLEDmeans the consumer stopped processing (for example via a falseBufferCallback), not that a file trace completed. This reader installs no cancellation path and never exposes the handle, so accepting 1223 can return a partial denial set as complete and untruncated. Treat every nonzero status as a decode failure.
// ERROR_SUCCESS (0) and ERROR_CANCELLED (1223) are both acceptable
// terminal states for a completed file trace.
if status.0 != 0 && status.0 != 1223 {
src/backends/learning_mode/windows/src/tdh_decode.rs:365
TDH_INTYPE_POINTERis 4 bytes when the event header hasEVENT_HEADER_FLAG_32_BIT_HEADERand 8 bytes forEVENT_HEADER_FLAG_64_BIT_HEADER; it is not determined by this decoder's host architecture. Always consuming 8 bytes corrupts every following property for 32-bit provider events. Pass the header flags/pointer width through the property decoder and use it here (and inavailable_element_bound).
TDH_INTYPE_POINTER if available >= 8 => (
// 64-bit pointer; on 32-bit hosts this would be 4 bytes but
// we only target x64. Unaligned read because ETW payload
// alignment is not guaranteed.
format!("{:#x}", unsafe { (data.cast::<u64>()).read_unaligned() }),
8,
src/backends/learning_mode/windows/src/path_norm.rs:51
- This normalization omits the equivalent Win32 verbatim (
\\?\) and DOS-device (\\.\) forms, so those events remain non-user-visible and can be emitted as duplicate aliases of the same file. The existing learning-mode parser explicitly handles and tests all three forms atsrc/host/plm/src/access_failure.rs:120-141,267-277.
if let Some(rest) = kernel_path.strip_prefix(r"\??\") {
if let Some(unc) = rest.strip_prefix(r"UNC\") {
return Some(format!(r"\\{unc}"));
}
return Some(rest.to_string());
src/backends/learning_mode/windows/src/extractors.rs:158
- Missing/empty
ObjectNamevalues are dropped only for capabilities, so a file or registry decode gap emits a denial whose path is empty. That contradicts the crate contract that records without a decoded resource identifier are omitted and collapses unrelated unnamed events into one unusable resource. Require a non-empty name for every resource type.
This issue also appears on line 355 of the same file.
let object_name = find_prop(&parts.props, "ObjectName")
.map(|v| v.trim_matches('"').to_string())
.unwrap_or_default();
if resource_type == ResourceType::Capability && object_name.is_empty() {
return None;
src/core/learning_mode_core/src/model.rs:102
filetimevalues are currently serialized as JSON numbers, but current WindowsFILETIMEvalues (~1.3e17) exceed JavaScript'sNumber.MAX_SAFE_INTEGER. The TypeScript/JSON consumers described by this wire model will silently round timestamps. Encode the value as a decimal string, or expose a safe-resolution timestamp representation, before fixing this as the public wire contract.
/// Kernel timestamp of the denial. On Windows this is `FILETIME`
/// (100-nanosecond intervals since 1601-01-01 UTC), copied from
/// `EVENT_RECORD.EventHeader.TimeStamp`. Other backends normalise
/// their native clocks onto the same epoch so consumers can treat
/// the field uniformly.
pub filetime: u64,
src/backends/learning_mode/windows/src/etl_decode.rs:57
- Raw mode accumulates every event and all decoded property strings before the CLI writes anything. Long ETLs have already caused multi-GB buffering in the PLM parser (
src/host/plm/src/event_parser.rs:257-263), solm_analyze --rawcan repeat that OOM failure. Stream raw events to a callback/writer (and update the example) instead of returning an unboundedVec.
This issue also appears on line 223 of the same file.
raw_events: Vec<DecodedEventParts>,
src/Cargo.toml:5
- Adding
learning_mode_corechanges the documented workspace architecture and also makeslearning_mode_windowsno longer depend only onwxc_common, but.github/copilot-instructions.md:194,203still describes the old crate layout/dependency boundary. The repository instructions require updating that architecture reference in the same PR.
"core/learning_mode_core",
c3c3627 to
f4ebb37
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (5)
src/backends/learning_mode/windows/src/etl_decode.rs:148
- This API buffers every decoded event and all rendered property strings before
lm_analyze --rawwrites anything. Long ETLs can therefore consume multiple GB; the existing PLM parser deliberately uses callback streaming for exactly this reason (src/host/plm/src/event_parser.rs:257-263). Expose a callback/writer-based traversal and emit raw events duringProcessTraceinstead of returning aVec.
pub fn decode_raw_events(source_path: &Path) -> Result<Vec<DecodedEventParts>, AnalyzeError> {
src/backends/learning_mode/windows/src/extractors.rs:342
- Registry
GENERIC_EXECUTEis currently classified asExecute, even thoughKEY_EXECUTEaliasesKEY_READ(as the adjacent comment notes). If this generic bit survives into the event mask, the emitted denial reports the wrong access type. IncludeGENERIC_EXECUTEin the registry read mask and leave the registry execute mask empty.
// Registry has no execute concept (KEY_EXECUTE aliases KEY_READ).
GENERIC_EXECUTE,
src/backends/learning_mode/windows/examples/lm_analyze.rs:119
- ETW event IDs are provider-scoped, but this histogram and each output row use only
event_id. The trace intentionally handles IDs 14 and 27 from two providers, so--rawmerges distinct schemas and omits the provider needed for schema discovery. Include the provider in both the key and rendered row.
let mut histogram: BTreeMap<u16, usize> = BTreeMap::new();
src/backends/learning_mode/windows/src/etl_decode.rs:123
- No test exercises this analyzer with a valid ETL: the current tests stop at a missing-file error or feed already-decoded hand-built properties into the extractor. Consequently, the OpenTrace callback setup, provider GUIDs, TDH metadata traversal, property offsets, and real event templates can all regress while the suite remains green. Add a representative sealed ETL fixture (or a Windows integration test that captures one) and assert the end-to-end result.
This issue also appears on line 148 of the same file.
process_trace_file(source_path, CollectionMode::Analyze)?.into_analysis()
src/core/learning_mode_core/src/model.rs:102
- Serializing a current Windows
FILETIME(~1.3e17) as a JSON number exceeds JavaScript'sNumber.MAX_SAFE_INTEGER, so TypeScript/Node consumers cannot parse this advertised SDK wire field exactly. Define a JSON-safe representation—such as a decimal string, or a documented lower-resolution timestamp—before establishing this public cross-platform contract.
pub filetime: u64,
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/backends/learning_mode/windows/src/etl_decode.rs:148
- Returning a
Vecbuffers every event and all decoded property strings beforelm_analyze --rawwrites anything, so memory grows with the entire ETL. This repository's existing parser deliberately streams via a callback because the previous event buffer reached multiple GB on hour-long traces (src/host/plm/src/event_parser.rs:257-263). Expose a callback/visitor or writer-based streaming API and update the example to maintain its histogram incrementally.
pub fn decode_raw_events(source_path: &Path) -> Result<Vec<DecodedEventParts>, AnalyzeError> {
src/backends/learning_mode/windows/src/path_norm.rs:51
- This only strips the NT
\??\prefix, leaving the\\?\verbatim and\\.\DOS-device forms unnormalized. The same kernel provider naturally renders those forms (seesrc/host/plm/src/access_failure.rs:267-277), so these records will retain a non-canonical path even though the public model promises drive-letter form. Handle all three prefixes, including theirUNC\form.
if let Some(rest) = kernel_path.strip_prefix(r"\??\") {
if let Some(unc) = rest.strip_prefix(r"UNC\") {
return Some(format!(r"\\{unc}"));
}
return Some(rest.to_string());
src/core/learning_mode_core/src/model.rs:102
- Serializing this
u64as a JSON number is not lossless for the TypeScript/JavaScript SDK consumers described by this wire model. Current FILETIME values (including the132847890123456789test value) exceedNumber.MAX_SAFE_INTEGER, so a normal JSON parse changes the timestamp. Encode the value as a decimal string, or define a lower-precision timestamp representation that remains safely representable.
/// Kernel timestamp of the denial. On Windows this is `FILETIME`
/// (100-nanosecond intervals since 1601-01-01 UTC), copied from
/// `EVENT_RECORD.EventHeader.TimeStamp`. Other backends normalise
/// their native clocks onto the same epoch so consumers can treat
/// the field uniformly.
pub filetime: u64,
src/backends/learning_mode/windows/src/extractors.rs:362
FILE_DELETE_CHILD(0x40) is missing from the file write mask, so a directory denial requesting only delete-child access is emitted asUnknowninstead ofWrite. The existing PLM classifier includes this right asDIRECTORY_DELETE_MASKinsrc/host/plm/src/config.rs:18,24-31; include it here as well so policy regeneration does not miss directory deletions.
FILE_WRITE_DATA
| FILE_APPEND_DATA
| FILE_WRITE_EA
| FILE_WRITE_ATTRIBUTES
| standard_write
| GENERIC_WRITE
| GENERIC_ALL,
f25c2c3 to
0d87ae8
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (2)
src/backends/learning_mode/windows/src/etl_decode.rs:94
- Unnormalizable file objects are still emitted as filesystem resources with their raw kernel names. For example, an
ObjectType="File"access to\Device\Mup\server\share\filereaches this fallback and becomes aresourceType: "file"denial whose path cannot be used in MXC's filesystem policy; named-pipe and other device-file events have the same problem. This also contradictsDeniedResource's user-visible canonical-path contract. Normalize supported network forms to UNC and omit or reclassify file objects that cannot be represented as filesystem policy paths instead of preserving the kernel path.
let path =
path_norm::to_user_visible(&raw.object_name).unwrap_or_else(|| raw.object_name.clone());
src/backends/learning_mode/windows/src/tdh_decode.rs:82
- The production TDH composition is currently untested: the tests below exercise primitive value formatters, while
decode_event_parts/decode_propertiesmetadata traversal (top-level properties, structs, parameterized counts/lengths, and offsets) is never run against a representativeEVENT_RECORD. The extractor tests start after this boundary with hand-built name/value pairs, so a TDH offset or property-name regression can pass all tests while sealed ETLs produce no denials or fail decoding. Add a synthetic metadata/payload fixture or a representative sealed-ETL integration test covering events 14, 27, and 28.
pub unsafe fn decode_event_parts(
event_record: *mut EVENT_RECORD,
) -> Result<DecodedEventParts, String> {
| //! Decode a sealed learning-mode `.etl` into the captureDenials RFC 7464 | ||
| //! JSON text sequence, or dump its raw ETW events for schema discovery. | ||
| //! | ||
| //! Usage: | ||
| //! | ||
| //! ```text | ||
| //! # Emit the DeniedResource JSON text sequence (0x1E-framed) to stdout: | ||
| //! cargo run -p learning_mode_windows --example lm_analyze -- <path-to.etl> --exit-code <code> | ||
| //! | ||
| //! # Dump every decoded event (id + property name/value pairs): | ||
| //! cargo run -p learning_mode_windows --example lm_analyze -- <path-to.etl> --raw | ||
| //! ``` | ||
| //! | ||
| //! Exit codes: `0` = decoded; `2` = wrong platform / bad args; `1` = decode | ||
| //! failed. | ||
|
|
There was a problem hiding this comment.
question: is it the developer who is running cargo run -p learning_mode_windows --example lm_analyze -- <path-to.etl> --exit-code <code> ? or lets say GitHub copilot on a user's machine (for my question assume the user has no clue GitHub ;copilot is using MXC and just wants it to perform some workload)?
| //! Sealed-ETL decoder: turns the `.etl` that [`crate::CaptureSession::finish`] | ||
| //! produces into cross-platform [`DeniedResource`]s. | ||
| //! |
There was a problem hiding this comment.
question: can any of https://github.com/microsoft/mxc/blob/119afb7121bbdc54a4810feb422c338b9321eef5/src/tools/mxc_diagnostic_console/src/etw.rs be reused here in this file?
| // Copyright (c) Microsoft Corporation. | ||
| // Licensed under the MIT License. | ||
|
|
||
| //! ETW event -> [`RawDenial`] extractors. |
There was a problem hiding this comment.
note: have the same question for this file as I do for etl_decode.rs. Just trying to see what can be shared and that I'm doing my due diligence to prevent duplication.
0d87ae8 to
daabc16
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (4)
src/backends/learning_mode/windows/src/path_norm.rs:144
- The fixed 260-WCHAR buffer can make
QueryDosDeviceWfail withERROR_INSUFFICIENT_BUFFERfor a long drive mapping. The currentn == 0branch silently omits that drive, soadd_raw_deniallater drops every\Device\...denial on it. Retry with a growing buffer when the last error is 122 instead of treating it as an absent drive.
let mut buf = [0u16; 260];
for c in b'A'..=b'Z' {
let letter = format!("{}:", c as char);
let wide: Vec<u16> = letter.encode_utf16().chain(std::iter::once(0)).collect();
// SAFETY: `wide` is a valid null-terminated wide string; `buf` is a
// valid mutable slice. The function writes at most `buf.len()` u16s.
let n = unsafe { QueryDosDeviceW(PCWSTR(wide.as_ptr()), Some(&mut buf)) };
if n == 0 {
continue;
src/Cargo.toml:5
- This adds a new cross-platform core crate and wire-format layer, but the repository architecture documentation in
.github/copilot-instructions.mdstill describes the learning-mode support only as the Windows backend crate. The project’s PR convention requires that file to be updated when architecture or key conventions change; please documentlearning_mode_coreand its boundary withlearning_mode_windows.
"core/learning_mode_core",
src/core/learning_mode_core/src/emit.rs:43
- The documented contract says serialization failures use
InvalidData, butio::Error::otherproducesErrorKind::Other. Callers matching the advertised kind would receive the wrong result; construct the error withInvalidDataexplicitly.
let json = serde_json::to_vec(frame).map_err(io::Error::other)?;
src/backends/learning_mode/windows/src/etl_decode.rs:266
ERROR_CANCELLEDmeans trace processing was canceled (for example by a buffer callback orCloseTrace), not that the file reached EOF. Accepting it here can return a partial denial set withdenied_resources_truncated == false. This path does not intentionally cancel processing, so onlyERROR_SUCCESSshould be treated as complete.
// ERROR_SUCCESS (0) and ERROR_CANCELLED (1223) are both acceptable
// terminal states for a completed file trace.
if status.0 != 0 && status.0 != 1223 {
Introduce the learning_mode_core crate as the cross-platform hinge of the captureDenials pipeline (PR4/PR5 foundation): - model: DeniedResource / ResourceType (file|ui|network|capability|other) / AccessType, camelCase wire contract with round-trip + key-stability tests. - summary: DenialSummary terminator (exitCode, totalDenials, deniedResourcesTruncated). - frame: self-describing type-tagged DenialFrame (denial|summary). - emit: RFC 7464 (0x1E-framed) NDJSON stream writer over any io::Write. - analyze: DenialAnalyzer decode trait + AnalyzeError, implemented per-OS by the backend ETL decoders (Windows first). No OS-specific code; 16 unit tests + 1 doc-test, clippy/fmt clean. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Decode the learning-mode `.etl` that CaptureSession::finish seals into cross-platform learning_mode_core::DeniedResource values, implementing the DenialAnalyzer trait. - path_norm: kernel-form -> drive-letter, incl. the `\??\C:\...` / `\??\UNC\...` DOS-devices prefix the trace actually emits. - tdh_decode: TDH EVENT_RECORD -> (event_id, props). - extractors: route event 14/4907 (access check) by ObjectType (File -> file, Key -> registry/other, empty -> capability) and derive accessType from AccessMask with a File/Registry right vocabulary (Write > Execute > Read); event 27 -> Ui; event 28 -> capability denial (Denied gate, payload ProcessId). Missing/unparseable mask degrades to Unknown rather than dropping the denial. - etl_decode: file-mode OpenTraceW/ProcessTrace loop + dedup by (user-visible path, accessType). - examples/lm_analyze: NDJSON emit + `--raw` event dump for validation. Event vocabulary and AccessMask classification confirmed on the GE_CURRENT x64 VM against real block-and-log (Mode="Normal") and allow-and-log (Mode="Permissive") captures: File/registry denials decode identically in both modes; capability denials arrive as empty-ObjectType event 14 (permissive) or event 28 (block). Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Factor the pure decode composition (extract_denial routing -> path normalisation -> dedup) out of EtlDenialAnalyzer::analyze into resources_from_events, and cover it with tests built from the event shapes captured on hardware for both learning modes: - block-and-log (Mode="Normal"): file/registry event 14 + capability event 28 - allow-and-log (Mode="Permissive"): capability folded into empty-ObjectType event 14 (no event 28) - empty-path capabilities from event 14 and event 28 collapse to one resource - non-actionable object types / not-denied records / unknown event IDs dropped A live ETW/TDH fixture read is intentionally not used: it needs the provider manifests registered on the machine, and a raw capture embeds token SIDs that don't belong in a public repo. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
Allocate TRACE_EVENT_INFO storage with its required alignment and decode UTF-16 payloads from byte pairs so unaligned ETW data never forms invalid Rust references. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
Stream raw ETL diagnostics, honor producer pointer width, normalize Windows path aliases, correct access masks and empty identifiers, and serialize FILETIME losslessly for JSON consumers. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
Describe the record-separator-framed output as an RFC 7464 JSON text sequence rather than NDJSON across the core API and diagnostic example. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
Normalize DOS devices, mapped drives, MUP redirectors, and DFS paths to absolute user-visible forms while omitting pipes, relative paths, and unknown device namespaces. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
Document lm_analyze as a developer-only tool and explain why the sealed, policy-oriented learning-mode decoder does not directly reuse the diagnostic console's real-time display consumer. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: dfea4d42-e8a5-42ef-978a-6434e8bf89ec
daabc16 to
6b645d5
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 15 out of 16 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (7)
src/core/learning_mode_core/src/emit.rs:43
- The documented error kind is not what this returns:
io::Error::othercreatesErrorKind::Other, while the public contract above promisesInvalidData. Preserve that contract so callers can classify serialization failures correctly.
let json = serde_json::to_vec(frame).map_err(io::Error::other)?;
src/backends/learning_mode/windows/src/etl_decode.rs:275
ERROR_CANCELLEDdoes not mean normal end-of-file;ProcessTracedocuments it as processing cancelled by the consumer (for example, a buffer callback returning false). This code installs no cancellation callback, so accepting 1223 can silently summarize a partially processed trace as complete. OnlyERROR_SUCCESSshould be accepted here.
// ERROR_SUCCESS (0) and ERROR_CANCELLED (1223) are both acceptable
// terminal states for a completed file trace.
if status.0 != 0 && status.0 != 1223 {
src/backends/learning_mode/windows/src/path_norm.rs:134
- The fixed 260-WCHAR buffer can make
QueryDosDeviceWfail withERROR_INSUFFICIENT_BUFFER; the currentn == 0branch then silently omits that drive. Valid denials on a long SUBST/redirector mapping will consequently be dropped during normalization. Retry with a growing buffer when that error is returned.
let mut buf = [0u16; 260];
src/backends/learning_mode/windows/examples/lm_analyze.rs:9
- This says normal
captureDenialsconsumes the trace automatically, but the BaseContainer teardown currently only callsCaptureSession::finish, reports the ETL path, and never invokesEtlDenialAnalyzeror the emitter (base_container_runner.rs:1590-1603). Please describe this as a manual ETL analyzer until production integration is added.
//! End users and SDK agents do not invoke it: the BaseContainer runner seals
//! and consumes the trace automatically during normal `captureDenials` use.
src/Cargo.toml:5
- Adding this cross-platform core crate changes the documented workspace architecture, but
.github/copilot-instructions.md:194,203still omits it and sayslearning_mode_windowsdepends only onwxc_common. Update those architecture notes in this PR so future changes use the correct dependency boundaries.
"core/learning_mode_core",
src/backends/learning_mode/windows/src/tdh_decode.rs:107
- The analyzer's production path depends on this metadata-driven decoder, but tests only exercise individual value formatters; none constructs metadata to drive
decode_propertiesthrough parameterized count/length, arrays, or structs, and the composition tests bypass TDH entirely. Add focused metadata fixtures (or a sealed-ETL integration fixture) so offset errors cannot silently corrupt every later property.
let props = decode_properties(buffer.as_bytes(), info, event_record, pointer_size)?;
src/backends/learning_mode/windows/src/tdh_decode.rs:185
StructStartIndexandNumOfStructMemberscome from TDH metadata, but their addition is unchecked. Malformed metadata can overflow here (panicking inside the extern ETW callback in checked builds) or wrap and silently skip/misdecode members in release builds. Validate the complete range againstPropertyCountbefore iterating.
let start_index = unsafe { prop_info.Anonymous1.structType.StructStartIndex } as usize;
let member_count = unsafe { prop_info.Anonymous1.structType.NumOfStructMembers } as usize;
for _ in 0..count {
for child_index in start_index..start_index + member_count {
| // If the record explicitly reports the check as not-denied, skip it; | ||
| // absence of the field is treated as a denial (fail open on decode gap). | ||
| if let Some(denied) = find_prop(&parts.props, "Denied") { | ||
| if !denied.trim_matches('"').eq_ignore_ascii_case("true") { | ||
| return None; | ||
| } | ||
| } |
📖 Description
Adds the denial-analysis layer on top of Integration PR #696.
learning_mode_coredenial model, summary, analysis, framing, and emission primitives.lm_analyzefor manually validating captured ETL files.This PR is intentionally stacked on #696 and should merge after it.
🔗 References
🔍 Validation
cargo test -p learning_mode_corecargo test -p learning_mode_windows --all-targetscargo clippy -p learning_mode_core -p learning_mode_windows --all-targets -- -D warningscargo check --workspace --all-targetscargo clippy --workspace --all-targets -- -D warnings✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
GitHub Actions runs the PR validation build automatically. The ADO pipeline
(
MXC-PR-Build) is the Azure version of the PR pipeline, kept in parity with the GitHubActions build; it runs on merge to
main, and Microsoft reviewers with write access can trigger iton a PR with
/azp run. See docs/pull-requests.md.If the
dependency-feed-checkcheck fails on a new dependency, the crate must be added tothe feed before the PR can pass. See docs/pull-requests.md
for the steps.
Microsoft Reviewers: Open in CodeFlow