fix(esp): bind the session export to a redacting crate boundary (#549)

[adamgell]

Closes #549.

The ESP "Export session" button wrote the raw store snapshot to a user-chosen file. redacted_export_projection existed for exactly this, was tested, and was called from nowhere. The UI masked the same values on screen, so a user saw masked values and exported cleartext.

RED first

38c71a32 adds the tests only. Real failure output on that commit:

running 4 tests
test the_exported_session_carries_no_planted_identifier ... FAILED
test the_exported_session_states_that_it_was_redacted ... FAILED
test exporting_does_not_mutate_the_local_session ... ok
test the_exported_session_carries_no_identifier_shaped_string ... FAILED

---- the_exported_session_carries_no_planted_identifier stdout ----
the exported session leaks the user principal name (adele.vance@contoso.onmicrosoft.com)

---- the_exported_session_carries_no_identifier_shaped_string stdout ----
the exported session leaks a MAC address at $.snapshot.activity[0].detail:
  client 192.0.2.77 nic 00:1A:2B:3C:4D:5E

---- the_exported_session_states_that_it_was_redacted stdout ----
assertion `left == right` failed: the export envelope must record that it is a redacted projection
  left: None
 right: Some(Bool(true))

test result: FAILED. 1 passed; 3 failed

TypeScript, same commit:

 × ESP session export boundary > never writes a cleartext session to the chosen file
AssertionError: expected '{\n  "kind": "esp-session-capture",\n...' not to contain
  'adele.vance@contoso.onmicrosoft.com'
 × ESP session export boundary > produces the export through the redacting backend boundary

6fb225fb is the fix. 622137bf adds one more test (a redacted export must still reload as a session, since export is also the replay format).

Where the boundary is, and why there

At the crate edge, in crates/cmtraceopen-parser/src/esp/export.rs.

EspSessionCapture is the only exportable shape for an ESP session:

• its snapshot field is private, and its only constructor from_snapshot runs redacted_export_projection, so no caller outside the crate can assemble a capture holding local values;
• there is deliberately no Deserialize impl and no field-wise constructor. Either would hand back the ability to build a capture around an unprojected snapshot;
• the envelope carries redacted: true. An export that cannot say whether it was redacted is not auditable.

This is the SccmRawEvidenceSnapshot::export arrangement (sccm/evidence.rs:329) rather than the "possible but discouraged" one: the point is that the unprojected value is unreachable from outside, not that the safe call is documented. Per-egress hygiene is what produced this bug, so the fix is not at the file-save call site.

Second, redacted_export_projection now reassembles the snapshot field by field, never with ... A field added to EspDiagnosticsSnapshot stops compiling there until someone decides how it is exported, instead of riding out through the clone() unexamined. That is the property configuration/redaction.rs already had and every other lane lacked.

App surface touched (kept minimal)

• export_esp_session (src-tauri/src/commands/esp_diagnostics.rs) builds the capture and writes it. Added because the projection lives in Rust and the writer must be on the same side of the boundary as the projection.
• exportEspSession in src/lib/commands.ts; EspDiagnosticsWorkspace.exportSession now calls it instead of serializing locally.
• buildEspSessionCapture and serializeEspSessionCapture are deleted from esp-session-capture.ts. Leaving them would leave a working way to write an unredacted capture. That module is now read-only (parse for "Open session"); its tests build the envelope inline.

No reducer semantics changed. No token or scoped-key mechanics introduced (ADR-004 / #550 untouched). No DsRegCmd changes (#556). No expected.json edited; none needed changing.

What the end-to-end test covers

crates/cmtraceopen-parser/tests/esp_export_boundary.rs. It does not call the projection directly: everything goes through exported_session_json, which is the export path (build an EspSessionCapture, serialize it). Two independent guards on the same bytes:

1. Named markers. A UPN, user profile name, tenant domain, tenant id, serial, EntDMID, SID, installer secret, bearer token, IPv4 and MAC are planted across identity, profile, enrollments, sessions, workloads, installer correlations, node cache, registration events, hardware, activity, coverage, raw evidence, and the Graph overlay. Each is asserted absent from the export, and a canary asserts the same marker IS present in the raw snapshot, so the assertions can never pass vacuously.
2. Shape scan. Every string in the exported JSON is walked and matched against mail/SID/MAC/IPv4 shapes, with the JSON path reported on failure. A field that starts carrying an identifier fails here without anyone updating the marker list. The walker asserts it found >50 strings so it cannot silently stop reaching the document.

Plus: the export declares redacted: true; exporting does not mutate the local session; a redacted export still deserializes as a snapshot.

The frontend test (EspDiagnosticsWorkspace.test.tsx, "ESP session export boundary") clicks Export with a UPN in the store and asserts nothing cleartext reaches a file write, and that the export goes through export_esp_session. That is what catches a future frontend bypass; a Rust test cannot see the frontend.

What it does NOT cover, honestly

• A newly added field is only half-caught. The field-by-field reassembly makes a new EspDiagnosticsSnapshot field a compile error, which forces a decision. The shape scan only catches a new field if a test populates it with an identifier shape. A new field carrying an identifier the fixture never sets, in a shape the scan does not know, passes both.
• Free-text bare identifiers still leak. A device serial or DNS tenant domain sitting in narrative text has no label and no distinctive shape, so the redaction rules leave it. They are masked where they are typed fields. This is pinned by bare_identifiers_in_free_text_are_a_known_residual_gap, which asserts the gap so it stays visible rather than assumed closed. Not fixed here: closing it is a redaction-policy change, not a boundary change.
• The shape scan cannot distinguish a dotted quad from a four-part version string. The fixture keeps version strings to three parts on purpose; a real four-part OS build would trip the IPv4 rule.
• No test drives the real Tauri command end to end. The Rust test exercises the capture type the command uses; the TS test asserts the command is invoked. Nothing runs export_esp_session against a real file.
• Replay fidelity drops. An exported capture now replays with masked values and without dropped raw-evidence records. That is the intended consequence of not writing cleartext, but it is a behavior change for the capture/replay workflow.

Egress paths still unprojected after this change

Named explicitly, none of them fixed here:

• analyze_esp_evidence (commands/esp_diagnostics.rs:96) returns EspDiagnosticsSnapshot over IPC unprojected.
• The live session emit stream (commands/esp_diagnostics.rs:67-73, ESP_SESSION_UPDATE_EVENT) sends every full snapshot to the webview unprojected, as does get_esp_diagnostics_session / start_esp_diagnostics_session via EspSessionEnvelope.
• Because of the two above, the frontend store still holds a cleartext snapshot. Anything new that serializes it (a future clipboard action, a devtools dump) is a fresh escape path. This change removes the only such path that existed in the ESP workspace; the ESP workspace has no clipboard copy today.
• intune.rs:64-71 returns IntuneAnalysisResult with no redaction module at all; intune.rs:350-354 writes %TEMP%/cmtrace-guid-diag.log on every analysis run.
• DsregcmdWorkspace.tsx:162 copies the full analysis JSON to the clipboard; intune/ScriptCodeViewer.tsx:213 copies recovered script bodies.

Whether the IPC return and the emit stream should also be projected is the ADR-004 question and is deliberately out of scope.

Gates

cargo test -p cmtraceopen-parser                 2472 passed; 0 failed
cargo clippy -p cmtraceopen-parser --all-targets -- -D warnings   clean
cargo check --locked --manifest-path src-tauri/Cargo.toml --all-targets   clean
npx tsc --noEmit                                 exit 0
npx vitest run                                   733 passed (53 files)
git diff --check                                 exit 0

Recommend running /code-review on this one: it changes a public crate type and a command surface.

🤖 Generated with Claude Code

Summary by CodeRabbit

• New Features

  • Added ESP diagnostics session export to a selected destination.
  • Exports include capture metadata, version information, and a clear redaction indicator.
  • Sensitive identifiers and related text are redacted before export while preserving the original diagnostics data.
  • Exported captures can be serialized, saved, and loaded for later review.
  • Added structured error handling for export and file-writing failures.

• Bug Fixes

  • Prevented cleartext sensitive values from being written during interface-based exports.

• Tests

  • Added coverage for redaction, source preservation, metadata, serialization, file output, and reloadability.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: c950a00a-e564-461b-bc95-36c8e0cf476a

📥 Commits

Reviewing files that changed from the base of the PR and between 7fdf066 and cc98d4d.

📒 Files selected for processing (4)

• crates/cmtraceopen-parser/src/esp/redaction.rs
• crates/cmtraceopen-parser/tests/esp_export_boundary.rs
• src-tauri/src/commands/esp_diagnostics.rs
• src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx

---

📝 Walkthrough

Walkthrough

The PR adds a redacted ESP session capture envelope and moves export construction, serialization, and file writing to a feature-gated Tauri command. The frontend now invokes that command, while the parser retains read-only capture parsing.

Changes

ESP session export

Layer / File(s)	Summary
Redacted capture contract crates/cmtraceopen-parser/src/esp/export.rs, crates/cmtraceopen-parser/src/esp/mod.rs, crates/cmtraceopen-parser/tests/esp_export_boundary.rs	Adds a versioned capture envelope. Construction applies redaction, preserves the source snapshot, supports read-only access and JSON serialization, and tests identifier removal and metadata.
Context-aware redaction projection crates/cmtraceopen-parser/src/esp/redaction.rs, crates/cmtraceopen-parser/tests/esp_export_boundary.rs	Adds shared classified-literal collection and scrubbing across snapshot fields, narrative text, evidence, provenance, and Graph data.
Backend export command src-tauri/src/commands/esp_diagnostics.rs, src-tauri/src/lib.rs, src/lib/commands.ts	Adds the export_esp_session command, structured serialization and write errors, command registration, and the frontend wrapper.
Frontend export handoff src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.tsx, src/workspaces/esp-diagnostics/esp-session-capture.ts, src/workspaces/esp-diagnostics/esp-session-capture.test.ts, src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx	Routes export through the backend. Removes frontend construction and serialization helpers, keeps parsing read-only, and tests backend redaction and parser compatibility.

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

Mergeability Score: 🔵 Low · up to cc98d

Concurrent session exports can collide on the temporary file used during writing, potentially causing one export to overwrite or fail another. The PR is otherwise mergeable with explicit owner awareness or follow-up for this bounded export-path risk.

Sequence Diagram(s)

sequenceDiagram
  participant Workspace as EspDiagnosticsWorkspace
  participant Commands as exportEspSession
  participant Backend as export_esp_session
  participant Capture as EspSessionCapture
  participant File as Destination file
  Workspace->>Commands: submit destination, snapshot, and metadata
  Commands->>Backend: invoke export_esp_session
  Backend->>Capture: construct redacted capture and serialize JSON
  Backend->>File: write serialized capture

Loading

Possibly related issues

• Autopilot export leaves bare serials, tenant domains, and device names in free text #564: Implements related ESP classified-literal collection and free-text scrubbing.

Possibly related PRs

• adamgell/cmtraceopen#389: Shares ESP redaction infrastructure used by the export boundary.
• adamgell/cmtraceopen#402: Modifies shared ESP redaction behavior.
• adamgell/cmtraceopen#460: Reuses shared ESP redaction APIs.

Suggested labels: security, test

🚥 Pre-merge checks | ✅ 4

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title follows Conventional Commits and accurately describes the ESP session export redaction boundary change.
Linked Issues check	✅ Passed	The changes fix the direct issue [#549] by enforcing redaction, adding metadata, and routing frontend exports through the backend command.
Out of Scope Changes check	✅ Passed	The code and tests remain within the linked issue scope for securing ESP session file exports.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch fix/esp-export-boundary

---

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

[coderabbitai]

Actionable comments posted: 4

🤖 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/esp_export_boundary.rs`:
- Around line 17-21: Update the narrative-text export assertions in the test
around bare device serials and tenant domains so both identifiers are absent
from the exported output. Remove the known-residual-gap documentation and rename
or revise bare_identifiers_in_free_text_are_a_known_residual_gap to enforce the
boundary contract, ensuring narrative fields are redacted or omitted rather than
preserving cleartext identifiers.

In `@src-tauri/src/commands/esp_diagnostics.rs`:
- Around line 138-145: Update the export-writing flow around the blocking
std::fs::write call to write contents to a uniquely named temporary file in the
destination directory, then replace the destination only after the temporary
write succeeds. Preserve EspExportError::Write reporting for task, write, and
replacement failures, and use Windows-compatible replacement semantics while
cleaning up any temporary file when appropriate.
- Around line 138-145: Update the ESP export write flow to use
tokio::fs::write(...).await directly instead of
tauri::async_runtime::spawn_blocking with std::fs::write. Preserve both error
mappings in the surrounding command, including distinct messages for the async
task/write failures, and run the required Cargo checks from src-tauri.

In `@src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx`:
- Around line 2488-2503: Update the test around the invoke call assertions so it
explicitly verifies that export_esp_session occurred and write_text_output_file
was not called, rather than iterating over an empty writtenText collection.
Remove the ineffective writtenText redaction loop and leave serialized-file
redaction coverage to the Rust export-boundary tests.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: dd13439f-57dc-47d8-bf25-50a60b4f9a17

📥 Commits

Reviewing files that changed from the base of the PR and between 924efc8 and 622137b.

📒 Files selected for processing (11)

• crates/cmtraceopen-parser/src/esp/export.rs
• crates/cmtraceopen-parser/src/esp/mod.rs
• crates/cmtraceopen-parser/src/esp/redaction.rs
• crates/cmtraceopen-parser/tests/esp_export_boundary.rs
• src-tauri/src/commands/esp_diagnostics.rs
• src-tauri/src/lib.rs
• src/lib/commands.ts
• src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx
• src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.tsx
• src/workspaces/esp-diagnostics/esp-session-capture.test.ts
• src/workspaces/esp-diagnostics/esp-session-capture.ts

[Copilot]

Pull request overview

This PR fixes ESP session export so exported captures are always produced through a redacting parser-crate boundary, preventing cleartext identifiers from being written to user-chosen files (issue #549).

Changes:

• Add a parser-crate export boundary type (EspSessionCapture) that applies redacted_export_projection on construction and records redacted: true in the export envelope.
• Move ESP session export to a new Tauri command (export_esp_session) and update the React workspace to call it (removing frontend-side capture building/serialization).
• Add end-to-end Rust tests guarding against identifier leakage in exported JSON and update frontend tests around the export boundary.

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.tsx	Switch export to backend exportEspSession command to ensure export goes through the redacting boundary.
src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx	Add UI-level tests asserting the export path uses export_esp_session (one assertion currently becomes vacuous; see PR comment).
src/workspaces/esp-diagnostics/esp-session-capture.ts	Remove write capabilities from the frontend capture module; keep parsing/reading only and add redacted? envelope support.
src/workspaces/esp-diagnostics/esp-session-capture.test.ts	Adjust tests to build an envelope inline (since frontend can’t/shouldn’t serialize captures anymore).
src/lib/commands.ts	Add exportEspSession() wrapper invoking the new backend command.
src-tauri/src/lib.rs	Register export_esp_session in the Tauri invoke handler.
src-tauri/src/commands/esp_diagnostics.rs	Implement export_esp_session command that constructs EspSessionCapture and writes its JSON to disk.
crates/cmtraceopen-parser/tests/esp_export_boundary.rs	Add end-to-end Rust tests ensuring exported bytes contain no planted identifiers/identifier-shaped strings and include redacted: true.
crates/cmtraceopen-parser/src/esp/redaction.rs	Make export projection reassemble EspDiagnosticsSnapshot field-by-field to force explicit handling of future fields.
crates/cmtraceopen-parser/src/esp/mod.rs	Export the new export module publicly from the ESP lane.
crates/cmtraceopen-parser/src/esp/export.rs	Add EspSessionCapture / EspSessionCaptureMeta as the enforced exportable capture envelope and serialization API.

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

[adamgell]

Hermes charter review — gate-state report

Reviewed exact remote head 622137bfe839cadc67623f27f97d12d94f553612 using git show origin/fix/esp-export-boundary:PATH; the shared checkout was not changed. This is a gate report, not a merge verdict.

Findings

1. Boundary closure: PARTIAL — the new file-export route is closed; the library-wide claim is not.

   EspSessionCapture itself is sound for the new route: snapshot is private; from_snapshot is the only constructor; it projects before storing; there is no Deserialize; Clone only clones an already-projected capture; and the accessor is immutable. I found no Default, field-wise constructor, or re-export that lets an outside caller create a capture around an unprojected snapshot.

   However, EspDiagnosticsSnapshot remains a public Serialize + Deserialize + Clone type and is directly returned over IPC and embedded in serializable EspSessionEnvelope/EspSessionUpdate values. Therefore the crate still permits outside callers/egress code to serialize an unprojected snapshot; the capture type closes the selected file-export path, not every library egress. That distinction matters because the implementation comment says EspSessionCapture is “the only exportable shape,” which is not literally true for the public snapshot API.

2. Residual free-text gap: HONEST and PINNED, with a bounded claim.

   The disclosure accurately says typed serial/domain values are masked while the same values in narrative text remain clear, and bare_identifiers_in_free_text_are_a_known_residual_gap proves both conditions on the actual export bytes. The canary/shape tests do not accidentally turn this into a vacuous pass. I would not treat this characterization as a redaction-policy failure: it is an explicitly retained residual. It is not an exhaustive proof about every possible untyped identifier, but the PR does not claim that it is.

3. Named residual egress inventory: INCOMPLETE as written.

   The PR correctly identifies analyze_esp_evidence, EspSessionEnvelope/ESP_SESSION_UPDATE_EVENT, IntuneAnalysisResult, %TEMP%/cmtrace-guid-diag.log, DsRegCmd clipboard JSON/raw text, and ScriptCodeViewer script copies. I also found src/workspaces/intune/EventTimelineRow.tsx:108-110, which copies event-derived text via writeText(buildClipboardText(event)); it is another Intune diagnostic egress omitted from the inventory. The broader frontend also has raw-output clipboard paths in MacosDiagProfilesTab.tsx:201-204 and secureboot/RawDataTab.tsx:30-38; if the inventory is intended to cover all diagnostic egresses, those are omitted too. If it is intentionally limited to this PR’s ESP/Intune scope, the comment needs to say so rather than calling the list complete.

4. Field-by-field exhaustiveness: PASS only at the top level; FAIL recursively.

   The EspDiagnosticsSnapshot destructure/rebuild at redaction.rs:693-737 has no struct update and will fail compilation when a top-level snapshot field is added. It does not provide the claimed recursive property. Nested helpers clone nested values and selectively mutate them. For example, redact_graph_overlay does not reassemble nested EspGraph* structs, and it does not redact EspGraphAppRecord.intent_state’s nested GraphSection<EspStatus> data/error. A new field added to those nested types will compile without any redaction decision, and existing nested status/error data can bypass the intended helper coverage. The PR’s claim should be narrowed to top-level fields or the nested projection helpers need the same exhaustive construction discipline.

5. Behavior preservation: PASS for export/replay; NO UNRELATED PRODUCTION DIFF FOUND.

   the_exported_session_still_loads_as_a_session deserializes the exported snapshot and checks schema/workload content. The projection operates on a clone and the preservation test confirms the local snapshot is unchanged. The production diff is confined to the new export route, parser export module, and redaction reconstruction; no reducer, scoped-token, DsRegCmd, or expected.json changes are present. The intended behavior change remains that replay sees redacted values and omitted restricted raw records.

Gate states observed

• Hermes charter review: POSTED by this comment; the recursive exhaustiveness finding remains open.
• CodeRabbit: CHANGES_REQUESTED at this head, including the valid observation that the residual-gap test intentionally asserts a leak, plus unrelated atomic-write/test-quality comments that are outside the stated charter question.
• CI at review time: TypeScript Check was FAILURE; Rust and Windows ESP checks were still IN_PROGRESS in the live PR metadata. CodeQL checks were successful.
• Scope conformance: PASS for the stated exclusions: no scoped-token mechanics, reducer semantic changes, DsRegCmd edits, or expected.json edits.

This review covered the exact PR head, the crate/API boundary, recursive projection/exhaustiveness, named egress inventory, residual characterization, replayability, scope, and live PR gate metadata. It did not treat the acknowledged non-export IPC/clipboard/temp-file paths as fixed by this PR.

[coderabbitai]

Actionable comments posted: 3

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)

crates/cmtraceopen-parser/src/esp/redaction.rs (1)

652-656: 🔒 Security & Privacy | 🟠 Major | 🏗️ Heavy lift

node_cache.expected_value is masked outside the shared walker, so its literal is never scrubbed from free text.

Lines 652-656 mask expected_value unconditionally. That value is a classified value the export refuses to emit, but it is not visited by for_each_masked_classified_mut, so collect_classified_literals never sees it. If an expected_value holds a value that no identity, profile, enrollment, hardware, or Graph classified field also holds, that exact string still survives in every narrative and evidence field.

The fixture hides this: crates/cmtraceopen-parser/tests/esp_export_boundary.rs line 383 sets expected_value to ENTDM_ID, which is already collected from identity.entdm_id. Change the fixture to a node-cache-only value to expose the gap.

This also weakens the invariant stated at lines 627-629 and 749-752. expected_value is a String, not an EspClassifiedString, so it cannot join the walker directly. Collect it separately before masking.

🛡️ Proposed fix: collect node-cache literals alongside classified literals

 fn collect_classified_literals(snapshot: &mut EspDiagnosticsSnapshot) -> ClassifiedLiterals {
     let mut values = BTreeSet::new();
     for_each_masked_classified_mut(snapshot, |classified| {
         values.insert(classified.value.clone());
     });
+    // Masked at the same boundary but not an `EspClassifiedString`, so it
+    // cannot ride the walker above.
+    for node in &snapshot.node_cache {
+        if let Some(expected) = &node.expected_value {
+            values.insert(expected.clone());
+        }
+    }
     ClassifiedLiterals::new(values)
 }

🤖 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/esp/redaction.rs` around lines 652 - 656,
Update the ESP redaction flow around the node-cache masking loop and
collect_classified_literals so each non-empty node.expected_value is added to
the classified-literal set before it is replaced with REDACTED. Preserve the
existing unconditional masking behavior, and update the esp_export_boundary
fixture to use a value unique to node_cache rather than ENTDM_ID so the
free-text scrubbing path is exercised.

🤖 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/esp/redaction.rs`:
- Around line 756-803: Make for_each_masked_classified_mut destructure each
visited EspIdentityEvidence, EspProfileEvidence, EspEnrollmentEvidence,
EspHardwareEvidence, and Graph managed-device value with explicit field bindings
and no `..`. Route the existing classified fields through visit_optional while
binding non-classified fields explicitly, so adding any nested field causes a
compile-time update requirement and all classified fields remain masked.
- Around line 1961-1972: Update redact_graph_overlay to redact all remaining
sensitive fields in graph.device_match.data and graph.autopilot_identity.data,
including the listed device, identity, name, user, and profile identifiers,
using the existing redaction helpers. Also redact GraphSectionError.request_id
and blocked_by alongside the existing error redaction calls, preserving
optional-field handling.

In `@crates/cmtraceopen-parser/tests/esp_export_boundary.rs`:
- Around line 212-234: Add an assertion in
a_degenerate_firmware_serial_does_not_scrub_unrelated_narrative that the
exported identity.serial_number remains masked rather than equal to "0". Keep
the existing narrative assertion unchanged, and use the exported typed-field
path to verify short serials are still scrubbed.

---

Outside diff comments:
In `@crates/cmtraceopen-parser/src/esp/redaction.rs`:
- Around line 652-656: Update the ESP redaction flow around the node-cache
masking loop and collect_classified_literals so each non-empty
node.expected_value is added to the classified-literal set before it is replaced
with REDACTED. Preserve the existing unconditional masking behavior, and update
the esp_export_boundary fixture to use a value unique to node_cache rather than
ENTDM_ID so the free-text scrubbing path is exercised.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: db0184a6-e41b-46d3-b8a6-1021a06667e4

📥 Commits

Reviewing files that changed from the base of the PR and between 622137b and cd72b0e.

📒 Files selected for processing (2)

• crates/cmtraceopen-parser/src/esp/redaction.rs
• crates/cmtraceopen-parser/tests/esp_export_boundary.rs

[coderabbitai]

Actionable comments posted: 7

🤖 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/esp/redaction.rs`:
- Around line 1204-1217: Extend redact_reference after the SID replacement and
before literals.scrub to apply azure_storage_credential_pattern,
ipv4_address_pattern, mac_address_pattern, and redact_ipv6_addresses, matching
the pass order used by redact_text_for_context. Prefer reusing a shared
redaction-prefix helper if one exists or can be introduced without changing
behavior, so both pipelines remain consistent.
- Around line 2030-2052: Update redact_graph_overlay to apply the same redaction
rule to EspGraphOverlay.request_id and GraphSectionError.request_id, ensuring
the overlay-level request ID cannot escape unmasked. If request IDs are
intentionally not sensitive because they are client-generated, document that in
the function’s existing doc comment and preserve the current masking behavior
for error.request_id.
- Around line 1059-1077: Update collect_export_redaction to populate literals by
calling collect_classified_literals(snapshot) during ExportRedaction
construction instead of using ClassifiedLiterals::default(). Remove the separate
caller-side assignment so every constructed ExportRedaction includes the
complete literal set and cannot be left in a fail-open state.
- Around line 681-745: Update redacted_export_projection in
crates/cmtraceopen-parser/src/esp/redaction.rs:681-745 to redact
delivery_optimization transfers and the applicable Graph sections using the
existing narrative/evidence helpers, or document explicitly why the subtree has
no exportable identifiers. Update the export-boundary fixture in
crates/cmtraceopen-parser/tests/esp_export_boundary.rs:412-456 to populate
delivery_optimization with an identified transfer, autopilot_identity, one of
apps/policies/scripts, and a GraphSectionError; add all new markers to
PLANTED_IDENTIFIERS so the guards exercise these paths.

In `@src-tauri/src/commands/esp_diagnostics.rs`:
- Around line 148-156: In the rename error branch, replace the blocking
std::fs::remove_file cleanup with awaited tokio::fs::remove_file(&tmp), matching
the existing async file I/O in the surrounding export flow while preserving the
existing error return.
- Around line 142-147: Update the temporary path construction in
export_esp_session to include a per-invocation unique component in addition to
the process ID, ensuring concurrent exports to the same destination never share
the same .tmp file. Keep the existing atomic write and rename flow unchanged.

In `@src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx`:
- Around line 2506-2519: Replace the duplicate assertion in the test named
“produces the export through the redacting backend boundary” with checks on the
mocked invoke payload. Verify the `export_esp_session` command receives the path
returned by `save()` as `destination` and the currently displayed snapshot and
metadata through the wrapper’s argument shape in `commands.ts`, while leaving
command-name coverage to the first test.

🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: 76d45280-ddb9-49e9-8f4e-08ed4a5c4d35

📥 Commits

Reviewing files that changed from the base of the PR and between cd72b0e and 7fdf066.

📒 Files selected for processing (4)

• crates/cmtraceopen-parser/src/esp/redaction.rs
• crates/cmtraceopen-parser/tests/esp_export_boundary.rs
• src-tauri/src/commands/esp_diagnostics.rs
• src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx