feat(intune): model configuration policy evidence (#363) - #526
Conversation
From codex/recovery-intune-363-configuration onto origin/main. Identity canonicalization, models, source/event/report classification, reducer with conflict/supersession, findings, redaction, analyze_configuration; 17 scenarios. Verified: focused 22/22, full parser 1965/0, strict clippy clean, wasm32 clean, diff --check clean. Lane-scoped.
- reducer: preserve observation order in terminal_dispositions (distinguish apply-then-remove from remove-then-apply; Contested when ordering unavailable/contradictory) instead of sort+dedup - reducer: Removed disposition + Removed local state now resolves to Removed (not Contradicted) - relocate ordering_is_contradictory helper before mod tests (clippy items-after-test-module) Subagent extraction + fixes; helper-placement repair and gate verification by parent. Full parser 1968/0, strict clippy clean, wasm32 clean, diff-check clean.
…eld (#363) Rebasing onto main picked up the additive event_version field that the Autopilot lane appended to NormalizedWindowsEvent. The one struct literal in this suite now initializes it to None, matching the shared model's documented additive convention. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
📝 WalkthroughWalkthroughAdds a typed Windows Intune configuration analysis pipeline. It resolves identities, projects evidence, reduces observations into snapshots, derives findings, applies deterministic redaction, and validates 17 scenarios. ChangesConfiguration evidence pipeline
Estimated code review effort: 5 (Critical) | ~120 minutes Sequence Diagram(s)sequenceDiagram
participant ConfigurationInput
participant sources
participant reducer
participant rules
participant redaction
ConfigurationInput->>sources: supply normalized events and reports
sources->>reducer: emit ConfigurationObservation values
reducer->>rules: provide ConfigurationSnapshot
rules-->>ConfigurationInput: attach derived findings
ConfigurationInput->>redaction: request redacted snapshot
redaction-->>ConfigurationInput: return deterministic redacted snapshot
Possibly related issues
Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
|
@coderabbitai full review |
✅ Action performedFull review finished. Your included review limit is currently reached under our Fair Usage Limits Policy. This review may still proceed through usage-based billing if eligible. Your next included review will be available in 55 minutes. |
There was a problem hiding this comment.
Actionable comments posted: 11
🤖 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/device/windows/configuration/identity.rs`:
- Around line 29-91: Document every undocumented public API item in
ConfigurationScope, ConfigurationSettingIdentity, and IdentityHints: add concise
Rust doc comments for all enum variants and the fields setting_id, policy_id,
display_name, scope, uri, setting_id, policy_id, and display_name. Keep the
existing behavior and public signatures unchanged.
- Around line 111-112: Normalize the resource-path comparison used by
push_scope_split so URI casing differences do not create separate scope groups;
either lowercase the key there or store a comparison-normalized resource_path
after strip_scope_segment. Preserve the original path if needed for output, and
add a fixture covering differing device/user casing such as Start versus start.
- Around line 272-278: The scope mapping around scope_segment must not map
ConfigurationScope::Unspecified to "Device"; preserve it as unspecified or use a
distinct scope-free identity value so it cannot merge with device-scoped
records. Update the related scope-token handling in the PolicyManager adapter
within the existing source flow to retain missing scope metadata, and add
coverage for a PolicyManager message without scope metadata.
- Around line 204-209: Update csp_uri_regex to exclude a terminal period from
unparenthesized CSP URI captures and to match parenthesized URIs followed by a
final period, while preserving existing comma, punctuation, and end-of-line
delimiters. Add tests covering both “CSP URI: ./Device/.../Node.” and “CSP URI:
(./Device/.../Node).” forms and verify the captured URI is correct.
In `@crates/cmtraceopen-parser/src/intune/device/windows/configuration/mod.rs`:
- Around line 25-31: Update the public pipeline documentation to state that
callers must invoke redacted_configuration_snapshot before exporting results; do
not describe it as the default export produced by analyze_configuration. Keep
the existing analyze_configuration flow and public APIs unchanged unless an
established high-level export path already performs redaction.
In `@crates/cmtraceopen-parser/src/intune/device/windows/configuration/models.rs`:
- Around line 28-38: Mark the public ConfigurationInput struct as
#[non_exhaustive] to allow future input fields without breaking downstream
users, and document the events and reports fields consistently with
generated_at_utc and coverage. Preserve the existing Default-based construction
path or provide builder-style setters so external callers have a supported way
to create instances.
In
`@crates/cmtraceopen-parser/src/intune/device/windows/configuration/redaction.rs`:
- Around line 78-80: Replace the unkeyed FNV computation in redaction_token with
keyed tokenization using a secret that is never included in exported output;
when no secret is available, create a per-export secret so tokens remain stable
only during that export. Update the surrounding export flow to provide and reuse
this secret for each value while preserving the existing redacted token format
where applicable.
- Around line 127-153: Update redact_named_value to detect URI-valued names such
as NodeUri and CspUri, then apply scrub_uri to those values before constructing
IntuneNamedValue. Preserve full redaction for Restricted sensitivity, sensitive
names, and values matching looks_like_identity, while using the scrubbed URI for
otherwise non-redacted URI entries.
In
`@crates/cmtraceopen-parser/src/intune/device/windows/configuration/reducer.rs`:
- Around line 240-285: The local-state resolution around applied_precedes must
use the final disposition in ordered rather than first-index comparisons, so
re-apply cycles reflect the current fact. Update the Removed, Superseded, and
Conflict branches to compare each terminal disposition against the last ordered
record, preserving the documented behavior that a final Applied disposition
falls through to Contested or Applied as explicitly documented. Extend
apply_then_remove_is_removed_but_reverse_observation_is_ordered_by_metadata with
the [Applied, Removed, Applied] and [Removed, Applied, Removed] cases.
In `@crates/cmtraceopen-parser/src/intune/device/windows/configuration/rules.rs`:
- Around line 446-462: Update superseding_source_is_observed to exclude a
replacement when it matches the observation’s own source_id, mirroring the guard
in competing_sources_are_observed. Require a distinct observed source before
returning true, while preserving the existing case-insensitive matching and
empty-replacement behavior.
In
`@crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/expected.json`:
- Around line 16-42: Update the reducer logic in is_superseded to preserve
source-aware supersedence when a record’s SupersededBy source is observed in the
related evidence, rather than classifying the result as contested/contradicted.
Then update the fixture expectations to use the superseded resolution and emit
configuration-superseded, removing the contradictory contention assertions while
retaining the replacement-source evidence.
🪄 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: a2aac0d9-6227-41b0-aabd-f8a3e67b360a
📒 Files selected for processing (63)
crates/cmtraceopen-parser/src/intune/device/windows/configuration/identity.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/mod.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/models.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/redaction.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/reducer.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/rules.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/sources.rscrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/applied-from-csp-event/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/applied-from-csp-event/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/applied-from-csp-event/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/evidence/intune-setting-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/conflict-between-two-sources/evidence/mdm-diagnostic-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/conflict-between-two-sources/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/conflict-between-two-sources/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/csp-terminal-error/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/csp-terminal-error/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/csp-terminal-error/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/duplicate-display-name-different-csp-paths/evidence/mdm-diagnostic-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/duplicate-display-name-different-csp-paths/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/duplicate-display-name-different-csp-paths/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/event-only-evidence/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/event-only-evidence/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/event-only-evidence/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/incomplete-channel-coverage/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/incomplete-channel-coverage/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/incomplete-channel-coverage/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/invalid-offset-and-contradictory-ordering/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/invalid-offset-and-contradictory-ordering/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/invalid-offset-and-contradictory-ordering/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/evidence/intune-setting-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/malformed-mdm-report/evidence/mdm-diagnostic-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/malformed-mdm-report/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/malformed-mdm-report/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/not-applicable-setting/evidence/mdm-diagnostic-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/not-applicable-setting/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/not-applicable-setting/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/removal-rollback/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/removal-rollback/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/removal-rollback/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/evidence/intune-setting-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/evidence/mdm-diagnostic-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/scope-mismatch-device-and-user/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/scope-mismatch-device-and-user/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/scope-mismatch-device-and-user/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/evidence/mdm-diagnostic-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/evidence/mdm-diagnostic-report/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/manifest.jsoncrates/cmtraceopen-parser/tests/intune_windows_configuration.rs
Charter review (cmtraceopen-code-review): three layers, findings verifiedReview per Gate states (observed, not asserted)
Findings, most severe first1. Redaction violates ADR-004 on five fronts (Major, triple-sourced). (a) 2. The two headline correlation scenarios pin the wrong story (Major, verified). 3. No direction-aware assessability gate (Major — the Autopilot class). A 404 CommandFailure whose 4. Lifecycle reconciliation uses first-occurrence 5. Scope fabrication joins user records into device transactions (Major). 6. Untyped 7. Order dependence with no permutation test (Major, ADR-003). 8. Duplicate observation escalates a clean lifecycle to Error (Minor). Re-collecting the same record (same 9. Conflict gate compares source ids byte-exact (Minor). Everywhere else is case-insensitive; one GUID in two casings/brace forms becomes "two competing sources" at Warning/High. 10. Minors: CodeRabbit thread dispositionsThreads 2, 3, 4, 7, 8, 9, 10, 11 are confirmed and folded into findings above. Thread 1 (document public variants) — valid maintenance, fold into the fix round. Thread 5 (doc contract wrong about default export) — confirmed by mechanical layer, fold in. Thread 6 ( Defended well (adversarial layer verified)Display-name never joins identities; per-scope transaction split works; no time-proximity correlation anywhere; unreliable timestamps block recency reasoning; cloud-vs-local contradictions cite both sides; unknown event ids/schemas stay non-terminal; unidentified records quarantine in Coverage statementCovered: full configuration module + test suite + fixture corpus + CodeRabbit thread triage, against ADR-001..004, the 13-question reducer checklist, and the adversary attack surface. Not covered: native adapter design (deliberately out of the pure lane), runtime performance profiling, and the Merge readiness is reported as the gate states above; merging is Adam's decision. Per the charter, this review changes no code — the fix round is a separate action. 🤖 Generated with Claude Code |
… choke point (#363) Charter review finding 1, and CodeRabbit threads 5, 7, 8. Findings and coverage bypassed redaction entirely: a finding summary is built from `label()`, which is the canonical URI, so a node path embedding an identity reached the export through prose while the same path was scrubbed in the setting beside it. Coverage `detail` passed through untouched. The old test pinned that leak in place by comparing whole findings before and after redaction. - Every field now leaves through `RedactionScope`: `token` for a classified value, `free_text` for prose, `uri` for a node path. The projection is built with a struct literal rather than clone-and-mutate, so a new snapshot field is a compile error here instead of a silent leak. - Token equality is scoped to one analysis (salted with `generatedAtUtc`). ADR-004 records the token algorithm and equality scope as provisional and forbids a new reducer from introducing a cross-export correlation token; no keyed-token API is invented, that stays the Store pilot's call and is documented as such. - Identity detection is case-insensitive, and identity spans are tokenized on their folded form, because the dedupe key is lowercased while `canonicalUri` keeps CSP casing. A SID was tokenized in one and shipped verbatim in the other. - `IntuneSensitivity::Sensitive` is handled instead of falling through: its free text is replaced wholesale, keeping only the identifiers the diagnosis is built from. - URI-valued named entries are scrubbed segment-wise rather than left intact. - Redaction is idempotent, using the compliance sibling's full-shape token check so a value that merely wears brackets cannot skip masking. - Free-text spans consume the family-owned `apps::windows::common::redact_text` rather than a second local whole-value predicate. Changed expectation: `redaction_preserves_the_diagnosis_and_is_deterministic` compared whole findings, which asserted that prose containing a raw node path must survive redaction unchanged. That is the leak, not the contract. ADR-004's invariant is that redaction does not alter a *conclusion*, so the test now pins the identifier, severity, confidence, and citations, and two new tests assert no SID or UPN reaches exported prose and that re-projecting is a no-op. The `deterministic-redaction` fixture gains a node whose path embeds an identity segment, which is the shape the previous corpus could not reach. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…vidence reports actually emit (#363) Charter review findings 2, 4, and 8, and CodeRabbit threads 9, 10, 11. The two flagship scenarios never reached `configuration-conflict` or `configuration-superseded`. `applied_precedes` required the *losing* policy to also emit an `Applied` row, which real MDM report snapshots do not do, so both fixtures pinned `contested`/`contradicted` with an empty findings list while their `assertions` prose claimed a conflict finding cited the competing rows. Swapping the two rows' record numbers flipped the diagnosis between contested/Error and conflicted/Warning. - Explicit linkage now settles the node first: a losing row carrying `WinningPolicyId` or `SupersededBy`, with that source present in the bundle, is the ADR-002 explicit shared key and outranks any reading of record order. A row naming itself substantiates nothing. - Failing an explicit link, a lifecycle is read from the *last* terminal disposition rather than a first-index comparison. `[Applied, Removed, Applied]` is a re-application; `[Removed, Applied]` is an unassign-then-reassign, not a disagreement. Only lifecycle dispositions participate: a `Rejected` mixed with an `Applied` stays contested, because ADR-003 forbids letting a later-looking success replace a failure without explicit retry linkage. - Records are deduplicated by artifact and ordinal *before* the comparability guard. Re-collecting one channel, or collecting it twice under two artifact ids, was degrading a clean `configuration-removed` (Info) into contested device evidence (Error) — duplication making the diagnosis worse. - Source identifiers compare case- and brace-insensitively, so one policy in two casings is not two competing sources. Changed fixture expectations, with why the prior one was unsafe: - `conflict-between-two-sources`: `contested`/`contradicted` and no conflict finding -> `conflicted`/`conflicted` and `configuration-conflict`. The old expectation pinned a diagnosis that depended on which row the report happened to list first and discarded the `WinningPolicyId` the evidence supplied. - `superseded-by-replacement-source`: same change to `superseded`. `Contradicted` is documented as a device-versus-service disagreement, and this bundle has no service evidence at all, so the old value was not reachable from its own definition. - Both now pin `findingEvidence`, so the citation claim is checked rather than asserted in prose. The harness gap itself is closed two ways: an assertion that claims a citation must pin `findingEvidence`, and a new test renumbers a correlated scenario's rows and requires the diagnosis and findings to be unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…verage (#363) Charter review finding 3, the Autopilot class. An 813 "set policy" plus a 404 command failure whose `Result:` token cannot be read, for the same node, reported `configuration-applied` at High confidence with no mention of the failure event at all. `local_state` consulted indeterminate records only when the terminal set was empty, `has_uninterpretable_evidence` gated no conclusion, and per-record `access_state` was populated and read nowhere. - A command-failure record whose status is unreadable is now tracked as an unassessable *failure*: direction survives even when detail does not. It blocks a terminal success for the same node and is reported by its own rule, `configuration-unassessable-failure`, rather than borrowing the contested- evidence sentence, which would have claimed two terminal outcomes where one record states none. - `access_state` is load-bearing: a record the collector could not fully read states no outcome, because the fields this analyzer keys on may be exactly the ones that were never read. - `push_applied` excludes settings carrying uninterpretable or unassessable- failure evidence, and drops to Medium confidence when the bundle has coverage gaps, naming them. Success is the claim a gap undermines: it rests partly on not having seen a failure, and a capped or unreadable artifact is where an unseen failure would be. A rejection is directly evidenced and keeps High. Changed fixture expectations, with why the prior one was unsafe: - `incomplete-channel-coverage` stated a High-confidence success from a bundle whose channel was capped, whose diagnostic report was permission-denied, and whose registry artifact was missing. ADR-001's invariant is that coverage gaps cannot raise confidence; High there asserted the analyzer had seen enough when three artifacts say it had not. It is now Medium, and so is every other scenario whose bundle is incomplete. - Confidence was never pinned by the corpus at all, which is why the previous value survived review. `expected.json` now carries `findingConfidence` and the harness compares it, so a confidence change is a fixture diff rather than an invisible one. Per-setting `hasUnassessableFailure` is pinned for the same reason. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…providers (#363) Charter review finding 5, and CodeRabbit thread 4. Two ways a record could join a transaction it never claimed membership of. - `observation_from_event` defaulted a missing scope token to `Device` and `policy_uri_from_message` then synthesized a joinable `Device/Vendor/MSFT/…` path. That contradicted this module's own contract — an unknown scope must not merge with a known one, because a device-scoped and a user-scoped instance of one CSP node are different settings. An unspecified scope now keeps an unspecified key and a scope-free path. Nothing about the node is lost: the scope-free path still shares a `resource_path` with the scoped instances, which is what the scope-split rule compares. - Message scraping was not provider-gated, so `CSP URI: (…)` inside any other component's rendered text joined a real MDM transaction. It is now gated the same way event ids already are: another provider's prose is that provider's vocabulary. Structured named data stays trusted whoever wrote it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ata (#363) Charter review finding 6, and CodeRabbit thread 10. ADR-002 forbids promoting an untyped local field into shared semantic authority, and three places did it. - `Command: Delete` beside a typed `Applied` outcome turned a stated application into a removal. The typed outcome — the report's `outcome`, or the event id via `ConfigurationEventKind` — is now the only thing that decides a disposition. The command verb is still carried on the observation as evidence. - A single row carrying `WinningProvider: GPO` substantiated `configuration-conflict` at Warning/High with one source in the bundle and the alleged winner nowhere in it. Naming a source is not observing one: the named winner must itself have stated something about the node, otherwise the finding is the unsubstantiated variant that asks for the missing artifact. - `superseding_source_is_observed` had no self-reference guard, so a row naming itself as its own replacement matched trivially. It now mirrors the conflict rule. Source identifiers compare normalized throughout, so one policy written braced and upper by the event provider and bare and lower by the report is one source rather than two competing ones. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rder (#363) Charter review finding 7. `applied_value`, `terminal_error`, and every merged identity field were first-wins over the caller's vector. Two sources applying different values exported whichever came first; two 404s flipped which HRESULT the rejection finding quoted; the merged `display_name` followed serializer order and could turn `configuration-duplicate-display-name` on and off over identical evidence. The determinism test re-ran the *same* vector, which cannot see any of that. - Observations are sorted into a canonical order by evidence reference before anything reads them, so neither the reduction nor the exported vector carries the caller's order. ADR-003 permits caller order as chronology only where the source contract defines it, and no configuration source does; real order is read from record ordinals. - Two device records applying different values export no value rather than an arbitrary one. The conflict rules already name the sources arguing over it. - Two different terminal codes resolve to the smallest by canonical ordering. Both records stay cited, so only the choice of which to lead with is stabilized. - Merged identity fields take the smallest stated value; the three URI views move as a unit so a canonical URI from one record cannot be paired with a raw URI from another. The sibling lanes' permutation pattern is restored: every ordering of a bundle's sibling records must produce a byte-identical snapshot, plus targeted tests for the disagreeing-value, disagreeing-code, merged-name, and duplicated-record cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…bit threads (#363) Charter review findings 9 and 10, and CodeRabbit threads 1, 2, and 3. - `IntuneSourceKind::Json` no longer carries device authority. The kind describes the encoding, not the origin: a portal export and a device-side agent state file are both JSON, and granting it device authority let an exported portal view resolve a setting the CSP was never shown to have applied. - `resolve(Removed, ReportedSuccess)` hard-coded `Removed` on the assumption that Intune reports a completed delete as a success. Nothing documents that vocabulary, and ADR-003 requires an unresolved authoritative contradiction to stay conservative rather than pick a winner, so it is now `Contradicted` and both statements are cited. If the reporting vocabulary is ever written down, that is one line. - `service_error` was populated and cited by no rule at all. The contradiction finding now quotes both sides' status codes, which is the whole point of putting the two statements side by side. - A new `configuration-artifact-without-usable-records` reports an artifact that was collected in full and produced nothing this build could turn into an outcome. `available` otherwise reads as "we looked and there was nothing wrong"; the honest statement is that the artifact answered no question. The `unknown-report-schema` scenario reaches it. - `push_scope_split` grouped on the exact `resource_path` bytes while the dedupe key is lowercased, so a device `Start/…` and a user `start/…` never met and the scope split went unreported (thread 2). - The CSP URI pattern handles a sentence-ending period in both the bare and parenthesized forms; one used to capture the period into the node path and the other matched nothing (thread 3). - Public variants and fields on `ConfigurationScope`, `ConfigurationSettingIdentity`, and `IdentityHints` are documented (thread 1). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…363) CodeRabbit thread 1, extended past the two types it named to the rest of the module's public surface: `ConfigurationInput`'s `events` and `reports`, every `ConfigurationObservation` field, and every variant of `ConfigurationDisposition`, `ConfigurationLocalState`, `ConfigurationServiceState`, and `ConfigurationResolution`. The comments say what a field is *for* rather than restating its name, so the load-bearing ones are recorded where a change would have to notice them: that `command_type` is evidence and never decides a disposition, that `generated_at_utc` is also the redaction token's scope, and that `observations` is in a canonical order rather than the caller's. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
There was a problem hiding this comment.
Actionable comments posted: 1
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/device/windows/configuration/reducer.rs (1)
520-529: 🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Contestedlocal evidence resolves toContradicted, which the public doc defines as a device/service disagreement.Line 527 maps
ConfigurationLocalState::ContestedtoConfigurationResolution::Contradicted. That path fires when only device records disagree andserviceisNoEvidence.ConfigurationResolution::Contradictedis documented inmodels.rs(Line 273) as "Device and service evidence state incompatible outcomes." A consumer reading the serializedcontradictedvalue will conclude that Intune reporting was present and disagreed, which is false for device-only contested settings.The rule layer already separates the two cases (
push_local_service_contradictionexcludeslocal == Contested,push_contested_device_evidencehandles it), so only the exported enum contract is wrong. Correct the variant doc to cover both shapes, or add a distinct resolution for device-internal disagreement.📝 Proposed doc correction in models.rs
- /// Device and service evidence state incompatible outcomes. + /// Two authoritative statements disagree: either device and service + /// evidence state incompatible outcomes, or device-side records contest + /// one another. See [`ConfigurationSetting::local`] to tell them apart. Contradicted,As per path instructions: "Treat every public item as a semver commitment: flag breaking changes to public types, signatures, or enum variants, and check that new public items are documented."
🤖 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/device/windows/configuration/reducer.rs` around lines 520 - 529, Update the public documentation for ConfigurationResolution::Contradicted in models.rs to describe both device/service incompatibility and device-internal contested evidence. Preserve the existing Contested-to-Contradicted mapping in the local_conclusion match and do not add a new public enum variant.Source: Path instructions
🤖 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/tests/intune_windows_configuration.rs`:
- Around line 1542-1554: Update the test around analyze_configuration to assert
that snapshot.settings contains exactly one setting before accessing
snapshot.settings.first(). Keep the existing evidence and unattributed
assertions unchanged so an additional transaction created from the foreign
record causes the test to fail.
---
Outside diff comments:
In
`@crates/cmtraceopen-parser/src/intune/device/windows/configuration/reducer.rs`:
- Around line 520-529: Update the public documentation for
ConfigurationResolution::Contradicted in models.rs to describe both
device/service incompatibility and device-internal contested evidence. Preserve
the existing Contested-to-Contradicted mapping in the local_conclusion match and
do not add a new public enum variant.
🪄 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: ca0f8de1-b39a-41e8-bc98-0c6783eb3eb3
📒 Files selected for processing (27)
crates/cmtraceopen-parser/src/intune/device/windows/configuration/identity.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/mod.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/models.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/redaction.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/reducer.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/rules.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/sources.rscrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/applied-from-csp-event/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/conflict-between-two-sources/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/csp-terminal-error/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/evidence/mdm-admin-channel/current/records.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/manifest.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/duplicate-display-name-different-csp-paths/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/event-only-evidence/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/incomplete-channel-coverage/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/invalid-offset-and-contradictory-ordering/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/malformed-mdm-report/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/not-applicable-setting/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/removal-rollback/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/scope-mismatch-device-and-user/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/expected.jsoncrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/expected.jsoncrates/cmtraceopen-parser/tests/intune_windows_configuration.rs
|
Hermes charter review Scope and method Reviewed PR #526 at exact head 3a0b174, using the charter load order from main and the required three layers: contract (ADR-001..004, reducer checklist, contract charter), adversarial (identity/ordering/coverage/conflict/redaction attack surface), and mechanical (branch code, focused tests/fixtures, exact-head GitHub gates). Branch files were inspected with git show only; no checkout, edits, thread resolution, or merge was performed. Verdict: CHANGES REQUESTED — one contract-layer blocking finding remains. Findings, ranked
Location: crates/cmtraceopen-parser/src/intune/device/windows/configuration/redaction.rs:145-166, 274-282; models.rs:352-356. The fix says the salt is the snapshot's generatedAtUtc and claims “no two exports share it.” The input contract does not enforce uniqueness, and generatedAtUtc is a timestamp, not a nonce or analysis identifier. Two independently generated exports can legitimately have the same second (the fixtures already use second-resolution UTC values), producing the same salt and therefore the same [redacted:...] token for a candidate value. An attacker can then join those exports despite the stated ADR-004 boundary. This is a concrete false privacy/correlation claim, not merely a cryptographic-strength concern. Disposition: keep the per-analysis equality boundary, but bind it to an actually unique analysis scope (or make the scope an explicit caller-supplied contract) and test two distinct analyses with identical generatedAtUtc. Do not describe generatedAtUtc alone as preventing cross-export joins.
The lane-local scope emits [redacted:...] for classified values and identity spans; family redact_text continues to emit [user:...] and [command:...]. The current implementation routes free text through the family helper after local SID/UPN masking, and the tests cover idempotence/equality within the lane. I adjudicate this as acceptable for this PR: [user:] and [command:] represent family-owned path/command masking, while [redacted:] represents this lane's analysis-scoped equality. They must not be treated as interchangeable tokens or promised to preserve equality across those representations. A future shared grammar change belongs to the family/framework owner, not a local compatibility alias.
The salt travels with the export, so it is not a keyed commitment and does not resist enumeration within one export. The PR documents that limitation accurately. ADR-004 leaves caller-controlled keying and equality scope provisional and assigns that decision to the Store/framework pilot. I therefore adjudicate the lane's decision not to invent a keyed-token API as correct, subject to finding 1: the chosen per-analysis scope must actually be unique. Contract-owner adjudication: Removed + ReportedSuccess The change from Removed to Contradicted is CORRECT. There is no documented contract in the loaded repository establishing that an Intune service “success” means a completed delete, and the service state explicitly models ReportedSuccess rather than a service-side Removed vocabulary. ADR-003 requires unresolved authoritative contradictions to remain conservative rather than choose an arbitrary winner. The prior Removed behavior had no documented basis. The retained test and both-side evidence are the right disposition. If Microsoft reporting semantics are later documented, that is a contract change, not a local precedence assumption. Contract-layer coverage The applicable checklist is satisfied for evidence/ confidence gating, explicit identity boundaries, caller-order independence, time-only prohibition, duplicate handling, family isolation, coverage gaps, citation membership, unsupported evidence, and redaction conclusion preservation, except for the uniqueness defect above. Native Windows adapters are explicitly out of scope for this pure parser lane and were not treated as a finding. Adversarial layer The fix round closes the previously reported attacks around explicit conflict/supersedence linkage, unreadable failure direction, scope fabrication, untyped command authority, permutation dependence, duplicate escalation, source-ID normalization, JSON authority, barren artifacts, and redaction leaks/idempotence. The remaining concrete attack is: generate two exports with the same generatedAtUtc and compare a known candidate value; the lane's tokens join them. Mechanical layer The branch's PR-body focused/full parser, Clippy, workspace check, wasm32, diff-check, and TypeScript claims are consistent with the fix-round description, and the GitHub Rust/CodeQL/TypeScript/E2E/MSRV checks observed at head passed. No local branch checkout or code execution against a changed worktree was performed, per the requested read-only branch-inspection constraint. The current GitHub state has Rust CI passed, CodeRabbit check completed successfully, but the latest CodeRabbit review state is COMMENTED rather than APPROVED; ESP Diagnostics (Windows) remains IN_PROGRESS. Thus CodeRabbit approved_at_head is FALSE and native/Windows CI is not complete. Gate states
Disposition of prior review feedback The ten prior finding clusters are not re-reported as open defects: the fix round's code/tests address them as described and verified in the branch inspection. The Removed/ReportedSuccess change is affirmed. The keyed-token question and two-vocabulary question are adjudicated above. #[non_exhaustive] remains correctly rejected as a family-wide change rather than a lane-local exception. Coverage statement Covered: main charter/routing/agent context, ADR-001..004, reducer checklist and staff charters, all changed configuration module files, focused test harness and fixture matrix, the eight fix-round commits, PR body, CodeRabbit state, and GitHub gate state at 3a0b174. Not covered: native adapter behavior (not in this PR), live Windows semantic acceptance, performance profiling, or cryptographic suitability beyond the ADR-004 scope/equality contract. Next: resolve the analysis-scope uniqueness contract/test, obtain CodeRabbit APPROVED at head, and wait for ESP Diagnostics (Windows) to complete before reconsidering merge readiness. |
… analysis scope (#363) The per-analysis token scope was salted with the snapshot's `generatedAtUtc` and the docs claimed no two exports could share it. That claim was false: a second-resolution UTC instant is not a nonce, two independent analyses can legitimately carry the same one (the fixtures already do), and when they did they minted identical `[redacted:...]` tokens for identical values. The cross-export join ADR-004 forbids therefore still worked. The scope is now `ConfigurationInput::analysis_scope`, supplied by the caller, because the caller is the only party that can know analysis identity: this crate is pure and wasm32-clean, with no clock, no entropy, and no state surviving a restart from which to mint one. The reducer digests it onto `ConfigurationSnapshot::analysis_scope`, so the caller's own identifier never reaches the export and the export can still name the scope its tokens belong to. The docs now state exactly what the scope guarantees (equality within one scope, no shared token between two differently-scoped analyses) and what it does not (uniqueness is the caller's obligation, an omitted scope buys no isolation at all, the token is still unkeyed and no defense against enumeration inside one export). Tests: a RED unit test pins the reported defect (two analyses, identical generatedAtUtc, no shared token) alongside the within-analysis equality it must not break; contract tests pin the same boundary end to end on a real scenario, plus the omitted-scope disclaimer. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Accepted in full, fixed in Scope chosen: (a) an explicit caller-supplied scope on the input contract. Why (a) and not the other two. The caller is the only party that can own analysis identity here. This crate is pure and
What the docs now claim, and what they disclaim. The module doc has explicit guarantees and does not guarantee sections rather than one prose sentence:
The digest, not the caller's raw scope value, is what travels: a caller is free to use a serial number or a case title as its scope, and this module exists to keep that kind of string out of an exported artifact. The digest is documented as a digest, not a secret — anyone who knows the scope material can recompute it. TDD. RED first, at the level of the defect: Green after the fix. Your named test — two distinct analyses with identical Gates at Not merged and not marked ready, per your gate states — CodeRabbit approval at the new head and the Windows job are still outstanding, and merging remains Adam's action. |
|
@coderabbitai review |
✅ Action performedReview finished.
|
Hermes charter re-review (head ef9e199)VerdictThe single blocking P1 from head I reviewed the exact delta
Verification
Gate states observed at exact head
This review covered the requested redaction-scope fix, its public/module contract, exact-head regression coverage, and the pure-crate boundary. It did not claim native Windows validation or override the still-pending CI/CodeRabbit gates. No files were changed, no threads were resolved, and nothing was merged. |
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/device/windows/configuration/redaction.rs`:
- Around line 195-205: Update analysis_scope_digest to use trimming only to
detect whitespace-only values, while hashing the original nonblank
analysis_scope bytes unchanged. Preserve the existing absent-scope behavior and
add a regression test covering Some("analysis-a") and Some(" analysis-a ") with
the same generated_at_utc, asserting their digests differ.
🪄 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: 656e77e1-a89a-4034-abbf-dd60115cd869
📒 Files selected for processing (6)
crates/cmtraceopen-parser/src/intune/device/windows/configuration/mod.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/models.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/redaction.rscrates/cmtraceopen-parser/src/intune/device/windows/configuration/reducer.rscrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/expected.jsoncrates/cmtraceopen-parser/tests/intune_windows_configuration.rs
Hermes's non-blocking wording note on the re-review: the module and field docs said two analyses with different scopes "never" share a token, while the differing-values guarantee two lines above already carried "barring hash collision". Same digest, same qualification - an unqualified never is the overclaiming this fix round exists to stop. Verified: configuration suite 45 passed, clippy -D warnings clean. Refs #363 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
analysis_scope_digest trimmed every nonblank scope before hashing, so "analysis-a" and " analysis-a " produced one digest and their exports could be joined by comparing tokens - the same isolation defect the caller-supplied scope was introduced to close, one normalization step lower. Only a whitespace-only value now counts as no scope; every other value is hashed as given, because the crate cannot know which bytes a caller's identity scheme treats as significant. Also asserts the transaction count in the foreign-provider test, so a scraped record opening its own transaction fails rather than passing on a first()-only check. Verified: scopes_differing_only_in_surrounding_whitespace_stay_distinct observed RED (identical tokens) before the fix; a_whitespace_only_scope_is_no_scope_at_all pins the surviving normalization. Parser suite 2221 passed 0 failed, clippy -D warnings clean, wasm32 check clean. Refs #363 Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
|
@coderabbitai review |
✅ Action performedReview finished.
|
|
@coderabbitai approve |
✅ Action performedComments resolved. Approval is disabled; enable |
There was a problem hiding this comment.
Pull request overview
This PR implements the new pure (wasm-clean) Intune Windows device configuration analysis lane at cmtraceopen_parser::intune::device::windows::configuration, producing a per-setting snapshot that separates receipt, local CSP outcome, and Intune service reporting, plus deterministic redaction and evidence-backed findings.
Changes:
- Added the
intune/device/windows/configurationmodule (identity resolution, typed observation projection, reducer, findings rules, and redaction projection). - Added a focused golden/fixture-driven contract test suite (
intune_windows_configuration.rs) with a full 17-scenario synthetic fixture matrix. - Added synthetic fixture bundles (manifests, evidence records, expected outputs) covering conflicts, supersedence, removals, scope splits, malformed/unsupported evidence, and redaction determinism.
Reviewed changes
Copilot reviewed 63 out of 63 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| crates/cmtraceopen-parser/src/intune/device/windows/configuration/mod.rs | Public module surface + pipeline entry point wiring. |
| crates/cmtraceopen-parser/src/intune/device/windows/configuration/identity.rs | Canonical CSP identity + message scraping helpers. |
| crates/cmtraceopen-parser/src/intune/device/windows/configuration/models.rs | Serialized contract/types for snapshots, observations, and states. |
| crates/cmtraceopen-parser/src/intune/device/windows/configuration/sources.rs | Projection from normalized inputs into typed per-record observations. |
| crates/cmtraceopen-parser/src/intune/device/windows/configuration/reducer.rs | Transaction reducer that folds observations into per-setting state. |
| crates/cmtraceopen-parser/src/intune/device/windows/configuration/rules.rs | Evidence-backed findings derived from snapshots. |
| crates/cmtraceopen-parser/src/intune/device/windows/configuration/redaction.rs | Deterministic redaction projection + token scope logic. |
| crates/cmtraceopen-parser/tests/intune_windows_configuration.rs | End-to-end contract tests over the fixture corpus. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/applied-from-csp-event/manifest.json | Fixture manifest for “applied from CSP event”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/applied-from-csp-event/expected.json | Expected snapshot projection for “applied from CSP event”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/applied-from-csp-event/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event evidence for “applied from CSP event”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/manifest.json | Fixture manifest for “cloud success vs local terminal error”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/expected.json | Expected snapshot projection for “cloud success vs local terminal error”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event evidence for the local failure. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/cloud-success-local-terminal-error/evidence/intune-setting-report/current/records.json | Synthetic normalized service report evidence for the cloud success. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/conflict-between-two-sources/manifest.json | Fixture manifest for “conflict between two sources”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/conflict-between-two-sources/expected.json | Expected snapshot projection for “conflict between two sources”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/conflict-between-two-sources/evidence/mdm-diagnostic-report/current/records.json | Synthetic normalized diagnostic report evidence for conflict linkage. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/csp-terminal-error/manifest.json | Fixture manifest for “CSP terminal error”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/csp-terminal-error/expected.json | Expected snapshot projection for “CSP terminal error”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/csp-terminal-error/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event evidence (404) with rendered message scraping. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/manifest.json | Fixture manifest for deterministic redaction behavior. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/expected.json | Expected redaction token stability/projection assertions. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/deterministic-redaction/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event evidence with sensitive fields/values. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/duplicate-display-name-different-csp-paths/manifest.json | Fixture manifest for duplicate display name across different CSP paths. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/duplicate-display-name-different-csp-paths/expected.json | Expected projection for duplicate display name scenario. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/duplicate-display-name-different-csp-paths/evidence/mdm-diagnostic-report/current/records.json | Synthetic normalized report rows for the duplicate display name case. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/event-only-evidence/manifest.json | Fixture manifest for “event-only evidence”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/event-only-evidence/expected.json | Expected projection for “event-only evidence”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/event-only-evidence/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event evidence without diagnostic report. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/incomplete-channel-coverage/manifest.json | Fixture manifest covering capped/denied/missing artifacts. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/incomplete-channel-coverage/expected.json | Expected projection for incomplete coverage semantics. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/incomplete-channel-coverage/evidence/mdm-admin-channel/current/records.json | Synthetic normalized events used in incomplete coverage scenario. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/invalid-offset-and-contradictory-ordering/manifest.json | Fixture manifest for timestamp offset + ordering contradictions. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/invalid-offset-and-contradictory-ordering/expected.json | Expected projection for unusable time provenance / ordering contradictions. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/invalid-offset-and-contradictory-ordering/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event evidence with invalid offset and record/time inversions. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/manifest.json | Fixture manifest for “local success vs cloud failure”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/expected.json | Expected projection for “local success vs cloud failure”. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event evidence for local applied. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/local-success-cloud-failure/evidence/intune-setting-report/current/records.json | Synthetic normalized service report evidence for reported failure. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/malformed-mdm-report/manifest.json | Fixture manifest for malformed report rows (parse failed). |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/malformed-mdm-report/expected.json | Expected projection for malformed report handling. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/malformed-mdm-report/evidence/mdm-diagnostic-report/current/records.json | Synthetic malformed normalized report row evidence. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/not-applicable-setting/manifest.json | Fixture manifest for “not applicable” terminal disposition. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/not-applicable-setting/expected.json | Expected projection for not-applicable terminal disposition. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/not-applicable-setting/evidence/mdm-diagnostic-report/current/records.json | Synthetic normalized report row for not-applicable outcome. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/removal-rollback/manifest.json | Fixture manifest for removal/rollback via delete event. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/removal-rollback/expected.json | Expected projection for removal lifecycle. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/removal-rollback/evidence/mdm-admin-channel/current/records.json | Synthetic normalized events (applied then deleted). |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/manifest.json | Fixture manifest for “report-only evidence” (no event channel). |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/expected.json | Expected projection for report-only evidence semantics. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/evidence/mdm-diagnostic-report/current/records.json | Synthetic normalized diagnostic report row evidence. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/report-only-evidence/evidence/intune-setting-report/current/records.json | Synthetic normalized imported Intune row evidence. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/scope-mismatch-device-and-user/manifest.json | Fixture manifest for device/user scope split. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/scope-mismatch-device-and-user/expected.json | Expected projection for device/user scope split behavior. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/scope-mismatch-device-and-user/evidence/mdm-admin-channel/current/records.json | Synthetic normalized events covering both scopes. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/manifest.json | Fixture manifest for explicit supersedence linkage. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/expected.json | Expected projection for superseded + replacement presence requirement. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/superseded-by-replacement-source/evidence/mdm-diagnostic-report/current/records.json | Synthetic normalized report evidence for superseded-by linkage. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/manifest.json | Fixture manifest for unknown event/report schema handling. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/expected.json | Expected projection for unknown/unsupported schema degradation. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/evidence/mdm-admin-channel/current/records.json | Synthetic normalized event with unrecognized event id. |
| crates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/unknown-report-schema/evidence/mdm-diagnostic-report/current/records.json | Synthetic normalized report row marked unsupported schema. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
| let scope = analysis_scope.filter(|scope| !scope.trim().is_empty())?; | ||
| let mut material = scope.as_bytes().to_vec(); | ||
| material.push(SCOPE_SEPARATOR); | ||
| material.extend_from_slice(generated_at_utc.as_bytes()); | ||
| Some(format!("{:016x}", fnv1a64(FNV_OFFSET_BASIS, &material))) |
Hermes confirmation (head adb8b05)Clearance carries to head
The added regression tests cover whitespace-differing scopes, whitespace-only absence, and same-scope equality; the PR head also reports all CI checks green. My prior blocking P1 is therefore cleared at this head. This is a confirmation of that clearance, not a merge action. |
Implements
cmtraceopen_parser::intune::device::windows::configuration: a pure, wasm-clean lane that explains whether an Intune/MDM configuration setting was delivered, processed by its CSP, applied, rejected, conflicted, superseded, removed, or left unresolved.What is done
crates/cmtraceopen-parser/src/intune/device/windows/configuration/:identity.rs: canonical CSP/OMA-URI identity (canonicalize_uri,resolve_identity). Settings with the same display name but different CSP paths, policy source IDs, scopes, or configuration sources stay separate transactions.sources.rs: typed observations from normalizedDeviceManagement-Enterprise-Diagnostics-Provider/Adminevents and MDM Diagnostic report / Intune setting-report rows. Preserves OMA-URI, policy source ID, setting name, scope, command/action, result code, activity ID, enrollment ID.models.rs: serialized contract (INTUNE_CONFIGURATION_SCHEMA_VERSION = 1, camelCase, deterministic ordering). The answer keeps three axes separate: receipt (ConfigurationReceiptState), local CSP outcome (ConfigurationLocalState), service reporting (ConfigurationServiceState), then oneConfigurationResolution.reducer.rs: one transaction per canonical identity; cloud reporting is supplemental to local CSP evidence; device/service disagreement resolves toContradictedwith both statements cited, never newest-wins.rules.rs: evidence-backed findings, including explicit next-artifact requests when event/report/registry evidence is missing.redaction.rs: default export redacts synthetic identity, user/device/tenant values, and custom setting payloads deterministically.crates/cmtraceopen-parser/tests/intune_windows_configuration.rs(22 tests) plus 17 fixture scenarios undercrates/cmtraceopen-parser/tests/fixtures/intune/device/windows/configuration/, all synthetic and markedSYNTHETIC FIXTURE, matching the compliance lane's manifest/expected conventions.Fixture matrix (all 17 required scenarios)
applied-from-csp-eventcsp-terminal-errorscope-mismatch-device-and-usernot-applicable-settingconflict-between-two-sourcessuperseded-by-replacement-sourceremoval-rollbacklocal-success-cloud-failurecloud-success-local-terminal-errorreport-only-evidenceevent-only-evidenceduplicate-display-name-different-csp-pathsmalformed-mdm-reportunknown-report-schemaincomplete-channel-coverageinvalid-offset-and-contradictory-orderingdeterministic-redactionShared model impact
NormalizedWindowsEvent/NormalizedSettingReportinsrc/intune/normalized.rswere not modified. This lane only consumes them. Rebasing onto current main picked up the additiveevent_version: Option<u32>field the Autopilot lane appended; the one struct literal in this suite now initializes it toNone(own commit, test-only).Deliberately not done
src-tauriyet, so there is nothing to test. That is native-side work outside the pure-lane scope of this PR and will land with the adapter.Assumptions
tests/fixtures/intune/device/windows/configuration/(the issue body's shorthand omitteddevice/).cargo fmt --check --allcurrently reports pre-existing diffs on files this PR does not touch (local rustfmt drift also present on main); no files owned by this PR are flagged, and none were reformatted.Verification (from the worktree root)
cargo test --locked -p cmtraceopen-parser --test intune_windows_configuration: 22 passed, 0 failedcargo test --locked -p cmtraceopen-parser: 2171 passed, 0 failed across all suitescargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings: cleancargo check --workspace: cleancargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown: cleangit diff --check: cleanCharter fix round
Eight commits addressing the 10 ranked finding clusters from the charter review
(
cmtraceopen-code-review, three layers: contract / adversarial / mechanical) plusall 11 CodeRabbit threads. Every behavioural fix is TDD'd; every changed fixture
expectation states why the previous one was unsafe.
RedactionScopechoke point; per-analysis token scope; case-insensitive identity detection;Sensitivehandled; idempotent; consumes the family'sredact_text1a40c04eWinningPolicyId/SupersededBylinkage (ADR-002) settles the node before any reading of order3b4bfd5daccess_statemade load-bearing; coverage gaps gate High confidence20b29acf3b4bfd5dDevice; unspecified keeps its own key; message scraping provider-gated31085935named_datadrives typed conclusionsCommand; named winner must be observed; self-reference guard0a7dc57bc461fc8f3b4bfd5d0a7dc57bJsonloses device authority;Removed/ReportedSuccessbecomesContradicted;service_errorcited; coverage honesty rule; re-exports pruned; CSP URI period;resource_pathcasing8325b9b8,3a0b174dCharter fix round 2 — Hermes P1
One blocking finding from the Hermes charter review at
3a0b174d: the per-analysistoken scope was salted with
generatedAtUtcand the docs claimed no two exportscould share it. A timestamp is not a nonce — two independent analyses can carry the
same second-resolution instant (the fixtures already do), so identical values minted
identical tokens and the cross-export join ADR-004 forbids still worked.
generatedAtUtcis not a per-analysis identityConfigurationInput::analysis_scope, digested ontoConfigurationSnapshot::analysis_scope; docs restated as a conditional guarantee with explicit disclaimersef9e199dRED first:
two_analyses_that_share_a_generated_at_utc_do_not_share_a_tokenfailedagainst the old scope with
left: "[redacted:50aa2e930aca3c17]" == right: "[redacted:50aa2e930aca3c17]". Green after the fix, alongside four new pins —one_analysis_split_across_two_projections_keeps_its_tokens(within-analysisequality is not collateral damage),
an_omitted_scope_is_documented_as_no_boundary_at_alland
the_scope_digest_does_not_republish_the_callers_scope_value, plus thecontract-level
two_analyses_with_the_same_generated_at_utc_export_no_shared_token,one_analysis_scope_keeps_its_tokens_across_separate_runs, andan_omitted_analysis_scope_makes_no_isolation_claimover a real scenario's wholeexport. The existing within-export equality and idempotence tests are unchanged and
green; the
deterministic-redactionfixture's pinned tokens moved because the saltdid, and nothing else in the corpus changed.
Considered and rejected: a process-generated nonce (option b) — a counter restarts
at zero every run, so in the realistic deployment of one analysis per app launch it
would isolate nothing while costing purity and reproducibility, i.e. exactly the
overclaiming-by-mechanism the finding was about. A composite of
generatedAtUtcplus other snapshot material (option c) — the uniqueness argument would have been
hand-wavy, and the instruction was not to ship it on those terms.
ADR-004 decision, and what is routed rather than answered
ADR-004 records the redaction token algorithm, caller-supplied key, and equality
scope as provisional and assigns them to the Store pilot. This lane therefore
scoped equality per analysis rather than inventing a keyed-token API, and takes
the identity of that analysis from the caller:
ConfigurationInput::analysis_scope.Equality holds within one scope — which is what makes a conflict legible after
redaction — and two analyses that supply different scope values share no token
vocabulary.
redaction_tokenis no longer public; tokens are only reachablethrough a scope a caller cannot construct without an analysis.
The scope is the caller's because nothing else can be: this crate is pure and
wasm32-unknown-unknownclean, with no clock, no entropy source, and no statesurviving a restart from which to mint a unique identifier. The reducer digests the
caller's value onto
ConfigurationSnapshot::analysis_scope, so the caller's ownidentifier (which may be a serial number or a case title) never reaches the export,
while the export can still name the scope its tokens belong to.
The docs state precisely what this does guarantee (equal values inside one
scope produce equal tokens; two differently-scoped analyses never share a token for
the same value) and what it does not (uniqueness is the caller's obligation and
is not checked here; omitting the scope falls back to
generatedAtUtcalone andbuys no isolation at all, which the snapshot reports as
analysisScope: null; thetoken is still unkeyed and no defense against enumeration inside one export).
Routed to the Store pilot / framework owner, not decided here:
not defend against an attacker who can enumerate candidate values and confirm a
match within one export. Closing that needs a caller-supplied secret and a
documented equality scope, which is ADR-004's stated owner.
analysis's scope; user-profile paths and inline credentials keep the family's
globally stable
[user:…]/[command:…]tokens fromapps::windows::common::redact_text. Unifying them is a family-wide change.#[non_exhaustive](CodeRabbit thread 6) — rejected here as a lane change;the family has zero instances and a change should be family-wide or not at all.
Contract changes worth a second look
resolve(Removed, ReportedSuccess)is nowContradictedrather thanRemoved.The old precedence assumed Intune reports a completed delete as a success; no
documented contract says so, and ADR-003 requires an unresolved authoritative
contradiction to stay conservative. This is the one line to change if that
vocabulary is ever written down.
expected.jsongainedfindingConfidenceand per-settinghasUnassessableFailure,both now compared by the harness. Confidence was previously unpinned, which is how
a High-confidence success survived a bundle with three incomplete artifacts.
configuration-artifact-without-usable-records(coverage honesty:an artifact read in full that settled nothing) and
configuration-unassessable-failure(a command failure whose status is unreadable).Vocabulary is 22 entries.
Gates (re-run after every slice)
Re-run at
ef9e199d:cargo test -p cmtraceopen-parser: 2219 passed, 0 failed (45 inintune_windows_configuration, 18 in the redaction unit module)cargo clippy --workspace --all-targets -- -D warnings: cleancargo check --workspace: cleancargo check -p cmtraceopen-parser --target wasm32-unknown-unknown: clean(the fix adds no OS-entropy or clock dependency)
npx tsc --noEmit: clean (no frontend surface touched)All 11 CodeRabbit threads have a reply naming the fix commit or the rejection
reasoning, and are resolved.
Closes #363
🤖 Generated with Claude Code
Summary by CodeRabbit
New Features
Tests