Skip to content

fix(sccm): repair client intake contracts - #432

Merged
adamgell merged 4 commits into
codex/sccm-319-pure-intakefrom
codex/sccm-319-contract-repair
Aug 2, 2026
Merged

fix(sccm): repair client intake contracts#432
adamgell merged 4 commits into
codex/sccm-319-pure-intakefrom
codex/sccm-319-contract-repair

Conversation

@adamgell

@adamgell adamgell commented Aug 1, 2026

Copy link
Copy Markdown
Owner

Summary

Stacked repair for draft PR #394 and issue #319 under epic #317. This branch starts at exact PR #394 head 7af8a3e0454a6b7b11c5e3ea28574dcae77956b7 and keeps the implementation inside the pure, wasm-compatible cmtraceopen-parser crate.

  • makes the shared SCCM catalog authoritative for every SCCM Client: add deterministic intake, coverage, and corpus foundation #319 client intake basename, including seven missing sources and immutable multi-consumer LocationServices.log membership
  • preserves Captured plus fragmentComplete: false as an explicit logical-record boundary, distinct from Capped
  • adds privacy-safe, versioned rotation lineage; binds a lineage to one canonical basename and normalized path fingerprint; rejects duplicate lineage/rotation identities
  • orders fragments by source/path identity, lineage, rotation rank, basename, and artifact ID for deterministic multi-root output
  • allows the exact catalog-bound client.msi.log request while retaining fail-closed rejection for unknown/mismatched identities, roles, globs, and unbounded collection language
  • aligns canonical ccmsetup.log casing across the written plan, fixture corpus, intake membership, and shared catalog

No ParserKind::Sccm, duplicate CCM parser, Windows I/O, registry, WMI, Tauri, network, database, native collector, or public LogEntry change is included.

TDD evidence

Focused regressions were observed red before implementation for:

  • intake basenames bypassing the shared catalog (ExecMgr.log first witness)
  • Captured plus incomplete boundary rejected as InvalidFragmentCompleteness
  • missing rotation_lineage fields at compile time
  • one lineage/rotation relabeled as another root
  • one lineage spanning different path fingerprints across different rotations
  • rotation-first interleaving of two source lineages
  • exact client.msi.log rejected at all four finding boundaries
  • exact shared/intake basename parity failing on ccmsetup.log

All are green at this head.

Fixture matrix

The existing six synthetic/sanitized #319 intake scenarios remain the acceptance corpus: complete, rotations, missing-root, access-denied, capped, and collision. The rotations case now carries one immutable lineage and one path fingerprint across current, .lo_, and numbered fragments; adversarial tests cover cross-root/lineage collisions and input reordering.

Verification

Exact head: 2b68537c7928e9af27f6c5ecda5c362f217d3da7

  • cargo test --locked -p cmtraceopen-parser --test sccm_client_intake — 40 passed
  • cargo test --locked -p cmtraceopen-parser --test sccm_spine_contract — 140 passed
  • cargo test --locked -p cmtraceopen-parser --quiet — 1,078 passed across parser targets
  • cargo clippy --locked -p cmtraceopen-parser --all-targets -- -D warnings — passed
  • cargo check --locked -p cmtraceopen-parser --target wasm32-unknown-unknown — passed
  • npx tsc --noEmit — passed
  • scoped rustfmt --edition 2021 --check for the five changed Rust files — passed
  • fixture JSON and git diff --check — passed; worktree clean
  • local CodeRabbit exact comparison to origin/codex/sccm-319-pure-intake — 0 findings after one valid catalog-casing repair
  • independent exact-head review — passed

cargo fmt --check --all still reports only the documented pre-existing ESP/Tauri formatting drift outside this seven-file stack; none of those files is changed here.

Dependency and acceptance state

The reviewed #318 pure spine dependency is available. This PR remains draft until fresh hosted exact-head review and CI complete. Native SCCM manifest/discovery, native temporary-directory tests, and actual Windows acceptance are still unimplemented; the in-progress SCCM Server lab is not claimed as validation. Issue #319 must remain open.

Summary by CodeRabbit

  • New Features

    • Expanded SCCM client log recognition across setup, policy, application, execution, reporting, and update logs.
    • Added rotation lineage tracking for related log fragments.
    • Improved recognition of multi-part artifact names, including client.msi.
  • Bug Fixes

    • Strengthened validation for duplicate artifacts, inconsistent source identities, and invalid rotation metadata.
    • Improved coverage reporting for incomplete captured log boundaries.
    • Corrected artifact ordering, source matching, and catalog-based artifact classification.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The PR expands the SCCM client catalog, centralizes source membership, adds rotation lineage to client intake records, strengthens identity validation, updates coverage handling, and improves punctuated artifact-name detection.

Changes

SCCM catalog and client intake

Layer / File(s) Summary
Catalog and source membership contract
crates/cmtraceopen-parser/src/sccm/catalog.rs, crates/cmtraceopen-parser/tests/sccm_spine_contract.rs
Adds client artifact entries, shared basename memberships, ccmsetup.log casing, conditional CCM-record declarations, and exact catalog request validation.
Catalog-backed intake source wiring
crates/cmtraceopen-parser/src/sccm/client/intake.rs, crates/cmtraceopen-parser/tests/sccm_client_intake.rs
Adds optional rotation lineage and derives source membership and basename validation from the shared catalog.
Rotation identity and coverage validation
crates/cmtraceopen-parser/src/sccm/client/intake.rs, crates/cmtraceopen-parser/tests/sccm_client_intake.rs, crates/cmtraceopen-parser/tests/fixtures/sccm/client/intake/rotations/*
Validates lineage, fingerprint, basename, and rotation identities. It preserves incomplete captured fragments and reports coverage gaps.
Punctuated artifact identity extraction
crates/cmtraceopen-parser/src/sccm/findings.rs
Recognizes punctuated catalog basename components and sorts and deduplicates returned ranges.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant IntakeArtifact
  participant ClientIntakeValidator
  participant SccmCatalog
  participant FragmentCoverage
  IntakeArtifact->>ClientIntakeValidator: submit source, lineage, fingerprint, and rotation metadata
  ClientIntakeValidator->>SccmCatalog: classify basename and resolve source membership
  SccmCatalog-->>ClientIntakeValidator: return catalog source identity
  ClientIntakeValidator->>FragmentCoverage: assess fragment completeness and coverage
  FragmentCoverage-->>ClientIntakeValidator: return coverage status and gap reason
  ClientIntakeValidator-->>IntakeArtifact: accept or reject declarations
Loading

Possibly related PRs

Suggested labels: test

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Docstring Coverage ✅ Passed Docstring coverage is 83.33% which is sufficient. The required threshold is 80.00%.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly and concisely identifies the primary change: repairing SCCM client intake contracts.
✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch

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

@github-actions github-actions Bot added bug Something isn't working enhancement New feature or request parser Log parser related sccm SCCM/ConfigMgr related labels Aug 1, 2026
@adamgell

adamgell commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai full review

Please perform a substantive full review of exact head 2b68537c7928e9af27f6c5ecda5c362f217d3da7 against stacked base 7af8a3e0454a6b7b11c5e3ea28574dcae77956b7. Focus on fail-closed catalog parity, coverage semantics, lineage/path binding, deterministic source-before-rotation ordering, privacy, and multi-dot request authorization. Do not treat compile-only evidence as approval.

@adamgell

adamgell commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

@coderabbitai review

Please run the full substantive review requested above on exact head 2b68537c7928e9af27f6c5ecda5c362f217d3da7; this explicit command is to override the draft-skip state.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown

@adamgell I will run a substantive full review of #432 at the specified head and stacked base. I will assess the requested runtime contracts, not only compile evidence.

✅ Action performed

Full review finished.

@coderabbitai

coderabbitai Bot commented Aug 1, 2026

Copy link
Copy Markdown
🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

base="7af8a3e0454a6b7b11c5e3ea28574dcae77956b7"
head="2b68537c7928e9af27f6c5ecda5c362f217d3da7"

git cat-file -e "${base}^{commit}"
git cat-file -e "${head}^{commit}"

echo "== Changed files =="
git diff --name-status "$base" "$head"

echo
echo "== Diff summary =="
git diff --stat "$base" "$head"

echo
echo "== Rust symbols in changed Rust files =="
git diff --name-only "$base" "$head" -- '*.rs' |
while IFS= read -r file; do
  echo "--- $file"
  ast-grep outline "$file" --items all --type function,struct,enum,impl,trait,const,static 2>/dev/null || true
done

echo
echo "== Focused identifiers in base-to-head diff =="
git diff --unified=80 "$base" "$head" -- \
  '*.rs' '*.json' |
rg -n -C 8 \
  'Sccm|SCCM|catalog|Catalog|lineage|Lineage|rotation|Rotation|fragmentComplete|Captured|Capped|LocationServices|client\.msi|ccmsetup|request|Request|fingerprint|Fingerprint|sort' || true

Length of output: 147380


🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Intake validation, catalog matching, ordering, and privacy helpers =="
nl -ba crates/cmtraceopen-parser/src/sccm/client/intake.rs | sed -n '430,920p'
nl -ba crates/cmtraceopen-parser/src/sccm/client/intake.rs | sed -n '920,1125p'

echo
echo "== Catalog declarations and parsing =="
nl -ba crates/cmtraceopen-parser/src/sccm/catalog.rs | sed -n '60,580p'
nl -ba crates/cmtraceopen-parser/src/sccm/catalog.rs | sed -n '580,740p'

echo
echo "== Artifact-request authorization implementation =="
rg -n -C 12 --type rust \
  'fn validate_artifact_request|validate_artifact_request\(|catalog_log_stem|exact.*catalog|has_unscoped_broad_collection_scope|requested_artifact_identity_ranges' \
  crates/cmtraceopen-parser/src/sccm

echo
echo "== All intake constructor sites, including non-test callers =="
rg -n -C 5 --type rust \
  'SccmClientIntakeArtifact\s*\{|rotation_lineage|assess_client_intake\s*\(' \
  crates/cmtraceopen-parser

Length of output: 1387


🏁 Script executed:

#!/bin/bash
set -euo pipefail

print_range() {
  local file="$1"
  local start="$2"
  local end="$3"
  echo "== ${file}:${start}-${end} =="
  sed -n "${start},${end}p" "$file" | awk -v n="$start" '{printf "%6d  %s\n", n++, $0}'
}

print_range crates/cmtraceopen-parser/src/sccm/client/intake.rs 430 920
print_range crates/cmtraceopen-parser/src/sccm/client/intake.rs 920 1125
print_range crates/cmtraceopen-parser/src/sccm/catalog.rs 60 580
print_range crates/cmtraceopen-parser/src/sccm/catalog.rs 580 740

echo "== Artifact-request authorization implementation =="
rg -n -C 12 --type rust \
  'fn validate_artifact_request|validate_artifact_request\(|catalog_log_stem|has_unscoped_broad_collection_scope|requested_artifact_identity_ranges' \
  crates/cmtraceopen-parser/src/sccm || true

echo "== All intake constructor and assessment call sites =="
rg -n -C 5 --type rust \
  'SccmClientIntakeArtifact\s*\{|rotation_lineage|assess_client_intake\s*\(' \
  crates/cmtraceopen-parser || true

Length of output: 203012


🏁 Script executed:

#!/bin/bash
set -euo pipefail

print_range() {
  local file="$1"
  local start="$2"
  local end="$3"
  echo "== ${file}:${start}-${end} =="
  sed -n "${start},${end}p" "$file" | awk -v n="$start" '{printf "%6d  %s\n", n++, $0}'
}

print_range crates/cmtraceopen-parser/src/sccm/findings.rs 720 1010
print_range crates/cmtraceopen-parser/src/sccm/findings.rs 1710 2040

echo "== Catalog membership invariant check =="
python3 - <<'PY'
import re
from pathlib import Path

source = Path("crates/cmtraceopen-parser/src/sccm/catalog.rs").read_text()
members_region = source.split("const CLIENT_SOURCE_MEMBERSHIPS:", 1)[1].split("const SOURCE_CATALOG:", 1)[0]
catalog_region = source.split("const SOURCE_CATALOG:", 1)[1].split("pub fn classify_artifact_name", 1)[0]

members = re.findall(r'basename:\s*"([^"]+)",\s*\n\s*logical_artifact_ids:', members_region)
catalog = re.findall(
    r'CatalogSpec\s*\{\s*\n\s*basename:\s*"([^"]+)",\s*\n\s*logical_name:\s*"([^"]+)",\s*\n\s*role:\s*SccmRole::(\w+)',
    catalog_region,
)
client_catalog = {f"{stem}.log" for stem, _, role in catalog if role == "Client"}
member_set = set(members)

print("membership count:", len(members), "unique:", len(member_set))
print("client catalog count:", len(client_catalog))
print("duplicate membership basenames:", sorted(x for x in member_set if members.count(x) > 1))
print("members absent from client catalog:", sorted(member_set - client_catalog))
print("client catalog entries intentionally outside intake:", sorted(client_catalog - member_set))
PY

echo "== Multi-dot request tests and neighboring authorization tests =="
rg -n -C 8 --type rust \
  'multi_dot|client\.msi|unbounded|glob|exact_catalog_identity|ArtifactRequestReason' \
  crates/cmtraceopen-parser/tests/sccm_spine_contract.rs

echo "== Diff integrity =="
git diff --check 7af8a3e0454a6b7b11c5e3ea28574dcae77956b7 2b68537c7928e9af27f6c5ecda5c362f217d3da7

Length of output: 54927


@adamgell Full review complete for exact head 2b68537c7928e9af27f6c5ecda5c362f217d3da7 against stacked base 7af8a3e0454a6b7b11c5e3ea28574dcae77956b7.

I found no actionable issues.

Reviewed areas:

  • Catalog parity: All 30 client intake memberships map to a client catalog entry. The only client catalog entry outside this intake contract is smsts.log. Intake matching requires catalog recognition, matching rotation metadata, and exact canonical filename spelling.
  • Coverage semantics: Captured with fragmentComplete: false remains Captured and produces an explicit boundary gap. Capped with fragmentComplete: true remains rejected.
  • Lineage and path binding: A lineage binds to one canonical basename and one normalized path fingerprint. Reused lineage/rotation identities and cross-root lineage relabeling fail closed.
  • Deterministic ordering: Fragment ordering is path fingerprint, lineage, rotation rank, basename, then artifact ID. The tests cover source-before-rotation ordering and input reordering.
  • Privacy: Lineage accepts only bounded synthetic values or versioned lowercase SHA-256 values. The public intake projection does not include raw host or original-path fields.
  • Multi-dot authorization: client.msi.log is accepted only through its exact catalog identity and matching client role. Unknown aliases, logical-ID or role mismatches, globs, rooted paths, and unbounded collection language are rejected at all tested public boundaries.
  • Diff integrity: git diff --check passed.

I did not treat compilation as approval. I reviewed the implementation and the adversarial regression coverage. The sandbox cannot run repository build or test commands, so I did not independently execute the stated test suite.

✅ Action performed

Full review finished.

@coderabbitai coderabbitai Bot added the test Testing related label Aug 1, 2026

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

Repairs and tightens the pure cmtraceopen-parser SCCM client intake contracts by making the shared SCCM source catalog authoritative for client intake basenames, adding privacy-safe rotation lineage identity, and refining validation + deterministic ordering so multi-root/rotation output remains stable and fail-closed.

Changes:

  • Centralizes client intake basename → logical-group membership in the shared SCCM catalog (including adding missing catalog entries and canonical ccmsetup.log casing), and verifies declared intake basenames are catalog-backed.
  • Introduces optional, validated rotation_lineage to bind rotations to a single canonical basename + normalized path fingerprint and reject lineage/rotation identity collisions.
  • Updates request-reason scope guarding to correctly recognize punctuated catalog identities (e.g., client.msi.log) after tokenization, and expands tests/fixtures accordingly.

Reviewed changes

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

Show a summary per file
File Description
crates/cmtraceopen-parser/src/sccm/catalog.rs Adds authoritative client intake membership list, fills catalog gaps (incl. client.msi), canonicalizes ccmsetup, and marks non-CCM supplemental sources.
crates/cmtraceopen-parser/src/sccm/client/intake.rs Adds rotation_lineage, enforces lineage/fingerprint binding rules, routes group membership through the shared catalog, and updates fragment ordering + gap reasoning.
crates/cmtraceopen-parser/src/sccm/findings.rs Extends bounded-request scope detection to match multi-component catalog identities split by punctuation (e.g., client.msi).
crates/cmtraceopen-parser/tests/sccm_client_intake.rs Adds/updates regression tests for catalog authority, lineage safety/binding, fragment ordering, and captured-incomplete boundary semantics.
crates/cmtraceopen-parser/tests/sccm_spine_contract.rs Adds contract tests ensuring exact client.msi.log requests are accepted while multi-dot scope remains fail-closed when unauthorized; updates expected catalog tuples.
crates/cmtraceopen-parser/tests/fixtures/sccm/client/intake/rotations/manifest.json Updates synthetic rotation fixture to use stable path fingerprints and explicit lineage IDs across rotations.
crates/cmtraceopen-parser/tests/fixtures/sccm/client/intake/rotations/expected.json Updates expected output to reflect shared fingerprint + rotation lineage fields and revised artifact IDs.

@adamgell

adamgell commented Aug 1, 2026

Copy link
Copy Markdown
Owner Author

Final #319 review checkpoint for exact stacked range 7af8a3e0454a6b7b11c5e3ea28574dcae77956b7..2b68537c7928e9af27f6c5ecda5c362f217d3da7:

  • Hosted CodeRabbit completed its requested substantive review with no actionable findings: fix(sccm): repair client intake contracts #432 (comment)
  • GitHub Copilot reviewed all 7 changed files at the exact head and generated no comments: fix(sccm): repair client intake contracts #432 (review)
  • A separate detached source audit found no actionable issues and independently reran: client intake 40/40, SCCM spine 140/140, full parser suite 1,078 tests, wasm32 check, strict parser Clippy across all targets, TypeScript check, changed-file rustfmt, JSON fixture parsing, and git diff --check — all green.

The only formatting caveat is the already-recorded repository-wide cargo fmt --all -- --check drift in unrelated files inherited from the stacked base; every Rust file in this PR range passes direct rustfmt --check.

PR remains draft. No merge, issue closure, or live Windows acceptance claim is being made; native manifest/discovery/temp-directory/ACL/rotation validation remains the explicit downstream gate.

@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

🧹 Nitpick comments (2)
crates/cmtraceopen-parser/src/sccm/catalog.rs (1)

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

Consider deriving uses_ccm_records from data instead of a hardcoded name match.

catalog_entry_uses_ccm_records hardcodes the exclusion list by logical_name string. Adding a future non-CCM supplement requires remembering to update this function separately from the CatalogSpec entry itself. Consider adding a uses_ccm_records: bool field directly to CatalogSpec so the flag is declared alongside each entry instead of inferred by string match elsewhere.

Existing tests (sccm_spine_contract.rs, sccm_client_intake.rs) already assert this flag per basename, so a rename would be caught, which limits the practical risk today.

🤖 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/sccm/catalog.rs` around lines 627 - 634, Add a
`uses_ccm_records` boolean field to `CatalogSpec`, populate it explicitly for
every catalog entry, and update catalog construction to read this field
directly. Remove `catalog_entry_uses_ccm_records` and the logical-name exclusion
matching, preserving the existing values for `clientMsi`, `reportingEvents`, and
all other entries.
crates/cmtraceopen-parser/src/sccm/client/intake.rs (1)

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

Hardcoded multi-group exception only covers LocationServices.log.

The _ arm assumes any basename with more than one matching group must be LocationServices.log mapped to "client-location-services-shared". If a future catalog membership adds a second multi-group basename, this function rejects every valid group assignment for it except that one hardcoded string.

Consider deriving the expected shared-group name from the matching groups themselves, or asserting on the membership's logical_artifact_ids directly, instead of a single hardcoded basename check.

🤖 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/sccm/client/intake.rs` around lines 1044 -
1058, Update is_expected_client_bundle_group’s multi-match arm to derive the
accepted shared group from the matching groups or the catalogued membership’s
logical_artifact_ids, rather than hardcoding “LocationServices.log” and
“client-location-services-shared”. Preserve the existing behavior for zero and
single matching groups while allowing future multi-group basenames to validate
their corresponding shared-group assignment.
🤖 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/sccm/client/intake.rs`:
- Around line 162-166: Update the rotation-boundary fixtures in
deployment/rotation-boundary/manifest.json and
task_sequence/rotation-boundary/manifest.json so both declarations sharing the
repeated pathFingerprint, current and lo, include the same rotation.lineageId
value. Preserve the existing fingerprint and other manifest fields.

---

Nitpick comments:
In `@crates/cmtraceopen-parser/src/sccm/catalog.rs`:
- Around line 627-634: Add a `uses_ccm_records` boolean field to `CatalogSpec`,
populate it explicitly for every catalog entry, and update catalog construction
to read this field directly. Remove `catalog_entry_uses_ccm_records` and the
logical-name exclusion matching, preserving the existing values for `clientMsi`,
`reportingEvents`, and all other entries.

In `@crates/cmtraceopen-parser/src/sccm/client/intake.rs`:
- Around line 1044-1058: Update is_expected_client_bundle_group’s multi-match
arm to derive the accepted shared group from the matching groups or the
catalogued membership’s logical_artifact_ids, rather than hardcoding
“LocationServices.log” and “client-location-services-shared”. Preserve the
existing behavior for zero and single matching groups while allowing future
multi-group basenames to validate their corresponding shared-group assignment.
🪄 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: 06c2dd6b-67a7-4268-bbcd-4b52736d121f

📥 Commits

Reviewing files that changed from the base of the PR and between 7af8a3e and 2b68537.

📒 Files selected for processing (7)
  • crates/cmtraceopen-parser/src/sccm/catalog.rs
  • crates/cmtraceopen-parser/src/sccm/client/intake.rs
  • crates/cmtraceopen-parser/src/sccm/findings.rs
  • crates/cmtraceopen-parser/tests/fixtures/sccm/client/intake/rotations/expected.json
  • crates/cmtraceopen-parser/tests/fixtures/sccm/client/intake/rotations/manifest.json
  • crates/cmtraceopen-parser/tests/sccm_client_intake.rs
  • crates/cmtraceopen-parser/tests/sccm_spine_contract.rs

Comment thread crates/cmtraceopen-parser/src/sccm/client/intake.rs
@adamgell
adamgell marked this pull request as ready for review August 2, 2026 01:12
@adamgell
adamgell merged commit e6328ac into codex/sccm-319-pure-intake Aug 2, 2026
3 checks passed
adamgell added a commit that referenced this pull request Aug 2, 2026
* test(sccm): define pure client intake contract

* feat(sccm): define pure client intake coverage

* test(sccm): harden client intake provenance boundaries

* feat(sccm): validate client intake provenance

* test(sccm): reject unsafe client intake provenance

RED: cargo test --locked -p cmtraceopen-parser --test sccm_client_intake -- --nocapture ran 13 tests: 11 passed and 2 intended privacy regressions failed.

* fix(sccm): bound client intake provenance paths

* test(sccm): pin bounded intake path namespaces

REVIEW RED: focused client intake ran 13 tests: 12 passed and the identity-slot bypass regression failed.

* fix(sccm): allowlist client intake path namespaces

* test(sccm): reject impossible intake timestamp paths

CODERABBIT RED: focused client intake ran 13 tests: 12 passed and the impossible timestamp regression failed.

* fix(sccm): validate intake timestamp path values

* test(sccm): reject unnamespaced path fingerprints

* fix(sccm): namespace client path fingerprints

* test(sccm): reject identity-bearing synthetic handles

* fix(sccm): bound synthetic fingerprint vocabulary

* test(sccm): reject numeric synthetic identities

* fix(sccm): constrain synthetic numeric markers

* test(sccm): reject free-form intake metadata

* fix(sccm): bound intake metadata vocabularies

* test(sccm): reject free-form unknown rotations

* fix(sccm): require opaque unknown rotations

* test(sccm): close intake identity channels

* fix(sccm): bind client intake identities

* test(sccm): preserve shared location rotations

* fix(sccm): bind shared location rotations

* test(sccm): tighten client intake review regressions

* fix(sccm): tighten client path fingerprint grammar

* test(sccm): reject complete capped client fragments

* fix(sccm): fail closed on complete capped fragments

* test(sccm): expose escaped client path probe gap

* test(sccm): harden client intake privacy probes

* test(sccm): expose client intake coverage contradictions

The client intake contract accepts three contradictions. A
Captured fragment admitting fragmentComplete=false passes while
the Capped mirror is rejected. A mixed group with one captured
fragment and one Absent sibling marker reports group coverage
Absent with a gap claiming no artifact was supplied, while the
same group still serializes the captured fragment, because
coverage_rank lets marker states outrank Captured and no test
pins mixed-state groups. Two Absent markers for the same source
under distinct caller labels are both accepted and project as
two fragments because canonical-identity dedup only covers
physical states.

Add failing tests pinning the intended semantics: Captured plus
incomplete fails closed via InvalidFragmentCompleteness, mixed
groups keep the physical coverage and emit per-source gaps that
name the absent or denied source, a capped mix keeps both the
group gap and the per-source gap, and duplicate non-physical
marker identities fail closed via DuplicateArtifactId.

Refs #319

* fix(sccm): keep mixed-group client captures coherent

Before this change the client intake accepted a Captured
fragment that admitted fragmentComplete=false, let coverage_rank
place Absent above Captured so a mixed group diagnosed itself as
unsupplied while serializing the captured fragment, and let two
non-physical markers double-declare the same source under
distinct caller labels.

Reject Captured plus incomplete via InvalidFragmentCompleteness,
mirroring the Capped plus complete rule, since Capped exists to
represent an incomplete capture. Compute group coverage from the
physical fragments whenever any exist and emit an explicit
per-source gap for each sibling marker that names the affected
source, so absence stays a diagnosis without erasing captured
evidence. Extend canonical-identity dedup to non-physical
markers keyed on basename, rotation, and optional path
fingerprint, mirroring the duplicate-identity idiom the server
lane uses for physical states; markers distinguished by explicit
fingerprints remain distinct sources.

Also document that physicalArtifacts intentionally includes
non-physical markers and that markers may carry a path
fingerprint, both flagged by review as worth stating as
intended.

Refs #319

* test(sccm): probe forward-slash user-root leak detection

The serialized privacy probe only matches the JSON-escaped
backslash form c:\\users, so a forward-slash normalized leak
such as c:/users/... with the RealUser sentinel stripped passes
every serialized privacy assertion undetected.

Add a failing meta-test that feeds the probe a forward-slash
normalized Windows user path and requires detection, per the
non-blocking hardening in the exact-head review.

Refs #319

* fix(sccm): match normalized user roots in privacy probe

The probe helper only recognized the JSON-escaped c:\\users
form, leaving forward-slash normalized Windows user paths
invisible to every serialized privacy assertion when the
RealUser sentinel is absent.

Also match the normalized c:/users form so either separator
style trips the probe, closing the review's non-blocking
hardening gap.

Refs #319

* test(sccm): expose marker collisions with physical sources

A fingerprint-less absent marker for a source that is also
physically declared is accepted, and the assessment then claims
no artifact for the source was supplied while serializing the
captured fragment, the exact self-contradiction class round 6
blocked on. The same root lets an unpinned marker and a
fingerprint-pinned marker double-declare one source, and lets a
pinned marker contradict physical evidence for its own source.

Add failing tests requiring the canonical source identity,
basename plus rotation, to intersect across all declarations:
a non-physical marker never shares basename and rotation with a
physical declaration regardless of fingerprints, an unpinned
marker collides with any other declaration for its source, and
both directions hold for either declaration order. Pin the
accepted direction, a marker for a genuinely distinct rotation
alongside the capture, and pin the intentionally two-valued
ParseFailed fragment completeness the review listed as the last
unpinned matrix cell. Document the client/server fingerprint
asymmetry as a follow-up.

Refs #319

* fix(sccm): intersect source identity across declarations

The marker identity set keyed on basename, rotation, and the
optional path fingerprint never intersected physical
declarations, so a fingerprint-less absent marker for a captured
source was accepted and the assessment asserted a coverage gap
its own fragments array disproved. The same root let an unpinned
marker and a fingerprint-pinned marker double-declare one
source, and let a pinned marker contradict physical evidence for
its own source whenever the fingerprints differed.

Track the canonical source identity, casefolded basename plus
rotation discriminator, for every declaration and intersect it
across all declaration shapes. A non-physical marker sharing its
source identity with a physical declaration fails closed with
CollidingPhysicalIdentity in either declaration order,
regardless of fingerprints, because physical evidence disproves
any absent, denied, or skipped claim about the same source. A
fingerprint-less marker collides with any other declaration for
its source via DuplicateArtifactId, while markers pinned to
distinct roots by distinct fingerprints stay distinct sources
when no physical declaration exists. Physical declarations keep
their existing fingerprint and relative-path dedup, so distinct
captured rotations and root collisions stay representable.

The sibling server intake closes this class by making the path
fingerprint mandatory everywhere; the client keeps optional
marker fingerprints for the committed all-absent fixtures, with
convergence documented as a follow-up in the intake tests.

Refs #319

* test(sccm): pin exact client intake error variants

Ten assertions checked only is_err(). SccmClientIntakeError has 14
variants, so each passed when any validator rejected the bundle,
including for a reason unrelated to the behavior under test. Each
now compares against the exact expected variant.

Pinning exposed two inputs that rejected for the wrong reason:

malformed_rotation_and_public_provenance_values_fail_closed declared
SccmRotation::Timestamped("2026-bad") to exercise the rotation
grammar, but synthetic_artifact derived an "unknown" group segment
from the rotated basename. The bundle failed on InvalidRelativePath
before reaching the rotation check, so the rotation contract was
never exercised. The artifact now carries a consistent relative path
and the malformed timestamp is the only contract it violates. The
rotation grammar itself lives in the SccmRotation Serialize impl,
which validate_bundle reaches through serde_json::to_value.

fragment_completeness_and_every_path_fingerprint_are_explicit_and_
unambiguous asserted that an AccessDenied marker with no relative
path fails its provenance contract. AccessDenied is not a physical
state, so MissingPhysicalProvenance is unreachable for that input;
the marker actually fails on InvalidFragmentCompleteness because it
retains the default fragmentComplete=true. The assertion now pins
that variant, and a new physical case with its relative path
stripped covers MissingPhysicalProvenance directly.

Refs #319

* refactor(sccm): own the sha256 digest width in one helper

is_lowercase_hex_handle served both 16-character root handles and
64-character SHA-256 digests, so every digest caller had to restate
its own length guard. Four call sites repeated it, and a new caller
that omitted it would silently accept a 16-character handle.

is_sha256_digest now owns the digest width and the four digest
callers use it. is_lowercase_hex_handle keeps its 16-or-64 root
grammar unchanged; narrowing the root contract would be a behavior
change, not a refactor. Both delegate to a shared is_lowercase_hex.

Refs #319

* test(sccm): casefold every serialized leak assertion

unsupported_physical_artifacts_retain_safe_provenance_without_raw_
host_or_path compared against the original-case serialized JSON, so
a projection that normalized case would leak RealUser or
real-user-host and still pass. It also used bare substring checks,
which miss the JSON-escaped and forward-slash normalized Windows
user roots that carry no sentinel.

Swept every serialized-leak assertion in the file. Two sites exist.
The first already casefolded and routed through
serialized_json_contains_windows_user_root; this commit brings the
second to the same shape. Positive assertions keep their exact case
on purpose, with a comment saying why, because they pin what the
projection must reproduce verbatim.

This class has now surfaced twice, so the contract is enforced
rather than documented: the helper carries a doc comment stating the
rule and a debug_assert that rejects a caller passing original-case
input. Verified the guard fires by temporarily feeding it the
uncased string.

Siblings were checked and are not this class.
sccm_client_intake_fixture_contract asserts a fixture lacks a [cut]
marker, and sccm_spine_contract asserts redaction of parsed message
strings, a surface that must retain C:\Windows\CCM\Logs verbatim.

Refs #319

* fix(sccm): preserve partial client intake coverage

* fix(sccm): repair client intake contracts (#432)

* fix(sccm): repair client intake contracts (#319)

* fix(sccm): bind multi-dot catalog requests (#319)

* fix(sccm): bind and order client source lineages (#319)

* fix(sccm): align client setup catalog identity (#319)

* test(sccm): construct invalid requests without serialization

* test(sccm): expose client intake wire gaps

* fix(sccm): validate client intake wire projections

* docs(sccm): align client intake delivery state

* test(sccm): expose incomplete client intake oracle

* test(sccm): bind complete client intake fixture oracle

* docs(sccm): enumerate client coverage states

* test(sccm): separate parse failure from fragment bounds

* fix(sccm): distinguish parse failure from fragment bounds

* docs(sccm): close client intake review nits

* docs(sccm): clarify client intake fixture contracts

* fix(sccm): stabilize client intake wire shape
adamgell added a commit that referenced this pull request Aug 4, 2026
* test(sccm): define pure client intake contract

* feat(sccm): define pure client intake coverage

* test(sccm): harden client intake provenance boundaries

* feat(sccm): validate client intake provenance

* test(sccm): reject unsafe client intake provenance

RED: cargo test --locked -p cmtraceopen-parser --test sccm_client_intake -- --nocapture ran 13 tests: 11 passed and 2 intended privacy regressions failed.

* fix(sccm): bound client intake provenance paths

* test(sccm): pin bounded intake path namespaces

REVIEW RED: focused client intake ran 13 tests: 12 passed and the identity-slot bypass regression failed.

* fix(sccm): allowlist client intake path namespaces

* test(sccm): reject impossible intake timestamp paths

CODERABBIT RED: focused client intake ran 13 tests: 12 passed and the impossible timestamp regression failed.

* fix(sccm): validate intake timestamp path values

* test(sccm): reject unnamespaced path fingerprints

* fix(sccm): namespace client path fingerprints

* test(sccm): reject identity-bearing synthetic handles

* fix(sccm): bound synthetic fingerprint vocabulary

* test(sccm): reject numeric synthetic identities

* fix(sccm): constrain synthetic numeric markers

* test(sccm): reject free-form intake metadata

* fix(sccm): bound intake metadata vocabularies

* test(sccm): reject free-form unknown rotations

* fix(sccm): require opaque unknown rotations

* test(sccm): close intake identity channels

* fix(sccm): bind client intake identities

* test(sccm): preserve shared location rotations

* fix(sccm): bind shared location rotations

* test(sccm): tighten client intake review regressions

* fix(sccm): tighten client path fingerprint grammar

* test(sccm): reject complete capped client fragments

* fix(sccm): fail closed on complete capped fragments

* test(sccm): expose escaped client path probe gap

* test(sccm): harden client intake privacy probes

* test(sccm): expose client intake coverage contradictions

The client intake contract accepts three contradictions. A
Captured fragment admitting fragmentComplete=false passes while
the Capped mirror is rejected. A mixed group with one captured
fragment and one Absent sibling marker reports group coverage
Absent with a gap claiming no artifact was supplied, while the
same group still serializes the captured fragment, because
coverage_rank lets marker states outrank Captured and no test
pins mixed-state groups. Two Absent markers for the same source
under distinct caller labels are both accepted and project as
two fragments because canonical-identity dedup only covers
physical states.

Add failing tests pinning the intended semantics: Captured plus
incomplete fails closed via InvalidFragmentCompleteness, mixed
groups keep the physical coverage and emit per-source gaps that
name the absent or denied source, a capped mix keeps both the
group gap and the per-source gap, and duplicate non-physical
marker identities fail closed via DuplicateArtifactId.

Refs #319

* fix(sccm): keep mixed-group client captures coherent

Before this change the client intake accepted a Captured
fragment that admitted fragmentComplete=false, let coverage_rank
place Absent above Captured so a mixed group diagnosed itself as
unsupplied while serializing the captured fragment, and let two
non-physical markers double-declare the same source under
distinct caller labels.

Reject Captured plus incomplete via InvalidFragmentCompleteness,
mirroring the Capped plus complete rule, since Capped exists to
represent an incomplete capture. Compute group coverage from the
physical fragments whenever any exist and emit an explicit
per-source gap for each sibling marker that names the affected
source, so absence stays a diagnosis without erasing captured
evidence. Extend canonical-identity dedup to non-physical
markers keyed on basename, rotation, and optional path
fingerprint, mirroring the duplicate-identity idiom the server
lane uses for physical states; markers distinguished by explicit
fingerprints remain distinct sources.

Also document that physicalArtifacts intentionally includes
non-physical markers and that markers may carry a path
fingerprint, both flagged by review as worth stating as
intended.

Refs #319

* test(sccm): probe forward-slash user-root leak detection

The serialized privacy probe only matches the JSON-escaped
backslash form c:\\users, so a forward-slash normalized leak
such as c:/users/... with the RealUser sentinel stripped passes
every serialized privacy assertion undetected.

Add a failing meta-test that feeds the probe a forward-slash
normalized Windows user path and requires detection, per the
non-blocking hardening in the exact-head review.

Refs #319

* fix(sccm): match normalized user roots in privacy probe

The probe helper only recognized the JSON-escaped c:\\users
form, leaving forward-slash normalized Windows user paths
invisible to every serialized privacy assertion when the
RealUser sentinel is absent.

Also match the normalized c:/users form so either separator
style trips the probe, closing the review's non-blocking
hardening gap.

Refs #319

* test(sccm): expose marker collisions with physical sources

A fingerprint-less absent marker for a source that is also
physically declared is accepted, and the assessment then claims
no artifact for the source was supplied while serializing the
captured fragment, the exact self-contradiction class round 6
blocked on. The same root lets an unpinned marker and a
fingerprint-pinned marker double-declare one source, and lets a
pinned marker contradict physical evidence for its own source.

Add failing tests requiring the canonical source identity,
basename plus rotation, to intersect across all declarations:
a non-physical marker never shares basename and rotation with a
physical declaration regardless of fingerprints, an unpinned
marker collides with any other declaration for its source, and
both directions hold for either declaration order. Pin the
accepted direction, a marker for a genuinely distinct rotation
alongside the capture, and pin the intentionally two-valued
ParseFailed fragment completeness the review listed as the last
unpinned matrix cell. Document the client/server fingerprint
asymmetry as a follow-up.

Refs #319

* fix(sccm): intersect source identity across declarations

The marker identity set keyed on basename, rotation, and the
optional path fingerprint never intersected physical
declarations, so a fingerprint-less absent marker for a captured
source was accepted and the assessment asserted a coverage gap
its own fragments array disproved. The same root let an unpinned
marker and a fingerprint-pinned marker double-declare one
source, and let a pinned marker contradict physical evidence for
its own source whenever the fingerprints differed.

Track the canonical source identity, casefolded basename plus
rotation discriminator, for every declaration and intersect it
across all declaration shapes. A non-physical marker sharing its
source identity with a physical declaration fails closed with
CollidingPhysicalIdentity in either declaration order,
regardless of fingerprints, because physical evidence disproves
any absent, denied, or skipped claim about the same source. A
fingerprint-less marker collides with any other declaration for
its source via DuplicateArtifactId, while markers pinned to
distinct roots by distinct fingerprints stay distinct sources
when no physical declaration exists. Physical declarations keep
their existing fingerprint and relative-path dedup, so distinct
captured rotations and root collisions stay representable.

The sibling server intake closes this class by making the path
fingerprint mandatory everywhere; the client keeps optional
marker fingerprints for the committed all-absent fixtures, with
convergence documented as a follow-up in the intake tests.

Refs #319

* test(sccm): pin exact client intake error variants

Ten assertions checked only is_err(). SccmClientIntakeError has 14
variants, so each passed when any validator rejected the bundle,
including for a reason unrelated to the behavior under test. Each
now compares against the exact expected variant.

Pinning exposed two inputs that rejected for the wrong reason:

malformed_rotation_and_public_provenance_values_fail_closed declared
SccmRotation::Timestamped("2026-bad") to exercise the rotation
grammar, but synthetic_artifact derived an "unknown" group segment
from the rotated basename. The bundle failed on InvalidRelativePath
before reaching the rotation check, so the rotation contract was
never exercised. The artifact now carries a consistent relative path
and the malformed timestamp is the only contract it violates. The
rotation grammar itself lives in the SccmRotation Serialize impl,
which validate_bundle reaches through serde_json::to_value.

fragment_completeness_and_every_path_fingerprint_are_explicit_and_
unambiguous asserted that an AccessDenied marker with no relative
path fails its provenance contract. AccessDenied is not a physical
state, so MissingPhysicalProvenance is unreachable for that input;
the marker actually fails on InvalidFragmentCompleteness because it
retains the default fragmentComplete=true. The assertion now pins
that variant, and a new physical case with its relative path
stripped covers MissingPhysicalProvenance directly.

Refs #319

* refactor(sccm): own the sha256 digest width in one helper

is_lowercase_hex_handle served both 16-character root handles and
64-character SHA-256 digests, so every digest caller had to restate
its own length guard. Four call sites repeated it, and a new caller
that omitted it would silently accept a 16-character handle.

is_sha256_digest now owns the digest width and the four digest
callers use it. is_lowercase_hex_handle keeps its 16-or-64 root
grammar unchanged; narrowing the root contract would be a behavior
change, not a refactor. Both delegate to a shared is_lowercase_hex.

Refs #319

* test(sccm): casefold every serialized leak assertion

unsupported_physical_artifacts_retain_safe_provenance_without_raw_
host_or_path compared against the original-case serialized JSON, so
a projection that normalized case would leak RealUser or
real-user-host and still pass. It also used bare substring checks,
which miss the JSON-escaped and forward-slash normalized Windows
user roots that carry no sentinel.

Swept every serialized-leak assertion in the file. Two sites exist.
The first already casefolded and routed through
serialized_json_contains_windows_user_root; this commit brings the
second to the same shape. Positive assertions keep their exact case
on purpose, with a comment saying why, because they pin what the
projection must reproduce verbatim.

This class has now surfaced twice, so the contract is enforced
rather than documented: the helper carries a doc comment stating the
rule and a debug_assert that rejects a caller passing original-case
input. Verified the guard fires by temporarily feeding it the
uncased string.

Siblings were checked and are not this class.
sccm_client_intake_fixture_contract asserts a fixture lacks a [cut]
marker, and sccm_spine_contract asserts redaction of parsed message
strings, a surface that must retain C:\Windows\CCM\Logs verbatim.

Refs #319

* fix(sccm): preserve partial client intake coverage

* fix(sccm): repair client intake contracts (#432)

* fix(sccm): repair client intake contracts (#319)

* fix(sccm): bind multi-dot catalog requests (#319)

* fix(sccm): bind and order client source lineages (#319)

* fix(sccm): align client setup catalog identity (#319)

* test(sccm): construct invalid requests without serialization

* test(sccm): expose client intake wire gaps

* fix(sccm): validate client intake wire projections

* docs(sccm): align client intake delivery state

* test(sccm): expose incomplete client intake oracle

* test(sccm): bind complete client intake fixture oracle

* docs(sccm): enumerate client coverage states

* test(sccm): separate parse failure from fragment bounds

* fix(sccm): distinguish parse failure from fragment bounds

* docs(sccm): close client intake review nits

* docs(sccm): clarify client intake fixture contracts

* fix(sccm): stabilize client intake wire shape
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Something isn't working enhancement New feature or request parser Log parser related sccm SCCM/ConfigMgr related test Testing related

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants