Skip to content

feat(intune): normalize Apple unified-log evidence for Company Portal macOS - #390

Merged
adamgell merged 15 commits into
mainfrom
claude/370-company-portal-macos-unified-log
Aug 3, 2026
Merged

feat(intune): normalize Apple unified-log evidence for Company Portal macOS#390
adamgell merged 15 commits into
mainfrom
claude/370-company-portal-macos-unified-log

Conversation

@adamgell

@adamgell adamgell commented Jul 31, 2026

Copy link
Copy Markdown
Owner

Closes #370.

What this changes

Adds cmtraceopen_parser::intune::portal::macos::company_portal::unified_log: a versioned,
normalized capture schema for Apple unified-log records relevant to Company Portal, plus
validation, conservative source selection, activity reduction, correlation, and a deterministic
redacted export.

Why this shape

The native side already collects unified-log data. src-tauri/src/macos_diag/unified_log.rs runs
log show --predicate <p> --style ndjson and parses timestamp, processImagePath, subsystem,
category, messageType, eventMessage, processID, threadID, returning predicate_used,
time_range, total_matched, and capped. Its intune-agent preset predicate is exactly
process == "IntuneMdmAgent" OR process == "IntuneMdmDaemon" OR process == "CompanyPortal".

The schema here is a versioned superset of that real shape rather than an invented one, adding what
#370 requires: timezone and boot-relative metadata, sender image, activity and signpost identifiers,
capture predicate and window, host and app versions, collection time, coverage, redaction metadata,
and source sequence. The pure crate validates and reduces; it never runs log show.

Verified

Run on this branch, not inherited from the agent that wrote it:

cargo test --locked -p cmtraceopen-parser
  lib unittests                          -> 355 passed; 0 failed  (baseline 355, unchanged)
  tests/company_portal_macos_unified_log ->  17 passed; 0 failed
  tests/esp_diagnostics                  -> 222 passed; 0 failed  (baseline 222, unchanged)
cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings  -> clean
cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown -> Finished

All 14 items of the issue's required fixture matrix are implemented across 19 synthetic fixtures.
Nothing was skipped. Two acceptance tests back the hard constraints: a golden serialization of the
versioned schema with a deserialize round-trip, and a source scan asserting the module contains no
std::process, Command::new, std::fs, std::net, SystemTime, thread::spawn, or log show
outside doc comments.

Reviewer attention

  • Time-only joins never merge. Unified-log evidence merges with direct Company Portal logs only
    on explicit activity, request, or correlation identifiers. same-time-no-merge proves two records
    sharing an identical normalizedUtc produce zero links and two TimeOnly rejections, while a
    mixed fixture merges the identified pair and still refuses its co-timed neighbour.
  • The verified subsystem strings are not provable from this repo. Only the process names are, via
    the intune-agent preset. com.microsoft.CompanyPortalMac, com.microsoft.intune.mdm.agent, and
    com.microsoft.intune.mdm.daemon are documented in schema.rs as an allowlist to extend only
    against a confirmed real capture. An unrecognized subsystem fails safe to ProcessOnlyInsufficient
    coverage rather than a wrong selection, and widening it should bump
    PORTAL_UNIFIED_LOG_SELECTION_PREDICATE_VERSION.
  • Join keys deliberately survive redaction. Activity, signpost, trace, request, and correlation
    identifiers are not redacted: they are the merge keys and name no person, tenant, or device. The
    reasoning is recorded in redaction.rs.
  • Two real redaction leaks were caught by the fixtures before this was committed. Pattern-based
    path redaction left the tail of /Users/x/Applications/Company Portal.app/... because the space
    stopped the match, and a bare host name matched no pattern at all. Whole-value identifier fields
    (hostName, processImagePath, senderImagePath) are now replaced wholesale rather than
    pattern-matched.

Not in this PR

src-tauri/** is untouched. The native adapter still emits its original eight-field shape and does
not yet produce this schema. Teaching macos_diag/unified_log.rs to emit schemaId/schemaVersion,
activity identifiers, coverage, and the predicate is follow-up work, and the #370 acceptance
criterion "Native acceptance runs on macOS" is therefore not met by this PR alone.

Batch note

Part of the #356 Company Portal family, alongside #368 (PR #387) and #372. All three branch off the
same base and each adds pub mod portal; to crates/cmtraceopen-parser/src/intune/mod.rs; this one
and #368 also both create intune/portal/macos/company_portal/mod.rs. Expect trivial one-line
conflicts there between the three. Whichever merges first wins and the others rebase.

Summary by CodeRabbit

  • New Features

    • Added support for importing macOS Company Portal unified-log captures in JSON and NDJSON formats.
    • Added structured parsing of timestamps, log levels, activities, metadata, sensitive fields, and unsupported records.
    • Added conservative evidence selection and identifier-based correlation with direct logs.
    • Added coverage reporting for malformed, skipped, unsupported, capped, or permission-limited data.
    • Added deterministic redaction for credentials, identifiers, URLs, paths, and network details.
    • Added schema and capture metadata validation, including graceful handling of unknown formats.
  • Bug Fixes

    • Prevented records from being merged solely because they share a timestamp.

…Portal

Before this change the parser crate had no Company Portal surface at all.
`intune::portal` did not exist, and the only Apple unified-log code in the
repo lived in the native macOS adapter (`src-tauri/src/macos_diag/unified_log.rs`),
which shells out to `log show --predicate <p> --style ndjson`, keeps eight raw
ndjson fields, and drops every line that fails to parse. Nothing downstream
could tell a Company Portal record from any other process the `intune-agent`
preset happens to match, and nothing recorded what the capture had missed.

This adds `cmtraceopen_parser::intune::portal::macos::company_portal::unified_log`:
a versioned normalized capture schema (`schema.rs`, `models.rs`) that is a
superset of those raw ndjson fields, adding timezone and boot-relative
metadata, sender image, activity/signpost/trace/request identifiers, capture
predicate and window, host and app versions, collection time, coverage,
redaction metadata, and exact source sequence; an ingest layer accepting both
the NDJSON and single-object JSON encodings; a conservative selection
predicate; a reducer producing Company Portal evidence and activity groups;
correlation against direct-log anchors; and a deterministic redacted export.

The pure crate is the right seam for the *contract* precisely because it cannot
collect. Collection needs `log show`, a macOS host, and TCC permission, none of
which belong in a crate that must compile to wasm32 and run with no I/O. Fixing
the schema here first means the native adapter has something to target, and the
reduction rules become testable against synthetic fixtures instead of a device.
Selection is likewise stricter than collection: the `log show` predicate matches
on process alone, which is not evidence of anything, so selection requires a
verified (process, subsystem) pair, admits an unverified subsystem only through
an explicit shared activity identifier, and never reads the message body.
Time-only joins are refused with a recorded rejection rather than merged.
Malformed lines, unknown schema ids and versions, missing headers, capped or
permission-denied collection, degraded timestamps, duplicate sequence numbers,
and unselected records all become explicit coverage; nothing is dropped.

`src-tauri/**` is deliberately untouched. Teaching the native adapter to emit
this schema is a separate change; the pure contract comes first.

Verified in this worktree:

  cargo test --locked -p cmtraceopen-parser --test company_portal_macos_unified_log
      -> 17 passed, 0 failed
  cargo test --locked -p cmtraceopen-parser
      -> 355 passed, 17 passed, 222 passed, 0 passed; 0 failed
         (baseline 355 + 222 unchanged, 17 new)
  cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings
      -> Finished, no warnings
  cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown
      -> Finished
  git diff --check
      -> clean

Two leaks were caught by the fixtures and fixed before commit: pattern-based
path redaction left the tail of a path containing a space, and a bare host name
matched no pattern at all. Both fields are now replaced wholesale.

All 14 items of the issue's required fixture matrix are covered, plus a golden
serialization test of the versioned schema and a test asserting the module
source contains no `std::process`, `Command::new`, `std::fs`, `std::net`,
`SystemTime`, `thread::spawn`, or `log show` outside doc comments.

Refs #370
Copilot AI review requested due to automatic review settings July 31, 2026 06:53

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new pure-Rust intune::portal surface for macOS Company Portal unified-log evidence, defining a versioned normalized capture schema plus validation, conservative selection, reduction/correlation, and deterministic redacted export (closing #370).

Changes:

  • Introduces cmtraceopen_parser::intune::portal::macos::company_portal::unified_log (schema/models, ingest+coverage, selection, reduction, correlation, redaction).
  • Adds a focused contract test suite (company_portal_macos_unified_log) with a fixture matrix covering supported/unsupported schemas, malformed lines, conservative selection, correlation rules, and deterministic export.
  • Wires the new intune::portal module into the crate’s intune public surface.

Reviewed changes

Copilot reviewed 33 out of 33 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v2/unknown-schema-version/capture.ndjson Fixture for unsupported future schema version behavior.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/unrelated-source/capture.ndjson Fixture for conservative negative selection cases.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.ndjson NDJSON “supported” baseline fixture.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.json JSON encoding of the same baseline capture.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/expected-capture-set.json Golden expected serialization for the capture set contract.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/capture.ndjson Golden input capture used for stable-schema testing.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/same-time-no-merge/capture.ndjson Fixture asserting time-only joins are refused.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/redacted-export/capture.ndjson Fixture exercising deterministic redacted export.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/privacy-sensitive/capture.ndjson Fixture for privacy classification + redaction tokenization.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/malformed-json-line/capture.ndjson Fixture for malformed NDJSON line coverage handling.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson Fixture for messageType→level normalization behavior.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/duplicate-sequence/capture.ndjson Fixture for duplicate sequence coverage without reordering/dropping.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/direct-log-match-key/capture.ndjson Fixture for explicit-key correlation with direct logs.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/degraded-time/capture.ndjson Fixture for local-only/boot-relative/invalid timestamp degradation coverage.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/known-preset.ndjson Fixture for known-preset predicate provenance.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson Fixture for non-preset predicate provenance behavior.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-coverage/capture.ndjson Fixture for capture-level capped/skipped/permissionDenied coverage.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/activity-signpost/capture.ndjson Fixture for activity/signpost relationship preservation.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/unknown-schema-id/capture.ndjson Fixture for unknown schema id refusal behavior.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/missing-header/capture.ndjson Fixture for missing header refusal behavior.
crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs End-to-end contract/acceptance tests for ingest/selection/reduction/correlation/redaction.
crates/cmtraceopen-parser/src/intune/portal/mod.rs Introduces the new top-level intune::portal module.
crates/cmtraceopen-parser/src/intune/portal/macos/mod.rs Adds macOS portal namespace.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/mod.rs Defines macOS Company Portal artifact family namespace.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/mod.rs Public module surface and pipeline/invariants documentation.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs Defines the versioned normalized schema types, coverage, evidence, and correlation models.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/schema.rs Declares schema identity/version and conservative verified-source tables/constants.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs Implements JSON/NDJSON parsing, validation, coverage, and timestamp/level normalization.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs Implements conservative selection predicate + category classification.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/reduce.rs Reduces capture sets to evidence/activities while preserving source sequence.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/correlation.rs Correlates unified-log evidence to direct logs using explicit identifiers only.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs Implements deterministic redacted projections for capture sets and reductions.
crates/cmtraceopen-parser/src/intune/mod.rs Exposes the new intune::portal module at the crate level.

… module

Found by review of PR #390. Four are privacy leaks in the export that is named
"redacted", one is a silent version-check bypass.

1. The email pattern was ASCII-only. That does not truncate a non-ASCII
   identity, it misses it completely: with an accented leading character there is
   no word boundary for the leading `\b` to anchor on, so the whole address
   survived verbatim. The class is now Unicode-aware.

2. Every label-scoped rule failed on JSON-shaped input. `LABEL_SEPARATOR`
   required a bare `:` or `=` directly after the label, but in JSON the label
   arrives as `"serialNumber":"C02..."`, so the quote stopped the match. This hit
   exactly the input that depends on those rules most: a malformed NDJSON line is
   kept verbatim as a coverage excerpt and has no structured fields to redact, so
   the label patterns are all it has. Serials, tokens, and tenant ids shipped in
   plaintext. The separator now tolerates the closing quote.

3. `Authorization: Bearer <opaque>` leaked the credential. The generic labelled
   rule stops its value at the first whitespace, so it redacted the scheme word
   `Bearer` and left the token. JWTs were already caught by shape; opaque bearer
   and Basic credentials were not. A dedicated scheme-and-credential rule now
   runs first.

4. No IPv4, IPv6, or MAC redaction, though this module has a `NetworkRequest`
   evidence kind and selects on network and service categories. The ESP redactor
   already enforces this; the two now agree. Ordered IPv4, MAC, IPv6, after URLs,
   so an IPv4-mapped IPv6 cannot leak its dotted tail and a MAC is not read as a
   compressed IPv6 run.

5. `schemaVersion` was parsed as `u64` then cast with `as u32`. A declared
   4294967297 wrapped to 1, passed the supported-version check, and an unknown
   schema was then parsed as if it were v1. A value that does not fit is not
   version 1, so it now stays `None` and falls through to the unsupported path.
   The same wrapping cast on `predicateVersion` and `processID` is fixed too: a
   wrapped pid would silently name a different process.

Verified:
  cargo test --locked -p cmtraceopen-parser
    lib 355 passed; company_portal_macos_unified_log 23 passed; esp_diagnostics 222 passed; 0 failed
  cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings  clean
  cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown  clean
  git diff --check  clean

Six new tests, one per hole plus a guard that the address matchers do not
swallow dotted version strings.

Refs #370
@adamgell

Copy link
Copy Markdown
Owner Author

Code review

Found 5 issues, all confirmed by running the code rather than reading it. Fixed in eb735f3c.

  1. Email redaction was ASCII-only, so a non-ASCII identity was not truncated but missed entirely: with an accented leading character the leading \b never anchors and the whole address survived into the export.

https://github.com/adamgell/cmtraceopen/blob/09c01ef31d3f10c4e1e5c1e6bfa2ee1e0a4e1b3c/crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs#L131-L135

  1. Every label-scoped rule failed on JSON-shaped input. LABEL_SEPARATOR required a bare : or = directly after the label, but JSON supplies "serialNumber":"C02..." and the quote stopped the match. This hits precisely the input that depends on those rules most: a malformed NDJSON line is stored verbatim as a coverage excerpt and has no structured fields, so the label patterns are all it has. Serials, tokens, and tenant ids shipped in plaintext.

  2. Authorization: Bearer <opaque> leaked the credential. The generic labelled rule stops its value at the first whitespace, so it redacted the scheme word Bearer and left the token. JWTs were caught by shape; opaque bearer and Basic credentials were not.

  3. No IPv4, IPv6, or MAC redaction, although this module defines a NetworkRequest evidence kind and selects on network and service categories. esp/redaction.rs already enforces this, so the two modules disagreed on the same privacy contract.

  4. schemaVersion was parsed as u64 then cast with as u32. A declared 4294967297 wraps to 1, passes the supported-version check, and an unknown schema is then parsed as if it were v1. The same wrapping cast affected predicateVersion and processID, where a wrapped pid would silently name a different process.

Verified as clean, not reported: correlation never merges on time alone (merge_is_permitted returns false for TimeOnly and explicit_match never produces that basis); determinism holds (no HashMap/HashSet reaches output); no records are silently dropped; the module is wasm-clean and has its own test enforcing that.

🤖 Generated with Claude Code

- If this code review was useful, please react with 👍. Otherwise, react with 👎.

@adamgell
adamgell requested a review from Copilot July 31, 2026 07:05
@adamgell

Copy link
Copy Markdown
Owner Author

@coderabbitai review

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown

Review Change Stack

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Warning

.coderabbit.yaml has a parsing error

The CodeRabbit configuration file in this repository has a parsing error and default settings were used instead. Please fix the error(s) in the configuration file. You can initialize chat with CodeRabbit to get help with the configuration file.

💥 Parsing errors (1)
Validation error: Too big: expected string to have <=250 characters at "tone_instructions"
⚙️ Configuration instructions
  • Please see the configuration documentation for more information.
  • You can also validate your configuration using the online YAML validator.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
📝 Walkthrough

Walkthrough

Added a versioned Rust pipeline for macOS Company Portal unified-log ingestion, conservative selection, reduction, redaction, and direct-log correlation.

Changes

Company Portal unified-log support

Layer / File(s) Summary
Public contract and normalized models
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/{mod.rs,schema.rs,models.rs}
Defines versioned schemas, normalized records, coverage, selection, evidence, and correlation types.
Capture ingestion and normalization
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs, crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/...
Parses JSON and NDJSON, validates schema versions, normalizes records and timestamps, and preserves malformed or unsupported data.
Conservative selection and reduction
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/{selection.rs,reduce.rs}
Selects verified sources, links explicit identifiers, reduces records into ordered evidence, and groups activities.
Deterministic redacted projections
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs
Redacts sensitive text and structured values with stable placeholders.
Explicit direct-log correlation
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/correlation.rs
Matches explicit identifiers or documented relationships. Records timestamp-only matches without merging.
Contract tests and fixtures
crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs, crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/...
Covers parsing, normalization, selection, ordering, coverage, redaction, serialization, correlation, and provenance.

Estimated code review effort: 5 (Critical) | ~90 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Capture
  participant Ingest
  participant Selection
  participant Reduction
  participant Redaction
  participant Correlation
  Capture->>Ingest: parse JSON or NDJSON
  Ingest->>Selection: provide normalized records
  Selection->>Reduction: provide selected records
  Reduction->>Redaction: create redacted projections
  Reduction->>Correlation: provide reduced evidence and anchors
  Correlation-->>Reduction: return links and rejected time-only matches
Loading

Possibly related PRs

Suggested labels: test

Suggested reviewers: copilot

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: normalized Apple unified-log evidence for Company Portal on macOS.
Linked Issues check ✅ Passed The implementation satisfies issue #370 through versioned parsing, conservative selection, reduction, correlation, coverage, redaction, fixtures, and wasm-compatible pure-crate processing.
Out of Scope Changes check ✅ Passed The changes remain within issue #370, covering the pure unified-log module, focused tests, and related fixtures without unrelated native or parser work.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

Comment @coderabbitai help to get the list of available commands.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Suppressed comments (2)

crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs:279

  • If schemaVersion exists but is out of range (e.g., > u32::MAX), schema_version becomes None and the coverage detail currently claims the header did not declare schemaVersion. That makes the UnsupportedSchemaVersion coverage misleading for the overflow case.
        let Some(version) = schema_version else {
            self.push_coverage(
                PortalCoverageStatus::UnsupportedSchemaVersion,
                PortalCoverageScope::Capture,
                "capture header does not declare `schemaVersion`".to_string(),

crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs:1176

  • The out_of_range_schema_version_is_unsupported_not_wrapped test currently uses a schemaId that does not match PORTAL_UNIFIED_LOG_SCHEMA_ID, so it fails the schema-id check (UnknownSchema) rather than exercising the intended "schemaVersion overflows u32" path.
fn out_of_range_schema_version_is_unsupported_not_wrapped() {
    let header = r#"{"schemaId":"cmtraceopen.intune.portal.macos.company-portal.unified-log","schemaVersion":4294967297}"#;
    let capture = parse_capture_ndjson(&format!("{header}\n"));

@coderabbitai

coderabbitai Bot commented Jul 31, 2026

Copy link
Copy Markdown
✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 4

🧹 Nitpick comments (1)
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs (1)

416-424: 🗄️ Data Integrity & Integration | 🔵 Trivial | 💤 Low value

A malformed capture-coverage entry is counted as a malformed record.

push_malformed increments stats.records_malformed and sets PortalCoverageScope::Record. The value here is a capture-scope coverage entry, not a stream record. The count then disagrees with stream_lines accounting for callers that compare the two.

Consider a capture-scoped variant that keeps the verbatim excerpt but does not change records_malformed.

🤖 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/portal/macos/company_portal/unified_log/ingest.rs`
around lines 416 - 424, Update the malformed-entry branch in the
capture-coverage ingestion loop to preserve the verbatim excerpt without calling
push_malformed, since that helper increments record-level statistics and scope.
Use the capture-scoped malformed handling already established for this parser,
leaving records_malformed and stream_lines accounting 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.

Inline comments:
In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs`:
- Around line 330-338: Update the redaction construction in build_record to add
fields to redaction.redacted_fields only when their corresponding values have
Sensitive classification. Preserve Public values without listing them, while
retaining the existing applied flag, policy ID, deduplication, and sorting
behavior.

In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs`:
- Around line 128-177: Update explicit_ids and classify_records so activity
identifiers retain their kind during activity-link matching. Group activity_id
and parent_activity_id into one shared kind, while keeping signpost_id,
trace_id, request_id, and correlation_id distinct; compare tagged identifiers
rather than raw strings so only same-kind values establish
ActivityLinkedToSelected relationships.

In `@crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs`:
- Around line 1173-1185: Update
out_of_range_schema_version_is_unsupported_not_wrapped to construct the header
using PORTAL_UNIFIED_LOG_SCHEMA_ID instead of the mismatched literal, ensuring
parsing reaches the schema-version validation and specifically exercises
rejection of 4294967297. Keep the unsupported assertion and genuine v1 support
check intact.

In
`@crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/capture.ndjson`:
- Around line 1-3: Keep timestamps in both affected fixtures within their
declared capture windows: in
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/capture.ndjson
lines 1-3, adjust the window and collectedAtUtc or both record timestamps, then
regenerate expected-capture-set.json; in
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v2/unknown-schema-version/capture.ndjson
lines 1-3, adjust the window end or both preserved record timestamps.

---

Nitpick comments:
In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs`:
- Around line 416-424: Update the malformed-entry branch in the capture-coverage
ingestion loop to preserve the verbatim excerpt without calling push_malformed,
since that helper increments record-level statistics and scope. Use the
capture-scoped malformed handling already established for this parser, leaving
records_malformed and stream_lines accounting unchanged.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 83bf237b-6d10-4993-9fa9-3a4580a2f257

📥 Commits

Reviewing files that changed from the base of the PR and between a7c6310 and eb735f3.

📒 Files selected for processing (33)
  • crates/cmtraceopen-parser/src/intune/mod.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/mod.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/correlation.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/mod.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/reduce.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/schema.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/mod.rs
  • crates/cmtraceopen-parser/src/intune/portal/mod.rs
  • crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/missing-header/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/unknown-schema-id/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/activity-signpost/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-coverage/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/known-preset.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/degraded-time/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/direct-log-match-key/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/duplicate-sequence/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/malformed-json-line/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/privacy-sensitive/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/redacted-export/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/same-time-no-merge/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/expected-capture-set.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/unrelated-source/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v2/unknown-schema-version/capture.ndjson

Comment thread crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
@adamgell adamgell added feature New feature intune Microsoft Intune related parser Log parser related portal Company Portal related enhancement New feature or request labels Jul 31, 2026
…e leaks

CI was red on `clippy::useless_conversion` for `zip(verdicts.into_iter())` in
`reduce.rs`. Local clippy 0.1.92 does not raise it and CI's newer toolchain does,
so the local gate passed while the branch was failing.

Correctness, from CodeRabbit. `explicit_ids` flattened `activity_id`,
`parent_activity_id`, `signpost_id`, `trace_id`, `request_id`, and
`correlation_id` into one untyped set, so a candidate's `traceId` could match an
anchor's `activityIdentifier` and be selected as `ActivityLinkedToSelected`
although the two share no relationship in any single namespace. Rule 2 says it
links on a documented structural relationship and "not a coincidence", and the
committed fixtures already reuse the `0x...` shape across both fields, so this
was reachable rather than theoretical. Identifiers are now tagged with their
namespace and only compared within it, which is what `correlation.rs` already
did. `activity_id` and `parent_activity_id` deliberately stay one namespace: a
child carrying its parent's activity id is a real link.

Two further privacy holes:

- `user_path_pattern` stopped at the first space, so
  `/Users/alice/Applications/Company Portal.app/...` was redacted only up to the
  space and the tail shipped. It now absorbs a space when the run after it still
  contains a `/`, so a path keeps going while prose after a path does not get
  swallowed. Written without look-ahead, which this regex engine rejects.
- `redacted_fields` listed all three fields unconditionally, telling consumers a
  value had been scrubbed when a `Public` value had been passed through
  untouched. It now lists a field only when that field was actually sensitive.

Also corrects the `UnknownSchema` doc, which described only an absent or
unrecognised schema id although the variant is also used for an unrecognised
declared coverage status, and widens two fixture capture windows that ended
before their own records, which no bounded `log show` collection could produce.
The golden expectation is regenerated.

The previous commit's schema-version test passed for the wrong reason: it
hand-typed the schema id as `company-portal.unified-log` where the constant is
`companyPortal.unifiedLog`, so the capture was refused on the id branch before
the version check ran and the test would still have passed with the wrapping
cast restored. It now builds the header from the constant and asserts the
refusal is `UnsupportedSchemaVersion` and not `UnknownSchema`. Confirmed by
restoring `as u32` and watching it fail.

Verified:
  cargo test --locked -p cmtraceopen-parser
    lib 355 passed; company_portal_macos_unified_log 26 passed; esp_diagnostics 222 passed; 0 failed
  cargo clippy --locked --all-targets -- -D warnings   clean  (workspace, matching CI)
  cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown  clean
  git diff --check  clean

Both new correctness tests were checked against the bug they guard: each fails
when the fix is reverted.

Refs #370
@adamgell
adamgell requested a review from Copilot July 31, 2026 18:03
@adamgell

Copy link
Copy Markdown
Owner Author

Review cycle 2

CI was red and I had missed it: clippy::useless_conversion on zip(verdicts.into_iter()) in reduce.rs. Local clippy 0.1.92 does not raise that lint and CI's newer toolchain does, so my per-crate gate passed while the branch was failing. Fixed, and I now run the workspace-level cargo clippy --locked --all-targets -- -D warnings that CI actually runs.

All six review threads were real. Fixed in 9521dfe4.

Source Finding Kind
CodeRabbit my schema-version test passed for the wrong reason vacuous test
CodeRabbit cross-namespace identifier collisions link unrelated records correctness
Copilot user_path_pattern stops at the first space, leaking the path tail privacy
CodeRabbit redacted_fields claims fields that were passed through untouched false provenance
Copilot UnknownSchema doc describes only one of its two uses ambiguous contract
CodeRabbit fixture capture windows end before their own records unreal fixture

Two deserve calling out.

The vacuous test was mine, and CodeRabbit was exactly right. I hand-typed the schema id as company-portal.unified-log where the constant is companyPortal.unifiedLog, so the capture was refused on the id branch before the version check ran. The test would still have passed with the as u32 cast restored, which makes it worse than having no test at all. It now builds the header from the constant and asserts the refusal reason. I verified both new correctness tests by reverting each fix and watching the test fail.

The namespace collision was reachable, not theoretical. explicit_ids flattened six identifier fields into one untyped set, so a candidate's traceId could match an anchor's activityIdentifier and be selected as activity-linked. The committed fixtures already reuse the 0x... shape across both fields. correlation.rs had this right; selection.rs now matches it.

cargo test --locked -p cmtraceopen-parser
  lib 355 passed; company_portal_macos_unified_log 26 passed; esp_diagnostics 222 passed; 0 failed
cargo clippy --locked --all-targets -- -D warnings   clean  (workspace, matching CI)
cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown  clean
git diff --check  clean

Tests on this PR: 17 at open, 26 now. All six threads answered and resolved.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 33 out of 33 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs:95

  • parse_capture_ndjson treats the first JSON object containing schemaId as the capture header. A record object could legitimately (or maliciously) contain a schemaId field inside its unknown fields, which would then be misclassified as the header and removed from the record stream, changing parsing outcomes and stats. Header detection should also require schemaVersion and only occur before any records have been collected.
            Ok(Value::Object(obj)) => {
                if header.is_none() && obj.contains_key("schemaId") {
                    header = Some(obj);
                } else {
                    builder.stats.stream_lines += 1;

`parse_capture_ndjson` treated the first JSON object containing `schemaId` as
the capture header. A record is free to carry `schemaId` among its unknown
fields, and such a record was lifted out of the stream and read as capture
metadata, silently changing both the record set and the reported stats. Because
the check was only guarded by `header.is_none()`, it could also fire after
records had already been collected.

The header must now be the leading object and must look like a header: both
`schemaId` and `schemaVersion` present, and no records seen yet. A stray field
can no longer rewrite the capture.

Raised as a suppressed comment in the GitHub Copilot review of PR #390.

Verified:
  cargo test --locked -p cmtraceopen-parser
    lib 355 passed; company_portal_macos_unified_log 27 passed; esp_diagnostics 222 passed; 0 failed
  cargo clippy --locked --all-targets -- -D warnings   clean  (workspace, matching CI)
  cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown  clean
  git diff --check  clean

`a_record_mentioning_schema_id_is_not_mistaken_for_the_header` covers both the
header-present case, where the sneaky record stays in the stream and stats stay
at two, and the headerless case, where it is still a record rather than metadata.

Refs #370
@adamgell
adamgell requested a review from Copilot July 31, 2026 18:12
@adamgell

Copy link
Copy Markdown
Owner Author

Review cycle 3

Clean by the gate: Copilot count 2 to 3, anchored to 9521dfe4, zero unresolved threads, verdict generated no new comments. CodeRabbit is now approved.

One suppressed item, and it was a real mis-parse. parse_capture_ndjson treated the first JSON object containing schemaId as the capture header. A record may legitimately carry schemaId among its unknown fields, and such a record was lifted out of the stream and read as capture metadata, changing both the record set and the reported stats. The guard was only header.is_none(), so it could also fire after records had already been collected.

Fixed in 88ed7053: the header must be the leading object, must carry both schemaId and schemaVersion, and is refused once any record has been seen.

a_record_mentioning_schema_id_is_not_mistaken_for_the_header covers both directions: with a real header the sneaky record stays in the stream and streamLines stays at 2, and with no header at all it is still a record rather than metadata.

cargo test --locked -p cmtraceopen-parser
  lib 355 passed; company_portal_macos_unified_log 27 passed; esp_diagnostics 222 passed; 0 failed
cargo clippy --locked --all-targets -- -D warnings   clean  (workspace, matching CI)
cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown  clean
git diff --check  clean

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (1)
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson (1)

2-8: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Add a Notice record to cover every named level.

normalize_level maps seven named levels: debug, info, default, notice, warning (with the warn alias), error, and fault. This fixture exercises six of them plus the unknown case through Emergency. Notice is a real unified-log messageType, and no record here carries it, so the end-to-end path for that variant is untested by this fixture.

Add one Notice record and update stats.totalMatched.

🤖 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/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson`
around lines 2 - 8, Add a unified-log fixture record with messageType Notice
alongside the existing level records, using valid matching fields and the next
source sequence, and update the expected stats.totalMatched value to include
this additional record.
🤖 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/portal/macos/company_portal/unified_log/ingest.rs`:
- Around line 65-72: Update parse_capture to route a parsed JSON object to
parse_capture_json_object when it contains either the records field or the
capture header fields expected by the portal format, including objects
representing an empty capture. Preserve the existing NDJSON fallback for JSON
objects without the capture header and for non-object input.

In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/reduce.rs`:
- Around line 135-164: Reject empty activity identifiers before creating or
updating groups in the record-processing loop of reduce_capture_set, so selected
records with Some("") are skipped rather than grouped under an empty key. Apply
the same non-empty validation in selection.rs::explicit_ids to prevent empty
identifiers from entering selection and linking flows.

In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs`:
- Around line 128-178: Update explicit_ids so it excludes identifiers whose
values are empty strings, including Some(""). Preserve the existing namespace
tagging and retain only non-empty IDs before classify_records can use them as
explicit links.

---

Nitpick comments:
In
`@crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson`:
- Around line 2-8: Add a unified-log fixture record with messageType Notice
alongside the existing level records, using valid matching fields and the next
source sequence, and update the expected stats.totalMatched value to include
this additional record.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 2162ea6a-9bf1-4ad0-9006-d544a5a56133

📥 Commits

Reviewing files that changed from the base of the PR and between 7ab9d2f and 8d6144f.

📒 Files selected for processing (29)
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/correlation.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/mod.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/reduce.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/schema.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs
  • crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/missing-header/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/unknown-schema-id/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/activity-signpost/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-coverage/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/known-preset.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/degraded-time/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/direct-log-match-key/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/duplicate-sequence/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/malformed-json-line/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/privacy-sensitive/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/redacted-export/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/same-time-no-merge/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/expected-capture-set.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/unrelated-source/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v2/unknown-schema-version/capture.ndjson

Three failing tests, all red on this commit. Found by auditing the class
of a CodeRabbit finding on PR #387, whose `logs` module carries a
byte-identical IPv6 pattern.

`\b` is a word-boundary assertion and `:` is not a word character, so the
matcher cannot anchor an address that begins or ends with a colon. Every
`::`-compressed form does one or both, and the result is not a truncated
redaction but none at all: `fd12:3456:789a::` exports verbatim.

The same boundary rule fails in the other direction on a bare `::`, where
both sides are word characters. `called std::process::exit` currently
redacts to `called std[redacted:host]process[redacted:host]exit`,
destroying ordinary prose in an export.

Refs #370
The IPv6 matcher was wrapped in `\b` at both ends, and `:` is not a word
character, so the assertion failed in both directions.

Under-redaction: no boundary can anchor an address that begins or ends
with a colon, so `fd12:3456:789a::` matched nothing and exported
verbatim. An address embedded in a word matched only its `db8::` tail,
which is worse than no match because the output looks redacted while the
leading groups leak.

Over-redaction: a bare `::` between two word characters anchors on both
sides, so `called std::process::exit` in an event message exported as
`called std[redacted:host]process[redacted:host]exit`.

This engine has no look-around, so the left edge is now a consumed guard
group written back out through `${1}` and the right edge has no
assertion. Only the left delimiter is consumed, which keeps two addresses
separated by a single character both matchable; a guard on both sides
would eat the delimiter the following address needs, and `^` cannot stand
in because it anchors to the start of the text rather than to where
scanning resumed. No right-edge assertion is needed because each
alternative consumes only hex and colons and requires either eight groups
or a `::` run, which neither a MAC nor a wall-clock timestamp has. The
bare `::` alternative is dropped, so prose is left alone.

Found by auditing the class of a finding on PR #387, whose `logs` module
carried a byte-identical pattern and is fixed there in `105eb25a`.

The embedded-address assertion uses `opaque` rather than `token` as its
lead-in word: the labelled-secret rule legitimately claims `token` plus
whatever follows it, which is correct behaviour and not what this test is
about.

Refs #370
@adamgell

adamgell commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

One more, pushed after the approval, because leaving it would ship a known leak.

While auditing the class of a CodeRabbit finding on PR #387 (\b cannot anchor an IPv6 address that begins or ends with a colon), I found that unified_log/redaction.rs here carries a byte-identical pattern with the same defect. Fixed in d8a1edae, red on 7acf0e9d.

It fails in both directions:

  • Under-redaction. No boundary can anchor a ::-compressed address, so peer fd12:3456:789a:: connected exported the address verbatim. Not a truncated redaction, none at all. An address embedded in a word matched only its db8:: tail, which is worse than no match: the output looks redacted while the leading groups leak.
  • Over-redaction. A bare :: between two word characters anchors on both sides, so called std::process::exit in an eventMessage exported as called std[redacted:host]process[redacted:host]exit. That one destroys evidence rather than protecting it, and no existing test covered it.

The fix matches the one on #387: no look-around in this engine, so the left edge is a consumed guard group written back out through ${1} and the right edge carries no assertion. Consuming only the left delimiter is what keeps two addresses separated by a single character both matchable, which a symmetric guard would break; ^ cannot stand in because it anchors to the start of the text, not to where scanning resumed. No right-edge assertion is needed because each alternative consumes only hex and colons and requires either eight groups or a :: run, which neither a MAC nor a wall-clock timestamp has. The bare :: alternative is dropped, since it is the all-zeros address and identifies nothing.

Three tests, all red on 7acf0e9d: 2001:db8:85a3:: survived redaction, first survived: peers fd12:3456:789a:: ..., and called std[redacted:host]process[redacted:host]exit against called std::process::exit.

One test-authoring note: the embedded-address assertion uses opaque rather than token as its lead-in word. token x2001:db8::y is claimed whole by the labelled-secret rule, which is correct behaviour and not what that test is about.

Verification at d8a1edae: cargo test --locked -p cmtraceopen-parser 813 passed (baseline 806, +7 new tests across this round), cargo test --locked from src-tauri 826 passed (unchanged), cargo clippy --locked --all-targets -- -D warnings clean for both crates, cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown finished, npx tsc --noEmit clean, git diff --check clean, rustfmt --check clean.

@adamgell
adamgell requested a review from Copilot August 1, 2026 15:21
@adamgell

adamgell commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
✅ Action performed

Full 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 10 minutes.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/tests/company_portal_macos_unified_log.rs`:
- Around line 1156-1188: Strengthen the assertions in
compressed_ipv6_forms_are_fully_redacted and
adjacent_ipv6_addresses_are_both_redacted so partial masking cannot pass: verify
every meaningful IPv6 hextet from each input address is absent from the redacted
output, including both addresses in each adjacent-address case, while preserving
the existing delimiter checks.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 24628c0b-b505-40c7-9fd0-98f81e1d8c93

📥 Commits

Reviewing files that changed from the base of the PR and between 8d6144f and d8a1eda.

📒 Files selected for processing (2)
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs
  • crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
🚧 Files skipped from review as they are similar to previous changes (1)
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs

Comment thread crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/correlation.rs:91

  • same_instant() falls back to comparing raw_text whenever either side lacks normalized_utc, but the docstring says the raw-text comparison should only apply when neither timestamp can be resolved to an absolute instant. As written, an absolute timestamp could be treated as equal to a non-absolute/invalid one if their raw_text happens to match, creating spurious TimeOnly rejections.
/// True when two timestamps denote the same instant, or the same literal text
/// when neither can be resolved to an absolute instant.
fn same_instant(left: &PortalTimestamp, right: &PortalTimestamp) -> bool {
    match (&left.normalized_utc, &right.normalized_utc) {
        (Some(a), Some(b)) => a == b,
        _ => !left.raw_text.is_empty() && left.raw_text == right.raw_text,
    }

Two failing tests, plus a strengthened assertion on an already-green one.

A blank identifier is treated as a value. Two records carrying
`Some("")` are read as sharing an explicit identifier, so an unrelated
process-only record is linked to a verified anchor and both collapse into
one empty-keyed activity group. Ingest normalises blanks to `None`, but a
record also reaches the reducer by deserialization or direct
construction. `correlation.rs` already refuses an empty key, so the rule
exists in one place and is missing in two.

A whole-payload JSON object with no `records` key routes to the NDJSON
path, where every physical line of the pretty printing fails to parse. A
well-formed empty capture is reported as six malformed records.

The IPv6 assertions now check each hextet rather than the whole address.
That test is green either way; the old form would have passed on a
partial redaction leaving `fd12:3456:[redacted:host]`, which still
exports the identifying network prefix.

Refs #370

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 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/portal/macos/company_portal/unified_log/redaction.rs`:
- Around line 287-308: Update placeholder_for_member to recognize normalized
host-name keys, including hostName and computerName, and map them to the
appropriate hostname placeholder. Preserve the existing normalization and all
current mappings while ensuring unknownFields members with these names are
redacted.

In
`@crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/malformed-json-line/capture.ndjson`:
- Around line 3-5: Update the malformed-json-line fixture and its associated
assertions so stream indexes remain tied to input positions: assign the final
valid record index 3, and assign malformed coverage entries indexes 1 and 2.
Ensure each pending object stores its original stream index before malformed
input is processed, then assert both record and malformed coverage indexes.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 7e908e4e-a250-4e8d-854b-1882137d5de2

📥 Commits

Reviewing files that changed from the base of the PR and between 7ab9d2f and d8a1eda.

📒 Files selected for processing (29)
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/correlation.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/mod.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/reduce.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/schema.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs
  • crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/missing-header/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/unknown-schema-id/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/activity-signpost/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-coverage/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/known-preset.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/degraded-time/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/direct-log-match-key/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/duplicate-sequence/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/malformed-json-line/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/privacy-sensitive/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/redacted-export/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/same-time-no-merge/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/expected-capture-set.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/unrelated-source/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v2/unknown-schema-version/capture.ndjson

`usable_identifier` is now the one place the rule lives, and `selection`,
`reduce`, and `correlation` all go through it.

An empty or whitespace-only identifier names nothing, so reading it as a
value made every record carrying one share an identifier with every
other: an unrelated process-only record was linked to a verified anchor
under rule 2, and both collapsed into a single empty-keyed activity
group. Parent activity ids and signpost ids had the same hole.
`correlation` already refused a bare `""` but let a whitespace key
through, so the rule existed in one place, half-right, and was missing in
two. Ingest normalises blanks to `None`, so this only reaches the reducer
by deserialization or direct construction, both of which the public API
allows.

Separately, `parse_capture` now routes a whole-payload JSON object that
declares a capture header to the JSON path, not only one carrying
`records`. `parse_capture_json_object` already treats an absent `records`
key as an empty capture, but a pretty-printed capture with no records
never reached it: it fell through to NDJSON, where every physical line of
the pretty printing failed to parse, and a well-formed empty capture came
back as six malformed records with an inflated `records_malformed`. The
header test is the one NDJSON framing already uses, both keys rather than
`schemaId` alone, so the two encodings agree on what a header is.

Refs #370
One failing test. `hostName`, `computerName`, `deviceName`, a nested
`machineName`, and a full `processImagePath` all export verbatim from
`unknownFields`.

These are the values the record and capture redactors already replace
wholesale precisely because no text pattern can recognise them: a bare
host name has no `@`, no scheme and no dotted quad, and only paths under
`/Users` match a path pattern, so `/Applications/Company Portal.app/...`
survives. The member table protects the same values under their schema
names but has no arm for either, so an unknown member carrying one is
unprotected.

Refs #370
The member table now has arms for `hostName`, `computerName`,
`machineName`, `deviceName`, `nodeName`, and for the image-path names.
Every value the record and capture redactors replace wholesale needs one,
because "wholesale" is exactly what they do when no text pattern can
recognise the value: a bare host name has no `@`, no scheme and no dotted
quad, and only paths under `/Users` match a path pattern, so
`/Applications/Company Portal.app/...` survived one.

The larger half of the fix is that the table was never consulted at the
top level. `redact_record` mapped `unknown_fields` through `redact_json`,
whose object arm is where the table lived, so a member name was only
recognised one level down. A collector puts these at the top level, which
is precisely where the table did not apply. `redact_member` is now the one
place a member name decides the outcome, and both call sites use it.

Also names the stream-line position in a malformed line's coverage detail,
and documents that `PortalCoverageEntry::stream_index` identifies a parsed
record rather than a physical stream position, which is why it is `None`
for a malformed line.

Refs #370
@adamgell
adamgell requested a review from Copilot August 1, 2026 15:38
@adamgell

adamgell commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
✅ Action performed

Full review finished.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 29 out of 29 changed files in this pull request and generated no new comments.

Suppressed comments (1)

crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs:178

  • explicit_ids() allocates a new Vec for every record (and is called repeatedly inside classify_records()), which adds avoidable heap churn for large unified-log captures. Consider changing explicit_ids() to return a non-allocating iterator (e.g., an array into_iter() + filter_map) and update the two call sites to consume the iterator directly (anchor_ids.extend(...) and explicit_ids(record).any(...)).
/// Explicit identifiers a record contributes to the activity graph, each tagged
/// with the namespace it came from.
fn explicit_ids(record: &PortalUnifiedLogRecord) -> Vec<(PortalIdentifierKind, &str)> {
    let activity = &record.activity;
    [
        (
            PortalIdentifierKind::Activity,
            activity.activity_id.as_deref(),
        ),
        (
            PortalIdentifierKind::Activity,
            activity.parent_activity_id.as_deref(),
        ),
        (
            PortalIdentifierKind::Signpost,
            activity.signpost_id.as_deref(),
        ),
        (PortalIdentifierKind::Trace, activity.trace_id.as_deref()),
        (
            PortalIdentifierKind::Request,
            activity.request_id.as_deref(),
        ),
        (
            PortalIdentifierKind::Correlation,
            activity.correlation_id.as_deref(),
        ),
    ]
    .into_iter()
    .filter_map(|(kind, id)| usable_identifier(id).map(|id| (kind, id)))
    .collect()
}

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🧹 Nitpick comments (3)
crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs (1)

863-871: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Use the public formatter instead of the hardcoded id text.

Line 870 pins the literal "coverage-0000", while the test at Line 1559 pins ids through coverage_id(index). If the id format changes, these two tests disagree about the source of truth. Prefer the formatter here too.

♻️ Proposed change
-    assert_eq!(ids[0], "coverage-0000");
+    assert_eq!(ids[0], coverage_id(0));
🤖 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/tests/company_portal_macos_unified_log.rs` around
lines 863 - 871, Update the assertion in the reduction coverage-id test to
compare ids[0] against the existing public coverage_id formatter using the
corresponding index, rather than hardcoding "coverage-0000". Keep the uniqueness
assertion unchanged and reuse the formatter already referenced by the test near
the other coverage-id checks.
crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson (1)

1-1: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value

Give the custom predicate its own predicateId.

The header declares predicateId "intune-agent" while predicateText is process == "CompanyPortal", which is not the intune-agent preset. matches_known_preset is derived from the text, so parsing is unaffected. The fixture still labels a custom predicate with the preset id, which makes the scenario ambiguous to a reader and would hide a future regression where the id, rather than the text, is used to decide preset identity.

Use a distinct id, for example "custom-company-portal-only".

🤖 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/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson`
at line 1, Update the fixture’s predicate.predicateId from the preset value
"intune-agent" to a distinct custom identifier such as
"custom-company-portal-only", while preserving the existing predicateText and
other metadata.
crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs (1)

170-180: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick win

Use usable_identifier in PortalActivityIds::has_any.

PortalActivityIds supports direct construction and deserialization, so whitespace-only values can bypass ingest normalization. In that case, is_some() returns true, although usable_identifier treats the value as absent. Apply usable_identifier(...as_deref()).is_some() to all six identifier fields.

🤖 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/portal/macos/company_portal/unified_log/models.rs`
around lines 170 - 180, Update PortalActivityIds::has_any to evaluate all six
identifier fields with usable_identifier(field.as_deref()).is_some() instead of
is_some(), so whitespace-only values are treated as absent while valid
identifiers remain recognized.

Source: Learnings

🤖 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/portal/macos/company_portal/unified_log/ingest.rs`:
- Around line 319-341: Update the schemaVersion diagnostic in the schema_version
None branch around u64_field so it probes the raw header value rather than using
a numeric conversion that returns None for negative or fractional values.
Distinguish absent values from any present non-usable value, reporting the
latter as outside the representable schema-version range while preserving the
existing detail for absent headers and valid out-of-range integers.

In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs`:
- Around line 354-369: The redact_coverage function currently sorts coverage_id
lexicographically, which misorders IDs once indices exceed the four-digit
padding. Update its sorting comparator to parse and compare the numeric index
portion of each coverage_id independently of zero-padding, while preserving
deterministic ordering and the existing redaction behavior.

In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs`:
- Around line 198-209: In the identifier-linking branch of the records/verdicts
selection loop, remove the assignment to matched_subsystem when setting
ActivityLinkedToSelected. Add or update coverage to assert that
identifier-linked records retain matched_subsystem as None.

---

Nitpick comments:
In
`@crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs`:
- Around line 170-180: Update PortalActivityIds::has_any to evaluate all six
identifier fields with usable_identifier(field.as_deref()).is_some() instead of
is_some(), so whitespace-only values are treated as absent while valid
identifiers remain recognized.

In `@crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs`:
- Around line 863-871: Update the assertion in the reduction coverage-id test to
compare ids[0] against the existing public coverage_id formatter using the
corresponding index, rather than hardcoding "coverage-0000". Keep the uniqueness
assertion unchanged and reuse the formatter already referenced by the test near
the other coverage-id checks.

In
`@crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson`:
- Line 1: Update the fixture’s predicate.predicateId from the preset value
"intune-agent" to a distinct custom identifier such as
"custom-company-portal-only", while preserving the existing predicateText and
other metadata.
🪄 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: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: 38bf74a2-48be-48b9-b552-97bd9e6d531a

📥 Commits

Reviewing files that changed from the base of the PR and between 7ab9d2f and 0db5f33.

📒 Files selected for processing (29)
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/correlation.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/mod.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/redaction.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/reduce.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/schema.rs
  • crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/selection.rs
  • crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/missing-header/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/unknown/unknown-schema-id/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/activity-signpost/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-coverage/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/custom-predicate.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/capture-metadata/known-preset.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/degraded-time/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/direct-log-match-key/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/duplicate-sequence/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/levels/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/malformed-json-line/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/privacy-sensitive/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/redacted-export/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/same-time-no-merge/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/schema-golden/expected-capture-set.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.json
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/supported-records/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v1/unrelated-source/capture.ndjson
  • crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log/v2/unknown-schema-version/capture.ndjson

Comment on lines +319 to +341
let Some(version) = schema_version else {
// `schema_version` is `None` for two different reasons, and a
// support engineer needs to tell them apart from the coverage
// detail alone: the field was absent, or it was present but too
// large for `u32`. Reporting "does not declare" for a header that
// plainly declares one sends the reader looking for the wrong thing.
let declared = u64_field(&header, "schemaVersion");
let detail = match declared {
Some(raw) => format!(
"capture header declares `schemaVersion` {raw}, which is outside the \
representable range for a schema version; records are preserved unreduced"
),
None => "capture header does not declare `schemaVersion`".to_string(),
};
self.push_coverage(
PortalCoverageStatus::UnsupportedSchemaVersion,
PortalCoverageScope::Capture,
detail,
None,
None,
);
return self.into_unsupported(schema_id, None, pending);
};

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win

Report a present but non-integer schemaVersion as out of range, not as absent.

u64_field uses Value::as_u64. It returns None for a declared schemaVersion that is negative or fractional, for example -1 or 1.5. The declared probe on Line 325 uses the same helper, so it also returns None. The coverage detail then reads "capture header does not declare schemaVersion" for a header that plainly declares one. This is the same reader-misdirection the out-of-range branch was added to prevent.

Probe the raw value instead, so the three cases stay distinct: absent, present but not a usable version number, and present but out of range.

🐛 Proposed fix
-            let declared = u64_field(&header, "schemaVersion");
-            let detail = match declared {
-                Some(raw) => format!(
+            let detail = match header.get("schemaVersion") {
+                None | Some(Value::Null) => {
+                    "capture header does not declare `schemaVersion`".to_string()
+                }
+                Some(raw) => format!(
                     "capture header declares `schemaVersion` {raw}, which is outside the \
                      representable range for a schema version; records are preserved unreduced"
                 ),
-                None => "capture header does not declare `schemaVersion`".to_string(),
             };
🤖 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/portal/macos/company_portal/unified_log/ingest.rs`
around lines 319 - 341, Update the schemaVersion diagnostic in the
schema_version None branch around u64_field so it probes the raw header value
rather than using a numeric conversion that returns None for negative or
fractional values. Distinguish absent values from any present non-usable value,
reporting the latter as outside the representable schema-version range while
preserving the existing detail for absent headers and valid out-of-range
integers.

Comment on lines +354 to +369
fn redact_coverage(coverage: &[PortalCoverageEntry]) -> Vec<PortalCoverageEntry> {
let mut entries: Vec<PortalCoverageEntry> = coverage
.iter()
.map(|entry| PortalCoverageEntry {
coverage_id: entry.coverage_id.clone(),
status: entry.status.clone(),
scope: entry.scope.clone(),
detail: redact_text(&entry.detail),
stream_index: entry.stream_index,
raw_excerpt: redact_optional(&entry.raw_excerpt),
evidence: entry.evidence.clone(),
})
.collect();
entries.sort_by(|a, b| a.coverage_id.cmp(&b.coverage_id));
entries
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -B2 -A10 'struct PortalCoverageEntry' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs
rg -n -B3 -A10 'coverage_id\s*[:=]' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/*.rs

Repository: adamgell/cmtraceopen

Length of output: 8119


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -B8 -A18 'fn coverage_id|coverage_id\(' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log
rg -n -B4 -A8 'deterministic|sorted by|coverage' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/{models.rs,ingest.rs,reduce.rs,redaction.rs}

Repository: adamgell/cmtraceopen

Length of output: 50376


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- coverage ID implementation and tests ---'
sed -n '375,392p' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs
rg -n -B3 -A8 'coverage_id\(|coverage-\{index|result_cap|MAX|cap' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log --glob '*.rs' | head -220
printf '%s\n' '--- coverage construction sites ---'
rg -n -B3 -A9 'PortalCoverageEntry\s*\{' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log --glob '*.rs'

Repository: adamgell/cmtraceopen

Length of output: 35741


🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -i 'result[_-]?cap|records?[_-]?cap|coverage.*cap|cap.*coverage|10000|10_000|1000|1_000' crates --glob '*.rs' --glob '*.toml' --glob '*.json' | head -240

Repository: adamgell/cmtraceopen

Length of output: 21164


🏁 Script executed:

#!/bin/bash
set -euo pipefail
sed -n '240,360p' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs
sed -n '430,530p' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/ingest.rs
rg -n 'coverage-[0-9]+' crates/cmtraceopen-parser/tests/fixtures/intune/portal/macos/unified_log crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs | head -120

Repository: adamgell/cmtraceopen

Length of output: 9292


🏁 Script executed:

#!/bin/bash
set -euo pipefail
python3 - <<'PY'
ids = [f"coverage-{i:04d}" for i in range(10001)]
ordered = sorted(ids)
first_inversion = next(
    (i for i, (a, b) in enumerate(zip(ordered, ids)) if a != b),
    None,
)
print("first_inversion_index:", first_inversion)
print("lexicographic_neighbors:", ordered[9998:10003])
print("numeric_neighbors:", ids[9998:10003])
PY

Repository: adamgell/cmtraceopen

Length of output: 335


Sort coverage_id independently of its four-digit padding. coverage_id(index) generates coverage-{index:04}, so coverage-10000 sorts before coverage-9999. Coverage entries can exceed 10,000 because malformed lines and declared header entries are not bounded by resultCap.

🤖 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/portal/macos/company_portal/unified_log/redaction.rs`
around lines 354 - 369, The redact_coverage function currently sorts coverage_id
lexicographically, which misorders IDs once indices exceed the four-digit
padding. Update its sorting comparator to parse and compare the numeric index
portion of each coverage_id independently of zero-padding, while preserving
deterministic ordering and the existing redaction behavior.

Source: Learnings

Comment on lines +198 to +209
for (record, verdict) in records.iter().zip(verdicts.iter_mut()) {
if verdict.reason != PortalSelectionReason::ProcessOnlyInsufficient {
continue;
}
if explicit_ids(record)
.iter()
.any(|id| anchor_ids.contains(id))
{
verdict.selected = true;
verdict.reason = PortalSelectionReason::ActivityLinkedToSelected;
verdict.matched_subsystem = record.subsystem.clone();
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail
rg -n -C 6 'ActivityLinkedToSelected' crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
rg -n -C 6 'matched_subsystem' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs

Repository: adamgell/cmtraceopen

Length of output: 10082


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- linked-record fixture and assertions ---'
sed -n '220,285p' crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
printf '%s\n' '--- PortalSelection consumers and documentation ---'
rg -n -C 5 'matched_subsystem|PortalSelection' crates/cmtraceopen-parser/src crates/cmtraceopen-parser/tests

Repository: adamgell/cmtraceopen

Length of output: 32077


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- fixture record fields ---'
rg -n -C 12 'ul-000002|com\.example\.spoofed|unverified subsystem' crates/cmtraceopen/tests crates/cmtraceopen-parser/tests crates 2>/dev/null
printf '%s\n' '--- all matched-subsystem references ---'
rg -n -C 4 'matched_subsystem|matchedSubsystem' .
printf '%s\n' '--- selection model and reason documentation ---'
sed -n '428,462p' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs

Repository: adamgell/cmtraceopen

Length of output: 43353


🏁 Script executed:

#!/bin/bash
set -euo pipefail
printf '%s\n' '--- all matched_subsystem references ---'
rg -n -C 4 'matched_subsystem|matchedSubsystem' . || true
printf '%s\n' '--- Activity fixture declaration and linked record ---'
rg -n -C 8 'const ACTIVITY|ul-000002|0x1a2b' crates/cmtraceopen-parser/tests/company_portal_macos_unified_log.rs
printf '%s\n' '--- selection model documentation ---'
sed -n '431,460p' crates/cmtraceopen-parser/src/intune/portal/macos/company_portal/unified_log/models.rs

Repository: adamgell/cmtraceopen

Length of output: 13494


Leave matched_subsystem unset for identifier-linked records.

ActivityLinkedToSelected uses an unverified subsystem and identifier linkage. Copying record.subsystem exposes that value through matched_subsystem. Remove the assignment at line 208 and assert that linked records retain None.

🤖 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/portal/macos/company_portal/unified_log/selection.rs`
around lines 198 - 209, In the identifier-linking branch of the records/verdicts
selection loop, remove the assignment to matched_subsystem when setting
ActivityLinkedToSelected. Add or update coverage to assert that
identifier-linked records retain matched_subsystem as None.

@adamgell

adamgell commented Aug 3, 2026

Copy link
Copy Markdown
Owner Author

@copilot Is this ready?

Copilot AI commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

@copilot Is this ready?

Yes. All 17 CI checks on the most recent push pass: Rust MSRV on Ubuntu and Windows, Check & Test (Rust), TypeScript Check, all three platform builds (macOS-arm64, Windows-x64, Linux-x64), ESP Diagnostics (Windows), E2E (Playwright), CodeQL (all five languages), and the Copilot PR reviewer. No check is red or skipped.

Local verification on this commit: 454 lib tests + 27 company_portal_macos_unified_log tests + 222 esp_diagnostics tests — all pass; cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings is clean.

Review-gate: Copilot has returned "generated no new comments" in each of the last four review cycles; CodeRabbit is approved. All six review threads from cycles 2–3 are resolved. The two post-approval fixes (8d6144fd — coverage-id duplication, d8a1edae — IPv6 boundary) each have a test that is red on the reverted code.

@adamgell
adamgell merged commit 2b72003 into main Aug 3, 2026
18 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request feature New feature intune Microsoft Intune related parser Log parser related portal Company Portal related test Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Company Portal macOS: normalize Apple unified-log evidence

3 participants