Consolidate Learning Mode analyzer coverage - #759
Conversation
Centralize Event 27 UI vocabulary, cover every Job UI limit, and make block/allow composition tests provider-accurate. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
Warning
Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.
Consolidates Learning Mode ETL analysis so block/permissive traces share the same canonical UI vocabulary and capability-denial extraction (including permissive Event 14 DACL-derived capability denials).
Changes:
- Added a canonical UI violation vocabulary module and updated Learning Mode violation extraction to use it.
- Extended TDH decoding to render binary properties as
hex:strings and added capability recovery from permissive Event 14 DACL data. - Updated ETL analyzer composition/tests to include recovered capability denials and enforce provider/event vocabulary.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/backends/learning_mode/windows/src/ui.rs | Introduces canonical Event 27 UI violation naming and tests. |
| src/backends/learning_mode/windows/src/tdh_decode.rs | Adds TDH_INTYPE_BINARY formatting as hex: for downstream parsing. |
| src/backends/learning_mode/windows/src/lib.rs | Registers new capability_dacl and ui modules on Windows. |
| src/backends/learning_mode/windows/src/extractors.rs | Switches Event 27 parsing to numeric category/detail and canonical UI names; adds tests. |
| src/backends/learning_mode/windows/src/etl_decode.rs | Composes DACL-derived capability denials alongside canonical extraction; updates tests/helpers. |
| src/backends/learning_mode/windows/src/capability_dacl.rs | New module to recover capability denials from permissive Event 14 DACL/ACE data with tests. |
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
Suppressed comments (3)
src/backends/learning_mode/windows/src/capability_dacl.rs:292
- The
.nth(4)selection is a magic index and is easy to break if the event template/property ordering changes. Consider introducing a named constant (e.g.,const COMPLEXDATA_DACL_INDEX: usize = 4;) and expanding the comment to explain why the 5thComplexDatais special for these traces. If possible, prefer selecting the intended payload by property name (or a more explicit heuristic) rather than a positional index.
if let Some(value) = parts
.props
.iter()
.filter(|(name, _)| name.eq_ignore_ascii_case("ComplexData"))
.nth(4)
.and_then(|(_, value)| value.strip_prefix("hex:"))
{
explicit.push(value);
}
src/backends/learning_mode/windows/src/extractors.rs:603
- This test duplicates the authoritative UI-limit mapping that now lives in
src/ui.rs(and is already unit-tested there). Duplicating the list in multiple places increases drift risk when newJOB_OBJECT_UILIMIT_*values are added. Consider narrowing this extractor-level test to a couple representative values (verifyingbuild_denial_from_learning_modecorrectly delegates toui::resource_name), and keep the exhaustive mapping assertion only inui.rs.
fn learning_mode_violation_maps_every_ui_limit() {
let expected = [
(0x0001, "Handles"),
(0x0002, "ReadClipboard"),
(0x0004, "WriteClipboard"),
(0x0008, "SystemParameters"),
(0x0010, "DisplaySettings"),
(0x0020, "GlobalAtoms"),
(0x0040, "Desktop"),
(0x0080, "ExitWindows"),
(0x0100, "IME"),
(0x0200, "Injection"),
];
src/backends/learning_mode/windows/src/capability_dacl.rs:567
- Building the capability index performs
DeriveCapabilitySidsFromNameonce per entry inKNOWN_CAPABILITIES(and does multiple allocations per iteration). Even though it’s guarded byOnceLock, the first permissive capability event will pay the full cost, which could be noticeable during ETL analysis. If this shows up in profiles, consider precomputing/embedding the SID map (or persisting it) to avoid repeated Win32 calls at runtime.
fn build_capability_index() -> CapabilityIndex {
let mut index =
CapabilityIndex::with_capacity(KNOWN_CAPABILITIES.len() * 2 + LEGACY_CAPABILITY_SIDS.len());
for &name in KNOWN_CAPABILITIES {
let wide: Vec<u16> = name.encode_utf16().chain(std::iter::once(0)).collect();
let mut group_sids: *mut PSID = std::ptr::null_mut();
let mut group_count = 0;
let mut capability_sids: *mut PSID = std::ptr::null_mut();
let mut capability_count = 0;
let result = unsafe {
DeriveCapabilitySidsFromName(
PWSTR(wide.as_ptr().cast_mut()),
&mut group_sids,
&mut group_count,
&mut capability_sids,
&mut capability_count,
)
};
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 52e48fee-23e1-4b05-803c-522812fcdda2
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated no new comments.
Suppressed comments (1)
src/backends/learning_mode/windows/src/capability_dacl.rs:258
extract_namesreturns a randomly seededHashSet, so iterating it directly makes the denial array order vary between runs for the same ETL. This bypasses the accumulator's first-seen ordering guarantee and, when the 10,000-denial bound is reached, can even change which capabilities are retained. Convert to a vector and impose a stable order (or preserve ACE encounter order) before composing the denials.
let names = extract_names(parts, CAPABILITY_INDEX.get_or_init(build_capability_index));
names
.into_iter()
|
/azp run |
|
Azure Pipelines: Successfully started running 1 pipeline(s). |
📖 Description
Consolidates Learning Mode block and permissive-event analysis behind the canonical ETL analyzer.
JOB_OBJECT_UILIMIT_*value;CapabilitySidproperties;🔗 References
🔍 Validation
cargo test -p learning_mode_windows --all-targets— 144 passed.cargo fmt --all -- --checkcargo clippy -p learning_mode_windows --all-targets -- -D warnings✅ Checklist
Cargo.lock, thedependency-feed-checkcheck passes (see docs/pull-requests.md)📋 Issue Type
Microsoft Reviewers: Open in CodeFlow