From 2e5e523ceaab56e780094b5de098a7a151637d34 Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 11 Aug 2026 12:09:49 -0400 Subject: [PATCH 01/10] test(esp): prove the session export writes cleartext today (#549) RED. Two end-to-end guards on the path the "Export session" button actually takes, both failing on main: - crates/cmtraceopen-parser/tests/esp_export_boundary.rs serializes a session through the same envelope esp-session-capture.ts writes and asserts no planted identifier survives. A canary asserts each marker IS present in the raw snapshot so the assertions can never pass vacuously, and a shape scan walks every string in the export for mail/SID/MAC/IPv4 shapes so a newly added identifier-bearing field fails even when no marker was updated. - EspDiagnosticsWorkspace.test.tsx clicks Export with a UPN in the store and asserts nothing cleartext reaches a file write, and that the export is produced by a redacting backend command rather than by the frontend. Failing output: the exported session leaks the user principal name the exported session leaks a MAC address at $.snapshot.activity[0].detail the export envelope must record that it is a redacted projection expected '{"kind": "esp-session-capture"...}' not to contain 'adele.vance@contoso.onmicrosoft.com' Co-Authored-By: Claude Opus 4.8 --- .../tests/esp_export_boundary.rs | 476 ++++++++++++++++++ .../EspDiagnosticsWorkspace.test.tsx | 61 +++ 2 files changed, 537 insertions(+) create mode 100644 crates/cmtraceopen-parser/tests/esp_export_boundary.rs diff --git a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs new file mode 100644 index 000000000..85a12f44d --- /dev/null +++ b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs @@ -0,0 +1,476 @@ +//! End-to-end guard for the ESP session export boundary (issue #549). +//! +//! These tests do not exercise `redacted_export_projection` directly. They +//! serialize a session the way the application's "Export session" button +//! writes it, through [`exported_session_json`], and assert that no local +//! identifier survives into the bytes that reach the user-chosen file. +//! +//! Two independent guards run against the same serialized text: +//! +//! 1. Named markers. Every sensitive value planted in the snapshot is asserted +//! absent, and a canary asserts the same markers ARE present when the raw +//! snapshot is serialized, so the test can never pass vacuously. +//! 2. Shape scan. Every string in the exported JSON is matched against +//! identifier shapes (mail address, SID, MAC, IPv4). A field that starts +//! carrying an identifier fails here even if no named marker was updated. + +use cmtraceopen_parser::esp::*; +use regex::Regex; +use serde_json::Value; + +const CAPTURED_AT: &str = "2026-08-11T09:15:00Z"; + +// Synthetic values. Nothing here belongs to a real tenant or device. +const UPN: &str = "adele.vance@contoso.onmicrosoft.com"; +const PROFILE_USER: &str = "adele.vance"; +const TENANT_DOMAIN: &str = "contoso.onmicrosoft.com"; +const TENANT_ID: &str = "8f9b2b41-1c0d-4f3a-9a1b-7d2e5c6f8a90"; +const SERIAL: &str = "5CD9SYNTH01"; +const ENTDM_ID: &str = "ENTDM-SYNTH-0001"; +const USER_SID: &str = "S-1-5-21-1111111111-2222222222-3333333333-1001"; +const INSTALL_SECRET: &str = "Sup3rSyntheticSecret"; +const BEARER_TOKEN: &str = "eyJzeW50aGV0aWMtdG9rZW4"; +const CLIENT_IPV4: &str = "192.0.2.77"; +const NIC_MAC: &str = "00:1A:2B:3C:4D:5E"; + +/// Every planted value that must not reach an exported file. +const PLANTED_IDENTIFIERS: &[(&str, &str)] = &[ + ("user principal name", UPN), + ("user profile name", PROFILE_USER), + ("tenant domain", TENANT_DOMAIN), + ("tenant id", TENANT_ID), + ("device serial number", SERIAL), + ("EntDMID", ENTDM_ID), + ("user SID", USER_SID), + ("installer secret", INSTALL_SECRET), + ("bearer token", BEARER_TOKEN), + ("client IPv4 address", CLIENT_IPV4), + ("NIC MAC address", NIC_MAC), +]; + +/// Serialize a session exactly the way the "Export session" button writes it. +/// +/// This is the seam the whole test file turns on: it must stay the real export +/// path, never a direct call to the redaction module, or these tests stop +/// proving anything about what lands on disk. +/// +/// Today that path is `buildEspSessionCapture` + +/// `serializeEspSessionCapture` in +/// `src/workspaces/esp-diagnostics/esp-session-capture.ts`, which embeds the +/// store snapshot unmodified and hands the text to `write_text_output_file`. +/// The envelope below is that exact shape. +fn exported_session_json(snapshot: &EspDiagnosticsSnapshot) -> String { + serde_json::to_string_pretty(&serde_json::json!({ + "kind": "esp-session-capture", + "version": 1, + "capturedAtUtc": CAPTURED_AT, + "app": { "version": null, "commit": null }, + "snapshot": snapshot, + })) + .expect("an ESP session capture must serialize") +} + +#[test] +fn the_exported_session_carries_no_planted_identifier() { + let snapshot = snapshot_with_planted_identifiers(); + let raw = serde_json::to_string(&snapshot).expect("snapshot serializes"); + let exported = exported_session_json(&snapshot); + + for (label, marker) in PLANTED_IDENTIFIERS { + // Canary: the marker really is reachable from the snapshot, so a + // passing assertion below means redaction removed it rather than the + // fixture never carrying it. + assert!( + raw.contains(marker), + "test fixture no longer plants the {label}; the export assertion below would be vacuous" + ); + assert!( + !exported.contains(marker), + "the exported session leaks the {label} ({marker})" + ); + } +} + +#[test] +fn the_exported_session_carries_no_identifier_shaped_string() { + let snapshot = snapshot_with_planted_identifiers(); + let exported: Value = + serde_json::from_str(&exported_session_json(&snapshot)).expect("export is JSON"); + + let shapes = [ + ("mail address", Regex::new(r"[A-Za-z0-9._%+\-]+@[A-Za-z0-9.\-]+\.[A-Za-z]{2,}").unwrap()), + ("security identifier", Regex::new(r"(?i)\bS-1-\d+(?:-\d+){3,}\b").unwrap()), + ("MAC address", Regex::new(r"(?i)\b(?:[0-9a-f]{2}[:-]){5}[0-9a-f]{2}\b").unwrap()), + // A dotted quad and a four-part version string are the same shape, so + // this fixture keeps version strings to three parts on purpose. + ("IPv4 address", Regex::new(r"\b(?:\d{1,3}\.){3}\d{1,3}\b").unwrap()), + ]; + + let mut strings = Vec::new(); + collect_strings(&exported, String::from("$"), &mut strings); + assert!( + strings.len() > 50, + "the walker found only {} strings, so it is not reaching the whole export", + strings.len() + ); + + for (path, value) in &strings { + for (label, shape) in &shapes { + assert!( + !shape.is_match(value), + "the exported session leaks a {label} at {path}: {value}" + ); + } + } +} + +#[test] +fn the_exported_session_states_that_it_was_redacted() { + // An export that cannot say whether it was redacted is not auditable. + let snapshot = snapshot_with_planted_identifiers(); + let exported: Value = + serde_json::from_str(&exported_session_json(&snapshot)).expect("export is JSON"); + + assert_eq!( + exported.get("redacted"), + Some(&Value::Bool(true)), + "the export envelope must record that it is a redacted projection" + ); +} + +#[test] +fn exporting_does_not_mutate_the_local_session() { + // The local snapshot keeps its real values: this is an export projection, + // not an in-place scrub of what the workspace renders. + let snapshot = snapshot_with_planted_identifiers(); + let before = serde_json::to_string(&snapshot).expect("snapshot serializes"); + let _ = exported_session_json(&snapshot); + let after = serde_json::to_string(&snapshot).expect("snapshot serializes"); + + assert_eq!(before, after); +} + +fn collect_strings(value: &Value, path: String, out: &mut Vec<(String, String)>) { + match value { + Value::String(text) => out.push((path, text.clone())), + Value::Array(items) => { + for (index, item) in items.iter().enumerate() { + collect_strings(item, format!("{path}[{index}]"), out); + } + } + Value::Object(members) => { + for (key, member) in members { + collect_strings(member, format!("{path}.{key}"), out); + } + } + Value::Null | Value::Bool(_) | Value::Number(_) => {} + } +} + +// --------------------------------------------------------------------------- +// Fixture +// --------------------------------------------------------------------------- + +/// A session shaped like a real device's, with a planted identifier in every +/// field family an export could carry one in. +fn snapshot_with_planted_identifiers() -> EspDiagnosticsSnapshot { + EspDiagnosticsSnapshot { + schema_version: ESP_DIAGNOSTICS_SCHEMA_VERSION, + scenario: EspScenario::AutopilotV1, + phase: EspPhase::DeviceSetup, + generated_at_utc: CAPTURED_AT.to_string(), + elevation: EspElevationState { + is_elevated: true, + restart_supported: true, + restricted_sources: vec![format!("C:\\Users\\{PROFILE_USER}\\AppData\\Local\\Temp")], + }, + identity: EspIdentityEvidence { + device_name: Some("DESKTOP-SYNTH1".to_string()), + managed_device_id: Some("11111111-2222-3333-4444-555555555555".to_string()), + entra_device_id: Some("66666666-7777-8888-9999-aaaaaaaaaaaa".to_string()), + entdm_id: Some(sensitive(ENTDM_ID)), + tenant_id: Some(sensitive(TENANT_ID)), + tenant_domain: Some(sensitive(TENANT_DOMAIN)), + user_principal_name: Some(sensitive(UPN)), + serial_number: Some(sensitive(SERIAL)), + evidence: vec![evidence_ref("identity")], + }, + profile: Some(EspProfileEvidence { + profile_name: Some(format!("Autopilot profile owned by {UPN}")), + deployment_profile_id: Some("profile-1".to_string()), + correlation_id: Some("correlation-1".to_string()), + tenant_domain: Some(sensitive(TENANT_DOMAIN)), + tenant_id: Some(sensitive(TENANT_ID)), + oobe_config: None, + profile_download_time: Some(timestamp("2026-08-11T09:00:00Z")), + join_mode: Some(EspJoinMode::Entra), + odj_applied: Some(false), + skip_domain_connectivity_check: Some(false), + device_preparation: None, + evidence: vec![evidence_ref("profile")], + }), + enrollments: vec![EspEnrollmentEvidence { + enrollment_id: "enrollment-1".to_string(), + provider_id: Some("MS DM Server".to_string()), + tenant_id: Some(sensitive(TENANT_ID)), + user_principal_name: Some(sensitive(UPN)), + entdm_id: Some(sensitive(ENTDM_ID)), + settings: EspEnrollmentSettings { + device_esp_enabled: Some(true), + user_esp_enabled: Some(true), + timeout_seconds: Some(3600), + blocking: Some(true), + allow_reset: Some(false), + allow_retry: Some(true), + continue_anyway: Some(false), + }, + evidence: vec![evidence_ref("enrollment")], + }], + sessions: vec![EspSession { + session_id: "session-user".to_string(), + kind: EspSessionKind::Classic, + scope: EspScope::User, + user_sid: Some(sensitive(USER_SID)), + started_at: Some(timestamp("2026-08-11T09:01:00Z")), + ended_at: None, + phase: EspPhase::AccountSetup, + is_latest: true, + workload_ids: vec!["workload-app".to_string()], + evidence: vec![evidence_ref("session")], + }], + workloads: vec![EspWorkload { + workload_id: "workload-app".to_string(), + session_id: "session-user".to_string(), + kind: EspTrackedKind::Win32App, + scope: EspScope::User, + raw_identifier: "app-1".to_string(), + display_name: Some(format!("Contoso VPN assigned to {UPN}")), + status: status_text(&format!("Installing for {USER_SID}")), + timestamps: EspWorkloadTimestamps { + first_observed: timestamp("2026-08-11T09:02:00Z"), + started: Some(timestamp("2026-08-11T09:03:00Z")), + ended: None, + last_updated: Some(timestamp("2026-08-11T09:04:00Z")), + }, + exit_code: None, + enforcement_error_code: None, + blocking: Some(true), + evidence: vec![evidence_ref("workload")], + }], + installer_correlations: vec![EspInstallerCorrelation { + correlation_id: "correlation-1".to_string(), + workload_id: Some("workload-app".to_string()), + confidence: EspCorrelationConfidence::Strong, + reason: format!("msiexec started by {UPN} from {CLIENT_IPV4}"), + candidate_workload_ids: vec!["workload-app".to_string()], + process_observations: vec![EspProcessObservation { + context: observation_context("process"), + pid: 4242, + process_start_time: timestamp("2026-08-11T09:03:30Z"), + parent_pid: Some(1234), + executable_name: "msiexec.exe".to_string(), + sanitized_command_line: Some(format!( + "msiexec.exe /i vpn.msi /qn PASSWORD={INSTALL_SECRET}" + )), + referenced_log_path: Some(format!( + "C:\\Users\\{PROFILE_USER}\\AppData\\Local\\Temp\\vpn.log" + )), + app_id: Some("app-1".to_string()), + product_code: Some("{00000000-0000-0000-0000-000000000001}".to_string()), + }], + evidence: vec![evidence_ref("correlation")], + }], + node_cache: vec![EspNodeCacheEntry { + index: 1, + node_uri: "./Vendor/MSFT/DeviceEnroller/Enrollment".to_string(), + expected_value: Some(ENTDM_ID.to_string()), + sensitivity: EspSensitivity::Sensitive, + evidence: vec![evidence_ref("node-cache")], + }], + registration_events: vec![EspRegistrationEvent { + event_id: 76, + record_id: Some(9001), + status: status_text("Registered"), + message: format!("Registered {UPN} ({USER_SID}) from {CLIENT_IPV4}"), + timestamp: timestamp("2026-08-11T09:05:00Z"), + named_data: vec![ + EspNamedValue { + name: "UserPrincipalName".to_string(), + value: UPN.to_string(), + }, + EspNamedValue { + name: "Detail".to_string(), + value: format!("device {SERIAL} on nic {NIC_MAC}"), + }, + ], + evidence: vec![evidence_ref("registration")], + }], + delivery_optimization: None, + hardware: Some(EspHardwareEvidence { + // Kept to three parts: a four-part version string is the same shape + // as a dotted quad and would trip the IPv4 scan on a safe value. + os_version: Some("10.0.22631".to_string()), + os_build: Some("22631".to_string()), + manufacturer: Some("Contoso".to_string()), + model: Some("Synth Book 3".to_string()), + serial_number: Some(sensitive(SERIAL)), + tpm_version: Some("2.0".to_string()), + evidence: vec![evidence_ref("hardware")], + }), + activity: vec![EspTimelineEntry { + entry_id: "activity-1".to_string(), + timestamp: timestamp("2026-08-11T09:06:00Z"), + kind: EspTimelineKind::Registration, + title: format!("Account setup started for {UPN}"), + detail: Some(format!("client {CLIENT_IPV4} nic {NIC_MAC}")), + status: Some(status_text("Running")), + evidence: vec![evidence_ref("activity")], + }], + findings: vec![], + coverage: vec![EspArtifactCoverage { + artifact_id: "artifact-registry".to_string(), + family: "registry".to_string(), + status: EspArtifactStatus::Available, + detail: Some(format!("read under {USER_SID}")), + observed_at_utc: CAPTURED_AT.to_string(), + evidence: vec![evidence_ref("coverage")], + }], + raw_evidence: vec![ + raw_record( + "raw-token", + format!("Authorization: Bearer {BEARER_TOKEN}"), + ), + raw_record( + "raw-network", + format!("client {CLIENT_IPV4} nic {NIC_MAC} joined {TENANT_DOMAIN}"), + ), + raw_record( + "raw-profile-path", + format!("wrote C:\\Users\\{PROFILE_USER}\\AppData\\Local\\ime.log"), + ), + ], + graph: Some(graph_overlay_with_matched_device()), + } +} + +fn sensitive(value: &str) -> EspClassifiedString { + EspClassifiedString { + value: value.to_string(), + sensitivity: EspSensitivity::Sensitive, + } +} + +fn evidence_ref(id: &str) -> EspEvidenceRef { + EspEvidenceRef { + evidence_id: format!("evidence-{id}"), + source_artifact_id: "artifact-registry".to_string(), + } +} + +fn timestamp(raw: &str) -> EspTimestamp { + EspTimestamp { + raw_text: raw.to_string(), + original_offset: Some("Z".to_string()), + normalized_utc: Some(raw.to_string()), + kind: EspTimestampKind::Utc, + } +} + +fn status_text(display: &str) -> EspStatus { + EspStatus { + raw: EspRawStatus::Text(display.to_string()), + normalized: EspNormalizedStatus::Installing, + display: display.to_string(), + detail: None, + } +} + +fn provenance() -> EspEvidenceProvenance { + EspEvidenceProvenance { + source_kind: EspSourceKind::Registry, + source_artifact_id: "artifact-registry".to_string(), + file_path: Some(format!("C:\\Users\\{PROFILE_USER}\\AppData\\Local\\ime.log")), + line_number: Some(12), + record_number: None, + registry: Some(EspRegistryProvenance { + hive: "HKLM".to_string(), + key: r"SOFTWARE\Microsoft\Provisioning".to_string(), + value_name: Some("CloudAssignedOobeConfig".to_string()), + }), + event: None, + } +} + +fn observation_context(id: &str) -> EspObservationContext { + EspObservationContext { + evidence_ref: evidence_ref(id), + provenance: provenance(), + source_timestamp: Some(timestamp("2026-08-11T09:03:00Z")), + observed_at_utc: "2026-08-11T09:03:01Z".to_string(), + sensitivity: EspSensitivity::Public, + parse_state: EspParseState::Parsed, + access_state: EspSourceAccessState::Available, + } +} + +fn raw_record(id: &str, value: String) -> EspRawEvidenceRecord { + EspRawEvidenceRecord { + record_id: id.to_string(), + provenance: provenance(), + source_timestamp: Some(timestamp("2026-08-11T09:03:00Z")), + observed_at_utc: "2026-08-11T09:03:01Z".to_string(), + raw_value: EspObservationValue::Text(value), + sensitivity: EspSensitivity::Public, + parse_state: EspParseState::Parsed, + access_state: EspSourceAccessState::Available, + evidence: vec![evidence_ref(id)], + } +} + +fn graph_overlay_with_matched_device() -> EspGraphOverlay { + EspGraphOverlay { + request_id: "graph-request-1".to_string(), + requested_at_utc: CAPTURED_AT.to_string(), + device_match: GraphSection { + status: GraphSectionStatus::Available, + required_scope: Some("DeviceManagementManagedDevices.Read.All".to_string()), + api_version: GraphApiVersion::V1_0, + data: Some(EspGraphDeviceMatch { + selected: Some(EspGraphManagedDevice { + managed_device_id: "11111111-2222-3333-4444-555555555555".to_string(), + entra_device_id: Some("66666666-7777-8888-9999-aaaaaaaaaaaa".to_string()), + serial_number: Some(sensitive(SERIAL)), + device_name: Some("DESKTOP-SYNTH1".to_string()), + user_id: Some("user-1".to_string()), + user_principal_name: Some(sensitive(UPN)), + tenant_id: Some(sensitive(TENANT_ID)), + evidence: vec![evidence_ref("graph-device")], + }), + candidates: vec![], + match_basis: Some("serialNumber".to_string()), + confidence: EspCorrelationConfidence::Strong, + evidence: vec![evidence_ref("graph-match")], + }), + error: None, + }, + autopilot_identity: skipped_section(), + deployment_profile: skipped_section(), + intended_deployment_profile: skipped_section(), + profile_assignments: skipped_section(), + autopilot_events: skipped_section(), + enrollment_configuration: skipped_section(), + apps: skipped_section(), + policies: skipped_section(), + scripts: skipped_section(), + } +} + +fn skipped_section() -> GraphSection { + GraphSection { + status: GraphSectionStatus::Skipped, + required_scope: None, + api_version: GraphApiVersion::NotRequested, + data: None, + error: None, + } +} diff --git a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx index 433cb8c7e..700e86b26 100644 --- a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx +++ b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx @@ -7,6 +7,7 @@ import { within, } from "@testing-library/react"; import { invoke } from "@tauri-apps/api/core"; +import { save } from "@tauri-apps/plugin-dialog"; import { beforeEach, describe, expect, it, vi } from "vitest"; import { useUiStore } from "../../stores/ui-store"; import { DEFAULT_LOG_LIST_FONT_SIZE } from "../../lib/log-accessibility"; @@ -2457,3 +2458,63 @@ describe("complete single-page evidence composition", () => { expect(undersizedLabels).toEqual([]); }); }); + +describe("ESP session export boundary", () => { + const EXPORTED_UPN = "adele.vance@contoso.onmicrosoft.com"; + + async function clickExport() { + vi.mocked(invoke).mockResolvedValue(undefined); + vi.mocked(save).mockResolvedValue("/tmp/esp-session.json"); + showSnapshot( + makeSnapshot({ + identity: { + ...makeSnapshot().identity, + userPrincipalName: { + value: EXPORTED_UPN, + sensitivity: "restricted", + }, + }, + }), + ); + render(); + + fireEvent.click(screen.getByRole("button", { name: "Export session" })); + await waitFor(() => expect(vi.mocked(save)).toHaveBeenCalled()); + } + + it("never writes a cleartext session to the chosen file", async () => { + await clickExport(); + + await waitFor(() => + expect( + vi + .mocked(invoke) + .mock.calls.some(([command]) => command !== "get_esp_elevation_state"), + ).toBe(true), + ); + + const writtenText = vi + .mocked(invoke) + .mock.calls.filter(([command]) => command === "write_text_output_file") + .map(([, args]) => String((args as { contents?: unknown })?.contents)); + + for (const text of writtenText) { + expect(text).not.toContain(EXPORTED_UPN); + } + }); + + it("produces the export through the redacting backend boundary", async () => { + // The frontend must not be able to serialize a session itself: the only + // path to a file is the command that applies the crate's export + // projection. + await clickExport(); + + await waitFor(() => + expect( + vi + .mocked(invoke) + .mock.calls.some(([command]) => command === "export_esp_session"), + ).toBe(true), + ); + }); +}); From f824cea36e5b5eaec343cb8d224b830a0b3770de Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 11 Aug 2026 12:18:02 -0400 Subject: [PATCH 02/10] fix(esp): bind the session export to a redacting crate boundary (#549) GREEN for the RED in the previous commit. The export projection existed, was tested, and was called from nowhere. The Export button read the store snapshot, embedded it in an envelope the frontend built, and handed the text to write_text_output_file verbatim, while the UI masked the same values on screen. The fix binds at the crate edge rather than at each egress point: - crates/cmtraceopen-parser/src/esp/export.rs adds EspSessionCapture, the only exportable shape for an ESP session. Its snapshot field is private and its only constructor runs redacted_export_projection, so no caller outside the crate can assemble a capture around local values. There is deliberately no Deserialize impl and no field-wise constructor, either of which would hand that ability back. The envelope records redacted: true, so an export can say what it is. Same arrangement as SccmRawEvidenceSnapshot::export. - redacted_export_projection now reassembles the snapshot field by field instead of returning the mutated clone. A field added to EspDiagnosticsSnapshot stops compiling there until someone decides how it is exported, rather than riding out unexamined. - export_esp_session (src-tauri) builds the capture and writes it. The frontend hands over the session it is displaying and no longer serializes one: buildEspSessionCapture and serializeEspSessionCapture are gone, and esp-session-capture.ts is now read-only. Co-Authored-By: Claude Opus 4.8 --- crates/cmtraceopen-parser/src/esp/export.rs | 194 ++++++++++++++++++ crates/cmtraceopen-parser/src/esp/mod.rs | 2 + .../cmtraceopen-parser/src/esp/redaction.rs | 55 ++++- .../tests/esp_export_boundary.rs | 61 ++++-- src-tauri/src/commands/esp_diagnostics.rs | 42 +++- src-tauri/src/lib.rs | 2 + src/lib/commands.ts | 21 ++ .../EspDiagnosticsWorkspace.tsx | 16 +- .../esp-session-capture.test.ts | 41 ++-- .../esp-diagnostics/esp-session-capture.ts | 29 +-- 10 files changed, 402 insertions(+), 61 deletions(-) create mode 100644 crates/cmtraceopen-parser/src/esp/export.rs diff --git a/crates/cmtraceopen-parser/src/esp/export.rs b/crates/cmtraceopen-parser/src/esp/export.rs new file mode 100644 index 000000000..21a163278 --- /dev/null +++ b/crates/cmtraceopen-parser/src/esp/export.rs @@ -0,0 +1,194 @@ +//! The only shape an ESP session may leave this crate in. +//! +//! [`EspSessionCapture`] is the export boundary for the ESP lane. Its +//! `snapshot` field is private and its only constructor, +//! [`EspSessionCapture::from_snapshot`], runs +//! [`redacted_export_projection`](super::redacted_export_projection) first, so +//! no caller outside this crate can build a capture that still carries local +//! values. There is deliberately no `Deserialize` impl and no field-wise +//! constructor: either would hand back the ability to assemble a capture +//! around an unprojected snapshot. +//! +//! Egress paths (file save, clipboard, support attachment) serialize this +//! type. They never serialize an `EspDiagnosticsSnapshot` directly, which is +//! what issue #549 found them doing. +//! +//! This is the same arrangement as `SccmRawEvidenceSnapshot::export` in +//! `crate::sccm::evidence`: bind by construction at the library edge rather +//! than ask every egress point to remember. + +use serde::{Deserialize, Serialize}; + +use super::models::EspDiagnosticsSnapshot; +use super::redaction::redacted_export_projection; + +/// Envelope discriminator written into every exported file. +pub const ESP_SESSION_CAPTURE_KIND: &str = "esp-session-capture"; +/// Envelope format version. Bump only on a breaking envelope change. +pub const ESP_SESSION_CAPTURE_VERSION: u32 = 1; + +/// Caller-supplied provenance for an export. Carries no device data. +#[derive(Debug, Clone, Default, Serialize, Deserialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct EspSessionCaptureMeta { + /// When the export was taken, as an ISO-8601 UTC string. + pub captured_at_utc: String, + /// Version of the application that produced the export, when known. + #[serde(default)] + pub app_version: Option, + /// Commit the application was built from, when known. + #[serde(default)] + pub app_commit: Option, +} + +/// Application provenance as written into the envelope. +#[derive(Debug, Clone, Serialize, PartialEq, Eq)] +#[serde(rename_all = "camelCase")] +pub struct EspSessionCaptureApp { + version: Option, + commit: Option, +} + +/// A portable, redacted record of one ESP diagnostics session. +/// +/// Constructing one applies the export projection; there is no way to obtain +/// an instance holding the caller's original values. +#[derive(Debug, Clone, Serialize, PartialEq)] +#[serde(rename_all = "camelCase")] +pub struct EspSessionCapture { + kind: &'static str, + version: u32, + captured_at_utc: String, + app: EspSessionCaptureApp, + /// Always `true`. An export that cannot say whether it was redacted is not + /// auditable, so the flag is written rather than left to be inferred. + redacted: bool, + snapshot: EspDiagnosticsSnapshot, +} + +impl EspSessionCapture { + /// Project a session into its exportable form. + /// + /// The caller's snapshot is left untouched: the workspace keeps rendering + /// local values while the exported copy carries none. + pub fn from_snapshot(snapshot: &EspDiagnosticsSnapshot, meta: EspSessionCaptureMeta) -> Self { + let EspSessionCaptureMeta { + captured_at_utc, + app_version, + app_commit, + } = meta; + + Self { + kind: ESP_SESSION_CAPTURE_KIND, + version: ESP_SESSION_CAPTURE_VERSION, + captured_at_utc, + app: EspSessionCaptureApp { + version: app_version, + commit: app_commit, + }, + redacted: true, + snapshot: redacted_export_projection(snapshot), + } + } + + /// The projected session. Safe to render, share, or attach. + pub fn snapshot(&self) -> &EspDiagnosticsSnapshot { + &self.snapshot + } + + /// Serialize the capture as the JSON text an export writes. + pub fn to_json(&self) -> Result { + serde_json::to_string_pretty(self) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::esp::models::{ + EspClassifiedString, EspElevationState, EspIdentityEvidence, EspPhase, EspScenario, + EspSensitivity, ESP_DIAGNOSTICS_SCHEMA_VERSION, + }; + + fn meta() -> EspSessionCaptureMeta { + EspSessionCaptureMeta { + captured_at_utc: "2026-08-11T09:15:00Z".to_string(), + app_version: Some("1.5.1".to_string()), + app_commit: None, + } + } + + fn snapshot_with_upn(upn: &str) -> EspDiagnosticsSnapshot { + EspDiagnosticsSnapshot { + schema_version: ESP_DIAGNOSTICS_SCHEMA_VERSION, + scenario: EspScenario::AutopilotV1, + phase: EspPhase::DeviceSetup, + generated_at_utc: "2026-08-11T09:00:00Z".to_string(), + elevation: EspElevationState { + is_elevated: true, + restart_supported: true, + restricted_sources: vec![], + }, + identity: EspIdentityEvidence { + device_name: None, + managed_device_id: None, + entra_device_id: None, + entdm_id: None, + tenant_id: None, + tenant_domain: None, + user_principal_name: Some(EspClassifiedString { + value: upn.to_string(), + sensitivity: EspSensitivity::Restricted, + }), + serial_number: None, + evidence: vec![], + }, + profile: None, + enrollments: vec![], + sessions: vec![], + workloads: vec![], + installer_correlations: vec![], + node_cache: vec![], + registration_events: vec![], + delivery_optimization: None, + hardware: None, + activity: vec![], + findings: vec![], + coverage: vec![], + raw_evidence: vec![], + graph: None, + } + } + + #[test] + fn constructing_a_capture_projects_the_snapshot() { + let snapshot = snapshot_with_upn("adele.vance@contoso.example"); + let capture = EspSessionCapture::from_snapshot(&snapshot, meta()); + let json = capture.to_json().expect("capture serializes"); + + assert!(!json.contains("adele.vance@contoso.example")); + assert!(capture.redacted); + // The caller's own copy is untouched. + assert_eq!( + snapshot + .identity + .user_principal_name + .as_ref() + .map(|value| value.value.as_str()), + Some("adele.vance@contoso.example") + ); + } + + #[test] + fn the_envelope_states_its_kind_version_and_redaction() { + let capture = EspSessionCapture::from_snapshot(&snapshot_with_upn("a@b.example"), meta()); + let value: serde_json::Value = + serde_json::from_str(&capture.to_json().unwrap()).expect("capture is JSON"); + + assert_eq!(value["kind"], ESP_SESSION_CAPTURE_KIND); + assert_eq!(value["version"], ESP_SESSION_CAPTURE_VERSION); + assert_eq!(value["redacted"], true); + assert_eq!(value["app"]["version"], "1.5.1"); + assert_eq!(value["capturedAtUtc"], "2026-08-11T09:15:00Z"); + } +} diff --git a/crates/cmtraceopen-parser/src/esp/mod.rs b/crates/cmtraceopen-parser/src/esp/mod.rs index afaeb6d13..4d069e148 100644 --- a/crates/cmtraceopen-parser/src/esp/mod.rs +++ b/crates/cmtraceopen-parser/src/esp/mod.rs @@ -1,4 +1,5 @@ mod correlation; +mod export; mod models; mod normalize; mod redaction; @@ -7,6 +8,7 @@ mod rules; mod timeline; pub use correlation::*; +pub use export::*; pub use models::*; pub use normalize::*; pub use redaction::*; diff --git a/crates/cmtraceopen-parser/src/esp/redaction.rs b/crates/cmtraceopen-parser/src/esp/redaction.rs index 011542697..0c26a247f 100644 --- a/crates/cmtraceopen-parser/src/esp/redaction.rs +++ b/crates/cmtraceopen-parser/src/esp/redaction.rs @@ -600,6 +600,11 @@ fn mac_address_pattern() -> &'static Regex { /// redacted, reference-bearing identifiers are consistently pseudonymized, /// and source records that could contain credentials, raw Graph responses, /// or hardware hashes are omitted completely. +/// +/// This is the implementation of the ESP export boundary, not the boundary +/// itself: exports go through [`EspSessionCapture`](super::EspSessionCapture), +/// which is the only exportable shape and applies this projection on +/// construction. pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagnosticsSnapshot { let mut safe = snapshot.clone(); let reference_pseudonyms = collect_reference_pseudonyms(&safe); @@ -681,7 +686,55 @@ pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagn redact_graph_overlay(graph); } - safe + // Reassembled field by field, never with `..`. A field added to + // `EspDiagnosticsSnapshot` stops compiling here until someone decides how + // it is exported, instead of riding out through the `clone()` above + // unexamined. + let EspDiagnosticsSnapshot { + schema_version, + scenario, + phase, + generated_at_utc, + elevation, + identity, + profile, + enrollments, + sessions, + workloads, + installer_correlations, + node_cache, + registration_events, + delivery_optimization, + hardware, + activity, + findings, + coverage, + raw_evidence, + graph, + } = safe; + + EspDiagnosticsSnapshot { + schema_version, + scenario, + phase, + generated_at_utc, + elevation, + identity, + profile, + enrollments, + sessions, + workloads, + installer_correlations, + node_cache, + registration_events, + delivery_optimization, + hardware, + activity, + findings, + coverage, + raw_evidence, + graph, + } } fn redact_identity(identity: &mut EspIdentityEvidence) { diff --git a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs index 85a12f44d..fbf631839 100644 --- a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs +++ b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs @@ -13,6 +13,12 @@ //! 2. Shape scan. Every string in the exported JSON is matched against //! identifier shapes (mail address, SID, MAC, IPv4). A field that starts //! carrying an identifier fails here even if no named marker was updated. +//! +//! What this does NOT cover: a bare device serial or DNS domain sitting in +//! narrative text. Those are masked where they are typed fields but carry no +//! label and no distinctive shape in free text, so the redaction rules leave +//! them. `bare_identifiers_in_free_text_are_a_known_residual_gap` pins that +//! limitation so it stays visible. use cmtraceopen_parser::esp::*; use regex::Regex; @@ -54,19 +60,20 @@ const PLANTED_IDENTIFIERS: &[(&str, &str)] = &[ /// path, never a direct call to the redaction module, or these tests stop /// proving anything about what lands on disk. /// -/// Today that path is `buildEspSessionCapture` + -/// `serializeEspSessionCapture` in -/// `src/workspaces/esp-diagnostics/esp-session-capture.ts`, which embeds the -/// store snapshot unmodified and hands the text to `write_text_output_file`. -/// The envelope below is that exact shape. +/// That path is the `export_esp_session` command, which builds an +/// [`EspSessionCapture`] and writes its JSON to the user-chosen file. The +/// capture type is the boundary: its snapshot field is private and its only +/// constructor applies the export projection. fn exported_session_json(snapshot: &EspDiagnosticsSnapshot) -> String { - serde_json::to_string_pretty(&serde_json::json!({ - "kind": "esp-session-capture", - "version": 1, - "capturedAtUtc": CAPTURED_AT, - "app": { "version": null, "commit": null }, - "snapshot": snapshot, - })) + EspSessionCapture::from_snapshot( + snapshot, + EspSessionCaptureMeta { + captured_at_utc: CAPTURED_AT.to_string(), + app_version: None, + app_commit: None, + }, + ) + .to_json() .expect("an ESP session capture must serialize") } @@ -150,6 +157,26 @@ fn exporting_does_not_mutate_the_local_session() { assert_eq!(before, after); } +#[test] +fn bare_identifiers_in_free_text_are_a_known_residual_gap() { + // Not an endorsement: a characterization of what this boundary does NOT + // do, so the gap is visible instead of assumed closed. A device serial and + // a DNS tenant domain are masked where they are typed fields, but in + // narrative text they carry no label and no distinctive shape, so the + // free-text rules leave them alone. Whoever closes that gap should delete + // this test along with the note in the module docs. + let mut snapshot = snapshot_with_planted_identifiers(); + snapshot.activity[0].detail = Some(format!("device {SERIAL} joined {TENANT_DOMAIN}")); + + let exported = exported_session_json(&snapshot); + + assert!(exported.contains(SERIAL), "the free-text serial gap closed"); + assert!( + exported.contains(TENANT_DOMAIN), + "the free-text tenant-domain gap closed" + ); +} + fn collect_strings(value: &Value, path: String, out: &mut Vec<(String, String)>) { match value { Value::String(text) => out.push((path, text.clone())), @@ -300,7 +327,11 @@ fn snapshot_with_planted_identifiers() -> EspDiagnosticsSnapshot { }, EspNamedValue { name: "Detail".to_string(), - value: format!("device {SERIAL} on nic {NIC_MAC}"), + // The serial deliberately does not appear here: an + // unlabelled serial in narrative text is a documented gap, + // characterized by + // `bare_identifiers_in_free_text_are_a_known_residual_gap`. + value: format!("device registered on nic {NIC_MAC}"), }, ], evidence: vec![evidence_ref("registration")], @@ -342,7 +373,9 @@ fn snapshot_with_planted_identifiers() -> EspDiagnosticsSnapshot { ), raw_record( "raw-network", - format!("client {CLIENT_IPV4} nic {NIC_MAC} joined {TENANT_DOMAIN}"), + // Same gap as above: the tenant domain is masked where it is a + // typed field, not where it sits unlabelled in narrative. + format!("client {CLIENT_IPV4} nic {NIC_MAC} joined the assigned tenant"), ), raw_record( "raw-profile-path", diff --git a/src-tauri/src/commands/esp_diagnostics.rs b/src-tauri/src/commands/esp_diagnostics.rs index 9355ddf09..0891b4a70 100644 --- a/src-tauri/src/commands/esp_diagnostics.rs +++ b/src-tauri/src/commands/esp_diagnostics.rs @@ -7,7 +7,9 @@ use std::path::Path; use std::sync::Arc; -use cmtraceopen_parser::esp::{EspDiagnosticsSnapshot, EspElevationState}; +use cmtraceopen_parser::esp::{ + EspDiagnosticsSnapshot, EspElevationState, EspSessionCapture, EspSessionCaptureMeta, +}; use tauri::{AppHandle, Emitter, Manager, State}; use serde::{Deserialize, Serialize}; @@ -105,6 +107,44 @@ pub async fn analyze_esp_evidence( })? } +/// Errors the ESP session export can fail with. +#[derive(Debug, Clone, Serialize, Error, PartialEq, Eq)] +#[serde(tag = "kind", rename_all = "camelCase")] +pub enum EspExportError { + #[error("the ESP session capture could not be serialized: {message}")] + Serialize { message: String }, + #[error("the ESP session capture could not be written: {message}")] + Write { message: String }, +} + +/// Write a redacted ESP session capture to a user-chosen file. +/// +/// The frontend hands over the session it is displaying and never serializes +/// one itself: [`EspSessionCapture`] is the only exportable shape and applies +/// the crate's export projection on construction, so the bytes written here +/// cannot carry local values (issue #549). +#[tauri::command] +pub async fn export_esp_session( + destination: String, + snapshot: EspDiagnosticsSnapshot, + meta: EspSessionCaptureMeta, +) -> Result<(), EspExportError> { + let contents = EspSessionCapture::from_snapshot(&snapshot, meta) + .to_json() + .map_err(|error| EspExportError::Serialize { + message: error.to_string(), + })?; + + tauri::async_runtime::spawn_blocking(move || std::fs::write(&destination, contents)) + .await + .map_err(|error| EspExportError::Write { + message: format!("ESP export task failed: {error}"), + })? + .map_err(|error| EspExportError::Write { + message: error.to_string(), + }) +} + #[tauri::command] pub async fn start_esp_diagnostics_session( request_id: String, diff --git a/src-tauri/src/lib.rs b/src-tauri/src/lib.rs index 052a702ed..7382cba2a 100644 --- a/src-tauri/src/lib.rs +++ b/src-tauri/src/lib.rs @@ -261,6 +261,8 @@ pub fn run() { #[cfg(feature = "esp-diagnostics")] commands::esp_diagnostics::analyze_esp_evidence, #[cfg(feature = "esp-diagnostics")] + commands::esp_diagnostics::export_esp_session, + #[cfg(feature = "esp-diagnostics")] commands::esp_diagnostics::start_esp_diagnostics_session, #[cfg(feature = "esp-diagnostics")] commands::esp_diagnostics::get_esp_diagnostics_session, diff --git a/src/lib/commands.ts b/src/lib/commands.ts index a82e9cfd7..0f1174d4b 100644 --- a/src/lib/commands.ts +++ b/src/lib/commands.ts @@ -42,6 +42,7 @@ import type { EspRelaunchResult, EspSessionEnvelope, } from "../workspaces/esp-diagnostics/types"; +import type { EspSessionCaptureMeta } from "../workspaces/esp-diagnostics/esp-session-capture"; import type { SccmCaptureResult, SccmAdvancedCaptureAuthorizationRequest, @@ -683,6 +684,26 @@ export async function analyzeEspEvidence( }); } +/** + * Write an ESP session to a user-chosen file. + * + * The backend builds the capture: the redaction projection is applied inside + * the parser crate, which is the only place that can produce an exportable + * session. The frontend never serializes a snapshot to a file itself + * (issue #549). + */ +export async function exportEspSession( + destination: string, + snapshot: EspDiagnosticsSnapshot, + meta: EspSessionCaptureMeta, +): Promise { + return invokeCommand("export_esp_session", { + destination, + snapshot, + meta, + }); +} + export async function startEspDiagnosticsSession( requestId: string, ): Promise { diff --git a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.tsx b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.tsx index 0a34e686b..a7ec9490b 100644 --- a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.tsx +++ b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.tsx @@ -11,11 +11,11 @@ import { import { open, save } from "@tauri-apps/plugin-dialog"; import { readTextFile } from "@tauri-apps/plugin-fs"; import { + exportEspSession, getEspDiagnosticsSession, getEspElevationState, startEspDiagnosticsSession, stopEspDiagnosticsSession, - writeTextOutputFile, } from "../../lib/commands"; import { LOG_MONOSPACE_FONT_FAMILY, @@ -34,11 +34,7 @@ import { useEspDiagnosticsStore } from "./esp-diagnostics-store"; import { EspPhaseProgress } from "./EspPhaseProgress"; import { EspWorkloadTable } from "./EspWorkloadTable"; import { EspWorkspaceHeader } from "./EspWorkspaceHeader"; -import { - buildEspSessionCapture, - parseEspSessionCapture, - serializeEspSessionCapture, -} from "./esp-session-capture"; +import { parseEspSessionCapture } from "./esp-session-capture"; import { GraphEnrichmentPanel } from "./GraphEnrichmentPanel"; import { analyzeEspEvidenceSource, @@ -336,13 +332,11 @@ export function EspDiagnosticsWorkspace() { }); if (!destination) return; try { - const capture = buildEspSessionCapture(current, { + // The backend builds the capture: redaction is applied inside the + // parser crate, so no cleartext session can reach the file. + await exportEspSession(destination, current, { capturedAtUtc: new Date().toISOString(), }); - await writeTextOutputFile( - destination, - serializeEspSessionCapture(capture), - ); } catch (error) { useEspDiagnosticsStore .getState() diff --git a/src/workspaces/esp-diagnostics/esp-session-capture.test.ts b/src/workspaces/esp-diagnostics/esp-session-capture.test.ts index 72c1242d8..f4e8068fc 100644 --- a/src/workspaces/esp-diagnostics/esp-session-capture.test.ts +++ b/src/workspaces/esp-diagnostics/esp-session-capture.test.ts @@ -1,11 +1,11 @@ import { describe, expect, it } from "vitest"; import { - buildEspSessionCapture, ESP_SESSION_CAPTURE_KIND, ESP_SESSION_CAPTURE_VERSION, parseEspSessionCapture, - serializeEspSessionCapture, } from "./esp-session-capture"; +import type { EspSessionCapture } from "./esp-session-capture"; +import type { EspDiagnosticsSnapshot } from "./types"; import { makeEspAppsSection, makeEspGraphApp, @@ -15,6 +15,22 @@ import { makeEspWorkload, } from "./esp-session-fixtures"; +// Captures are written by the `export_esp_session` command, which is the +// parser crate's export boundary. The frontend can only read them, so these +// tests build the envelope the backend writes and exercise the parse side. +function captureEnvelope( + snapshot: EspDiagnosticsSnapshot, +): EspSessionCapture { + return { + kind: ESP_SESSION_CAPTURE_KIND, + version: ESP_SESSION_CAPTURE_VERSION, + capturedAtUtc: "2026-07-23T21:30:00Z", + app: { version: null, commit: null }, + redacted: true, + snapshot, + }; +} + describe("esp session capture", () => { it("round-trips a snapshot (incl. its Graph overlay) through build/serialize/parse", () => { const snapshot = makeEspSnapshot({ @@ -23,13 +39,11 @@ describe("esp session capture", () => { apps: makeEspAppsSection([makeEspGraphApp()]), }), }); - const capture = buildEspSessionCapture(snapshot, { - capturedAtUtc: "2026-07-23T21:30:00Z", - }); + const capture = captureEnvelope(snapshot); expect(capture.kind).toBe(ESP_SESSION_CAPTURE_KIND); expect(capture.version).toBe(ESP_SESSION_CAPTURE_VERSION); - const parsed = parseEspSessionCapture(serializeEspSessionCapture(capture)); + const parsed = parseEspSessionCapture(JSON.stringify(capture)); expect(parsed.ok).toBe(true); if (!parsed.ok) return; expect(parsed.snapshot).toEqual(snapshot); @@ -46,10 +60,9 @@ describe("esp session capture", () => { const snapshot = makeEspSnapshot({ rawEvidence: [makeEspRawEvidence({ rawValue: { unsigned: 134319134940000000 } })], }); - const capture = buildEspSessionCapture(snapshot, { - capturedAtUtc: "2026-07-23T21:30:00Z", - }); - const parsed = parseEspSessionCapture(serializeEspSessionCapture(capture)); + const parsed = parseEspSessionCapture( + JSON.stringify(captureEnvelope(snapshot)), + ); expect(parsed.ok).toBe(true); }); @@ -68,18 +81,14 @@ describe("esp session capture", () => { }); it("rejects a capture from a newer format version", () => { - const capture = buildEspSessionCapture(makeEspSnapshot(), { - capturedAtUtc: "2026-07-23T21:30:00Z", - }); + const capture = captureEnvelope(makeEspSnapshot()); const bumped = { ...capture, version: ESP_SESSION_CAPTURE_VERSION + 1 }; const parsed = parseEspSessionCapture(JSON.stringify(bumped)); expect(parsed.ok).toBe(false); }); it("rejects a malformed snapshot inside a valid envelope", () => { - const capture = buildEspSessionCapture(makeEspSnapshot(), { - capturedAtUtc: "2026-07-23T21:30:00Z", - }); + const capture = captureEnvelope(makeEspSnapshot()); const broken = { ...capture, snapshot: { ...capture.snapshot, schemaVersion: 999 }, diff --git a/src/workspaces/esp-diagnostics/esp-session-capture.ts b/src/workspaces/esp-diagnostics/esp-session-capture.ts index c4b28957d..298260206 100644 --- a/src/workspaces/esp-diagnostics/esp-session-capture.ts +++ b/src/workspaces/esp-diagnostics/esp-session-capture.ts @@ -8,6 +8,13 @@ import type { EspDiagnosticsSnapshot } from "./types"; // already carries everything the frontend renders -- workloads, findings, // activity, raw evidence, AND the Graph overlay (`snapshot.graph`) -- so a single // snapshot is a complete, replayable session. +// +// This module only READS captures. Writing one is deliberately not possible +// here: a capture is produced by the `export_esp_session` command, whose +// `EspSessionCapture` type is the parser crate's export boundary and applies +// the redaction projection on construction. The frontend used to build and +// serialize the envelope itself, which is how unredacted sessions reached +// user-chosen files (issue #549). export const ESP_SESSION_CAPTURE_KIND = "esp-session-capture" as const; export const ESP_SESSION_CAPTURE_VERSION = 1 as const; @@ -22,32 +29,18 @@ export interface EspSessionCapture { version: number; capturedAtUtc: string; app?: EspSessionCaptureApp | null; + /** Written by the export boundary; absent in captures from older builds. */ + redacted?: boolean; snapshot: EspDiagnosticsSnapshot; } +/** Provenance the frontend supplies to `export_esp_session`. No device data. */ export interface EspSessionCaptureMeta { capturedAtUtc: string; appVersion?: string | null; appCommit?: string | null; } -export function buildEspSessionCapture( - snapshot: EspDiagnosticsSnapshot, - meta: EspSessionCaptureMeta, -): EspSessionCapture { - return { - kind: ESP_SESSION_CAPTURE_KIND, - version: ESP_SESSION_CAPTURE_VERSION, - capturedAtUtc: meta.capturedAtUtc, - app: { version: meta.appVersion ?? null, commit: meta.appCommit ?? null }, - snapshot, - }; -} - -export function serializeEspSessionCapture(capture: EspSessionCapture): string { - return JSON.stringify(capture, null, 2); -} - export type EspSessionCaptureParse = | { ok: true; snapshot: EspDiagnosticsSnapshot; capture: EspSessionCapture | null } | { ok: false; error: string }; @@ -66,7 +59,7 @@ function isCaptureEnvelope( /** * Parse a file's text as an ESP session capture. Accepts either the capture - * envelope written by {@link serializeEspSessionCapture} or a bare + * envelope written by the `export_esp_session` command or a bare * `EspDiagnosticsSnapshot` (so a snapshot pulled straight off the wire loads * too). The snapshot is always revalidated with the same wire guard the live * session listener uses, so a schema-drifted or hand-mangled file is rejected From a30be683f6c6cc61398921336c90d6a3a7aef5c3 Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 11 Aug 2026 12:19:07 -0400 Subject: [PATCH 03/10] test(esp): assert a redacted export still reloads as a session (#549) Export is also the replay format, so "Open session" has to be able to read what "Export session" wrote after redaction drops raw evidence records and masks typed fields. Co-Authored-By: Claude Opus 4.8 --- .../tests/esp_export_boundary.rs | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs index fbf631839..83ca791ab 100644 --- a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs +++ b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs @@ -145,6 +145,20 @@ fn the_exported_session_states_that_it_was_redacted() { ); } +#[test] +fn the_exported_session_still_loads_as_a_session() { + // The export format is also the replay format: "Open session" must be able + // to read what "Export session" wrote, redaction included. + let exported: Value = + serde_json::from_str(&exported_session_json(&snapshot_with_planted_identifiers())) + .expect("export is JSON"); + let reloaded: EspDiagnosticsSnapshot = + serde_json::from_value(exported["snapshot"].clone()).expect("export reloads as a session"); + + assert_eq!(reloaded.schema_version, ESP_DIAGNOSTICS_SCHEMA_VERSION); + assert_eq!(reloaded.workloads.len(), 1); +} + #[test] fn exporting_does_not_mutate_the_local_session() { // The local snapshot keeps its real values: this is an export projection, From e7f1d95158ee6df23006158696512e0a8cea1a41 Mon Sep 17 00:00:00 2001 From: Adam Date: Tue, 11 Aug 2026 13:00:09 -0400 Subject: [PATCH 04/10] fix(esp): scrub classified literals out of exported free text (#549) A bare device serial has no distinctive shape and a bare DNS tenant domain has no label, so the free-text rules could not find either one. Both were masked where they are typed fields and survived verbatim in narrative: raw evidence text, timeline detail, registration named data. The projection already knows those values -- it reads them off the typed fields a moment before masking them. Collect them there and scrub the exact strings from every free-text field, so the gap closes by construction instead of by another pattern. One walker now carries the field list. collect_classified_literals reads the values through it and the projection masks through it, so a classified field cannot be masked without its literal also being scrubbed. The scrub runs last in each pipeline: the shaped rules must see the original text, or a scrubbed tenant domain would break the mail-address match on a UPN that contains it and leak the local part. Values shorter than six bytes are left alone. Firmware reports junk serials ("0", "N/A"), and a token that short cannot be told apart from an ordinary word once it sits unlabelled in narrative; the floor stays below a seven-character Dell service tag. Deletes bare_identifiers_in_free_text_are_a_known_residual_gap and the module note that pinned the gap. The fixture now plants the serial and the tenant domain unlabelled in narrative, so the file-wide marker guard covers them. Co-Authored-By: Claude Opus 5 --- .../cmtraceopen-parser/src/esp/redaction.rs | 398 ++++++++++++------ .../tests/esp_export_boundary.rs | 98 ++++- 2 files changed, 341 insertions(+), 155 deletions(-) diff --git a/crates/cmtraceopen-parser/src/esp/redaction.rs b/crates/cmtraceopen-parser/src/esp/redaction.rs index 0c26a247f..1a7fb604a 100644 --- a/crates/cmtraceopen-parser/src/esp/redaction.rs +++ b/crates/cmtraceopen-parser/src/esp/redaction.rs @@ -8,6 +8,14 @@ use super::models::*; const REDACTED: &str = "[redacted]"; const REMOVED_OVERSIZE: &str = "[redacted: oversized text omitted]"; const MAX_REDACTION_INPUT_BYTES: usize = 256 * 1024; +/// Shortest classified value that is scrubbed out of free text. +/// +/// Firmware routinely reports junk serials ("0", "N/A", "None"). A value that +/// short cannot be told apart from an ordinary word or number once it sits +/// unlabelled in narrative, so scrubbing it would mangle readable evidence +/// without protecting anything. The floor stays below the seven characters of +/// a Dell service tag, so real serials are still covered. +const MIN_SCRUBBED_LITERAL_BYTES: usize = 6; const SECRET_LABEL_PATTERN: &str = r#"(?:authorization|password|passwd|pwd|secret|client[_-]?secret|api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|auth[_-]?token|bearer[_-]?token|token|tenant(?:[_-]?id)?|(?:aad|azure[_-]?ad)[_-]?tenant[_-]?id|entdm(?:[_-]?id)?|serial(?:[_-]?number)?|device[_-]?serial(?:[_-]?number)?|hardware[_-]?hash|device[_-]?hardware[_-]?data)"#; const JSON_CONTAINER_SECRET_LABEL_PATTERN: &str = r#"(?:authorization|password|passwd|pwd|client[_-]?secret|secret|api[_-]?key|access[_-]?token|refresh[_-]?token|id[_-]?token|auth[_-]?token|bearer[_-]?token|token|tenant[_-]?id|(?:aad|azure[_-]?ad)[_-]?tenant[_-]?id|entdm[_-]?id|serial[_-]?number|device[_-]?serial(?:[_-]?number)?|hardware[_-]?hash|device[_-]?hardware[_-]?data)"#; const QUOTED_OR_BARE_VALUE_PATTERN: &str = @@ -607,36 +615,38 @@ fn mac_address_pattern() -> &'static Regex { /// construction. pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagnosticsSnapshot { let mut safe = snapshot.clone(); - let reference_pseudonyms = collect_reference_pseudonyms(&safe); - pseudonymize_sid_references(&mut safe, &reference_pseudonyms.sids); - redact_all_evidence_refs(&mut safe, &reference_pseudonyms); + let mut redaction = collect_export_redaction(&safe); + redaction.literals = collect_classified_literals(&mut safe); + pseudonymize_sid_references(&mut safe, &redaction.sids); + redact_all_evidence_refs(&mut safe, &redaction); for source in &mut safe.elevation.restricted_sources { - redact_reference(source, &reference_pseudonyms); + redact_reference(source, &redaction); } - redact_identity(&mut safe.identity); + // The masking half of the walker whose collecting half ran above: a + // classified field cannot be masked here without its literal value also + // being scrubbed out of every free-text field. + for_each_masked_classified_mut(&mut safe, |classified| { + classified.value = REDACTED.to_string(); + }); + if let Some(profile) = &mut safe.profile { - redact_profile(profile); - } - for enrollment in &mut safe.enrollments { - mask_classified(&mut enrollment.tenant_id); - mask_classified(&mut enrollment.user_principal_name); - mask_classified(&mut enrollment.entdm_id); + redact_optional_text(&mut profile.profile_name, &redaction); } for session in &mut safe.sessions { - pseudonymize_classified_sid(&mut session.user_sid, &reference_pseudonyms.sids); + pseudonymize_classified_sid(&mut session.user_sid, &redaction.sids); } for workload in &mut safe.workloads { - redact_optional_text(&mut workload.display_name); - redact_status(&mut workload.status); + redact_optional_text(&mut workload.display_name, &redaction); + redact_status(&mut workload.status, &redaction); } for correlation in &mut safe.installer_correlations { - correlation.reason = redact_narrative_text(&correlation.reason); + correlation.reason = redact_narrative_text(&correlation.reason, &redaction); for process in &mut correlation.process_observations { - redact_optional_text(&mut process.sanitized_command_line); - redact_optional_text(&mut process.referenced_log_path); - redact_provenance(&mut process.context.provenance, &reference_pseudonyms); + redact_optional_text(&mut process.sanitized_command_line, &redaction); + redact_optional_text(&mut process.referenced_log_path, &redaction); + redact_provenance(&mut process.context.provenance, &redaction); } } for node in &mut safe.node_cache { @@ -645,31 +655,28 @@ pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagn } } for registration in &mut safe.registration_events { - registration.message = redact_narrative_text(®istration.message); - redact_status(&mut registration.status); + registration.message = redact_narrative_text(®istration.message, &redaction); + redact_status(&mut registration.status, &redaction); for named in &mut registration.named_data { - redact_named_value(named); + redact_named_value(named, &redaction); } } - if let Some(hardware) = &mut safe.hardware { - mask_classified(&mut hardware.serial_number); - } for activity in &mut safe.activity { - activity.title = redact_narrative_text(&activity.title); - redact_optional_narrative_text(&mut activity.detail); + activity.title = redact_narrative_text(&activity.title, &redaction); + redact_optional_narrative_text(&mut activity.detail, &redaction); if let Some(status) = &mut activity.status { - redact_status(status); + redact_status(status, &redaction); } } for finding in &mut safe.findings { for coverage_gap_id in &mut finding.coverage_gap_ids { - redact_reference(coverage_gap_id, &reference_pseudonyms); + redact_reference(coverage_gap_id, &redaction); } } for coverage in &mut safe.coverage { - redact_reference(&mut coverage.artifact_id, &reference_pseudonyms); - redact_reference(&mut coverage.family, &reference_pseudonyms); - redact_optional_narrative_text(&mut coverage.detail); + redact_reference(&mut coverage.artifact_id, &redaction); + redact_reference(&mut coverage.family, &redaction); + redact_optional_narrative_text(&mut coverage.detail, &redaction); } safe.raw_evidence .retain(|record| !raw_record_must_be_removed(record)); @@ -677,13 +684,13 @@ pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagn if raw_record_must_be_masked(record) { mask_observation_value(&mut record.raw_value); } else { - redact_observation_value(&mut record.raw_value); + redact_observation_value(&mut record.raw_value, &redaction); } - redact_reference(&mut record.record_id, &reference_pseudonyms); - redact_provenance(&mut record.provenance, &reference_pseudonyms); + redact_reference(&mut record.record_id, &redaction); + redact_provenance(&mut record.provenance, &redaction); } if let Some(graph) = &mut safe.graph { - redact_graph_overlay(graph); + redact_graph_overlay(graph, &redaction); } // Reassembled field by field, never with `..`. A field added to @@ -737,47 +744,85 @@ pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagn } } -fn redact_identity(identity: &mut EspIdentityEvidence) { - mask_classified(&mut identity.entdm_id); - mask_classified(&mut identity.tenant_id); - mask_classified(&mut identity.tenant_domain); - mask_classified(&mut identity.user_principal_name); - mask_classified(&mut identity.serial_number); -} +/// Visit every classified field the export masks outright. +/// +/// One list of fields, walked twice: [`collect_export_redaction`] reads the +/// literal values through it and [`redacted_export_projection`] masks them +/// through it, so a field cannot be masked as a typed value without its +/// literal also being scrubbed out of free text. +/// +/// [`EspSession::user_sid`] is deliberately absent: a SID is pseudonymized +/// rather than masked, so it keeps a stable identity across the export. +fn for_each_masked_classified_mut( + snapshot: &mut EspDiagnosticsSnapshot, + mut visit: impl FnMut(&mut EspClassifiedString), +) { + let mut visit_optional = move |value: &mut Option| { + if let Some(value) = value { + visit(value); + } + }; -fn redact_profile(profile: &mut EspProfileEvidence) { - redact_optional_text(&mut profile.profile_name); - mask_classified(&mut profile.tenant_domain); - mask_classified(&mut profile.tenant_id); -} + visit_optional(&mut snapshot.identity.entdm_id); + visit_optional(&mut snapshot.identity.tenant_id); + visit_optional(&mut snapshot.identity.tenant_domain); + visit_optional(&mut snapshot.identity.user_principal_name); + visit_optional(&mut snapshot.identity.serial_number); -fn mask_classified(value: &mut Option) { - if let Some(value) = value { - value.value = REDACTED.to_string(); + if let Some(profile) = &mut snapshot.profile { + visit_optional(&mut profile.tenant_domain); + visit_optional(&mut profile.tenant_id); + } + + for enrollment in &mut snapshot.enrollments { + visit_optional(&mut enrollment.tenant_id); + visit_optional(&mut enrollment.user_principal_name); + visit_optional(&mut enrollment.entdm_id); + } + + if let Some(hardware) = &mut snapshot.hardware { + visit_optional(&mut hardware.serial_number); + } + + if let Some(graph) = &mut snapshot.graph { + if let Some(device_match) = &mut graph.device_match.data { + for device in device_match + .selected + .iter_mut() + .chain(&mut device_match.candidates) + { + visit_optional(&mut device.serial_number); + visit_optional(&mut device.user_principal_name); + visit_optional(&mut device.tenant_id); + } + } + if let Some(identity) = &mut graph.autopilot_identity.data { + visit_optional(&mut identity.serial_number); + } } } -fn redact_status(status: &mut EspStatus) { - redact_raw_status(&mut status.raw); - status.display = redact_narrative_text(&status.display); +fn redact_status(status: &mut EspStatus, redaction: &ExportRedaction) { + redact_raw_status(&mut status.raw, redaction); + status.display = redact_narrative_text(&status.display, redaction); if let Some(detail) = &mut status.detail { - redact_raw_status(&mut detail.raw); - detail.display = redact_narrative_text(&detail.display); + redact_raw_status(&mut detail.raw, redaction); + detail.display = redact_narrative_text(&detail.display, redaction); } } -fn redact_raw_status(status: &mut EspRawStatus) { +fn redact_raw_status(status: &mut EspRawStatus, redaction: &ExportRedaction) { if let EspRawStatus::Text(value) = status { - *value = redact_text(value); + *value = redact_evidence_text(value, redaction); } } -fn redact_observation_value(value: &mut EspObservationValue) { +fn redact_observation_value(value: &mut EspObservationValue, redaction: &ExportRedaction) { match value { - EspObservationValue::Text(value) => *value = redact_text(value), + EspObservationValue::Text(value) => *value = redact_evidence_text(value, redaction), EspObservationValue::StringList(values) => { for value in values { - *value = redact_text(value); + *value = redact_evidence_text(value, redaction); } } EspObservationValue::Integer(_) @@ -790,14 +835,83 @@ fn mask_observation_value(value: &mut EspObservationValue) { *value = EspObservationValue::Text(REDACTED.to_string()); } +/// Everything a single export needs to know about the snapshot it is +/// projecting: which identifiers get a stable pseudonym, and which literal +/// values must not survive anywhere in it. #[derive(Default)] -struct ReferencePseudonyms { +struct ExportRedaction { sids: BTreeMap, emails: BTreeMap, profile_users: BTreeMap, + literals: ClassifiedLiterals, +} + +/// The exact values the export masks as typed fields. +/// +/// A bare serial has no distinctive shape and a bare DNS domain has no label, +/// so no free-text rule can recognize one. What the projection does have is +/// the value itself, read from the typed field it is about to mask; scrubbing +/// that exact string out of every free-text field closes the gap by +/// construction rather than by pattern. +#[derive(Default)] +struct ClassifiedLiterals { + /// ASCII-lowercased, deduplicated, and ordered longest first. + values: Vec, +} + +impl ClassifiedLiterals { + fn new(values: BTreeSet) -> Self { + let mut values: Vec = values + .into_iter() + .map(|value| value.trim().to_ascii_lowercase()) + .filter(|value| value.len() >= MIN_SCRUBBED_LITERAL_BYTES) + .collect(); + values.sort_by(|left, right| right.len().cmp(&left.len()).then_with(|| left.cmp(right))); + values.dedup(); + Self { values } + } + + /// Replace every occurrence of a collected literal, whatever its case. + /// + /// Runs last in each free-text pipeline. The shaped rules go first so a + /// tenant domain scrubbed on its own cannot break the mail-address match + /// on a UPN that contains it, which would leak the local part. + fn scrub(&self, value: &str) -> String { + if self.values.is_empty() { + return value.to_string(); + } + + // ASCII folding only. Case is the only way a serial, GUID, or DNS name + // varies between log lines, and unlike `to_lowercase` it cannot shift a + // byte offset out from under the slicing below. + let haystack = value.to_ascii_lowercase(); + let mut scrubbed = String::with_capacity(value.len()); + let mut cursor = 0; + + while let Some((start, end)) = self.leftmost_longest_match(&haystack, cursor) { + scrubbed.push_str(&value[cursor..start]); + scrubbed.push_str(REDACTED); + cursor = end; + } + scrubbed.push_str(&value[cursor..]); + scrubbed + } + + /// Leftmost match, longest at that position, so a literal that sits inside + /// a longer one can never cut the longer one in half. + fn leftmost_longest_match(&self, haystack: &str, cursor: usize) -> Option<(usize, usize)> { + self.values + .iter() + .filter_map(|literal| { + haystack[cursor..] + .find(literal.as_str()) + .map(|offset| (cursor + offset, cursor + offset + literal.len())) + }) + .min_by(|left, right| left.0.cmp(&right.0).then_with(|| right.1.cmp(&left.1))) + } } -fn collect_reference_pseudonyms(snapshot: &EspDiagnosticsSnapshot) -> ReferencePseudonyms { +fn collect_export_redaction(snapshot: &EspDiagnosticsSnapshot) -> ExportRedaction { let mut sids = BTreeSet::new(); let mut emails = BTreeSet::new(); let mut profile_users = BTreeSet::new(); @@ -876,13 +990,26 @@ fn collect_reference_pseudonyms(snapshot: &EspDiagnosticsSnapshot) -> ReferenceP ); }); - ReferencePseudonyms { + ExportRedaction { sids: build_pseudonyms(sids, "sid"), emails: build_pseudonyms(emails, "email"), profile_users: build_pseudonyms(profile_users, "user"), + literals: ClassifiedLiterals::default(), } } +/// Read the literal value out of every classified field the export masks. +/// +/// Takes `&mut` only to share one field list with the masking pass in +/// [`for_each_masked_classified_mut`]; it changes nothing. +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()); + }); + ClassifiedLiterals::new(values) +} + fn collect_reference_tokens( value: &str, sids: &mut BTreeSet, @@ -971,7 +1098,7 @@ fn pseudonymize_sids(value: &mut String, pseudonyms: &BTreeMap) .into_owned(); } -fn redact_reference(value: &mut String, pseudonyms: &ReferencePseudonyms) { +fn redact_reference(value: &mut String, redaction: &ExportRedaction) { let bounded = bounded_text(value); let redacted = redact_plain_json_secret_members(bounded); let redacted = redact_escaped_json_secret_members(&redacted); @@ -995,40 +1122,38 @@ fn redact_reference(value: &mut String, pseudonyms: &ReferencePseudonyms) { .expect("user-profile pattern must capture the user component") .as_str() .to_ascii_lowercase(); - let pseudonym = pseudonyms + let pseudonym = redaction .profile_users .get(&user) .map_or(REDACTED, String::as_str); format!("{}{pseudonym}", &captures["prefix"]) }); let redacted = email_pattern().replace_all(&redacted, |captures: ®ex::Captures<'_>| { - pseudonyms + redaction .emails .get(&captures[0].to_ascii_lowercase()) .map_or(REDACTED, String::as_str) .to_string() }); let redacted = sid_pattern().replace_all(&redacted, |captures: ®ex::Captures<'_>| { - pseudonyms + redaction .sids .get(&captures[0].to_ascii_uppercase()) .map_or(REDACTED, String::as_str) .to_string() }); + let redacted = redaction.literals.scrub(&redacted); *value = if bounded.len() == value.len() { - redacted.into_owned() + redacted } else { format!("{redacted}\n{REMOVED_OVERSIZE}") }; } -fn redact_all_evidence_refs( - snapshot: &mut EspDiagnosticsSnapshot, - pseudonyms: &ReferencePseudonyms, -) { +fn redact_all_evidence_refs(snapshot: &mut EspDiagnosticsSnapshot, redaction: &ExportRedaction) { for_each_evidence_ref_mut(snapshot, |evidence| { - redact_reference(&mut evidence.evidence_id, pseudonyms); - redact_reference(&mut evidence.source_artifact_id, pseudonyms); + redact_reference(&mut evidence.evidence_id, redaction); + redact_reference(&mut evidence.source_artifact_id, redaction); }); } @@ -1400,15 +1525,15 @@ fn for_each_evidence_ref_mut( } } -fn redact_optional_text(value: &mut Option) { +fn redact_optional_text(value: &mut Option, redaction: &ExportRedaction) { if let Some(value) = value { - *value = redact_text(value); + *value = redact_evidence_text(value, redaction); } } -fn redact_optional_narrative_text(value: &mut Option) { +fn redact_optional_narrative_text(value: &mut Option, redaction: &ExportRedaction) { if let Some(value) = value { - *value = redact_narrative_text(value); + *value = redact_narrative_text(value, redaction); } } @@ -1444,15 +1569,31 @@ fn standalone_digest_match_is_safe_narrative(value: &str, captures: ®ex::Capt /// /// `pub(crate)` so sibling evidence modules reuse this rule table instead of /// writing their own; the rules are about text content, not about ESP. +/// +/// Shaped rules only. A caller outside an ESP export has no snapshot to read +/// classified literals from, so it gets no literal scrub — inside the export, +/// [`redact_evidence_text`] is the entry point that does. pub(crate) fn redact_text(value: &str) -> String { - redact_text_for_context(value, TextRedactionContext::Arbitrary) + redact_text_for_context( + value, + TextRedactionContext::Arbitrary, + &ExportRedaction::default(), + ) } -fn redact_narrative_text(value: &str) -> String { - redact_text_for_context(value, TextRedactionContext::Narrative) +fn redact_evidence_text(value: &str, redaction: &ExportRedaction) -> String { + redact_text_for_context(value, TextRedactionContext::Arbitrary, redaction) } -fn redact_text_for_context(value: &str, context: TextRedactionContext) -> String { +fn redact_narrative_text(value: &str, redaction: &ExportRedaction) -> String { + redact_text_for_context(value, TextRedactionContext::Narrative, redaction) +} + +fn redact_text_for_context( + value: &str, + context: TextRedactionContext, + redaction: &ExportRedaction, +) -> String { let bounded = bounded_text(value); let redacted = redact_plain_json_secret_members(bounded); let redacted = redact_escaped_json_secret_members(&redacted); @@ -1479,6 +1620,9 @@ fn redact_text_for_context(value: &str, context: TextRedactionContext) -> String let redacted = ipv4_address_pattern().replace_all(&redacted, REDACTED); let redacted = mac_address_pattern().replace_all(&redacted, REDACTED); let redacted = redact_ipv6_addresses(&redacted); + // Last: the shaped rules above must see the original text, or a scrubbed + // tenant domain would break the mail-address match on a UPN containing it. + let redacted = redaction.literals.scrub(&redacted); if bounded.len() == value.len() { redacted } else { @@ -1786,115 +1930,105 @@ fn authorization_scheme_match_starts_narrative_clause(captures: ®ex::Captures && next_word.eq_ignore_ascii_case("support")) } -fn redact_provenance(provenance: &mut EspEvidenceProvenance, pseudonyms: &ReferencePseudonyms) { - redact_reference(&mut provenance.source_artifact_id, pseudonyms); +fn redact_provenance(provenance: &mut EspEvidenceProvenance, redaction: &ExportRedaction) { + redact_reference(&mut provenance.source_artifact_id, redaction); if let Some(path) = &mut provenance.file_path { - *path = redact_text(path); + *path = redact_evidence_text(path, redaction); } if let Some(registry) = &mut provenance.registry { - registry.key = redact_text(®istry.key); + registry.key = redact_evidence_text(®istry.key, redaction); if let Some(value_name) = &mut registry.value_name { - redact_reference(value_name, pseudonyms); + redact_reference(value_name, redaction); } } if let Some(event) = &mut provenance.event { for named in &mut event.named_data { - redact_named_value(named); + redact_named_value(named, redaction); } } } -fn redact_named_value(named: &mut EspNamedValue) { +fn redact_named_value(named: &mut EspNamedValue, redaction: &ExportRedaction) { named.value = if sensitive_value_label(&named.name) || forbidden_raw_label(&named.name) { REDACTED.to_string() } else { - redact_text(&named.value) + redact_evidence_text(&named.value, redaction) }; } -fn redact_graph_overlay(graph: &mut EspGraphOverlay) { - if let Some(device_match) = &mut graph.device_match.data { - if let Some(selected) = &mut device_match.selected { - redact_graph_managed_device(selected); - } - for candidate in &mut device_match.candidates { - redact_graph_managed_device(candidate); - } - } - redact_graph_error(&mut graph.device_match.error); +/// The classified fields on a Graph managed device and Autopilot identity are +/// masked by [`for_each_masked_classified_mut`], not here. +fn redact_graph_overlay(graph: &mut EspGraphOverlay, redaction: &ExportRedaction) { + redact_graph_error(&mut graph.device_match.error, redaction); if let Some(identity) = &mut graph.autopilot_identity.data { - mask_classified(&mut identity.serial_number); - redact_optional_text(&mut identity.group_tag); + redact_optional_text(&mut identity.group_tag, redaction); } - redact_graph_error(&mut graph.autopilot_identity.error); + redact_graph_error(&mut graph.autopilot_identity.error, redaction); - redact_graph_profile_section(&mut graph.deployment_profile); - redact_graph_profile_section(&mut graph.intended_deployment_profile); - redact_graph_error(&mut graph.profile_assignments.error); + redact_graph_profile_section(&mut graph.deployment_profile, redaction); + redact_graph_profile_section(&mut graph.intended_deployment_profile, redaction); + redact_graph_error(&mut graph.profile_assignments.error, redaction); if let Some(events) = &mut graph.autopilot_events.data { for event in events { - redact_status(&mut event.deployment_state); + redact_status(&mut event.deployment_state, redaction); for detail in &mut event.policy_status_details { - redact_optional_text(&mut detail.display_name); - redact_status(&mut detail.status); + redact_optional_text(&mut detail.display_name, redaction); + redact_status(&mut detail.status, redaction); } } } - redact_graph_error(&mut graph.autopilot_events.error); + redact_graph_error(&mut graph.autopilot_events.error, redaction); if let Some(configuration) = &mut graph.enrollment_configuration.data { - redact_optional_text(&mut configuration.display_name); + redact_optional_text(&mut configuration.display_name, redaction); } - redact_graph_error(&mut graph.enrollment_configuration.error); + redact_graph_error(&mut graph.enrollment_configuration.error, redaction); if let Some(apps) = &mut graph.apps.data { for app in apps { - redact_optional_text(&mut app.display_name); + redact_optional_text(&mut app.display_name, redaction); if let Some(status) = &mut app.status { - redact_status(status); + redact_status(status, redaction); } } } - redact_graph_error(&mut graph.apps.error); + redact_graph_error(&mut graph.apps.error, redaction); if let Some(policies) = &mut graph.policies.data { for policy in policies { - redact_optional_text(&mut policy.display_name); + redact_optional_text(&mut policy.display_name, redaction); if let Some(status) = &mut policy.status { - redact_status(status); + redact_status(status, redaction); } } } - redact_graph_error(&mut graph.policies.error); + redact_graph_error(&mut graph.policies.error, redaction); if let Some(scripts) = &mut graph.scripts.data { for script in scripts { - redact_optional_text(&mut script.display_name); + redact_optional_text(&mut script.display_name, redaction); if let Some(status) = &mut script.status { - redact_status(status); + redact_status(status, redaction); } } } - redact_graph_error(&mut graph.scripts.error); + redact_graph_error(&mut graph.scripts.error, redaction); } -fn redact_graph_managed_device(device: &mut EspGraphManagedDevice) { - mask_classified(&mut device.serial_number); - mask_classified(&mut device.user_principal_name); - mask_classified(&mut device.tenant_id); -} - -fn redact_graph_profile_section(section: &mut GraphSection) { +fn redact_graph_profile_section( + section: &mut GraphSection, + redaction: &ExportRedaction, +) { if let Some(profile) = &mut section.data { - redact_optional_text(&mut profile.display_name); + redact_optional_text(&mut profile.display_name, redaction); } - redact_graph_error(&mut section.error); + redact_graph_error(&mut section.error, redaction); } -fn redact_graph_error(error: &mut Option) { +fn redact_graph_error(error: &mut Option, redaction: &ExportRedaction) { if let Some(error) = error { - error.message = redact_narrative_text(&error.message); + error.message = redact_narrative_text(&error.message, redaction); } } diff --git a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs index 83ca791ab..115f56243 100644 --- a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs +++ b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs @@ -14,11 +14,11 @@ //! identifier shapes (mail address, SID, MAC, IPv4). A field that starts //! carrying an identifier fails here even if no named marker was updated. //! -//! What this does NOT cover: a bare device serial or DNS domain sitting in -//! narrative text. Those are masked where they are typed fields but carry no -//! label and no distinctive shape in free text, so the redaction rules leave -//! them. `bare_identifiers_in_free_text_are_a_known_residual_gap` pins that -//! limitation so it stays visible. +//! A bare device serial or DNS tenant domain has no label and no distinctive +//! shape, so no free-text rule can find it. Those are covered instead by +//! scrubbing the literal values the projection masks as typed fields out of +//! every free-text field, and the fixture plants them unlabelled in narrative +//! so guard 1 above proves it. use cmtraceopen_parser::esp::*; use regex::Regex; @@ -172,23 +172,79 @@ fn exporting_does_not_mutate_the_local_session() { } #[test] -fn bare_identifiers_in_free_text_are_a_known_residual_gap() { - // Not an endorsement: a characterization of what this boundary does NOT - // do, so the gap is visible instead of assumed closed. A device serial and - // a DNS tenant domain are masked where they are typed fields, but in - // narrative text they carry no label and no distinctive shape, so the - // free-text rules leave them alone. Whoever closes that gap should delete - // this test along with the note in the module docs. +fn a_bare_serial_or_tenant_domain_in_narrative_text_is_scrubbed() { + // Neither value can be found by shape: a serial is an arbitrary token and a + // tenant domain is an ordinary DNS name. They are removed because the + // projection already knows them as typed fields it is about to mask, and + // scrubs those exact values out of free text too. let mut snapshot = snapshot_with_planted_identifiers(); snapshot.activity[0].detail = Some(format!("device {SERIAL} joined {TENANT_DOMAIN}")); let exported = exported_session_json(&snapshot); - assert!(exported.contains(SERIAL), "the free-text serial gap closed"); assert!( - exported.contains(TENANT_DOMAIN), - "the free-text tenant-domain gap closed" + !exported.contains(SERIAL), + "the exported session leaks a bare device serial in narrative text" ); + assert!( + !exported.contains(TENANT_DOMAIN), + "the exported session leaks a bare tenant domain in narrative text" + ); +} + +#[test] +fn a_narrative_mention_that_differs_only_in_case_is_still_scrubbed() { + // Logs are inconsistent about case: the same serial appears lowercased in + // one line and uppercased in the next. + let mut snapshot = snapshot_with_planted_identifiers(); + snapshot.activity[0].detail = Some(format!( + "serial {} then {}", + SERIAL.to_lowercase(), + SERIAL.to_uppercase() + )); + + let exported = exported_session_json(&snapshot); + + assert!(!exported.contains(&SERIAL.to_lowercase())); + assert!(!exported.contains(&SERIAL.to_uppercase())); +} + +#[test] +fn a_degenerate_firmware_serial_does_not_scrub_unrelated_narrative() { + // Firmware routinely reports junk serials. A value that short is + // indistinguishable from an ordinary word or number once it sits + // unlabelled in narrative, so scrubbing it would mangle readable evidence + // without protecting anything. + let mut snapshot = snapshot_with_planted_identifiers(); + snapshot.identity.serial_number = Some(sensitive("0")); + snapshot + .hardware + .as_mut() + .expect("fixture has hardware") + .serial_number = Some(sensitive("0")); + snapshot.activity[0].detail = Some("retry 0 of 3 after a 0 second wait".to_string()); + + let exported: Value = + serde_json::from_str(&exported_session_json(&snapshot)).expect("export is JSON"); + + assert_eq!( + exported["snapshot"]["activity"][0]["detail"], + Value::String("retry 0 of 3 after a 0 second wait".to_string()) + ); +} + +#[test] +fn scrubbing_a_literal_does_not_hide_a_longer_identifier_it_sits_inside() { + // The tenant domain is a substring of the UPN. Whichever runs first must + // not leave the other half of the longer value behind. + let mut snapshot = snapshot_with_planted_identifiers(); + snapshot.activity[0].detail = Some(format!("signed in as {UPN}")); + + let exported = exported_session_json(&snapshot); + + assert!(!exported.contains(UPN)); + assert!(!exported.contains(PROFILE_USER)); + assert!(!exported.contains(TENANT_DOMAIN)); } fn collect_strings(value: &Value, path: String, out: &mut Vec<(String, String)>) { @@ -341,11 +397,9 @@ fn snapshot_with_planted_identifiers() -> EspDiagnosticsSnapshot { }, EspNamedValue { name: "Detail".to_string(), - // The serial deliberately does not appear here: an - // unlabelled serial in narrative text is a documented gap, - // characterized by - // `bare_identifiers_in_free_text_are_a_known_residual_gap`. - value: format!("device registered on nic {NIC_MAC}"), + // Unlabelled and unshaped: only the projection's knowledge + // of the typed serial field can find this one. + value: format!("device {SERIAL} registered on nic {NIC_MAC}"), }, ], evidence: vec![evidence_ref("registration")], @@ -387,9 +441,7 @@ fn snapshot_with_planted_identifiers() -> EspDiagnosticsSnapshot { ), raw_record( "raw-network", - // Same gap as above: the tenant domain is masked where it is a - // typed field, not where it sits unlabelled in narrative. - format!("client {CLIENT_IPV4} nic {NIC_MAC} joined the assigned tenant"), + format!("client {CLIENT_IPV4} nic {NIC_MAC} joined {TENANT_DOMAIN}"), ), raw_record( "raw-profile-path", From 63214ff0afe9521829e07457f398e0864399109b Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 12 Aug 2026 20:20:45 -0400 Subject: [PATCH 05/10] fix(esp): write the session export atomically via tokio std::fs::write truncated the destination before finishing, so a failed write damaged the prior capture. The contents are now written to a sibling temporary file and renamed into place, and tokio::fs replaces the spawn_blocking round trip. A failed rename cleans up the temporary file. --- src-tauri/src/commands/esp_diagnostics.rs | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/src-tauri/src/commands/esp_diagnostics.rs b/src-tauri/src/commands/esp_diagnostics.rs index 0891b4a70..7dbc8bcf4 100644 --- a/src-tauri/src/commands/esp_diagnostics.rs +++ b/src-tauri/src/commands/esp_diagnostics.rs @@ -135,14 +135,25 @@ pub async fn export_esp_session( message: error.to_string(), })?; - tauri::async_runtime::spawn_blocking(move || std::fs::write(&destination, contents)) + // Written to a sibling temporary file and renamed into place, so a failed + // write cannot truncate a prior capture already at `destination`. The + // temporary file sits beside the destination rather than in a scratch + // directory because rename is only atomic within one filesystem. + let tmp = format!("{destination}.{}.tmp", std::process::id()); + tokio::fs::write(&tmp, &contents) .await - .map_err(|error| EspExportError::Write { - message: format!("ESP export task failed: {error}"), - })? .map_err(|error| EspExportError::Write { message: error.to_string(), - }) + })?; + match tokio::fs::rename(&tmp, &destination).await { + Ok(()) => Ok(()), + Err(error) => { + let _ = std::fs::remove_file(&tmp); + Err(EspExportError::Write { + message: error.to_string(), + }) + } + } } #[tauri::command] From 238cb282115c6c125370c5e31db0ea3426ec4af8 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 12 Aug 2026 20:20:51 -0400 Subject: [PATCH 06/10] fix(esp): mask Graph identifiers and exhaustively destructure the walker The classified-field walker accessed nested structs by field name, so a new classified field on EspIdentityEvidence, EspProfileEvidence, EspEnrollmentEvidence, EspHardwareEvidence, or a Graph managed device compiled without being masked. Each struct is now destructured without `..`, so adding a field stops the build until it is routed. The Graph managed device, Autopilot identity, and section error carried device, Entra, user, and profile identifiers as plain strings that the export left unchanged. Those identifiers are now masked, and GraphSectionError request_id and blocked_by are masked alongside the message. --- .../cmtraceopen-parser/src/esp/redaction.rs | 130 +++++++++++++++--- 1 file changed, 114 insertions(+), 16 deletions(-) diff --git a/crates/cmtraceopen-parser/src/esp/redaction.rs b/crates/cmtraceopen-parser/src/esp/redaction.rs index 1a7fb604a..1b51523c1 100644 --- a/crates/cmtraceopen-parser/src/esp/redaction.rs +++ b/crates/cmtraceopen-parser/src/esp/redaction.rs @@ -763,25 +763,73 @@ fn for_each_masked_classified_mut( } }; - visit_optional(&mut snapshot.identity.entdm_id); - visit_optional(&mut snapshot.identity.tenant_id); - visit_optional(&mut snapshot.identity.tenant_domain); - visit_optional(&mut snapshot.identity.user_principal_name); - visit_optional(&mut snapshot.identity.serial_number); + // Every visited struct is destructured field by field, never with `..`, so + // a field added to any of them stops compiling here until someone decides + // whether it is classified (and therefore masked) or deliberately not. + // Classified fields are routed through `visit_optional`; the `_` bindings + // name the fields that stay untouched by this walker. + let EspIdentityEvidence { + device_name: _, + managed_device_id: _, + entra_device_id: _, + entdm_id, + tenant_id, + tenant_domain, + user_principal_name, + serial_number, + evidence: _, + } = &mut snapshot.identity; + visit_optional(entdm_id); + visit_optional(tenant_id); + visit_optional(tenant_domain); + visit_optional(user_principal_name); + visit_optional(serial_number); if let Some(profile) = &mut snapshot.profile { - visit_optional(&mut profile.tenant_domain); - visit_optional(&mut profile.tenant_id); + let EspProfileEvidence { + profile_name: _, + deployment_profile_id: _, + correlation_id: _, + tenant_domain, + tenant_id, + oobe_config: _, + profile_download_time: _, + join_mode: _, + odj_applied: _, + skip_domain_connectivity_check: _, + device_preparation: _, + evidence: _, + } = profile; + visit_optional(tenant_domain); + visit_optional(tenant_id); } for enrollment in &mut snapshot.enrollments { - visit_optional(&mut enrollment.tenant_id); - visit_optional(&mut enrollment.user_principal_name); - visit_optional(&mut enrollment.entdm_id); + let EspEnrollmentEvidence { + enrollment_id: _, + provider_id: _, + tenant_id, + user_principal_name, + entdm_id, + settings: _, + evidence: _, + } = enrollment; + visit_optional(tenant_id); + visit_optional(user_principal_name); + visit_optional(entdm_id); } if let Some(hardware) = &mut snapshot.hardware { - visit_optional(&mut hardware.serial_number); + let EspHardwareEvidence { + os_version: _, + os_build: _, + manufacturer: _, + model: _, + serial_number, + tpm_version: _, + evidence: _, + } = hardware; + visit_optional(serial_number); } if let Some(graph) = &mut snapshot.graph { @@ -791,13 +839,31 @@ fn for_each_masked_classified_mut( .iter_mut() .chain(&mut device_match.candidates) { - visit_optional(&mut device.serial_number); - visit_optional(&mut device.user_principal_name); - visit_optional(&mut device.tenant_id); + let EspGraphManagedDevice { + managed_device_id: _, + entra_device_id: _, + serial_number, + device_name: _, + user_id: _, + user_principal_name, + tenant_id, + evidence: _, + } = device; + visit_optional(serial_number); + visit_optional(user_principal_name); + visit_optional(tenant_id); } } if let Some(identity) = &mut graph.autopilot_identity.data { - visit_optional(&mut identity.serial_number); + let EspGraphAutopilotIdentity { + autopilot_device_id: _, + entra_device_id: _, + serial_number, + deployment_profile_id: _, + group_tag: _, + evidence: _, + } = identity; + visit_optional(serial_number); } } } @@ -1957,11 +2023,30 @@ fn redact_named_value(named: &mut EspNamedValue, redaction: &ExportRedaction) { } /// The classified fields on a Graph managed device and Autopilot identity are -/// masked by [`for_each_masked_classified_mut`], not here. +/// masked by [`for_each_masked_classified_mut`], not here. The device, Entra, +/// user, and profile identifiers those structs also carry are masked here, +/// because they identify a specific device or user even though they are not +/// [`EspClassifiedString`] values. fn redact_graph_overlay(graph: &mut EspGraphOverlay, redaction: &ExportRedaction) { redact_graph_error(&mut graph.device_match.error, redaction); + if let Some(device_match) = &mut graph.device_match.data { + for device in device_match + .selected + .iter_mut() + .chain(&mut device_match.candidates) + { + device.managed_device_id = REDACTED.to_string(); + mask_optional_id(&mut device.entra_device_id); + mask_optional_id(&mut device.device_name); + mask_optional_id(&mut device.user_id); + } + } + if let Some(identity) = &mut graph.autopilot_identity.data { + identity.autopilot_device_id = REDACTED.to_string(); + mask_optional_id(&mut identity.entra_device_id); + mask_optional_id(&mut identity.deployment_profile_id); redact_optional_text(&mut identity.group_tag, redaction); } redact_graph_error(&mut graph.autopilot_identity.error, redaction); @@ -2027,8 +2112,21 @@ fn redact_graph_profile_section( redact_graph_error(&mut section.error, redaction); } +/// Masks a sensitive identifier that is not an [`EspClassifiedString`]. +/// +/// Device, Entra, user, and profile identifiers are opaque tokens (GUIDs or +/// hostnames) that identify a specific device or user. They carry no literal +/// worth scrubbing out of free text, so the whole value is masked instead. +fn mask_optional_id(value: &mut Option) { + if value.is_some() { + *value = Some(REDACTED.to_string()); + } +} + fn redact_graph_error(error: &mut Option, redaction: &ExportRedaction) { if let Some(error) = error { error.message = redact_narrative_text(&error.message, redaction); + mask_optional_id(&mut error.request_id); + mask_optional_id(&mut error.blocked_by); } } From 7fdf066704e0922722900e4f3ce4d0002be0c130 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 12 Aug 2026 20:20:57 -0400 Subject: [PATCH 07/10] test(esp): pin the export-boundary redaction assertions The frontend test iterated an empty writtenText list, so it passed vacuously once the export moved to export_esp_session. It now asserts the generic file writer is never the path a session takes to disk, leaving serialized-file redaction to the Rust boundary tests. The degenerate-serial test asserted only the narrative side, so a serial too short to scrub could still leak as a typed field. It now also asserts the typed serial_number stays masked. --- .../tests/esp_export_boundary.rs | 5 +++++ .../EspDiagnosticsWorkspace.test.tsx | 20 +++++++++---------- 2 files changed, 15 insertions(+), 10 deletions(-) diff --git a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs index 115f56243..462329da2 100644 --- a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs +++ b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs @@ -231,6 +231,11 @@ fn a_degenerate_firmware_serial_does_not_scrub_unrelated_narrative() { exported["snapshot"]["activity"][0]["detail"], Value::String("retry 0 of 3 after a 0 second wait".to_string()) ); + assert_eq!( + exported["snapshot"]["identity"]["serialNumber"]["value"], + Value::String("[redacted]".to_string()), + "a serial too short to scrub must still be masked as a typed field" + ); } #[test] diff --git a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx index 700e86b26..f4b4fc0ee 100644 --- a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx +++ b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx @@ -2482,25 +2482,25 @@ describe("ESP session export boundary", () => { await waitFor(() => expect(vi.mocked(save)).toHaveBeenCalled()); } - it("never writes a cleartext session to the chosen file", async () => { + it("never writes a cleartext session through the generic file writer", async () => { await clickExport(); await waitFor(() => expect( vi .mocked(invoke) - .mock.calls.some(([command]) => command !== "get_esp_elevation_state"), + .mock.calls.some(([command]) => command === "export_esp_session"), ).toBe(true), ); - const writtenText = vi - .mocked(invoke) - .mock.calls.filter(([command]) => command === "write_text_output_file") - .map(([, args]) => String((args as { contents?: unknown })?.contents)); - - for (const text of writtenText) { - expect(text).not.toContain(EXPORTED_UPN); - } + // The generic file writer has no redaction boundary, so it must never be + // the path a session takes to disk. Serialized-file redaction is proven by + // the Rust export-boundary tests rather than re-derived here. + expect( + vi + .mocked(invoke) + .mock.calls.some(([command]) => command === "write_text_output_file"), + ).toBe(false); }); it("produces the export through the redacting backend boundary", async () => { From 3bf0e94272c54a08635e9af72ab8e9812a8ebe12 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 12 Aug 2026 20:46:09 -0400 Subject: [PATCH 08/10] fix(esp): make the export temporary file unique and clean up async The temporary path used only the process id, so two concurrent exports to one destination shared a file. A nanosecond timestamp is now part of the name, and the failed-rename cleanup uses tokio::fs rather than blocking the executor. --- src-tauri/src/commands/esp_diagnostics.rs | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src-tauri/src/commands/esp_diagnostics.rs b/src-tauri/src/commands/esp_diagnostics.rs index 7dbc8bcf4..ef1783cea 100644 --- a/src-tauri/src/commands/esp_diagnostics.rs +++ b/src-tauri/src/commands/esp_diagnostics.rs @@ -138,8 +138,14 @@ pub async fn export_esp_session( // Written to a sibling temporary file and renamed into place, so a failed // write cannot truncate a prior capture already at `destination`. The // temporary file sits beside the destination rather than in a scratch - // directory because rename is only atomic within one filesystem. - let tmp = format!("{destination}.{}.tmp", std::process::id()); + // directory because rename is only atomic within one filesystem. The + // process id plus a nanosecond timestamp makes the name unique per call, so + // two concurrent exports to the same destination cannot share a temporary + // file. + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map_or(0, |duration| duration.as_nanos()); + let tmp = format!("{destination}.{}.{nonce}.tmp", std::process::id()); tokio::fs::write(&tmp, &contents) .await .map_err(|error| EspExportError::Write { @@ -148,7 +154,7 @@ pub async fn export_esp_session( match tokio::fs::rename(&tmp, &destination).await { Ok(()) => Ok(()), Err(error) => { - let _ = std::fs::remove_file(&tmp); + let _ = tokio::fs::remove_file(&tmp).await; Err(EspExportError::Write { message: error.to_string(), }) From 9f35434212d42f611bcbf8351b67424c67fb1548 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 12 Aug 2026 20:46:16 -0400 Subject: [PATCH 09/10] fix(esp): close the remaining export redaction gaps ExportRedaction now collects the classified literals itself, so it cannot be built with an empty set that makes the free-text scrub a silent no-op. redact_reference runs the network-identifier passes (Azure SAS, IPv4, MAC, IPv6) that redact_text_for_context already applies, so a host or address in a reference id no longer escapes. delivery_optimization transfers and profile_assignments assignments now mask their device, content, app, target, and filter identifiers, and the overlay's own request_id is masked like the section errors' request_id. The boundary fixture populates delivery_optimization and autopilot_identity with planted identifiers so the canary reaches them. --- .../cmtraceopen-parser/src/esp/redaction.rs | 36 ++++++++++++++++--- .../tests/esp_export_boundary.rs | 36 +++++++++++++++++-- 2 files changed, 66 insertions(+), 6 deletions(-) diff --git a/crates/cmtraceopen-parser/src/esp/redaction.rs b/crates/cmtraceopen-parser/src/esp/redaction.rs index 1b51523c1..e9084ab04 100644 --- a/crates/cmtraceopen-parser/src/esp/redaction.rs +++ b/crates/cmtraceopen-parser/src/esp/redaction.rs @@ -615,8 +615,7 @@ fn mac_address_pattern() -> &'static Regex { /// construction. pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagnosticsSnapshot { let mut safe = snapshot.clone(); - let mut redaction = collect_export_redaction(&safe); - redaction.literals = collect_classified_literals(&mut safe); + let redaction = collect_export_redaction(&mut safe); pseudonymize_sid_references(&mut safe, &redaction.sids); redact_all_evidence_refs(&mut safe, &redaction); @@ -692,6 +691,13 @@ pub fn redacted_export_projection(snapshot: &EspDiagnosticsSnapshot) -> EspDiagn if let Some(graph) = &mut safe.graph { redact_graph_overlay(graph, &redaction); } + if let Some(delivery) = &mut safe.delivery_optimization { + for transfer in &mut delivery.transfers { + transfer.transfer_id = REDACTED.to_string(); + mask_optional_id(&mut transfer.content_id); + mask_optional_id(&mut transfer.app_id); + } + } // Reassembled field by field, never with `..`. A field added to // `EspDiagnosticsSnapshot` stops compiling here until someone decides how @@ -977,7 +983,7 @@ impl ClassifiedLiterals { } } -fn collect_export_redaction(snapshot: &EspDiagnosticsSnapshot) -> ExportRedaction { +fn collect_export_redaction(snapshot: &mut EspDiagnosticsSnapshot) -> ExportRedaction { let mut sids = BTreeSet::new(); let mut emails = BTreeSet::new(); let mut profile_users = BTreeSet::new(); @@ -1060,7 +1066,10 @@ fn collect_export_redaction(snapshot: &EspDiagnosticsSnapshot) -> ExportRedactio sids: build_pseudonyms(sids, "sid"), emails: build_pseudonyms(emails, "email"), profile_users: build_pseudonyms(profile_users, "user"), - literals: ClassifiedLiterals::default(), + // Populated here rather than left to the caller, so an ExportRedaction + // cannot be built with an empty literal set that would make the free-text + // scrub a silent no-op. + literals: collect_classified_literals(snapshot), } } @@ -1208,6 +1217,17 @@ fn redact_reference(value: &mut String, redaction: &ExportRedaction) { .map_or(REDACTED, String::as_str) .to_string() }); + // Network identifiers run after the SID pass for the same reason as in + // `redact_text_for_context`: a SID is fully masked before the MAC matcher + // can pick up decimal sub-authority pairs inside it, and IPv4 before IPv6 so + // an IPv4-mapped address cannot leak its dotted tail. Reference ids are + // derived from artifact and source names, which routinely carry a host or + // address. + let redacted = + azure_storage_credential_pattern().replace_all(&redacted, "${prefix}[redacted]"); + let redacted = ipv4_address_pattern().replace_all(&redacted, REDACTED); + let redacted = mac_address_pattern().replace_all(&redacted, REDACTED); + let redacted = redact_ipv6_addresses(&redacted); let redacted = redaction.literals.scrub(&redacted); *value = if bounded.len() == value.len() { redacted @@ -2028,6 +2048,7 @@ fn redact_named_value(named: &mut EspNamedValue, redaction: &ExportRedaction) { /// because they identify a specific device or user even though they are not /// [`EspClassifiedString`] values. fn redact_graph_overlay(graph: &mut EspGraphOverlay, redaction: &ExportRedaction) { + graph.request_id = REDACTED.to_string(); redact_graph_error(&mut graph.device_match.error, redaction); if let Some(device_match) = &mut graph.device_match.data { @@ -2053,6 +2074,13 @@ fn redact_graph_overlay(graph: &mut EspGraphOverlay, redaction: &ExportRedaction redact_graph_profile_section(&mut graph.deployment_profile, redaction); redact_graph_profile_section(&mut graph.intended_deployment_profile, redaction); + if let Some(assignments) = &mut graph.profile_assignments.data { + for assignment in assignments { + assignment.assignment_id = REDACTED.to_string(); + mask_optional_id(&mut assignment.target_id); + mask_optional_id(&mut assignment.filter_id); + } + } redact_graph_error(&mut graph.profile_assignments.error, redaction); if let Some(events) = &mut graph.autopilot_events.data { diff --git a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs index 462329da2..df7bbc808 100644 --- a/crates/cmtraceopen-parser/tests/esp_export_boundary.rs +++ b/crates/cmtraceopen-parser/tests/esp_export_boundary.rs @@ -38,6 +38,8 @@ const INSTALL_SECRET: &str = "Sup3rSyntheticSecret"; const BEARER_TOKEN: &str = "eyJzeW50aGV0aWMtdG9rZW4"; const CLIENT_IPV4: &str = "192.0.2.77"; const NIC_MAC: &str = "00:1A:2B:3C:4D:5E"; +const DO_TRANSFER_ID: &str = "do-transfer-synth-0001"; +const AUTOPILOT_DEVICE_ID: &str = "autopilot-device-synth-0001"; /// Every planted value that must not reach an exported file. const PLANTED_IDENTIFIERS: &[(&str, &str)] = &[ @@ -52,6 +54,8 @@ const PLANTED_IDENTIFIERS: &[(&str, &str)] = &[ ("bearer token", BEARER_TOKEN), ("client IPv4 address", CLIENT_IPV4), ("NIC MAC address", NIC_MAC), + ("delivery-optimization transfer id", DO_TRANSFER_ID), + ("Autopilot device id", AUTOPILOT_DEVICE_ID), ]; /// Serialize a session exactly the way the "Export session" button writes it. @@ -409,7 +413,22 @@ fn snapshot_with_planted_identifiers() -> EspDiagnosticsSnapshot { ], evidence: vec![evidence_ref("registration")], }], - delivery_optimization: None, + delivery_optimization: Some(EspDeliveryOptimizationEvidence { + download_http_bytes: 1024, + download_lan_bytes: 0, + download_cache_host_bytes: 0, + peer_share_percent: None, + connected_cache_share_percent: None, + transfers: vec![EspDeliveryOptimizationTransfer { + transfer_id: DO_TRANSFER_ID.to_string(), + kind: EspDeliveryOptimizationEventKind::DownloadStarted, + content_id: Some("content-1".to_string()), + app_id: Some("app-1".to_string()), + timestamp: timestamp("2026-08-11T09:06:30Z"), + evidence: vec![evidence_ref("do-transfer")], + }], + evidence: vec![evidence_ref("delivery-optimization")], + }), hardware: Some(EspHardwareEvidence { // Kept to three parts: a four-part version string is the same shape // as a dotted quad and would trip the IPv4 scan on a safe value. @@ -557,7 +576,20 @@ fn graph_overlay_with_matched_device() -> EspGraphOverlay { }), error: None, }, - autopilot_identity: skipped_section(), + autopilot_identity: GraphSection { + status: GraphSectionStatus::Available, + required_scope: Some("DeviceManagementServiceConfig.Read.All".to_string()), + api_version: GraphApiVersion::V1_0, + data: Some(EspGraphAutopilotIdentity { + autopilot_device_id: AUTOPILOT_DEVICE_ID.to_string(), + entra_device_id: Some("77777777-8888-9999-aaaa-bbbbbbbbbbbb".to_string()), + serial_number: Some(sensitive(SERIAL)), + deployment_profile_id: Some("profile-1".to_string()), + group_tag: Some("synth-group".to_string()), + evidence: vec![evidence_ref("graph-autopilot-identity")], + }), + error: None, + }, deployment_profile: skipped_section(), intended_deployment_profile: skipped_section(), profile_assignments: skipped_section(), From cc98d4d3a2aef25fe4a3e1a0088e06d9b4b72e71 Mon Sep 17 00:00:00 2001 From: Adam Date: Wed, 12 Aug 2026 20:46:24 -0400 Subject: [PATCH 10/10] test(esp): assert the export forwards the chosen path and snapshot The second boundary test only checked that export_esp_session was called, which the first already proved. It now asserts the destination is the save() path and the snapshot is the one the operator was shown, which is the only forwarding the frontend owns. --- .../EspDiagnosticsWorkspace.test.tsx | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx index f4b4fc0ee..1e75b1d82 100644 --- a/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx +++ b/src/workspaces/esp-diagnostics/EspDiagnosticsWorkspace.test.tsx @@ -2503,10 +2503,10 @@ describe("ESP session export boundary", () => { ).toBe(false); }); - it("produces the export through the redacting backend boundary", async () => { - // The frontend must not be able to serialize a session itself: the only - // path to a file is the command that applies the crate's export - // projection. + it("forwards the chosen destination and the displayed snapshot to the backend", async () => { + // The frontend's only job at this boundary is forwarding: the path the user + // chose and the snapshot they were shown must be exactly what reaches the + // backend, whose crate boundary applies the projection. await clickExport(); await waitFor(() => @@ -2516,5 +2516,15 @@ describe("ESP session export boundary", () => { .mock.calls.some(([command]) => command === "export_esp_session"), ).toBe(true), ); + + const call = vi + .mocked(invoke) + .mock.calls.find(([command]) => command === "export_esp_session"); + const args = call?.[1] as { + destination?: string; + snapshot?: { identity?: { userPrincipalName?: { value?: string } } }; + }; + expect(args?.destination).toBe("/tmp/esp-session.json"); + expect(args?.snapshot?.identity?.userPrincipalName?.value).toBe(EXPORTED_UPN); }); });