fix(intune): preserve scoped names across retries and events - #438
Merged
Conversation
github-actions
Bot
added
bug
Owner
Author
|
@coderabbitai review Please review the exact current head. Focus on retry-state inheritance and isolation, selected-object name scope, explicit identity boundaries, and safe fallback enrichment across all three event paths. |
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: ASSERTIVE Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughChangesIntune identity enrichment
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant IntuneMessage
participant IdentityContext
participant DownloadOrEventExtractor
participant GUIDRegistry
IntuneMessage->>IdentityContext: classify explicit identity and retain JSON scope
IdentityContext-->>DownloadOrEventExtractor: GUID, local name, and suppression state
DownloadOrEventExtractor->>GUIDRegistry: resolve name only when fallback is allowed
GUIDRegistry-->>DownloadOrEventExtractor: resolved name
Possibly related issues
Possibly related PRs
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Current HEAD and merge base =='
git status --short
git rev-parse HEAD
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
printf 'merge-base=%s\n' "$base"
printf '%s\n' '== Changed files =='
git diff --name-status "$base"...HEAD
printf '%s\n' '== Diff summary =='
git diff --stat "$base"...HEAD
printf '%s\n' '== Intune-related changed diff =='
git diff --unified=80 "$base"...HEAD -- \
':(glob)**/*.rs' \
| rg -n -i -C 3 'intune|retry|download|appworkload|sidecar|guid|registry|explicit|identity|name' \
|| true
printf '%s\n' '== Rust files with relevant terms =='
rg -n -i -C 3 \
'retry|content.?id|appworkload|sidecar|guid.?registry|explicit.?identity|selected.?object|intune' \
-g '*.rs' \
.
printf '%s\n' '== Structural outline for changed Rust files =='
git diff --name-only --diff-filter=ACMR "$base"...HEAD -- '*.rs' \
| while IFS= read -r file; do
printf '\n--- %s ---\n' "$file"
ast-grep outline "$file" --items all || true
doneLength of output: 201585 🏁 Script executed: #!/bin/bash
set -euo pipefail
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
printf '%s\n' '== Exact current HEAD =='
git rev-parse HEAD
git status --short
printf '%s\n' '== Changed hunks: download state =='
git diff --unified=25 "$base"...HEAD -- crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '%s\n' '== Changed hunks: identity scope =='
git diff --unified=25 "$base"...HEAD -- crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '%s\n' '== Changed hunks: event enrichment =='
git diff --unified=25 "$base"...HEAD -- crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '%s\n' '== Download implementation =='
nl -ba crates/cmtraceopen-parser/src/intune/download_stats.rs | sed -n '100,570p'
printf '%s\n' '== GUID identity and local-name implementation =='
nl -ba crates/cmtraceopen-parser/src/intune/guid_registry.rs | sed -n '530,860p'
printf '%s\n' '== Event extraction and name enrichment implementation =='
nl -ba crates/cmtraceopen-parser/src/intune/event_tracker.rs | sed -n '450,750p'
nl -ba crates/cmtraceopen-parser/src/intune/event_tracker.rs | sed -n '1290,1495p'
printf '%s\n' '== New focused regression tests =='
rg -n -A90 -B8 \
'retry_keeps_explicit_name_suppression_across_attempts|retry_suppression_is_isolated_between_concurrent_content_ids|downloads_do_not_take_names_outside_the_selected_identity_object|selected_ancestor_does_not_take_a_repeated_descendant_identity_name|conflicting_duplicate_names_in_the_selected_object_fail_closed|ingested_descendant_name_does_not_enrich_outer_selected_download|appworkload_events_do_not_enrich_names_outside_selected_identity_scope|non_appworkload_events_do_not_enrich_ambiguous_explicit_names|event_name_enrichment_remains_for_safe_identity_contexts' \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rsLength of output: 68216 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '%s\n' '== Identity selection and local-name scope at current HEAD =='
sed -n '540,790p' crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '%s\n' '== Download retry state and final enrichment at current HEAD =='
sed -n '114,465p' crates/cmtraceopen-parser/src/intune/download_stats.rs
sed -n '498,565p' crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '%s\n' '== Event identity and enrichment paths at current HEAD =='
sed -n '471,730p' crates/cmtraceopen-parser/src/intune/event_tracker.rs
sed -n '1312,1488p' crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '%s\n' '== Targeted download regression tests =='
rg -n -A75 -B4 \
'fn (retry_keeps_explicit_name_suppression_across_attempts|retry_suppression_is_isolated_between_concurrent_content_ids|explicit_name_ambiguity_suppresses_existing_registry_enrichment|non_explicit_download_keeps_registry_fallback)' \
crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '%s\n' '== Targeted event regression tests =='
rg -n -A80 -B4 \
'fn (appworkload_events_do_not_enrich_names_outside_selected_identity_scope|non_appworkload_events_do_not_enrich_ambiguous_explicit_names|event_name_enrichment_remains_for_safe_identity_contexts|non_appworkload_events_respect_explicit_identity_boundaries)' \
crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '%s\n' '== Changed-file whitespace check =='
git diff --check "$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"...HEAD -- \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rsLength of output: 54690
I verified the requested boundaries:
The focused regression tests cover these cases. ✅ Action performedReview finished.
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR tightens Intune identity/name handling so explicit JSON identities act as hard boundaries, preventing stale global GUID registry names from being reattached during retries or when multiple downloads/events overlap. It introduces a scoped “identity + object-local name” context and threads it through event and download extraction to keep enrichment safe and deterministic.
Changes:
- Added
ExplicitAppIdentityContextto resolve an explicit identity and its object-local unambiguous name in one bounded scan, with explicit-identity boundaries suppressing unsafe fallback enrichment. - Updated event extraction to respect explicit identity boundaries and only enrich names from the selected object-local context (or registry fallback when no explicit identity exists).
- Updated download extraction to keep explicit-name suppression monotonic across retries and isolate suppression per content ID; expanded tests to cover ambiguity and boundary cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| crates/cmtraceopen-parser/src/intune/guid_registry.rs | Adds explicit identity+local-name context selection, ambiguity handling for duplicate names, and shared name-enrichment helper. |
| crates/cmtraceopen-parser/src/intune/event_tracker.rs | Uses explicit identity context to prevent unsafe name enrichment and stale registry fallback across event paths. |
| crates/cmtraceopen-parser/src/intune/download_stats.rs | Carries “suppress registry enrichment” through retries/content IDs and derives download identity from explicit identity context with expanded tests. |
![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
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/intune/event_tracker.rs (1)
608-617: 🚀 Performance & Scalability | 🔵 Trivial | 🏗️ Heavy liftSame re-scan pattern for AppWorkload events.
explicit_app_identity_context(msg)scans at line 608, thenguid_registry::extract_app_id(msg)(line 615) re-scans on theAbsentpath. Same root cause as lines 495-508 andguid_registry.rs.🤖 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/intune/event_tracker.rs` around lines 608 - 617, Update the AppWorkload GUID resolution around explicit_app_identity_context so the Absent branch reuses the already-scanned identity context instead of calling guid_registry::extract_app_id(msg) again. Preserve extract_guid(msg) and the existing Valid, Invalid, and Absent outcomes while eliminating the redundant scan.
🤖 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/intune/download_stats.rs`:
- Around line 387-420: The Absent branch of extract_download_identity
redundantly rescans msg through extract_app_id and extract_app_name after
explicit_app_identity_context has already scanned it. Reuse the parsed context
or extend explicit_app_identity_context to provide the needed fallback identity
and display name, then remove the independent extract_app_id/extract_app_name
scans while preserving the existing heuristic behavior.
In `@crates/cmtraceopen-parser/src/intune/event_tracker.rs`:
- Around line 495-508: The Absent branch in the event-tracking GUID resolution
repeats a full message scan through guid_registry::extract_app_id. Reuse the
parsed data already produced by guid_registry::explicit_app_identity_context, or
expose the needed extracted app ID from that result, so the fallback avoids
rescanning line.message while preserving the existing GUID extraction precedence
and behavior.
In `@crates/cmtraceopen-parser/src/intune/guid_registry.rs`:
- Around line 619-648: Update explicit_app_identity_context and its callers so
the existing JsonFieldScan is reused when explicit identity is Absent, rather
than calling extract_app_id and scan_has_name_field to rescan the message.
Expose or pass the scan-backed fallback logic through the relevant
download_stats.rs and event_tracker.rs paths, preserving current identity and
local-name results while ensuring one bounded scan handles both resolutions.
---
Outside diff comments:
In `@crates/cmtraceopen-parser/src/intune/event_tracker.rs`:
- Around line 608-617: Update the AppWorkload GUID resolution around
explicit_app_identity_context so the Absent branch reuses the already-scanned
identity context instead of calling guid_registry::extract_app_id(msg) again.
Preserve extract_guid(msg) and the existing Valid, Invalid, and Absent outcomes
while eliminating the redundant scan.
🪄 Autofix (Beta)
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: 466f713f-8f60-4b18-8727-8fb7d2df70c2
📒 Files selected for processing (3)
crates/cmtraceopen-parser/src/intune/download_stats.rscrates/cmtraceopen-parser/src/intune/event_tracker.rscrates/cmtraceopen-parser/src/intune/guid_registry.rs
Owner
Author
|
Exact-head correction pushed at |
Owner
Author
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested='9feb4451e1467d2dc3eab0c5c5cfcb24be7227db'
actual="$(git rev-parse HEAD)"
printf 'HEAD=%s\nREQUESTED=%s\n' "$actual" "$requested"
git status --short
git cat-file -e "${requested}^{commit}"
base="$(git merge-base "$requested" origin/main 2>/dev/null || git merge-base "$requested" main)"
printf 'BASE=%s\n' "$base"
printf '\n== Feature range ==\n'
git diff --name-status "$base" "$requested"
git diff --stat "$base" "$requested"
printf '\n== Changed-module outlines ==\n'
for file in \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
do
printf '\n--- %s ---\n' "$file"
ast-grep outline "$file" --items all
done
printf '\n== Feature diff: relevant modules ==\n'
git diff --unified=35 "$base" "$requested" -- \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Identity-context and enrichment definitions/call sites ==\n'
rg -n -C 8 \
'explicit_app_identity_context|ExplicitAppIdentityContext|ExplicitAppIdentity|enrich_event_name_with_name|resolve.*name|named.*fallback|AppWorkload|Sidecar' \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Retry-state definitions/transitions ==\n'
rg -n -C 10 \
'retry|Retry|failed|Failed|content_id|content_id|suppres' \
crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '\n== Feature tests ==\n'
rg -n -C 4 \
'#\[test\]|retry|concurrent|selected.*object|explicit.*identity|fallback|AppWorkload|sidecar|ambiguous|duplicate' \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Diff whitespace check ==\n'
git diff --check "$base" "$requested"Length of output: 202561 🏁 Script executed: #!/bin/bash
set -euo pipefail
requested='9feb4451e1467d2dc3eab0c5c5cfcb24be7227db'
printf 'HEAD=%s\n' "$(git rev-parse HEAD)"
printf '\n== Exact-head correction ==\n'
git show --format=fuller --stat "$requested"
git diff --unified=70 "${requested}^" "$requested" -- \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Download retry control flow ==\n'
nl -ba crates/cmtraceopen-parser/src/intune/download_stats.rs | sed -n '112,245p'
nl -ba crates/cmtraceopen-parser/src/intune/download_stats.rs | sed -n '380,570p'
printf '\n== Identity scan and selection control flow ==\n'
nl -ba crates/cmtraceopen-parser/src/intune/guid_registry.rs | sed -n '120,235p'
nl -ba crates/cmtraceopen-parser/src/intune/guid_registry.rs | sed -n '560,805p'
printf '\n== Ordinary-event extraction and enrichment ==\n'
nl -ba crates/cmtraceopen-parser/src/intune/event_tracker.rs | sed -n '471,575p'
printf '\n== AppWorkload and sidecar extraction and enrichment ==\n'
nl -ba crates/cmtraceopen-parser/src/intune/event_tracker.rs | sed -n '570,760p'
printf '\n== All registry enrichment call sites ==\n'
rg -n -C 12 \
'\.enrich_event_name\(|enrich_event_name_with_name\(|\.resolve_fallback_name\(|registry\.resolve\(' \
crates/cmtraceopen-parser/src/intune
printf '\n== Relevant exact test bodies ==\n'
nl -ba crates/cmtraceopen-parser/src/intune/download_stats.rs | sed -n '995,1315p'
nl -ba crates/cmtraceopen-parser/src/intune/event_tracker.rs | sed -n '2085,2440p'
nl -ba crates/cmtraceopen-parser/src/intune/guid_registry.rs | sed -n '1410,1878p'
printf '\n== All production callers of bounded context functions ==\n'
rg -n -C 4 \
'explicit_app_identity_context(?:_with_named_guid_fallback)?\(' \
crates/cmtraceopen-parser/src \
-g '*.rs'Length of output: 29202 🏁 Script executed: #!/bin/bash
set -euo pipefail
printf '\n== Download retry control flow ==\n'
sed -n '112,245p' crates/cmtraceopen-parser/src/intune/download_stats.rs
sed -n '380,570p' crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '\n== Identity scan and selection control flow ==\n'
sed -n '120,235p' crates/cmtraceopen-parser/src/intune/guid_registry.rs
sed -n '560,805p' crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Ordinary-event extraction and enrichment ==\n'
sed -n '471,575p' crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '\n== AppWorkload and sidecar extraction and enrichment ==\n'
sed -n '570,760p' crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '\n== All registry enrichment call sites ==\n'
rg -n -C 12 \
'\.enrich_event_name\(|enrich_event_name_with_name\(|\.resolve_fallback_name\(|registry\.resolve\(' \
crates/cmtraceopen-parser/src/intune
printf '\n== Focused download tests ==\n'
sed -n '995,1315p' crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '\n== Focused event tests ==\n'
sed -n '2085,2440p' crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '\n== Focused identity tests ==\n'
sed -n '1410,1878p' crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== All production bounded-context callers ==\n'
rg -n -C 4 \
'explicit_app_identity_context(?:_with_named_guid_fallback)?\(' \
crates/cmtraceopen-parser/src \
-g '*.rs'
printf '\n== Changed range whitespace check ==\n'
base="$(git merge-base HEAD origin/main 2>/dev/null || git merge-base HEAD main)"
git diff --check "$base" HEADLength of output: 143258
I verified the full feature range and the exact-head correction:
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/intune/guid_registry.rs`:
- Around line 43-47: Document the fallback_app_id field in
ExplicitAppIdentityContext with its contract: it is populated only for
ExplicitAppIdentity::Absent when the caller enables the legacy named-GUID
fallback, and remains None for Valid and Invalid identities.
🪄 Autofix (Beta)
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: ce6d612f-6436-4f6c-a3cb-3b31a04f1a9c
📒 Files selected for processing (3)
crates/cmtraceopen-parser/src/intune/download_stats.rscrates/cmtraceopen-parser/src/intune/event_tracker.rscrates/cmtraceopen-parser/src/intune/guid_registry.rs
Owner
Author
|
@coderabbitai review Please review exact head |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested='6320011b'
resolved="$(git rev-parse "${requested}^{commit}")"
head="$(git rev-parse HEAD)"
printf 'REQUESTED=%s\nRESOLVED=%s\nHEAD=%s\n' "$requested" "$resolved" "$head"
git status --short
git cat-file -e "${resolved}^{commit}"
base="$(git merge-base "$resolved" origin/main 2>/dev/null || git merge-base "$resolved" main)"
printf 'MERGE_BASE=%s\n' "$base"
printf '\n== Feature-range files ==\n'
git diff --name-status "$base" "$resolved"
git diff --stat "$base" "$resolved"
printf '\n== Changed Intune module outlines ==\n'
for file in \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
do
if git diff --quiet "$base" "$resolved" -- "$file"; then
continue
fi
printf '\n--- %s ---\n' "$file"
ast-grep outline "$file" --items all
done
printf '\n== Full feature diff for relevant modules ==\n'
git diff --unified=45 "$base" "$resolved" -- \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Relevant definitions and call sites ==\n'
rg -n -C 8 \
'explicit_app_identity_context|ExplicitAppIdentityContext|ExplicitAppIdentity|enrich_event_name_with_name|resolve_fallback_name|named.?fallback|AppWorkload|sidecar|retry|content_id|suppres' \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Whitespace check ==\n'
git diff --check "$base" "$resolved"Length of output: 202039 🏁 Script executed: #!/bin/bash
set -euo pipefail
requested="$(git rev-parse '6320011b^{commit}')"
previous='9feb4451e1467d2dc3eab0c5c5cfcb24be7227db'
printf 'HEAD=%s\nREQUESTED=%s\nPREVIOUS=%s\n' "$(git rev-parse HEAD)" "$requested" "$previous"
printf '\n== Correction-range commits ==\n'
git log --format='%H%n%s%n%b%n' "${previous}..${requested}"
printf '\n== Correction-range diff ==\n'
git diff --unified=80 "$previous" "$requested" -- \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Event extraction implementations ==\n'
sed -n '450,690p' crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '\n== Bounded-context callers outside tests ==\n'
rg -n -C 7 \
'explicit_app_identity_context(?:_with_named_guid_fallback)?\(' \
crates/cmtraceopen-parser/src \
-g '*.rs' \
-g '!**/tests/**'
printf '\n== Named-fallback counter tests ==\n'
rg -n -A55 -B10 \
'named_guid_fallback_extraction_count|reset_named_guid_fallback_extraction_count' \
crates/cmtraceopen-parser/src/intune/download_stats.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/guid_registry.rs
printf '\n== Correction-range whitespace check ==\n'
git diff --check "$previous" "$requested"Length of output: 47399
I reviewed the full feature range and the correction range.
✅ 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 (1)
crates/cmtraceopen-parser/src/intune/guid_registry.rs (1)
922-928: 🎯 Functional Correctness | 🟡 Minor | ⚡ Quick winValidate the full GUID shape before stripping the suffix.
The suffix check accepts any 36-character string containing only hexadecimal characters and dashes. This accepts malformed values such as 36 dashes or invalid dash positions.
enrich_event_name_with_namecan then remove non-GUID text and rewrite the event name. Reuseexact_guid(inner).is_some()or enforce the same8-4-4-4-12shape asguid_re(). (raw.githubusercontent.com)Proposed fix
- let is_full_guid = - inner.len() == 36 && inner.chars().all(|c| c.is_ascii_hexdigit() || c == '-'); + let is_full_guid = exact_guid(inner).is_some();🤖 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/intune/guid_registry.rs` around lines 922 - 928, Update enrich_event_name_with_name and the underlying strip_short_guid_suffix validation to require a complete GUID before stripping the suffix, using exact_guid(inner).is_some() or the established guid_re() 8-4-4-4-12 shape. Preserve rewriting only for valid GUID suffixes and leave malformed suffixes 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/intune/guid_registry.rs`:
- Around line 922-928: Update enrich_event_name_with_name and the underlying
strip_short_guid_suffix validation to require a complete GUID before stripping
the suffix, using exact_guid(inner).is_some() or the established guid_re()
8-4-4-4-12 shape. Preserve rewriting only for valid GUID suffixes and leave
malformed suffixes unchanged.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro Plus
Run ID: fa5a69a2-067f-40b1-8da0-01fbdbeaaca5
📒 Files selected for processing (1)
crates/cmtraceopen-parser/src/intune/guid_registry.rs
Owner
Author
|
Verified the new exact-head CodeRabbit finding before changing code. It was valid: the previous suffix check accepted any 36-character hex/dash string, including malformed dash placement, and could rewrite a non-GUID event-name suffix. Addressed test-first at
Verification on exact head
@coderabbitai review |
🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -euo pipefail
requested="$(git rev-parse '477caa2c^{commit}')"
head="$(git rev-parse HEAD)"
printf 'REQUESTED=%s\nHEAD=%s\n' "$requested" "$head"
git status --short
git cat-file -e "${requested}^{commit}"
base="$(git merge-base "$requested" origin/main 2>/dev/null || git merge-base "$requested" main)"
printf 'MERGE_BASE=%s\n' "$base"
printf '\n== Exact-fix commit ==\n'
git show --format=fuller --stat "$requested"
git diff --unified=80 "${requested}^" "$requested" -- \
crates/cmtraceopen-parser/src/intune/guid_registry.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '\n== Full feature range for affected modules ==\n'
git diff --unified=35 "$base" "$requested" -- \
crates/cmtraceopen-parser/src/intune/guid_registry.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '\n== Affected module outlines ==\n'
ast-grep outline crates/cmtraceopen-parser/src/intune/guid_registry.rs --items all
ast-grep outline crates/cmtraceopen-parser/src/intune/event_tracker.rs --items all
printf '\n== Validator and enrichment definitions plus production call sites ==\n'
rg -n -C 10 \
'enrich_event_name_with_name|is_exact_guid|exact.*guid|extract_guid|enrich_event_name\(' \
crates/cmtraceopen-parser/src/intune/guid_registry.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs \
crates/cmtraceopen-parser/src/intune/download_stats.rs
printf '\n== Focused suffix-validation tests ==\n'
rg -n -A100 -B10 \
'malformed.*suffix|full.*guid|short.*guid|enrich_event_name_with_name|event.*suffix' \
crates/cmtraceopen-parser/src/intune/guid_registry.rs \
crates/cmtraceopen-parser/src/intune/event_tracker.rs
printf '\n== All production uses of helper ==\n'
rg -n -C 8 'enrich_event_name_with_name\s*\(' crates/cmtraceopen-parser/src -g '*.rs'
printf '\n== Whitespace checks ==\n'
git diff --check "${requested}^" "$requested"
git diff --check "$base" "$requested"Length of output: 183334
The shared helper applies this validation to registry enrichment and explicit object-local enrichment. Therefore, ordinary, AppWorkload, and sidecar event paths use the corrected suffix check. The regression tests cover malformed full-length suffixes, valid full GUID suffixes, and legacy short GUID suffixes. ✅ Action performedReview finished.
|
Contributor
Reviewed the exact head |
Owner
Author
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.
Summary
This is a focused post-merge correction to the Intune batch-two work. It does not change public parser models.
Test-first evidence
190a88e8: retry and event name-laundering regressionsb2b16aad: scoped identity/name context and inherited retry suppressionVerification
cargo test --locked -p cmtraceopen-parsercargo clippy --locked -p cmtraceopen-parser --all-targets --all-features -- -D warningscargo +1.88.0 check --locked -p cmtraceopen-parser --target wasm32-unknown-unknowngit diff --checkReview gates
Summary by CodeRabbit