Document stable telemetry privacy contract - #818
Conversation
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
|
Azure Pipelines: There may be pipelines that require an authorized user to comment /azp run to run. |
There was a problem hiding this comment.
Pull request overview
Documents the proposed stable telemetry privacy, consent, and administrative-policy contract ahead of implementation.
Changes:
- Defines consent wording, persistence, presenter, and SDK contracts.
- Specifies deny-only administrative policy behavior.
- Documents telemetry event identities and privacy-review inventory.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
docs/telemetry/telemetry.md |
Adds stable consent and policy overview. |
docs/telemetry/telemetry-policy.md |
Defines Windows administrative policy. |
docs/telemetry/telemetry-consent-privacy-review.md |
Records disclosure, inventory, and controls. |
docs/telemetry/telemetry-consent-design.md |
Specifies consent behavior and public surfaces. |
Suppressed comments (2)
docs/telemetry/telemetry-consent-privacy-review.md:27
- The “random” guarantee is not upheld by the documented relay contract: for non-provision phases,
correlationVectoris caller-supplied andis_relayablechecks only its shape (src/core/wxc/src/main.rs:289-295andcorrelation_vector.rs:404-412). A caller can encode 128 bits of arbitrary data in a canonical base and have that base emitted, potentially including identity data that this inventory says is excluded. Require provenance/authentication for relayed vectors, or describe and review this identifier as caller-controlled rather than random.
- Random app-session and sandbox-lifecycle correlation identifiers
docs/telemetry/telemetry-consent-privacy-review.md:65
- These unresolved decisions conflict with the PR description, which calls the data inventory, exclusions, and localization boundary an approved normative contract. The document cannot serve as the stable implementation contract while those same items still await confirmation. Either record the approved decisions here or mark the relevant contract and PR claims as pending.
1. Confirm the listed included/excluded data categories.
2. Confirm service-side retention, access, regional, and deletion requirements.
3. Confirm whether additional locale or accessibility requirements apply at
initial stable release.
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
Suppressed comments (2)
docs/telemetry/telemetry.md:327
- This leaves the “final data inventory” explicitly pending even though this PR is described as defining the approved, exact normative inventory that follow-on implementation PRs should implement. Since material inventory changes require a new consent-resource version, implementing against an unconfirmed inventory can invalidate the supposedly approved wording. Finalize the inventory/privacy review here, or clearly narrow the PR and consent wording to a non-approved draft contract before dependent implementation proceeds.
The following items remain pending and must be confirmed before release:
- The final data inventory, including the privacy implications of
caller-controlled lifecycle correlation identifiers
docs/telemetry/telemetry-administrative-policy.md:112
- These Intune instructions cannot be followed with the material provided: importing a custom administrative template requires both the ADMX and its matching en-US ADML, but this document supplies only the ADMX and a list of strings/presentation elements the reader would have to author. Include a complete
Mxc.admlexample (or point to a shipped pair) and instruct administrators to upload both files.
Import `Mxc.admx` (above) via **Devices → Configuration → Import ADMX**, then
create a Configuration profile from the imported template and set
**AllowTelemetry**.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
Suppressed comments (2)
docs/telemetry/telemetry.md:217
- This data inventory ends mid-sentence, so readers never learn what the relayed identifier contains and the section never states the promised customer-content exclusions. Complete the bullet and explicitly enumerate the remaining metadata and exclusions.
- A lifecycle correlation identifier; for relayed state-aware phases
docs/telemetry/telemetry.md:166
- This edit introduces a lowercase sentence start and leaves the duplicated article in “a ‘a panic occurred’ sentinel.” Rephrase the sentence so the limitation reads grammatically.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
Suppressed comments (3)
docs/telemetry/telemetry-administrative-policy.md:101
- This is not enough to deploy the advertised policy: Group Policy and Intune require a valid matching ADML resource file, and Intune requires the ADMX and en-US ADML to be uploaded together. A prose list of resource IDs cannot be imported. Please provide the complete
Mxc.adml(preferably as a checked-in ADMX/ADML pair) and update the deployment steps to instruct administrators to use both files.
The matching `Mxc.adml` needs `MXC`, `AllowTelemetry`, `AllowTelemetry_Help`,
`Off`, `Required` and `Optional` strings plus a `dropdownList` presentation for
`AllowTelemetry_Enum`.
docs/telemetry/telemetry.md:166
- Capitalize the new sentence and remove the duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a
docs/telemetry/telemetry.md:217
- The normative data inventory ends mid-sentence at EOF, so it never states how caller-relayed lifecycle identifiers are constrained. Complete the bullet with the validation/reseeding behavior; otherwise the document does not preserve the stated guarantee that arbitrary customer content cannot be uploaded.
- A lifecycle correlation identifier; for relayed state-aware phases
179c96d to
769d0f0
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated 4 comments.
Suppressed comments (8)
docs/schema.md:129
- The example still places
telemetryinsideexperimental, despite this PR defining it as a stable top-level setting. Copying this example would produce the old wire shape and still require--experimental. Close the experimental object before the telemetry block.
"telemetry": { // Telemetry (Windows only)
"enabled": true // Emit TraceLogging ETW events via pure Rust tracelogging crate
}
README.md:239
- This reads as currently supported, but this PR does not add a top-level
telemetryfield; the parser still accepts onlyexperimental.telemetrybehind--experimental. Mark this as a proposed contract or defer the user-facing enablement instructions to the runtime integration PR so users are not given a rejected configuration.
Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured
sdk/node/README.md:435
- The Node SDK currently exposes telemetry only under
ContainerConfig.experimental.telemetry(sdk/node/src/types.ts:326-332), not as top-leveltelemetry.enabled. This user-facing text therefore documents an API/config shape that this PR does not provide. Label it as planned or move it to the SDK implementation PR.
## Telemetry Consent
Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.
Telemetry consent behavior follows
[`docs/telemetry/telemetry-consent-design.md`](https://github.com/microsoft/mxc/blob/main/docs/telemetry/telemetry-consent-design.md):
sdk/dotnet/README.md:99
SandboxPolicy.TelemetryEnabledand a phase-levelTelemetryEnabledproperty do not exist anywhere in the .NET SDK in this PR, so this instruction cannot compile for current consumers. Describe these names as planned, or publish this section with the .NET implementation PR.
MXC telemetry is Windows-only and remains off until both of these are true:
1. the user has explicitly granted MXC-owned consent, and
2. the caller opts this invocation in via telemetry settings.
Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.
docs/telemetry/telemetry-administrative-policy.md:85
- This formula omits the explicit per-run request and provider-availability gates defined in the consent contract, and
build_has_telemetry_enabledis not the runtime switch. As written, it suggests consent plus policy can collect withouttelemetry.enabled: true. Use the same four gates as the normative contract.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled
docs/schema.md:139
- Use singular agreement because only
sandboxIdremains in this sentence.
> top-level fields are state-aware-only — a one-shot request carrying
> `sandboxId` is rejected with a parse error. Lifecycle correlation is internal
docs/telemetry/telemetry.md:135
- Capitalize the new sentence and remove the duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a
docs/host-prep.md:139
- Use
abefore the adverbcentrally.
from an elevated context (e.g. a scheduled task, an centrally deployed
769d0f0 to
6164f27
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 18 out of 19 changed files in this pull request and generated no new comments.
Suppressed comments (12)
tests/scripts/run_telemetry_etw_smoke_test.ps1:127
- The consent seed does not match the canonical resource version defined in this PR (
1). Because the contract treats an unknown prompt version as non-consent, a record containing1.0will fail closed once consent gating is implemented and this smoke test will capture no events.
promptResourceVersion = '1.0'
sdk/node/src/state-aware.ts:76
- The Rust executor still returns
result.correlationVectorfrom provision and consumes the relayed value on later phases (src/core/wxc/src/main.rs:260-320;src/core/wxc_common/src/state_aware_request.rs:88-92). Dropping it here causes every later phase to reseed independently, breaking lifecycle correlation while the runtime still advertises and emits it. Keep the relay until the Rust wire/runtime contract is removed atomically.
return {
sandboxId: result.sandboxId as SandboxId<C>,
metadata: result.metadata,
schemas/dev/mxc-config.schema.0.8.0-dev.json:958
- The schema is generated from
wxc_common::wire, whereMxcConfig.correlation_vectorstill exists (src/core/wxc_common/src/wire.rs:65-75).scripts/versioning/check-schema-codegen.js:45-73performs an exact regeneration check, so this manual removal is guaranteed to fail CI. Remove the field from the Rust source and regenerate the schema in the same change.
},
README.md:244
- The new explicit-consent rule is contradicted by the retained Data Collection paragraph at line 250, which says use of the software itself operates as consent. That conflicts with this PR's normative rule that consent must be an explicit current-version grant and must never be inferred. Reconcile the legal paragraph with the new consent contract before treating this as canonical user guidance.
The configuration flag is an additional per-run opt-in; it cannot grant consent
or bypass an administrative block. Telemetry remains off unless every applicable
gate is open. MXC does not use the Windows Diagnostics & feedback setting as a
substitute for application consent.
docs/telemetry/telemetry-administrative-policy.md:85
- This formula omits the required per-run request and provider-availability gates, and replaces them with a build-time condition not present in the four-gate consent contract. That can be read as allowing collection whenever policy and consent permit it, even when the run did not opt in.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled
sdk/node/src/state-aware-types.ts:382
- This removes a public SDK result property as part of a PR described as normative documentation only, with implementation assigned to later PRs in the stack. The same diff also changes request behavior and tests, so merging PR 1 changes the SDK before the stated implementation PRs. Keep this PR contract-only or update the stack/scope and include the corresponding Rust runtime change here.
export interface ProvisionResult<C extends StateAwareContainmentBackend> {
sandboxId: SandboxId<C>;
metadata?: ProvisionMetadataFor<C>;
README.md:239
- These are presented as instructions for the currently shipped CLI, but this PR does not implement them: the Rust wire model/parser still accepts only
experimental.telemetryand requires--experimental(src/core/wxc_common/src/wire.rs:460-493,config_parser.rs:1429-1436). Since the PR description says stable runtime integration follows in #820, keep current usage instructions here until that implementation lands, or clearly label this as a future contract.
This issue also appears on line 241 of the same file.
Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured
sdk/node/README.md:435
- This documents a top-level Node.js telemetry option that does not exist in this PR:
ContainerConfigstill exposes telemetry only underexperimental(sdk/node/src/types.ts:326-332). Readers cannot follow this guidance until the later SDK/runtime changes land, so defer it or identify it explicitly as a future API.
## Telemetry Consent
Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.
Telemetry consent behavior follows
[`docs/telemetry/telemetry-consent-design.md`](https://github.com/microsoft/mxc/blob/main/docs/telemetry/telemetry-consent-design.md):
sdk/dotnet/README.md:99
SandboxPolicy.TelemetryEnabledis not present anywhere in the current C# SDK, so this instructs users to use a nonexistent API. The PR stack says the .NET telemetry surface is added only in #822; defer this usage guidance to that PR or clearly mark it as proposed rather than available.
MXC telemetry is Windows-only and remains off until both of these are true:
1. the user has explicitly granted MXC-owned consent, and
2. the caller opts this invocation in via telemetry settings.
Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.
docs/schema.md:130
- The example still nests
telemetryinsideexperimental, despite relabeling it stable and the new contract requiring a top-level setting. As written, the sample contradicts lines 138-139 and the other new documentation. Move it to the root when the stable parser lands, or keep the experimental label/syntax until then.
"telemetry": { // Telemetry (Windows only)
"enabled": true // Emit TraceLogging ETW events via pure Rust tracelogging crate
}
}
docs/telemetry/telemetry.md:135
- Correct the sentence capitalization and duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a
docs/host-prep.md:139
- Use the correct article before “centrally.”
from an elevated context (e.g. a scheduled task, an centrally deployed
6164f27 to
5d547a3
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (8)
tests/scripts/run_telemetry_etw_smoke_test.ps1:128
- This seed does not match the consent-store schema used by the dependent consent implementation: it requires
schemaVersion, an integerpromptResourceVersionof1, and usespromptedMxcVersion/updatedAtEpoch. As written, deserialization fails at the missingschemaVersion, so fail-closed gating rejects the grant and the smoke test cannot deterministically emit telemetry.
$consentRecord = @{
version = 2
consent = 'granted'
promptResourceVersion = '1.0'
promptLocale = 'en-US'
tests/scripts/run_telemetry_etw_smoke_test.ps1:232
- At this stack level the ETW provider does not emit
mxc.sandbox_kind, so no captured event can satisfy this new required-field set and the smoke test always fails. The emitter change is planned in the dependent runtime PR; defer this assertion to that PR or include the corresponding provider change here.
$expectedFields = @('mxc.backend', 'mxc.sandbox_kind', 'mxc.exit_code', 'mxc.outcome', 'mxc.duration_ms')
docs/telemetry/telemetry-administrative-policy.md:85
- This equation substitutes a build-time flag for the required per-run opt-in and omits provider availability, contradicting the four-gate contract in
telemetry-consent-design.md. Use the same gate names so administrators do not infer that consent plus an enabled build is sufficient to collect.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled
docs/schema.md:129
- The example still places
telemetryinsideexperimental, even though this PR defines it as a stable top-level setting. A user copying this example would send the legacy shape and contradict the README contract. Close the experimental object before declaring telemetry.
"telemetry": { // Telemetry (Windows only)
"enabled": true // Emit TraceLogging ETW events via pure Rust tracelogging crate
}
tests/scripts/run_telemetry_etw_smoke_test.ps1:132
- On Windows PowerShell 5.1,
Set-Content -Encoding utf8writes a UTF-8 BOM. The consent reader passes the file directly toserde_json::from_str, which rejects that BOM, so the seeded grant fails closed on the Windows shell this script is expected to support. Write BOM-less UTF-8 explicitly.
$consentRecord | ConvertTo-Json -Depth 4 | Set-Content -Path $consentFile -Encoding utf8
tests/scripts/run_telemetry_etw_smoke_test.ps1:120
- The executable search also accepts release builds, but the dependent consent implementation compiles
MXC_TEST_LOCALAPPDATA_OVERRIDEout of release binaries and resolves LocalAppData through the Windows known-folder API rather than this environment variable. If only a release binary exists, this test reads the real user's consent and is no longer isolated or deterministic. Skip release binaries (or seed consent through a supported public test surface).
$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'
docs/telemetry/telemetry.md:184
- The data inventory omits
__TlgCV__, although the current provider and the dependent runtime implementation still write that field on both public events. Since this PR is the normative privacy inventory, either disclose the random lifecycle correlation vector here and in the event tables, or remove it from the implementation before stabilization.
- State-aware lifecycle phase
- `UTCReplace_AppSessionGuid`, which asks the telemetry pipeline to supply a
random per-session app identifier
docs/telemetry/telemetry.md:135
- Capitalize the new sentence and remove the duplicated article.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a
5d547a3 to
a8647b1
Compare
a8647b1 to
814b542
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (15)
tests/scripts/run_telemetry_etw_smoke_test.ps1:232
- The current ETW writer emits
mxc.backend,mxc.exit_code,mxc.outcome,mxc.duration_ms,mxc.failure_reason,mxc.phase, and__TlgCV__, but notmxc.sandbox_kind(src/mxc_telemetry/src/lib.rs:181-194). This new mandatory check therefore makes the smoke test fail for every event produced by this PR; add the assertion only with the event-field implementation.
$expectedFields = @('mxc.backend', 'mxc.sandbox_kind', 'mxc.exit_code', 'mxc.outcome', 'mxc.duration_ms')
docs/telemetry/telemetry-administrative-policy.md:86
- This formula omits two gates from the normative four-gate contract: the per-run request and provider availability. As written, the following table also says an unrestricted/consented user always collects even when the run did not opt in or the provider is unavailable.
MXC collects diagnostic data only when **every** gate is open:
collect = policy_permits AND user_consented AND build_has_telemetry_enabled
tests/scripts/run_telemetry_etw_smoke_test.ps1:127
- This seed uses prompt resource version
1.0, while the canonical consent resource indocs/telemetry/telemetry-consent-design.md:28defines version1. Because unsupported prompt versions must not authorize collection, these two normative artifacts must use exactly the same representation or the seeded grant will fail closed.
promptResourceVersion = '1.0'
tests/scripts/run_telemetry_etw_smoke_test.ps1:132
- Windows PowerShell 5.1 writes a UTF-8 BOM for
Set-Content -Encoding utf8. A strict JSON reader such asserde_jsonrejects that leading BOM, so this consent seed can be treated as malformed and fail closed on the Windows environment this test targets. Write UTF-8 without a BOM.
$consentRecord | ConvertTo-Json -Depth 4 | Set-Content -Path $consentFile -Encoding utf8
docs/schema.md:129
- Despite being labeled stable, this object is still structurally inside
experimental, so copying the documented example producesexperimental.telemetryrather than the top-leveltelemetryrequired everywhere else in this PR. Move it outside the experimental object.
"telemetry": { // Telemetry (Windows only)
"enabled": true // Emit TraceLogging ETW events via pure Rust tracelogging crate
}
tests/scripts/run_telemetry_etw_smoke_test.ps1:123
- Seeding user consent does not make capture deterministic when
HKLM\SOFTWARE\Policies\Mxc\AllowTelemetrycan still block collection. On a managed host the test will report a product failure even though suppressing events is the required behavior. Detect a blocking/unreadable policy in preflight and skip with an explicit reason, or use a supported test-only policy isolation mechanism.
# Seed an explicit granted consent record for this test run so event capture
# does not depend on host-global consent state.
docs/telemetry/telemetry-consent-design.md:203
- This release gate says the wording and data/control inventory still require privacy/legal approval, while the PR summary calls the contract approved and
telemetry.md:193says the wording is approved for release review. Clarify the actual approval state consistently; otherwise reviewers and later implementation PRs cannot tell whether this normative contract is finalized.
The versioned wording and data/control inventory still require Microsoft
privacy/legal approval before release.
README.md:239
- The new explicit-consent requirement conflicts with the unchanged Data Collection statement at line 250 that “Your use of the software operates as your consent.” That language makes ordinary use a grant, while this contract requires a persisted explicit
Yes; the privacy text needs to be reconciled before this can be normative.
Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured
sdk/node/README.md:251
- This example now discards
correlationVector, but the current SDK still returns it fromprovisionSandboxand sends onlyoptions.correlationVectoron later phases (sdk/node/src/state-aware.ts:70-78,96). Following the updated example silently removes lifecycle correlation until the later implementation PR actually internalizes it.
const { sandboxId } = await provisionSandbox(
'isolation_session',
{ network: { defaultPolicy: 'allow', allowLocalNetwork: true } },
{ experimental: true },
);
docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:398
- The documented result no longer includes
correlationVector, but the current TypeScript API still declares and returns that field (sdk/node/src/state-aware-types.ts:380-392,state-aware.ts:70-79). Removing it from the contract before the implementation changes makes this API reference inaccurate.
interface ProvisionResult<C extends StateAwareContainmentBackend> {
sandboxId: SandboxId<C>;
metadata?: ProvisionMetadataFor<C>;
}
docs/schema.md:140
- The current parser does not reject this field as unknown:
wire::Configstill declarescorrelation_vectorand the state-aware parser consumes it (src/core/wxc_common/src/wire.rs:65-75). This statement should remain aligned with the current schema until the implementation PR removes the field; otherwise valid current lifecycle requests are documented as invalid.
> is rejected with a parse error. Callers cannot supply `correlationVector`;
> 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
sdk/dotnet/README.md:99
SandboxPolicy.TelemetryEnabledand phase-levelTelemetryEnableddo not exist in the C# SDK (SandboxPolicy.cscurrently exposes only version, filesystem, network, UI, and timeout). Because this PR explicitly precedes implementation, the public SDK README should describe this as a planned contract or defer the concrete API documentation until the implementing PR lands.
Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.
docs/telemetry/telemetry-consent-design.md:106
- This calls the persisted document schema version 2 but does not define the schema-version key or the exact keys/types for MXC version, source, and timestamp. The smoke test already has to guess these details. Since multiple executables and SDKs must share this stable consent store, provide a complete normative JSON example with required/optional fields and exact version representation.
The persisted consent record is a versioned JSON document. The current schema
version is 2 and records:
- `consent`: `granted` or `denied`
- `promptResourceVersion`
- `promptLocale`
- MXC version, source, and update timestamp for local audit/support provenance
docs/telemetry/telemetry.md:135
- Capitalize the sentence after the period.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a
sdk/node/README.md:432
- This public README documents top-level telemetry as currently available, but the current Node config type only exposes
experimental.telemetry(sdk/node/src/types.ts:326-332) and the runtime still parses that experimental location. Rephrase this as the planned stable contract or defer it to the implementation PR so users are not directed to a config shape that is currently ignored/rejected.
Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Copilot-Session: 2da373b9-0a51-4aa4-a968-14f05f27b5c7
814b542 to
51a2ca4
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 9 out of 9 changed files in this pull request and generated no new comments.
Suppressed comments (14)
tests/scripts/run_telemetry_etw_smoke_test.ps1:232
- This assertion cannot pass on this PR:
mxc_telemetry::log_executioncurrently emitsmxc.backend, exit code, outcome, duration, failure reason, phase, and__TlgCV__, but notmxc.sandbox_kind. Defer this field assertion to the runtime-integration PR (or add the emitter change here); otherwise the checked-in smoke test is broken immediately.
$expectedFields = @('mxc.backend', 'mxc.sandbox_kind', 'mxc.exit_code', 'mxc.outcome', 'mxc.duration_ms')
tests/scripts/run_telemetry_etw_smoke_test.ps1:130
- The seeded record does not match the consent-store contract: the dependent consent reader expects
schemaVersion(notversion), a numericpromptResourceVersion, andupdatedAtEpoch. As written, the missing schema version makes this record malformed, so fail-closed gating treats it as undetermined and this smoke test captures no events.
$consentRecord = @{
version = 2
consent = 'granted'
promptResourceVersion = '1.0'
promptLocale = 'en-US'
source = 'run_telemetry_etw_smoke_test.ps1'
updatedAtUtc = [DateTime]::UtcNow.ToString('o')
README.md:238
- This new explicit-consent requirement conflicts with the unchanged Data Collection notice below, which says “Your use of the software operates as your consent to these practices.” Under the normative contract, product use cannot replace an explicit current-version Yes decision. Reconcile or qualify that legacy notice so users are not given two incompatible consent rules.
2. Explicit per-user telemetry consent on Windows
docs/schema.md:140
- This says
correlationVectoris rejected, but the parser's current top-level wire model accepts it for state-aware requests and the Node SDK still emits it. That removal belongs with the later runtime/SDK implementation; until then this reference must describe the contract users actually have, or clearly identify the paragraph as a future shape.
> is rejected with a parse error. Callers cannot supply `correlationVector`;
> 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/schema.md:129
- The stable telemetry block is still nested inside
experimental, contradicting the newly documented top-leveltelemetry.enabledcontract. Move it outside the experimental object so the schema example shows the intended wire shape.
This issue also appears on line 138 of the same file.
"telemetry": { // Telemetry (Windows only)
"enabled": true // Emit TraceLogging ETW events via pure Rust tracelogging crate
}
README.md:244
- The PR description says stable runtime integration follows in #820, but this user-facing README presents top-level telemetry, consent, and policy gating as behavior available now. On this branch the parser only accepts
experimental.telemetrybehind--experimental, and no consent/policy gate exists. Keep current usage documented here or clearly mark the entire section as a future contract until the implementation lands.
This issue also appears on line 238 of the same file.
Telemetry requires:
1. Top-level `"telemetry": { "enabled": true }` in the JSON config
2. Explicit per-user telemetry consent on Windows
3. An administrative policy that permits collection, when a policy is configured
The configuration flag is an additional per-run opt-in; it cannot grant consent
or bypass an administrative block. Telemetry remains off unless every applicable
gate is open. MXC does not use the Windows Diagnostics & feedback setting as a
substitute for application consent.
sdk/dotnet/README.md:164
SandboxPolicy.TelemetryEnabledand the phase-levelTelemetryEnabledsetting do not exist in the .NET SDK on this branch, so this reads as instructions for an API that consumers cannot compile against. Since the PR explicitly precedes SDK implementation, describe this as the planned surface (or defer it to #822).
Telemetry remains off by default unless the caller opts in with `SandboxPolicy.TelemetryEnabled = true` (or the equivalent phase-level `TelemetryEnabled` setting for state-aware requests) and applicable Windows consent/policy gates permit collection.
sdk/node/README.md:439
- The Node SDK currently exposes telemetry only under
experimental.telemetryand has no consent presenter/persistence API, but this section describes the planned stable top-level and presenter behavior as current. Mark this section as planned until #822 lands, or move it to that implementation PR, so the published README remains usable for the package version in this branch.
Telemetry is off-by-default unless the caller opts in with top-level `telemetry.enabled: true` and the applicable Windows consent/policy gates permit collection.
Telemetry consent behavior follows
[`docs/telemetry/telemetry-consent-design.md`](https://github.com/microsoft/mxc/blob/main/docs/telemetry/telemetry-consent-design.md):
the SDK stays UI-agnostic, renders the canonical resource verbatim through a
host presenter, persists only explicit yes/no decisions, treats dismissal and
failures as non-grants, and never lets policy or transport failures opt a user
in.
docs/telemetry/telemetry-consent-design.md:16
- “No prompt” also describes the
AlreadyGrantedpath below, where MXC intentionally does not invoke a presenter but an existing current-version grant still authorizes telemetry. Say “never requesting consent” (or equivalent) to distinguish absence of any grant from prompt suppression after a valid grant.
No prompt, no response, the literal string "No", dismissal, withdrawal,
malformed state, an unknown prompt version, or any read/write failure means no
docs/telemetry/telemetry.md:135
- Capitalize the sentence after the period.
> different code (`-1`). the `101` here is a "a panic occurred" sentinel, not a
docs/state-aware-lifecycle/mxc-state-aware-sandbox-api.md:398
- The current
ProvisionResultstill includescorrelationVector, and the Node SDK still relays it throughSandboxSpawnOptions. Removing it from the API reference before the implementation PR removes that contract makes this document inaccurate for the code/package on this branch. Retain the field for now or explicitly label this interface as the proposed post-integration shape.
interface ProvisionResult<C extends StateAwareContainmentBackend> {
sandboxId: SandboxId<C>;
metadata?: ProvisionMetadataFor<C>;
}
docs/telemetry/telemetry-administrative-policy.md:85
- This formula omits two gates required by the consent contract: the per-run request and provider availability. Include the full four-way conjunction; otherwise this policy document says collection occurs when the normative consent design says it must remain off.
collect = policy_permits AND user_consented AND build_has_telemetry_enabled
docs/telemetry/telemetry.md:85
- This field is described as part of the currently emitted event, but no emitter change is included here and
mxc_telemetry::log_execution/log_errordo not writemxc.sandbox_kind. The PR description calls this a proposed event contract, so label this field as proposed until runtime integration lands rather than documenting data that current builds do not send.
| `mxc.sandbox_kind` | string | Containment kind requested by the caller (`process`, `vm`, or a concrete backend name) |
sdk/node/README.md:252
- This example now drops the
correlationVectorreturned by the currentprovisionSandboximplementation, so users enabling the existing experimental telemetry lose cross-phase correlation. Keep relaying it until the later implementation actually removes the field; merely changing the README does not change the current SDK/wire contract.
const { sandboxId } = await provisionSandbox(
'isolation_session',
{ network: { defaultPolicy: 'allow', allowLocalNetwork: true } },
{ experimental: true },
);
const opts = { experimental: true };
.github/copilot-instructions.md.Summary
Defines the approved normative privacy and telemetry contract before implementation: exact consent wording, data inventory and exclusions, fail-closed consent behavior, administrative policy ceiling, localization boundary, and proposed event contract.
This is PR 1 of 5. Review this PR first; implementation follows in the dependent PRs.
Stack
Review only this PR's diff. Later PRs are intentionally based on the preceding branch.
Microsoft Reviewers: Open in CodeFlow