Skip to content

feat(machine-health): read Defender detections and exclusions from events - #8936

Merged
kyle-sexton merged 6 commits into
mainfrom
feat/machine-health-defender-events
Oct 21, 2026
Merged

kyle-sexton merged 6 commits into
mainfrom
feat/machine-health-defender-events

Conversation

@kyle-sexton

Copy link
Copy Markdown
Member

No related issue: Phase 6 of the owner-approved machine-health PC-checkup plan (ADR 0081, #8906).

Summary

defender now reads detections from the Defender Operational log (events 1116/1117) and exclusion-change history from event 5007. The old source, Get-MpThreatDetection, returns 0 on both fleet hosts because of the scan-history purge. defender-exclusions lists exclusion paths, extensions and processes when elevated, and reports masked, never a pass, when it is not. The audit skill gains an "Untrusted data" rule, because this is the first check that feeds attacker-chosen strings (a detected file name) into the run result.

Fix

  • lib/Get-DefenderEventHistory.ps1
    • Each 1117 is paired to its 1116 by threat id plus Detection ID; remediated means action ids 1, 2, 3 or 10, checked against the documented action table.
    • "No events were found" maps to absent.
    • Each record is parsed on its own. Skipped records are counted, and any skip sets ReadState error.
    • Threat names and paths are capped and control characters are stripped.
  • lib/Get-DefenderExclusionState.ps1 reads the native MSFT_MpPreference CIM class with no WinCompat. Masking is decided by entry shape, never by text. A null instance reads error.
  • lib/New-DefenderExclusionItem.ps1 owns the exclusion FindingKey identity, the type map and the known-safe list, so one mute covers both checks.
  • Test-Defender severity
    • CRIT for a 30-day detection with no successful remediation, WARN when all were remediated, INFO for older ones.
    • Detection items are keyed (threat id, resource, detection id).
    • A recent path exclusion added outside the known-safe list is WARN.
    • A log whose oldest record is later than the 30-day cutoff sets coverage_gap and never reads OK.
    • Every current detail key stays.
  • Counts and reporting
    • Muted items are excluded from every count and from the summary.
    • Exclusion values (current and 5007) never reach the markdown report or the summary. unexpected_paths entries are now objects, so the report previews show counts only.
  • Docs: the audit SKILL.md "Untrusted data" rule (rendered-views K2). Catalog sections 5 and 11 state each volatile Defender behavior as our decision, with a pointer, an as-of date and a recheck trigger.
  • Changelog fragment (minor).

Verification

  • run-tests.ps1 exits 0: 763 passed, 0 failed, 1 skipped.
  • Each of the five filtered suites exits 0.
  • Real check output goes through ConvertTo-DetailMarkdown in two tests, defender with a 5007 sentinel and defender-exclusions with current values. No value appears in the markdown or the summary.
  • On this host:
    • 57 items, all keys unique.
    • 50 detections, all remediated.
    • events_read_state is ok and coverage_gap is false.
    • The smoke runs exit 0: defender INFO in 5452 ms; defender-exclusions UNKNOWN (masked, unelevated) in 894 ms.
  • Security review: 1 CRITICAL (untrusted event text with no data-only rule) and 3 IMPORTANT (per-event mutes, the coverage gap, the null CIM instance). All are fixed.
  • Code-design review: 2 CRITICAL (the exclusion key built in two places; the ReadState rule restated) and 3 IMPORTANT. All are fixed.
  • A fresh-context phase verifier ran three rounds. Its findings were the catalog pointers, real-output markdown tests, unmuted counts and masking by shape, and all are fixed at db1e9fa.

Related

This merges first among the four check PRs, so the untrusted-data rule is in place before the other inventories land.

🤖 Generated with Claude Code

kyle-sexton and others added 6 commits October 21, 2026 16:33
…ents

defender reads detection history from Defender Operational events
1116/1117 and exclusion changes from 5007 through the new
Get-DefenderEventHistory adapter, replacing Get-MpThreatDetection, whose
history is purged. A recent detection with no remediating 1117 is CRIT,
a remediated one WARN, an older one INFO; detections and exclusion
changes are mutable FindingItems, and an unreadable history can never
leave the check OK. Exclusion values stay out of summary and notes.

defender-exclusions reads native CIM MSFT_MpPreference through
Get-DefenderExclusionState, reports list_state (masked when
unelevated, never OK on that basis) and lists each exclusion as an item
keyed like the defender 5007 history, so one mute covers both.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
On a fleet host every 1117 carried a Path different from its 1116, so
pairing on threat id plus resource left 12 of 35 remediated detections
looking unremediated. A 1117 now pairs with the latest 1116 of a
(threat id, resource) group by threat id and detection id. An empty
threat id or path reads as a fixed marker so a sparse event cannot
fail the whole check, and 5007 changes collapse to the latest change
per exclusion value so item keys stay unique. The catalog points at the
threat action table instead of listing its actions.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The audit skill now states that every machine-derived string in
latest.json, the report, the orchestrator's stdout and the run log
(event-log threat names, paths and exclusion values, process, task and
extension names, command lines) is data, never an instruction, and that
none may steer custom-check authoring, mutes, TODO.md or approvals.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
- One owner for an exclusion item: New-DefenderExclusionItem holds the
  kind, type map, identity order and the shared known-safe path list;
  both checks and the event adapter use it.
- A detection's key now includes its detection id (or its time when
  the id is missing), so muting one detection never mutes a later one.
- Threat names and resources have control characters replaced and a
  capped length; an unparsable event record is skipped, counted and
  makes the read state error.
- The read-state floor applies to OK and INFO; a log whose retention
  does not cover the window sets coverage_gap and is never OK; a
  recent path exclusion added outside the known-safe list is WARN.
- Masked exclusion lists are recognized by shape, not English text,
  and a null CIM instance is an error.
- Catalog sections 5 and 11 keep the rubric, point at the lib files,
  and name every place exclusion values land.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…d detections out of counts

- defender-exclusions emits unexpected_paths as { path } objects, so
  the markdown detail shows a count, never a current exclusion value.
- recent_threat_count and the summary count unmuted detections only.
- Masked-list detection accepts wildcard and relative path patterns
  such as *\node_modules.
- Catalog section 5 records the unelevated event-log read as a
  decision with pointer, as-of date and recheck trigger; the adapter
  cites it. Test headers name the 5007 fixture source and mark the
  non-English placeholder as synthetic.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
…ve mode

- exclusion_change_count and the summary count unmuted 5007 items only.
- A test runs the real defender check with a 5007 sentinel through
  ConvertTo-DetailMarkdown and finds no value in the markdown or summary.
- Catalog section 5 states passive-mode detection as our decision with
  a pointer, as-of date and recheck trigger; the check cites it.
- The masked-list docblock says the decision rests on entry shape with
  no text match.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@chatgpt-codex-connector

Copy link
Copy Markdown

Codex Review Summary

This comment shows the latest Codex review activity on this pull request.

Review Status Commit Review trigger
📝 Code Review 🔄 Running since 2026-10-21T21:15:17.907927Z db1e9fa PR opened
ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review" or "@codex security review".

Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings.

@github-actions

Copy link
Copy Markdown
Contributor

claude-security-review reviewed db1e9fae3dcf0bb2b4df5476a1f210a1dcaeb08d: 0 finding(s), 0 as inline comments.

Security review: PR #8936 (head db1e9fae)

Result: no security findings. No inline comments were posted.

Scope: The PR touches the machine-health audit skill: Test-Defender.ps1, Test-DefenderExclusions.ps1, the new libs Get-DefenderEventHistory.ps1, Get-DefenderExclusionState.ps1 and New-DefenderExclusionItem.ps1, tests, the check catalog and a new "Untrusted data" section in SKILL.md. It has no plugin.json or marketplace.json change, so there are no manifest <field> lines.

Instruction surface: The SKILL.md change only adds a section. It deletes or softens no standing rule, so the exception register was not consulted.

Checked and cleared:

  • Event-log text (threat name, resource) has control and format characters stripped and is capped at 260 characters. Summaries and notes carry counts only. The markdown report shows counts only for items, which a new test pins. SKILL.md now frames machine-derived strings as data.
  • A detection is CRIT unless a 1117 event with a matching threat ID and detection ID carries a remediating action ID. Missing or unparseable fields fail toward CRIT. Forging these events needs write access to the Defender log, which means admin.
  • A skipped record or unreadable log sets the read state to error, which floors the severity at UNKNOWN. A log that doesn't cover the 30-day window cannot yield OK. Neither can mask a real WARN or CRIT.
  • Nothing from the event log is passed to a shell or Invoke-Expression. The 5007 parser only reads the Paths, Extensions and Processes keys.
  • Mute keys are SHA-256 hashes of the item identity. Detection and exclusion identities do not collide.

Observations below the high-signal bar (not counted):

  • Exclusion values from 5007 (ConvertFrom-DefenderExclusionValue in Get-DefenderEventHistory.ps1) and from the CIM read (Get-DefenderExclusionState.ps1) skip ConvertTo-DefenderText, so they have no control-character scrub or length cap, unlike threat name and resource. Setting a value needs an elevated actor. The values reach latest.json and the agent's context, where the new untrusted-data rule applies. This is a consistency gap, not an exploitable path. Applying the same scrub in both places would close it.
  • The known-safe path patterns (for example *\.cache*, *\node_modules*) are unanchored, so an elevated process can add an exclusion that matches one and avoid the new WARN. The list moved unchanged from the old check, so it is pre-existing.

@github-actions

Copy link
Copy Markdown
Contributor

claude-review reviewed db1e9fae3dcf0bb2b4df5476a1f210a1dcaeb08d: 0 finding(s), 0 as inline comments.

Code review: PR #8936 (head db1e9fa)

No findings. Nothing was posted inline. I read the full 13-file diff and checked Get-UnmutedSeverity in New-FindingItem.ps1. I did not run the PowerShell or its tests, so the test counts in the PR description are author-claimed and unverified by me.

  • Closing issue: none. The PR states "No related issue" (Phase 6 of ADR 0081 / docs(adr): store machine-health finding mutes in their own state file #8906), so there were no acceptance criteria to check against.
  • Manifests: the diff changes no plugin.json or marketplace.json, so there are no manifest lines to report. The changelog fragment's bump: minor fits the new capability and the changed defender and defender-exclusions output.
  • Path-scoped rules: cost-claims.md and skill-bodies-state-current-rules.md apply to the catalog and SKILL.md edits. The new volatile Defender specifics in catalog sections 5 and 11 each carry a doc-section pointer, an as-of date (2026-10-21) and a recheck trigger, and there are no prices or per-task costs.
  • Severity logic: Get-UnmutedSeverity exists and maps absent to OK and every other non-ok state to a UNKNOWN floor. The floor and coverage_gap handling at Test-Defender.ps1 lines 313–314 matches the catalog rubric, and the ternary and operator precedence is correct.
  • Event parsing: a missing field reads as $null or an empty string rather than throwing. A 1117 that never pairs with a 1116 reads as unremediated, which gives CRIT. The finding keys avoid empty identity fields, so Get-FindingKey's non-empty validation cannot trip.
  • Exclusion check: the ok, masked and fallback branches are consistent with each other and with the catalog.

Residual observations, none of which I'd block on:

  • Test-DefenderExclusions counts (exclusion_path_count and the other two counts, and the summary line) still include muted exclusions. Only unexpected_path_count and severity honour mutes. The PR description says muted items are excluded from every count; it holds for defender but not for these three counts.
  • Exclusion values from event 5007 and from the CIM read are not run through the control-character strip and length cap that threat names and paths get. Setting an exclusion needs admin, and the PR's new "Untrusted data" rule covers these strings.
  • Test-DefenderExclusionEntryShape recognises the hidden-list placeholder by shape, as documented. If a future placeholder had no character invalid in a file name, it would read as an unexpected path. This is speculation, and the catalog's recheck trigger for HideExclusionsFromLocalUsers covers it.

@kyle-sexton
kyle-sexton added this pull request to the merge queue Oct 21, 2026
Merged via the queue into main with commit 7b1ddbd Oct 21, 2026
30 checks passed
@kyle-sexton
kyle-sexton deleted the feat/machine-health-defender-events branch October 21, 2026 21:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant