From 69f0762db8943e8bdc07d42b77442d179b444820 Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Tue, 18 Aug 2026 11:59:41 -0700 Subject: [PATCH 01/10] Harden telemetry consent foundation Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- .github/copilot-instructions.md | 20 +- docs/schema.md | 2 +- .../telemetry-administrative-policy.md | 10 +- docs/telemetry/telemetry-consent-design.md | 35 +- docs/telemetry/telemetry.md | 3 +- sdk/dotnet/README.md | 6 +- src/core/wxc/src/main.rs | 1 + src/core/wxc_common/Cargo.toml | 6 + src/core/wxc_common/build.rs | 133 + .../resources/telemetry/consent/en-US.json | 27 + src/core/wxc_common/src/telemetry/consent.rs | 2197 +++++++++++++++++ .../src/telemetry/consent_prompt.rs | 118 + src/core/wxc_common/src/telemetry/events.rs | 8 + src/core/wxc_common/src/telemetry/mod.rs | 116 +- src/core/wxc_common/src/telemetry/policy.rs | 577 +++++ src/mxc_telemetry/src/lib.rs | 137 +- .../scripts/run_telemetry_etw_smoke_test.ps1 | 66 +- 17 files changed, 3357 insertions(+), 105 deletions(-) create mode 100644 src/core/wxc_common/build.rs create mode 100644 src/core/wxc_common/resources/telemetry/consent/en-US.json create mode 100644 src/core/wxc_common/src/telemetry/consent.rs create mode 100644 src/core/wxc_common/src/telemetry/consent_prompt.rs create mode 100644 src/core/wxc_common/src/telemetry/policy.rs diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 6427bb4e1..68f191ad7 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -135,7 +135,8 @@ npm test npm run test:integration # C# SDK (from sdk/dotnet/) -dotnet test Microsoft.Mxc.Sdk.slnx # requires mxc_ffi built (cargo build -p mxc_ffi); resolver finds it in src/target/{debug,release} +dotnet test Microsoft.Mxc.Sdk.slnx # Debug only; requires mxc_ffi built (cargo build -p mxc_ffi); resolver finds it in src/target/{debug,release} + # the telemetry tests need the debug-only MXC_TEST_LOCALAPPDATA_OVERRIDE, so `-c Release` fails by design # Local PowerShell helpers — run from repo root, require built binaries tests\scripts\run_test_configs.ps1 # All test configs via wxc_test_driver @@ -148,6 +149,7 @@ tests\scripts\run_windows_sandbox_one_shot_tests.ps1 # Windows Sandbox one tests\scripts\run_windows_sandbox_state_aware_tests.ps1 # Windows Sandbox state-aware lifecycle E2E (provision/start/exec*/stop/deprovision; requires the Windows Sandbox optional feature; skips if absent) tests\scripts\run_lxc_all_tests.sh # All LXC tests (Linux) tests\scripts\run_bwrap_all_tests.sh # All Bubblewrap tests (Linux, requires bwrap) +tests\scripts\run_telemetry_consent_smoke_test.ps1 # Telemetry consent + policy CLI E2E (Windows; debug binary only) # E2E test crate — Rust executor integration tests (from src/) cargo test -p wxc_e2e_tests # Invokes MXC binaries directly @@ -221,8 +223,9 @@ Core references: - `docs/examples.md` — annotated configuration examples (see also `tests/examples/` and `tests/configs/`) - `docs/diagnostics.md` — diagnostic logging knobs (env vars, log file format) - `docs/ci-validation-infrastructure.md` — validation (E2E) test matrix: workflows and job names, catalog format, per-backend coverage and status, and the runbook for adding/removing an OS, backend, or plan -- `docs/host-prep.md` — `wxc-host-prep.exe` host setup binary (`prepare-system-drive` / `unprepare-system-drive` for the AppContainer ACEs on the system-drive root, plus `prepare-null-device` / `verify-null-device` / `dump-null-device` for the `\Device\Null` security descriptor that AppContainer-based backends require). Owns elevation via embedded `requireAdministrator` manifest — `wxc-exec.exe` no longer self-elevates. - `docs/sandbox-policy/0.7.0/policy.md` — sandbox policy 0.7.0 specification +- `docs/sandbox-policy/v1/policy.md` — sandbox policy v1 specification +- `docs/telemetry/telemetry.md` — telemetry overview; `docs/telemetry/telemetry-consent-design.md` (Windows-only consent design and per-SDK surface) and `docs/telemetry/telemetry-administrative-policy.md` (the MDM / Group Policy ceiling) Per-backend guides: @@ -302,6 +305,18 @@ The parser deserializes JSON directly into the typed wire model (`wxc_common::wi - macOS: `mxc-exec-mac` (Seatbelt) - Target triples: `x86_64-pc-windows-msvc`, `aarch64-pc-windows-msvc`, `x86_64-unknown-linux-gnu`, `aarch64-unknown-linux-gnu`, `aarch64-apple-darwin` +### Telemetry consent + +Telemetry is **Windows-only** and is never collected without explicit user consent. Three independent conditions must all hold before anything is emitted: the user has granted consent, the administrative (MDM / Group Policy) ceiling permits it, and the config kill-switch has not disabled it. `wxc_common::telemetry::is_enabled()` is the single place those three terms are combined — do not re-derive enablement anywhere else. + +- **Everything fails closed.** Any error, unreadable value, corrupt file, missing native library, or ambiguity must resolve to "no telemetry" (`Undetermined` consent / `Blocked` policy), never to a permissive state. +- **Policy may restrict, but may never substitute for, consent.** The administrative policy is a deny-only ceiling: it can subtract from what a user permitted, never add to it. An administrator cannot opt a user in — a denied or never-asked user stays opted out even under `AllowTelemetry=3`. Keep the terms combined with `&&`; never add a policy value or config path that grants collection on its own. +- **MXC owns its own consent state.** It must never read or infer from the Windows system telemetry consent. The consent store is a per-user JSON file; the policy is `HKLM\SOFTWARE\Policies\Mxc` → `AllowTelemetry` (`REG_DWORD`). +- **One definition, distributed to the bindings.** The Rust `ConsentState` / `PolicyState` enums are the source of truth; the FFI, C#, and TypeScript layers marshal the same strings. Keep those spellings in sync anywhere this branch surfaces them. +- **Test isolation.** The consent store and the policy key are process-global, each behind its own mutex. Use `wxc_common::telemetry::test_support::TelemetryTestEnv` whenever a test needs both; constructing `PolicyKeyGuard` and `LocalAppDataGuard` directly in the same test risks a lock-order deadlock. The consent-store override is compiled into test binaries and into debug `test-support` builds only, so release binaries do not honor it. The `wxc_common` `test-support` feature re-exports the policy override for downstream crates' integration tests (`mxc_ffi` uses it) and must stay a dev-dependency-only feature. +- **Read-only queries must never be able to crash the host.** `NeedsConsentPrompt`/`needsTelemetryConsentPrompt` and `GetPolicy`/`getTelemetryPolicy` fail closed on *any* failure and never throw — including a non-`Success` FFI status, which covers a caught panic. The consent *read* and *write* still throw, because their callers must distinguish "not decided" from "could not read" and "did not persist"; when they do, they raise only the binding's documented exception type (`MxcException`), wrapping anything unexpected rather than letting a raw type escape. +- **Never swallow a failure silently.** Fail-closed return values are indistinguishable from legitimate ones, so a broken install would otherwise be invisible. Every swallowed failure is reported once per distinct failure per process (deduplicated — hosts poll these getters), and the reporter itself must never throw. At the FFI boundary, `catch_unwind` sites log the panic payload before returning `MXC_STATUS_PANIC`, which would otherwise be discarded. + ### Package versioning All Rust crates use `version.workspace = true` to inherit the version from `src/Cargo.toml` `[workspace.package]`. The npm SDK version in `sdk/node/package.json` and the C# SDK version (`` in `sdk/dotnet/Microsoft.Mxc.Sdk/Microsoft.Mxc.Sdk.csproj`) must match. Run `node scripts/check-version-sync.js` to validate they are in sync. When bumping the version, update `src/Cargo.toml` (workspace version), `sdk/node/package.json`, and the C# csproj in the same commit. @@ -315,6 +330,7 @@ When changing behavior covered by existing documentation, update the relevant do - **SDK API changes** (new exports, changed signatures, new options) → update `sdk/node/README.md` and the JSDoc in `sdk/node/src/index.ts` (TypeScript SDK); the Rust `mxc-sdk` crate docs/`README.md`; and `sdk/dotnet/README.md` (C# SDK). If the `mxc_ffi` C ABI surface changes, the C# P/Invoke regenerates on the next C# build; keep the `ErrorCode` parity + bindings-codegen gates green. - **New containment backends or major backend changes** → update the relevant doc in `docs/` (e.g., `lxc-support/lxc-backend.md`, `windows-sandbox/windows-sandbox.md`) - **Versioning or promotion changes** → update `docs/versioning.md` +- **Telemetry consent or policy changes** → update `docs/telemetry/telemetry-consent-design.md` and/or `docs/telemetry/telemetry-administrative-policy.md`, and keep the Rust/C#/TypeScript spellings aligned in the files present on this branch ### Policy versioning diff --git a/docs/schema.md b/docs/schema.md index 31215484d..f41d34f88 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -197,7 +197,7 @@ cannot mix both formats in one request. > it is rejected as an unknown field because lifecycle correlation is internal > to MXC and is not part of the request or response contract. See > [`docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md`](state-aware-lifecycle/mxc-state-aware-sandbox-api.md) -> and [`docs/telemetry/telemetry.md`](telemetry/telemetry.md). +> and [`docs/telemetry/telemetry.md`](telemetry/telemetry.md#correlating-a-lifecycle). ### Working Directory diff --git a/docs/telemetry/telemetry-administrative-policy.md b/docs/telemetry/telemetry-administrative-policy.md index 00400aaaf..8d01b85c6 100644 --- a/docs/telemetry/telemetry-administrative-policy.md +++ b/docs/telemetry/telemetry-administrative-policy.md @@ -63,8 +63,9 @@ HKLM\SOFTWARE\Policies\Mxc\AllowTelemetry directly. For example: ### Verifying -Whatever consent-management/status surface ships must report the user's -recorded choice separately from the administrative ceiling. +The consent status surface reports the user's recorded choice separately from +the administrative ceiling. The consent APIs expose the stored and effective +states, the policy state, and whether a prompt is needed. For example, if a user previously granted consent and an administrator later sets `AllowTelemetry=0`, the status must still make both facts visible: @@ -127,8 +128,9 @@ administrative policy is an availability control, not an expression of a user's informed choice, and no Microsoft guidance treats an admin "allow" as consent on the user's behalf. -The implementation should enforce that rule in one place rather than -re-deriving it in each host surface. +The implementation stack enforces this rule in one place rather than +re-deriving it in each host surface. The telemetry gate combines the per-run +request with the consent and administrative-policy states before each event. ## Relationship to the Windows `AllowTelemetry` policy diff --git a/docs/telemetry/telemetry-consent-design.md b/docs/telemetry/telemetry-consent-design.md index e6e7a0041..ad1e4eacf 100644 --- a/docs/telemetry/telemetry-consent-design.md +++ b/docs/telemetry/telemetry-consent-design.md @@ -1,6 +1,9 @@ # Telemetry consent design -This document defines the Windows telemetry-consent contract. +This document defines the implemented Windows telemetry-consent contract. +MXC ships the canonical consent resource, persistent consent storage, and +consent-management APIs described below. Telemetry remains off by default and +is enabled only when every gate in this contract permits collection. MXC telemetry is Windows-only and fails closed. Collection requires all four conditions at the time each event is written: @@ -19,9 +22,11 @@ collection but can never opt a user in. ## Canonical consent resource -MXC owns one immutable, versioned consent resource. Every EXE and SDK presenter -must show every supplied field verbatim. Hosts control layout, accessibility, -and native UI, but may not substitute wording. +The immutable, versioned authoring resource is +`src/core/wxc_common/resources/telemetry/consent/en-US.json`. The build embeds +it in `wxc_common::telemetry::consent_prompt`. Every EXE and SDK presenter must +show every supplied field verbatim. Hosts control layout, accessibility, and +native UI, but may not substitute wording. Current resource: @@ -32,15 +37,17 @@ Current resource: ### Title -> Help improve Microsoft eXecution Container (MXC) +```text +Help improve Microsoft eXecution Container (MXC) +``` ### Body -> Help improve MXC by sharing optional diagnostic data with Microsoft. -> If enabled, MXC sends diagnostic information about product usage, -> performance, and reliability. MXC does not send your commands, file paths, -> credentials, or other customer content. -> You can change your choice at any time. +```text +Help improve MXC by sharing optional diagnostic data with Microsoft. +If enabled, MXC sends diagnostic information about product usage, performance, and reliability. MXC does not send your commands, file paths, credentials, or other customer content. +You can change your choice at any time. +``` ### Actions @@ -160,7 +167,7 @@ in control; neither creates consent. A consent request made while policy is blocked does not invoke the presenter or persist dormant consent. Withdrawal remains available while blocked. -## Per-run enablement +## Per-run enablement and integration boundary Consent is not an execution switch. Each run must also request telemetry with the stable top-level `telemetry.enabled: true` setting. The switch never @@ -194,10 +201,10 @@ rename the provider or events. ## Validation and release gate -Implementations should include deterministic coverage for policy evaluation, +Implementations must include deterministic coverage for policy evaluation, prompt gating, withdrawal, corruption recovery, and concurrent consent-store mutation. The checked-in ETW smoke test only validates event emission; it is not a substitute for consent-flow coverage. -The versioned wording and data/control inventory still require Microsoft -privacy/legal approval before release. +Release requires Microsoft privacy/legal approval of the versioned wording and +data/control inventory. diff --git a/docs/telemetry/telemetry.md b/docs/telemetry/telemetry.md index a734d7195..ed8e9b630 100644 --- a/docs/telemetry/telemetry.md +++ b/docs/telemetry/telemetry.md @@ -97,7 +97,7 @@ Emitted on execution errors. | Field | Type | Description | |-------|------|-------------| | `mxc.sandbox_kind` | string | Containment kind requested by the caller (`process`, `vm`, or a concrete backend name) | -| `mxc.backend` | string | Concrete containment backend selected on the host | +| `mxc.backend` | string | Containment backend name | | `mxc.error_type` | string | Error category (`config_error`, `policy_error`, `process_error`, `timeout`, `init_error`, `internal_error`, `cancelled`, `unknown`) | | `mxc.exit_code` | int32 | Process exit code | | `mxc.phase` | string | State-aware lifecycle phase; empty for one-shot executions | @@ -107,7 +107,6 @@ Emitted on execution errors. > bounded `error_type` category and the numeric `exit_code` — never the > message string itself. - ### Crash telemetry (panic hook) When telemetry is active, the executors install a global diff --git a/sdk/dotnet/README.md b/sdk/dotnet/README.md index 3101dd2cc..bec7dd9d0 100644 --- a/sdk/dotnet/README.md +++ b/sdk/dotnet/README.md @@ -175,9 +175,9 @@ and its An IT administrator can still block MXC telemetry device-wide via MXC's own registry policy setting. See [`docs/telemetry/telemetry-administrative-policy.md`](../../docs/telemetry/telemetry-administrative-policy.md) -for the stable registry contract and interaction rules. Any eventual .NET -policy/consent query must fail closed rather than upgrading an unreadable -device state into collection. +for the stable registry contract and interaction rules. Policy and consent +queries fail closed rather than upgrading an unreadable device state into +collection. ## Projects diff --git a/src/core/wxc/src/main.rs b/src/core/wxc/src/main.rs index 52ef9e01a..e4b51a8fe 100644 --- a/src/core/wxc/src/main.rs +++ b/src/core/wxc/src/main.rs @@ -422,6 +422,7 @@ fn run_state_aware_main( telemetry_active, telemetry::TelemetryContext { backend, + sandbox_kind: backend, phase, correlation_vector: &correlation, }, diff --git a/src/core/wxc_common/Cargo.toml b/src/core/wxc_common/Cargo.toml index cd16e158c..b0a283a3b 100644 --- a/src/core/wxc_common/Cargo.toml +++ b/src/core/wxc_common/Cargo.toml @@ -9,6 +9,8 @@ microvm = ["dep:nanvix_common", "dep:uuid"] # Enables JSON Schema generation from the dedicated wire model. Off by default # so normal builds don't carry schemars. schema-gen = ["dep:schemars"] +# Exposes telemetry test harnesses to downstream integration tests. +test-support = [] [dependencies] serde = { workspace = true } @@ -38,3 +40,7 @@ libc = { workspace = true } [dev-dependencies] tempfile.workspace = true + +[build-dependencies] +serde.workspace = true +serde_json.workspace = true diff --git a/src/core/wxc_common/build.rs b/src/core/wxc_common/build.rs new file mode 100644 index 000000000..684a14102 --- /dev/null +++ b/src/core/wxc_common/build.rs @@ -0,0 +1,133 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +use serde::Deserialize; +use std::collections::HashSet; +use std::env; +use std::fs; +use std::io; +use std::path::PathBuf; + +#[derive(Debug, Deserialize)] +struct ConsentResource { + #[serde(rename = "resourceVersion")] + resource_version: u32, + locale: String, + messages: ConsentMessages, + #[serde(rename = "learnMoreUrl")] + learn_more_url: String, +} + +#[derive(Debug, Deserialize)] +struct ConsentMessages { + title: ConsentMessage, + body: ConsentMessage, + #[serde(rename = "affirmativeLabel")] + affirmative_label: ConsentMessage, + #[serde(rename = "negativeLabel")] + negative_label: ConsentMessage, + #[serde(rename = "learnMoreLabel")] + learn_more_label: ConsentMessage, +} + +#[derive(Debug, Deserialize)] +struct ConsentMessage { + id: String, + text: String, +} + +fn invalid_resource(message: impl Into) -> io::Error { + io::Error::new(io::ErrorKind::InvalidData, message.into()) +} + +fn rust_string(value: &str) -> String { + format!("{value:?}") +} + +fn main() -> Result<(), Box> { + let manifest_dir = PathBuf::from(env::var("CARGO_MANIFEST_DIR")?); + let resource_path = manifest_dir.join("resources/telemetry/consent/en-US.json"); + println!("cargo:rerun-if-changed={}", resource_path.display()); + + let resource: ConsentResource = serde_json::from_str(&fs::read_to_string(&resource_path)?)?; + if resource.locale != "en-US" { + return Err(invalid_resource(format!( + "the fallback consent resource must use locale en-US, got {}", + resource.locale + )) + .into()); + } + if resource.resource_version == 0 { + return Err(invalid_resource("the consent resource version must be nonzero").into()); + } + if resource.learn_more_url.is_empty() { + return Err(invalid_resource("the consent resource must include a learn-more URL").into()); + } + + let messages = [ + &resource.messages.title, + &resource.messages.body, + &resource.messages.affirmative_label, + &resource.messages.negative_label, + &resource.messages.learn_more_label, + ]; + if messages + .iter() + .any(|message| message.id.is_empty() || message.text.is_empty()) + { + return Err(invalid_resource("consent message IDs and text must be nonempty").into()); + } + let ids: HashSet<&str> = messages.iter().map(|message| message.id.as_str()).collect(); + if ids.len() != messages.len() { + return Err(invalid_resource("consent message IDs must be unique").into()); + } + + let generated = format!( + r#" pub const CONSENT_RESOURCE_VERSION: u32 = {}; + pub const FALLBACK_LOCALE: &str = {}; + + pub static EN_US_CONSENT_PROMPT: ConsentPrompt = ConsentPrompt {{ + resource_version: CONSENT_RESOURCE_VERSION, + locale: FALLBACK_LOCALE, + title: ConsentMessage {{ + id: {}, + text: {}, + }}, + body: ConsentMessage {{ + id: {}, + text: {}, + }}, + affirmative_label: ConsentMessage {{ + id: {}, + text: {}, + }}, + negative_label: ConsentMessage {{ + id: {}, + text: {}, + }}, + learn_more_label: ConsentMessage {{ + id: {}, + text: {}, + }}, + learn_more_url: {}, +}}; +"#, + resource.resource_version, + rust_string(&resource.locale), + rust_string(&resource.messages.title.id), + rust_string(&resource.messages.title.text), + rust_string(&resource.messages.body.id), + rust_string(&resource.messages.body.text), + rust_string(&resource.messages.affirmative_label.id), + rust_string(&resource.messages.affirmative_label.text), + rust_string(&resource.messages.negative_label.id), + rust_string(&resource.messages.negative_label.text), + rust_string(&resource.messages.learn_more_label.id), + rust_string(&resource.messages.learn_more_label.text), + rust_string(&resource.learn_more_url), + ); + + let out_dir = PathBuf::from(env::var("OUT_DIR")?); + fs::write(out_dir.join("telemetry_consent_resource.rs"), generated)?; + Ok(()) +} diff --git a/src/core/wxc_common/resources/telemetry/consent/en-US.json b/src/core/wxc_common/resources/telemetry/consent/en-US.json new file mode 100644 index 000000000..228efa0c7 --- /dev/null +++ b/src/core/wxc_common/resources/telemetry/consent/en-US.json @@ -0,0 +1,27 @@ +{ + "resourceVersion": 1, + "locale": "en-US", + "messages": { + "title": { + "id": "telemetry-consent-title", + "text": "Help improve Microsoft eXecution Container (MXC)" + }, + "body": { + "id": "telemetry-consent-body", + "text": "Help improve MXC by sharing optional diagnostic data with Microsoft.\nIf enabled, MXC sends diagnostic information about product usage, performance, and reliability. MXC does not send your commands, file paths, credentials, or other customer content.\nYou can change your choice at any time." + }, + "affirmativeLabel": { + "id": "telemetry-consent-affirmative-label", + "text": "Yes" + }, + "negativeLabel": { + "id": "telemetry-consent-negative-label", + "text": "No" + }, + "learnMoreLabel": { + "id": "telemetry-consent-learn-more-label", + "text": "Privacy Statement" + } + }, + "learnMoreUrl": "https://go.microsoft.com/fwlink/?linkid=521839" +} diff --git a/src/core/wxc_common/src/telemetry/consent.rs b/src/core/wxc_common/src/telemetry/consent.rs new file mode 100644 index 000000000..773ab6088 --- /dev/null +++ b/src/core/wxc_common/src/telemetry/consent.rs @@ -0,0 +1,2197 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Persisted, per-user telemetry consent. +//! +//! See `docs/telemetry/telemetry-consent-design.md` for the full design and +//! privacy rationale. In short: +//! +//! - MXC does not and must not collect telemetry on any platform other than +//! Windows, so this module is the **single** consent surface — and it is +//! compiled out entirely on non-Windows targets, replaced by a stub that +//! never touches disk and always reports [`ConsentState::NotApplicable`]. +//! - On Windows, consent is a per-user choice persisted at +//! `%LOCALAPPDATA%\mxc\telemetry-consent.json`. The default (no file, or an +//! unreadable/corrupt one) is [`ConsentState::Undetermined`], which is +//! treated as "not collecting" everywhere telemetry gating is decided +//! ([`super::is_enabled`]) — MXC fails closed, never open. +//! - This module never emits a telemetry event for a consent transition +//! itself; flipping the flag is a silent, local, atomic file write. + +// Serde, the persisted record, and its helpers exist only for the Windows +// consent store; the non-Windows stub persists nothing. +#[cfg(target_os = "windows")] +use serde::{Deserialize, Serialize}; +use std::fmt; + +use super::consent_prompt::{prompt_for_locale, ConsentPrompt}; + +/// Current schema version for the persisted consent record. Bump when the +/// on-disk shape changes in a way that isn't purely additive; unknown/older +/// versions are treated as [`ConsentState::Undetermined`] on read (fail +/// closed) rather than guessed at. +#[cfg(target_os = "windows")] +const CONSENT_SCHEMA_VERSION: u32 = 2; + +/// The user's telemetry consent decision. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConsentState { + /// The user has explicitly agreed to telemetry collection. + Granted, + /// The user has explicitly declined telemetry collection. + Denied, + /// No decision has been recorded yet (fresh install, or a corrupt/missing + /// store). Treated identically to `Denied` for gating purposes — the only + /// difference is that a host application should offer a first-run prompt + /// when it sees this state. + Undetermined, + /// Not a Windows host. MXC does not collect telemetry here, so consent is + /// not a meaningful concept — hosts must not offer a consent prompt at + /// all on these platforms. + NotApplicable, +} + +/// Why persisted consent does not currently authorize collection. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConsentStatusReason { + /// No consent record exists for this user. + NoRecord, + /// The consent store could not be read. + StoreUnreadable, + /// The consent record was not valid JSON or did not have a recognized + /// consent value. + StoreMalformed, + /// The persisted consent schema is not supported by this build. + ConsentSchemaUnsupported, + /// A stored grant predates versioned canonical consent language. + PromptVersionMissing, + /// A stored grant references a different canonical prompt version. + PromptVersionUnsupported, + /// Telemetry consent is not applicable on this platform. + NotApplicable, +} + +impl ConsentStatusReason { + /// Stable wire representation used by status APIs and bindings. + pub fn as_str(&self) -> &'static str { + match self { + Self::NoRecord => "no-record", + Self::StoreUnreadable => "store-unreadable", + Self::StoreMalformed => "store-malformed", + Self::ConsentSchemaUnsupported => "consent-schema-unsupported", + Self::PromptVersionMissing => "prompt-version-missing", + Self::PromptVersionUnsupported => "prompt-version-unsupported", + Self::NotApplicable => "not-applicable", + } + } +} + +/// Persisted and effective telemetry consent at one point in time. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ConsentStatus { + /// Decision found in the consent record, when one could be recovered. + pub stored_state: ConsentState, + /// State used by the telemetry gate after validating the record and prompt + /// version. + pub effective_state: ConsentState, + /// Fail-closed reason when the record is absent, invalid, or inapplicable. + pub reason: Option, +} + +impl ConsentStatus { + /// Whether an explicit consent request may offer the canonical prompt. + pub fn needs_prompt(&self) -> bool { + matches!(self.effective_state, ConsentState::Undetermined) + } +} + +/// Explicit result returned by a consent presenter. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConsentDecision { + Yes, + No, + Dismissed, +} + +/// Result of requesting or withdrawing telemetry consent. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum ConsentActionResult { + Granted, + Denied, + Dismissed, + Withdrawn, + AlreadyGranted, + PolicyBlocked, + NotApplicable, +} + +/// Consent action result together with the resulting status and policy. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ConsentActionOutcome { + pub result: ConsentActionResult, + pub status: ConsentStatus, + pub policy: super::policy::PolicyState, +} + +/// Failure to present or persist a consent decision. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum ConsentActionError { + Presenter(String), + Persist(String), +} + +impl fmt::Display for ConsentActionError { + fn fmt(&self, formatter: &mut fmt::Formatter<'_>) -> fmt::Result { + match self { + Self::Presenter(message) => write!(formatter, "consent presenter failed: {message}"), + Self::Persist(message) => write!(formatter, "failed to persist consent: {message}"), + } + } +} + +impl std::error::Error for ConsentActionError {} + +impl ConsentState { + /// Stable, lowercase wire representation used by the CLI flags, the FFI + /// boundary, and the SDKs. Kept separate from `Debug` so the on-the-wire + /// strings never drift if the Rust variant names change. + pub fn as_str(&self) -> &'static str { + match self { + ConsentState::Granted => "granted", + ConsentState::Denied => "denied", + ConsentState::Undetermined => "undetermined", + ConsentState::NotApplicable => "not-applicable", + } + } + + /// Whether telemetry may be collected under this consent state alone + /// (still subject to the explicit config kill-switch — see + /// [`super::is_enabled`]). + pub fn allows_collection(&self) -> bool { + matches!(self, ConsentState::Granted) + } + + /// Whether a hosting application should offer its own first-run consent + /// prompt for this state. + /// + /// This shared predicate keeps all consumer surfaces consistent. It is + /// false for [`NotApplicable`](ConsentState::NotApplicable), because MXC + /// collects no telemetry off Windows. + pub fn needs_prompt(&self) -> bool { + matches!(self, ConsentState::Undetermined) + } +} + +/// Whether a hosting application should offer its own first-run telemetry +/// consent prompt right now. +/// +/// This is [`get_consent`]`().`[`needs_prompt`](ConsentState::needs_prompt)`()` +/// additionally suppressed when an administrator has denied telemetry via +/// [`super::policy`]. Prompting under an administrative denial would be asking +/// the user to decide something MXC would then ignore, so the answer there is +/// always `false` — regardless of whether a decision has been recorded. +pub fn needs_consent_prompt() -> bool { + if super::policy::is_blocked_by_policy() { + return false; + } + get_consent().needs_prompt() +} + +/// The on-disk consent record. Additive fields only; `source` and +/// `prompted_mxc_version` are provenance for support/debugging and are never +/// transmitted anywhere. +#[cfg(target_os = "windows")] +#[derive(Debug, Clone, Serialize, Deserialize)] +struct ConsentRecord { + #[serde(rename = "schemaVersion")] + schema_version: u32, + consent: String, + #[serde(default)] + source: String, + #[serde(rename = "promptedMxcVersion", default)] + prompted_mxc_version: String, + #[serde(rename = "promptResourceVersion", default)] + prompt_resource_version: Option, + #[serde(rename = "promptLocale", default)] + prompt_locale: String, + /// Unix seconds at the time of the last grant/revoke. Debug/support + /// provenance only — never used for gating — so a plain epoch integer is + /// enough; `#[serde(default)]` means older or foreign records missing + /// this field just read back as `0` rather than failing to parse. + #[serde(rename = "updatedAtEpoch", default)] + updated_at_epoch: u64, +} + +/// Returns the current, persisted telemetry consent state. +/// +/// Fail-closed: a missing file, an unreadable file, unparseable JSON, or an +/// unrecognized `schemaVersion` all resolve to [`ConsentState::Undetermined`] +/// — never to `Granted`. Always [`ConsentState::NotApplicable`] on +/// non-Windows platforms, without any filesystem access. +pub fn get_consent() -> ConsentState { + get_status().effective_state +} + +/// Returns persisted and effective consent plus a typed fail-closed reason. +pub fn get_status() -> ConsentStatus { + platform::read_status() +} + +#[cfg(test)] +pub(crate) fn set_consent(granted: bool, source: &str) -> Result<(), String> { + platform::write(granted, source) +} + +/// Request telemetry consent through a host-owned presenter. +/// +/// The presenter receives the complete canonical resource. Only an explicit +/// [`ConsentDecision::Yes`] returned from this invocation can create a grant. +pub fn request_consent( + locale: Option<&str>, + presenter: F, +) -> Result +where + F: FnOnce(&ConsentPrompt) -> Result, +{ + let (prompt, policy, status) = match consent_preflight(locale) { + ConsentPreflight::Complete(outcome) => return Ok(outcome), + ConsentPreflight::Present { + prompt, + policy, + status, + } => (prompt, policy, status), + }; + + let decision = presenter(prompt).map_err(ConsentActionError::Presenter)?; + persist_presented_decision(decision, prompt, policy, status) +} + +/// Asynchronous counterpart to [`request_consent`]. +/// +/// The presenter callback is asynchronous. The surrounding synchronous +/// registry/filesystem work is offloaded to short-lived worker threads so the +/// caller's executor thread does not run the blocking persistence path itself. +pub async fn request_consent_async( + locale: Option<&str>, + presenter: F, +) -> Result +where + F: FnOnce(&ConsentPrompt) -> Fut, + Fut: std::future::Future>, +{ + let locale = locale.map(str::to_owned); + match BlockingTask::spawn(move || consent_preflight(locale.as_deref())).await { + ConsentPreflight::Complete(outcome) => Ok(outcome), + ConsentPreflight::Present { + prompt, + policy, + status, + } => { + let decision = presenter(prompt) + .await + .map_err(ConsentActionError::Presenter)?; + BlockingTask::spawn(move || { + persist_presented_decision(decision, prompt, policy, status) + }) + .await + } + } +} + +struct BlockingTask { + shared: std::sync::Arc>, +} + +struct BlockingTaskState { + result: std::sync::Mutex>>, + waker: std::sync::Mutex>, +} + +impl BlockingTask { + fn spawn(operation: impl FnOnce() -> T + Send + 'static) -> Self { + let shared = std::sync::Arc::new(BlockingTaskState { + result: std::sync::Mutex::new(None), + waker: std::sync::Mutex::new(None), + }); + let worker = std::sync::Arc::clone(&shared); + std::thread::spawn(move || { + let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(operation)); + *worker.result.lock().unwrap_or_else(|e| e.into_inner()) = Some(result); + if let Some(waker) = worker + .waker + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take() + { + waker.wake(); + } + }); + Self { shared } + } +} + +impl std::future::Future for BlockingTask { + type Output = T; + + fn poll( + self: std::pin::Pin<&mut Self>, + cx: &mut std::task::Context<'_>, + ) -> std::task::Poll { + if let Some(result) = self + .shared + .result + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take() + { + return std::task::Poll::Ready(match result { + Ok(value) => value, + Err(payload) => std::panic::resume_unwind(payload), + }); + } + + *self.shared.waker.lock().unwrap_or_else(|e| e.into_inner()) = Some(cx.waker().clone()); + + match self + .shared + .result + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take() + { + Some(result) => { + self.shared + .waker + .lock() + .unwrap_or_else(|e| e.into_inner()) + .take(); + std::task::Poll::Ready(match result { + Ok(value) => value, + Err(payload) => std::panic::resume_unwind(payload), + }) + } + None => std::task::Poll::Pending, + } + } +} + +enum ConsentPreflight { + Complete(ConsentActionOutcome), + Present { + prompt: &'static ConsentPrompt, + policy: super::policy::PolicyState, + status: ConsentStatus, + }, +} + +fn consent_preflight(locale: Option<&str>) -> ConsentPreflight { + let policy = super::policy::get_policy(); + let status = get_status(); + + if status.effective_state == ConsentState::NotApplicable { + return ConsentPreflight::Complete(ConsentActionOutcome { + result: ConsentActionResult::NotApplicable, + status, + policy, + }); + } + if !policy.allows_collection() { + return ConsentPreflight::Complete(ConsentActionOutcome { + result: ConsentActionResult::PolicyBlocked, + status, + policy, + }); + } + if status.effective_state == ConsentState::Granted { + return ConsentPreflight::Complete(ConsentActionOutcome { + result: ConsentActionResult::AlreadyGranted, + status, + policy, + }); + } + + let prompt = prompt_for_locale(locale); + ConsentPreflight::Present { + prompt, + policy, + status, + } +} + +fn persist_presented_decision( + decision: ConsentDecision, + prompt: &ConsentPrompt, + policy: super::policy::PolicyState, + status: ConsentStatus, +) -> Result { + platform::with_store_lock(|| { + let current_policy = super::policy::get_policy(); + let current_status = platform::read_status_unlocked(); + if current_status.effective_state == ConsentState::NotApplicable { + return Ok(ConsentActionOutcome { + result: ConsentActionResult::NotApplicable, + status: current_status, + policy: current_policy, + }); + } + if !current_policy.allows_collection() { + return Ok(ConsentActionOutcome { + result: ConsentActionResult::PolicyBlocked, + status: current_status, + policy: current_policy, + }); + } + if current_status.effective_state == ConsentState::Granted + && !matches!(decision, ConsentDecision::No) + { + return Ok(ConsentActionOutcome { + result: ConsentActionResult::AlreadyGranted, + status: current_status, + policy: current_policy, + }); + } + let is_concurrent_grant_being_withdrawn = matches!(decision, ConsentDecision::No) + && current_status.effective_state == ConsentState::Granted + && status.effective_state != ConsentState::Granted; + if current_policy != policy + || (current_status.effective_state != status.effective_state + && !is_concurrent_grant_being_withdrawn) + { + return Err( + "consent state changed while the presenter was open; no decision was written" + .to_string(), + ); + } + + match decision { + ConsentDecision::Yes => { + platform::write_presented(true, "prompt", prompt)?; + Ok(ConsentActionOutcome { + result: ConsentActionResult::Granted, + status: platform::read_status_unlocked(), + policy: current_policy, + }) + } + ConsentDecision::No => { + platform::write_presented(false, "prompt", prompt)?; + Ok(ConsentActionOutcome { + result: ConsentActionResult::Denied, + status: platform::read_status_unlocked(), + policy: current_policy, + }) + } + ConsentDecision::Dismissed => Ok(ConsentActionOutcome { + result: ConsentActionResult::Dismissed, + status: current_status, + policy: current_policy, + }), + } + }) + .map_err(ConsentActionError::Persist) +} + +/// Idempotently withdraw telemetry consent. +/// +/// Withdrawal does not require a permitting administrative policy. Off +/// Windows it succeeds as a typed `NotApplicable` result without storage. +pub fn withdraw_consent() -> Result { + let current_status = get_status(); + if current_status.effective_state == ConsentState::NotApplicable { + return Ok(ConsentActionOutcome { + result: ConsentActionResult::NotApplicable, + status: current_status, + policy: super::policy::get_policy(), + }); + } + + platform::with_store_lock(|| { + platform::begin_withdrawal()?; + platform::write(false, "withdrawal")?; + platform::finish_withdrawal()?; + Ok(ConsentActionOutcome { + result: ConsentActionResult::Withdrawn, + status: platform::read_status_unlocked(), + policy: super::policy::get_policy(), + }) + }) + .map_err(ConsentActionError::Persist) +} + +/// Current time as Unix seconds. Debug/support provenance only (see +/// [`ConsentRecord::updated_at_epoch`]) — a raw epoch avoids pulling in a +/// date-formatting dependency, or hand-rolling calendar math, just to stamp +/// a field nothing ever gates on. +#[cfg(target_os = "windows")] +fn now_epoch_seconds() -> u64 { + use std::time::{SystemTime, UNIX_EPOCH}; + SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0) +} + +// --------------------------------------------------------------------------- +// Windows implementation — real, persisted, per-user consent store. +// --------------------------------------------------------------------------- + +#[cfg(target_os = "windows")] +mod platform { + use super::{ + now_epoch_seconds, ConsentRecord, ConsentState, ConsentStatus, ConsentStatusReason, + CONSENT_SCHEMA_VERSION, + }; + use crate::telemetry::consent_prompt::{CONSENT_RESOURCE_VERSION, FALLBACK_LOCALE}; + use std::fs; + use std::io::{Read, Write}; + use std::path::{Path, PathBuf}; + use std::time::Duration; + + /// Number of attempts for filesystem operations that can transiently fail + /// under Windows file-lock contention (AV real-time scanning, the search + /// indexer, or a concurrent `wxc-exec` grant/revoke in another process). + const IO_RETRY_ATTEMPTS: u32 = 5; + #[cfg(test)] + const IO_RETRY_DELAY: Duration = Duration::ZERO; + #[cfg(not(test))] + const IO_RETRY_DELAY: Duration = Duration::from_millis(20); + #[cfg(test)] + const STORE_LOCK_RETRY_DELAY: Duration = Duration::from_millis(1); + #[cfg(not(test))] + const STORE_LOCK_RETRY_DELAY: Duration = Duration::from_millis(20); + + /// `ERROR_SHARING_VIOLATION` — another handle holds the file with a + /// conflicting share mode (AV scanner, indexer, racing sibling process). + const ERROR_SHARING_VIOLATION: i32 = 32; + /// `ERROR_LOCK_VIOLATION` — a byte-range lock is held on the file. + const ERROR_LOCK_VIOLATION: i32 = 33; + const MAX_CONSENT_FILE_BYTES: u64 = 16 * 1024; + const WITHDRAWAL_PENDING_FILE: &str = "telemetry-consent.withdrawal-pending"; + const CONSENT_LOCK_FILE: &str = "telemetry-consent.lock"; + const WITHDRAWAL_MARKER_STALE_AFTER: Duration = Duration::from_secs(5); + + /// Resolves the per-user `%LocalAppData%` directory through the Windows + /// known-folder API rather than trusting `LOCALAPPDATA`. A parent process + /// controls the child's environment, so trusting that variable could + /// redirect the consent store and forge a grant. The resolved path is + /// cached for the process, but only after a successful resolution so a + /// transient failure can be retried later in the same process. + fn known_folder_local_app_data() -> Option { + static CACHED: std::sync::OnceLock = std::sync::OnceLock::new(); + cache_successful_resolution(&CACHED, resolve_known_folder_local_app_data) + } + + fn cache_successful_resolution( + cache: &std::sync::OnceLock, + resolver: impl FnOnce() -> Option, + ) -> Option { + if let Some(cached) = cache.get() { + return Some(cached.clone()); + } + + let resolved = resolver()?; + let _ = cache.set(resolved.clone()); + Some(resolved) + } + + /// Opens the current **process** token with `TOKEN_QUERY | TOKEN_IMPERSONATE`. + /// + /// Passing `None` to `SHGetKnownFolderPath` would resolve against the + /// calling *thread's* token — the impersonated one, if the host has + /// impersonated another user. That would both invalidate the memoization + /// above and let an embedding host redirect the consent store simply by + /// impersonating, which is the same class of attack the known-folder API + /// is being used to prevent in the first place. Binding explicitly to the + /// process token keeps the answer a stable property of the process. + /// + /// `SHGetKnownFolderPath` requires the token it is handed to be opened + /// with both `TOKEN_QUERY` *and* `TOKEN_IMPERSONATE` (it duplicates the + /// handle into an impersonation token internally); on systems that + /// enforce that contract, a `TOKEN_QUERY`-only handle makes resolution + /// fail, so consent would always read as `Undetermined` and every + /// grant/revoke would fail closed. + fn process_token() -> Option { + use windows::Win32::Foundation::HANDLE; + use windows::Win32::Security::{TOKEN_IMPERSONATE, TOKEN_QUERY}; + use windows::Win32::System::Threading::{GetCurrentProcess, OpenProcessToken}; + + let mut token = HANDLE::default(); + // SAFETY: `GetCurrentProcess` returns a pseudo-handle that needs no + // release, and `token` is a valid out-pointer for the duration of the + // call. On success the returned handle is immediately wrapped in + // `OwnedHandle`, whose `Drop` closes it exactly once. + unsafe { + OpenProcessToken( + GetCurrentProcess(), + TOKEN_QUERY | TOKEN_IMPERSONATE, + &mut token, + ) + } + .ok()?; + Some(crate::process_util::OwnedHandle::new(token)) + } + + fn resolve_known_folder_local_app_data() -> Option { + use crate::string_util::CoTaskMemPWSTR; + use windows::Win32::UI::Shell::{ + FOLDERID_LocalAppData, SHGetKnownFolderPath, KF_FLAG_DEFAULT, + }; + + // SAFETY: `FOLDERID_LocalAppData` is a valid, static, well-known GUID + // constant. The token argument is this process's own token, so the + // result is independent of any thread impersonation the host may have + // in effect (see `process_token`); it stays alive for the duration of + // the call because `token` is still in scope. The returned `PWSTR` is + // COM-allocated and is immediately wrapped in `CoTaskMemPWSTR`, whose + // `Drop` frees it via `CoTaskMemFree` exactly once. + let token = process_token()?; + let pwstr = unsafe { + SHGetKnownFolderPath(&FOLDERID_LocalAppData, KF_FLAG_DEFAULT, Some(token.get())) + } + .ok()?; + let owned = CoTaskMemPWSTR::new(pwstr.0); + let resolved = owned.to_string_lossy(); + if resolved.is_empty() { + None + } else { + Some(PathBuf::from(resolved)) + } + } + + /// Test-only escape hatch so deterministic tests — this crate's own unit + /// tests, `mxc_ffi`'s Rust tests, and a local `dotnet test`/`npm test` run + /// against a debug-profile native binary — can redirect the consent store + /// to a throwaway temp directory. + /// + /// Active only under `cfg(test)` (this crate's own test harness, which is + /// how CI exercises it — CI runs `cargo test --release`, so gating on + /// `debug_assertions` alone would silently drop every consent test from + /// CI *and* let them read and overwrite the real store) or in a debug + /// build (which is what lets `mxc_ffi`'s cross-crate tests, where + /// `cfg(test)` does not apply to this crate, redirect it). + /// + /// Neither condition holds for a binary MXC ships: a release + /// `wxc-exec.exe` is not compiled with `--test` and has + /// `debug_assertions` off, so this branch and the env-var read backing it + /// are compiled out entirely, and a parent process has no way to redirect + /// the store. + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] + fn debug_local_app_data_override() -> Option { + crate::telemetry::consent::test_support::current_local_app_data_override() + .or_else(crate::telemetry::consent::test_support::inherited_local_app_data_override) + } + + fn local_app_data_dir() -> Option { + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] + if let Some(over) = debug_local_app_data_override() { + return Some(over); + } + known_folder_local_app_data() + } + + struct StoreLock { + file: Option, + } + + impl Drop for StoreLock { + fn drop(&mut self) { + self.file.take(); + } + } + + pub(super) fn with_store_lock( + operation: impl FnOnce() -> Result, + ) -> Result { + use std::os::windows::fs::OpenOptionsExt; + + let path = consent_file_path().ok_or_else(|| { + "could not resolve %LocalAppData%; cannot lock telemetry consent".to_string() + })?; + let dir = path + .parent() + .ok_or_else(|| "invalid telemetry consent path".to_string())?; + fs::create_dir_all(dir).map_err(|e| format!("failed to create {}: {e}", dir.display()))?; + let lock_path = dir.join(CONSENT_LOCK_FILE); + let mut attempts = 0; + let file = loop { + let mut options = fs::OpenOptions::new(); + options + .write(true) + .create_new(true) + .custom_flags(windows::Win32::Storage::FileSystem::FILE_FLAG_DELETE_ON_CLOSE.0); + match options.open(&lock_path) { + Ok(file) => break file, + Err(error) + if attempts + 1 < IO_RETRY_ATTEMPTS + && (error.kind() == std::io::ErrorKind::AlreadyExists + || is_transient_io_error(&error)) => + { + attempts += 1; + std::thread::sleep(STORE_LOCK_RETRY_DELAY); + } + Err(error) => { + return Err(format!("failed to lock telemetry consent: {error}")); + } + } + }; + let _lock = StoreLock { file: Some(file) }; + operation() + } + + /// Test-only accessor for [`known_folder_local_app_data`], which is + /// otherwise private to this module. + #[cfg(test)] + pub(super) fn known_folder_local_app_data_for_test() -> Option { + known_folder_local_app_data() + } + + #[cfg(test)] + pub(super) fn local_app_data_dir_for_test() -> Option { + local_app_data_dir() + } + + #[cfg(test)] + pub(super) fn cache_successful_resolution_for_test( + cache: &std::sync::OnceLock, + resolver: impl FnOnce() -> Option, + ) -> Option { + cache_successful_resolution(cache, resolver) + } + + /// Per-user consent store path: `\mxc\telemetry-consent.json`. + /// Per-user (not `%ProgramData%`) because consent is a personal choice — + /// multiple people sharing one machine each control their own, with no + /// elevation required to change it (mirrors `wxc-exec.exe` never + /// self-elevating; see `docs/host-prep.md`). + fn consent_file_path() -> Option { + local_app_data_dir().map(|dir| dir.join("mxc").join("telemetry-consent.json")) + } + + /// Whether an I/O error is plausibly a *transient* lock/share conflict + /// worth waiting out, as opposed to a settled answer. + /// + /// This distinction is load-bearing for startup latency, not just tidiness: + /// the overwhelmingly common state is "no consent file yet" (every fresh + /// install, and every user who has not yet been prompted), which surfaces + /// as `NotFound`. Retrying that would add `IO_RETRY_ATTEMPTS - 1` sleeps — + /// 80 ms — to the consent read on the critical path of *every* sandbox + /// launch, to re-confirm a result that cannot change. + fn is_transient_io_error(e: &std::io::Error) -> bool { + if matches!( + e.raw_os_error(), + Some(ERROR_SHARING_VIOLATION) | Some(ERROR_LOCK_VIOLATION) + ) { + return true; + } + matches!( + e.kind(), + std::io::ErrorKind::PermissionDenied | std::io::ErrorKind::Interrupted + ) + } + + /// Retries `op` up to [`IO_RETRY_ATTEMPTS`] times with a short fixed + /// delay, to ride out transient Windows file-lock contention (AV + /// scanning, indexing, a racing sibling process) rather than failing the + /// very first time a lock is briefly held by someone else. + /// + /// Only [`is_transient_io_error`] failures are retried; anything else + /// (notably `NotFound`) is returned immediately. + fn with_io_retry(mut op: impl FnMut() -> std::io::Result) -> std::io::Result { + let mut attempt = 0; + loop { + match op() { + Ok(v) => return Ok(v), + Err(e) if attempt + 1 < IO_RETRY_ATTEMPTS && is_transient_io_error(&e) => { + attempt += 1; + std::thread::sleep(IO_RETRY_DELAY); + } + Err(e) => return Err(e), + } + } + } + + /// Test-only accessor for [`with_io_retry`], which is otherwise private + /// to this module. + #[cfg(test)] + pub(super) fn with_io_retry_for_test( + op: impl FnMut() -> std::io::Result, + ) -> std::io::Result { + with_io_retry(op) + } + + /// Test-only constructor for a synthetic transient error, so retry tests + /// don't have to guess which raw OS codes this module treats as transient. + #[cfg(test)] + pub(super) fn transient_io_error_for_test() -> std::io::Error { + std::io::Error::from_raw_os_error(ERROR_SHARING_VIOLATION) + } + + #[cfg(test)] + pub(super) const IO_RETRY_ATTEMPTS_FOR_TEST: u32 = IO_RETRY_ATTEMPTS; + + #[derive(Debug, Clone, Copy, PartialEq, Eq)] + pub(super) enum WithdrawalMarkerState { + Absent, + Pending, + Stale, + } + + fn withdrawal_marker_state(marker: &Path) -> std::io::Result { + match with_io_retry(|| fs::metadata(marker)) { + Ok(metadata) => { + let is_stale = metadata + .modified() + .ok() + .and_then(|modified| std::time::SystemTime::now().duration_since(modified).ok()) + .is_some_and(|age| age >= WITHDRAWAL_MARKER_STALE_AFTER); + Ok(if is_stale { + WithdrawalMarkerState::Stale + } else { + WithdrawalMarkerState::Pending + }) + } + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + Ok(WithdrawalMarkerState::Absent) + } + Err(error) => Err(error), + } + } + + #[cfg(test)] + pub(super) fn read_status_unlocked_with_marker_for_test( + marker_state: std::io::Result, + ) -> ConsentStatus { + read_status_unlocked_inner(|_| marker_state, recover_stale_withdrawal_marker) + } + + pub(super) fn read_status() -> ConsentStatus { + read_status_unlocked_inner(withdrawal_marker_state, |path, marker| { + with_store_lock(|| recover_stale_withdrawal_marker(path, marker)) + }) + } + + pub(super) fn read_status_unlocked() -> ConsentStatus { + read_status_unlocked_inner(withdrawal_marker_state, recover_stale_withdrawal_marker) + } + + fn read_status_unlocked_inner( + marker_check: impl FnOnce(&Path) -> std::io::Result, + recover_stale: impl FnOnce(&Path, &Path) -> Result<(), String>, + ) -> ConsentStatus { + fn unreadable_status() -> ConsentStatus { + ConsentStatus { + stored_state: ConsentState::Undetermined, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::StoreUnreadable), + } + } + + fn no_record_status() -> ConsentStatus { + ConsentStatus { + stored_state: ConsentState::Undetermined, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::NoRecord), + } + } + + let Some(path) = consent_file_path() else { + return unreadable_status(); + }; + let Some(dir) = path.parent() else { + return unreadable_status(); + }; + let marker_path = dir.join(WITHDRAWAL_PENDING_FILE); + match marker_check(&marker_path) { + Ok(WithdrawalMarkerState::Absent) => {} + Ok(WithdrawalMarkerState::Pending) | Err(_) => { + return unreadable_status(); + } + Ok(WithdrawalMarkerState::Stale) => { + if recover_stale(&path, &marker_path).is_err() { + return unreadable_status(); + } + } + } + let data = match with_io_retry(|| read_bounded(&path)) { + Ok(data) => data, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => { + return no_record_status() + } + Err(_) => return unreadable_status(), + }; + let Ok(record) = serde_json::from_str::(&data) else { + return ConsentStatus { + stored_state: ConsentState::Undetermined, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::StoreMalformed), + }; + }; + + let stored_state = match record.consent.as_str() { + "granted" => ConsentState::Granted, + "denied" => ConsentState::Denied, + _ => { + return ConsentStatus { + stored_state: ConsentState::Undetermined, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::StoreMalformed), + }; + } + }; + + if stored_state == ConsentState::Denied { + return ConsentStatus { + stored_state, + effective_state: ConsentState::Denied, + reason: None, + }; + } + + if record.schema_version != CONSENT_SCHEMA_VERSION { + return ConsentStatus { + stored_state, + effective_state: ConsentState::Undetermined, + reason: Some(if record.prompt_resource_version.is_none() { + ConsentStatusReason::PromptVersionMissing + } else { + ConsentStatusReason::ConsentSchemaUnsupported + }), + }; + } + + match record.prompt_resource_version { + Some(CONSENT_RESOURCE_VERSION) if !record.prompt_locale.is_empty() => ConsentStatus { + stored_state, + effective_state: ConsentState::Granted, + reason: None, + }, + None => ConsentStatus { + stored_state, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::PromptVersionMissing), + }, + Some(_) => ConsentStatus { + stored_state, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::PromptVersionUnsupported), + }, + } + } + + fn read_bounded(path: &Path) -> std::io::Result { + let file = fs::File::open(path)?; + let mut bytes = Vec::new(); + file.take(MAX_CONSENT_FILE_BYTES + 1) + .read_to_end(&mut bytes)?; + if bytes.len() as u64 > MAX_CONSENT_FILE_BYTES { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidData, + "telemetry consent file exceeds the supported size", + )); + } + String::from_utf8(bytes) + .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error)) + } + + fn recover_stale_withdrawal_marker(_path: &Path, marker: &Path) -> Result<(), String> { + match withdrawal_marker_state(marker) + .map_err(|e| format!("failed to inspect stale withdrawal marker: {e}"))? + { + WithdrawalMarkerState::Absent => Ok(()), + WithdrawalMarkerState::Pending | WithdrawalMarkerState::Stale => write_record( + false, + "withdrawal-recovery", + Some(CONSENT_RESOURCE_VERSION), + FALLBACK_LOCALE, + ), + } + } + + /// Best-effort cleanup of a leftover temp file; failures here are not + /// reported since the operation they were cleaning up after has already + /// failed (or, on the success path, this is a pure best-effort tidy-up). + fn remove_best_effort(path: &Path) { + let _ = fs::remove_file(path); + } + + pub(super) fn write(granted: bool, source: &str) -> Result<(), String> { + write_record( + granted, + source, + Some(CONSENT_RESOURCE_VERSION), + FALLBACK_LOCALE, + ) + } + + pub(super) fn write_presented( + granted: bool, + source: &str, + prompt: &crate::telemetry::consent_prompt::ConsentPrompt, + ) -> Result<(), String> { + write_record( + granted, + source, + Some(prompt.resource_version), + prompt.locale, + ) + } + + pub(super) fn begin_withdrawal() -> Result<(), String> { + let path = consent_file_path().ok_or_else(|| { + "could not resolve %LocalAppData%; cannot persist telemetry consent".to_string() + })?; + let dir = path + .parent() + .ok_or_else(|| "invalid telemetry consent path".to_string())?; + fs::create_dir_all(dir).map_err(|e| format!("failed to create {}: {e}", dir.display()))?; + with_io_retry(|| fs::write(dir.join(WITHDRAWAL_PENDING_FILE), b"pending")) + .map_err(|e| format!("failed to mark withdrawal pending: {e}")) + } + + pub(super) fn finish_withdrawal() -> Result<(), String> { + let path = consent_file_path().ok_or_else(|| { + "could not resolve %LocalAppData%; cannot persist telemetry consent".to_string() + })?; + clear_withdrawal_marker(&path) + } + + fn write_record( + granted: bool, + source: &str, + prompt_resource_version: Option, + prompt_locale: &str, + ) -> Result<(), String> { + let path = consent_file_path().ok_or_else(|| { + "could not resolve %LocalAppData%; cannot persist telemetry consent".to_string() + })?; + let dir = path + .parent() + .ok_or_else(|| "invalid telemetry consent path".to_string())?; + fs::create_dir_all(dir).map_err(|e| format!("failed to create {}: {e}", dir.display()))?; + + let record = ConsentRecord { + schema_version: CONSENT_SCHEMA_VERSION, + consent: if granted { "granted" } else { "denied" }.to_string(), + source: source.to_string(), + prompted_mxc_version: crate::telemetry::version().to_string(), + prompt_resource_version, + prompt_locale: prompt_locale.to_string(), + updated_at_epoch: now_epoch_seconds(), + }; + let json = serde_json::to_string_pretty(&record) + .map_err(|e| format!("failed to serialize telemetry consent record: {e}"))?; + + // Atomic write: write to a *unique* temp file in the same directory + // (process id + a random suffix, so two concurrent `wxc-exec` + // grant/revoke invocations never share — and thus never race on — + // the same temp path), then rename over the real path. A crash + // mid-write never leaves a torn/corrupt file in place of a + // previously-valid one; if the rename itself fails, the temp file is + // removed rather than left behind as a leaked, orphaned artifact. + let unique = format!("{}-{:x}", std::process::id(), random_suffix()); + let tmp_path = path.with_extension(format!("json.{unique}.tmp")); + + if let Err(e) = with_io_retry(|| { + let mut file = fs::File::create(&tmp_path)?; + file.write_all(json.as_bytes())?; + file.sync_all() + }) { + remove_best_effort(&tmp_path); + return Err(format!("failed to write {}: {e}", tmp_path.display())); + } + if let Err(e) = with_io_retry(|| fs::rename(&tmp_path, &path)) { + remove_best_effort(&tmp_path); + return Err(format!("failed to finalize {}: {e}", path.display())); + } + clear_withdrawal_marker(&path) + } + + fn clear_withdrawal_marker(path: &Path) -> Result<(), String> { + let dir = path + .parent() + .ok_or_else(|| "invalid telemetry consent path".to_string())?; + match with_io_retry(|| fs::remove_file(dir.join(WITHDRAWAL_PENDING_FILE))) { + Ok(()) => Ok(()), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => Ok(()), + Err(e) => Err(format!("failed to clear withdrawal marker: {e}")), + } + } + + /// Returns a process-local unique suffix for a temporary filename. + fn random_suffix() -> u64 { + use std::sync::atomic::{AtomicU64, Ordering}; + static NEXT_SUFFIX: AtomicU64 = AtomicU64::new(0); + + NEXT_SUFFIX.fetch_add(1, Ordering::Relaxed) + } +} + +// --------------------------------------------------------------------------- +// Non-Windows stub — no file, no prompt, no pretend consent. +// --------------------------------------------------------------------------- + +#[cfg(not(target_os = "windows"))] +mod platform { + use super::{ConsentState, ConsentStatus, ConsentStatusReason}; + + pub(super) fn with_store_lock( + operation: impl FnOnce() -> Result, + ) -> Result { + operation() + } + + pub(super) fn read_status() -> ConsentStatus { + ConsentStatus { + stored_state: ConsentState::NotApplicable, + effective_state: ConsentState::NotApplicable, + reason: Some(ConsentStatusReason::NotApplicable), + } + } + + pub(super) fn read_status_unlocked() -> ConsentStatus { + read_status() + } + + pub(super) fn write(_granted: bool, _source: &str) -> Result<(), String> { + Err("telemetry is Windows-only; consent is not applicable on this platform".to_string()) + } + + pub(super) fn begin_withdrawal() -> Result<(), String> { + Err("telemetry is Windows-only; consent is not applicable on this platform".to_string()) + } + + pub(super) fn finish_withdrawal() -> Result<(), String> { + Ok(()) + } + + pub(super) fn write_presented( + _granted: bool, + _source: &str, + _prompt: &crate::telemetry::consent_prompt::ConsentPrompt, + ) -> Result<(), String> { + Err("telemetry is Windows-only; consent is not applicable on this platform".to_string()) + } +} + +// --------------------------------------------------------------------------- +// Test support — shared with `telemetry::mod`'s `is_enabled` tests so both +// modules can safely mutate the process-global consent-store override +// without racing each other under parallel test execution. +// +// This redirects `MXC_TEST_LOCALAPPDATA_OVERRIDE`, a debug-build-only escape +// hatch (see `platform::debug_local_app_data_override` above) — never the +// real `LOCALAPPDATA` variable, and never present at all in a release build. +// --------------------------------------------------------------------------- + +#[cfg(any(test, all(feature = "test-support", debug_assertions)))] +pub mod test_support { + use std::path::{Path, PathBuf}; + use std::sync::{Mutex, MutexGuard}; + + const LOCAL_APPDATA_OVERRIDE_ENV: &str = "MXC_TEST_LOCALAPPDATA_OVERRIDE"; + const LOCAL_APPDATA_OVERRIDE_OWNER_ENV: &str = "MXC_TEST_LOCALAPPDATA_OVERRIDE_OWNER_PID"; + + /// `get_consent`/`set_consent` read the debug-only override, which is + /// process-global state. Guarded internally by [`LocalAppDataGuard::set`] + /// so a caller can never forget to hold it — see that type's doc comment. + /// `pub(crate)` only so the rare test that must mutate the override + /// directly (bypassing the guard, e.g. to test the "no override set" + /// fallback path) can still serialize against guard-holding tests. + pub(crate) static ENV_LOCK: Mutex<()> = Mutex::new(()); + static LOCAL_APP_DATA_OVERRIDE: Mutex> = Mutex::new(None); + + /// Redirects the consent store (Windows only) to a fresh temp directory + /// for the lifetime of the guard, restoring the previous value on drop. + /// Acquires and holds [`ENV_LOCK`] for the guard's entire lifetime, so + /// tests cannot accidentally construct the guard without the lock. + /// A no-op holder on non-Windows, where consent is `NotApplicable` + /// regardless — kept as a real (if inert) guard type so callers don't + /// need `#[cfg]` at every call site. + pub struct LocalAppDataGuard { + _lock: MutexGuard<'static, ()>, + #[cfg(target_os = "windows")] + previous: Option, + #[cfg(target_os = "windows")] + previous_owner: Option, + #[cfg(target_os = "windows")] + previous_override: Option, + } + + impl LocalAppDataGuard { + #[cfg(target_os = "windows")] + pub fn set(path: &Path) -> Self { + let lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let previous = std::env::var_os(LOCAL_APPDATA_OVERRIDE_ENV); + let previous_owner = std::env::var_os(LOCAL_APPDATA_OVERRIDE_OWNER_ENV); + let previous_override = LOCAL_APP_DATA_OVERRIDE + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone(); + std::env::set_var(LOCAL_APPDATA_OVERRIDE_ENV, path); + std::env::set_var( + LOCAL_APPDATA_OVERRIDE_OWNER_ENV, + std::process::id().to_string(), + ); + *LOCAL_APP_DATA_OVERRIDE + .lock() + .unwrap_or_else(|e| e.into_inner()) = Some(path.to_path_buf()); + Self { + _lock: lock, + previous, + previous_owner, + previous_override, + } + } + + #[cfg(not(target_os = "windows"))] + pub fn set(_path: &Path) -> Self { + let lock = ENV_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + Self { _lock: lock } + } + } + + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] + pub(crate) fn current_local_app_data_override() -> Option { + LOCAL_APP_DATA_OVERRIDE + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone() + } + + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] + pub(crate) fn inherited_local_app_data_override() -> Option { + let path = std::env::var_os(LOCAL_APPDATA_OVERRIDE_ENV) + .filter(|value| !value.is_empty()) + .map(PathBuf::from)?; + let owner_pid = std::env::var(LOCAL_APPDATA_OVERRIDE_OWNER_ENV) + .ok() + .and_then(|value| value.parse::().ok()); + if owner_pid == Some(std::process::id()) { + return None; + } + Some(path) + } + + #[cfg(target_os = "windows")] + impl Drop for LocalAppDataGuard { + fn drop(&mut self) { + *LOCAL_APP_DATA_OVERRIDE + .lock() + .unwrap_or_else(|e| e.into_inner()) = self.previous_override.clone(); + match &self.previous { + Some(v) => std::env::set_var(LOCAL_APPDATA_OVERRIDE_ENV, v), + None => std::env::remove_var(LOCAL_APPDATA_OVERRIDE_ENV), + } + match &self.previous_owner { + Some(v) => std::env::set_var(LOCAL_APPDATA_OVERRIDE_OWNER_ENV, v), + None => std::env::remove_var(LOCAL_APPDATA_OVERRIDE_OWNER_ENV), + } + } + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::test_support::LocalAppDataGuard; + use super::*; + + fn block_on(future: F) -> F::Output { + fn raw_waker() -> std::task::RawWaker { + fn clone(_: *const ()) -> std::task::RawWaker { + raw_waker() + } + fn wake(_: *const ()) {} + fn wake_by_ref(_: *const ()) {} + fn drop(_: *const ()) {} + + std::task::RawWaker::new( + std::ptr::null(), + &std::task::RawWakerVTable::new(clone, wake, wake_by_ref, drop), + ) + } + + let waker = unsafe { std::task::Waker::from_raw(raw_waker()) }; + let mut future = std::pin::pin!(future); + let mut context = std::task::Context::from_waker(&waker); + + loop { + match future.as_mut().poll(&mut context) { + std::task::Poll::Ready(result) => return result, + std::task::Poll::Pending => std::thread::sleep(std::time::Duration::from_millis(1)), + } + } + } + + #[test] + fn consent_state_as_str_is_stable() { + assert_eq!(ConsentState::Granted.as_str(), "granted"); + assert_eq!(ConsentState::Denied.as_str(), "denied"); + assert_eq!(ConsentState::Undetermined.as_str(), "undetermined"); + assert_eq!(ConsentState::NotApplicable.as_str(), "not-applicable"); + } + + #[test] + fn only_granted_allows_collection() { + assert!(ConsentState::Granted.allows_collection()); + assert!(!ConsentState::Denied.allows_collection()); + assert!(!ConsentState::Undetermined.allows_collection()); + assert!(!ConsentState::NotApplicable.allows_collection()); + } + + #[test] + fn only_undetermined_needs_prompt() { + assert!(ConsentState::Undetermined.needs_prompt()); + assert!(!ConsentState::Granted.needs_prompt()); + assert!(!ConsentState::Denied.needs_prompt()); + // NotApplicable means "not Windows", where MXC collects nothing and + // therefore must never ask. Prompting here would be a privacy defect, + // not merely a redundant dialog. + assert!(!ConsentState::NotApplicable.needs_prompt()); + } + + #[cfg(target_os = "windows")] + #[test] + fn now_epoch_seconds_is_plausible() { + // Sanity bound: some time after this test was written, and not an + // absurd far-future value from an overflow/unit bug. + let secs = now_epoch_seconds(); + assert!(secs > 1_700_000_000, "epoch seconds too small: {secs}"); + assert!(secs < 4_000_000_000, "epoch seconds too large: {secs}"); + } + + #[cfg(not(target_os = "windows"))] + #[test] + fn non_windows_is_always_not_applicable_and_write_fails() { + let _guard = LocalAppDataGuard::set(std::path::Path::new("unused")); + assert_eq!(get_consent(), ConsentState::NotApplicable); + assert_eq!( + request_consent(None, |_| panic!("non-Windows consent must not present")) + .unwrap() + .result, + ConsentActionResult::NotApplicable + ); + assert_eq!( + block_on(request_consent_async(None, |_| async { + panic!("non-Windows async consent must not present") + })) + .unwrap() + .result, + ConsentActionResult::NotApplicable + ); + assert_eq!( + withdraw_consent().unwrap().result, + ConsentActionResult::NotApplicable + ); + assert!(set_consent(true, "cli").is_err()); + assert!(set_consent(false, "cli").is_err()); + assert!( + !needs_consent_prompt(), + "must never ask for consent where nothing is collected" + ); + } + + #[cfg(target_os = "windows")] + mod windows_tests { + use super::{block_on, LocalAppDataGuard}; + use crate::telemetry::consent::{ + get_consent, get_status, needs_consent_prompt, platform, request_consent, + request_consent_async, set_consent, withdraw_consent, ConsentActionError, + ConsentActionResult, ConsentDecision, ConsentState, ConsentStatusReason, + }; + + fn age_withdrawal_marker(store_root: &std::path::Path) { + let marker = store_root + .join("mxc") + .join("telemetry-consent.withdrawal-pending"); + let file = std::fs::OpenOptions::new() + .write(true) + .open(&marker) + .expect("open withdrawal marker"); + let stale_time = std::time::SystemTime::now() - std::time::Duration::from_secs(10); + file.set_times(std::fs::FileTimes::new().set_modified(stale_time)) + .expect("age withdrawal marker"); + } + + #[test] + fn needs_consent_prompt_tracks_the_store() { + let tmp = tempfile::tempdir().unwrap(); + // Also isolates the policy key: `needs_consent_prompt` consults it, + // so without this the test reads the real machine policy and fails + // on an administratively managed device. + let _env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + assert!(needs_consent_prompt(), "fresh store must prompt"); + set_consent(false, "prompt").unwrap(); + assert!( + !needs_consent_prompt(), + "a recorded denial must not re-prompt" + ); + set_consent(true, "settings-toggle").unwrap(); + assert!( + !needs_consent_prompt(), + "a recorded grant must not re-prompt" + ); + } + + #[test] + fn needs_consent_prompt_is_suppressed_by_blocking_policy() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(0); + assert!( + !needs_consent_prompt(), + "a blocking policy must suppress a meaningless prompt" + ); + } + + #[test] + fn fresh_store_is_undetermined() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + assert_eq!(get_consent(), ConsentState::Undetermined); + } + + #[test] + fn grant_then_read_round_trips() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + set_consent(true, "cli").expect("grant should succeed"); + assert_eq!(get_consent(), ConsentState::Granted); + } + + #[test] + fn deny_then_read_round_trips() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + set_consent(false, "cli").expect("deny should succeed"); + assert_eq!(get_consent(), ConsentState::Denied); + } + + #[test] + fn consent_can_be_flipped_repeatedly() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + set_consent(true, "prompt").unwrap(); + assert_eq!(get_consent(), ConsentState::Granted); + set_consent(false, "settings-toggle").unwrap(); + assert_eq!(get_consent(), ConsentState::Denied); + set_consent(true, "settings-toggle").unwrap(); + assert_eq!(get_consent(), ConsentState::Granted); + } + + #[test] + fn presenter_yes_persists_a_current_version_grant() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + + let outcome = request_consent(Some("en-US"), |prompt| { + assert_eq!(prompt.resource_version, 1); + assert_eq!(prompt.locale, "en-US"); + Ok(ConsentDecision::Yes) + }) + .expect("consent request"); + + assert_eq!(outcome.result, ConsentActionResult::Granted); + assert_eq!(outcome.status.stored_state, ConsentState::Granted); + assert_eq!(outcome.status.effective_state, ConsentState::Granted); + assert_eq!(outcome.status.reason, None); + } + + #[test] + fn async_presenter_yes_persists_a_current_version_grant() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + + let outcome = block_on(request_consent_async(Some("en-US"), |prompt| { + assert_eq!(prompt.resource_version, 1); + assert_eq!(prompt.locale, "en-US"); + async { Ok(ConsentDecision::Yes) } + })) + .expect("async consent request"); + + assert_eq!(outcome.result, ConsentActionResult::Granted); + assert_eq!(outcome.status.stored_state, ConsentState::Granted); + assert_eq!(outcome.status.effective_state, ConsentState::Granted); + assert_eq!(outcome.status.reason, None); + } + + #[test] + fn async_presenter_no_persists_a_denial() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + + let outcome = block_on(request_consent_async(None, |_| async { + Ok(ConsentDecision::No) + })) + .expect("async denial"); + + assert_eq!(outcome.result, ConsentActionResult::Denied); + assert_eq!(get_consent(), ConsentState::Denied); + } + + #[test] + fn async_dismissal_preserves_the_prior_state() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(false, "prompt").unwrap(); + + let outcome = block_on(request_consent_async(None, |_| async { + Ok(ConsentDecision::Dismissed) + })) + .expect("async dismissal"); + + assert_eq!(outcome.result, ConsentActionResult::Dismissed); + assert_eq!(get_consent(), ConsentState::Denied); + } + + #[test] + fn async_presenter_error_preserves_the_prior_state() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(false, "prompt").unwrap(); + + let error = block_on(request_consent_async(None, |_| async { + Err("host UI failed".to_string()) + })) + .expect_err("async presenter failure"); + + assert_eq!( + error, + ConsentActionError::Presenter("host UI failed".to_string()) + ); + assert_eq!(get_consent(), ConsentState::Denied); + } + + #[test] + fn async_policy_block_skips_the_presenter() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(0); + + let outcome = block_on(request_consent_async(None, |_| async { + panic!("blocked policy must not present") + })) + .expect("blocked result"); + + assert_eq!(outcome.result, ConsentActionResult::PolicyBlocked); + } + + #[test] + fn policy_change_while_presenter_is_open_blocks_stale_grant() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + + let outcome = request_consent(Some("en-US"), |_| { + env.set_policy_value(0); + Ok(ConsentDecision::Yes) + }) + .expect("policy change should produce a typed outcome"); + + assert_eq!(outcome.result, ConsentActionResult::PolicyBlocked); + assert_eq!(outcome.status.effective_state, ConsentState::Undetermined); + assert_eq!(get_consent(), ConsentState::Undetermined); + } + + #[test] + fn consent_change_while_presenter_is_open_rejects_stale_decision() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + + let error = request_consent(Some("en-US"), |_| { + set_consent(false, "concurrent-withdrawal").unwrap(); + Ok(ConsentDecision::Yes) + }) + .expect_err("stale presenter decision must not overwrite a newer denial"); + + assert_eq!( + error, + ConsentActionError::Persist( + "consent state changed while the presenter was open; no decision was written" + .to_string() + ) + ); + assert_eq!(get_consent(), ConsentState::Denied); + } + + #[test] + fn explicit_denial_withdraws_a_concurrent_grant() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + + let outcome = request_consent(Some("en-US"), |_| { + set_consent(true, "concurrent-grant").unwrap(); + Ok(ConsentDecision::No) + }) + .expect("explicit denial should be persisted"); + + assert_eq!(outcome.result, ConsentActionResult::Denied); + assert_eq!(get_consent(), ConsentState::Denied); + } + + #[test] + fn explicit_request_can_replace_a_prior_denial() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(false, "prompt").unwrap(); + + let outcome = request_consent(None, |_| Ok(ConsentDecision::Yes)).unwrap(); + + assert_eq!(outcome.result, ConsentActionResult::Granted); + assert_eq!(get_consent(), ConsentState::Granted); + } + + #[test] + fn dismissed_or_failed_presenter_does_not_rewrite_prior_state() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(false, "prompt").unwrap(); + + let dismissed = + request_consent(None, |_| Ok(ConsentDecision::Dismissed)).expect("dismissal"); + assert_eq!(dismissed.result, ConsentActionResult::Dismissed); + assert_eq!(get_consent(), ConsentState::Denied); + + let error = request_consent(None, |_| Err("host UI failed".to_string())) + .expect_err("presenter failure"); + assert_eq!( + error, + ConsentActionError::Presenter("host UI failed".to_string()) + ); + assert_eq!(get_consent(), ConsentState::Denied); + } + + #[test] + fn policy_block_and_current_grant_skip_the_presenter() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(0); + let blocked = request_consent(None, |_| panic!("blocked policy must not present")) + .expect("blocked result"); + assert_eq!(blocked.result, ConsentActionResult::PolicyBlocked); + + env.set_policy_value(3); + set_consent(true, "prompt").unwrap(); + let granted = request_consent(None, |_| panic!("current grant must not re-present")) + .expect("already granted result"); + assert_eq!(granted.result, ConsentActionResult::AlreadyGranted); + } + + #[test] + fn withdrawal_is_idempotent_and_ignores_blocking_policy() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(true, "prompt").unwrap(); + env.set_policy_value(0); + + for _ in 0..2 { + let outcome = withdraw_consent().expect("withdrawal"); + assert_eq!(outcome.result, ConsentActionResult::Withdrawn); + assert_eq!(outcome.status.effective_state, ConsentState::Denied); + } + } + + #[test] + fn withdrawal_reports_the_current_policy_after_waiting_for_the_store_lock() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(true, "prompt").unwrap(); + + let outcome = std::thread::scope(|scope| { + let handle = platform::with_store_lock(|| { + let handle = scope.spawn(|| withdraw_consent().expect("withdrawal")); + env.set_policy_value(0); + Ok(handle) + }) + .expect("store lock"); + handle.join().unwrap() + }); + + assert_eq!(outcome.result, ConsentActionResult::Withdrawn); + assert_eq!(outcome.status.effective_state, ConsentState::Denied); + assert_eq!(outcome.policy, crate::telemetry::PolicyState::Blocked); + } + + #[test] + fn consent_read_does_not_wait_for_the_store_lock() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + set_consent(true, "prompt").unwrap(); + + let (status_tx, status_rx) = std::sync::mpsc::channel(); + std::thread::scope(|scope| { + let read_handle = platform::with_store_lock(|| { + let status_tx = status_tx.clone(); + let handle = scope.spawn(move || { + status_tx.send(get_status()).expect("send consent status"); + }); + let status = status_rx + .recv_timeout(std::time::Duration::from_millis(100)) + .expect("consent read must not wait for the store lock"); + assert_eq!(status.effective_state, ConsentState::Granted); + Ok(handle) + }) + .expect("store lock"); + read_handle.join().unwrap(); + }); + } + + #[test] + fn corrupt_file_is_undetermined_not_granted() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("telemetry-consent.json"), "not json at all").unwrap(); + assert_eq!(get_consent(), ConsentState::Undetermined); + } + + #[test] + fn oversized_file_is_rejected_without_unbounded_read() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("telemetry-consent.json"), + vec![b'x'; 16 * 1024 + 1], + ) + .unwrap(); + let status = get_status(); + assert_eq!(status.effective_state, ConsentState::Undetermined); + assert_eq!(status.reason, Some(ConsentStatusReason::StoreUnreadable)); + } + + #[test] + fn pending_withdrawal_marker_fails_closed() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + platform::begin_withdrawal().unwrap(); + + let status = get_status(); + assert_eq!(status.effective_state, ConsentState::Undetermined); + assert_eq!(status.reason, Some(ConsentStatusReason::StoreUnreadable)); + } + + #[test] + fn pending_withdrawal_marker_metadata_errors_fail_closed() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + set_consent(true, "prompt").unwrap(); + + let status = + platform::read_status_unlocked_with_marker_for_test(Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "marker temporarily unreadable", + ))); + + assert_eq!(status.stored_state, ConsentState::Undetermined); + assert_eq!(status.effective_state, ConsentState::Undetermined); + assert_eq!(status.reason, Some(ConsentStatusReason::StoreUnreadable)); + } + + #[test] + fn successful_grant_repairs_a_stale_withdrawal_marker() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + platform::begin_withdrawal().unwrap(); + + let outcome = request_consent(None, |_| Ok(ConsentDecision::Yes)).unwrap(); + + assert_eq!(outcome.result, ConsentActionResult::Granted); + assert_eq!(outcome.status.effective_state, ConsentState::Granted); + assert_eq!(get_consent(), ConsentState::Granted); + } + + #[test] + fn stale_withdrawal_marker_recovers_to_denied() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(true, "prompt").unwrap(); + platform::begin_withdrawal().unwrap(); + age_withdrawal_marker(tmp.path()); + + let status = get_status(); + + assert_eq!(status.stored_state, ConsentState::Denied); + assert_eq!(status.effective_state, ConsentState::Denied); + assert_eq!(status.reason, None); + assert_eq!(get_consent(), ConsentState::Denied); + let record = + std::fs::read_to_string(tmp.path().join("mxc").join("telemetry-consent.json")) + .unwrap(); + assert!(record.contains(r#""source": "withdrawal-recovery""#)); + assert!( + !tmp.path() + .join("mxc") + .join("telemetry-consent.withdrawal-pending") + .exists(), + "recovery must clear the stale withdrawal marker" + ); + } + + #[test] + fn stale_withdrawal_marker_on_denied_record_is_cleared() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(false, "cli").unwrap(); + platform::begin_withdrawal().unwrap(); + age_withdrawal_marker(tmp.path()); + + let status = get_status(); + + assert_eq!(status.stored_state, ConsentState::Denied); + assert_eq!(status.effective_state, ConsentState::Denied); + assert_eq!(status.reason, None); + assert!( + !tmp.path() + .join("mxc") + .join("telemetry-consent.withdrawal-pending") + .exists(), + "recovery must eventually clear a leftover withdrawal marker" + ); + } + + #[test] + fn stale_withdrawal_recovery_is_serialized_with_concurrent_writers() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(true, "prompt").unwrap(); + platform::begin_withdrawal().unwrap(); + age_withdrawal_marker(tmp.path()); + + std::thread::scope(|scope| { + let (reader, writer) = platform::with_store_lock(|| { + let (reader_started_tx, reader_started_rx) = std::sync::mpsc::channel(); + let (writer_started_tx, writer_started_rx) = std::sync::mpsc::channel(); + let reader = scope.spawn(move || { + reader_started_tx.send(()).unwrap(); + get_status() + }); + let writer = scope.spawn(move || { + writer_started_tx.send(()).unwrap(); + withdraw_consent().map(|_| ()) + }); + reader_started_rx.recv().unwrap(); + writer_started_rx.recv().unwrap(); + assert!( + !reader.is_finished() && !writer.is_finished(), + "recovery and writer must wait for the store lock" + ); + Ok((reader, writer)) + }) + .unwrap(); + + let reader_status = reader.join().unwrap(); + writer.join().unwrap().unwrap(); + assert_eq!(reader_status.effective_state, ConsentState::Denied); + assert_eq!(get_consent(), ConsentState::Denied); + assert!(!tmp + .path() + .join("mxc") + .join("telemetry-consent.withdrawal-pending") + .exists()); + }); + } + + #[test] + fn unknown_schema_version_is_undetermined() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("telemetry-consent.json"), + r#"{"schemaVersion":999,"consent":"granted"}"#, + ) + .unwrap(); + assert_eq!(get_consent(), ConsentState::Undetermined); + } + + #[test] + fn unsupported_grant_schema_reports_a_typed_reason() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("telemetry-consent.json"), + r#"{"schemaVersion":999,"consent":"granted","promptResourceVersion":1,"promptLocale":"en-US"}"#, + ) + .unwrap(); + + let status = get_status(); + + assert_eq!(status.stored_state, ConsentState::Granted); + assert_eq!(status.effective_state, ConsentState::Undetermined); + assert_eq!( + status.reason, + Some(ConsentStatusReason::ConsentSchemaUnsupported) + ); + } + + #[test] + fn unsupported_prompt_version_reports_a_typed_reason() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("telemetry-consent.json"), + r#"{"schemaVersion":2,"consent":"granted","promptResourceVersion":999,"promptLocale":"en-US"}"#, + ) + .unwrap(); + + let status = get_status(); + + assert_eq!(status.stored_state, ConsentState::Granted); + assert_eq!(status.effective_state, ConsentState::Undetermined); + assert_eq!( + status.reason, + Some(ConsentStatusReason::PromptVersionUnsupported) + ); + } + + #[test] + fn concurrent_grant_revoke_never_corrupts_the_store() { + // Regression test for the write-race finding: many threads + // hammering set_consent() against the same redirected store + // concurrently (the override env var is process-global, so + // every spawned thread inherits the same redirected path from + // the outer guard) must never leave a torn/missing file behind; + // the final state must be a fully-formed, parseable record. + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + + // Every writer must *succeed*, not merely avoid corrupting the + // file. Asserting only on the final file's parseability would + // still pass if 15 of 16 writers lost a temp-name or rename + // race, which is precisely the failure this fix targets. + let results: Vec> = std::thread::scope(|scope| { + let handles: Vec<_> = (0..16) + .map(|i| scope.spawn(move || set_consent(i % 2 == 0, "cli"))) + .collect(); + handles.into_iter().map(|h| h.join().unwrap()).collect() + }); + for (i, result) in results.iter().enumerate() { + assert!(result.is_ok(), "concurrent writer {i} failed: {result:?}"); + } + + // Whatever the last writer's outcome was, the store must be + // Granted or Denied — never Undetermined (which would mean a + // corrupt/missing file from a torn write). + let state = get_consent(); + assert!(matches!( + state, + ConsentState::Granted | ConsentState::Denied + )); + } + + /// Pre-versioned grants must be invalidated because they cannot prove + /// the canonical prompt was shown. A prior denial remains denied. + #[test] + fn legacy_records_preserve_provenance_but_require_reconsent_for_grants() { + for (stored, effective, reason) in [ + ( + "granted", + ConsentState::Undetermined, + Some(ConsentStatusReason::PromptVersionMissing), + ), + ("denied", ConsentState::Denied, None), + ] { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("telemetry-consent.json"), + format!( + r#"{{"schemaVersion":1,"consent":"{stored}","source":"prompt","promptedMxcVersion":"0.7.0","updatedAtUtc":"2026-01-02T03:04:05Z"}}"# + ), + ) + .unwrap(); + let status = get_status(); + assert_eq!( + status.stored_state, + if stored == "granted" { + ConsentState::Granted + } else { + ConsentState::Denied + } + ); + assert_eq!(status.effective_state, effective); + assert_eq!(status.reason, reason); + } + } + + /// `with_io_retry` must ride out transient lock contention but must + /// not sleep through a settled answer. The `NotFound` case is the + /// startup-latency one: it is the state of every machine that has + /// never recorded a decision, on the critical path of every launch. + #[test] + fn io_retry_retries_transient_errors_then_succeeds() { + use crate::telemetry::consent::platform; + let mut calls = 0; + let result = platform::with_io_retry_for_test(|| { + calls += 1; + if calls < 3 { + Err(platform::transient_io_error_for_test()) + } else { + Ok(calls) + } + }); + assert_eq!(result.unwrap(), 3); + assert_eq!(calls, 3); + } + + #[test] + fn io_retry_gives_up_after_the_attempt_budget() { + use crate::telemetry::consent::platform; + let mut calls = 0; + let result = platform::with_io_retry_for_test(|| { + calls += 1; + Err::<(), _>(platform::transient_io_error_for_test()) + }); + assert!(result.is_err()); + assert_eq!(calls, platform::IO_RETRY_ATTEMPTS_FOR_TEST); + } + + #[test] + fn io_retry_does_not_retry_not_found() { + use crate::telemetry::consent::platform; + let mut calls = 0; + let started = std::time::Instant::now(); + let result = platform::with_io_retry_for_test(|| { + calls += 1; + Err::<(), _>(std::io::Error::new( + std::io::ErrorKind::NotFound, + "no consent file", + )) + }); + assert!(result.is_err()); + assert_eq!(calls, 1, "NotFound must not be retried"); + assert!( + started.elapsed() < std::time::Duration::from_millis(20), + "NotFound must not sleep on the retry delay" + ); + } + + /// The whole point of the above: reading a fresh (nonexistent) store + /// is the common case and must be effectively instant. + #[test] + fn fresh_store_read_does_not_sleep() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let started = std::time::Instant::now(); + assert_eq!(get_consent(), ConsentState::Undetermined); + assert!( + started.elapsed() < std::time::Duration::from_millis(40), + "fresh-store read took {:?}; the retry loop is sleeping on NotFound again", + started.elapsed() + ); + } + + #[test] + fn missing_local_app_data_resolution_falls_back_to_real_known_folder() { + // With no debug override set, the real per-user known-folder + // path must still resolve (it always exists on a real Windows + // profile), so exercise that fallback without touching the real + // consent file itself. Locks ENV_LOCK directly (rather than via + // LocalAppDataGuard) since this test removes the override + // entirely instead of redirecting it. + let _lock = super::super::test_support::ENV_LOCK + .lock() + .unwrap_or_else(|e| e.into_inner()); + let previous = std::env::var_os("MXC_TEST_LOCALAPPDATA_OVERRIDE"); + let previous_owner = std::env::var_os("MXC_TEST_LOCALAPPDATA_OVERRIDE_OWNER_PID"); + std::env::remove_var("MXC_TEST_LOCALAPPDATA_OVERRIDE"); + std::env::remove_var("MXC_TEST_LOCALAPPDATA_OVERRIDE_OWNER_PID"); + let resolved = + crate::telemetry::consent::platform::known_folder_local_app_data_for_test(); + if let Some(v) = previous { + std::env::set_var("MXC_TEST_LOCALAPPDATA_OVERRIDE", v); + } + if let Some(v) = previous_owner { + std::env::set_var("MXC_TEST_LOCALAPPDATA_OVERRIDE_OWNER_PID", v); + } + assert!( + resolved.is_some(), + "known-folder API should resolve a real path" + ); + } + + #[test] + fn local_app_data_override_reaches_child_processes() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + + let status = std::process::Command::new(std::env::current_exe().unwrap()) + .arg("child_process_local_app_data_override_probe") + .arg("--nocapture") + .env("MXC_CHILD_OVERRIDE_PROBE", tmp.path()) + .status() + .expect("spawn child test probe"); + + assert!(status.success(), "child probe failed: {status}"); + } + + #[test] + fn child_process_local_app_data_override_probe() { + let Some(expected) = std::env::var_os("MXC_CHILD_OVERRIDE_PROBE") else { + return; + }; + + assert_eq!( + platform::local_app_data_dir_for_test(), + Some(std::path::PathBuf::from(expected)) + ); + assert_eq!(get_status().reason, Some(ConsentStatusReason::NoRecord)); + } + + #[test] + fn known_folder_resolution_only_caches_successful_results() { + let cache = std::sync::OnceLock::new(); + let mut calls = 0; + + assert_eq!( + platform::cache_successful_resolution_for_test(&cache, || { + calls += 1; + None + }), + None + ); + assert_eq!( + platform::cache_successful_resolution_for_test(&cache, || { + calls += 1; + Some(std::path::PathBuf::from(r"C:\Users\test\AppData\Local")) + }), + Some(std::path::PathBuf::from(r"C:\Users\test\AppData\Local")) + ); + assert_eq!( + platform::cache_successful_resolution_for_test(&cache, || { + calls += 1; + Some(std::path::PathBuf::from(r"C:\Users\test\AppData\Roaming")) + }), + Some(std::path::PathBuf::from(r"C:\Users\test\AppData\Local")) + ); + assert_eq!(calls, 2, "a failed lookup must not be cached forever"); + } + } +} diff --git a/src/core/wxc_common/src/telemetry/consent_prompt.rs b/src/core/wxc_common/src/telemetry/consent_prompt.rs new file mode 100644 index 000000000..84597e5a0 --- /dev/null +++ b/src/core/wxc_common/src/telemetry/consent_prompt.rs @@ -0,0 +1,118 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Canonical, versioned telemetry consent resources. +//! +//! The versioned JSON resource is embedded at build time. Presenters must +//! render every field verbatim. + +/// One independently localizable consent message. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ConsentMessage { + /// Stable localization identifier. + pub id: &'static str, + /// Complete localized text. + pub text: &'static str, +} + +/// Canonical telemetry consent resource. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ConsentPrompt { + /// Version stored with a grant. + pub resource_version: u32, + /// BCP 47 locale. + pub locale: &'static str, + /// Title. + pub title: ConsentMessage, + /// Disclosure body. + pub body: ConsentMessage, + /// Affirmative action. + pub affirmative_label: ConsentMessage, + /// Negative action. + pub negative_label: ConsentMessage, + /// Learn-more label. + pub learn_more_label: ConsentMessage, + /// Learn-more URL. + pub learn_more_url: &'static str, +} + +include!(concat!(env!("OUT_DIR"), "/telemetry_consent_resource.rs")); + +/// Resolve the resource for a locale, falling back to `en-US`. +pub fn prompt_for_locale(locale: Option<&str>) -> &'static ConsentPrompt { + locale + .and_then(find_prompt) + .unwrap_or(&EN_US_CONSENT_PROMPT) +} + +fn find_prompt(locale: &str) -> Option<&'static ConsentPrompt> { + is_english_locale(locale).then_some(&EN_US_CONSENT_PROMPT) +} + +fn is_english_locale(locale: &str) -> bool { + locale + .trim() + .split(['-', '_']) + .next() + .is_some_and(|language| language.eq_ignore_ascii_case("en")) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn canonical_resource_is_complete_and_versioned() { + let prompt = prompt_for_locale(Some("en-US")); + assert_eq!(prompt.resource_version, CONSENT_RESOURCE_VERSION); + assert_eq!(prompt.locale, "en-US"); + assert_eq!( + prompt.title.text, + "Help improve Microsoft eXecution Container (MXC)" + ); + assert_eq!( + prompt.body.text, + "Help improve MXC by sharing optional diagnostic data with Microsoft.\n\ + If enabled, MXC sends diagnostic information about product usage, performance, \ + and reliability. MXC does not send your commands, file paths, credentials, or \ + other customer content.\n\ + You can change your choice at any time." + ); + assert_eq!(prompt.affirmative_label.text, "Yes"); + assert_eq!(prompt.negative_label.text, "No"); + assert_eq!(prompt.learn_more_label.text, "Privacy Statement"); + assert_eq!( + prompt.learn_more_url, + "https://go.microsoft.com/fwlink/?linkid=521839" + ); + } + + #[test] + fn locale_lookup_normalizes_english_and_falls_back_to_en_us() { + for locale in [ + None, + Some("en"), + Some("EN_us"), + Some("en-GB"), + Some("fr-FR"), + ] { + assert_eq!(prompt_for_locale(locale), &EN_US_CONSENT_PROMPT); + } + } + + #[test] + fn message_ids_are_stable_and_unique() { + let prompt = &EN_US_CONSENT_PROMPT; + let ids = [ + prompt.title.id, + prompt.body.id, + prompt.affirmative_label.id, + prompt.negative_label.id, + prompt.learn_more_label.id, + ]; + for (index, id) in ids.iter().enumerate() { + assert!(!id.is_empty()); + assert!(!ids[..index].contains(id), "duplicate message id: {id}"); + } + } +} diff --git a/src/core/wxc_common/src/telemetry/events.rs b/src/core/wxc_common/src/telemetry/events.rs index c4c011bbe..a9b9398a2 100644 --- a/src/core/wxc_common/src/telemetry/events.rs +++ b/src/core/wxc_common/src/telemetry/events.rs @@ -54,6 +54,7 @@ impl std::fmt::Display for FailureReason { #[derive(Debug, Clone, Copy)] pub struct TelemetryContext<'a> { pub backend: &'a str, + pub sandbox_kind: &'a str, /// State-aware lifecycle phase — one of `provision|start|exec|stop| /// deprovision`, or `""` for one-shot (non-state-aware) executions. pub phase: &'a str, @@ -66,6 +67,7 @@ pub struct TelemetryContext<'a> { /// Data for an MXC.Execution ETW event. pub struct ExecutionEvent<'a> { pub backend: &'a str, + pub sandbox_kind: &'a str, pub exit_code: i32, pub outcome: &'a str, pub duration_ms: u64, @@ -91,6 +93,7 @@ pub fn log_execution(event: &ExecutionEvent<'_>) { mxc_telemetry::log_execution( event.backend, + event.sandbox_kind, event.exit_code, event.outcome, event.duration_ms, @@ -113,6 +116,7 @@ pub fn log_execution(event: &ExecutionEvent<'_>) { pub fn log_error(ctx: TelemetryContext<'_>, error_type: FailureReason, exit_code: i32) { mxc_telemetry::log_error( ctx.backend, + ctx.sandbox_kind, error_type.as_str(), exit_code, ctx.phase, @@ -138,6 +142,7 @@ pub(super) mod test_sink { #[derive(Debug, Clone, PartialEq, Eq)] pub struct CapturedExecution { pub backend: String, + pub sandbox_kind: String, pub exit_code: i32, pub outcome: String, pub duration_ms: u64, @@ -150,6 +155,7 @@ pub(super) mod test_sink { #[derive(Debug, Clone, PartialEq, Eq)] pub struct CapturedError { pub backend: String, + pub sandbox_kind: String, pub error_type: FailureReason, pub exit_code: i32, pub phase: String, @@ -201,6 +207,7 @@ pub(super) mod test_sink { .unwrap_or_else(|e| e.into_inner()) .push(CapturedExecution { backend: event.backend.to_owned(), + sandbox_kind: event.sandbox_kind.to_owned(), exit_code: event.exit_code, outcome: event.outcome.to_owned(), duration_ms: event.duration_ms, @@ -223,6 +230,7 @@ pub(super) mod test_sink { .unwrap_or_else(|e| e.into_inner()) .push(CapturedError { backend: ctx.backend.to_owned(), + sandbox_kind: ctx.sandbox_kind.to_owned(), error_type, exit_code, phase: ctx.phase.to_owned(), diff --git a/src/core/wxc_common/src/telemetry/mod.rs b/src/core/wxc_common/src/telemetry/mod.rs index 15a85d4eb..d777ed177 100644 --- a/src/core/wxc_common/src/telemetry/mod.rs +++ b/src/core/wxc_common/src/telemetry/mod.rs @@ -11,12 +11,15 @@ //! //! On non-Windows platforms, all telemetry functions are no-ops. +pub mod consent; +pub mod consent_prompt; pub mod correlation_vector; pub mod events; +pub mod policy; use std::time::Duration; -use std::sync::atomic::{AtomicBool, Ordering}; +use std::sync::atomic::Ordering; use std::sync::Mutex; use crate::logger::Logger; @@ -24,7 +27,36 @@ use crate::models::{ContainmentBackend, FailurePhase, ScriptResponse, TelemetryC use crate::mxc_error::{MxcError, MxcErrorCode}; use crate::state_aware_dispatch::DispatchOutcome; +pub use consent::ConsentState; pub use events::{log_error, log_execution, ExecutionEvent, FailureReason, TelemetryContext}; +pub use policy::PolicyState; + +#[cfg(any(test, all(feature = "test-support", debug_assertions)))] +pub mod test_support { + use super::consent::test_support::LocalAppDataGuard; + use super::policy::test_support::PolicyKeyGuard; + + /// Lock-order-safe redirect guard for tests that need both the consent + /// store and the policy key redirected away from real user/machine state. + pub struct TelemetryTestEnv { + _consent: LocalAppDataGuard, + policy: PolicyKeyGuard, + } + + impl TelemetryTestEnv { + /// Redirect both telemetry globals for the lifetime of the guard. + pub fn new(store: &std::path::Path) -> Self { + let policy = PolicyKeyGuard::new(); + let _consent = LocalAppDataGuard::set(store); + Self { _consent, policy } + } + + #[cfg_attr(not(target_os = "windows"), allow(dead_code))] + pub fn set_policy_value(&self, value: u32) { + self.policy.set_value(value); + } + } +} /// Conventional process exit code for a Rust panic/abort. Used as the reported /// `exit_code` on crash telemetry, since the panicking process has not (and @@ -76,7 +108,7 @@ static PROCESS_CORRELATION_VECTOR: Mutex> = Mutex::new(None); /// handler) can race the main thread's normal completion emit; this guard makes /// emission exactly-once so a single dispatch never yields duplicate /// `MXC.Execution` records. -static HAS_EMITTED: AtomicBool = AtomicBool::new(false); +static HAS_EMITTED: std::sync::atomic::AtomicBool = std::sync::atomic::AtomicBool::new(false); #[cfg(test)] thread_local! { @@ -141,13 +173,16 @@ pub fn version() -> &'static str { /// Resolve whether telemetry is enabled for this invocation. /// /// Resolution: -/// - `experimental.telemetry.enabled` in JSON config — explicit override. -/// - Default: off (telemetry requires explicit opt-in). +/// - `telemetry.enabled` in JSON config — explicit kill-switch/opt-in. +/// - Persisted user consent state — must be `Granted`. +/// - Administrative policy ceiling — must allow collection. /// -/// Note: Consent is the SDK consumer's responsibility. MXC does not implement -/// consent prompts or persistent consent storage. +/// All three terms must hold. The function fails closed if either consent or +/// policy cannot be read: those helpers resolve to non-collecting states. pub fn is_enabled(config: &TelemetryConfig) -> bool { config.enabled.unwrap_or(false) + && consent::get_consent().allows_collection() + && policy::get_policy().allows_collection() } /// Initialize the TraceLogging ETW provider. @@ -223,6 +258,7 @@ pub fn emit_completion( log_execution(&ExecutionEvent { backend, + sandbox_kind: backend, exit_code: response.exit_code, outcome, duration_ms: elapsed.as_millis() as u64, @@ -241,6 +277,7 @@ pub fn emit_completion( log_error( TelemetryContext { backend, + sandbox_kind: backend, phase: "", correlation_vector: "", }, @@ -274,6 +311,7 @@ pub fn emit_early_exit(active: bool, containment: &ContainmentBackend, reason: F log_execution(&ExecutionEvent { backend, + sandbox_kind: backend, exit_code: 1, outcome: "failure", duration_ms: 0, @@ -287,6 +325,7 @@ pub fn emit_early_exit(active: bool, containment: &ContainmentBackend, reason: F log_error( TelemetryContext { backend, + sandbox_kind: backend, phase: "", correlation_vector: "", }, @@ -420,6 +459,7 @@ fn crash_event<'a>( ) -> ExecutionEvent<'a> { ExecutionEvent { backend: ctx.backend, + sandbox_kind: ctx.sandbox_kind, exit_code, outcome: "failure", duration_ms: 0, @@ -484,6 +524,7 @@ pub fn emit_panic() { emit_crash( TelemetryContext { backend: process_backend(), + sandbox_kind: process_backend(), phase: process_phase(), correlation_vector: &correlation_vector, }, @@ -513,6 +554,7 @@ pub fn emit_cancellation() { emit_crash( TelemetryContext { backend: process_backend(), + sandbox_kind: process_backend(), phase: process_phase(), correlation_vector: &correlation_vector, }, @@ -560,6 +602,7 @@ fn plan_state_aware<'a>( Ok(DispatchOutcome::Envelope(_)) => StateAwareEvents { execution: ExecutionEvent { backend: ctx.backend, + sandbox_kind: ctx.sandbox_kind, exit_code: 0, outcome: "success", duration_ms, @@ -574,6 +617,7 @@ fn plan_state_aware<'a>( StateAwareEvents { execution: ExecutionEvent { backend: ctx.backend, + sandbox_kind: ctx.sandbox_kind, exit_code: *exit_code, outcome: if failed { "failure" } else { "success" }, duration_ms, @@ -593,6 +637,7 @@ fn plan_state_aware<'a>( StateAwareEvents { execution: ExecutionEvent { backend: ctx.backend, + sandbox_kind: ctx.sandbox_kind, exit_code: 1, outcome: "failure", duration_ms, @@ -660,14 +705,6 @@ mod tests { /// Mirrors the `TEST_LOCK` pattern in `mxc_telemetry`. static TEST_LOCK: Mutex<()> = Mutex::new(()); - #[test] - fn is_enabled_explicit_true() { - let config = TelemetryConfig { - enabled: Some(true), - }; - assert!(is_enabled(&config)); - } - #[test] fn is_enabled_explicit_false() { let config = TelemetryConfig { @@ -682,6 +719,42 @@ mod tests { assert!(!is_enabled(&config)); } + #[cfg(target_os = "windows")] + #[test] + fn is_enabled_explicit_true_requires_granted_consent_and_permitting_policy() { + let store = tempfile::tempdir().expect("temp dir"); + let env = test_support::TelemetryTestEnv::new(store.path()); + let config = TelemetryConfig { + enabled: Some(true), + }; + + assert!( + !is_enabled(&config), + "undetermined consent must fail closed" + ); + + consent::set_consent(true, "test").expect("set consent"); + assert!( + is_enabled(&config), + "granted + allowed policy should enable" + ); + + env.set_policy_value(0); + assert!( + !is_enabled(&config), + "blocked policy must suppress telemetry even with granted consent" + ); + } + + #[cfg(not(target_os = "windows"))] + #[test] + fn is_enabled_explicit_true_is_false_off_windows() { + let config = TelemetryConfig { + enabled: Some(true), + }; + assert!(!is_enabled(&config)); + } + #[test] fn version_is_not_empty() { assert!(!version().is_empty()); @@ -737,12 +810,14 @@ mod tests { assert_eq!(exec.outcome, "failure"); assert_eq!(exec.failure_reason, Some(FailureReason::InternalError)); assert_eq!(exec.phase, "exec"); + assert_eq!(exec.sandbox_kind, "isolation_session"); assert_eq!(exec.correlation_vector, "iso:wxc-abcd"); let errors = events::test_sink::take_errors(); assert_eq!(errors.len(), 1, "panic emits exactly one MXC.Error"); let error = &errors[0]; assert_eq!(error.backend, "isolation_session"); + assert_eq!(error.sandbox_kind, "isolation_session"); assert_eq!(error.error_type, FailureReason::InternalError); assert_eq!(error.exit_code, PANIC_EXIT_CODE); assert_eq!(error.phase, "exec"); @@ -772,10 +847,12 @@ mod tests { assert_eq!(exec.outcome, "failure"); assert_eq!(exec.failure_reason, Some(FailureReason::Cancelled)); assert_eq!(exec.phase, "start"); + assert_eq!(exec.sandbox_kind, "isolation_session"); assert_eq!(exec.correlation_vector, "iso:wxc-abcd"); let errors = events::test_sink::take_errors(); assert_eq!(errors.len(), 1); + assert_eq!(errors[0].sandbox_kind, "isolation_session"); assert_eq!(errors[0].error_type, FailureReason::Cancelled); assert_eq!(errors[0].exit_code, CANCELLED_EXIT_CODE); @@ -868,6 +945,7 @@ mod tests { false, TelemetryContext { backend: "isolation_session", + sandbox_kind: "isolation_session", phase: "exec", correlation_vector: "iso:wxc-abcd", }, @@ -934,6 +1012,7 @@ mod tests { let event = crash_event( TelemetryContext { backend: "lxc", + sandbox_kind: "lxc", phase: "exec", correlation_vector: "iso:wxc-abcd", }, @@ -950,6 +1029,7 @@ mod tests { let cancel = crash_event( TelemetryContext { backend: "appcontainer", + sandbox_kind: "appcontainer", phase: "", correlation_vector: "", }, @@ -970,6 +1050,7 @@ mod tests { let panic = plan_crash( TelemetryContext { backend: "lxc", + sandbox_kind: "lxc", phase: "exec", correlation_vector: "iso:wxc-abcd", }, @@ -992,6 +1073,7 @@ mod tests { let cancel = plan_crash( TelemetryContext { backend: "isolation_session", + sandbox_kind: "isolation_session", phase: "", correlation_vector: "", }, @@ -1028,6 +1110,7 @@ mod tests { for phase in PHASES { let ctx = TelemetryContext { backend: "isolation_session", + sandbox_kind: "isolation_session", phase, correlation_vector: correlation, }; @@ -1139,6 +1222,7 @@ mod tests { true, TelemetryContext { backend: "isolation_session", + sandbox_kind: "isolation_session", phase: "provision", correlation_vector: "corr-provision", }, @@ -1148,6 +1232,7 @@ mod tests { let execs = events::test_sink::take_executions(); assert_eq!(execs.len(), 1); assert_eq!(execs[0].phase, "provision"); + assert_eq!(execs[0].sandbox_kind, "isolation_session"); assert_eq!(execs[0].correlation_vector, "corr-provision"); assert_eq!(execs[0].outcome, "success"); assert!(events::test_sink::take_errors().is_empty()); @@ -1160,6 +1245,7 @@ mod tests { true, TelemetryContext { backend: "isolation_session", + sandbox_kind: "isolation_session", phase: "start", correlation_vector: "corr-start", }, @@ -1169,12 +1255,14 @@ mod tests { let execs = events::test_sink::take_executions(); assert_eq!(execs.len(), 1); assert_eq!(execs[0].phase, "start"); + assert_eq!(execs[0].sandbox_kind, "isolation_session"); assert_eq!(execs[0].correlation_vector, "corr-start"); assert_eq!(execs[0].outcome, "failure"); let errors = events::test_sink::take_errors(); assert_eq!(errors.len(), 1); assert_eq!(errors[0].error_type, FailureReason::PolicyError); assert_eq!(errors[0].phase, "start"); + assert_eq!(errors[0].sandbox_kind, "isolation_session"); assert_eq!(errors[0].correlation_vector, "corr-start"); reset_for_test(); diff --git a/src/core/wxc_common/src/telemetry/policy.rs b/src/core/wxc_common/src/telemetry/policy.rs new file mode 100644 index 000000000..2433b9c74 --- /dev/null +++ b/src/core/wxc_common/src/telemetry/policy.rs @@ -0,0 +1,577 @@ +// Copyright (c) Microsoft Corporation. +// Licensed under the MIT License. + +//! Administrative (MDM / Group Policy) telemetry policy. +//! +//! See `docs/telemetry/telemetry-administrative-policy.md` for the admin-facing reference and +//! `docs/telemetry/telemetry-consent-design.md` for how this composes with +//! user consent. In short: +//! +//! - An administrator (via Intune, another MDM, or Group Policy) may **deny** +//! MXC telemetry machine-wide by setting +//! `HKLM\SOFTWARE\Policies\Mxc\AllowTelemetry` (`REG_DWORD`). +//! - The policy is a **ceiling, never a grant**. An administrator who permits +//! telemetry does not thereby consent on the user's behalf: MXC still +//! requires an explicit, persisted [`super::consent::ConsentState::Granted`] +//! from the user. There is no policy value that can turn telemetry on. +//! - MXC deliberately does **not** read the Windows-wide `AllowTelemetry` +//! setting. Microsoft's Policy CSP documentation scopes that policy to "the +//! operating system and apps that are considered part of Windows" and states +//! it "doesn't apply to any additional apps installed by your organization", +//! and the Windows Business Division privacy guidance requires that +//! app-classified components "build their own notice and consent experience +//! ... and should not rely on the Windows diagnostic consent". Reading the +//! OS setting would also mean reading Windows *consent* state, which MXC is +//! expressly forbidden from doing — the supported OS APIs for this +//! (`TelIsTelemetryTypeAllowed` and friends) fold the user's Settings-app +//! choice into their answer. +//! - Windows-only, like every other telemetry surface. On other platforms MXC +//! collects nothing at all, so there is nothing for a policy to restrict and +//! this module compiles down to a stub. +//! - Fails closed: an unreadable or unrecognized policy value denies. + +/// The administrator's telemetry decision for this machine. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum PolicyState { + /// No administrative policy is configured. Telemetry is governed solely by + /// the user's own consent decision. This is *not* a grant. + Unrestricted, + /// An administrator has permitted optional (usage) telemetry, the category + /// MXC emits. Still requires user consent before anything is collected. + Allowed, + /// An administrator has denied MXC telemetry, or the configured value could + /// not be understood. Nothing is collected regardless of user consent, and + /// hosts must not offer a consent prompt. + Blocked, + /// Not a Windows host. MXC collects no telemetry here, so administrative + /// policy is not a meaningful concept. + NotApplicable, +} + +impl PolicyState { + /// Stable, lowercase wire representation used by the CLI flags, the FFI + /// boundary, and the SDKs. Kept separate from `Debug` so the on-the-wire + /// strings never drift if the Rust variant names change. + pub fn as_str(&self) -> &'static str { + match self { + PolicyState::Unrestricted => "unrestricted", + PolicyState::Allowed => "allowed", + PolicyState::Blocked => "blocked", + PolicyState::NotApplicable => "not-applicable", + } + } + + /// Whether telemetry collection is administratively permitted. + /// + /// True for every state except [`Blocked`](PolicyState::Blocked) — + /// including [`NotApplicable`](PolicyState::NotApplicable), because off + /// Windows the *consent* gate is what stops collection, and double-denying + /// here would wrongly imply an administrator had acted. + /// + /// This never means "collect": it is one conjunct of + /// [`super::is_enabled`], which also requires user consent. + pub fn allows_collection(&self) -> bool { + !matches!(self, PolicyState::Blocked) + } +} + +/// The `REG_DWORD` value an administrator sets to permit the optional (usage) +/// telemetry category that MXC emits, mirroring the Windows diagnostic-data +/// scale where `3` is Optional/Full. +#[cfg(target_os = "windows")] +const POLICY_VALUE_OPTIONAL: u32 = 3; + +#[cfg(all( + target_os = "windows", + any(test, all(feature = "test-support", debug_assertions)) +))] +static DEBUG_POLICY_KEY_OVERRIDE: std::sync::Mutex> = std::sync::Mutex::new(None); + +#[cfg(all( + target_os = "windows", + any(test, all(feature = "test-support", debug_assertions)) +))] +const POLICY_KEY_OVERRIDE_ENV: &str = "MXC_TEST_POLICY_KEY_OVERRIDE"; +#[cfg(all( + target_os = "windows", + any(test, all(feature = "test-support", debug_assertions)) +))] +const POLICY_KEY_OVERRIDE_OWNER_ENV: &str = "MXC_TEST_POLICY_KEY_OVERRIDE_OWNER_PID"; + +/// Returns the current administrative telemetry policy. +/// +/// Fail-closed: a value that is present but not understood resolves to +/// [`PolicyState::Blocked`]. An *absent* policy resolves to +/// [`PolicyState::Unrestricted`] — the unmanaged default, where the user's own +/// consent decision governs. +pub fn get_policy() -> PolicyState { + platform::read() +} + +/// Whether an administrator has denied MXC telemetry on this machine. +/// +/// Convenience over [`get_policy`]; the inverse of +/// [`PolicyState::allows_collection`]. +pub fn is_blocked_by_policy() -> bool { + !get_policy().allows_collection() +} + +// --------------------------------------------------------------------------- +// Windows implementation +// --------------------------------------------------------------------------- + +#[cfg(target_os = "windows")] +mod platform { + use super::{PolicyState, POLICY_VALUE_OPTIONAL}; + use winreg::enums::HKEY_LOCAL_MACHINE; + use winreg::RegKey; + + /// Machine-wide policy key. Under `SOFTWARE\Policies`, so it is writable + /// only by administrators — a standard user cannot forge a permit here. + /// + /// Deliberately *not* under `Policies\Microsoft`, even though MXC is a + /// Microsoft product. Windows forbids ADMX-ingested policies from writing + /// under `System`, `Software\Microsoft`, or `Software\Policies\Microsoft` + /// except for a hardcoded allowlist (Office, Edge, OneDrive, VisualStudio, + /// …) that MXC is not on and cannot join without a Windows servicing + /// change. A key under those prefixes would make `Mxc.admx` un-ingestible + /// by Intune and every other MDM. `SOFTWARE\Policies\` is the shape + /// Microsoft's own ADMX-ingestion documentation uses for third-party apps. + /// + /// This is the administrator-facing contract documented in + /// `docs/telemetry/telemetry-administrative-policy.md`; changing it breaks deployed + /// policy. + const POLICY_SUBKEY: &str = r"SOFTWARE\Policies\Mxc"; + const POLICY_VALUE_NAME: &str = "AllowTelemetry"; + + /// The outcome of looking for the policy value. + /// + /// The distinction between [`Absent`](PolicyValue::Absent) and + /// [`Unreadable`](PolicyValue::Unreadable) is load-bearing: only a + /// genuinely missing policy means "unmanaged". Anything that exists but + /// cannot be understood must deny, or an administrator who misconfigured + /// the value would silently get collection instead of the block they + /// intended. + enum PolicyValue { + /// The key or the value genuinely does not exist — no policy is + /// configured, so the machine is unmanaged for MXC telemetry. + Absent, + /// A `REG_DWORD` was read successfully. + Value(u32), + /// A policy is configured but could not be read: wrong value type + /// (e.g. `REG_SZ`), access denied, or a corrupt/failing registry. + Unreadable, + } + + pub(super) fn read() -> PolicyState { + policy_state_from_value(read_policy_value()) + } + + fn policy_state_from_value(value: PolicyValue) -> PolicyState { + match value { + PolicyValue::Absent => PolicyState::Unrestricted, + PolicyValue::Value(POLICY_VALUE_OPTIONAL) => PolicyState::Allowed, + // Every other value — including `0` (off) and `1` (required-only, + // a category MXC does not emit) — denies. Unrecognized values deny + // too rather than being guessed at: fail closed. + PolicyValue::Value(_) => PolicyState::Blocked, + // Fail closed. An administrator who typed the value in as a string, + // or a machine whose registry we cannot read, must not be treated + // as unmanaged. + PolicyValue::Unreadable => PolicyState::Blocked, + } + } + + #[cfg(test)] + pub(super) fn unreadable_policy_state_for_test() -> PolicyState { + policy_state_from_value(PolicyValue::Unreadable) + } + + /// Reads the policy value, distinguishing "not configured" from + /// "configured but unreadable". + /// + /// `winreg` surfaces registry failures as [`std::io::Error`]; a missing key + /// or value is `ERROR_FILE_NOT_FOUND`, which maps to + /// [`std::io::ErrorKind::NotFound`]. Every other error — notably + /// `ErrorKind::InvalidData` for a non-`REG_DWORD` value, and + /// `PermissionDenied` for an ACL that hides the key — is a policy we cannot + /// evaluate, and therefore a deny. + fn read_policy_value() -> PolicyValue { + let (hive, subkey) = policy_location(); + let key = match RegKey::predef(hive).open_subkey(subkey) { + Ok(key) => key, + Err(e) if e.kind() == std::io::ErrorKind::NotFound => return PolicyValue::Absent, + Err(_) => return PolicyValue::Unreadable, + }; + match key.get_value::(POLICY_VALUE_NAME) { + Ok(value) => PolicyValue::Value(value), + Err(e) if e.kind() == std::io::ErrorKind::NotFound => PolicyValue::Absent, + Err(_) => PolicyValue::Unreadable, + } + } + + /// Resolves the hive and subkey to read the policy from. + /// + /// Always the real machine policy location, except under test where + /// [`debug_policy_key_override`] can redirect it under `HKEY_CURRENT_USER` + /// so tests can exercise the real registry code path without requiring + /// administrator rights. The override is compiled out of every shipped + /// binary, so a release build can never be pointed at a user-writable key. + fn policy_location() -> (winreg::HKEY, String) { + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] + if let Some(subkey) = debug_policy_key_override() { + return (winreg::enums::HKEY_CURRENT_USER, subkey); + } + (HKEY_LOCAL_MACHINE, POLICY_SUBKEY.to_string()) + } + + /// Test-only hook: redirects the policy read to `HKCU\`. + /// + /// Active under `cfg(test)` and debug `test-support` feature builds. + /// Never present in shipped or release-profile binaries. + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] + fn debug_policy_key_override() -> Option { + super::DEBUG_POLICY_KEY_OVERRIDE + .lock() + .unwrap_or_else(|e| e.into_inner()) + .clone() + .or_else(|| { + same_process_policy_key_override( + std::env::var_os(super::POLICY_KEY_OVERRIDE_ENV), + std::env::var_os(super::POLICY_KEY_OVERRIDE_OWNER_ENV), + std::process::id(), + ) + }) + } + + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] + /// Environment bridge for language-binding tests that load the debug + /// native library in the same process. The owner PID prevents an ambient + /// override from leaking into child processes. + fn same_process_policy_key_override( + key: Option, + owner: Option, + process_id: u32, + ) -> Option { + let owner = owner?.to_str()?.parse::().ok()?; + if owner != process_id { + return None; + } + let key = key?.into_string().ok()?; + (!key.is_empty()).then_some(key) + } + + /// The production key path is not exercised by redirected policy tests, so + /// this guards the one property that could silently break administrators. + #[cfg(test)] + mod key_path { + use super::POLICY_SUBKEY; + + /// Windows refuses to let an ADMX-ingested policy write under these + /// prefixes (outside a hardcoded allowlist MXC is not on). A key under + /// one of them cannot be deployed by Intune or any other MDM, so moving + /// MXC's key back under `Policies\Microsoft` — a natural-looking + /// "correction" for a Microsoft product — must fail loudly here rather + /// than at an administrator's next ADMX import. + #[test] + fn is_not_under_an_admx_ingestion_blocked_prefix() { + const BLOCKED_PREFIXES: [&str; 3] = [ + r"SYSTEM\", + r"SOFTWARE\MICROSOFT\", + r"SOFTWARE\POLICIES\MICROSOFT\", + ]; + + let upper = POLICY_SUBKEY.to_ascii_uppercase(); + for blocked in BLOCKED_PREFIXES { + assert!( + !upper.starts_with(blocked), + "policy key {POLICY_SUBKEY:?} sits under {blocked:?}, which Windows \ + forbids ADMX-ingested policies from writing; see \ + docs/telemetry/telemetry-administrative-policy.md" + ); + } + } + } + + #[cfg(test)] + mod same_process_override { + use super::same_process_policy_key_override; + + #[test] + fn requires_the_current_process_as_owner() { + assert_eq!( + same_process_policy_key_override( + Some("Software\\MxcTest".into()), + Some("42".into()), + 42, + ), + Some("Software\\MxcTest".to_string()) + ); + assert_eq!( + same_process_policy_key_override( + Some("Software\\MxcTest".into()), + Some("41".into()), + 42, + ), + None + ); + assert_eq!( + same_process_policy_key_override(Some("Software\\MxcTest".into()), None, 42), + None + ); + } + } +} + +// --------------------------------------------------------------------------- +// Non-Windows stub — nothing is collected here, so nothing is restricted. +// --------------------------------------------------------------------------- + +#[cfg(not(target_os = "windows"))] +mod platform { + use super::PolicyState; + + pub(super) fn read() -> PolicyState { + PolicyState::NotApplicable + } +} + +// --------------------------------------------------------------------------- +// Test support — policy redirection stays in process and guard construction +// remains serialized to preserve the shared lock order. +// --------------------------------------------------------------------------- + +#[cfg(any(test, all(feature = "test-support", debug_assertions)))] +pub mod test_support { + use std::sync::{Mutex, MutexGuard}; + + /// Serializes guard construction and preserves the lock order shared with + /// the consent test environment. + pub static POLICY_LOCK: Mutex<()> = Mutex::new(()); + + /// Redirects the policy read (Windows only) to a freshly created, unique + /// `HKCU` subkey for the lifetime of the guard, deleting it and restoring + /// the previous override on drop. + /// + /// Using a real registry key rather than a stubbed value means the tests + /// exercise the actual `winreg` read path. `HKCU` needs no elevation. + /// + /// An inert holder on non-Windows, where policy is `NotApplicable` + /// regardless — kept as a real guard type so call sites need no `#[cfg]`. + /// + /// **Within `wxc_common`, never construct this directly alongside the + /// consent guard** — use `crate::telemetry::test_support::TelemetryTestEnv`, + /// which fixes the acquisition order of the two process-global locks. + pub struct PolicyKeyGuard { + _lock: MutexGuard<'static, ()>, + #[cfg(target_os = "windows")] + subkey: String, + #[cfg(target_os = "windows")] + previous_override: Option, + } + + // `Default` is deliberately not implemented: constructing this guard takes a + // process-global lock and mutates the environment, which is not what a + // caller reaching for `Default::default()` expects. + #[allow(clippy::new_without_default)] + impl PolicyKeyGuard { + /// Creates the guard with no policy value set (the unmanaged default). + #[cfg(target_os = "windows")] + pub fn new() -> Self { + use std::sync::atomic::{AtomicU32, Ordering}; + static COUNTER: AtomicU32 = AtomicU32::new(0); + + let lock = POLICY_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let subkey = format!( + r"Software\MxcTelemetryPolicyTest\{}-{}", + std::process::id(), + COUNTER.fetch_add(1, Ordering::Relaxed) + ); + let hkcu = winreg::RegKey::predef(winreg::enums::HKEY_CURRENT_USER); + // Remove any leftover from a previously crashed run before use. + let _ = hkcu.delete_subkey_all(&subkey); + hkcu.create_subkey(&subkey).expect("create test policy key"); + let previous_override = super::DEBUG_POLICY_KEY_OVERRIDE + .lock() + .unwrap_or_else(|e| e.into_inner()) + .replace(subkey.clone()); + + Self { + _lock: lock, + subkey, + previous_override, + } + } + + #[cfg(not(target_os = "windows"))] + pub fn new() -> Self { + let lock = POLICY_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + Self { _lock: lock } + } + + /// Writes the `AllowTelemetry` policy value into the redirected key. + #[cfg(target_os = "windows")] + pub fn set_value(&self, value: u32) { + let (key, _) = winreg::RegKey::predef(winreg::enums::HKEY_CURRENT_USER) + .create_subkey(&self.subkey) + .expect("open test policy key"); + key.set_value("AllowTelemetry", &value) + .expect("set test policy value"); + } + + #[cfg(not(target_os = "windows"))] + pub fn set_value(&self, _value: u32) {} + + /// Writes `AllowTelemetry` as a `REG_SZ` instead of a `REG_DWORD`, to + /// exercise the wrong-value-type path an administrator can easily hit + /// by typing the value in by hand. + #[cfg(target_os = "windows")] + pub fn set_string_value(&self, value: &str) { + let (key, _) = winreg::RegKey::predef(winreg::enums::HKEY_CURRENT_USER) + .create_subkey(&self.subkey) + .expect("open test policy key"); + key.set_value("AllowTelemetry", &value.to_string()) + .expect("set test policy string value"); + } + + #[cfg(not(target_os = "windows"))] + pub fn set_string_value(&self, _value: &str) {} + } + + #[cfg(target_os = "windows")] + impl Drop for PolicyKeyGuard { + fn drop(&mut self) { + *super::DEBUG_POLICY_KEY_OVERRIDE + .lock() + .unwrap_or_else(|e| e.into_inner()) = self.previous_override.take(); + let _ = winreg::RegKey::predef(winreg::enums::HKEY_CURRENT_USER) + .delete_subkey_all(&self.subkey); + } + } +} + +// --------------------------------------------------------------------------- +// Tests +// --------------------------------------------------------------------------- + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn wire_strings_are_stable() { + assert_eq!(PolicyState::Unrestricted.as_str(), "unrestricted"); + assert_eq!(PolicyState::Allowed.as_str(), "allowed"); + assert_eq!(PolicyState::Blocked.as_str(), "blocked"); + assert_eq!(PolicyState::NotApplicable.as_str(), "not-applicable"); + } + + #[test] + fn only_blocked_denies_collection() { + assert!(PolicyState::Unrestricted.allows_collection()); + assert!(PolicyState::Allowed.allows_collection()); + assert!(PolicyState::NotApplicable.allows_collection()); + assert!(!PolicyState::Blocked.allows_collection()); + } + + #[cfg(not(target_os = "windows"))] + #[test] + fn policy_is_not_applicable_off_windows() { + assert_eq!(get_policy(), PolicyState::NotApplicable); + assert!(!is_blocked_by_policy()); + } +} + +#[cfg(all(test, target_os = "windows"))] +mod windows_tests { + use super::test_support::PolicyKeyGuard; + use super::*; + + #[test] + fn absent_policy_is_unrestricted() { + let _guard = PolicyKeyGuard::new(); + assert_eq!(get_policy(), PolicyState::Unrestricted); + assert!(!is_blocked_by_policy()); + } + + #[test] + fn optional_level_is_allowed() { + let guard = PolicyKeyGuard::new(); + guard.set_value(3); + assert_eq!(get_policy(), PolicyState::Allowed); + assert!(!is_blocked_by_policy()); + } + + #[test] + fn off_level_is_blocked() { + let guard = PolicyKeyGuard::new(); + guard.set_value(0); + assert_eq!(get_policy(), PolicyState::Blocked); + assert!(is_blocked_by_policy()); + } + + /// Level 1 is "required diagnostic data only". MXC emits + /// product-and-service-usage data, which is *optional*, so a + /// required-only machine must collect nothing. + #[test] + fn required_only_level_is_blocked() { + let guard = PolicyKeyGuard::new(); + guard.set_value(1); + assert_eq!(get_policy(), PolicyState::Blocked); + } + + /// Fail closed: a value outside the documented scale is a + /// misconfiguration, and MXC must not read it as permission. + #[test] + fn unrecognized_value_is_blocked() { + let guard = PolicyKeyGuard::new(); + for value in [2u32, 4, 99, u32::MAX] { + guard.set_value(value); + assert_eq!( + get_policy(), + PolicyState::Blocked, + "value {value} must fail closed" + ); + } + } + + /// An administrator who sets `AllowTelemetry` as a string rather than a + /// `REG_DWORD` has still expressed an intent to manage this machine. The + /// value cannot be evaluated, so it must deny — never be mistaken for an + /// unmanaged machine, which would let a prior consent grant re-enable + /// collection the administrator meant to stop. + #[test] + fn wrong_value_type_is_blocked_not_unrestricted() { + let guard = PolicyKeyGuard::new(); + for value in ["0", "3", "", "not-a-number"] { + guard.set_string_value(value); + assert_eq!( + get_policy(), + PolicyState::Blocked, + "REG_SZ {value:?} must fail closed, not read as unmanaged" + ); + assert!(is_blocked_by_policy()); + } + } + + /// The precise regression this guards: a wrong-typed value must not + /// resolve to the same state as no policy at all. + #[test] + fn wrong_value_type_is_distinguishable_from_absent() { + let guard = PolicyKeyGuard::new(); + assert_eq!(get_policy(), PolicyState::Unrestricted); + guard.set_string_value("0"); + assert_ne!( + get_policy(), + PolicyState::Unrestricted, + "a malformed policy must not be indistinguishable from an unmanaged machine" + ); + } + + #[test] + fn unreadable_policy_read_fails_closed() { + let state = crate::telemetry::policy::platform::unreadable_policy_state_for_test(); + assert_eq!(state, PolicyState::Blocked); + assert!(!state.allows_collection()); + } +} diff --git a/src/mxc_telemetry/src/lib.rs b/src/mxc_telemetry/src/lib.rs index a59dfefad..1e2218926 100644 --- a/src/mxc_telemetry/src/lib.rs +++ b/src/mxc_telemetry/src/lib.rs @@ -60,7 +60,7 @@ mod provider { /// concurrent `init`/`shutdown` race from leaving the wrapper flag and the /// provider's real state out of sync (e.g. flag set to registered after a /// `register()` that actually failed, or a double register/unregister). - static REGISTERED: Mutex = Mutex::new(false); + static REGISTERED: Mutex = Mutex::new(0); /// Lock-free "is the provider currently registered" flag. /// @@ -100,9 +100,10 @@ mod provider { }); let mut registered = REGISTERED.lock().unwrap_or_else(|e| e.into_inner()); - if *registered { + if *registered > 0 { // Already registered — the tracelogging crate panics on double - // register, so we must not call `register()` again. + // register, so retain a reference instead. + *registered += 1; return true; } @@ -110,12 +111,12 @@ mod provider { // executable (not a DLL), so unload ordering is not a concern. let status = unsafe { MXC_PROVIDER.register() }; if status != 0 { - // Registration failed; leave `*registered` false so the wrapper + // Registration failed; leave the reference count at zero so the wrapper // state matches reality and a later attempt can retry. return false; } - *registered = true; + *registered = 1; ACTIVE.store(true, Ordering::Release); true } @@ -123,9 +124,11 @@ mod provider { /// Unregister the ETW provider. pub fn shutdown() { let mut registered = REGISTERED.lock().unwrap_or_else(|e| e.into_inner()); - if *registered { + if *registered > 1 { + *registered -= 1; + } else if *registered == 1 { MXC_PROVIDER.unregister(); - *registered = false; + *registered = 0; ACTIVE.store(false, Ordering::Release); } } @@ -144,8 +147,10 @@ mod provider { /// the shared base prefix). Carries no `sandbox_id` / UPN or other caller /// identity — privacy-safe by construction. Empty for one-shot executions /// (which are a single process already correlated by `AppSessionGuid`). + #[allow(clippy::too_many_arguments)] pub fn log_execution( backend: &str, + sandbox_kind: &str, exit_code: i32, outcome: &str, duration_ms: u64, @@ -179,6 +184,7 @@ mod provider { bool8("UTCReplace_AppSessionGuid", &true), }), str8("mxc.backend", backend), + str8("mxc.sandbox_kind", sandbox_kind), i32("mxc.exit_code", &exit_code), str8("mxc.outcome", outcome), u64("mxc.duration_ms", &duration_ms), @@ -209,6 +215,7 @@ mod provider { /// under `__TlgCV__` (see [`log_execution`]); empty for one-shot executions. pub fn log_error( backend: &str, + sandbox_kind: &str, error_type: &str, exit_code: i32, phase: &str, @@ -240,6 +247,7 @@ mod provider { bool8("UTCReplace_AppSessionGuid", &true), }), str8("mxc.backend", backend), + str8("mxc.sandbox_kind", sandbox_kind), str8("mxc.error_type", error_type), i32("mxc.exit_code", &exit_code), // State-aware lifecycle phase (provision|start|exec|stop| @@ -267,8 +275,10 @@ mod provider { false } + #[allow(clippy::too_many_arguments)] pub fn log_execution( _backend: &str, + _sandbox_kind: &str, _exit_code: i32, _outcome: &str, _duration_ms: u64, @@ -280,6 +290,7 @@ mod provider { pub fn log_error( _backend: &str, + _sandbox_kind: &str, _error_type: &str, _exit_code: i32, _phase: &str, @@ -303,6 +314,35 @@ mod tests { /// `init`/`shutdown` must not run concurrently. static TEST_LOCK: Mutex<()> = Mutex::new(()); + #[test] + fn event_names_preserve_collector_contract() { + let source = include_str!("lib.rs"); + let provider_source = source + .split("// Tests") + .next() + .expect("provider source precedes tests"); + assert!( + provider_source.contains("\"MXC.Execution\","), + "execution event identity changed" + ); + assert!( + provider_source.contains("\"MXC.Error\","), + "error event identity changed" + ); + let error_section = provider_source + .split("\"MXC.Error\",") + .nth(1) + .unwrap_or_default(); + assert!( + error_section.contains("str8(\"mxc.backend\", backend)"), + "MXC.Error must emit mxc.backend" + ); + assert!( + error_section.contains("str8(\"mxc.sandbox_kind\", sandbox_kind)"), + "MXC.Error must emit mxc.sandbox_kind" + ); + } + #[test] fn init_shutdown_roundtrip() { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); @@ -318,9 +358,20 @@ mod tests { #[test] fn double_init_is_safe() { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - let _ = init("0.0.0-test", "dev"); - let _ = init("0.0.0-test", "dev"); + let first = init("0.0.0-test", "dev"); + let second = init("0.0.0-test", "dev"); + assert_eq!(second, first); + shutdown(); + assert_eq!( + is_active(), + first, + "one retained registration must remain active only when init succeeded" + ); shutdown(); + assert!( + !is_active(), + "the final release must leave the provider inactive" + ); } #[test] @@ -345,6 +396,7 @@ mod tests { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _ = init("0.0.0-test", "dev"); log_execution( + "test_backend", "test_backend", 0, "success", @@ -361,6 +413,7 @@ mod tests { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _ = init("0.0.0-test", "dev"); log_error( + "test_backend", "test_backend", "config_error", 1, @@ -373,8 +426,17 @@ mod tests { #[test] fn log_without_init() { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); - log_execution("test_backend", 0, "success", 50, "none", "", ""); - log_error("test_backend", "unknown", 1, "", ""); + log_execution( + "test_backend", + "test_backend", + 0, + "success", + 50, + "none", + "", + "", + ); + log_error("test_backend", "test_backend", "unknown", 1, "", ""); } #[test] @@ -383,6 +445,7 @@ mod tests { let _ = init("0.0.0-test", "dev"); shutdown(); log_execution( + "test_backend", "test_backend", 1, "failure", @@ -391,17 +454,63 @@ mod tests { "exec", "iso:wxc-abcd", ); - log_error("test_backend", "process_error", 1, "exec", "iso:wxc-abcd"); + log_error( + "test_backend", + "test_backend", + "process_error", + 1, + "exec", + "iso:wxc-abcd", + ); } #[test] fn handles_empty_strings() { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); let _ = init("", ""); - log_execution("", 0, "", 0, "", "", ""); - log_error("", "", 0, "", ""); + log_execution("", "", 0, "", 0, "", "", ""); + log_error("", "", "", 0, "", ""); shutdown(); } + + #[test] + fn concurrent_init_shutdown_is_safe() { + let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + let results = std::thread::scope(|scope| { + let handles: Vec<_> = (0..8) + .map(|_| { + scope.spawn(|| { + let active = init("0.0.0-concurrent-test", "dev"); + if active { + log_execution( + "test_backend", + "test_backend", + 0, + "success", + 0, + "", + "", + "", + ); + shutdown(); + } + active + }) + }) + .collect(); + handles + .into_iter() + .map(|handle| handle.join().unwrap()) + .collect::>() + }); + + if cfg!(target_os = "windows") { + assert!(results.iter().all(|active| *active)); + assert!(!is_active()); + } else { + assert!(results.iter().all(|active| !*active)); + } + } } // --------------------------------------------------------------------------- diff --git a/tests/scripts/run_telemetry_etw_smoke_test.ps1 b/tests/scripts/run_telemetry_etw_smoke_test.ps1 index b27603fa4..6c7642559 100644 --- a/tests/scripts/run_telemetry_etw_smoke_test.ps1 +++ b/tests/scripts/run_telemetry_etw_smoke_test.ps1 @@ -111,26 +111,6 @@ if (Test-Path $etlDir) { Remove-Item -Recurse -Force $etlDir } New-Item -ItemType Directory -Path $etlDir -Force | Out-Null $etlFile = Join-Path $etlDir 'mxc_trace.etl' -# --------------------------------------------------------------------------- -# Setup: isolated consent seed for deterministic telemetry gating -# --------------------------------------------------------------------------- -$localAppDataOverride = Join-Path $etlDir 'localappdata' -$consentDir = Join-Path $localAppDataOverride 'mxc' -New-Item -ItemType Directory -Path $consentDir -Force | Out-Null -$consentFile = Join-Path $consentDir 'telemetry-consent.json' - -# Seed an explicit granted consent record for this test run so event capture -# does not depend on host-global consent state. -$consentRecord = @{ - version = 2 - consent = 'granted' - promptResourceVersion = '1.0' - promptLocale = 'en-US' - source = 'run_telemetry_etw_smoke_test.ps1' - updatedAtUtc = [DateTime]::UtcNow.ToString('o') -} -$consentRecord | ConvertTo-Json -Depth 4 | Set-Content -Path $consentFile -Encoding utf8 - # --------------------------------------------------------------------------- # Step 1: Start ETW trace session # --------------------------------------------------------------------------- @@ -152,37 +132,21 @@ Write-Host "ETW session started, writing to $etlFile" Write-Host "`n--- Running wxc-exec with telemetry ---" -ForegroundColor Yellow try { - $previousLocalAppData = $env:LOCALAPPDATA - $previousTestOverride = $env:MXC_TEST_LOCALAPPDATA_OVERRIDE - - $env:LOCALAPPDATA = $localAppDataOverride - $env:MXC_TEST_LOCALAPPDATA_OVERRIDE = $localAppDataOverride - try { - # Run with --experimental to enable the telemetry section in this branch. - # The provider is registered during init (before execution); completion - # telemetry events are emitted after dispatch returns. - $executionTimeoutSeconds = 60 - $proc = Start-Process -FilePath $wxcExe ` - -ArgumentList "--debug", "--experimental", $configFile ` - -PassThru -NoNewWindow - if (-not $proc.WaitForExit($executionTimeoutSeconds * 1000)) { - Stop-Process -Id $proc.Id -Force - $proc.WaitForExit() - throw "wxc-exec timed out after $executionTimeoutSeconds seconds" - } - Write-Host "wxc-exec exited with code $($proc.ExitCode)" - } finally { - if ($null -eq $previousLocalAppData) { - Remove-Item Env:LOCALAPPDATA -ErrorAction SilentlyContinue - } else { - $env:LOCALAPPDATA = $previousLocalAppData - } - if ($null -eq $previousTestOverride) { - Remove-Item Env:MXC_TEST_LOCALAPPDATA_OVERRIDE -ErrorAction SilentlyContinue - } else { - $env:MXC_TEST_LOCALAPPDATA_OVERRIDE = $previousTestOverride - } + # Run with --experimental to enable the telemetry section. The provider is + # registered during init (before execution); the MXC.Execution / MXC.Error + # events are emitted on completion, after the runner returns. The sandbox + # itself may fail (e.g. AppContainer prerequisites), but completion + # telemetry still fires for the failure, so events should be captured. + $executionTimeoutSeconds = 60 + $proc = Start-Process -FilePath $wxcExe ` + -ArgumentList "--debug", "--experimental", $configFile ` + -PassThru -NoNewWindow + if (-not $proc.WaitForExit($executionTimeoutSeconds * 1000)) { + Stop-Process -Id $proc.Id -Force + $proc.WaitForExit() + throw "wxc-exec timed out after $executionTimeoutSeconds seconds" } + Write-Host "wxc-exec exited with code $($proc.ExitCode)" } catch { Write-Host "wxc-exec failed to run: $_" -ForegroundColor Yellow # Continue - even a crash after init may have emitted events. @@ -229,7 +193,7 @@ $eventCount = ([regex]::Matches($xmlContent, '') $matchingEvent = $null From ccb27e8034259f61aad3a6762b0028265b63a9a1 Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Wed, 19 Aug 2026 11:11:25 -0700 Subject: [PATCH 02/10] Reject stale consent presenter decisions Track a durable consent-record revision so same-state concurrent writes cannot be overwritten by a stale presenter response. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- src/core/wxc_common/src/telemetry/consent.rs | 97 ++++++++++++++++++-- 1 file changed, 89 insertions(+), 8 deletions(-) diff --git a/src/core/wxc_common/src/telemetry/consent.rs b/src/core/wxc_common/src/telemetry/consent.rs index 773ab6088..579c0957e 100644 --- a/src/core/wxc_common/src/telemetry/consent.rs +++ b/src/core/wxc_common/src/telemetry/consent.rs @@ -220,6 +220,10 @@ struct ConsentRecord { /// this field just read back as `0` rather than failing to parse. #[serde(rename = "updatedAtEpoch", default)] updated_at_epoch: u64, + /// Monotonic record identity used to reject a stale presenter decision + /// even when a concurrent writer preserves the effective consent state. + #[serde(default)] + revision: u64, } /// Returns the current, persisted telemetry consent state. @@ -239,7 +243,7 @@ pub fn get_status() -> ConsentStatus { #[cfg(test)] pub(crate) fn set_consent(granted: bool, source: &str) -> Result<(), String> { - platform::write(granted, source) + platform::with_store_lock(|| platform::write(granted, source)) } /// Request telemetry consent through a host-owned presenter. @@ -253,17 +257,19 @@ pub fn request_consent( where F: FnOnce(&ConsentPrompt) -> Result, { - let (prompt, policy, status) = match consent_preflight(locale) { + let (prompt, policy, status, revision) = match consent_preflight(locale) { ConsentPreflight::Complete(outcome) => return Ok(outcome), + ConsentPreflight::Failed(error) => return Err(ConsentActionError::Persist(error)), ConsentPreflight::Present { prompt, policy, status, - } => (prompt, policy, status), + revision, + } => (prompt, policy, status, revision), }; let decision = presenter(prompt).map_err(ConsentActionError::Presenter)?; - persist_presented_decision(decision, prompt, policy, status) + persist_presented_decision(decision, prompt, policy, status, revision) } /// Asynchronous counterpart to [`request_consent`]. @@ -282,16 +288,18 @@ where let locale = locale.map(str::to_owned); match BlockingTask::spawn(move || consent_preflight(locale.as_deref())).await { ConsentPreflight::Complete(outcome) => Ok(outcome), + ConsentPreflight::Failed(error) => Err(ConsentActionError::Persist(error)), ConsentPreflight::Present { prompt, policy, status, + revision, } => { let decision = presenter(prompt) .await .map_err(ConsentActionError::Presenter)?; BlockingTask::spawn(move || { - persist_presented_decision(decision, prompt, policy, status) + persist_presented_decision(decision, prompt, policy, status, revision) }) .await } @@ -377,16 +385,25 @@ impl std::future::Future for BlockingTask { enum ConsentPreflight { Complete(ConsentActionOutcome), + Failed(String), Present { prompt: &'static ConsentPrompt, policy: super::policy::PolicyState, status: ConsentStatus, + revision: Option, }, } fn consent_preflight(locale: Option<&str>) -> ConsentPreflight { let policy = super::policy::get_policy(); - let status = get_status(); + let (status, revision) = match platform::with_store_lock(|| { + let status = platform::read_status_unlocked(); + let revision = platform::read_revision_unlocked()?; + Ok((status, revision)) + }) { + Ok(snapshot) => snapshot, + Err(error) => return ConsentPreflight::Failed(error), + }; if status.effective_state == ConsentState::NotApplicable { return ConsentPreflight::Complete(ConsentActionOutcome { @@ -415,6 +432,7 @@ fn consent_preflight(locale: Option<&str>) -> ConsentPreflight { prompt, policy, status, + revision, } } @@ -423,10 +441,12 @@ fn persist_presented_decision( prompt: &ConsentPrompt, policy: super::policy::PolicyState, status: ConsentStatus, + revision: Option, ) -> Result { platform::with_store_lock(|| { let current_policy = super::policy::get_policy(); let current_status = platform::read_status_unlocked(); + let current_revision = platform::read_revision_unlocked()?; if current_status.effective_state == ConsentState::NotApplicable { return Ok(ConsentActionOutcome { result: ConsentActionResult::NotApplicable, @@ -454,7 +474,8 @@ fn persist_presented_decision( && current_status.effective_state == ConsentState::Granted && status.effective_state != ConsentState::Granted; if current_policy != policy - || (current_status.effective_state != status.effective_state + || ((current_status.effective_state != status.effective_state + || current_revision != revision) && !is_concurrent_grant_being_withdrawn) { return Err( @@ -558,6 +579,10 @@ mod platform { const STORE_LOCK_RETRY_DELAY: Duration = Duration::from_millis(1); #[cfg(not(test))] const STORE_LOCK_RETRY_DELAY: Duration = Duration::from_millis(20); + #[cfg(test)] + const STORE_LOCK_RETRY_ATTEMPTS: u32 = 1_000; + #[cfg(not(test))] + const STORE_LOCK_RETRY_ATTEMPTS: u32 = IO_RETRY_ATTEMPTS; /// `ERROR_SHARING_VIOLATION` — another handle holds the file with a /// conflicting share mode (AV scanner, indexer, racing sibling process). @@ -721,7 +746,7 @@ mod platform { match options.open(&lock_path) { Ok(file) => break file, Err(error) - if attempts + 1 < IO_RETRY_ATTEMPTS + if attempts + 1 < STORE_LOCK_RETRY_ATTEMPTS && (error.kind() == std::io::ErrorKind::AlreadyExists || is_transient_io_error(&error)) => { @@ -873,6 +898,13 @@ mod platform { read_status_unlocked_inner(withdrawal_marker_state, recover_stale_withdrawal_marker) } + pub(super) fn read_revision_unlocked() -> Result, String> { + let Some(path) = consent_file_path() else { + return Ok(None); + }; + Ok(read_record(&path)?.map(|record| record.revision)) + } + fn read_status_unlocked_inner( marker_check: impl FnOnce(&Path) -> std::io::Result, recover_stale: impl FnOnce(&Path, &Path) -> Result<(), String>, @@ -992,6 +1024,19 @@ mod platform { .map_err(|error| std::io::Error::new(std::io::ErrorKind::InvalidData, error)) } + fn read_record(path: &Path) -> Result, String> { + let data = match with_io_retry(|| read_bounded(path)) { + Ok(data) => data, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => return Ok(None), + Err(error) => { + return Err(format!( + "failed to read telemetry consent record revision: {error}" + )) + } + }; + Ok(serde_json::from_str(&data).ok()) + } + fn recover_stale_withdrawal_marker(_path: &Path, marker: &Path) -> Result<(), String> { match withdrawal_marker_state(marker) .map_err(|e| format!("failed to inspect stale withdrawal marker: {e}"))? @@ -1067,6 +1112,10 @@ mod platform { .parent() .ok_or_else(|| "invalid telemetry consent path".to_string())?; fs::create_dir_all(dir).map_err(|e| format!("failed to create {}: {e}", dir.display()))?; + let revision = read_record(&path)? + .map_or(0, |record| record.revision) + .checked_add(1) + .ok_or_else(|| "telemetry consent record revision is exhausted".to_string())?; let record = ConsentRecord { schema_version: CONSENT_SCHEMA_VERSION, @@ -1076,6 +1125,7 @@ mod platform { prompt_resource_version, prompt_locale: prompt_locale.to_string(), updated_at_epoch: now_epoch_seconds(), + revision, }; let json = serde_json::to_string_pretty(&record) .map_err(|e| format!("failed to serialize telemetry consent record: {e}"))?; @@ -1151,6 +1201,10 @@ mod platform { read_status() } + pub(super) fn read_revision_unlocked() -> Result, String> { + Ok(None) + } + pub(super) fn write(_granted: bool, _source: &str) -> Result<(), String> { Err("telemetry is Windows-only; consent is not applicable on this platform".to_string()) } @@ -1622,6 +1676,33 @@ mod tests { assert_eq!(get_consent(), ConsentState::Denied); } + #[test] + fn same_state_change_while_presenter_is_open_rejects_stale_decision() { + let tmp = tempfile::tempdir().unwrap(); + let env = crate::telemetry::test_support::TelemetryTestEnv::new(tmp.path()); + env.set_policy_value(3); + set_consent(false, "initial-denial").unwrap(); + + let error = request_consent(Some("en-US"), |_| { + set_consent(false, "concurrent-withdrawal").unwrap(); + Ok(ConsentDecision::Yes) + }) + .expect_err("stale presenter decision must not overwrite a newer denial"); + + assert_eq!( + error, + ConsentActionError::Persist( + "consent state changed while the presenter was open; no decision was written" + .to_string() + ) + ); + assert_eq!(get_consent(), ConsentState::Denied); + let record = + std::fs::read_to_string(tmp.path().join("mxc").join("telemetry-consent.json")) + .unwrap(); + assert!(record.contains(r#""source": "concurrent-withdrawal""#)); + } + #[test] fn explicit_denial_withdraws_a_concurrent_grant() { let tmp = tempfile::tempdir().unwrap(); From ea68d7e37f7959fc9d266edbcfab64eaa36cc42f Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Wed, 19 Aug 2026 17:44:00 -0700 Subject: [PATCH 03/10] Tag WINEXT telemetry with MXC Part A Emit privacy product 11 and Client Diagnostic Data category 1 on every MXC event while retaining the approved Product and Service Usage tag. Document the optional non-CORE classification and lock the fields with contract tests. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- docs/telemetry/telemetry.md | 24 ++++++++++++++++ src/mxc_telemetry/src/lib.rs | 54 ++++++++++++++++++++++++++++++++++++ 2 files changed, 78 insertions(+) diff --git a/docs/telemetry/telemetry.md b/docs/telemetry/telemetry.md index ed8e9b630..edd488a40 100644 --- a/docs/telemetry/telemetry.md +++ b/docs/telemetry/telemetry.md @@ -41,12 +41,36 @@ Rust constants and `write_event!` struct fields. | **Sampling keywords** | `MICROSOFT_KEYWORD_MEASURES` named constant | Raw `u64` in `keyword(...)` | `const MICROSOFT_KEYWORD_MEASURES: u64 = 0x0000_4000_0000_0000` | | **Common event fields** | `_GENERIC_PARTB_FIELDS_ENABLED` pattern | `struct("Name", { ... })` in `write_event!` | `struct("COMMON_MXC_PARAMS", { Version, Channel, IsDebugging, UTCReplace_AppSessionGuid })` | | **Provider lifecycle** | `IMPLEMENT_TRACELOGGING_CLASS` singleton | `define_provider!` static + `register()`/`unregister()` | `OnceLock` for version/channel, manual lifecycle | +| **Privacy Product** | Common Schema Part A product | `u16(...)` field | `PartA_PrivacyProduct = 11` (`MXC (Microsoft Execution Containers)`) on all events | +| **Privacy Data Category** | Common Schema Part A data category | `u16(...)` field | `PartA_PrivacyDataCategory = 1` (`Client Diagnostic Data`) on all events | | **Privacy Data Tags** | `TelemetryPrivacyDataTag(PDT_*)` | `u64("PartA_PrivTags", &val)` field | `PDT_PRODUCT_AND_SERVICE_USAGE` on all events | | **Activity tracking** | `DEFINE_TELEMETRY_ACTIVITY` | Manual `Opcode` | Not needed for current events | The remaining gap (activity tracking) is not needed for current events. If needed later, it can be added incrementally. +## Common Event Fields (Part A) + +Every MXC telemetry event carries the WinExt-required Common Schema privacy +metadata: + +| Field | Type | Value | +|-------|------|-------| +| `PartA_PrivacyProduct` | uint16 | `11` — `MXC (Microsoft Execution Containers)` | +| `PartA_PrivacyDataCategory` | uint16 | `1` — `Client Diagnostic Data` | +| `PartA_PrivTags` | uint64 | `Product and Service Usage Data` | + +Product `11` is privacy-approved and registered in the WinExt product list. +The OS/Common Schema catalog update is tracked by UTC bug `63666074`. MXC's +events use the sampled `MICROSOFT_KEYWORD_MEASURES` keyword and explicit +product consent, so they are optional diagnostic data rather than required +(CORE) or critical-optional events. If the sampling level, event purpose, or +collected fields change, the privacy tag and approval classification must be +re-evaluated. + +The WinExt provider-group identifier remains a secure build input. It must not +be copied into source, tests, documentation, or commit messages. + ## Common Event Fields (Part C) Every MXC telemetry event includes a `COMMON_MXC_PARAMS` struct grouping diff --git a/src/mxc_telemetry/src/lib.rs b/src/mxc_telemetry/src/lib.rs index 1e2218926..95fe28f2d 100644 --- a/src/mxc_telemetry/src/lib.rs +++ b/src/mxc_telemetry/src/lib.rs @@ -44,6 +44,13 @@ mod provider { /// Applied via a `PartA_PrivTags` field per the `TelemetryPrivacyDataTag` pattern. pub(crate) const PDT_PRODUCT_AND_SERVICE_USAGE: u64 = 0x0000_0000_0200_0000; + /// Privacy-approved WinExt product identifier for + /// MXC (Microsoft Execution Containers). + pub(crate) const PRIVACY_PRODUCT_MXC: u16 = 11; + + /// WinExt permits only Client Diagnostic Data. + pub(crate) const PRIVACY_DATA_CATEGORY_CLIENT_DIAGNOSTIC_DATA: u16 = 1; + /// Cached provider state, set once at init and read on every event. struct ProviderState { version: String, @@ -176,6 +183,11 @@ mod provider { // MXC.Error (Warning) and any future provider malfunction. level(Informational), keyword(MICROSOFT_KEYWORD_MEASURES), + u16("PartA_PrivacyProduct", &PRIVACY_PRODUCT_MXC), + u16( + "PartA_PrivacyDataCategory", + &PRIVACY_DATA_CATEGORY_CLIENT_DIAGNOSTIC_DATA, + ), u64("PartA_PrivTags", &PDT_PRODUCT_AND_SERVICE_USAGE), struct("COMMON_MXC_PARAMS", { str8("Version", &state.version), @@ -239,6 +251,11 @@ mod provider { // reserved for genuine provider/telemetry malfunctions. level(Warning), keyword(MICROSOFT_KEYWORD_MEASURES), + u16("PartA_PrivacyProduct", &PRIVACY_PRODUCT_MXC), + u16( + "PartA_PrivacyDataCategory", + &PRIVACY_DATA_CATEGORY_CLIENT_DIAGNOSTIC_DATA, + ), u64("PartA_PrivTags", &PDT_PRODUCT_AND_SERVICE_USAGE), struct("COMMON_MXC_PARAMS", { str8("Version", &state.version), @@ -343,6 +360,43 @@ mod tests { ); } + #[test] + fn winext_part_a_fields_preserve_privacy_contract() { + let source = include_str!("lib.rs"); + let provider_source = source + .split("// Tests") + .next() + .expect("provider source precedes tests"); + + assert!(provider_source.contains("const PRIVACY_PRODUCT_MXC: u16 = 11;")); + assert!(provider_source + .contains("const PRIVACY_DATA_CATEGORY_CLIENT_DIAGNOSTIC_DATA: u16 = 1;")); + assert_eq!( + provider_source + .matches("u16(\"PartA_PrivacyProduct\", &PRIVACY_PRODUCT_MXC)") + .count(), + 2, + "every MXC event must carry privacy product 11" + ); + assert_eq!( + provider_source + .matches( + "\"PartA_PrivacyDataCategory\",\n \ + &PRIVACY_DATA_CATEGORY_CLIENT_DIAGNOSTIC_DATA," + ) + .count(), + 2, + "every MXC event must carry Client Diagnostic Data category 1" + ); + assert_eq!( + provider_source + .matches("u64(\"PartA_PrivTags\", &PDT_PRODUCT_AND_SERVICE_USAGE)") + .count(), + 2, + "every MXC event must retain its approved privacy tag" + ); + } + #[test] fn init_shutdown_roundtrip() { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); From 2615fa1c1698a84ef5b832098a22b84133700be5 Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Wed, 19 Aug 2026 18:10:02 -0700 Subject: [PATCH 04/10] Make WINEXT contract test format-independent Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- src/mxc_telemetry/src/lib.rs | 14 +++++++++----- 1 file changed, 9 insertions(+), 5 deletions(-) diff --git a/src/mxc_telemetry/src/lib.rs b/src/mxc_telemetry/src/lib.rs index 95fe28f2d..5caf02d53 100644 --- a/src/mxc_telemetry/src/lib.rs +++ b/src/mxc_telemetry/src/lib.rs @@ -380,13 +380,17 @@ mod tests { ); assert_eq!( provider_source - .matches( - "\"PartA_PrivacyDataCategory\",\n \ - &PRIVACY_DATA_CATEGORY_CLIENT_DIAGNOSTIC_DATA," - ) + .matches("\"PartA_PrivacyDataCategory\"") .count(), 2, - "every MXC event must carry Client Diagnostic Data category 1" + "every MXC event must carry a privacy data category" + ); + assert_eq!( + provider_source + .matches("&PRIVACY_DATA_CATEGORY_CLIENT_DIAGNOSTIC_DATA") + .count(), + 2, + "every MXC event must use Client Diagnostic Data category 1" ); assert_eq!( provider_source From 6b024ed2ab55f571d583613946426593c0543384 Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Wed, 19 Aug 2026 20:25:16 -0700 Subject: [PATCH 05/10] Request delete access for consent lock Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- src/core/wxc_common/src/telemetry/consent.rs | 23 +++++++++++++++++++- 1 file changed, 22 insertions(+), 1 deletion(-) diff --git a/src/core/wxc_common/src/telemetry/consent.rs b/src/core/wxc_common/src/telemetry/consent.rs index 579c0957e..8e0241689 100644 --- a/src/core/wxc_common/src/telemetry/consent.rs +++ b/src/core/wxc_common/src/telemetry/consent.rs @@ -727,6 +727,8 @@ mod platform { operation: impl FnOnce() -> Result, ) -> Result { use std::os::windows::fs::OpenOptionsExt; + use windows::Win32::Foundation::GENERIC_WRITE; + use windows::Win32::Storage::FileSystem::{DELETE, FILE_FLAG_DELETE_ON_CLOSE}; let path = consent_file_path().ok_or_else(|| { "could not resolve %LocalAppData%; cannot lock telemetry consent".to_string() @@ -742,7 +744,8 @@ mod platform { options .write(true) .create_new(true) - .custom_flags(windows::Win32::Storage::FileSystem::FILE_FLAG_DELETE_ON_CLOSE.0); + .access_mode(GENERIC_WRITE.0 | DELETE.0) + .custom_flags(FILE_FLAG_DELETE_ON_CLOSE.0); match options.open(&lock_path) { Ok(file) => break file, Err(error) @@ -1507,6 +1510,24 @@ mod tests { assert_eq!(get_consent(), ConsentState::Undetermined); } + #[test] + fn store_lock_is_deleted_when_released() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let lock_path = tmp.path().join("mxc").join("telemetry-consent.lock"); + + platform::with_store_lock(|| { + assert!(lock_path.exists(), "lock file must exist while held"); + Ok(()) + }) + .expect("acquire store lock"); + + assert!( + !lock_path.exists(), + "delete-on-close must remove the released lock" + ); + } + #[test] fn grant_then_read_round_trips() { let tmp = tempfile::tempdir().unwrap(); From e2c4c54dfc18c81ec4c50b72135d08c14a1fa709 Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Tue, 25 Aug 2026 15:05:36 -0700 Subject: [PATCH 06/10] Clarify telemetry consent and policy failures Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- src/core/wxc_common/src/telemetry/consent.rs | 48 +++++++---- src/core/wxc_common/src/telemetry/policy.rs | 86 +++++++++++++++++++- 2 files changed, 113 insertions(+), 21 deletions(-) diff --git a/src/core/wxc_common/src/telemetry/consent.rs b/src/core/wxc_common/src/telemetry/consent.rs index 8e0241689..e43475d7d 100644 --- a/src/core/wxc_common/src/telemetry/consent.rs +++ b/src/core/wxc_common/src/telemetry/consent.rs @@ -98,13 +98,6 @@ pub struct ConsentStatus { pub reason: Option, } -impl ConsentStatus { - /// Whether an explicit consent request may offer the canonical prompt. - pub fn needs_prompt(&self) -> bool { - matches!(self.effective_state, ConsentState::Undetermined) - } -} - /// Explicit result returned by a consent presenter. #[derive(Debug, Clone, Copy, PartialEq, Eq)] pub enum ConsentDecision { @@ -171,13 +164,12 @@ impl ConsentState { matches!(self, ConsentState::Granted) } - /// Whether a hosting application should offer its own first-run consent - /// prompt for this state. + /// Whether this consent state represents an absent user decision. /// - /// This shared predicate keeps all consumer surfaces consistent. It is - /// false for [`NotApplicable`](ConsentState::NotApplicable), because MXC - /// collects no telemetry off Windows. - pub fn needs_prompt(&self) -> bool { + /// This is deliberately private and policy-blind. Host applications must + /// use [`needs_consent_prompt`], which also suppresses the prompt under an + /// administrative block. + fn needs_prompt(&self) -> bool { matches!(self, ConsentState::Undetermined) } } @@ -228,10 +220,11 @@ struct ConsentRecord { /// Returns the current, persisted telemetry consent state. /// -/// Fail-closed: a missing file, an unreadable file, unparseable JSON, or an -/// unrecognized `schemaVersion` all resolve to [`ConsentState::Undetermined`] -/// — never to `Granted`. Always [`ConsentState::NotApplicable`] on -/// non-Windows platforms, without any filesystem access. +/// Fail-closed: a missing file, an unreadable file, or unparseable JSON resolves +/// to [`ConsentState::Undetermined`]. An unrecognized `schemaVersion` +/// invalidates a stored grant to `Undetermined`, while a stored denial remains +/// denied. Always [`ConsentState::NotApplicable`] on non-Windows platforms, +/// without any filesystem access. pub fn get_consent() -> ConsentState { get_status().effective_state } @@ -2014,7 +2007,7 @@ mod tests { } #[test] - fn unknown_schema_version_is_undetermined() { + fn unsupported_schema_grant_is_undetermined() { let tmp = tempfile::tempdir().unwrap(); let _guard = LocalAppDataGuard::set(tmp.path()); let dir = tmp.path().join("mxc"); @@ -2027,6 +2020,25 @@ mod tests { assert_eq!(get_consent(), ConsentState::Undetermined); } + #[test] + fn unsupported_schema_denial_remains_denied() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("telemetry-consent.json"), + r#"{"schemaVersion":999,"consent":"denied"}"#, + ) + .unwrap(); + + let status = get_status(); + + assert_eq!(status.stored_state, ConsentState::Denied); + assert_eq!(status.effective_state, ConsentState::Denied); + assert_eq!(status.reason, None); + } + #[test] fn unsupported_grant_schema_reports_a_typed_reason() { let tmp = tempfile::tempdir().unwrap(); diff --git a/src/core/wxc_common/src/telemetry/policy.rs b/src/core/wxc_common/src/telemetry/policy.rs index 2433b9c74..a54679e95 100644 --- a/src/core/wxc_common/src/telemetry/policy.rs +++ b/src/core/wxc_common/src/telemetry/policy.rs @@ -103,7 +103,9 @@ const POLICY_KEY_OVERRIDE_OWNER_ENV: &str = "MXC_TEST_POLICY_KEY_OVERRIDE_OWNER_ /// Fail-closed: a value that is present but not understood resolves to /// [`PolicyState::Blocked`]. An *absent* policy resolves to /// [`PolicyState::Unrestricted`] — the unmanaged default, where the user's own -/// consent decision governs. +/// consent decision governs. Registry failures are reported to stderr once per +/// distinct failure so the fail-closed result does not hide a broken policy +/// deployment from operators. pub fn get_policy() -> PolicyState { platform::read() } @@ -123,6 +125,9 @@ pub fn is_blocked_by_policy() -> bool { #[cfg(target_os = "windows")] mod platform { use super::{PolicyState, POLICY_VALUE_OPTIONAL}; + use std::collections::HashSet; + use std::io::Write; + use std::sync::{Mutex, OnceLock}; use winreg::enums::HKEY_LOCAL_MACHINE; use winreg::RegKey; @@ -163,6 +168,45 @@ mod platform { Unreadable, } + #[derive(Default)] + struct FailureReporter { + reported: Mutex>, + } + + impl FailureReporter { + fn report(&self, signature: String, emit: impl FnOnce(&str)) { + let is_new = self + .reported + .lock() + .unwrap_or_else(|error| error.into_inner()) + .insert(signature.clone()); + if is_new { + emit(&signature); + } + } + } + + fn report_policy_read_failure(operation: &str, error: &std::io::Error) { + static FAILURES: OnceLock = OnceLock::new(); + let signature = format!( + "{operation}: kind={:?}, os_error={:?}, error={error}", + error.kind(), + error.raw_os_error() + ); + FAILURES + .get_or_init(FailureReporter::default) + .report(signature, |detail| { + let stderr = std::io::stderr(); + let mut handle = stderr.lock(); + let _ = writeln!( + handle, + "mxc: failed to read telemetry administrative policy ({detail}); \ + treating policy as blocked" + ); + let _ = handle.flush(); + }); + } + pub(super) fn read() -> PolicyState { policy_state_from_value(read_policy_value()) } @@ -201,12 +245,18 @@ mod platform { let key = match RegKey::predef(hive).open_subkey(subkey) { Ok(key) => key, Err(e) if e.kind() == std::io::ErrorKind::NotFound => return PolicyValue::Absent, - Err(_) => return PolicyValue::Unreadable, + Err(error) => { + report_policy_read_failure("open policy key", &error); + return PolicyValue::Unreadable; + } }; match key.get_value::(POLICY_VALUE_NAME) { Ok(value) => PolicyValue::Value(value), Err(e) if e.kind() == std::io::ErrorKind::NotFound => PolicyValue::Absent, - Err(_) => PolicyValue::Unreadable, + Err(error) => { + report_policy_read_failure("read AllowTelemetry value", &error); + PolicyValue::Unreadable + } } } @@ -321,6 +371,36 @@ mod platform { ); } } + + #[cfg(test)] + mod failure_reporter { + use super::FailureReporter; + use std::sync::Mutex; + + #[test] + fn reports_each_distinct_failure_once() { + let reporter = FailureReporter::default(); + let emitted = Mutex::new(Vec::new()); + + for signature in [ + "open: access denied", + "open: access denied", + "read: wrong type", + ] { + reporter.report(signature.to_string(), |message| { + emitted + .lock() + .unwrap_or_else(|error| error.into_inner()) + .push(message.to_string()); + }); + } + + assert_eq!( + *emitted.lock().unwrap_or_else(|error| error.into_inner()), + ["open: access denied", "read: wrong type"] + ); + } + } } // --------------------------------------------------------------------------- From 7601ac5bbca58a9076c1054a3724ea077d59a303 Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Tue, 25 Aug 2026 18:19:23 -0700 Subject: [PATCH 07/10] Harden telemetry consent persistence Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- .github/copilot-instructions.md | 7 +- docs/schema.md | 2 +- docs/telemetry/telemetry-consent-design.md | 10 +- src/core/wxc_common/src/telemetry/consent.rs | 302 ++++++++++++++++--- src/core/wxc_common/src/telemetry/mod.rs | 20 ++ src/core/wxc_common/src/telemetry/policy.rs | 24 +- 6 files changed, 291 insertions(+), 74 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index e4fdd7fdf..8680513c0 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -138,8 +138,7 @@ npm test npm run test:integration # C# SDK (from sdk/dotnet/) -dotnet test --solution Microsoft.Mxc.Sdk.slnx # Debug only; builds mxc_ffi via cargo (Rust toolchain required) - # telemetry tests need the debug-only MXC_TEST_LOCALAPPDATA_OVERRIDE, so `-c Release` fails by design +dotnet test --solution Microsoft.Mxc.Sdk.slnx # builds mxc_ffi via cargo (Rust toolchain required) # Local PowerShell helpers — run from repo root, require built binaries tests\scripts\run_test_configs.ps1 # All test configs via wxc_test_driver @@ -153,8 +152,6 @@ tests\scripts\run_windows_sandbox_state_aware_tests.ps1 # Windows Sandbox st tests\scripts\run_lxc_all_tests.sh # All LXC tests (Linux) tests\scripts\run_bwrap_all_tests.sh # All Bubblewrap tests (Linux, requires bwrap). Must NOT run as root — several tests assert the sandbox drops capabilities, which cannot hold under a root launcher; the script refuses root explicitly. sudo tests\scripts\run_bwrap_inbound_deny_test.sh # Bubblewrap inbound default-deny E2E (root-only: needs host CAP_NET_ADMIN to read the sandbox netns and inject a peer). Reported as skipped by the suite above; CI runs it separately from run_ci_backend_tests.sh. -tests\scripts\run_telemetry_consent_smoke_test.ps1 # Telemetry consent + policy CLI E2E (Windows; debug binary only) - # E2E test crate — Rust executor integration tests (from src/) cargo test -p wxc_e2e_tests # Invokes MXC binaries directly cargo test -p wxc_e2e_tests -- --ignored # Include stress tests (run_on_repeat) @@ -227,8 +224,8 @@ Core references: - `docs/examples.md` — annotated configuration examples (see also `tests/examples/` and `tests/configs/`) - `docs/diagnostics.md` — diagnostic logging knobs (env vars, log file format) - `docs/ci-validation-infrastructure.md` — validation (E2E) test matrix: workflows and job names, catalog format, per-backend coverage and status, and the runbook for adding/removing an OS, backend, or plan +- `docs/host-prep.md` — `wxc-host-prep.exe` host setup binary (`prepare-system-drive` / `unprepare-system-drive` for the AppContainer ACEs on the system-drive root, plus `prepare-null-device` / `verify-null-device` / `dump-null-device` for the `\Device\Null` security descriptor that AppContainer-based backends require). Owns elevation via embedded `requireAdministrator` manifest — `wxc-exec.exe` no longer self-elevates. - `docs/sandbox-policy/0.7.0/policy.md` — sandbox policy 0.7.0 specification -- `docs/sandbox-policy/v1/policy.md` — sandbox policy v1 specification - `docs/telemetry/telemetry.md` — telemetry overview; `docs/telemetry/telemetry-consent-design.md` (Windows-only consent design and per-SDK surface) and `docs/telemetry/telemetry-administrative-policy.md` (the MDM / Group Policy ceiling) Per-backend guides: diff --git a/docs/schema.md b/docs/schema.md index 9b3f443af..9bddcefe6 100644 --- a/docs/schema.md +++ b/docs/schema.md @@ -222,7 +222,7 @@ cannot mix both formats in one request. > it is rejected as an unknown field because lifecycle correlation is internal > to MXC and is not part of the request or response contract. See > [`docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md`](state-aware-lifecycle/mxc-state-aware-sandbox-api.md) -> and [`docs/telemetry/telemetry.md`](telemetry/telemetry.md#correlating-a-lifecycle). +> and [`docs/telemetry/telemetry.md`](telemetry/telemetry.md). ### Working Directory diff --git a/docs/telemetry/telemetry-consent-design.md b/docs/telemetry/telemetry-consent-design.md index ad1e4eacf..084823e89 100644 --- a/docs/telemetry/telemetry-consent-design.md +++ b/docs/telemetry/telemetry-consent-design.md @@ -112,11 +112,11 @@ version is 2 and records: - `promptLocale` - MXC version, source, and update timestamp for local audit/support provenance -Stored and effective state are reported separately. A legacy grant without the -current prompt version remains visible as `storedState: "granted"` but is -`effectiveState: "undetermined"` with reason `prompt-version-missing` or -`prompt-version-unsupported`. It never authorizes collection. Legacy denial -remains denied. +Stored and effective state are reported separately. A grant without the +current prompt version and canonical prompt locale remains visible as +`storedState: "granted"` but is `effectiveState: "undetermined"` with reason +`prompt-version-missing` or `prompt-version-unsupported`. It never authorizes +collection. Legacy denial remains denied. Only two operations mutate the store: diff --git a/src/core/wxc_common/src/telemetry/consent.rs b/src/core/wxc_common/src/telemetry/consent.rs index e43475d7d..f5ef9e50e 100644 --- a/src/core/wxc_common/src/telemetry/consent.rs +++ b/src/core/wxc_common/src/telemetry/consent.rs @@ -555,9 +555,12 @@ mod platform { CONSENT_SCHEMA_VERSION, }; use crate::telemetry::consent_prompt::{CONSENT_RESOURCE_VERSION, FALLBACK_LOCALE}; + use crate::telemetry::FailureReporter; use std::fs; use std::io::{Read, Write}; + use std::os::windows::ffi::OsStrExt; use std::path::{Path, PathBuf}; + use std::sync::OnceLock; use std::time::Duration; /// Number of attempts for filesystem operations that can transiently fail @@ -587,6 +590,22 @@ mod platform { const CONSENT_LOCK_FILE: &str = "telemetry-consent.lock"; const WITHDRAWAL_MARKER_STALE_AFTER: Duration = Duration::from_secs(5); + fn report_consent_store_failure(signature: String) { + static FAILURES: OnceLock = OnceLock::new(); + FAILURES + .get_or_init(FailureReporter::default) + .report(signature, |detail| { + let stderr = std::io::stderr(); + let mut handle = stderr.lock(); + let _ = writeln!( + handle, + "mxc: telemetry consent store failure ({detail}); \ + treating consent as undetermined" + ); + let _ = handle.flush(); + }); + } + /// Resolves the per-user `%LocalAppData%` directory through the Windows /// known-folder API rather than trusting `LOCALAPPDATA`. A parent process /// controls the child's environment, so trusting that variable could @@ -881,17 +900,37 @@ mod platform { pub(super) fn read_status_unlocked_with_marker_for_test( marker_state: std::io::Result, ) -> ConsentStatus { - read_status_unlocked_inner(|_| marker_state, recover_stale_withdrawal_marker) + read_status_unlocked_inner( + |_| marker_state, + recover_stale_withdrawal_marker, + report_consent_store_failure, + ) + } + + #[cfg(test)] + pub(super) fn read_status_unlocked_with_reporter_for_test( + marker_state: std::io::Result, + report: impl FnMut(String), + ) -> ConsentStatus { + read_status_unlocked_inner(|_| marker_state, recover_stale_withdrawal_marker, report) } pub(super) fn read_status() -> ConsentStatus { - read_status_unlocked_inner(withdrawal_marker_state, |path, marker| { - with_store_lock(|| recover_stale_withdrawal_marker(path, marker)) - }) + match with_store_lock(|| Ok(read_status_unlocked())) { + Ok(status) => status, + Err(error) => { + report_consent_store_failure(format!("lock consent store: {error}")); + unreadable_status() + } + } } pub(super) fn read_status_unlocked() -> ConsentStatus { - read_status_unlocked_inner(withdrawal_marker_state, recover_stale_withdrawal_marker) + read_status_unlocked_inner( + withdrawal_marker_state, + recover_stale_withdrawal_marker, + report_consent_store_failure, + ) } pub(super) fn read_revision_unlocked() -> Result, String> { @@ -904,15 +943,8 @@ mod platform { fn read_status_unlocked_inner( marker_check: impl FnOnce(&Path) -> std::io::Result, recover_stale: impl FnOnce(&Path, &Path) -> Result<(), String>, + mut report: impl FnMut(String), ) -> ConsentStatus { - fn unreadable_status() -> ConsentStatus { - ConsentStatus { - stored_state: ConsentState::Undetermined, - effective_state: ConsentState::Undetermined, - reason: Some(ConsentStatusReason::StoreUnreadable), - } - } - fn no_record_status() -> ConsentStatus { ConsentStatus { stored_state: ConsentState::Undetermined, @@ -922,19 +954,30 @@ mod platform { } let Some(path) = consent_file_path() else { + report("resolve consent store path".to_string()); return unreadable_status(); }; let Some(dir) = path.parent() else { + report("resolve consent store parent directory".to_string()); return unreadable_status(); }; let marker_path = dir.join(WITHDRAWAL_PENDING_FILE); match marker_check(&marker_path) { Ok(WithdrawalMarkerState::Absent) => {} - Ok(WithdrawalMarkerState::Pending) | Err(_) => { + Ok(WithdrawalMarkerState::Pending) => { + return unreadable_status(); + } + Err(error) => { + report(format!( + "inspect withdrawal marker: kind={:?}, os_error={:?}, error={error}", + error.kind(), + error.raw_os_error() + )); return unreadable_status(); } Ok(WithdrawalMarkerState::Stale) => { - if recover_stale(&path, &marker_path).is_err() { + if let Err(error) = recover_stale(&path, &marker_path) { + report(format!("recover stale withdrawal marker: {error}")); return unreadable_status(); } } @@ -944,25 +987,29 @@ mod platform { Err(error) if error.kind() == std::io::ErrorKind::NotFound => { return no_record_status() } - Err(_) => return unreadable_status(), + Err(error) => { + report(format!( + "read consent record: kind={:?}, os_error={:?}, error={error}", + error.kind(), + error.raw_os_error() + )); + return unreadable_status(); + } }; - let Ok(record) = serde_json::from_str::(&data) else { - return ConsentStatus { - stored_state: ConsentState::Undetermined, - effective_state: ConsentState::Undetermined, - reason: Some(ConsentStatusReason::StoreMalformed), - }; + let record = match serde_json::from_str::(&data) { + Ok(record) => record, + Err(error) => { + report(format!("parse consent record: {error}")); + return malformed_status(); + } }; let stored_state = match record.consent.as_str() { "granted" => ConsentState::Granted, "denied" => ConsentState::Denied, _ => { - return ConsentStatus { - stored_state: ConsentState::Undetermined, - effective_state: ConsentState::Undetermined, - reason: Some(ConsentStatusReason::StoreMalformed), - }; + report("validate consent record: unrecognized consent value".to_string()); + return malformed_status(); } }; @@ -987,11 +1034,21 @@ mod platform { } match record.prompt_resource_version { - Some(CONSENT_RESOURCE_VERSION) if !record.prompt_locale.is_empty() => ConsentStatus { - stored_state, - effective_state: ConsentState::Granted, - reason: None, - }, + Some(CONSENT_RESOURCE_VERSION) if record.prompt_locale == FALLBACK_LOCALE => { + ConsentStatus { + stored_state, + effective_state: ConsentState::Granted, + reason: None, + } + } + Some(CONSENT_RESOURCE_VERSION) => { + report("validate consent record: unsupported prompt locale".to_string()); + ConsentStatus { + stored_state, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::PromptVersionUnsupported), + } + } None => ConsentStatus { stored_state, effective_state: ConsentState::Undetermined, @@ -1005,6 +1062,22 @@ mod platform { } } + fn unreadable_status() -> ConsentStatus { + ConsentStatus { + stored_state: ConsentState::Undetermined, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::StoreUnreadable), + } + } + + fn malformed_status() -> ConsentStatus { + ConsentStatus { + stored_state: ConsentState::Undetermined, + effective_state: ConsentState::Undetermined, + reason: Some(ConsentStatusReason::StoreMalformed), + } + } + fn read_bounded(path: &Path) -> std::io::Result { let file = fs::File::open(path)?; let mut bytes = Vec::new(); @@ -1084,8 +1157,13 @@ mod platform { .parent() .ok_or_else(|| "invalid telemetry consent path".to_string())?; fs::create_dir_all(dir).map_err(|e| format!("failed to create {}: {e}", dir.display()))?; - with_io_retry(|| fs::write(dir.join(WITHDRAWAL_PENDING_FILE), b"pending")) - .map_err(|e| format!("failed to mark withdrawal pending: {e}")) + let marker = dir.join(WITHDRAWAL_PENDING_FILE); + with_io_retry(|| { + let mut file = fs::File::create(&marker)?; + file.write_all(b"pending")?; + file.sync_all() + }) + .map_err(|e| format!("failed to durably mark withdrawal pending: {e}")) } pub(super) fn finish_withdrawal() -> Result<(), String> { @@ -1129,7 +1207,8 @@ mod platform { // Atomic write: write to a *unique* temp file in the same directory // (process id + a random suffix, so two concurrent `wxc-exec` // grant/revoke invocations never share — and thus never race on — - // the same temp path), then rename over the real path. A crash + // the same temp path), then replace the real path with write-through + // semantics. A crash // mid-write never leaves a torn/corrupt file in place of a // previously-valid one; if the rename itself fails, the temp file is // removed rather than left behind as a leaked, orphaned artifact. @@ -1144,13 +1223,51 @@ mod platform { remove_best_effort(&tmp_path); return Err(format!("failed to write {}: {e}", tmp_path.display())); } - if let Err(e) = with_io_retry(|| fs::rename(&tmp_path, &path)) { + if let Err(e) = with_io_retry(|| replace_file_write_through(&tmp_path, &path)) { remove_best_effort(&tmp_path); return Err(format!("failed to finalize {}: {e}", path.display())); } clear_withdrawal_marker(&path) } + pub(super) fn replace_file_write_through( + source: &Path, + destination: &Path, + ) -> std::io::Result<()> { + use windows::core::PCWSTR; + use windows::Win32::Storage::FileSystem::{ + MoveFileExW, MOVEFILE_REPLACE_EXISTING, MOVEFILE_WRITE_THROUGH, + }; + + let source: Vec = source.as_os_str().encode_wide().chain(Some(0)).collect(); + let destination: Vec = destination + .as_os_str() + .encode_wide() + .chain(Some(0)) + .collect(); + + // SAFETY: both paths are valid, NUL-terminated UTF-16 buffers that + // remain alive for the duration of the call. + unsafe { + MoveFileExW( + PCWSTR(source.as_ptr()), + PCWSTR(destination.as_ptr()), + MOVEFILE_REPLACE_EXISTING | MOVEFILE_WRITE_THROUGH, + ) + } + .map_err(windows_error_to_io) + } + + fn windows_error_to_io(error: windows::core::Error) -> std::io::Error { + let hresult = error.code().0 as u32; + let raw_error = if hresult & 0xffff_0000 == 0x8007_0000 { + (hresult & 0xffff) as i32 + } else { + hresult as i32 + }; + std::io::Error::from_raw_os_error(raw_error) + } + fn clear_withdrawal_marker(path: &Path) -> Result<(), String> { let dir = path .parent() @@ -1821,27 +1938,33 @@ mod tests { } #[test] - fn consent_read_does_not_wait_for_the_store_lock() { + fn consent_read_waits_for_the_store_lock() { let tmp = tempfile::tempdir().unwrap(); let _guard = LocalAppDataGuard::set(tmp.path()); set_consent(true, "prompt").unwrap(); + let (started_tx, started_rx) = std::sync::mpsc::channel(); let (status_tx, status_rx) = std::sync::mpsc::channel(); std::thread::scope(|scope| { let read_handle = platform::with_store_lock(|| { - let status_tx = status_tx.clone(); let handle = scope.spawn(move || { + started_tx.send(()).expect("send reader start"); status_tx.send(get_status()).expect("send consent status"); }); - let status = status_rx - .recv_timeout(std::time::Duration::from_millis(100)) - .expect("consent read must not wait for the store lock"); - assert_eq!(status.effective_state, ConsentState::Granted); + started_rx.recv().expect("reader start"); + assert!( + status_rx + .recv_timeout(std::time::Duration::from_millis(100)) + .is_err(), + "consent read must wait while a writer owns the store lock" + ); Ok(handle) }) .expect("store lock"); read_handle.join().unwrap(); }); + let status = status_rx.recv().expect("consent status after lock release"); + assert_eq!(status.effective_state, ConsentState::Granted); } #[test] @@ -1898,6 +2021,65 @@ mod tests { assert_eq!(status.reason, Some(ConsentStatusReason::StoreUnreadable)); } + #[test] + fn swallowed_store_failures_are_reported() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let reports = std::sync::Mutex::new(Vec::new()); + + let status = platform::read_status_unlocked_with_reporter_for_test( + Err(std::io::Error::new( + std::io::ErrorKind::PermissionDenied, + "marker temporarily unreadable", + )), + |message| { + reports + .lock() + .unwrap_or_else(|error| error.into_inner()) + .push(message); + }, + ); + + assert_eq!(status.reason, Some(ConsentStatusReason::StoreUnreadable)); + assert_eq!( + reports + .lock() + .unwrap_or_else(|error| error.into_inner()) + .as_slice(), + ["inspect withdrawal marker: kind=PermissionDenied, os_error=None, error=marker temporarily unreadable"] + ); + } + + #[test] + fn malformed_store_is_reported() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("telemetry-consent.json"), "not json at all").unwrap(); + let reports = std::sync::Mutex::new(Vec::new()); + + let status = platform::read_status_unlocked_with_reporter_for_test( + Ok(platform::WithdrawalMarkerState::Absent), + |message| { + reports + .lock() + .unwrap_or_else(|error| error.into_inner()) + .push(message); + }, + ); + + assert_eq!(status.reason, Some(ConsentStatusReason::StoreMalformed)); + assert!( + reports + .lock() + .unwrap_or_else(|error| error.into_inner()) + .iter() + .any(|message| message.starts_with("parse consent record:")), + "malformed consent must produce a diagnostic" + ); + } + #[test] fn successful_grant_repairs_a_stale_withdrawal_marker() { let tmp = tempfile::tempdir().unwrap(); @@ -2083,6 +2265,42 @@ mod tests { ); } + #[test] + fn unsupported_prompt_locale_cannot_authorize_collection() { + let tmp = tempfile::tempdir().unwrap(); + let _guard = LocalAppDataGuard::set(tmp.path()); + let dir = tmp.path().join("mxc"); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write( + dir.join("telemetry-consent.json"), + r#"{"schemaVersion":2,"consent":"granted","promptResourceVersion":1,"promptLocale":"fr-FR"}"#, + ) + .unwrap(); + + let status = get_status(); + + assert_eq!(status.stored_state, ConsentState::Granted); + assert_eq!(status.effective_state, ConsentState::Undetermined); + assert_eq!( + status.reason, + Some(ConsentStatusReason::PromptVersionUnsupported) + ); + } + + #[test] + fn write_through_replace_handles_new_and_existing_records() { + let tmp = tempfile::tempdir().unwrap(); + let destination = tmp.path().join("telemetry-consent.json"); + + for contents in ["first", "replacement"] { + let source = tmp.path().join(format!("{contents}.tmp")); + std::fs::write(&source, contents).unwrap(); + platform::replace_file_write_through(&source, &destination).unwrap(); + assert_eq!(std::fs::read_to_string(&destination).unwrap(), contents); + assert!(!source.exists()); + } + } + #[test] fn concurrent_grant_revoke_never_corrupts_the_store() { // Regression test for the write-race finding: many threads diff --git a/src/core/wxc_common/src/telemetry/mod.rs b/src/core/wxc_common/src/telemetry/mod.rs index d777ed177..53db58fcf 100644 --- a/src/core/wxc_common/src/telemetry/mod.rs +++ b/src/core/wxc_common/src/telemetry/mod.rs @@ -31,6 +31,26 @@ pub use consent::ConsentState; pub use events::{log_error, log_execution, ExecutionEvent, FailureReason, TelemetryContext}; pub use policy::PolicyState; +#[cfg(target_os = "windows")] +#[derive(Default)] +struct FailureReporter { + reported: Mutex>, +} + +#[cfg(target_os = "windows")] +impl FailureReporter { + fn report(&self, signature: String, emit: impl FnOnce(&str)) { + let is_new = self + .reported + .lock() + .unwrap_or_else(|error| error.into_inner()) + .insert(signature.clone()); + if is_new { + emit(&signature); + } + } +} + #[cfg(any(test, all(feature = "test-support", debug_assertions)))] pub mod test_support { use super::consent::test_support::LocalAppDataGuard; diff --git a/src/core/wxc_common/src/telemetry/policy.rs b/src/core/wxc_common/src/telemetry/policy.rs index a54679e95..a2508fbf2 100644 --- a/src/core/wxc_common/src/telemetry/policy.rs +++ b/src/core/wxc_common/src/telemetry/policy.rs @@ -125,9 +125,9 @@ pub fn is_blocked_by_policy() -> bool { #[cfg(target_os = "windows")] mod platform { use super::{PolicyState, POLICY_VALUE_OPTIONAL}; - use std::collections::HashSet; + use crate::telemetry::FailureReporter; use std::io::Write; - use std::sync::{Mutex, OnceLock}; + use std::sync::OnceLock; use winreg::enums::HKEY_LOCAL_MACHINE; use winreg::RegKey; @@ -168,24 +168,6 @@ mod platform { Unreadable, } - #[derive(Default)] - struct FailureReporter { - reported: Mutex>, - } - - impl FailureReporter { - fn report(&self, signature: String, emit: impl FnOnce(&str)) { - let is_new = self - .reported - .lock() - .unwrap_or_else(|error| error.into_inner()) - .insert(signature.clone()); - if is_new { - emit(&signature); - } - } - } - fn report_policy_read_failure(operation: &str, error: &std::io::Error) { static FAILURES: OnceLock = OnceLock::new(); let signature = format!( @@ -374,7 +356,7 @@ mod platform { #[cfg(test)] mod failure_reporter { - use super::FailureReporter; + use crate::telemetry::FailureReporter; use std::sync::Mutex; #[test] From 939d1db8f9f05f4af50e460279a84e617e0d227e Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Tue, 25 Aug 2026 19:01:12 -0700 Subject: [PATCH 08/10] Enforce live telemetry authorization Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- .github/copilot-instructions.md | 9 ++- sdk/dotnet/README.md | 4 +- src/core/wxc_common/src/telemetry/consent.rs | 28 ++++---- src/core/wxc_common/src/telemetry/mod.rs | 76 +++++++++++++++++++- 4 files changed, 96 insertions(+), 21 deletions(-) diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md index 8680513c0..4f974f355 100644 --- a/.github/copilot-instructions.md +++ b/.github/copilot-instructions.md @@ -327,15 +327,14 @@ The parser deserializes JSON directly into the typed wire model (`wxc_common::wi ### Telemetry consent -Telemetry is **Windows-only** and is never collected without explicit user consent. Three independent conditions must all hold before anything is emitted: the user has granted consent, the administrative (MDM / Group Policy) ceiling permits it, and the config kill-switch has not disabled it. `wxc_common::telemetry::is_enabled()` is the single place those three terms are combined — do not re-derive enablement anywhere else. +Telemetry is **Windows-only** and is never collected without explicit user consent. Three independent conditions must all hold before anything is emitted: the user has granted consent, the administrative (MDM / Group Policy) ceiling permits it, and the config kill-switch has not disabled it. `wxc_common::telemetry::is_enabled()` combines those terms at provider initialization, and the shared emit authorization guard rechecks live consent and policy immediately before each logical emission. - **Everything fails closed.** Any error, unreadable value, corrupt file, missing native library, or ambiguity must resolve to "no telemetry" (`Undetermined` consent / `Blocked` policy), never to a permissive state. - **Policy may restrict, but may never substitute for, consent.** The administrative policy is a deny-only ceiling: it can subtract from what a user permitted, never add to it. An administrator cannot opt a user in — a denied or never-asked user stays opted out even under `AllowTelemetry=3`. Keep the terms combined with `&&`; never add a policy value or config path that grants collection on its own. - **MXC owns its own consent state.** It must never read or infer from the Windows system telemetry consent. The consent store is a per-user JSON file; the policy is `HKLM\SOFTWARE\Policies\Mxc` → `AllowTelemetry` (`REG_DWORD`). -- **One definition, distributed to the bindings.** The Rust `ConsentState` / `PolicyState` enums are the source of truth; the FFI, C#, and TypeScript layers marshal the same strings. Keep those spellings in sync anywhere this branch surfaces them. -- **Test isolation.** The consent store and the policy key are process-global, each behind its own mutex. Use `wxc_common::telemetry::test_support::TelemetryTestEnv` whenever a test needs both; constructing `PolicyKeyGuard` and `LocalAppDataGuard` directly in the same test risks a lock-order deadlock. The consent-store override is compiled into test binaries and into debug `test-support` builds only, so release binaries do not honor it. The `wxc_common` `test-support` feature re-exports the policy override for downstream crates' integration tests (`mxc_ffi` uses it) and must stay a dev-dependency-only feature. -- **Read-only queries must never be able to crash the host.** `NeedsConsentPrompt`/`needsTelemetryConsentPrompt` and `GetPolicy`/`getTelemetryPolicy` fail closed on *any* failure and never throw — including a non-`Success` FFI status, which covers a caught panic. The consent *read* and *write* still throw, because their callers must distinguish "not decided" from "could not read" and "did not persist"; when they do, they raise only the binding's documented exception type (`MxcException`), wrapping anything unexpected rather than letting a raw type escape. -- **Never swallow a failure silently.** Fail-closed return values are indistinguishable from legitimate ones, so a broken install would otherwise be invisible. Every swallowed failure is reported once per distinct failure per process (deduplicated — hosts poll these getters), and the reporter itself must never throw. At the FFI boundary, `catch_unwind` sites log the panic payload before returning `MXC_STATUS_PANIC`, which would otherwise be discarded. +- **One definition.** The Rust `ConsentState` / `PolicyState` enums and their stable string representations are the source of truth. Keep those spellings aligned anywhere this branch surfaces them. +- **Test isolation.** The consent store and the policy key are process-global, each behind its own mutex. Use `wxc_common::telemetry::test_support::TelemetryTestEnv` whenever a test needs both; constructing `PolicyKeyGuard` and `LocalAppDataGuard` directly in the same test risks a lock-order deadlock. The consent-store override is compiled into test binaries and into debug `test-support` builds only, so release binaries do not honor it. Keep the `test-support` feature confined to test/development dependencies. +- **Never swallow a failure silently.** Fail-closed return values are indistinguishable from legitimate ones, so a broken install would otherwise be invisible. Every swallowed failure is reported once per distinct failure per process, and the reporter itself must never throw. ### Package versioning diff --git a/sdk/dotnet/README.md b/sdk/dotnet/README.md index 4c9f95e23..ddff1eec1 100644 --- a/sdk/dotnet/README.md +++ b/sdk/dotnet/README.md @@ -176,8 +176,8 @@ An IT administrator can still block MXC telemetry device-wide via MXC's own registry policy setting. See [`docs/telemetry/telemetry-administrative-policy.md`](../../docs/telemetry/telemetry-administrative-policy.md) for the stable registry contract and interaction rules. Policy and consent -queries fail closed rather than upgrading an unreadable device state into -collection. +queries are not yet exposed by the .NET SDK; any eventual query must fail +closed rather than upgrading an unreadable device state into collection. ## Projects diff --git a/src/core/wxc_common/src/telemetry/consent.rs b/src/core/wxc_common/src/telemetry/consent.rs index f5ef9e50e..8fe9fbd6b 100644 --- a/src/core/wxc_common/src/telemetry/consent.rs +++ b/src/core/wxc_common/src/telemetry/consent.rs @@ -509,8 +509,9 @@ fn persist_presented_decision( /// Withdrawal does not require a permitting administrative policy. Off /// Windows it succeeds as a typed `NotApplicable` result without storage. pub fn withdraw_consent() -> Result { - let current_status = get_status(); - if current_status.effective_state == ConsentState::NotApplicable { + #[cfg(not(target_os = "windows"))] + { + let current_status = get_status(); return Ok(ConsentActionOutcome { result: ConsentActionResult::NotApplicable, status: current_status, @@ -518,17 +519,20 @@ pub fn withdraw_consent() -> Result { }); } - platform::with_store_lock(|| { - platform::begin_withdrawal()?; - platform::write(false, "withdrawal")?; - platform::finish_withdrawal()?; - Ok(ConsentActionOutcome { - result: ConsentActionResult::Withdrawn, - status: platform::read_status_unlocked(), - policy: super::policy::get_policy(), + #[cfg(target_os = "windows")] + { + platform::with_store_lock(|| { + platform::begin_withdrawal()?; + platform::write(false, "withdrawal")?; + platform::finish_withdrawal()?; + Ok(ConsentActionOutcome { + result: ConsentActionResult::Withdrawn, + status: platform::read_status_unlocked(), + policy: super::policy::get_policy(), + }) }) - }) - .map_err(ConsentActionError::Persist) + .map_err(ConsentActionError::Persist) + } } /// Current time as Unix seconds. Debug/support provenance only (see diff --git a/src/core/wxc_common/src/telemetry/mod.rs b/src/core/wxc_common/src/telemetry/mod.rs index 53db58fcf..545ccba93 100644 --- a/src/core/wxc_common/src/telemetry/mod.rs +++ b/src/core/wxc_common/src/telemetry/mod.rs @@ -140,6 +140,10 @@ thread_local! { /// telemetry test's forced-active state and trip the global panic hook into /// the sink. Never set outside tests. static TEST_FORCE_ACTIVE: std::cell::Cell = const { std::cell::Cell::new(false) }; + /// Test-only authorization override for emit-path tests that run on + /// non-Windows hosts or intentionally isolate event mapping from the + /// persisted consent and policy stores. + static TEST_FORCE_AUTHORIZED: std::cell::Cell> = const { std::cell::Cell::new(None) }; } /// Whether the emit glue should proceed. In production this is exactly @@ -153,6 +157,19 @@ fn emit_active() -> bool { mxc_telemetry::is_active() } +/// Re-check live authorization immediately before one logical emission. +/// +/// Provider registration records only that telemetry was authorized at +/// initialization. Consent withdrawal or a newly blocking administrative +/// policy must suppress the next emission from an already-running process. +fn emission_is_authorized() -> bool { + #[cfg(test)] + if let Some(authorized) = TEST_FORCE_AUTHORIZED.with(|value| value.get()) { + return authorized; + } + consent::get_consent().allows_collection() && policy::get_policy().allows_collection() +} + /// Claim the single terminal-emit slot for this process. Returns `true` if a /// terminal event has *already* been emitted (so the caller should skip), /// `false` for the first caller (which then owns emission). @@ -173,6 +190,7 @@ fn reset_for_test() { .lock() .unwrap_or_else(|e| e.into_inner()) = None; TEST_FORCE_ACTIVE.with(|f| f.set(false)); + TEST_FORCE_AUTHORIZED.with(|value| value.set(None)); events::test_sink::clear(); } @@ -267,6 +285,10 @@ pub fn emit_completion( if !active { return; } + if !emission_is_authorized() { + shutdown(); + return; + } if already_emitted() { return; } @@ -323,6 +345,10 @@ pub fn emit_early_exit(active: bool, containment: &ContainmentBackend, reason: F if !active { return; } + if !emission_is_authorized() { + shutdown(); + return; + } if already_emitted() { return; } @@ -537,7 +563,7 @@ fn emit_crash(ctx: TelemetryContext<'_>, exit_code: i32, reason: FailureReason) /// where the OS reclaims the ETW registration at process exit. It also carries /// **no** panic message text, which can contain paths or other PII. pub fn emit_panic() { - if !emit_active() || already_emitted() { + if !emit_active() || !emission_is_authorized() || already_emitted() { return; } let correlation_vector = process_correlation_vector(); @@ -567,7 +593,7 @@ pub fn emit_panic() { /// tears the process down via `ExitProcess`, and the main thread may still be /// live. It is allocation-light and emits no free-form text. pub fn emit_cancellation() { - if !emit_active() || already_emitted() { + if !emit_active() || !emission_is_authorized() || already_emitted() { return; } let correlation_vector = process_correlation_vector(); @@ -700,6 +726,10 @@ pub fn emit_state_aware( if !active { return; } + if !emission_is_authorized() { + shutdown(); + return; + } if already_emitted() { return; } @@ -816,6 +846,7 @@ mod tests { reset_for_test(); events::test_sink::install(); TEST_FORCE_ACTIVE.with(|f| f.set(true)); + TEST_FORCE_AUTHORIZED.with(|value| value.set(Some(true))); set_process_context(&ContainmentBackend::IsolationSession); set_process_phase("exec"); set_process_correlation_vector("iso:wxc-abcd"); @@ -854,6 +885,7 @@ mod tests { reset_for_test(); events::test_sink::install(); TEST_FORCE_ACTIVE.with(|f| f.set(true)); + TEST_FORCE_AUTHORIZED.with(|value| value.set(Some(true))); set_process_context(&ContainmentBackend::IsolationSession); set_process_phase("start"); set_process_correlation_vector("iso:wxc-abcd"); @@ -888,6 +920,7 @@ mod tests { reset_for_test(); events::test_sink::install(); TEST_FORCE_ACTIVE.with(|f| f.set(true)); + TEST_FORCE_AUTHORIZED.with(|value| value.set(Some(true))); set_process_context(&ContainmentBackend::IsolationSession); set_process_phase("exec"); @@ -1235,6 +1268,7 @@ mod tests { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); reset_for_test(); events::test_sink::install(); + TEST_FORCE_AUTHORIZED.with(|value| value.set(Some(true))); // Provision-style success envelope → one MXC.Execution, no MXC.Error. let envelope = Ok(DispatchOutcome::Envelope(serde_json::json!({}))); @@ -1260,6 +1294,7 @@ mod tests { // Fresh slot for the error case (the emit above claimed it once). reset_for_test(); events::test_sink::install(); + TEST_FORCE_AUTHORIZED.with(|value| value.set(Some(true))); let err = Err(MxcError::policy_validation("bad policy")); emit_state_aware( true, @@ -1298,6 +1333,7 @@ mod tests { let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); reset_for_test(); events::test_sink::install(); + TEST_FORCE_AUTHORIZED.with(|value| value.set(Some(true))); // Register the real ETW provider; on Windows this makes is_active() true. assert!( @@ -1321,4 +1357,40 @@ mod tests { mxc_telemetry::shutdown(); reset_for_test(); } + + #[cfg(windows)] + #[test] + fn live_withdrawal_and_policy_block_suppress_the_next_emission() { + let store = tempfile::tempdir().expect("temp dir"); + let env = test_support::TelemetryTestEnv::new(store.path()); + env.set_policy_value(3); + consent::set_consent(true, "test").expect("set consent"); + + let _lock = TEST_LOCK.lock().unwrap_or_else(|e| e.into_inner()); + reset_for_test(); + events::test_sink::install(); + TEST_FORCE_ACTIVE.with(|f| f.set(true)); + set_process_context(&ContainmentBackend::IsolationSession); + + consent::withdraw_consent().expect("withdraw consent"); + emit_panic(); + assert!( + events::test_sink::take_executions().is_empty(), + "withdrawal must suppress the next logical emission" + ); + + reset_for_test(); + events::test_sink::install(); + TEST_FORCE_ACTIVE.with(|f| f.set(true)); + consent::set_consent(true, "test").expect("restore consent"); + env.set_policy_value(0); + + emit_cancellation(); + assert!( + events::test_sink::take_executions().is_empty(), + "a newly blocking policy must suppress the next logical emission" + ); + + reset_for_test(); + } } From f024e4415648ff402c28a939e4e6e86c9d919029 Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Tue, 25 Aug 2026 19:40:08 -0700 Subject: [PATCH 09/10] Fix non-Windows consent lint Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- src/core/wxc_common/src/telemetry/consent.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/src/core/wxc_common/src/telemetry/consent.rs b/src/core/wxc_common/src/telemetry/consent.rs index 8fe9fbd6b..8978a6681 100644 --- a/src/core/wxc_common/src/telemetry/consent.rs +++ b/src/core/wxc_common/src/telemetry/consent.rs @@ -512,11 +512,11 @@ pub fn withdraw_consent() -> Result { #[cfg(not(target_os = "windows"))] { let current_status = get_status(); - return Ok(ConsentActionOutcome { + Ok(ConsentActionOutcome { result: ConsentActionResult::NotApplicable, status: current_status, policy: super::policy::get_policy(), - }); + }) } #[cfg(target_os = "windows")] @@ -1322,18 +1322,11 @@ mod platform { Ok(None) } + #[cfg(test)] pub(super) fn write(_granted: bool, _source: &str) -> Result<(), String> { Err("telemetry is Windows-only; consent is not applicable on this platform".to_string()) } - pub(super) fn begin_withdrawal() -> Result<(), String> { - Err("telemetry is Windows-only; consent is not applicable on this platform".to_string()) - } - - pub(super) fn finish_withdrawal() -> Result<(), String> { - Ok(()) - } - pub(super) fn write_presented( _granted: bool, _source: &str, From 55ce0a7ff5033976b0887143e168c62a42d7214f Mon Sep 17 00:00:00 2001 From: Ramon Arjona Date: Tue, 25 Aug 2026 20:01:02 -0700 Subject: [PATCH 10/10] Report invalid telemetry policy values Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7 --- .../telemetry-administrative-policy.md | 4 + src/core/wxc_common/src/telemetry/policy.rs | 74 +++++++++++++++---- 2 files changed, 64 insertions(+), 14 deletions(-) diff --git a/docs/telemetry/telemetry-administrative-policy.md b/docs/telemetry/telemetry-administrative-policy.md index 8d01b85c6..ba398403c 100644 --- a/docs/telemetry/telemetry-administrative-policy.md +++ b/docs/telemetry/telemetry-administrative-policy.md @@ -44,6 +44,10 @@ Any value MXC cannot read or cannot parse — a wrong type, a corrupt value, a registry error — is treated as `Blocked`. Telemetry collection is never the outcome of a failure. +MXC reports each distinct unreadable or unrecognized policy failure once per +process so operators can distinguish a broken deployment from an intentional +block without weakening the fail-closed result. + ## Deploying it ### Registry deployment diff --git a/src/core/wxc_common/src/telemetry/policy.rs b/src/core/wxc_common/src/telemetry/policy.rs index a2508fbf2..786f243d8 100644 --- a/src/core/wxc_common/src/telemetry/policy.rs +++ b/src/core/wxc_common/src/telemetry/policy.rs @@ -103,9 +103,9 @@ const POLICY_KEY_OVERRIDE_OWNER_ENV: &str = "MXC_TEST_POLICY_KEY_OVERRIDE_OWNER_ /// Fail-closed: a value that is present but not understood resolves to /// [`PolicyState::Blocked`]. An *absent* policy resolves to /// [`PolicyState::Unrestricted`] — the unmanaged default, where the user's own -/// consent decision governs. Registry failures are reported to stderr once per -/// distinct failure so the fail-closed result does not hide a broken policy -/// deployment from operators. +/// consent decision governs. Registry failures and unrecognized values are +/// reported to stderr once per distinct failure so the fail-closed result does +/// not hide a broken policy deployment from operators. pub fn get_policy() -> PolicyState { platform::read() } @@ -168,13 +168,8 @@ mod platform { Unreadable, } - fn report_policy_read_failure(operation: &str, error: &std::io::Error) { + fn report_policy_failure(signature: String) { static FAILURES: OnceLock = OnceLock::new(); - let signature = format!( - "{operation}: kind={:?}, os_error={:?}, error={error}", - error.kind(), - error.raw_os_error() - ); FAILURES .get_or_init(FailureReporter::default) .report(signature, |detail| { @@ -182,25 +177,51 @@ mod platform { let mut handle = stderr.lock(); let _ = writeln!( handle, - "mxc: failed to read telemetry administrative policy ({detail}); \ + "mxc: telemetry administrative policy failure ({detail}); \ treating policy as blocked" ); let _ = handle.flush(); }); } + fn report_policy_read_failure(operation: &str, error: &std::io::Error) { + report_policy_failure(format!( + "{operation}: kind={:?}, os_error={:?}, error={error}", + error.kind(), + error.raw_os_error() + )); + } + + fn report_unrecognized_policy_value(value: u32) { + report_policy_failure(format!( + "read AllowTelemetry value: unrecognized DWORD value {value}" + )); + } + pub(super) fn read() -> PolicyState { policy_state_from_value(read_policy_value()) } fn policy_state_from_value(value: PolicyValue) -> PolicyState { + policy_state_from_value_with_reporter(value, report_unrecognized_policy_value) + } + + fn policy_state_from_value_with_reporter( + value: PolicyValue, + report_unrecognized: impl FnOnce(u32), + ) -> PolicyState { match value { PolicyValue::Absent => PolicyState::Unrestricted, PolicyValue::Value(POLICY_VALUE_OPTIONAL) => PolicyState::Allowed, - // Every other value — including `0` (off) and `1` (required-only, - // a category MXC does not emit) — denies. Unrecognized values deny - // too rather than being guessed at: fail closed. - PolicyValue::Value(_) => PolicyState::Blocked, + // Values `0` (off) and `1` (required-only, a category MXC does not + // emit) are recognized blocking levels. + PolicyValue::Value(0) | PolicyValue::Value(1) => PolicyState::Blocked, + // Anything else is a broken deployment. Deny and report it rather + // than silently guessing at the administrator's intent. + PolicyValue::Value(value) => { + report_unrecognized(value); + PolicyState::Blocked + } // Fail closed. An administrator who typed the value in as a string, // or a machine whose registry we cannot read, must not be treated // as unmanaged. @@ -213,6 +234,15 @@ mod platform { policy_state_from_value(PolicyValue::Unreadable) } + #[cfg(test)] + pub(super) fn policy_value_report_for_test(value: u32) -> (PolicyState, Option) { + let mut reported = None; + let state = policy_state_from_value_with_reporter(PolicyValue::Value(value), |value| { + reported = Some(value); + }); + (state, reported) + } + /// Reads the policy value, distinguishing "not configured" from /// "configured but unreadable". /// @@ -636,4 +666,20 @@ mod windows_tests { assert_eq!(state, PolicyState::Blocked); assert!(!state.allows_collection()); } + + #[test] + fn only_unrecognized_dword_values_are_reported() { + for value in [0, 1, 3] { + let (_, reported) = + crate::telemetry::policy::platform::policy_value_report_for_test(value); + assert_eq!(reported, None, "recognized value {value} was reported"); + } + + for value in [2, 4, 99, u32::MAX] { + let (state, reported) = + crate::telemetry::policy::platform::policy_value_report_for_test(value); + assert_eq!(state, PolicyState::Blocked); + assert_eq!(reported, Some(value)); + } + } }